diff --git a/CHANGELOG.md b/CHANGELOG.md index 89c6de9a22a..5b177ca602a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai - TUI: route `/status` through the shared session-status command, keep commentary hidden in history, strip raw envelope metadata from async command notices, preserve fallback streaming before per-attempt failures finalize, and restore Kitty keyboard state on exit or fatal crashes. (#49130, #59985, #60043, #61463) Thanks @biefan, @MoerAI, @jwchmodx, and @100yenadmin. - 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. - 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/node-host/exec-policy.test.ts b/src/node-host/exec-policy.test.ts index 44944e3c27e..6dab750fe1b 100644 --- a/src/node-host/exec-policy.test.ts +++ b/src/node-host/exec-policy.test.ts @@ -127,12 +127,13 @@ describe("evaluateSystemRunPolicy", () => { expect(denied.errorMessage).toBe("SYSTEM_RUN_DENIED: allowlist miss"); }); - it("treats shell wrappers as allowlist misses", () => { - const denied = expectDeniedDecision( + it("keeps POSIX shell wrapper decisions tied to allowlist analysis", () => { + const allowed = expectAllowedDecision( evaluateSystemRunPolicy(buildPolicyParams({ shellWrapperInvocation: true })), ); - expect(denied.shellWrapperBlocked).toBe(true); - expect(denied.errorMessage).toContain("shell wrappers like sh/bash/zsh -c"); + expect(allowed.shellWrapperBlocked).toBe(false); + expect(allowed.analysisOk).toBe(true); + expect(allowed.allowlistSatisfied).toBe(true); }); it("keeps Windows-specific guidance for cmd.exe wrappers", () => { diff --git a/src/node-host/exec-policy.ts b/src/node-host/exec-policy.ts index 6500be269a1..e72f12842eb 100644 --- a/src/node-host/exec-policy.ts +++ b/src/node-host/exec-policy.ts @@ -61,9 +61,16 @@ export function evaluateSystemRunPolicy(params: { cmdInvocation: boolean; shellWrapperInvocation: boolean; }): SystemRunPolicyDecision { - const shellWrapperBlocked = params.security === "allowlist" && params.shellWrapperInvocation; + // POSIX node execution intentionally uses `/bin/sh -lc` as a transport wrapper. + // Keep allowlist decisions based on the analyzed inner shell payload there. + // Windows `cmd.exe /c` wrappers still require explicit approval because they + // change execution semantics for builtins and quoting/parsing behavior. const windowsShellWrapperBlocked = - shellWrapperBlocked && params.isWindows && params.cmdInvocation; + params.security === "allowlist" && + params.shellWrapperInvocation && + params.isWindows && + params.cmdInvocation; + const shellWrapperBlocked = windowsShellWrapperBlocked; const analysisOk = shellWrapperBlocked ? false : params.analysisOk; const allowlistSatisfied = shellWrapperBlocked ? false : params.allowlistSatisfied; const approvedByAsk = params.approvalDecision !== null || params.approved === true; diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index d67d3cd46e1..145fa2bad11 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -383,6 +383,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { sendExecFinishedEvent?: HandleSystemRunInvokeOptions["sendExecFinishedEvent"]; sendNodeEvent?: HandleSystemRunInvokeOptions["sendNodeEvent"]; skillBinsCurrent?: () => Promise>; + isCmdExeInvocation?: HandleSystemRunInvokeOptions["isCmdExeInvocation"]; }): Promise<{ runCommand: MockedRunCommand; runViaMacAppExecHost: MockedRunViaMacAppExecHost; @@ -441,7 +442,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { execHostFallbackAllowed: true, resolveExecSecurity: () => params.security ?? "full", resolveExecAsk: () => params.ask ?? "off", - isCmdExeInvocation: () => false, + isCmdExeInvocation: params.isCmdExeInvocation ?? (() => false), sanitizeEnv: () => undefined, runCommand, runViaMacAppExecHost, @@ -1512,6 +1513,93 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }); + it.runIf(process.platform !== "win32")( + "auto-runs allowlisted inner scripts through transport shell wrappers", + async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-wrapper-inner-")); + try { + const scriptsDir = path.join(tempDir, "scripts"); + fs.mkdirSync(scriptsDir, { recursive: true }); + const scriptPath = path.join(scriptsDir, "check_mail.sh"); + fs.writeFileSync(scriptPath, "#!/bin/sh\necho ok\n"); + fs.chmodSync(scriptPath, 0o755); + + await withTempApprovalsHome({ + approvals: createAllowlistOnMissApprovals({ + agents: { + main: { + allowlist: [{ pattern: scriptPath }], + }, + }, + }), + run: async () => { + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["/bin/sh", "-lc", "./scripts/check_mail.sh --limit 5"], + rawCommand: '/bin/sh -lc "./scripts/check_mail.sh --limit 5"', + cwd: tempDir, + security: "allowlist", + ask: "on-miss", + runCommand: vi.fn(async () => createLocalRunResult("shell-wrapper-inner-ok")), + }); + + expect(invoke.runCommand).toHaveBeenCalledTimes(1); + expectInvokeOk(invoke.sendInvokeResult, { + payloadContains: "shell-wrapper-inner-ok", + }); + }, + }); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }, + ); + + it("keeps cmd.exe transport wrappers approval-gated on Windows", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cmd-wrapper-allow-")); + try { + const scriptPath = path.join(tempDir, "check_mail.cmd"); + fs.writeFileSync(scriptPath, "@echo off\r\necho ok\r\n"); + + await withTempApprovalsHome({ + approvals: createAllowlistOnMissApprovals({ + agents: { + main: { + allowlist: [{ pattern: scriptPath }], + }, + }, + }), + run: async () => { + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["cmd.exe", "/d", "/s", "/c", `${scriptPath} --limit 5`], + cwd: tempDir, + security: "allowlist", + ask: "on-miss", + isCmdExeInvocation: (argv) => { + const token = argv[0]?.trim(); + if (!token) { + return false; + } + const base = path.win32.basename(token).toLowerCase(); + return base === "cmd.exe" || base === "cmd"; + }, + }); + + 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 7433abb1d28..cedcfb8af6c 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -359,9 +359,10 @@ async function evaluateSystemRunPolicyPhase( .find((entry) => entry !== null) ?? null) : null; const isWindows = process.platform === "win32"; - const cmdInvocation = parsed.shellPayload - ? opts.isCmdExeInvocation(segments[0]?.argv ?? []) - : opts.isCmdExeInvocation(parsed.argv); + // 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); const durableApprovalSatisfied = hasDurableExecApproval({ analysisOk, segmentAllowlistEntries, @@ -407,13 +408,8 @@ async function evaluateSystemRunPolicyPhase( return null; } - // Fail closed if policy/runtime drift re-allows unapproved shell wrappers. - if ( - security === "allowlist" && - parsed.shellPayload && - !policy.approvedByAsk && - !durableApprovalSatisfied - ) { + // Fail closed if policy/runtime drift re-allows Windows shell wrappers. + if (policy.shellWrapperBlocked && !policy.approvedByAsk && !durableApprovalSatisfied) { await sendSystemRunDenied(opts, parsed.execution, { reason: "approval-required", message: "SYSTEM_RUN_DENIED: approval required",