Skip to content

Commit 81b4339

Browse files
author
bussyjd
committed
Merge PR #564: security(controller): reject cross-namespace spec.agent.ref
2 parents cea22ab + 1c990b3 commit 81b4339

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

internal/serviceoffercontroller/agent_resolver.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ func (c *Controller) resolveAgentOffer(ctx context.Context, offer *monetizeapi.S
3535
if ref.Name == "" || ref.Namespace == "" {
3636
return false, fmt.Errorf("type=agent offer %s/%s missing spec.agent.ref", offer.Namespace, offer.Name)
3737
}
38+
if ref.Namespace != offer.Namespace {
39+
// Confused-deputy guard: the verifier route source injects the
40+
// hermes-api-server API_SERVER_KEY from ref.Namespace into the
41+
// outbound Authorization header. Allowing a cross-namespace ref
42+
// would let any principal with serviceoffers write in namespace A
43+
// expose Hermes /api in namespace B as an x402-gated route under
44+
// attacker-controlled path and payTo, granting paying buyers
45+
// authenticated proxy access to the victim agent.
46+
return false, fmt.Errorf("type=agent offer %s/%s: spec.agent.ref.namespace %q must equal offer namespace", offer.Namespace, offer.Name, ref.Namespace)
47+
}
3848

3949
raw, err := c.agents.Namespace(ref.Namespace).Get(ctx, ref.Name, metav1.GetOptions{})
4050
if apierrors.IsNotFound(err) {

internal/serviceoffercontroller/agent_resolver_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestResolveAgentOffer_PopulatesFromReadyAgent(t *testing.T) {
3030
c := newResolverTestController(t, agent)
3131

3232
offer := &monetizeapi.ServiceOffer{
33+
ObjectMeta: metav1.ObjectMeta{Namespace: "agent-quant"},
3334
Spec: monetizeapi.ServiceOfferSpec{
3435
Type: "agent",
3536
Agent: monetizeapi.ServiceOfferAgent{
@@ -83,6 +84,7 @@ func TestResolveAgentOffer_NotReadyAgentClearsResolution(t *testing.T) {
8384
c := newResolverTestController(t, agent)
8485

8586
offer := &monetizeapi.ServiceOffer{
87+
ObjectMeta: metav1.ObjectMeta{Namespace: "agent-quant"},
8688
Spec: monetizeapi.ServiceOfferSpec{
8789
Type: "agent",
8890
Agent: monetizeapi.ServiceOfferAgent{
@@ -113,6 +115,7 @@ func TestResolveAgentOffer_MissingAgentReturnsNotReady(t *testing.T) {
113115
c := newResolverTestController(t)
114116

115117
offer := &monetizeapi.ServiceOffer{
118+
ObjectMeta: metav1.ObjectMeta{Namespace: "agent-missing"},
116119
Spec: monetizeapi.ServiceOfferSpec{
117120
Type: "agent",
118121
Agent: monetizeapi.ServiceOfferAgent{
@@ -147,6 +150,52 @@ func TestResolveAgentOffer_RejectsMissingRef(t *testing.T) {
147150
}
148151
}
149152

153+
// TestResolveAgentOffer_RejectsCrossNamespaceRef guards the confused-deputy
154+
// invariant: an offer in namespace A must not be allowed to reference an agent
155+
// in namespace B, because the verifier route source injects ref.Namespace's
156+
// hermes-api-server API_SERVER_KEY as the upstream Authorization. Allowing a
157+
// cross-namespace ref would let any principal with serviceoffers write expose
158+
// another tenant's Hermes /api as an x402-gated route under attacker-controlled
159+
// path + payTo.
160+
func TestResolveAgentOffer_RejectsCrossNamespaceRef(t *testing.T) {
161+
agent := &monetizeapi.Agent{
162+
TypeMeta: metav1.TypeMeta{APIVersion: "obol.org/v1alpha1", Kind: "Agent"},
163+
ObjectMeta: metav1.ObjectMeta{Name: "victim", Namespace: "agent-victim"},
164+
Status: monetizeapi.AgentStatus{
165+
Phase: monetizeapi.AgentPhaseReady,
166+
Endpoint: "http://hermes.agent-victim.svc.cluster.local:8642",
167+
},
168+
}
169+
c := newResolverTestController(t, agent)
170+
171+
offer := &monetizeapi.ServiceOffer{
172+
ObjectMeta: metav1.ObjectMeta{Name: "spoof", Namespace: "attacker-ns"},
173+
Spec: monetizeapi.ServiceOfferSpec{
174+
Type: "agent",
175+
Agent: monetizeapi.ServiceOfferAgent{
176+
Ref: monetizeapi.ServiceOfferAgentRef{Name: "victim", Namespace: "agent-victim"},
177+
},
178+
},
179+
}
180+
status := monetizeapi.ServiceOfferStatus{
181+
AgentResolution: &monetizeapi.ServiceOfferAgentResolution{Model: "stale"},
182+
}
183+
184+
ok, err := c.resolveAgentOffer(context.Background(), offer, &status)
185+
if err == nil {
186+
t.Fatal("expected error for cross-namespace spec.agent.ref")
187+
}
188+
if ok {
189+
t.Fatal("expected ok=false for cross-namespace ref")
190+
}
191+
if status.AgentResolution == nil || status.AgentResolution.Model != "stale" {
192+
// Guard fires before touching status: the caller is responsible for
193+
// the failure-mode condition update, and we should not silently wipe
194+
// a prior AgentResolution.
195+
t.Errorf("guard must reject without mutating status.AgentResolution; got %+v", status.AgentResolution)
196+
}
197+
}
198+
150199
func newResolverTestController(t *testing.T, agents ...*monetizeapi.Agent) *Controller {
151200
t.Helper()
152201
objs := make([]runtime.Object, 0, len(agents))

0 commit comments

Comments
 (0)