diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a3315a3c40..1623a5eaf2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Node exec approvals: preserve shell/dispatch-wrapper argv semantics during approval hardening so approved wrapper commands (for example `env sh -c ...`) cannot drift into a different runtime command shape, and add regression coverage for both approval-plan generation and approved runtime execution paths. Thanks @tdjackey for reporting. - Sandbox/Bootstrap context boundary hardening: reject symlink/hardlink alias bootstrap seed files that resolve outside the source workspace and switch post-compaction `AGENTS.md` context reads to boundary-verified file opens, preventing host file content from being injected via workspace aliasing. Thanks @tdjackey for reporting. - Browser/Security output boundary hardening: replace check-then-rename output commits with root-bound fd-verified writes, unify install/skills canonical path-boundary checks, and add regression coverage for symlink-rebind race paths across browser output and shared fs-safe write flows. Thanks @tdjackey for reporting. - Gateway/Security hardening: tie loopback-origin dev allowance to actual local socket clients (not Host header claims), add explicit warnings/metrics when `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` accepts websocket origins, harden safe-regex detection for quantified ambiguous alternation patterns (for example `(a|aa)+`), and bound large regex-evaluation inputs for session-filter and log-redaction paths. diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts new file mode 100644 index 00000000000..b0904435dc6 --- /dev/null +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -0,0 +1,87 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + buildSystemRunApprovalPlan, + hardenApprovedExecutionPaths, +} from "./invoke-system-run-plan.js"; + +describe("hardenApprovedExecutionPaths", () => { + it.runIf(process.platform !== "win32")( + "preserves shell-wrapper argv during approval hardening", + () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-wrapper-")); + try { + const prepared = buildSystemRunApprovalPlan({ + command: ["env", "sh", "-c", "echo SAFE"], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + expect(prepared.plan.argv).toEqual(["env", "sh", "-c", "echo SAFE"]); + expect(prepared.cmdText).toBe("echo SAFE"); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")( + "preserves dispatch-wrapper argv during approval hardening", + () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-dispatch-wrapper-")); + try { + const hardened = hardenApprovedExecutionPaths({ + approvedByAsk: true, + argv: ["env", "tr", "a", "b"], + shellCommand: null, + cwd: tmp, + }); + expect(hardened.ok).toBe(true); + if (!hardened.ok) { + throw new Error("unreachable"); + } + expect(hardened.argv).toEqual(["env", "tr", "a", "b"]); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")( + "pins direct PATH-token executable during approval hardening", + () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-direct-pin-")); + const binDir = path.join(tmp, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const link = path.join(binDir, "poccmd"); + fs.symlinkSync("/bin/echo", link); + const expected = fs.realpathSync(link); + const oldPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`; + try { + const hardened = hardenApprovedExecutionPaths({ + approvedByAsk: true, + argv: ["poccmd", "SAFE"], + shellCommand: null, + cwd: tmp, + }); + expect(hardened.ok).toBe(true); + if (!hardened.ok) { + throw new Error("unreachable"); + } + expect(hardened.argv).toEqual([expected, "SAFE"]); + } finally { + if (oldPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = oldPath; + } + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); +}); diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index cbcb4484ca8..15b8dad9f7e 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -56,6 +56,7 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean { export function hardenApprovedExecutionPaths(params: { approvedByAsk: boolean; argv: string[]; + shellCommand: string | null; cwd: string | undefined; }): { ok: true; argv: string[]; cwd: string | undefined } | { ok: false; message: string } { if (!params.approvedByAsk) { @@ -115,7 +116,19 @@ export function hardenApprovedExecutionPaths(params: { return { ok: true, argv: params.argv, cwd: hardenedCwd }; } + // Preserve shell-wrapper semantics. Rewriting argv[0] for wrappers can change + // runtime behavior (for example: env sh -c ... -> /bin/sh sh -c ...). + if (params.shellCommand !== null) { + return { ok: true, argv: params.argv, cwd: hardenedCwd }; + } + const resolution = resolveCommandResolutionFromArgv(params.argv, hardenedCwd); + // Preserve transparent wrapper semantics for approval-based execution. + // Pinning the effective executable while keeping wrapper argv shape can shift + // positional arguments and execute a different command than approved. + if ((resolution?.wrapperChain?.length ?? 0) > 0) { + return { ok: true, argv: params.argv, cwd: hardenedCwd }; + } const pinnedExecutable = resolution?.resolvedRealPath ?? resolution?.resolvedPath; if (!pinnedExecutable) { return { @@ -149,6 +162,7 @@ export function buildSystemRunApprovalPlan(params: { const hardening = hardenApprovedExecutionPaths({ approvedByAsk: true, argv: command.argv, + shellCommand: command.shellCommand, cwd: normalizeString(params.cwd) ?? undefined, }); if (!hardening.ok) { diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 03e1d0c10f4..3bfd806e88f 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -478,6 +478,43 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { fs.rmSync(tmp, { recursive: true, force: true }); } }); + + it.runIf(process.platform !== "win32")( + "preserves wrapper argv for approved env shell commands", + async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approved-wrapper-")); + const marker = path.join(tmp, "marker"); + const attackerScript = path.join(tmp, "sh"); + fs.writeFileSync(attackerScript, "#!/bin/sh\necho exploited > marker\n"); + fs.chmodSync(attackerScript, 0o755); + const runCommand = vi.fn(async (argv: string[]) => { + if (argv[0] === "/bin/sh" && argv[1] === "sh" && argv[2] === "-c") { + fs.writeFileSync(marker, "rewritten"); + } + return createLocalRunResult(); + }); + const sendInvokeResult = vi.fn(async () => {}); + try { + await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["env", "sh", "-c", "echo SAFE"], + cwd: tmp, + approved: true, + security: "allowlist", + ask: "on-miss", + runCommand, + sendInvokeResult, + }); + const runArgs = vi.mocked(runCommand).mock.calls[0]?.[0] as string[] | undefined; + expect(runArgs).toEqual(["env", "sh", "-c", "echo SAFE"]); + expect(fs.existsSync(marker)).toBe(false); + expectInvokeOk(sendInvokeResult); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + it("denies ./sh wrapper spoof in allowlist on-miss mode before execution", async () => { const marker = path.join(os.tmpdir(), `openclaw-wrapper-spoof-${process.pid}-${Date.now()}`); const runCommand = vi.fn(async () => { diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index f8bf21f651e..94173ec005f 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -300,6 +300,7 @@ async function evaluateSystemRunPolicyPhase( const hardenedPaths = hardenApprovedExecutionPaths({ approvedByAsk: policy.approvedByAsk, argv: parsed.argv, + shellCommand: parsed.shellCommand, cwd: parsed.cwd, }); if (!hardenedPaths.ok) {