From fc065b2693bfaeff795a88d080ff5cf30fe8addf Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Fri, 8 May 2026 10:18:41 +0530 Subject: [PATCH] Harden macOS shell wrapper allowlist parsing [AI] (#78518) * fix: harden shell wrapper allowlist parsing * fix: harden shell wrapper approval binding * docs: add changelog entry for PR merge --------- Co-authored-by: Ishaan --- CHANGELOG.md | 1 + .../OpenClaw/ExecApprovalEvaluation.swift | 3 +- .../OpenClaw/ExecCommandResolution.swift | 27 +- .../OpenClaw/ExecInlineCommandParser.swift | 278 ++++++++++++++++++ .../OpenClaw/ExecShellWrapperParser.swift | 126 +++++++- .../ExecSystemRunCommandValidator.swift | 50 +--- .../OpenClawIPCTests/ExecAllowlistTests.swift | 175 ++++++++++- .../ExecSystemRunCommandValidatorTests.swift | 42 +++ src/infra/command-explainer/extract.ts | 17 +- src/infra/exec-approvals-allow-always.test.ts | 119 +++++--- src/infra/exec-approvals-allowlist.ts | 42 +-- src/infra/exec-wrapper-resolution.test.ts | 6 +- src/infra/exec-wrapper-resolution.ts | 2 + src/infra/exec-wrapper-trust-plan.test.ts | 37 ++- src/infra/exec-wrapper-trust-plan.ts | 13 +- src/infra/shell-inline-command.test.ts | 14 + src/infra/shell-inline-command.ts | 195 +++++++++++- src/infra/shell-wrapper-resolution.ts | 107 ++++++- src/infra/system-run-command.test.ts | 79 ++++- src/infra/system-run-command.ts | 13 +- src/node-host/invoke-system-run-plan.ts | 7 + src/node-host/invoke-system-run.test.ts | 2 +- .../fixtures/system-run-command-contract.json | 49 ++- 23 files changed, 1200 insertions(+), 204 deletions(-) create mode 100644 apps/macos/Sources/OpenClaw/ExecInlineCommandParser.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 66826cb9dcd..ede73371f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,6 +172,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Harden macOS shell wrapper allowlist parsing [AI]. (#78518) Thanks @pgondhi987. - Gateway/macOS: `openclaw gateway stop` now uses `launchctl bootout` by default instead of unconditionally calling `launchctl disable`, so KeepAlive auto-recovery still works after unexpected crashes; use the new `--disable` flag to opt into the persistent-disable behavior when a manual stop should survive reboots. Fixes #77934. Thanks @bmoran1022. - Gateway/macOS: `repairLaunchAgentBootstrap` no longer kickstarts an already-running LaunchAgent, preventing unnecessary service restarts and session disconnects when repair runs against a healthy gateway. Fixes #77428. Thanks @ramitrkar-hash. - Gateway/macOS: `openclaw gateway stop --disable` now persists the LaunchAgent disable bit even after a previous bootout left the service not loaded, keeping the explicit stay-down path reliable. (#78412) Thanks @wdeveloper16. diff --git a/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift b/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift index e39db84534f..d358082258c 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift @@ -43,7 +43,8 @@ enum ExecApprovalEvaluator { let allowAlwaysPatterns = ExecCommandResolution.resolveAllowAlwaysPatterns( command: command, cwd: cwd, - env: env) + env: env, + rawCommand: allowlistRawCommand) let allowlistMatches = security == .allowlist ? ExecAllowlistMatcher.matchAll(entries: approvals.allowlist, resolutions: allowlistResolutions) : [] diff --git a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift index dff2d59cfec..df2562aed7b 100644 --- a/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift +++ b/apps/macos/Sources/OpenClaw/ExecCommandResolution.swift @@ -27,7 +27,7 @@ struct ExecCommandResolution { { // Allowlist resolution must follow actual argv execution for wrappers. // `rawCommand` is caller-supplied display text and may be canonicalized. - let shell = ExecShellWrapperParser.extract(command: command, rawCommand: nil) + let shell = ExecShellWrapperParser.extractForAllowlist(command: command, rawCommand: rawCommand) if shell.isWrapper { // Fail closed when env modifiers precede a shell wrapper. This mirrors // system-run binding behavior where such invocations must stay bound to @@ -68,7 +68,8 @@ struct ExecCommandResolution { static func resolveAllowAlwaysPatterns( command: [String], cwd: String?, - env: [String: String]?) -> [String] + env: [String: String]?, + rawCommand: String? = nil) -> [String] { var patterns: [String] = [] var seen = Set() @@ -76,6 +77,7 @@ struct ExecCommandResolution { command: command, cwd: cwd, env: env, + rawCommand: rawCommand, depth: 0, patterns: &patterns, seen: &seen) @@ -152,6 +154,7 @@ struct ExecCommandResolution { command: [String], cwd: String?, env: [String: String]?, + rawCommand: String?, depth: Int, patterns: inout [String], seen: inout Set) @@ -162,13 +165,19 @@ struct ExecCommandResolution { if let token0 = command.first?.trimmingCharacters(in: .whitespacesAndNewlines), ExecCommandToken.basenameLower(token0) == "env", - let envUnwrapped = ExecEnvInvocationUnwrapper.unwrap(command), - !envUnwrapped.isEmpty + let envUnwrapped = ExecEnvInvocationUnwrapper.unwrapWithMetadata(command), + !envUnwrapped.command.isEmpty { + if envUnwrapped.usesModifiers, + self.isAllowlistShellWrapper(command: envUnwrapped.command, rawCommand: rawCommand) + { + return + } self.collectAllowAlwaysPatterns( - command: envUnwrapped, + command: envUnwrapped.command, cwd: cwd, env: env, + rawCommand: rawCommand, depth: depth + 1, patterns: &patterns, seen: &seen) @@ -180,13 +189,14 @@ struct ExecCommandResolution { command: shellMultiplexer, cwd: cwd, env: env, + rawCommand: rawCommand, depth: depth + 1, patterns: &patterns, seen: &seen) return } - let shell = ExecShellWrapperParser.extract(command: command, rawCommand: nil) + let shell = ExecShellWrapperParser.extractForAllowlist(command: command, rawCommand: rawCommand) if shell.isWrapper { guard let shellCommand = shell.command, let segments = self.splitShellCommandChain(shellCommand) @@ -202,6 +212,7 @@ struct ExecCommandResolution { command: tokens, cwd: cwd, env: env, + rawCommand: nil, depth: depth + 1, patterns: &patterns, seen: &seen) @@ -218,6 +229,10 @@ struct ExecCommandResolution { patterns.append(pattern) } + private static func isAllowlistShellWrapper(command: [String], rawCommand: String?) -> Bool { + ExecShellWrapperParser.extractForAllowlist(command: command, rawCommand: rawCommand).isWrapper + } + private static func unwrapShellMultiplexerInvocation(_ argv: [String]) -> [String]? { guard let token0 = argv.first?.trimmingCharacters(in: .whitespacesAndNewlines), !token0.isEmpty else { return nil diff --git a/apps/macos/Sources/OpenClaw/ExecInlineCommandParser.swift b/apps/macos/Sources/OpenClaw/ExecInlineCommandParser.swift new file mode 100644 index 00000000000..a2a0cf15dda --- /dev/null +++ b/apps/macos/Sources/OpenClaw/ExecInlineCommandParser.swift @@ -0,0 +1,278 @@ +import Foundation + +enum ExecInlineCommandParser { + struct Match { + let tokenIndex: Int + let inlineCommand: String? + let valueTokenOffset: Int + + init(tokenIndex: Int, inlineCommand: String?, valueTokenOffset: Int = 1) { + self.tokenIndex = tokenIndex + self.inlineCommand = inlineCommand + self.valueTokenOffset = valueTokenOffset + } + } + + private struct CombinedCommandFlag { + let attachedCommand: String? + let separateValueCount: Int + } + + private static let posixShellOptionsWithSeparateValues = Set([ + "--init-file", + "--rcfile", + "-O", + "-o", + "+O", + "+o", + ]) + + static func hasPosixInteractiveStartupBeforeInlineCommand( + _ argv: [String], + flags: Set) -> Bool + { + var idx = 1 + var sawInteractiveMode = false + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if token == "--" { + return false + } + if self.isPosixInteractiveModeOption(token) { + sawInteractiveMode = true + } + if flags.contains(token) || self.isCombinedCommandFlag(token) { + return sawInteractiveMode + } + if !token.hasPrefix("-"), !token.hasPrefix("+") { + return false + } + let combinedValueCount = self.combinedSeparateValueOptionCount(token) + if combinedValueCount > 0 { + idx += 1 + combinedValueCount + continue + } + if self.consumesSeparateValue(token) { + idx += 2 + continue + } + idx += 1 + } + return false + } + + static func hasPosixLoginStartupBeforeInlineCommand( + _ argv: [String], + flags: Set) -> Bool + { + var idx = 1 + var sawLoginMode = false + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if token == "--" { + return false + } + if token == "--login" || self.isPosixShortOption(token, containing: "l") { + sawLoginMode = true + } + if flags.contains(token) || self.isCombinedCommandFlag(token) { + return sawLoginMode + } + if !token.hasPrefix("-"), !token.hasPrefix("+") { + return false + } + let combinedValueCount = self.combinedSeparateValueOptionCount(token) + if combinedValueCount > 0 { + idx += 1 + combinedValueCount + continue + } + if self.consumesSeparateValue(token) { + idx += 2 + continue + } + idx += 1 + } + return false + } + + static func hasFishInitCommandOption(_ argv: [String]) -> Bool { + var idx = 1 + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if token == "--" { + return false + } + if token == "-C" || token == "--init-command" { + return true + } + if token.hasPrefix("-C"), token != "-C" { + return true + } + if token.hasPrefix("--init-command=") { + return true + } + if !token.hasPrefix("-"), !token.hasPrefix("+") { + return false + } + idx += 1 + } + return false + } + + static func hasFishAttachedCommandOption(_ argv: [String]) -> Bool { + var idx = 1 + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if token == "--" { + return false + } + if token.hasPrefix("-c"), token != "-c" { + return true + } + if !token.hasPrefix("-"), !token.hasPrefix("+") { + return false + } + idx += 1 + } + return false + } + + static func findMatch( + _ argv: [String], + flags: Set, + allowCombinedC: Bool) -> Match? + { + var idx = 1 + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if token == "--" { + break + } + let comparableToken = allowCombinedC ? token : token.lowercased() + if flags.contains(comparableToken) { + return Match(tokenIndex: idx, inlineCommand: nil) + } + if allowCombinedC, let combined = self.parseCombinedCommandFlag(token) { + if let attachedCommand = combined.attachedCommand { + return Match(tokenIndex: idx, inlineCommand: attachedCommand, valueTokenOffset: 0) + } + return Match( + tokenIndex: idx, + inlineCommand: nil, + valueTokenOffset: 1 + combined.separateValueCount) + } + if allowCombinedC, !token.hasPrefix("-"), !token.hasPrefix("+") { + break + } + let combinedValueCount = allowCombinedC ? self.combinedSeparateValueOptionCount(token) : 0 + if combinedValueCount > 0 { + idx += 1 + combinedValueCount + continue + } + if allowCombinedC, self.consumesSeparateValue(token) { + idx += 2 + continue + } + idx += 1 + } + return nil + } + + static func extractInlineCommand( + _ argv: [String], + flags: Set, + allowCombinedC: Bool) -> String? + { + guard let match = self.findMatch(argv, flags: flags, allowCombinedC: allowCombinedC) else { + return nil + } + if let inlineCommand = match.inlineCommand { + return inlineCommand + } + let nextIndex = match.tokenIndex + match.valueTokenOffset + let payload = nextIndex < argv.count + ? argv[nextIndex].trimmingCharacters(in: .whitespacesAndNewlines) + : "" + return payload.isEmpty ? nil : payload + } + + private static func isCombinedCommandFlag(_ token: String) -> Bool { + self.parseCombinedCommandFlag(token) != nil + } + + private static func parseCombinedCommandFlag(_ token: String) -> CombinedCommandFlag? { + let chars = Array(token) + guard chars.count >= 2, chars[0] == "-", chars[1] != "-" else { + return nil + } + let optionChars = Array(chars.dropFirst()) + guard let commandFlagIndex = optionChars.firstIndex(of: "c") else { + return nil + } + if optionChars.contains("-") { + return nil + } + let suffix = String(optionChars.dropFirst(commandFlagIndex + 1)) + if !suffix.isEmpty, + suffix.range(of: #"[^A-Za-z]"#, options: .regularExpression) != nil + { + return CombinedCommandFlag(attachedCommand: suffix, separateValueCount: 0) + } + let separateValueCount = optionChars.reduce(0) { count, char in + count + ((char == "o" || char == "O") ? 1 : 0) + } + return CombinedCommandFlag(attachedCommand: nil, separateValueCount: separateValueCount) + } + + private static func combinedSeparateValueOptionCount(_ token: String) -> Int { + let chars = Array(token) + guard chars.count >= 2, chars[0] == "-" || chars[0] == "+", chars[1] != "-" else { + return 0 + } + if chars.dropFirst().contains("-") { + return 0 + } + return chars.dropFirst().reduce(0) { count, char in + count + ((char == "o" || char == "O") ? 1 : 0) + } + } + + private static func consumesSeparateValue(_ token: String) -> Bool { + self.posixShellOptionsWithSeparateValues.contains(token) + } + + private static func isPosixInteractiveModeOption(_ token: String) -> Bool { + token == "--interactive" || self.isPosixShortOption(token, containing: "i") + } + + private static func isPosixShortOption(_ token: String, containing option: Character) -> Bool { + let chars = Array(token) + guard chars.count >= 2, chars[0] == "-", chars[1] != "-" else { + return false + } + if chars.dropFirst().contains("-") { + return false + } + return chars.dropFirst().contains(option) + } +} diff --git a/apps/macos/Sources/OpenClaw/ExecShellWrapperParser.swift b/apps/macos/Sources/OpenClaw/ExecShellWrapperParser.swift index 06851a7d065..0533f2fc3d4 100644 --- a/apps/macos/Sources/OpenClaw/ExecShellWrapperParser.swift +++ b/apps/macos/Sources/OpenClaw/ExecShellWrapperParser.swift @@ -6,9 +6,10 @@ enum ExecShellWrapperParser { let command: String? static let notWrapper = ParsedShellWrapper(isWrapper: false, command: nil) + static let blockedWrapper = ParsedShellWrapper(isWrapper: true, command: nil) } - private enum Kind { + private enum Kind: Equatable { case posix case cmd case powershell @@ -27,14 +28,34 @@ enum ExecShellWrapperParser { WrapperSpec(kind: .cmd, names: ["cmd.exe", "cmd"]), WrapperSpec(kind: .powershell, names: ["powershell", "powershell.exe", "pwsh", "pwsh.exe"]), ] + private static let loginStartupShellNames = Set(["ash", "bash", "dash", "fish", "ksh", "sh", "zsh"]) static func extract(command: [String], rawCommand: String?) -> ParsedShellWrapper { let trimmedRaw = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" let preferredRaw = trimmedRaw.isEmpty ? nil : trimmedRaw - return self.extract(command: command, preferredRaw: preferredRaw, depth: 0) + return self.extract( + command: command, + preferredRaw: preferredRaw, + failClosedOnStartupWrappers: false, + depth: 0) } - private static func extract(command: [String], preferredRaw: String?, depth: Int) -> ParsedShellWrapper { + static func extractForAllowlist(command: [String], rawCommand: String?) -> ParsedShellWrapper { + let trimmedRaw = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + let preferredRaw = trimmedRaw.isEmpty ? nil : trimmedRaw + return self.extract( + command: command, + preferredRaw: preferredRaw, + failClosedOnStartupWrappers: true, + depth: 0) + } + + private static func extract( + command: [String], + preferredRaw: String?, + failClosedOnStartupWrappers: Bool, + depth: Int) -> ParsedShellWrapper + { guard depth < ExecEnvInvocationUnwrapper.maxWrapperDepth else { return .notWrapper } @@ -47,19 +68,96 @@ enum ExecShellWrapperParser { guard let unwrapped = ExecEnvInvocationUnwrapper.unwrap(command) else { return .notWrapper } - return self.extract(command: unwrapped, preferredRaw: preferredRaw, depth: depth + 1) + return self.extract( + command: unwrapped, + preferredRaw: preferredRaw, + failClosedOnStartupWrappers: failClosedOnStartupWrappers, + depth: depth + 1) } guard let spec = self.wrapperSpecs.first(where: { $0.names.contains(base0) }) else { return .notWrapper } + if spec.kind == .posix, + base0 == "fish", + ExecInlineCommandParser.hasFishAttachedCommandOption(command) + { + return .blockedWrapper + } + let includeLegacyLoginInlineForm = failClosedOnStartupWrappers && + !self.legacyLoginInlinePayloadMatchesRaw( + command: command, + spec: spec, + base0: base0, + preferredRaw: preferredRaw) + if self.startupWrapperRequiresFullArgv( + command: command, + spec: spec, + base0: base0, + includeLegacyLoginInlineForm: includeLegacyLoginInlineForm) + { + return .blockedWrapper + } guard let payload = self.extractPayload(command: command, spec: spec) else { return .notWrapper } - let normalized = preferredRaw ?? payload + let normalized = failClosedOnStartupWrappers ? payload : preferredRaw ?? payload return ParsedShellWrapper(isWrapper: true, command: normalized) } + private static func startupWrapperRequiresFullArgv( + command: [String], + spec: WrapperSpec, + base0: String, + includeLegacyLoginInlineForm: Bool) -> Bool + { + guard spec.kind == .posix else { + return false + } + if base0 == "fish", + ExecInlineCommandParser.hasFishInitCommandOption(command) + { + return true + } + if self.loginStartupShellNames.contains(base0), + ExecInlineCommandParser.hasPosixLoginStartupBeforeInlineCommand( + command, + flags: self.posixInlineFlags) + { + return includeLegacyLoginInlineForm || !self.isLegacyShLoginInlineForm(command, base0: base0) + } + return ExecInlineCommandParser.hasPosixInteractiveStartupBeforeInlineCommand( + command, + flags: self.posixInlineFlags) + } + + private static func isLegacyLoginInlineForm(_ command: [String]) -> Bool { + guard command.count > 1 else { + return false + } + return command[1].trimmingCharacters(in: .whitespacesAndNewlines) == "-lc" + } + + private static func isLegacyShLoginInlineForm(_ command: [String], base0: String) -> Bool { + base0 == "sh" && self.isLegacyLoginInlineForm(command) + } + + private static func legacyLoginInlinePayloadMatchesRaw( + command: [String], + spec: WrapperSpec, + base0: String, + preferredRaw: String?) -> Bool + { + guard let preferredRaw, + base0 == "sh", + self.isLegacyLoginInlineForm(command), + let payload = self.extractPayload(command: command, spec: spec) + else { + return false + } + return payload == preferredRaw.trimmingCharacters(in: .whitespacesAndNewlines) + } + private static func extractPayload(command: [String], spec: WrapperSpec) -> String? { switch spec.kind { case .posix: @@ -72,12 +170,10 @@ enum ExecShellWrapperParser { } private static func extractPosixInlineCommand(_ command: [String]) -> String? { - let flag = command.count > 1 ? command[1].trimmingCharacters(in: .whitespacesAndNewlines) : "" - guard self.posixInlineFlags.contains(flag.lowercased()) else { - return nil - } - let payload = command.count > 2 ? command[2].trimmingCharacters(in: .whitespacesAndNewlines) : "" - return payload.isEmpty ? nil : payload + ExecInlineCommandParser.extractInlineCommand( + command, + flags: self.posixInlineFlags, + allowCombinedC: true) } private static func extractCmdInlineCommand(_ command: [String]) -> String? { @@ -97,10 +193,10 @@ enum ExecShellWrapperParser { if token.isEmpty { continue } if token == "--" { break } if self.powershellInlineFlags.contains(token) { - let payload = idx + 1 < command.count - ? command[idx + 1].trimmingCharacters(in: .whitespacesAndNewlines) - : "" - return payload.isEmpty ? nil : payload + return ExecInlineCommandParser.extractInlineCommand( + command, + flags: self.powershellInlineFlags, + allowCombinedC: false) } } return nil diff --git a/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift index f10880d698e..177a6bed515 100644 --- a/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift +++ b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift @@ -326,40 +326,12 @@ enum ExecSystemRunCommandValidator { return current } - private struct InlineCommandTokenMatch { - var tokenIndex: Int - var inlineCommand: String? - } - private static func findInlineCommandTokenMatch( _ argv: [String], flags: Set, - allowCombinedC: Bool) -> InlineCommandTokenMatch? + allowCombinedC: Bool) -> ExecInlineCommandParser.Match? { - var idx = 1 - while idx < argv.count { - let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) - if token.isEmpty { - idx += 1 - continue - } - let lower = token.lowercased() - if lower == "--" { - break - } - if flags.contains(lower) { - return InlineCommandTokenMatch(tokenIndex: idx, inlineCommand: nil) - } - if allowCombinedC, let inlineOffset = self.combinedCommandInlineOffset(token) { - let inline = String(token.dropFirst(inlineOffset)) - .trimmingCharacters(in: .whitespacesAndNewlines) - return InlineCommandTokenMatch( - tokenIndex: idx, - inlineCommand: inline.isEmpty ? nil : inline) - } - idx += 1 - } - return nil + ExecInlineCommandParser.findMatch(argv, flags: flags, allowCombinedC: allowCombinedC) } private static func resolveInlineCommandTokenIndex( @@ -373,24 +345,10 @@ enum ExecSystemRunCommandValidator { if match.inlineCommand != nil { return match.tokenIndex } - let nextIndex = match.tokenIndex + 1 + let nextIndex = match.tokenIndex + match.valueTokenOffset return nextIndex < argv.count ? nextIndex : nil } - private static func combinedCommandInlineOffset(_ token: String) -> Int? { - let chars = Array(token.lowercased()) - guard chars.count >= 2, chars[0] == "-", chars[1] != "-" else { - return nil - } - if chars.dropFirst().contains("-") { - return nil - } - guard let commandIndex = chars.firstIndex(of: "c"), commandIndex > 0 else { - return nil - } - return commandIndex + 1 - } - private static func extractShellInlinePayload( _ argv: [String], normalizedWrapper: String) -> String? @@ -421,7 +379,7 @@ enum ExecSystemRunCommandValidator { if let inlineCommand = match.inlineCommand { return inlineCommand } - let nextIndex = match.tokenIndex + 1 + let nextIndex = match.tokenIndex + match.valueTokenOffset return self.trimmedNonEmpty(nextIndex < argv.count ? argv[nextIndex] : nil) } diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index b2742234590..2fbfddda1d9 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -111,7 +111,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist splits shell chains`() { - let command = ["/bin/sh", "-lc", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test"] + let command = ["/bin/sh", "-c", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test"] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test", @@ -122,9 +122,109 @@ struct ExecAllowlistTests { #expect(resolutions[1].executableName == "touch") } + @Test func `resolve for allowlist splits posix combined c flag payloads`() { + for command in [ + ["/bin/bash", "-xc", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-ec", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-euxc", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-cx", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-O", "extglob", "-xc", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-co", "vi", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-oc", "vi", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-cO", "extglob", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-xo", "vi", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-xO", "extglob", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "+xo", "vi", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--rcfile", "/tmp/rc", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--init-file=/tmp/rc", "-c", "/usr/bin/printf safe_marker"], + ] { + 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 `resolve for allowlist treats c after posix shell operand as direct exec`() { + for command in [ + ["/bin/bash", "./script.sh", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-x", "-C", "echo ok", "-c", "/usr/bin/printf safe_marker"], + ] { + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: "/tmp", + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.count == 1) + #expect(resolutions[0].resolvedPath == "/bin/bash") + #expect(resolutions[0].executableName == "bash") + } + } + + @Test func `resolve for allowlist fails closed for interactive posix shell wrappers`() { + for command in [ + ["/bin/bash", "-i", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-ic", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--rcfile", "/tmp/payload.sh", "-i", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "--interactive", "-c", "/usr/bin/printf safe_marker"], + ] { + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.isEmpty) + } + } + + @Test func `resolve for allowlist fails closed for login shell wrappers`() { + for command in [ + ["/bin/bash", "-l", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--login", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "-xlc", "/usr/bin/printf safe_marker"], + ["/bin/dash", "-lc", "/usr/bin/printf safe_marker"], + ["ash", "-lc", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "-l", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "--login", "-c", "/usr/bin/printf safe_marker"], + ["/bin/sh", "-lc", "/usr/bin/printf safe_marker"], + ["/bin/sh", "-x", "-lc", "/usr/bin/printf safe_marker"], + ["/usr/bin/env", "/bin/sh", "-lc", "/usr/bin/printf safe_marker"], + ] { + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.isEmpty) + } + } + + @Test func `resolve for allowlist fails closed for fish init command wrappers`() { + for command in [ + ["/usr/bin/fish", "--init-command=/tmp/payload.fish", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "--init-command", "/tmp/payload.fish", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "-C", "/tmp/payload.fish", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "-C/tmp/payload.fish", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "--init-command", "-c; /tmp/payload.fish", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "-C", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "-c/tmp/payload.fish", "/usr/bin/printf safe_marker"], + ] { + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.isEmpty) + } + } + @Test func `resolve for allowlist uses wrapper argv payload even with canonical raw command`() { - let command = ["/bin/sh", "-lc", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test"] - let canonicalRaw = "/bin/sh -lc \"echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test\"" + let command = ["/bin/sh", "-c", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test"] + let canonicalRaw = "/bin/sh -c \"echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test\"" let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: canonicalRaw, @@ -135,6 +235,25 @@ struct ExecAllowlistTests { #expect(resolutions[1].executableName == "touch") } + @Test func `resolve for allowlist preserves generated sh lc raw payload binding`() { + let command = ["/bin/sh", "-lc", "/usr/bin/printf safe_marker"] + let resolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: "/usr/bin/printf safe_marker", + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(resolutions.count == 1) + #expect(resolutions[0].resolvedPath == "/usr/bin/printf") + #expect(resolutions[0].executableName == "printf") + + let rawlessResolutions = ExecCommandResolution.resolveForAllowlist( + command: command, + rawCommand: nil, + cwd: nil, + env: ["PATH": "/usr/bin:/bin"]) + #expect(rawlessResolutions.isEmpty) + } + @Test func `resolve for allowlist fails closed for env modified shell wrappers`() { let command = ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo allowlisted"] let canonicalRaw = "/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc \"echo allowlisted\"" @@ -158,7 +277,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist keeps quoted operators in single segment`() { - let command = ["/bin/sh", "-lc", "echo \"a && b\""] + let command = ["/bin/sh", "-c", "echo \"a && b\""] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo \"a && b\"", @@ -169,7 +288,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist fails closed on command substitution`() { - let command = ["/bin/sh", "-lc", "echo $(/usr/bin/touch /tmp/openclaw-allowlist-test-subst)"] + let command = ["/bin/sh", "-c", "echo $(/usr/bin/touch /tmp/openclaw-allowlist-test-subst)"] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo $(/usr/bin/touch /tmp/openclaw-allowlist-test-subst)", @@ -179,7 +298,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist fails closed on quoted command substitution`() { - let command = ["/bin/sh", "-lc", "echo \"ok $(/usr/bin/touch /tmp/openclaw-allowlist-test-quoted-subst)\""] + let command = ["/bin/sh", "-c", "echo \"ok $(/usr/bin/touch /tmp/openclaw-allowlist-test-quoted-subst)\""] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo \"ok $(/usr/bin/touch /tmp/openclaw-allowlist-test-quoted-subst)\"", @@ -189,7 +308,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist fails closed on line-continued command substitution`() { - let command = ["/bin/sh", "-lc", "echo $\\\n(/usr/bin/touch /tmp/openclaw-allowlist-test-line-cont-subst)"] + let command = ["/bin/sh", "-c", "echo $\\\n(/usr/bin/touch /tmp/openclaw-allowlist-test-line-cont-subst)"] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo $\\\n(/usr/bin/touch /tmp/openclaw-allowlist-test-line-cont-subst)", @@ -201,7 +320,7 @@ struct ExecAllowlistTests { @Test func `resolve for allowlist fails closed on chained line-continued command substitution`() { let command = [ "/bin/sh", - "-lc", + "-c", "echo ok && $\\\n(/usr/bin/touch /tmp/openclaw-allowlist-test-chained-line-cont-subst)", ] let resolutions = ExecCommandResolution.resolveForAllowlist( @@ -213,7 +332,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist fails closed on quoted backticks`() { - let command = ["/bin/sh", "-lc", "echo \"ok `/usr/bin/id`\""] + let command = ["/bin/sh", "-c", "echo \"ok `/usr/bin/id`\""] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "echo \"ok `/usr/bin/id`\"", @@ -226,7 +345,7 @@ struct ExecAllowlistTests { let fixtures = try Self.loadShellParserParityCases() for fixture in fixtures { let resolutions = ExecCommandResolution.resolveForAllowlist( - command: ["/bin/sh", "-lc", fixture.command], + command: ["/bin/sh", "-c", fixture.command], rawCommand: fixture.command, cwd: nil, env: ["PATH": "/usr/bin:/bin"]) @@ -276,7 +395,7 @@ struct ExecAllowlistTests { let command = [ "/usr/bin/env", "/bin/sh", - "-lc", + "-c", "echo allowlisted && /usr/bin/touch /tmp/openclaw-allowlist-test", ] let resolutions = ExecCommandResolution.resolveForAllowlist( @@ -290,7 +409,7 @@ struct ExecAllowlistTests { } @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 command = ["/bin/sh", "-c", "env /usr/bin/touch /tmp/openclaw-allowlist-test"] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "env /usr/bin/touch /tmp/openclaw-allowlist-test", @@ -302,7 +421,7 @@ struct ExecAllowlistTests { } @Test func `resolve for allowlist preserves env assignments inside shell segments`() { - let command = ["/bin/sh", "-lc", "env FOO=bar /usr/bin/touch /tmp/openclaw-allowlist-test"] + let command = ["/bin/sh", "-c", "env FOO=bar /usr/bin/touch /tmp/openclaw-allowlist-test"] let resolutions = ExecCommandResolution.resolveForAllowlist( command: command, rawCommand: "env FOO=bar /usr/bin/touch /tmp/openclaw-allowlist-test", @@ -326,8 +445,8 @@ struct ExecAllowlistTests { } @Test func `approval evaluator resolves shell payload from canonical wrapper text`() async { - let command = ["/bin/sh", "-lc", "/usr/bin/printf ok"] - let rawCommand = "/bin/sh -lc \"/usr/bin/printf ok\"" + let command = ["/bin/sh", "-c", "/usr/bin/printf ok"] + let rawCommand = "/bin/sh -c \"/usr/bin/printf ok\"" let evaluation = await ExecApprovalEvaluator.evaluate( command: command, rawCommand: rawCommand, @@ -350,6 +469,32 @@ struct ExecAllowlistTests { #expect(patterns == ["/usr/bin/printf"]) } + @Test func `allow always patterns fail closed for env modified shell wrappers`() { + let patterns = ExecCommandResolution.resolveAllowAlwaysPatterns( + command: [ + "/usr/bin/env", + "BASH_ENV=/tmp/payload.sh", + "/bin/sh", + "-lc", + "/usr/bin/printf ok", + ], + cwd: nil, + env: ["PATH": "/usr/bin:/bin"], + rawCommand: "/usr/bin/printf ok") + + #expect(patterns.isEmpty) + } + + @Test func `allow always patterns preserve generated sh lc raw payload binding`() { + let patterns = ExecCommandResolution.resolveAllowAlwaysPatterns( + command: ["/bin/sh", "-lc", "/usr/bin/printf safe_marker"], + cwd: nil, + env: ["PATH": "/usr/bin:/bin"], + rawCommand: "/usr/bin/printf safe_marker") + + #expect(patterns == ["/usr/bin/printf"]) + } + @Test func `match all requires every segment to match`() { let first = ExecCommandResolution( rawExecutable: "echo", diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift index 351eea52df5..b5cf277f579 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift @@ -85,6 +85,48 @@ struct ExecSystemRunCommandValidatorTests { } } + @Test func `fish attached c command requires canonical raw command binding`() { + let command = ["/usr/bin/fish", "-c/tmp/payload.fish", "/usr/bin/printf safe_marker"] + let result = ExecSystemRunCommandValidator.resolve( + command: command, + rawCommand: "/usr/bin/printf safe_marker") + + switch result { + case .ok: + Issue.record("expected rawCommand mismatch for attached fish command payload") + case let .invalid(message): + #expect(message.contains("rawCommand does not match command")) + } + } + + @Test func `startup shell wrappers require canonical raw command binding`() { + for command in [ + ["/bin/bash", "-lc", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--rcfile", "/tmp/payload.sh", "-i", "-c", "/usr/bin/printf safe_marker"], + ["/bin/bash", "--login", "-c", "/usr/bin/printf safe_marker"], + ["/usr/bin/fish", "--init-command=/tmp/payload.fish", "-c", "/usr/bin/printf safe_marker"], + ] { + let legacy = ExecSystemRunCommandValidator.resolve( + command: command, + rawCommand: "/usr/bin/printf safe_marker") + switch legacy { + case .ok: + Issue.record("expected rawCommand mismatch for startup shell wrapper") + case let .invalid(message): + #expect(message.contains("rawCommand does not match command")) + } + + let canonicalRaw = ExecCommandFormatter.displayString(for: command) + let canonical = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: canonicalRaw) + switch canonical { + case let .ok(resolved): + #expect(resolved.displayCommand == canonicalRaw) + case let .invalid(message): + Issue.record("unexpected invalid result for canonical raw command: \(message)") + } + } + } + private static func loadContractCases() throws -> [SystemRunCommandContractCase] { let fixtureURL = try self.findContractFixtureURL() let data = try Data(contentsOf: fixtureURL) diff --git a/src/infra/command-explainer/extract.ts b/src/infra/command-explainer/extract.ts index 02097f13f58..6fd8bd1a64b 100644 --- a/src/infra/command-explainer/extract.ts +++ b/src/infra/command-explainer/extract.ts @@ -10,6 +10,7 @@ import { import { normalizeExecutableToken } from "../exec-wrapper-resolution.js"; import { extractShellWrapperCommand, + extractShellWrapperInlineCommand, isShellWrapperExecutable, POSIX_SHELL_WRAPPERS, resolveShellWrapperTransportArgv, @@ -876,14 +877,11 @@ function shellWrapperPayloadForParsing( dynamicArguments: DynamicArgument[], ): { command: string; spanBase: SpanBase } | null { const shellWrapper = extractShellWrapperCommand(argv); - if ( - !shellWrapper.isWrapper || - !shellWrapper.command || - isDynamicPayload(shellWrapper.command, dynamicArguments) - ) { + const payload = shellWrapper.command ?? extractShellWrapperInlineCommand(argv); + if (!shellWrapper.isWrapper || !payload || isDynamicPayload(payload, dynamicArguments)) { return null; } - const spanBase = payloadBaseFromArguments(shellWrapper.command, argumentsList); + const spanBase = payloadBaseFromArguments(payload, argumentsList); if (!spanBase) { return null; } @@ -892,7 +890,7 @@ function shellWrapperPayloadForParsing( if (!canParseShellWrapperPayload(transportArgv, commandFlag?.flag ?? null)) { return null; } - return { command: shellWrapper.command, spanBase }; + return { command: payload, spanBase }; } type InlineEvalHit = InterpreterInlineEvalHit; @@ -947,7 +945,8 @@ function recordCommandRisks( } const shellWrapper = extractShellWrapperCommand(argv); - if (shellWrapper.isWrapper && shellWrapper.command) { + const shellWrapperPayload = shellWrapper.command ?? extractShellWrapperInlineCommand(argv); + if (shellWrapper.isWrapper && shellWrapperPayload) { const transportArgv = resolveShellWrapperTransportArgv(argv) ?? argv; const shellExecutable = transportArgv[0] ?? executable; const commandFlag = shellCommandFlag(transportArgv, 1) ?? shellCommandFlag(argv, 1); @@ -956,7 +955,7 @@ function recordCommandRisks( kind: "shell-wrapper", executable: shellExecutable, flag: commandFlag?.flag ?? "-c", - payload: shellWrapper.command, + payload: shellWrapperPayload, text, span, }); diff --git a/src/infra/exec-approvals-allow-always.test.ts b/src/infra/exec-approvals-allow-always.test.ts index 1f19e0b4cff..644c7dc302e 100644 --- a/src/infra/exec-approvals-allow-always.test.ts +++ b/src/infra/exec-approvals-allow-always.test.ts @@ -324,8 +324,8 @@ describe("resolveAllowAlwaysPatterns", () => { const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: "/bin/zsh -lc 'whoami'", - argv: ["/bin/zsh", "-lc", "whoami"], + raw: "/bin/zsh -c 'whoami'", + argv: ["/bin/zsh", "-c", "whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: "/bin/zsh", @@ -353,8 +353,8 @@ describe("resolveAllowAlwaysPatterns", () => { const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: "/bin/zsh -lc 'whoami && ls && whoami'", - argv: ["/bin/zsh", "-lc", "whoami && ls && whoami"], + raw: "/bin/zsh -c 'whoami && ls && whoami'", + argv: ["/bin/zsh", "-c", "whoami && ls && whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: "/bin/zsh", @@ -437,12 +437,49 @@ describe("resolveAllowAlwaysPatterns", () => { } }); + it("rejects startup shell inline payloads for allow-always and inline-chain allowlist fallback", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const tool = makeExecutable(dir, "openclaw-ok"); + const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` }; + const safeBins = resolveSafeBins(undefined); + + for (const command of [ + `bash --login -c "openclaw-ok && openclaw-ok"`, + `bash -i -c "openclaw-ok && openclaw-ok"`, + `bash -lc "openclaw-ok && openclaw-ok"`, + `bash --login -c '$0 "$1"' ${tool} marker`, + `bash -i -c '$0 "$1"' ${tool} marker`, + `bash -lc '$0 "$1"' ${tool} marker`, + ]) { + const { persisted } = resolvePersistedPatterns({ + command, + dir, + env, + safeBins, + }); + expect(persisted).toEqual([]); + + const second = evaluateShellAllowlist({ + command, + allowlist: [{ pattern: tool }], + safeBins, + cwd: dir, + env, + platform: process.platform, + }); + expect(second.allowlistSatisfied).toBe(false); + } + }); + it("rejects shell-wrapper positional argv carriers", () => { if (process.platform === "win32") { return; } expectPositionalArgvCarrierResult({ - command: `sh -lc '$0 "$1"' touch {marker}`, + command: `sh -c '$0 "$1"' touch {marker}`, expectPersisted: true, }); }); @@ -452,7 +489,7 @@ describe("resolveAllowAlwaysPatterns", () => { return; } expectPositionalArgvCarrierResult({ - command: `sh -lc 'exec -- "$0" "$1"' touch {marker}`, + command: `sh -c 'exec -- "$0" "$1"' touch {marker}`, expectPersisted: true, }); }); @@ -462,7 +499,7 @@ describe("resolveAllowAlwaysPatterns", () => { return; } expectPositionalArgvCarrierResult({ - command: `sh -lc "'$0' "$1"" touch {marker}`, + command: `sh -c "'$0' "$1"" touch {marker}`, expectPersisted: false, }); }); @@ -472,7 +509,7 @@ describe("resolveAllowAlwaysPatterns", () => { return; } expectPositionalArgvCarrierResult({ - command: `sh -lc "exec + command: `sh -c "exec $0 \\"$1\\"" touch {marker}`, expectPersisted: false, }); @@ -489,7 +526,7 @@ $0 \\"$1\\"" touch {marker}`, const marker = path.join(dir, "marker"); const { persisted } = resolvePersistedPatterns({ - command: `sh -lc 'echo blocked; $0 "$1"' touch ${marker}`, + command: `sh -c 'echo blocked; $0 "$1"' touch ${marker}`, dir, env, safeBins, @@ -497,7 +534,7 @@ $0 \\"$1\\"" touch {marker}`, expect(persisted).not.toContain(touch); const second = evaluateShellAllowlist({ - command: `sh -lc 'echo blocked; $0 "$1"' touch ${marker}`, + command: `sh -c 'echo blocked; $0 "$1"' touch ${marker}`, allowlist: [{ pattern: touch }], safeBins, cwd: dir, @@ -515,7 +552,7 @@ $0 \\"$1\\"" touch {marker}`, expectAllowAlwaysBypassBlocked({ dir, firstCommand: "bash scripts/save_crystal.sh", - secondCommand: "bash -lc 'scripts/save_crystal.sh'", + secondCommand: "bash -c 'scripts/save_crystal.sh'", env, persistedPattern: script, }); @@ -564,8 +601,8 @@ $0 \\"$1\\"" touch {marker}`, const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: "/usr/local/bin/zsh -lc whoami", - argv: ["/usr/local/bin/zsh", "-lc", "whoami"], + raw: "/usr/local/bin/zsh -c whoami", + argv: ["/usr/local/bin/zsh", "-c", "whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: "/usr/local/bin/zsh", @@ -591,8 +628,8 @@ $0 \\"$1\\"" touch {marker}`, const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: "/usr/bin/nice /bin/zsh -lc whoami", - argv: ["/usr/bin/nice", "/bin/zsh", "-lc", "whoami"], + raw: "/usr/bin/nice /bin/zsh -c whoami", + argv: ["/usr/bin/nice", "/bin/zsh", "-c", "whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: "/usr/bin/nice", @@ -619,8 +656,8 @@ $0 \\"$1\\"" touch {marker}`, const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: "/usr/bin/time -p /bin/zsh -lc whoami", - argv: ["/usr/bin/time", "-p", "/bin/zsh", "-lc", "whoami"], + raw: "/usr/bin/time -p /bin/zsh -c whoami", + argv: ["/usr/bin/time", "-p", "/bin/zsh", "-c", "whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: "/usr/bin/time", @@ -650,8 +687,8 @@ $0 \\"$1\\"" touch {marker}`, const patterns = resolveAllowAlwaysPatterns({ segments: [ { - raw: `${busybox} sh -lc whoami`, - argv: [busybox, "sh", "-lc", "whoami"], + raw: `${busybox} sh -c whoami`, + argv: [busybox, "sh", "-c", "whoami"], resolution: makeMockCommandResolution({ execution: makeMockExecutableResolution({ rawExecutable: busybox, @@ -744,8 +781,8 @@ $0 \\"$1\\"" touch {marker}`, const env = makePathEnv(dir); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/caffeinate -d -w 42 /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/caffeinate -d -w 42 /bin/zsh -lc 'id > marker'", + firstCommand: "/usr/bin/caffeinate -d -w 42 /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/caffeinate -d -w 42 /bin/zsh -c 'id > marker'", env, persistedPattern: echo, }); @@ -761,8 +798,8 @@ $0 \\"$1\\"" touch {marker}`, const env = makePathEnv(dir); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/nice /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/nice /bin/zsh -lc 'id > marker'", + firstCommand: "/usr/bin/nice /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/nice /bin/zsh -c 'id > marker'", env, persistedPattern: echo, }); @@ -779,8 +816,8 @@ $0 \\"$1\\"" touch {marker}`, expectAllowAlwaysBypassBlocked({ dir, firstCommand: - "/usr/bin/sandbox-exec -p '(deny default) (allow process*)' /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/sandbox-exec -p '(allow default)' /bin/zsh -lc 'id > marker'", + "/usr/bin/sandbox-exec -p '(deny default) (allow process*)' /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/sandbox-exec -p '(allow default)' /bin/zsh -c 'id > marker'", env, persistedPattern: echo, }); @@ -796,8 +833,8 @@ $0 \\"$1\\"" touch {marker}`, const env = makePathEnv(dir); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/time -p /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/time -p /bin/zsh -lc 'id > marker'", + firstCommand: "/usr/bin/time -p /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/time -p /bin/zsh -c 'id > marker'", env, persistedPattern: echo, }); @@ -813,15 +850,15 @@ $0 \\"$1\\"" touch {marker}`, const env = makePathEnv(dir); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/arch -arm64 /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/arch -arm64 /bin/zsh -lc 'id > marker-arch'", + firstCommand: "/usr/bin/arch -arm64 /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/arch -arm64 /bin/zsh -c 'id > marker-arch'", env, persistedPattern: echo, }); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/xcrun /bin/zsh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/xcrun /bin/zsh -lc 'id > marker-xcrun'", + firstCommand: "/usr/bin/xcrun /bin/zsh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/xcrun /bin/zsh -c 'id > marker-xcrun'", env, persistedPattern: echo, }); @@ -873,7 +910,7 @@ $0 \\"$1\\"" touch {marker}`, const safeBins = resolveSafeBins(undefined); const { persisted } = resolvePersistedPatterns({ - command: `sh -lc '$0 "$@"' awk '{print $1}' data.csv`, + command: `sh -c '$0 "$@"' awk '{print $1}' data.csv`, dir, env, safeBins, @@ -881,7 +918,7 @@ $0 \\"$1\\"" touch {marker}`, expect(persisted).toEqual([]); const second = evaluateShellAllowlist({ - command: `sh -lc '$0 "$@"' awk 'BEGIN{system("id > /tmp/pwned")}'`, + command: `sh -c '$0 "$@"' awk 'BEGIN{system("id > /tmp/pwned")}'`, allowlist: persisted.map((pattern) => ({ pattern })), safeBins, cwd: dir, @@ -901,8 +938,8 @@ $0 \\"$1\\"" touch {marker}`, const env = makePathEnv(dir); expectAllowAlwaysBypassBlocked({ dir, - firstCommand: "/usr/bin/script -q /dev/null /bin/sh -lc 'echo warmup-ok'", - secondCommand: "/usr/bin/script -q /dev/null /bin/sh -lc 'id > marker'", + firstCommand: "/usr/bin/script -q /dev/null /bin/sh -c 'echo warmup-ok'", + secondCommand: "/usr/bin/script -q /dev/null /bin/sh -c 'id > marker'", env, persistedPattern: echo, }); @@ -935,7 +972,7 @@ $0 \\"$1\\"" touch {marker}`, const safeBins = resolveSafeBins(undefined); const { persisted } = resolvePersistedPatterns({ - command: `sh -lc '$0 "$@"' env echo SAFE`, + command: `sh -c '$0 "$@"' env echo SAFE`, dir, env, safeBins, @@ -943,7 +980,7 @@ $0 \\"$1\\"" touch {marker}`, expect(persisted).toEqual([]); const second = evaluateShellAllowlist({ - command: `sh -lc '$0 "$@"' env BASH_ENV=/tmp/payload.sh bash -lc 'id > /tmp/pwned'`, + command: `sh -c '$0 "$@"' env BASH_ENV=/tmp/payload.sh bash -c 'id > /tmp/pwned'`, allowlist: [{ pattern: envPath }], safeBins, cwd: dir, @@ -963,7 +1000,7 @@ $0 \\"$1\\"" touch {marker}`, const safeBins = resolveSafeBins(undefined); const { persisted } = resolvePersistedPatterns({ - command: `sh -lc '$0 "$@"' bash -lc 'echo safe'`, + command: `sh -c '$0 "$@"' bash -c 'echo safe'`, dir, env, safeBins, @@ -971,7 +1008,7 @@ $0 \\"$1\\"" touch {marker}`, expect(persisted).toEqual([]); const second = evaluateShellAllowlist({ - command: `sh -lc '$0 "$@"' bash -lc 'id > /tmp/pwned'`, + command: `sh -c '$0 "$@"' bash -c 'id > /tmp/pwned'`, allowlist: [{ pattern: bashPath }], safeBins, cwd: dir, @@ -991,7 +1028,7 @@ $0 \\"$1\\"" touch {marker}`, const safeBins = resolveSafeBins(undefined); const { persisted } = resolvePersistedPatterns({ - command: `sh -lc '$0 "$@"' xargs echo SAFE`, + command: `sh -c '$0 "$@"' xargs echo SAFE`, dir, env, safeBins, @@ -999,7 +1036,7 @@ $0 \\"$1\\"" touch {marker}`, expect(persisted).toEqual([]); const second = evaluateShellAllowlist({ - command: `sh -lc '$0 "$@"' xargs sh -lc 'id > /tmp/pwned'`, + command: `sh -c '$0 "$@"' xargs sh -c 'id > /tmp/pwned'`, allowlist: [{ pattern: xargsPath }], safeBins, cwd: dir, diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index 01a436f43b2..1414915c3f3 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -32,7 +32,7 @@ import { } from "./exec-safe-bin-policy.js"; import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js"; import { - extractShellWrapperInlineCommand, + extractBindableShellWrapperInlineCommand, isShellWrapperExecutable, normalizeExecutableToken, POWERSHELL_WRAPPERS, @@ -426,7 +426,7 @@ function resolveSegmentAllowlistMatch(params: { candidatePath && executableResolution ? { ...executableResolution, resolvedPath: candidatePath } : executableResolution; - const inlineCommand = extractShellWrapperInlineCommand(allowlistSegment.argv); + const inlineCommand = extractBindableShellWrapperInlineCommand(allowlistSegment.argv); const isPositionalCarrierInvocation = inlineCommand !== null && isDirectShellPositionalCarrierInvocation(inlineCommand); const executableMatch = isPositionalCarrierInvocation @@ -437,11 +437,14 @@ function resolveSegmentAllowlistMatch(params: { effectiveArgv, params.context.platform, ); - const shellPositionalArgvCandidatePath = resolveShellWrapperPositionalArgvCandidatePath({ - segment: allowlistSegment, - cwd: params.context.cwd, - env: params.context.env, - }); + const shellPositionalArgvCandidatePath = + inlineCommand !== null + ? resolveShellWrapperPositionalArgvCandidatePath({ + segment: allowlistSegment, + cwd: params.context.cwd, + env: params.context.env, + }) + : undefined; const shellPositionalArgvMatch = shellPositionalArgvCandidatePath ? matchAllowlist( params.context.allowlist, @@ -971,15 +974,6 @@ function collectAllowAlwaysPatterns(params: { addAllowAlwaysPattern(params.out, candidatePath, argPattern); return; } - const positionalArgvPath = resolveShellWrapperPositionalArgvCandidatePath({ - segment, - cwd: params.cwd, - env: params.env, - }); - if (positionalArgvPath) { - addAllowAlwaysPattern(params.out, positionalArgvPath); - return; - } const isPowerShellFileInvocation = POWERSHELL_WRAPPERS.has(normalizeExecutableToken(segment.argv[0] ?? "")) && segment.argv.some((t) => { @@ -990,9 +984,19 @@ function collectAllowAlwaysPatterns(params: { const lower = normalizeLowercaseStringOrEmpty(t); return lower === "-command" || lower === "-c" || lower === "--command"; }); - const inlineCommand = isPowerShellFileInvocation - ? null - : (trustPlan.shellInlineCommand ?? extractShellWrapperInlineCommand(segment.argv)); + const inlineCommand = isPowerShellFileInvocation ? null : trustPlan.shellInlineCommand; + const positionalArgvPath = + inlineCommand !== null + ? resolveShellWrapperPositionalArgvCandidatePath({ + segment, + cwd: params.cwd, + env: params.env, + }) + : undefined; + if (positionalArgvPath) { + addAllowAlwaysPattern(params.out, positionalArgvPath); + return; + } if (!inlineCommand) { const scriptPath = resolveShellWrapperScriptCandidatePath({ segment, diff --git a/src/infra/exec-wrapper-resolution.test.ts b/src/infra/exec-wrapper-resolution.test.ts index 32d87d73bdc..94f64d900c3 100644 --- a/src/infra/exec-wrapper-resolution.test.ts +++ b/src/infra/exec-wrapper-resolution.test.ts @@ -471,12 +471,12 @@ describe("extractShellWrapperCommand", () => { { argv: ["bash", "-lc", "echo hi"], expectedInline: "echo hi", - expectedCommand: { isWrapper: true, command: "echo hi" }, + expectedCommand: { isWrapper: true, command: null }, }, { argv: ["busybox", "sh", "-lc", "echo hi"], expectedInline: "echo hi", - expectedCommand: { isWrapper: true, command: "echo hi" }, + expectedCommand: { isWrapper: true, command: null }, }, { argv: ["env", "--", "pwsh", "-Command", "Get-Date"], @@ -494,7 +494,7 @@ describe("extractShellWrapperCommand", () => { }); test("prefers an explicit raw command override when provided", () => { - expect(extractShellWrapperCommand(["bash", "-lc", "echo hi"], " run this instead ")).toEqual({ + expect(extractShellWrapperCommand(["bash", "-c", "echo hi"], " run this instead ")).toEqual({ isWrapper: true, command: "run this instead", }); diff --git a/src/infra/exec-wrapper-resolution.ts b/src/infra/exec-wrapper-resolution.ts index 91ca1e3a4c9..2f8b89d7b1e 100644 --- a/src/infra/exec-wrapper-resolution.ts +++ b/src/infra/exec-wrapper-resolution.ts @@ -8,9 +8,11 @@ export { unwrapKnownDispatchWrapperInvocation, } from "./dispatch-wrapper-resolution.js"; export { + extractBindableShellWrapperInlineCommand, extractShellWrapperCommand, extractShellWrapperInlineCommand, hasEnvManipulationBeforeShellWrapper, + isBlockedShellWrapperCommand, isShellWrapperExecutable, isShellWrapperInvocation, POSIX_SHELL_WRAPPERS, diff --git a/src/infra/exec-wrapper-trust-plan.test.ts b/src/infra/exec-wrapper-trust-plan.test.ts index f07b11290a9..c6cad59b5a2 100644 --- a/src/infra/exec-wrapper-trust-plan.test.ts +++ b/src/infra/exec-wrapper-trust-plan.test.ts @@ -6,10 +6,10 @@ describe("resolveExecWrapperTrustPlan", () => { { name: "unwraps transparent caffeinate wrappers before shell policy checks", enabled: process.platform !== "win32", - argv: ["/usr/bin/caffeinate", "-d", "-w", "42", "sh", "-lc", "echo hi"], + argv: ["/usr/bin/caffeinate", "-d", "-w", "42", "sh", "-c", "echo hi"], expected: { - argv: ["sh", "-lc", "echo hi"], - policyArgv: ["sh", "-lc", "echo hi"], + argv: ["sh", "-c", "echo hi"], + policyArgv: ["sh", "-c", "echo hi"], wrapperChain: ["caffeinate"], policyBlocked: false, shellWrapperExecutable: true, @@ -19,10 +19,10 @@ describe("resolveExecWrapperTrustPlan", () => { { name: "unwraps dispatch wrappers and shell multiplexers into one trust plan", enabled: process.platform !== "win32", - argv: ["/usr/bin/time", "-p", "busybox", "sh", "-lc", "echo hi"], + argv: ["/usr/bin/time", "-p", "busybox", "sh", "-c", "echo hi"], expected: { - argv: ["sh", "-lc", "echo hi"], - policyArgv: ["busybox", "sh", "-lc", "echo hi"], + argv: ["sh", "-c", "echo hi"], + policyArgv: ["busybox", "sh", "-c", "echo hi"], wrapperChain: ["time", "busybox"], policyBlocked: false, shellWrapperExecutable: true, @@ -32,10 +32,10 @@ describe("resolveExecWrapperTrustPlan", () => { { name: "unwraps script wrappers before evaluating nested shell payloads", enabled: process.platform === "darwin" || process.platform === "freebsd", - argv: ["/usr/bin/script", "-q", "/dev/null", "sh", "-lc", "echo hi"], + argv: ["/usr/bin/script", "-q", "/dev/null", "sh", "-c", "echo hi"], expected: { - argv: ["sh", "-lc", "echo hi"], - policyArgv: ["sh", "-lc", "echo hi"], + argv: ["sh", "-c", "echo hi"], + policyArgv: ["sh", "-c", "echo hi"], wrapperChain: ["script"], policyBlocked: false, shellWrapperExecutable: true, @@ -45,16 +45,29 @@ describe("resolveExecWrapperTrustPlan", () => { { name: "unwraps sandbox-exec wrappers before evaluating nested shell payloads", enabled: process.platform !== "win32", - argv: ["/usr/bin/sandbox-exec", "-p", "(allow default)", "sh", "-lc", "echo hi"], + argv: ["/usr/bin/sandbox-exec", "-p", "(allow default)", "sh", "-c", "echo hi"], expected: { - argv: ["sh", "-lc", "echo hi"], - policyArgv: ["sh", "-lc", "echo hi"], + argv: ["sh", "-c", "echo hi"], + policyArgv: ["sh", "-c", "echo hi"], wrapperChain: ["sandbox-exec"], policyBlocked: false, shellWrapperExecutable: true, shellInlineCommand: "echo hi", }, }, + { + name: "omits startup shell inline payloads from trust plans", + enabled: process.platform !== "win32", + argv: ["bash", "--login", "-c", "echo hi"], + expected: { + argv: ["bash", "--login", "-c", "echo hi"], + policyArgv: ["bash", "--login", "-c", "echo hi"], + wrapperChain: [], + policyBlocked: false, + shellWrapperExecutable: true, + shellInlineCommand: null, + }, + }, { name: "fails closed for unsupported shell multiplexer applets", enabled: true, diff --git a/src/infra/exec-wrapper-trust-plan.ts b/src/infra/exec-wrapper-trust-plan.ts index bad51530746..04c90681294 100644 --- a/src/infra/exec-wrapper-trust-plan.ts +++ b/src/infra/exec-wrapper-trust-plan.ts @@ -4,7 +4,7 @@ import { unwrapKnownDispatchWrapperInvocation, } from "./dispatch-wrapper-resolution.js"; import { - extractShellWrapperInlineCommand, + extractBindableShellWrapperInlineCommand, isShellWrapperExecutable, unwrapKnownShellMultiplexerInvocation, } from "./shell-wrapper-resolution.js"; @@ -46,15 +46,20 @@ function finalizeExecWrapperTrustPlan( const rawExecutable = argv[0]?.trim() ?? ""; const shellWrapperExecutable = !policyBlocked && rawExecutable.length > 0 && isShellWrapperExecutable(rawExecutable); - return { + const plan: ExecWrapperTrustPlan = { argv, policyArgv, wrapperChain, policyBlocked, - blockedWrapper, shellWrapperExecutable, - shellInlineCommand: shellWrapperExecutable ? extractShellWrapperInlineCommand(argv) : null, + shellInlineCommand: shellWrapperExecutable + ? extractBindableShellWrapperInlineCommand(argv) + : null, }; + if (blockedWrapper !== undefined) { + plan.blockedWrapper = blockedWrapper; + } + return plan; } export function resolveExecWrapperTrustPlan( diff --git a/src/infra/shell-inline-command.test.ts b/src/infra/shell-inline-command.test.ts index 4cea7c67c43..7ffe08c43b9 100644 --- a/src/infra/shell-inline-command.test.ts +++ b/src/infra/shell-inline-command.test.ts @@ -38,6 +38,20 @@ describe("resolveInlineCommandMatch", () => { opts: { allowCombinedC: true }, expected: { command: "echo hi", valueTokenIndex: 1 }, }, + { + name: "keeps post-c no-argument shell flags separate from the command", + argv: ["bash", "-cx", "echo hi"], + flags: POSIX_INLINE_COMMAND_FLAGS, + opts: { allowCombinedC: true }, + expected: { command: "echo hi", valueTokenIndex: 2 }, + }, + { + name: "keeps post-c stdin shell flags separate from the command", + argv: ["bash", "-cs", "echo hi"], + flags: POSIX_INLINE_COMMAND_FLAGS, + opts: { allowCombinedC: true }, + expected: { command: "echo hi", valueTokenIndex: 2 }, + }, { name: "rejects combined -c forms when disabled", argv: ["sh", "-cecho hi"], diff --git a/src/infra/shell-inline-command.ts b/src/infra/shell-inline-command.ts index 13690e41e4e..56dd7224dbf 100644 --- a/src/infra/shell-inline-command.ts +++ b/src/infra/shell-inline-command.ts @@ -12,35 +12,212 @@ export const POWERSHELL_INLINE_COMMAND_FLAGS = new Set([ "-e", ]); +const POSIX_SHELL_OPTIONS_WITH_SEPARATE_VALUES = new Set([ + "--init-file", + "--rcfile", + "-O", + "-o", + "+O", + "+o", +]); + +function isCombinedCommandFlag(token: string): boolean { + return parseCombinedCommandFlag(token) !== null; +} + +function parseCombinedCommandFlag( + token: string, +): { attachedCommand: string | null; separateValueCount: number } | null { + if (token.length < 2 || token[0] !== "-" || token[1] === "-") { + return null; + } + const optionChars = token.slice(1); + const commandFlagIndex = optionChars.indexOf("c"); + if (commandFlagIndex === -1 || optionChars.includes("-")) { + return null; + } + const suffix = optionChars.slice(commandFlagIndex + 1); + if (suffix && !/^[A-Za-z]+$/.test(suffix)) { + return { attachedCommand: suffix, separateValueCount: 0 }; + } + return { + attachedCommand: null, + separateValueCount: [...optionChars].filter((char) => char === "o" || char === "O").length, + }; +} + +function combinedSeparateValueOptionCount(token: string): number { + if ( + token.length < 2 || + (token[0] !== "-" && token[0] !== "+") || + token[1] === "-" || + token.slice(1).includes("-") + ) { + return 0; + } + return [...token.slice(1)].filter((char) => char === "o" || char === "O").length; +} + +function consumesSeparateValue(token: string): boolean { + return POSIX_SHELL_OPTIONS_WITH_SEPARATE_VALUES.has(token); +} + +function isPosixInteractiveModeOption(token: string): boolean { + return token === "--interactive" || isPosixShortOption(token, "i"); +} + +function isPosixShortOption(token: string, option: string): boolean { + if (token.length < 2 || token[0] !== "-" || token[1] === "-") { + return false; + } + const optionChars = token.slice(1); + return !optionChars.includes("-") && optionChars.includes(option); +} + +function advancePosixInlineOptionScan(token: string): number { + const combinedValueCount = combinedSeparateValueOptionCount(token); + if (combinedValueCount > 0) { + return 1 + combinedValueCount; + } + if (consumesSeparateValue(token)) { + return 2; + } + return 1; +} + export function resolveInlineCommandMatch( argv: string[], flags: ReadonlySet, options: { allowCombinedC?: boolean } = {}, ): { command: string | null; valueTokenIndex: number | null } { - for (let i = 1; i < argv.length; i += 1) { + for (let i = 1; i < argv.length; ) { const token = argv[i]?.trim(); if (!token) { + i += 1; continue; } const lower = normalizeLowercaseStringOrEmpty(token); if (lower === "--") { break; } - if (flags.has(lower)) { + const comparableToken = options.allowCombinedC ? token : lower; + if (flags.has(comparableToken)) { const valueTokenIndex = i + 1 < argv.length ? i + 1 : null; const command = argv[i + 1]?.trim(); return { command: command ? command : null, valueTokenIndex }; } - if (options.allowCombinedC && /^-[^-]*c[^-]*$/i.test(token)) { - const commandIndex = lower.indexOf("c"); - const inline = token.slice(commandIndex + 1).trim(); - if (inline) { - return { command: inline, valueTokenIndex: i }; + if (options.allowCombinedC && isCombinedCommandFlag(token)) { + const combined = parseCombinedCommandFlag(token); + if (combined?.attachedCommand != null) { + return { command: combined.attachedCommand.trim() || null, valueTokenIndex: i }; } - const valueTokenIndex = i + 1 < argv.length ? i + 1 : null; - const command = argv[i + 1]?.trim(); + const valueTokenIndex = i + 1 + (combined?.separateValueCount ?? 0); + const command = argv[valueTokenIndex]?.trim(); return { command: command ? command : null, valueTokenIndex }; } + if (options.allowCombinedC && !token.startsWith("-") && !token.startsWith("+")) { + break; + } + i += options.allowCombinedC ? advancePosixInlineOptionScan(token) : 1; } return { command: null, valueTokenIndex: null }; } + +export function hasPosixInteractiveStartupBeforeInlineCommand( + argv: string[], + flags: ReadonlySet, +): boolean { + let sawInteractiveMode = false; + for (let i = 1; i < argv.length; ) { + const token = argv[i]?.trim(); + if (!token) { + i += 1; + continue; + } + if (token === "--") { + return false; + } + if (isPosixInteractiveModeOption(token)) { + sawInteractiveMode = true; + } + if (flags.has(token) || isCombinedCommandFlag(token)) { + return sawInteractiveMode; + } + if (!token.startsWith("-") && !token.startsWith("+")) { + return false; + } + i += advancePosixInlineOptionScan(token); + } + return false; +} + +export function hasPosixLoginStartupBeforeInlineCommand( + argv: string[], + flags: ReadonlySet, +): boolean { + let sawLoginMode = false; + for (let i = 1; i < argv.length; ) { + const token = argv[i]?.trim(); + if (!token) { + i += 1; + continue; + } + if (token === "--") { + return false; + } + if (token === "--login" || isPosixShortOption(token, "l")) { + sawLoginMode = true; + } + if (flags.has(token) || isCombinedCommandFlag(token)) { + return sawLoginMode; + } + if (!token.startsWith("-") && !token.startsWith("+")) { + return false; + } + i += advancePosixInlineOptionScan(token); + } + return false; +} + +export function hasFishInitCommandOption(argv: string[]): boolean { + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]?.trim(); + if (!token) { + continue; + } + if (token === "--") { + return false; + } + if ( + token === "-C" || + token === "--init-command" || + (token.startsWith("-C") && token !== "-C") || + token.startsWith("--init-command=") + ) { + return true; + } + if (!token.startsWith("-") && !token.startsWith("+")) { + return false; + } + } + return false; +} + +export function hasFishAttachedCommandOption(argv: string[]): boolean { + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]?.trim(); + if (!token) { + continue; + } + if (token === "--") { + return false; + } + if (token.startsWith("-c") && token !== "-c") { + return true; + } + if (!token.startsWith("-") && !token.startsWith("+")) { + return false; + } + } + return false; +} diff --git a/src/infra/shell-wrapper-resolution.ts b/src/infra/shell-wrapper-resolution.ts index 3979a35d1ff..c9595ae7a59 100644 --- a/src/infra/shell-wrapper-resolution.ts +++ b/src/infra/shell-wrapper-resolution.ts @@ -6,6 +6,10 @@ import { } from "./dispatch-wrapper-resolution.js"; import { normalizeExecutableToken } from "./exec-wrapper-tokens.js"; import { + hasFishAttachedCommandOption, + hasFishInitCommandOption, + hasPosixInteractiveStartupBeforeInlineCommand, + hasPosixLoginStartupBeforeInlineCommand, POSIX_INLINE_COMMAND_FLAGS, POWERSHELL_INLINE_COMMAND_FLAGS, resolveInlineCommandMatch, @@ -37,6 +41,7 @@ const SHELL_WRAPPER_CANONICAL = new Set([ ...WINDOWS_CMD_WRAPPER_NAMES, ...POWERSHELL_WRAPPER_NAMES, ]); +const LOGIN_STARTUP_SHELL_WRAPPER_CANONICAL = new Set(POSIX_SHELL_WRAPPER_NAMES); type ShellWrapperKind = "posix" | "cmd" | "powershell"; @@ -235,6 +240,49 @@ function extractShellWrapperPayload(argv: string[], spec: ShellWrapperSpec): str throw new Error("Unsupported shell wrapper kind"); } +function isLegacyLoginInlineForm(argv: string[]): boolean { + return argv[1]?.trim() === "-lc"; +} + +function isLegacyShLoginInlineForm(argv: string[], baseExecutable: string): boolean { + return baseExecutable === "sh" && isLegacyLoginInlineForm(argv); +} + +function formatShellWrapperArgv(argv: string[]): string { + return argv + .map((arg) => { + if (arg.length === 0) { + return '""'; + } + return /\s|"/.test(arg) ? `"${arg.replace(/"/g, '\\"')}"` : arg; + }) + .join(" "); +} + +function startupWrapperRequiresFullArgv(params: { + argv: string[]; + spec: ShellWrapperSpec; + baseExecutable: string; + includeLegacyLoginInlineForm: boolean; +}): boolean { + if (params.spec.kind !== "posix") { + return false; + } + if (params.baseExecutable === "fish" && hasFishInitCommandOption(params.argv)) { + return true; + } + if ( + LOGIN_STARTUP_SHELL_WRAPPER_CANONICAL.has(params.baseExecutable) && + hasPosixLoginStartupBeforeInlineCommand(params.argv, POSIX_INLINE_COMMAND_FLAGS) + ) { + return ( + params.includeLegacyLoginInlineForm || + !isLegacyShLoginInlineForm(params.argv, params.baseExecutable) + ); + } + return hasPosixInteractiveStartupBeforeInlineCommand(params.argv, POSIX_INLINE_COMMAND_FLAGS); +} + function hasEnvManipulationBeforeShellWrapperInternal( argv: string[], depth: number, @@ -270,12 +318,52 @@ function extractShellWrapperCommandInternal( rawCommand: string | null, depth: number, ): ShellWrapperCommand { - const resolved = resolveShellWrapperSpecAndArgvInternal(argv, depth); + const candidate = resolveShellWrapperCandidate({ argv, depth, state: null }); + if (!candidate) { + return { isWrapper: false, command: null }; + } + + const baseExecutable = normalizeExecutableToken(candidate.token0); + const wrapper = findShellWrapperSpec(baseExecutable); + if (!wrapper) { + return { isWrapper: false, command: null }; + } + const payload = extractShellWrapperPayload(candidate.argv, wrapper); + if (!payload) { + return { isWrapper: false, command: null }; + } + if ( + wrapper.kind === "posix" && + baseExecutable === "fish" && + hasFishAttachedCommandOption(candidate.argv) + ) { + return { isWrapper: true, command: null }; + } + const rawMatchesPayload = rawCommand === payload; + const rawMatchesCanonicalArgv = rawCommand === formatShellWrapperArgv(candidate.argv); + const allowLegacyShLoginPayloadBinding = + isLegacyShLoginInlineForm(candidate.argv, baseExecutable) && + (rawMatchesPayload || rawMatchesCanonicalArgv); + if ( + startupWrapperRequiresFullArgv({ + argv: candidate.argv, + spec: wrapper, + baseExecutable, + includeLegacyLoginInlineForm: !allowLegacyShLoginPayloadBinding, + }) + ) { + return { isWrapper: true, command: null }; + } + + const resolved = resolveShellWrapperSpecAndArgvInternal(candidate.argv, depth); if (!resolved) { return { isWrapper: false, command: null }; } - return { isWrapper: true, command: rawCommand ?? resolved.payload }; + return { + isWrapper: true, + command: rawMatchesCanonicalArgv ? resolved.payload : (rawCommand ?? resolved.payload), + }; } export function resolveShellWrapperTransportArgv(argv: string[]): string[] | null { @@ -283,8 +371,14 @@ export function resolveShellWrapperTransportArgv(argv: string[]): string[] | nul } export function extractShellWrapperInlineCommand(argv: string[]): string | null { - const extracted = extractShellWrapperCommandInternal(argv, null, 0); - return extracted.isWrapper ? extracted.command : null; + return resolveShellWrapperSpecAndArgvInternal(argv, 0)?.payload ?? null; +} + +export function extractBindableShellWrapperInlineCommand( + argv: string[], + rawCommand?: string | null, +): string | null { + return extractShellWrapperCommandInternal(argv, normalizeRawCommand(rawCommand), 0).command; } export function extractShellWrapperCommand( @@ -293,3 +387,8 @@ export function extractShellWrapperCommand( ): ShellWrapperCommand { return extractShellWrapperCommandInternal(argv, normalizeRawCommand(rawCommand), 0); } + +export function isBlockedShellWrapperCommand(argv: string[], rawCommand?: string | null): boolean { + const extracted = extractShellWrapperCommandInternal(argv, normalizeRawCommand(rawCommand), 0); + return extracted.isWrapper && extracted.command === null; +} diff --git a/src/infra/system-run-command.test.ts b/src/infra/system-run-command.test.ts index a63321f767f..ca43a4d2b54 100644 --- a/src/infra/system-run-command.test.ts +++ b/src/infra/system-run-command.test.ts @@ -34,8 +34,12 @@ describe("system run command helpers", () => { expect(formatExecCommand(["runner "])).toBe('"runner "'); }); - test("extractShellCommandFromArgv extracts sh -lc command", () => { - expect(extractShellCommandFromArgv(["/bin/sh", "-lc", "echo hi"])).toBe("echo hi"); + test("extractShellCommandFromArgv fails closed for rawless sh -lc command", () => { + expect(extractShellCommandFromArgv(["/bin/sh", "-lc", "echo hi"])).toBe(null); + }); + + test("extractShellCommandFromArgv extracts sh -c command", () => { + expect(extractShellCommandFromArgv(["/bin/sh", "-c", "echo hi"])).toBe("echo hi"); }); test("extractShellCommandFromArgv extracts cmd.exe /c command", () => { @@ -43,16 +47,16 @@ describe("system run command helpers", () => { }); 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", "bash", "-c", "echo hi"])).toBe("echo hi"); expect(extractShellCommandFromArgv(["/usr/bin/env", "FOO=bar", "zsh", "-c", "echo hi"])).toBe( "echo hi", ); }); test.each([ - { argv: ["/usr/bin/nice", "/bin/bash", "-lc", "echo hi"], expected: "echo hi" }, + { argv: ["/usr/bin/nice", "/bin/bash", "-c", "echo hi"], expected: "echo hi" }, { - argv: ["/usr/bin/timeout", "--signal=TERM", "5", "zsh", "-lc", "echo hi"], + argv: ["/usr/bin/timeout", "--signal=TERM", "5", "zsh", "-c", "echo hi"], expected: "echo hi", }, { @@ -74,7 +78,7 @@ describe("system run command helpers", () => { { argv: ["pwsh", "-EncodedCommand", "ZQBjAGgAbwA="], expected: "ZQBjAGgAbwA=" }, { argv: ["powershell", "-enc", "ZQBjAGgAbwA="], expected: "ZQBjAGgAbwA=" }, { argv: ["busybox", "sh", "-c", "echo hi"], expected: "echo hi" }, - { argv: ["toybox", "ash", "-lc", "echo hi"], expected: "echo hi" }, + { argv: ["toybox", "ash", "-c", "echo hi"], expected: "echo hi" }, ])("extractShellCommandFromArgv unwraps %j", ({ argv, expected }) => { expect(extractShellCommandFromArgv(argv)).toBe(expected); }); @@ -131,6 +135,26 @@ describe("system run command helpers", () => { expect(res.previewText).toBe("echo hi"); }); + test("validateSystemRunCommandConsistency preserves legacy sh -lc payload binding only for sh", () => { + const sh = expectValidResult( + validateSystemRunCommandConsistency({ + argv: ["/bin/sh", "-lc", "/usr/bin/printf ok"], + rawCommand: "/usr/bin/printf ok", + allowLegacyShellText: true, + }), + ); + expect(sh.previewText).toBe("/usr/bin/printf ok"); + + expectRawCommandMismatch({ + argv: ["/bin/bash", "-lc", "/usr/bin/printf ok"], + rawCommand: "/usr/bin/printf ok", + }); + }); + + test("extractShellCommandFromArgv treats uppercase posix C as a shell option, not command mode", () => { + expect(extractShellCommandFromArgv(["/bin/bash", "-C", "echo hi"])).toBe(null); + }); + test("validateSystemRunCommandConsistency rejects shell-only rawCommand for positional-argv carrier wrappers", () => { expectRawCommandMismatch({ argv: ["/bin/sh", "-lc", '$0 "$1"', "/usr/bin/touch", "/tmp/marker"], @@ -141,7 +165,7 @@ describe("system run command helpers", () => { test("validateSystemRunCommandConsistency accepts rawCommand matching env shell wrapper argv", () => { const res = expectValidResult( validateSystemRunCommandConsistency({ - argv: ["/usr/bin/env", "bash", "-lc", "echo hi"], + argv: ["/usr/bin/env", "bash", "-c", "echo hi"], rawCommand: "echo hi", allowLegacyShellText: true, }), @@ -156,6 +180,33 @@ describe("system run command helpers", () => { }); }); + test.each([ + { argv: ["/bin/bash", "--login", "-c", "/usr/bin/printf ok"] }, + { argv: ["/bin/bash", "-i", "-c", "/usr/bin/printf ok"] }, + { argv: ["/usr/bin/fish", "--init-command=/tmp/payload.fish", "-c", "/usr/bin/printf ok"] }, + ])( + "validateSystemRunCommandConsistency rejects shell-only rawCommand for startup wrapper %j", + ({ argv }) => { + expectRawCommandMismatch({ + argv, + rawCommand: "/usr/bin/printf ok", + }); + }, + ); + + test("validateSystemRunCommandConsistency accepts full rawCommand for startup wrapper argv", () => { + const raw = '/bin/bash --login -c "/usr/bin/printf ok"'; + const res = expectValidResult( + validateSystemRunCommandConsistency({ + argv: ["/bin/bash", "--login", "-c", "/usr/bin/printf ok"], + rawCommand: raw, + }), + ); + expect(res.shellPayload).toBe(null); + expect(res.commandText).toBe(raw); + expect(res.previewText).toBe(null); + }); + test("validateSystemRunCommandConsistency accepts full rawCommand for env assignment prelude", () => { const raw = '/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo hi"'; const res = expectValidResult( @@ -164,7 +215,7 @@ describe("system run command helpers", () => { rawCommand: raw, }), ); - expect(res.shellPayload).toBe("echo hi"); + expect(res.shellPayload).toBe(null); expect(res.commandText).toBe(raw); expect(res.previewText).toBe(null); }); @@ -241,9 +292,9 @@ describe("system run command helpers", () => { resolveSystemRunCommand({ command: ["/usr/bin/arch", "-arm64", "/bin/sh", "-lc", "echo hi"], }), - expectedShellPayload: process.platform === "darwin" ? "echo hi" : null, + expectedShellPayload: null, expectedCommandText: '/usr/bin/arch -arm64 /bin/sh -lc "echo hi"', - expectedPreviewText: process.platform === "darwin" ? "echo hi" : null, + expectedPreviewText: null, }, { name: "resolveSystemRunCommand unwraps xcrun before deriving shell previews", @@ -251,9 +302,9 @@ describe("system run command helpers", () => { resolveSystemRunCommand({ command: ["/usr/bin/xcrun", "/bin/sh", "-lc", "echo hi"], }), - expectedShellPayload: process.platform === "darwin" ? "echo hi" : null, + expectedShellPayload: null, expectedCommandText: '/usr/bin/xcrun /bin/sh -lc "echo hi"', - expectedPreviewText: process.platform === "darwin" ? "echo hi" : null, + expectedPreviewText: null, }, { name: "resolveSystemRunCommandRequest accepts legacy shell payloads but returns canonical command text", @@ -273,7 +324,7 @@ describe("system run command helpers", () => { resolveSystemRunCommand({ command: ["/bin/sh", "-lc", '$0 "$1"', "/usr/bin/touch", "/tmp/marker"], }), - expectedShellPayload: '$0 "$1"', + expectedShellPayload: null, expectedCommandText: '/bin/sh -lc "$0 \\"$1\\"" /usr/bin/touch /tmp/marker', expectedPreviewText: null, }, @@ -283,7 +334,7 @@ describe("system run command helpers", () => { resolveSystemRunCommand({ command: ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo hi"], }), - expectedShellPayload: "echo hi", + expectedShellPayload: null, expectedCommandText: '/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc "echo hi"', expectedPreviewText: null, }, diff --git a/src/infra/system-run-command.ts b/src/infra/system-run-command.ts index f9974f648e9..36b1d7c505b 100644 --- a/src/infra/system-run-command.ts +++ b/src/infra/system-run-command.ts @@ -105,8 +105,15 @@ function hasTrailingPositionalArgvAfterInlineCommand(argv: string[]): boolean { return wrapperArgv.slice(inlineCommandIndex + 1).some((entry) => entry.trim().length > 0); } -function buildSystemRunCommandDisplay(argv: string[]): SystemRunCommandDisplay { - const shellWrapperResolution = extractShellWrapperCommand(argv); +function buildSystemRunCommandDisplay( + argv: string[], + rawCommand: string | null, +): SystemRunCommandDisplay { + const rawlessShellWrapperResolution = extractShellWrapperCommand(argv); + const shellWrapperResolution = + rawlessShellWrapperResolution.command === null && rawCommand !== null + ? extractShellWrapperCommand(argv, rawCommand) + : rawlessShellWrapperResolution; const shellPayload = shellWrapperResolution.command; const shellWrapperPositionalArgv = hasTrailingPositionalArgvAfterInlineCommand(argv); const envManipulationBeforeShellWrapper = @@ -133,7 +140,7 @@ export function validateSystemRunCommandConsistency(params: { allowLegacyShellText?: boolean; }): SystemRunCommandValidation { const raw = normalizeRawCommandText(params.rawCommand); - const display = buildSystemRunCommandDisplay(params.argv); + const display = buildSystemRunCommandDisplay(params.argv, raw); if (raw) { const matchesCanonicalArgv = raw === display.commandText; diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index 0df201108d4..40fbe0ce3d3 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -8,6 +8,7 @@ import type { import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js"; import { isInterpreterLikeSafeBin } from "../infra/exec-safe-bin-runtime-policy.js"; import { + isBlockedShellWrapperCommand, POSIX_SHELL_WRAPPERS, normalizeExecutableToken, unwrapKnownDispatchWrapperInvocation, @@ -1303,6 +1304,12 @@ export function buildSystemRunApprovalPlan(params: { if (command.argv.length === 0) { return { ok: false, message: "command required" }; } + if (command.shellPayload === null && isBlockedShellWrapperCommand(command.argv)) { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command", + }; + } const hardening = hardenApprovedExecutionPaths({ approvedByAsk: true, argv: command.argv, diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index de3265952af..e027f0b2ac9 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -1528,7 +1528,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { const tempDir = createFixtureDir("openclaw-shell-wrapper-allow-"); const prepared = buildSystemRunApprovalPlan({ - command: ["/bin/sh", "-lc", "cd ."], + command: ["/bin/sh", "-c", "cd ."], cwd: tempDir, }); expect(prepared.ok).toBe(true); diff --git a/test/fixtures/system-run-command-contract.json b/test/fixtures/system-run-command-contract.json index 943981078ea..dc3b45f323f 100644 --- a/test/fixtures/system-run-command-contract.json +++ b/test/fixtures/system-run-command-contract.json @@ -26,6 +26,15 @@ "displayCommand": "/bin/sh -lc \"echo hi\"" } }, + { + "name": "non-sh login shell wrapper requires full argv display binding", + "command": ["/bin/bash", "-lc", "/usr/bin/printf ok"], + "rawCommand": "/usr/bin/printf ok", + "expected": { + "valid": false, + "errorContains": "rawCommand does not match command" + } + }, { "name": "shell wrapper positional argv carrier requires full argv display binding", "command": ["/bin/sh", "-lc", "$0 \"$1\"", "/usr/bin/touch", "/tmp/marker"], @@ -46,11 +55,11 @@ }, { "name": "env wrapper shell payload accepted at ingress when prelude has no env modifiers", - "command": ["/usr/bin/env", "bash", "-lc", "echo hi"], + "command": ["/usr/bin/env", "sh", "-lc", "echo hi"], "rawCommand": "echo hi", "expected": { "valid": true, - "displayCommand": "/usr/bin/env bash -lc \"echo hi\"" + "displayCommand": "/usr/bin/env sh -lc \"echo hi\"" } }, { @@ -79,6 +88,42 @@ "valid": true, "displayCommand": "/usr/bin/env BASH_ENV=/tmp/payload.sh bash -lc \"echo hi\"" } + }, + { + "name": "login shell wrapper requires full argv display binding", + "command": ["/bin/bash", "--login", "-c", "/usr/bin/printf ok"], + "rawCommand": "/usr/bin/printf ok", + "expected": { + "valid": false, + "errorContains": "rawCommand does not match command" + } + }, + { + "name": "login shell wrapper accepts canonical full argv raw command", + "command": ["/bin/bash", "--login", "-c", "/usr/bin/printf ok"], + "rawCommand": "/bin/bash --login -c \"/usr/bin/printf ok\"", + "expected": { + "valid": true, + "displayCommand": "/bin/bash --login -c \"/usr/bin/printf ok\"" + } + }, + { + "name": "interactive shell wrapper requires full argv display binding", + "command": ["/bin/bash", "-i", "-c", "/usr/bin/printf ok"], + "rawCommand": "/usr/bin/printf ok", + "expected": { + "valid": false, + "errorContains": "rawCommand does not match command" + } + }, + { + "name": "fish init-command wrapper requires full argv display binding", + "command": ["/usr/bin/fish", "--init-command=/tmp/payload.fish", "-c", "/usr/bin/printf ok"], + "rawCommand": "/usr/bin/printf ok", + "expected": { + "valid": false, + "errorContains": "rawCommand does not match command" + } } ] }