Narrow go-sdk cacheScope:public to private on projected results

mcpcompat/server/server.go

@@ -766,20 +766,14 @@ 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.
+			// 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)
 			}
@@ -799,6 +793,86 @@ 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, 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:
+		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:
+		// 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 == cacheScopePublic {
+		c.CacheScope = cacheScopePrivate
+	}
+}
+
 // 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.

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"
@@ -443,3 +446,148 @@ 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) })
+	})
+
+	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
+// 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)
+	}
+}