From dbb7c45ef3c5e3fe55cdd1ca13ec88ec41f32376 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:42:27 +0700 Subject: [PATCH] mcp: limit SSE discover supportedVersions to SSE-defined revisions HTTP+SSE is only defined by older MCP revisions. SSEServerTransport now implements ProtocolVersionSupporter so server/discover does not advertise 2026-07-28 (stdio + Streamable HTTP only). Also avoid marking the session initialized from discover when the requested version is not transport-supported, so Modern-first clients can fall back to the legacy initialize handshake over SSE. Fixes #1112 --- mcp/server.go | 30 +++++++++++++---------- mcp/sse.go | 7 ++++++ mcp/sse_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/mcp/server.go b/mcp/server.go index b5805488..602bc283 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -888,18 +888,24 @@ func (s *Server) discover(_ context.Context, req *ServerRequest[*DiscoverParams] if versions == nil { versions = slices.Clone(supportedProtocolVersions) } - // Read the request-scoped identity/capabilities before acquiring the - // session lock: these accessors may fall back to Session.InitializeParams - // (which also locks Session.mu), so calling them from inside updateState - // would self-deadlock. - init := &InitializeParams{ - ProtocolVersion: req.ProtocolVersion(), - Capabilities: req.ClientCapabilities(), - ClientInfo: req.ClientInfo(), - } - req.Session.updateState(func(state *ServerSessionState) { - state.InitializeParams = init - }) + // Only establish session identity via discover when the transport can + // actually serve the requested version. Otherwise the client may fall + // back to the legacy initialize handshake (e.g. HTTP+SSE, which does not + // define 2026-07-28), and must not already appear initialized. + if slices.Contains(versions, req.ProtocolVersion()) { + // Read the request-scoped identity/capabilities before acquiring the + // session lock: these accessors may fall back to Session.InitializeParams + // (which also locks Session.mu), so calling them from inside updateState + // would self-deadlock. + init := &InitializeParams{ + ProtocolVersion: req.ProtocolVersion(), + Capabilities: req.ClientCapabilities(), + ClientInfo: req.ClientInfo(), + } + req.Session.updateState(func(state *ServerSessionState) { + state.InitializeParams = init + }) + } res := &DiscoverResult{ SupportedVersions: versions, Capabilities: s.capabilities(), diff --git a/mcp/sse.go b/mcp/sse.go index 7efaa95f..fb327567 100644 --- a/mcp/sse.go +++ b/mcp/sse.go @@ -189,6 +189,13 @@ func (t *SSEServerTransport) Connect(context.Context) (Connection, error) { return &sseServerConn{t: t}, nil } +// SupportsProtocolVersion reports whether the HTTP+SSE transport can serve the +// given protocol version. MCP 2026-07-28 defines only the stdio and Streamable +// HTTP bindings, so this (deprecated) transport does not advertise that revision. +func (t *SSEServerTransport) SupportsProtocolVersion(version string) bool { + return version < protocolVersion20260728 +} + func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // DNS rebinding protection: auto-enabled for localhost servers. // See: https://modelcontextprotocol.io/specification/2025-11-25/basic/security_best_practices#local-mcp-server-compromise diff --git a/mcp/sse_test.go b/mcp/sse_test.go index 86b2cf1b..c0ff445a 100644 --- a/mcp/sse_test.go +++ b/mcp/sse_test.go @@ -12,12 +12,76 @@ import ( "net" "net/http" "net/http/httptest" + "slices" "sync/atomic" "testing" "github.com/google/go-cmp/cmp" ) +// TestSSEServerTransport_SupportedVersions verifies that the deprecated +// HTTP+SSE transport does not advertise protocol revisions that only define +// stdio and Streamable HTTP (see #1112). +func TestSSEServerTransport_SupportedVersions(t *testing.T) { + var tr SSEServerTransport + versions := filterSupportedVersions(&tr) + if slices.Contains(versions, protocolVersion20260728) { + t.Errorf("filterSupportedVersions(SSEServerTransport) = %v, must not include %q", + versions, protocolVersion20260728) + } + if !slices.Contains(versions, protocolVersion20241105) { + t.Errorf("filterSupportedVersions(SSEServerTransport) = %v, want %q (SSE-defined revision)", + versions, protocolVersion20241105) + } + if len(versions) == 0 { + t.Error("filterSupportedVersions(SSEServerTransport) is empty; want legacy SSE revisions") + } + for _, v := range supportedProtocolVersions { + got := tr.SupportsProtocolVersion(v) + want := v < protocolVersion20260728 + if got != want { + t.Errorf("SupportsProtocolVersion(%q) = %v, want %v", v, got, want) + } + } +} + +// TestSSEHandler_DiscoverFallsBackToInitialize checks that a Modern-first +// client connecting over HTTP+SSE does not stick on 2026-07-28: discover +// advertises only SSE-defined revisions, and Connect falls back to legacy +// initialize successfully. +func TestSSEHandler_DiscoverFallsBackToInitialize(t *testing.T) { + ctx := context.Background() + server := NewServer(testImpl, nil) + sseHandler := NewSSEHandler(func(*http.Request) *Server { return server }, nil) + httpServer := httptest.NewServer(sseHandler) + defer httpServer.Close() + + client := NewClient(testImpl, nil) + // Default Client.Connect probes with the latest protocol version (2026-07-28). + cs, err := client.Connect(ctx, &SSEClientTransport{Endpoint: httpServer.URL}, nil) + if err != nil { + t.Fatalf("Connect over SSE with modern client: %v", err) + } + defer cs.Close() + + ir := cs.InitializeResult() + if ir == nil { + t.Fatal("InitializeResult is nil after Connect") + } + if ir.ProtocolVersion == protocolVersion20260728 { + t.Errorf("InitializeResult.ProtocolVersion = %q; HTTP+SSE must not negotiate that revision", + ir.ProtocolVersion) + } + if ir.ProtocolVersion != protocolVersion20251125 { + // Client.Connect falls back to protocolVersion20251125 for legacy initialize. + t.Errorf("InitializeResult.ProtocolVersion = %q, want %q (legacy fallback)", + ir.ProtocolVersion, protocolVersion20251125) + } + if err := cs.Ping(ctx, nil); err != nil { + t.Errorf("Ping after SSE fallback initialize: %v", err) + } +} + func TestSSEServer(t *testing.T) { for _, closeServerFirst := range []bool{false, true} { t.Run(fmt.Sprintf("closeServerFirst=%t", closeServerFirst), func(t *testing.T) {