Feat: AIAC OPA plugin integration + live enforcement (phase 2, PR 1)

[oblinder]

Summary

First PR in a series toward #646 (AIAC phase 2 — platform integration: live onboarding trigger + live enforcement). This PR delivers OPA plugin integration within the AIAC system plus live enforcement: AIAC now publishes the rules it generates to the enforcement layer's policy resource, and real requests are allowed/denied by the deployed AuthBridge OPA plugin.

Later PRs in the series will cover the remaining #646 scope (e.g. the live IdP-event onboarding trigger path).

What's in this PR

PDP OPA policy writer → live enforcement

• Rewrote the OPA policy writer from a filesystem stub into an always-on writer of per-agent AuthorizationPolicy Custom Resources (agent.rossoctl.dev/v1alpha1) on the live Kubernetes API via server-side apply; the bundle-service composes these CRs into per-pod OPA bundles the plugin loads.
• Reworked the Rego generator: fixed authbridge.client.{inbound,outbound}.request packages matching the live AuthBridge OPA plugin input shape (input.identity.*, input.mcp.params.name), with de-prefixed outbound scope values.
• Removed the superseded Keycloak composite-role writer and fixed every dangling reference across the PRD, specs, and the k8s deployment guide.
• Rego dump to REGO_OUTPUT_DIR is now purely additive local-debug output, gated by POLICY_WRITER_DUMP_REGO (default off); it never gates the CR write. Error mapping: malformed agent_id → 400, k8s API failure → 502, delete of an absent CR → 204.

AuthBridge OPA plugin + token exchange

• Plugin changes to support outbound Rego rules (in addition to inbound).
• Token-exchange plugin/delegation changes supporting scoped token exchange for the agent's onward call to the tool.

Kubernetes wiring

• Added the aiac-pdp-policy-writer ServiceAccount + cluster-scoped RBAC (get,list,create,update,patch,delete on authorizationpolicies, no watch) and wired the SA into the aiac-interface pod.
• Turned the prod Rego dump off (dropped REGO_OUTPUT_DIR + /rego mount), kept read-only rootfs + /tmp; added PLATFORM_SOURCE_CLIENTS.

Integration tests — real OPA loop

• Rebuilt the OPA integration tests to close the real enforcement loop through the AuthBridge OPA plugin on a live Kind cluster: onboard → poll bundle-service → real HTTP request through AuthBridge → assert allow/deny from the deployed plugin over bare tool names. Retired the opa eval / probe model (opa no longer needed on PATH). Suite skips cleanly when the cluster is not wired.
• Added scripts/opa-kind-{enable,disable,driver,restore}.sh to stand up / tear down the OPA pipeline on Kind.

Docs

• Rewrote the pdp-policy-writer-opa spec to match the CR-writer implementation.

Supporting refactors carried in this branch

• Renamed Policy Store → Policy Model Store across specs, code, and tests.
• Renamed kagenti → rossoctl (realm, operator contract, brand, repo paths).

Testing

• .venv/bin/pytest test/ -m "not integration" — green.
• Integration tests (-m integration) close the real OPA loop against a live rossoctl/Kind cluster with the AuthBridge OPA pipeline wired in (see docs/opa-kind-runbook.md); they skip cleanly when the cluster/env is absent.

Related

Part of #646. Sub-issue of the AIAC epic #643; depends on phase 1 (#645).

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[clawgenti]

This PR wires AIAC into the live OPA enforcement loop: OPA policy-writer→CR, outbound delegation identity surface in input, and the Kind integration test suite. All CI checks pass (CodeQL, Bandit, Hadolint, Go/Python/Shell lint, DCO — 38/38 commits signed). No .claude/.vscode changes.

