diff --git a/docs/tools/code-mode.md b/docs/tools/code-mode.md index 57f0ec660351..5293231887d4 100644 --- a/docs/tools/code-mode.md +++ b/docs/tools/code-mode.md @@ -418,7 +418,8 @@ Rules: - `code` is the documented model-facing field. - `command` is accepted as an exec-compatible alias for hook policies and trusted rewrites (the normal OpenClaw shell exec tool also uses a `command` - field); when both are present, the values must match. + field). Blank aliases are treated as absent; when both aliases are non-empty, + their values must match. - `language` defaults to `"javascript"`; the schema exposes it as a flat string enum (`"javascript" | "typescript"`), not a `oneOf`/`anyOf` union, since some providers reject those shapes. 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 2c1c76d29093..54070bd0e540 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 @@ -961,6 +961,40 @@ describe("before_tool_call hook deduplication (#15502)", () => { }, ); + beforeToolCallHook.mockClear(); + const blankCodeAliasResult = await def.execute( + "call-code-mode-exec-blank-code", + { code: "", command: "return 3;" }, + undefined, + undefined, + extensionContext, + ); + + expect(blankCodeAliasResult.details).toMatchObject({ + status: "blocked", + reason: "blocked before code-mode execution", + }); + expect(beforeToolCallHook).toHaveBeenCalledWith( + { + toolName: "exec", + params: { code: "return 3;", command: "return 3;" }, + toolKind: "code_mode_exec", + toolInputKind: "javascript", + runId: "run-main", + toolCallId: "call-code-mode-exec-blank-code", + }, + { + toolName: "exec", + toolKind: "code_mode_exec", + toolInputKind: "javascript", + agentId: "main", + sessionKey: "agent:main:main", + sessionId: "session-main", + runId: "run-main", + toolCallId: "call-code-mode-exec-blank-code", + }, + ); + beforeToolCallHook.mockClear(); const typescriptResult = await def.execute( "call-code-mode-exec-typescript", @@ -1276,6 +1310,49 @@ describe("before_tool_call hook deduplication (#15502)", () => { }); }); + it("fails closed when a hook blanks one code-mode exec alias", async () => { + // A blank alias from the caller is treated as absent, but a hook that + // deliberately blanks `code` is a policy decision: mirror it so neither + // alias survives, rather than silently running the original command. + beforeToolCallHook = installBeforeToolCallHook({ + runBeforeToolCallImpl: async () => ({ params: { code: "" } }), + }); + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + const tool = markCodeModeControlTool( + asAgentTool({ + name: CODE_MODE_EXEC_TOOL_NAME, + execute, + description: "exec", + parameters: {}, + }), + ); + const [def] = toToolDefinitions([tool], { + agentId: "main", + sessionKey: "agent:main:main", + sessionId: "session-main", + runId: "run-main", + }); + if (!def) { + throw new Error("missing custom tool definition"); + } + const extensionContext = {} as Parameters[4]; + + await def.execute( + "call-code-mode-exec-blank-rewrite", + { code: "", command: "return 1;" }, + undefined, + undefined, + extensionContext, + ); + + expect(execute).toHaveBeenCalledWith( + "call-code-mode-exec-blank-rewrite", + { code: "", command: "" }, + undefined, + undefined, + ); + }); + it("renormalizes trusted policy rewrites before code-mode exec hooks observe params", async () => { resetGlobalHookRunner(); const normalHook = vi.fn(async () => undefined); @@ -1308,6 +1385,9 @@ describe("before_tool_call hook deduplication (#15502)", () => { }, }; } + if (eventValue.toolCallId === "call-code-mode-trusted-blank") { + return { params: { code: "", command: "return 4;" } }; + } return undefined; }, }, @@ -1360,6 +1440,13 @@ describe("before_tool_call hook deduplication (#15502)", () => { undefined, extensionContext, ); + await def.execute( + "call-code-mode-trusted-blank", + { code: "return 4;", command: "return 4;" }, + undefined, + undefined, + extensionContext, + ); expect(normalHook).toHaveBeenNthCalledWith( 1, @@ -1471,6 +1558,23 @@ describe("before_tool_call hook deduplication (#15502)", () => { undefined, undefined, ); + expect(normalHook).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ params: { code: "", command: "" } }), + expect.anything(), + ); + expect(trustedObserver).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ params: { code: "", command: "" } }), + expect.anything(), + ); + expect(execute).toHaveBeenNthCalledWith( + 3, + "call-code-mode-trusted-blank", + { code: "", command: "" }, + undefined, + undefined, + ); expect( consumeAdjustedParamsForToolCall("call-code-mode-trusted-command", "run-main"), ).toEqual({ command: "return 2;", code: "return 2;" }); @@ -1481,6 +1585,10 @@ describe("before_tool_call hook deduplication (#15502)", () => { command: "const value: number = 3;", language: "typescript", }); + expect(consumeAdjustedParamsForToolCall("call-code-mode-trusted-blank", "run-main")).toEqual({ + code: "", + command: "", + }); } finally { setActivePluginRegistry(createEmptyPluginRegistry()); resetGlobalHookRunner(); diff --git a/src/agents/agent-tools.before-tool-call.policy.ts b/src/agents/agent-tools.before-tool-call.policy.ts index c8a63d068ecc..5e2a8c29dfce 100644 --- a/src/agents/agent-tools.before-tool-call.policy.ts +++ b/src/agents/agent-tools.before-tool-call.policy.ts @@ -48,7 +48,7 @@ import type { } from "./agent-tools.before-tool-call.types.js"; import { getCodeModeExecBeforeHookMetadataForToolKind, - normalizeCodeModeExecBeforeHookParamsForToolKind, + reconcileCodeModeExecBeforeHookParams, } from "./code-mode-control-tools.js"; import { admitSingleToolCallLoop } from "./tool-loop-admission.js"; import { normalizeToolPolicyName } from "./tool-policy.js"; @@ -206,6 +206,9 @@ export async function runBeforeToolCallHook(args: { ...(args.ctx?.requester ? { requester: args.ctx.requester } : {}), }); const toolContext = buildToolContext(toolIdentity); + // Policies form a mutation chain. Reconcile each decision against the prior + // alias pair so an explicit blank rewrite remains fail-closed. + let trustedPolicyParams = normalizedParams; const trustedPolicyResult = shouldRunTrustedPolicies ? await runTrustedToolPolicies( { @@ -224,13 +227,16 @@ export async function runBeforeToolCallHook(args: { ...(args.ctx?.config ? { config: args.ctx.config } : {}), deriveEvent: deriveToolEventParams, normalizeEvent(eventValue) { - const normalizedEventParams = normalizeCodeModeExecBeforeHookParamsForToolKind({ - toolKind: eventValue.toolKind, - params: eventValue.params, + const normalizedEventParams = reconcileCodeModeExecBeforeHookParams({ + owner: { toolKind: eventValue.toolKind }, + originalParams: trustedPolicyParams, + hookParams: trustedPolicyParams, + adjustedParams: eventValue.params, }); if (!isPlainObject(normalizedEventParams)) { return undefined; } + trustedPolicyParams = normalizedEventParams; const normalizedEventIdentity = getCodeModeExecBeforeHookMetadataForToolKind({ toolKind: eventValue.toolKind, params: normalizedEventParams, @@ -277,11 +283,7 @@ export async function runBeforeToolCallHook(args: { trustedApprovalResolution = approvalOutcome.approvalResolution; } } - const rawPolicyAdjustedParams = trustedApprovalParams ?? trustedPolicyResult?.params ?? params; - const policyAdjustedParams = normalizeCodeModeExecBeforeHookParamsForToolKind({ - toolKind: args.toolKind, - params: rawPolicyAdjustedParams, - }); + const policyAdjustedParams = trustedApprovalParams ?? trustedPolicyResult?.params ?? params; const policyAdjustedToolIdentity = getCodeModeExecBeforeHookMetadataForToolKind({ toolKind: args.toolKind, diff --git a/src/agents/agent-tools.before-tool-call.wrapper.ts b/src/agents/agent-tools.before-tool-call.wrapper.ts index bca32e26ca76..ef6fdfdb90b8 100644 --- a/src/agents/agent-tools.before-tool-call.wrapper.ts +++ b/src/agents/agent-tools.before-tool-call.wrapper.ts @@ -126,7 +126,7 @@ export function finalizeBeforeToolCallExecutionParams(params: { finalizerMode: "adapter" | "wrapped"; }): unknown { const reconciledParams = reconcileCodeModeExecBeforeHookParams({ - tool: params.tool, + owner: { tool: params.tool }, originalParams: params.preparedParams, hookParams: params.hookParams, adjustedParams: params.adjustedParams, diff --git a/src/agents/code-mode-control-tools.ts b/src/agents/code-mode-control-tools.ts index 57a88c968eb8..947e0bf1c0b4 100644 --- a/src/agents/code-mode-control-tools.ts +++ b/src/agents/code-mode-control-tools.ts @@ -2,6 +2,7 @@ * Tags Code Mode exec/wait control tools and normalizes hook params for the * exec-compatible before-tool-call surface. */ +import { readNonBlankString } from "@openclaw/normalization-core/string-coerce"; import { isPlainObject } from "../utils.js"; import { normalizeToolPolicyName } from "./tool-policy.js"; import type { AnyAgentTool } from "./tools/common.js"; @@ -67,15 +68,15 @@ function normalizeCodeModeExecParams(params: unknown): unknown { if (!isPlainObject(params)) { return params; } - const code = params.code; - const command = params.command; - if (typeof code === "string" && typeof command !== "string") { + const code = readNonBlankString(params.code); + const command = readNonBlankString(params.command); + if (code !== undefined && command === undefined) { // Code-mode accepts both `code` and generic exec `command`; keep them paired // so downstream hooks can read either shape. - return { ...params, command: params.code }; + return { ...params, command: code }; } - if (typeof command === "string" && typeof code !== "string") { - return { ...params, code: params.command }; + if (command !== undefined && code === undefined) { + return { ...params, code: command }; } return params; } @@ -121,26 +122,21 @@ export function normalizeCodeModeExecBeforeHookParams(params: { return normalizeCodeModeExecParams(params.params); } -/** Normalize before-hook params when only the code-mode tool kind is available. */ -export function normalizeCodeModeExecBeforeHookParamsForToolKind(params: { - toolKind: unknown; - params: unknown; -}): unknown { - if (params.toolKind !== CODE_MODE_EXEC_TOOL_KIND) { - return params.params; - } - return normalizeCodeModeExecParams(params.params); -} +type CodeModeExecReconcileOwner = { tool: AnyAgentTool } | { toolKind: unknown }; -/** Reconcile hook-adjusted `code` and `command` fields after code-mode normalization. */ +/** Reconcile policy- or hook-adjusted aliases after raw-input normalization. */ export function reconcileCodeModeExecBeforeHookParams(params: { - tool: AnyAgentTool; + owner: CodeModeExecReconcileOwner; originalParams: unknown; hookParams: unknown; adjustedParams: unknown; }): unknown { + const isCodeModeExecOwner = + "tool" in params.owner + ? isCodeModeExecTool(params.owner.tool) + : params.owner.toolKind === CODE_MODE_EXEC_TOOL_KIND; if ( - !isCodeModeExecTool(params.tool) || + !isCodeModeExecOwner || !isPlainObject(params.originalParams) || !isPlainObject(params.hookParams) || !isPlainObject(params.adjustedParams) diff --git a/src/agents/code-mode-runtime.ts b/src/agents/code-mode-runtime.ts index eb04a55cd11d..ce02cadf52f5 100644 --- a/src/agents/code-mode-runtime.ts +++ b/src/agents/code-mode-runtime.ts @@ -1,4 +1,5 @@ import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { readNonBlankString } from "@openclaw/normalization-core/string-coerce"; import { uniqueValues } from "@openclaw/normalization-core/string-normalization"; import { parse, tokenizer } from "acorn"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -282,17 +283,15 @@ export function readCode(args: unknown): { restartSafe: boolean; } { const params = asToolParamsRecord(args); - const codeParam = params.code; - const commandParam = params.command; - if ( - typeof codeParam === "string" && - typeof commandParam === "string" && - codeParam !== commandParam - ) { + // Full-schema tool calls can materialize an unused alias as blank. + // Only nonblank aliases participate in divergence checks. + const codeAlias = readNonBlankString(params.code); + const commandAlias = readNonBlankString(params.command); + if (codeAlias !== undefined && commandAlias !== undefined && codeAlias !== commandAlias) { throw new ToolInputError("code and command must match when both are provided."); } - const code = typeof commandParam === "string" ? commandParam : codeParam; - if (typeof code !== "string" || !code.trim()) { + const code = commandAlias ?? codeAlias; + if (code === undefined) { throw new ToolInputError("code or command must be a non-empty string."); } const language = params.language; diff --git a/src/agents/code-mode.guest.test.ts b/src/agents/code-mode.guest.test.ts index 3ed135c981f7..940c9e444b56 100644 --- a/src/agents/code-mode.guest.test.ts +++ b/src/agents/code-mode.guest.test.ts @@ -65,6 +65,52 @@ describe("Code Mode guest execution", () => { ).rejects.toThrow("code and command must match when both are provided"); }); + it.each([ + { alias: "blank code", args: { code: "", command: "return 7;" } }, + { alias: "whitespace code", args: { code: " ", command: "return 7;" } }, + { alias: "blank command", args: { code: "return 7;", command: "" } }, + { alias: "whitespace command", args: { code: "return 7;", command: " \n " } }, + ])("runs the populated alias when the other is $alias", async ({ args }) => { + const { config, catalogRef, tools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...tools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + const result = resultDetails( + await expectDefined(tools[0], "tools[0] test invariant").execute( + "code-call-blank-alias", + args, + ), + ); + + expect(result.status).toBe("completed"); + expect(result.value).toBe(7); + }); + + it("still rejects when both aliases are blank", async () => { + const { config, catalogRef, tools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...tools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + await expect( + expectDefined(tools[0], "tools[0] test invariant").execute("code-call-blank-both", { + code: "", + command: " ", + }), + ).rejects.toThrow("code or command must be a non-empty string"); + }); + it.each([ { code: "ls -la /workspace/" }, { code: "ls -1" },