From cef77d1aeb49c87dac664a408b023db13bb84604 Mon Sep 17 00:00:00 2001 From: PollyBot13 Date: Mon, 17 Aug 2026 04:48:40 +0200 Subject: [PATCH] fix(codex): reject malformed native tool arguments (#124649) * fix(codex): reject invalid native tool arguments * fix(codex): preserve raw argument preparation Signed-off-by: sallyom --------- Signed-off-by: sallyom Co-authored-by: sallyom --- config/assertion-safety-baseline.txt | 2 +- .../src/app-server/dynamic-tool-build.test.ts | 2 +- .../src/app-server/dynamic-tools.test.ts | 346 +++++++++++++++++- .../codex/src/app-server/dynamic-tools.ts | 89 ++++- ...enclaw-owned-tool-runtime-contract.test.ts | 2 +- .../outcome-fallback-runtime-contract.test.ts | 7 +- .../src/app-server/side-question.test.ts | 12 +- ...s.before-tool-call.integration.e2e.test.ts | 26 ++ .../agent-tools.before-tool-call.wrapper.ts | 17 +- .../agent-tools.execution-validation.ts | 27 ++ .../openclaw-owned-tool-runtime-contract.ts | 2 +- 11 files changed, 494 insertions(+), 38 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index e98a10680521..4aa2490d183c 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -191,7 +191,7 @@ extensions/codex/src/app-server/config-utils.ts 1 extensions/codex/src/app-server/context-engine-projection.ts 4 extensions/codex/src/app-server/dynamic-tool-build.ts 1 extensions/codex/src/app-server/dynamic-tool-response-state.ts 1 -extensions/codex/src/app-server/dynamic-tools.ts 7 +extensions/codex/src/app-server/dynamic-tools.ts 6 extensions/codex/src/app-server/effective-mcp-catalog.ts 1 extensions/codex/src/app-server/elicitation-bridge.ts 1 extensions/codex/src/app-server/event-projector-reasoning.ts 1 diff --git a/extensions/codex/src/app-server/dynamic-tool-build.test.ts b/extensions/codex/src/app-server/dynamic-tool-build.test.ts index 2b05c1e27c33..e332c5400dc0 100644 --- a/extensions/codex/src/app-server/dynamic-tool-build.test.ts +++ b/extensions/codex/src/app-server/dynamic-tool-build.test.ts @@ -137,7 +137,7 @@ function createRuntimeDynamicTool(name: string): RuntimeDynamicToolForTest { parameters: { type: "object", properties: {}, - additionalProperties: false, + additionalProperties: true, }, execute: vi.fn(async () => ({ content: [{ type: "text" as const, text: `${name} done` }], diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts index d96fbb49c8c3..77813088a67e 100644 --- a/extensions/codex/src/app-server/dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/dynamic-tools.test.ts @@ -7,6 +7,7 @@ import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-harness"; import { HEARTBEAT_RESPONSE_TOOL_NAME, embeddedAgentLog, + getPluginToolMeta, wrapToolWithBeforeToolCallHook, } from "openclaw/plugin-sdk/agent-harness-runtime"; import { @@ -72,7 +73,7 @@ function createTool(overrides: Partial): AnyAgentTool { return { name: "tts", description: "Convert text to speech.", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: vi.fn(), ...overrides, } as unknown as AnyAgentTool; @@ -206,12 +207,302 @@ async function handleMessageToolCall( }); } +const STRICT_INSTRUCTION_SCHEMA = { + type: "object", + properties: { instruction: { type: "string" } }, + required: ["instruction"], + additionalProperties: false, +} as const; + +type SchemaToolNamespace = + | null + | typeof CODEX_OPENCLAW_DIRECT_DYNAMIC_TOOL_NAMESPACE + | typeof CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE; + +async function runSchemaToolCall(params: { + arguments: JsonValue; + callId: string; + name?: string; + namespace?: SchemaToolNamespace; + parameters?: AnyAgentTool["parameters"]; + prepareArguments?: AnyAgentTool["prepareArguments"]; +}) { + const name = params.name ?? "strict_tool"; + const namespace = params.namespace ?? null; + const execute = vi.fn(async () => textToolResult("done")); + const tool = createTool({ + name, + parameters: params.parameters ?? STRICT_INSTRUCTION_SCHEMA, + prepareArguments: params.prepareArguments, + execute, + }); + const bridge = createCodexDynamicToolBridge({ + tools: [tool], + signal: new AbortController().signal, + loading: namespace === CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE ? "searchable" : undefined, + directToolNames: + namespace === CODEX_OPENCLAW_DIRECT_DYNAMIC_TOOL_NAMESPACE ? [name] : undefined, + }); + const response = await bridge.handleToolCall({ + threadId: "thread-1", + turnId: "turn-1", + callId: params.callId, + namespace, + tool: name, + arguments: params.arguments, + }); + return { bridge, execute, response, tool }; +} + +function expectSchemaRejection( + response: Awaited>["response"], + execute: ReturnType, + message: string, +) { + expect(execute).not.toHaveBeenCalled(); + expect(response).toMatchObject({ + success: false, + executionStarted: false, + contentItems: [{ type: "inputText", text: expect.stringContaining(message) }], + }); +} + afterEach(() => { resetGlobalHookRunner(); setActivePluginRegistry(createEmptyPluginRegistry()); }); describe("createCodexDynamicToolBridge", () => { + it("rejects invalid deferred-tool arguments before execution", async () => { + const { execute, response } = await runSchemaToolCall({ + arguments: { instruction: 47 }, + callId: "call-deferred-invalid", + namespace: CODEX_OPENCLAW_DYNAMIC_TOOL_NAMESPACE, + }); + + expectSchemaRejection(response, execute, "instruction: must be string"); + }); + + it("rejects the beta.2 session_status wrong-type regression before execution", async () => { + const { execute, response } = await runSchemaToolCall({ + arguments: { sessionKey: 47 }, + callId: "call-session-status", + name: "session_status", + parameters: { + type: "object", + properties: { sessionKey: { type: "string" } }, + additionalProperties: false, + }, + namespace: CODEX_OPENCLAW_DIRECT_DYNAMIC_TOOL_NAMESPACE, + }); + + expectSchemaRejection(response, execute, "sessionKey: must be string"); + }); + + it("bounds high-cardinality validation errors returned to Codex", async () => { + const propertyNames = Array.from({ length: 10 }, (_, index) => `field${index}`); + const invalidArguments = Object.fromEntries(propertyNames.map((name) => [name, 47])); + const { execute, response } = await runSchemaToolCall({ + arguments: invalidArguments, + callId: "call-many-invalid-fields", + name: "bounded_validation_tool", + parameters: { + type: "object", + properties: Object.fromEntries(propertyNames.map((name) => [name, { type: "string" }])), + required: propertyNames, + additionalProperties: false, + }, + }); + + expectSchemaRejection(response, execute, "more violation(s) omitted"); + const contentItem = requireRecord(response.contentItems[0], "validation response"); + expect(contentItem.text).toEqual(expect.any(String)); + expect((contentItem.text as string).length).toBeLessThanOrEqual(800); + }); + + it("bounds aggregated unexpected-property details returned to Codex", async () => { + const invalidArguments = Object.fromEntries( + Array.from({ length: 20 }, (_, index) => [`unexpected_property_${index}`, true]), + ); + const { execute, response } = await runSchemaToolCall({ + arguments: invalidArguments, + callId: "call-many-unexpected-fields", + name: "bounded_additional_properties_tool", + parameters: { + type: "object", + properties: {}, + additionalProperties: false, + }, + }); + + expectSchemaRejection(response, execute, "[detail truncated]"); + const contentItem = requireRecord(response.contentItems[0], "validation response"); + expect(contentItem.text).toEqual(expect.any(String)); + expect((contentItem.text as string).length).toBeLessThanOrEqual(260); + }); + + it("prepares raw null arguments before native Codex schema validation", async () => { + const prepareArguments = vi.fn(function (this: AnyAgentTool, arguments_: unknown) { + return arguments_ === null ? { preparedBy: this.name } : arguments_; + }); + const { execute, response } = await runSchemaToolCall({ + arguments: null, + callId: "call-null-compatibility", + name: "optional_object_tool", + parameters: { + type: "object", + properties: { preparedBy: { type: "string" } }, + additionalProperties: false, + }, + prepareArguments, + namespace: CODEX_OPENCLAW_DIRECT_DYNAMIC_TOOL_NAMESPACE, + }); + + expect(prepareArguments).toHaveBeenCalledWith(null); + expect(response).toEqual(expectInputText("done")); + expectExecuteCall(execute, { + callId: "call-null-compatibility", + args: { preparedBy: "optional_object_tool" }, + }); + }); + + it.each([ + { + label: "repairs primitive input", + initial: 47, + adjusted: { instruction: "repaired" }, + executes: true, + }, + { + label: "repairs invalid input", + initial: { instruction: 47 }, + adjusted: { instruction: "repaired" }, + executes: true, + }, + { + label: "rejects invalid rewritten input", + initial: { instruction: "inspect" }, + adjusted: { instruction: 47 }, + executes: false, + }, + ])("validates final arguments after a hook $label", async (testCase) => { + const beforeToolCall = vi.fn(async () => ({ params: testCase.adjusted })); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "before_tool_call", handler: beforeToolCall }]), + ); + const callId = `call-hook-schema-${testCase.executes}`; + const { execute, response } = await runSchemaToolCall({ + arguments: testCase.initial, + callId, + name: "hook_schema_tool", + }); + + if (testCase.executes) { + expect(response).toEqual(expectInputText("done")); + expectExecuteCall(execute, { + callId, + args: { instruction: "repaired" }, + }); + } else { + expectSchemaRejection(response, execute, "instruction: must be string"); + } + }); + + it("leaves external MCP tool argument validation to the MCP bridge", async () => { + const tool = createOwnerBackedContractTool({ + pluginId: "mcp-bridge", + name: "external_mcp_tool", + result: textToolResult("mcp handled"), + }); + tool.parameters = { + type: "object", + properties: { instruction: { type: "string" } }, + required: ["instruction"], + additionalProperties: false, + }; + const meta = getPluginToolMeta(tool); + expect(meta).toBeDefined(); + Object.assign(meta ?? {}, { + mcp: { + serverName: "external", + safeServerName: "external", + toolName: tool.name, + operation: "tool", + }, + }); + const bridge = createCodexDynamicToolBridge({ + tools: [tool], + signal: new AbortController().signal, + }); + + const response = await bridge.handleToolCall({ + threadId: "thread-1", + turnId: "turn-1", + callId: "call-external-mcp", + namespace: null, + tool: tool.name, + arguments: { instruction: 47 }, + }); + + expect(response).toEqual(expectInputText("mcp handled")); + }); + + it("scopes normalized input validation to the projected Codex wrapper", async () => { + const execute = vi.fn(async () => textToolResult("done")); + const source = wrapToolWithBeforeToolCallHook( + createTool({ + name: "provider_scoped_tool", + parameters: { + type: "object", + properties: { instruction: { type: "string" } }, + required: ["instruction"], + additionalProperties: false, + }, + execute, + }), + ); + const bridge = createCodexDynamicToolBridge({ + tools: [source], + signal: new AbortController().signal, + }); + + const response = await bridge.handleToolCall({ + threadId: "thread-1", + turnId: "turn-1", + callId: "call-provider-scoped-codex", + namespace: null, + tool: source.name, + arguments: { instruction: 47 }, + }); + + expect(response.success).toBe(false); + expect(execute).not.toHaveBeenCalled(); + + await source.execute?.("call-provider-scoped-source", { instruction: 47 }); + + expect(execute).toHaveBeenCalledOnce(); + }); + + it("isolates cached validators for same-name tools with different schemas", async () => { + const call = (instructionType: "string" | "number", callId: string) => + runSchemaToolCall({ + arguments: { instruction: "inspect" }, + callId, + name: "schema_cache_tool", + parameters: { + type: "object", + properties: { instruction: { type: instructionType } }, + required: ["instruction"], + additionalProperties: false, + }, + }); + const stringCase = await call("string", "call-cache-string"); + const numberCase = await call("number", "call-cache-number"); + + expect(stringCase.response).toEqual(expectInputText("done")); + expectSchemaRejection(numberCase.response, numberCase.execute, "instruction: must be number"); + }); + it("surfaces a rejected owner-backed memory write before a false final claim", async () => { const tool = createOwnerBackedContractTool({ pluginId: "memory-lancedb", @@ -911,29 +1202,44 @@ describe("createCodexDynamicToolBridge", () => { }); }); - it("repairs a null dynamic-tool schema type before Codex registration", () => { - const bridge = createCodexDynamicToolBridge({ - tools: [ - createTool({ - name: "codex_app__automation_update", - parameters: { - type: null, - properties: { - action: { type: "string", description: null }, - }, - } as never, - }), - ], - signal: new AbortController().signal, + it("validates execution against the repaired schema published to Codex", async () => { + const { bridge, execute, response } = await runSchemaToolCall({ + arguments: { action: "inspect" }, + callId: "call-repaired-schema", + name: "codex_app__automation_update", + parameters: { + type: null, + properties: { action: { type: "string", description: null } }, + } as never, }); expect(flattenSpecsWithNamespace(bridge.specs)[0]?.inputSchema).toEqual({ type: "object", - properties: { - action: { type: "string" }, - }, + properties: { action: { type: "string" } }, }); expect(bridge.telemetry.quarantinedTools).toEqual([]); + expect(response).toEqual(expectInputText("done")); + expectExecuteCall(execute, { + callId: "call-repaired-schema", + args: { action: "inspect" }, + }); + }); + + it("enforces the strict empty-object schema published to Codex", async () => { + const { bridge, execute, response } = await runSchemaToolCall({ + arguments: { unexpected: true }, + callId: "call-strict-empty-schema", + name: "strict_empty_tool", + parameters: {}, + }); + + expect(flattenSpecsWithNamespace(bridge.specs)[0]?.inputSchema).toEqual({ + type: "object", + properties: {}, + required: [], + additionalProperties: false, + }); + expectSchemaRejection(response, execute, 'must not have additional properties: "unexpected"'); }); it("quarantines dynamic tools with unsupported input schemas", async () => { @@ -4403,7 +4709,7 @@ describe("createCodexDynamicToolBridge", () => { callId: "call-terminal-middleware", namespace: null, tool: "web_fetch", - arguments: { url: "https://private.example" }, + arguments: {}, }); expect(onToolOutcome).toHaveBeenLastCalledWith( @@ -4459,7 +4765,7 @@ describe("createCodexDynamicToolBridge", () => { callId: "call-terminal-middleware-error", namespace: null, tool: "web_fetch", - arguments: { url: "https://private.example" }, + arguments: {}, }); expect(onToolOutcome).toHaveBeenLastCalledWith( diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts index d4521295ccfe..5d2c8f1439f8 100644 --- a/extensions/codex/src/app-server/dynamic-tools.ts +++ b/extensions/codex/src/app-server/dynamic-tools.ts @@ -44,6 +44,10 @@ import { } from "openclaw/plugin-sdk/agent-harness-runtime"; import { emitTrustedDiagnosticEvent } from "openclaw/plugin-sdk/diagnostic-runtime"; import { expectDefined } from "openclaw/plugin-sdk/expect-runtime"; +import { + type JsonSchemaObject, + validateJsonSchemaValue, +} from "openclaw/plugin-sdk/json-schema-runtime"; import type { ImageContent, TextContent } from "openclaw/plugin-sdk/llm"; import { normalizeOpenAIToolSchemas } from "openclaw/plugin-sdk/provider-tools"; import { @@ -105,7 +109,7 @@ type ProjectedCodexDynamicTool = { tool: AnyAgentTool; name: string; description: string; - inputSchema: JsonValue; + inputSchema: JsonSchemaObject & JsonValue; }; type CodexDynamicToolSchemaQuarantine = { @@ -113,6 +117,57 @@ type CodexDynamicToolSchemaQuarantine = { violations: readonly string[]; }; +const INTERNAL_TOOL_EXECUTION_VALIDATION = Symbol.for("openclaw.internalToolExecutionValidation"); +const MAX_CODEX_DYNAMIC_TOOL_VALIDATION_ERRORS = 4; +const MAX_CODEX_DYNAMIC_TOOL_VALIDATION_ERROR_CHARS = 160; +const CODEX_DYNAMIC_TOOL_VALIDATION_TRUNCATED_SUFFIX = " [detail truncated]"; + +function shouldValidateCodexDynamicToolInput(tool: AnyAgentTool): boolean { + return getPluginToolMeta(tool)?.mcp?.operation !== "tool"; +} + +function assertCodexDynamicToolInputMatchesSchema(params: { + toolName: string; + schema: JsonSchemaObject; + value: unknown; +}): void { + const validation = validateJsonSchemaValue({ + schema: params.schema, + cacheKey: `codex-dynamic-tool-input:${params.toolName}:${JSON.stringify(params.schema)}`, + value: params.value, + }); + if (validation.ok) { + return; + } + const visibleErrors = validation.errors.slice(0, MAX_CODEX_DYNAMIC_TOOL_VALIDATION_ERRORS); + const details = visibleErrors + .map((error) => { + if (error.text.length <= MAX_CODEX_DYNAMIC_TOOL_VALIDATION_ERROR_CHARS) { + return error.text; + } + return `${error.text.slice( + 0, + MAX_CODEX_DYNAMIC_TOOL_VALIDATION_ERROR_CHARS - + CODEX_DYNAMIC_TOOL_VALIDATION_TRUNCATED_SUFFIX.length, + )}${CODEX_DYNAMIC_TOOL_VALIDATION_TRUNCATED_SUFFIX}`; + }) + .join("; "); + const omitted = validation.errors.length - visibleErrors.length; + const omittedSuffix = omitted > 0 ? `; ${omitted} more violation(s) omitted` : ""; + throw new Error(`Invalid arguments for tool "${params.toolName}": ${details}${omittedSuffix}.`); +} + +function createCodexDynamicToolValidationControl(params: { + toolCallId: string; + validate: (value: unknown) => void; +}): Record { + return { + [INTERNAL_TOOL_EXECUTION_VALIDATION]: true, + toolCallId: params.toolCallId, + validate: params.validate, + }; +} + function applyCurrentMessageProvider( toolName: string, args: Record, @@ -589,7 +644,8 @@ export function createCodexDynamicToolBridge(params: { }); } const { tool, name: toolName } = toolEntry; - const args = asNonArrayRecord(call.arguments); + const rawArguments = call.arguments; + const args = asNonArrayRecord(rawArguments); const startedAt = Date.now(); const signal = composeAbortSignals(params.signal, options?.signal); let didStartExecution = false; @@ -623,7 +679,9 @@ export function createCodexDynamicToolBridge(params: { } }; try { - const toolArgs = tool.prepareArguments ? tool.prepareArguments(args) : args; + // Compatibility preparation owns raw arguments; record coercion must not run first. + const prepare = tool.prepareArguments; + const toolArgs = prepare ? Reflect.apply(prepare, tool, [rawArguments]) : args; const preparedArgs = toolName === "message" && isRecord(toolArgs) ? await prepareCodexRemoteWorkspaceMessageMedia({ @@ -648,7 +706,21 @@ export function createCodexDynamicToolBridge(params: { : undefined, }; didDispatchExecution = true; - const rawResult = await tool.execute(call.callId, preparedArgs, signal); + const executionArgs: unknown[] = [call.callId, preparedArgs, signal]; + if (shouldValidateCodexDynamicToolInput(tool)) { + executionArgs.push( + createCodexDynamicToolValidationControl({ + toolCallId: call.callId, + validate: (value) => + assertCodexDynamicToolInputMatchesSchema({ + toolName, + schema: toolEntry.inputSchema, + value, + }), + }), + ); + } + const rawResult = await Reflect.apply(tool.execute, tool, executionArgs); captureExecutionBoundary(); const telemetryRawResult = sanitizeToolResult(rawResult); const rawIsError = isToolResultError(rawResult); @@ -1119,11 +1191,18 @@ function projectCodexDynamicTools(tools: readonly AnyAgentTool[]): { quarantinedTools.push({ tool: descriptor.name, violations: projection.violations }); continue; } + if (!isRecord(projection.schema)) { + quarantinedTools.push({ + tool: descriptor.name, + violations: [`${descriptor.name}.inputSchema must be a JSON object schema`], + }); + continue; + } projectedTools.push({ tool, name: descriptor.name, description: descriptor.description, - inputSchema: projection.schema as JsonValue, + inputSchema: projection.schema, }); } return { tools: projectedTools, quarantinedTools }; diff --git a/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts b/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts index 4696f209c483..d643ea21d2f5 100644 --- a/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts +++ b/extensions/codex/src/app-server/openclaw-owned-tool-runtime-contract.test.ts @@ -16,7 +16,7 @@ function createContractTool(overrides: Partial): AnyAgentTool { return { name: "exec", description: "Run a command.", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: vi.fn(), ...overrides, } as unknown as AnyAgentTool; diff --git a/extensions/codex/src/app-server/outcome-fallback-runtime-contract.test.ts b/extensions/codex/src/app-server/outcome-fallback-runtime-contract.test.ts index e2e222254a0a..67ed2fb6f2cb 100644 --- a/extensions/codex/src/app-server/outcome-fallback-runtime-contract.test.ts +++ b/extensions/codex/src/app-server/outcome-fallback-runtime-contract.test.ts @@ -420,7 +420,12 @@ describe("Outcome/fallback runtime contract - Codex app-server adapter", () => { { name: "cron", description: "Cron", - parameters: { type: "object", properties: {} }, + parameters: { + type: "object", + properties: { action: { type: "string" } }, + required: ["action"], + additionalProperties: false, + }, execute: vi.fn(async () => toolResult), } as never, ], diff --git a/extensions/codex/src/app-server/side-question.test.ts b/extensions/codex/src/app-server/side-question.test.ts index d15a833aae1a..dbb75ebb4567 100644 --- a/extensions/codex/src/app-server/side-question.test.ts +++ b/extensions/codex/src/app-server/side-question.test.ts @@ -445,7 +445,7 @@ async function runSideQuestionWithManagedWebSearchCall( { name: "web_search", description: "Search the web", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: toolExecuteMock, }, ]); @@ -516,13 +516,13 @@ describe("runCodexAppServerSideQuestion", () => { { name: "wiki_status", description: "Check wiki status", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: toolExecuteMock, }, { name: "web_search", description: "Search the web", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: toolExecuteMock, }, ]); @@ -1160,7 +1160,7 @@ describe("runCodexAppServerSideQuestion", () => { { name: "web_search", description: "Search the web", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: toolExecuteMock, }, ], @@ -1236,7 +1236,7 @@ describe("runCodexAppServerSideQuestion", () => { { name: "web_search", description: "Search the web", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: toolExecuteMock, }, ] @@ -2404,7 +2404,7 @@ describe("runCodexAppServerSideQuestion", () => { { name: "computer", description: "Control a desktop", - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: computerExecute, }, ]); diff --git a/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts b/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts index 04d944c0bc1e..2c1c76d29093 100644 --- a/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts +++ b/src/agents/agent-tools.before-tool-call.integration.e2e.test.ts @@ -218,6 +218,32 @@ describe("before_tool_call hook integration", () => { expect(consumeTrackedToolExecutionStarted("call-1")).toBeUndefined(); }); + it("consumes private execution validation through the standard update slot", async () => { + beforeToolCallHook = installBeforeToolCallHook({ enabled: false }); + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + const tool = wrapToolWithBeforeToolCallHook(asAgentTool({ name: "Read", execute })); + const validate = vi.fn(() => { + throw new Error("invalid projected arguments"); + }); + const validationControl = { + [Symbol.for("openclaw.internalToolExecutionValidation")]: true, + toolCallId: "call-private-validation", + validate, + }; + + await expect( + Reflect.apply(tool.execute, tool, [ + "call-private-validation", + { path: 47 }, + undefined, + validationControl, + ]), + ).rejects.toThrow("invalid projected arguments"); + + expect(validate).toHaveBeenCalledWith({ path: 47 }); + expect(execute).not.toHaveBeenCalled(); + }); + it("records structured replay trust only for concrete core-owned tools", async () => { beforeToolCallHook = installBeforeToolCallHook({ enabled: false }); const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); diff --git a/src/agents/agent-tools.before-tool-call.wrapper.ts b/src/agents/agent-tools.before-tool-call.wrapper.ts index 8b21d1d094e7..bca32e26ca76 100644 --- a/src/agents/agent-tools.before-tool-call.wrapper.ts +++ b/src/agents/agent-tools.before-tool-call.wrapper.ts @@ -57,7 +57,10 @@ import { createInternalExecutionPreparer, readInternalExecutionControl, } from "./agent-tools.execution-preparer.js"; -import { validateToolExecutionParams } from "./agent-tools.execution-validation.js"; +import { + readInternalToolExecutionValidation, + validateToolExecutionParams, +} from "./agent-tools.execution-validation.js"; import { BEFORE_TOOL_CALL_DIAGNOSTIC_OPTIONS, BEFORE_TOOL_CALL_HOOK_CONTEXT, @@ -304,6 +307,13 @@ export function wrapToolWithBeforeToolCallHook( if (prepareControl) { executionArgs.pop(); } + const onUpdateValidation = readInternalToolExecutionValidation(onUpdate); + const internalValidation = + onUpdateValidation ?? readInternalToolExecutionValidation(executionArgs.at(-1)); + const forwardedOnUpdate = onUpdateValidation ? undefined : onUpdate; + if (!onUpdateValidation && internalValidation) { + executionArgs.pop(); + } const toolCallOrdinal = ctx?.allocateToolOutcomeOrdinal?.(toolCallId); const preExecutionStartedAt = Date.now(); const normalizedToolName = normalizeToolPolicyName(toolName || "tool"); @@ -466,6 +476,9 @@ export function wrapToolWithBeforeToolCallHook( // Hooks can repair or rewrite arguments; only the final execution // shape is safe to validate, after vetoes but before side effects. await validateToolExecutionParams(toolCallId, executeParams); + if (internalValidation?.toolCallId === toolCallId) { + await internalValidation.validate(executeParams); + } await reconcileLoopCallExecutionParams({ ctx, toolName: normalizedToolName, @@ -519,7 +532,7 @@ export function wrapToolWithBeforeToolCallHook( toolCallId, executeParams, signal, - onUpdate, + forwardedOnUpdate, ...executionArgs, ); } catch (error) { diff --git a/src/agents/agent-tools.execution-validation.ts b/src/agents/agent-tools.execution-validation.ts index 0866c5e670b4..d03c0a8bfe3f 100644 --- a/src/agents/agent-tools.execution-validation.ts +++ b/src/agents/agent-tools.execution-validation.ts @@ -7,6 +7,12 @@ type ScopedToolExecutionValidator = { }; const executionValidators = new AsyncLocalStorage(); +const INTERNAL_TOOL_EXECUTION_VALIDATION = Symbol.for("openclaw.internalToolExecutionValidation"); + +type InternalToolExecutionValidation = { + toolCallId: string; + validate: ToolExecutionValidator; +}; /** Keep per-call validation inside the policy wrapper's final execution boundary. */ export async function runWithToolExecutionValidation( @@ -27,3 +33,24 @@ export async function validateToolExecutionParams( await scopedValidator.validate(params); } } + +/** Read the private validation control carried by one native harness call. */ +export function readInternalToolExecutionValidation( + value: unknown, +): InternalToolExecutionValidation | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + const marker = Reflect.get(value, INTERNAL_TOOL_EXECUTION_VALIDATION); + const toolCallId = Reflect.get(value, "toolCallId"); + const validate = Reflect.get(value, "validate"); + if (marker !== true || typeof toolCallId !== "string" || typeof validate !== "function") { + return undefined; + } + return { + toolCallId, + validate: async (params) => { + await Reflect.apply(validate, undefined, [params]); + }, + }; +} diff --git a/src/plugin-sdk/test-helpers/agents/openclaw-owned-tool-runtime-contract.ts b/src/plugin-sdk/test-helpers/agents/openclaw-owned-tool-runtime-contract.ts index f6580cb11ad0..adb234d5189e 100644 --- a/src/plugin-sdk/test-helpers/agents/openclaw-owned-tool-runtime-contract.ts +++ b/src/plugin-sdk/test-helpers/agents/openclaw-owned-tool-runtime-contract.ts @@ -73,7 +73,7 @@ export function createOwnerBackedContractTool(params: { name: params.name, label: `${params.name} owner contract tool`, description: `${params.name} owner contract tool`, - parameters: { type: "object", properties: {} }, + parameters: { type: "object", properties: {}, additionalProperties: true }, execute: vi.fn(async () => params.result), } as AnyAgentTool; setPluginToolMeta(tool, {