Skip to content

Commit 604cc4c

Browse files
MatteoMoriclaude
andcommitted
fix(mcp): enforce agent allow-list per session, not per request
Parse the ?agents= allow-list in the NewStreamableHTTPHandler factory, which is called once on the MCP initialize request. Closing over the result in the tool handlers makes the filter immutable for the session's lifetime, eliminating a bypass window where a follow-up request without the query parameter would produce a nil allow-list. Remove context plumbing (contextKey, allowedAgentsKey, allowedAgentsFromContext) and pass allowed directly to handleListAgents and handleInvokeAgent. ServeHTTP is now a one-liner passthrough. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: MatteoMori <morimatteo14@gmail.com>
1 parent eb3eb62 commit 604cc4c

File tree

2 files changed

+33
-75
lines changed

2 files changed

+33
-75
lines changed

go/core/internal/mcp/mcp_handler.go

Lines changed: 33 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,6 @@ import (
2121
"trpc.group/trpc-go/trpc-a2a-go/protocol"
2222
)
2323

24-
// contextKey is an unexported type for context keys used within this package.
25-
// Using a dedicated type prevents collisions with keys from other packages.
26-
type contextKey int
27-
28-
const (
29-
// allowedAgentsKey is the context key that carries the set of agent refs
30-
// permitted for the current MCP session. A nil value means all agents
31-
// are allowed (no filtering applied).
32-
allowedAgentsKey contextKey = iota
33-
)
34-
3524
// MCPHandler handles MCP requests and bridges them to A2A endpoints.
3625
//
3726
// Agent filtering:
@@ -44,12 +33,15 @@ const (
4433
// and invoke_agent rejects calls targeting any agent not in the list.
4534
// Omitting the parameter preserves the original behaviour: all agents are
4635
// accessible.
36+
//
37+
// The allow-list is parsed once per session in the server factory (on the
38+
// initialize request) and closed over in the tool handlers, making it
39+
// immutable for the session's lifetime.
4740
type MCPHandler struct {
4841
kubeClient client.Client
4942
a2aBaseURL string
5043
authenticator auth.AuthProvider
5144
httpHandler *mcpsdk.StreamableHTTPHandler
52-
server *mcpsdk.Server
5345
a2aClients sync.Map
5446
}
5547

@@ -86,46 +78,52 @@ func NewMCPHandler(kubeClient client.Client, a2aBaseURL string, authenticator au
8678
authenticator: authenticator,
8779
}
8880

89-
// Create MCP server
90-
impl := &mcpsdk.Implementation{
81+
// The server factory is called exactly once per MCP session (on the
82+
// initialize request). Parsing the allow-list here and closing over it in
83+
// the tool handlers makes the filter immutable for the session's lifetime
84+
// — no context plumbing, no per-request re-parsing, no bypass window.
85+
handler.httpHandler = mcpsdk.NewStreamableHTTPHandler(
86+
func(r *http.Request) *mcpsdk.Server {
87+
return handler.newMCPServer(parseAllowedAgents(r))
88+
},
89+
nil,
90+
)
91+
92+
return handler, nil
93+
}
94+
95+
// newMCPServer creates a new MCP server with the given agent allow-list
96+
// closed over in the tool handlers. A nil allow-list means all agents are
97+
// accessible.
98+
func (h *MCPHandler) newMCPServer(allowed map[string]struct{}) *mcpsdk.Server {
99+
server := mcpsdk.NewServer(&mcpsdk.Implementation{
91100
Name: "kagent-agents",
92101
Version: version.Version,
93-
}
94-
server := mcpsdk.NewServer(impl, nil)
95-
handler.server = server
102+
}, nil)
96103

97-
// Add list_agents tool
98104
mcpsdk.AddTool[ListAgentsInput, ListAgentsOutput](
99105
server,
100106
&mcpsdk.Tool{
101107
Name: "list_agents",
102108
Description: "List invokable kagent agents (accepted + deploymentReady)",
103109
},
104-
handler.handleListAgents,
110+
func(ctx context.Context, req *mcpsdk.CallToolRequest, input ListAgentsInput) (*mcpsdk.CallToolResult, ListAgentsOutput, error) {
111+
return h.handleListAgents(ctx, req, input, allowed)
112+
},
105113
)
106114

107-
// Add invoke_agent tool
108115
mcpsdk.AddTool[InvokeAgentInput, InvokeAgentOutput](
109116
server,
110117
&mcpsdk.Tool{
111118
Name: "invoke_agent",
112119
Description: "Invoke a kagent agent via A2A",
113120
},
114-
handler.handleInvokeAgent,
115-
)
116-
117-
// Create HTTP handler. The getServer factory receives the original *http.Request,
118-
// whose context carries any values stored by ServeHTTP (e.g. the agent allow-list).
119-
// The MCP SDK propagates the request context to tool handlers, preserving those
120-
// values even after detaching the cancellation signal for long-running streams.
121-
handler.httpHandler = mcpsdk.NewStreamableHTTPHandler(
122-
func(*http.Request) *mcpsdk.Server {
123-
return server
121+
func(ctx context.Context, req *mcpsdk.CallToolRequest, input InvokeAgentInput) (*mcpsdk.CallToolResult, InvokeAgentOutput, error) {
122+
return h.handleInvokeAgent(ctx, req, input, allowed)
124123
},
125-
nil,
126124
)
127125

128-
return handler, nil
126+
return server
129127
}
130128

131129
// parseAllowedAgents reads the "agents" query parameter from the request and
@@ -157,21 +155,10 @@ func parseAllowedAgents(r *http.Request) map[string]struct{} {
157155
return set
158156
}
159157

160-
// allowedAgentsFromContext returns the agent allow-list stored in ctx, or nil
161-
// if no filtering is active for the current session.
162-
func allowedAgentsFromContext(ctx context.Context) map[string]struct{} {
163-
v := ctx.Value(allowedAgentsKey)
164-
if v == nil {
165-
return nil
166-
}
167-
allowed, _ := v.(map[string]struct{})
168-
return allowed
169-
}
170-
171158
// handleListAgents handles the list_agents MCP tool.
172159
// When an agent allow-list is active for the session, only agents whose ref
173160
// appears in the list are returned.
174-
func (h *MCPHandler) handleListAgents(ctx context.Context, req *mcpsdk.CallToolRequest, input ListAgentsInput) (*mcpsdk.CallToolResult, ListAgentsOutput, error) {
161+
func (h *MCPHandler) handleListAgents(ctx context.Context, req *mcpsdk.CallToolRequest, input ListAgentsInput, allowed map[string]struct{}) (*mcpsdk.CallToolResult, ListAgentsOutput, error) {
175162
log := ctrllog.FromContext(ctx).WithName("mcp-handler").WithValues("tool", "list_agents")
176163

177164
agentList := &v1alpha2.AgentList{}
@@ -184,9 +171,6 @@ func (h *MCPHandler) handleListAgents(ctx context.Context, req *mcpsdk.CallToolR
184171
}, ListAgentsOutput{}, nil
185172
}
186173

187-
// Retrieve the optional allow-list for this MCP session.
188-
allowed := allowedAgentsFromContext(ctx)
189-
190174
agents := make([]AgentSummary, 0)
191175
for _, agent := range agentList.Items {
192176
// Only include agents that are both accepted and deployment-ready.
@@ -250,7 +234,7 @@ func (h *MCPHandler) handleListAgents(ctx context.Context, req *mcpsdk.CallToolR
250234
// handleInvokeAgent handles the invoke_agent MCP tool.
251235
// When an agent allow-list is active for the session, requests targeting an
252236
// agent not in the list are rejected before any A2A call is made.
253-
func (h *MCPHandler) handleInvokeAgent(ctx context.Context, req *mcpsdk.CallToolRequest, input InvokeAgentInput) (*mcpsdk.CallToolResult, InvokeAgentOutput, error) {
237+
func (h *MCPHandler) handleInvokeAgent(ctx context.Context, req *mcpsdk.CallToolRequest, input InvokeAgentInput, allowed map[string]struct{}) (*mcpsdk.CallToolResult, InvokeAgentOutput, error) {
254238
log := ctrllog.FromContext(ctx).WithName("mcp-handler").WithValues("tool", "invoke_agent")
255239

256240
// Parse agent reference (namespace/name or just name)
@@ -270,7 +254,7 @@ func (h *MCPHandler) handleInvokeAgent(ctx context.Context, req *mcpsdk.CallTool
270254
// This is the hard access-control boundary: if the caller's MCP session was
271255
// scoped to a subset of agents (via the "agents" query parameter), any attempt
272256
// to invoke an agent outside that subset is rejected here.
273-
if allowed := allowedAgentsFromContext(ctx); allowed != nil {
257+
if allowed != nil {
274258
if _, ok := allowed[agentRef]; !ok {
275259
log.Info("Rejected invoke_agent: agent not in allow-list", "agent", agentRef)
276260
return &mcpsdk.CallToolResult{
@@ -399,16 +383,7 @@ func (h *MCPHandler) handleInvokeAgent(ctx context.Context, req *mcpsdk.CallTool
399383
}
400384

401385
// ServeHTTP implements http.Handler.
402-
//
403-
// If the request URL contains an "agents" query parameter, ServeHTTP parses it
404-
// into an allow-list and stores it in the request context before delegating to
405-
// the underlying MCP handler. The MCP SDK propagates this context to all tool
406-
// handlers for the session, enabling per-session agent filtering.
407386
func (h *MCPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
408-
if allowed := parseAllowedAgents(r); allowed != nil {
409-
ctx := context.WithValue(r.Context(), allowedAgentsKey, allowed)
410-
r = r.WithContext(ctx)
411-
}
412387
h.httpHandler.ServeHTTP(w, r)
413388
}
414389

go/core/internal/mcp/mcp_handler_test.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package mcp
33
import (
44
"bufio"
55
"bytes"
6-
"context"
76
"encoding/json"
87
"fmt"
98
"net/http"
@@ -90,22 +89,6 @@ func TestParseAllowedAgents(t *testing.T) {
9089
}
9190
}
9291

93-
// --- allowedAgentsFromContext unit tests ---
94-
95-
func TestAllowedAgentsFromContext(t *testing.T) {
96-
t.Run("returns nil when no value in context", func(t *testing.T) {
97-
got := allowedAgentsFromContext(context.Background())
98-
assert.Nil(t, got)
99-
})
100-
101-
t.Run("returns set stored in context", func(t *testing.T) {
102-
want := map[string]struct{}{"kagent/k8s-agent": {}}
103-
ctx := context.WithValue(context.Background(), allowedAgentsKey, want)
104-
got := allowedAgentsFromContext(ctx)
105-
assert.Equal(t, want, got)
106-
})
107-
}
108-
10992
// --- MCP handler integration tests ---
11093

11194
// scheme holds the CRD types used in fake client construction.

0 commit comments

Comments
 (0)