From 659fdb5b1a76b3e8770594a4bf24c5d02da5d87f Mon Sep 17 00:00:00 2001 From: n0liu Date: Wed, 29 Jul 2026 10:25:21 +0800 Subject: [PATCH 1/2] mcp: fix ListTools panic and null normalization on malformed tools filterValidTools panicked with a nil-pointer dereference when a server returned a malformed "tools":[null] response, and normalized a "tools":null response into a non-nil empty slice. Guard nil tool elements (log and exclude them) so untrusted server input can no longer crash the client, and preserve a nil tools slice as nil instead of laundering it into a non-nil empty slice. Fixes #1119 --- mcp/streamable_headers.go | 11 +++++++++++ mcp/streamable_headers_test.go | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index 3989c016..925db7d6 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -259,9 +259,20 @@ func generateParamHeaders(tool *Tool, params json.RawMessage) map[string]string // filterValidTools returns only tools that have valid // x-mcp-header annotations. Invalid tools are logged and excluded. func filterValidTools(logger *slog.Logger, tools []*Tool) []*Tool { + // Preserve a nil slice as nil rather than normalizing a malformed + // "tools":null response into a non-nil empty slice. + if tools == nil { + return nil + } logger = ensureLogger(logger) result := make([]*Tool, 0, len(tools)) for _, tool := range tools { + // A nil tool (e.g. from a malformed "tools":[null] response) is + // invalid; exclude it instead of dereferencing it. + if tool == nil { + logger.Error("excluding nil tool from tools/list") + continue + } if err := validateParamHeaderAnnotations(tool); err != nil { logger.Error("excluding tool from tools/list", "tool", tool.Name, "error", err) continue diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index 52d52447..93199d09 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -971,6 +971,29 @@ func TestFilterValidTools(t *testing.T) { } } +func TestFilterValidToolsNilHandling(t *testing.T) { + // A malformed "tools":[null] response must not panic the client, and a + // "tools":null response must not be normalized into a non-nil empty slice. + t.Run("nil tool element is excluded without panicking", func(t *testing.T) { + got := filterValidTools(nil, []*Tool{nil}) + if len(got) != 0 { + t.Fatalf("filterValidTools returned %d tools, want 0", len(got)) + } + }) + t.Run("nil slice is preserved as nil", func(t *testing.T) { + if got := filterValidTools(nil, nil); got != nil { + t.Fatalf("filterValidTools returned non-nil slice (len=%d) for nil input, want nil", len(got)) + } + }) + t.Run("valid tools are kept alongside nil elements", func(t *testing.T) { + valid := &Tool{Name: "valid"} + got := filterValidTools(nil, []*Tool{nil, valid, nil}) + if len(got) != 1 || got[0].Name != "valid" { + t.Fatalf("filterValidTools returned %d tools, want [valid]", len(got)) + } + }) +} + func TestSetStandardHeadersWithParamHeaders(t *testing.T) { toolSchema := map[string]any{ "type": "object", From b73df060d35e96f3d3a909b494624d71474f41a1 Mon Sep 17 00:00:00 2001 From: n0liu Date: Thu, 30 Jul 2026 09:31:28 +0800 Subject: [PATCH 2/2] mcp: address review feedback - Drop the nil-slice guard: returning nil vs an empty slice is not observable to callers, so the special case is unnecessary. The nil-*tool* guard that fixes the panic on "tools":[null] stays. - Merge the nil-handling checks into the existing TestFilterValidTools instead of a separate test function. --- mcp/streamable_headers.go | 5 ----- mcp/streamable_headers_test.go | 30 ++++++++---------------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index 925db7d6..7d06399b 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -259,11 +259,6 @@ func generateParamHeaders(tool *Tool, params json.RawMessage) map[string]string // filterValidTools returns only tools that have valid // x-mcp-header annotations. Invalid tools are logged and excluded. func filterValidTools(logger *slog.Logger, tools []*Tool) []*Tool { - // Preserve a nil slice as nil rather than normalizing a malformed - // "tools":null response into a non-nil empty slice. - if tools == nil { - return nil - } logger = ensureLogger(logger) result := make([]*Tool, 0, len(tools)) for _, tool := range tools { diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index 93199d09..def7ad1f 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -969,29 +969,15 @@ func TestFilterValidTools(t *testing.T) { t.Errorf("filterValidTools returned [%s, %s, %s, %s], want [valid, plain, nested-valid, valid-jsonschema]", result[0].Name, result[1].Name, result[2].Name, result[3].Name) } -} -func TestFilterValidToolsNilHandling(t *testing.T) { - // A malformed "tools":[null] response must not panic the client, and a - // "tools":null response must not be normalized into a non-nil empty slice. - t.Run("nil tool element is excluded without panicking", func(t *testing.T) { - got := filterValidTools(nil, []*Tool{nil}) - if len(got) != 0 { - t.Fatalf("filterValidTools returned %d tools, want 0", len(got)) - } - }) - t.Run("nil slice is preserved as nil", func(t *testing.T) { - if got := filterValidTools(nil, nil); got != nil { - t.Fatalf("filterValidTools returned non-nil slice (len=%d) for nil input, want nil", len(got)) - } - }) - t.Run("valid tools are kept alongside nil elements", func(t *testing.T) { - valid := &Tool{Name: "valid"} - got := filterValidTools(nil, []*Tool{nil, valid, nil}) - if len(got) != 1 || got[0].Name != "valid" { - t.Fatalf("filterValidTools returned %d tools, want [valid]", len(got)) - } - }) + // Regression for #1119: a malformed "tools":[null] response must not panic; + // nil tools are treated as invalid and excluded. + if got := filterValidTools(nil, []*Tool{nil}); len(got) != 0 { + t.Errorf("filterValidTools([nil]) returned %d tools, want 0", len(got)) + } + if got := filterValidTools(nil, []*Tool{nil, valid, nil}); len(got) != 1 || got[0].Name != "valid" { + t.Errorf("filterValidTools([nil, valid, nil]) = %d tools, want [valid]", len(got)) + } } func TestSetStandardHeadersWithParamHeaders(t *testing.T) {