Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ require (
github.com/shirou/gopsutil/v4 v4.26.5
github.com/spf13/viper v1.21.0
github.com/stacklok/toolhive-catalog v0.20260720.0
github.com/stacklok/toolhive-core v0.0.34-0.20260724174534-e86653447aee
github.com/stacklok/toolhive-core v0.0.34
github.com/stretchr/testify v1.11.1
github.com/swaggo/swag/v2 v2.0.0-rc5
github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd
Expand Down Expand Up @@ -90,7 +90,7 @@ require (
github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.32 // indirect
github.com/go-openapi/runtime/server-middleware v0.30.0 // indirect
github.com/klauspost/cpuid/v2 v2.3.0 // indirect
github.com/modelcontextprotocol/go-sdk v1.6.1 // indirect
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3 // indirect
github.com/oklog/ulid/v2 v2.1.1 // indirect
github.com/segmentio/encoding v0.5.4 // indirect
github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ github.com/moby/sys/userns v0.1.0 h1:tVLXkFOxVu9A64/yh59slHVv9ahO9UIev4JZusOLG/g
github.com/moby/sys/userns v0.1.0/go.mod h1:IHUYgu/kao6N8YZlp9Cf444ySSvCmDlmzUcYfDHOl28=
github.com/moby/term v0.5.2 h1:6qk3FJAFDs6i/q3W/pQ97SX192qKfZgGjCQqfCJkgzQ=
github.com/moby/term v0.5.2/go.mod h1:d3djjFCrjnB+fl8NJux+EJzu0msscUP+f8it8hPkFLc=
github.com/modelcontextprotocol/go-sdk v1.6.1 h1:0zOSupjKUxPKSocPT1Wtago+mUHU2/uZ4xSOY0FGReU=
github.com/modelcontextprotocol/go-sdk v1.6.1/go.mod h1:kzm3kzFL1/+AziGOE0nUs3gvPoNxMCvkxokMkuFapXQ=
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3 h1:SEAY9IduDif4iApnZgpFkjFIdo3askSGZVbZIYyTy6I=
github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts=
github.com/modelcontextprotocol/registry v1.8.0 h1:x/seX0ji4iqRUpSovmkBcGbxfRiZZ7dgPwBcpCJrSTM=
github.com/modelcontextprotocol/registry v1.8.0/go.mod h1:G6AUpTpZSekQvcLl5griUjijEE8vedARE/TyCaHEFdo=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
Expand Down Expand Up @@ -787,8 +787,8 @@ github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU=
github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY=
github.com/stacklok/toolhive-catalog v0.20260720.0 h1:Ct2BnLmEpZCNsChd6CTbiSfBARbwoztpdbOl8uWkPIY=
github.com/stacklok/toolhive-catalog v0.20260720.0/go.mod h1:G33nbS4PplpBl7lk/hPiIYpsmBl5OATvpohpXJujSfg=
github.com/stacklok/toolhive-core v0.0.34-0.20260724174534-e86653447aee h1:A8gdDgE7CE1j8BHP782vEMMfu5pMUgmfHADtNvrgJDw=
github.com/stacklok/toolhive-core v0.0.34-0.20260724174534-e86653447aee/go.mod h1:WWt0RUWgA3r39Oej3SBmRswjkopYn9YK2b/mdR9j+WM=
github.com/stacklok/toolhive-core v0.0.34 h1:vhblHGLC+Lp7I+TaIcJsiuzUQo6463o3IooSxIJlhiM=
github.com/stacklok/toolhive-core v0.0.34/go.mod h1:7Cgo1ALJHolkHw/xo7dN9TvFSX0LsKJwXXn0z5zvlyI=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
Expand Down
32 changes: 26 additions & 6 deletions pkg/mcp/parser_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,33 @@ func TestParsingMiddlewareWithRealMCPClients(t *testing.T) {
assert.NotEmpty(t, readResult.Contents)

// Verify that all requests were parsed by the middleware
assert.GreaterOrEqual(t, len(parsedRequests), 4, "Expected at least 4 parsed requests (initialize, list tools, call tool, list resources, read resource)")

// Verify specific parsed requests
foundInitialize := false
assert.GreaterOrEqual(t, len(parsedRequests), 4, "Expected at least 4 parsed requests (session establishment, list tools, call tool, list resources, read resource)")

// Verify specific parsed requests. mcpcompat's client is Modern-first
// (SEP-2575) on every transport, so both arms send server/discover
// first. The SSE server transport advertises 2026-07-28 unconditionally
// (no ProtocolVersionSupporter gate), so discover succeeds and
// "initialize" is never sent there. The streamable-HTTP server here is
// stateful (no WithStateless), which gates 2026-07-28 on being
// stateless, so its discover negotiates down and it additionally sends
// "initialize".
foundSessionEstablished := false
foundToolCall := false
foundResourceRead := false
var methodsSeen []string
for _, parsed := range parsedRequests {
methodsSeen = append(methodsSeen, parsed.Method)
switch parsed.Method {
case "initialize":
foundInitialize = true
foundSessionEstablished = true
assert.Equal(t, "test-client", parsed.ResourceID)
case "server/discover":
if tc.transport == "sse" {
// parser.go maps server/discover to the static ResourceID
// "discover" (not a client-name ResourceID).
foundSessionEstablished = true
assert.Equal(t, "discover", parsed.ResourceID)
}
case "tools/call":
foundToolCall = true
assert.Equal(t, "test_tool", parsed.ResourceID)
Expand All @@ -163,7 +179,11 @@ func TestParsingMiddlewareWithRealMCPClients(t *testing.T) {
assert.Equal(t, "test://resource", parsed.ResourceID)
}
}
assert.True(t, foundInitialize, "Initialize request should have been parsed")
assert.True(t, foundSessionEstablished, "session establishment request (initialize or server/discover) should have been parsed")
if tc.transport == "sse" {
assert.Contains(t, methodsSeen, "server/discover", "SSE arm should negotiate via server/discover")
assert.NotContains(t, methodsSeen, "initialize", "SSE arm is Modern-first and should never fall back to initialize here")
}
assert.True(t, foundToolCall, "Tool call request should have been parsed")
assert.True(t, foundResourceRead, "Resource read request should have been parsed")
})
Expand Down
129 changes: 116 additions & 13 deletions pkg/mcp/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"maps"
"strings"
)

Expand Down Expand Up @@ -55,16 +56,83 @@ const metaKeyClientInfo = "io.modelcontextprotocol/clientInfo"
// 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.
// metaKeyLogLevel carries the per-request minimum log level on Modern
// (2026-07-28) requests (draft schema RequestMetaObject; go-sdk protocol.go).
// It is a reserved per-hop key that must be stripped before a Legacy backend
// hop, but — unlike the other reserved keys — its mere presence is NOT a claim
// of the Modern revision: go-sdk's validateRequestMeta gates Modern-ness purely
// on protocolVersion, and SEP-2577 already deprecates logLevel. It therefore
// belongs in the egress strip set (ReservedModernMetaKeys) but not the ingress
// signal set (modernSignalMetaKeys).
const metaKeyLogLevel = "io.modelcontextprotocol/logLevel"

// ReservedModernMetaKeys is the EGRESS/strip set: every reserved per-hop
// io.modelcontextprotocol/* control key that vMCP must remove before forwarding
// a caller-supplied _meta onto a Legacy (session-based, stateful) backend hop,
// where these keys are invalid — see StripReservedModernMeta.
//
// Exported so a Modern client (which sets these keys, the mirror of the
// classifier that reads them) can strip a caller's copies before overlaying
// its own authoritative values — see ModernRequestMeta.
var ReservedModernMetaKeys = []string{metaKeyProtocolVersion, metaKeyClientInfo, metaKeyClientCapabilities}
// Exported so a Modern client (the mirror of the classifier that reads these
// keys) can strip a caller's copies before overlaying its own authoritative
// values — see ModernRequestMeta / mergeModernMeta.
//
// This is deliberately NOT the Modern-detection set: that is
// modernSignalMetaKeys, a strict subset. "Strip this key on the way out" and
// "this key means the request is Modern" are separate decisions, because a
// reserved key can require the former without the latter (logLevel). Conflating
// them would make a request rejectable merely for carrying a
// strippable-but-not-signalling key.
var ReservedModernMetaKeys = []string{
metaKeyProtocolVersion,
metaKeyClientInfo,
metaKeyClientCapabilities,
metaKeyLogLevel,
}

// modernSignalMetaKeys is the INGRESS/detection set consumed by hasModernSignal:
// the reserved keys a Legacy client never sets, whose presence — independent of
// whether the 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 them turns into a rejection, never a
// downgrade.
//
// It is a strict subset of ReservedModernMetaKeys: logLevel is intentionally
// excluded so a request carrying only logLevel — which go-sdk's
// validateRequestMeta accepts (gating purely on protocolVersion) and which
// SEP-2577 deprecates — is classified Legacy rather than misdetected as Modern
// and then rejected.
var modernSignalMetaKeys = []string{
metaKeyProtocolVersion,
metaKeyClientInfo,
metaKeyClientCapabilities,
}

// StripReservedModernMeta returns a copy of meta with every ReservedModernMetaKeys
// entry removed, leaving all other caller-supplied keys (including trace-context
// keys) untouched. The input is never mutated (maps.Clone).
//
// Use this at every Legacy backend egress that forwards a caller-supplied _meta
// map: a downstream Modern request's reserved io.modelcontextprotocol/* _meta
// claims a per-request protocol version that is only valid on a stateless
// Modern hop. If it leaks onto a Legacy (session-based, stateful) backend call,
// go-sdk v1.7 rejects the request outright (HTTP 400: "protocol version ...
// is only supported on stateless HTTP servers") because ANY _meta.protocolVersion
// on a stateful streamable-HTTP server is invalid, regardless of its value. vMCP
// is the backend's actual MCP peer on this hop, not the downstream caller, so
// these reserved keys must never cross it.
//
// nil or empty input returns nil (matching mergeModernMeta's caller-tolerant
// convention); a non-empty map with none of the reserved keys present is
// returned as-is (via maps.Clone, so callers still get a copy, not the original).
func StripReservedModernMeta(meta map[string]any) map[string]any {
if len(meta) == 0 {
return nil
}
stripped := maps.Clone(meta)
for _, k := range ReservedModernMetaKeys {
delete(stripped, k)
}
return stripped
}

// ModernRequestMeta builds the reserved _meta object every Modern (2026-07-28)
// request must carry: protocolVersion, clientInfo, and (empty) clientCapabilities.
Expand Down Expand Up @@ -445,14 +513,15 @@ func metaFromParamsMap(paramsMap map[string]any) map[string]any {
}

// 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).
// either the header exactly names MCPVersionModern, or _meta carries any of the
// modernSignalMetaKeys (regardless of whether their values are well-formed).
// It reads modernSignalMetaKeys, NOT ReservedModernMetaKeys — a key can be
// reserved-for-stripping without signalling Modern (logLevel).
func hasModernSignal(meta map[string]any, protoHeader string) bool {
if protoHeader == MCPVersionModern {
return true
}
for _, key := range ReservedModernMetaKeys {
for _, key := range modernSignalMetaKeys {
if _, ok := meta[key]; ok {
return true
}
Expand Down Expand Up @@ -522,3 +591,37 @@ func decodeSentinelName(v string) (string, error) {
}
return string(decoded), nil
}

// EncodeSentinelName encodes v into the draft spec's base64 sentinel format
// (=?base64?<payload>?=) when required — the mirror of decodeSentinelName.
// Encoding is required when EITHER:
// - v is not safely representable as a plain ASCII header value: any byte
// falls outside printable ASCII 0x21-0x7E, which also covers leading/
// trailing whitespace and CR/LF (both fall below 0x21 and would
// otherwise make net/http reject the request outright), or
// - v already matches the sentinel pattern (has both sentinelPrefix and
// sentinelSuffix), which must be escaped so the server doesn't mistake a
// literal name for an encoded payload.
//
// Otherwise v is returned unchanged. The result always round-trips through
// decodeSentinelName back to v.
func EncodeSentinelName(v string) string {
if !needsSentinelEncoding(v) {
return v
}
return sentinelPrefix + base64.StdEncoding.EncodeToString([]byte(v)) + sentinelSuffix
}

// needsSentinelEncoding reports whether v requires sentinel encoding; see
// EncodeSentinelName for the two conditions checked.
func needsSentinelEncoding(v string) bool {
if strings.HasPrefix(v, sentinelPrefix) && strings.HasSuffix(v, sentinelSuffix) {
return true
}
for i := 0; i < len(v); i++ {
if v[i] < 0x21 || v[i] > 0x7E {
return true
}
}
return false
}
108 changes: 108 additions & 0 deletions pkg/mcp/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,24 @@ func TestClassifyRevision(t *testing.T) {
require.NoError(t, err)
},
},
{
// logLevel is a reserved key that must be STRIPPED on egress but is
// NOT a Modern signal (go-sdk's validateRequestMeta gates purely on
// protocolVersion; SEP-2577 deprecates logLevel). Guards the split
// between ReservedModernMetaKeys and modernSignalMetaKeys: if
// hasModernSignal ever iterated the strip set again, this request
// would be misdetected Modern and rejected instead of classified
// Legacy.
name: "legacy: logLevel reserved key alone is not a Modern signal",
method: "tools/call",
meta: map[string]any{metaKeyLogLevel: "debug"},
protoHeader: "",
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",
Expand Down Expand Up @@ -618,3 +636,93 @@ func TestDecodeSentinelName(t *testing.T) {
})
}
}

