Skip to content

security(controller): reject cross-namespace spec.agent.ref#564

Closed
bussyjd wants to merge 1 commit into
mainfrom
fix/agent-ref-namespace-confused-deputy
Closed

security(controller): reject cross-namespace spec.agent.ref#564
bussyjd wants to merge 1 commit into
mainfrom
fix/agent-ref-namespace-confused-deputy

Conversation

@bussyjd
Copy link
Copy Markdown
Collaborator

@bussyjd bussyjd commented May 28, 2026

Summary

Closes the confused-deputy gap flagged by @OisinKyne on #556 (review comment).

resolveAgentOffer accepted any spec.agent.ref.namespace, while the verifier route source (introduced in #556) keys the hermes-api-server API_SERVER_KEY lookup off offer.Spec.Agent.Ref.Namespace. Combined with the cluster-wide openclaw-monetize-write ClusterRoleBinding, any principal holding the Hermes obol-agent SA token could publish a ServiceOffer in their own namespace referencing another tenant's Agent. The route would be published with the victim's API_SERVER_KEY as upstream Authorization, attacker-controlled path, and attacker-controlled payment.payTo. Paying buyers would gain authenticated proxy access to the victim Hermes /api/* (chat, skills, sub-agent management, wallet ops).

Fix

Single fail-closed guard at the controller boundary: if spec.agent.ref.namespace != metadata.namespace, the offer is rejected before any route is published or any bearer is resolved. Matches the existing missing-ref error pattern; the legitimate flow (mother creates the offer in the agent's own namespace) is preserved.

if ref.Namespace != offer.Namespace {
    return false, fmt.Errorf("type=agent offer %s/%s: spec.agent.ref.namespace %q must equal offer namespace", offer.Namespace, offer.Name, ref.Namespace)
}

Why now

Not exploitable in today's single Hermes mother layout (only one principal has serviceoffers write, and it already has cluster-wide agents/secrets write over its own children). Becomes exploitable the moment sub-agent SAs split out of the mother or multi-mother marketplace mode lands. Cheap to enforce the invariant now.

Validation

go test ./internal/serviceoffercontroller/... ./internal/x402/... -count=1

New regression: TestResolveAgentOffer_RejectsCrossNamespaceRef asserts the guard fires and status.AgentResolution is not mutated. Existing happy-path tests updated to set metadata.namespace so they exercise the post-guard codepath.

Test plan

  • go test ./internal/serviceoffercontroller/... ./internal/x402/... passes
  • New TestResolveAgentOffer_RejectsCrossNamespaceRef covers the guard
  • Pre-existing happy-path tests still pass (metadata.namespace now set)
  • Reviewer to confirm invariant is acceptable vs. an allow-list / SAR-based approach for future cross-tenant flows

resolveAgentOffer accepted any ref.Namespace, while the verifier route
source in PR #556 keys the hermes-api-server API_SERVER_KEY lookup off
offer.Spec.Agent.Ref.Namespace. Combined with the cluster-wide
openclaw-monetize-write ClusterRoleBinding, any principal holding the
Hermes obol-agent SA token could publish a ServiceOffer in their own
namespace referencing another tenant's Agent, causing the verifier to
inject the victim's API_SERVER_KEY as upstream Authorization on a route
under attacker-controlled path and payTo. Paying buyers would gain
authenticated proxy access to the victim Hermes /api/* (chat, skills,
sub-agent management, wallet ops).

Fail-closed in the controller before any route is published or any
bearer is resolved. The legitimate flow (mother creates offer in the
agent's own namespace) is preserved.

Flagged on PR #556 by @OisinKyne; not exploitable in today's single-
mother layout, but lands ahead of sub-agent SA split / multi-mother
marketplace mode.
@bussyjd
Copy link
Copy Markdown
Collaborator Author

bussyjd commented May 28, 2026

Aggregated into #566 (release/v0.10.0-rc8 → main). Originally split off from the review of #556 — see that PR's discussion thread for the security analysis that motivated this guard.

@bussyjd bussyjd closed this May 28, 2026
@bussyjd bussyjd deleted the fix/agent-ref-namespace-confused-deputy branch May 28, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant