From 24cab4dcfdc9cda10967680f800de12fdfbd7ea1 Mon Sep 17 00:00:00 2001 From: ozz Date: Fri, 24 Jul 2026 19:24:44 +0000 Subject: [PATCH 1/3] feat(mcpcompat): mark stateless list/discover results cacheScope private Follow-up to #187. Under stateless serving (MCP 2026-07-28), go-sdk stamps cacheScope:"public" on tools/list, resources/list, resources/templates/list, prompts/list, and server/discover result bodies (via its unexported setDefaultCacheableValues). These results are served through the shim's per-identity before-hook projection, so a "public" cache hint on an identity-filtered result is wrong: an MCP-aware cache could serve one identity's projected view to another. cacheScope is an in-body MCP field, so it must be corrected in the result body, not via an HTTP header. setDefaultCacheableValues runs inside the go-sdk method handlers, so the result already carries cacheScope when the shim's receiving middleware sees it on the return path (encoding happens later), and the five result types expose a settable exported Cacheable.CacheScope. So on the stateless path only, rewrite "public" -> "private" after dispatch. The stateful path is untouched (a stored session already scopes results per session). Replaces the deferred TODO from #187 with the actual fix. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MRh394VZgnqbYjJwjrcJWm --- mcpcompat/server/server.go | 68 ++++++++++++---- mcpcompat/server/stateless_test.go | 126 +++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 14 deletions(-) diff --git a/mcpcompat/server/server.go b/mcpcompat/server/server.go index 425aacb..7ffe5ef 100644 --- a/mcpcompat/server/server.go +++ b/mcpcompat/server/server.go @@ -766,20 +766,24 @@ func (s *MCPServer) sessionDispatchMiddleware(srv *gosdk.Server, stateless bool) // fireBeforeHooks for the extraction and fallback behavior. s.fireBeforeHooks(ctx, method, req) res, err := next(ctx, method, req) - // TODO(cacheScope): go-sdk's setDefaultCacheableValues stamps - // cacheScope:"public" (Cacheable.CacheScope, an IN-BODY MCP result - // field, NOT an HTTP header) on 2026-07-28 list/discover result - // bodies, but leaves ttlMs=0. Per the spec a zero TTL means - // "immediately stale", so a bare "public" scope with ttlMs==0 is - // inert: nothing may actually cache it yet. The cross-identity - // cache-share hazard for a per-identity-projected result only - // materializes once something sets a positive ttlMs. When that - // happens, the fix is an IN-BODY rewrite of cacheScope to - // "private" on per-identity-projected results — either here - // (this middleware already holds res) or in the vMCP envelope — - // NOT an HTTP Cache-Control header, which cannot neutralize a - // cacheScope an MCP-aware cache reads from the body. Deferred: - // no consumer sets a positive ttlMs yet. Tracked as follow-up. + // go-sdk's setDefaultCacheableValues (mcp/protocol.go) unconditionally + // stamps cacheScope:"public" (Cacheable.CacheScope, an IN-BODY MCP + // result field, NOT an HTTP header) on every list/discover result, + // with no awareness of stateless per-identity projection. Under + // stateless, list/discover results are produced by this middleware's + // own before-hook projection (fireBeforeHooks) and per-request + // ephemeral session binding (bindSessionForDispatch), so the same + // request can yield a different, identity-scoped result each time — + // a "public" hint on such a result would let an MCP-aware cache serve + // one identity's projected list to another. Rewrite it to "private" + // here, since cacheScope lives in the body: an HTTP Cache-Control + // header cannot neutralize a scope hint an MCP-aware cache reads from + // the body. This only narrows the SDK's default; it never widens + // cache-sharing. The stateful path is untouched (no per-identity + // projection there, so "public" is the SDK's intended default). + if err == nil && stateless { + stripPublicCacheScope(res) + } if err != nil && method == string(mcp.MethodToolsCall) { err = translateUnknownToolError(err, req) } @@ -799,6 +803,42 @@ func (s *MCPServer) sessionDispatchMiddleware(srv *gosdk.Server, stateless bool) } } +// stripPublicCacheScope rewrites an in-body "public" Cacheable.CacheScope hint +// to "private" on the go-sdk results that carry one. +// +// go-sdk stamps a "public" CacheScope (setDefaultCacheableValues, +// mcp/protocol.go) on every list/discover result AND on resources/read results, +// regardless of whether the body was produced per-identity — which it is under +// stateless serving (see sessionDispatchMiddleware). resources/read is included: +// its handler runs with the caller's bridged identity and can return +// per-identity content, so a "public" hint on it is the same cross-identity +// cache-leak as on a projected list. gosdk.GetPromptResult and CompleteResult do +// not embed Cacheable and are never stamped, so they need no handling here. A +// nil res, one of an unhandled type, or one already scoped "private", is a +// no-op. +func stripPublicCacheScope(res gosdk.Result) { + var c *gosdk.Cacheable + switch r := res.(type) { + case *gosdk.ListToolsResult: + c = &r.Cacheable + case *gosdk.ListResourcesResult: + c = &r.Cacheable + case *gosdk.ListResourceTemplatesResult: + c = &r.Cacheable + case *gosdk.ListPromptsResult: + c = &r.Cacheable + case *gosdk.DiscoverResult: + c = &r.Cacheable + case *gosdk.ReadResourceResult: + c = &r.Cacheable + default: + return + } + if c.CacheScope == "public" { + c.CacheScope = "private" + } +} + // bindSessionForDispatch binds the ClientSession for this dispatch into ctx, // selecting the binding strategy for the session's shape: // - no go-sdk session at all (ss==nil): nothing to bind. diff --git a/mcpcompat/server/stateless_test.go b/mcpcompat/server/stateless_test.go index 5a8836e..b3e69d9 100644 --- a/mcpcompat/server/stateless_test.go +++ b/mcpcompat/server/stateless_test.go @@ -84,6 +84,132 @@ func TestStateless_BasicRequestResponse(t *testing.T) { }) } +// TestStateless_ListAndDiscoverCacheScopePrivate verifies the fix for the +// cacheScope cross-identity hazard: go-sdk's setDefaultCacheableValues stamps +// every list/discover result's IN-BODY Cacheable.CacheScope with "public", +// with no awareness that, under stateless serving, that same result is +// produced by the shim's per-identity before-hook projection +// (sessionDispatchMiddleware). sessionDispatchMiddleware must rewrite that +// hint to "private" on the stateless path before the response is serialized, +// so an MCP-aware cache never treats one identity's projected list/discover +// result as shareable with another identity. +func TestStateless_ListAndDiscoverCacheScopePrivate(t *testing.T) { + t.Parallel() + + const resourceURI = "file:///r" + mcpSrv := server.NewMCPServer("stateless-cachescope", "1.0.0") + addGreetTool(mcpSrv) + mcpSrv.AddResource( + mcp.Resource{URI: resourceURI, Name: "r", MIMEType: "text/plain"}, + func(context.Context, mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + return []mcp.ResourceContents{ + mcp.TextResourceContents{URI: resourceURI, MIMEType: "text/plain", Text: "hi"}, + }, nil + }, + ) + s := server.NewStreamableHTTPServer(mcpSrv, server.WithStateless(true)) + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + t.Run("tools/list", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + resp := postRPC(ctx, t, ts.URL, "", `{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`) + r := readFirstResult(t, resp) + require.Nil(t, r.Error, "tools/list should not error") + var res struct { + CacheScope string `json:"cacheScope"` + } + require.NoError(t, json.Unmarshal(r.Result, &res)) + assert.Equal(t, "private", res.CacheScope, + "a stateless tools/list result is per-identity projected and must never be marked cacheScope:public") + }) + + t.Run("server/discover", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // server/discover is only recognized when the request's _meta carries + // io.modelcontextprotocol/protocolVersion:"2026-07-28" (SEP-2575's + // per-request new-protocol signal; see go-sdk's ServerSession.handle), + // so this is sent via a raw request rather than postRPC: postRPC always + // sets Mcp-Protocol-Version: 2025-06-18, which go-sdk rejects as + // mismatched against a 2026-07-28 _meta on the very same request. + body := `{"jsonrpc":"2.0","id":2,"method":"server/discover","params":{"_meta":{` + + `"io.modelcontextprotocol/protocolVersion":"2026-07-28",` + + `"io.modelcontextprotocol/clientInfo":{"name":"test","version":"1.0"},` + + `"io.modelcontextprotocol/clientCapabilities":{}}}}` + req, err := http.NewRequestWithContext(ctx, http.MethodPost, ts.URL, strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", bothAcceptMediaTypesForTest) + req.Header.Set("MCP-Protocol-Version", "2026-07-28") + // 2026-07-28's standard-headers requirement (SEP-2575) mandates an + // Mcp-Method header mirroring the body's method for any request once + // Mcp-Protocol-Version >= 2026-07-28. + req.Header.Set("Mcp-Method", "server/discover") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + r := readFirstResult(t, resp) + require.Nil(t, r.Error, "server/discover should not error") + var res struct { + CacheScope string `json:"cacheScope"` + } + require.NoError(t, json.Unmarshal(r.Result, &res)) + assert.Equal(t, "private", res.CacheScope, + "a stateless server/discover result must never be marked cacheScope:public") + }) + + t.Run("resources/read", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // resources/read runs its handler with the caller's bridged identity and + // can return per-identity content, so its go-sdk-default cacheScope:"public" + // hint is the same cross-identity cache-leak risk as a projected list. + resp := postRPC(ctx, t, ts.URL, "", + `{"jsonrpc":"2.0","id":3,"method":"resources/read","params":{"uri":"file:///r"}}`) + r := readFirstResult(t, resp) + require.Nil(t, r.Error, "resources/read should not error") + var res struct { + CacheScope string `json:"cacheScope"` + } + require.NoError(t, json.Unmarshal(r.Result, &res)) + assert.Equal(t, "private", res.CacheScope, + "a stateless resources/read result is served per-identity and must never be marked cacheScope:public") + }) +} + +// TestStateful_ListToolsCacheScopeUntouched pins the divergence from +// TestStateless_ListAndDiscoverCacheScopePrivate: a stateful server has no +// per-identity projection concern (every session is its own, non-shared +// go-sdk ServerSession, not a per-request ephemeral binding), so +// sessionDispatchMiddleware must leave go-sdk's default cacheScope:"public" +// alone on the stateful path. +func TestStateful_ListToolsCacheScopeUntouched(t *testing.T) { + t.Parallel() + + mcpSrv := server.NewMCPServer("stateful-cachescope", "1.0.0") + addGreetTool(mcpSrv) + s := server.NewStreamableHTTPServer(mcpSrv, server.WithStateless(false)) + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + sid := initSession(t, ts.URL) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + resp := postRPC(ctx, t, ts.URL, sid, `{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}`) + r := readFirstResult(t, resp) + require.Nil(t, r.Error, "tools/list should not error") + var res struct { + CacheScope string `json:"cacheScope"` + } + require.NoError(t, json.Unmarshal(r.Result, &res)) + assert.Equal(t, "public", res.CacheScope, + "the stateful path must not be touched by the stateless cacheScope rewrite") +} + // TestStateless_GETAndDELETENotAllowed verifies that a stateless server has no // stream to open and no session to terminate, so go-sdk answers both GET and // DELETE with 405, carrying the RFC 9110-mandated Allow header. From 8500ded672255928eea635f174b26620c20e8c03 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 09:24:23 +0200 Subject: [PATCH 2/3] fix(mcpcompat): narrow cacheScope on every serving mode, not just stateless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up to 24cab4d. Three fixes. 1. The `&& stateless` guard rested on an inverted premise. The comment said the stateful path has "no per-identity projection there", but stateful is projected *more*, not less: registerAndSync installs the SessionWithTools/ Resources/ResourceTemplates/Prompts overlays onto that session's own go-sdk server (syncSession*), and that path runs only when !stateless — stateless projection is tools-only (see WithStateless's KNOWN LIMITATION block). The identity bridge (valueBridgeContext) is likewise not gated on serving mode, which is the stated justification for including resources/read in the first place and applies verbatim to stateful. So the rewrite now runs on every serving mode. Per the PR's own invariant this only narrows the SDK's default and never widens cache-sharing, which makes unconditional the simpler as well as the safer option. Not exploitable today on either path: nothing sets TTLMs, and both the 2026-07-28 caching spec and go-sdk's own client cache treat ttlMs<=0 as never-cacheable. This is defense-in-depth, and the point is that the stateless/stateful split was not a real boundary. TestStateful_ListToolsCacheScopeUntouched pinned the leaky value as intended; it is now TestStateful_ListToolsCacheScopePrivate and asserts the opposite. It fails against the previous commit. 2. A go-sdk bump adding Cacheable to a new result type would have been silently missed by the type switch — the exact leak the removed TODO warned about, and go-sdk is pinned to a PRE-RELEASE. Every Cacheable embedder satisfies the exported gosdk.CacheableResult for free, so the default arm now warns when it sees a cacheable result it does not cover. Warn, not panic: a missed cache hint must not take down a request on a goroutine go-sdk never recovers. Logs the type only, never the body. stripPublicCacheScope becomes a method to reach s.logger. 3. The doc comment claimed "a nil res ... is a no-op", which holds for an untyped-nil gosdk.Result but not for a typed-nil pointer of a covered type, which panics on the field read. Unreachable today (go-sdk's own pointer-receiver setDefaultCacheableValues would panic on such a value first, and wrapResourceHandler always allocates), so the comment is corrected rather than a guard added for a path that cannot be hit. Also: named the two cacheScope union members as constants (go-sdk exports none), and added TestStripPublicCacheScope covering all six result types directly — three arms (prompts/list, resources/list, resources/templates/list) had no coverage, and the blackbox tests in stateless_test.go cannot reach the unexported helper. task lint (0 issues), task test (race, all packages), task license-check: all clean. Co-Authored-By: Claude Opus 5 --- mcpcompat/server/server.go | 90 +++++++++++++++------- mcpcompat/server/server_internal_test.go | 95 ++++++++++++++++++++++++ mcpcompat/server/stateless_test.go | 27 ++++--- 3 files changed, 172 insertions(+), 40 deletions(-) diff --git a/mcpcompat/server/server.go b/mcpcompat/server/server.go index 7ffe5ef..f72dcc2 100644 --- a/mcpcompat/server/server.go +++ b/mcpcompat/server/server.go @@ -766,23 +766,13 @@ func (s *MCPServer) sessionDispatchMiddleware(srv *gosdk.Server, stateless bool) // fireBeforeHooks for the extraction and fallback behavior. s.fireBeforeHooks(ctx, method, req) res, err := next(ctx, method, req) - // go-sdk's setDefaultCacheableValues (mcp/protocol.go) unconditionally - // stamps cacheScope:"public" (Cacheable.CacheScope, an IN-BODY MCP - // result field, NOT an HTTP header) on every list/discover result, - // with no awareness of stateless per-identity projection. Under - // stateless, list/discover results are produced by this middleware's - // own before-hook projection (fireBeforeHooks) and per-request - // ephemeral session binding (bindSessionForDispatch), so the same - // request can yield a different, identity-scoped result each time — - // a "public" hint on such a result would let an MCP-aware cache serve - // one identity's projected list to another. Rewrite it to "private" - // here, since cacheScope lives in the body: an HTTP Cache-Control - // header cannot neutralize a scope hint an MCP-aware cache reads from - // the body. This only narrows the SDK's default; it never widens - // cache-sharing. The stateful path is untouched (no per-identity - // projection there, so "public" is the SDK's intended default). - if err == nil && stateless { - stripPublicCacheScope(res) + // Narrow go-sdk's default in-body cacheScope:"public" hint to + // "private" on results this shim serves per-identity — which is + // every serving mode, stateful included. See stripPublicCacheScope + // for the full rationale and why this is a body rewrite rather than + // an HTTP Cache-Control header. + if err == nil { + s.stripPublicCacheScope(res) } if err != nil && method == string(mcp.MethodToolsCall) { err = translateUnknownToolError(err, req) @@ -803,20 +793,50 @@ func (s *MCPServer) sessionDispatchMiddleware(srv *gosdk.Server, stateless bool) } } +// The two members of the MCP cacheScope union. go-sdk models CacheScope as a +// bare string and exports no constants for it, so name them here. +const ( + cacheScopePublic = "public" + cacheScopePrivate = "private" +) + // stripPublicCacheScope rewrites an in-body "public" Cacheable.CacheScope hint // to "private" on the go-sdk results that carry one. // // go-sdk stamps a "public" CacheScope (setDefaultCacheableValues, // mcp/protocol.go) on every list/discover result AND on resources/read results, -// regardless of whether the body was produced per-identity — which it is under -// stateless serving (see sessionDispatchMiddleware). resources/read is included: -// its handler runs with the caller's bridged identity and can return -// per-identity content, so a "public" hint on it is the same cross-identity -// cache-leak as on a projected list. gosdk.GetPromptResult and CompleteResult do -// not embed Cacheable and are never stamped, so they need no handling here. A -// nil res, one of an unhandled type, or one already scoped "private", is a -// no-op. -func stripPublicCacheScope(res gosdk.Result) { +// regardless of whether the body was produced per-identity — which, in this +// shim, it is: +// +// - Every dispatch runs the handler under the caller's bridged identity +// (the valueBridgeContext in sessionDispatchMiddleware is not gated on +// serving mode), so resources/read can return per-identity content. +// - Stateless serving projects per-identity tools via the before-list hooks +// (fireBeforeHooks) over a per-request ephemeral session +// (bindSessionForDispatch). +// - Stateful serving projects MORE, not less: registerAndSync installs the +// SessionWithTools/Resources/ResourceTemplates/Prompts overlays onto that +// session's own go-sdk server (syncSession*), and that path runs only when +// !stateless. ToolHive's vMCP layer is the consumer. +// +// A "public" hint on any of those bodies is a cross-identity cache-leak signal: +// per the 2026-07-28 caching spec it tells any client, shared gateway or +// caching proxy that it MAY serve the response to a different user, and it does +// so regardless of the endpoint being authenticated. So the rewrite applies to +// every serving mode. It must be an in-body rewrite, not an HTTP Cache-Control +// header, which cannot neutralize a scope hint an MCP-aware cache reads out of +// the body. It only ever narrows the SDK's default; it never widens +// cache-sharing. +// +// gosdk.GetPromptResult, CompleteResult and CallToolResult do not embed +// Cacheable and are never stamped, so they need no handling here. A res of an +// unhandled type, or one already scoped "private", is a no-op — as is an +// untyped-nil gosdk.Result, which matches no case arm. A typed-nil pointer of a +// covered type would panic on the field read, but go-sdk cannot hand one back on +// a success path: its own pointer-receiver setDefaultCacheableValues would have +// panicked on it first, and wrapResourceHandler always allocates a non-nil +// result. +func (s *MCPServer) stripPublicCacheScope(res gosdk.Result) { var c *gosdk.Cacheable switch r := res.(type) { case *gosdk.ListToolsResult: @@ -832,10 +852,24 @@ func stripPublicCacheScope(res gosdk.Result) { case *gosdk.ReadResourceResult: c = &r.Cacheable default: + // Anything that embeds Cacheable satisfies gosdk.CacheableResult for + // free (GetCacheScope/GetTTLMs are promoted from the embedded struct), + // so a result landing here while reporting a "public" scope is one a + // newer go-sdk started stamping and the switch above does not cover — + // the one way a go-sdk bump could silently reintroduce the leak this + // function exists to prevent. Warn rather than panic: a missed cache + // hint must not take down a request on a goroutine go-sdk never + // recovers. Log the type only, never the body. + if cr, ok := res.(gosdk.CacheableResult); ok && cr.GetCacheScope() == cacheScopePublic { + if s.logger != nil { + s.logger.Warn("unhandled cacheable MCP result type: leaving cacheScope public", + "type", fmt.Sprintf("%T", res)) + } + } return } - if c.CacheScope == "public" { - c.CacheScope = "private" + if c.CacheScope == cacheScopePublic { + c.CacheScope = cacheScopePrivate } } diff --git a/mcpcompat/server/server_internal_test.go b/mcpcompat/server/server_internal_test.go index 558dd06..0878bfd 100644 --- a/mcpcompat/server/server_internal_test.go +++ b/mcpcompat/server/server_internal_test.go @@ -443,3 +443,98 @@ func TestBindSessionForDispatch_StatelessForcesEphemeralRegardlessOfSessionID(t assert.NotSame(t, csA, csB, "each stateless dispatch must get its own ephemeral session, even under a shared client-supplied session id") } + +// TestStripPublicCacheScope covers the mapping directly, so all six go-sdk +// result types that embed Cacheable are exercised without standing up an HTTP +// server per type (the blackbox tests in stateless_test.go cover three of them +// end-to-end and cannot reach this unexported helper at all). The go-sdk +// version is pinned to a PRE-RELEASE, so the completeness of this switch is +// exactly what a version bump can silently break. +func TestStripPublicCacheScope(t *testing.T) { + t.Parallel() + + // Every gosdk result type that embeds Cacheable, i.e. every type + // setDefaultCacheableValues is called on. Keep in sync with the switch. + covered := map[string]gosdk.Result{ + "ListToolsResult": &gosdk.ListToolsResult{}, + "ListResourcesResult": &gosdk.ListResourcesResult{}, + "ListResourceTemplatesResult": &gosdk.ListResourceTemplatesResult{}, + "ListPromptsResult": &gosdk.ListPromptsResult{}, + "DiscoverResult": &gosdk.DiscoverResult{}, + "ReadResourceResult": &gosdk.ReadResourceResult{}, + } + + s := NewMCPServer("strip-cachescope", "1.0.0") + + for name, res := range covered { + t.Run(name+"/public is narrowed", func(t *testing.T) { + t.Parallel() + cr, ok := res.(gosdk.CacheableResult) + require.True(t, ok, "%s must satisfy CacheableResult via its embedded Cacheable", name) + // Stamp it the way go-sdk's setDefaultCacheableValues would. + setCacheScopeForTest(t, res, cacheScopePublic) + s.stripPublicCacheScope(res) + assert.Equal(t, cacheScopePrivate, cr.GetCacheScope(), + "%s is served per-identity and must be narrowed to private", name) + }) + } + + t.Run("already private is a no-op", func(t *testing.T) { + t.Parallel() + res := &gosdk.ListToolsResult{Cacheable: gosdk.Cacheable{CacheScope: cacheScopePrivate, TTLMs: 5}} + s.stripPublicCacheScope(res) + assert.Equal(t, cacheScopePrivate, res.CacheScope) + assert.Equal(t, 5, res.TTLMs, "ttlMs must never be touched") + }) + + t.Run("unset scope is left alone", func(t *testing.T) { + t.Parallel() + // Only the literal "public" is narrowed: an unset scope is not a + // cache-share signal, and inventing one is not this function's job. + res := &gosdk.ListToolsResult{} + s.stripPublicCacheScope(res) + assert.Empty(t, res.CacheScope) + }) + + t.Run("non-cacheable result is a no-op", func(t *testing.T) { + t.Parallel() + // CallToolResult, GetPromptResult and CompleteResult do not embed + // Cacheable, so they have no scope to narrow and must not trip the + // unhandled-type warning either. + for _, res := range []gosdk.Result{ + &gosdk.CallToolResult{}, &gosdk.GetPromptResult{}, &gosdk.CompleteResult{}, + } { + assert.NotImplements(t, (*gosdk.CacheableResult)(nil), res, + "%T must not embed Cacheable, or it belongs in the switch", res) + s.stripPublicCacheScope(res) + } + }) + + t.Run("untyped nil result is a no-op", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { s.stripPublicCacheScope(nil) }) + }) +} + +// setCacheScopeForTest stamps a CacheScope onto a result the way go-sdk's +// unexported setDefaultCacheableValues does, without duplicating the switch +// under test. +func setCacheScopeForTest(t *testing.T, res gosdk.Result, scope string) { + t.Helper() + switch r := res.(type) { + case *gosdk.ListToolsResult: + r.CacheScope = scope + case *gosdk.ListResourcesResult: + r.CacheScope = scope + case *gosdk.ListResourceTemplatesResult: + r.CacheScope = scope + case *gosdk.ListPromptsResult: + r.CacheScope = scope + case *gosdk.DiscoverResult: + r.CacheScope = scope + case *gosdk.ReadResourceResult: + r.CacheScope = scope + default: + t.Fatalf("setCacheScopeForTest: unsupported result type %T", res) + } +} diff --git a/mcpcompat/server/stateless_test.go b/mcpcompat/server/stateless_test.go index b3e69d9..74655d6 100644 --- a/mcpcompat/server/stateless_test.go +++ b/mcpcompat/server/stateless_test.go @@ -90,9 +90,10 @@ func TestStateless_BasicRequestResponse(t *testing.T) { // with no awareness that, under stateless serving, that same result is // produced by the shim's per-identity before-hook projection // (sessionDispatchMiddleware). sessionDispatchMiddleware must rewrite that -// hint to "private" on the stateless path before the response is serialized, -// so an MCP-aware cache never treats one identity's projected list/discover -// result as shareable with another identity. +// hint to "private" before the response is serialized, so an MCP-aware cache +// never treats one identity's projected list/discover result as shareable with +// another identity. See TestStateful_ListToolsCacheScopePrivate for the same +// guarantee on the stateful path. func TestStateless_ListAndDiscoverCacheScopePrivate(t *testing.T) { t.Parallel() @@ -181,13 +182,15 @@ func TestStateless_ListAndDiscoverCacheScopePrivate(t *testing.T) { }) } -// TestStateful_ListToolsCacheScopeUntouched pins the divergence from -// TestStateless_ListAndDiscoverCacheScopePrivate: a stateful server has no -// per-identity projection concern (every session is its own, non-shared -// go-sdk ServerSession, not a per-request ephemeral binding), so -// sessionDispatchMiddleware must leave go-sdk's default cacheScope:"public" -// alone on the stateful path. -func TestStateful_ListToolsCacheScopeUntouched(t *testing.T) { +// TestStateful_ListToolsCacheScopePrivate pins that the rewrite is NOT +// stateless-only. A stateful server is per-identity projected at least as much +// as a stateless one: registerAndSync installs the SessionWithTools/Resources/ +// ResourceTemplates/Prompts overlays onto that session's own go-sdk server +// (syncSession*), and that path runs only when !stateless — stateless projects +// tools alone. The identity bridge in sessionDispatchMiddleware is likewise not +// gated on serving mode. So go-sdk's default cacheScope:"public" is just as +// wrong here, and must be narrowed to "private" on this path too. +func TestStateful_ListToolsCacheScopePrivate(t *testing.T) { t.Parallel() mcpSrv := server.NewMCPServer("stateful-cachescope", "1.0.0") @@ -206,8 +209,8 @@ func TestStateful_ListToolsCacheScopeUntouched(t *testing.T) { CacheScope string `json:"cacheScope"` } require.NoError(t, json.Unmarshal(r.Result, &res)) - assert.Equal(t, "public", res.CacheScope, - "the stateful path must not be touched by the stateless cacheScope rewrite") + assert.Equal(t, "private", res.CacheScope, + "a stateful tools/list result carries this session's tool overlay and must never be marked cacheScope:public") } // TestStateless_GETAndDELETENotAllowed verifies that a stateless server has no From f8df7d67445f15cb37ea36151cca31528f5ac3b7 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 09:39:19 +0200 Subject: [PATCH 3/3] test(mcpcompat): cover the unhandled-cacheable-type warn path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review catch on 8500ded: that commit added a default arm to stripPublicCacheScope specifically to make a future go-sdk Cacheable type loud instead of silent, and then shipped it with zero coverage on the one branch it exists for. The existing "non-cacheable result is a no-op" subtest only passes CallToolResult/GetPromptResult/CompleteResult, none of which satisfy gosdk.CacheableResult, so none of them reach the arm's GetCacheScope check at all. Adds fakeCacheableResult, standing in for a result type a newer go-sdk embeds Cacheable into without the switch being updated: gosdk.ResultBase supplies the unexported marker that makes it a gosdk.Result, and the embedded Cacheable promotes GetCacheScope/GetTTLMs, so it satisfies gosdk.CacheableResult exactly as a real new SDK type would. Two subtests: the arm reports it and leaves the body untouched (asserting the warning names the type but does NOT carry result body fields), and the arm tolerates a nil logger, since WithLogger is optional. This matters more than the usual coverage argument: the `covered` map in TestStripPublicCacheScope is hand-maintained against the switch, so a seventh Cacheable type passes both silently. The runtime warning is the only actual backstop, which makes an untested warning close to no backstop. Also verified empirically, rather than by assertion, that TestStateful_ListToolsCacheScopePrivate is load-bearing: restoring the `&& stateless` gate makes it fail with public vs private, while TestStripPublicCacheScope still passes (it calls the helper directly and bypasses the middleware). The two tests guard different things — gate-correctness and switch-completeness — and neither covers the other. task lint (0 issues), task test (race, all packages), task license-check: all clean. Co-Authored-By: Claude Opus 5 --- mcpcompat/server/server_internal_test.go | 53 ++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/mcpcompat/server/server_internal_test.go b/mcpcompat/server/server_internal_test.go index 0878bfd..778cd8b 100644 --- a/mcpcompat/server/server_internal_test.go +++ b/mcpcompat/server/server_internal_test.go @@ -4,8 +4,11 @@ package server import ( + "bytes" "context" "encoding/json" + "log/slog" + "strings" "testing" gosdk "github.com/modelcontextprotocol/go-sdk/mcp" @@ -514,6 +517,56 @@ func TestStripPublicCacheScope(t *testing.T) { t.Parallel() assert.NotPanics(t, func() { s.stripPublicCacheScope(nil) }) }) + + t.Run("unhandled cacheable type is left public and warned about", func(t *testing.T) { + t.Parallel() + // The scenario the default arm exists for: a go-sdk version newer than + // the pinned pre-release embeds Cacheable into a result type the switch + // does not cover. Such a type satisfies gosdk.CacheableResult for free, + // so the arm must notice it, leave the body alone, and say so — this is + // the one backstop against a version bump silently reintroducing the + // cross-identity leak, since the covered map above is hand-maintained. + var buf bytes.Buffer + logged := NewMCPServer("strip-cachescope-warn", "1.0.0", + WithLogger(slog.New(slog.NewTextHandler(&buf, nil)))) + res := &fakeCacheableResult{Cacheable: gosdk.Cacheable{CacheScope: cacheScopePublic}} + require.Implements(t, (*gosdk.CacheableResult)(nil), res, + "the fake must satisfy CacheableResult, or it is not exercising the default arm") + + logged.stripPublicCacheScope(res) + + assert.Equal(t, cacheScopePublic, res.CacheScope, + "an unhandled type cannot be narrowed; the arm only reports it") + assert.Contains(t, buf.String(), "unhandled cacheable MCP result type", + "an unhandled cacheable result must be reported, not silently skipped") + assert.Contains(t, buf.String(), "fakeCacheableResult", + "the warning must name the offending type so the switch can be updated") + assert.NotContains(t, strings.ToLower(buf.String()), "cachescope=public", + "the warning must log the type only, never result body fields") + }) + + t.Run("unhandled cacheable type with no logger does not panic", func(t *testing.T) { + t.Parallel() + // WithLogger is optional, so the warn path must tolerate a nil logger. + unlogged := NewMCPServer("strip-cachescope-nolog", "1.0.0") + require.Nil(t, unlogged.logger, "this subtest is only meaningful without a logger") + assert.NotPanics(t, func() { + unlogged.stripPublicCacheScope(&fakeCacheableResult{ + Cacheable: gosdk.Cacheable{CacheScope: cacheScopePublic}, + }) + }) + }) +} + +// fakeCacheableResult stands in for a result type that a future go-sdk embeds +// Cacheable into without stripPublicCacheScope's switch being updated to match. +// gosdk.ResultBase supplies the unexported marker method that makes it a +// gosdk.Result; the embedded Cacheable promotes GetCacheScope/GetTTLMs, which is +// what makes it a gosdk.CacheableResult too — exactly as a real new SDK type +// would. +type fakeCacheableResult struct { + gosdk.ResultBase + gosdk.Cacheable } // setCacheScopeForTest stamps a CacheScope onto a result the way go-sdk's