From 26fd8a6a46a6eecc1f1dbc90a475307b2f033489 Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 2 Oct 2026 04:37:12 -0400 Subject: [PATCH] feat(mcp): ChatGPT tools declare their OAuth scope; a missing scope answers the sign-in challenge (TASK-3321 U2c) (#1720) * feat(mcp): ChatGPT tools declare their OAuth scope; a missing scope answers the sign-in challenge (TASK-3321 U2c) Every ChatGPT catalog tool now carries securitySchemes, an oauth2 scheme naming the one scope it needs: pad:read for a read tool and pad:write for anything else. The Apps SDK reads it at the tool's top level and keeps _meta.securitySchemes as a mirror. mcp-go's Tool marshals a fixed set of keys, so the tool carries the _meta copy, and WithChatGPTToolSchemes adds the top-level copy to a single tools/list JSON response on the ChatGPT mount. Every other request and response passes through byte for byte. When a write tool is called with a read-only grant, the handler answers before dispatching anything: an isError result whose _meta["mcp/www_authenticate"] is a Bearer challenge with error="insufficient_scope" and an error_description, naming the protected-resource document of the mount the request arrived on (server.MCPResourceMetadataURLFromContext). That is what makes ChatGPT offer to reconnect with the wider grant. The wire golden is built through the production transport (newChatGPTTransport). Only the ChatGPT golden moved; /mcp is byte-identical. ChatGPTToolSurfaceVersion stays 0.1: it is unstable and unpublished. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * test(mcp): ChatGPT tools/list is a JSON body with top-level securitySchemes when asked the way ChatGPT asks (TASK-3321 U2c) Streamable HTTP upgrades a response to an event stream only by flushing notifications mid-request (the remote transport has no event store), and WithChatGPTToolSchemes hands the transport a writer that cannot flush. So tools/list requested with Accept: application/json, text/event-stream is a JSON body, and every tool carries securitySchemes at the top level. One leg pins that through the production transport, and another pins that the buffered writer never implements http.Flusher. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC --- cmd/pad/chatgpt_catalog_test.go | 2 +- cmd/pad/chatgpt_scope_test.go | 207 +++++++++ cmd/pad/cmd_server_chatgpt.go | 11 +- .../testdata/mcp_wire/chatgpt_tools_list.json | 420 +++++++++++++++++- internal/mcp/chatgpt_catalog.go | 8 + internal/mcp/chatgpt_security.go | 191 ++++++++ internal/mcp/chatgpt_security_test.go | 81 ++++ internal/server/middleware_mcp_auth.go | 8 + 8 files changed, 904 insertions(+), 24 deletions(-) create mode 100644 cmd/pad/chatgpt_scope_test.go create mode 100644 internal/mcp/chatgpt_security.go create mode 100644 internal/mcp/chatgpt_security_test.go 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)) +}