Skip to content

Commit dd41fad

Browse files
committed
fix(macos): enforce path-only exec allowlist patterns
1 parent 2712883 commit dd41fad

5 files changed

Lines changed: 81 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Docs: https://docs.openclaw.ai
2020
- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting.
2121
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
2222
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.
23-
- Security/macOS app beta: harden `system.run` allowlist handling by evaluating shell chains per segment, treating control/expansion syntax as approval-required misses, and failing closed on unsafe parse cases (including quoted command substitution/backticks). Default installs are unaffected unless `tools.exec.host` is explicitly enabled. This ships in the next npm release. Thanks @tdjackey for reporting.
23+
- Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting.
2424
- Security/Archive: block zip symlink escapes during archive extraction.
2525
- Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported, and canonicalize resolved Discord allowlist names to IDs at runtime without rewriting config files. Thanks @tdjackey for reporting.
2626
- Security/Gateway: block node-role connections when device identity metadata is missing.

apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ enum ExecAllowlistMatcher {
55
guard let resolution, !entries.isEmpty else { return nil }
66
let rawExecutable = resolution.rawExecutable
77
let resolvedPath = resolution.resolvedPath
8-
let executableName = resolution.executableName
98

109
for entry in entries {
1110
let pattern = entry.pattern.trimmingCharacters(in: .whitespacesAndNewlines)
@@ -14,8 +13,6 @@ enum ExecAllowlistMatcher {
1413
if hasPath {
1514
let target = resolvedPath ?? rawExecutable
1615
if self.matches(pattern: pattern, target: target) { return entry }
17-
} else if self.matches(pattern: pattern, target: executableName) {
18-
return entry
1916
}
2017
}
2118
return nil

apps/macos/Sources/OpenClaw/ExecApprovals.swift

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ enum ExecApprovalsStore {
306306
}
307307

308308
static func ensureFile() -> ExecApprovalsFile {
309-
var file = self.loadFile()
309+
var file = self.normalizeIncoming(self.loadFile())
310310
if file.socket == nil { file.socket = ExecApprovalsSocketConfig(path: nil, token: nil) }
311311
let path = file.socket?.path?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
312312
if path.isEmpty {
@@ -316,6 +316,18 @@ enum ExecApprovalsStore {
316316
if token.isEmpty {
317317
file.socket?.token = self.generateToken()
318318
}
319+
if var agents = file.agents {
320+
for (key, entry) in agents {
321+
guard let allowlist = entry.allowlist else { continue }
322+
let migrated = allowlist.map { self.migrateLegacyPattern($0) }
323+
if migrated != allowlist {
324+
var next = entry
325+
next.allowlist = migrated
326+
agents[key] = next
327+
}
328+
}
329+
file.agents = agents.isEmpty ? nil : agents
330+
}
319331
if file.agents == nil { file.agents = [:] }
320332
self.saveFile(file)
321333
return file
@@ -400,7 +412,7 @@ enum ExecApprovalsStore {
400412

401413
static func addAllowlistEntry(agentId: String?, pattern: String) {
402414
let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines)
403-
guard !trimmed.isEmpty else { return }
415+
guard !trimmed.isEmpty, self.isPathPattern(trimmed) else { return }
404416
self.updateFile { file in
405417
let key = self.agentKey(agentId)
406418
var agents = file.agents ?? [:]
@@ -453,7 +465,7 @@ enum ExecApprovalsStore {
453465
lastUsedCommand: item.lastUsedCommand,
454466
lastResolvedPath: item.lastResolvedPath)
455467
}
456-
.filter { !$0.pattern.isEmpty }
468+
.filter { !$0.pattern.isEmpty && self.isPathPattern($0.pattern) }
457469
entry.allowlist = cleaned
458470
agents[key] = entry
459471
file.agents = agents
@@ -523,6 +535,37 @@ enum ExecApprovalsStore {
523535
return trimmed.isEmpty ? nil : trimmed.lowercased()
524536
}
525537

538+
private static func isPathPattern(_ pattern: String) -> Bool {
539+
pattern.contains("/") || pattern.contains("~") || pattern.contains("\\")
540+
}
541+
542+
private static func migrateLegacyPattern(_ entry: ExecAllowlistEntry) -> ExecAllowlistEntry {
543+
let trimmedPattern = entry.pattern.trimmingCharacters(in: .whitespacesAndNewlines)
544+
let trimmedResolved = entry.lastResolvedPath?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
545+
guard !trimmedPattern.isEmpty else {
546+
return ExecAllowlistEntry(
547+
id: entry.id,
548+
pattern: trimmedPattern,
549+
lastUsedAt: entry.lastUsedAt,
550+
lastUsedCommand: entry.lastUsedCommand,
551+
lastResolvedPath: entry.lastResolvedPath)
552+
}
553+
if self.isPathPattern(trimmedPattern) || trimmedResolved.isEmpty || !self.isPathPattern(trimmedResolved) {
554+
return ExecAllowlistEntry(
555+
id: entry.id,
556+
pattern: trimmedPattern,
557+
lastUsedAt: entry.lastUsedAt,
558+
lastUsedCommand: entry.lastUsedCommand,
559+
lastResolvedPath: entry.lastResolvedPath)
560+
}
561+
return ExecAllowlistEntry(
562+
id: entry.id,
563+
pattern: trimmedResolved,
564+
lastUsedAt: entry.lastUsedAt,
565+
lastUsedCommand: entry.lastUsedCommand,
566+
lastResolvedPath: entry.lastResolvedPath)
567+
}
568+
526569
private static func mergeAgents(
527570
current: ExecApprovalsAgent,
528571
legacy: ExecApprovalsAgent) -> ExecApprovalsAgent

apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,22 @@ struct SystemRunSettingsView: View {
105105
.foregroundStyle(.secondary)
106106
} else {
107107
HStack(spacing: 8) {
108-
TextField("Add allowlist pattern (case-insensitive globs)", text: self.$newPattern)
108+
TextField("Add allowlist path pattern (case-insensitive globs)", text: self.$newPattern)
109109
.textFieldStyle(.roundedBorder)
110110
Button("Add") {
111111
let pattern = self.newPattern.trimmingCharacters(in: .whitespacesAndNewlines)
112-
guard !pattern.isEmpty else { return }
112+
guard self.model.isPathPattern(pattern) else { return }
113113
self.model.addEntry(pattern)
114114
self.newPattern = ""
115115
}
116116
.buttonStyle(.bordered)
117-
.disabled(self.newPattern.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
117+
.disabled(!self.model.isPathPattern(self.newPattern))
118118
}
119119

120+
Text("Path patterns only. Basename entries like \"echo\" are ignored.")
121+
.font(.footnote)
122+
.foregroundStyle(.secondary)
123+
120124
if self.model.entries.isEmpty {
121125
Text("No allowlisted commands yet.")
122126
.font(.footnote)
@@ -370,15 +374,19 @@ final class ExecApprovalsSettingsModel {
370374
func addEntry(_ pattern: String) {
371375
guard !self.isDefaultsScope else { return }
372376
let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines)
373-
guard !trimmed.isEmpty else { return }
377+
guard self.isPathPattern(trimmed) else { return }
374378
self.entries.append(ExecAllowlistEntry(pattern: trimmed, lastUsedAt: nil))
375379
ExecApprovalsStore.updateAllowlist(agentId: self.selectedAgentId, allowlist: self.entries)
376380
}
377381

378382
func updateEntry(_ entry: ExecAllowlistEntry, id: UUID) {
379383
guard !self.isDefaultsScope else { return }
380384
guard let index = self.entries.firstIndex(where: { $0.id == id }) else { return }
381-
self.entries[index] = entry
385+
var next = entry
386+
let trimmed = next.pattern.trimmingCharacters(in: .whitespacesAndNewlines)
387+
guard self.isPathPattern(trimmed) else { return }
388+
next.pattern = trimmed
389+
self.entries[index] = next
382390
ExecApprovalsStore.updateAllowlist(agentId: self.selectedAgentId, allowlist: self.entries)
383391
}
384392

@@ -393,6 +401,12 @@ final class ExecApprovalsSettingsModel {
393401
self.entries.first(where: { $0.id == id })
394402
}
395403

404+
func isPathPattern(_ pattern: String) -> Bool {
405+
let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines)
406+
guard !trimmed.isEmpty else { return false }
407+
return trimmed.contains("/") || trimmed.contains("~") || trimmed.contains("\\")
408+
}
409+
396410
func refreshSkillBins(force: Bool = false) async {
397411
guard self.autoAllowSkills else {
398412
self.skillBins = []

apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import Foundation
22
import Testing
33
@testable import OpenClaw
44

5+
/// These cases cover optional `security=allowlist` behavior.
6+
/// Default install posture remains deny-by-default for exec on macOS node-host.
57
struct ExecAllowlistTests {
68
@Test func matchUsesResolvedPath() {
79
let entry = ExecAllowlistEntry(pattern: "/opt/homebrew/bin/rg")
@@ -14,15 +16,26 @@ struct ExecAllowlistTests {
1416
#expect(match?.pattern == entry.pattern)
1517
}
1618

17-
@Test func matchUsesBasenameForSimplePattern() {
19+
@Test func matchIgnoresBasenamePattern() {
1820
let entry = ExecAllowlistEntry(pattern: "rg")
1921
let resolution = ExecCommandResolution(
2022
rawExecutable: "rg",
2123
resolvedPath: "/opt/homebrew/bin/rg",
2224
executableName: "rg",
2325
cwd: nil)
2426
let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution)
25-
#expect(match?.pattern == entry.pattern)
27+
#expect(match == nil)
28+
}
29+
30+
@Test func matchIgnoresBasenameForRelativeExecutable() {
31+
let entry = ExecAllowlistEntry(pattern: "echo")
32+
let resolution = ExecCommandResolution(
33+
rawExecutable: "./echo",
34+
resolvedPath: "/tmp/oc-basename/echo",
35+
executableName: "echo",
36+
cwd: "/tmp/oc-basename")
37+
let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution)
38+
#expect(match == nil)
2639
}
2740

2841
@Test func matchIsCaseInsensitive() {

0 commit comments

Comments
 (0)