From 1c990b3a0dd930e53baab91661c343213204c9a0 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Thu, 28 May 2026 17:08:40 +0400 Subject: [PATCH] security(controller): reject cross-namespace spec.agent.ref 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. --- .../serviceoffercontroller/agent_resolver.go | 10 ++++ .../agent_resolver_test.go | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/internal/serviceoffercontroller/agent_resolver.go b/internal/serviceoffercontroller/agent_resolver.go index 85c7e675..6d397b3b 100644 --- a/internal/serviceoffercontroller/agent_resolver.go +++ b/internal/serviceoffercontroller/agent_resolver.go @@ -35,6 +35,16 @@ func (c *Controller) resolveAgentOffer(ctx context.Context, offer *monetizeapi.S if ref.Name == "" || ref.Namespace == "" { return false, fmt.Errorf("type=agent offer %s/%s missing spec.agent.ref", offer.Namespace, offer.Name) } + if ref.Namespace != offer.Namespace { + // Confused-deputy guard: the verifier route source injects the + // hermes-api-server API_SERVER_KEY from ref.Namespace into the + // outbound Authorization header. Allowing a cross-namespace ref + // would let any principal with serviceoffers write in namespace A + // expose Hermes /api in namespace B as an x402-gated route under + // attacker-controlled path and payTo, granting paying buyers + // authenticated proxy access to the victim agent. + return false, fmt.Errorf("type=agent offer %s/%s: spec.agent.ref.namespace %q must equal offer namespace", offer.Namespace, offer.Name, ref.Namespace) + } raw, err := c.agents.Namespace(ref.Namespace).Get(ctx, ref.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { diff --git a/internal/serviceoffercontroller/agent_resolver_test.go b/internal/serviceoffercontroller/agent_resolver_test.go index 71ad36ee..716b615e 100644 --- a/internal/serviceoffercontroller/agent_resolver_test.go +++ b/internal/serviceoffercontroller/agent_resolver_test.go @@ -30,6 +30,7 @@ func TestResolveAgentOffer_PopulatesFromReadyAgent(t *testing.T) { c := newResolverTestController(t, agent) offer := &monetizeapi.ServiceOffer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "agent-quant"}, Spec: monetizeapi.ServiceOfferSpec{ Type: "agent", Agent: monetizeapi.ServiceOfferAgent{ @@ -83,6 +84,7 @@ func TestResolveAgentOffer_NotReadyAgentClearsResolution(t *testing.T) { c := newResolverTestController(t, agent) offer := &monetizeapi.ServiceOffer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "agent-quant"}, Spec: monetizeapi.ServiceOfferSpec{ Type: "agent", Agent: monetizeapi.ServiceOfferAgent{ @@ -113,6 +115,7 @@ func TestResolveAgentOffer_MissingAgentReturnsNotReady(t *testing.T) { c := newResolverTestController(t) offer := &monetizeapi.ServiceOffer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "agent-missing"}, Spec: monetizeapi.ServiceOfferSpec{ Type: "agent", Agent: monetizeapi.ServiceOfferAgent{ @@ -147,6 +150,52 @@ func TestResolveAgentOffer_RejectsMissingRef(t *testing.T) { } } +// TestResolveAgentOffer_RejectsCrossNamespaceRef guards the confused-deputy +// invariant: an offer in namespace A must not be allowed to reference an agent +// in namespace B, because the verifier route source injects ref.Namespace's +// hermes-api-server API_SERVER_KEY as the upstream Authorization. Allowing a +// cross-namespace ref would let any principal with serviceoffers write expose +// another tenant's Hermes /api as an x402-gated route under attacker-controlled +// path + payTo. +func TestResolveAgentOffer_RejectsCrossNamespaceRef(t *testing.T) { + agent := &monetizeapi.Agent{ + TypeMeta: metav1.TypeMeta{APIVersion: "obol.org/v1alpha1", Kind: "Agent"}, + ObjectMeta: metav1.ObjectMeta{Name: "victim", Namespace: "agent-victim"}, + Status: monetizeapi.AgentStatus{ + Phase: monetizeapi.AgentPhaseReady, + Endpoint: "http://hermes.agent-victim.svc.cluster.local:8642", + }, + } + c := newResolverTestController(t, agent) + + offer := &monetizeapi.ServiceOffer{ + ObjectMeta: metav1.ObjectMeta{Name: "spoof", Namespace: "attacker-ns"}, + Spec: monetizeapi.ServiceOfferSpec{ + Type: "agent", + Agent: monetizeapi.ServiceOfferAgent{ + Ref: monetizeapi.ServiceOfferAgentRef{Name: "victim", Namespace: "agent-victim"}, + }, + }, + } + status := monetizeapi.ServiceOfferStatus{ + AgentResolution: &monetizeapi.ServiceOfferAgentResolution{Model: "stale"}, + } + + ok, err := c.resolveAgentOffer(context.Background(), offer, &status) + if err == nil { + t.Fatal("expected error for cross-namespace spec.agent.ref") + } + if ok { + t.Fatal("expected ok=false for cross-namespace ref") + } + if status.AgentResolution == nil || status.AgentResolution.Model != "stale" { + // Guard fires before touching status: the caller is responsible for + // the failure-mode condition update, and we should not silently wipe + // a prior AgentResolution. + t.Errorf("guard must reject without mutating status.AgentResolution; got %+v", status.AgentResolution) + } +} + func newResolverTestController(t *testing.T, agents ...*monetizeapi.Agent) *Controller { t.Helper() objs := make([]runtime.Object, 0, len(agents))