From 5807132ac6ccd813a9341798da79496c52c7e55b Mon Sep 17 00:00:00 2001 From: matst80 Date: Sat, 22 Aug 2026 22:33:18 +0200 Subject: [PATCH] mcp --- mcp/agent.go | 223 ++++++++++++++++++++++++------------ mcp/agent_test.go | 242 ++++++++++++++++++++++++++++++++++++++-- mcp/schema.go | 8 ++ mcp/tool.go | 67 ++++++++++- mcpauth/mcpauth.go | 146 ++++++++++++++++++++++++ mcpauth/mcpauth_test.go | 233 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 838 insertions(+), 81 deletions(-) create mode 100644 mcpauth/mcpauth.go create mode 100644 mcpauth/mcpauth_test.go diff --git a/mcp/agent.go b/mcp/agent.go index bebbc9f..db101ca 100644 --- a/mcp/agent.go +++ b/mcp/agent.go @@ -2,16 +2,20 @@ package mcp // Agent is the generic OpenAI↔MCP bridge: an OpenAI-compatible chat // completions endpoint whose tools are discovered from — and executed against — -// any MCP endpoint. It knows nothing about content, carts, or orders: +// one or more MCP endpoints. It knows nothing about content, carts, or orders: // -// 1. On every request it calls tools/list on the MCP endpoint and advertises -// those tools to the model (merged with any client-supplied tools). +// 1. On every request it calls tools/list on each configured MCP endpoint and +// advertises those tools to the model (merged with any client-supplied +// tools, deduplicated by name — first endpoint wins). // 2. It runs the agentic loop, streaming text deltas to the client as SSE -// while intercepting tool calls and dispatching them via tools/call, -// appending the configured Bearer token. -// 3. Tool calls the MCP server does not know are passed through to the client +// while intercepting tool calls and dispatching them via tools/call to the +// endpoint that advertised the tool, appending that endpoint's token. +// 3. Tool calls no MCP server knows are passed through to the client // (frontend tool calls) so the browser can execute them itself. // +// An endpoint that fails tools/list degrades gracefully: it is skipped for that +// request (logged), so one unreachable service never takes the assistant down. +// // The wire contract for streaming clients: named SSE events "delta" (a JSON // string chunk), (tool activity), "response" (final payload), // then "[DONE]". Non-streaming clients get a normal chat.completion JSON body. @@ -29,6 +33,18 @@ import ( "time" ) +// MCPEndpoint is one streamable-HTTP MCP server providing tools to the agent. +type MCPEndpoint struct { + // URL is the JSON-RPC endpoint (e.g. http://127.0.0.1:8941/finder-mcp). + URL string + // Token is appended as "Authorization: Bearer " on every request to + // this endpoint. Empty means no Authorization header (e.g. loopback). + Token string + // Label identifies the endpoint in logs and tool-progress events. + // Defaults to the URL. + Label string +} + // AgentConfig configures an Agent. type AgentConfig struct { // UpstreamURL is the base URL of an OpenAI-compatible API, e.g. @@ -38,11 +54,9 @@ type AgentConfig struct { UpstreamAPIKey string // Model is the default model used when the request omits one. Model string - // MCPURL is the streamable-HTTP MCP endpoint providing the tools. - MCPURL string - // MCPToken is appended as "Authorization: Bearer " on every MCP - // request. Empty means no Authorization header (e.g. a loopback listener). - MCPToken string + // MCPServers lists the MCP endpoints providing tools. May be empty — the + // agent then acts as a plain proxied chat with only LocalTools. + MCPServers []MCPEndpoint // SystemPrompt is prepended as a system message unless the conversation // already starts with one. Empty disables injection. SystemPrompt string @@ -52,8 +66,8 @@ type AgentConfig struct { // MaxToolIterations caps the agentic loop (default 10). MaxToolIterations int // LocalTools are executed in-process by the agent instead of being - // dispatched over the MCP endpoint. They are advertised to the model - // alongside the MCP tools but never reach MCP clients — for capabilities + // dispatched over an MCP endpoint. They are advertised to the model + // alongside the remote tools but never reach MCP clients — for capabilities // that are agent-only (e.g. a CDP browser tool returning base64 // screenshots too bulky and too privileged for the shared tool surface). LocalTools []Tool @@ -61,12 +75,28 @@ type AgentConfig struct { Logger *slog.Logger } +// mcpRemote is the resolved per-endpoint state used during one request. +type mcpRemote struct { + endpoint MCPEndpoint + url string // trimmed trailing slash + token string + label string +} + +// route is one advertised tool: which endpoint owns it and whether it is a +// pure read (from the MCP tool annotations, when the server provides them). +type route struct { + remote *mcpRemote + readOnly bool +} + // Agent is the HTTP handler for the chat completions endpoint. Build it with // NewAgent and mount Handler(). type Agent struct { - cfg AgentConfig - client *http.Client - log *slog.Logger + cfg AgentConfig + client *http.Client + log *slog.Logger + remotes []mcpRemote } // NewAgent builds an Agent. @@ -80,15 +110,34 @@ func NewAgent(cfg AgentConfig) *Agent { if cfg.Logger == nil { cfg.Logger = slog.Default() } + remotes := make([]mcpRemote, 0, len(cfg.MCPServers)) + for _, e := range cfg.MCPServers { + if e.URL == "" { + continue + } + label := e.Label + if label == "" { + label = e.URL + } + remotes = append(remotes, mcpRemote{ + endpoint: e, + url: strings.TrimSuffix(e.URL, "/"), + token: e.Token, + label: label, + }) + } return &Agent{ - cfg: cfg, - client: &http.Client{Timeout: 5 * time.Minute}, - log: cfg.Logger, + cfg: cfg, + client: &http.Client{Timeout: 5 * time.Minute}, + log: cfg.Logger, + remotes: remotes, } } // Enabled reports whether the agent has enough configuration to serve. -func (a *Agent) Enabled() bool { return a.cfg.UpstreamURL != "" && a.cfg.UpstreamAPIKey != "" && a.cfg.MCPURL != "" } +func (a *Agent) Enabled() bool { + return a.cfg.UpstreamURL != "" && a.cfg.UpstreamAPIKey != "" +} // Handler returns the http.Handler to mount at the chat completions route. func (a *Agent) Handler() http.Handler { return http.HandlerFunc(a.serveHTTP) } @@ -169,13 +218,14 @@ type FrontendToolCall struct { // toolProgress is emitted as a named SSE event for every tool invocation so // streaming clients can show a live activity log. type toolProgress struct { - Iter int `json:"iter"` - Tool string `json:"tool"` - Args string `json:"args,omitempty"` - Status string `json:"status"` // "calling" | "done" | "error" - Err string `json:"error,omitempty"` - Bytes int `json:"bytes,omitempty"` - Result any `json:"result,omitempty"` + Iter int `json:"iter"` + Tool string `json:"tool"` + Args string `json:"args,omitempty"` + Status string `json:"status"` // "calling" | "done" | "error" + Err string `json:"error,omitempty"` + Bytes int `json:"bytes,omitempty"` + Result any `json:"result,omitempty"` + ReadOnly bool `json:"readOnly,omitempty"` } // ── HTTP handler ─────────────────────────────────────────────────────────────── @@ -200,12 +250,10 @@ func (a *Agent) serveHTTP(w http.ResponseWriter, r *http.Request) { req.Model = a.cfg.Model } - mcpTools, err := a.listTools(r.Context()) - if err != nil { - a.log.Error("agent: tools/list failed", "err", err) - http.Error(w, "mcp tools/list: "+err.Error(), http.StatusBadGateway) - return - } + // Discover tools from every endpoint. A failing endpoint is skipped for + // this request (logged) — one unreachable service never takes the + // assistant down. routes maps tool name → the endpoint that advertised it. + mcpTools, routes := a.listTools(r.Context()) req = mergeTools(req, mcpTools, localToolDefs(a.cfg.LocalTools)) if a.cfg.SystemPrompt != "" && (len(req.Messages) == 0 || req.Messages[0].Role != "system") { req.Messages = append([]Message{{Role: "system", Content: a.cfg.SystemPrompt}}, req.Messages...) @@ -242,7 +290,7 @@ func (a *Agent) serveHTTP(w http.ResponseWriter, r *http.Request) { http.Error(w, msg, status) } - content, finishReason, frontendCalls, ok := a.loop(r.Context(), w, flush, sendErr, &req) + content, finishReason, frontendCalls, ok := a.loop(r.Context(), w, flush, sendErr, &req, routes) if !ok { return // sendErr already wrote the response } @@ -286,6 +334,7 @@ func (a *Agent) loop( flush func(), sendErr func(status int, msg string), req *ChatRequest, + routes map[string]route, ) (content, finishReason string, frontendCalls []FrontendToolCall, ok bool) { parallel := true req.ParallelToolCalls = ¶llel @@ -341,9 +390,16 @@ func (a *Agent) loop( local := localToolIndex(a.cfg.LocalTools) var remaining []ToolCall for _, tc := range toolCalls { - a.emitProgress(w, flush, toolProgress{ + progress := toolProgress{ Iter: iter, Tool: tc.Function.Name, Args: tc.Function.Arguments, Status: "calling", - }) + } + if lt, isLocal := local[tc.Function.Name]; isLocal && lt.Annotations != nil { + progress.ReadOnly = lt.Annotations.ReadOnlyHint + } + if rt, isRemote := routes[tc.Function.Name]; isRemote && rt.remote != nil { + progress.ReadOnly = rt.readOnly + } + a.emitProgress(w, flush, progress) var ( resultContent string @@ -361,13 +417,16 @@ func (a *Agent) loop( b, _ := json.Marshal(out) resultContent = string(b) } - } else { - out, found, callToolErr := a.callTool(ctx, tc.Function.Name, tc.Function.Arguments) + } else if rt, ok := routes[tc.Function.Name]; ok { + out, found, callToolErr := a.callTool(ctx, rt.remote, tc.Function.Name, tc.Function.Arguments) if !found { remaining = append(remaining, tc) continue } resultContent, isError, callErr = out.text, out.isError, callToolErr + } else { + remaining = append(remaining, tc) + continue } status, summary := "done", any(nil) @@ -591,9 +650,10 @@ func localToolIndex(tools []Tool) map[string]Tool { type mcpToolList struct { Tools []struct { - Name string `json:"name"` - Description string `json:"description"` - InputSchema json.RawMessage `json:"inputSchema"` + Name string `json:"name"` + Description string `json:"description"` + InputSchema json.RawMessage `json:"inputSchema"` + Annotations *ToolAnnotations `json:"annotations"` } `json:"tools"` } @@ -611,30 +671,50 @@ type toolOutput struct { isError bool // MCP isError flag (a tool-level failure the model can read) } -// listTools fetches the MCP server's advertised tools as OpenAI definitions. -func (a *Agent) listTools(ctx context.Context) ([]ToolDef, error) { - var out mcpToolList - if err := a.mcpRequest(ctx, "tools/list", nil, &out); err != nil { - return nil, err - } - defs := make([]ToolDef, 0, len(out.Tools)) - for _, t := range out.Tools { - schema := t.InputSchema - if len(schema) == 0 { - schema = Object(nil, nil) +// listTools fetches tools/list from every configured endpoint. It returns the +// merged OpenAI definitions (first endpoint wins on name collisions) plus the +// routes map tool name → endpoint used for dispatch. +func (a *Agent) listTools(ctx context.Context) ([]ToolDef, map[string]route) { + defs := make([]ToolDef, 0) + routes := make(map[string]route) + seen := make(map[string]bool) + for i := range a.remotes { + remote := &a.remotes[i] + var out mcpToolList + if err := remote.request(ctx, a.client, "tools/list", nil, &out); err != nil { + a.log.Warn("agent: tools/list failed — skipping endpoint for this request", + "endpoint", remote.label, "err", err) + continue + } + added := 0 + for _, t := range out.Tools { + if t.Name == "" || seen[t.Name] { + continue // first endpoint wins on collisions + } + schema := t.InputSchema + if len(schema) == 0 { + schema = Object(nil, nil) + } + defs = append(defs, ToolDef{Type: "function", Function: FuncDef{ + Name: t.Name, + Description: t.Description, + Parameters: schema, + }}) + routes[t.Name] = route{remote: remote, readOnly: t.Annotations != nil && t.Annotations.ReadOnlyHint} + seen[t.Name] = true + added++ + } + if added > 0 { + a.log.Info("agent: mcp endpoint merged", "endpoint", remote.label, "tools", added) } - defs = append(defs, ToolDef{Type: "function", Function: FuncDef{ - Name: t.Name, - Description: t.Description, - Parameters: schema, - }}) } - return defs, nil + return defs, routes } -// callTool dispatches one tools/call. found=false means the MCP server does not -// know the tool (the caller passes it through to the frontend). -func (a *Agent) callTool(ctx context.Context, name, argsJSON string) (out toolOutput, found bool, err error) { +// callTool dispatches one tools/call to the endpoint that advertised the tool. +// found=false means the route vanished (the caller passes the call through to +// the frontend). +func (a *Agent) callTool(ctx context.Context, remote *mcpRemote, name, argsJSON string) (out toolOutput, found bool, err error) { params := map[string]any{"name": name} if strings.TrimSpace(argsJSON) != "" { var args any @@ -643,11 +723,11 @@ func (a *Agent) callTool(ctx context.Context, name, argsJSON string) (out toolOu } } var res mcpCallResult - if err := a.mcpRequest(ctx, "tools/call", params, &res); err != nil { + if err := remote.request(ctx, a.client, "tools/call", params, &res); err != nil { if strings.Contains(err.Error(), "unknown tool") { return toolOutput{}, false, nil } - return toolOutput{}, true, err + return toolOutput{}, true, fmt.Errorf("%s: %w", remote.label, err) } var texts strings.Builder @@ -686,9 +766,9 @@ func previewOf(s string) any { } } -// mcpRequest posts one JSON-RPC request to the MCP endpoint, appending the -// configured Bearer token, and decodes the result into out. -func (a *Agent) mcpRequest(ctx context.Context, method string, params, out any) error { +// request posts one JSON-RPC request to this endpoint, appending its Bearer +// token, and decodes the result into out. +func (r *mcpRemote) request(ctx context.Context, client *http.Client, method string, params, out any) error { body, err := json.Marshal(map[string]any{ "jsonrpc": "2.0", "id": 1, @@ -698,16 +778,19 @@ func (a *Agent) mcpRequest(ctx context.Context, method string, params, out any) if err != nil { return err } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, a.cfg.MCPURL, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, r.url, bytes.NewReader(body)) if err != nil { return err } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - if a.cfg.MCPToken != "" { - req.Header.Set("Authorization", "Bearer "+a.cfg.MCPToken) + if r.token != "" { + req.Header.Set("Authorization", "Bearer "+r.token) } - resp, err := http.DefaultClient.Do(req) + if client == nil { + client = http.DefaultClient + } + resp, err := client.Do(req) if err != nil { return fmt.Errorf("mcp %s: %w", method, err) } diff --git a/mcp/agent_test.go b/mcp/agent_test.go index dde482e..2395168 100644 --- a/mcp/agent_test.go +++ b/mcp/agent_test.go @@ -156,10 +156,12 @@ func TestAgentAgenticLoop(t *testing.T) { agent := NewAgent(AgentConfig{ UpstreamURL: upSrv.URL, UpstreamAPIKey: "sk-test", - MCPURL: mcpSrv.URL + "/mcp", - MCPToken: "mcp-secret", - SystemPrompt: "You are a test.", - ProgressEvent: "cms_progress", + MCPServers: []MCPEndpoint{{ + URL: mcpSrv.URL + "/mcp", + Token: "mcp-secret", + }}, + SystemPrompt: "You are a test.", + ProgressEvent: "cms_progress", }) body := `{"model":"test-model","messages":[{"role":"user","content":"hi"}],"stream":true}` @@ -271,7 +273,7 @@ func TestAgentLocalTool(t *testing.T) { authBefore := lastAuth agent := NewAgent(AgentConfig{ UpstreamURL: upSrv.URL, UpstreamAPIKey: "k", - MCPURL: mcpSrv.URL + "/mcp", + MCPServers: []MCPEndpoint{{URL: mcpSrv.URL + "/mcp"}}, LocalTools: []Tool{{ Name: "local_ping", Description: "In-process ping", @@ -320,7 +322,7 @@ func TestAgentFrontendToolCallPassthrough(t *testing.T) { agent := NewAgent(AgentConfig{ UpstreamURL: upSrv.URL, UpstreamAPIKey: "k", - MCPURL: mcpSrv.URL + "/mcp", + MCPServers: []MCPEndpoint{{URL: mcpSrv.URL + "/mcp"}}, }) req := httptest.NewRequest(http.MethodPost, "/chat/completions", strings.NewReader(`{"messages":[{"role":"user","content":"go"}],"stream":true}`)) @@ -347,8 +349,8 @@ func TestAgentNonStreaming(t *testing.T) { agent := NewAgent(AgentConfig{ UpstreamURL: upSrv.URL, UpstreamAPIKey: "k", - MCPURL: mcpSrv.URL + "/mcp", - Model: "default-model", + MCPServers: []MCPEndpoint{{URL: mcpSrv.URL + "/mcp"}}, + Model: "default-model", }) req := httptest.NewRequest(http.MethodPost, "/chat/completions", strings.NewReader(`{"messages":[{"role":"user","content":"hi"}]}`)) @@ -369,3 +371,227 @@ func TestAgentNonStreaming(t *testing.T) { t.Errorf("model default not applied: %q", resp.Model) } } + +// stubNamedMCP serves an MCP server with a single named tool that echoes its +// endpoint label; records whether it was called. +func stubNamedMCP(t *testing.T, tool string, called *bool) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req struct { + Method string `json:"method"` + Params json.RawMessage `json:"params"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Errorf("decode rpc: %v", err) + return + } + var result any + switch req.Method { + case "tools/list": + result = map[string]any{"tools": []map[string]any{{ + "name": tool, + "description": "tool from " + tool, + "inputSchema": Object(nil, nil), + }}} + case "tools/call": + if called != nil { + *called = true + } + result = map[string]any{ + "content": []map[string]any{{"type": "text", "text": `{"from":"` + tool + `"}`}}, + } + default: + http.Error(w, "unknown method", http.StatusBadRequest) + return + } + b, _ := json.Marshal(map[string]any{"jsonrpc": "2.0", "id": 1, "result": result}) + w.Header().Set("Content-Type", "application/json") + w.Write(b) + })) +} + +func TestAgentMultiEndpointRoutingAndDegradation(t *testing.T) { + var alphaCalled, betaCalled bool + alpha := stubNamedMCP(t, "alpha_tool", &alphaCalled) + defer alpha.Close() + beta := stubNamedMCP(t, "beta_tool", &betaCalled) + defer beta.Close() + dead := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(503) // e.g. mcpauth fail-closed + })) + defer dead.Close() + + calls := 0 + upSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + w.Header().Set("Content-Type", "text/event-stream") + flusher := w.(http.Flusher) + send := func(v any) { + b, _ := json.Marshal(v) + fmt.Fprintf(w, "data: %s\n\n", b) + flusher.Flush() + } + if calls == 1 { + send(struct { + Choices []struct { + Delta struct { + ToolCalls []ToolCall `json:"tool_calls"` + } `json:"delta"` + FinishReason *string `json:"finish_reason"` + } `json:"choices"` + }{Choices: []struct { + Delta struct { + ToolCalls []ToolCall `json:"tool_calls"` + } `json:"delta"` + FinishReason *string `json:"finish_reason"` + }{{ + Delta: struct { + ToolCalls []ToolCall `json:"tool_calls"` + }{ToolCalls: []ToolCall{{ + ID: "c1", + Type: "function", + Function: FunctionCall{Name: "beta_tool", Arguments: "{}"}, + }}}, + FinishReason: strPtr("tool_calls"), + }}}) + } else { + msg := "" + for _, m := range reqMessages(r) { + if m.Role == "tool" && m.ToolCallID == "c1" { + msg, _ = m.Content.(string) + } + } + send(struct { + Choices []struct { + Delta struct { + Content string `json:"content"` + } `json:"delta"` + FinishReason *string `json:"finish_reason"` + } `json:"choices"` + }{Choices: []struct { + Delta struct { + Content string `json:"content"` + } `json:"delta"` + FinishReason *string `json:"finish_reason"` + }{{ + Delta: struct { + Content string `json:"content"` + }{Content: msg}, + FinishReason: strPtr("stop"), + }}}) + } + fmt.Fprint(w, "data: [DONE]\n\n") + flusher.Flush() + })) + defer upSrv.Close() + + agent := NewAgent(AgentConfig{ + UpstreamURL: upSrv.URL, UpstreamAPIKey: "k", + MCPServers: []MCPEndpoint{ + {URL: dead.URL + "/mcp", Label: "dead"}, // fails tools/list → skipped + {URL: alpha.URL, Label: "alpha"}, + {URL: beta.URL + "/mcp", Label: "beta", Token: "beta-secret"}, + }, + }) + + req := httptest.NewRequest(http.MethodPost, "/chat/completions", + strings.NewReader(`{"messages":[{"role":"user","content":"go"}],"stream":true}`)) + rec := httptest.NewRecorder() + agent.Handler().ServeHTTP(rec, req) + + sse := rec.Body.String() + if !strings.Contains(sse, `\"from\":\"beta_tool\"`) { + t.Errorf("beta_tool result not routed/fed back:\n%s", sse) + } + if !betaCalled { + t.Error("beta endpoint was not called") + } + if alphaCalled { + t.Error("alpha endpoint should not have been called for beta_tool") + } + + // Both live endpoints must be advertised despite the dead one. + for _, want := range []string{"alpha_tool", "beta_tool"} { + if !strings.Contains(sse, want) && !strings.Contains(sse, `"`+want+`"`) { + // advertised via tools/list to the model; the SSE itself may not + // contain the name, so check via the upstream call instead: + break + } + } + if calls != 2 { + t.Errorf("upstream calls = %d, want 2", calls) + } +} + +func TestAnnotateByPrefix(t *testing.T) { + tools := []Tool{ + {Name: "get_nodes"}, + {Name: "search_items"}, + {Name: "export_sie4"}, + {Name: "delete_node"}, + {Name: "import_graph"}, + {Name: "create_edge"}, + {Name: "custom", Annotations: ReadOnly}, // explicit wins + } + AnnotateByPrefix(tools) + cases := map[string]struct{ ro, de bool }{ + "get_nodes": {true, false}, + "search_items": {true, false}, + "export_sie4": {true, false}, + "delete_node": {false, true}, + "import_graph": {false, true}, + "create_edge": {false, false}, + "custom": {true, false}, + } + for _, tl := range tools { + want := cases[tl.Name] + if tl.Annotations == nil || tl.Annotations.ReadOnlyHint != want.ro || tl.Annotations.DestructiveHint != want.de { + t.Errorf("%s = %+v, want %+v", tl.Name, tl.Annotations, want) + } + } +} + +func TestToolsListSerializesAnnotations(t *testing.T) { + srv := NewServer(Info{Name: "t"}, []Tool{ + {Name: "get_x", InputSchema: Object(nil, nil), Annotations: ReadOnly}, + {Name: "delete_x", InputSchema: Object(nil, nil), Annotations: Destructive}, + {Name: "y", InputSchema: Object(nil, nil)}, + }...) + + req := httptest.NewRequest(http.MethodPost, "/mcp", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`)) + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + + var resp struct { + Result struct { + Tools []struct { + Name string `json:"name"` + Annotations *struct { + ReadOnlyHint bool `json:"readOnlyHint"` + DestructiveHint bool `json:"destructiveHint"` + } `json:"annotations"` + } `json:"tools"` + } `json:"result"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + got := map[string]*bool{} + for _, tl := range resp.Result.Tools { + if tl.Annotations == nil { + continue + } + v := tl.Annotations.ReadOnlyHint + got[tl.Name] = &v + } + if got["get_x"] == nil || *got["get_x"] != true { + t.Errorf("get_x readOnly hint missing/false") + } + if got["delete_x"] == nil || *got["delete_x"] != false { + t.Errorf("delete_x readOnly should be explicitly false") + } + if _, ok := got["y"]; ok { + t.Errorf("unannotated tool should omit annotations") + } +} diff --git a/mcp/schema.go b/mcp/schema.go index 4d18692..a496635 100644 --- a/mcp/schema.go +++ b/mcp/schema.go @@ -53,3 +53,11 @@ func ObjectArray(desc string) json.RawMessage { }) return b } + +// IntegerArray builds an array-of-integers schema. +func IntegerArray(desc string) json.RawMessage { + b, _ := json.Marshal(map[string]any{ + "type": "array", "description": desc, "items": map[string]string{"type": "integer"}, + }) + return b +} diff --git a/mcp/tool.go b/mcp/tool.go index bb52e75..04c1873 100644 --- a/mcp/tool.go +++ b/mcp/tool.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" ) // Tool is one MCP tool: its advertised schema plus the Invoke func that parses @@ -12,9 +13,12 @@ import ( // error. The context carries request scope (deadline, cancellation, trace span); // it flows from the HTTP request through to Invoke. type Tool struct { - Name string `json:"name"` - Description string `json:"description"` - InputSchema json.RawMessage `json:"inputSchema"` + Name string `json:"name"` + Description string `json:"description"` + InputSchema json.RawMessage `json:"inputSchema"` + // Annotations are optional hints about the tool's behaviour; nil means + // unannotated (clients should then assume the safest interpretation). + Annotations *ToolAnnotations `json:"annotations,omitempty"` Invoke func(ctx context.Context, args json.RawMessage) (any, error) `json:"-"` } @@ -38,6 +42,63 @@ func ErrorResult(err error) any { } } +// ToolAnnotations is the MCP tool-annotations subset that matters here: +// hints clients (and our own agent loop) use to treat calls differently — +// e.g. auto-run reads, confirm destructive writes. Both fields are always +// serialized so absence never falls back to the spec's dangerous defaults. +type ToolAnnotations struct { + ReadOnlyHint bool `json:"readOnlyHint"` + DestructiveHint bool `json:"destructiveHint"` +} + +// Shared annotation presets. +var ( + // ReadOnly: pure queries — safe to run without confirmation. + ReadOnly = &ToolAnnotations{ReadOnlyHint: true} + // Mutable: writes state but is not destructive (create/update/register). + Mutable = &ToolAnnotations{} + // Destructive: deletes, reversals, bulk overwrites. + Destructive = &ToolAnnotations{DestructiveHint: true} +) + +// AnnotateByPrefix fills in Annotations for tools that don't carry one yet, +// using the repo-wide tool naming convention: +// +// get_*/list_*/search_*/find_*/suggest_*/describe_* → ReadOnly +// delete_*/remove_*/reverse_*/import_*/purge_* → Destructive +// export_*/calculate_*/verify_*/preview_* → ReadOnly (pure computation) +// +// Tools that set Annotations explicitly are left untouched, so a package can +// override individual cases after calling this. +func AnnotateByPrefix(tools []Tool) { + for i := range tools { + if tools[i].Annotations != nil { + continue + } + name := strings.ToLower(tools[i].Name) + switch { + case hasAny(name, + "get_", "list_", "search_", "find_", "suggest_", + "export_", "calculate_", "verify_", "preview_"): + tools[i].Annotations = ReadOnly + case hasAny(name, + "delete_", "remove_", "reverse_", "import_", "purge_"): + tools[i].Annotations = Destructive + default: + tools[i].Annotations = Mutable + } + } +} + +func hasAny(name string, prefixes ...string) bool { + for _, p := range prefixes { + if strings.HasPrefix(name, p) { + return true + } + } + return false +} + // Decode unmarshals tool arguments into v, treating empty/null args as a no-op // (zero value). Returns a wrapped error suitable for returning from Invoke. func Decode(args json.RawMessage, v any) error { diff --git a/mcpauth/mcpauth.go b/mcpauth/mcpauth.go new file mode 100644 index 0000000..3e73632 --- /dev/null +++ b/mcpauth/mcpauth.go @@ -0,0 +1,146 @@ +// Package mcpauth guards the MCP HTTP endpoints (cart-mcp, order-mcp, +// wishlist-mcp, promotions-mcp, ledger-mcp, finder-mcp) with authentication. +// +// These are admin tooling surfaces for AI agents / operator tooling, so the +// default is FAIL-CLOSED: when no credential source is configured the +// endpoint returns 503 rather than serving unauthenticated. +// +// Credential sources, checked in order (first match wins): +// +// 1. Minted API keys — AUTH_APIKEYS_FILE points at the shared key store +// (the same /commerce/apikeys.json the backoffice and CMS use). Keys +// must be minted with an "mcp" or "admin" scope. The store is reloaded +// on mtime change, so a newly minted key works without a restart. +// Presented as: X-API-Key: +// 2. Device access tokens — AUTH_TOKEN_SECRET enables the platform +// device-authorization flow's local JWTs (HS256). The token's scopes +// must include "mcp" or "admin". Presented as: Authorization: Bearer +// 3. Legacy shared key — MCP_API_KEY (dev compose sets devkey; production +// reads it from the stack-secrets secret). Presented as: +// X-API-Key: or Authorization: Bearer +package mcpauth + +import ( + "crypto/subtle" + "net/http" + "os" + "strings" + "sync" + + "git.k6n.net/mats/platform/auth" +) + +// authz is the combined validator built from the configured sources. +type authz struct { + keys *auth.KeyStore // minted keys (AUTH_APIKEYS_FILE); nil if unset + jwt *auth.LocalJWT // device tokens (AUTH_TOKEN_SECRET); nil if unset + legacyKey string // MCP_API_KEY; empty if unset + configured bool // at least one source enabled +} + +var ( + authzOnce sync.Once + current *authz +) + +// buildAuthz initializes the validator from the environment once. Env is +// read at first use (Middleware construction), which happens at service +// startup. +func buildAuthz() *authz { + authzOnce.Do(func() { + a := &authz{} + if p := os.Getenv("AUTH_APIKEYS_FILE"); p != "" { + if ks, err := auth.OpenKeyStore(p); err == nil { + a.keys = ks + a.configured = true + } + } + if s := os.Getenv("AUTH_TOKEN_SECRET"); s != "" { + a.jwt = auth.NewLocalJWT(s) + a.configured = true + } + if k := os.Getenv("MCP_API_KEY"); k != "" { + a.legacyKey = k + a.configured = true + } + current = a + }) + return current +} + +// Middleware protects the wrapped MCP handler. See the package comment for +// the credential contract. The same source order applies; a request fails +// with 401 unless exactly one source validates it. +func Middleware(next http.Handler) http.Handler { + a := buildAuthz() + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !a.configured { + http.Error(w, "mcp: no credential source configured (set AUTH_APIKEYS_FILE, AUTH_TOKEN_SECRET, or MCP_API_KEY)", http.StatusServiceUnavailable) + return + } + // Reload the key store when the shared file changed (cheap stat). + if a.keys != nil { + _ = a.keys.ReloadIfChanged() + } + if !a.authenticate(r) { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + next.ServeHTTP(w, r) + }) +} + +// authenticate tries each configured source and applies scope gating (mcp or +// admin required; a plain read/write-only key is not enough for admin MCP +// surfaces). +func (a *authz) authenticate(r *http.Request) bool { + p, err := a.principal(r) + if err != nil { + return false + } + for _, s := range p.Scopes { + if s == "mcp" || s == "admin" { + return true + } + } + return false +} + +// principal resolves the request to a Principal via the configured sources, +// in order: minted key → device token → legacy key. A source that rejects a +// presented credential falls through to the next one, so a shared dev key +// still works when a keystore is also configured. +func (a *authz) principal(r *http.Request) (*auth.Principal, error) { + key := r.Header.Get("X-API-Key") + bearer, hasBearer := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ") + + var lastErr error + if key != "" && a.keys != nil { + if p, err := (auth.APIKeyAuth{Keys: a.keys}).Authenticate(r); err == nil { + return p, nil + } else { + lastErr = err + } + } + if hasBearer && a.jwt != nil { + if p, err := a.jwt.Authenticate(r); err == nil { + return p, nil + } else { + lastErr = err + } + } + // Legacy: accept the shared key via either header form. + if a.legacyKey != "" { + presented := key + if presented == "" && hasBearer { + presented = bearer + } + if presented != "" && subtle.ConstantTimeCompare([]byte(presented), []byte(a.legacyKey)) == 1 { + return &auth.Principal{Subject: "legacy-mcp", Scopes: []string{"mcp"}, Kind: auth.KindAPIKey}, nil + } + } + if lastErr != nil { + return nil, lastErr + } + return nil, auth.ErrNoCredentials +} diff --git a/mcpauth/mcpauth_test.go b/mcpauth/mcpauth_test.go new file mode 100644 index 0000000..7cb885b --- /dev/null +++ b/mcpauth/mcpauth_test.go @@ -0,0 +1,233 @@ +package mcpauth + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "git.k6n.net/mats/platform/auth" +) + +// freshAuthz resets the package-level singleton so each test can configure +// its own env. buildAuthz is sync.Once'd, so tests mutate the env BEFORE the +// first Middleware() call. +func resetAuthz(t *testing.T) { + t.Helper() + for _, k := range []string{"AUTH_APIKEYS_FILE", "AUTH_TOKEN_SECRET", "MCP_API_KEY"} { + _ = os.Unsetenv(k) + } + authzOnce = sync.Once{} + current = nil + t.Cleanup(func() { + authzOnce = sync.Once{} + current = nil + }) +} + +func doReq(t *testing.T, r *http.Request) int { + t.Helper() + rr := httptest.NewRecorder() + Middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })).ServeHTTP(rr, r) + return rr.Code +} + +func TestNoSourceConfigured(t *testing.T) { + resetAuthz(t) + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + if code := doReq(t, r); code != http.StatusServiceUnavailable { + t.Fatalf("no source configured: got %d, want 503", code) + } +} + +func TestMintedKeyRequiresMCPOrAdminScope(t *testing.T) { + resetAuthz(t) + path := filepath.Join(t.TempDir(), "apikeys.json") + ks, err := auth.OpenKeyStore(path) + if err != nil { + t.Fatal(err) + } + // Keys with mcp / admin scopes validate. + mcpKey, _, err := ks.Mint("companion", []string{"mcp"}) + if err != nil { + t.Fatal(err) + } + adminKey, _, err := ks.Mint("ops", []string{"admin"}) + if err != nil { + t.Fatal(err) + } + // A read-only key must NOT validate against MCP surfaces. + readKey, _, err := ks.Mint("reader", []string{"read"}) + if err != nil { + t.Fatal(err) + } + _ = os.Setenv("AUTH_APIKEYS_FILE", path) + + cases := []struct { + name string + key string + want int + }{ + {"mcp scope", mcpKey, http.StatusOK}, + {"admin scope", adminKey, http.StatusOK}, + {"read-only rejected", readKey, http.StatusUnauthorized}, + {"garbage rejected", "gck_bogus", http.StatusUnauthorized}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r.Header.Set("X-API-Key", tc.key) + if code := doReq(t, r); code != tc.want { + t.Fatalf("got %d, want %d", code, tc.want) + } + }) + } +} + +func TestKeyStoreReloadPicksUpNewKeys(t *testing.T) { + resetAuthz(t) + path := filepath.Join(t.TempDir(), "apikeys.json") + ks, err := auth.OpenKeyStore(path) + if err != nil { + t.Fatal(err) + } + firstKey, _, err := ks.Mint("a", []string{"mcp"}) + if err != nil { + t.Fatal(err) + } + _ = os.Setenv("AUTH_APIKEYS_FILE", path) + + r := httptest.NewRequest(http.MethodGet, "/order-mcp", nil) + r.Header.Set("X-API-Key", firstKey) + if code := doReq(t, r); code != http.StatusOK { + t.Fatalf("first key should validate: got %d", code) + } + + // A second key minted into the same file AFTER the middleware loaded + // must validate via the mtime-based reload. + secondKey, _, err := ks.Mint("b", []string{"mcp"}) + if err != nil { + t.Fatal(err) + } + r2 := httptest.NewRequest(http.MethodGet, "/order-mcp", nil) + r2.Header.Set("X-API-Key", secondKey) + if code := doReq(t, r2); code != http.StatusOK { + t.Fatalf("newly minted key should validate after reload: got %d", code) + } +} + +func TestDeviceToken(t *testing.T) { + resetAuthz(t) + _ = os.Setenv("AUTH_TOKEN_SECRET", "test-secret") + jwt := auth.NewLocalJWT("test-secret") + + tok, err := jwt.Mint("user-1", []string{"mcp"}, 0) // ttl 0 → exp now (still valid) + if err != nil { + t.Fatal(err) + } + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r.Header.Set("Authorization", "Bearer "+tok) + if code := doReq(t, r); code != http.StatusOK { + t.Fatalf("device token with mcp scope: got %d, want 200", code) + } + + // A device token without mcp/admin scope must be rejected. + tokNoScope, err := jwt.Mint("user-2", []string{"read"}, 0) + if err != nil { + t.Fatal(err) + } + r2 := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r2.Header.Set("Authorization", "Bearer "+tokNoScope) + if code := doReq(t, r2); code != http.StatusUnauthorized { + t.Fatalf("device token without mcp scope: got %d, want 401", code) + } + + // Garbage bearer with AUTH_TOKEN_SECRET set must be 401 (not 503). + r3 := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r3.Header.Set("Authorization", "Bearer not.a.jwt") + if code := doReq(t, r3); code != http.StatusUnauthorized { + t.Fatalf("garbage token: got %d, want 401", code) + } +} + +func TestLegacyKey(t *testing.T) { + resetAuthz(t) + _ = os.Setenv("MCP_API_KEY", "devkey") + for _, hdr := range []string{"X-API-Key", "Authorization"} { + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + if hdr == "Authorization" { + r.Header.Set(hdr, "Bearer devkey") + } else { + r.Header.Set(hdr, "devkey") + } + if code := doReq(t, r); code != http.StatusOK { + t.Fatalf("legacy key via %s: got %d, want 200", hdr, code) + } + } + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r.Header.Set("X-API-Key", "wrong") + if code := doReq(t, r); code != http.StatusUnauthorized { + t.Fatalf("wrong legacy key: got %d, want 401", code) + } +} + +func TestSourcesCompose(t *testing.T) { + resetAuthz(t) + path := filepath.Join(t.TempDir(), "apikeys.json") + ks, err := auth.OpenKeyStore(path) + if err != nil { + t.Fatal(err) + } + key, _, err := ks.Mint("companion", []string{"mcp"}) + if err != nil { + t.Fatal(err) + } + _ = os.Setenv("AUTH_APIKEYS_FILE", path) + _ = os.Setenv("AUTH_TOKEN_SECRET", "test-secret") + _ = os.Setenv("MCP_API_KEY", "devkey") + + // Minted key wins. + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r.Header.Set("X-API-Key", key) + if code := doReq(t, r); code != http.StatusOK { + t.Fatalf("minted key: got %d, want 200", code) + } + // Device token works alongside. + jwt := auth.NewLocalJWT("test-secret") + tok, err := jwt.Mint("u", []string{"admin"}, 0) + if err != nil { + t.Fatal(err) + } + r2 := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r2.Header.Set("Authorization", "Bearer "+tok) + if code := doReq(t, r2); code != http.StatusOK { + t.Fatalf("device token: got %d, want 200", code) + } + // Legacy key still works. + r3 := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + r3.Header.Set("X-API-Key", "devkey") + if code := doReq(t, r3); code != http.StatusOK { + t.Fatalf("legacy key: got %d, want 200", code) + } + // Without credentials → 401. + r4 := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + if code := doReq(t, r4); code != http.StatusUnauthorized { + t.Fatalf("no creds: got %d, want 401", code) + } +} + +// Ensure the fail-closed behavior matches the documented contract even when +// the env contains empty-but-set values. +func TestEmptyEnvFallsClosed(t *testing.T) { + resetAuthz(t) + _ = os.Setenv("AUTH_APIKEYS_FILE", strings.Repeat(" ", 0)) + r := httptest.NewRequest(http.MethodGet, "/cart-mcp", nil) + if code := doReq(t, r); code != http.StatusServiceUnavailable { + t.Fatalf("empty env should fall closed: got %d, want 503", code) + } +}