Skip to content

Commit de6bac3

Browse files
ngutmanngutman
authored
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)
1 parent 7d20881 commit de6bac3

5 files changed

Lines changed: 133 additions & 36 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ Docs: https://docs.openclaw.ai
4040
- 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.
4141
- 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.
4242
- 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.
43+
- 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.
4344
- 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.
4445
- 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.
4546
- 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)

‎src/infra/exec-wrapper-resolution.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
isShellWrapperExecutable,
99
normalizeExecutableToken,
1010
resolveDispatchWrapperTrustPlan,
11+
resolveShellWrapperTransportArgv,
1112
unwrapEnvInvocation,
1213
unwrapKnownDispatchWrapperInvocation,
1314
unwrapKnownShellMultiplexerInvocation,
@@ -382,6 +383,25 @@ describe("hasEnvManipulationBeforeShellWrapper", () => {
382383
});
383384
});
384385

386+
describe("resolveShellWrapperTransportArgv", () => {
387+
test.each([
388+
{
389+
argv: ["env", "cmd.exe", "/d", "/s", "/c", "echo hi"],
390+
expected: ["cmd.exe", "/d", "/s", "/c", "echo hi"],
391+
},
392+
{
393+
argv: ["env", "FOO=bar", "cmd.exe", "/d", "/s", "/c", "echo hi"],
394+
expected: ["cmd.exe", "/d", "/s", "/c", "echo hi"],
395+
},
396+
{
397+
argv: ["bash", "script.sh"],
398+
expected: null,
399+
},
400+
])("resolves wrapper transport argv for %j", ({ argv, expected }) => {
401+
expect(resolveShellWrapperTransportArgv(argv)).toEqual(expected);
402+
});
403+
});
404+
385405
describe("extractShellWrapperCommand", () => {
386406
test.each([
387407
{

‎src/infra/shell-wrapper-resolution.ts‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,48 @@ export type ShellWrapperCommand = {
5656
command: string | null;
5757
};
5858

59+
function resolveShellWrapperSpecAndArgvInternal(
60+
argv: string[],
61+
depth: number,
62+
): { argv: string[]; wrapper: ShellWrapperSpec; payload: string } | null {
63+
if (!isWithinDispatchClassificationDepth(depth)) {
64+
return null;
65+
}
66+
67+
const token0 = argv[0]?.trim();
68+
if (!token0) {
69+
return null;
70+
}
71+
72+
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv);
73+
if (dispatchUnwrap.kind === "blocked") {
74+
return null;
75+
}
76+
if (dispatchUnwrap.kind === "unwrapped") {
77+
return resolveShellWrapperSpecAndArgvInternal(dispatchUnwrap.argv, depth + 1);
78+
}
79+
80+
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv);
81+
if (shellMultiplexerUnwrap.kind === "blocked") {
82+
return null;
83+
}
84+
if (shellMultiplexerUnwrap.kind === "unwrapped") {
85+
return resolveShellWrapperSpecAndArgvInternal(shellMultiplexerUnwrap.argv, depth + 1);
86+
}
87+
88+
const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0));
89+
if (!wrapper) {
90+
return null;
91+
}
92+
93+
const payload = extractShellWrapperPayload(argv, wrapper);
94+
if (!payload) {
95+
return null;
96+
}
97+
98+
return { argv, wrapper, payload };
99+
}
100+
59101
function isWithinDispatchClassificationDepth(depth: number): boolean {
60102
return depth <= MAX_DISPATCH_WRAPPER_DEPTH;
61103
}
@@ -213,42 +255,16 @@ function extractShellWrapperCommandInternal(
213255
rawCommand: string | null,
214256
depth: number,
215257
): ShellWrapperCommand {
216-
if (!isWithinDispatchClassificationDepth(depth)) {
217-
return { isWrapper: false, command: null };
218-
}
219-
220-
const token0 = argv[0]?.trim();
221-
if (!token0) {
258+
const resolved = resolveShellWrapperSpecAndArgvInternal(argv, depth);
259+
if (!resolved) {
222260
return { isWrapper: false, command: null };
223261
}
224262

225-
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(argv);
226-
if (dispatchUnwrap.kind === "blocked") {
227-
return { isWrapper: false, command: null };
228-
}
229-
if (dispatchUnwrap.kind === "unwrapped") {
230-
return extractShellWrapperCommandInternal(dispatchUnwrap.argv, rawCommand, depth + 1);
231-
}
232-
233-
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv);
234-
if (shellMultiplexerUnwrap.kind === "blocked") {
235-
return { isWrapper: false, command: null };
236-
}
237-
if (shellMultiplexerUnwrap.kind === "unwrapped") {
238-
return extractShellWrapperCommandInternal(shellMultiplexerUnwrap.argv, rawCommand, depth + 1);
239-
}
240-
241-
const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0));
242-
if (!wrapper) {
243-
return { isWrapper: false, command: null };
244-
}
245-
246-
const payload = extractShellWrapperPayload(argv, wrapper);
247-
if (!payload) {
248-
return { isWrapper: false, command: null };
249-
}
263+
return { isWrapper: true, command: rawCommand ?? resolved.payload };
264+
}
250265

