From 1d45febea8ac67d254a1316d03ff65f3fbcaf613 Mon Sep 17 00:00:00 2001 From: Nimrod Gutman Date: Thu, 19 Mar 2026 15:31:26 +0200 Subject: [PATCH] fix(macos): fail closed on env-modified shell wrappers --- .../OpenClaw/ExecCommandResolution.swift | 11 ++++++++++ .../OpenClawIPCTests/ExecAllowlistTests.swift | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift index 921e5ebf658..bdb7bb7286b 100644 --- a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift +++ b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift @@ -77,6 +77,11 @@ struct ExecCommandResolution { guard depth <= ExecWrapperResolution.maxWrapperDepth, !command.isEmpty else { return [] } + if ExecWrapperResolution.hasEnvManipulationBeforeShellWrapper(command) { + // Fail closed for semantic env wrappers that can alter shell lookup + // semantics before we would analyze inner shell payloads. + return [] + } let shell = ExecShellWrapperParser.extract(command: command, rawCommand: rawCommand) if shell.isWrapper { @@ -194,6 +199,12 @@ struct ExecCommandResolution { guard depth <= Self.maxAllowAlwaysTraversalDepth, !command.isEmpty else { return } + if ExecWrapperResolution.hasEnvManipulationBeforeShellWrapper(command) { + // Mirror the conservative node-host policy for env-modified shell + // launches: require explicit approval each time instead of persisting + // an inner-executable pattern that the modified environment can subvert. + return + } // Allow-always persistence intentionally peels known dispatch wrappers // directly so approvals stay scoped to the launched executable instead of diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index b6c25818e48..90aaccaab79 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -382,6 +382,17 @@ struct ExecAllowlistTests { #expect(resolutions[0].executableName == "env") } + @Test func `resolve for allowlist fails closed on env manipulation before shell wrapper`() { + let command = ["/usr/bin/env", "PATH=/tmp", "/bin/sh", "-lc", "whoami"] + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + + #expect(resolutions.isEmpty) + } + @Test func `resolve for allowlist preserves env wrapper with modifiers`() { let command = ["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"] let resolutions = ExecCommandResolution.resolveForAllowlist( @@ -435,6 +446,15 @@ struct ExecAllowlistTests { #expect(patterns == ["/usr/bin/printf"]) } + @Test func `allow always patterns fail closed on env manipulation before shell wrapper`() { + let patterns = ExecCommandResolution.resolveAllowAlwaysPatterns( + command: ["/usr/bin/env", "PATH=/tmp", "/bin/sh", "-lc", "whoami"], + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + + #expect(patterns.isEmpty) + } + @Test func `allow always patterns unwrap dispatch wrappers before shell wrappers`() throws { let tmp = try makeTempDirForTests() let whoami = tmp.appendingPathComponent("whoami")