mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-23 16:01:17 +00:00
fix(security): harden exec approval boundaries
This commit is contained in:
@@ -25,8 +25,16 @@ struct ExecCommandResolution {
|
||||
cwd: String?,
|
||||
env: [String: String]?) -> [ExecCommandResolution]
|
||||
{
|
||||
let shell = ExecShellWrapperParser.extract(command: command, rawCommand: rawCommand)
|
||||
// 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)
|
||||
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
|
||||
// full argv and must not be auto-allowlisted by payload-only matches.
|
||||
if ExecSystemRunCommandValidator.hasEnvManipulationBeforeShellWrapper(command) {
|
||||
return []
|
||||
}
|
||||
guard let shellCommand = shell.command,
|
||||
let segments = self.splitShellCommandChain(shellCommand)
|
||||
else {
|
||||
@@ -46,7 +54,12 @@ struct ExecCommandResolution {
|
||||
return resolutions
|
||||
}
|
||||
|
||||
guard let resolution = self.resolve(command: command, rawCommand: rawCommand, cwd: cwd, env: env) else {
|
||||
guard let resolution = self.resolveForAllowlistCommand(
|
||||
command: command,
|
||||
rawCommand: rawCommand,
|
||||
cwd: cwd,
|
||||
env: env)
|
||||
else {
|
||||
return []
|
||||
}
|
||||
return [resolution]
|
||||
@@ -70,6 +83,23 @@ struct ExecCommandResolution {
|
||||
}
|
||||
|
||||
static func resolve(command: [String], cwd: String?, env: [String: String]?) -> ExecCommandResolution? {
|
||||
let effective = ExecEnvInvocationUnwrapper.unwrapTransparentDispatchWrappersForResolution(command)
|
||||
guard let raw = effective.first?.trimmingCharacters(in: .whitespacesAndNewlines), !raw.isEmpty else {
|
||||
return nil
|
||||
}
|
||||
return self.resolveExecutable(rawExecutable: raw, cwd: cwd, env: env)
|
||||
}
|
||||
|
||||
private static func resolveForAllowlistCommand(
|
||||
command: [String],
|
||||
rawCommand: String?,
|
||||
cwd: String?,
|
||||
env: [String: String]?) -> ExecCommandResolution?
|
||||
{
|
||||
let trimmedRaw = rawCommand?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
|
||||
if !trimmedRaw.isEmpty, let token = self.parseFirstToken(trimmedRaw) {
|
||||
return self.resolveExecutable(rawExecutable: token, cwd: cwd, env: env)
|
||||
}
|
||||
let effective = ExecEnvInvocationUnwrapper.unwrapDispatchWrappersForResolution(command)
|
||||
guard let raw = effective.first?.trimmingCharacters(in: .whitespacesAndNewlines), !raw.isEmpty else {
|
||||
return nil
|
||||
|
||||
@@ -110,4 +110,50 @@ enum ExecEnvInvocationUnwrapper {
|
||||
}
|
||||
return current
|
||||
}
|
||||
|
||||
private static func unwrapTransparentEnvInvocation(_ command: [String]) -> [String]? {
|
||||
var idx = 1
|
||||
while idx < command.count {
|
||||
let token = command[idx].trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
if token.isEmpty {
|
||||
idx += 1
|
||||
continue
|
||||
}
|
||||
if token == "--" {
|
||||
idx += 1
|
||||
break
|
||||
}
|
||||
if token == "-" {
|
||||
return nil
|
||||
}
|
||||
if self.isEnvAssignment(token) {
|
||||
return nil
|
||||
}
|
||||
if token.hasPrefix("-"), token != "-" {
|
||||
return nil
|
||||
}
|
||||
break
|
||||
}
|
||||
guard idx < command.count else { return nil }
|
||||
return Array(command[idx...])
|
||||
}
|
||||
|
||||
static func unwrapTransparentDispatchWrappersForResolution(_ command: [String]) -> [String] {
|
||||
var current = command
|
||||
var depth = 0
|
||||
while depth < self.maxWrapperDepth {
|
||||
guard let token = current.first?.trimmingCharacters(in: .whitespacesAndNewlines), !token.isEmpty else {
|
||||
break
|
||||
}
|
||||
guard ExecCommandToken.basenameLower(token) == "env" else {
|
||||
break
|
||||
}
|
||||
guard let unwrapped = self.unwrapTransparentEnvInvocation(current), !unwrapped.isEmpty else {
|
||||
break
|
||||
}
|
||||
current = unwrapped
|
||||
depth += 1
|
||||
}
|
||||
return current
|
||||
}
|
||||
}
|
||||
|
||||
@@ -53,23 +53,27 @@ enum ExecSystemRunCommandValidator {
|
||||
let envManipulationBeforeShellWrapper = self.hasEnvManipulationBeforeShellWrapper(command)
|
||||
let shellWrapperPositionalArgv = self.hasTrailingPositionalArgvAfterInlineCommand(command)
|
||||
let mustBindDisplayToFullArgv = envManipulationBeforeShellWrapper || shellWrapperPositionalArgv
|
||||
let formattedArgv = ExecCommandFormatter.displayString(for: command)
|
||||
let previewCommand: String? = if let shellCommand, !mustBindDisplayToFullArgv {
|
||||
let canonicalDisplay = ExecCommandFormatter.displayString(for: command)
|
||||
let legacyShellDisplay: String? = if let shellCommand, !mustBindDisplayToFullArgv {
|
||||
shellCommand
|
||||
} else {
|
||||
nil
|
||||
}
|
||||
|
||||
if let raw = normalizedRaw, raw != formattedArgv, raw != previewCommand {
|
||||
return .invalid(message: "INVALID_REQUEST: rawCommand does not match command")
|
||||
if let raw = normalizedRaw {
|
||||
let matchesCanonical = raw == canonicalDisplay
|
||||
let matchesLegacyShellText = legacyShellDisplay == raw
|
||||
if !matchesCanonical, !matchesLegacyShellText {
|
||||
return .invalid(message: "INVALID_REQUEST: rawCommand does not match command")
|
||||
}
|
||||
}
|
||||
|
||||
return .ok(ResolvedCommand(
|
||||
displayCommand: formattedArgv,
|
||||
displayCommand: canonicalDisplay,
|
||||
evaluationRawCommand: self.allowlistEvaluationRawCommand(
|
||||
normalizedRaw: normalizedRaw,
|
||||
shellIsWrapper: shell.isWrapper,
|
||||
previewCommand: previewCommand)))
|
||||
previewCommand: legacyShellDisplay)))
|
||||
}
|
||||
|
||||
static func allowlistEvaluationRawCommand(command: [String], rawCommand: String?) -> String? {
|
||||
@@ -149,7 +153,12 @@ enum ExecSystemRunCommandValidator {
|
||||
idx += 1
|
||||
continue
|
||||
}
|
||||
if token == "--" || token == "-" {
|
||||
if token == "--" {
|
||||
idx += 1
|
||||
break
|
||||
}
|
||||
if token == "-" {
|
||||
usesModifiers = true
|
||||
idx += 1
|
||||
break
|
||||
}
|
||||
@@ -221,7 +230,7 @@ enum ExecSystemRunCommandValidator {
|
||||
return Array(argv[appletIndex...])
|
||||
}
|
||||
|
||||
private static func hasEnvManipulationBeforeShellWrapper(
|
||||
static func hasEnvManipulationBeforeShellWrapper(
|
||||
_ argv: [String],
|
||||
depth: Int = 0,
|
||||
envManipulationSeen: Bool = false) -> Bool
|
||||
|
||||
@@ -110,6 +110,41 @@ struct ExecAllowlistTests {
|
||||
#expect(resolutions[1].executableName == "touch")
|
||||
}
|
||||
|
||||
@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 resolutions = ExecCommandResolution.resolveForAllowlist(
|
||||
command: command,
|
||||
rawCommand: canonicalRaw,
|
||||
cwd: nil,
|
||||
env: ["PATH": "/usr/bin:/bin"])
|
||||
#expect(resolutions.count == 2)
|
||||
#expect(resolutions[0].executableName == "echo")
|
||||
#expect(resolutions[1].executableName == "touch")
|
||||
}
|
||||
|
||||
@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\""
|
||||
let resolutions = ExecCommandResolution.resolveForAllowlist(
|
||||
command: command,
|
||||
rawCommand: canonicalRaw,
|
||||
cwd: nil,
|
||||
env: ["PATH": "/usr/bin:/bin"])
|
||||
#expect(resolutions.isEmpty)
|
||||
}
|
||||
|
||||
@Test func `resolve for allowlist fails closed for env dash shell wrappers`() {
|
||||
let command = ["/usr/bin/env", "-", "bash", "-lc", "echo allowlisted"]
|
||||
let canonicalRaw = "/usr/bin/env - bash -lc \"echo allowlisted\""
|
||||
let resolutions = ExecCommandResolution.resolveForAllowlist(
|
||||
command: command,
|
||||
rawCommand: canonicalRaw,
|
||||
cwd: nil,
|
||||
env: ["PATH": "/usr/bin:/bin"])
|
||||
#expect(resolutions.isEmpty)
|
||||
}
|
||||
|
||||
@Test func `resolve for allowlist keeps quoted operators in single segment`() {
|
||||
let command = ["/bin/sh", "-lc", "echo \"a && b\""]
|
||||
let resolutions = ExecCommandResolution.resolveForAllowlist(
|
||||
@@ -200,6 +235,16 @@ struct ExecAllowlistTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Test func `resolve keeps env dash wrapper as effective executable`() {
|
||||
let resolution = ExecCommandResolution.resolve(
|
||||
command: ["/usr/bin/env", "-", "/usr/bin/printf", "ok"],
|
||||
cwd: nil,
|
||||
env: ["PATH": "/usr/bin:/bin"])
|
||||
#expect(resolution?.rawExecutable == "/usr/bin/env")
|
||||
#expect(resolution?.resolvedPath == "/usr/bin/env")
|
||||
#expect(resolution?.executableName == "env")
|
||||
}
|
||||
|
||||
@Test func `resolve for allowlist treats plain sh invocation as direct exec`() {
|
||||
let command = ["/bin/sh", "./script.sh"]
|
||||
let resolutions = ExecCommandResolution.resolveForAllowlist(
|
||||
|
||||
@@ -64,6 +64,27 @@ struct ExecSystemRunCommandValidatorTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Test func `env dash shell wrapper requires canonical raw command binding`() {
|
||||
let command = ["/usr/bin/env", "-", "bash", "-lc", "echo hi"]
|
||||
|
||||
let legacy = ExecSystemRunCommandValidator.resolve(command: command, rawCommand: "echo hi")
|
||||
switch legacy {
|
||||
case .ok:
|
||||
Issue.record("expected rawCommand mismatch for env dash prelude")
|
||||
case let .invalid(message):
|
||||
#expect(message.contains("rawCommand does not match command"))
|
||||
}
|
||||
|
||||
let canonicalRaw = "/usr/bin/env - bash -lc \"echo hi\""
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user