diff --git a/docs/protocol.md b/docs/protocol.md index 77816716..909ca1c1 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -308,8 +308,11 @@ tool over HTTP, the SDK serializes the matching argument(s) as `Mcp-Param-` request headers (using `=?base64?...?=` encoding for non-ASCII values). On the server, the headers are validated against the body and either accepted or rejected with `-32020`. Properties so annotated must -be typed `string`, `integer`, or `boolean`; tools that fail validation at -registration time are silently dropped from `tools/list`. +be typed `string`, `integer`, or `boolean`, and must be reached from the +schema root through `properties` keys alone (not through `items`, `oneOf`, +`anyOf`, `allOf`, `not`, `if`/`then`/`else`, `$ref`, or similar keywords); +tools that fail validation at registration time are silently dropped from +`tools/list`. If a `tools/call` is rejected with `-32020`, `ClientSession.CallTool` fetches the tool's current definition with `tools/list` and retries once if the tool's `x-mcp-header` annotations changed. diff --git a/internal/docs/protocol.src.md b/internal/docs/protocol.src.md index e246f09e..0c005ab0 100644 --- a/internal/docs/protocol.src.md +++ b/internal/docs/protocol.src.md @@ -232,8 +232,11 @@ tool over HTTP, the SDK serializes the matching argument(s) as `Mcp-Param-` request headers (using `=?base64?...?=` encoding for non-ASCII values). On the server, the headers are validated against the body and either accepted or rejected with `-32020`. Properties so annotated must -be typed `string`, `integer`, or `boolean`; tools that fail validation at -registration time are silently dropped from `tools/list`. +be typed `string`, `integer`, or `boolean`, and must be reached from the +schema root through `properties` keys alone (not through `items`, `oneOf`, +`anyOf`, `allOf`, `not`, `if`/`then`/`else`, `$ref`, or similar keywords); +tools that fail validation at registration time are silently dropped from +`tools/list`. If a `tools/call` is rejected with `-32020`, `ClientSession.CallTool` fetches the tool's current definition with `tools/list` and retries once if the tool's `x-mcp-header` annotations changed. diff --git a/mcp/streamable_headers.go b/mcp/streamable_headers.go index 366686d6..2207d784 100644 --- a/mcp/streamable_headers.go +++ b/mcp/streamable_headers.go @@ -306,9 +306,20 @@ func filterValidTools(logger *slog.Logger, tools []*Tool) []*Tool { } // validateParamHeaderAnnotations checks that a tool's x-mcp-header annotations -// are valid. Annotations may appear on properties at any nesting -// depth within the inputSchema and must be unique across all of them. +// are valid. Annotations may appear on properties at any nesting depth within +// the inputSchema, as long as they are reached from the root through +// "properties" keys alone, and must be unique across all of them. func validateParamHeaderAnnotations(tool *Tool) error { + var schema map[string]any + if err := remarshal(tool.InputSchema, &schema); err == nil { + // The root is not a property, but its properties are reachable. + if _, ok := schema["x-mcp-header"]; ok { + return errors.New("x-mcp-header cannot be applied to the schema root") + } + if err := checkParamHeaderPlacement(schema, true); err != nil { + return err + } + } props := unmarshalSchemaProperties(tool.InputSchema) if len(props) == 0 { return nil @@ -353,6 +364,65 @@ func validateParamHeadersIn(props map[string]headerSchemaProperty, prefix string return nil } +// JSON Schema keywords, other than "properties", whose values are subschemas. +// The 2026-07-28 spec allows x-mcp-header only on a property reached from the +// schema root through "properties" keys alone, so an annotation found through +// any of these makes the tool invalid. +var ( + // The value is a schema or an array of schemas. + subschemaKeywords = []string{ + "additionalItems", "additionalProperties", "allOf", "anyOf", "contains", + "contentSchema", "else", "if", "items", "not", "oneOf", "prefixItems", + "propertyNames", "then", "unevaluatedItems", "unevaluatedProperties", + } + // The value is an object whose values are schemas. + subschemaMapKeywords = []string{"$defs", "definitions", "dependencies", "dependentSchemas", "patternProperties"} +) + +// checkParamHeaderPlacement reports an x-mcp-header annotation in schema, or +// in any of its subschemas, that is not on a property reached from the schema +// root through "properties" keys alone. reachable reports whether schema is the +// root or such a property; an annotation on the root itself is checked by the +// caller. +func checkParamHeaderPlacement(schema any, reachable bool) error { + m, ok := schema.(map[string]any) + if !ok { + return nil + } + if header, ok := m["x-mcp-header"]; ok && !reachable { + return fmt.Errorf("x-mcp-header %q is not on a property reached from the schema root through \"properties\" alone", fmt.Sprint(header)) + } + props, _ := m["properties"].(map[string]any) + for _, prop := range props { + if err := checkParamHeaderPlacement(prop, reachable); err != nil { + return err + } + } + for _, kw := range subschemaKeywords { + switch v := m[kw].(type) { + case map[string]any: + if err := checkParamHeaderPlacement(v, false); err != nil { + return err + } + case []any: + for _, sub := range v { + if err := checkParamHeaderPlacement(sub, false); err != nil { + return err + } + } + } + } + for _, kw := range subschemaMapKeywords { + subs, _ := m[kw].(map[string]any) + for _, sub := range subs { + if err := checkParamHeaderPlacement(sub, false); err != nil { + return err + } + } + } + return nil +} + // validateHeaderName checks that a header name matches the HTTP field-name // token syntax (1*tchar). func validateHeaderName(name string) error { diff --git a/mcp/streamable_headers_test.go b/mcp/streamable_headers_test.go index 00f845f7..07c0c3d7 100644 --- a/mcp/streamable_headers_test.go +++ b/mcp/streamable_headers_test.go @@ -912,6 +912,26 @@ func TestValidateToolParamHeaders(t *testing.T) { wantErr: true, wantErrSub: "primitive types", }, + { + name: "jsonschema.Schema x-mcp-header under items is rejected", + tool: &Tool{ + Name: "test", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "tags": { + Type: "array", + Items: &jsonschema.Schema{ + Type: "string", + Extra: map[string]any{"x-mcp-header": "Tag"}, + }, + }, + }, + }, + }, + wantErr: true, + wantErrSub: "not on a property reached", + }, } for _, tt := range tests { @@ -931,6 +951,94 @@ func TestValidateToolParamHeaders(t *testing.T) { } } +// TestValidateParamHeaderPlacement checks that an x-mcp-header annotation is +// rejected unless it is on a property reached from the schema root through +// "properties" keys alone, as the 2026-07-28 spec requires. See #1250. +func TestValidateParamHeaderPlacement(t *testing.T) { + tests := []struct { + name string + schema string + wantErr bool + }{ + { + name: "nested properties", + schema: `{"type":"object","properties":{"config":{"type":"object","properties":{"region":{"type":"string","x-mcp-header":"Region"}}}}}`, + }, + { + name: "annotation-like values that are not schemas", + schema: `{"type":"object","properties":{"x-mcp-header":{"type":"object","default":{"x-mcp-header":"A"},"const":{"x-mcp-header":"B"},"enum":[{"x-mcp-header":"C"}]}}}`, + }, + { + name: "schema root", + schema: `{"type":"string","x-mcp-header":"Root"}`, + wantErr: true, + }, + { + name: "items", + schema: `{"type":"object","properties":{"tags":{"type":"array","items":{"type":"string","x-mcp-header":"Tag"}}}}`, + wantErr: true, + }, + { + name: "oneOf", + schema: `{"type":"object","properties":{"region":{"oneOf":[{"type":"integer"},{"type":"string","x-mcp-header":"Region"}]}}}`, + wantErr: true, + }, + { + name: "anyOf", + schema: `{"type":"object","anyOf":[{"properties":{"region":{"type":"string","x-mcp-header":"Region"}}}]}`, + wantErr: true, + }, + { + name: "allOf", + schema: `{"type":"object","allOf":[{"properties":{"region":{"type":"string","x-mcp-header":"Region"}}}]}`, + wantErr: true, + }, + { + name: "not", + schema: `{"type":"object","properties":{"region":{"type":"string","not":{"type":"string","x-mcp-header":"Region"}}}}`, + wantErr: true, + }, + { + name: "if", + schema: `{"type":"object","if":{"properties":{"mode":{"type":"string","x-mcp-header":"Mode"}}}}`, + wantErr: true, + }, + { + name: "then", + schema: `{"type":"object","then":{"properties":{"region":{"type":"string","x-mcp-header":"Region"}}}}`, + wantErr: true, + }, + { + name: "else", + schema: `{"type":"object","else":{"properties":{"region":{"type":"string","x-mcp-header":"Region"}}}}`, + wantErr: true, + }, + { + name: "$defs entry reached through $ref", + schema: `{"type":"object","properties":{"region":{"$ref":"#/$defs/region"}},"$defs":{"region":{"type":"string","x-mcp-header":"Region"}}}`, + wantErr: true, + }, + { + name: "additionalProperties", + schema: `{"type":"object","additionalProperties":{"type":"string","x-mcp-header":"Extra"}}`, + wantErr: true, + }, + { + name: "patternProperties", + schema: `{"type":"object","patternProperties":{"^r":{"type":"string","x-mcp-header":"Region"}}}`, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateParamHeaderAnnotations(&Tool{Name: "test", InputSchema: json.RawMessage(tt.schema)}) + if gotErr := err != nil; gotErr != tt.wantErr { + t.Errorf("validateParamHeaderAnnotations() = %v, want error: %t", err, tt.wantErr) + } + }) + } +} + func TestFilterValidTools(t *testing.T) { valid := &Tool{ Name: "valid", @@ -994,7 +1102,20 @@ func TestFilterValidTools(t *testing.T) { }, } - result := filterValidTools(nil, []*Tool{valid, invalid, noAnnotation, nestedValid, validJsonSchema, invalidJsonSchema}) + unreachable := &Tool{ + Name: "unreachable", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "tags": map[string]any{ + "type": "array", + "items": map[string]any{"type": "string", "x-mcp-header": "Tag"}, + }, + }, + }, + } + + result := filterValidTools(nil, []*Tool{valid, invalid, noAnnotation, nestedValid, validJsonSchema, invalidJsonSchema, unreachable}) if len(result) != 4 { t.Fatalf("filterValidTools returned %d tools, want 4", len(result)) }