diff --git a/CHANGELOG.md b/CHANGELOG.md index 49095b6d75a..ae2459d479e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai - Security/Synology Chat: enforce fail-closed allowlist behavior for DM ingress so `dmPolicy: "allowlist"` with empty `allowedUserIds` rejects all senders instead of allowing unauthorized dispatch. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Native images: enforce `tools.fs.workspaceOnly` for native prompt image auto-load (including history refs), preventing out-of-workspace sandbox mounts from being implicitly ingested as vision input. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Exec approvals: bind `system.run` command display/approval text to full argv when shell-wrapper inline payloads carry positional argv values, and reject payload-only `rawCommand` mismatches for those wrapper-carrier forms, preventing hidden command execution under misleading approval text. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Exec companion host: forward canonical `system.run` display text (not payload-only shell snippets) to the macOS exec host, and enforce rawCommand/argv consistency there for shell-wrapper positional-argv carriers and env-modifier preludes, preventing companion-side approval/display drift. This ships in the next npm release. - Security/Exec: limit default safe-bin trusted directories to immutable system paths (`/bin`, `/usr/bin`) and require explicit opt-in (`tools.exec.safeBinTrustedDirs`) for package-manager/user bin paths (for example Homebrew), add security-audit findings for risky trusted-dir choices, warn at runtime when explicitly trusted dirs are group/world writable, and add doctor hints when configured `safeBins` resolve outside trusted dirs. This ships in the next npm release. Thanks @tdjackey for reporting. - Telegram/Media fetch: prioritize IPv4 before IPv6 in SSRF pinned DNS address ordering so media downloads still work on hosts with broken IPv6 routing. (#24295, #23975) Thanks @Glucksberg. - Telegram/Replies: when markdown formatting renders to empty HTML (for example syntax-only chunks in threaded replies), retry delivery with plain text, and fail loud when both formatted and plain payloads are empty to avoid false delivered states. (#25096, #25091) Thanks @Glucksberg. diff --git a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift index 362a7da01d8..130e9473117 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift @@ -361,7 +361,24 @@ private enum ExecHostExecutor { reason: "invalid") } - let context = await self.buildContext(request: request, command: command) + let validatedCommand = ExecSystemRunCommandValidator.resolve( + command: command, + rawCommand: request.rawCommand) + let displayCommand: String + switch validatedCommand { + case .ok(let resolved): + displayCommand = resolved.displayCommand + case .invalid(let message): + return self.errorResponse( + code: "INVALID_REQUEST", + message: message, + reason: "invalid") + } + + let context = await self.buildContext( + request: request, + command: command, + rawCommand: displayCommand) if context.security == .deny { return self.errorResponse( code: "UNAVAILABLE", @@ -451,10 +468,14 @@ private enum ExecHostExecutor { timeoutMs: request.timeoutMs) } - private static func buildContext(request: ExecHostRequest, command: [String]) async -> ExecApprovalContext { + private static func buildContext( + request: ExecHostRequest, + command: [String], + rawCommand: String?) async -> ExecApprovalContext + { await ExecApprovalEvaluator.evaluate( command: command, - rawCommand: request.rawCommand, + rawCommand: rawCommand, cwd: request.cwd, envOverrides: request.env, agentId: request.agentId) diff --git a/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift new file mode 100644 index 00000000000..f3bdac5e71e --- /dev/null +++ b/apps/macos/Sources/OpenClaw/ExecSystemRunCommandValidator.swift @@ -0,0 +1,416 @@ +import Foundation + +enum ExecSystemRunCommandValidator { + struct ResolvedCommand { + let displayCommand: String + } + + enum ValidationResult { + case ok(ResolvedCommand) + case invalid(message: String) + } + + private static let shellWrapperNames = Set([ + "ash", + "bash", + "cmd", + "dash", + "fish", + "ksh", + "powershell", + "pwsh", + "sh", + "zsh", + ]) + + private static let posixOrPowerShellInlineWrapperNames = Set([ + "ash", + "bash", + "dash", + "fish", + "ksh", + "powershell", + "pwsh", + "sh", + "zsh", + ]) + + private static let shellMultiplexerWrapperNames = Set(["busybox", "toybox"]) + private static let posixInlineCommandFlags = Set(["-lc", "-c", "--command"]) + private static let powershellInlineCommandFlags = Set(["-c", "-command", "--command"]) + + private static let envOptionsWithValue = Set([ + "-u", + "--unset", + "-c", + "--chdir", + "-s", + "--split-string", + "--default-signal", + "--ignore-signal", + "--block-signal", + ]) + private static let envFlagOptions = Set(["-i", "--ignore-environment", "-0", "--null"]) + private static let envInlineValuePrefixes = [ + "-u", + "-c", + "-s", + "--unset=", + "--chdir=", + "--split-string=", + "--default-signal=", + "--ignore-signal=", + "--block-signal=", + ] + + private struct EnvUnwrapResult { + let argv: [String] + let usesModifiers: Bool + } + + static func resolve(command: [String], rawCommand: String?) -> ValidationResult { + let normalizedRaw = self.normalizeRaw(rawCommand) + let shell = ExecShellWrapperParser.extract(command: command, rawCommand: nil) + let shellCommand = shell.isWrapper ? self.trimmedNonEmpty(shell.command) : nil + + let envManipulationBeforeShellWrapper = self.hasEnvManipulationBeforeShellWrapper(command) + let shellWrapperPositionalArgv = self.hasTrailingPositionalArgvAfterInlineCommand(command) + let mustBindDisplayToFullArgv = envManipulationBeforeShellWrapper || shellWrapperPositionalArgv + + let inferred: String + if let shellCommand, !mustBindDisplayToFullArgv { + inferred = shellCommand + } else { + inferred = ExecCommandFormatter.displayString(for: command) + } + + if let raw = normalizedRaw, raw != inferred { + return .invalid(message: "INVALID_REQUEST: rawCommand does not match command") + } + + return .ok(ResolvedCommand(displayCommand: normalizedRaw ?? inferred)) + } + + private static func normalizeRaw(_ rawCommand: String?) -> String? { + let trimmed = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + return trimmed.isEmpty ? nil : trimmed + } + + private static func trimmedNonEmpty(_ value: String?) -> String? { + let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + return trimmed.isEmpty ? nil : trimmed + } + + private static func normalizeExecutableToken(_ token: String) -> String { + let base = ExecCommandToken.basenameLower(token) + if base.hasSuffix(".exe") { + return String(base.dropLast(4)) + } + return base + } + + private static func isEnvAssignment(_ token: String) -> Bool { + token.range(of: #"^[A-Za-z_][A-Za-z0-9_]*=.*"#, options: .regularExpression) != nil + } + + private static func hasEnvInlineValuePrefix(_ lowerToken: String) -> Bool { + self.envInlineValuePrefixes.contains { lowerToken.hasPrefix($0) } + } + + private static func unwrapEnvInvocationWithMetadata(_ argv: [String]) -> EnvUnwrapResult? { + var idx = 1 + var expectsOptionValue = false + var usesModifiers = false + + while idx < argv.count { + let token = argv[idx].trimmingCharacters(in: .whitespacesAndNewlines) + if token.isEmpty { + idx += 1 + continue + } + if expectsOptionValue { + expectsOptionValue = false + usesModifiers = true + idx += 1 + continue + } + if token == "--" || token == "-" { + idx += 1 + break + } + if self.isEnvAssignment(token) { + usesModifiers = true + idx += 1 + continue + } + if !token.hasPrefix("-") || token == "-" { + break + } + + let lower = token.lowercased() + let flag = lower.split(separator: "=", maxSplits: 1).first.map(String.init) ?? lower + if self.envFlagOptions.contains(flag) { + usesModifiers = true + idx += 1 + continue + } + if self.envOptionsWithValue.contains(flag) { + usesModifiers = true + if !lower.contains("=") { + expectsOptionValue = true + } + idx += 1 + continue + } + if self.hasEnvInlineValuePrefix(lower) { + usesModifiers = true + idx += 1 + continue + } + return nil + } + + if expectsOptionValue { + return nil + } + guard idx < argv.count else { + return nil + } + return EnvUnwrapResult(argv: Array(argv[idx...]), usesModifiers: usesModifiers) + } + + private static func unwrapShellMultiplexerInvocation(_ argv: [String]) -> [String]? { + guard let token0 = self.trimmedNonEmpty(argv.first) else { + return nil + } + let wrapper = self.normalizeExecutableToken(token0) + guard self.shellMultiplexerWrapperNames.contains(wrapper) else { + return nil + } + + var appletIndex = 1 + if appletIndex < argv.count && argv[appletIndex].trimmingCharacters(in: .whitespacesAndNewlines) == "--" { + appletIndex += 1 + } + guard appletIndex < argv.count else { + return nil + } + let applet = argv[appletIndex].trimmingCharacters(in: .whitespacesAndNewlines) + guard !applet.isEmpty else { + return nil + } + let normalizedApplet = self.normalizeExecutableToken(applet) + guard self.shellWrapperNames.contains(normalizedApplet) else { + return nil + } + return Array(argv[appletIndex...]) + } + + private static func hasEnvManipulationBeforeShellWrapper( + _ argv: [String], + depth: Int = 0, + envManipulationSeen: Bool = false) -> Bool + { + if depth >= ExecEnvInvocationUnwrapper.maxWrapperDepth { + return false + } + guard let token0 = self.trimmedNonEmpty(argv.first) else { + return false + } + + let normalized = self.normalizeExecutableToken(token0) + if normalized == "env" { + guard let envUnwrap = self.unwrapEnvInvocationWithMetadata(argv) else { + return false + } + return self.hasEnvManipulationBeforeShellWrapper( + envUnwrap.argv, + depth: depth + 1, + envManipulationSeen: envManipulationSeen || envUnwrap.usesModifiers) + } + + if let shellMultiplexer = self.unwrapShellMultiplexerInvocation(argv) { + return self.hasEnvManipulationBeforeShellWrapper( + shellMultiplexer, + depth: depth + 1, + envManipulationSeen: envManipulationSeen) + } + + guard self.shellWrapperNames.contains(normalized) else { + return false + } + guard self.extractShellInlinePayload(argv, normalizedWrapper: normalized) != nil else { + return false + } + return envManipulationSeen + } + + private static func hasTrailingPositionalArgvAfterInlineCommand(_ argv: [String]) -> Bool { + let wrapperArgv = self.unwrapShellWrapperArgv(argv) + guard let token0 = self.trimmedNonEmpty(wrapperArgv.first) else { + return false + } + let wrapper = self.normalizeExecutableToken(token0) + guard self.posixOrPowerShellInlineWrapperNames.contains(wrapper) else { + return false + } + + let inlineCommandIndex: Int? + if wrapper == "powershell" || wrapper == "pwsh" { + inlineCommandIndex = self.resolveInlineCommandTokenIndex( + wrapperArgv, + flags: self.powershellInlineCommandFlags, + allowCombinedC: false) + } else { + inlineCommandIndex = self.resolveInlineCommandTokenIndex( + wrapperArgv, + flags: self.posixInlineCommandFlags, + allowCombinedC: true) + } + guard let inlineCommandIndex else { + return false + } + let start = inlineCommandIndex + 1 + guard start < wrapperArgv.count else { + return false + } + return wrapperArgv[start...].contains { !$0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } + } + + private static func unwrapShellWrapperArgv(_ argv: [String]) -> [String] { + var current = argv + for _ in 0.., + allowCombinedC: Bool) -> Int? + { + 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 idx + 1 < argv.count ? idx + 1 : nil + } + if allowCombinedC, let inlineOffset = self.combinedCommandInlineOffset(token) { + let inline = String(token.dropFirst(inlineOffset)) + .trimmingCharacters(in: .whitespacesAndNewlines) + if !inline.isEmpty { + return idx + } + return idx + 1 < argv.count ? idx + 1 : nil + } + idx += 1 + } + return 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? + { + if normalizedWrapper == "cmd" { + return self.extractCmdInlineCommand(argv) + } + if normalizedWrapper == "powershell" || normalizedWrapper == "pwsh" { + return self.extractInlineCommandByFlags( + argv, + flags: self.powershellInlineCommandFlags, + allowCombinedC: false) + } + return self.extractInlineCommandByFlags( + argv, + flags: self.posixInlineCommandFlags, + allowCombinedC: true) + } + + private static func extractInlineCommandByFlags( + _ argv: [String], + flags: Set, + allowCombinedC: Bool) -> String? + { + 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 self.trimmedNonEmpty(idx + 1 < argv.count ? argv[idx + 1] : nil) + } + if allowCombinedC, let inlineOffset = self.combinedCommandInlineOffset(token) { + let inline = String(token.dropFirst(inlineOffset)) + if let inlineValue = self.trimmedNonEmpty(inline) { + return inlineValue + } + return self.trimmedNonEmpty(idx + 1 < argv.count ? argv[idx + 1] : nil) + } + idx += 1 + } + return nil + } + + private static func extractCmdInlineCommand(_ argv: [String]) -> String? { + guard let idx = argv.firstIndex(where: { + let token = $0.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + return token == "/c" || token == "/k" + }) else { + return nil + } + let tailIndex = idx + 1 + guard tailIndex < argv.count else { + return nil + } + let payload = argv[tailIndex...].joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) + return payload.isEmpty ? nil : payload + } +} diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift new file mode 100644 index 00000000000..c6ad3319a65 --- /dev/null +++ b/apps/macos/Tests/OpenClawIPCTests/ExecSystemRunCommandValidatorTests.swift @@ -0,0 +1,50 @@ +import Foundation +import Testing +@testable import OpenClaw + +struct ExecSystemRunCommandValidatorTests { + @Test func rejectsPayloadOnlyRawForPositionalCarrierWrappers() { + let command = ["/bin/sh", "-lc", #"$0 "$1""#, "/usr/bin/touch", "/tmp/marker"] + let result = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: #"$0 "$1""#) + switch result { + case .ok: + Issue.record("expected rawCommand mismatch") + case .invalid(let message): + #expect(message.contains("rawCommand does not match command")) + } + } + + @Test func acceptsCanonicalDisplayForPositionalCarrierWrappers() { + let command = ["/bin/sh", "-lc", #"$0 "$1""#, "/usr/bin/touch", "/tmp/marker"] + let expected = ExecCommandFormatter.displayString(for: command) + let result = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: expected) + switch result { + case .ok(let resolved): + #expect(resolved.displayCommand == expected) + case .invalid(let message): + Issue.record("unexpected validation failure: \(message)") + } + } + + @Test func acceptsShellPayloadRawForTransparentEnvWrapper() { + let command = ["/usr/bin/env", "bash", "-lc", "echo hi"] + let result = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: "echo hi") + switch result { + case .ok(let resolved): + #expect(resolved.displayCommand == "echo hi") + case .invalid(let message): + Issue.record("unexpected validation failure: \(message)") + } + } + + @Test func rejectsShellPayloadRawForEnvModifierPrelude() { + let command = ["/usr/bin/env", "BASH_ENV=/tmp/payload.sh", "bash", "-lc", "echo hi"] + let result = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: "echo hi") + switch result { + case .ok: + Issue.record("expected rawCommand mismatch") + case .invalid(let message): + #expect(message.contains("rawCommand does not match command")) + } + } +} diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 2c6c55bd1ab..a3be712655d 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -121,6 +121,32 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { ); }); + it("forwards canonical cmdText to mac app exec host for positional-argv shell wrappers", async () => { + const { runViaMacAppExecHost } = await runSystemInvoke({ + preferMacAppExecHost: true, + command: ["/bin/sh", "-lc", '$0 "$1"', "/usr/bin/touch", "/tmp/marker"], + runViaResponse: { + ok: true, + payload: { + success: true, + stdout: "app-ok", + stderr: "", + timedOut: false, + exitCode: 0, + error: null, + }, + }, + }); + + expect(runViaMacAppExecHost).toHaveBeenCalledWith({ + approvals: expect.anything(), + request: expect.objectContaining({ + command: ["/bin/sh", "-lc", '$0 "$1"', "/usr/bin/touch", "/tmp/marker"], + rawCommand: '/bin/sh -lc "$0 \\"$1\\"" /usr/bin/touch /tmp/marker', + }), + }); + }); + it("handles transparent env wrappers in allowlist mode", async () => { const { runCommand, sendInvokeResult } = await runSystemInvoke({ preferMacAppExecHost: false, diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 897d8ebd111..c9b107a5d31 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -291,7 +291,6 @@ export async function handleSystemRunInvoke(opts: HandleSystemRunInvokeOptions): } const argv = command.argv; - const rawCommand = command.rawCommand ?? ""; const shellCommand = command.shellCommand; const cmdText = command.cmdText; const agentId = opts.params.agentId?.trim() || undefined; @@ -388,7 +387,9 @@ export async function handleSystemRunInvoke(opts: HandleSystemRunInvokeOptions): if (useMacAppExec) { const execRequest: ExecHostRequest = { command: plannedAllowlistArgv ?? argv, - rawCommand: rawCommand || shellCommand || null, + // Forward canonical display text so companion approval/prompt surfaces bind to + // the exact command context already validated on the node-host. + rawCommand: cmdText || null, cwd: opts.params.cwd ?? null, env: envOverrides ?? null, timeoutMs: opts.params.timeoutMs ?? null,