From cd5c3fc3b779d803faf4099771c7b2dbaca5f324 Mon Sep 17 00:00:00 2001 From: VectorPeak Date: Tue, 7 Jul 2026 18:26:01 +0800 Subject: [PATCH] fix(mcp): reject non-object tool call arguments (#99180) Co-authored-by: Peter Steinberger --- src/gateway/mcp-http.handlers.ts | 7 +++- src/gateway/mcp-http.test.ts | 69 +++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/gateway/mcp-http.handlers.ts b/src/gateway/mcp-http.handlers.ts index 1b5334f84dd4..f501c360e491 100644 --- a/src/gateway/mcp-http.handlers.ts +++ b/src/gateway/mcp-http.handlers.ts @@ -2,6 +2,7 @@ // Implements initialize, tools/list, tools/call, and notification handling. import crypto from "node:crypto"; import { ContentBlockSchema, type ContentBlock } from "@modelcontextprotocol/sdk/types.js"; +import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { runBeforeToolCallHook, type HookContext } from "../agents/agent-tools.before-tool-call.js"; import { formatToolExecutionErrorMessage, @@ -99,7 +100,11 @@ export async function handleMcpJsonRpc(params: { return jsonRpcResult(id, { tools: params.toolSchema }); case "tools/call": { const toolName = typeof methodParams?.name === "string" ? methodParams.name.trim() : ""; - const toolArgs = (methodParams?.arguments ?? {}) as Record; + const rawToolArgs = methodParams?.arguments; + if (rawToolArgs !== undefined && !isRecord(rawToolArgs)) { + return jsonRpcError(id, -32602, "Invalid params: tools/call arguments must be an object"); + } + const toolArgs = rawToolArgs ?? {}; if (!toolName) { return jsonRpcResult(id, { content: [{ type: "text", text: "Tool not available: unknown" }], diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts index b9a51f0c29ff..4494eb5a7a38 100644 --- a/src/gateway/mcp-http.test.ts +++ b/src/gateway/mcp-http.test.ts @@ -1153,21 +1153,88 @@ describe("mcp loopback server", () => { }); it("executes tools for loopback callers", async () => { - const cronExecute = vi.fn(async () => ({ + const cronExecute = vi.fn(async () => ({ content: [{ type: "text", text: "CRON_EXECUTED" }], })); + const args = { action: "status" }; mockScopedTools([makeMessageTool(), makeCronTool({ execute: cronExecute })]); const { runtime } = await startLoopbackServerForTest(); const payload = await callMainSessionTool({ token: runtime?.ownerToken, name: "cron", + args, }); expect(cronExecute).toHaveBeenCalledTimes(1); + expect(getBeforeToolCallHookInput(0).params).toEqual(args); + expect(cronExecute.mock.calls[0]?.[1]).toEqual(args); expectMcpResultText(payload, "CRON_EXECUTED"); }); + it.each([ + ["null", null], + ["array", []], + ["string", "bad"], + ])("rejects %s tool call arguments before hooks or execution", async (_label, badArguments) => { + const execute = vi.fn(async () => ({ + content: [{ type: "text", text: "EXECUTED" }], + })); + mockScopedTools([makeMessageTool({ execute })]); + const { runtime, port } = await startLoopbackServerForTest(); + + const response = await sendRaw({ + port, + token: runtime.ownerToken, + headers: jsonHeaders(), + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "message", arguments: badArguments }, + }), + }); + + expect(response.status).toBe(200); + expect(await readMcpPayload(response)).toEqual({ + jsonrpc: "2.0", + id: 1, + error: { + code: -32602, + message: "Invalid params: tools/call arguments must be an object", + }, + }); + expect(runBeforeToolCallHookMock).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + }); + + it("keeps omitted tool call arguments as an empty object", async () => { + const execute = vi.fn(async () => ({ + content: [{ type: "text", text: "EXECUTED" }], + })); + mockScopedTools([makeMessageTool({ execute })]); + const { runtime, port } = await startLoopbackServerForTest(); + + const response = await sendRaw({ + port, + token: runtime.ownerToken, + headers: jsonHeaders(), + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "message" }, + }), + }); + + expect(response.status).toBe(200); + expectMcpResultText(await readMcpPayload(response), "EXECUTED"); + expect(runBeforeToolCallHookMock).toHaveBeenCalledTimes(1); + expect(getBeforeToolCallHookInput(0).params).toEqual({}); + expect(execute).toHaveBeenCalledTimes(1); + expect(execute.mock.calls[0]?.[1]).toEqual({}); + }); + it("preserves valid MCP content blocks returned by loopback tools", async () => { const content = [ { type: "text", text: "caption", annotations: { audience: ["user"] } },