• Suggestion: mint_token() in scripts/opa-kind-driver.sh uses password=${user} (password == username) — appropriate for the local Kind dev cluster and clearly documented, but a comment guarding it from production use would add clarity.
• Nit: In the generated outbound Rego, agent_role_scopes is emitted but explicitly not referenced by allow; a brief in-Rego comment (# informational / debugging only — not part of allow) would prevent future policy authors from wondering why it's there.

---

Reviewed by clawgenti using the github-pr-review skill

[clawgenti]

suggestion: password=${user} is the right pattern for this Kind dev cluster (credentials == username per the runbook), but it's easy to cargo-copy this helper into a staging or production script. A one-liner guard — e.g. # DEV ONLY: password == username; see opa-kind-runbook.md prerequisites — right before the curl call would make the intent explicit and reduce copy-paste risk.

[oblinder]

Done in 2233b70 — added a DEV ONLY guard comment right before the curl grant in mint_token, spelling out that password == username is valid only for this Kind dev cluster's seeded users (per the runbook prerequisites) and must not be cargo-copied into a staging/production script. Thanks.

[clawgenti]

nit: agent_role_scopes is rendered here but explicitly not used in allow (the docstring notes "informational / debugging"). A short inline comment in the generated Rego template — e.g. # informational only; not referenced by allow right after the variable is rendered — would prevent future policy authors from hunting for where agent_role_scopes feeds into the decision and assuming it is accidentally omitted.

[oblinder]

Done in 2233b70 — the generated outbound Rego now emits # informational/debugging only — not referenced by allow immediately before agent_role_scopes, so a future policy author reading the bundle sees plainly that it feeds logging/debugging only and that target_scopes[input.identity.service_id] is the capability gate.

[abigailgold]

Few more small comments:

Must-fix:

• As discussed with the authors, we want this feature to be off by default since it is potentially blocking. This includes changes to existing plugins (opa and token exchange). Also please find an additional reviewer for these specific changes (in existing plugins) to give the final ok.

Nits:

• upsert_policy (POST /policy, the batch endpoint) applies agents one at a time with no rollback: if agent N in a batch has a malformed agent_id, agents 1..N-1 are already server-side-applied and stay applied when the batch returns 400. This is called out explicitly in a comment (# A malformed agent_id aborts the batch with a 400 naming it; agents already applied before that point stay written (no rollback)), and is arguably fine given SSA is idempotent and the caller can retry — but it's a real partial-failure semantic worth the caller (the PDP orchestration on the other side of this HTTP call) being aware of. Suggestion: confirm the batch caller either treats 400 as "some agents may have applied" or pre-validates agent_ids before calling /policy.
• New dependency: kubernetes (Python client) added unpinned to aiac/src/aiac/pdp/service/policy/opa/requirements.txt. This matches the existing convention in that file (fastapi/uvicorn/pydantic are also unpinned), so it's consistent with current repo practice rather than a new problem — but as the PDP writer now talks directly to the live K8s control plane with create/update/patch/delete RBAC, an unpinned client library version is a slightly higher-stakes gap than it was for the old filesystem stub. Suggestion: consider pinning kubernetes to a known-good major version given the elevated blast radius of this service now that it mutates cluster state.
• Security note worth flagging in tokenexchange/plugin.go: this is a case of intentionally trusting unverified claims from client-supplied input for policy input enrichment. The risk is contained because (a) it only affects the subject field surfaced for logging/policy input, not the actual authorization gate on this leg — the outbound allow decision in the Rego (from the earlier PDP review) is keyed on target_scopes[input.identity.service_id] and subject_role_scopes[role], both of which come from the delegation hop's audience/scopes (which token-exchange did establish legitimately via its own token minting), not from the unverified subject. But if any future policy branches on input.identity.subject for an outbound allow decision, that would be an actual auth bypass, since a caller could forge a bearer's sub claim freely. Worth a code comment warning future policy authors of that trap, if one doesn't already exist elsewhere in the spec docs.

[oblinder]

@clawgenti thanks for the review — both points are now addressed in 2233b70:

• Suggestion (mint_token password==username): added a DEV ONLY guard comment right before the curl grant, spelling out that it is valid only for this Kind dev cluster's seeded users (per the runbook prerequisites) and must not be cargo-copied into a staging/production script. (reply on the inline thread)
• Nit (outbound Rego agent_role_scopes): the generator now emits # informational/debugging only — not referenced by allow immediately before agent_role_scopes, so a policy author reading the bundle sees plainly it feeds logging/debugging only and that target_scopes[input.identity.service_id] is the capability gate. (reply on the inline thread)

Non-integration suite stays green (472 passed).

[clawgenti]

Large, well-structured PR delivering OPA plugin integration and live enforcement across the AIAC stack. All CI checks pass, all 39 commits carry DCO sign-off, and no agent/IDE config files are touched.

One minor documentation nit on the new driver script (see inline); everything else looks solid.

---

Reviewed by clawgenti using the github-pr-review skill

[clawgenti]

nit: admin_token() hardcodes admin/admin for the Keycloak master realm, but unlike mint_token() (which has a # DEV ONLY: password == username … guard comment at line 122–124) this function has no equivalent caveat. Recommend adding a short # DEV ONLY: admin/admin is the seeded Kind cluster default — never copy into staging/prod note here for consistency and to protect against cargo-copying.

[oblinder]

Done in ba30155 — added a DEV ONLY guard comment right before the curl grant in admin_token(), mirroring mint_token()'s existing caveat: warns that admin/admin is the seeded Kind cluster default and should never be cargo-copied into a staging/production script.

[oblinder]

@abigailgold thanks — responding to all three.

Must-fix: off by default / least-intrusive

We were asked to be as least-intrusive as possible — i.e. any new feature should be turned off by default. We're contributing three things in this PR:

1. Being triggered, the AIAC system now creates a set of AuthorizationPolicy Custom Resources.
   Why there is no impact on the system here: this PR implements only a partial integration. The activating trigger is not implemented yet — the system is activated (and eventually produces the AuthorizationPolicy CRs) on an external API call only. No one is going to do that for now, so the system, even if deployed, remains inactive.

2. The OPA plugins pipeline (A2A, MCP, Token Exchange and OPA plugins) is configured to run in the AuthBridge pipeline.
   Why there is no impact on the system here: the mentioned plugins are deployed on demand only, and by default they don't exist. No OPA exists either, so no additional disable switch is needed.

3. The existing Token Exchange plugin has been enhanced to parse and populate an additional field (subject ID) to be available in the OPA plugin.
   Why there is no impact on the system here: the Token Exchange plugin is not deployed by default, as mentioned above. However, it's worth mentioning that existing demo setups may require this plugin to be installed in the AuthBridge pipeline — in which case the plugin will come up including our enhancement. We'd like to leave this feature in with no switch-off, but ask an additional reviewer (from the AuthBridge team) to review this change.

Nit: upsert_policy batch partial-failure

Confirmed the semantic and checked the caller. The only caller of the batch POST /policy is the Policy Computation Engine (aiac/src/aiac/policy/computation/engine.py), which builds PolicyModel(agents=derived) where every agent_id is a store-derived, already-validated service id — not caller-supplied free text. So a malformed agent_id mid-batch cannot arise from this path, and server-side apply is idempotent, so a retry safely re-applies the whole set. The # ...no rollback comment stays as the honest contract for any future caller that does pass unvalidated ids.

Nit: unpinned kubernetes client

Done in 2233b70 — pinned to the tested major (kubernetes>=36.0.3,<37) in aiac/src/aiac/pdp/service/policy/opa/requirements.txt, with a comment noting the elevated blast radius now that the writer mutates cluster state. Left as a major-cap (not an exact pin) to stay consistent with the file's convention while closing the surprise-37.x gap.

[clawgenti]

Substantial, well-structured PR: the OPA outbound identity synthesis, delegation chain exposure, CR-writer lift, and Rego generator rework all hang together coherently and the test coverage is solid. A couple of small gaps worth noting.

Findings:

• plugin_test.go several buildInput call sites (e.g. lines 344, 382, 405, 426, 453) pass "" as agentID — when pctx.Extensions.Delegation is non-nil those tests will get a synthesized input.identity["client_id"] = "", but none of them assert on that field. Add at least a nil-check or document the empty-string contract.
• rego.py source_ok if { not input.identity.client_id } — on the outbound leg buildOutboundIdentity always emits client_id (even when agentID is ""), so this bypass rule will never fire outbound. On inbound it fires only when the OPA plugin has no validated JWT. Worth a short comment clarifying the intended trigger surface so future authors don't assume it covers the outbound pass-through case.

---

Reviewed by clawgenti using the github-pr-review skill

[clawgenti]

This call passes "" as agentID. If pctx.Extensions.Delegation were non-nil here, buildOutboundIdentity would emit "client_id": "" — an observable but untested behavior. The delegation tests below (e.g. TestBuildInput_WithDelegation) also pass "" and don't assert on client_id. Consider adding an assertion on client_id in the delegation-present tests, or explicitly documenting that "" is the expected value when no agentID is configured.

[oblinder]

Done in cf5bf7d — added TestBuildInput_OutboundIdentityEmptyClientIDWhenAgentIDUnset, which drives buildInput(pctx, inc, "") with a delegation hop present and asserts explicitly that the synthesized input.identity.client_id is present as the empty string (not omitted). That pins the previously-untested "" contract. Outbound allow keys on service_id/scopes, never client_id, so "" is benign — the test just makes the observable value explicit. Thanks for the catch.

[clawgenti]

source_ok if { not input.identity.client_id } — on the outbound leg buildOutboundIdentity always populates client_id (even as "" when agentID is unset), so not input.identity.client_id will never be true outbound. On inbound it fires only when no JWT was validated. A short comment here explaining the intended trigger (inbound-only, unauthenticated callers) would prevent future readers from assuming this covers outbound pass-through traffic.

[oblinder]

Done in cf5bf7d — added a comment above the rule in generate_inbound_rego clarifying its intended trigger surface: it is inbound-only in practice, firing for unauthenticated callers (no validated JWT, so input.identity.client_id is unset). It never fires outbound, since buildOutboundIdentity always populates client_id (as "" when agent_id is unset), so not input.identity.client_id is never true there. Thanks.

[clawgenti]

Large but well-structured phase-2 landing: OPA CR writer, AuthBridge outbound delegation identity, token-exchange bearer-subject extraction, and the Policy Store → Policy Model Store / kagenti → rossoctl renames all land cleanly together.

• upsert_agent ignores the URL agent_id (/policy/agents/{agent_id}): the route param is accepted but unused — _upsert_agent(model) derives namespace/name from model.agent_id instead. The test confirms this intentionally (json=_agent(SPIFFE) against /policy/agents/ignored), but the URL path segment is dead code and could mislead callers who assume it overrides the body. Consider either removing the param or asserting they match.
• Batch upsert is not atomic: upsert_policy aborts on the first 400/502 but leaves previously applied CRs written. The docstring acknowledges this ("no rollback"), but there's no compensating rollback or partial-failure response — callers get a 400 without knowing which agents succeeded. Fine for now given the use case, but worth a TODO comment so it's not surprised later.
• subjectFromToken uses jwt.ParseInsecure: correctly documented as best-effort / observability-only, never for auth. The comment chain from recordDelegationHop → buildOutboundIdentity → README is thorough and consistent. No issue, just confirming the trust boundary is clearly stated.

---

Reviewed by clawgenti using the github-pr-review skill

[clawgenti]

The route param agent_id is declared here but never used — _upsert_agent(model) derives namespace/name from model.agent_id. The test exercises this with /policy/agents/ignored to prove the body wins. Either document the param as intentionally ignored (legacy compatibility?) or drop it; as-is it will confuse callers who expect the URL to be authoritative.

[oblinder]

Done in aa19fc1 — documented the param as intentionally ignored rather than dropping it. The request body is authoritative: _upsert_agent(model) derives namespace/name from model.agent_id (via identity_ref), the single source of truth, so a mismatched or placeholder URL segment (e.g. /policy/agents/ignored) never affects which CR is written. The added comment says exactly that, and notes the segment is kept only to give the route a RESTful shape. Thanks.

[clawgenti]

All prior review nits addressed cleanly — the upsert_agent path-param comment (last commit), source_ok bypass comment in rego.py, and TestBuildInput_OutboundIdentityEmptyClientIDWhenAgentIDUnset test all land correctly. All CI checks pass, all commits signed.

All checks pass. Ready for human review.

---

Reviewed by clawgenti using the github-pr-review skill

[esnible]

Large, well-structured phase-2 PR wiring AIAC into live OPA enforcement: a CR-based PDP policy writer with server-side apply, a reworked Rego generator matching the live AuthBridge plugin input shape, outbound delegation/identity synthesis in the Go plugins, and a rebuilt integration-test suite closing the real enforcement loop on Kind. This has already been through several rounds of automated review with fixes landed for each round (RELEASE_NAMESPACE restore, dev-only guards, Rego comments, empty-client_id test coverage). I independently re-verified the current diff rather than trusting prior comments.

Author: oblinder (MEMBER — maintainer)
Areas reviewed: Go (AuthBridge plugins), Python (PDP/OPA writer, Rego generator), Helm/K8s (RBAC), Shell, Docs
Agent/IDE config (.claude/.vscode): none
Commits: 39 unique commits, all signed-off
CI status: all green (CodeQL, Bandit, Hadolint, DCO, Go/Python/Shell/YAML lint, PR title check)

No must-fix issues. RBAC for aiac-pdp-policy-writer is minimal (get/list/create/update/patch/delete on authorizationpolicies only, no watch) and the cluster-wide scope is justified since agents can live in arbitrary namespaces. identity_ref() validates namespace/name as DNS-1123 labels before use in k8s object names, blocking injection. The source_ok bypass concern raised in earlier review rounds is confirmed resolved — the outbound Rego package never emits source_ok at all. Two minor items inline, neither blocking.

[esnible]

suggestion: This comment says the unverified sub claim is "never for an auth decision," but it does flow through Delegation.Origin → buildOutboundIdentity's subject field → the outbound Rego's subject_ok gate, which does gate allow on the outbound leg. Before this PR, the outbound leg had no fallback here, so subject was always empty and subject_ok was effectively vacuous for that field; now an attacker-supplied/garbage bearer's unverified sub claim can influence the outbound decision if a policy author keys on it (as the shipped example policy does).

This is an existing, already-partially-documented trust boundary (plugin.go's buildDelegationInput doc explicitly notes there's "no inbound JWT to validate" on the outbound leg), so I'm not flagging it as a blocker — just suggesting tightening this comment (e.g. "not independently verified; consumed by outbound subject_ok only when a policy author chooses to gate on it") so it isn't miscited later as a hard security guarantee.

[oblinder]

Good call — tightened in 6f78aed so the comment can't be miscited as a hard guarantee. The subjectFromToken godoc now spells out the trust boundary explicitly: the sub claim is not independently verified, it is surfaced only as input.identity.subject (delegation provenance), and AuthBridge itself makes no auth decision from it — but any policy that gates allow on input.identity.subject (e.g. the outbound subject_ok rule) is trusting a forgeable value, which is a deliberate choice the policy author owns. The inline comment at the ParseInsecure call site now reads unverified: … forgeable, so gating allow on it is a policy-author trust decision (see godoc) instead of the old "not for auth". Thanks.

[esnible]

nit: Batch upsert is non-atomic (documented here) with no bound on batch size. Acceptable as-is per this comment's own caveat, but consider filing a follow-up issue/TODO reference so the partial-write behavior is tracked rather than living only in a docstring.

[oblinder]

Done in 6f78aed — filed a follow-up so the partial-write behavior is tracked rather than living only in the docstring: s-and-p-team#143 (batch POST /policy is non-atomic and unbounded). The inline comment now references that issue and notes possible hardening directions (pre-validate all ids atomically, partial-success response body, bound the batch size). It stays acceptable for the current caller — the Policy Computation Engine builds the batch from store-derived, already-validated service ids, and SSA is idempotent so a retry re-applies the whole set. Thanks.