From 7743b71b1cd5d617d16d59c958a0656d5c29df74 Mon Sep 17 00:00:00 2001
From: n0liu <landuoduo123@gmail.com>
Date: Thu, 30 Jul 2026 16:06:14 +0800
Subject: [PATCH] mcp: fix ListTools panic and null normalization on malformed
 tools (#1120)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes #1119.

`ClientSession.ListTools` post-processes the server response through
`filterValidTools`, which has two problems on malformed input:

1. **Nil-pointer panic on `"tools":[null]`.** A `nil` `*Tool` element
reaches `validateParamHeaderAnnotations(tool)`, which dereferences
`tool.InputSchema`, crashing the client on untrusted server input.
2. **`"tools":null` is normalized into a non-nil empty slice.**
`make([]*Tool, 0, len(tools))` is always non-nil, so a nil `Tools` slice
from a malformed response is silently laundered into valid-looking data.

Reproduction:

```go
filterValidTools(nil, []*Tool{nil}) // panics: nil pointer dereference
filterValidTools(nil, nil)          // returns non-nil slice (len 0), want nil
```

Fix (`mcp/streamable_headers.go`):

- Skip (log and exclude) nil tool elements instead of dereferencing
them, so a malformed `"tools":[null]` can no longer crash the client.
This is consistent with the function’s existing contract of excluding
invalid tools.
- Return early with `nil` for a nil input slice so a `"tools":null`
response is preserved as nil rather than normalized into an empty slice.

Added `TestFilterValidToolsNilHandling` covering both defects plus the
mixed valid/nil case. `go test ./mcp/` passes; `gofmt` and `go vet` are
clean.

Note on scope: this keeps the change minimal and within
`filterValidTools`’s existing signature. If maintainers would rather
surface a hard protocol error for a malformed `"tools":null` /
`"tools":[null]` response at the `ListTools` level (rather than
tolerating and filtering it), I’m happy to follow up with that larger
change.
---
 mcp/streamable_headers.go      | 6 ++++++
 mcp/streamable_headers_test.go | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go
index 3989c01..7d06399 100644
--- a/mcp/streamable_headers.go
+++ b/mcp/streamable_headers.go
@@ -262,6 +262,12 @@ func filterValidTools(logger *slog.Logger, tools []*Tool) []*Tool {
 	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 52d5244..def7ad1 100644
--- a/mcp/streamable_headers_test.go
+++ b/mcp/streamable_headers_test.go
@@ -969,6 +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)
 	}
+
+	// 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) {