From 2701fd0bbff0ab3c73560fa4f5d8d9f00a5fd75f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 15 Oct 2026 13:21:59 +0200 Subject: [PATCH] fix(transport): answer 404 for an unknown or expired Mcp-Session-Id The Streamable HTTP transport requires a server to answer requests that carry a terminated or expired session ID with 404, so the client knows to re-initialize. POST /mcp instead minted a new session (with enableSSE) or served the request statelessly under the dead ID (without SSE), and GET /mcp opened a stream on a brand-new session. With sessionTtlMs expiry this meant idle clients silently lost their negotiated state. The session-owner preHandler now also rejects unknown IDs, on POST, GET and DELETE alike. POST gets a JSON-RPC error body (-32001 "Session not found"), which is also what a session owned by another user now answers, so the two cases stay indistinguishable. `initialize` carrying an unknown (or foreign) ID is accepted and gets a fresh session, since that is how a client recovers. Requests without the header behave as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UF4S3w7iA83V9DVsasLJzM --- README.md | 16 ++- src/routes/mcp.ts | 86 +++++++---- test/session-owner-binding.test.ts | 21 ++- test/tool-context-access.test.ts | 24 +++- test/unknown-session.test.ts | 220 +++++++++++++++++++++++++++++ 5 files changed, 334 insertions(+), 33 deletions(-) create mode 100644 test/unknown-session.test.ts diff --git a/README.md b/README.md index bfd15b35..48f990ed 100644 --- a/README.md +++ b/README.md @@ -247,9 +247,23 @@ enabled: `Allow: POST, DELETE`. With SSE disabled, requests that do not carry an `Mcp-Session-Id` keep working statelessly and -no session is created for them. With SSE enabled, every request without a known session id gets +no session is created for them. With SSE enabled, every request without an `Mcp-Session-Id` gets a new session, as before. +A request that carries an `Mcp-Session-Id` the server does not know (never issued, terminated +with `DELETE`, or expired) is answered `404` on `POST`, `GET` and `DELETE`, and no new session is +created for it. On `POST` the body is a JSON-RPC error: + +```json +{ "jsonrpc": "2.0", "id": null, "error": { "code": -32001, "message": "Session not found" } } +``` + +As the Streamable HTTP transport requires, the client should then start over with an +`initialize` request without the header. An `initialize` that still carries the stale id is +accepted too and gets a fresh session. A session bound to another user's access token is +treated exactly like an unknown one. With the memory backend, sessions live in a single +process, so deployments running several instances need sticky sessions or the `redis` option. + Sessions expire after `sessionTtlMs` milliseconds without activity (default `3600000`, one hour), so clients that disconnect without sending `DELETE` do not leave sessions behind. Every request carrying the session id, every message stored for it, and every heartbeat on an open SSE diff --git a/src/routes/mcp.ts b/src/routes/mcp.ts index efdd63ba..8d44a1c4 100644 --- a/src/routes/mcp.ts +++ b/src/routes/mcp.ts @@ -24,6 +24,9 @@ import { isStdioRequest } from '../stdio-marker.ts' const MAX_TIMER_DELAY_MS = 2 ** 31 - 1 +// Implementation-defined server error, the same code the official SDKs use +const SESSION_NOT_FOUND = -32001 + interface MCPPubSubRoutesOptions { enableSSE: boolean opts: MCPPluginOptions @@ -213,25 +216,47 @@ const mcpPubSubRoutesPlugin: FastifyPluginAsync = async (request.method === 'GET' ? (request.query as any)?.['mcp-session-id'] : undefined) } + function sendSessionNotFound (request: FastifyRequest, reply: FastifyReply) { + const body = request.method === 'POST' + ? { jsonrpc: JSONRPC_VERSION, id: null, error: { code: SESSION_NOT_FOUND, message: 'Session not found' } } + : { error: 'Session not found' } + return reply.code(404).type('application/json').send(body) + } + /** - * A session created under an access token belongs to that token's issuer and - * subject: two authorization servers may assign the same `sub` to different - * users, so the subject alone does not identify the owner. Anyone else presenting its ID gets the same 404 as for an unknown session: - * the Streamable HTTP transport defines 404 as "this session does not exist - * for you", and a 403 would confirm to an attacker that the ID is live. - * Runs ahead of anything else that reads the session. + * The Streamable HTTP transport requires 404 for a session ID the server does + * not know (never issued, terminated or expired), so the client learns it has + * to start over with `initialize` instead of silently getting a new session. + * + * A session created under an access token also belongs to that token's issuer + * and subject: two authorization servers may assign the same `sub` to different + * users, so the subject alone does not identify the owner. Anyone else + * presenting its ID is treated exactly as for an unknown session, since a + * different answer would confirm to an attacker that the ID is live. + * + * `initialize` with an unknown ID is let through and gets a fresh session: + * it is how a client recovers, and a client that kept its stale ID around + * should not be stuck. Runs ahead of anything else that reads the session. */ - async function enforceSessionOwner (request: FastifyRequest, reply: FastifyReply): Promise { + async function enforceSession (request: FastifyRequest, reply: FastifyReply): Promise { const sessionId = getRequestSessionId(request) if (!sessionId) return const session = await sessionStore.get(sessionId) - if (session?.ownerSub === undefined) return - - if (session.ownerSub !== request.tokenPayload?.sub || session.ownerIss !== request.tokenIssuer) { + if (session) { + if (session.ownerSub === undefined) return + if (session.ownerSub === request.tokenPayload?.sub && session.ownerIss === request.tokenIssuer) return request.log.warn({ sessionId }, 'Rejected MCP request for a session owned by a different user') - return reply.code(404).type('application/json').send({ error: 'Session not found' }) + } else { + request.log.info({ sessionId }, 'Rejected MCP request for an unknown or expired session') } + + if (request.method === 'POST' && isInitializeRequest(request.body)) { + ;(request as any).mcpIgnoreSessionId = true + return + } + + return sendSessionNotFound(request, reply) } // Scoped to the /mcp routes only: this plugin is not encapsulated, so an @@ -241,8 +266,8 @@ const mcpPubSubRoutesPlugin: FastifyPluginAsync = async // as it would if it were registered as a global preHandler hook, so the // session owner check sees the verified token. const mcpPreHandler: preHandlerHookHandler[] = authPreHandler - ? [authPreHandler, enforceSessionOwner, reconcileProtocolVersion] - : [enforceSessionOwner, reconcileProtocolVersion] + ? [authPreHandler, enforceSession, reconcileProtocolVersion] + : [enforceSession, reconcileProtocolVersion] // Heartbeats refresh session activity, so they must come more often than the // idle TTL or a live stream's session could expire between two of them @@ -435,8 +460,16 @@ const mcpPubSubRoutesPlugin: FastifyPluginAsync = async return reply.code(400).type('application/json').send(idError) } - let sessionId = request.headers['mcp-session-id'] as string | undefined + // enforceSession already answered 404 for an unknown ID, except on + // `initialize`, which starts over on a fresh session + let sessionId = (request as any).mcpIgnoreSessionId + ? undefined + : request.headers['mcp-session-id'] as string | undefined const existingSession = sessionId ? await sessionStore.get(sessionId) : null + if (sessionId && !existingSession) { + // Expired or deleted since enforceSession looked it up + return sendSessionNotFound(request, reply) + } // Sessions belong to Streamable HTTP, not to SSE. Without SSE we only open // one on initialize: other requests keep working statelessly as before, @@ -542,6 +575,17 @@ const mcpPubSubRoutesPlugin: FastifyPluginAsync = async request.log.info({ sessionId }, 'Handling SSE request') + // enforceSession already answered 404 for an unknown ID; this covers a + // session that expired or was deleted since then + let existingSession: SessionMetadata | null = null + if (sessionId) { + existingSession = await sessionStore.get(sessionId) + if (!existingSession) { + return sendSessionNotFound(request, reply) + } + await sessionStore.touch?.(existingSession.id) + } + // We are opting out of Fastify proper reply.hijack() @@ -551,18 +595,8 @@ const mcpPubSubRoutesPlugin: FastifyPluginAsync = async raw.setHeader('Content-type', 'text/event-stream') raw.setHeader('Cache-Control', 'no-cache') - let session: SessionMetadata - if (sessionId) { - const existingSession = await sessionStore.get(sessionId) - if (existingSession) { - session = existingSession - await sessionStore.touch?.(session.id) - } else { - session = await createSession(request) - raw.setHeader('Mcp-Session-Id', session.id) - } - } else { - session = await createSession(request) + const session = existingSession ?? await createSession(request) + if (!existingSession) { raw.setHeader('Mcp-Session-Id', session.id) } diff --git a/test/session-owner-binding.test.ts b/test/session-owner-binding.test.ts index e597064b..3778b94d 100644 --- a/test/session-owner-binding.test.ts +++ b/test/session-owner-binding.test.ts @@ -63,11 +63,13 @@ async function buildApp (t: TestContext, redis?: Redis, options: BuildOptions = return app } -async function initialize (app: Awaited>, token: string): Promise { +async function initialize (app: Awaited>, token: string, staleSessionId?: string): Promise { const response = await app.inject({ method: 'POST', url: '/mcp', - headers: { authorization: `Bearer ${token}` }, + headers: staleSessionId + ? { authorization: `Bearer ${token}`, 'mcp-session-id': staleSessionId } + : { authorization: `Bearer ${token}` }, payload: { jsonrpc: JSONRPC_VERSION, id: 1, @@ -121,9 +123,22 @@ async function assertHijackRejected ( // let him in on any of the three methods. const post = await callWhoami(app, bob, sessionId) t.assert.strictEqual(post.statusCode, 404) - t.assert.deepStrictEqual(post.json(), { error: 'Session not found' }) + // Same answer as for a session that does not exist at all + t.assert.deepStrictEqual(post.json(), { + jsonrpc: JSONRPC_VERSION, + id: null, + error: { code: -32001, message: 'Session not found' } + }) t.assert.strictEqual(post.headers['mcp-session-id'], undefined) + // initialize with an unknown ID starts a fresh session, so it must do the + // same here: Bob gets his own session, not Alice's and not a tell-tale 404 + const bobSessionId = await initialize(app, bob, sessionId) + t.assert.notStrictEqual(bobSessionId, sessionId) + const bobOwn = await callWhoami(app, bob, bobSessionId) + t.assert.strictEqual(bobOwn.statusCode, 200) + t.assert.strictEqual(bobOwn.json().result.content[0].text, bobClaims.sub) + const get = await app.inject({ method: 'GET', url: '/mcp', diff --git a/test/tool-context-access.test.ts b/test/tool-context-access.test.ts index 2f636f3a..ec5675be 100644 --- a/test/tool-context-access.test.ts +++ b/test/tool-context-access.test.ts @@ -9,7 +9,7 @@ import type { ReadResourceResult, GetPromptResult } from '../src/schema.ts' -import { JSONRPC_VERSION } from '../src/schema.ts' +import { JSONRPC_VERSION, LATEST_PROTOCOL_VERSION } from '../src/schema.ts' describe('Tool Context Access', () => { describe('Request Object Access', () => { @@ -320,12 +320,30 @@ describe('Tool Context Access', () => { } } + // An unknown session id is answered 404, so open a real session first + const init = await app.inject({ + method: 'POST', + url: '/mcp', + payload: { + jsonrpc: JSONRPC_VERSION, + id: 0, + method: 'initialize', + params: { + protocolVersion: LATEST_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: 'test-client', version: '1.0.0' } + } + } + }) + const sessionId = init.headers['mcp-session-id'] as string + t.assert.ok(sessionId) + const response = await app.inject({ method: 'POST', url: '/mcp', payload: request, headers: { - 'mcp-session-id': 'test-session-123' + 'mcp-session-id': sessionId } }) @@ -333,7 +351,7 @@ describe('Tool Context Access', () => { const body = response.json() as JSONRPCResultResponse const result = body.result as CallToolResult const textContent = result.content[0] as { type: 'text', text: string } - t.assert.ok(textContent.text.includes('test-session-123'), 'SessionId should still be accessible') + t.assert.ok(textContent.text.includes(sessionId), 'SessionId should still be accessible') }) }) diff --git a/test/unknown-session.test.ts b/test/unknown-session.test.ts new file mode 100644 index 00000000..922653e5 --- /dev/null +++ b/test/unknown-session.test.ts @@ -0,0 +1,220 @@ +import { test, describe } from 'node:test' +import type { TestContext } from 'node:test' +import { setTimeout as sleep } from 'node:timers/promises' +import Fastify from 'fastify' +import type { Redis } from 'ioredis' +import mcpPlugin from '../src/index.ts' +import { JSONRPC_VERSION, LATEST_PROTOCOL_VERSION } from '../src/schema.ts' +import { testWithRedis } from './redis-test-utils.ts' + +// The Streamable HTTP transport: once a session is terminated or expired the +// server MUST answer requests carrying its ID with 404, so the client knows to +// start over with a fresh `initialize`. + +const initializeRequest = { + jsonrpc: JSONRPC_VERSION, + id: 1, + method: 'initialize', + params: { + protocolVersion: LATEST_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: 'test-client', version: '1.0.0' } + } +} + +const whoamiCall = { + jsonrpc: JSONRPC_VERSION, + id: 2, + method: 'tools/call', + params: { name: 'whoami', arguments: {} } +} + +const sessionNotFound = { + jsonrpc: JSONRPC_VERSION, + id: null, + error: { code: -32001, message: 'Session not found' } +} + +const STALE_ID = 'f0d1a2b3-0000-4000-8000-000000000000' + +async function buildApp (t: TestContext, opts: { enableSSE: boolean, sessionTtlMs?: number, redis?: Redis }) { + const app = Fastify({ logger: false }) + t.after(() => app.close()) + + const { redis, ...rest } = opts + await app.register(mcpPlugin, { + serverInfo: { name: 'test-server', version: '1.0.0' }, + ...rest, + ...(redis ? { redis: { host: redis.options.host!, port: redis.options.port!, db: redis.options.db! } } : {}) + }) + + const calls: string[] = [] + app.mcpAddTool({ + name: 'whoami', + description: 'Returns the session id the tool ran under' + }, async (_params, { sessionId }) => { + calls.push(sessionId ?? 'none') + return { content: [{ type: 'text', text: sessionId ?? 'none' }] } + }) + + await app.ready() + return { app, calls } +} + +function post (app: Awaited>['app'], payload: unknown, sessionId?: string) { + return app.inject({ + method: 'POST', + url: '/mcp', + headers: sessionId ? { 'mcp-session-id': sessionId } : {}, + payload: payload as any + }) +} + +for (const enableSSE of [true, false]) { + describe(`unknown Mcp-Session-Id (enableSSE: ${enableSSE})`, () => { + test('a request with an unknown session id is answered 404 and not served', async (t: TestContext) => { + const { app, calls } = await buildApp(t, { enableSSE }) + + const response = await post(app, whoamiCall, STALE_ID) + + t.assert.strictEqual(response.statusCode, 404) + t.assert.deepStrictEqual(response.json(), sessionNotFound) + t.assert.strictEqual(response.headers['mcp-session-id'], undefined, 'no new session is handed out') + t.assert.deepStrictEqual(calls, [], 'the tool must not run') + }) + + test('a notification with an unknown session id is answered 404', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE }) + + const response = await post(app, { jsonrpc: JSONRPC_VERSION, method: 'notifications/initialized' }, STALE_ID) + + t.assert.strictEqual(response.statusCode, 404) + t.assert.deepStrictEqual(response.json(), sessionNotFound) + t.assert.strictEqual(response.headers['mcp-session-id'], undefined) + }) + + test('initialize with a stale session id starts a fresh session', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE }) + + const response = await post(app, initializeRequest, STALE_ID) + + t.assert.strictEqual(response.statusCode, 200) + const sessionId = response.headers['mcp-session-id'] + t.assert.ok(sessionId) + t.assert.notStrictEqual(sessionId, STALE_ID) + + const follow = await post(app, whoamiCall, sessionId as string) + t.assert.strictEqual(follow.statusCode, 200) + t.assert.strictEqual(follow.json().result.content[0].text, sessionId) + }) + + test('a terminated session is answered 404 afterwards', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE }) + + const init = await post(app, initializeRequest) + const sessionId = init.headers['mcp-session-id'] as string + t.assert.ok(sessionId) + + const del = await app.inject({ method: 'DELETE', url: '/mcp', headers: { 'mcp-session-id': sessionId } }) + t.assert.strictEqual(del.statusCode, 204) + + const response = await post(app, whoamiCall, sessionId) + t.assert.strictEqual(response.statusCode, 404) + t.assert.deepStrictEqual(response.json(), sessionNotFound) + }) + + test('an expired session is answered 404 and the client can re-initialize', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE, sessionTtlMs: 50 }) + + const init = await post(app, initializeRequest) + const sessionId = init.headers['mcp-session-id'] as string + t.assert.ok(sessionId) + + await sleep(120) + + const response = await post(app, whoamiCall, sessionId) + t.assert.strictEqual(response.statusCode, 404) + t.assert.deepStrictEqual(response.json(), sessionNotFound) + + const reinit = await post(app, initializeRequest) + t.assert.strictEqual(reinit.statusCode, 200) + t.assert.ok(reinit.headers['mcp-session-id']) + t.assert.notStrictEqual(reinit.headers['mcp-session-id'], sessionId) + }) + + test('requests without a session id keep working', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE }) + + const response = await post(app, whoamiCall) + + t.assert.strictEqual(response.statusCode, 200) + const sessionId = response.headers['mcp-session-id'] + if (enableSSE) { + t.assert.ok(sessionId, 'with SSE a session is still created on demand') + t.assert.strictEqual(response.json().result.content[0].text, sessionId) + } else { + t.assert.strictEqual(sessionId, undefined, 'without SSE the request is served statelessly') + t.assert.strictEqual(response.json().result.content[0].text, 'none') + } + }) + }) +} + +describe('unknown Mcp-Session-Id on GET /mcp', () => { + test('is answered 404 instead of opening a stream on a new session', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE: true }) + + const viaHeader = await app.inject({ + method: 'GET', + url: '/mcp', + headers: { accept: 'text/event-stream', 'mcp-session-id': STALE_ID } + }) + t.assert.strictEqual(viaHeader.statusCode, 404) + t.assert.deepStrictEqual(viaHeader.json(), { error: 'Session not found' }) + t.assert.strictEqual(viaHeader.headers['mcp-session-id'], undefined) + + const viaQuery = await app.inject({ + method: 'GET', + url: `/mcp?mcp-session-id=${STALE_ID}`, + headers: { accept: 'text/event-stream' } + }) + t.assert.strictEqual(viaQuery.statusCode, 404) + t.assert.strictEqual(viaQuery.headers['mcp-session-id'], undefined) + }) + + test('without a session id still opens a stream on a new session', async (t: TestContext) => { + const { app } = await buildApp(t, { enableSSE: true }) + + const response = await app.inject({ + method: 'GET', + url: '/mcp', + payloadAsStream: true, + headers: { accept: 'text/event-stream' } + }) + t.assert.strictEqual(response.statusCode, 200) + t.assert.ok(response.headers['mcp-session-id']) + response.stream().destroy() + }) +}) + +describe('unknown Mcp-Session-Id (redis)', () => { + testWithRedis('POST and GET answer 404 for a session redis does not know', async (redis, t: TestContext) => { + const { app, calls } = await buildApp(t, { enableSSE: true, redis }) + + const response = await post(app, whoamiCall, STALE_ID) + t.assert.strictEqual(response.statusCode, 404) + t.assert.deepStrictEqual(response.json(), sessionNotFound) + t.assert.deepStrictEqual(calls, []) + + const get = await app.inject({ + method: 'GET', + url: '/mcp', + headers: { accept: 'text/event-stream', 'mcp-session-id': STALE_ID } + }) + t.assert.strictEqual(get.statusCode, 404) + + const init = await post(app, initializeRequest, STALE_ID) + t.assert.strictEqual(init.statusCode, 200) + t.assert.notStrictEqual(init.headers['mcp-session-id'], STALE_ID) + }) +})