mirror of https://github.com/openclaw/openclaw.git
fix(macos): enforce path-only exec allowlist patterns
This commit is contained in:
parent
2712883d16
commit
dd41fadcaf
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = []
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
Loading…
Reference in New Issue