-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Describe the bug
Hi! First off — thank you for publishing GHSA-345p-7cg4-v4c7 and for the patch.
I’m posting because I think one sentence in the advisory may be a little misleading for one specific deployment pattern (shared server + multiple connect() calls).
In the advisory’s “What is affected” section, it says:
Final responses (the return value from a tool/resource/prompt handler): Not affected. These use a captured transport reference and route correctly even with server re-use.
In my experience (details below), final responses can be affected in the shared-server / multi-connect case: they may be sent on the wrong transport, which results in either a misroute or the requesting client timing out.
So this may just be a wording clarification request rather than a code bug.
To Reproduce
Steps to reproduce the behavior:
- Use
@modelcontextprotocol/sdkv1.25.3 - Use stateful
StreamableHTTPServerTransport - Create a single shared
McpServerinstance once at startup and reuse it across sessions. - For each new session, create a new transport and call
await mcpServer.connect(transport)(overwriting the server’s transport).
Example:
// mcp-server/src/index.ts (before fix)
const mcpServer = createMcpServer();
const app = createApp(mcpServer);
// mcp-server/src/app.ts (before fix)
export function createApp(mcpServer: McpServer, options?: CreateAppOptions) {
// inside POST handler
if (!transport) {
transport = new StreamableHTTPServerTransport({
sessionIdGenerator: () => newSid,
eventStore,
retryInterval
});
// ... transport.send wrapper for logging ...
await mcpServer.connect(transport); // <-- BUG: shared server connect overwrites _transport
sessions.set(newSid, transport);
res.setHeader("Mcp-Session-Id", newSid);
}
await transport.handleRequest(req, res, req.body);
}- Connect client A (session A), then connect client B (session B).
- Send a request from client A after client B has connected.
- Observe that the final response is sent using the wrong transport and fails with timeout(from client A).
Expected behavior
This is mostly a documentation/advisory wording suggestion:
The shared-server + multi-connect pattern is in scope for Issue 2, and it seems like final responses should be listed as “affected” as well (or at least “can be affected”), since in this setup the captured transport reference can be the last-connected transport, not the transport that received the request, hence misroute eventually.
Additional context
• Reproduced in @modelcontextprotocol/sdk v1.25.3
• Transport: stateful
• Deployment pattern: single shared McpServer instance connected to multiple clients, no ALB / no load balancer
I fixed this by creating a new McpServer per session (passing a factory into createApp() and calling createMcpServer() when creating a new session), then connecting only that server to that session’s transport. After that, I no longer see the issue.
Thanks again!