// TestEncodeSentinelName pins EncodeSentinelName as the exact mirror of
// decodeSentinelName: every case must round-trip back to the original value.
func TestEncodeSentinelName(t *testing.T) {
t.Parallel()

tests := []struct {
name string
input string
wantSame bool // true if the value must pass through unchanged
}{
{name: "plain ASCII name unchanged", input: "my-tool", wantSame: true},
{name: "URI with colon and slash unchanged", input: "file:///tmp/foo.txt", wantSame: true},
{name: "accented character encoded", input: "café-résumé", wantSame: false},
{name: "CJK character encoded", input: "工具", wantSame: false},
{name: "CR in value encoded", input: "bad\rname", wantSame: false},
{name: "LF in value encoded", input: "bad\nname", wantSame: false},
{name: "value already shaped like a sentinel is escaped", input: sentinelEncode("my-tool"), wantSame: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := EncodeSentinelName(tt.input)
if tt.wantSame {
assert.Equal(t, tt.input, got)
} else {
assert.NotEqual(t, tt.input, got)
}

decoded, err := decodeSentinelName(got)
require.NoError(t, err)
assert.Equal(t, tt.input, decoded, "must round-trip through decodeSentinelName")
})
}
}

// TestStripReservedModernMeta pins the copy-before-mutate contract: it removes
// exactly the reserved io.modelcontextprotocol/* keys, never mutates the
// caller's map, and returns nil for empty input.
func TestStripReservedModernMeta(t *testing.T) {
t.Parallel()

t.Run("nil input returns nil", func(t *testing.T) {
t.Parallel()
assert.Nil(t, StripReservedModernMeta(nil))
})

t.Run("empty input returns nil", func(t *testing.T) {
t.Parallel()
assert.Nil(t, StripReservedModernMeta(map[string]any{}))
})

t.Run("removes reserved keys, preserves the rest", func(t *testing.T) {
t.Parallel()
in := map[string]any{
metaKeyProtocolVersion: MCPVersionModern,
metaKeyClientInfo: map[string]any{"name": "x"},
metaKeyClientCapabilities: map[string]any{},
metaKeyLogLevel: "debug",
"progressToken": "tok-1",
"traceparent": "00-abc-def-01",
"custom": 42,
}
got := StripReservedModernMeta(in)
for _, k := range ReservedModernMetaKeys {
assert.NotContains(t, got, k, "reserved key %q must be stripped", k)
}
assert.Equal(t, "tok-1", got["progressToken"])
assert.Equal(t, "00-abc-def-01", got["traceparent"])
assert.Equal(t, 42, got["custom"])
})

t.Run("does not mutate the caller's map", func(t *testing.T) {
t.Parallel()
in := map[string]any{metaKeyProtocolVersion: MCPVersionModern, "custom": 1}
_ = StripReservedModernMeta(in)
assert.Contains(t, in, metaKeyProtocolVersion, "caller's map must be untouched")
assert.Len(t, in, 2)
})

t.Run("no reserved keys returns a copy, not the original", func(t *testing.T) {
t.Parallel()
in := map[string]any{"custom": 1}
got := StripReservedModernMeta(in)
require.Equal(t, in, got)
got["custom"] = 2
assert.Equal(t, 1, in["custom"], "returned value must be a copy")
})
}
Loading
Loading