fix(security): harden system.run companion command binding

This commit is contained in:
Peter Steinberger
2026-02-25 00:01:53 +00:00
parent 8680240f7e
commit 55cf92578d
6 changed files with 520 additions and 5 deletions

View File

@@ -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.

View File

@@ -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)

View File

@@ -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..<ExecEnvInvocationUnwrapper.maxWrapperDepth {
guard let token0 = self.trimmedNonEmpty(current.first) else {
break
}
let normalized = self.normalizeExecutableToken(token0)
if normalized == "env" {
guard let envUnwrap = self.unwrapEnvInvocationWithMetadata(current),
!envUnwrap.usesModifiers,
!envUnwrap.argv.isEmpty
else {
break
}
current = envUnwrap.argv
continue
}
if let shellMultiplexer = self.unwrapShellMultiplexerInvocation(current) {
current = shellMultiplexer
continue
}
break
}
return current
}
private static func resolveInlineCommandTokenIndex(
_ argv: [String],
flags: Set<String>,
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<String>,
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
}
}

View File

@@ -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"))
}
}
}

View File

@@ -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,

View File

@@ -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,