Skip to content

Reject backend tools with invalid x-mcp-header - #6013

Merged
JAORMX merged 2 commits into
mainfrom
validate-xmcp-header-annotations
Jul 27, 2026
Merged

Reject backend tools with invalid x-mcp-header#6013
JAORMX merged 2 commits into
mainfrom
validate-xmcp-header-annotations

Conversation

@JAORMX

@JAORMX JAORMX commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

SEP-2243 ("HTTP standardization") lets an MCP server designate 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. The extension constrains what that annotation's value may be, 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 — x-mcp-header had zero matches anywhere in the repo.

Why this is not contained to the tool that carries it. vMCP republishes aggregated backend schemas to its own downstream clients. An invalid annotation forwarded in the aggregated tools/list would make a conformant downstream client reject the entire list, taking every other backend's tools down with it. Dropping the one offending tool is the narrower failure. Separately, a CRLF in an annotation stops being cosmetic the moment mirroring lands (the stacked PR), because the value becomes an outgoing header name.

  • Validate at newCapabilityListFromMCP — the single ingestion seam the Legacy and Modern paths share, so neither can regress independently.
  • The annotation's vocabulary and constraint checks live in one place (pkg/mcp/xmcpheader.go), so this ingestion check and the call-time header derivation that follows cannot drift in their reading of the spec. This is deliberately the mistake called out for the reserved _meta keys in Strip reserved io.modelcontextprotocol/* keys from backend response _meta (shared Legacy+Modern helper) #5986 ("a third independent copy").
  • 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 over an attacker-supplied schema is a stack-exhaustion vector.

Constraints enforced, per SEP-2243: non-empty; valid HTTP field-name token (RFC 9110 tchar, which also excludes control characters and whitespace); case-insensitively unique across the whole inputSchema (HTTP field names are case-insensitive, so two spellings collide on the wire); and permitted only on a parameter declaring string, integer, or boolean — notably not number, which the SEP excludes because a float has no canonical wire spelling.

Type of change

  • Bug fix

Test plan

  • Unit tests (task test) — full suite; the only failures are pkg/api and pkg/secrets/keyring, both environmental (no available runtime found, keyring unavailable) and verified identical on main
  • Linting (task lint-fix) — 0 issues
  • Manual testing (described below)

pkg/mcp/xmcpheader_test.go covers each constraint in both directions, plus the traversal cases that make the walk non-trivial: nesting inside object properties, array items, and oneOf/anyOf/allOf branches (those carry real schemas in this repo's tool sets — #5976 fixed ingestion dropping them — so an annotation inside a branch would otherwise go unvalidated). pkg/vmcp/client/xmcpheader_ingestion_test.go asserts the rejection at the seam.

Manual: ran go vet ./... as a separate gate. Worth knowing for other PRs: task test's formatter prints only failing packages and silently swallows build failures, so a package whose tests do not compile reads as green. That is how I initially misread this branch. go vet ./... catches it.

API Compatibility

  • This PR does not break the v1beta1 API.

No operator API surface is touched — no CRD, no api/v1* type.

Does this introduce a user-facing change?

Yes, though it is unreachable in practice today. A backend tool whose x-mcp-header annotation violates SEP-2243 is no longer advertised through vMCP, and a WARN naming the backend, the tool, and the violation is logged. No tool in any catalog currently uses the annotation (zero matches repo-wide), so no existing deployment changes behaviour.

Special notes for reviewers

The judgement call worth challenging is treating a missing or non-string type on an annotated parameter as a violation. SEP-2243 permits the annotation only on primitive parameters, and an undeclared type is not a declared primitive; mirroring also cannot serialize a value whose type it does not know. So this rejects rather than guessing a spelling. It is the strict reading and it is documented as such at ParamHeaders. Note the constraint applies only to parameters that are annotated — an unannotated parameter with no type is untouched, so the strictness cannot leak onto ordinary tools.

This is deliberately narrower than #6002 item 2, which also asks for the egress mirroring. That half needs the inputSchema at tools/call time and so changes BackendClient.CallTool; it is the stacked follow-up rather than being crammed in here.

Also note #6002 item 1 is already implemented on main (it landed in #5993 itself) — details here.

Refs #6002

Generated with Claude Code

@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Jul 27, 2026
rdimitrov
rdimitrov previously approved these changes Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.56522% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.14%. Comparing base (f1dae94) to head (ad85feb).

Files with missing lines Patch % Lines
pkg/mcp/xmcpheader.go 90.95% 12 Missing and 5 partials ⚠️
pkg/vmcp/client/client.go 84.21% 0 Missing and 3 partials ⚠️
pkg/vmcp/composer/workflow_engine.go 40.00% 2 Missing and 1 partial ⚠️
pkg/vmcp/core/core_calls.go 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6013      +/-   ##
==========================================
+ Coverage   72.10%   72.14%   +0.03%     
==========================================
  Files         720      721       +1     
  Lines       74745    74960     +215     
==========================================
+ Hits        53898    54078     +180     
- Misses      16991    17023      +32     
- Partials     3856     3859       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/L Large PR: 600-999 lines changed labels Jul 27, 2026
JAORMX added 2 commits July 27, 2026 13:31
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
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
@JAORMX
JAORMX force-pushed the validate-xmcp-header-annotations branch from 7937889 to ad85feb Compare July 27, 2026 13:34
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jul 27, 2026
@JAORMX
JAORMX requested a review from rdimitrov July 27, 2026 13:37
@JAORMX
JAORMX merged commit 83394ee into main Jul 27, 2026
48 checks passed
@JAORMX
JAORMX deleted the validate-xmcp-header-annotations branch July 27, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants