diff --git a/.changeset/neat-planets-wash.md b/.changeset/neat-planets-wash.md new file mode 100644 index 0000000000..09341d9525 --- /dev/null +++ b/.changeset/neat-planets-wash.md @@ -0,0 +1,7 @@ +--- +"@modelcontextprotocol/core": patch +"@modelcontextprotocol/node": patch +"@modelcontextprotocol/server": patch +--- + +Fix invalid JSON-RPC request error handling for malformed request payloads. diff --git a/packages/core/src/shared/stdio.ts b/packages/core/src/shared/stdio.ts index 7283a5ef96..a5e08841c2 100644 --- a/packages/core/src/shared/stdio.ts +++ b/packages/core/src/shared/stdio.ts @@ -1,6 +1,8 @@ import type { JSONRPCMessage } from '../types/index.js'; import { JSONRPCMessageSchema } from '../types/index.js'; +export type JSONRPCMessageValidationError = Error & { rawMessage?: unknown }; + /** * Buffers a continuous stdio stream into discrete JSON-RPC messages. */ @@ -42,7 +44,15 @@ export class ReadBuffer { } export function deserializeMessage(line: string): JSONRPCMessage { - return JSONRPCMessageSchema.parse(JSON.parse(line)); + const rawMessage = JSON.parse(line); + try { + return JSONRPCMessageSchema.parse(rawMessage); + } catch (error) { + if (error instanceof Error) { + (error as JSONRPCMessageValidationError).rawMessage = rawMessage; + } + throw error; + } } export function serializeMessage(message: JSONRPCMessage): string { diff --git a/packages/core/test/shared/stdio.test.ts b/packages/core/test/shared/stdio.test.ts index 65d1de0eaa..2a972405ab 100644 --- a/packages/core/test/shared/stdio.test.ts +++ b/packages/core/test/shared/stdio.test.ts @@ -1,4 +1,4 @@ -import { ReadBuffer } from '../../src/shared/stdio.js'; +import { deserializeMessage, ReadBuffer } from '../../src/shared/stdio.js'; import type { JSONRPCMessage } from '../../src/types/index.js'; const testMessage: JSONRPCMessage = { @@ -112,4 +112,10 @@ describe('non-JSON line filtering', () => { expect(() => readBuffer.readMessage()).toThrow(); }); + + test('should attach the raw parsed message to schema validation errors', () => { + const rawMessage = { jsonrpc: '2.0', id: 1, method_: 'tools/list' }; + + expect(() => deserializeMessage(JSON.stringify(rawMessage))).toThrowError(expect.objectContaining({ rawMessage })); + }); }); diff --git a/packages/middleware/node/test/streamableHttp.test.ts b/packages/middleware/node/test/streamableHttp.test.ts index 460f72d93f..f77680938c 100644 --- a/packages/middleware/node/test/streamableHttp.test.ts +++ b/packages/middleware/node/test/streamableHttp.test.ts @@ -788,10 +788,7 @@ describe('Zod v4', () => { expect(response.status).toBe(400); const errorData = await response.json(); - expect(errorData).toMatchObject({ - jsonrpc: '2.0', - error: expect.anything() - }); + expectErrorResponse(errorData, -32_600, /Invalid Request/); }); it('should reject requests to uninitialized server', async () => { diff --git a/packages/server/src/server/stdio.ts b/packages/server/src/server/stdio.ts index ac2dd3f784..f67293a08a 100644 --- a/packages/server/src/server/stdio.ts +++ b/packages/server/src/server/stdio.ts @@ -1,9 +1,24 @@ import type { Readable, Writable } from 'node:stream'; -import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core'; -import { ReadBuffer, serializeMessage } from '@modelcontextprotocol/core'; +import type { JSONRPCMessage, JSONRPCMessageValidationError, RequestId, Transport } from '@modelcontextprotocol/core'; +import { INVALID_REQUEST, ReadBuffer, serializeMessage } from '@modelcontextprotocol/core'; import { process } from '@modelcontextprotocol/server/_shims'; +function getRequestIdFromInvalidMessage(rawMessage: unknown): RequestId | undefined { + if (rawMessage === null || typeof rawMessage !== 'object') { + return undefined; + } + + const id = (rawMessage as { id?: unknown }).id; + if (typeof id === 'string') { + return id; + } + if (typeof id === 'number' && Number.isInteger(id)) { + return id; + } + return undefined; +} + /** * Server transport for stdio: this communicates with an MCP client by reading from the current process' `stdin` and writing to `stdout`. * @@ -72,6 +87,19 @@ export class StdioServerTransport implements Transport { this.onmessage?.(message); } catch (error) { this.onerror?.(error as Error); + const id = getRequestIdFromInvalidMessage((error as JSONRPCMessageValidationError).rawMessage); + if (id !== undefined) { + this.send({ + jsonrpc: '2.0', + id, + error: { + code: INVALID_REQUEST, + message: 'Invalid Request' + } + }).catch(sendError => { + this.onerror?.(sendError); + }); + } } } } diff --git a/packages/server/src/server/streamableHttp.ts b/packages/server/src/server/streamableHttp.ts index 6284189dd9..fd936204f5 100644 --- a/packages/server/src/server/streamableHttp.ts +++ b/packages/server/src/server/streamableHttp.ts @@ -10,6 +10,7 @@ import type { AuthInfo, JSONRPCMessage, MessageExtraInfo, RequestId, Transport } from '@modelcontextprotocol/core'; import { DEFAULT_NEGOTIATED_PROTOCOL_VERSION, + INVALID_REQUEST, isInitializeRequest, isJSONRPCErrorResponse, isJSONRPCRequest, @@ -658,7 +659,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { : [JSONRPCMessageSchema.parse(rawMessage)]; } catch (error) { this.onerror?.(error as Error); - return this.createJsonErrorResponse(400, -32_700, 'Parse error: Invalid JSON-RPC message'); + return this.createJsonErrorResponse(400, INVALID_REQUEST, 'Invalid Request: Invalid JSON-RPC message'); } // Check if this is an initialization request diff --git a/packages/server/test/server/stdio.test.ts b/packages/server/test/server/stdio.test.ts index 92671cacd9..dab885ad8e 100644 --- a/packages/server/test/server/stdio.test.ts +++ b/packages/server/test/server/stdio.test.ts @@ -24,6 +24,10 @@ beforeEach(() => { }); }); +function waitForTransportWrites() { + return new Promise(resolve => setTimeout(resolve, 0)); +} + test('should start then close cleanly', async () => { const server = new StdioServerTransport(input, output); server.onerror = error => { @@ -103,6 +107,40 @@ test('should read multiple messages', async () => { expect(readMessages).toEqual(messages); }); +test('should send invalid request error when malformed JSON-RPC request has an id', async () => { + const server = new StdioServerTransport(input, output); + + let receivedError: Error | undefined; + server.onerror = error => { + receivedError = error; + }; + + await server.start(); + input.push(JSON.stringify({ jsonrpc: '2.0', id: 1, method_: 'tools/list' }) + '\n'); + await waitForTransportWrites(); + + expect(receivedError).toBeDefined(); + expect(outputBuffer.readMessage()).toEqual({ + jsonrpc: '2.0', + id: 1, + error: { + code: -32_600, + message: 'Invalid Request' + } + }); +}); + +test('should not respond when malformed JSON-RPC notification has no id', async () => { + const server = new StdioServerTransport(input, output); + server.onerror = () => {}; + + await server.start(); + input.push(JSON.stringify({ jsonrpc: '2.0', method_: 'notifications/initialized' }) + '\n'); + await waitForTransportWrites(); + + expect(outputBuffer.readMessage()).toBeNull(); +}); + test('should close and fire onerror when stdout errors', async () => { const server = new StdioServerTransport(input, output); diff --git a/packages/server/test/server/streamableHttp.test.ts b/packages/server/test/server/streamableHttp.test.ts index 7a23dd56bb..6213ef7fe4 100644 --- a/packages/server/test/server/streamableHttp.test.ts +++ b/packages/server/test/server/streamableHttp.test.ts @@ -333,6 +333,15 @@ describe('Zod v4', () => { expectErrorResponse(errorData, -32_700, /Parse error.*Invalid JSON/); }); + it('should reject invalid JSON-RPC messages as invalid requests', async () => { + const request = createRequest('POST', { jsonrpc: '2.0', id: 1, method_: 'tools/list' } as unknown as JSONRPCMessage); + const response = await transport.handleRequest(request); + + expect(response.status).toBe(400); + const errorData = await response.json(); + expectErrorResponse(errorData, -32_600, /Invalid Request.*Invalid JSON-RPC message/); + }); + it('should accept notifications without session and return 202', async () => { sessionId = await initializeServer();