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
This commit is contained in:
Robert Findley 2025-11-19 17:01:50 -05:00 • committed by GitHub
commit 1943780255
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 168 additions and 0 deletions

View file

@ -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

View file

@ -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) {

View file

@ -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,

View file

@ -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 == '.'
}

View file

@ -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", "user@domain.com", `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)
}
})
}
})
}