From dd41fadcaf58fd9deb963d6e163c56161e7b35dd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 22:58:18 +0100 Subject: [PATCH] fix(macos): enforce path-only exec allowlist patterns --- CHANGELOG.md | 2 +- .../OpenClaw/ExecAllowlistMatcher.swift | 3 -- .../Sources/OpenClaw/ExecApprovals.swift | 49 +++++++++++++++++-- .../OpenClaw/SystemRunSettingsView.swift | 24 +++++++-- .../OpenClawIPCTests/ExecAllowlistTests.swift | 17 ++++++- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f801a6a042..9f1efd75ab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ Docs: https://docs.openclaw.ai - 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. - Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating. - 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. -- 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. +- 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. - Security/Archive: block zip symlink escapes during archive extraction. - 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. - Security/Gateway: block node-role connections when device identity metadata is missing. diff --git a/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift b/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift index 4a7484c15a2..94fa780fce4 100644 --- a/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift +++ b/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift @@ -5,7 +5,6 @@ enum ExecAllowlistMatcher { guard let resolution, !entries.isEmpty else { return nil } let rawExecutable = resolution.rawExecutable let resolvedPath = resolution.resolvedPath - let executableName = resolution.executableName for entry in entries { let pattern = entry.pattern.trimmingCharacters(in: .whitespacesAndNewlines) @@ -14,8 +13,6 @@ enum ExecAllowlistMatcher { if hasPath { let target = resolvedPath ?? rawExecutable if self.matches(pattern: pattern, target: target) { return entry } - } else if self.matches(pattern: pattern, target: executableName) { - return entry } } return nil diff --git a/apps/macos/Sources/OpenClaw/ExecApprovals.swift b/apps/macos/Sources/OpenClaw/ExecApprovals.swift index 338525d6427..4e078edbc05 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovals.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovals.swift @@ -306,7 +306,7 @@ enum ExecApprovalsStore { } static func ensureFile() -> ExecApprovalsFile { - var file = self.loadFile() + var file = self.normalizeIncoming(self.loadFile()) if file.socket == nil { file.socket = ExecApprovalsSocketConfig(path: nil, token: nil) } let path = file.socket?.path?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" if path.isEmpty { @@ -316,6 +316,18 @@ enum ExecApprovalsStore { if token.isEmpty { file.socket?.token = self.generateToken() } + if var agents = file.agents { + for (key, entry) in agents { + guard let allowlist = entry.allowlist else { continue } + let migrated = allowlist.map { self.migrateLegacyPattern($0) } + if migrated != allowlist { + var next = entry + next.allowlist = migrated + agents[key] = next + } + } + file.agents = agents.isEmpty ? nil : agents + } if file.agents == nil { file.agents = [:] } self.saveFile(file) return file @@ -400,7 +412,7 @@ enum ExecApprovalsStore { static func addAllowlistEntry(agentId: String?, pattern: String) { let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return } + guard !trimmed.isEmpty, self.isPathPattern(trimmed) else { return } self.updateFile { file in let key = self.agentKey(agentId) var agents = file.agents ?? [:] @@ -453,7 +465,7 @@ enum ExecApprovalsStore { lastUsedCommand: item.lastUsedCommand, lastResolvedPath: item.lastResolvedPath) } - .filter { !$0.pattern.isEmpty } + .filter { !$0.pattern.isEmpty && self.isPathPattern($0.pattern) } entry.allowlist = cleaned agents[key] = entry file.agents = agents @@ -523,6 +535,37 @@ enum ExecApprovalsStore { return trimmed.isEmpty ? nil : trimmed.lowercased() } + private static func isPathPattern(_ pattern: String) -> Bool { + pattern.contains("/") || pattern.contains("~") || pattern.contains("\\") + } + + private static func migrateLegacyPattern(_ entry: ExecAllowlistEntry) -> ExecAllowlistEntry { + let trimmedPattern = entry.pattern.trimmingCharacters(in: .whitespacesAndNewlines) + let trimmedResolved = entry.lastResolvedPath?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + guard !trimmedPattern.isEmpty else { + return ExecAllowlistEntry( + id: entry.id, + pattern: trimmedPattern, + lastUsedAt: entry.lastUsedAt, + lastUsedCommand: entry.lastUsedCommand, + lastResolvedPath: entry.lastResolvedPath) + } + if self.isPathPattern(trimmedPattern) || trimmedResolved.isEmpty || !self.isPathPattern(trimmedResolved) { + return ExecAllowlistEntry( + id: entry.id, + pattern: trimmedPattern, + lastUsedAt: entry.lastUsedAt, + lastUsedCommand: entry.lastUsedCommand, + lastResolvedPath: entry.lastResolvedPath) + } + return ExecAllowlistEntry( + id: entry.id, + pattern: trimmedResolved, + lastUsedAt: entry.lastUsedAt, + lastUsedCommand: entry.lastUsedCommand, + lastResolvedPath: entry.lastResolvedPath) + } + private static func mergeAgents( current: ExecApprovalsAgent, legacy: ExecApprovalsAgent) -> ExecApprovalsAgent diff --git a/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift b/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift index b9bd6bd0c8c..d5fa76b05ca 100644 --- a/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift +++ b/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift @@ -105,18 +105,22 @@ struct SystemRunSettingsView: View { .foregroundStyle(.secondary) } else { HStack(spacing: 8) { - TextField("Add allowlist pattern (case-insensitive globs)", text: self.$newPattern) + TextField("Add allowlist path pattern (case-insensitive globs)", text: self.$newPattern) .textFieldStyle(.roundedBorder) Button("Add") { let pattern = self.newPattern.trimmingCharacters(in: .whitespacesAndNewlines) - guard !pattern.isEmpty else { return } + guard self.model.isPathPattern(pattern) else { return } self.model.addEntry(pattern) self.newPattern = "" } .buttonStyle(.bordered) - .disabled(self.newPattern.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + .disabled(!self.model.isPathPattern(self.newPattern)) } + Text("Path patterns only. Basename entries like \"echo\" are ignored.") + .font(.footnote) + .foregroundStyle(.secondary) + if self.model.entries.isEmpty { Text("No allowlisted commands yet.") .font(.footnote) @@ -370,7 +374,7 @@ final class ExecApprovalsSettingsModel { func addEntry(_ pattern: String) { guard !self.isDefaultsScope else { return } let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return } + guard self.isPathPattern(trimmed) else { return } self.entries.append(ExecAllowlistEntry(pattern: trimmed, lastUsedAt: nil)) ExecApprovalsStore.updateAllowlist(agentId: self.selectedAgentId, allowlist: self.entries) } @@ -378,7 +382,11 @@ final class ExecApprovalsSettingsModel { func updateEntry(_ entry: ExecAllowlistEntry, id: UUID) { guard !self.isDefaultsScope else { return } guard let index = self.entries.firstIndex(where: { $0.id == id }) else { return } - self.entries[index] = entry + var next = entry + let trimmed = next.pattern.trimmingCharacters(in: .whitespacesAndNewlines) + guard self.isPathPattern(trimmed) else { return } + next.pattern = trimmed + self.entries[index] = next ExecApprovalsStore.updateAllowlist(agentId: self.selectedAgentId, allowlist: self.entries) } @@ -393,6 +401,12 @@ final class ExecApprovalsSettingsModel { self.entries.first(where: { $0.id == id }) } + func isPathPattern(_ pattern: String) -> Bool { + let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return false } + return trimmed.contains("/") || trimmed.contains("~") || trimmed.contains("\\") + } + func refreshSkillBins(force: Bool = false) async { guard self.autoAllowSkills else { self.skillBins = [] diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index 89ab97748ac..c76a72e18a0 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -2,6 +2,8 @@ import Foundation import Testing @testable import OpenClaw +/// These cases cover optional `security=allowlist` behavior. +/// Default install posture remains deny-by-default for exec on macOS node-host. struct ExecAllowlistTests { @Test func matchUsesResolvedPath() { let entry = ExecAllowlistEntry(pattern: "/opt/homebrew/bin/rg") @@ -14,7 +16,7 @@ struct ExecAllowlistTests { #expect(match?.pattern == entry.pattern) } - @Test func matchUsesBasenameForSimplePattern() { + @Test func matchIgnoresBasenamePattern() { let entry = ExecAllowlistEntry(pattern: "rg") let resolution = ExecCommandResolution( rawExecutable: "rg", @@ -22,7 +24,18 @@ struct ExecAllowlistTests { executableName: "rg", cwd: nil) let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution) - #expect(match?.pattern == entry.pattern) + #expect(match == nil) + } + + @Test func matchIgnoresBasenameForRelativeExecutable() { + let entry = ExecAllowlistEntry(pattern: "echo") + let resolution = ExecCommandResolution( + rawExecutable: "./echo", + resolvedPath: "/tmp/oc-basename/echo", + executableName: "echo", + cwd: "/tmp/oc-basename") + let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution) + #expect(match == nil) } @Test func matchIsCaseInsensitive() {