From a8ee8d86a19926f3b9db1d3ffc89f3fde66c612c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=82=9D=E8=B0=A7?= Date: Fri, 3 Jul 2026 09:51:30 +0800 Subject: [PATCH] fix(mcp): guard nil initializeParams on id:null with new-protocol _meta MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a request carries id:null (IsCall() returns false), validateRequestMeta returns initializeParams=nil but usesNewProtocol=true. The subsequent protocol-version check dereferences the nil pointer, causing SIGSEGV. Add a nil guard matching the pattern already used at the initialization check (line 1846). Regression test exercises completion/complete, prompts/get, and tools/call with id:null + new-protocol _meta — all three previously panicked before the fix. Fixes #1043 --- mcp/server.go | 2 +- mcp/server_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/mcp/server.go b/mcp/server.go index 21b5a722..e19418d5 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1812,7 +1812,7 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, return nil, perRequestErr } - if validatedMeta.usesNewProtocol && + if validatedMeta.usesNewProtocol && validatedMeta.initializeParams != nil && !slices.Contains(supportedProtocolVersions, validatedMeta.initializeParams.ProtocolVersion) { data, _ := json.Marshal(UnsupportedProtocolVersionData{ Supported: supportedProtocolVersions, diff --git a/mcp/server_test.go b/mcp/server_test.go index fc9db1f5..4e2d8e16 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -1451,3 +1451,81 @@ func TestServerSessionHandle_RejectsRemovedMethodsOnNewProtocol(t *testing.T) { }) } } + +// TestServerSessionHandle_NullIDWithNewProtocol_NoPanic verifies that sending +// a request with id:null (which makes IsCall() return false, triggering the +// notification path in validateRequestMeta) and per-request _meta for the new +// protocol does not panic due to nil initializeParams dereference. +// +// Regression test for https://github.com/modelcontextprotocol/go-sdk/issues/1043. +func TestServerSessionHandle_NullIDWithNewProtocol_NoPanic(t *testing.T) { + ss := &ServerSession{server: NewServer(testImpl, nil)} + + tests := []struct { + name string + method string + params any + }{ + { + name: "completion/complete with id:null and new-protocol _meta", + method: "completion/complete", + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "repro", "version": "0.1.0"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + "ref": map[string]any{"type": "ref/prompt", "name": "test"}, + "argument": map[string]any{"name": "arg", "value": "x"}, + }, + }, + { + name: "prompts/get with id:null and new-protocol _meta", + method: "prompts/get", + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "repro", "version": "0.1.0"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + "name": "test", + }, + }, + { + name: "tools/call with id:null and new-protocol _meta", + method: "tools/call", + params: map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "repro", "version": "0.1.0"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + "name": "test", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Use zero-value ID (id:null) — IsValid() returns false, IsCall() returns false. + req := &jsonrpc.Request{ + ID: jsonrpc.ID{}, // id:null + Method: tc.method, + Params: mustMarshal(tc.params), + } + + // Verify IsCall() is false — this is the code path that triggers the bug. + if req.IsCall() { + t.Fatal("expected IsCall() to be false for zero-value ID") + } + + _, err := ss.handle(context.Background(), req) + + // The server should NOT panic. It may return an error, and that's fine — + // we just need to verify the nil-dereference crash is gone. Common + // error responses include "method invalid during initialization" since + // id:null is not a proper call. + _ = err // no panic = pass + }) + } +}