From de6bac331cde02ea19389e46c7e4385f0b31cc49 Mon Sep 17 00:00:00 2001 From: Nimrod Gutman Date: Tue, 7 Apr 2026 14:27:06 +0300 Subject: [PATCH] fix(exec): detect cmd wrapper carriers (#62439) * fix(exec): detect cmd wrapper carriers * fix(exec): block env cmd wrapper carriers * fix: keep cmd wrapper carriers approval-gated (#62439) (thanks @ngutman) --- CHANGELOG.md | 1 + src/infra/exec-wrapper-resolution.test.ts | 20 ++++++ src/infra/shell-wrapper-resolution.ts | 80 ++++++++++++++--------- src/node-host/invoke-system-run.test.ts | 58 ++++++++++++++++ src/node-host/invoke-system-run.ts | 10 +-- 5 files changed, 133 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f10c1b2c4..896a70a1d4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Docs: https://docs.openclaw.ai - Sessions/model selection: resolve the explicitly selected session model separately from runtime fallback resolution so session status and live model switching stay aligned with the chosen model. - iOS/Watch exec approvals: keep Apple Watch review and approval recovery working while the iPhone is locked or backgrounded, including reconnect recovery, pending approval persistence, notification cleanup, and APNs-backed watch refresh recovery. (#61757) Thanks @ngutman. - Nodes/exec approvals: keep `host=node` POSIX transport shell wrappers (`/bin/sh -lc ...`) aligned with inner-command allowlist analysis so allowlisted scripts stop prompting unnecessarily, while Windows `cmd.exe` wrapper runs stay approval-gated. (#62401) Thanks @ngutman. +- Nodes/exec approvals: keep Windows `cmd.exe /c` wrapper runs approval-gated even when `env` carriers, including env-assignment carriers, wrap the shell invocation. (#62439) Thanks @ngutman. - Agents/context overflow: combine oversized and aggregate tool-result recovery in one pass and restore a total-context overflow backstop so recoverable sessions retry instead of failing early. (#61651) Thanks @Takhoffman. - Agents/exec: preserve explicit `host=node` routing under elevated defaults when `tools.exec.host=auto`, fail loud on invalid elevated cross-host overrides, and keep `strictInlineEval` commands blocked after approval timeouts instead of falling through to automatic execution. (#61739) Thanks @obviyus. - Providers/Ollama: honor the selected provider's `baseUrl` during streaming so multi-Ollama setups stop routing every stream to the first configured Ollama endpoint. (#61678) diff --git a/src/infra/exec-wrapper-resolution.test.ts b/src/infra/exec-wrapper-resolution.test.ts index 686789826f4..e3159c3a593 100644 --- a/src/infra/exec-wrapper-resolution.test.ts +++ b/src/infra/exec-wrapper-resolution.test.ts @@ -8,6 +8,7 @@ import { isShellWrapperExecutable, normalizeExecutableToken, resolveDispatchWrapperTrustPlan, + resolveShellWrapperTransportArgv, unwrapEnvInvocation, unwrapKnownDispatchWrapperInvocation, unwrapKnownShellMultiplexerInvocation, @@ -382,6 +383,25 @@ describe("hasEnvManipulationBeforeShellWrapper", () => { }); }); +describe("resolveShellWrapperTransportArgv", () => { + test.each([ + { + argv: ["env", "cmd.exe", "/d", "/s", "/c", "echo hi"], + expected: ["cmd.exe", "/d", "/s", "/c", "echo hi"], + }, + { + argv: ["env", "FOO=bar", "cmd.exe", "/d", "/s", "/c", "echo hi"], + expected: ["cmd.exe", "/d", "/s", "/c", "echo hi"], + }, + { + argv: ["bash", "script.sh"], + expected: null, + }, + ])("resolves wrapper transport argv for %j", ({ argv, expected }) => { + expect(resolveShellWrapperTransportArgv(argv)).toEqual(expected); + }); +}); + describe("extractShellWrapperCommand", () => { test.each([ { diff --git a/src/infra/shell-wrapper-resolution.ts b/src/infra/shell-wrapper-resolution.ts index ea06a55672a..1c3403e5d61 100644 --- a/src/infra/shell-wrapper-resolution.ts +++ b/src/infra/shell-wrapper-resolution.ts @@ -56,6 +56,48 @@ export type ShellWrapperCommand = { command: string | null; }; +function resolveShellWrapperSpecAndArgvInternal( + argv: string[], + depth: number, +): { argv: string[]; wrapper: ShellWrapperSpec; payload: string } | null { + if (!isWithinDispatchClassificationDepth(depth)) { + return null; + } + + const token0 = argv[0]?.trim(); + if (!token0) { + return null; + } + + const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv); + if (dispatchUnwrap.kind === "blocked") { + return null; + } + if (dispatchUnwrap.kind === "unwrapped") { + return resolveShellWrapperSpecAndArgvInternal(dispatchUnwrap.argv, depth + 1); + } + + const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv); + if (shellMultiplexerUnwrap.kind === "blocked") { + return null; + } + if (shellMultiplexerUnwrap.kind === "unwrapped") { + return resolveShellWrapperSpecAndArgvInternal(shellMultiplexerUnwrap.argv, depth + 1); + } + + const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0)); + if (!wrapper) { + return null; + } + + const payload = extractShellWrapperPayload(argv, wrapper); + if (!payload) { + return null; + } + + return { argv, wrapper, payload }; +} + function isWithinDispatchClassificationDepth(depth: number): boolean { return depth <= MAX_DISPATCH_WRAPPER_DEPTH; } @@ -213,42 +255,16 @@ function extractShellWrapperCommandInternal( rawCommand: string | null, depth: number, ): ShellWrapperCommand { - if (!isWithinDispatchClassificationDepth(depth)) { + const resolved = resolveShellWrapperSpecAndArgvInternal(argv, depth); + if (!resolved) { return { isWrapper: false, command: null }; } - const token0 = argv[0]?.trim(); - if (!token0) { - return { isWrapper: false, command: null }; - } + return { isWrapper: true, command: rawCommand ?? resolved.payload }; +} - const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv); - if (dispatchUnwrap.kind === "blocked") { - return { isWrapper: false, command: null }; - } - if (dispatchUnwrap.kind === "unwrapped") { - return extractShellWrapperCommandInternal(dispatchUnwrap.argv, rawCommand, depth + 1); - } - - const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv); - if (shellMultiplexerUnwrap.kind === "blocked") { - return { isWrapper: false, command: null }; - } - if (shellMultiplexerUnwrap.kind === "unwrapped") { - return extractShellWrapperCommandInternal(shellMultiplexerUnwrap.argv, rawCommand, depth + 1); - } - - const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0)); - if (!wrapper) { - return { isWrapper: false, command: null }; - } - - const payload = extractShellWrapperPayload(argv, wrapper); - if (!payload) { - return { isWrapper: false, command: null }; - } - - return { isWrapper: true, command: rawCommand ?? payload }; +export function resolveShellWrapperTransportArgv(argv: string[]): string[] | null { + return resolveShellWrapperSpecAndArgvInternal(argv, 0)?.argv ?? null; } export function extractShellWrapperInlineCommand(argv: string[]): string | null { diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 145fa2bad11..3b8a8d9dba1 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -1600,6 +1600,64 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }); + it.each([ + { + name: "keeps env cmd.exe transport wrappers approval-gated on Windows", + command: ["env", "cmd.exe", "/d", "/s", "/c"], + }, + { + name: "keeps env-assignment cmd.exe transport wrappers approval-gated on Windows", + command: ["env", "FOO=bar", "cmd.exe", "/d", "/s", "/c"], + }, + ])("$name", async ({ command }) => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-env-cmd-wrapper-allow-")); + try { + const scriptPath = path.join(tempDir, "check_mail.cmd"); + fs.writeFileSync(scriptPath, "@echo off\r\necho ok\r\n"); + const wrappedCommand = [...command, `${scriptPath} --limit 5`]; + + await withTempApprovalsHome({ + approvals: createAllowlistOnMissApprovals({ + agents: { + main: { + allowlist: [{ pattern: scriptPath }], + }, + }, + }), + run: async () => { + const seenArgv: string[][] = []; + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: wrappedCommand, + cwd: tempDir, + security: "allowlist", + ask: "on-miss", + isCmdExeInvocation: (argv) => { + seenArgv.push([...argv]); + const token = argv[0]?.trim(); + if (!token) { + return false; + } + const base = path.win32.basename(token).toLowerCase(); + return base === "cmd.exe" || base === "cmd"; + }, + }); + + expect(seenArgv).toContainEqual(["cmd.exe", "/d", "/s", "/c", `${scriptPath} --limit 5`]); + expect(invoke.runCommand).not.toHaveBeenCalled(); + expectApprovalRequiredDenied({ + sendNodeEvent: invoke.sendNodeEvent, + sendInvokeResult: invoke.sendInvokeResult, + }); + }, + }); + } finally { + platformSpy.mockRestore(); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + it("reuses exact-command durable trust for shell-wrapper reruns", async () => { if (process.platform === "win32") { return; diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index cedcfb8af6c..abd51fe450b 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -20,6 +20,7 @@ import { detectInterpreterInlineEvalArgv, } from "../infra/exec-inline-eval.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; +import { resolveShellWrapperTransportArgv } from "../infra/exec-wrapper-resolution.js"; import { inspectHostExecEnvOverrides, sanitizeSystemRunEnvOverrides, @@ -359,10 +360,11 @@ async function evaluateSystemRunPolicyPhase( .find((entry) => entry !== null) ?? null) : null; const isWindows = process.platform === "win32"; - // Detect Windows wrapper transport from the original request argv, not the - // analyzed inner shell payload. Once parsing unwraps `cmd.exe /c ...`, the - // inner segments no longer retain the wrapper marker we need for policy. - const cmdInvocation = opts.isCmdExeInvocation(parsed.argv); + // Detect Windows wrapper transport from the same shell-wrapper view used to + // derive the inner payload. That keeps `cmd.exe /c` approval-gated even when + // dispatch carriers like `env FOO=bar ...` wrap the shell invocation. + const cmdDetectionArgv = resolveShellWrapperTransportArgv(parsed.argv) ?? parsed.argv; + const cmdInvocation = opts.isCmdExeInvocation(cmdDetectionArgv); const durableApprovalSatisfied = hasDurableExecApproval({ analysisOk, segmentAllowlistEntries,