mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 04:47:03 -06:00
fix(agents): run code-mode exec when a model sends a blank code alias (#126599)
Treat blank code and command aliases as absent while preserving mismatch rejection when both aliases contain different instructions. Keep trusted hook and policy rewrites normalized at the Code Mode owner boundary. Co-authored-by: Marvinthebored <marvin.assistant@lindsey.jp> Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<typeof def.execute>[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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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" },
|
||||
|
||||
Reference in New Issue
Block a user