251-
return { isWrapper: true, command: rawCommand ?? payload };
266+
export function resolveShellWrapperTransportArgv(argv: string[]): string[] | null {
267+
return resolveShellWrapperSpecAndArgvInternal(argv, 0)?.argv ?? null;
252268
}
253269

254270
export function extractShellWrapperInlineCommand(argv: string[]): string | null {

‎src/node-host/invoke-system-run.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,6 +1600,64 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
16001600
}
16011601
});
16021602

1603+
it.each([
1604+
{
1605+
name: "keeps env cmd.exe transport wrappers approval-gated on Windows",
1606+
command: ["env", "cmd.exe", "/d", "/s", "/c"],
1607+
},
1608+
{
1609+
name: "keeps env-assignment cmd.exe transport wrappers approval-gated on Windows",
1610+
command: ["env", "FOO=bar", "cmd.exe", "/d", "/s", "/c"],
1611+
},
1612+
])("$name", async ({ command }) => {
1613+
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
1614+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-env-cmd-wrapper-allow-"));
1615+
try {
1616+
const scriptPath = path.join(tempDir, "check_mail.cmd");
1617+
fs.writeFileSync(scriptPath, "@echo off\r\necho ok\r\n");
1618+
const wrappedCommand = [...command, `${scriptPath} --limit 5`];
1619+
1620+
await withTempApprovalsHome({
1621+
approvals: createAllowlistOnMissApprovals({
1622+
agents: {
1623+
main: {
1624+
allowlist: [{ pattern: scriptPath }],
1625+
},
1626+
},
1627+
}),
1628+
run: async () => {
1629+
const seenArgv: string[][] = [];
1630+
const invoke = await runSystemInvoke({
1631+
preferMacAppExecHost: false,
1632+
command: wrappedCommand,
1633+
cwd: tempDir,
1634+
security: "allowlist",
1635+
ask: "on-miss",
1636+
isCmdExeInvocation: (argv) => {
1637+
seenArgv.push([...argv]);
1638+
const token = argv[0]?.trim();
1639+
if (!token) {
1640+
return false;
1641+
}
1642+
const base = path.win32.basename(token).toLowerCase();
1643+
return base === "cmd.exe" || base === "cmd";
1644+
},
1645+
});
1646+
1647+
expect(seenArgv).toContainEqual(["cmd.exe", "/d", "/s", "/c", `${scriptPath} --limit 5`]);
1648+
expect(invoke.runCommand).not.toHaveBeenCalled();
1649+
expectApprovalRequiredDenied({
1650+
sendNodeEvent: invoke.sendNodeEvent,
1651+
sendInvokeResult: invoke.sendInvokeResult,
1652+
});
1653+
},
1654+
});
1655+
} finally {
1656+
platformSpy.mockRestore();
1657+
fs.rmSync(tempDir, { recursive: true, force: true });
1658+
}
1659+
});
1660+
16031661
it("reuses exact-command durable trust for shell-wrapper reruns", async () => {
16041662
if (process.platform === "win32") {
16051663
return;

‎src/node-host/invoke-system-run.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
detectInterpreterInlineEvalArgv,
2121
} from "../infra/exec-inline-eval.js";
2222
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
23+
import { resolveShellWrapperTransportArgv } from "../infra/exec-wrapper-resolution.js";
2324
import {
2425
inspectHostExecEnvOverrides,
2526
sanitizeSystemRunEnvOverrides,
@@ -359,10 +360,11 @@ async function evaluateSystemRunPolicyPhase(
359360
.find((entry) => entry !== null) ?? null)
360361
: null;
361362
const isWindows = process.platform === "win32";
362-
// Detect Windows wrapper transport from the original request argv, not the
363-
// analyzed inner shell payload. Once parsing unwraps `cmd.exe /c ...`, the
364-
// inner segments no longer retain the wrapper marker we need for policy.
365-
const cmdInvocation = opts.isCmdExeInvocation(parsed.argv);
363+
// Detect Windows wrapper transport from the same shell-wrapper view used to
364+
// derive the inner payload. That keeps `cmd.exe /c` approval-gated even when
365+
// dispatch carriers like `env FOO=bar ...` wrap the shell invocation.
366+
const cmdDetectionArgv = resolveShellWrapperTransportArgv(parsed.argv) ?? parsed.argv;
367+
const cmdInvocation = opts.isCmdExeInvocation(cmdDetectionArgv);
366368
const durableApprovalSatisfied = hasDurableExecApproval({
367369
analysisOk,
368370
segmentAllowlistEntries,

0 commit comments

Comments
 (0)