mirror of
https://fastgit.cc/github.com/openclaw/openclaw
synced 2026-05-01 06:36:23 +08:00
fix: match bare exec allowlist commands
Co-authored-by: Kengwei Lu <kengwei@kvvlu.com> Co-authored-by: ZC <chenzhangcode@163.com> Co-authored-by: dengluozhang <275862143+dengluozhang@users.noreply.github.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<String>()
|
||||
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 }
|
||||
|
||||
@@ -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("\\")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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`() {
|
||||
|
||||
@@ -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`() {
|
||||
|
||||
@@ -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"])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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:
|
||||
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string>();
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user