From e5717697e05537cabca91a693537b860c93f6430 Mon Sep 17 00:00:00 2001 From: ar8327 Date: Thu, 29 Oct 2026 18:27:11 +0800 Subject: [PATCH 1/2] mcp: avoid mutating client request parameters Reusing request params across ClientSession calls could send the first session's metadata on subsequent calls, and concurrent calls sharing params raced when CallTool set default Arguments or injectRequestMeta updated Meta. Shallow-copy the request struct before setting default Arguments or Meta, and clone params.GetMeta() with maps.Clone before injecting per-request metadata. Explicit caller metadata overrides are preserved. Fixes #1472 Signed-off-by: ar8327 --- mcp/client.go | 7 ++- mcp/client_params_test.go | 127 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 mcp/client_params_test.go diff --git a/mcp/client.go b/mcp/client.go index f81d0c14..8d2b37aa 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -549,8 +549,11 @@ func injectRequestMeta[T any, P interface { res := cs.state.InitializeResult if params == nil { params = new(T) + } else { + copy := *params + params = © } - m := params.GetMeta() + m := maps.Clone(params.GetMeta()) if m == nil { m = map[string]any{} } @@ -1380,6 +1383,8 @@ func (cs *ClientSession) CallTool(ctx context.Context, params *CallToolParams) ( } if params.Arguments == nil { // Avoid sending nil over the wire. + copy := *params + params = © params.Arguments = map[string]any{} } if tool := cs.lookupTool(params.Name); tool != nil { diff --git a/mcp/client_params_test.go b/mcp/client_params_test.go new file mode 100644 index 00000000..ea1ddad2 --- /dev/null +++ b/mcp/client_params_test.go @@ -0,0 +1,127 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package mcp + +import ( + "context" + "maps" + "sync" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestInjectRequestMetaPreservesParams(t *testing.T) { + cs := &ClientSession{ + client: NewClient(testImpl, nil), + state: clientSessionState{InitializeResult: &InitializeResult{ + ProtocolVersion: protocolVersion20260728, + }}, + } + for _, meta := range []Meta{ + nil, + {}, + {"custom": "value"}, + {MetaKeyProtocolVersion: "override", MetaKeyClientInfo: "client", MetaKeyClientCapabilities: "capabilities"}, + } { + params := &GetPromptParams{Name: "prompt", Meta: meta} + wantMeta := maps.Clone(meta) + got := injectRequestMeta(cs, params) + if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { + t.Errorf("caller metadata changed (-want +got):\n%s", diff) + } + if got == params { + t.Error("injectRequestMeta returned caller-owned params") + } + if got.Name != params.Name { + t.Error("prompt name changed") + } + for _, key := range []string{MetaKeyProtocolVersion, MetaKeyClientInfo, MetaKeyClientCapabilities} { + if _, ok := got.Meta[key]; !ok { + t.Errorf("missing metadata key %q", key) + } + if value, ok := wantMeta[key]; ok && !cmp.Equal(value, got.Meta[key]) { + t.Errorf("explicit metadata key %q was overwritten", key) + } + } + got.Meta["custom"] = "changed" + if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { + t.Errorf("metadata map is shared (-want +got):\n%s", diff) + } + } + if got := injectRequestMeta(cs, (*GetPromptParams)(nil)); got == nil || got.Meta == nil { + t.Error("nil params did not receive metadata") + } +} + +func TestCallToolPreservesSharedParams(t *testing.T) { + for _, version := range []string{protocolVersion20251125, protocolVersion20260728} { + t.Run(version, func(t *testing.T) { + ctx := t.Context() + s := NewServer(testImpl, nil) + s.AddTool(&Tool{Name: "ping", InputSchema: map[string]any{"type": "object"}}, + func(_ context.Context, req *CallToolRequest) (*CallToolResult, error) { + name := "legacy" + if info, ok := req.Params.Meta[MetaKeyClientInfo].(map[string]any); ok { + name, _ = info["name"].(string) + } + return &CallToolResult{Content: []Content{&TextContent{Text: name}}}, nil + }) + var clients []*ClientSession + for _, name := range []string{"client-a", "client-b"} { + ct, st := NewInMemoryTransports() + ss, err := s.Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = ss.Close() }) + cs, err := NewClient(&Implementation{Name: name, Version: "1"}, nil).Connect(ctx, ct, + &ClientSessionOptions{ProtocolVersion: version}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cs.Close() }) + clients = append(clients, cs) + } + call := func(cs *ClientSession, params *CallToolParams) { + t.Helper() + result, err := cs.CallTool(ctx, params) + if err != nil { + t.Error(err) + return + } + want := "legacy" + if version == protocolVersion20260728 { + want = cs.client.impl.Name + } + if got := result.Content[0].(*TextContent).Text; got != want { + t.Errorf("server received client name %q, want %q", got, want) + } + } + for _, concurrent := range []bool{false, true} { + for _, meta := range []Meta{nil, {"custom": "value"}} { + params := &CallToolParams{Name: "ping", Meta: meta} + want := &CallToolParams{Name: "ping", Meta: maps.Clone(meta)} + if concurrent { + var wg sync.WaitGroup + for range 4 { + for _, cs := range clients { + wg.Go(func() { call(cs, params) }) + } + } + wg.Wait() + } else { + for _, cs := range clients { + call(cs, params) + } + } + if diff := cmp.Diff(want, params); diff != "" { + t.Errorf("caller params changed (-want +got):\n%s", diff) + } + } + } + }) + } +} From 1e119a58978ff36325f3af594793810fcdb72d6d Mon Sep 17 00:00:00 2001 From: ar8327 Date: Thu, 29 Oct 2026 20:24:27 +0800 Subject: [PATCH 2/2] test(mcp): move request param ownership tests to client_test.go Signed-off-by: ar8327 --- mcp/client_params_test.go | 127 -------------------------------------- mcp/client_test.go | 115 ++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 127 deletions(-) delete mode 100644 mcp/client_params_test.go diff --git a/mcp/client_params_test.go b/mcp/client_params_test.go deleted file mode 100644 index ea1ddad2..00000000 --- a/mcp/client_params_test.go +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright 2025 The Go MCP SDK Authors. All rights reserved. -// Use of this source code is governed by the license -// that can be found in the LICENSE file. - -package mcp - -import ( - "context" - "maps" - "sync" - "testing" - - "github.com/google/go-cmp/cmp" -) - -func TestInjectRequestMetaPreservesParams(t *testing.T) { - cs := &ClientSession{ - client: NewClient(testImpl, nil), - state: clientSessionState{InitializeResult: &InitializeResult{ - ProtocolVersion: protocolVersion20260728, - }}, - } - for _, meta := range []Meta{ - nil, - {}, - {"custom": "value"}, - {MetaKeyProtocolVersion: "override", MetaKeyClientInfo: "client", MetaKeyClientCapabilities: "capabilities"}, - } { - params := &GetPromptParams{Name: "prompt", Meta: meta} - wantMeta := maps.Clone(meta) - got := injectRequestMeta(cs, params) - if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { - t.Errorf("caller metadata changed (-want +got):\n%s", diff) - } - if got == params { - t.Error("injectRequestMeta returned caller-owned params") - } - if got.Name != params.Name { - t.Error("prompt name changed") - } - for _, key := range []string{MetaKeyProtocolVersion, MetaKeyClientInfo, MetaKeyClientCapabilities} { - if _, ok := got.Meta[key]; !ok { - t.Errorf("missing metadata key %q", key) - } - if value, ok := wantMeta[key]; ok && !cmp.Equal(value, got.Meta[key]) { - t.Errorf("explicit metadata key %q was overwritten", key) - } - } - got.Meta["custom"] = "changed" - if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { - t.Errorf("metadata map is shared (-want +got):\n%s", diff) - } - } - if got := injectRequestMeta(cs, (*GetPromptParams)(nil)); got == nil || got.Meta == nil { - t.Error("nil params did not receive metadata") - } -} - -func TestCallToolPreservesSharedParams(t *testing.T) { - for _, version := range []string{protocolVersion20251125, protocolVersion20260728} { - t.Run(version, func(t *testing.T) { - ctx := t.Context() - s := NewServer(testImpl, nil) - s.AddTool(&Tool{Name: "ping", InputSchema: map[string]any{"type": "object"}}, - func(_ context.Context, req *CallToolRequest) (*CallToolResult, error) { - name := "legacy" - if info, ok := req.Params.Meta[MetaKeyClientInfo].(map[string]any); ok { - name, _ = info["name"].(string) - } - return &CallToolResult{Content: []Content{&TextContent{Text: name}}}, nil - }) - var clients []*ClientSession - for _, name := range []string{"client-a", "client-b"} { - ct, st := NewInMemoryTransports() - ss, err := s.Connect(ctx, st, nil) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = ss.Close() }) - cs, err := NewClient(&Implementation{Name: name, Version: "1"}, nil).Connect(ctx, ct, - &ClientSessionOptions{ProtocolVersion: version}) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = cs.Close() }) - clients = append(clients, cs) - } - call := func(cs *ClientSession, params *CallToolParams) { - t.Helper() - result, err := cs.CallTool(ctx, params) - if err != nil { - t.Error(err) - return - } - want := "legacy" - if version == protocolVersion20260728 { - want = cs.client.impl.Name - } - if got := result.Content[0].(*TextContent).Text; got != want { - t.Errorf("server received client name %q, want %q", got, want) - } - } - for _, concurrent := range []bool{false, true} { - for _, meta := range []Meta{nil, {"custom": "value"}} { - params := &CallToolParams{Name: "ping", Meta: meta} - want := &CallToolParams{Name: "ping", Meta: maps.Clone(meta)} - if concurrent { - var wg sync.WaitGroup - for range 4 { - for _, cs := range clients { - wg.Go(func() { call(cs, params) }) - } - } - wg.Wait() - } else { - for _, cs := range clients { - call(cs, params) - } - } - if diff := cmp.Diff(want, params); diff != "" { - t.Errorf("caller params changed (-want +got):\n%s", diff) - } - } - } - }) - } -} diff --git a/mcp/client_test.go b/mcp/client_test.go index bc6dc86e..12bf52e7 100644 --- a/mcp/client_test.go +++ b/mcp/client_test.go @@ -12,7 +12,9 @@ import ( "errors" "fmt" "log/slog" + "maps" "slices" + "sync" "sync/atomic" "testing" @@ -1396,3 +1398,116 @@ func TestListRootsCapabilityDisabled(t *testing.T) { }) } } + +func TestInjectRequestMetaPreservesParams(t *testing.T) { + cs := &ClientSession{ + client: NewClient(testImpl, nil), + state: clientSessionState{InitializeResult: &InitializeResult{ + ProtocolVersion: protocolVersion20260728, + }}, + } + for _, meta := range []Meta{ + nil, + {}, + {"custom": "value"}, + {MetaKeyProtocolVersion: "override", MetaKeyClientInfo: "client", MetaKeyClientCapabilities: "capabilities"}, + } { + params := &GetPromptParams{Name: "prompt", Meta: meta} + wantMeta := maps.Clone(meta) + got := injectRequestMeta(cs, params) + if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { + t.Errorf("caller metadata changed (-want +got):\n%s", diff) + } + if got == params { + t.Error("injectRequestMeta returned caller-owned params") + } + if got.Name != params.Name { + t.Error("prompt name changed") + } + for _, key := range []string{MetaKeyProtocolVersion, MetaKeyClientInfo, MetaKeyClientCapabilities} { + if _, ok := got.Meta[key]; !ok { + t.Errorf("missing metadata key %q", key) + } + if value, ok := wantMeta[key]; ok && !cmp.Equal(value, got.Meta[key]) { + t.Errorf("explicit metadata key %q was overwritten", key) + } + } + got.Meta["custom"] = "changed" + if diff := cmp.Diff(wantMeta, params.Meta); diff != "" { + t.Errorf("metadata map is shared (-want +got):\n%s", diff) + } + } + if got := injectRequestMeta(cs, (*GetPromptParams)(nil)); got == nil || got.Meta == nil { + t.Error("nil params did not receive metadata") + } +} + +func TestCallToolPreservesSharedParams(t *testing.T) { + for _, version := range []string{protocolVersion20251125, protocolVersion20260728} { + t.Run(version, func(t *testing.T) { + ctx := t.Context() + s := NewServer(testImpl, nil) + s.AddTool(&Tool{Name: "ping", InputSchema: map[string]any{"type": "object"}}, + func(_ context.Context, req *CallToolRequest) (*CallToolResult, error) { + name := "legacy" + if info, ok := req.Params.Meta[MetaKeyClientInfo].(map[string]any); ok { + name, _ = info["name"].(string) + } + return &CallToolResult{Content: []Content{&TextContent{Text: name}}}, nil + }) + var clients []*ClientSession + for _, name := range []string{"client-a", "client-b"} { + ct, st := NewInMemoryTransports() + ss, err := s.Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = ss.Close() }) + cs, err := NewClient(&Implementation{Name: name, Version: "1"}, nil).Connect(ctx, ct, + &ClientSessionOptions{ProtocolVersion: version}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cs.Close() }) + clients = append(clients, cs) + } + call := func(cs *ClientSession, params *CallToolParams) { + t.Helper() + result, err := cs.CallTool(ctx, params) + if err != nil { + t.Error(err) + return + } + want := "legacy" + if version == protocolVersion20260728 { + want = cs.client.impl.Name + } + if got := result.Content[0].(*TextContent).Text; got != want { + t.Errorf("server received client name %q, want %q", got, want) + } + } + for _, concurrent := range []bool{false, true} { + for _, meta := range []Meta{nil, {"custom": "value"}} { + params := &CallToolParams{Name: "ping", Meta: meta} + want := &CallToolParams{Name: "ping", Meta: maps.Clone(meta)} + if concurrent { + var wg sync.WaitGroup + for range 4 { + for _, cs := range clients { + wg.Go(func() { call(cs, params) }) + } + } + wg.Wait() + } else { + for _, cs := range clients { + call(cs, params) + } + } + if diff := cmp.Diff(want, params); diff != "" { + t.Errorf("caller params changed (-want +got):\n%s", diff) + } + } + } + }) + } +}