mcp: validate tool names (#640)

Implement SEP 986, enforcing restrictions on tool names. Since we're
already at v1 we couldn't turn this into a panic, so use an error log
instead. This is consistent with the typescript SDK. A note is left in
our 'rough edges' doc to panic in v2.

Fixes #621

Author: findleyr

docs/rough_edges.md

@@ -10,3 +10,9 @@ v2.
   of the SDK where event persistence and delivery were combined.
 
   **Workaround**: `Open` may be implemented as a no-op.
+
+- Enforcing valid tool names: with
+  [SEP-986](https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986)
+  landing after the SDK was at v1, we missed an opportunity to panic on invalid
+  tool names. Instead, we have to simply produce an error log. In v2, we should
+  panic.

internal/docs/rough_edges.src.md

@@ -9,3 +9,9 @@ v2.
   of the SDK where event persistence and delivery were combined.
 
   **Workaround**: `Open` may be implemented as a no-op.
+
+- Enforcing valid tool names: with
+  [SEP-986](https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986)
+  landing after the SDK was at v1, we missed an opportunity to panic on invalid
+  tool names. Instead, we have to simply produce an error log. In v2, we should
+  panic.

mcp/server.go

@@ -191,6 +191,9 @@ func (s *Server) RemovePrompts(names ...string) {
 // Most users should use the top-level function [AddTool], which handles all these
 // responsibilities.
 func (s *Server) AddTool(t *Tool, h ToolHandler) {
+	if err := validateToolName(t.Name); err != nil {
+		s.opts.Logger.Error(fmt.Sprintf("AddTool: invalid tool name %q: %v", t.Name, err))
+	}
 	if t.InputSchema == nil {
 		// This prevents the tool author from forgetting to write a schema where
 		// one should be provided. If we papered over this by supplying the empty

mcp/server_test.go

@@ -5,9 +5,11 @@
 package mcp
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"log"
+	"log/slog"
 	"slices"
 	"strings"
 	"testing"
@@ -490,6 +492,69 @@ func TestAddTool(t *testing.T) {
 	}
 }
 
+func TestAddToolNameValidation(t *testing.T) {
+	tests := []struct {
+		label             string
+		name              string
+		wantLogContaining string
+	}{
+		{
+			label:             "empty name",
+			name:              "",
+			wantLogContaining: `tool name cannot be empty`,
+		},
+		{
+			label:             "long name",
+			name:              strings.Repeat("a", 129),
+			wantLogContaining: "exceeds maximum length of 128 characters",
+		},
+		{
+			label:             "name with spaces",
+			name:              "get user profile",
+			wantLogContaining: `tool name contains invalid characters: \" \"`,
+		},
+		{
+			label:             "name with multiple invalid chars",
+			name:              "user name@domain,com",
+			wantLogContaining: `tool name contains invalid characters: \" \", \"@\", \",\"`,
+		},
+		{
+			label:             "name with unicode",
+			name:              "tool-ñame",
+			wantLogContaining: `tool name contains invalid characters: \"ñ\"`,
+		},
+		{
+			label:             "valid name",
+			name:              "valid-tool_name.123",
+			wantLogContaining: "", // No log expected
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.label, func(t *testing.T) {
+			var buf bytes.Buffer
+			s := NewServer(testImpl, &ServerOptions{
+				Logger: slog.New(slog.NewTextHandler(&buf, nil)),
+			})
+
+			// Use the generic AddTool as it also calls validateToolName.
+			AddTool(s, &Tool{Name: test.name}, func(context.Context, *CallToolRequest, any) (*CallToolResult, any, error) {
+				return nil, nil, nil
+			})
+
+			logOutput := buf.String()
+			if test.wantLogContaining != "" {
+				if !strings.Contains(logOutput, test.wantLogContaining) {
+					t.Errorf("log output =\n%s\nwant containing %q", logOutput, test.wantLogContaining)
+				}
+			} else {
+				if logOutput != "" {
+					t.Errorf("expected empty log output, got %q", logOutput)
+				}
+			}
+		})
+	}
+}
+
 type schema = jsonschema.Schema
 
 func testToolForSchema[In, Out any](t *testing.T, tool *Tool, in string, out Out, wantIn, wantOut any, wantErrContaining string) {

mcp/shared.go

@@ -34,12 +34,14 @@ const (
 	// It is the version that the client sends in the initialization request, and
 	// the default version used by the server.
 	latestProtocolVersion   = protocolVersion20250618
+	protocolVersion20251125 = "2025-11-25" // not yet released
 	protocolVersion20250618 = "2025-06-18"
 	protocolVersion20250326 = "2025-03-26"
 	protocolVersion20241105 = "2024-11-05"
 )
 
 var supportedProtocolVersions = []string{
+	protocolVersion20251125,
 	protocolVersion20250618,
 	protocolVersion20250326,
 	protocolVersion20241105,

mcp/tool.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	"github.com/google/jsonschema-go/jsonschema"
 )
@@ -101,3 +102,38 @@ func applySchema(data json.RawMessage, resolved *jsonschema.Resolved) (json.RawM
 	}
 	return data, nil
 }
+
+// validateToolName checks whether name is a valid tool name, reporting a
+// non-nil error if not.
+func validateToolName(name string) error {
+	if name == "" {
+		return fmt.Errorf("tool name cannot be empty")
+	}
+	if len(name) > 128 {
+		return fmt.Errorf("tool name exceeds maximum length of 128 characters (current: %d)", len(name))
+	}
+	// For consistency with other SDKs, report characters in the order the appear
+	// in the name.
+	var invalidChars []string
+	seen := make(map[rune]bool)
+	for _, r := range name {
+		if !validToolNameRune(r) {
+			if !seen[r] {
+				invalidChars = append(invalidChars, fmt.Sprintf("%q", string(r)))
+				seen[r] = true
+			}
+		}
+	}
+	if len(invalidChars) > 0 {
+		return fmt.Errorf("tool name contains invalid characters: %s", strings.Join(invalidChars, ", "))
+	}
+	return nil
+}
+
+// validToolNameRune reports whether r is valid within tool names.
+func validToolNameRune(r rune) bool {
+	return (r >= 'a' && r <= 'z') ||
+		(r >= 'A' && r <= 'Z') ||
+		(r >= '0' && r <= '9') ||
+		r == '_' || r == '-' || r == '.'
+}

mcp/tool_test.go

@@ -145,3 +145,53 @@ func TestToolErrorHandling(t *testing.T) {
 		}
 	})
 }
