diff --git a/CHANGELOG.md b/CHANGELOG.md index a24d71f82e3..92530d9a646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - Agents/Codex: allow `gpt-5.3-codex-spark` in forward-compat fallback, live model filtering, and thinking presets, and fix model-picker recognition for spark. (#14990) Thanks @L-U-C-K-Y. - OpenAI Codex/Auth: bridge OpenClaw OAuth profiles into `pi` `auth.json` so model discovery and models-list registry resolution can use Codex OAuth credentials. (#15184) Thanks @loiie45e. - Agents/Transcript policy: sanitize OpenAI/Codex tool-call ids during transcript policy normalization to prevent invalid tool-call identifiers from propagating into session history. (#15279) Thanks @divisonofficer. +- Agents/Nodes: harden node exec approval decision handling in the `nodes` tool run path by failing closed on unexpected approval decisions, and add regression coverage for approval-required retry/deny/timeout flows. (#4726) Thanks @rmorse. - Models/Codex: resolve configured `openai-codex/gpt-5.3-codex-spark` through forward-compat fallback during `models list`, so it is not incorrectly tagged as missing when runtime resolution succeeds. (#15174) Thanks @loiie45e. - macOS Voice Wake: fix a crash in trigger trimming for CJK/Unicode transcripts by matching and slicing on original-string ranges instead of transformed-string indices. (#11052) Thanks @Flash-LHR. - Heartbeat: prevent scheduler silent-death races during runner reloads, preserve retry cooldown backoff under wake bursts, and prioritize user/action wake causes over interval/retry reasons when coalescing. (#15108) Thanks @joeykrug. diff --git a/src/agents/openclaw-tools.camera.e2e.test.ts b/src/agents/openclaw-tools.camera.e2e.test.ts index 802a8c662fa..6411b443624 100644 --- a/src/agents/openclaw-tools.camera.e2e.test.ts +++ b/src/agents/openclaw-tools.camera.e2e.test.ts @@ -132,4 +132,125 @@ describe("nodes run", () => { invokeTimeoutMs: 45_000, }); }); + + it("requests approval and retries with allow-once decision", async () => { + let invokeCalls = 0; + callGateway.mockImplementation(async ({ method, params }) => { + if (method === "node.list") { + return { nodes: [{ nodeId: "mac-1", commands: ["system.run"] }] }; + } + if (method === "node.invoke") { + invokeCalls += 1; + if (invokeCalls === 1) { + throw new Error("SYSTEM_RUN_DENIED: approval required"); + } + expect(params).toMatchObject({ + nodeId: "mac-1", + command: "system.run", + params: { + command: ["echo", "hi"], + approved: true, + approvalDecision: "allow-once", + }, + }); + return { payload: { stdout: "", stderr: "", exitCode: 0, success: true } }; + } + if (method === "exec.approval.request") { + expect(params).toMatchObject({ + command: "echo hi", + host: "node", + timeoutMs: 120_000, + }); + return { decision: "allow-once" }; + } + throw new Error(`unexpected method: ${String(method)}`); + }); + + const tool = createOpenClawTools().find((candidate) => candidate.name === "nodes"); + if (!tool) { + throw new Error("missing nodes tool"); + } + + await tool.execute("call1", { + action: "run", + node: "mac-1", + command: ["echo", "hi"], + }); + expect(invokeCalls).toBe(2); + }); + + it("fails with user denied when approval decision is deny", async () => { + callGateway.mockImplementation(async ({ method }) => { + if (method === "node.list") { + return { nodes: [{ nodeId: "mac-1", commands: ["system.run"] }] }; + } + if (method === "node.invoke") { + throw new Error("SYSTEM_RUN_DENIED: approval required"); + } + if (method === "exec.approval.request") { + return { decision: "deny" }; + } + throw new Error(`unexpected method: ${String(method)}`); + }); + + const tool = createOpenClawTools().find((candidate) => candidate.name === "nodes"); + if (!tool) { + throw new Error("missing nodes tool"); + } + + await expect( + tool.execute("call1", { + action: "run", + node: "mac-1", + command: ["echo", "hi"], + }), + ).rejects.toThrow("exec denied: user denied"); + }); + + it("fails closed for timeout and invalid approval decisions", async () => { + const tool = createOpenClawTools().find((candidate) => candidate.name === "nodes"); + if (!tool) { + throw new Error("missing nodes tool"); + } + + callGateway.mockImplementation(async ({ method }) => { + if (method === "node.list") { + return { nodes: [{ nodeId: "mac-1", commands: ["system.run"] }] }; + } + if (method === "node.invoke") { + throw new Error("SYSTEM_RUN_DENIED: approval required"); + } + if (method === "exec.approval.request") { + return {}; + } + throw new Error(`unexpected method: ${String(method)}`); + }); + await expect( + tool.execute("call1", { + action: "run", + node: "mac-1", + command: ["echo", "hi"], + }), + ).rejects.toThrow("exec denied: approval timed out"); + + callGateway.mockImplementation(async ({ method }) => { + if (method === "node.list") { + return { nodes: [{ nodeId: "mac-1", commands: ["system.run"] }] }; + } + if (method === "node.invoke") { + throw new Error("SYSTEM_RUN_DENIED: approval required"); + } + if (method === "exec.approval.request") { + return { decision: "allow-never" }; + } + throw new Error(`unexpected method: ${String(method)}`); + }); + await expect( + tool.execute("call1", { + action: "run", + node: "mac-1", + command: ["echo", "hi"], + }), + ).rejects.toThrow("exec denied: invalid approval decision"); + }); }); diff --git a/src/agents/tools/nodes-tool.ts b/src/agents/tools/nodes-tool.ts index dd7ec97fe21..4a1d1b2cdf8 100644 --- a/src/agents/tools/nodes-tool.ts +++ b/src/agents/tools/nodes-tool.ts @@ -436,17 +436,74 @@ export function createNodesTool(options?: { typeof params.needsScreenRecording === "boolean" ? params.needsScreenRecording : undefined; - const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", gatewayOpts, { + const runParams = { + command, + cwd, + env, + timeoutMs: commandTimeoutMs, + needsScreenRecording, + agentId, + sessionKey, + }; + + // First attempt without approval flags. + try { + const raw = await callGatewayTool<{ payload?: unknown }>("node.invoke", gatewayOpts, { + nodeId, + command: "system.run", + params: runParams, + timeoutMs: invokeTimeoutMs, + idempotencyKey: crypto.randomUUID(), + }); + return jsonResult(raw?.payload ?? {}); + } catch (firstErr) { + const msg = firstErr instanceof Error ? firstErr.message : String(firstErr); + if (!msg.includes("SYSTEM_RUN_DENIED: approval required")) { + throw firstErr; + } + } + + // Node requires approval – create a pending approval request on + // the gateway and wait for the user to approve/deny via the UI. + const APPROVAL_TIMEOUT_MS = 120_000; + const cmdText = command.join(" "); + const approvalResult = await callGatewayTool( + "exec.approval.request", + { ...gatewayOpts, timeoutMs: APPROVAL_TIMEOUT_MS + 5_000 }, + { + command: cmdText, + cwd, + host: "node", + agentId, + sessionKey, + timeoutMs: APPROVAL_TIMEOUT_MS, + }, + ); + const decisionRaw = + approvalResult && typeof approvalResult === "object" + ? (approvalResult as { decision?: unknown }).decision + : undefined; + const approvalDecision = + decisionRaw === "allow-once" || decisionRaw === "allow-always" ? decisionRaw : null; + + if (!approvalDecision) { + if (decisionRaw === "deny") { + throw new Error("exec denied: user denied"); + } + if (decisionRaw === undefined || decisionRaw === null) { + throw new Error("exec denied: approval timed out"); + } + throw new Error("exec denied: invalid approval decision"); + } + + // Retry with the approval decision. + const raw = await callGatewayTool<{ payload?: unknown }>("node.invoke", gatewayOpts, { nodeId, command: "system.run", params: { - command, - cwd, - env, - timeoutMs: commandTimeoutMs, - needsScreenRecording, - agentId, - sessionKey, + ...runParams, + approved: true, + approvalDecision, }, timeoutMs: invokeTimeoutMs, idempotencyKey: crypto.randomUUID(), diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index ca121c84abf..2aa85b20d46 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -57,6 +57,7 @@ describe("config io write", () => { defaults: { cliBackends: { codex: { + command: "codex", env: { OPENAI_API_KEY: "${OPENAI_API_KEY}", }, @@ -110,9 +111,14 @@ describe("config io write", () => { configPath, JSON.stringify( { - channels: { - discord: { - allowFrom: ["${DISCORD_USER_ID}", "123"], + agents: { + defaults: { + cliBackends: { + codex: { + command: "codex", + args: ["${DISCORD_USER_ID}", "123"], + }, + }, }, }, }, @@ -131,23 +137,41 @@ describe("config io write", () => { expect(snapshot.valid).toBe(true); const next = structuredClone(snapshot.config); - const allowFrom = Array.isArray(next.channels?.discord?.allowFrom) - ? next.channels?.discord?.allowFrom - : []; - next.channels = { - ...next.channels, - discord: { - ...next.channels?.discord, - allowFrom: [...allowFrom, "456"], + const codexBackend = next.agents?.defaults?.cliBackends?.codex; + const args = Array.isArray(codexBackend?.args) ? codexBackend?.args : []; + next.agents = { + ...next.agents, + defaults: { + ...next.agents?.defaults, + cliBackends: { + ...next.agents?.defaults?.cliBackends, + codex: { + ...codexBackend, + command: typeof codexBackend?.command === "string" ? codexBackend.command : "codex", + args: [...args, "456"], + }, + }, }, }; await io.writeConfigFile(next); const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as { - channels: { discord?: { allowFrom?: string[] } }; + agents: { + defaults: { + cliBackends: { + codex: { + args: string[]; + }; + }; + }; + }; }; - expect(persisted.channels.discord?.allowFrom).toEqual(["${DISCORD_USER_ID}", "123", "456"]); + expect(persisted.agents.defaults.cliBackends.codex.args).toEqual([ + "${DISCORD_USER_ID}", + "123", + "456", + ]); }); }); });