diff --git a/internal/mcpserver/annotations.go b/internal/mcpserver/annotations.go new file mode 100644 index 0000000..f8f31ac --- /dev/null +++ b/internal/mcpserver/annotations.go @@ -0,0 +1,159 @@ +package mcpserver + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "strconv" +) + +// ensureReadOnlyHint guarantees every tool in a tools/list response carries an +// explicit readOnlyHint. The Go MCP SDK marshals ToolAnnotations.ReadOnlyHint +// with `omitempty`, so a write tool's correct value (readOnlyHint:false) is +// dropped from the wire. OpenAI's ChatGPT Apps submission, however, requires +// readOnlyHint, openWorldHint, and destructiveHint to be present (true or +// false) on every tool. This shim re-adds the spec-default readOnlyHint:false +// wherever a tool's annotations object omits it (i.e. our save_recipe tool). +// +// Only tools/list responses are buffered and rewritten; every other request — +// including streaming tool calls — passes through untouched so streaming is +// preserved. +func ensureReadOnlyHint(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + next.ServeHTTP(w, r) + return + } + // Peek the request body to see if this is a tools/list call, then + // restore it for the wrapped handler. + body, err := io.ReadAll(io.LimitReader(r.Body, 1<<20)) + _ = r.Body.Close() + r.Body = io.NopCloser(bytes.NewReader(body)) + if err != nil || !bytes.Contains(body, []byte(`"tools/list"`)) { + next.ServeHTTP(w, r) + return + } + + rec := &bufferingWriter{header: make(http.Header), body: &bytes.Buffer{}} + next.ServeHTTP(rec, r) + out := fixToolsListBody(rec.body.Bytes()) + + h := w.Header() + for k, v := range rec.header { + h[k] = v + } + h.Set("Content-Length", strconv.Itoa(len(out))) + status := rec.status + if status == 0 { + status = http.StatusOK + } + w.WriteHeader(status) + _, _ = w.Write(out) + }) +} + +// bufferingWriter captures a handler's response so it can be rewritten before +// being flushed to the real client. +type bufferingWriter struct { + header http.Header + status int + body *bytes.Buffer +} + +func (b *bufferingWriter) Header() http.Header { return b.header } +func (b *bufferingWriter) WriteHeader(code int) { b.status = code } +func (b *bufferingWriter) Write(p []byte) (int, error) { return b.body.Write(p) } + +// fixToolsListBody rewrites a tools/list response body (either a plain JSON +// response or an SSE-framed one) to backfill missing readOnlyHint annotations. +func fixToolsListBody(body []byte) []byte { + trimmed := bytes.TrimLeft(body, " \t\r\n") + if len(trimmed) > 0 && trimmed[0] == '{' { + if fixed, ok := fixReadOnlyHintJSON(body); ok { + return fixed + } + return body + } + // SSE framing (text/event-stream): rewrite each data: line's JSON payload. + if bytes.Contains(body, []byte("data:")) { + lines := bytes.Split(body, []byte("\n")) + changed := false + for i, ln := range lines { + t := bytes.TrimLeft(ln, " \t") + if !bytes.HasPrefix(t, []byte("data:")) { + continue + } + payload := bytes.TrimSpace(t[len("data:"):]) + if fixed, ok := fixReadOnlyHintJSON(payload); ok { + lines[i] = append([]byte("data: "), fixed...) + changed = true + } + } + if changed { + return bytes.Join(lines, []byte("\n")) + } + } + return body +} + +// fixReadOnlyHintJSON parses a single JSON-RPC response object and, for every +// tool in result.tools that has an annotations object without a readOnlyHint +// key, adds readOnlyHint:false. It returns the (possibly rewritten) payload and +// whether any change was made. On any parse failure it returns the input +// unchanged so a malformed body is never corrupted. +func fixReadOnlyHintJSON(payload []byte) ([]byte, bool) { + var msg map[string]json.RawMessage + if err := json.Unmarshal(payload, &msg); err != nil { + return payload, false + } + var result map[string]json.RawMessage + if err := json.Unmarshal(msg["result"], &result); err != nil { + return payload, false + } + var tools []map[string]json.RawMessage + if err := json.Unmarshal(result["tools"], &tools); err != nil { + return payload, false + } + + changed := false + for _, tool := range tools { + annRaw, ok := tool["annotations"] + if !ok { + continue + } + var ann map[string]json.RawMessage + if err := json.Unmarshal(annRaw, &ann); err != nil { + continue + } + if _, has := ann["readOnlyHint"]; has { + continue + } + ann["readOnlyHint"] = json.RawMessage("false") + newAnn, err := json.Marshal(ann) + if err != nil { + continue + } + tool["annotations"] = newAnn + changed = true + } + if !changed { + return payload, false + } + + newTools, err := json.Marshal(tools) + if err != nil { + return payload, false + } + result["tools"] = newTools + newResult, err := json.Marshal(result) + if err != nil { + return payload, false + } + msg["result"] = newResult + out, err := json.Marshal(msg) + if err != nil { + return payload, false + } + return out, true +} diff --git a/internal/mcpserver/annotations_test.go b/internal/mcpserver/annotations_test.go new file mode 100644 index 0000000..715665b --- /dev/null +++ b/internal/mcpserver/annotations_test.go @@ -0,0 +1,129 @@ +package mcpserver + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// toolAnnotations unmarshals a JSON-RPC tools/list body and returns each tool's +// annotations keyed by tool name. +func toolAnnotations(t *testing.T, body []byte) map[string]map[string]any { + t.Helper() + var parsed struct { + Result struct { + Tools []struct { + Name string `json:"name"` + Annotations map[string]any `json:"annotations"` + } `json:"tools"` + } `json:"result"` + } + if err := json.Unmarshal(body, &parsed); err != nil { + t.Fatalf("unmarshal result: %v\nbody: %s", err, body) + } + out := make(map[string]map[string]any) + for _, tool := range parsed.Result.Tools { + out[tool.Name] = tool.Annotations + } + return out +} + +func TestFixReadOnlyHintJSON_BackfillsMissing(t *testing.T) { + in := `{"jsonrpc":"2.0","id":1,"result":{"tools":[` + + `{"name":"save_recipe","annotations":{"destructiveHint":false,"openWorldHint":true}},` + + `{"name":"get_recipe","annotations":{"readOnlyHint":true,"destructiveHint":false,"openWorldHint":false}}` + + `]}}` + + out, changed := fixReadOnlyHintJSON([]byte(in)) + if !changed { + t.Fatal("expected the body to be changed") + } + ann := toolAnnotations(t, out) + + save := ann["save_recipe"] + if v, ok := save["readOnlyHint"].(bool); !ok || v != false { + t.Fatalf("save_recipe readOnlyHint = %v (%T), want false", save["readOnlyHint"], save["readOnlyHint"]) + } + // Its other annotations must survive intact. + if v, ok := save["destructiveHint"].(bool); !ok || v != false { + t.Fatalf("save_recipe destructiveHint = %v, want false", save["destructiveHint"]) + } + if v, ok := save["openWorldHint"].(bool); !ok || v != true { + t.Fatalf("save_recipe openWorldHint = %v, want true", save["openWorldHint"]) + } + // A tool that already declares readOnlyHint keeps its value. + if v, ok := ann["get_recipe"]["readOnlyHint"].(bool); !ok || v != true { + t.Fatalf("get_recipe readOnlyHint = %v, want true", ann["get_recipe"]["readOnlyHint"]) + } +} + +func TestFixReadOnlyHintJSON_NoChangeWhenComplete(t *testing.T) { + in := `{"jsonrpc":"2.0","id":1,"result":{"tools":[` + + `{"name":"get_recipe","annotations":{"readOnlyHint":true,"destructiveHint":false,"openWorldHint":false}}` + + `]}}` + if _, changed := fixReadOnlyHintJSON([]byte(in)); changed { + t.Fatal("expected no change when readOnlyHint already present") + } +} + +func TestFixReadOnlyHintJSON_MalformedUntouched(t *testing.T) { + in := []byte(`not json at all`) + out, changed := fixReadOnlyHintJSON(in) + if changed || string(out) != string(in) { + t.Fatal("malformed body must be returned unchanged") + } +} + +func TestFixToolsListBody_SSE(t *testing.T) { + body := "event: message\n" + + `data: {"jsonrpc":"2.0","id":1,"result":{"tools":[{"name":"save_recipe","annotations":{"openWorldHint":true}}]}}` + + "\n\n" + out := fixToolsListBody([]byte(body)) + if !strings.Contains(string(out), `"readOnlyHint":false`) { + t.Fatalf("expected readOnlyHint backfilled in SSE data line, got: %s", out) + } + if !strings.HasPrefix(string(out), "event: message\n") { + t.Fatalf("SSE framing not preserved: %s", out) + } +} + +func TestEnsureReadOnlyHint_Middleware(t *testing.T) { + // Stub MCP handler that returns a tools/list response missing readOnlyHint. + stub := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"jsonrpc":"2.0","id":1,"result":{"tools":[`+ + `{"name":"save_recipe","annotations":{"destructiveHint":false,"openWorldHint":true}}`+ + `]}}`) + }) + h := ensureReadOnlyHint(stub) + + req := httptest.NewRequest(http.MethodPost, "/mcp", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`)) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + ann := toolAnnotations(t, rec.Body.Bytes()) + if v, ok := ann["save_recipe"]["readOnlyHint"].(bool); !ok || v != false { + t.Fatalf("middleware did not backfill readOnlyHint:false, got: %s", rec.Body.Bytes()) + } +} + +func TestEnsureReadOnlyHint_PassesThroughNonToolsList(t *testing.T) { + const raw = `{"jsonrpc":"2.0","id":1,"result":{"content":[]}}` + stub := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, raw) + }) + h := ensureReadOnlyHint(stub) + + req := httptest.NewRequest(http.MethodPost, "/mcp", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"save_recipe"}}`)) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Body.String() != raw { + t.Fatalf("tools/call body must pass through untouched, got: %s", rec.Body.String()) + } +} diff --git a/internal/mcpserver/server.go b/internal/mcpserver/server.go index ad52342..2e77ae0 100644 --- a/internal/mcpserver/server.go +++ b/internal/mcpserver/server.go @@ -110,5 +110,5 @@ func NewHandler(cfg *config.Config, deps *Deps) http.Handler { return auth.RequireBearerToken(verifier, &auth.RequireBearerTokenOptions{ ResourceMetadataURL: deps.OAuth.Issuer() + "/.well-known/oauth-protected-resource/mcp", - })(mcpHandler) + })(ensureReadOnlyHint(mcpHandler)) } diff --git a/internal/mcpserver/tools.go b/internal/mcpserver/tools.go index c49fb61..f4f527f 100644 --- a/internal/mcpserver/tools.go +++ b/internal/mcpserver/tools.go @@ -341,8 +341,9 @@ func registerTools(server *mcp.Server, deps *Deps) { ), // Read-only: queries the open web; makes no changes to the user's account. Annotations: &mcp.ToolAnnotations{ - ReadOnlyHint: true, - OpenWorldHint: boolPtr(true), + ReadOnlyHint: true, + DestructiveHint: boolPtr(false), + OpenWorldHint: boolPtr(true), }, }, deps.searchRecipes) @@ -357,8 +358,9 @@ func registerTools(server *mcp.Server, deps *Deps) { ), // Read-only: fetches and extracts an external URL; saves nothing. Annotations: &mcp.ToolAnnotations{ - ReadOnlyHint: true, - OpenWorldHint: boolPtr(true), + ReadOnlyHint: true, + DestructiveHint: boolPtr(false), + OpenWorldHint: boolPtr(true), }, }, deps.previewRecipe) @@ -391,8 +393,9 @@ func registerTools(server *mcp.Server, deps *Deps) { ), // Read-only: reads the user's own saved collection only (closed world). Annotations: &mcp.ToolAnnotations{ - ReadOnlyHint: true, - OpenWorldHint: boolPtr(false), + ReadOnlyHint: true, + DestructiveHint: boolPtr(false), + OpenWorldHint: boolPtr(false), }, }, deps.listMyRecipes) @@ -407,8 +410,9 @@ func registerTools(server *mcp.Server, deps *Deps) { ), // Read-only: reads one recipe from the user's own collection (closed world). Annotations: &mcp.ToolAnnotations{ - ReadOnlyHint: true, - OpenWorldHint: boolPtr(false), + ReadOnlyHint: true, + DestructiveHint: boolPtr(false), + OpenWorldHint: boolPtr(false), }, }, deps.getRecipe) }