Skip to content

Adding an opa plugin#427

Merged
huang195 merged 2 commits into
rossoctl:mainfrom
davidhadas:opa-plugin
Jun 4, 2026
Merged

Adding an opa plugin#427
huang195 merged 2 commits into
rossoctl:mainfrom
davidhadas:opa-plugin

Conversation

@davidhadas

@davidhadas davidhadas commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This is an OPA Plugin that is working against an OPA bundle service using the standard OPA SDK

Related issue(s)

rossoctl/rossoctl#1789

#fixes #544

opa

OPA (Open Policy Agent) plugin for AuthBridge. Downloads policy bundles from a
Kagenti Bundle Server based on the agent's identity and evaluates requests and
responses against the loaded policy using four fixed decision paths.

How it works

  1. At startup the plugin reads the agent's client ID from /shared/client-id.txt
    (mounted by the kagenti-operator from a Keycloak-credentials Secret).
  2. It creates an embedded OPA engine via the OPA Go SDK and configures it to
    fetch bundles/<agent-id>.tar.gz from the bundle server.
  3. The SDK downloads the bundle, activates the policy, and begins periodic
    polling for updates (respecting ETag / If-None-Match for lightweight
    304 responses).
  4. On every request and response the plugin queries the appropriate decision
    path based on traffic direction and phase. If the policy denies, the plugin
    returns HTTP 403. If the path is undefined (rule not present in the bundle),
    the plugin skips evaluation — treating the absence of a rule as "no opinion".

The plugin reports not-ready (Ready() == false) until the first bundle is
successfully loaded. The Kubernetes readiness probe holds traffic off the pod
until then, so requests never arrive before the policy is active.

Decision paths

The plugin uses four fixed OPA decision paths, one for each evaluation point:

Path Phase Purpose
authbridge/inbound/request Inbound request Primary authorization — validate caller identity, enforce access control
authbridge/inbound/response Inbound response Fine-grained response evaluation (rare, expensive) — inspect response body/headers
authbridge/outbound/request Outbound request Control outgoing requests to external systems — ensure delegated tokens are used in line with task intent
authbridge/outbound/response Outbound response Protect the agent from data attacks in responses (rare)

Each path is independent. A bundle only needs to include the rules it cares
about — undefined paths are skipped (treated as allow). Most deployments will
only define authbridge/inbound/request.

Configuration

Default configuration

Most deployments only need bundle_url. The lean default input is sufficient
for tool-access-control and model-restriction policies:

pipeline:
  inbound:
    plugins:
      - name: jwt-validation
        config: { ... }
      - name: opa
        config:
          bundle_url: "http://bundle-server.kagenti.svc:8080"
  outbound:
    plugins:
      - name: opa
        config:
          bundle_url: "http://bundle-server.kagenti.svc:8080"
      - name: token-exchange
        config: { ... }

With this config, policies can decide based on caller identity, tool names,
model names, hosts, and methods — without any bulk content crossing the OPA
evaluation boundary.

Example: content-filtering policy

If a policy needs to inspect the actual user prompt (e.g., block tasks
containing sensitive keywords), extend the input with a2a.content:

- name: opa
  config:
    bundle_url: "http://bundle-server.kagenti.svc:8080"
    include:
      - "a2a.content"

This adds a2a.parts[].content, a2a.artifact, and a2a.error_message to
the OPA input so the policy can match against user-submitted text.

Example: full MCP arguments + LLM conversation audit

A deployment that needs to write policies against tool argument values and
the full conversation history:

- name: opa
  config:
    bundle_url: "http://bundle-server.kagenti.svc:8080"
    include:
      - "mcp.params"            # full tool arguments (not just name/uri)
      - "mcp.result"            # tool response data (response path)
      - "inference.messages"    # full conversation history
      - "inference.tools.detail" # tool descriptions and JSON schemas

Config fields

Field Required Default Description
bundle_url yes Base URL of the Kagenti Bundle Server
agent_id_file no /shared/client-id.txt Path to the file containing the agent's client ID
agent_id no Inline agent ID; when set, agent_id_file is ignored
polling_min_delay no 10 Minimum bundle polling interval in seconds
polling_max_delay no 120 Maximum bundle polling interval in seconds
include no [] List of optional field groups to expose in the OPA input (see below)

The include mechanism

