diff --git a/pkg/vmcp/server/authz_integration_test.go b/pkg/vmcp/server/authz_integration_test.go index 65025a52ef..5b3d9e28ba 100644 --- a/pkg/vmcp/server/authz_integration_test.go +++ b/pkg/vmcp/server/authz_integration_test.go @@ -173,6 +173,15 @@ func buildCedarAuthzServer( Authz: authzCfg, AuditConfig: auditCfg, CodeModeConfig: codeModeCfg, + // Required for TestIntegration_CedarAuthzDenial_ModernPath_IsAudited to + // exercise what it claims: dispatchModern's re-homed call gate. Without + // it the kill switch refuses the Modern request in classifyingHandler and + // dispatchModern never runs. (Before the classifier learned to refuse an + // unserved revision, the request instead fell through to the SDK path, so + // that test passed on a denial from a different gate entirely.) Safe for + // the Legacy-shaped tests sharing this helper: classifyingHandler passes + // Legacy traffic through regardless of this flag. + ModernDispatchEnabled: true, }, router.NewSessionRouter(&vmcp.RoutingTable{}), backendClient, diff --git a/pkg/vmcp/server/classification.go b/pkg/vmcp/server/classification.go index 4e4a66a283..16e2386ad4 100644 --- a/pkg/vmcp/server/classification.go +++ b/pkg/vmcp/server/classification.go @@ -9,13 +9,21 @@ import ( mcpparser "github.com/stacklok/toolhive/pkg/mcp" ) +// methodServerDiscover is the Modern (2026-07-28) capability-probe method. It is +// special-cased in two places — the kill-switch branch below and dispatchModern's +// method switch — because it is how a client learns which revisions a server +// supports, so it must never be refused on version grounds. +const methodServerDiscover = "server/discover" + // classifyingHandler classifies a parsed MCP request as Legacy (2025-11-25) or // Modern (2026-07-28) at the decode seam, rejects a malformed Modern request // with the correct JSON-RPC error before it reaches dispatch, and — when // Config.ModernDispatchEnabled — routes a well-formed Modern request to // dispatchModern instead of the SDK. Legacy traffic always falls through to -// next unchanged, as does a well-formed Modern request while the switch is -// off (default), matching pre-Modern-dispatch wire behavior byte for byte. +// next unchanged. While the switch is off (default), a well-formed Modern +// request is refused with a conformant -32022 rather than falling through — +// see the kill-switch branch for why, and for why server/discover is exempt +// and does still fall through. // // ValidateHeaderConsistency (Mcp-Method/Mcp-Name) only applies to Modern // requests: a Legacy request carrying a stray Mcp-Method/Mcp-Name header @@ -67,10 +75,34 @@ func (s *Server) classifyingHandler(next http.Handler) http.Handler { } // TEMPORARY kill-switch (default off): until Modern dispatch is - // conformance-validated, a well-formed Modern request falls through to - // the SDK path unless explicitly enabled. See issue #5959. + // conformance-validated, vMCP does not serve the Modern revision unless + // explicitly enabled. See issue #5959. + // + // Answer that conformantly here rather than letting the request reach the + // SDK. The draft's Streamable HTTP "Protocol Version Header" section + // requires a server that does not implement a requested version -- "whether + // the version is unknown to the server, or is a known version the server has + // chosen not to support" -- to reply 400 with an UnsupportedProtocolVersionError + // listing the versions it does support. A disabled kill switch is exactly the + // second case. Falling through instead yields go-sdk's stateful-server + // rejection, which is a 400 with a PLAIN-TEXT body whose text is Go-API advice + // for the server author ("set StreamableHTTPOptions.Stateless = true") -- not + // parseable as a protocol error and carrying no version list. + // + // server/discover is deliberately exempt: it is how a client learns which + // revisions a server supports, so rejecting it on version grounds would leave + // the client no way to negotiate down. go-sdk exempts it for the same reason, + // and its stateful path answers discover correctly, so the fall-through is + // still right for that one method. if !s.config.ModernDispatchEnabled { - next.ServeHTTP(w, r) + if parsed.Method == methodServerDiscover { + next.ServeHTTP(w, r) + return + } + mcpparser.WriteClassificationError(w, parsed.ID, &mcpparser.UnsupportedVersionError{ + Requested: mcpparser.MCPVersionModern, + Supported: []string{mcpparser.MCPVersionLegacy}, + }) return } diff --git a/pkg/vmcp/server/classification_test.go b/pkg/vmcp/server/classification_test.go index 5e15a59bfa..f727e627dd 100644 --- a/pkg/vmcp/server/classification_test.go +++ b/pkg/vmcp/server/classification_test.go @@ -102,11 +102,23 @@ func TestClassifyingHandler(t *testing.T) { }, { // Same well-formed Modern request, but with the kill-switch at its - // default (off): dispatch must not happen and the request falls - // through to the SDK path unchanged, byte-identical to pre-Modern- - // dispatch wire behavior. - name: "well-formed modern request falls through to next when the kill-switch is off", - parsed: wellFormedModernToolsList(), + // default (off): vMCP does not serve the Modern revision, which the + // draft says must be answered with 400 + UnsupportedProtocolVersion + // listing the supported versions. It must NOT reach next: falling + // through lands on go-sdk's stateful rejection, a plain-text 400 no + // client can parse as a protocol error. + name: "well-formed modern request is refused with -32022 when the kill-switch is off", + parsed: wellFormedModernToolsList(), + protocolHeader: mcpparser.MCPVersionModern, + wantCode: mcpparser.CodeUnsupportedProtocolVersion, + }, + { + // server/discover is the one exemption: it is how a client learns + // which revisions the server supports, so refusing it on version + // grounds would leave the client unable to negotiate down. go-sdk's + // stateful path answers discover, so the fall-through is correct. + name: "server/discover still falls through when the kill-switch is off", + parsed: wellFormedModernDiscover(), protocolHeader: mcpparser.MCPVersionModern, wantPassthrough: true, }, @@ -299,3 +311,18 @@ func wellFormedModernToolsList() *mcpparser.ParsedMCPRequest { MCPMethodHeader: "tools/list", } } + +// wellFormedModernDiscover is wellFormedModernToolsList for the one method the +// kill-switch branch exempts from the unsupported-version refusal. +func wellFormedModernDiscover() *mcpparser.ParsedMCPRequest { + return &mcpparser.ParsedMCPRequest{ + Method: methodServerDiscover, + ID: "1", + IsRequest: true, + Meta: map[string]any{ + metaKeyProtocolVersion: mcpparser.MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + MCPMethodHeader: methodServerDiscover, + } +} diff --git a/pkg/vmcp/server/modern_dispatch.go b/pkg/vmcp/server/modern_dispatch.go index 2efa391bec..d705fb8f14 100644 --- a/pkg/vmcp/server/modern_dispatch.go +++ b/pkg/vmcp/server/modern_dispatch.go @@ -86,7 +86,7 @@ func (s *Server) dispatchModern(w http.ResponseWriter, r *http.Request, parsed * s.dispatchModernResourceTemplatesList(ctx, w, parsed, identity) case "prompts/list": s.dispatchModernPromptsList(ctx, w, parsed, identity) - case "server/discover": + case methodServerDiscover: s.dispatchModernDiscover(ctx, w, parsed, identity) case "tools/call": s.dispatchModernToolCall(ctx, w, parsed, identity)