diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go index 5321ba9929..753b903782 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,63 @@ 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, 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 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 { + 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: + 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. +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 +308,70 @@ func ClassifyRevision(method string, meta map[string]any, protoHeader string) (R return RevisionModern, nil } +// 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. +// +// 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 == "" { + 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 == "" { + 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 +} + // 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 +450,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..8d8e59f6fd 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,226 @@ 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: "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() + require.NoError(t, err) + }, + }, + { + name: "Mcp-Method mismatches body method", + parsed: &ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "my-tool", + MCPMethodHeader: "resources/read", + MCPNameHeader: "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, 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", + MCPMethodHeader: "tools/call", + 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", + MCPMethodHeader: "tools/call", + 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", + MCPMethodHeader: "tools/call", + 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", + MCPMethodHeader: "tools/call", + 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", + MCPMethodHeader: "tools/call", + 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) + }) + } +} diff --git a/pkg/vmcp/server/classification.go b/pkg/vmcp/server/classification.go new file mode 100644 index 0000000000..e8c19d3414 --- /dev/null +++ b/pkg/vmcp/server/classification.go @@ -0,0 +1,55 @@ +// 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 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 +// 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") + rev, err := mcpparser.ClassifyRevision(parsed.Method, parsed.Meta, protoHeader) + if 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 new file mode 100644 index 0000000000..215219fac4 --- /dev/null +++ b/pkg/vmcp/server/classification_test.go @@ -0,0 +1,206 @@ +// 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" +) + +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, + }, + { + // 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/list", + Meta: map[string]any{ + metaKeyProtocolVersion: mcpparser.MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + MCPMethodHeader: "tools/list", + }, + protocolHeader: mcpparser.MCPVersionModern, + 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: mcpparser.MCPVersionModern, + 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: mcpparser.MCPVersionModern, + }, + }, + protocolHeader: mcpparser.MCPVersionModern, + wantCode: mcpparser.CodeMissingClientCapability, + }, + { + // 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", + }, + wantPassthrough: true, + }, + { + name: "sentinel-encoded Mcp-Name header mismatched against ResourceID is a header mismatch", + parsed: &mcpparser.ParsedMCPRequest{ + 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", + }, + protocolHeader: mcpparser.MCPVersionModern, + 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) + }) + } +} diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go index 991c317aee..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 → telemetry + // Code wraps: auth → rate-limit → audit → MCP-parsing → telemetry → classification // Execution order: recovery → body-limit → header-val → auth → - // rate-limit → audit → MCP-parsing → 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,6 +621,14 @@ func (s *Server) Handler(_ context.Context) (http.Handler, error) { var mcpHandler http.Handler = streamableServer + // 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). 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") diff --git a/pkg/vmcp/server/session_management_realbackend_integration_test.go b/pkg/vmcp/server/session_management_realbackend_integration_test.go index f05a7907fa..bb48668887 100644 --- a/pkg/vmcp/server/session_management_realbackend_integration_test.go +++ b/pkg/vmcp/server/session_management_realbackend_integration_test.go @@ -237,6 +237,119 @@ 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_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() +}