The OPA input document is lean by default — it contains only structural
metadata needed for authorization decisions (method, path, host, identity, tool
names, model names). Bulk content fields (conversation history, full tool
arguments, prompt text) are excluded unless explicitly requested via the
include config list.

Each entry in include is a named group key that unlocks additional fields:

Key What it unlocks Default Size concern
mcp.params.name params.name in MCP input ON Tiny
mcp.params.uri params.uri in MCP input ON Tiny
mcp.params.<key> Any specific params key (e.g., mcp.params.cursor) OFF Varies
mcp.params Full params map (all keys) OFF Can be large
mcp.result result map (response path) OFF Can be very large
mcp.error error object (code + message + data) OFF Usually small
a2a.content parts[].content, artifact, error_message OFF KBs (prompts/responses)
inference.messages Full messages[] array OFF Tens of KBs
inference.completion completion text OFF Can be large
inference.tools.detail Tool description + parameters OFF Moderate
inference.tool_calls tool_calls[] with arguments OFF Can be large

Default-on keys (mcp.params.name, mcp.params.uri) are always included even
with an empty include list.

OPA input document

Default (lean) input

{
  "direction": "inbound",
  "method": "POST",
  "path": "/api/v1/invoke",
  "host": "my-agent",
  "headers": {
    "authorization": "Bearer eyJ...",
    "content-type": "application/json"
  },
  "identity": {
    "subject": "user-123",
    "client_id": "caller-agent",
    "scopes": ["openid", "profile"]
  },
  "agent": {
    "client_id": "my-agent"
  },
  "a2a": {
    "method": "invoke",
    "session_id": "sess-123",
    "task_id": "task-789",
    "role": "user"
  },
  "mcp": {
    "method": "tools/call",
    "params": { "name": "create_issue", "uri": "file:///workspace/main.go" }
  },
  "inference": {
    "model": "gpt-4",
    "stream": false,
    "max_tokens": 4000,
    "tools": ["create_issue", "list_issues"]
  }
}

Notes:

  • mcp.params contains only the default-on keys (name, uri) that are
    present in the original params. Add more via include.
  • inference.tools is a string array of tool names only (not full objects).
    Use inference.tools.detail in include for descriptions and parameters.

With include: ["a2a.content", "mcp.params", "inference.messages", "inference.tools.detail"]

{
  "a2a": {
    "method": "invoke",
    "session_id": "sess-123",
    "task_id": "task-789",
    "role": "user",
    "parts": [
      { "kind": "text", "content": "Create a GitHub issue for bug XYZ" }
    ],
    "artifact": "Issue #42 created",
    "error_message": ""
  },
  "mcp": {
    "method": "tools/call",
    "params": { "name": "create_issue", "arguments": {"title": "Bug XYZ", "body": "..."} }
  },
  "inference": {
    "model": "gpt-4",
    "stream": false,
    "max_tokens": 4000,
    "messages": [
      {"role": "user", "content": "Help me create an issue"}
    ],
    "tools": [
      {"name": "create_issue", "description": "Creates a GitHub issue", "parameters": {"type": "object"}}
    ]
  }
}

On the response path the document also includes:

{
  "response": {
    "status_code": 200,
    "headers": {
      "content-type": "application/json"
    }
  }
}

Input field reference

Field Present Description
direction always "inbound" or "outbound"
method always HTTP method (GET, POST, ...)
path always Request URL path
host always HTTP Host header value
headers always Flattened request headers (lowercase keys, multi-values joined with ,)
identity when jwt-validation ran Subject, client ID, and scopes from the validated JWT
agent when agent identity is set The agent's own client ID
a2a when a2a-parser ran A2A protocol metadata (method, session_id, task_id, role)
mcp when mcp-parser ran MCP method + filtered params
inference when inference-parser ran Model, stream, max_tokens, tool names
response response path only Status code and response headers

Policy contract

Each decision path evaluates to an allow rule. The plugin supports two
return shapes:

Boolean -- the simplest form:

package authbridge.inbound.request

default allow := false

allow if {
    input.identity.subject != ""
}

Object with reason -- for detailed deny messages:

package authbridge.inbound.request

default allow := false

allow if {
    input.identity.subject != ""
}

reason := "anonymous access not permitted" if {
    not allow
}

Example policies

Tool access control (MCP) — inbound request

package authbridge.inbound.request

default allow := false

# Define allowed tools per client
allowed_tools := {
    "github-agent": ["create_issue", "list_issues", "get_issue"],
    "admin-agent": ["create_issue", "list_issues", "get_issue", "delete_issue"],
}

# Allow tool calls only if the tool is in the allowed list
allow if {
    input.mcp.method == "tools/call"
    input.identity.client_id
    tool_name := input.mcp.params.name
    allowed_tools[input.identity.client_id][_] == tool_name
}

# Allow tool listing for all authenticated users
allow if {
    input.mcp.method == "tools/list"
    input.identity.subject != ""
}

LLM model restriction (Inference) — outbound request

package authbridge.outbound.request

default allow := {"allow": false, "reason": "default deny"}

approved_models := ["gpt-4", "gpt-3.5-turbo", "claude-3-sonnet"]

allow := {"allow": true} if {
    input.inference.model
    approved_models[_] == input.inference.model
    not excessive_token_request
}

excessive_token_request if {
    input.inference.max_tokens
    input.inference.max_tokens > 4000
}

allow := {"allow": false, "reason": "token limit exceeds policy"} if {
    excessive_token_request
}

Dangerous tool combination block (Inference) — outbound request

package authbridge.outbound.request

default allow := false

allow if {
    input.inference.model
    not has_dangerous_combo
}

has_dangerous_combo if {
    tools := input.inference.tools
    has_value(tools, "write_file")
    has_value(tools, "execute_command")
}

has_value(arr, val) if {
    arr[_] == val
}

Task content filtering (A2A) — requires include: ["a2a.content"]

package authbridge.inbound.request

default allow := {"allow": false, "reason": "default deny"}

allow := {"allow": true} if {
    input.identity.subject != ""
    not contains_sensitive_keywords
}

contains_sensitive_keywords if {
    input.a2a.parts[_].content
    task := lower(input.a2a.parts[_].content)
    sensitive := ["delete database", "drop table", "rm -rf", "sudo"]
    some keyword in sensitive
    contains(task, keyword)
}

allow := {"allow": false, "reason": "task contains sensitive keyword"} if {
    contains_sensitive_keywords
}

Multi-path bundle example

A single bundle can contain rules for multiple decision paths:

bundles/my-agent.tar.gz
  authbridge/
    inbound/
      request.rego      # package authbridge.inbound.request
    outbound/
      request.rego      # package authbridge.outbound.request

The plugin interprets the decision as follows:

Result Action
true Allow
false Deny with "policy denied"
{"allow": true} Allow
{"allow": false} Deny with "policy denied"
{"allow": false, "reason": "..."} Deny with the provided reason
Anything else Deny (safe default)

Bundle layout

The bundle server must serve a standard OPA bundle at the path
bundles/<agent-id>.tar.gz. A minimal bundle contains a single .rego file
for the inbound request path:

bundles/my-agent.tar.gz
  authbridge/
    inbound/
      request.rego

A full bundle covering all four decision paths:

bundles/my-agent.tar.gz
  authbridge/
    inbound/
      request.rego       # package authbridge.inbound.request
      response.rego      # package authbridge.inbound.response (optional)
    outbound/
      request.rego       # package authbridge.outbound.request (optional)
      response.rego      # package authbridge.outbound.response (optional)

Only include the paths you need — the plugin skips evaluation for any
undefined path.

See the OPA bundle documentation
for the full specification.

Pipeline ordering

The plugin declares After: ["jwt-validation", "a2a-parser", "mcp-parser", "inference-parser"]
(soft ordering). When these plugins are present in the same pipeline, OPA runs
after them so input.identity, input.a2a, input.mcp, and input.inference
are populated. If any are absent, OPA still runs — the corresponding input
fields will be missing and the policy must handle that case.

Deny behavior

  • Request path: OPA not initialized or decision error -> 503. Policy deny -> 403. Path undefined -> skip.
  • Response path: OPA not initialized -> skip. Policy deny -> 403. Path undefined -> skip.
  • Before bundle loads: the readiness probe holds traffic off the pod. If a
    request arrives anyway (e.g. in tests), the plugin denies with 503.

Session events

The plugin records Invocation entries for every decision:

Action Reason When
allow policy_allowed / response_policy_allowed OPA returned allow
deny policy_denied / response_policy_denied OPA returned deny
deny decision_error / response_decision_error OPA evaluation failed
skip opa_not_ready Response path, OPA not yet initialized
skip no_policy_rule Decision path undefined in bundle (no rule for this phase)

These appear in the session events API (:9094) and in abctl.

Summary by CodeRabbit

  • New Features

    • Added an OPA-based authorization plugin to evaluate inbound/outbound policies and included it in compiled binaries.
  • Documentation

    • Added comprehensive OPA plugin docs covering configuration, decision paths, input shape, examples, bundle layout, and behavior notes.
  • Tests

    • Expanded tests for configuration, input shaping, decision interpretation, header handling, lifecycle/readiness, and agent-ID edge cases.
  • Chores

    • Expanded dependency lists and build-time plugin imports across components to support the OPA integration.

@davidhadas

Copy link
Copy Markdown
Contributor Author

As for security scoring - OPA relies on

Low score ≠ vulnerability.
It usually means “small project with minimal maintenance processes.”

We can engage with the OPA community about them if we choose.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an OPA-backed authorization plugin to AuthBridge: configuration, SDK lifecycle and bundle activation, four decision paths for inbound/outbound request/response, include-set-driven input shaping for extensions, comprehensive tests and docs, and compile-time registration in envoy/lite/proxy with go.mod updates.

Changes

OPA Authorization Plugin

Layer / File(s) Summary
OPA Plugin Documentation and Registry
authbridge/authlib/plugins/opa/README.md, authbridge/authlib/plugins/README.md, authbridge/authlib/plugins/plugins_test.go
Adds full OPA plugin documentation and registers opa in plugin registry/tests.
Configuration, Types, and Registration
authbridge/authlib/plugins/opa/plugin.go
Defines opaConfig, include-set parsing/defaults, plugin type OPA, registration under "opa", decision-path constants, and compile-time interface assertions.
Init / SDK Startup and Readiness
authbridge/authlib/plugins/opa/plugin.go
Init starts SDK immediately if agent_id present or waits on agent_id_file; startOPA builds agent-scoped SDK config (URL-escaped bundle path), instantiates SDK decider, signals readiness, and provides Shutdown/Ready.
Decision Routing: OnRequest & OnResponse
authbridge/authlib/plugins/opa/plugin.go
Selects inbound/outbound × request/response path; OnRequest denies if decider missing, OnResponse skips if missing; both call SDK, treat undefined as skip, handle errors, and record allow/deny outcomes.
OPA Input Building & Helpers
authbridge/authlib/plugins/opa/plugin.go
Constructs OPA input from pipeline context and extensions with include-set gating (A2A/MCP/Inference), flattens headers, and interprets decision results (boolean or {allow,reason}).
Unit Tests
authbridge/authlib/plugins/opa/plugin_test.go
Extensive tests: configuration validation and defaults, include-set logic, interpretDecision, flattenHeaders, buildInput (basic + A2A/MCP/Inference gating), OnRequest/OnResponse runtime behaviors, mock/capturing deciders, and buildOPAConfig path-escaping assertions.
go.mod Updates & Binary Integration
authbridge/authlib/go.mod, authbridge/cmd/*/go.mod, authbridge/cmd/*/main.go
Adds github.com/open-policy-agent/opa v1.4.2 to authlib; expands indirect deps across multiple go.mod files; blank-imports OPA plugin into envoy, lite, and proxy mains for compile-time registration.

Sequence Diagram(s)

sequenceDiagram
  participant Pipeline as AuthBridgePipeline
  participant OPAPlugin as OPA plugin
  participant OPASDK as OPASDK
  participant BundleServer as BundleServer
  Pipeline->>OPAPlugin: Configure / Init
  OPAPlugin->>OPASDK: startOPA(bundle: bundles/<agentID>.tar.gz)
  OPASDK->>BundleServer: GET bundle_url/bundles/<agentID>.tar.gz
  BundleServer-->>OPASDK: bundle tar.gz
  OPASDK-->>OPAPlugin: Ready
  Pipeline->>OPAPlugin: OnRequest/OnResponse(input)
  OPAPlugin->>OPASDK: Evaluate(path, input)
  OPASDK-->>OPAPlugin: decision (allow|deny|undefined|error)
  OPAPlugin-->>Pipeline: continue|reject (record outcome)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

  • feature: AuthBridge OPA Plugin #426 — This PR implements and registers an AuthBridge OPA plugin (code, docs, tests, and build imports) consistent with the feature requested in #426.

Poem

