[Server] Reject malformed MCP session ID headers#384
Conversation
chr-hertel
left a comment
There was a problem hiding this comment.
Thanks for working on the patch @jstar0! Just one minor thing and we can merge it in 👍
|
|
||
| try { | ||
| $this->sessionId = $sessionIdString ? Uuid::fromString($sessionIdString) : null; | ||
| } catch (\InvalidArgumentException) { |
There was a problem hiding this comment.
Let's be specific please and catch the Symfony\Component\Uid\Exception\InvalidArgumentException instead, that's what we're expecting.
There was a problem hiding this comment.
Narrowed the catch to Symfony\Component\Uid\Exception\InvalidArgumentException in d07b7ba. The focused session-header regressions, full transport test, full unit suite, style check, and PHPStan all pass.
There was a problem hiding this comment.
The supported dependency matrix showed that the namespaced exception is not available on Symfony UID 5.4/6.4: PHPStan reported the caught class as missing, and those jobs still throw the global InvalidArgumentException from Uuid::fromString(). I restored the common parent catch and documented the version split in bc1d3bc. Newer UID exceptions subclass that parent, and the catch remains scoped directly around Uuid::fromString(), so this covers all supported Symfony versions without broadening the surrounding request handling.
Summary
Fixes #380.
Malformed or repeated
Mcp-Session-Idrequest headers currently reach UUID parsing insideStreamableHttpTransport, so client-supplied bad session metadata can surface as an internal server error.Changes
Mcp-Session-Idheaders with400 Bad Request.Mcp-Session-Idvalues with400 Bad Request.Verification
Focused regressions were also run for the malformed and duplicate session-id header cases.
Risk
This is limited to invalid Streamable HTTP session-id request metadata. Valid session ids continue to be parsed as UUIDs, missing session ids still flow through the existing request handling, and well-formed but stale session ids still return the existing not-found response.