diff --git a/cmd/pad/chatgpt_catalog_test.go b/cmd/pad/chatgpt_catalog_test.go index 6fc8d2add..5b365316f 100644 --- a/cmd/pad/chatgpt_catalog_test.go +++ b/cmd/pad/chatgpt_catalog_test.go @@ -29,7 +29,7 @@ func TestChatGPTWireGolden(t *testing.T) { if err != nil { t.Fatal(err) } - ts := httptest.NewServer(mcpserver.NewRemoteTransport(srv.MCP(), &padMCPGenerateOnlySessionIDManager{})) + ts := httptest.NewServer(newChatGPTTransport(srv)) defer ts.Close() post := func(body, session string) ([]byte, string) { t.Helper() diff --git a/cmd/pad/chatgpt_scope_test.go b/cmd/pad/chatgpt_scope_test.go new file mode 100644 index 000000000..307dffc93 --- /dev/null +++ b/cmd/pad/chatgpt_scope_test.go @@ -0,0 +1,207 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/PerpetualSoftware/pad/internal/cmdhelp" + mcpserver "github.com/PerpetualSoftware/pad/internal/mcp" + padserver "github.com/PerpetualSoftware/pad/internal/server" +) + +// TASK-3321 U2c: a ChatGPT write tool called with a read-only grant answers +// the insufficient_scope challenge that makes ChatGPT ask for the wider +// grant, and nothing is dispatched. Driven through the production +// transport, behind the mount's resource stamp, so the request context the +// challenge is built from is the one a real call carries. + +const scopeTestResource = "https://mcp.example.test/mcp/chatgpt" + +type chatGPTScopeHarness struct { + t *testing.T + ts *httptest.Server + rec *recordingDispatcher +} + +func newChatGPTScopeHarness(t *testing.T) *chatGPTScopeHarness { + t.Helper() + root := newRootCmd() + doc := cmdhelp.Build(root, root, cmdhelp.Options{Binary: "pad", Version: fullVersion(), Homepage: padHomepage, MaxDepth: -1}) + rec := &recordingDispatcher{} + gpt, err := newChatGPTMCPServer(doc, rec) + if err != nil { + t.Fatal(err) + } + transport := newChatGPTTransport(gpt) + // What MCPBearerAuth stashes: the token's scopes, as a JSON array. + withScopes := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := padserver.WithTokenScopes(r.Context(), r.Header.Get("X-Test-Scopes")) + transport.ServeHTTP(w, r.WithContext(ctx)) + }) + ts := httptest.NewServer(padserver.WithMCPResource(scopeTestResource)(withScopes)) + t.Cleanup(ts.Close) + return &chatGPTScopeHarness{t: t, ts: ts, rec: rec} +} + +func (h *chatGPTScopeHarness) post(body, session, scopes string) ([]byte, string) { + h.t.Helper() + req, _ := http.NewRequest(http.MethodPost, h.ts.URL, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + req.Header.Set("X-Test-Scopes", scopes) + if session != "" { + req.Header.Set("Mcp-Session-Id", session) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + h.t.Fatal(err) + } + defer resp.Body.Close() + raw, _ := io.ReadAll(resp.Body) + return raw, resp.Header.Get("Mcp-Session-Id") +} + +type scopeCallResult struct { + IsError bool `json:"isError"` + Meta map[string]any `json:"_meta"` +} + +// call runs one tools/call with the given token scopes. +func (h *chatGPTScopeHarness) call(tool string, args map[string]any, scopes string) scopeCallResult { + h.t.Helper() + _, session := h.post(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":`+goldenClientInfo+`}`, "", scopes) + h.post(`{"jsonrpc":"2.0","method":"notifications/initialized"}`, session, scopes) + body, _ := json.Marshal(map[string]any{ + "jsonrpc": "2.0", "id": 2, "method": "tools/call", + "params": map[string]any{"name": tool, "arguments": args}, + }) + raw, _ := h.post(string(body), session, scopes) + var env struct { + Result scopeCallResult `json:"result"` + } + if err := json.Unmarshal(raw, &env); err != nil { + h.t.Fatalf("%s: undecodable response %s", tool, raw) + } + return env.Result +} + +func scopeTestArgs(e mcpserver.ChatGPTTool) map[string]any { + sample := map[string]any{ + "workspace": "ws", "ref": "TASK-1", "query": "login", "collection": "tasks", + "title": "A title", "message": "A comment", "target": "TASK-2", "content": "Body", + "status": "open", "limit": float64(5), "link_type": "blocks", + } + args := map[string]any{} + for _, p := range e.Params { + if v, ok := sample[p]; ok { + args[p] = v + } + } + return args +} + +func challengeOf(r scopeCallResult) string { + list, _ := r.Meta["mcp/www_authenticate"].([]any) + if len(list) != 1 { + return "" + } + s, _ := list[0].(string) + return s +} + +func TestChatGPTScope_ReadGrantOnAWriteToolIsChallenged(t *testing.T) { + h := newChatGPTScopeHarness(t) + const wantMeta = `resource_metadata="https://mcp.example.test/.well-known/oauth-protected-resource/mcp/chatgpt"` + writes := 0 + for _, e := range mcpserver.ChatGPTCatalog { + if e.Hints.ReadOnly { + continue + } + writes++ + // `null` is what an OAuth token with no granted scope is stashed + // as (oauthScopesToJSON); the legacy unrestricted PAT forms never + // reach this mount, which refuses PATs. + for _, scopes := range []string{`["pad:read"]`, `null`} { + res := h.call(e.Name, scopeTestArgs(e), scopes) + c := challengeOf(res) + if !res.IsError || !strings.HasPrefix(c, "Bearer ") || !strings.Contains(c, wantMeta) || + !strings.Contains(c, `error="insufficient_scope"`) || !strings.Contains(c, `error_description="`) { + t.Errorf("%s with %s: isError=%v challenge=%q", e.Name, scopes, res.IsError, c) + } + if calls := h.rec.take(); len(calls) != 0 { + t.Errorf("%s with %s dispatched %d calls", e.Name, scopes, len(calls)) + } + } + } + if writes == 0 { + t.Fatal("the catalog has no write tools; the leg proves nothing") + } +} + +// The challenge fires only on a missing scope: a read tool with a read +// grant, and a write tool with a write grant, reach the dispatcher. +func TestChatGPTScope_SufficientGrantsAreNotChallenged(t *testing.T) { + h := newChatGPTScopeHarness(t) + for _, e := range mcpserver.ChatGPTCatalog { + scopes := `["pad:read"]` + if !e.Hints.ReadOnly { + scopes = `["pad:write"]` + } + res := h.call(e.Name, scopeTestArgs(e), scopes) + if c := challengeOf(res); c != "" { + t.Errorf("%s with %s was challenged: %q", e.Name, scopes, c) + } + if calls := h.rec.take(); len(calls) == 0 { + t.Errorf("%s with %s dispatched nothing", e.Name, scopes) + } + } +} + +// tools/list asked for the way ChatGPT asks (Accept: application/json, +// text/event-stream) answers a JSON body whose every tool carries +// securitySchemes at the top level. Streamable HTTP can answer with an +// event stream instead, but only by flushing notifications mid-request, +// and WithChatGPTToolSchemes hands the transport a writer that cannot +// flush (TestBufferedResponseCannotStream), so the answer is JSON. +func TestChatGPTToolsList_JSONWithTopLevelSchemes(t *testing.T) { + h := newChatGPTScopeHarness(t) + _, session := h.post(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":`+goldenClientInfo+`}`, "", `["pad:read"]`) + h.post(`{"jsonrpc":"2.0","method":"notifications/initialized"}`, session, `["pad:read"]`) + req, _ := http.NewRequest(http.MethodPost, h.ts.URL, strings.NewReader(`{"jsonrpc":"2.0","id":2,"method":"tools/list"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + req.Header.Set("Mcp-Session-Id", session) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + raw, _ := io.ReadAll(resp.Body) + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { + t.Fatalf("tools/list answered %q, want a JSON body: %s", ct, raw) + } + var env struct { + Result struct { + Tools []map[string]json.RawMessage `json:"tools"` + } `json:"result"` + } + if err := json.Unmarshal(raw, &env); err != nil { + t.Fatalf("undecodable tools/list: %s", raw) + } + if len(env.Result.Tools) != len(mcpserver.ChatGPTCatalog) { + t.Fatalf("tools/list listed %d tools, want %d", len(env.Result.Tools), len(mcpserver.ChatGPTCatalog)) + } + for _, tool := range env.Result.Tools { + var meta struct { + SecuritySchemes json.RawMessage `json:"securitySchemes"` + } + _ = json.Unmarshal(tool["_meta"], &meta) + if len(tool["securitySchemes"]) == 0 || string(tool["securitySchemes"]) != string(meta.SecuritySchemes) { + t.Errorf("%s: top-level securitySchemes %s, _meta copy %s", tool["name"], tool["securitySchemes"], meta.SecuritySchemes) + } + } +} diff --git a/cmd/pad/cmd_server_chatgpt.go b/cmd/pad/cmd_server_chatgpt.go index c90de889d..996df4b11 100644 --- a/cmd/pad/cmd_server_chatgpt.go +++ b/cmd/pad/cmd_server_chatgpt.go @@ -3,6 +3,7 @@ package main import ( "fmt" "log/slog" + "net/http" "os" "strings" @@ -63,7 +64,13 @@ func wireChatGPTMCP(srv *server.Server, doc *cmdhelp.Document, dispatcher mcpser slog.Error("chatgpt mcp: building the catalog failed; /mcp/chatgpt is not served", "error", err) return } - transport := mcpserver.NewRemoteTransport(gpt.MCP(), &padMCPGenerateOnlySessionIDManager{}) - srv.SetChatGPTMCPTransport(transport, ep.ChatGPTResourceURL, enabled, gpt.IsKnownCallName) + srv.SetChatGPTMCPTransport(newChatGPTTransport(gpt), ep.ChatGPTResourceURL, enabled, gpt.IsKnownCallName) slog.Info("ChatGPT MCP transport constructed", "url", ep.ChatGPTResourceURL, "enabled", enabled) } + +// newChatGPTTransport is the HTTP handler /mcp/chatgpt serves: the remote +// transport, with each tool's securitySchemes lifted to the top level of +// tools/list (TASK-3321 U2c). The wire golden test builds the same one. +func newChatGPTTransport(gpt *mcpserver.Server) http.Handler { + return mcpserver.WithChatGPTToolSchemes(mcpserver.NewRemoteTransport(gpt.MCP(), &padMCPGenerateOnlySessionIDManager{})) +} diff --git a/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json b/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json index 750353269..8a58b6009 100644 --- a/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json +++ b/cmd/pad/testdata/mcp_wire/chatgpt_tools_list.json @@ -1,6 +1,16 @@ { "tools": [ { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": false, @@ -34,9 +44,27 @@ ], "type": "object" }, - "name": "add_comment" + "name": "add_comment", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": true, @@ -61,9 +89,27 @@ ], "type": "object" }, - "name": "archive_item" + "name": "archive_item", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": false, @@ -125,9 +171,27 @@ ], "type": "object" }, - "name": "create_item" + "name": "create_item", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -152,9 +216,27 @@ ], "type": "object" }, - "name": "get_item" + "name": "get_item", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -179,9 +261,27 @@ ], "type": "object" }, - "name": "get_playbook" + "name": "get_playbook", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -201,9 +301,27 @@ ], "type": "object" }, - "name": "get_workspace_overview" + "name": "get_workspace_overview", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -228,9 +346,27 @@ ], "type": "object" }, - "name": "item_dependencies" + "name": "item_dependencies", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -259,9 +395,27 @@ ], "type": "object" }, - "name": "item_history" + "name": "item_history", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": false, @@ -303,9 +457,27 @@ ], "type": "object" }, - "name": "link_items" + "name": "link_items", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -325,9 +497,27 @@ ], "type": "object" }, - "name": "list_collections" + "name": "list_collections", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -352,9 +542,27 @@ ], "type": "object" }, - "name": "list_comments" + "name": "list_comments", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -414,9 +622,27 @@ ], "type": "object" }, - "name": "list_items" + "name": "list_items", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -436,9 +662,27 @@ ], "type": "object" }, - "name": "list_playbooks" + "name": "list_playbooks", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -451,9 +695,27 @@ "required": [], "type": "object" }, - "name": "list_workspaces" + "name": "list_workspaces", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -473,9 +735,27 @@ ], "type": "object" }, - "name": "project_dashboard" + "name": "project_dashboard", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -495,9 +775,27 @@ ], "type": "object" }, - "name": "ready_items" + "name": "ready_items", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -529,9 +827,27 @@ ], "type": "object" }, - "name": "recent_activity" + "name": "recent_activity", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": false, @@ -556,9 +872,27 @@ ], "type": "object" }, - "name": "restore_item" + "name": "restore_item", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -607,9 +941,27 @@ ], "type": "object" }, - "name": "search" + "name": "search", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": false, "destructiveHint": false, @@ -678,9 +1030,27 @@ ], "type": "object" }, - "name": "update_item" + "name": "update_item", + "securitySchemes": [ + { + "scopes": [ + "pad:write" + ], + "type": "oauth2" + } + ] }, { + "_meta": { + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] + }, "annotations": { "readOnlyHint": true, "destructiveHint": false, @@ -700,7 +1070,15 @@ ], "type": "object" }, - "name": "what_next" + "name": "what_next", + "securitySchemes": [ + { + "scopes": [ + "pad:read" + ], + "type": "oauth2" + } + ] } ] } diff --git a/internal/mcp/chatgpt_catalog.go b/internal/mcp/chatgpt_catalog.go index fe85649b5..e86722ea1 100644 --- a/internal/mcp/chatgpt_catalog.go +++ b/internal/mcp/chatgpt_catalog.go @@ -453,6 +453,9 @@ func buildChatGPTTool(t ChatGPTTool, params []ParamDef) mcp.Tool { opts = append(opts, paramDefToToolOption(p)) } tool := mcp.NewTool(t.Name, opts...) + // TASK-3321 U2c: the scope this tool needs. tools/list also carries it + // at the top level (WithChatGPTToolSchemes); this is the _meta mirror. + tool.Meta = mcp.NewMetaFromMap(map[string]any{"securitySchemes": chatGPTSecuritySchemes(t)}) if len(t.Required) > 0 { tool.InputSchema.Required = append([]string(nil), t.Required...) sort.Strings(tool.InputSchema.Required) @@ -469,6 +472,11 @@ func chatGPTHandler(t ChatGPTTool, source server.ToolHandlerFunc) server.ToolHan declared[p] = true } return func(ctx context.Context, req mcp.CallToolRequest) (*CallToolResult, error) { + // TASK-3321 U2c: a write with a read-only grant answers the + // challenge that lets ChatGPT ask for the wider one. + if !chatGPTScopeAllowed(ctx, t) { + return chatGPTScopeDenied(ctx), nil + } in := req.GetArguments() var undeclared []string for k := range in { diff --git a/internal/mcp/chatgpt_security.go b/internal/mcp/chatgpt_security.go new file mode 100644 index 000000000..2d620c454 --- /dev/null +++ b/internal/mcp/chatgpt_security.go @@ -0,0 +1,191 @@ +package mcp + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "strconv" + "strings" + + "github.com/mark3labs/mcp-go/mcp" + + padserver "github.com/PerpetualSoftware/pad/internal/server" +) + +// TASK-3321 U2c: what OpenAI's Apps SDK reads to drive sign-in. +// +// Every ChatGPT tool declares the OAuth scope it needs as `securitySchemes` +// (developers.openai.com/apps-sdk/build/auth). A read tool needs pad:read; +// anything else needs pad:write. When the connection's token lacks the +// scope, the tool answers an error result carrying +// _meta["mcp/www_authenticate"], a WWW-Authenticate challenge naming the +// mount's protected-resource document, which is what makes ChatGPT offer +// to reconnect with the wider grant. + +const ( + chatGPTScopeRead = "pad:read" + chatGPTScopeWrite = "pad:write" +) + +// chatGPTScope is the one scope tool t needs. +func chatGPTScope(t ChatGPTTool) string { + if t.Hints.ReadOnly { + return chatGPTScopeRead + } + return chatGPTScopeWrite +} + +// chatGPTSecuritySchemes is tool t's securitySchemes value: OAuth only, +// since no Pad tool is callable anonymously. +func chatGPTSecuritySchemes(t ChatGPTTool) []any { + return []any{map[string]any{"type": "oauth2", "scopes": []string{chatGPTScope(t)}}} +} + +// chatGPTScopeAllowed reports whether the token behind ctx carries the +// scope tool t needs. A read tool only issues reads, and every token the +// mount accepts grants those, so only a write tool is checked: with the +// policy every mutating in-process request is held to. +func chatGPTScopeAllowed(ctx context.Context, t ChatGPTTool) bool { + if t.Hints.ReadOnly { + return true + } + return padserver.TokenScopeAllows(padserver.TokenScopesFromContext(ctx), http.MethodPost, "") +} + +// chatGPTScopeDenied is the answer to a write tool called with a read-only +// grant. With no metadata document to name (a request that did not arrive +// on a mount), it is a plain refusal: a challenge without one cannot start +// a sign-in. +func chatGPTScopeDenied(ctx context.Context) *CallToolResult { + const description = "Pad needs permission to make changes. Reconnect Pad and allow it to edit your workspaces." + res := NewErrorResult(ErrorPayload{Code: ErrPermissionDenied, Message: description}) + meta := padserver.MCPResourceMetadataURLFromContext(ctx) + if meta == "" { + return res + } + challenge := `Bearer resource_metadata="` + meta + `", error="insufficient_scope", scope="` + + chatGPTScopeRead + " " + chatGPTScopeWrite + `", error_description="` + description + `"` + res.Meta = mcp.NewMetaFromMap(map[string]any{"mcp/www_authenticate": []string{challenge}}) + return res +} + +// chatGPTListBodyLimit bounds how much of a request body +// WithChatGPTToolSchemes reads to decide whether it is tools/list. A +// tools/list request is a few hundred bytes; a longer body's prefix is +// not a whole JSON value, so it is passed on untouched. +const chatGPTListBodyLimit = 64 << 10 + +// WithChatGPTToolSchemes copies each tool's _meta.securitySchemes to the +// tool's top level in a tools/list response. The Apps SDK reads the top +// level and keeps _meta as a mirror for older clients, but mcp-go's Tool +// marshals a fixed set of keys, so the top-level copy is added here. Only +// a single (non-batch) tools/list request's JSON response is rewritten; +// every other request and response passes through byte for byte. +func WithChatGPTToolSchemes(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.Body == nil { + next.ServeHTTP(w, r) + return + } + head, err := io.ReadAll(io.LimitReader(r.Body, chatGPTListBodyLimit+1)) + r.Body = readCloser{io.MultiReader(bytes.NewReader(head), r.Body), r.Body} + if err != nil || !isToolsList(head) { + next.ServeHTTP(w, r) + return + } + rec := &bufferedResponse{header: http.Header{}, status: http.StatusOK} + next.ServeHTTP(rec, r) + body := rec.body.Bytes() + if rec.status == http.StatusOK && strings.HasPrefix(rec.header.Get("Content-Type"), "application/json") { + if out, ok := liftSecuritySchemes(body); ok { + body = out + rec.header.Set("Content-Length", strconv.Itoa(len(body))) + } + } + for k, v := range rec.header { + w.Header()[k] = v + } + w.WriteHeader(rec.status) + _, _ = w.Write(body) + }) +} + +type readCloser struct { + io.Reader + io.Closer +} + +// isToolsList reports whether body is a single JSON-RPC tools/list request. +func isToolsList(body []byte) bool { + var req struct { + Method string `json:"method"` + } + return json.Unmarshal(body, &req) == nil && req.Method == "tools/list" +} + +// liftSecuritySchemes rewrites a tools/list JSON-RPC response so each tool +// with _meta.securitySchemes also carries it at the top level. Every other +// value keeps its bytes. ok is false when the body is not that shape. +func liftSecuritySchemes(body []byte) ([]byte, bool) { + var env map[string]json.RawMessage + if json.Unmarshal(body, &env) != nil || env["result"] == nil { + return nil, false + } + var result map[string]json.RawMessage + if json.Unmarshal(env["result"], &result) != nil || result["tools"] == nil { + return nil, false + } + var tools []map[string]json.RawMessage + if json.Unmarshal(result["tools"], &tools) != nil { + return nil, false + } + for _, tool := range tools { + var meta map[string]json.RawMessage + if json.Unmarshal(tool["_meta"], &meta) != nil || meta["securitySchemes"] == nil { + continue + } + tool["securitySchemes"] = meta["securitySchemes"] + } + var err error + if result["tools"], err = json.Marshal(tools); err != nil { + return nil, false + } + if env["result"], err = json.Marshal(result); err != nil { + return nil, false + } + out, err := json.Marshal(env) + if err != nil { + return nil, false + } + if bytes.HasSuffix(body, []byte("\n")) { + out = append(out, '\n') + } + return out, true +} + +// bufferedResponse holds a response so it can be rewritten. It must not +// implement http.Flusher: mcp-go upgrades a response to an event stream +// only through a writer that can flush (and the remote transport has no +// event store), so without one tools/list is always a JSON body +// (TestBufferedResponseCannotStream). +type bufferedResponse struct { + header http.Header + status int + wrote bool + body bytes.Buffer +} + +func (b *bufferedResponse) Header() http.Header { return b.header } + +func (b *bufferedResponse) WriteHeader(status int) { + if !b.wrote { + b.status, b.wrote = status, true + } +} + +func (b *bufferedResponse) Write(p []byte) (int, error) { + b.wrote = true + return b.body.Write(p) +} diff --git a/internal/mcp/chatgpt_security_test.go b/internal/mcp/chatgpt_security_test.go new file mode 100644 index 000000000..3ea47a7e6 --- /dev/null +++ b/internal/mcp/chatgpt_security_test.go @@ -0,0 +1,81 @@ +package mcp + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TASK-3321 U2c: WithChatGPTToolSchemes rewrites exactly one thing, a +// tools/list JSON response, and hands everything else through byte for +// byte, request body included. + +const listWithMeta = `{"jsonrpc":"2.0","id":2,"result":{"tools":[{"name":"a","_meta":{"securitySchemes":[{"type":"oauth2","scopes":["pad:read"]}]}},{"name":"b"}]}}` + "\n" + +// stub answers with fixed bytes and records the request body it read. +func stubTransport(contentType string, status int, answer string, seen *[]byte) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *seen, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", contentType) + w.WriteHeader(status) + _, _ = io.WriteString(w, answer) + }) +} + +func serveSchemes(h http.Handler, body string) *httptest.ResponseRecorder { + rr := httptest.NewRecorder() + h.ServeHTTP(rr, httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))) + return rr +} + +func TestWithChatGPTToolSchemes_LiftsOnToolsList(t *testing.T) { + var seen []byte + req := `{"jsonrpc":"2.0","id":2,"method":"tools/list"}` + rr := serveSchemes(WithChatGPTToolSchemes(stubTransport("application/json", 200, listWithMeta, &seen)), req) + if string(seen) != req { + t.Fatalf("the transport read %q, want the request body intact", seen) + } + want := `{"id":2,"jsonrpc":"2.0","result":{"tools":[{"_meta":{"securitySchemes":[{"type":"oauth2","scopes":["pad:read"]}]},"name":"a","securitySchemes":[{"type":"oauth2","scopes":["pad:read"]}]},{"name":"b"}]}}` + "\n" + if rr.Body.String() != want { + t.Fatalf("got %s\nwant %s", rr.Body.String(), want) + } +} + +func TestWithChatGPTToolSchemes_PassesEverythingElseThrough(t *testing.T) { + list := `{"jsonrpc":"2.0","id":2,"method":"tools/list"}` + cases := []struct { + name, body, contentType string + status int + }{ + {"another method", `{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"a"}}`, "application/json", 200}, + {"a batch", "[" + list + "]", "application/json", 200}, + {"an oversized body", `{"jsonrpc":"2.0","id":2,"method":"tools/list","pad":"` + strings.Repeat("x", chatGPTListBodyLimit) + `"}`, "application/json", 200}, + {"an event stream", list, "text/event-stream", 200}, + {"a refusal", list, "application/json", 400}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var seen []byte + rr := serveSchemes(WithChatGPTToolSchemes(stubTransport(c.contentType, c.status, listWithMeta, &seen)), c.body) + if !bytes.Equal(seen, []byte(c.body)) { + t.Errorf("the transport read %d bytes, want the %d-byte body intact", len(seen), len(c.body)) + } + if rr.Code != c.status || rr.Body.String() != listWithMeta { + t.Errorf("response changed: %d %s", rr.Code, rr.Body.String()) + } + }) + } +} + +// The writer WithChatGPTToolSchemes hands the transport must not stream: +// mcp-go upgrades a response to an event stream only through a writer that +// can flush, and a streamed tools/list would leave the rewrite nothing to +// rewrite. +func TestBufferedResponseCannotStream(t *testing.T) { + if _, ok := any(&bufferedResponse{}).(http.Flusher); ok { + t.Fatal("bufferedResponse implements http.Flusher, so tools/list could be answered as an event stream") + } +} diff --git a/internal/server/middleware_mcp_auth.go b/internal/server/middleware_mcp_auth.go index 6d41988f1..5382652d4 100644 --- a/internal/server/middleware_mcp_auth.go +++ b/internal/server/middleware_mcp_auth.go @@ -660,3 +660,11 @@ func mcpResourceFromContext(ctx context.Context) string { v, _ := ctx.Value(ctxMCPResource).(string) return v } + +// MCPResourceMetadataURLFromContext is the protected-resource metadata URL +// of the MCP mount the request arrived on, or "" off a mount. The ChatGPT +// catalog names it in a tool's insufficient_scope challenge (TASK-3321 +// U2c). +func MCPResourceMetadataURLFromContext(ctx context.Context) string { + return protectedResourceMetadataURL(mcpResourceFromContext(ctx)) +}