fix(macos): align exec command parity (#50386)

* fix(macos): align exec command parity

* fix(macos): address exec review follow-ups
This commit is contained in:
Nimrod Gutman 2026-03-19 13:51:17 +02:00 committed by GitHub
parent 009a10bce2
commit c4a4050ce4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 268 additions and 43 deletions

View File

@ -9,6 +9,7 @@ struct ExecApprovalEvaluation {
let env: [String: String]
let resolution: ExecCommandResolution?
let allowlistResolutions: [ExecCommandResolution]
let allowAlwaysPatterns: [String]
let allowlistMatches: [ExecAllowlistEntry]
let allowlistSatisfied: Bool
let allowlistMatch: ExecAllowlistEntry?
@ -31,9 +32,16 @@ enum ExecApprovalEvaluator {
let shellWrapper = ExecShellWrapperParser.extract(command: command, rawCommand: rawCommand).isWrapper
let env = HostEnvSanitizer.sanitize(overrides: envOverrides, shellWrapper: shellWrapper)
let displayCommand = ExecCommandFormatter.displayString(for: command, rawCommand: rawCommand)
let allowlistRawCommand = ExecSystemRunCommandValidator.allowlistEvaluationRawCommand(
command: command,
rawCommand: rawCommand)
let allowlistResolutions = ExecCommandResolution.resolveForAllowlist(
command: command,
rawCommand: rawCommand,
rawCommand: allowlistRawCommand,
cwd: cwd,
env: env)
let allowAlwaysPatterns = ExecCommandResolution.resolveAllowAlwaysPatterns(
command: command,
cwd: cwd,
env: env)
let allowlistMatches = security == .allowlist
@ -60,6 +68,7 @@ enum ExecApprovalEvaluator {
env: env,
resolution: allowlistResolutions.first,
allowlistResolutions: allowlistResolutions,
allowAlwaysPatterns: allowAlwaysPatterns,
allowlistMatches: allowlistMatches,
allowlistSatisfied: allowlistSatisfied,
allowlistMatch: allowlistSatisfied ? allowlistMatches.first : nil,

View File

@ -378,7 +378,7 @@ private enum ExecHostExecutor {
let context = await self.buildContext(
request: request,
command: validatedRequest.command,
rawCommand: validatedRequest.displayCommand)
rawCommand: validatedRequest.evaluationRawCommand)
switch ExecHostRequestEvaluator.evaluate(
context: context,
@ -476,13 +476,7 @@ private enum ExecHostExecutor {
{
guard decision == .allowAlways, context.security == .allowlist else { return }
var seenPatterns = Set<String>()
for candidate in context.allowlistResolutions {
guard let pattern = ExecApprovalHelpers.allowlistPattern(
command: context.command,
resolution: candidate)
else {
continue
}
for pattern in context.allowAlwaysPatterns {
if seenPatterns.insert(pattern).inserted {
ExecApprovalsStore.addAllowlistEntry(agentId: context.agentId, pattern: pattern)
}

View File

@ -52,6 +52,23 @@ struct ExecCommandResolution {
return [resolution]
}
static func resolveAllowAlwaysPatterns(
command: [String],
cwd: String?,
env: [String: String]?) -> [String]
{
var patterns: [String] = []
var seen = Set<String>()
self.collectAllowAlwaysPatterns(
command: command,
cwd: cwd,
env: env,
depth: 0,
patterns: &patterns,
seen: &seen)
return patterns
}
static func resolve(command: [String], cwd: String?, env: [String: String]?) -> ExecCommandResolution? {
let effective = ExecEnvInvocationUnwrapper.unwrapDispatchWrappersForResolution(command)
guard let raw = effective.first?.trimmingCharacters(in: .whitespacesAndNewlines), !raw.isEmpty else {
@ -101,6 +118,115 @@ struct ExecCommandResolution {
return self.resolveExecutable(rawExecutable: raw, cwd: cwd, env: env)
}
private static func collectAllowAlwaysPatterns(
command: [String],
cwd: String?,
env: [String: String]?,
depth: Int,
patterns: inout [String],
seen: inout Set<String>)
{
guard depth < 3, !command.isEmpty else {
return
}
if let token0 = command.first?.trimmingCharacters(in: .whitespacesAndNewlines),
ExecCommandToken.basenameLower(token0) == "env",
let envUnwrapped = ExecEnvInvocationUnwrapper.unwrap(command),
!envUnwrapped.isEmpty
{
self.collectAllowAlwaysPatterns(
command: envUnwrapped,
cwd: cwd,
env: env,
depth: depth + 1,
patterns: &patterns,
seen: &seen)
return
}
if let shellMultiplexer = self.unwrapShellMultiplexerInvocation(command) {
self.collectAllowAlwaysPatterns(
command: shellMultiplexer,
cwd: cwd,
env: env,
depth: depth + 1,
patterns: &patterns,
seen: &seen)
return
}
let shell = ExecShellWrapperParser.extract(command: command, rawCommand: nil)
if shell.isWrapper {
guard let shellCommand = shell.command,
let segments = self.splitShellCommandChain(shellCommand)
else {
return
}
for segment in segments {
let tokens = self.tokenizeShellWords(segment)
guard !tokens.isEmpty else {
continue
}
self.collectAllowAlwaysPatterns(
command: tokens,
cwd: cwd,
env: env,
depth: depth + 1,
patterns: &patterns,
seen: &seen)
}
return
}
guard let resolution = self.resolve(command: command, cwd: cwd, env: env),
let pattern = ExecApprovalHelpers.allowlistPattern(command: command, resolution: resolution),
seen.insert(pattern).inserted
else {
return
}
patterns.append(pattern)
}
private static func unwrapShellMultiplexerInvocation(_ argv: [String]) -> [String]? {
guard let token0 = argv.first?.trimmingCharacters(in: .whitespacesAndNewlines), !token0.isEmpty else {
return nil
}
let wrapper = ExecCommandToken.basenameLower(token0)
guard wrapper == "busybox" || wrapper == "toybox" 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 = ExecCommandToken.basenameLower(applet)
let shellWrappers = Set([
"ash",
"bash",
"dash",
"fish",
"ksh",
"powershell",
"pwsh",
"sh",
"zsh",
])
guard shellWrappers.contains(normalizedApplet) else {
return nil
}
return Array(argv[appletIndex...])
}
private static func parseFirstToken(_ command: String) -> String? {
let trimmed = command.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return nil }

View File

@ -12,14 +12,24 @@ enum ExecCommandToken {
enum ExecEnvInvocationUnwrapper {
static let maxWrapperDepth = 4
struct UnwrapResult {
let command: [String]
let usesModifiers: Bool
}
private static func isEnvAssignment(_ token: String) -> Bool {
let pattern = #"^[A-Za-z_][A-Za-z0-9_]*=.*"#
return token.range(of: pattern, options: .regularExpression) != nil
}
static func unwrap(_ command: [String]) -> [String]? {
self.unwrapWithMetadata(command)?.command
}
static func unwrapWithMetadata(_ command: [String]) -> UnwrapResult? {
var idx = 1
var expectsOptionValue = false
var usesModifiers = false
while idx < command.count {
let token = command[idx].trimmingCharacters(in: .whitespacesAndNewlines)
if token.isEmpty {
@ -28,6 +38,7 @@ enum ExecEnvInvocationUnwrapper {
}
if expectsOptionValue {
expectsOptionValue = false
usesModifiers = true
idx += 1
continue
}
@ -36,6 +47,7 @@ enum ExecEnvInvocationUnwrapper {
break
}
if self.isEnvAssignment(token) {
usesModifiers = true
idx += 1
continue
}
@ -43,10 +55,12 @@ enum ExecEnvInvocationUnwrapper {
let lower = token.lowercased()
let flag = lower.split(separator: "=", maxSplits: 1).first.map(String.init) ?? lower
if ExecEnvOptions.flagOnly.contains(flag) {
usesModifiers = true
idx += 1
continue
}
if ExecEnvOptions.withValue.contains(flag) {
usesModifiers = true
if !lower.contains("=") {
expectsOptionValue = true
}
@ -63,6 +77,7 @@ enum ExecEnvInvocationUnwrapper {
lower.hasPrefix("--ignore-signal=") ||
lower.hasPrefix("--block-signal=")
{
usesModifiers = true
idx += 1
continue
}
@ -70,8 +85,8 @@ enum ExecEnvInvocationUnwrapper {
}
break
}
guard idx < command.count else { return nil }
return Array(command[idx...])
guard !expectsOptionValue, idx < command.count else { return nil }
return UnwrapResult(command: Array(command[idx...]), usesModifiers: usesModifiers)
}
static func unwrapDispatchWrappersForResolution(_ command: [String]) -> [String] {
@ -84,10 +99,13 @@ enum ExecEnvInvocationUnwrapper {
guard ExecCommandToken.basenameLower(token) == "env" else {
break
}
guard let unwrapped = self.unwrap(current), !unwrapped.isEmpty else {
guard let unwrapped = self.unwrapWithMetadata(current), !unwrapped.command.isEmpty else {
break
}
current = unwrapped
if unwrapped.usesModifiers {
break
}
current = unwrapped.command
depth += 1
}
return current

View File

@ -3,6 +3,7 @@ import Foundation
struct ExecHostValidatedRequest {
let command: [String]
let displayCommand: String
let evaluationRawCommand: String?
}
enum ExecHostPolicyDecision {
@ -27,7 +28,10 @@ enum ExecHostRequestEvaluator {
rawCommand: request.rawCommand)
switch validatedCommand {
case let .ok(resolved):
return .success(ExecHostValidatedRequest(command: command, displayCommand: resolved.displayCommand))
return .success(ExecHostValidatedRequest(
command: command,
displayCommand: resolved.displayCommand,
evaluationRawCommand: resolved.evaluationRawCommand))
case let .invalid(message):
return .failure(
ExecHostError(

View File

@ -3,6 +3,7 @@ import Foundation
enum ExecSystemRunCommandValidator {
struct ResolvedCommand {
let displayCommand: String
let evaluationRawCommand: String?
}
enum ValidationResult {
@ -52,18 +53,43 @@ enum ExecSystemRunCommandValidator {
let envManipulationBeforeShellWrapper = self.hasEnvManipulationBeforeShellWrapper(command)
let shellWrapperPositionalArgv = self.hasTrailingPositionalArgvAfterInlineCommand(command)
let mustBindDisplayToFullArgv = envManipulationBeforeShellWrapper || shellWrapperPositionalArgv
let inferred: String = if let shellCommand, !mustBindDisplayToFullArgv {
let formattedArgv = ExecCommandFormatter.displayString(for: command)
let previewCommand: String? = if let shellCommand, !mustBindDisplayToFullArgv {
shellCommand
} else {
ExecCommandFormatter.displayString(for: command)
nil
}
if let raw = normalizedRaw, raw != inferred {
if let raw = normalizedRaw, raw != formattedArgv, raw != previewCommand {
return .invalid(message: "INVALID_REQUEST: rawCommand does not match command")
}
return .ok(ResolvedCommand(displayCommand: normalizedRaw ?? inferred))
return .ok(ResolvedCommand(
displayCommand: formattedArgv,
evaluationRawCommand: self.allowlistEvaluationRawCommand(
normalizedRaw: normalizedRaw,
shellIsWrapper: shell.isWrapper,
previewCommand: previewCommand)))
}
static func allowlistEvaluationRawCommand(command: [String], rawCommand: String?) -> String? {
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 previewCommand: String? = if let shellCommand, !mustBindDisplayToFullArgv {
shellCommand
} else {
nil
}
return self.allowlistEvaluationRawCommand(
normalizedRaw: normalizedRaw,
shellIsWrapper: shell.isWrapper,
previewCommand: previewCommand)
}
private static func normalizeRaw(_ rawCommand: String?) -> String? {
@ -76,6 +102,20 @@ enum ExecSystemRunCommandValidator {
return trimmed.isEmpty ? nil : trimmed
}
private static func allowlistEvaluationRawCommand(
normalizedRaw: String?,
shellIsWrapper: Bool,
previewCommand: String?) -> String?
{
guard shellIsWrapper else {
return normalizedRaw
}
guard let normalizedRaw else {
return nil
}
return normalizedRaw == previewCommand ? normalizedRaw : nil
}
private static func normalizeExecutableToken(_ token: String) -> String {
let base = ExecCommandToken.basenameLower(token)
if base.hasSuffix(".exe") {

View File

@ -507,8 +507,7 @@ actor MacNodeRuntime {
persistAllowlist: persistAllowlist,
security: evaluation.security,
agentId: evaluation.agentId,
command: command,
allowlistResolutions: evaluation.allowlistResolutions)
allowAlwaysPatterns: evaluation.allowAlwaysPatterns)
if evaluation.security == .allowlist, !evaluation.allowlistSatisfied, !evaluation.skillAllow, !approvedByAsk {
await self.emitExecEvent(
@ -795,15 +794,11 @@ extension MacNodeRuntime {
persistAllowlist: Bool,
security: ExecSecurity,
agentId: String?,
command: [String],
allowlistResolutions: [ExecCommandResolution])
allowAlwaysPatterns: [String])
{
guard persistAllowlist, security == .allowlist else { return }
var seenPatterns = Set<String>()
for candidate in allowlistResolutions {
guard let pattern = ExecApprovalHelpers.allowlistPattern(command: command, resolution: candidate) else {
continue
}
for pattern in allowAlwaysPatterns {
if seenPatterns.insert(pattern).inserted {
ExecApprovalsStore.addAllowlistEntry(agentId: agentId, pattern: pattern)
}

View File

@ -45,7 +45,7 @@ import Testing
let nodePath = tmp.appendingPathComponent("node_modules/.bin/node")
let scriptPath = tmp.appendingPathComponent("bin/openclaw.js")
try makeExecutableForTests(at: nodePath)
try "#!/bin/sh\necho v22.0.0\n".write(to: nodePath, atomically: true, encoding: .utf8)
try "#!/bin/sh\necho v22.16.0\n".write(to: nodePath, atomically: true, encoding: .utf8)
try FileManager().setAttributes([.posixPermissions: 0o755], ofItemAtPath: nodePath.path)
try makeExecutableForTests(at: scriptPath)

View File

@ -240,7 +240,7 @@ struct ExecAllowlistTests {
#expect(resolutions[0].executableName == "touch")
}
@Test func `resolve for allowlist unwraps env assignments inside shell segments`() {
@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 resolutions = ExecCommandResolution.resolveForAllowlist(
command: command,
@ -248,11 +248,11 @@ struct ExecAllowlistTests {
cwd: nil,
env: ["PATH": "/usr/bin:/bin"])
#expect(resolutions.count == 1)
#expect(resolutions[0].resolvedPath == "/usr/bin/touch")
#expect(resolutions[0].executableName == "touch")
#expect(resolutions[0].resolvedPath == "/usr/bin/env")
#expect(resolutions[0].executableName == "env")
}
@Test func `resolve for allowlist unwraps env to effective direct executable`() {
@Test func `resolve for allowlist preserves env wrapper with modifiers`() {
let command = ["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"]
let resolutions = ExecCommandResolution.resolveForAllowlist(
command: command,
@ -260,8 +260,33 @@ struct ExecAllowlistTests {
cwd: nil,
env: ["PATH": "/usr/bin:/bin"])
#expect(resolutions.count == 1)
#expect(resolutions[0].resolvedPath == "/usr/bin/printf")
#expect(resolutions[0].executableName == "printf")
#expect(resolutions[0].resolvedPath == "/usr/bin/env")
#expect(resolutions[0].executableName == "env")
}
@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 evaluation = await ExecApprovalEvaluator.evaluate(
command: command,
rawCommand: rawCommand,
cwd: nil,
envOverrides: ["PATH": "/usr/bin:/bin"],
agentId: nil)
#expect(evaluation.displayCommand == rawCommand)
#expect(evaluation.allowlistResolutions.count == 1)
#expect(evaluation.allowlistResolutions[0].resolvedPath == "/usr/bin/printf")
#expect(evaluation.allowlistResolutions[0].executableName == "printf")
}
@Test func `allow always patterns unwrap env wrapper modifiers to the inner executable`() {
let patterns = ExecCommandResolution.resolveAllowAlwaysPatterns(
command: ["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"],
cwd: nil,
env: ["PATH": "/usr/bin:/bin"])
#expect(patterns == ["/usr/bin/printf"])
}
@Test func `match all requires every segment to match`() {

View File

@ -21,13 +21,12 @@ struct ExecApprovalsStoreRefactorTests {
try await self.withTempStateDir { _ in
_ = ExecApprovalsStore.ensureFile()
let url = ExecApprovalsStore.fileURL()
let firstWriteDate = try Self.modificationDate(at: url)
let firstIdentity = try Self.fileIdentity(at: url)
try await Task.sleep(nanoseconds: 1_100_000_000)
_ = ExecApprovalsStore.ensureFile()
let secondWriteDate = try Self.modificationDate(at: url)
let secondIdentity = try Self.fileIdentity(at: url)
#expect(firstWriteDate == secondWriteDate)
#expect(firstIdentity == secondIdentity)
}
}
@ -81,12 +80,12 @@ struct ExecApprovalsStoreRefactorTests {
}
}
private static func modificationDate(at url: URL) throws -> Date {
private static func fileIdentity(at url: URL) throws -> Int {
let attributes = try FileManager().attributesOfItem(atPath: url.path)
guard let date = attributes[.modificationDate] as? Date else {
struct MissingDateError: Error {}
throw MissingDateError()
guard let identifier = (attributes[.systemFileNumber] as? NSNumber)?.intValue else {
struct MissingIdentifierError: Error {}
throw MissingIdentifierError()
}
return date
return identifier
}
}

View File

@ -77,6 +77,7 @@ struct ExecHostRequestEvaluatorTests {
env: [:],
resolution: nil,
allowlistResolutions: [],
allowAlwaysPatterns: [],
allowlistMatches: [],
allowlistSatisfied: allowlistSatisfied,
allowlistMatch: nil,

View File

@ -50,6 +50,20 @@ struct ExecSystemRunCommandValidatorTests {
}
}
@Test func `validator keeps canonical wrapper text out of allowlist raw parsing`() {
let command = ["/bin/sh", "-lc", "/usr/bin/printf ok"]
let rawCommand = "/bin/sh -lc \"/usr/bin/printf ok\""
let result = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: rawCommand)
switch result {
case let .ok(resolved):
#expect(resolved.displayCommand == rawCommand)
#expect(resolved.evaluationRawCommand == nil)
case let .invalid(message):
Issue.record("unexpected invalid result: \(message)")
}
}
private static func loadContractCases() throws -> [SystemRunCommandContractCase] {
let fixtureURL = try self.findContractFixtureURL()
let data = try Data(contentsOf: fixtureURL)