+
+func TestValidateToolName(t *testing.T) {
+	t.Run("valid", func(t *testing.T) {
+		validTests := []struct {
+			label    string
+			toolName string
+		}{
+			{"simple alphanumeric names", "getUser"},
+			{"names with underscores", "get_user_profile"},
+			{"names with dashes", "user-profile-update"},
+			{"names with dots", "admin.tools.list"},
+			{"mixed character names", "DATA_EXPORT_v2.1"},
+			{"single character names", "a"},
+			{"128 character names", strings.Repeat("a", 128)},
+		}
+		for _, test := range validTests {
+			t.Run(test.label, func(t *testing.T) {
+				if err := validateToolName(test.toolName); err != nil {
+					t.Errorf("validateToolName(%q) = %v, want nil", test.toolName, err)
+				}
+			})
+		}
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		invalidTests := []struct {
+			label             string
+			toolName          string
+			wantErrContaining string
+		}{
+			{"empty names", "", "tool name cannot be empty"},
+			{"names longer than 128 characters", strings.Repeat("a", 129), "tool name exceeds maximum length of 128 characters (current: 129)"},
+			{"names with spaces", "get user profile", `tool name contains invalid characters: " "`},
+			{"names with commas", "get,user,profile", `tool name contains invalid characters: ","`},
+			{"names with forward slashes", "user/profile/update", `tool name contains invalid characters: "/"`},
+			{"names with other special chars", "[email protected]", `tool name contains invalid characters: "@"`},
+			{"names with multiple invalid chars", "user name@domain,com", `tool name contains invalid characters: " ", "@", ","`},
+			{"names with unicode characters", "user-ñame", `tool name contains invalid characters: "ñ"`},
+		}
+		for _, test := range invalidTests {
+			t.Run(test.label, func(t *testing.T) {
+				if err := validateToolName(test.toolName); err == nil || !strings.Contains(err.Error(), test.wantErrContaining) {
+					t.Errorf("validateToolName(%q) = %v, want error containing %q", test.toolName, err, test.wantErrContaining)
+				}
+			})
+		}
+
+	})
+
+}