diff --git a/pkg/authz/middleware.go b/pkg/authz/middleware.go index 1bfc2c5831..71200c2fa7 100644 --- a/pkg/authz/middleware.go +++ b/pkg/authz/middleware.go @@ -54,6 +54,22 @@ var MCPMethodToFeatureOperation = map[string]featureOperation{ "features/list": {Feature: "", Operation: authorizers.MCPOperationList}, // Capability discovery "roots/list": {Feature: "", Operation: ""}, // Root directory discovery + // server/discover is intentionally NOT allow-listed: it default-denies (403) for now. + // Its response enumerates tool/resource descriptors and would bypass + // ResponseFilteringWriter (which only filters tools/list, prompts/list, resources/list, + // and find_tool). When Modern serving is wired up (#5830), add it as allow + + // response-filter, not always-allowed. + + // Subscriptions - always allowed for now. This method carries no single resource + // identifier the parser extracts (params are a notification-type filter with an + // optional resourceSubscriptions array), so routing it through Cedar with an empty + // ResourceID would risk matching a broad allow rule. Notification delivery and + // per-resource authorization of resourceSubscriptions URIs are future work. + // + // TODO(#5755): when subscription notification delivery is implemented, replace this + // always-allowed entry with real per-resource authorization of resourceSubscriptions URIs. + "subscriptions/listen": {Feature: "", Operation: ""}, + // Logging and client preferences - always allowed "logging/setLevel": {Feature: "", Operation: ""}, // Client preference for server logging diff --git a/pkg/authz/middleware_test.go b/pkg/authz/middleware_test.go index 9e93943485..ff32beb33f 100644 --- a/pkg/authz/middleware_test.go +++ b/pkg/authz/middleware_test.go @@ -330,6 +330,28 @@ func TestMiddleware(t *testing.T) { expectStatus: http.StatusForbidden, expectAuthorized: false, }, + { + name: "Server discover default-denies (not allow-listed)", + method: "server/discover", + params: map[string]interface{}{}, + claims: jwt.MapClaims{ + "sub": "user123", + "name": "John Doe", + }, + expectStatus: http.StatusForbidden, + expectAuthorized: false, + }, + { + name: "Subscriptions listen is always allowed", + method: "subscriptions/listen", + params: map[string]interface{}{}, + claims: jwt.MapClaims{ + "sub": "user123", + "name": "John Doe", + }, + expectStatus: http.StatusOK, + expectAuthorized: true, + }, { name: "Sampling createMessage is denied by default (security-sensitive)", method: "sampling/createMessage", @@ -434,6 +456,29 @@ func TestMiddleware(t *testing.T) { } } +// TestSubscriptionsListenIsAllowlistedPendingDelivery guards a deliberate, temporary +// exception: subscriptions/listen is always-allowed only because notification delivery +// for it is not yet implemented, so it exposes no data. When delivery lands, this entry +// must become a real Feature/Operation with per-resource authorization of +// resourceSubscriptions URIs (see TODO(#5755) in MCPMethodToFeatureOperation) — this test +// should fail at that point as a reminder to update it deliberately. +func TestSubscriptionsListenIsAllowlistedPendingDelivery(t *testing.T) { + t.Parallel() + require.Equal(t, featureOperation{}, MCPMethodToFeatureOperation["subscriptions/listen"]) +} + +// TestServerDiscoverIsNotAllowlisted guards a deliberate omission: server/discover must +// stay absent from MCPMethodToFeatureOperation so it default-denies (403) until Modern +// serving is wired up with proper response filtering (#5830). Its response enumerates +// tool/resource descriptors, and re-adding it as always-allowed would let a Cedar-restricted +// client bypass ResponseFilteringWriter and enumerate the full catalog. This test forces a +// conscious decision if someone re-adds the entry. +func TestServerDiscoverIsNotAllowlisted(t *testing.T) { + t.Parallel() + _, ok := MCPMethodToFeatureOperation["server/discover"] + require.False(t, ok, "server/discover must not be allow-listed until Modern serving with response filtering lands (#5830)") +} + // TestMiddlewareWithGETRequest tests that the middleware doesn't panic with GET requests. func TestMiddlewareWithGETRequest(t *testing.T) { t.Parallel() diff --git a/pkg/mcp/parser.go b/pkg/mcp/parser.go index 361f4be170..cd4f816812 100644 --- a/pkg/mcp/parser.go +++ b/pkg/mcp/parser.go @@ -41,6 +41,21 @@ type ParsedMCPRequest struct { // Meta contains the _meta field from the request params for protocol-level metadata // such as progress tokens, trace IDs, or custom namespaced metadata Meta map[string]interface{} + // MCPMethodHeader is the value of the Modern (stateless MCP) "Mcp-Method" + // request header, if present. Mandatory on every Modern POST per the draft spec. + MCPMethodHeader string + // MCPNameHeader is the raw, as-received value of the Modern (stateless MCP) + // "Mcp-Name" request header, if present. Required for tools/call, + // resources/read, prompts/get. Stored undecoded: the spec allows the header + // value to be sentinel-encoded (=?base64?...?=), and a caller comparing it to + // the plain body name/uri during validation must decode the header first. + MCPNameHeader string + // ClientInfo is the client implementation info surfaced via _meta for Modern + // (stateless) requests, sourced from _meta["io.modelcontextprotocol/clientInfo"]. + ClientInfo map[string]interface{} + // ProtocolVersion is the per-request protocol version surfaced via _meta for + // Modern (stateless) requests, sourced from _meta["io.modelcontextprotocol/protocolVersion"]. + ProtocolVersion string // IsRequest indicates if this is a JSON-RPC request (vs response or notification) IsRequest bool // IsBatch indicates if this is a batch request @@ -95,6 +110,8 @@ func ParsingMiddleware(next http.Handler) http.Handler { // Parse the MCP request and store in context parsedRequest := parseMCPRequest(bodyBytes) if parsedRequest != nil { + parsedRequest.MCPMethodHeader = r.Header.Get("Mcp-Method") + parsedRequest.MCPNameHeader = r.Header.Get("Mcp-Name") ctx := context.WithValue(r.Context(), MCPRequestContextKey, parsedRequest) r = r.WithContext(ctx) } @@ -158,6 +175,7 @@ func parseMCPRequest(bodyBytes []byte) *ParsedMCPRequest { // Extract resource ID, arguments, and meta based on the method resourceID, arguments, meta := extractResourceAndArguments(req.Method, req.Params) + clientInfo, protocolVersion := extractModernMeta(meta) // Determine the ID - will be nil for notifications var id interface{} @@ -166,14 +184,16 @@ func parseMCPRequest(bodyBytes []byte) *ParsedMCPRequest { } return &ParsedMCPRequest{ - Method: req.Method, - ID: id, - Params: req.Params, - ResourceID: resourceID, - Arguments: arguments, - Meta: meta, - IsRequest: true, - IsBatch: false, // TODO: Add batch request support if needed + Method: req.Method, + ID: id, + Params: req.Params, + ResourceID: resourceID, + Arguments: arguments, + Meta: meta, + ClientInfo: clientInfo, + ProtocolVersion: protocolVersion, + IsRequest: true, + IsBatch: false, // TODO: Add batch request support if needed } } @@ -219,6 +239,7 @@ var staticResourceIDs = map[string]string{ "notifications/resources/list_changed": "resources", "notifications/resources/updated": "resources", "notifications/tools/list_changed": "tools", + "server/discover": "discover", } func extractResourceAndArguments(method string, params json.RawMessage) (string, map[string]interface{}, map[string]interface{}) { @@ -243,6 +264,17 @@ func extractResourceAndArguments(method string, params json.RawMessage) (string, return resourceID, arguments, meta } +// extractModernMeta surfaces the Modern (stateless MCP) clientInfo and +// protocolVersion fields from a parsed _meta map, if present. It delegates to +// the reserved-key helpers in revision.go so the guarded type assertions live +// in one place; a wrong-shaped value is treated as absent rather than causing +// an error. +func extractModernMeta(meta map[string]interface{}) (clientInfo map[string]interface{}, protocolVersion string) { + clientInfo, _ = objectMetaValue(meta, metaKeyClientInfo) + protocolVersion, _ = stringMetaValue(meta, metaKeyProtocolVersion) + return clientInfo, protocolVersion +} + // getStaticResourceID returns the static resource ID for methods that don't need parameter parsing func getStaticResourceID(method string) string { if resourceID, exists := staticResourceIDs[method]; exists { @@ -495,3 +527,23 @@ func GetMCPMeta(ctx context.Context) map[string]interface{} { } return nil } + +// GetMCPClientInfo is a convenience function to get the Modern (stateless MCP) +// per-request clientInfo from the context. +// Returns nil if no parsed request is available or clientInfo is not present. +func GetMCPClientInfo(ctx context.Context) map[string]interface{} { + if parsed := GetParsedMCPRequest(ctx); parsed != nil { + return parsed.ClientInfo + } + return nil +} + +// GetMCPProtocolVersion is a convenience function to get the Modern (stateless +// MCP) per-request protocol version from the context. +// Returns "" if no parsed request is available or protocolVersion is not present. +func GetMCPProtocolVersion(ctx context.Context) string { + if parsed := GetParsedMCPRequest(ctx); parsed != nil { + return parsed.ProtocolVersion + } + return "" +} diff --git a/pkg/mcp/parser_test.go b/pkg/mcp/parser_test.go index e8778b43db..b52569b5b2 100644 --- a/pkg/mcp/parser_test.go +++ b/pkg/mcp/parser_test.go @@ -84,6 +84,17 @@ func TestParsingMiddleware(t *testing.T) { expectedID: int64(4), expectedResID: "ping", }, + { + name: "server/discover request", + method: "POST", + path: "/messages", + contentType: "application/json", + body: `{"jsonrpc":"2.0","id":10,"method":"server/discover","params":{}}`, + expectParsed: true, + expectedMethod: "server/discover", + expectedID: int64(10), + expectedResID: "discover", + }, { name: "GET request - not parsed", method: "GET", @@ -224,6 +235,67 @@ func TestParsingMiddleware(t *testing.T) { } } +func TestParsingMiddlewareModernHeaders(t *testing.T) { + t.Parallel() + tests := []struct { + name string + mcpMethodHeader string + mcpNameHeader string + expectedMCPMethod string + expectedMCPName string + }{ + { + name: "both headers set", + mcpMethodHeader: "tools/call", + mcpNameHeader: "some-tool", + expectedMCPMethod: "tools/call", + expectedMCPName: "some-tool", + }, + { + name: "neither header set", + expectedMCPMethod: "", + expectedMCPName: "", + }, + { + name: "sentinel-encoded Mcp-Name stored undecoded", + mcpMethodHeader: "tools/call", + mcpNameHeader: "=?base64?dG9vbA==?=", + expectedMCPMethod: "tools/call", + expectedMCPName: "=?base64?dG9vbA==?=", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var capturedCtx context.Context + testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCtx = r.Context() + w.WriteHeader(http.StatusOK) + }) + + middleware := ParsingMiddleware(testHandler) + body := `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"weather"}}` + req := httptest.NewRequest("POST", "/messages", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + if tt.mcpMethodHeader != "" { + req.Header.Set("Mcp-Method", tt.mcpMethodHeader) + } + if tt.mcpNameHeader != "" { + req.Header.Set("Mcp-Name", tt.mcpNameHeader) + } + w := httptest.NewRecorder() + + middleware.ServeHTTP(w, req) + + parsed := GetParsedMCPRequest(capturedCtx) + require.NotNil(t, parsed) + assert.Equal(t, tt.expectedMCPMethod, parsed.MCPMethodHeader) + assert.Equal(t, tt.expectedMCPName, parsed.MCPNameHeader) + }) + } +} + func TestExtractResourceAndArguments(t *testing.T) { t.Parallel() tests := []struct { @@ -1051,6 +1123,124 @@ func TestMetaFieldParsing(t *testing.T) { } } +func TestModernMetaParsing(t *testing.T) { + t.Parallel() + tests := []struct { + name string + body string + expectedClientInfo map[string]interface{} + expectedProtocolVersion string + }{ + { + name: "clientInfo and protocolVersion present", + body: `{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "weather", + "_meta": { + "io.modelcontextprotocol/clientInfo": {"name": "test-client", "version": "1.0"}, + "io.modelcontextprotocol/protocolVersion": "2026-07-28" + } + } + }`, + expectedClientInfo: map[string]interface{}{ + "name": "test-client", + "version": "1.0", + }, + expectedProtocolVersion: "2026-07-28", + }, + { + name: "_meta absent", + body: `{ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": { + "name": "weather" + } + }`, + expectedClientInfo: nil, + expectedProtocolVersion: "", + }, + { + name: "_meta present without modern keys", + body: `{ + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": { + "name": "weather", + "_meta": { + "progressToken": "abc123" + } + } + }`, + expectedClientInfo: nil, + expectedProtocolVersion: "", + }, + { + name: "protocolVersion wrong type", + body: `{ + "jsonrpc": "2.0", + "id": 4, + "method": "tools/call", + "params": { + "name": "weather", + "_meta": { + "io.modelcontextprotocol/protocolVersion": 12345 + } + } + }`, + expectedClientInfo: nil, + expectedProtocolVersion: "", + }, + { + name: "clientInfo wrong type", + body: `{ + "jsonrpc": "2.0", + "id": 5, + "method": "tools/call", + "params": { + "name": "weather", + "_meta": { + "io.modelcontextprotocol/clientInfo": "not-an-object" + } + } + }`, + expectedClientInfo: nil, + expectedProtocolVersion: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var capturedCtx context.Context + testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedCtx = r.Context() + w.WriteHeader(http.StatusOK) + }) + + middleware := ParsingMiddleware(testHandler) + req := httptest.NewRequest("POST", "/messages", bytes.NewBufferString(tt.body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + middleware.ServeHTTP(w, req) + + parsed := GetParsedMCPRequest(capturedCtx) + require.NotNil(t, parsed) + assert.Equal(t, tt.expectedClientInfo, parsed.ClientInfo) + assert.Equal(t, tt.expectedProtocolVersion, parsed.ProtocolVersion) + + assert.Equal(t, tt.expectedClientInfo, GetMCPClientInfo(capturedCtx)) + assert.Equal(t, tt.expectedProtocolVersion, GetMCPProtocolVersion(capturedCtx)) + }) + } +} + func TestMetaFieldInvalidTypes(t *testing.T) { t.Parallel() tests := []struct { @@ -1391,6 +1581,11 @@ func TestExtractResourceAndArgumentsNilParams(t *testing.T) { method: "notifications/initialized", expectedResourceID: "initialized", }, + { + name: "server/discover", + method: "server/discover", + expectedResourceID: "discover", + }, } for _, tt := range tests { diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go new file mode 100644 index 0000000000..65b6001836 --- /dev/null +++ b/pkg/mcp/revision.go @@ -0,0 +1,297 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import "fmt" + +// Revision identifies which MCP protocol era a request belongs to. +type Revision int + +const ( + // RevisionLegacy is the current 2025-11-25 MCP revision: session-based, + // initialize handshake, Mcp-Session-Id. + RevisionLegacy Revision = iota + // RevisionModern is the 2026-07-28 MCP revision: stateless, no initialize, + // protocol metadata carried per-request in _meta. + RevisionModern +) + +// MCPVersionModern is the single Modern (stateless) protocol version this +// build understands. +const MCPVersionModern = "2026-07-28" + +// metaKeyProtocolVersion is the reserved _meta key that carries the per-request +// protocol version on Modern (stateless) MCP requests, per the draft MCP schema's +// RequestMetaObject. +const metaKeyProtocolVersion = "io.modelcontextprotocol/protocolVersion" + +// metaKeyClientInfo is the reserved _meta key that carries the per-request client +// implementation info on Modern (stateless) MCP requests, per the draft MCP +// schema's RequestMetaObject. +const metaKeyClientInfo = "io.modelcontextprotocol/clientInfo" + +// metaKeyClientCapabilities is the reserved _meta key that carries the per-request +// client capabilities on Modern (stateless) MCP requests, per the draft MCP +// schema's RequestMetaObject. +const metaKeyClientCapabilities = "io.modelcontextprotocol/clientCapabilities" + +// reservedModernMetaKeys are the _meta keys a Legacy client never sets. The +// presence of any one of them — independent of whether its value is +// well-formed — is itself a claim of the Modern revision, and must not be +// silently downgraded to Legacy. Only a malformed/absent protocolVersion +// alongside one of these keys turns into a rejection, never a downgrade. +var reservedModernMetaKeys = []string{metaKeyProtocolVersion, metaKeyClientInfo, metaKeyClientCapabilities} + +// The following JSON-RPC error codes are defined by the draft MCP spec +// (schema/draft/schema.ts) for the stateless "Modern" revision. They are +// declared as local literals, matching this repo's existing convention for +// JSON-RPC codes (e.g. streamable_proxy.go's -32603). The only SDK with +// equivalents, modelcontextprotocol/go-sdk, is reachable solely as a +// transitive dependency (via toolhive-core's mcpcompat) and is pinned to an +// older draft snapshot (2026-06-30, CodeHeaderMismatch = -32001) with no +// equivalents at all for the other two codes below, so importing it would +// risk wiring in stale values. Revisit once the go-sdk dependency is bumped +// to a revision that matches MCPVersionModern. +// +// These are exported so other packages (e.g. the HTTP layer) can reference +// the same wire values instead of hardcoding or redeclaring them. +const ( + // CodeHeaderMismatch signals a mismatch between the MCP-Protocol-Version + // header and the _meta protocol version (schema.ts HeaderMismatchError). + CodeHeaderMismatch int64 = -32020 + // CodeMissingClientCapability signals that _meta is missing a client + // capability required for the request (schema.ts MissingRequiredClientCapabilityError). + CodeMissingClientCapability int64 = -32021 + // CodeUnsupportedProtocolVersion signals that the _meta protocol version + // is not one this server supports (schema.ts UnsupportedProtocolVersionError). + CodeUnsupportedProtocolVersion int64 = -32022 + // CodeInvalidParams is the standard JSON-RPC Invalid Params code, used + // as a fallback when the draft spec defines no dedicated code for the failure. + CodeInvalidParams int64 = -32602 +) + +// HeaderMismatchError indicates the MCP-Protocol-Version HTTP header did not match +// the io.modelcontextprotocol/protocolVersion carried in the request's _meta field. +// Neither value wins: this is a hard rejection of the request, not a signal to +// proceed with either value. +type HeaderMismatchError struct { + // Header is the MCP-Protocol-Version header value. + Header string + // Body is the _meta protocol version value. + Body string +} + +func (e *HeaderMismatchError) Error() string { + return fmt.Sprintf("MCP-Protocol-Version header %q does not match _meta protocol version %q", e.Header, e.Body) +} + +// Code implements CodedError. +func (*HeaderMismatchError) Code() int64 { return CodeHeaderMismatch } + +// Data implements CodedError. +func (e *HeaderMismatchError) Data() map[string]any { + return map[string]any{"header": e.Header, "body": e.Body} +} + +// UnsupportedVersionError indicates the _meta protocol version named a Modern +// revision this server does not support. +type UnsupportedVersionError struct { + // Requested is the _meta protocol version the client asked for. + Requested string + // Supported lists the protocol versions this server supports. + Supported []string +} + +func (e *UnsupportedVersionError) Error() string { + return fmt.Sprintf("unsupported MCP protocol version %q (supported: %v)", e.Requested, e.Supported) +} + +// Code implements CodedError. +func (*UnsupportedVersionError) Code() int64 { return CodeUnsupportedProtocolVersion } + +// Data implements CodedError. +func (e *UnsupportedVersionError) Data() map[string]any { + return map[string]any{"supported": e.Supported, "requested": e.Requested} +} + +// MissingClientCapabilityError indicates a Modern request's _meta is missing +// clientCapabilities (clientInfo is optional per the draft schema and is not +// checked here). +// +// The draft types MissingRequiredClientCapabilityError.data.requiredCapabilities +// as a ClientCapabilities object, not a list of names. The classifier cannot +// compute per-method required capabilities (that check is deferred to the +// caller/handler layer), so RequiredCapabilities is populated best-effort and +// may be an empty object. +type MissingClientCapabilityError struct { + // RequiredCapabilities is the ClientCapabilities object the request was + // missing, if known. + RequiredCapabilities map[string]any +} + +func (e *MissingClientCapabilityError) Error() string { + return fmt.Sprintf("request _meta is missing required client capabilities: %v", e.RequiredCapabilities) +} + +// Code implements CodedError. +func (*MissingClientCapabilityError) Code() int64 { return CodeMissingClientCapability } + +// Data implements CodedError. +func (e *MissingClientCapabilityError) Data() map[string]any { + return map[string]any{"requiredCapabilities": e.RequiredCapabilities} +} + +// MissingModernMetadataError indicates the request carried a Modern signal +// via one of the reserved io.modelcontextprotocol/* _meta keys — with no +// MCP-Protocol-Version header present at all — but _meta carried no valid +// protocolVersion. (When a header IS present, a missing or invalid body +// protocolVersion is a header/body mismatch instead; see HeaderMismatchError.) +// The draft spec defines no dedicated error code for this reserved-key-only +// case, so it falls back to the standard JSON-RPC Invalid Params code. +type MissingModernMetadataError struct{} + +func (*MissingModernMetadataError) Error() string { + return "request carries a Modern signal but no valid io.modelcontextprotocol/protocolVersion in _meta" +} + +// Code implements CodedError. +func (*MissingModernMetadataError) Code() int64 { return CodeInvalidParams } + +// Data implements CodedError. +func (*MissingModernMetadataError) Data() map[string]any { return map[string]any{} } + +// ClassifyRevision determines whether a single MCP request is Legacy or Modern, +// and whether it is valid. +// +// When err != nil, the caller MUST reject the request; the returned Revision is +// then purely informational — it records that the request claimed Modern, not +// that classification succeeded. +// +// method == "initialize" always classifies Legacy immediately, unconditionally — +// Modern never sends initialize (it is the Legacy session-start marker by +// definition) — which also guards against a spoofed Modern _meta on a Legacy +// call. +// +// Otherwise, a request "signals" Modern if the MCP-Protocol-Version header is +// exactly MCPVersionModern, OR _meta carries ANY of the reserved +// io.modelcontextprotocol/* keys (protocolVersion, clientInfo, +// clientCapabilities) — presence of the key is the signal, independent of +// whether its value is well-formed, since a Legacy client never sets these +// keys at all. A request with no signal anywhere classifies Legacy, the safe +// default. A request with a signal is never silently downgraded: it either +// classifies Modern with a nil error, or Modern with an error the caller must +// reject on. +// +// A non-empty MCP-Protocol-Version header that names some OTHER version (not +// MCPVersionModern) and carries no reserved _meta key is, by design, not a +// Modern signal: the request body's _meta is authoritative for the protocol +// version, and an unrecognized header value alone classifies Legacy rather +// than erroring. +// +// Given a Modern signal, checks run in this order: +// 1. meta[metaKeyProtocolVersion] must be a non-empty string. If it is not, +// and protoHeader is non-empty, the body has nothing valid to match against +// the header: this is a header/body mismatch (*HeaderMismatchError). If +// protoHeader is empty — the signal came only from a reserved _meta key, +// so there is no header to mismatch against — the request is malformed +// (*MissingModernMetadataError). +// 2. that string must equal MCPVersionModern, or it names an unsupported +// version (*UnsupportedVersionError). +// 3. if protoHeader is non-empty it must equal the body version, or the two +// conflict (*HeaderMismatchError, a hard rejection — neither value wins). +// 4. _meta must carry clientCapabilities (clientInfo is optional per the +// draft schema), or the client capabilities are missing +// (*MissingClientCapabilityError, with per-method requirements deferred to +// the caller). +// +// A request that passes all four classifies Modern with a nil error. +func ClassifyRevision(method string, meta map[string]any, protoHeader string) (Revision, error) { + if method == "initialize" { + return RevisionLegacy, nil + } + + if !hasModernSignal(meta, protoHeader) { + return RevisionLegacy, nil + } + + bodyVersion, hasBodyVersion := stringMetaValue(meta, metaKeyProtocolVersion) + if !hasBodyVersion { + if protoHeader != "" { + return RevisionModern, &HeaderMismatchError{Header: protoHeader, Body: ""} + } + // TODO: this always returns -32602, but ClassifyRevision is transport-agnostic + // and cannot tell "stdio, no header concept" (where -32602 is correct) apart from + // "HTTP, header omitted" (where the draft's Server Validation rules make a missing + // MCP-Protocol-Version header itself a -32020 HeaderMismatch condition). Revisit + // once the classifier is wired into the HTTP request path and can be given + // transport context. + return RevisionModern, &MissingModernMetadataError{} + } + + if bodyVersion != MCPVersionModern { + return RevisionModern, &UnsupportedVersionError{Requested: bodyVersion, Supported: []string{MCPVersionModern}} + } + + if protoHeader != "" && protoHeader != bodyVersion { + return RevisionModern, &HeaderMismatchError{Header: protoHeader, Body: bodyVersion} + } + + if !hasObjectMetaValue(meta, metaKeyClientCapabilities) { + return RevisionModern, &MissingClientCapabilityError{RequiredCapabilities: map[string]any{}} + } + + return RevisionModern, nil +} + +// hasModernSignal reports whether the request signals the Modern revision: +// either the header exactly names MCPVersionModern, or _meta carries any of +// the reserved Modern-only keys (regardless of whether their values are +// well-formed). +func hasModernSignal(meta map[string]any, protoHeader string) bool { + if protoHeader == MCPVersionModern { + return true + } + for _, key := range reservedModernMetaKeys { + if _, ok := meta[key]; ok { + return true + } + } + return false +} + +// stringMetaValue reports the non-empty string value of meta[key], if present. +func stringMetaValue(meta map[string]any, key string) (string, bool) { + raw, ok := meta[key] + if !ok { + return "", false + } + s, ok := raw.(string) + if !ok || s == "" { + return "", false + } + return s, true +} + +// hasObjectMetaValue reports whether meta[key] is present and decodes as a JSON +// object; see objectMetaValue for the shape this covers. +func hasObjectMetaValue(meta map[string]any, key string) bool { + _, ok := objectMetaValue(meta, key) + return ok +} + +// objectMetaValue reports the value of meta[key] if it decodes as a JSON +// object (map[string]any) — the shape clientInfo and clientCapabilities take +// in _meta. A missing key or a wrong-typed value (e.g. a string or number) +// both count as "not present". +func objectMetaValue(meta map[string]any, key string) (map[string]any, bool) { + raw, ok := meta[key] + if !ok { + return nil, false + } + obj, ok := raw.(map[string]any) + if !ok { + return nil, false + } + return obj, true +} diff --git a/pkg/mcp/revision_test.go b/pkg/mcp/revision_test.go new file mode 100644 index 0000000000..824611cea3 --- /dev/null +++ b/pkg/mcp/revision_test.go @@ -0,0 +1,321 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var _ CodedError = (*HeaderMismatchError)(nil) +var _ CodedError = (*UnsupportedVersionError)(nil) +var _ CodedError = (*MissingClientCapabilityError)(nil) +var _ CodedError = (*MissingModernMetadataError)(nil) + +func validModernMeta() map[string]any { + return map[string]any{ + metaKeyProtocolVersion: MCPVersionModern, + metaKeyClientInfo: map[string]any{"name": "test-client", "version": "1.0.0"}, + metaKeyClientCapabilities: map[string]any{}, + } +} + +func TestClassifyRevision(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + method string + meta map[string]any + protoHeader string + expectedRev Revision + checkErr func(t *testing.T, err error) + }{ + { + name: "modern: valid meta, no header", + method: "tools/call", + meta: validModernMeta(), + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "modern: valid meta, matching header", + method: "tools/call", + meta: validModernMeta(), + protoHeader: MCPVersionModern, + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "modern: mismatched header", + method: "tools/call", + meta: validModernMeta(), + protoHeader: "2025-11-25", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, "2025-11-25", mismatchErr.Header) + assert.Equal(t, MCPVersionModern, mismatchErr.Body) + assert.Equal(t, map[string]any{"header": "2025-11-25", "body": MCPVersionModern}, mismatchErr.Data()) + }, + }, + { + name: "modern: unsupported future body version", + method: "tools/call", + meta: map[string]any{ + metaKeyProtocolVersion: "2099-01-01", + }, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var unsupportedErr *UnsupportedVersionError + require.ErrorAs(t, err, &unsupportedErr) + assert.Equal(t, CodeUnsupportedProtocolVersion, unsupportedErr.Code()) + data := unsupportedErr.Data() + assert.Equal(t, "2099-01-01", data["requested"]) + assert.Equal(t, []string{MCPVersionModern}, data["supported"]) + }, + }, + { + name: "modern header but meta absent entirely is a header mismatch", + method: "tools/call", + meta: nil, + protoHeader: MCPVersionModern, + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, MCPVersionModern, mismatchErr.Header) + assert.Empty(t, mismatchErr.Body) + }, + }, + { + name: "modern header but meta missing protocol version key is a header mismatch", + method: "tools/call", + meta: map[string]any{"other": "value"}, + protoHeader: MCPVersionModern, + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + }, + }, + { + name: "modern header but body version wrong-typed is a header mismatch", + method: "tools/call", + meta: map[string]any{metaKeyProtocolVersion: 42}, + protoHeader: MCPVersionModern, + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + }, + }, + { + name: "modern header but body version empty string is a header mismatch", + method: "tools/call", + meta: map[string]any{metaKeyProtocolVersion: ""}, + protoHeader: MCPVersionModern, + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + }, + }, + { + name: "modern: clientInfo omitted is valid", + method: "tools/call", + meta: map[string]any{ + metaKeyProtocolVersion: MCPVersionModern, + metaKeyClientCapabilities: map[string]any{}, + }, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "modern: missing clientCapabilities", + method: "tools/call", + meta: map[string]any{ + metaKeyProtocolVersion: MCPVersionModern, + metaKeyClientInfo: map[string]any{"name": "test-client"}, + }, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var missingCapErr *MissingClientCapabilityError + require.ErrorAs(t, err, &missingCapErr) + assert.Equal(t, CodeMissingClientCapability, missingCapErr.Code()) + }, + }, + { + name: "legacy: absent meta", + method: "tools/call", + meta: nil, + protoHeader: "", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "legacy: meta missing protocol version key", + method: "tools/call", + meta: map[string]any{"other": "value"}, + protoHeader: "", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "legacy: unrecognized header version, no reserved meta key", + method: "tools/call", + meta: map[string]any{"other": "value"}, + protoHeader: "2099-01-01", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "modern signal: reserved protocolVersion key wrong-typed", + method: "tools/call", + meta: map[string]any{metaKeyProtocolVersion: 42}, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var missingMetaErr *MissingModernMetadataError + require.ErrorAs(t, err, &missingMetaErr) + assert.Equal(t, CodeInvalidParams, missingMetaErr.Code()) + }, + }, + { + name: "modern signal: reserved protocolVersion key empty string", + method: "tools/call", + meta: map[string]any{metaKeyProtocolVersion: ""}, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var missingMetaErr *MissingModernMetadataError + require.ErrorAs(t, err, &missingMetaErr) + assert.Equal(t, CodeInvalidParams, missingMetaErr.Code()) + }, + }, + { + name: "modern signal via clientCapabilities key, no protocolVersion", + method: "tools/call", + meta: map[string]any{metaKeyClientCapabilities: map[string]any{}}, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var missingMetaErr *MissingModernMetadataError + require.ErrorAs(t, err, &missingMetaErr) + assert.Equal(t, CodeInvalidParams, missingMetaErr.Code()) + }, + }, + { + name: "modern signal via clientInfo key, broken protocolVersion", + method: "tools/call", + meta: map[string]any{ + metaKeyClientInfo: map[string]any{"name": "test-client"}, + metaKeyProtocolVersion: "", + }, + protoHeader: "", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var missingMetaErr *MissingModernMetadataError + require.ErrorAs(t, err, &missingMetaErr) + assert.Equal(t, CodeInvalidParams, missingMetaErr.Code()) + }, + }, + { + name: "modern signal via reserved key with non-modern header is a header mismatch", + method: "tools/call", + meta: map[string]any{ + metaKeyClientCapabilities: map[string]any{}, + }, + protoHeader: "2025-11-25", + expectedRev: RevisionModern, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + var mismatchErr *HeaderMismatchError + require.ErrorAs(t, err, &mismatchErr) + assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) + assert.Equal(t, "2025-11-25", mismatchErr.Header) + assert.Empty(t, mismatchErr.Body) + }, + }, + { + name: "legacy: initialize with nil meta", + method: "initialize", + meta: nil, + protoHeader: "", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + name: "legacy: initialize wins over spoofed modern meta and header", + method: "initialize", + meta: validModernMeta(), + protoHeader: MCPVersionModern, + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rev, err := ClassifyRevision(tt.method, tt.meta, tt.protoHeader) + + assert.Equal(t, tt.expectedRev, rev) + tt.checkErr(t, err) + }) + } +} diff --git a/pkg/telemetry/middleware.go b/pkg/telemetry/middleware.go index 1b9f5eec6e..d3d137506c 100644 --- a/pkg/telemetry/middleware.go +++ b/pkg/telemetry/middleware.go @@ -396,7 +396,16 @@ func (*HTTPMiddleware) addNetworkAttributes(span trace.Span, r *http.Request, ba } // addMethodSpecificAttributes adds attributes specific to certain MCP methods. +// Despite the name, the mcp.client.name block below runs for every method: under +// the Modern (stateless) MCP revision there is no initialize request, so per-request +// _meta.clientInfo is the only source of client attribution. It is a no-op for +// Legacy requests, which carry no _meta.clientInfo and still get mcp.client.name +// from the "initialize" case below. func (m *HTTPMiddleware) addMethodSpecificAttributes(span trace.Span, parsedMCP *mcpparser.ParsedMCPRequest) { + if name, ok := parsedMCP.ClientInfo["name"].(string); ok && name != "" { + span.SetAttributes(attribute.String("mcp.client.name", name)) + } + switch parsedMCP.Method { case string(mcp.MethodToolsCall): // New gen_ai namespace attributes (always emitted) diff --git a/pkg/telemetry/middleware_test.go b/pkg/telemetry/middleware_test.go index 32797cc25e..a582e5802c 100644 --- a/pkg/telemetry/middleware_test.go +++ b/pkg/telemetry/middleware_test.go @@ -1719,6 +1719,68 @@ func TestHTTPMiddleware_LegacyAttributes_Disabled(t *testing.T) { assert.NotContains(t, span.attributes, "mcp.tool.arguments") }, }, + { + name: "addMethodSpecificAttributes - Modern clientInfo sets mcp.client.name on non-initialize span", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + parsedMCP := &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ResourceID: "github_search", + ClientInfo: map[string]interface{}{"name": "acme-client", "version": "1.0"}, + } + + middleware.addMethodSpecificAttributes(span, parsedMCP) + + assert.Equal(t, "acme-client", span.attributes["mcp.client.name"]) + }, + }, + { + name: "addMethodSpecificAttributes - nil ClientInfo on non-initialize method sets nothing (Legacy no-op)", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + parsedMCP := &mcpparser.ParsedMCPRequest{ + Method: "tools/list", + } + + middleware.addMethodSpecificAttributes(span, parsedMCP) + + assert.NotContains(t, span.attributes, "mcp.client.name") + }, + }, + { + name: "addMethodSpecificAttributes - Legacy initialize still sets mcp.client.name from ResourceID", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + parsedMCP := &mcpparser.ParsedMCPRequest{ + Method: "initialize", + ResourceID: "legacy-client", + } + + middleware.addMethodSpecificAttributes(span, parsedMCP) + + assert.Equal(t, "legacy-client", span.attributes["mcp.client.name"]) + }, + }, + { + name: "addMethodSpecificAttributes - ClientInfo present but name missing or non-string sets nothing, no panic", + testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) { + t.Helper() + parsedMCP := &mcpparser.ParsedMCPRequest{ + Method: "tools/call", + ClientInfo: map[string]interface{}{"version": "1.0"}, + } + assert.NotPanics(t, func() { + middleware.addMethodSpecificAttributes(span, parsedMCP) + }) + assert.NotContains(t, span.attributes, "mcp.client.name") + + parsedMCP.ClientInfo = map[string]interface{}{"name": 42} + assert.NotPanics(t, func() { + middleware.addMethodSpecificAttributes(span, parsedMCP) + }) + assert.NotContains(t, span.attributes, "mcp.client.name") + }, + }, { name: "finalizeSpan - new response names, no legacy", testFunc: func(t *testing.T, middleware *HTTPMiddleware, span *mockSpan) {