From 18e591153ca2d56e5f3faba7b9b93d08cb66a0d4 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 21 Jul 2026 23:00:21 +0100 Subject: [PATCH 1/6] Validate Mcp-Method/Mcp-Name headers against MCP body vMCP's stateless decode seam needs to reject Modern (2026-07-28) requests whose Mcp-Method/Mcp-Name headers contradict the parsed JSON-RPC body, per the draft spec's Server Validation rules. Add ValidateHeaderConsistency alongside the existing ClassifyRevision, reusing the -32020 HeaderMismatch code and decoding the draft spec's base64 sentinel wrapper for Mcp-Name before comparing. Co-Authored-By: Claude Sonnet 5 --- pkg/mcp/revision.go | 93 +++++++++++++++++++ pkg/mcp/revision_test.go | 187 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 280 insertions(+) diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go index 5321ba9929..82c596a4a6 100644 --- a/pkg/mcp/revision.go +++ b/pkg/mcp/revision.go @@ -4,8 +4,10 @@ package mcp import ( + "encoding/base64" "encoding/json" "fmt" + "strings" ) // Revision identifies which MCP protocol era a request belongs to. @@ -62,6 +64,8 @@ var reservedModernMetaKeys = []string{metaKeyProtocolVersion, metaKeyClientInfo, const ( // CodeHeaderMismatch signals a mismatch between the MCP-Protocol-Version // header and the _meta protocol version (schema.ts HeaderMismatchError). + // Also reused by RequestHeaderMismatchError for the Mcp-Method/Mcp-Name + // headers: keep both error types' Code() in sync with this constant. CodeHeaderMismatch int64 = -32020 // CodeMissingClientCapability signals that _meta is missing a client // capability required for the request (schema.ts MissingRequiredClientCapabilityError). @@ -97,6 +101,33 @@ func (e *HeaderMismatchError) Data() map[string]any { return map[string]any{"header": e.Header, "body": e.Body} } +// RequestHeaderMismatchError indicates a Modern (2026-07-28) request header +// contradicted the value carried in the request body. This is the +// generalized sibling of HeaderMismatchError for the Mcp-Method/Mcp-Name +// headers: HeaderMismatchError is specific to MCP-Protocol-Version, while +// this type covers any other header/body consistency check and names which +// header failed. +type RequestHeaderMismatchError struct { + // Header is the name of the mismatched header (e.g. "Mcp-Method", "Mcp-Name"). + Header string + // HeaderValue is the (decoded, where applicable) value carried by the header. + HeaderValue string + // BodyValue is the value the header was compared against in the request body. + BodyValue string +} + +func (e *RequestHeaderMismatchError) Error() string { + return fmt.Sprintf("%s header %q does not match request body value %q", e.Header, e.HeaderValue, e.BodyValue) +} + +// Code implements CodedError. +func (*RequestHeaderMismatchError) Code() int64 { return CodeHeaderMismatch } + +// Data implements CodedError. +func (e *RequestHeaderMismatchError) Data() map[string]any { + return map[string]any{"header": e.Header, "headerValue": e.HeaderValue, "bodyValue": e.BodyValue} +} + // UnsupportedVersionError indicates the _meta protocol version named a Modern // revision this server does not support. type UnsupportedVersionError struct { @@ -247,6 +278,41 @@ func ClassifyRevision(method string, meta map[string]any, protoHeader string) (R return RevisionModern, nil } +// ValidateHeaderConsistency checks the Modern (2026-07-28) Mcp-Method and +// Mcp-Name request headers, when present, against the corresponding parsed +// request body fields (Method and ResourceID). +// A header value that contradicts the body is a hard rejection +// (*RequestHeaderMismatchError); a Mcp-Name header is decoded via +// decodeSentinelName before comparison, since the draft spec allows it to be +// sentinel-encoded. Requests carrying neither header return nil unconditionally, +// which is the correct no-op for Legacy traffic: Legacy clients never send +// these headers. +// +// This function does not yet enforce presence/requiredness of these headers +// on Modern requests, only consistency when present, for the same +// transport-ambiguity reason noted on the TODO above (this classifier cannot +// yet tell stdio's "no header concept" apart from HTTP's "header omitted"). +func ValidateHeaderConsistency(parsed *ParsedMCPRequest) error { + if parsed.MCPMethodHeader != "" && parsed.MCPMethodHeader != parsed.Method { + return &RequestHeaderMismatchError{Header: "Mcp-Method", HeaderValue: parsed.MCPMethodHeader, BodyValue: parsed.Method} + } + + if parsed.MCPNameHeader != "" { + decoded, err := decodeSentinelName(parsed.MCPNameHeader) + if err != nil || decoded != parsed.ResourceID { + headerValue := func() string { + if err != nil { + return parsed.MCPNameHeader + } + return decoded + }() + return &RequestHeaderMismatchError{Header: "Mcp-Name", HeaderValue: headerValue, BodyValue: parsed.ResourceID} + } + } + + return nil +} + // ExtractMeta pulls the "_meta" object out of raw JSON-RPC request params, for // use with ClassifyRevision. It is deliberately tolerant: absent params, params // that don't decode as a JSON object, or a "_meta" value that isn't itself an @@ -325,3 +391,30 @@ func objectMetaValue(meta map[string]any, key string) (map[string]any, bool) { } return obj, true } + +// sentinelPrefix and sentinelSuffix wrap a base64-encoded Mcp-Name header +// value per the draft MCP spec. This is NOT RFC 2047 encoded-word syntax +// (there is no encoding-letter field); both markers are literal and +// case-sensitive. +const ( + sentinelPrefix = "=?base64?" + sentinelSuffix = "?=" +) + +// decodeSentinelName decodes a Mcp-Name header value that may be wrapped in +// the draft spec's base64 sentinel format (=?base64??=). A value +// that isn't wrapped in the sentinel markers is returned unchanged, since it +// is already the plain name/uri. A wrapped value whose payload fails to +// base64-decode is reported as an error. +func decodeSentinelName(v string) (string, error) { + if !strings.HasPrefix(v, sentinelPrefix) || !strings.HasSuffix(v, sentinelSuffix) { + return v, nil + } + + payload := strings.TrimSuffix(strings.TrimPrefix(v, sentinelPrefix), sentinelSuffix) + decoded, err := base64.StdEncoding.DecodeString(payload) + if err != nil { + return "", fmt.Errorf("decoding base64 sentinel Mcp-Name payload: %w", err) + } + return string(decoded), nil +} diff --git a/pkg/mcp/revision_test.go b/pkg/mcp/revision_test.go index c3db7e3505..06ef5d04ec 100644 --- a/pkg/mcp/revision_test.go +++ b/pkg/mcp/revision_test.go @@ -4,6 +4,7 @@ package mcp import ( + "encoding/base64" "encoding/json" "testing" @@ -12,6 +13,7 @@ import ( ) var _ CodedError = (*HeaderMismatchError)(nil) +var _ CodedError = (*RequestHeaderMismatchError)(nil) var _ CodedError = (*UnsupportedVersionError)(nil) var _ CodedError = (*MissingClientCapabilityError)(nil) var _ CodedError = (*MissingModernMetadataError)(nil) @@ -393,3 +395,188 @@ func TestExtractMeta(t *testing.T) { }) } } + +func sentinelEncode(name string) string { + return "=?base64?" + base64.StdEncoding.EncodeToString([]byte(name)) + "?=" +} + +func TestValidateHeaderConsistency(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + parsed *ParsedMCPRequest + checkErr func(t *testing.T, err error) + }{ + { + name: "both headers absent is a legacy no-op", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "Mcp-Method matches body method", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "Mcp-Method mismatches body method", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "resources/read", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, int64(-32020), mismatchErr.Code()) + assert.Equal(t, "Mcp-Method", mismatchErr.Header) + assert.Equal(t, "resources/read", mismatchErr.HeaderValue) + assert.Equal(t, "tools/call", mismatchErr.BodyValue) + }, + }, + { + name: "Mcp-Name plain string matches ResourceID", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPNameHeader: "my-tool", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "Mcp-Name plain string mismatches ResourceID", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPNameHeader: "other-tool", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, "Mcp-Name", mismatchErr.Header) + assert.Equal(t, "other-tool", mismatchErr.HeaderValue) + assert.Equal(t, "my-tool", mismatchErr.BodyValue) + }, + }, + { + name: "Mcp-Name sentinel-encoded decodes to matching ResourceID", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPNameHeader: sentinelEncode("my-tool"), + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "Mcp-Name sentinel-encoded decodes to mismatching value", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPNameHeader: sentinelEncode("other-tool"), + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, "Mcp-Name", mismatchErr.Header) + assert.Equal(t, "other-tool", mismatchErr.HeaderValue) + assert.Equal(t, "my-tool", mismatchErr.BodyValue) + }, + }, + { + name: "Mcp-Name sentinel wrapper with invalid base64 payload", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPNameHeader: "=?base64?not-valid-base64!!?=", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, "Mcp-Name", mismatchErr.Header) + assert.Equal(t, "=?base64?not-valid-base64!!?=", mismatchErr.HeaderValue) + assert.Equal(t, "my-tool", mismatchErr.BodyValue) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateHeaderConsistency(tt.parsed) + tt.checkErr(t, err) + }) + } +} + +func TestDecodeSentinelName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want string + wantErr bool + }{ + { + name: "non-sentinel value passes through unchanged", + input: "my-tool", + want: "my-tool", + }, + { + name: "empty value passes through unchanged", + input: "", + want: "", + }, + { + name: "valid sentinel decodes the base64 payload", + input: sentinelEncode("my-tool"), + want: "my-tool", + }, + { + name: "sentinel wrapper with invalid base64 payload errors", + input: "=?base64?not-valid-base64!!?=", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := decodeSentinelName(tt.input) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} From c1b7c5e5c67c0dc57532dafdbc5b923efd97d380 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 21 Jul 2026 23:09:15 +0100 Subject: [PATCH 2/6] Add vMCP decode-seam classification middleware Introduce classificationMiddleware to reject malformed Modern (2026-07-28) requests at the vMCP decode seam using the existing ClassifyRevision/ValidateHeaderConsistency helpers, ahead of wiring it into the server's middleware chain. Not yet composed into server.go: Legacy traffic is unaffected until that follow-up step. Co-Authored-By: Claude Sonnet 5 --- pkg/vmcp/server/classification.go | 46 +++++++ pkg/vmcp/server/classification_test.go | 169 +++++++++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 pkg/vmcp/server/classification.go create mode 100644 pkg/vmcp/server/classification_test.go diff --git a/pkg/vmcp/server/classification.go b/pkg/vmcp/server/classification.go new file mode 100644 index 0000000000..3eeb514fca --- /dev/null +++ b/pkg/vmcp/server/classification.go @@ -0,0 +1,46 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "net/http" + + mcpparser "github.com/stacklok/toolhive/pkg/mcp" +) + +// classificationMiddleware classifies a parsed MCP request as Legacy +// (2025-11-25) or Modern (2026-07-28) at the decode seam and rejects a +// malformed Modern request with the correct JSON-RPC error before it reaches +// dispatch. The classified mcpparser.Revision is deliberately dropped: it is +// not stashed in context or anywhere else, since no downstream consumer reads +// it yet. Legacy traffic and well-formed Modern requests both fall through to +// the same next handler unchanged — Modern-specific dispatch is a later +// phase (toolhive issue #5756). +// +// This middleware makes no authentication/authorization decision and confers +// no elevated trust on requests that pass it — it only validates protocol +// shape. It must run after ParsingMiddleware (so GetParsedMCPRequest is +// populated) and is expected to run after any auth middleware in the chain. +func classificationMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + parsed := mcpparser.GetParsedMCPRequest(r.Context()) + if parsed == nil { + next.ServeHTTP(w, r) + return + } + + protoHeader := r.Header.Get("MCP-Protocol-Version") + if _, err := mcpparser.ClassifyRevision(parsed.Method, parsed.Meta, protoHeader); err != nil { + mcpparser.WriteClassificationError(w, parsed.ID, err) + return + } + + if err := mcpparser.ValidateHeaderConsistency(parsed); err != nil { + mcpparser.WriteClassificationError(w, parsed.ID, err) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/pkg/vmcp/server/classification_test.go b/pkg/vmcp/server/classification_test.go new file mode 100644 index 0000000000..a1404a472a --- /dev/null +++ b/pkg/vmcp/server/classification_test.go @@ -0,0 +1,169 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package server + +import ( + "context" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + mcpparser "github.com/stacklok/toolhive/pkg/mcp" +) + +// Reserved Modern _meta keys, mirrored from pkg/mcp/revision.go's unexported +// constants since classification_test.go cannot import them directly. +const ( + metaKeyProtocolVersion = "io.modelcontextprotocol/protocolVersion" + metaKeyClientInfo = "io.modelcontextprotocol/clientInfo" + metaKeyClientCapabilities = "io.modelcontextprotocol/clientCapabilities" + modernProtocolVersionValue = "2026-07-28" +) + +func sentinelEncode(v string) string { + return "=?base64?" + base64.StdEncoding.EncodeToString([]byte(v)) + "?=" +} + +type classificationErrorBody struct { + Error struct { + Code int64 `json:"code"` + } `json:"error"` +} + +func TestClassificationMiddleware(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + parsed *mcpparser.ParsedMCPRequest + protocolHeader string + wantPassthrough bool + wantCode int64 + }{ + { + name: "nil parsed request passes through", + parsed: nil, + wantPassthrough: true, + }, + { + name: "legacy body with no modern signal passes through", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + }, + wantPassthrough: true, + }, + { + name: "modern header and complete meta pass through", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + Meta: map[string]any{ + metaKeyProtocolVersion: modernProtocolVersionValue, + metaKeyClientCapabilities: map[string]any{}, + }, + }, + protocolHeader: modernProtocolVersionValue, + wantPassthrough: true, + }, + { + name: "modern signal via reserved key with no protocolVersion and no header is invalid params", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + Meta: map[string]any{ + metaKeyClientInfo: map[string]any{}, + }, + }, + wantCode: mcpparser.CodeInvalidParams, + }, + { + name: "modern header/body protocolVersion mismatch is a header mismatch", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + Meta: map[string]any{ + metaKeyProtocolVersion: modernProtocolVersionValue, + metaKeyClientCapabilities: map[string]any{}, + }, + }, + protocolHeader: "2099-01-01", + wantCode: mcpparser.CodeHeaderMismatch, + }, + { + name: "modern unsupported protocolVersion is rejected", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + Meta: map[string]any{ + metaKeyProtocolVersion: "1.0", + }, + }, + wantCode: mcpparser.CodeUnsupportedProtocolVersion, + }, + { + name: "modern missing clientCapabilities is rejected", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + Meta: map[string]any{ + metaKeyProtocolVersion: modernProtocolVersionValue, + }, + }, + protocolHeader: modernProtocolVersionValue, + wantCode: mcpparser.CodeMissingClientCapability, + }, + { + name: "mismatched Mcp-Method header is a header mismatch", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + MCPMethodHeader: "resources/read", + }, + wantCode: mcpparser.CodeHeaderMismatch, + }, + { + name: "sentinel-encoded Mcp-Name header mismatched against ResourceID is a header mismatch", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "echo", + MCPNameHeader: sentinelEncode("other-tool"), + }, + wantCode: mcpparser.CodeHeaderMismatch, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + if tt.parsed != nil { + ctx = context.WithValue(ctx, mcpparser.MCPRequestContextKey, tt.parsed) + } + + req := httptest.NewRequest(http.MethodPost, "/mcp", nil).WithContext(ctx) + if tt.protocolHeader != "" { + req.Header.Set("MCP-Protocol-Version", tt.protocolHeader) + } + + nextCalled := false + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + nextCalled = true + w.WriteHeader(http.StatusOK) + }) + + rec := httptest.NewRecorder() + classificationMiddleware(next).ServeHTTP(rec, req) + + if tt.wantPassthrough { + assert.True(t, nextCalled, "expected the request to fall through to next") + return + } + + assert.False(t, nextCalled, "expected classification to short-circuit before next") + var body classificationErrorBody + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + assert.Equal(t, tt.wantCode, body.Error.Code) + }) + } +} From 7d6f09bd4ad8328aca0ea05a989b21405dd88a12 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 21 Jul 2026 23:18:33 +0100 Subject: [PATCH 3/6] Wire classification middleware into vMCP handler chain Compose classificationMiddleware between MCP-parsing and telemetry so malformed Modern (2026-07-28) requests are rejected before dispatch, while Legacy traffic and well-formed Modern requests fall through unchanged. Add an end-to-end test through the real handler chain covering the wiring itself, complementing the unit-level table added alongside the middleware. Co-Authored-By: Claude Sonnet 5 --- pkg/vmcp/server/server.go | 10 +++- ...management_realbackend_integration_test.go | 54 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index 991c317aee..5a192ea8d1 100644 --- a/pkg/vmcp/server/server.go +++ b/pkg/vmcp/server/server.go @@ -598,9 +598,9 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { } // MCP endpoint - apply middleware chain (wrapping order, execution happens in reverse): - // Code wraps: auth → rate-limit → audit → MCP-parsing → telemetry + // Code wraps: auth → rate-limit → audit → MCP-parsing → classification → telemetry // Execution order: recovery → body-limit → header-val → auth → - // rate-limit → audit → MCP-parsing → telemetry → handler + // rate-limit → audit → MCP-parsing → classification → telemetry → handler // // Upstream token refresh failures are detected inside AuthMiddleware itself: // GetAllUpstreamCredentials returns a non-empty failed-provider slice when @@ -626,6 +626,12 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { slog.Info("telemetry middleware enabled for MCP endpoints") } + // Classify Modern (2026-07-28) vs Legacy at the decode seam and reject + // malformed Modern requests before dispatch. No routing change: Legacy + // and well-formed Modern requests both fall through to the same handler + // (Modern dispatch lands in Phase 2, #5756). + mcpHandler = classificationMiddleware(mcpHandler) + // Apply MCP parsing middleware to extract JSON-RPC method from request body. // This runs before telemetry so that recordMetrics can label metrics with the // actual mcp_method (e.g. "tools/call", "initialize") instead of "unknown". diff --git a/pkg/vmcp/server/session_management_realbackend_integration_test.go b/pkg/vmcp/server/session_management_realbackend_integration_test.go index f05a7907fa..e5dc288b85 100644 --- a/pkg/vmcp/server/session_management_realbackend_integration_test.go +++ b/pkg/vmcp/server/session_management_realbackend_integration_test.go @@ -237,6 +237,60 @@ func TestIntegration_RealBackend_NonSSEGetRejectedWithNotAcceptable(t *testing.T "GET without Accept: text/event-stream must be rejected with 406") } +// TestIntegration_RealBackend_ModernRequestRejectedByClassification verifies +// the classificationMiddleware wiring end-to-end through the real chain (not +// just the unit-level table covering classificationMiddleware in isolation): +// a request that signals Modern (2026-07-28) via a reserved _meta key, but +// carries a mismatched MCP-Protocol-Version header, is rejected with a +// -32020 (HeaderMismatch) JSON-RPC error before ever reaching the backend. +func TestIntegration_RealBackend_ModernRequestRejectedByClassification(t *testing.T) { + t.Parallel() + + // Rejected by classificationMiddleware before dispatch, so no real MCP + // backend is needed. + ts := newRealTestServer(t, "http://127.0.0.1:0") + + body := map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": map[string]any{ + // Presence of a reserved io.modelcontextprotocol/* _meta key is + // itself a Modern signal, regardless of its value (see + // pkg/mcp/revision.go). No protocolVersion is present, so the + // non-empty header below cannot be validated against the body + // and the request is a hard rejection. + "_meta": map[string]any{"io.modelcontextprotocol/clientInfo": map[string]any{"name": "test"}}, + "name": "echo", + "arguments": map[string]any{}, + }, + } + payload, err := json.Marshal(body) + require.NoError(t, err) + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, ts.URL+"/mcp", bytes.NewReader(payload)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("MCP-Protocol-Version", "2025-11-25") + + resp, err := ts.Client().Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "body: %s", string(respBody)) + + var rpc struct { + Error struct { + Code int64 `json:"code"` + } `json:"error"` + } + require.NoError(t, json.Unmarshal(respBody, &rpc), "body: %s", string(respBody)) + assert.Equal(t, int64(-32020), rpc.Error.Code, "expected CodeHeaderMismatch") +} + // TestIntegration_RealBackend_Termination verifies the session termination path // against a real backend: a DELETE request closes the backend connection, and // subsequent requests with the terminated session ID are rejected. From 35cd72725ec7279006bd7585c0caa62e49db506f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 22 Jul 2026 13:05:54 +0100 Subject: [PATCH 4/6] Run telemetry before classification in vMCP chain Classification previously wrapped telemetry, so a rejected Modern request short-circuited before telemetry ever ran, silently dropping request/error metrics and traces for exactly the requests worth observing. Reorder so telemetry wraps classification instead: it still sees the parsed MCP context and now also records the outcome of rejected requests. Co-Authored-By: Claude Sonnet 5 --- pkg/vmcp/server/server.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index 5a192ea8d1..0a87e51de1 100644 --- a/pkg/vmcp/server/server.go +++ b/pkg/vmcp/server/server.go @@ -598,9 +598,9 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { } // MCP endpoint - apply middleware chain (wrapping order, execution happens in reverse): - // Code wraps: auth → rate-limit → audit → MCP-parsing → classification → telemetry + // Code wraps: auth → rate-limit → audit → MCP-parsing → telemetry → classification // Execution order: recovery → body-limit → header-val → auth → - // rate-limit → audit → MCP-parsing → classification → telemetry → handler + // rate-limit → audit → MCP-parsing → telemetry → classification → handler // // Upstream token refresh failures are detected inside AuthMiddleware itself: // GetAllUpstreamCredentials returns a non-empty failed-provider slice when @@ -621,17 +621,19 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { var mcpHandler http.Handler = streamableServer - if s.config.TelemetryProvider != nil { - mcpHandler = s.config.TelemetryProvider.Middleware(s.config.Name, "streamable-http")(mcpHandler) - slog.Info("telemetry middleware enabled for MCP endpoints") - } - // Classify Modern (2026-07-28) vs Legacy at the decode seam and reject // malformed Modern requests before dispatch. No routing change: Legacy // and well-formed Modern requests both fall through to the same handler - // (Modern dispatch lands in Phase 2, #5756). + // (Modern dispatch lands in Phase 2, #5756). Applied before telemetry + // (i.e. it runs closer to the handler) so a rejection is still recorded + // by the telemetry middleware instead of bypassing it entirely. mcpHandler = classificationMiddleware(mcpHandler) + if s.config.TelemetryProvider != nil { + mcpHandler = s.config.TelemetryProvider.Middleware(s.config.Name, "streamable-http")(mcpHandler) + slog.Info("telemetry middleware enabled for MCP endpoints") + } + // Apply MCP parsing middleware to extract JSON-RPC method from request body. // This runs before telemetry so that recordMetrics can label metrics with the // actual mcp_method (e.g. "tools/call", "initialize") instead of "unknown". From a4552f86332e6da9c59427e92f9e9c12fc57f731 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 22 Jul 2026 20:49:44 +0200 Subject: [PATCH 5/6] Enforce Mcp-Method/Mcp-Name presence on Modern requests Address review feedback: ValidateHeaderConsistency ran unconditionally (even for Legacy requests) and only checked header/body consistency when a header was present, never rejecting a Modern request for omitting a required one. The classifier's stdio-vs-HTTP ambiguity excuse for deferring this never applied here, since these headers are only ever populated from real HTTP headers. Gate the check on the classified revision, require Mcp-Method on every Modern request and Mcp-Name on tools/call, resources/read, and prompts/get, and give missing/malformed/mismatched headers distinct error messages (same -32020 wire code throughout). Add an end-to-end Mcp-Method mismatch test and a regression test locking in that telemetry still records requests classification rejects. Co-Authored-By: Claude Sonnet 5 --- pkg/mcp/revision.go | 119 ++++++++++++---- pkg/mcp/revision_test.go | 70 ++++++--- pkg/vmcp/server/classification.go | 27 ++-- pkg/vmcp/server/classification_test.go | 69 ++++++--- ...management_realbackend_integration_test.go | 59 ++++++++ pkg/vmcp/server/telemetry_integration_test.go | 134 ++++++++++++++++++ 6 files changed, 406 insertions(+), 72 deletions(-) diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go index 82c596a4a6..e3947801be 100644 --- a/pkg/mcp/revision.go +++ b/pkg/mcp/revision.go @@ -101,23 +101,51 @@ func (e *HeaderMismatchError) Data() map[string]any { return map[string]any{"header": e.Header, "body": e.Body} } +// requestHeaderMismatchReason distinguishes why a Mcp-Method/Mcp-Name header +// check failed, so Error() can describe the actual problem instead of always +// reporting a value "mismatch" — the wire code (CodeHeaderMismatch, -32020) +// is the same in every case; only the human-readable message differs. +type requestHeaderMismatchReason int + +const ( + // headerMismatchReasonValue is the default: the header and body both + // carry a value, but they disagree. + headerMismatchReasonValue requestHeaderMismatchReason = iota + // headerMismatchReasonMissing means the header was required but absent. + headerMismatchReasonMissing + // headerMismatchReasonMalformed means the header value itself could not + // be decoded (e.g. invalid base64 in a Mcp-Name sentinel). + headerMismatchReasonMalformed +) + // RequestHeaderMismatchError indicates a Modern (2026-07-28) request header -// contradicted the value carried in the request body. This is the -// generalized sibling of HeaderMismatchError for the Mcp-Method/Mcp-Name -// headers: HeaderMismatchError is specific to MCP-Protocol-Version, while -// this type covers any other header/body consistency check and names which -// header failed. +// contradicted, was missing from, or was malformed relative to the value +// carried in the request body. This is the generalized sibling of +// HeaderMismatchError for the Mcp-Method/Mcp-Name headers: HeaderMismatchError +// is specific to MCP-Protocol-Version, while this type covers any other +// header/body consistency check and names which header failed. type RequestHeaderMismatchError struct { - // Header is the name of the mismatched header (e.g. "Mcp-Method", "Mcp-Name"). + // Header is the name of the header that failed the check (e.g. "Mcp-Method", "Mcp-Name"). Header string // HeaderValue is the (decoded, where applicable) value carried by the header. HeaderValue string // BodyValue is the value the header was compared against in the request body. BodyValue string + + // reason is unexported: it only shapes Error()'s message, not Data()'s + // three-key shape or Code()'s wire value. + reason requestHeaderMismatchReason } func (e *RequestHeaderMismatchError) Error() string { - return fmt.Sprintf("%s header %q does not match request body value %q", e.Header, e.HeaderValue, e.BodyValue) + switch e.reason { + case headerMismatchReasonMissing: + return fmt.Sprintf("%s header is missing (required for this request)", e.Header) + case headerMismatchReasonMalformed: + return fmt.Sprintf("%s header value %q is malformed", e.Header, e.HeaderValue) + default: + return fmt.Sprintf("%s header %q does not match request body value %q", e.Header, e.HeaderValue, e.BodyValue) + } } // Code implements CodedError. @@ -278,37 +306,66 @@ func ClassifyRevision(method string, meta map[string]any, protoHeader string) (R return RevisionModern, nil } -// ValidateHeaderConsistency checks the Modern (2026-07-28) Mcp-Method and -// Mcp-Name request headers, when present, against the corresponding parsed -// request body fields (Method and ResourceID). -// A header value that contradicts the body is a hard rejection -// (*RequestHeaderMismatchError); a Mcp-Name header is decoded via -// decodeSentinelName before comparison, since the draft spec allows it to be -// sentinel-encoded. Requests carrying neither header return nil unconditionally, -// which is the correct no-op for Legacy traffic: Legacy clients never send -// these headers. +// nameRequiredMethods are the Modern (2026-07-28) methods for which the draft +// spec's "Server Validation" section requires a Mcp-Name header naming the +// body's tool/resource/prompt identifier. Methods outside this set (e.g. +// tools/list) have no per-request name to check, so Mcp-Name is never +// required for them — though if a client sends one anyway, it is still +// validated for consistency. +var nameRequiredMethods = map[string]bool{ + "tools/call": true, + "resources/read": true, + "prompts/get": true, +} + +// ValidateHeaderConsistency enforces the Modern (2026-07-28) Mcp-Method and +// Mcp-Name request headers against the corresponding parsed request body +// fields (Method and ResourceID). +// +// Caller contract: only call this for a request ClassifyRevision has already +// classified Modern. Mcp-Method/Mcp-Name are HTTP-only fields — populated +// solely by ParsingMiddleware reading real HTTP headers (see parser.go) — +// so there is no stdio/HTTP transport ambiguity here to excuse a Legacy +// request from this check; the caller must simply not invoke this function +// for Legacy traffic in the first place. // -// This function does not yet enforce presence/requiredness of these headers -// on Modern requests, only consistency when present, for the same -// transport-ambiguity reason noted on the TODO above (this classifier cannot -// yet tell stdio's "no header concept" apart from HTTP's "header omitted"). +// Mcp-Method is required on every Modern request: a missing or empty header +// is itself a rejection (*RequestHeaderMismatchError), not a silent no-op, +// and a present-but-different value is a mismatch. +// +// Mcp-Name is required only for the methods in nameRequiredMethods +// (tools/call, resources/read, prompts/get) — for any other method, an +// absent Mcp-Name header is fine. When present, it is decoded via +// decodeSentinelName before comparison against ResourceID, since the draft +// spec allows it to be sentinel-encoded; a decode failure or mismatch is a +// rejection. func ValidateHeaderConsistency(parsed *ParsedMCPRequest) error { - if parsed.MCPMethodHeader != "" && parsed.MCPMethodHeader != parsed.Method { + if parsed.MCPMethodHeader == "" { + return &RequestHeaderMismatchError{Header: "Mcp-Method", reason: headerMismatchReasonMissing} + } + if parsed.MCPMethodHeader != parsed.Method { return &RequestHeaderMismatchError{Header: "Mcp-Method", HeaderValue: parsed.MCPMethodHeader, BodyValue: parsed.Method} } - if parsed.MCPNameHeader != "" { - decoded, err := decodeSentinelName(parsed.MCPNameHeader) - if err != nil || decoded != parsed.ResourceID { - headerValue := func() string { - if err != nil { - return parsed.MCPNameHeader - } - return decoded - }() - return &RequestHeaderMismatchError{Header: "Mcp-Name", HeaderValue: headerValue, BodyValue: parsed.ResourceID} + if parsed.MCPNameHeader == "" { + if nameRequiredMethods[parsed.Method] { + return &RequestHeaderMismatchError{Header: "Mcp-Name", reason: headerMismatchReasonMissing} + } + return nil + } + + decoded, err := decodeSentinelName(parsed.MCPNameHeader) + if err != nil { + return &RequestHeaderMismatchError{ + Header: "Mcp-Name", + HeaderValue: parsed.MCPNameHeader, + BodyValue: parsed.ResourceID, + reason: headerMismatchReasonMalformed, } } + if decoded != parsed.ResourceID { + return &RequestHeaderMismatchError{Header: "Mcp-Name", HeaderValue: decoded, BodyValue: parsed.ResourceID} + } return nil } diff --git a/pkg/mcp/revision_test.go b/pkg/mcp/revision_test.go index 06ef5d04ec..8d8e59f6fd 100644 --- a/pkg/mcp/revision_test.go +++ b/pkg/mcp/revision_test.go @@ -409,22 +409,54 @@ func TestValidateHeaderConsistency(t *testing.T) { checkErr func(t *testing.T, err error) }{ { - name: "both headers absent is a legacy no-op", + name: "missing Mcp-Method header is rejected", parsed: &ParsedMCPRequest{ Method: "tools/call", ResourceID: "my-tool", }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, "Mcp-Method", mismatchErr.Header) + }, + }, + { + name: "method not in the name-required set needs no Mcp-Name", + parsed: &ParsedMCPRequest{ + Method: "tools/list", + MCPMethodHeader: "tools/list", + }, checkErr: func(t *testing.T, err error) { t.Helper() require.NoError(t, err) }, }, + { + name: "method in the name-required set missing Mcp-Name is rejected", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + }, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *RequestHeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, "Mcp-Name", mismatchErr.Header) + }, + }, { name: "Mcp-Method matches body method", parsed: &ParsedMCPRequest{ Method: "tools/call", ResourceID: "my-tool", MCPMethodHeader: "tools/call", + MCPNameHeader: "my-tool", }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -437,6 +469,7 @@ func TestValidateHeaderConsistency(t *testing.T) { Method: "tools/call", ResourceID: "my-tool", MCPMethodHeader: "resources/read", + MCPNameHeader: "my-tool", }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -452,9 +485,10 @@ func TestValidateHeaderConsistency(t *testing.T) { { name: "Mcp-Name plain string matches ResourceID", parsed: &ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "my-tool", - MCPNameHeader: "my-tool", + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + MCPNameHeader: "my-tool", }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -464,9 +498,10 @@ func TestValidateHeaderConsistency(t *testing.T) { { name: "Mcp-Name plain string mismatches ResourceID", parsed: &ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "my-tool", - MCPNameHeader: "other-tool", + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + MCPNameHeader: "other-tool", }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -482,9 +517,10 @@ func TestValidateHeaderConsistency(t *testing.T) { { name: "Mcp-Name sentinel-encoded decodes to matching ResourceID", parsed: &ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "my-tool", - MCPNameHeader: sentinelEncode("my-tool"), + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + MCPNameHeader: sentinelEncode("my-tool"), }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -494,9 +530,10 @@ func TestValidateHeaderConsistency(t *testing.T) { { name: "Mcp-Name sentinel-encoded decodes to mismatching value", parsed: &ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "my-tool", - MCPNameHeader: sentinelEncode("other-tool"), + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + MCPNameHeader: sentinelEncode("other-tool"), }, checkErr: func(t *testing.T, err error) { t.Helper() @@ -511,9 +548,10 @@ func TestValidateHeaderConsistency(t *testing.T) { { name: "Mcp-Name sentinel wrapper with invalid base64 payload", parsed: &ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "my-tool", - MCPNameHeader: "=?base64?not-valid-base64!!?=", + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "tools/call", + MCPNameHeader: "=?base64?not-valid-base64!!?=", }, checkErr: func(t *testing.T, err error) { t.Helper() diff --git a/pkg/vmcp/server/classification.go b/pkg/vmcp/server/classification.go index 3eeb514fca..e8c19d3414 100644 --- a/pkg/vmcp/server/classification.go +++ b/pkg/vmcp/server/classification.go @@ -12,11 +12,17 @@ import ( // classificationMiddleware classifies a parsed MCP request as Legacy // (2025-11-25) or Modern (2026-07-28) at the decode seam and rejects a // malformed Modern request with the correct JSON-RPC error before it reaches -// dispatch. The classified mcpparser.Revision is deliberately dropped: it is -// not stashed in context or anywhere else, since no downstream consumer reads -// it yet. Legacy traffic and well-formed Modern requests both fall through to -// the same next handler unchanged — Modern-specific dispatch is a later -// phase (toolhive issue #5756). +// dispatch. The classified mcpparser.Revision is used only to gate +// ValidateHeaderConsistency (see below); it is not otherwise stashed in +// context or anywhere else, since no downstream consumer reads it yet. +// Legacy traffic and well-formed Modern requests both fall through to the +// same next handler unchanged — Modern-specific dispatch is a later phase +// (toolhive issue #5756). +// +// ValidateHeaderConsistency (Mcp-Method/Mcp-Name) only applies to Modern +// requests: a Legacy request carrying a stray Mcp-Method/Mcp-Name header +// (e.g. from a misbehaving proxy) must not be rejected for it, since Legacy +// clients never send these headers and have no obligation to omit them. // // This middleware makes no authentication/authorization decision and confers // no elevated trust on requests that pass it — it only validates protocol @@ -31,14 +37,17 @@ func classificationMiddleware(next http.Handler) http.Handler { } protoHeader := r.Header.Get("MCP-Protocol-Version") - if _, err := mcpparser.ClassifyRevision(parsed.Method, parsed.Meta, protoHeader); err != nil { + rev, err := mcpparser.ClassifyRevision(parsed.Method, parsed.Meta, protoHeader) + if err != nil { mcpparser.WriteClassificationError(w, parsed.ID, err) return } - if err := mcpparser.ValidateHeaderConsistency(parsed); err != nil { - mcpparser.WriteClassificationError(w, parsed.ID, err) - return + if rev == mcpparser.RevisionModern { + if err := mcpparser.ValidateHeaderConsistency(parsed); err != nil { + mcpparser.WriteClassificationError(w, parsed.ID, err) + return + } } next.ServeHTTP(w, r) diff --git a/pkg/vmcp/server/classification_test.go b/pkg/vmcp/server/classification_test.go index a1404a472a..215219fac4 100644 --- a/pkg/vmcp/server/classification_test.go +++ b/pkg/vmcp/server/classification_test.go @@ -20,10 +20,9 @@ import ( // Reserved Modern _meta keys, mirrored from pkg/mcp/revision.go's unexported // constants since classification_test.go cannot import them directly. const ( - metaKeyProtocolVersion = "io.modelcontextprotocol/protocolVersion" - metaKeyClientInfo = "io.modelcontextprotocol/clientInfo" - metaKeyClientCapabilities = "io.modelcontextprotocol/clientCapabilities" - modernProtocolVersionValue = "2026-07-28" + metaKeyProtocolVersion = "io.modelcontextprotocol/protocolVersion" + metaKeyClientInfo = "io.modelcontextprotocol/clientInfo" + metaKeyClientCapabilities = "io.modelcontextprotocol/clientCapabilities" ) func sentinelEncode(v string) string { @@ -59,15 +58,18 @@ func TestClassificationMiddleware(t *testing.T) { wantPassthrough: true, }, { + // tools/list is deliberately not in the Mcp-Name-required set, so this + // case only needs Mcp-Method (required on every Modern request) to pass. name: "modern header and complete meta pass through", parsed: &mcpparser.ParsedMCPRequest{ - Method: "tools/call", + Method: "tools/list", Meta: map[string]any{ - metaKeyProtocolVersion: modernProtocolVersionValue, + metaKeyProtocolVersion: mcpparser.MCPVersionModern, metaKeyClientCapabilities: map[string]any{}, }, + MCPMethodHeader: "tools/list", }, - protocolHeader: modernProtocolVersionValue, + protocolHeader: mcpparser.MCPVersionModern, wantPassthrough: true, }, { @@ -85,7 +87,7 @@ func TestClassificationMiddleware(t *testing.T) { parsed: &mcpparser.ParsedMCPRequest{ Method: "tools/call", Meta: map[string]any{ - metaKeyProtocolVersion: modernProtocolVersionValue, + metaKeyProtocolVersion: mcpparser.MCPVersionModern, metaKeyClientCapabilities: map[string]any{}, }, }, @@ -107,28 +109,63 @@ func TestClassificationMiddleware(t *testing.T) { parsed: &mcpparser.ParsedMCPRequest{ Method: "tools/call", Meta: map[string]any{ - metaKeyProtocolVersion: modernProtocolVersionValue, + metaKeyProtocolVersion: mcpparser.MCPVersionModern, }, }, - protocolHeader: modernProtocolVersionValue, + protocolHeader: mcpparser.MCPVersionModern, wantCode: mcpparser.CodeMissingClientCapability, }, { - name: "mismatched Mcp-Method header is a header mismatch", + // No Modern signal anywhere (no header, no reserved _meta key), so this + // classifies Legacy and ValidateHeaderConsistency must not run at all — + // a stray/mismatched Mcp-Method header on Legacy traffic is not an error. + name: "legacy request carrying a stray Mcp-Method header passes through unchanged", parsed: &mcpparser.ParsedMCPRequest{ Method: "tools/call", MCPMethodHeader: "resources/read", }, - wantCode: mcpparser.CodeHeaderMismatch, + wantPassthrough: true, }, { name: "sentinel-encoded Mcp-Name header mismatched against ResourceID is a header mismatch", parsed: &mcpparser.ParsedMCPRequest{ - Method: "tools/call", - ResourceID: "echo", - MCPNameHeader: sentinelEncode("other-tool"), + Method: "tools/call", + ResourceID: "echo", + Meta: map[string]any{ + metaKeyProtocolVersion: mcpparser.MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + MCPMethodHeader: "tools/call", + MCPNameHeader: sentinelEncode("other-tool"), + }, + protocolHeader: mcpparser.MCPVersionModern, + wantCode: mcpparser.CodeHeaderMismatch, + }, + { + name: "modern request missing required Mcp-Method header is rejected", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/list", + Meta: map[string]any{ + metaKeyProtocolVersion: mcpparser.MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + }, + protocolHeader: mcpparser.MCPVersionModern, + wantCode: mcpparser.CodeHeaderMismatch, + }, + { + name: "modern tools/call request missing required Mcp-Name header is rejected", + parsed: &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "echo", + Meta: map[string]any{ + metaKeyProtocolVersion: mcpparser.MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + MCPMethodHeader: "tools/call", }, - wantCode: mcpparser.CodeHeaderMismatch, + protocolHeader: mcpparser.MCPVersionModern, + wantCode: mcpparser.CodeHeaderMismatch, }, } diff --git a/pkg/vmcp/server/session_management_realbackend_integration_test.go b/pkg/vmcp/server/session_management_realbackend_integration_test.go index e5dc288b85..bb48668887 100644 --- a/pkg/vmcp/server/session_management_realbackend_integration_test.go +++ b/pkg/vmcp/server/session_management_realbackend_integration_test.go @@ -291,6 +291,65 @@ func TestIntegration_RealBackend_ModernRequestRejectedByClassification(t *testin assert.Equal(t, int64(-32020), rpc.Error.Code, "expected CodeHeaderMismatch") } +// TestIntegration_RealBackend_ModernRequestRejectedByHeaderMismatch verifies +// the header-consistency path end-to-end through the real chain: a +// well-formed Modern (2026-07-28) request — valid _meta.protocolVersion, +// valid clientCapabilities, and a matching MCP-Protocol-Version header — is +// still rejected with -32020 (HeaderMismatch) when its Mcp-Method HTTP header +// disagrees with the JSON-RPC body's actual method. This exercises the real +// ParsingMiddleware -> classificationMiddleware flow (unlike +// TestClassificationMiddleware in classification_test.go, which injects a +// pre-built ParsedMCPRequest and bypasses ParsingMiddleware), and covers a +// genuine Mcp-Method/body mismatch rather than the protocolVersion mismatch +// already covered by TestIntegration_RealBackend_ModernRequestRejectedByClassification. +func TestIntegration_RealBackend_ModernRequestRejectedByHeaderMismatch(t *testing.T) { + t.Parallel() + + // Rejected by classificationMiddleware before dispatch, so no real MCP + // backend is needed. + ts := newRealTestServer(t, "http://127.0.0.1:0") + + body := map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": map[string]any{ + "_meta": map[string]any{ + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientCapabilities": map[string]any{}, + }, + "name": "echo", + "arguments": map[string]any{}, + }, + } + payload, err := json.Marshal(body) + require.NoError(t, err) + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, ts.URL+"/mcp", bytes.NewReader(payload)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("MCP-Protocol-Version", "2026-07-28") + // Mismatched: the body's actual method is "tools/call". + req.Header.Set("Mcp-Method", "resources/read") + + resp, err := ts.Client().Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "body: %s", string(respBody)) + + var rpc struct { + Error struct { + Code int64 `json:"code"` + } `json:"error"` + } + require.NoError(t, json.Unmarshal(respBody, &rpc), "body: %s", string(respBody)) + assert.Equal(t, int64(-32020), rpc.Error.Code, "expected CodeHeaderMismatch") +} + // TestIntegration_RealBackend_Termination verifies the session termination path // against a real backend: a DELETE request closes the backend connection, and // subsequent requests with the terminated session ID are rejected. diff --git a/pkg/vmcp/server/telemetry_integration_test.go b/pkg/vmcp/server/telemetry_integration_test.go index 62ad095cbb..1702475259 100644 --- a/pkg/vmcp/server/telemetry_integration_test.go +++ b/pkg/vmcp/server/telemetry_integration_test.go @@ -386,3 +386,137 @@ func TestIntegration_TelemetryMiddleware(t *testing.T) { cancelServer() } + +// TestIntegration_TelemetryRunsBeforeClassificationRejection is a regression +// guard for the middleware ordering in server.go: classificationMiddleware +// must stay closer to the handler than the telemetry middleware, so a +// request that classificationMiddleware rejects is still recorded as an +// incoming request instead of being dropped before telemetry ever sees it. +// If classification is ever reordered in front of telemetry, this test +// starts failing because the rejected request would never reach it. +// +// Note: like TestIntegration_TelemetryMiddleware, this does not use +// t.Parallel() since telemetry.NewProvider sets global OTel providers. +// +//nolint:paralleltest // shares global OTel provider state with other telemetry tests +func TestIntegration_TelemetryRunsBeforeClassificationRejection(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + ctx := context.Background() + + telemetryProvider, err := telemetry.NewProvider(ctx, telemetry.Config{ + ServiceName: "vmcp-telemetry-ordering-test", + ServiceVersion: "1.0.0", + EnablePrometheusMetricsPath: true, + }) + require.NoError(t, err) + t.Cleanup(func() { telemetryProvider.Shutdown(ctx) }) + + mockBackendClient := mocks.NewMockBackendClient(ctrl) + mockBackendClient.EXPECT(). + ListCapabilities(gomock.Any(), gomock.Any()). + Return(&vmcp.CapabilityList{}, nil). + AnyTimes() + + backends := []vmcp.Backend{ + { + ID: "search-svc", + Name: "Search Service", + BaseURL: "http://search-svc:8080", + TransportType: "streamable-http", + HealthStatus: vmcp.BackendHealthy, + }, + } + + rt := router.NewSessionRouter(&vmcp.RoutingTable{}) + agg := aggregator.NewDefaultAggregator( + mockBackendClient, aggregator.NewPrefixConflictResolver("{workload}_"), nil, nil) + factory := newBackendAwareTestFactory(nil, &vmcp.RoutingTable{}) + + srv, err := New(ctx, &Config{ + Name: "telemetry-ordering-vmcp", + Version: "1.0.0", + Host: "127.0.0.1", + Port: 0, // Random available port + TelemetryProvider: telemetryProvider, + SessionFactory: factory, + Aggregator: agg, + }, rt, mockBackendClient, vmcp.NewImmutableRegistry(backends), nil) + require.NoError(t, err) + + serverCtx, cancelServer := context.WithCancel(ctx) + t.Cleanup(cancelServer) + + serverErrCh := make(chan error, 1) + go func() { + if err := srv.Start(serverCtx); err != nil && !errors.Is(err, context.Canceled) { + serverErrCh <- err + } + }() + + select { + case <-srv.Ready(): + case err := <-serverErrCh: + t.Fatalf("Server failed to start: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("Server timeout waiting for ready") + } + + baseURL := "http://" + srv.Address() + + // Same malformed-Modern rejection payload as + // TestIntegration_RealBackend_ModernRequestRejectedByClassification: a + // reserved _meta key signals Modern, but no valid protocolVersion is + // present and the header names a different (Legacy) version, so + // classificationMiddleware rejects with -32020 before dispatch. + body := map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": map[string]any{ + "_meta": map[string]any{"io.modelcontextprotocol/clientInfo": map[string]any{"name": "test"}}, + "name": "echo", + "arguments": map[string]any{}, + }, + } + payload, err := json.Marshal(body) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, baseURL+"/mcp", bytes.NewReader(payload)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("MCP-Protocol-Version", "2025-11-25") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "body: %s", string(respBody)) + + var rpc struct { + Error struct { + Code int64 `json:"code"` + } `json:"error"` + } + require.NoError(t, json.Unmarshal(respBody, &rpc), "body: %s", string(respBody)) + require.Equal(t, int64(-32020), rpc.Error.Code, "expected CodeHeaderMismatch") + + metricsResp, err := http.Get(baseURL + "/metrics") + require.NoError(t, err) + defer metricsResp.Body.Close() + require.Equal(t, http.StatusOK, metricsResp.StatusCode) + + metricsBody, err := io.ReadAll(metricsResp.Body) + require.NoError(t, err) + metrics := string(metricsBody) + + assert.Contains(t, metrics, "toolhive_mcp_requests", + "telemetry must record the request even though classification rejected it downstream") + assert.Contains(t, metrics, `server="telemetry-ordering-vmcp"`, + "request metrics should identify this vMCP server") + + cancelServer() +} From 98b76d4f3ff784b88b696e05122784c3d33ece9f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 22 Jul 2026 21:16:50 +0200 Subject: [PATCH 6/6] Fix exhaustive-switch lint failure in RequestHeaderMismatchError golangci-lint's exhaustive check flagged the missing explicit case for headerMismatchReasonValue, which in turn made the constant look unused since it was only ever reached via default. Co-Authored-By: Claude Sonnet 5 --- pkg/mcp/revision.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go index e3947801be..753b903782 100644 --- a/pkg/mcp/revision.go +++ b/pkg/mcp/revision.go @@ -139,6 +139,8 @@ type RequestHeaderMismatchError struct { func (e *RequestHeaderMismatchError) Error() string { switch e.reason { + case headerMismatchReasonValue: + return fmt.Sprintf("%s header %q does not match request body value %q", e.Header, e.HeaderValue, e.BodyValue) case headerMismatchReasonMissing: return fmt.Sprintf("%s header is missing (required for this request)", e.Header) case headerMismatchReasonMalformed: