diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts index 95358619a4b6..92a7902e2723 100644 --- a/src/gateway/mcp-http.test.ts +++ b/src/gateway/mcp-http.test.ts @@ -866,6 +866,81 @@ describe("mcp loopback server", () => { expect(response.status).toBe(415); }); + it("returns JSON-RPC parse errors only for invalid JSON", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const response = await sendRaw({ + port: server.port, + token: runtime?.ownerToken, + headers: { "content-type": "application/json" }, + body: "{", + }); + const payload = (await response.json()) as { + id?: unknown; + error?: { code?: number; message?: string }; + }; + + expect(response.status).toBe(400); + expect(payload.id).toBeNull(); + expect(payload.error).toMatchObject({ + code: -32700, + message: "Parse error", + }); + }); + + it("returns internal errors for valid JSON when gateway tool resolution fails", async () => { + resolveGatewayScopedToolsMock.mockImplementationOnce(() => { + throw new Error("tool resolution exploded"); + }); + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const response = await sendRaw({ + port: server.port, + token: runtime?.ownerToken, + headers: { "content-type": "application/json" }, + body: mcpToolsListBody(42), + }); + const payload = (await response.json()) as { + id?: unknown; + error?: { code?: number; message?: string }; + }; + + expect(response.status).toBe(500); + expect(payload.id).toBe(42); + expect(payload.error).toMatchObject({ + code: -32603, + message: "Internal error", + }); + }); + + it("returns invalid request errors for malformed batch entries without resetting the request", async () => { + server = await startMcpLoopbackServer(0); + const runtime = getActiveMcpLoopbackRuntime(); + const response = await sendRaw({ + port: server.port, + token: runtime?.ownerToken, + headers: { "content-type": "application/json" }, + body: `[null,${mcpToolsListBody(7)}]`, + }); + const payload = (await response.json()) as Array<{ + id?: unknown; + error?: { code?: number; message?: string }; + result?: { tools?: Array<{ name: string }> }; + }>; + + expect(response.status).toBe(200); + expect(payload).toHaveLength(2); + expect(payload[0]).toMatchObject({ + id: null, + error: { + code: -32600, + message: "Invalid Request", + }, + }); + expect(payload[1]?.id).toBe(7); + expect(payload[1]?.result?.tools?.map((tool) => tool.name)).toContain("message"); + }); + it("returns 413 instead of resetting oversized request bodies", async () => { server = await startMcpLoopbackServer(0); const runtime = getActiveMcpLoopbackRuntime(); diff --git a/src/gateway/mcp-http.ts b/src/gateway/mcp-http.ts index 0b08c4d0598a..990bac166a87 100644 --- a/src/gateway/mcp-http.ts +++ b/src/gateway/mcp-http.ts @@ -42,6 +42,50 @@ type McpLoopbackServer = { let activeMcpLoopbackServer: McpLoopbackServer | undefined; let activeMcpLoopbackServerPromise: Promise | null = null; +function createMcpJsonParseError(error: unknown): Error & { code: "mcp_json_parse_error" } { + return Object.assign(new Error("MCP JSON parse error"), { + cause: error, + code: "mcp_json_parse_error" as const, + }); +} + +function isMcpJsonParseError(error: unknown): error is Error & { code: "mcp_json_parse_error" } { + return ( + typeof error === "object" && + error !== null && + (error as { code?: unknown }).code === "mcp_json_parse_error" + ); +} + +function parseMcpJsonBody(body: string): JsonRpcRequest | JsonRpcRequest[] { + try { + return JSON.parse(body) as JsonRpcRequest | JsonRpcRequest[]; + } catch (error) { + throw createMcpJsonParseError(error); + } +} + +function readJsonRpcRequestId(message: unknown) { + if (!isRecord(message)) { + return null; + } + const id = message.id; + return typeof id === "string" || typeof id === "number" || id === null ? id : undefined; +} + +function isJsonRpcRequest(message: unknown): message is JsonRpcRequest { + return isRecord(message) && message.jsonrpc === "2.0" && typeof message.method === "string"; +} + +function jsonRpcInternalError(parsed: JsonRpcRequest | JsonRpcRequest[] | undefined) { + if (Array.isArray(parsed)) { + return parsed.map((message) => + jsonRpcError(readJsonRpcRequestId(message), -32603, "Internal error"), + ); + } + return jsonRpcError(readJsonRpcRequestId(parsed), -32603, "Internal error"); +} + function shouldLogMcpLoopbackTraffic(): boolean { return ( isTruthyEnvValue(process.env.OPENCLAW_CLI_BACKEND_LOG_OUTPUT) || @@ -106,9 +150,10 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ const requestAbort = createRequestAbortSignal(req, res); void (async () => { + let parsed: JsonRpcRequest | JsonRpcRequest[] | undefined; try { const body = await readMcpHttpBody(req); - const parsed: JsonRpcRequest | JsonRpcRequest[] = JSON.parse(body); + parsed = parseMcpJsonBody(body); const cfg = getRuntimeConfig(); const requestContext = resolveMcpRequestContext(req, cfg, auth); const scopedTools = toolCache.resolve({ @@ -128,7 +173,9 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ const messages = Array.isArray(parsed) ? parsed : [parsed]; logMcpLoopbackTraffic("request", { batchSize: messages.length, - methods: messages.map((message) => message.method), + methods: messages.map((message) => + isJsonRpcRequest(message) ? message.method : undefined, + ), sessionKey: requestContext.sessionKey, inboundEventKind: requestContext.inboundEventKind, senderIsOwner: requestContext.senderIsOwner === true, @@ -137,6 +184,10 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ }); const responses: object[] = []; for (const message of messages) { + if (!isJsonRpcRequest(message)) { + responses.push(jsonRpcError(readJsonRpcRequestId(message), -32600, "Invalid Request")); + continue; + } const response = await handleMcpJsonRpc({ message, tools: scopedTools.tools, @@ -186,9 +237,12 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ res.end(JSON.stringify({ error: "payload_too_large" }), () => { req.destroy(); }); - } else { + } else if (isMcpJsonParseError(error)) { res.writeHead(400, { "Content-Type": "application/json" }); res.end(JSON.stringify(jsonRpcError(null, -32700, "Parse error"))); + } else { + res.writeHead(500, { "Content-Type": "application/json" }); + res.end(JSON.stringify(jsonRpcInternalError(parsed))); } } } finally {