From ce51cdbecc7f445099f16b01d2d0a1c01bf7f865 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 27 Jul 2026 12:38:14 +0000 Subject: [PATCH 1/2] Reject backend tools with invalid x-mcp-header SEP-2243 lets an MCP server designate individual tool parameters for mirroring into HTTP request headers, via an x-mcp-header annotation in the tool's inputSchema. The extension constrains the annotation's value, and requires a Streamable HTTP client to reject any tool definition that violates those constraints. vMCP is its backends' client and had no such validation: nothing in the repo matched x-mcp-header at all. Left unchecked, an invalid annotation is not contained to the tool that carries it. vMCP republishes aggregated backend schemas to its own downstream clients, so a conformant downstream client would reject the entire aggregated tools/list, taking every other backend's tools down with it. A CRLF in an annotation is worse than cosmetic once mirroring lands, since the value becomes an outgoing header name. Validate at newCapabilityListFromMCP -- the single ingestion seam the Legacy and Modern paths share -- and drop only the offending tool, which is the narrower failure. The annotation's vocabulary and constraint checks live in one place, pkg/mcp, so the ingestion check and the call-time header derivation that follows cannot drift in their reading of the spec. The traversal is depth-capped: annotations are legal at any nesting depth and a backend can advertise any tool list it likes, so an unbounded recursive walk would be a stack-exhaustion vector. Refs #6002 --- pkg/mcp/xmcpheader.go | 295 ++++++++++++++++ pkg/mcp/xmcpheader_test.go | 342 +++++++++++++++++++ pkg/vmcp/client/client.go | 26 +- pkg/vmcp/client/xmcpheader_ingestion_test.go | 115 +++++++ 4 files changed, 773 insertions(+), 5 deletions(-) create mode 100644 pkg/mcp/xmcpheader.go create mode 100644 pkg/mcp/xmcpheader_test.go create mode 100644 pkg/vmcp/client/xmcpheader_ingestion_test.go diff --git a/pkg/mcp/xmcpheader.go b/pkg/mcp/xmcpheader.go new file mode 100644 index 0000000000..62af5f6484 --- /dev/null +++ b/pkg/mcp/xmcpheader.go @@ -0,0 +1,295 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "errors" + "fmt" + "slices" + "strings" +) + +// SEP-2243 ("HTTP standardization") lets an MCP server mark individual tool +// parameters for mirroring into HTTP request headers, via an x-mcp-header +// annotation inside the parameter's schema in the tool's inputSchema. A server +// MAY use the annotation; a client MUST honour it, sending each designated +// parameter's value as the header Mcp-Param-{name} on the tools/call request. A +// server that designated a parameter and did not receive its header rejects the +// call with -32020. +// +// This file owns the annotation's vocabulary and its validation. It is +// deliberately the single place that reads x-mcp-header, so the two consumers -- +// ingestion-time validation of a backend's advertised tools, and call-time +// derivation of the outgoing headers -- cannot drift in their reading of the +// constraints. (Compare the "third independent copy" problem called out for the +// reserved _meta keys in #5986.) +const ( + // XMCPHeaderAnnotation is the JSON Schema extension key a server sets on a + // tool parameter to designate it for header mirroring. Its value is the + // header's name suffix, NOT the full header name. + XMCPHeaderAnnotation = "x-mcp-header" + + // ParamHeaderPrefix prefixes every mirrored parameter header: an annotation + // of "Region" is sent as "Mcp-Param-Region". + ParamHeaderPrefix = "Mcp-Param-" +) + +// maxSchemaDepth bounds the inputSchema traversal. Annotations are legal at any +// nesting depth, so the walk has to recurse, and the schema is attacker-supplied +// from vMCP's perspective (a backend can advertise any tool list it likes). A +// depth cap keeps a hostile or accidentally-recursive schema from exhausting the +// stack. 64 is far past any hand-written tool schema. +const maxSchemaDepth = 64 + +// ErrSchemaTooDeep is returned when an inputSchema nests past maxSchemaDepth. +// It is deliberately distinguishable from a constraint violation: the schema may +// be perfectly valid and merely beyond what this walk will inspect, so a caller +// that wants to fail open on depth alone can single it out. +var ErrSchemaTooDeep = errors.New("tool inputSchema exceeds maximum inspection depth") + +// ParamHeader is one x-mcp-header annotation found in a tool's inputSchema. +type ParamHeader struct { + // Path is the property path from the root of inputSchema to the annotated + // parameter, e.g. ["filter", "region"] for a nested property. Length is + // always >= 1. + Path []string + + // Name is the annotation's value -- the suffix appended to + // ParamHeaderPrefix, not the full header name. Use HeaderName for that. + Name string + + // Type is the annotated parameter's declared JSON Schema type, guaranteed by + // validation to be one of "string", "integer", or "boolean". + Type string +} + +// HeaderName is the full HTTP header name this annotation mirrors into. +func (p ParamHeader) HeaderName() string { + return ParamHeaderPrefix + p.Name +} + +// ParamHeaders walks a tool's inputSchema and returns every x-mcp-header +// annotation it declares, in a deterministic order (by path). A schema with no +// annotations -- the overwhelmingly common case -- returns nil, nil. +// +// It returns an error when the schema violates any of SEP-2243's constraints on +// the annotation: +// +// - the value must be a non-empty string; +// - it must be a valid HTTP field-name token (RFC 9110 tchar), which also +// excludes control characters and whitespace; +// - it must be unique, case-insensitively, within the whole inputSchema +// (HTTP field names are case-insensitive, so two spellings would collide on +// the wire); +// - it may only annotate a parameter whose declared type is "string", +// "integer", or "boolean" -- notably NOT "number", which SEP-2243 excludes +// because a float has no canonical wire spelling. +// +// A missing or non-string "type" on an annotated parameter is treated as a +// violation. SEP-2243 permits the annotation only on primitive parameters, and +// an undeclared type is not a declared primitive; equally, mirroring cannot +// serialize a value whose type it does not know. This is the strict reading: +// it rejects the tool rather than guessing a spelling for the header value. +// +// Traversal covers "properties", array "items", and the "oneOf"/"anyOf"/"allOf" +// combinators, so an annotation is found wherever SEP-2243 allows one. Nesting +// past maxSchemaDepth yields ErrSchemaTooDeep. +func ParamHeaders(schema map[string]any) ([]ParamHeader, error) { + if len(schema) == 0 { + return nil, nil + } + // seen maps the case-folded annotation value to the path that first claimed + // it, so a duplicate can name both colliding parameters in its error. + seen := map[string]string{} + var found []ParamHeader + if err := walkSchema(schema, nil, 0, seen, &found); err != nil { + return nil, err + } + if len(found) == 0 { + return nil, nil + } + // Deterministic order so callers (and tests) see a stable sequence + // regardless of Go's map iteration order. + slices.SortFunc(found, func(a, b ParamHeader) int { + return strings.Compare(strings.Join(a.Path, "."), strings.Join(b.Path, ".")) + }) + return found, nil +} + +// ValidateParamHeaders reports whether a tool's inputSchema declares only +// SEP-2243-conformant x-mcp-header annotations. It is ParamHeaders with the +// annotations discarded, for the ingestion-time check where only the verdict +// matters. +func ValidateParamHeaders(schema map[string]any) error { + _, err := ParamHeaders(schema) + return err +} + +// walkSchema recurses one schema node, appending any annotation it finds to +// found. path is the property path to this node ("" at the root, which cannot +// itself be annotated -- an annotation designates a parameter, and the root is +// the parameter object). +func walkSchema(node map[string]any, path []string, depth int, seen map[string]string, found *[]ParamHeader) error { + if depth > maxSchemaDepth { + return fmt.Errorf("%w (%d)", ErrSchemaTooDeep, maxSchemaDepth) + } + + // An annotation on the root node designates no parameter, so it is ignored + // rather than treated as an error: len(path) == 0 only at the root. + if len(path) > 0 { + if raw, ok := node[XMCPHeaderAnnotation]; ok { + hdr, err := parseAnnotation(raw, node, path, seen) + if err != nil { + return err + } + *found = append(*found, hdr) + } + } + + return walkChildren(node, path, depth, seen, found) +} + +// walkChildren recurses into every sub-schema of node that SEP-2243 allows an +// annotation to appear in: object properties, array element schemas, and the +// oneOf/anyOf/allOf combinator branches. Split from walkSchema to keep each +// within the cyclomatic limit. +func walkChildren(node map[string]any, path []string, depth int, seen map[string]string, found *[]ParamHeader) error { + if props, ok := node["properties"].(map[string]any); ok { + for name, sub := range props { + subSchema, ok := sub.(map[string]any) + if !ok { + continue // not a schema object; nothing to inspect + } + if err := walkSchema(subSchema, childPath(path, name), depth+1, seen, found); err != nil { + return err + } + } + } + + // Array element schemas: "items" is a schema object in the JSON Schema + // dialect MCP uses. The element carries no property name of its own, so it + // inherits the array's path with an "[]" marker for legibility in errors. + if items, ok := node["items"].(map[string]any); ok { + if err := walkSchema(items, childPath(path, "[]"), depth+1, seen, found); err != nil { + return err + } + } + + return walkCombinators(node, path, depth, seen, found) +} + +// walkCombinators recurses into the oneOf/anyOf/allOf branches of node. These +// carry real schemas in this repo's backend tool sets (#5976 fixed ingestion +// dropping them), so an annotation inside a branch must be validated like any +// other. +func walkCombinators( + node map[string]any, path []string, depth int, seen map[string]string, found *[]ParamHeader, +) error { + for _, combinator := range []string{"oneOf", "anyOf", "allOf"} { + branches, ok := node[combinator].([]any) + if !ok { + continue + } + for i, branch := range branches { + branchSchema, ok := branch.(map[string]any) + if !ok { + continue + } + marker := fmt.Sprintf("%s[%d]", combinator, i) + if err := walkSchema(branchSchema, childPath(path, marker), depth+1, seen, found); err != nil { + return err + } + } + } + return nil +} + +// childPath returns path with seg appended, always in freshly allocated storage. +// Appending to path directly would let sibling recursions share (and overwrite) +// one backing array, so a path captured deeper in the walk could be rewritten by +// the next sibling. Allocating per child keeps every captured path independent. +func childPath(path []string, seg string) []string { + child := make([]string, len(path)+1) + copy(child, path) + child[len(path)] = seg + return child +} + +// parseAnnotation validates a single x-mcp-header annotation against SEP-2243 +// and records it in seen for the uniqueness check. +func parseAnnotation(raw any, node map[string]any, path []string, seen map[string]string) (ParamHeader, error) { + where := strings.Join(path, ".") + + name, ok := raw.(string) + if !ok { + return ParamHeader{}, fmt.Errorf( + "parameter %q: %s must be a string, got %T", where, XMCPHeaderAnnotation, raw) + } + if name == "" { + return ParamHeader{}, fmt.Errorf("parameter %q: %s must not be empty", where, XMCPHeaderAnnotation) + } + if bad, invalid := firstNonTokenChar(name); invalid { + return ParamHeader{}, fmt.Errorf( + "parameter %q: %s value %q is not a valid HTTP field-name token (offending character %q)", + where, XMCPHeaderAnnotation, name, bad) + } + + // HTTP field names are case-insensitive, so two annotations differing only in + // case would mirror onto the same header and one would silently win. + folded := strings.ToLower(name) + if first, dup := seen[folded]; dup { + return ParamHeader{}, fmt.Errorf( + "%s value %q on parameter %q collides case-insensitively with parameter %q", + XMCPHeaderAnnotation, name, where, first) + } + seen[folded] = where + + declared, ok := node["type"].(string) + if !ok { + return ParamHeader{}, fmt.Errorf( + "parameter %q: %s requires a declared primitive type (string, integer, or boolean)", + where, XMCPHeaderAnnotation) + } + switch declared { + case "string", "integer", "boolean": + default: + return ParamHeader{}, fmt.Errorf( + "parameter %q: %s is not permitted on type %q (only string, integer, or boolean)", + where, XMCPHeaderAnnotation, declared) + } + + // childPath already gives each branch its own storage, so cloning is belt and + // braces -- it keeps the returned Path independent of the walk even if the + // traversal's path handling is later changed. + return ParamHeader{Path: slices.Clone(path), Name: name, Type: declared}, nil +} + +// firstNonTokenChar returns the first byte of s that is not an RFC 9110 tchar, +// reporting true when one exists. Operating on bytes rather than runes is +// correct here: every tchar is ASCII, so any multi-byte rune is invalid and its +// leading byte is a faithful thing to name in the error. +func firstNonTokenChar(s string) (string, bool) { + for i := 0; i < len(s); i++ { + if !isTokenChar(s[i]) { + return string(s[i]), true + } + } + return "", false +} + +// isTokenChar reports whether c is an RFC 9110 tchar, the character set HTTP +// field names are drawn from. +func isTokenChar(c byte) bool { + switch { + case c >= 'a' && c <= 'z', + c >= 'A' && c <= 'Z', + c >= '0' && c <= '9': + return true + } + switch c { + case '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~': + return true + } + return false +} diff --git a/pkg/mcp/xmcpheader_test.go b/pkg/mcp/xmcpheader_test.go new file mode 100644 index 0000000000..76505c2065 --- /dev/null +++ b/pkg/mcp/xmcpheader_test.go @@ -0,0 +1,342 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// objSchema builds an object inputSchema with the given properties, the shape +// every tool inputSchema takes at its root. +func objSchema(props map[string]any) map[string]any { + return map[string]any{"type": "object", "properties": props} +} + +// annotated builds a leaf parameter schema of the given type carrying an +// x-mcp-header annotation. +func annotated(typ, header string) map[string]any { + return map[string]any{"type": typ, XMCPHeaderAnnotation: header} +} + +func TestParamHeaders_Accepted(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + schema map[string]any + want []ParamHeader + }{ + { + name: "nil schema has no annotations", + schema: nil, + }, + { + name: "empty schema has no annotations", + schema: map[string]any{}, + }, + { + // The overwhelmingly common case: SEP-2243 makes the annotation + // optional for servers, so almost every real tool has none. + name: "unannotated schema has no annotations", + schema: objSchema(map[string]any{ + "query": map[string]any{"type": "string"}, + }), + }, + { + // The SEP's own worked example (execute_sql / Region). + name: "single string annotation", + schema: objSchema(map[string]any{ + "region": annotated("string", "Region"), + "query": map[string]any{"type": "string"}, + }), + want: []ParamHeader{{Path: []string{"region"}, Name: "Region", Type: "string"}}, + }, + { + name: "integer and boolean are permitted primitives", + schema: objSchema(map[string]any{ + "attempts": annotated("integer", "Attempts"), + "dry_run": annotated("boolean", "Dry-Run"), + }), + want: []ParamHeader{ + {Path: []string{"attempts"}, Name: "Attempts", Type: "integer"}, + {Path: []string{"dry_run"}, Name: "Dry-Run", Type: "boolean"}, + }, + }, + { + // SEP-2243: "These annotations can be applied to properties at any + // nesting depth." + name: "annotation nested inside an object property", + schema: objSchema(map[string]any{ + "filter": objSchema(map[string]any{ + "region": annotated("string", "Region"), + }), + }), + want: []ParamHeader{{Path: []string{"filter", "region"}, Name: "Region", Type: "string"}}, + }, + { + name: "annotation inside array items", + schema: objSchema(map[string]any{ + "targets": map[string]any{ + "type": "array", + "items": objSchema(map[string]any{"zone": annotated("string", "Zone")}), + }, + }), + want: []ParamHeader{{Path: []string{"targets", "[]", "zone"}, Name: "Zone", Type: "string"}}, + }, + { + // oneOf/anyOf/allOf carry real schemas in this repo's tool sets -- + // #5976 fixed ingestion dropping them -- so the walk must descend them + // or an annotation inside a combinator branch would go unvalidated. + name: "annotation inside a oneOf branch", + schema: objSchema(map[string]any{ + "target": map[string]any{ + "oneOf": []any{ + map[string]any{"type": "string"}, + objSchema(map[string]any{"zone": annotated("string", "Zone")}), + }, + }, + }), + want: []ParamHeader{{Path: []string{"target", "oneOf[1]", "zone"}, Name: "Zone", Type: "string"}}, + }, + { + // Distinct spellings that do not collide case-insensitively are fine. + name: "two distinct annotations", + schema: objSchema(map[string]any{ + "region": annotated("string", "Region"), + "priority": annotated("string", "Priority"), + }), + want: []ParamHeader{ + {Path: []string{"priority"}, Name: "Priority", Type: "string"}, + {Path: []string{"region"}, Name: "Region", Type: "string"}, + }, + }, + { + // An annotation on the root designates no parameter, so it is ignored + // rather than rejected -- the root IS the parameter object. + name: "annotation on the root node is ignored", + schema: map[string]any{ + "type": "object", + XMCPHeaderAnnotation: "Bogus", + "properties": map[string]any{"query": map[string]any{"type": "string"}}, + }, + }, + { + // All tchar punctuation is legal in an HTTP field name. + name: "tchar punctuation is a valid token", + schema: objSchema(map[string]any{ + "weird": annotated("string", "A!#$%&'*+-.^_`|~9z"), + }), + want: []ParamHeader{{Path: []string{"weird"}, Name: "A!#$%&'*+-.^_`|~9z", Type: "string"}}, + }, + { + // A non-schema value where a schema is expected must be skipped, not + // panicked on: the schema is backend-supplied and need not be sane. + name: "non-object property value is skipped", + schema: objSchema(map[string]any{ + "bogus": "not a schema", + "region": annotated("string", "Region"), + }), + want: []ParamHeader{{Path: []string{"region"}, Name: "Region", Type: "string"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParamHeaders(tt.schema) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + // ValidateParamHeaders is ParamHeaders with the result discarded, so + // it must agree on every accepted schema. + assert.NoError(t, ValidateParamHeaders(tt.schema)) + }) + } +} + +func TestParamHeaders_Rejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + schema map[string]any + wantErrPart string + }{ + { + name: "empty annotation value", + schema: objSchema(map[string]any{"region": annotated("string", "")}), + wantErrPart: "must not be empty", + }, + { + name: "non-string annotation value", + schema: objSchema(map[string]any{ + "region": map[string]any{"type": "string", XMCPHeaderAnnotation: 42}, + }), + wantErrPart: "must be a string", + }, + { + // A space is not a tchar; allowing it would let a backend inject a + // second header or a request line into the outgoing request. + name: "annotation containing a space", + schema: objSchema(map[string]any{"region": annotated("string", "My Region")}), + wantErrPart: "not a valid HTTP field-name token", + }, + { + // CRLF is the header-injection vector specifically. + name: "annotation containing CRLF", + schema: objSchema(map[string]any{"region": annotated("string", "R\r\nX: y")}), + wantErrPart: "not a valid HTTP field-name token", + }, + { + name: "annotation containing a colon", + schema: objSchema(map[string]any{"region": annotated("string", "R:egion")}), + wantErrPart: "not a valid HTTP field-name token", + }, + { + // SEP-2243 excludes number explicitly: a float has no canonical wire + // spelling, so mirroring it would be lossy. + name: "number is not a permitted type", + schema: objSchema(map[string]any{"ratio": annotated("number", "Ratio")}), + wantErrPart: `not permitted on type "number"`, + }, + { + name: "object is not a permitted type", + schema: objSchema(map[string]any{"blob": annotated("object", "Blob")}), + wantErrPart: `not permitted on type "object"`, + }, + { + name: "array is not a permitted type", + schema: objSchema(map[string]any{"list": annotated("array", "List")}), + wantErrPart: `not permitted on type "array"`, + }, + { + name: "missing type is not a declared primitive", + schema: objSchema(map[string]any{ + "region": map[string]any{XMCPHeaderAnnotation: "Region"}, + }), + wantErrPart: "requires a declared primitive type", + }, + { + // A union type is not a single declared primitive, so mirroring cannot + // know the spelling. + name: "union type is not a declared primitive", + schema: objSchema(map[string]any{ + "region": map[string]any{ + "type": []any{"string", "null"}, + XMCPHeaderAnnotation: "Region", + }, + }), + wantErrPart: "requires a declared primitive type", + }, + { + name: "exact duplicate annotation", + schema: objSchema(map[string]any{ + "a": annotated("string", "Region"), + "b": annotated("string", "Region"), + }), + wantErrPart: "collides case-insensitively", + }, + { + // HTTP field names are case-insensitive, so these two would mirror onto + // the same header and one would silently win. + name: "case-insensitive duplicate annotation", + schema: objSchema(map[string]any{ + "a": annotated("string", "Region"), + "b": annotated("string", "REGION"), + }), + wantErrPart: "collides case-insensitively", + }, + { + // Uniqueness is scoped to the whole inputSchema, not to one object + // level, so a nested collision must also be caught. + name: "duplicate across nesting levels", + schema: objSchema(map[string]any{ + "region": annotated("string", "Region"), + "filter": objSchema(map[string]any{"r": annotated("string", "region")}), + }), + wantErrPart: "collides case-insensitively", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParamHeaders(tt.schema) + require.Error(t, err) + assert.Nil(t, got, "a rejected schema must yield no annotations") + assert.Contains(t, err.Error(), tt.wantErrPart) + // ValidateParamHeaders must agree on every rejected schema too. + assert.Error(t, ValidateParamHeaders(tt.schema)) + }) + } +} + +func TestParamHeaders_DepthLimit(t *testing.T) { + t.Parallel() + + // Nest well past maxSchemaDepth. A backend can advertise any tool list it + // likes, so an unbounded recursive walk is a stack-exhaustion vector. + deep := map[string]any{"type": "string", XMCPHeaderAnnotation: "Deep"} + for range maxSchemaDepth + 10 { + deep = objSchema(map[string]any{"next": deep}) + } + + got, err := ParamHeaders(deep) + require.Error(t, err) + assert.Nil(t, got) + assert.True(t, errors.Is(err, ErrSchemaTooDeep), "want ErrSchemaTooDeep, got %v", err) +} + +func TestParamHeaders_ShallowNestingIsNotRejected(t *testing.T) { + t.Parallel() + + // The depth cap must not reject legitimately nested schemas. Build one just + // inside the limit and confirm the annotation is still found, so a future + // change to maxSchemaDepth that makes the walk too strict fails here. + const depth = maxSchemaDepth - 2 + schema := map[string]any{"type": "string", XMCPHeaderAnnotation: "Deep"} + for range depth { + schema = objSchema(map[string]any{"next": schema}) + } + + got, err := ParamHeaders(schema) + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, "Deep", got[0].Name) + assert.Len(t, got[0].Path, depth) +} + +func TestParamHeader_HeaderName(t *testing.T) { + t.Parallel() + + // The annotation value is the header's SUFFIX; the wire name carries the + // Mcp-Param- prefix. Conflating the two is the easy mistake for a caller. + p := ParamHeader{Path: []string{"region"}, Name: "Region", Type: "string"} + assert.Equal(t, "Mcp-Param-Region", p.HeaderName()) + assert.True(t, strings.HasPrefix(p.HeaderName(), ParamHeaderPrefix)) +} + +func TestParamHeaders_SiblingPathsAreIndependent(t *testing.T) { + t.Parallel() + + // Guards childPath: appending to the shared path slice would let one branch's + // captured path be overwritten by the next sibling's traversal, silently + // mirroring a value onto the wrong parameter's header. + schema := objSchema(map[string]any{ + "first": objSchema(map[string]any{"alpha": annotated("string", "Alpha")}), + "second": objSchema(map[string]any{"beta": annotated("string", "Beta")}), + }) + + got, err := ParamHeaders(schema) + require.NoError(t, err) + require.Len(t, got, 2) + assert.Equal(t, []string{"first", "alpha"}, got[0].Path) + assert.Equal(t, []string{"second", "beta"}, got[1].Path) +} diff --git a/pkg/vmcp/client/client.go b/pkg/vmcp/client/client.go index 8fcffa83f9..c2f9ec5b78 100644 --- a/pkg/vmcp/client/client.go +++ b/pkg/vmcp/client/client.go @@ -1269,21 +1269,37 @@ func newCapabilityListFromMCP( tools []mcp.Tool, resources []mcp.Resource, templates []mcp.ResourceTemplate, prompts []mcp.Prompt, ) *vmcp.CapabilityList { capabilities := &vmcp.CapabilityList{ - Tools: make([]vmcp.Tool, len(tools)), + Tools: make([]vmcp.Tool, 0, len(tools)), Resources: make([]vmcp.Resource, len(resources)), ResourceTemplates: make([]vmcp.ResourceTemplate, len(templates)), Prompts: make([]vmcp.Prompt, len(prompts)), } - for i, tool := range tools { - capabilities.Tools[i] = vmcp.Tool{ + for _, tool := range tools { + inputSchema := conversion.ConvertToolInputSchema(tool.InputSchema) + + // SEP-2243 requires a Streamable HTTP client to reject a tool definition + // whose x-mcp-header annotations violate the extension's constraints, and + // vMCP is this backend's client. Rejecting here — at the single ingestion + // seam both the Legacy and Modern paths share — also protects vMCP's own + // downstream clients: an invalid annotation republished in the aggregated + // tools/list would make a conformant downstream client reject the whole + // list, taking every other backend's tools down with it. Dropping the one + // offending tool is the narrower failure. + if err := mcpparser.ValidateParamHeaders(inputSchema); err != nil { + slog.Warn("rejecting backend tool with invalid x-mcp-header annotation", + "backend", backendID, "tool", tool.Name, "error", err) + continue + } + + capabilities.Tools = append(capabilities.Tools, vmcp.Tool{ Name: tool.Name, Description: tool.Description, - InputSchema: conversion.ConvertToolInputSchema(tool.InputSchema), + InputSchema: inputSchema, OutputSchema: conversion.ConvertToolOutputSchema(tool.OutputSchema), Annotations: conversion.ConvertToolAnnotations(tool.Annotations), BackendID: backendID, - } + }) } for i, resource := range resources { diff --git a/pkg/vmcp/client/xmcpheader_ingestion_test.go b/pkg/vmcp/client/xmcpheader_ingestion_test.go new file mode 100644 index 0000000000..5a4c877e2d --- /dev/null +++ b/pkg/vmcp/client/xmcpheader_ingestion_test.go @@ -0,0 +1,115 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive-core/mcpcompat/mcp" +) + +// toolWithSchema builds a backend tool whose inputSchema has a single property. +func toolWithSchema(name string, prop map[string]any) mcp.Tool { + return mcp.Tool{ + Name: name, + InputSchema: mcp.ToolInputSchema{ + Type: "object", + Properties: map[string]any{"region": prop}, + }, + } +} + +// toolNames extracts the ingested tool names, the observable outcome of the +// SEP-2243 rejection. +func toolNames(t *testing.T, tools []mcp.Tool) []string { + t.Helper() + caps := newCapabilityListFromMCP("backend-1", tools, nil, nil, nil) + require.NotNil(t, caps) + names := make([]string, 0, len(caps.Tools)) + for _, tool := range caps.Tools { + names = append(names, tool.Name) + } + return names +} + +// TestNewCapabilityListFromMCP_RejectsInvalidXMCPHeader pins the SEP-2243 +// requirement that a Streamable HTTP client reject a tool definition whose +// x-mcp-header annotations violate the extension's constraints. vMCP is the +// backend's client, so the rejection happens at ingestion. +// +// Crucially it must reject only the OFFENDING tool: the aggregated tools/list +// spans every backend, so failing the whole list would take unrelated tools down +// with it. +func TestNewCapabilityListFromMCP_RejectsInvalidXMCPHeader(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + prop map[string]any + wantKept bool + }{ + { + name: "unannotated tool is kept", + prop: map[string]any{"type": "string"}, + wantKept: true, + }, + { + name: "valid annotation is kept", + prop: map[string]any{"type": "string", "x-mcp-header": "Region"}, + wantKept: true, + }, + { + // SEP-2243 excludes number: a float has no canonical wire spelling. + name: "number-typed annotation is rejected", + prop: map[string]any{"type": "number", "x-mcp-header": "Region"}, + wantKept: false, + }, + { + // The header-injection vector: a CRLF in the annotation would let a + // backend forge additional headers on vMCP's outgoing request. + name: "CRLF in the annotation is rejected", + prop: map[string]any{"type": "string", "x-mcp-header": "R\r\nX: y"}, + wantKept: false, + }, + { + name: "empty annotation is rejected", + prop: map[string]any{"type": "string", "x-mcp-header": ""}, + wantKept: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := toolNames(t, []mcp.Tool{toolWithSchema("subject", tt.prop)}) + if tt.wantKept { + assert.Equal(t, []string{"subject"}, got) + return + } + assert.Empty(t, got, "a tool with an invalid x-mcp-header must be rejected") + }) + } +} + +// TestNewCapabilityListFromMCP_RejectionIsScopedToTheOffendingTool is the +// non-vacuous half of the pin above: rejecting the whole backend's tool list (or +// panicking, or preserving a zero-valued gap in the slice) would all satisfy +// "the bad tool is absent". This asserts the good tools survive alongside it. +func TestNewCapabilityListFromMCP_RejectionIsScopedToTheOffendingTool(t *testing.T) { + t.Parallel() + + got := toolNames(t, []mcp.Tool{ + toolWithSchema("before", map[string]any{"type": "string"}), + toolWithSchema("offender", map[string]any{"type": "number", "x-mcp-header": "Region"}), + toolWithSchema("after", map[string]any{"type": "string", "x-mcp-header": "Zone"}), + }) + + // Order is preserved and only the offender is missing — no zero-valued gap + // left behind by the index-assignment the loop used to do. + assert.Equal(t, []string{"before", "after"}, got) +} From ad85feb18c906d88a76595d25acb5bec26b547b3 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 27 Jul 2026 16:26:15 +0300 Subject: [PATCH 2/2] Mirror x-mcp-header params on the Modern egress (#6014) SEP-2243 lets a server designate tool parameters, via x-mcp-header in its inputSchema, whose values it also requires as Mcp-Param-{name} HTTP headers -- and a server that designated a parameter and does not receive its header rejects the call with -32020. vMCP never sent them, so any Modern backend annotating a tool parameter was uncallable through vMCP, surfacing only as a generic backend failure. Deriving the headers needs the tool's inputSchema, which BackendTarget does not carry and the routing table does not hold. The aggregated view in the core does hold it, and the core is already on the tools/call path, so the headers are derived there and passed explicitly to CallTool. The alternative -- caching schemas inside the backend client -- was rejected: it would add a staleness window of exactly the kind #5992 documents for the revision cache, to avoid a lookup the core already has in hand. Values come from the caller and are therefore untrusted. A control character is refused rather than stripped, since the value would otherwise forge headers on vMCP's outgoing request, and refusing fails the call closed instead of letting the backend answer -32020 for a reason the caller cannot see. Mirrored headers are also set before the protocol headers, so no derived entry can displace Mcp-Method, Mcp-Name, or MCP-Protocol-Version. Legacy backends are untouched: SEP-2243 belongs to 2026-07-28, and a Legacy backend neither expects the headers nor rejects their absence. The legacy pkg/vmcp/server/adapter path holds no schema and so does not mirror; that limitation is recorded at the call site. The live Serve path routes tools/call through the core. Refs #6002 --- pkg/mcp/xmcpheader.go | 182 +++++++++++++ pkg/mcp/xmcpheader_mirror_test.go | 256 ++++++++++++++++++ .../auth_error_mapping_regression_test.go | 2 +- pkg/vmcp/client/client.go | 26 +- pkg/vmcp/client/client_test.go | 2 +- .../client/header_forward_integration_test.go | 2 +- pkg/vmcp/client/meta_integration_test.go | 10 +- pkg/vmcp/client/modern.go | 12 + pkg/vmcp/client/modern_calls_test.go | 4 +- pkg/vmcp/client/modern_integration_test.go | 2 +- pkg/vmcp/client/modern_test.go | 12 +- pkg/vmcp/client/reclassify_test.go | 2 +- pkg/vmcp/client/revision_realbackend_test.go | 4 +- pkg/vmcp/client/xmcpheader_egress_test.go | 88 ++++++ .../composite_output_integration_test.go | 4 +- .../composer/elicitation_integration_test.go | 2 +- pkg/vmcp/composer/foreach_test.go | 10 +- pkg/vmcp/composer/security_test.go | 4 +- pkg/vmcp/composer/testhelpers_test.go | 8 +- .../workflow_audit_integration_test.go | 6 +- pkg/vmcp/composer/workflow_engine.go | 14 +- pkg/vmcp/composer/workflow_engine_test.go | 50 ++-- pkg/vmcp/core/admission_test.go | 6 +- pkg/vmcp/core/core_calls.go | 45 ++- pkg/vmcp/core/core_calls_test.go | 12 +- pkg/vmcp/core/xmcpheader_derivation_test.go | 133 +++++++++ .../backendtelemetry/backendtelemetry.go | 3 +- pkg/vmcp/mocks/mock_backend_client.go | 8 +- pkg/vmcp/server/adapter/handler_factory.go | 10 +- .../server/adapter/handler_factory_test.go | 10 +- pkg/vmcp/server/integration_test.go | 2 +- .../session_management_integration_test.go | 2 +- pkg/vmcp/server/telemetry_integration_test.go | 2 +- pkg/vmcp/types.go | 12 +- 34 files changed, 851 insertions(+), 96 deletions(-) create mode 100644 pkg/mcp/xmcpheader_mirror_test.go create mode 100644 pkg/vmcp/client/xmcpheader_egress_test.go create mode 100644 pkg/vmcp/core/xmcpheader_derivation_test.go diff --git a/pkg/mcp/xmcpheader.go b/pkg/mcp/xmcpheader.go index 62af5f6484..4bcd689537 100644 --- a/pkg/mcp/xmcpheader.go +++ b/pkg/mcp/xmcpheader.go @@ -4,9 +4,12 @@ package mcp import ( + "encoding/json" "errors" "fmt" + "math" "slices" + "strconv" "strings" ) @@ -205,6 +208,185 @@ func walkCombinators( return nil } +// maxSafeInteger is JavaScript's Number.MAX_SAFE_INTEGER (2^53 - 1). SEP-2243 +// requires a mirrored integer to sit within this range, since a peer that parses +// the header with JSON-number semantics could not round-trip a larger value. +const maxSafeInteger = 1<<53 - 1 + +// ErrUnmirrorableValue is returned when a designated parameter's value cannot be +// mirrored into a header: a control character (header injection), a non-integral +// or out-of-safe-range integer, or a value whose type contradicts the schema. +var ErrUnmirrorableValue = errors.New("parameter value cannot be mirrored into an HTTP header") + +// MirrorParamHeaders derives the Mcp-Param-* headers to send with a tools/call, +// given the tool's x-mcp-header annotations (from ParamHeaders) and the call's +// arguments. The result maps full header names to values, ready to set on the +// request; it is nil when nothing is to be mirrored. +// +// A designated parameter that is absent from args contributes no header. That is +// deliberate rather than an error: an optional parameter the caller did not +// supply has no value to mirror, and SEP-2243's -32020 covers the server's view +// of a genuinely missing designated value. +// +// Annotations reached through an array element ("[]" in the path) are skipped: an +// array holds many elements and a header holds one value, so there is no +// well-defined single value to send. Combinator segments (oneOf/anyOf/allOf) are +// dropped when resolving, because they are structural to the schema and absent +// from the arguments the schema describes. +// +// It returns an error wrapping ErrUnmirrorableValue when a present value cannot +// be safely rendered. Values originate from the caller (ultimately a model), so +// they are untrusted: a CR, LF, or NUL in a string would let a caller forge +// additional headers on vMCP's outgoing request, and is refused rather than +// silently stripped. +func MirrorParamHeaders(headers []ParamHeader, args map[string]any) (map[string]string, error) { + if len(headers) == 0 || len(args) == 0 { + return nil, nil + } + var out map[string]string + for _, h := range headers { + if slices.Contains(h.Path, "[]") { + continue + } + value, ok := resolveArg(args, h.Path) + if !ok { + continue + } + rendered, err := renderHeaderValue(h, value) + if err != nil { + return nil, err + } + if out == nil { + out = map[string]string{} + } + out[h.HeaderName()] = rendered + } + return out, nil +} + +// ParamHeadersForSchema is ParamHeaders followed by MirrorParamHeaders: it takes +// a tool's inputSchema and a call's arguments and returns the Mcp-Param-* headers +// to send. It exists so the several call sites that mirror headers share one +// reading of the two-step dance rather than each re-deriving it. +// +// A schema error is returned unwrapped from ParamHeaders; a value error wraps +// ErrUnmirrorableValue. Callers distinguish them because the first indicts the +// backend's tool definition and the second the caller's arguments. +func ParamHeadersForSchema(schema map[string]any, args map[string]any) (map[string]string, error) { + annotations, err := ParamHeaders(schema) + if err != nil { + return nil, err + } + if len(annotations) == 0 { + return nil, nil + } + return MirrorParamHeaders(annotations, args) +} + +// resolveArg walks path through args and returns the value at its end. Segments +// naming a combinator are skipped: they exist in the schema's structure, not in +// the data it describes. A missing or non-object intermediate reports false. +func resolveArg(args map[string]any, path []string) (any, bool) { + current := any(args) + for _, seg := range path { + if isCombinatorSegment(seg) { + continue + } + obj, ok := current.(map[string]any) + if !ok { + return nil, false + } + current, ok = obj[seg] + if !ok { + return nil, false + } + } + return current, true +} + +// isCombinatorSegment reports whether a path segment is a combinator marker +// emitted by walkCombinators (e.g. "oneOf[0]") rather than a property name. +func isCombinatorSegment(seg string) bool { + for _, combinator := range []string{"oneOf", "anyOf", "allOf"} { + if strings.HasPrefix(seg, combinator+"[") && strings.HasSuffix(seg, "]") { + return true + } + } + return false +} + +// renderHeaderValue converts a designated parameter's value to its header +// spelling, enforcing the schema's declared type and SEP-2243's integer range. +func renderHeaderValue(h ParamHeader, value any) (string, error) { + where := strings.Join(h.Path, ".") + switch h.Type { + case "string": + s, ok := value.(string) + if !ok { + return "", fmt.Errorf("%w: parameter %q is declared string but got %T", ErrUnmirrorableValue, where, value) + } + if bad, invalid := firstControlChar(s); invalid { + return "", fmt.Errorf( + "%w: parameter %q contains a control character (%q)", ErrUnmirrorableValue, where, bad) + } + return s, nil + case "boolean": + b, ok := value.(bool) + if !ok { + return "", fmt.Errorf("%w: parameter %q is declared boolean but got %T", ErrUnmirrorableValue, where, value) + } + return strconv.FormatBool(b), nil + case "integer": + return renderIntegerHeaderValue(where, value) + default: + // Unreachable: ParamHeaders admits only the three types above. + return "", fmt.Errorf("%w: parameter %q has unsupported type %q", ErrUnmirrorableValue, where, h.Type) + } +} + +// renderIntegerHeaderValue renders an integer-declared parameter. JSON decoding +// yields float64 for every number, so an integer arrives as a float that must be +// checked for integrality and for SEP-2243's safe-integer range; the int/int64 +// cases cover arguments built in Go rather than decoded from JSON. +func renderIntegerHeaderValue(where string, value any) (string, error) { + switch n := value.(type) { + case float64: + if n != math.Trunc(n) { + return "", fmt.Errorf("%w: parameter %q is declared integer but got %v", ErrUnmirrorableValue, where, n) + } + if n > maxSafeInteger || n < -maxSafeInteger { + return "", fmt.Errorf( + "%w: parameter %q value %v is outside the safe integer range", ErrUnmirrorableValue, where, n) + } + return strconv.FormatInt(int64(n), 10), nil + case int: + return renderIntegerHeaderValue(where, float64(n)) + case int64: + return renderIntegerHeaderValue(where, float64(n)) + case json.Number: + i, err := n.Int64() + if err != nil { + return "", fmt.Errorf("%w: parameter %q is declared integer but got %q", ErrUnmirrorableValue, where, n) + } + return renderIntegerHeaderValue(where, float64(i)) + default: + return "", fmt.Errorf( + "%w: parameter %q is declared integer but got %T", ErrUnmirrorableValue, where, value) + } +} + +// firstControlChar returns the first control character in s, reporting true when +// one exists. CR and LF are the header-injection vectors; NUL and the other C0 +// controls are equally illegal in a header value. +func firstControlChar(s string) (string, bool) { + for _, r := range s { + if r < 0x20 || r == 0x7f { + return string(r), true + } + } + return "", false +} + // childPath returns path with seg appended, always in freshly allocated storage. // Appending to path directly would let sibling recursions share (and overwrite) // one backing array, so a path captured deeper in the walk could be rewritten by diff --git a/pkg/mcp/xmcpheader_mirror_test.go b/pkg/mcp/xmcpheader_mirror_test.go new file mode 100644 index 0000000000..5c12d42cb6 --- /dev/null +++ b/pkg/mcp/xmcpheader_mirror_test.go @@ -0,0 +1,256 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParamHeadersForSchema(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + schema map[string]any + args map[string]any + want map[string]string + }{ + { + name: "no annotations mirrors nothing", + schema: objSchema(map[string]any{"query": map[string]any{"type": "string"}}), + args: map[string]any{"query": "select 1"}, + }, + { + // The SEP's worked example: region is designated, query is not. + name: "designated string is mirrored and others are not", + schema: objSchema(map[string]any{ + "region": annotated("string", "Region"), + "query": map[string]any{"type": "string"}, + }), + args: map[string]any{"region": "eu-west1", "query": "select 1"}, + want: map[string]string{"Mcp-Param-Region": "eu-west1"}, + }, + { + name: "boolean is mirrored as true/false", + schema: objSchema(map[string]any{"dry_run": annotated("boolean", "Dry-Run")}), + args: map[string]any{"dry_run": true}, + want: map[string]string{"Mcp-Param-Dry-Run": "true"}, + }, + { + // JSON decoding yields float64 for every number, so the integral + // float path is the one real clients exercise. + name: "integer arriving as a JSON float is mirrored without a decimal point", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": float64(3)}, + want: map[string]string{"Mcp-Param-Attempts": "3"}, + }, + { + name: "negative integer is mirrored", + schema: objSchema(map[string]any{"offset": annotated("integer", "Offset")}), + args: map[string]any{"offset": float64(-7)}, + want: map[string]string{"Mcp-Param-Offset": "-7"}, + }, + { + name: "integer arriving as a Go int is mirrored", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": 42}, + want: map[string]string{"Mcp-Param-Attempts": "42"}, + }, + { + name: "integer arriving as json.Number is mirrored", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": json.Number("42")}, + want: map[string]string{"Mcp-Param-Attempts": "42"}, + }, + { + // An optional designated parameter the caller did not supply has no + // value to mirror. Omitting the header is correct, not an error. + name: "absent designated parameter contributes no header", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{"query": "select 1"}, + }, + { + name: "nested designated parameter is resolved through the path", + schema: objSchema(map[string]any{ + "filter": objSchema(map[string]any{"region": annotated("string", "Region")}), + }), + args: map[string]any{"filter": map[string]any{"region": "eu-west1"}}, + want: map[string]string{"Mcp-Param-Region": "eu-west1"}, + }, + { + name: "nested path with a missing intermediate contributes no header", + schema: objSchema(map[string]any{ + "filter": objSchema(map[string]any{"region": annotated("string", "Region")}), + }), + args: map[string]any{"other": "x"}, + }, + { + // Combinator segments are structural to the schema and absent from the + // data, so they are skipped when resolving the value's location. + name: "combinator segments are dropped when resolving", + schema: objSchema(map[string]any{ + "target": map[string]any{ + "oneOf": []any{ + objSchema(map[string]any{"zone": annotated("string", "Zone")}), + }, + }, + }), + args: map[string]any{"target": map[string]any{"zone": "a"}}, + want: map[string]string{"Mcp-Param-Zone": "a"}, + }, + { + // An array holds many elements and a header holds one value, so there + // is no well-defined single value to mirror. + name: "annotation behind an array element is skipped", + schema: objSchema(map[string]any{ + "targets": map[string]any{ + "type": "array", + "items": objSchema(map[string]any{"zone": annotated("string", "Zone")}), + }, + }), + args: map[string]any{"targets": []any{map[string]any{"zone": "a"}}}, + }, + { + name: "two designated parameters both mirror", + schema: objSchema(map[string]any{ + "region": annotated("string", "Region"), + "priority": annotated("string", "Priority"), + }), + args: map[string]any{"region": "eu-west1", "priority": "high"}, + want: map[string]string{ + "Mcp-Param-Region": "eu-west1", + "Mcp-Param-Priority": "high", + }, + }, + { + name: "empty args mirrors nothing", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParamHeadersForSchema(tt.schema, tt.args) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestParamHeadersForSchema_UnmirrorableValues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + schema map[string]any + args map[string]any + wantErrPart string + }{ + { + // The header-injection case. The value comes from the caller + // (ultimately a model), so it is untrusted: a CRLF must be refused, + // not silently stripped, or a caller could forge headers on vMCP's + // outgoing request. + name: "CRLF in a string value is refused", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{"region": "eu\r\nX-Evil: 1"}, + wantErrPart: "control character", + }, + { + name: "bare newline in a string value is refused", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{"region": "eu\nwest"}, + wantErrPart: "control character", + }, + { + name: "NUL in a string value is refused", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{"region": "eu\x00west"}, + wantErrPart: "control character", + }, + { + name: "non-integral value for an integer parameter is refused", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": 1.5}, + wantErrPart: "declared integer", + }, + { + // SEP-2243 bounds a mirrored integer to JavaScript's safe range. + name: "integer above the safe range is refused", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": float64(maxSafeInteger) * 4}, + wantErrPart: "safe integer range", + }, + { + name: "integer below the safe range is refused", + schema: objSchema(map[string]any{"attempts": annotated("integer", "Attempts")}), + args: map[string]any{"attempts": float64(-maxSafeInteger) * 4}, + wantErrPart: "safe integer range", + }, + { + name: "type mismatch against the schema is refused", + schema: objSchema(map[string]any{"region": annotated("string", "Region")}), + args: map[string]any{"region": 42}, + wantErrPart: "declared string", + }, + { + name: "non-boolean for a boolean parameter is refused", + schema: objSchema(map[string]any{"dry_run": annotated("boolean", "Dry-Run")}), + args: map[string]any{"dry_run": "yes"}, + wantErrPart: "declared boolean", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParamHeadersForSchema(tt.schema, tt.args) + require.Error(t, err) + assert.Nil(t, got) + assert.Contains(t, err.Error(), tt.wantErrPart) + assert.True(t, errors.Is(err, ErrUnmirrorableValue), + "a bad VALUE must be distinguishable from a bad annotation: %v", err) + }) + } +} + +// TestParamHeadersForSchema_SafeIntegerBoundary pins the inclusive edge of the +// safe-integer range, the off-by-one a reviewer would reasonably doubt. +func TestParamHeadersForSchema_SafeIntegerBoundary(t *testing.T) { + t.Parallel() + + schema := objSchema(map[string]any{"n": annotated("integer", "N")}) + + got, err := ParamHeadersForSchema(schema, map[string]any{"n": float64(maxSafeInteger)}) + require.NoError(t, err, "MAX_SAFE_INTEGER itself is in range") + assert.Equal(t, map[string]string{"Mcp-Param-N": "9007199254740991"}, got) + + got, err = ParamHeadersForSchema(schema, map[string]any{"n": float64(-maxSafeInteger)}) + require.NoError(t, err, "-MAX_SAFE_INTEGER is in range") + assert.Equal(t, map[string]string{"Mcp-Param-N": "-9007199254740991"}, got) +} + +// TestParamHeadersForSchema_InvalidAnnotationIsNotAValueError keeps the two error +// classes separable: callers blame the caller's arguments for one and the +// backend's tool definition for the other. +func TestParamHeadersForSchema_InvalidAnnotationIsNotAValueError(t *testing.T) { + t.Parallel() + + schema := objSchema(map[string]any{"ratio": annotated("number", "Ratio")}) + + got, err := ParamHeadersForSchema(schema, map[string]any{"ratio": 1.5}) + require.Error(t, err) + assert.Nil(t, got) + assert.False(t, errors.Is(err, ErrUnmirrorableValue), + "a malformed ANNOTATION must not masquerade as a bad value: %v", err) +} diff --git a/pkg/vmcp/client/auth_error_mapping_regression_test.go b/pkg/vmcp/client/auth_error_mapping_regression_test.go index 3cb0ca9f6e..dba3dad3e7 100644 --- a/pkg/vmcp/client/auth_error_mapping_regression_test.go +++ b/pkg/vmcp/client/auth_error_mapping_regression_test.go @@ -296,7 +296,7 @@ func TestRegression_BackendToolErrorWith401_NotClassifiedAsAuthFailure(t *testin ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - result, err := h.CallTool(ctx, target, "test-tool", map[string]any{"arg": "val"}, nil) + result, err := h.CallTool(ctx, target, "test-tool", map[string]any{"arg": "val"}, nil, nil) if err != nil { if errors.Is(err, vmcp.ErrAuthenticationFailed) { diff --git a/pkg/vmcp/client/client.go b/pkg/vmcp/client/client.go index c2f9ec5b78..3cdfbb32b9 100644 --- a/pkg/vmcp/client/client.go +++ b/pkg/vmcp/client/client.go @@ -1061,7 +1061,7 @@ func discoverModernCapabilities(ctx context.Context, hc *http.Client, endpoint s Capabilities mcp.ServerCapabilities `json:"capabilities"` SupportedVersions []string `json:"supportedVersions"` } - if err := modernCall(ctx, hc, endpoint, "server/discover", nil, "", &discover); err != nil { + if err := modernCall(ctx, hc, endpoint, "server/discover", nil, "", nil, &discover); err != nil { return nil, err } // Exact-match on MCPVersionModern (2026-07-28): vMCP's shim only speaks that @@ -1243,7 +1243,7 @@ func modernListAll[T any]( ) ([]T, error) { return pagination.ListAll(ctx, func(ctx context.Context, cursor mcp.Cursor) ([]T, mcp.Cursor, error) { var page map[string]json.RawMessage - if err := modernCall(ctx, hc, endpoint, method, cursorParams(cursor), "", &page); err != nil { + if err := modernCall(ctx, hc, endpoint, method, cursorParams(cursor), "", nil, &page); err != nil { return nil, "", err } var items []T @@ -1610,14 +1610,18 @@ func (h *httpBackendClient) CallTool( toolName string, arguments map[string]any, meta map[string]any, + paramHeaders map[string]string, ) (*vmcp.ToolCallResult, error) { slog.Debug("calling tool on backend", "tool", toolName, "backend", target.WorkloadName) var out *vmcp.ToolCallResult err := h.dispatch(ctx, target, func(ctx context.Context, rev mcpparser.Revision) error { var err error if rev == mcpparser.RevisionModern { - out, err = h.modernCallTool(ctx, target, toolName, arguments, meta) + out, err = h.modernCallTool(ctx, target, toolName, arguments, meta, paramHeaders) } else { + // paramHeaders is deliberately dropped here: SEP-2243's Mcp-Param-* + // mirroring belongs to the 2026-07-28 revision, and a Legacy backend + // neither expects the headers nor rejects their absence. out, err = h.legacyCallTool(ctx, target, toolName, arguments, meta) } return err @@ -1630,8 +1634,14 @@ func (h *httpBackendClient) CallTool( // the body identifier and the Mcp-Name header — the server rejects a mismatch // (-32020). The caller's _meta is forwarded (modernCall strips reserved keys and // overlays vMCP's) and the result _meta is forwarded back to core. +// +// paramHeaders carries the SEP-2243 Mcp-Param-* headers derived from the tool's +// x-mcp-header-designated arguments. A backend that designated a parameter and +// does not receive its header rejects the call with -32020, so omitting them +// makes any annotating Modern backend uncallable. func (h *httpBackendClient) modernCallTool( ctx context.Context, target *vmcp.BackendTarget, toolName string, arguments, meta map[string]any, + paramHeaders map[string]string, ) (*vmcp.ToolCallResult, error) { backendToolName := target.GetBackendCapabilityName(toolName) if backendToolName != toolName { @@ -1647,7 +1657,9 @@ func (h *httpBackendClient) modernCallTool( params["_meta"] = meta } var result mcp.CallToolResult - if err := modernCall(ctx, hc, target.BaseURL, "tools/call", params, backendToolName, &result); err != nil { + if err := modernCall( + ctx, hc, target.BaseURL, "tools/call", params, backendToolName, paramHeaders, &result, + ); err != nil { return nil, fmt.Errorf("%w: tool call failed on backend %s: %w", vmcp.ErrBackendUnavailable, target.WorkloadID, err) } return toolResultFromMCP(&result, toolName, target.WorkloadID), nil @@ -1831,7 +1843,7 @@ func (h *httpBackendClient) modernReadResource( Meta map[string]any `json:"_meta"` } params := map[string]any{"uri": backendURI} - if err := modernCall(ctx, hc, target.BaseURL, "resources/read", params, backendURI, &res); err != nil { + if err := modernCall(ctx, hc, target.BaseURL, "resources/read", params, backendURI, nil, &res); err != nil { return nil, fmt.Errorf("resource read failed on backend %s: %w", target.WorkloadID, err) } mcpContents := make([]mcp.ResourceContents, len(res.Contents)) @@ -1949,7 +1961,7 @@ func (h *httpBackendClient) modernGetPrompt( } `json:"messages"` Meta map[string]any `json:"_meta"` } - if err := modernCall(ctx, hc, target.BaseURL, "prompts/get", params, backendPromptName, &res); err != nil { + if err := modernCall(ctx, hc, target.BaseURL, "prompts/get", params, backendPromptName, nil, &res); err != nil { return nil, fmt.Errorf("prompt get failed on backend %s: %w", target.WorkloadID, err) } messages := make([]vmcp.PromptMessage, 0, len(res.Messages)) @@ -2071,7 +2083,7 @@ func (h *httpBackendClient) modernComplete( HasMore bool `json:"hasMore"` } `json:"completion"` } - err = modernCall(ctx, hc, target.BaseURL, "completion/complete", params, "", &res) + err = modernCall(ctx, hc, target.BaseURL, "completion/complete", params, "", nil, &res) if errors.Is(err, mcp.ErrMethodNotFound) { return &vmcp.CompletionResult{Values: []string{}}, nil } diff --git a/pkg/vmcp/client/client_test.go b/pkg/vmcp/client/client_test.go index b1935adc2c..efc7c50c3c 100644 --- a/pkg/vmcp/client/client_test.go +++ b/pkg/vmcp/client/client_test.go @@ -374,7 +374,7 @@ func TestHTTPBackendClient_CallTool_WithMockFactory(t *testing.T) { } backendClient.setRevision(target.WorkloadID, mcpparser.RevisionLegacy) - result, err := backendClient.CallTool(context.Background(), target, "test_tool", map[string]any{}, nil) + result, err := backendClient.CallTool(context.Background(), target, "test_tool", map[string]any{}, nil, nil) require.Error(t, err) assert.Nil(t, result) diff --git a/pkg/vmcp/client/header_forward_integration_test.go b/pkg/vmcp/client/header_forward_integration_test.go index 45dacc88ba..41caa2f12d 100644 --- a/pkg/vmcp/client/header_forward_integration_test.go +++ b/pkg/vmcp/client/header_forward_integration_test.go @@ -73,7 +73,7 @@ func TestHeaderForward_EndToEnd_ThroughHTTPBackendClient(t *testing.T) { // CallTool drives the full Initialize → tools/list flow through the // streamable-HTTP transport. We don't care about the call result — only // that the request reached the test server with the configured headers. - _, _ = backendClient.CallTool(ctx, target, "anything", map[string]any{}, nil) + _, _ = backendClient.CallTool(ctx, target, "anything", map[string]any{}, nil, nil) captured.mu.Lock() defer captured.mu.Unlock() diff --git a/pkg/vmcp/client/meta_integration_test.go b/pkg/vmcp/client/meta_integration_test.go index 8c8fc563b4..1edb86f8be 100644 --- a/pkg/vmcp/client/meta_integration_test.go +++ b/pkg/vmcp/client/meta_integration_test.go @@ -58,7 +58,7 @@ func TestMetaPreservation_CallTool(t *testing.T) { // Call tool through vMCP backend client result, err := backendClient.CallTool(ctx, target, "test_tool_with_meta", map[string]any{ "input": "test-value", - }, nil) + }, nil, nil) // Verify call succeeded require.NoError(t, err) @@ -104,7 +104,7 @@ func TestMetaPreservation_CallTool_NoMeta(t *testing.T) { // Call tool that doesn't return _meta result, err := backendClient.CallTool(ctx, target, "test_tool_no_meta", map[string]any{ "input": "test-value", - }, nil) + }, nil, nil) require.NoError(t, err) require.NotNil(t, result) @@ -151,7 +151,7 @@ func TestMetaPreservation_CallTool_Error(t *testing.T) { // Call tool that returns an error with _meta result, err := backendClient.CallTool(ctx, target, "test_tool_error", map[string]any{ "input": "trigger-error", - }, nil) + }, nil, nil) // Should return result (not a Go error) when tool returns IsError=true require.NoError(t, err, "IsError=true is not a transport error, should return result") @@ -321,7 +321,7 @@ func TestOutboundMetaTraceContext(t *testing.T) { //nolint:paralleltest // Mutat defer cancel() defer span.End() - _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil) + _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil, nil) require.NoError(t, err) got := capture.get("tool") @@ -367,7 +367,7 @@ func TestOutboundMetaTraceContext(t *testing.T) { //nolint:paralleltest // Mutat ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil) + _, err := backendClient.CallTool(ctx, target, "test_tool_capture_meta", nil, nil, nil) require.NoError(t, err) got := capture.get("tool") diff --git a/pkg/vmcp/client/modern.go b/pkg/vmcp/client/modern.go index 7b1e6b3133..b62e645b88 100644 --- a/pkg/vmcp/client/modern.go +++ b/pkg/vmcp/client/modern.go @@ -138,6 +138,11 @@ var modernRequestID atomic.Int64 // header-forward/trace chain (see buildBackendRoundTripper); modernCall adds no // transport concerns of its own. // +// paramHeaders are the SEP-2243 Mcp-Param-* headers (already keyed by full header +// name and validated by pkg/mcp), set before the protocol headers below so a +// caller-derived entry can never overwrite Mcp-Method, Mcp-Name, or +// MCP-Protocol-Version. Only tools/call ever supplies them. +// // Errors: // - errWrongEra: the peer is not Modern (bare 4xx/5xx-free rejection, empty or // non-JSON body, or neither result nor error). @@ -156,6 +161,7 @@ func modernCall( endpoint, method string, params map[string]any, name string, + paramHeaders map[string]string, out any, ) error { id := modernRequestID.Add(1) @@ -180,6 +186,12 @@ func modernCall( if err != nil { return fmt.Errorf("building %s request: %w", method, err) } + // SEP-2243 mirrored parameter headers go on FIRST, so the protocol headers + // below win on any collision. A backend cannot name a designated parameter + // "Method" and hijack Mcp-Method, since Set overwrites. + for hdrName, hdrValue := range paramHeaders { + req.Header.Set(hdrName, hdrValue) + } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json, text/event-stream") req.Header.Set("MCP-Protocol-Version", mcpparser.MCPVersionModern) diff --git a/pkg/vmcp/client/modern_calls_test.go b/pkg/vmcp/client/modern_calls_test.go index 7570295f2f..838df7d85a 100644 --- a/pkg/vmcp/client/modern_calls_test.go +++ b/pkg/vmcp/client/modern_calls_test.go @@ -65,7 +65,7 @@ func TestModernCallTool(t *testing.T) { h, target := modernClient(t, srv.URL) target.OriginalCapabilityName = "backend_echo" // advertised "echo" -> backend "backend_echo" - res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hi"}, map[string]any{"caller": "meta"}) + res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hi"}, map[string]any{"caller": "meta"}, nil) require.NoError(t, err) assert.Equal(t, "tools/call", hdr.Get("Mcp-Method")) @@ -190,7 +190,7 @@ func TestIntegration_ModernCallTool_NameRequired(t *testing.T) { TransportType: "streamable-http", } - res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello modern"}, nil) + res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello modern"}, nil, nil) require.NoError(t, err) rev, ok := h.cachedRevision(target.WorkloadID) diff --git a/pkg/vmcp/client/modern_integration_test.go b/pkg/vmcp/client/modern_integration_test.go index e4371467de..c248dbc1be 100644 --- a/pkg/vmcp/client/modern_integration_test.go +++ b/pkg/vmcp/client/modern_integration_test.go @@ -144,7 +144,7 @@ func TestIntegration_ModernCall_Discover(t *testing.T) { } `json:"capabilities"` SupportedVersions []string `json:"supportedVersions"` } - err := modernCall(context.Background(), hc, vmcpSrv.URL+"/mcp", "server/discover", nil, "", &out) + err := modernCall(context.Background(), hc, vmcpSrv.URL+"/mcp", "server/discover", nil, "", nil, &out) require.NoError(t, err) // Request shaping reached the real server intact. diff --git a/pkg/vmcp/client/modern_test.go b/pkg/vmcp/client/modern_test.go index bd367efd6e..b851ac9b55 100644 --- a/pkg/vmcp/client/modern_test.go +++ b/pkg/vmcp/client/modern_test.go @@ -94,7 +94,7 @@ func TestModernCall_RequestShaping(t *testing.T) { if tt.callerMeta != nil { params["_meta"] = tt.callerMeta } - err := modernCall(context.Background(), srv.Client(), srv.URL, tt.method, params, tt.mcpName, nil) + err := modernCall(context.Background(), srv.Client(), srv.URL, tt.method, params, tt.mcpName, nil, nil) require.NoError(t, err) assert.Equal(t, "application/json", gotReq.Header.Get("Content-Type")) @@ -135,7 +135,7 @@ func TestModernCall_CallerMetaNotMutated(t *testing.T) { callerMeta := map[string]any{"userKey": "v"} params := map[string]any{"_meta": callerMeta, "name": "x"} - require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "tools/list", params, "", nil)) + require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "tools/list", params, "", nil, nil)) assert.Equal(t, map[string]any{"userKey": "v"}, callerMeta, "caller _meta must be untouched") assert.NotContains(t, params, "does-not-add-keys") @@ -160,7 +160,7 @@ func TestModernCall_Decode(t *testing.T) { ResultType string `json:"resultType"` SupportedVersions []string `json:"supportedVersions"` } - require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", &out)) + require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", nil, &out)) assert.Equal(t, "complete", out.ResultType) assert.Equal(t, []string{"2026-07-28"}, out.SupportedVersions) } @@ -190,7 +190,7 @@ func TestModernCall_SSEResponse(t *testing.T) { t.Cleanup(srv.Close) var out map[string]any - require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", &out)) + require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", nil, &out)) assert.Equal(t, "complete", out["resultType"]) assert.Equal(t, true, out["ok"]) } @@ -307,7 +307,7 @@ func TestModernCall_ErrorMapping(t *testing.T) { })) t.Cleanup(srv.Close) - err := modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", nil) + err := modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", nil, nil) require.Error(t, err) if tt.wantErr != nil { assert.ErrorIs(t, err, tt.wantErr) @@ -347,7 +347,7 @@ func TestModernCall_LargeSSEEvent(t *testing.T) { var got struct { Blob string `json:"blob"` } - require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", &got)) + require.NoError(t, modernCall(context.Background(), srv.Client(), srv.URL, "server/discover", nil, "", nil, &got)) assert.Len(t, got.Blob, len(big)) } diff --git a/pkg/vmcp/client/reclassify_test.go b/pkg/vmcp/client/reclassify_test.go index d446f07ffe..197673623b 100644 --- a/pkg/vmcp/client/reclassify_test.go +++ b/pkg/vmcp/client/reclassify_test.go @@ -209,7 +209,7 @@ func TestDispatch_NoDoubleExecOnLegacyBody(t *testing.T) { target := &vmcp.BackendTarget{WorkloadID: "b", BaseURL: srv.URL, TransportType: "streamable-http"} h.setRevision(target.WorkloadID, mcpparser.RevisionModern) // mis-cached Modern - _, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "x"}, nil) + _, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "x"}, nil, nil) require.Error(t, err, "a Legacy-shaped body must surface as an error, not a blank success") assert.EqualValues(t, 1, toolCalls.Load(), "the side-effecting tool must run exactly once (no double-exec)") diff --git a/pkg/vmcp/client/revision_realbackend_test.go b/pkg/vmcp/client/revision_realbackend_test.go index eadacdab67..59dbae6541 100644 --- a/pkg/vmcp/client/revision_realbackend_test.go +++ b/pkg/vmcp/client/revision_realbackend_test.go @@ -148,7 +148,7 @@ func TestLegacyCallTool_StripsReservedMeta_RealBackend(t *testing.T) { "custom-caller-key": "custom-value", } - res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello legacy"}, callerMeta) + res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello legacy"}, callerMeta, nil) require.NoError(t, err, "reserved Modern _meta must not leak onto the Legacy backend hop") require.Len(t, res.Content, 1) assert.Equal(t, "hello legacy", res.Content[0].Text) @@ -212,7 +212,7 @@ func TestCallTool_MisCachedLegacy_ForwardingAgainstStatelessBackend(t *testing.T } h.setRevision(target.WorkloadID, mcpparser.RevisionLegacy) // mis-cached - res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello forwarding"}, nil) + res, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hello forwarding"}, nil, nil) require.NoError(t, err, "the negotiated-Modern version gate must suppress the standalone stream so the per-call forwarding path survives") require.Len(t, res.Content, 1) diff --git a/pkg/vmcp/client/xmcpheader_egress_test.go b/pkg/vmcp/client/xmcpheader_egress_test.go new file mode 100644 index 0000000000..5f9cdfe0a1 --- /dev/null +++ b/pkg/vmcp/client/xmcpheader_egress_test.go @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestModernCallTool_MirrorsParamHeaders is the wire-level pin for SEP-2243 +// mirroring: the Mcp-Param-* headers the core derived must actually reach the +// backend on a Modern tools/call. A backend that designated a parameter and does +// not receive its header answers -32020, so dropping them anywhere along +// CallTool -> modernCallTool -> modernCall makes that backend uncallable. +func TestModernCallTool_MirrorsParamHeaders(t *testing.T) { + t.Parallel() + + srv, hdr, _ := bodyRecordingServer(t, map[string]any{ + "content": []any{map[string]any{"type": "text", "text": "ok"}}, + }) + h, target := modernClient(t, srv.URL) + + _, err := h.CallTool(context.Background(), target, "execute_sql", + map[string]any{"region": "eu-west1", "query": "select 1"}, + nil, + map[string]string{"Mcp-Param-Region": "eu-west1"}, + ) + require.NoError(t, err) + + assert.Equal(t, "eu-west1", hdr.Get("Mcp-Param-Region")) + // The protocol headers must be unaffected by the mirrored ones. + assert.Equal(t, "tools/call", hdr.Get("Mcp-Method")) + assert.Equal(t, "2026-07-28", hdr.Get("MCP-Protocol-Version")) +} + +// TestModernCallTool_ParamHeadersCannotOverrideProtocolHeaders is the reason +// mirrored headers are set BEFORE the protocol headers rather than after. A +// backend controls the x-mcp-header annotation names, so it could designate a +// parameter named "Method" (yielding Mcp-Param-Method, harmless) — but a caller +// or a future derivation bug producing a bare "Mcp-Method" key must not be able +// to redirect the request to another method, which would slip past the +// authz/audit decision already made for the real one. +func TestModernCallTool_ParamHeadersCannotOverrideProtocolHeaders(t *testing.T) { + t.Parallel() + + srv, hdr, _ := bodyRecordingServer(t, map[string]any{ + "content": []any{map[string]any{"type": "text", "text": "ok"}}, + }) + h, target := modernClient(t, srv.URL) + + _, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hi"}, nil, + map[string]string{ + "Mcp-Method": "resources/read", + "Mcp-Name": "spoofed", + "MCP-Protocol-Version": "1999-01-01", + "Mcp-Param-Region": "eu-west1", + }, + ) + require.NoError(t, err) + + assert.Equal(t, "tools/call", hdr.Get("Mcp-Method"), "protocol header must win") + assert.Equal(t, "echo", hdr.Get("Mcp-Name"), "protocol header must win") + assert.Equal(t, "2026-07-28", hdr.Get("MCP-Protocol-Version"), "protocol header must win") + assert.Equal(t, "eu-west1", hdr.Get("Mcp-Param-Region"), "the genuine mirrored header still lands") +} + +// TestModernCallTool_NoParamHeadersSendsNone confirms the common case adds +// nothing to the wire: almost no tool carries an x-mcp-header annotation, so nil +// must stay nil rather than becoming an empty header. +func TestModernCallTool_NoParamHeadersSendsNone(t *testing.T) { + t.Parallel() + + srv, hdr, _ := bodyRecordingServer(t, map[string]any{ + "content": []any{map[string]any{"type": "text", "text": "ok"}}, + }) + h, target := modernClient(t, srv.URL) + + _, err := h.CallTool(context.Background(), target, "echo", map[string]any{"input": "hi"}, nil, nil) + require.NoError(t, err) + + for name := range *hdr { + assert.NotContains(t, name, "Mcp-Param", "no Mcp-Param-* header may be sent when none were derived") + } +} diff --git a/pkg/vmcp/composer/composite_output_integration_test.go b/pkg/vmcp/composer/composite_output_integration_test.go index a8d6a446e6..e55c46540e 100644 --- a/pkg/vmcp/composer/composite_output_integration_test.go +++ b/pkg/vmcp/composer/composite_output_integration_test.go @@ -720,9 +720,9 @@ func TestCompositeToolWithOutputConfig_ErrorHandlingWithRetry(t *testing.T) { // Fail once, then succeed gomock.InOrder( - te.Backend.EXPECT().CallTool(gomock.Any(), target, "api.flaky_call", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "api.flaky_call", gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errors.New("temporary failure")), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "api.flaky_call", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "api.flaky_call", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"data": "success_after_retry"}, Content: []vmcp.Content{}, diff --git a/pkg/vmcp/composer/elicitation_integration_test.go b/pkg/vmcp/composer/elicitation_integration_test.go index ca2f84f278..a26b37f72b 100644 --- a/pkg/vmcp/composer/elicitation_integration_test.go +++ b/pkg/vmcp/composer/elicitation_integration_test.go @@ -79,7 +79,7 @@ func TestWorkflowEngine_ExecuteElicitationStep_Accept(t *testing.T) { } te.Backend.EXPECT().CallTool(gomock.Any(), deployTarget, "deploy_tool", map[string]any{ "env": "production", - }, gomock.Any()).Return(deployResult, nil) + }, gomock.Any(), gomock.Any()).Return(deployResult, nil) result, err := engine.ExecuteWorkflow(context.Background(), workflow, nil) require.NoError(t, err) diff --git a/pkg/vmcp/composer/foreach_test.go b/pkg/vmcp/composer/foreach_test.go index c4ecda88fd..44e6ac293e 100644 --- a/pkg/vmcp/composer/foreach_test.go +++ b/pkg/vmcp/composer/foreach_test.go @@ -179,8 +179,8 @@ func TestForEachStep_ErrorContinue(t *testing.T) { Return(target, nil).Times(2) callCount := int32(0) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "osv.query_vulnerability", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ interface{}, _ interface{}, _ interface{}, _ map[string]any, _ interface{}) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "osv.query_vulnerability", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ interface{}, _ interface{}, _ interface{}, _ map[string]any, _ interface{}, _ interface{}) (*vmcp.ToolCallResult, error) { n := atomic.AddInt32(&callCount, 1) if n == 1 { return nil, fmt.Errorf("network error") @@ -345,7 +345,7 @@ func TestForEachStep_BoundedParallelism(t *testing.T) { } te.Router.EXPECT().RouteTool(gomock.Any(), "osv.query_vulnerability"). Return(target, nil).Times(5) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "osv.query_vulnerability", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "osv.query_vulnerability", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"vulns": []any{}}, }, nil).Times(5) @@ -400,8 +400,8 @@ func TestForEachStep_TemplateContext(t *testing.T) { // Use a sync.Map to safely capture args from concurrent goroutines var capturedArgs sync.Map - te.Backend.EXPECT().CallTool(gomock.Any(), target, "echo.echo", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ interface{}, _ interface{}, _ interface{}, args map[string]any, _ interface{}) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "echo.echo", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ interface{}, _ interface{}, _ interface{}, args map[string]any, _ interface{}, _ interface{}) (*vmcp.ToolCallResult, error) { // Key by the index value to avoid ordering issues capturedArgs.Store(args["index"], args["value"]) return &vmcp.ToolCallResult{ diff --git a/pkg/vmcp/composer/security_test.go b/pkg/vmcp/composer/security_test.go index 8c143a91df..96a28b7cf2 100644 --- a/pkg/vmcp/composer/security_test.go +++ b/pkg/vmcp/composer/security_test.go @@ -93,8 +93,8 @@ func TestWorkflowEngine_RetryCountCapping(t *testing.T) { te.Router.EXPECT().RouteTool(gomock.Any(), "test.tool").Return(target, nil) callCount := 0 - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). - DoAndReturn(func(context.Context, *vmcp.BackendTarget, string, map[string]any, map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(context.Context, *vmcp.BackendTarget, string, map[string]any, map[string]any, map[string]string) (*vmcp.ToolCallResult, error) { callCount++ return nil, fmt.Errorf("fail") }).MaxTimes(12) // 1 initial + 10 retries max diff --git a/pkg/vmcp/composer/testhelpers_test.go b/pkg/vmcp/composer/testhelpers_test.go index be1cdb89ba..60fbdbd8b2 100644 --- a/pkg/vmcp/composer/testhelpers_test.go +++ b/pkg/vmcp/composer/testhelpers_test.go @@ -61,7 +61,7 @@ func (te *testEngine) expectToolCall(toolName string, args, output map[string]an IsError: false, Meta: nil, } - te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, args, gomock.Any()).Return(result, nil) + te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, args, gomock.Any(), gomock.Any()).Return(result, nil) } // expectToolCallWithError is a helper to set up failing tool call expectations. @@ -71,7 +71,7 @@ func (te *testEngine) expectToolCallWithError(toolName string, args map[string]a BaseURL: "http://test:8080", } te.Router.EXPECT().RouteTool(gomock.Any(), toolName).Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, args, gomock.Any()).Return(nil, err) + te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, args, gomock.Any(), gomock.Any()).Return(nil, err) } // expectToolCallWithAnyArgsAndError is a helper for failing calls with any args. @@ -81,7 +81,7 @@ func (te *testEngine) expectToolCallWithAnyArgsAndError(toolName string, err err BaseURL: "http://test:8080", } te.Router.EXPECT().RouteTool(gomock.Any(), toolName).Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, gomock.Any(), gomock.Any()).Return(nil, err) + te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, err) } // expectToolCallWithAnyArgs is a helper for calls where args are dynamically generated. @@ -97,7 +97,7 @@ func (te *testEngine) expectToolCallWithAnyArgs(toolName string, output map[stri IsError: false, Meta: nil, } - te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, gomock.Any(), gomock.Any()).Return(result, nil) + te.Backend.EXPECT().CallTool(gomock.Any(), target, toolName, gomock.Any(), gomock.Any(), gomock.Any()).Return(result, nil) } // newWorkflowContext creates a test workflow context. diff --git a/pkg/vmcp/composer/workflow_audit_integration_test.go b/pkg/vmcp/composer/workflow_audit_integration_test.go index 76351004d6..3318f01e98 100644 --- a/pkg/vmcp/composer/workflow_audit_integration_test.go +++ b/pkg/vmcp/composer/workflow_audit_integration_test.go @@ -245,11 +245,11 @@ func TestWorkflowEngine_WithAuditor_RetryStep(t *testing.T) { // Fail twice, succeed on third attempt (CallTool is called three times) gomock.InOrder( - te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errors.New("temp failure")), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errors.New("temp failure")), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "flaky_tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"success": true}, Content: []vmcp.Content{}, diff --git a/pkg/vmcp/composer/workflow_engine.go b/pkg/vmcp/composer/workflow_engine.go index 71d5d07aac..1efc47ec2c 100644 --- a/pkg/vmcp/composer/workflow_engine.go +++ b/pkg/vmcp/composer/workflow_engine.go @@ -17,6 +17,7 @@ import ( "golang.org/x/sync/errgroup" "github.com/stacklok/toolhive/pkg/audit" + mcpparser "github.com/stacklok/toolhive/pkg/mcp" "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/config" "github.com/stacklok/toolhive/pkg/vmcp/conversion" @@ -467,11 +468,22 @@ func (e *workflowEngine) callToolWithRetry( expBackoff.MaxInterval = 60 * initialDelay // Cap at 60x the initial delay expBackoff.Reset() + // SEP-2243 Mcp-Param-* headers for this step's backend tool. A composite step + // calls a backend tool exactly as a direct call would, so an annotating backend + // must receive the same mirrored headers or it answers -32020. Derived once + // outside the retry loop: args do not change between attempts. + paramHeaders, headerErr := mcpparser.ParamHeadersForSchema(e.getToolInputSchema(ctx, step.Tool), args) + if headerErr != nil { + // Zero attempts: the step never reached the backend. + return nil, 0, fmt.Errorf( + "deriving parameter headers for step %q tool %q: %w", step.ID, step.Tool, headerErr) + } + attemptCount := 0 operation := func() (*vmcp.ToolCallResult, error) { attemptCount++ // TODO: For composite tools, we may want to propagate metadata from the parent request - result, err := e.backendClient.CallTool(ctx, target, step.Tool, args, nil) + result, err := e.backendClient.CallTool(ctx, target, step.Tool, args, nil, paramHeaders) if err != nil { slog.Warn("tool call failed for step", "step", step.ID, "attempt", attemptCount, "max_attempts", maxRetries+1, "error", err) diff --git a/pkg/vmcp/composer/workflow_engine_test.go b/pkg/vmcp/composer/workflow_engine_test.go index f921144270..475fe99ec5 100644 --- a/pkg/vmcp/composer/workflow_engine_test.go +++ b/pkg/vmcp/composer/workflow_engine_test.go @@ -93,9 +93,9 @@ func TestWorkflowEngine_ExecuteWorkflow_WithRetry(t *testing.T) { // Fail once, then succeed gomock.InOrder( - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errors.New("temp fail")), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"ok": true}, Content: []vmcp.Content{}, @@ -133,7 +133,7 @@ func TestWorkflowEngine_ExecuteWorkflow_IsErrorHandling(t *testing.T) { // Return IsError=true twice, then succeed // This verifies that IsError=true triggers retry logic gomock.InOrder( - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ IsError: true, Content: []vmcp.Content{{ @@ -141,7 +141,7 @@ func TestWorkflowEngine_ExecuteWorkflow_IsErrorHandling(t *testing.T) { Text: "Tool execution failed: invalid input", }}, }, nil), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ IsError: true, Content: []vmcp.Content{{ @@ -149,7 +149,7 @@ func TestWorkflowEngine_ExecuteWorkflow_IsErrorHandling(t *testing.T) { Text: "Tool execution failed: temporary error", }}, }, nil), - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"ok": true}, Content: []vmcp.Content{}, @@ -186,7 +186,7 @@ func TestWorkflowEngine_ExecuteWorkflow_IsErrorExhaustsRetries(t *testing.T) { te.Router.EXPECT().RouteTool(gomock.Any(), "test.tool").Return(target, nil) // Always return IsError=true to exhaust all retries - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ IsError: true, Content: []vmcp.Content{{ @@ -283,8 +283,8 @@ func TestWorkflowEngine_ExecuteWorkflow_Timeout(t *testing.T) { target := &vmcp.BackendTarget{WorkloadID: "test", BaseURL: "http://test:8080"} // Both steps can run in parallel, so expect multiple calls te.Router.EXPECT().RouteTool(gomock.Any(), "test.tool").Return(target, nil).AnyTimes() - te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "test.tool", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { // Sleep longer than workflow timeout, but respect context cancellation select { case <-time.After(100 * time.Millisecond): @@ -469,8 +469,8 @@ func TestWorkflowEngine_ParallelExecution(t *testing.T) { // fetch_logs mockRouter.EXPECT().RouteTool(gomock.Any(), "test.fetch").Return(target, nil) - mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.fetch", map[string]any{"type": "logs"}, gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.fetch", map[string]any{"type": "logs"}, gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { trackStart("fetch_logs") time.Sleep(50 * time.Millisecond) trackEnd("fetch_logs") @@ -482,8 +482,8 @@ func TestWorkflowEngine_ParallelExecution(t *testing.T) { // fetch_metrics mockRouter.EXPECT().RouteTool(gomock.Any(), "test.fetch").Return(target, nil) - mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.fetch", map[string]any{"type": "metrics"}, gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.fetch", map[string]any{"type": "metrics"}, gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { trackStart("fetch_metrics") time.Sleep(50 * time.Millisecond) trackEnd("fetch_metrics") @@ -495,8 +495,8 @@ func TestWorkflowEngine_ParallelExecution(t *testing.T) { // create_report mockRouter.EXPECT().RouteTool(gomock.Any(), "test.report").Return(target, nil) - mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.report", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + mockBackend.EXPECT().CallTool(gomock.Any(), target, "test.report", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { trackStart("create_report") time.Sleep(30 * time.Millisecond) trackEnd("create_report") @@ -615,8 +615,8 @@ func TestWorkflowEngine_ExecuteWorkflow_WithWorkflowMetadata(t *testing.T) { } te.Router.EXPECT().RouteTool(gomock.Any(), "data.fetch").Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "data.fetch", map[string]any{"source": "test-source"}, gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "data.fetch", map[string]any{"source": "test-source"}, gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { time.Sleep(10 * time.Millisecond) return &vmcp.ToolCallResult{ StructuredContent: map[string]any{"result": "raw-data"}, @@ -625,8 +625,8 @@ func TestWorkflowEngine_ExecuteWorkflow_WithWorkflowMetadata(t *testing.T) { }) te.Router.EXPECT().RouteTool(gomock.Any(), "data.process").Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "data.process", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "data.process", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { time.Sleep(10 * time.Millisecond) return &vmcp.ToolCallResult{ StructuredContent: map[string]any{"value": "processed-data"}, @@ -719,8 +719,8 @@ func TestWorkflowEngine_WorkflowMetadataAvailableInTemplates(t *testing.T) { BaseURL: "http://test:8080", } te.Router.EXPECT().RouteTool(gomock.Any(), "tool.second").Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "tool.second", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "tool.second", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { // Verify workflow metadata was expanded in arguments workflowID, ok := args["workflow_id"].(string) assert.True(t, ok, "workflow_id should be a string") @@ -781,7 +781,7 @@ func TestWorkflowEngine_SessionEngine_CoercesTemplateStringToTypedArg(t *testing // Expect the backend to receive the coerced integer, not the string "42". coercedArgs := map[string]any{"limit": int64(42)} mockBackend.EXPECT(). - CallTool(gomock.Any(), target, "count_items", coercedArgs, gomock.Any()). + CallTool(gomock.Any(), target, "count_items", coercedArgs, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{StructuredContent: map[string]any{"items": []any{}}, Content: []vmcp.Content{}}, nil) workflow := &WorkflowDefinition{ @@ -832,7 +832,7 @@ func TestWorkflowEngine_SessionEngine_ToolNotInList_ReturnsNilSchema(t *testing. // Args pass through unmodified (string stays a string). rawArgs := map[string]any{"value": "hello"} mockBackend.EXPECT(). - CallTool(gomock.Any(), target, "other_tool", rawArgs, gomock.Any()). + CallTool(gomock.Any(), target, "other_tool", rawArgs, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{StructuredContent: map[string]any{"ok": true}, Content: []vmcp.Content{}}, nil) workflow := &WorkflowDefinition{ @@ -872,7 +872,7 @@ func TestWorkflowEngine_EmbeddedResourceAccessibleFromTemplate(t *testing.T) { } te.Router.EXPECT().RouteTool(gomock.Any(), "registry.get_referrer_content").Return(target, nil) te.Backend.EXPECT().CallTool(gomock.Any(), target, "registry.get_referrer_content", - map[string]any{"image": "ghcr.io/org/repo:latest"}, gomock.Any()). + map[string]any{"image": "ghcr.io/org/repo:latest"}, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{ "contentType": "sbom", @@ -887,8 +887,8 @@ func TestWorkflowEngine_EmbeddedResourceAccessibleFromTemplate(t *testing.T) { // Step 2: verify the template-expanded args pull from the right namespaces. te.Router.EXPECT().RouteTool(gomock.Any(), "sbom.analyze").Return(target, nil) - te.Backend.EXPECT().CallTool(gomock.Any(), target, "sbom.analyze", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args map[string]any, _ map[string]any) (*vmcp.ToolCallResult, error) { + te.Backend.EXPECT().CallTool(gomock.Any(), target, "sbom.analyze", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args map[string]any, _ map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { // .content.resource comes from the Content array's embedded resource assert.Equal(t, `{"spdxVersion":"SPDX-2.3","name":"mypackage"}`, args["sbom_data"]) // .output.format comes from structuredContent diff --git a/pkg/vmcp/core/admission_test.go b/pkg/vmcp/core/admission_test.go index 8a0064d1d1..cba2edafb8 100644 --- a/pkg/vmcp/core/admission_test.go +++ b/pkg/vmcp/core/admission_test.go @@ -545,7 +545,7 @@ func TestAdmission_ListCallLookupEnforceSameDecision(t *testing.T) { // Only the permitted tool ever reaches the backend. want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"ok": true}} - m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "weather", gomock.Any(), gomock.Any()).Return(want, nil) + m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "weather", gomock.Any(), gomock.Any(), gomock.Any()).Return(want, nil) c, err := New(cfg) require.NoError(t, err) @@ -601,7 +601,7 @@ func TestAdmission_AnnotationGatedDecisionMatchesListAndCall(t *testing.T) { // Only the read-only tool is callable, so only it reaches the backend. want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"ok": true}} - m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "ro", gomock.Any(), gomock.Any()).Return(want, nil) + m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "ro", gomock.Any(), gomock.Any(), gomock.Any()).Return(want, nil) c, err := New(cfg) require.NoError(t, err) @@ -684,7 +684,7 @@ func TestAdmission_CallToolForwardsArgsToAuthorizer(t *testing.T) { // Only the args-satisfying call reaches the backend. want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"ok": true}} - m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "deploy", gomock.Any(), gomock.Any()).Return(want, nil) + m.client.EXPECT().CallTool(gomock.Any(), gomock.Any(), "deploy", gomock.Any(), gomock.Any(), gomock.Any()).Return(want, nil) c, err := New(cfg) require.NoError(t, err) diff --git a/pkg/vmcp/core/core_calls.go b/pkg/vmcp/core/core_calls.go index 0505073ece..54231404a5 100644 --- a/pkg/vmcp/core/core_calls.go +++ b/pkg/vmcp/core/core_calls.go @@ -10,8 +10,10 @@ import ( "fmt" "log/slog" "maps" + "slices" "github.com/stacklok/toolhive/pkg/auth" + mcpparser "github.com/stacklok/toolhive/pkg/mcp" "github.com/stacklok/toolhive/pkg/vmcp" "github.com/stacklok/toolhive/pkg/vmcp/composer" "github.com/stacklok/toolhive/pkg/vmcp/router" @@ -66,7 +68,18 @@ func (c *coreVMCP) CallTool( } return nil, fmt.Errorf("routing tool %q: %w", name, err) } - result, err := c.backendClient.CallTool(ctx, target, name, argsCopy, metaCopy) + // SEP-2243 Mcp-Param-* mirroring: a backend MAY designate tool parameters, + // via x-mcp-header in its inputSchema, whose values it also expects as HTTP + // headers — and rejects the call with -32020 if they are missing. Deriving + // that here is what makes an annotating backend callable at all. The core is + // the right place because the aggregated view already holds the tool's + // schema, so the backend client needs no schema cache of its own. + paramHeaders, err := paramHeadersFor(agg.Tools, name, argsCopy) + if err != nil { + return nil, err + } + + result, err := c.backendClient.CallTool(ctx, target, name, argsCopy, metaCopy, paramHeaders) if err != nil { return nil, err } @@ -249,6 +262,36 @@ func executeComposite( }, nil } +// paramHeadersFor derives the SEP-2243 Mcp-Param-* headers for a call to the +// named tool, from the tool's own x-mcp-header annotations and the call's +// arguments. Returns nil when the tool is not in the view or declares no +// annotations — the common case, and cheap: ParamHeaders exits immediately on a +// schema with no annotation. +// +// A malformed annotation cannot reach here: pkg/vmcp/client rejects such a tool +// at ingestion, so it is never advertised. An UNMIRRORABLE ARGUMENT can, though — +// the caller supplies the values — and is surfaced as an invalid-parameters error +// rather than dropped, because silently omitting the header would make the +// backend answer -32020 and turn a caller mistake into an opaque backend failure. +func paramHeadersFor(tools []vmcp.Tool, name string, args map[string]any) (map[string]string, error) { + idx := slices.IndexFunc(tools, func(t vmcp.Tool) bool { return t.Name == name }) + if idx < 0 { + return nil, nil + } + headers, err := mcpparser.ParamHeadersForSchema(tools[idx].InputSchema, args) + if err == nil { + return headers, nil + } + if errors.Is(err, mcpparser.ErrUnmirrorableValue) { + // The caller's argument value is at fault (a control character, a + // non-integral integer), so name it as invalid input. + return nil, fmt.Errorf("%w: tool %q: %w", vmcp.ErrInvalidInput, name, err) + } + // A malformed annotation. Defensive: ingestion already rejected this shape, so + // reaching here is an internal inconsistency, not the caller's mistake. + return nil, fmt.Errorf("tool %q has an invalid x-mcp-header annotation: %w", name, err) +} + // compositeErrorResult builds a tool-level error result for a failed workflow. func compositeErrorResult(msg string) *vmcp.ToolCallResult { return &vmcp.ToolCallResult{ diff --git a/pkg/vmcp/core/core_calls_test.go b/pkg/vmcp/core/core_calls_test.go index a2102d8aad..1e8e6185ce 100644 --- a/pkg/vmcp/core/core_calls_test.go +++ b/pkg/vmcp/core/core_calls_test.go @@ -35,7 +35,7 @@ func TestCallTool_RoutesToBackend(t *testing.T) { want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"result": "ok"}} m.client.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), "tool_a", gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), gomock.Any(), "tool_a", gomock.Any(), gomock.Any(), gomock.Any()). Return(want, nil) c, err := New(cfg) @@ -74,8 +74,8 @@ func TestCallTool_CopyBeforeMutate(t *testing.T) { // The backend client mutates the maps it receives; the caller's originals // must be untouched because CallTool forwards clones. m.client.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), "tool_a", gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args, meta map[string]any) (*vmcp.ToolCallResult, error) { + CallTool(gomock.Any(), gomock.Any(), "tool_a", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *vmcp.BackendTarget, _ string, args, meta map[string]any, _ map[string]string) (*vmcp.ToolCallResult, error) { args["injected"] = true meta["injected"] = true return &vmcp.ToolCallResult{}, nil @@ -110,7 +110,7 @@ func TestCallTool_CompositeWorkflow(t *testing.T) { // The composite workflow's single tool step routes to the backend through the // per-call composer built from the aggregated routing table. m.client.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), "be1.echo", gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), gomock.Any(), "be1.echo", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{StructuredContent: map[string]any{"ok": true}}, nil) c, err := New(cfg) @@ -258,7 +258,7 @@ func TestCallTool_ResolvesRenamedTool(t *testing.T) { }) m.client.EXPECT(). - CallTool(gomock.Any(), target, "be1.echo", gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), target, "be1.echo", gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{}, nil) c, err := New(cfg) @@ -308,7 +308,7 @@ func TestCompositeNameConflict_AdvertisedEqualsExecuted(t *testing.T) { // CallTool("shared") must route to the backend, not execute the composite. want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"from": "backend"}} - m.client.EXPECT().CallTool(gomock.Any(), beTarget, "shared", gomock.Any(), gomock.Any()).Return(want, nil) + m.client.EXPECT().CallTool(gomock.Any(), beTarget, "shared", gomock.Any(), gomock.Any(), gomock.Any()).Return(want, nil) got, err := c.CallTool(context.Background(), nil, "shared", nil, nil) require.NoError(t, err) diff --git a/pkg/vmcp/core/xmcpheader_derivation_test.go b/pkg/vmcp/core/xmcpheader_derivation_test.go new file mode 100644 index 0000000000..9cb3825246 --- /dev/null +++ b/pkg/vmcp/core/xmcpheader_derivation_test.go @@ -0,0 +1,133 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package core + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive/pkg/vmcp" + "github.com/stacklok/toolhive/pkg/vmcp/aggregator" +) + +// annotatedTool is backendTool with an x-mcp-header annotation on one parameter, +// the SEP-2243 worked example (execute_sql / Region). +func annotatedTool(name, header string) vmcp.Tool { + t := backendTool(name) + t.InputSchema = map[string]any{ + "type": "object", + "properties": map[string]any{ + "region": map[string]any{"type": "string", "x-mcp-header": header}, + "query": map[string]any{"type": "string"}, + }, + } + return t +} + +// TestCallTool_DerivesParamHeadersFromSchema is the pin that makes the whole +// feature non-vacuous: the core is the only layer holding both the tool's +// inputSchema and the call's arguments, so if it fails to derive the Mcp-Param-* +// headers nothing downstream can. Asserted on the exact map handed to the backend +// client rather than on a wire capture, because that hand-off is the seam this +// change introduces. +func TestCallTool_DerivesParamHeadersFromSchema(t *testing.T) { + t.Parallel() + cfg, m := baseConfig(t) + + target := backendTarget() + expectAggregation(m, &aggregator.AggregatedCapabilities{ + Tools: []vmcp.Tool{annotatedTool("execute_sql", "Region")}, + RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{"execute_sql": target}}, + }) + + var gotHeaders map[string]string + m.client.EXPECT(). + CallTool(gomock.Any(), gomock.Any(), "execute_sql", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, + paramHeaders map[string]string, + ) (*vmcp.ToolCallResult, error) { + gotHeaders = paramHeaders + return &vmcp.ToolCallResult{}, nil + }) + + c, err := New(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = c.Close() }) + + _, err = c.CallTool(context.Background(), nil, "execute_sql", + map[string]any{"region": "eu-west1", "query": "select 1"}, nil) + require.NoError(t, err) + + // Only the designated parameter is mirrored; "query" carries no annotation. + assert.Equal(t, map[string]string{"Mcp-Param-Region": "eu-west1"}, gotHeaders) +} + +// TestCallTool_NoAnnotationsDerivesNoHeaders covers the common case: a tool with +// no x-mcp-header annotation must hand the backend client nil, not an empty map, +// so nothing is added to the wire. +func TestCallTool_NoAnnotationsDerivesNoHeaders(t *testing.T) { + t.Parallel() + cfg, m := baseConfig(t) + + target := backendTarget() + expectAggregation(m, &aggregator.AggregatedCapabilities{ + Tools: []vmcp.Tool{backendTool("tool_a")}, + RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{"tool_a": target}}, + }) + + var gotHeaders map[string]string + called := false + m.client.EXPECT(). + CallTool(gomock.Any(), gomock.Any(), "tool_a", gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ context.Context, _ *vmcp.BackendTarget, _ string, _ map[string]any, _ map[string]any, + paramHeaders map[string]string, + ) (*vmcp.ToolCallResult, error) { + gotHeaders = paramHeaders + called = true + return &vmcp.ToolCallResult{}, nil + }) + + c, err := New(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = c.Close() }) + + _, err = c.CallTool(context.Background(), nil, "tool_a", map[string]any{"a": 1}, nil) + require.NoError(t, err) + assert.True(t, called) + assert.Nil(t, gotHeaders) +} + +// TestCallTool_UnmirrorableArgumentFailsBeforeDispatch pins the fail-closed +// direction. A caller-supplied value carrying a CRLF must abort the call rather +// than be dropped: dropping it would send no header, the backend would answer +// -32020, and a caller mistake would surface as an opaque backend failure. The +// backend client must therefore never be reached — asserted by setting no EXPECT +// on it, so any call fails the gomock controller. +func TestCallTool_UnmirrorableArgumentFailsBeforeDispatch(t *testing.T) { + t.Parallel() + cfg, m := baseConfig(t) + + target := backendTarget() + expectAggregation(m, &aggregator.AggregatedCapabilities{ + Tools: []vmcp.Tool{annotatedTool("execute_sql", "Region")}, + RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{"execute_sql": target}}, + }) + + c, err := New(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = c.Close() }) + + _, err = c.CallTool(context.Background(), nil, "execute_sql", + map[string]any{"region": "eu\r\nX-Evil: 1"}, nil) + require.Error(t, err) + assert.True(t, errors.Is(err, vmcp.ErrInvalidInput), + "a bad argument value must be reported as invalid input, got %v", err) +} diff --git a/pkg/vmcp/internal/backendtelemetry/backendtelemetry.go b/pkg/vmcp/internal/backendtelemetry/backendtelemetry.go index de2b132bae..806c1a5726 100644 --- a/pkg/vmcp/internal/backendtelemetry/backendtelemetry.go +++ b/pkg/vmcp/internal/backendtelemetry/backendtelemetry.go @@ -265,6 +265,7 @@ func (t telemetryBackendClient) CallTool( toolName string, arguments map[string]any, meta map[string]any, + paramHeaders map[string]string, ) (_ *vmcp.ToolCallResult, retErr error) { attrs := []attribute.KeyValue{ attribute.String("tool_name", toolName), // backward compat @@ -276,7 +277,7 @@ func (t telemetryBackendClient) CallTool( } ctx, done := t.record(ctx, target, "call_tool", toolName, &retErr, attrs...) defer done() - return t.backendClient.CallTool(ctx, target, toolName, arguments, meta) + return t.backendClient.CallTool(ctx, target, toolName, arguments, meta, paramHeaders) } func (t telemetryBackendClient) ReadResource( diff --git a/pkg/vmcp/mocks/mock_backend_client.go b/pkg/vmcp/mocks/mock_backend_client.go index 2a4958d211..c29abb899b 100644 --- a/pkg/vmcp/mocks/mock_backend_client.go +++ b/pkg/vmcp/mocks/mock_backend_client.go @@ -81,18 +81,18 @@ func (m *MockBackendClient) EXPECT() *MockBackendClientMockRecorder { } // CallTool mocks base method. -func (m *MockBackendClient) CallTool(ctx context.Context, target *vmcp.BackendTarget, toolName string, arguments, meta map[string]any) (*vmcp.ToolCallResult, error) { +func (m *MockBackendClient) CallTool(ctx context.Context, target *vmcp.BackendTarget, toolName string, arguments, meta map[string]any, paramHeaders map[string]string) (*vmcp.ToolCallResult, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CallTool", ctx, target, toolName, arguments, meta) + ret := m.ctrl.Call(m, "CallTool", ctx, target, toolName, arguments, meta, paramHeaders) ret0, _ := ret[0].(*vmcp.ToolCallResult) ret1, _ := ret[1].(error) return ret0, ret1 } // CallTool indicates an expected call of CallTool. -func (mr *MockBackendClientMockRecorder) CallTool(ctx, target, toolName, arguments, meta any) *gomock.Call { +func (mr *MockBackendClientMockRecorder) CallTool(ctx, target, toolName, arguments, meta, paramHeaders any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CallTool", reflect.TypeOf((*MockBackendClient)(nil).CallTool), ctx, target, toolName, arguments, meta) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CallTool", reflect.TypeOf((*MockBackendClient)(nil).CallTool), ctx, target, toolName, arguments, meta, paramHeaders) } // Complete mocks base method. diff --git a/pkg/vmcp/server/adapter/handler_factory.go b/pkg/vmcp/server/adapter/handler_factory.go index f8a3dfb7a5..7581b68684 100644 --- a/pkg/vmcp/server/adapter/handler_factory.go +++ b/pkg/vmcp/server/adapter/handler_factory.go @@ -92,8 +92,14 @@ func (f *DefaultHandlerFactory) CreateToolHandler( // Extract metadata from request to forward to backend meta := conversion.FromMCPMeta(request.Params.Meta) - // Call the backend tool - the backend client handles name translation and metadata forwarding - result, err := f.backendClient.CallTool(ctx, target, toolName, args, meta) + // Call the backend tool - the backend client handles name translation and metadata forwarding. + // + // No SEP-2243 Mcp-Param-* headers are mirrored on this path: deriving them + // needs the tool's inputSchema, and this factory holds only a router and a + // backend client. That is a limitation of this legacy adapter, not a policy + // choice — the live Serve path routes tools/call through the core, which has + // the aggregated schema and does mirror (see core_calls.go paramHeadersFor). + result, err := f.backendClient.CallTool(ctx, target, toolName, args, meta, nil) if err != nil { // Only actual network/transport errors reach here now (IsError=true is handled in result) if errors.Is(err, vmcp.ErrBackendUnavailable) { diff --git a/pkg/vmcp/server/adapter/handler_factory_test.go b/pkg/vmcp/server/adapter/handler_factory_test.go index d2d36856fb..81b3c95b44 100644 --- a/pkg/vmcp/server/adapter/handler_factory_test.go +++ b/pkg/vmcp/server/adapter/handler_factory_test.go @@ -67,7 +67,7 @@ func TestDefaultHandlerFactory_CreateToolHandler(t *testing.T) { CallTool(gomock.Any(), target, "test_tool", map[string]any{ "input": "test", "count": 42, - }, gomock.Any()). + }, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{StructuredContent: expectedResult}, nil) }, request: mcp.CallToolRequest{ @@ -172,7 +172,7 @@ func TestDefaultHandlerFactory_CreateToolHandler(t *testing.T) { Return(target, nil) mockClient.EXPECT(). - CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any()). + CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ Content: []vmcp.Content{ {Type: vmcp.ContentTypeText, Text: "tool execution failed"}, @@ -206,7 +206,7 @@ func TestDefaultHandlerFactory_CreateToolHandler(t *testing.T) { Return(target, nil) mockClient.EXPECT(). - CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any()). + CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any(), gomock.Any()). Return(nil, vmcp.ErrBackendUnavailable) }, request: mcp.CallToolRequest{ @@ -235,7 +235,7 @@ func TestDefaultHandlerFactory_CreateToolHandler(t *testing.T) { Return(target, nil) mockClient.EXPECT(). - CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any()). + CallTool(gomock.Any(), target, "test_tool", map[string]any{"input": "test"}, gomock.Any(), gomock.Any()). Return(nil, errors.New("unknown backend error")) }, request: mcp.CallToolRequest{ @@ -269,7 +269,7 @@ func TestDefaultHandlerFactory_CreateToolHandler(t *testing.T) { // Handler factory now passes the client-facing name (backend1_fetch) // Backend client handles translation to original name (fetch) mockClient.EXPECT(). - CallTool(gomock.Any(), target, "backend1_fetch", map[string]any{"url": "https://example.com"}, gomock.Any()). + CallTool(gomock.Any(), target, "backend1_fetch", map[string]any{"url": "https://example.com"}, gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{StructuredContent: expectedResult}, nil) }, request: mcp.CallToolRequest{ diff --git a/pkg/vmcp/server/integration_test.go b/pkg/vmcp/server/integration_test.go index 4256219b4d..f73b6bfb13 100644 --- a/pkg/vmcp/server/integration_test.go +++ b/pkg/vmcp/server/integration_test.go @@ -395,7 +395,7 @@ func TestIntegration_AuditLogging(t *testing.T) { AnyTimes() mockBackendClient.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{ "result": "Sunny, 72°F", diff --git a/pkg/vmcp/server/session_management_integration_test.go b/pkg/vmcp/server/session_management_integration_test.go index ec46d751ed..a6b1850db1 100644 --- a/pkg/vmcp/server/session_management_integration_test.go +++ b/pkg/vmcp/server/session_management_integration_test.go @@ -212,7 +212,7 @@ func buildTestServerWithOptions( // own CallTool is bypassed on the Serve path). Return a deterministic result so call // tests can assert on it. mockBackendClient.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{Content: []vmcp.Content{{Type: "text", Text: "fake result"}}}, nil). AnyTimes() diff --git a/pkg/vmcp/server/telemetry_integration_test.go b/pkg/vmcp/server/telemetry_integration_test.go index 80b35a7666..0a23b05e43 100644 --- a/pkg/vmcp/server/telemetry_integration_test.go +++ b/pkg/vmcp/server/telemetry_integration_test.go @@ -173,7 +173,7 @@ func TestIntegration_TelemetryMiddleware(t *testing.T) { // Use MinTimes(1) to verify the backend client is actually called during tool execution. // If the tool call doesn't reach the backend client, this will cause a test failure. mockBackendClient.EXPECT(). - CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + CallTool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&vmcp.ToolCallResult{ StructuredContent: map[string]any{"result": "found"}, Content: []vmcp.Content{}, diff --git a/pkg/vmcp/types.go b/pkg/vmcp/types.go index ff2bd9d425..46a06fb3aa 100644 --- a/pkg/vmcp/types.go +++ b/pkg/vmcp/types.go @@ -730,8 +730,18 @@ type BackendClient interface { // CallTool invokes a tool on the backend MCP server. // The meta parameter contains _meta fields from the client request that should be forwarded to the backend. // Returns the complete tool result including _meta field from the backend response. + // + // paramHeaders carries the SEP-2243 Mcp-Param-* headers mirrored from the tool's + // x-mcp-header-designated arguments, keyed by full header name. It is supplied by + // the caller because deriving it needs the tool's inputSchema, which lives in the + // aggregated capability view rather than on BackendTarget; passing it explicitly + // keeps this client free of a schema cache and its staleness window. nil or empty + // means nothing to mirror, which is the common case. It applies only to a Modern + // (2026-07-28) backend hop -- SEP-2243 is part of that revision -- and is ignored + // for a Legacy backend. CallTool( - ctx context.Context, target *BackendTarget, toolName string, arguments map[string]any, meta map[string]any, + ctx context.Context, target *BackendTarget, toolName string, arguments map[string]any, + meta map[string]any, paramHeaders map[string]string, ) (*ToolCallResult, error) // ReadResource retrieves a resource from the backend MCP server.