From 2b63592be57782c8946e521bc81286933f0f99c7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 09:51:51 +0100 Subject: [PATCH] fix: harden exec allowlist wrapper resolution --- CHANGELOG.md | 1 + .../OpenClaw/ExecCommandResolution.swift | 126 +++++++++++- .../OpenClawIPCTests/ExecAllowlistTests.swift | 24 +++ src/infra/exec-approvals-analysis.ts | 103 +++++++++- src/infra/exec-approvals.test.ts | 25 +++ src/infra/system-run-command.test.ts | 27 +++ src/infra/system-run-command.ts | 189 ++++++++++++++---- 7 files changed, 453 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89f10aa260f..2f4dc0bfa15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai - Security/Audit: add `openclaw security audit` detection for open group policies that expose runtime/filesystem tools without sandbox/workspace guards (`security.exposure.open_groups_with_runtime_or_fs`). - Security/Exec env: block request-scoped `HOME` and `ZDOTDIR` overrides in host exec env sanitizers (Node + macOS), preventing shell startup-file execution before allowlist-evaluated command bodies. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Hooks auth: normalize hook auth rate-limit client IP keys so IPv4 and IPv4-mapped IPv6 addresses share one throttle bucket, preventing dual-form auth-attempt budget bypasses. This ships in the next npm release. Thanks @aether-ai-agent for reporting. +- Security/Exec approvals: treat `env` and shell-dispatch wrappers as transparent during allowlist analysis on node-host and macOS companion paths so policy checks match the effective executable/inline shell payload instead of the wrapper binary, blocking wrapper-smuggled allowlist bypasses. This ships in the next npm release. Thanks @tdjackey for reporting. - Telegram/WSL2: disable `autoSelectFamily` by default on WSL2 and memoize WSL2 detection in Telegram network decision logic to avoid repeated sync `/proc/version` probes on fetch/send paths. (#21916) Thanks @MizukiMachine. - Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus. - Slack/Slash commands: preserve the Bolt app receiver when registering external select options handlers so monitor startup does not crash on runtimes that require bound `app.options` calls. (#23209) Thanks @0xgaia. diff --git a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift index 8910163456f..fc77509b97a 100644 --- a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift +++ b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift @@ -54,7 +54,8 @@ struct ExecCommandResolution: Sendable { } static func resolve(command: [String], cwd: String?, env: [String: String]?) -> ExecCommandResolution? { - guard let raw = command.first?.trimmingCharacters(in: .whitespacesAndNewlines), !raw.isEmpty else { + let effective = self.unwrapDispatchWrappersForResolution(command) + guard let raw = effective.first?.trimmingCharacters(in: .whitespacesAndNewlines), !raw.isEmpty else { return nil } return self.resolveExecutable(rawExecutable: raw, cwd: cwd, env: env) @@ -119,9 +120,19 @@ struct ExecCommandResolution: Sendable { let trimmedRaw = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" let preferredRaw = trimmedRaw.isEmpty ? nil : trimmedRaw - if ["sh", "bash", "zsh", "dash", "ksh"].contains(base0) { + if base0 == "env" { + guard let unwrapped = self.unwrapEnvInvocation(command) else { + return (false, nil) + } + return self.extractShellCommandFromArgv(command: unwrapped, rawCommand: rawCommand) + } + + if ["ash", "sh", "bash", "zsh", "dash", "ksh", "fish"].contains(base0) { let flag = command.count > 1 ? command[1].trimmingCharacters(in: .whitespacesAndNewlines) : "" - guard flag == "-lc" || flag == "-c" else { return (false, nil) } + let normalizedFlag = flag.lowercased() + guard normalizedFlag == "-lc" || normalizedFlag == "-c" || normalizedFlag == "--command" else { + return (false, nil) + } let payload = command.count > 2 ? command[2].trimmingCharacters(in: .whitespacesAndNewlines) : "" let normalized = preferredRaw ?? (payload.isEmpty ? nil : payload) return (true, normalized) @@ -139,9 +150,118 @@ struct ExecCommandResolution: Sendable { return (true, normalized) } + if ["powershell", "powershell.exe", "pwsh", "pwsh.exe"].contains(base0) { + for idx in 1.. Bool { + let pattern = #"^[A-Za-z_][A-Za-z0-9_]*=.*"# + return token.range(of: pattern, options: .regularExpression) != nil + } + + private static func unwrapEnvInvocation(_ command: [String]) -> [String]? { + var idx = 1 + var expectsOptionValue = false + while idx < command.count { + let token = command[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if expectsOptionValue { + expectsOptionValue = false + idx += 1 + continue + } + if token == "--" || token == "-" { + idx += 1 + break + } + if self.isEnvAssignment(token) { + idx += 1 + continue + } + if token.hasPrefix("-"), token != "-" { + let lower = token.lowercased() + let flag = lower.split(separator: "=", maxSplits: 1).first.map(String.init) ?? lower + if self.envFlagOptions.contains(flag) { + idx += 1 + continue + } + if self.envOptionsWithValue.contains(flag) { + if !lower.contains("=") { + expectsOptionValue = true + } + idx += 1 + continue + } + if lower.hasPrefix("-u") || + lower.hasPrefix("-c") || + lower.hasPrefix("-s") || + lower.hasPrefix("--unset=") || + lower.hasPrefix("--chdir=") || + lower.hasPrefix("--split-string=") || + lower.hasPrefix("--default-signal=") || + lower.hasPrefix("--ignore-signal=") || + lower.hasPrefix("--block-signal=") + { + idx += 1 + continue + } + return nil + } + break + } + guard idx < command.count else { return nil } + return Array(command[idx...]) + } + + private static func unwrapDispatchWrappersForResolution(_ command: [String]) -> [String] { + var current = command + var depth = 0 + while depth < 4 { + guard let token = current.first?.trimmingCharacters(in: .whitespacesAndNewlines), !token.isEmpty else { + break + } + guard self.basenameLower(token) == "env" else { + break + } + guard let unwrapped = self.unwrapEnvInvocation(current), !unwrapped.isEmpty else { + break + } + current = unwrapped + depth += 1 + } + return current + } + private enum ShellTokenContext { case unquoted case doubleQuoted diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index 17f4a1e24ce..e2705ce48db 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -171,6 +171,30 @@ struct ExecAllowlistTests { #expect(resolutions[0].executableName == "sh") } + @Test func resolveForAllowlistUnwrapsEnvShellWrapperChains() { + let command = ["/usr/bin/env", "/bin/sh", "-lc", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test"] + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.count == 2) + #expect(resolutions[0].executableName == "echo") + #expect(resolutions[1].executableName == "touch") + } + + @Test func resolveForAllowlistUnwrapsEnvToEffectiveDirectExecutable() { + let command = ["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"] + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.count == 1) + #expect(resolutions[0].resolvedPath == "/usr/bin/printf") + #expect(resolutions[0].executableName == "printf") + } + @Test func matchAllRequiresEverySegmentToMatch() { let first = ExecCommandResolution( rawExecutable: "echo", diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index 8eecd13a0a6..5914ea1b37b 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -12,6 +12,106 @@ export type CommandResolution = { executableName: string; }; +const ENV_OPTIONS_WITH_VALUE = new Set([ + "-u", + "--unset", + "-c", + "--chdir", + "-s", + "--split-string", + "--default-signal", + "--ignore-signal", + "--block-signal", +]); +const ENV_FLAG_OPTIONS = new Set(["-i", "--ignore-environment", "-0", "--null"]); + +function basenameLower(token: string): string { + const win = path.win32.basename(token); + const posix = path.posix.basename(token); + const base = win.length < posix.length ? win : posix; + return base.trim().toLowerCase(); +} + +function isEnvAssignment(token: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token); +} + +function unwrapEnvInvocation(argv: string[]): string[] | null { + let idx = 1; + let expectsOptionValue = false; + while (idx < argv.length) { + const token = argv[idx]?.trim() ?? ""; + if (!token) { + idx += 1; + continue; + } + if (expectsOptionValue) { + expectsOptionValue = false; + idx += 1; + continue; + } + if (token === "--" || token === "-") { + idx += 1; + break; + } + if (isEnvAssignment(token)) { + idx += 1; + continue; + } + if (token.startsWith("-") && token !== "-") { + const lower = token.toLowerCase(); + const [flag] = lower.split("=", 2); + if (ENV_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + if (ENV_OPTIONS_WITH_VALUE.has(flag)) { + if (!lower.includes("=")) { + expectsOptionValue = true; + } + idx += 1; + continue; + } + if ( + lower.startsWith("-u") || + lower.startsWith("-c") || + lower.startsWith("-s") || + lower.startsWith("--unset=") || + lower.startsWith("--chdir=") || + lower.startsWith("--split-string=") || + lower.startsWith("--default-signal=") || + lower.startsWith("--ignore-signal=") || + lower.startsWith("--block-signal=") + ) { + idx += 1; + continue; + } + return null; + } + break; + } + return idx < argv.length ? argv.slice(idx) : null; +} + +function unwrapDispatchWrappersForResolution(argv: string[]): string[] { + let current = argv; + for (let depth = 0; depth < 4; depth += 1) { + const token0 = current[0]?.trim(); + if (!token0) { + break; + } + if (basenameLower(token0) !== "env") { + break; + } + const unwrapped = unwrapEnvInvocation(current); + if (!unwrapped || unwrapped.length === 0) { + break; + } + current = unwrapped; + } + return current; +} + function isExecutableFile(filePath: string): boolean { try { const stat = fs.statSync(filePath); @@ -101,7 +201,8 @@ export function resolveCommandResolutionFromArgv( cwd?: string, env?: NodeJS.ProcessEnv, ): CommandResolution | null { - const rawExecutable = argv[0]?.trim(); + const effectiveArgv = unwrapDispatchWrappersForResolution(argv); + const rawExecutable = effectiveArgv[0]?.trim(); if (!rawExecutable) { return null; } diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 993c43e2a3f..9a8cdc19d8b 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -18,6 +18,7 @@ import { normalizeSafeBins, requiresExecApproval, resolveCommandResolution, + resolveCommandResolutionFromArgv, resolveAllowAlwaysPatterns, resolveExecApprovals, resolveExecApprovalsFromFile, @@ -241,6 +242,30 @@ describe("exec approvals command resolution", () => { } } }); + + it("unwraps env wrapper argv to resolve the effective executable", () => { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const exeName = process.platform === "win32" ? "rg.exe" : "rg"; + const exe = path.join(binDir, exeName); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + + const resolution = resolveCommandResolutionFromArgv( + ["/usr/bin/env", "FOO=bar", "rg", "-n", "needle"], + undefined, + makePathEnv(binDir), + ); + expect(resolution?.resolvedPath).toBe(exe); + expect(resolution?.executableName).toBe(exeName); + }); + + it("unwraps env wrapper with shell inner executable", () => { + const resolution = resolveCommandResolutionFromArgv(["/usr/bin/env", "bash", "-lc", "echo hi"]); + expect(resolution?.rawExecutable).toBe("bash"); + expect(resolution?.executableName.toLowerCase()).toContain("bash"); + }); }); describe("exec approvals shell parsing", () => { diff --git a/src/infra/system-run-command.test.ts b/src/infra/system-run-command.test.ts index 74dce641fdc..22d23d889ec 100644 --- a/src/infra/system-run-command.test.ts +++ b/src/infra/system-run-command.test.ts @@ -29,6 +29,25 @@ describe("system run command helpers", () => { expect(extractShellCommandFromArgv(["cmd.exe", "/d", "/s", "/c", "echo hi"])).toBe("echo hi"); }); + test("extractShellCommandFromArgv unwraps /usr/bin/env shell wrappers", () => { + expect(extractShellCommandFromArgv(["/usr/bin/env", "bash", "-lc", "echo hi"])).toBe("echo hi"); + expect(extractShellCommandFromArgv(["/usr/bin/env", "FOO=bar", "zsh", "-c", "echo hi"])).toBe( + "echo hi", + ); + }); + + test("extractShellCommandFromArgv supports fish and pwsh wrappers", () => { + expect(extractShellCommandFromArgv(["fish", "-c", "echo hi"])).toBe("echo hi"); + expect(extractShellCommandFromArgv(["pwsh", "-Command", "Get-Date"])).toBe("Get-Date"); + }); + + test("extractShellCommandFromArgv ignores env wrappers when no shell wrapper follows", () => { + expect(extractShellCommandFromArgv(["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"])).toBe( + null, + ); + expect(extractShellCommandFromArgv(["/usr/bin/env", "FOO=bar"])).toBe(null); + }); + test("extractShellCommandFromArgv includes trailing cmd.exe args after /c", () => { expect(extractShellCommandFromArgv(["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"])).toBe( "echo SAFE&&whoami", @@ -63,6 +82,14 @@ describe("system run command helpers", () => { expect(res.ok).toBe(true); }); + test("validateSystemRunCommandConsistency accepts rawCommand matching env shell wrapper argv", () => { + const res = validateSystemRunCommandConsistency({ + argv: ["/usr/bin/env", "bash", "-lc", "echo hi"], + rawCommand: "echo hi", + }); + expect(res.ok).toBe(true); + }); + test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => { expectRawCommandMismatch({ argv: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], diff --git a/src/infra/system-run-command.ts b/src/infra/system-run-command.ts index 4d61c2e2464..a8b7c3050ee 100644 --- a/src/infra/system-run-command.ts +++ b/src/infra/system-run-command.ts @@ -33,6 +33,156 @@ function basenameLower(token: string): string { return base.trim().toLowerCase(); } +const POSIX_SHELL_WRAPPERS = new Set(["ash", "bash", "dash", "fish", "ksh", "sh", "zsh"]); +const WINDOWS_CMD_WRAPPERS = new Set(["cmd.exe", "cmd"]); +const POWERSHELL_WRAPPERS = new Set(["powershell", "powershell.exe", "pwsh", "pwsh.exe"]); +const ENV_OPTIONS_WITH_VALUE = new Set([ + "-u", + "--unset", + "-c", + "--chdir", + "-s", + "--split-string", + "--default-signal", + "--ignore-signal", + "--block-signal", +]); +const ENV_FLAG_OPTIONS = new Set(["-i", "--ignore-environment", "-0", "--null"]); + +function isEnvAssignment(token: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token); +} + +function unwrapEnvInvocation(argv: string[]): string[] | null { + let idx = 1; + let expectsOptionValue = false; + while (idx < argv.length) { + const token = argv[idx]?.trim() ?? ""; + if (!token) { + idx += 1; + continue; + } + if (expectsOptionValue) { + expectsOptionValue = false; + idx += 1; + continue; + } + if (token === "--" || token === "-") { + idx += 1; + break; + } + if (isEnvAssignment(token)) { + idx += 1; + continue; + } + if (token.startsWith("-") && token !== "-") { + const lower = token.toLowerCase(); + const [flag] = lower.split("=", 2); + if (ENV_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + if (ENV_OPTIONS_WITH_VALUE.has(flag)) { + if (!lower.includes("=")) { + expectsOptionValue = true; + } + idx += 1; + continue; + } + if ( + lower.startsWith("-u") || + lower.startsWith("-c") || + lower.startsWith("-s") || + lower.startsWith("--unset=") || + lower.startsWith("--chdir=") || + lower.startsWith("--split-string=") || + lower.startsWith("--default-signal=") || + lower.startsWith("--ignore-signal=") || + lower.startsWith("--block-signal=") + ) { + idx += 1; + continue; + } + return null; + } + break; + } + return idx < argv.length ? argv.slice(idx) : null; +} + +function extractPosixShellInlineCommand(argv: string[]): string | null { + const flag = argv[1]?.trim(); + if (!flag) { + return null; + } + const lower = flag.toLowerCase(); + if (lower !== "-lc" && lower !== "-c" && lower !== "--command") { + return null; + } + const cmd = argv[2]?.trim(); + return cmd ? cmd : null; +} + +function extractCmdInlineCommand(argv: string[]): string | null { + const idx = argv.findIndex((item) => String(item).trim().toLowerCase() === "/c"); + if (idx === -1) { + return null; + } + const tail = argv.slice(idx + 1).map((item) => String(item)); + if (tail.length === 0) { + return null; + } + const cmd = tail.join(" ").trim(); + return cmd.length > 0 ? cmd : null; +} + +function extractPowerShellInlineCommand(argv: string[]): string | null { + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]?.trim(); + if (!token) { + continue; + } + const lower = token.toLowerCase(); + if (lower === "--") { + break; + } + if (lower === "-c" || lower === "-command" || lower === "--command") { + const cmd = argv[i + 1]?.trim(); + return cmd ? cmd : null; + } + } + return null; +} + +function extractShellCommandFromArgvInternal(argv: string[], depth: number): string | null { + if (depth >= 4) { + return null; + } + const token0 = argv[0]?.trim(); + if (!token0) { + return null; + } + + const base0 = basenameLower(token0); + if (base0 === "env") { + const unwrapped = unwrapEnvInvocation(argv); + if (!unwrapped) { + return null; + } + return extractShellCommandFromArgvInternal(unwrapped, depth + 1); + } + if (POSIX_SHELL_WRAPPERS.has(base0)) { + return extractPosixShellInlineCommand(argv); + } + if (WINDOWS_CMD_WRAPPERS.has(base0)) { + return extractCmdInlineCommand(argv); + } + if (POWERSHELL_WRAPPERS.has(base0)) { + return extractPowerShellInlineCommand(argv); + } + return null; +} + export function formatExecCommand(argv: string[]): string { return argv .map((arg) => { @@ -50,44 +200,7 @@ export function formatExecCommand(argv: string[]): string { } export function extractShellCommandFromArgv(argv: string[]): string | null { - const token0 = argv[0]?.trim(); - if (!token0) { - return null; - } - - const base0 = basenameLower(token0); - - // POSIX-style shells: sh -lc "" - if ( - base0 === "sh" || - base0 === "bash" || - base0 === "zsh" || - base0 === "dash" || - base0 === "ksh" - ) { - const flag = argv[1]?.trim(); - if (flag !== "-lc" && flag !== "-c") { - return null; - } - const cmd = argv[2]; - return typeof cmd === "string" ? cmd : null; - } - - // Windows cmd.exe: cmd.exe /d /s /c "" - if (base0 === "cmd.exe" || base0 === "cmd") { - const idx = argv.findIndex((item) => String(item).trim().toLowerCase() === "/c"); - if (idx === -1) { - return null; - } - const tail = argv.slice(idx + 1).map((item) => String(item)); - if (tail.length === 0) { - return null; - } - const cmd = tail.join(" ").trim(); - return cmd.length > 0 ? cmd : null; - } - - return null; + return extractShellCommandFromArgvInternal(argv, 0); } export function validateSystemRunCommandConsistency(params: {