diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cf51487790..747a58bc816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Exec approvals: allow bare command-name allowlist patterns to match PATH-resolved executable basenames without trusting `./tool` or absolute path-selected binaries. Fixes #71315. Thanks @chen-zhang-cs-code and @dengluozhang. - Config/recovery: skip whole-file last-known-good rollback when invalidity is scoped to `plugins.entries.*`, preserving unrelated user settings during plugin schema or host-version skew. Fixes #71289. Thanks @jalehman. - Agents/tools: keep resolved reply-run configs from being overwritten by stale runtime snapshots, and let empty web runtime metadata fall back to configured provider auto-detection so standard and queued turns expose the same tool set. Fixes #71355. Thanks @c-g14. - Compaction: honor explicit `agents.defaults.compaction.keepRecentTokens` for manual `/compact`, re-distill safeguard summaries instead of snowballing previous summaries, and enable safeguard summary quality checks by default. Fixes #71357. Thanks @WhiteGiverMa. diff --git a/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift b/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift index ad40d2c3803..ef8db7ed76d 100644 --- a/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift +++ b/apps/macos/Sources/OpenClaw/ExecAllowlistMatcher.swift @@ -9,8 +9,14 @@ enum ExecAllowlistMatcher { for entry in entries { switch ExecApprovalHelpers.validateAllowlistPattern(entry.pattern) { case let .valid(pattern): - let target = resolvedPath ?? rawExecutable - if self.matches(pattern: pattern, target: target) { return entry } + if ExecApprovalHelpers.patternHasPathSelector(pattern) { + let target = resolvedPath ?? rawExecutable + if self.matches(pattern: pattern, target: target) { return entry } + } else if pattern != "*", + !ExecApprovalHelpers.patternHasPathSelector(rawExecutable), + self.matchesExecutableBasename(pattern: pattern, resolution: resolution) { + return entry + } case .invalid: continue } @@ -34,6 +40,20 @@ enum ExecAllowlistMatcher { return matches } + private static func matchesExecutableBasename( + pattern: String, + resolution: ExecCommandResolution) -> Bool + { + var candidates = Set() + if !resolution.executableName.isEmpty { + candidates.insert(resolution.executableName) + } + if let resolvedPath = resolution.resolvedPath, !resolvedPath.isEmpty { + candidates.insert(URL(fileURLWithPath: resolvedPath).lastPathComponent) + } + return candidates.contains { self.matches(pattern: pattern, target: $0) } + } + private static func matches(pattern: String, target: String) -> Bool { let trimmed = pattern.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { return false } diff --git a/apps/macos/Sources/OpenClaw/ExecApprovals.swift b/apps/macos/Sources/OpenClaw/ExecApprovals.swift index 141da33ad48..d79ff5fc16c 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovals.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovals.swift @@ -616,6 +616,17 @@ enum ExecApprovalsStore { let trimmedResolved = entry.lastResolvedPath?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" let normalizedResolved = trimmedResolved.isEmpty ? nil : trimmedResolved + if !ExecApprovalHelpers.patternHasPathSelector(trimmedPattern), + !trimmedResolved.isEmpty, + case let .valid(migratedPattern) = ExecApprovalHelpers.validateAllowlistPattern(trimmedResolved) { + return ExecAllowlistEntry( + id: entry.id, + pattern: migratedPattern, + lastUsedAt: entry.lastUsedAt, + lastUsedCommand: entry.lastUsedCommand, + lastResolvedPath: normalizedResolved) + } + switch ExecApprovalHelpers.validateAllowlistPattern(trimmedPattern) { case let .valid(pattern): return ExecAllowlistEntry( @@ -724,11 +735,10 @@ enum ExecApprovalHelpers { static func validateAllowlistPattern(_ pattern: String?) -> ExecAllowlistPatternValidation { let trimmed = pattern?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" guard !trimmed.isEmpty else { return .invalid(.empty) } - guard self.containsPathComponent(trimmed) else { return .invalid(.missingPathComponent) } return .valid(trimmed) } - static func isPathPattern(_ pattern: String?) -> Bool { + static func isValidAllowlistPattern(_ pattern: String?) -> Bool { switch self.validateAllowlistPattern(pattern) { case .valid: true @@ -737,6 +747,11 @@ enum ExecApprovalHelpers { } } + static func isPathPattern(_ pattern: String?) -> Bool { + let trimmed = pattern?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + return self.patternHasPathSelector(trimmed) + } + static func parseDecision(_ raw: String?) -> ExecApprovalDecision? { let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" guard !trimmed.isEmpty else { return nil } @@ -759,7 +774,7 @@ enum ExecApprovalHelpers { return pattern.isEmpty ? nil : pattern } - private static func containsPathComponent(_ pattern: String) -> Bool { + static func patternHasPathSelector(_ pattern: String) -> Bool { pattern.contains("/") || pattern.contains("~") || pattern.contains("\\") } } diff --git a/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift b/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift index 7c047e01d03..121d56268c6 100644 --- a/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift +++ b/apps/macos/Sources/OpenClaw/SystemRunSettingsView.swift @@ -105,7 +105,7 @@ struct SystemRunSettingsView: View { .foregroundStyle(.secondary) } else { HStack(spacing: 8) { - TextField("Add allowlist path pattern (case-insensitive globs)", text: self.$newPattern) + TextField("Add command name or path glob", text: self.$newPattern) .textFieldStyle(.roundedBorder) Button("Add") { if self.model.addEntry(self.newPattern) == nil { @@ -113,10 +113,10 @@ struct SystemRunSettingsView: View { } } .buttonStyle(.bordered) - .disabled(!self.model.isPathPattern(self.newPattern)) + .disabled(!self.model.isValidPattern(self.newPattern)) } - Text("Path patterns only. Basename entries like \"echo\" are ignored.") + Text("Bare names match PATH-resolved commands. Use a path glob for a specific binary.") .font(.footnote) .foregroundStyle(.secondary) if let validationMessage = self.model.allowlistValidationMessage { @@ -424,8 +424,8 @@ final class ExecApprovalsSettingsModel { self.entries.first(where: { $0.id == id }) } - func isPathPattern(_ pattern: String) -> Bool { - ExecApprovalHelpers.isPathPattern(pattern) + func isValidPattern(_ pattern: String) -> Bool { + ExecApprovalHelpers.isValidAllowlistPattern(pattern) } func refreshSkillBins(force: Bool = false) async { diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift index 5917a8f62b0..0f65147220b 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecAllowlistTests.swift @@ -66,22 +66,34 @@ struct ExecAllowlistTests { #expect(match?.pattern == entry.pattern) } - @Test func `match ignores basename pattern`() { + @Test func `match accepts basename pattern for PATH resolved executable`() { let entry = ExecAllowlistEntry(pattern: "rg") let resolution = Self.homebrewRGResolution() let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution) - #expect(match == nil) + #expect(match?.pattern == entry.pattern) } - @Test func `match ignores basename for relative executable`() { + @Test func `match accepts basename glob for PATH resolved executable`() { + let entry = ExecAllowlistEntry(pattern: "r?") + let resolution = Self.homebrewRGResolution() + let match = ExecAllowlistMatcher.match(entries: [entry], resolution: resolution) + #expect(match?.pattern == entry.pattern) + } + + @Test func `match ignores basename for path selected executable`() { let entry = ExecAllowlistEntry(pattern: "echo") - let resolution = ExecCommandResolution( + let relativeResolution = 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) + let absoluteResolution = ExecCommandResolution( + rawExecutable: "/tmp/oc-basename/echo", + resolvedPath: "/tmp/oc-basename/echo", + executableName: "echo", + cwd: "/tmp/oc-basename") + #expect(ExecAllowlistMatcher.match(entries: [entry], resolution: relativeResolution) == nil) + #expect(ExecAllowlistMatcher.match(entries: [entry], resolution: absoluteResolution) == nil) } @Test func `match is case insensitive`() { diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalHelpersTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalHelpersTests.swift index 17f9f27d2a0..6db0779f348 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalHelpersTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalHelpersTests.swift @@ -33,18 +33,13 @@ struct ExecApprovalHelpersTests { #expect(ExecApprovalHelpers.isPathPattern("/usr/bin/rg")) #expect(ExecApprovalHelpers.isPathPattern(" ~/bin/rg ")) #expect(!ExecApprovalHelpers.isPathPattern("rg")) + #expect(ExecApprovalHelpers.isValidAllowlistPattern("rg")) if case let .invalid(reason) = ExecApprovalHelpers.validateAllowlistPattern(" ") { #expect(reason == .empty) } else { Issue.record("Expected empty pattern rejection") } - - if case let .invalid(reason) = ExecApprovalHelpers.validateAllowlistPattern("echo") { - #expect(reason == .missingPathComponent) - } else { - Issue.record("Expected basename pattern rejection") - } } @Test func `requires ask matches policy`() { diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift index cd270d00fd2..eaaa452cfa5 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift @@ -31,7 +31,7 @@ struct ExecApprovalsStoreRefactorTests { } @Test - func `update allowlist reports rejected basename pattern`() async throws { + func `update allowlist accepts basename pattern`() async throws { try await self.withTempStateDir { _ in let rejected = ExecApprovalsStore.updateAllowlist( agentId: "main", @@ -39,12 +39,10 @@ struct ExecApprovalsStoreRefactorTests { ExecAllowlistEntry(pattern: "echo"), ExecAllowlistEntry(pattern: "/bin/echo"), ]) - #expect(rejected.count == 1) - #expect(rejected.first?.reason == .missingPathComponent) - #expect(rejected.first?.pattern == "echo") + #expect(rejected.isEmpty) let resolved = ExecApprovalsStore.resolve(agentId: "main") - #expect(resolved.allowlist.map(\.pattern) == ["/bin/echo"]) + #expect(resolved.allowlist.map(\.pattern) == ["echo", "/bin/echo"]) } } diff --git a/docs/platforms/macos.md b/docs/platforms/macos.md index f033f88e064..80473a6ddd5 100644 --- a/docs/platforms/macos.md +++ b/docs/platforms/macos.md @@ -102,7 +102,7 @@ Example: Notes: -- `allowlist` entries are glob patterns for resolved binary paths. +- `allowlist` entries are glob patterns for resolved binary paths, or bare command names for PATH-invoked commands. - Raw shell command text that contains shell control or expansion syntax (`&&`, `||`, `;`, `|`, `` ` ``, `$`, `<`, `>`, `(`, `)`) is treated as an allowlist miss and requires explicit approval (or allowlisting the shell binary). - Choosing “Always Allow” in the prompt adds that command to the allowlist. - `system.run` environment overrides are filtered (drops `PATH`, `DYLD_*`, `LD_*`, `NODE_OPTIONS`, `PYTHON*`, `PERL*`, `RUBYOPT`, `SHELLOPTS`, `PS4`) and then merged with the app’s environment. diff --git a/docs/tools/exec-approvals-advanced.md b/docs/tools/exec-approvals-advanced.md index e9496dd5aa1..782b7ee51ae 100644 --- a/docs/tools/exec-approvals-advanced.md +++ b/docs/tools/exec-approvals-advanced.md @@ -102,13 +102,13 @@ automatically. ### Safe bins versus allowlist -| Topic | `tools.exec.safeBins` | Allowlist (`exec-approvals.json`) | -| ---------------- | ------------------------------------------------------ | ------------------------------------------------------------ | -| Goal | Auto-allow narrow stdin filters | Explicitly trust specific executables | -| Match type | Executable name + safe-bin argv policy | Resolved executable path glob pattern | -| Argument scope | Restricted by safe-bin profile and literal-token rules | Path match only; arguments are otherwise your responsibility | -| Typical examples | `head`, `tail`, `tr`, `wc` | `jq`, `python3`, `node`, `ffmpeg`, custom CLIs | -| Best use | Low-risk text transforms in pipelines | Any tool with broader behavior or side effects | +| Topic | `tools.exec.safeBins` | Allowlist (`exec-approvals.json`) | +| ---------------- | ------------------------------------------------------ | ---------------------------------------------------------------------------------- | +| Goal | Auto-allow narrow stdin filters | Explicitly trust specific executables | +| Match type | Executable name + safe-bin argv policy | Resolved executable path glob, or bare command-name glob for PATH-invoked commands | +| Argument scope | Restricted by safe-bin profile and literal-token rules | Path match only; arguments are otherwise your responsibility | +| Typical examples | `head`, `tail`, `tr`, `wc` | `jq`, `python3`, `node`, `ffmpeg`, custom CLIs | +| Best use | Low-risk text transforms in pipelines | Any tool with broader behavior or side effects | Configuration location: diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index a36925ffa11..b28a09a7e15 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -248,13 +248,17 @@ This is defense-in-depth for interpreter loaders that do not map cleanly to one ## Allowlist (per agent) Allowlists are **per agent**. If multiple agents exist, switch which agent you’re -editing in the macOS app. Patterns are **case-insensitive glob matches**. -Patterns should resolve to **binary paths** (basename-only entries are ignored). +editing in the macOS app. Patterns are glob matches. +Patterns can be resolved binary path globs or bare command-name globs. Bare names +match only commands invoked through PATH, so `rg` can match `/opt/homebrew/bin/rg` +when the command is `rg`, but not `./rg` or `/tmp/rg`. Use a path glob when you +want to trust one specific binary location. Legacy `agents.default` entries are migrated to `agents.main` on load. Shell chains such as `echo ok && pwd` still need every top-level segment to satisfy allowlist rules. Examples: +- `rg` - `~/Projects/**/bin/peekaboo` - `~/.local/bin/*` - `/opt/homebrew/bin/rg` diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 33a76b12e73..6f0a06504e2 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -175,11 +175,13 @@ only path. ## Allowlist + safe bins -Manual allowlist enforcement matches **resolved binary paths only** (no basename matches). When -`security=allowlist`, shell commands are auto-allowed only if every pipeline segment is -allowlisted or a safe bin. Chaining (`;`, `&&`, `||`) and redirections are rejected in -allowlist mode unless every top-level segment satisfies the allowlist (including safe bins). -Redirections remain unsupported. +Manual allowlist enforcement matches resolved binary path globs and bare command-name +globs. Bare names match only commands invoked through PATH, so `rg` can match +`/opt/homebrew/bin/rg` when the command is `rg`, but not `./rg` or `/tmp/rg`. +When `security=allowlist`, shell commands are auto-allowed only if every pipeline +segment is allowlisted or a safe bin. Chaining (`;`, `&&`, `||`) and redirections +are rejected in allowlist mode unless every top-level segment satisfies the +allowlist (including safe bins). Redirections remain unsupported. Durable `allow-always` trust does not bypass that rule: a chained command still requires every top-level segment to match. diff --git a/src/infra/exec-allowlist-matching.test.ts b/src/infra/exec-allowlist-matching.test.ts index 9da1fc40328..723a335fa0e 100644 --- a/src/infra/exec-allowlist-matching.test.ts +++ b/src/infra/exec-allowlist-matching.test.ts @@ -11,6 +11,7 @@ describe("exec allowlist matching", () => { it("handles wildcard and path matching semantics", () => { const cases: Array<{ entries: ExecAllowlistEntry[]; expectedPattern: string | null }> = [ { entries: [{ pattern: "RG" }], expectedPattern: null }, + { entries: [{ pattern: "not-rg" }], expectedPattern: null }, { entries: [{ pattern: "/opt/**/rg" }], expectedPattern: "/opt/**/rg" }, { entries: [{ pattern: "/opt/*/rg" }], expectedPattern: null }, ]; @@ -20,6 +21,35 @@ describe("exec allowlist matching", () => { } }); + it("matches bare command-name patterns against PATH-resolved executable basenames", () => { + expect(matchAllowlist([{ pattern: "rg" }], baseResolution)?.pattern).toBe("rg"); + expect(matchAllowlist([{ pattern: "r?" }], baseResolution)?.pattern).toBe("r?"); + expect(matchAllowlist([{ pattern: "homebrew" }], baseResolution)).toBeNull(); + }); + + it("does not let bare command-name patterns match path-selected executables", () => { + const relativeResolution = { + rawExecutable: "./rg", + resolvedPath: "/tmp/openclaw-workspace/rg", + executableName: "rg", + }; + const absoluteResolution = { + rawExecutable: "/tmp/openclaw-workspace/rg", + resolvedPath: "/tmp/openclaw-workspace/rg", + executableName: "rg", + }; + + expect(matchAllowlist([{ pattern: "rg" }], relativeResolution)).toBeNull(); + expect(matchAllowlist([{ pattern: "rg" }], absoluteResolution)).toBeNull(); + }); + + it("honors Windows argPattern checks for bare command-name matches", () => { + const entries = [{ pattern: "rg", argPattern: "^--json$" }]; + + expect(matchAllowlist(entries, baseResolution, ["rg", "--json"], "win32")?.pattern).toBe("rg"); + expect(matchAllowlist(entries, baseResolution, ["rg", "--files"], "win32")).toBeNull(); + }); + it("matches bare wildcard patterns against arbitrary resolved executables", () => { const cases = [ baseResolution, diff --git a/src/infra/exec-command-resolution.ts b/src/infra/exec-command-resolution.ts index 0869debd95d..fce7d7bc893 100644 --- a/src/infra/exec-command-resolution.ts +++ b/src/infra/exec-command-resolution.ts @@ -315,6 +315,30 @@ function matchArgPattern(argPattern: string, argv: string[], platform?: string | } } +function hasPathSelector(value: string): boolean { + return value.includes("/") || value.includes("\\") || value.includes("~"); +} + +function matchesExecutableBasenamePattern( + pattern: string, + resolution: ExecutableResolution, +): boolean { + // Bare command-name allowlist entries are for PATH-resolved commands. A raw + // path such as ./rg or /tmp/rg must use a path allowlist entry so a workspace + // binary cannot inherit trust from a global command-name entry. + if (hasPathSelector(resolution.rawExecutable)) { + return false; + } + const candidates = new Set(); + if (resolution.executableName) { + candidates.add(resolution.executableName); + } + if (resolution.resolvedPath) { + candidates.add(path.basename(resolution.resolvedPath)); + } + return [...candidates].some((candidate) => matchesExecAllowlistPattern(pattern, candidate)); +} + export function matchAllowlist( entries: ExecAllowlistEntry[], resolution: ExecutableResolution | null, @@ -348,11 +372,10 @@ export function matchAllowlist( if (!pattern) { continue; } - const hasPath = pattern.includes("/") || pattern.includes("\\") || pattern.includes("~"); - if (!hasPath) { - continue; - } - if (!matchesExecAllowlistPattern(pattern, resolvedPath)) { + const patternMatches = hasPathSelector(pattern) + ? matchesExecAllowlistPattern(pattern, resolvedPath) + : pattern !== "*" && matchesExecutableBasenamePattern(pattern, resolution); + if (!patternMatches) { continue; } if (!useArgPattern) {