From f55e51afb5e2c084453dd5d7c30ba32cdf087556 Mon Sep 17 00:00:00 2001 From: Nimrod Gutman Date: Thu, 19 Mar 2026 14:27:05 +0200 Subject: [PATCH] fix(macos): address wrapper review feedback --- .../OpenClaw/ExecCommandResolution.swift | 15 ++++++++--- .../ExecSystemRunCommandValidator.swift | 8 +----- .../OpenClaw/ExecWrapperResolution.swift | 1 - .../OpenClawIPCTests/ExecAllowlistTests.swift | 26 +++++++++++++++++++ 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift index 22af07f2ba2..f6daf9f7523 100644 --- a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift +++ b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift @@ -12,11 +12,20 @@ struct ExecCommandResolution { cwd: String?, env: [String: String]?) -> ExecCommandResolution? { + let effective = ExecWrapperResolution.unwrapDispatchWrappersForResolution(command) + guard let effectiveRaw = effective.first?.trimmingCharacters(in: .whitespacesAndNewlines), !effectiveRaw.isEmpty else { + return nil + } + let trimmedRaw = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" if !trimmedRaw.isEmpty, let token = self.parseFirstToken(trimmedRaw) { - return self.resolveExecutable(rawExecutable: token, cwd: cwd, env: env) + let normalizedToken = ExecWrapperResolution.normalizeExecutableToken(token) + let normalizedEffective = ExecWrapperResolution.normalizeExecutableToken(effectiveRaw) + if normalizedToken == normalizedEffective { + return self.resolveExecutable(rawExecutable: token, cwd: cwd, env: env) + } } - return self.resolve(command: command, cwd: cwd, env: env) + return self.resolveExecutable(rawExecutable: effectiveRaw, cwd: cwd, env: env) } static func resolveForAllowlist( @@ -126,7 +135,7 @@ struct ExecCommandResolution { patterns: inout [String], seen: inout Set) { - guard depth < 3, !command.isEmpty else { + guard depth <= ExecWrapperResolution.maxWrapperDepth, !command.isEmpty else { return } diff --git a/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift index 430c0a1344b..7ccbb5eb5c9 100644 --- a/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift +++ b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift @@ -94,13 +94,7 @@ enum ExecSystemRunCommandValidator { return normalizedRaw == previewCommand ? normalizedRaw : nil } - private static func hasEnvManipulationBeforeShellWrapper( - _ argv: [String], - depth: Int = 0, - envManipulationSeen: Bool = false) -> Bool - { - _ = depth - _ = envManipulationSeen + private static func hasEnvManipulationBeforeShellWrapper(_ argv: [String]) -> Bool { return ExecWrapperResolution.hasEnvManipulationBeforeShellWrapper(argv) } diff --git a/apps/macos/Sources/OpenClaw/ExecWrapperResolution.swift b/apps/macos/Sources/OpenClaw/ExecWrapperResolution.swift index 8f0dd8337b6..b57adce0371 100644 --- a/apps/macos/Sources/OpenClaw/ExecWrapperResolution.swift +++ b/apps/macos/Sources/OpenClaw/ExecWrapperResolution.swift @@ -229,7 +229,6 @@ enum ExecWrapperResolution { policyBlocked: true, blockedWrapper: wrapper) case let .unwrapped(wrapper, argv): - guard !argv.isEmpty else { break } wrappers.append(wrapper) if self.isSemanticDispatchWrapperUsage(wrapper: wrapper, argv: current) { return DispatchWrapperExecutionPlan( diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index 6a1ed72bd5f..23ad35da76c 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -264,6 +264,19 @@ struct ExecAllowlistTests { #expect(resolutions[1].executableName == "whoami") } + @Test func `resolve for allowlist unwraps direct dispatch wrappers with canonical raw command`() { + let command = ["/usr/bin/nice", "/usr/bin/printf", "ok"] + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: "/usr/bin/nice /usr/bin/printf ok", + 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 `resolve for allowlist unwraps env dispatch wrappers inside shell segments`() { let command = ["/bin/sh", "-lc", "env /usr/bin/touch /tmp/openclaw-allowlist-test"] let resolutions = ExecCommandResolution.resolveForAllowlist( @@ -377,6 +390,19 @@ struct ExecAllowlistTests { #expect(patterns.isEmpty) } + @Test func `allow always patterns support max transparent wrapper depth`() throws { + let tmp = try makeTempDirForTests() + let whoami = tmp.appendingPathComponent("whoami") + try makeExecutableForTests(at: whoami) + + let patterns = ExecCommandResolution.resolveAllowAlwaysPatterns( + command: ["nice", "nohup", "timeout", "5", "stdbuf", "-o", "L", "whoami"], + cwd: tmp.path, + env: ["PATH": "\(tmp.path):/usr/bin:/bin"]) + + #expect(patterns == [whoami.path]) + } + @Test func `match all requires every segment to match`() { let first = ExecCommandResolution( rawExecutable: "echo",