🐰 I nibble bundles in the moonlit code,
Four paths I check where tiny policies strode,
I shape the input, flatten headers neat,
Then hop to verdicts: allow, deny, or skip a beat. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.40% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Adding an opa plugin' clearly and concisely describes the main change in the changeset: the introduction of a new OPA (Open Policy Agent) plugin for AuthBridge.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@authbridge/authlib/go.mod`:
- Around line 34-40: Remove the duplicate require entry for
github.com/goccy/go-json v0.10.3 in the go.mod (the repeated
"github.com/goccy/go-json v0.10.3 // indirect" lines); keep a single entry and
then run go mod tidy to reconcile module metadata. Locate the duplicate in the
authlib go.mod, delete the redundant line, and verify with go mod tidy that no
other duplicates remain.

In `@authbridge/authlib/plugins/opa/plugin.go`:
- Around line 247-250: The OnRequest branch that returns pipeline.DenyStatus
when p.decider == nil must also emit a session event so not-ready denials are
observable; locate the OPA.OnRequest method, and before returning
pipeline.DenyStatus(503, "upstream.unreachable", "opa policy engine not
initialized") add a call to the plugin's session-event emitter (use the existing
observability helper used elsewhere in this plugin, e.g., p.emitSessionEvent /
p.EmitSessionEvent or the module's session event helper) with action "deny" and
the same reason/metadata so the denial is recorded in session observability.
- Around line 198-208: The code in buildOPAConfig constructs the bundle resource
using p.agentID directly ("bundles/%s.tar.gz"), which allows path traversal or
invalid characters; update buildOPAConfig to validate or sanitize p.agentID
before use (e.g., enforce a strict whitelist regexp like /^[A-Za-z0-9._-]+$/ or
similar) and reject or error on invalid values, or alternatively
percent-encode/URL-escape the agent id when interpolating into the resource
string; ensure the check/escape is applied to p.agentID before creating the
fmt.Sprintf("bundles/%s.tar.gz", p.agentID) value so the OPA bundle resource
cannot be manipulated by malicious input.
- Around line 93-100: There is a data race because OPA fields p.decider and
p.agentID are written in background goroutines (Init/startOPA) and read in
OnRequest/OnResponse; fix by protecting these fields with a sync.Mutex on the
OPA struct or replacing them with an atomic.Value and always read/write via the
chosen concurrency primitive (update Init, startOPA, OnRequest, OnResponse to
use it consistently). In buildOPAConfig validate/sanitize the agentID before
using it to build the bundle path (reject any agentID containing '/', '\\', ".."
or path separators or return an error) so fmt.Sprintf("bundles/%s.tar.gz",
p.agentID) cannot escape directories. Finally ensure the “not-ready” path in
OnRequest emits observability like OnResponse does (call
pctx.Skip("opa_not_ready") or pctx.Record(...) before returning
pipeline.DenyStatus(...)) so session events are recorded when p.decider == nil.

In `@authbridge/authlib/plugins/opa/README.md`:
- Around line 388-395: The README.md contains multiple fenced code blocks
lacking language identifiers (causing MD040); update each backtick fence in the
shown sections (around the example file trees at lines referenced) to include a
language tag such as text (e.g., change ``` to ```text) for every tree/list
block (the three blocks at 388-395 and the other blocks around 414-419 and
423-432) so markdownlint and docs CI pass; ensure you only add the language
identifier to the opening fences and leave the inner content unchanged.

In `@authbridge/authlib/plugins/README.md`:
- Line 30: The table row for the `opa` plugin is in the wrong section; remove
the `| `opa` | OPA policy evaluation on inbound and outbound requests via bundle
download |` row from the "Reusable building blocks for plugin authors" table and
add the same row into the "Built-in plugins" table (preserving table formatting
and column order), and then run the repository markdown spellcheck action to
ensure no MD lint errors remain; locate these edits by searching for the `opa`
plugin entry and the table headers "Reusable building blocks for plugin authors"
and "Built-in plugins" in README.md.

In `@authbridge/cmd/authbridge-lite/go.mod`:
- Around line 65-69: The go.mod contains duplicate require entries for the same
golang.org/x/* modules; open go.mod and remove the duplicated require lines so
each module (golang.org/x/crypto, golang.org/x/net, golang.org/x/sync,
golang.org/x/sys, golang.org/x/text) appears only once with the intended
version, then run `go mod tidy` to verify and update the file; ensure you edit
the duplicate entries in the go.mod require block (look for the repeated module
names) rather than changing other files.

In `@authbridge/cmd/authbridge-lite/main.go`:
- Around line 41-44: The file comment about "Auth gates only: drop the parsers
and token-broker." is out of sync with the imports: jwtvalidation
(github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation),
opa (github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/opa) and
tokenexchange
(github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange)
are still being compiled; either update the comment to accurately list the
included plugins (jwtvalidation, opa, tokenexchange) or remove the tokenexchange
import if token-broker/tokenexchange should indeed be dropped—locate the import
block in main.go and make the corresponding change so the comment and the
imported plugin set match.

In `@authbridge/cmd/authbridge-proxy/go.mod`:
- Around line 65-69: The go.mod contains duplicate require entries for the
golang.org/x modules (golang.org/x/crypto, golang.org/x/net, golang.org/x/sync,
golang.org/x/sys, golang.org/x/text); remove the redundant duplicates so each of
those module requirements appears only once in the require block, keeping the
desired version (e.g., v0.48.0, v0.49.0, v0.19.0, v0.41.0, v0.34.0) and running
`go mod tidy` afterwards to ensure module graph consistency (look for the
require lines referencing golang.org/x/crypto, golang.org/x/net,
golang.org/x/sync, golang.org/x/sys, golang.org/x/text).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d76c28e-2767-411d-b54a-f7d213fa3645

📥 Commits

Reviewing files that changed from the base of the PR and between 0a8dfbb and c998e9d.

⛔ Files ignored due to path filters (4)
  • authbridge/authlib/go.sum is excluded by !**/*.sum
  • authbridge/cmd/authbridge-envoy/go.sum is excluded by !**/*.sum
  • authbridge/cmd/authbridge-lite/go.sum is excluded by !**/*.sum
  • authbridge/cmd/authbridge-proxy/go.sum is excluded by !**/*.sum
📒 Files selected for processing (12)
  • authbridge/authlib/go.mod
  • authbridge/authlib/plugins/README.md
  • authbridge/authlib/plugins/opa/README.md
  • authbridge/authlib/plugins/opa/plugin.go
  • authbridge/authlib/plugins/opa/plugin_test.go
  • authbridge/authlib/plugins/plugins_test.go
  • authbridge/cmd/authbridge-envoy/go.mod
  • authbridge/cmd/authbridge-envoy/main.go
  • authbridge/cmd/authbridge-lite/go.mod
  • authbridge/cmd/authbridge-lite/main.go
  • authbridge/cmd/authbridge-proxy/go.mod
  • authbridge/cmd/authbridge-proxy/main.go

Comment thread authbridge/authlib/go.mod Outdated
Comment thread authbridge/authlib/plugins/opa/plugin.go
Comment thread authbridge/authlib/plugins/opa/plugin.go Outdated
Comment thread authbridge/authlib/plugins/opa/plugin.go
Comment thread authbridge/authlib/plugins/opa/README.md
Comment thread authbridge/authlib/plugins/README.md Outdated
Comment thread authbridge/cmd/authbridge-lite/go.mod
Comment thread authbridge/cmd/authbridge-lite/main.go
Comment thread authbridge/cmd/authbridge-proxy/go.mod
@huang195

huang195 commented Jun 2, 2026

Copy link
Copy Markdown
Member

@davidhadas can you do a conflict resolution before I start reviewing this PR? I think some of the changes in this PR has already been merged in earlier PRs

- Add new OPA plugin that integrates Open Policy Agent for authorization decisions
- Plugin supports synchronous policy evaluation with configurable timeout
- Includes comprehensive test coverage
- Update plugin registry and documentation
- Fix observability for agent ID escaping and synchronization

Signed-off-by: David Hadas <david.hadas@gmail.com>
@davidhadas

Copy link
Copy Markdown
Contributor Author

@huang195, I believe it is now resolved

@davidhadas davidhadas moved this from New /:ToDo to In review / Needs Testing in Rossoctl Issue Prioritization Jun 4, 2026
@davidhadas
davidhadas force-pushed the opa-plugin branch 4 times, most recently from 7794039 to 6768456 Compare June 4, 2026 19:51
@davidhadas

Copy link
Copy Markdown
Contributor Author

@huang195
Vulnerabilities removed - using later versions of containerd

as for OpenSSF Scorecard
It is informational only:
Low scores for transitive OPA dependencies - no vulnerabilities, no action needed
It can be considered a false positive

@huang195 huang195 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

Clean, well-structured plugin — registration is identical across all three data-plane binaries, OPA v1.4.2 is pinned consistently with no version skew, Capabilities() correctly omits ReadsBody (it consumes parser extensions, not the raw body), url.PathEscape guards the agent-id path segment, and Shutdown properly stops the SDK and the bundle poller. I verified the headline: genuine OPA evaluation errors fail closed (403) — exactly right for an authz gate.

The blocking items are all about fail-open behavior and credential hygiene on what is a security-critical authorization plugin:

  • #1/#2 — a startup window and a no-bundle-loaded case where requests are allowed with no policy applied.
  • #3 — the raw Authorization bearer token is placed into the OPA policy input unnecessarily.

Details inline. The rest are suggestions/nits.

Areas reviewed: plugin logic + fail-mode semantics + concurrency + input construction, registration across the 3 binaries, go.mod/go.sum deps, READMEs. Commits: 1 feat (DCO ✓). CI: all green.

Minor, non-inline: abctl/go.sum carries the full OPA transitive hash set although abctl doesn't register the plugin (a go mod tidy workspace artifact — harmless); and p.agentID is written in the Init goroutine and read in startOPA/buildOPAConfig as a plain field (benign in practice, but an unsynchronized cross-goroutine access).

}

func (p *OPA) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action {
dec := p.decider.Load()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must-fix — startup fail-open window. OnRequest gates only on p.decider.Load() != nil; it never consults p.ready. Per the OPA SDK docs, sdk.New with a Ready channel set returns immediately (non-blocking) and signals readiness asynchronously by closing the channel — so startOPA stores the decider (line 195) before any bundle is active, and p.ready flips only later. A request arriving in that gap queries a decision path with no activated policy → the SDK returns an undefined-decision error → IsUndefinedErr (line 279) → Skip("no_policy_rule")Continue (allow). So a request in the pre-bundle window is allowed with zero policy applied.

The README says the readiness probe holds traffic off the pod, but a security gate shouldn't fail open in its own logic if the probe is missing, racy, or the caller is in-process. Fix: gate OnRequest on p.ready.Load() (deny 503 until ready), or pass Block: true to sdk.New so the decider is only stored once the first bundle activates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Ready() stays false until the bundle activates — p.ready.Store(true) only fires after <-readyCh (line 198). The pipeline's Ready() (line 263 of pipeline.go) iterates all plugins implementing Readier; OPA's Ready() returns false → pipeline reports not-ready → /readyz
    returns 503.
  2. OnRequest is never called before the bundle loads — because Kubernetes won't route traffic to the pod until /readyz passes. The listeners start immediately (line 261-262 of main.go), but the readiness probe on :9091/readyz gates Kubernetes Service endpoints. No endpoint → no
    traffic → OnRequest never fires.

This means we seem to not have a race condition as described.
The REAME seems wrong
Still, as protective coding, let add !p.ready.Load() to the OnRequest gate

Input: input,
})
if err != nil {
if sdk.IsUndefinedErr(err) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must-fix — "no bundle active" is indistinguishable from "no rule defined." This IsUndefinedErr → allow branch fires both when (a) a bundle loaded but lacks a rule for this path (the intended "no opinion → skip" design) and (b) no bundle ever loaded (bundle server unreachable at startup). Combined with #1, a bundle-server outage at startup degrades to allow-all for any request that reaches the plugin, rather than fail-closed. Consider a distinct "engine up but no policy active" state that denies, so a broken policy-distribution path can't be mistaken for an operator's deliberate omission of a rule.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, we are not suppose to see requests is Ready() is not up. - i.e. if bundle not loaded.
seems the previous fix suffice

"method": pctx.Method,
"path": pctx.Path,
"host": pctx.Host,
"headers": flattenHeaders(pctx.Headers),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must-fix — raw Authorization bearer token placed into the policy input. buildInput sets headers: flattenHeaders(pctx.Headers) with no redaction, so the bearer token (and any cookie) end up in the OPA input document. Identity is already exposed structurally via input.identity (subject / client_id / scopes), so the raw token is unnecessary — and it becomes a leak surface for operator-authored Rego and for OPA decision logs if those are enabled. Recommend stripping authorization, cookie, and proxy-authorization from the flattened headers by default (or gating full headers behind an include key, like the body fields already are).

(For accuracy: the policy is evaluated in-process, so this isn't sent over the wire to the bundle server — the exposure is the Rego + decision-log surface, not the network.)

func (p *OPA) OnResponse(ctx context.Context, pctx *pipeline.Context) pipeline.Action {
dec := p.decider.Load()
if dec == nil {
pctx.Skip("opa_not_ready")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — response-path asymmetry. With a nil decider, OnResponse does Skip("opa_not_ready") → allow, whereas OnRequest denies 503. Defensible since response evaluation is the "rare, expensive" fine-graining path — but it means an outbound/response data-exfiltration policy is silently unenforced during the startup window. Worth confirming this is intended if response-side policies are meant to be load-bearing for egress protection.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both sides should be treated equally

return nil
}

func (p *OPA) buildOPAConfig() ([]byte, string, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — unauthenticated/un-pinned bundle fetch. buildOPAConfig emits only services.kagenti.url; there's no tls block (custom CA) or credentials block, and no bundle-signature verification. TLS relies on OPA SDK defaults (standard cert validation — good, not InsecureSkipVerify), but there's no way to pin a private CA for an internal bundle server or to authenticate to it. For a component that fetches authorization policy, a rogue or MITM bundle server could ship an allow-all bundle. At minimum document this trust assumption; ideally add a tls / bundle_signing config passthrough.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an early version of the OPA focusing on function. TLS is intentionally left out for this first commit. IT is suitable for clusters in which HTTP is considered sufficient and for evaluating the new OPA functionality. Later PR will handle TLS, Certificates etc. It is planed that for spiffe, we will use mTLS with spiffe certs. for service accounts we will use TLS with tokens. But again, this is not in this PR.

| Path | Phase | Purpose |
|------|-------|---------|
| `authbridge/inbound/request` | Inbound request | Primary authorization — validate caller identity, enforce access control |
| `authbridge/inbound/response` | Inbound response | Fine-grained response evaluation (rare, expensive) — inspect response body/headers |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — doc drift. This row says the inbound/response path can "inspect response body/headers," but OnResponse only populates status_code and headers in the input and never reads pctx.ResponseBody. The input-field reference and JSON example further down are accurate; just drop "body" here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buildInput function doesn't directly include any raw HTTP body. Instead, it includes parsed protocol content from the extensions (A2A parts/content, MCP params/result, inference messages) — which are populated by upstream parser plugins that did
read the body.

So on both request and response paths, buildInput already has access to the parsed protocol extensions. On the response path specifically:

  • pctx.Extensions.MCP with mcp.result (gated by include)
  • pctx.Extensions.A2A with content (gated by include)
  • pctx.Extensions.Inference with completion (gated by include)

The response path calls buildInput(pctx, p.inc) at line 321 — so it already gets all the parsed protocol data from upstream parsers, gated by the same include mechanism. Then it adds response.status_code and response.headers on top.

The code already provides response body content through the parsed extensions — the same include keys (mcp.result, a2a.content, inference.completion) work on both request and response paths. The raw HTTP body isn't exposed (and shouldn't be — it would be unparsed bytes), but the
meaningful structured content is available through the parser extensions.

Readme updated

@huang195 huang195 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — all three blocking items from my previous review are resolved:

  • Startup fail-open (#1): OnRequest now gates on !p.ready.Load() and denies 503 until the first bundle activates. The fail-open window is closed.
  • No-bundle vs no-rule (#2): covered by the same ready-gate — requests can't reach the IsUndefinedErr→skip branch before a policy is active.
  • Token in policy input (#3): flattenHeaders now redacts authorization, proxy-authorization, cookie, and set-cookie.

The response path (OnResponse) is now symmetric with the request path (denies 503 when not ready), and the README decision-path table no longer claims response-body inspection. Thanks for the detailed walk-through of the readiness-probe flow.

Accepting the TLS/bundle-auth deferral (#5) as out of scope for this first functional PR, per your note that mTLS/SPIFFE lands later — worth a one-line "no transport security yet" caveat in the README's trust section when convenient, but non-blocking.

Areas reviewed: plugin logic + fail-mode semantics, header redaction, registration across the 3 binaries, READMEs. CI: all green. Approving.

Assisted-By: Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

feature: OPA Plugin

3 participants