From 767fd9f222fda451a28deed3a6dbfd8a32df715e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 16:52:47 +0100 Subject: [PATCH] fix: classify /tools/invoke errors and sanitize 500s (#13185) (thanks @davidrudduck) --- CHANGELOG.md | 1 + docs/gateway/tools-invoke-http-api.md | 3 +- src/agents/tools/common.ts | 25 ++++--- src/gateway/tools-invoke-http.test.ts | 100 ++++++++++++++++++++++++++ src/gateway/tools-invoke-http.ts | 30 ++++++++ 5 files changed, 150 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c5e466ac..57e956c4df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai - Security/Canvas: serve A2UI assets via the shared safe-open path (`openFileWithinRoot`) to close traversal/TOCTOU gaps, with traversal and symlink regression coverage. (#10525) Thanks @abdelsfane. - Security/WhatsApp: enforce `0o600` on `creds.json` and `creds.json.bak` on save/backup/restore paths to reduce credential file exposure. (#10529) Thanks @abdelsfane. - Security/Gateway + ACP: block high-risk tools (`sessions_spawn`, `sessions_send`, `gateway`, `whatsapp_login`) from HTTP `/tools/invoke` by default with `gateway.tools.{allow,deny}` overrides, and harden ACP permission selection to fail closed when tool identity/options are ambiguous while supporting `allow_always`/`reject_always`. (#15390) Thanks @aether-ai-agent. +- Gateway/Tools Invoke: sanitize `/tools/invoke` execution failures while preserving `400` for tool input errors and returning `500` for unexpected runtime failures, with regression coverage and docs updates. (#13185) Thanks @davidrudduck. - MS Teams: preserve parsed mention entities/text when appending OneDrive fallback file links, and accept broader real-world Teams mention ID formats (`29:...`, `8:orgid:...`) while still rejecting placeholder patterns. (#15436) Thanks @hyojin. - Security/Audit: distinguish external webhooks (`hooks.enabled`) from internal hooks (`hooks.internal.enabled`) in attack-surface summaries to avoid false exposure signals when only internal hooks are enabled. (#13474) Thanks @mcaxtr. - Security/Onboarding: clarify multi-user DM isolation remediation with explicit `openclaw config set session.dmScope ...` commands in security audit, doctor security, and channel onboarding guidance. (#13129) Thanks @VintLin. diff --git a/docs/gateway/tools-invoke-http-api.md b/docs/gateway/tools-invoke-http-api.md index c44ad5b466..ad246e08b4 100644 --- a/docs/gateway/tools-invoke-http-api.md +++ b/docs/gateway/tools-invoke-http-api.md @@ -89,11 +89,12 @@ To help group policies resolve context, you can optionally set: ## Responses - `200` → `{ ok: true, result }` -- `400` → `{ ok: false, error: { type, message } }` (invalid request or tool error) +- `400` → `{ ok: false, error: { type, message } }` (invalid request or tool input error) - `401` → unauthorized - `429` → auth rate-limited (`Retry-After` set) - `404` → tool not available (not found or not allowlisted) - `405` → method not allowed +- `500` → `{ ok: false, error: { type, message } }` (unexpected tool execution error; sanitized message) ## Example diff --git a/src/agents/tools/common.ts b/src/agents/tools/common.ts index 94993fd4a1..5921ecb16d 100644 --- a/src/agents/tools/common.ts +++ b/src/agents/tools/common.ts @@ -18,6 +18,15 @@ export type ActionGate> = ( defaultValue?: boolean, ) => boolean; +export class ToolInputError extends Error { + readonly status = 400; + + constructor(message: string) { + super(message); + this.name = "ToolInputError"; + } +} + export function createActionGate>( actions: T | undefined, ): ActionGate { @@ -49,14 +58,14 @@ export function readStringParam( const raw = params[key]; if (typeof raw !== "string") { if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } const value = trim ? raw.trim() : raw; if (!value && !allowEmpty) { if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } @@ -80,7 +89,7 @@ export function readStringOrNumberParam( } } if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } @@ -106,7 +115,7 @@ export function readNumberParam( } if (value === undefined) { if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } @@ -137,7 +146,7 @@ export function readStringArrayParam( .filter(Boolean); if (values.length === 0) { if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } @@ -147,14 +156,14 @@ export function readStringArrayParam( const value = raw.trim(); if (!value) { if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } return [value]; } if (required) { - throw new Error(`${label} required`); + throw new ToolInputError(`${label} required`); } return undefined; } @@ -181,7 +190,7 @@ export function readReactionParams( allowEmpty: true, }); if (remove && !emoji) { - throw new Error(options.removeErrorMessage); + throw new ToolInputError(options.removeErrorMessage); } return { emoji, remove, isEmpty: !emoji }; } diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index b7d2cbd37c..98f047e4a1 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -1,5 +1,6 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { ToolInputError } from "../agents/tools/common.js"; import { createTestRegistry } from "../test-utils/channel-plugins.js"; import { resetTestPluginRegistry, setTestPluginRegistry, testState } from "./test-helpers.mocks.js"; import { installGatewayTestHooks, getFreePort, startGatewayServer } from "./test-helpers.server.js"; @@ -50,6 +51,31 @@ const invokeAgentsList = async (params: { }); }; +const invokeTool = async (params: { + port: number; + tool: string; + args?: Record; + action?: string; + headers?: Record; + sessionKey?: string; +}) => { + const body: Record = { + tool: params.tool, + args: params.args ?? {}, + }; + if (params.action) { + body.action = params.action; + } + if (params.sessionKey) { + body.sessionKey = params.sessionKey; + } + return await fetch(`http://127.0.0.1:${params.port}/tools/invoke`, { + method: "POST", + headers: { "content-type": "application/json", ...params.headers }, + body: JSON.stringify(body), + }); +}; + describe("POST /tools/invoke", () => { let sharedPort = 0; let sharedServer: Awaited>; @@ -330,4 +356,78 @@ describe("POST /tools/invoke", () => { }); expect(resMain.status).toBe(200); }); + + it("maps tool input errors to 400 and unexpected execution errors to 500", async () => { + const registry = createTestRegistry(); + registry.tools.push({ + pluginId: "tools-invoke-test", + source: "test", + names: ["tools_invoke_test"], + optional: false, + factory: () => ({ + label: "Tools Invoke Test", + name: "tools_invoke_test", + description: "Test-only tool.", + parameters: { + type: "object", + properties: { + mode: { type: "string" }, + }, + required: ["mode"], + additionalProperties: false, + }, + execute: async (_toolCallId, args) => { + const mode = (args as { mode?: unknown }).mode; + if (mode === "input") { + throw new ToolInputError("mode invalid"); + } + if (mode === "crash") { + throw new Error("boom"); + } + return { ok: true }; + }, + }), + }); + setTestPluginRegistry(registry); + const { writeConfigFile } = await import("../config/config.js"); + await writeConfigFile({ + plugins: { enabled: true }, + // oxlint-disable-next-line typescript/no-explicit-any + } as any); + + const token = resolveGatewayToken(); + + try { + const inputRes = await invokeTool({ + port: sharedPort, + tool: "tools_invoke_test", + args: { mode: "input" }, + headers: { authorization: `Bearer ${token}` }, + sessionKey: "main", + }); + expect(inputRes.status).toBe(400); + const inputBody = await inputRes.json(); + expect(inputBody.ok).toBe(false); + expect(inputBody.error?.type).toBe("tool_error"); + expect(inputBody.error?.message).toBe("mode invalid"); + + const crashRes = await invokeTool({ + port: sharedPort, + tool: "tools_invoke_test", + args: { mode: "crash" }, + headers: { authorization: `Bearer ${token}` }, + sessionKey: "main", + }); + expect(crashRes.status).toBe(500); + const crashBody = await crashRes.json(); + expect(crashBody.ok).toBe(false); + expect(crashBody.error?.type).toBe("tool_error"); + expect(crashBody.error?.message).toBe("tool execution failed"); + } finally { + await writeConfigFile({ + // oxlint-disable-next-line typescript/no-explicit-any + } as any); + resetTestPluginRegistry(); + } + }); }); diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index 5256e3ff96..b6ecac7d4e 100644 --- a/src/gateway/tools-invoke-http.ts +++ b/src/gateway/tools-invoke-http.ts @@ -15,6 +15,7 @@ import { resolveToolProfilePolicy, stripPluginOnlyAllowlist, } from "../agents/tool-policy.js"; +import { ToolInputError } from "../agents/tools/common.js"; import { loadConfig } from "../config/config.js"; import { resolveMainSessionKey } from "../config/sessions.js"; import { logWarn } from "../logger.js"; @@ -116,6 +117,28 @@ function mergeActionIntoArgsIfSupported(params: { return { ...args, action }; } +function getErrorMessage(err: unknown): string { + if (err instanceof Error) { + return err.message || String(err); + } + if (typeof err === "string") { + return err; + } + return String(err); +} + +function isToolInputError(err: unknown): boolean { + if (err instanceof ToolInputError) { + return true; + } + return ( + typeof err === "object" && + err !== null && + "name" in err && + (err as { name?: unknown }).name === "ToolInputError" + ); +} + export async function handleToolsInvokeHttpRequest( req: IncomingMessage, res: ServerResponse, @@ -348,6 +371,13 @@ export async function handleToolsInvokeHttpRequest( const result = await (tool as any).execute?.(`http-${Date.now()}`, toolArgs); sendJson(res, 200, { ok: true, result }); } catch (err) { + if (isToolInputError(err)) { + sendJson(res, 400, { + ok: false, + error: { type: "tool_error", message: getErrorMessage(err) || "invalid tool arguments" }, + }); + return true; + } logWarn(`tools-invoke: tool execution failed: ${String(err)}`); sendJson(res, 500, { ok: false,