feat(fs): path-scoped RWX permissions for file and exec tools#42653
feat(fs): path-scoped RWX permissions for file and exec tools#42653subrih wants to merge 29 commits into
Conversation
cd9a026 to
087d4a0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 632be92d4a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryThis PR adds a comprehensive path-scoped RWX permission system for file and exec tools, enforced at two layers: a tool-layer glob-match check (applies to all platforms) and an OS-layer sandbox (macOS Issues found:
Confidence Score: 3/5
|
da1d317 to
6a950d5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a950d52a8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
6a950d5 to
0c2ef0f
Compare
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
0c2ef0f to
6c89437
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c89437b50
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
aa719d0 to
84d6151
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84d6151a6c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
84d6151 to
d98ee91
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d98ee91d8b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
d98ee91 to
f274358
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f274358fe8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f274358 to
e9db2b5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9db2b5199
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
e9db2b5 to
ddd0468
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddd0468a1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…layer policy check
… seatbelt mid-path wildcards, O_EXCL profile files
…ooter after auto-expand
…xpand, fail-closed on broken file
…rofile file cleanup
…ead-only check, seatbelt ? wildcard strip
…--proc in permissive mode
…st-token fallback
…~ in scripts keys
…d scripts entries
… file-like path auto-expand
…e, explicit bwrap /tmp guard
…arg, PATHEXT on win32, bwrap test skip on non-Linux
…rators, skip Unix-only resolveArgv0 tests on Windows
… test reset exports
…p reset export, npmrc comment
…/bin/sh and network* intent
…dation, doc cleanup
…l base, docs rewrite
…ides, permAllows format check, script entry shape check
…s all enforcement layers - permAllowsWrite (bwrap), permToOps/deniedOps (seatbelt): guard all positional perm accesses with VALID_PERM_RE - catchAllPerm/tmpPerm (seatbelt): validate rawPerm before positional access; fail closed to '---' - hasScriptOverride (exec-runtime): check entry shape (non-null object, not array) before setting bypass flag - scripts["policy"] merged into overrideRules in applyScriptPolicyOverride (was silently dropped) - mergeAccessPolicy: reject non-object script entries before propagating - validateAccessPolicyFileStructure: recurse into per-script entries to catch removed deny/default fields - validateAccessPolicyConfig: reject non-object entries, validate sha256 format, emit mid-path wildcard diagnostics for scripts["policy"] AND per-script policy blocks (previously only config.policy) - env-prefix regex: handle escaped quotes in double-quoted values ((?:[^"\\]|\\.)*) - _resetBwrapAvailableCacheForTest: export added for test isolation - Tests added for all of the above
- bwrap: '---' rules on SYSTEM_RO_BIND_PATHS (/etc /usr /bin /lib /sbin /opt) now emit --tmpfs in restrictive mode — previously the deny branch was gated to permissive mode only, leaving syscalls inside the sandbox able to read /etc/passwd etc. despite policy - seatbelt: bracket globs [abc] now detected as wildcards (/[*?[]/ and strip regex updated); previously emitted as SBPL literals matching only a file literally named '[abc]' - access-policy-file: mergeAccessPolicy fast-path (!base) returns shallow copy instead of reference — autoExpandBareDir was mutating the cached agents['*'].policy in-place, corrupting all subsequent resolveAccessPolicyForAgent calls in the same process - access-policy: sha256 comparison normalizes to lowercase (.toLowerCase()) — validation regex accepts uppercase (/i) but crypto.digest always returns lowercase, causing uppercase sha256 in config to silently deny exec at runtime with no useful error - Tests added for all four findings
…e SEATBELT_WRITE_OPS - mergeAccessPolicy !base fast-path: scripts["policy"] and per-script entry.policy were shallow-copied, leaving them as references into the cached _fileCache object. autoExpandBareDir mutations would propagate back into the cache, violating the invariant established by the policy-copy fix. Now deep-copied via Object.fromEntries map. - exec-sandbox-seatbelt: replace hardcoded "file-write*" with SEATBELT_WRITE_OPS constant in the /tmp write allowance branch, consistent with all other allowance lines in the file. - Tests added for nested scripts deep-copy invariant.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c941eae23
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| permissions: fsPolicy.permissions, | ||
| workspaceRoot, | ||
| }); |
There was a problem hiding this comment.
Enforce access-policy checks for sandboxed file tools
This wiring adds permissions only on the host read/write/edit constructors, but sandboxed sessions take a different path (createSandboxedReadTool/createSandboxedWriteTool/createSandboxedEditTool) that never calls checkAccessPolicy. As a result, when sandboxRoot is active, file-tool operations can still read/write paths that the new access policy is supposed to deny, which is a policy bypass in sandboxed runs.
Useful? React with 👍 / 👎.
| const pa = patternToPath(a, homeDir); | ||
| const pb = patternToPath(b, homeDir); | ||
| return (pa?.length ?? 0) - (pb?.length ?? 0); |
There was a problem hiding this comment.
Sort bwrap mounts with perm-aware path keys
The bwrap rule sorter calls patternToPath without passing each rule's perm, so mid-path wildcard rules (for example "/srv/*/tools/**") resolve to null during sorting and all get the same key length. That makes mount precedence depend on JSON insertion order instead of specificity; if a broader wildcard rule appears later, bwrap applies it last and can override a narrower mount's intended permission.
Useful? React with 👍 / 👎.
Problem
The exec allowlist controls which binaries an agent can run but cannot restrict which paths those binaries touch. A permitted
/bin/lson~/workspaceis equally permitted on~/.ssh. There is no mechanism to scope an agent's filesystem access to specific directory trees. Closes #39979.Root Cause
No path-level enforcement exists at either the tool layer or the OS layer. All read/write/edit/exec tool calls succeed on any path the OS user can access.
Fix
Add an optional
~/.openclaw/access-policy.jsonsidecar that defines per-agent RWX rules enforced at two layers:safeRealpath) then check the resolved path against the active policy before executingsandbox-execusing a generated Seatbelt (SBPL) profile; on Linux withbwrapinstalled, bubblewrap bind-mount sandboxing is used instead. On Linux withoutbwrapor on Windows, exec runs unconfined with a one-time warning.Policy format:
{ "version": 1, "agents": { "*": { "policy": { "/**": "r--", "~/workspace/**": "rwx", "/tmp/**": "rw-", "~/.ssh/**": "---" }, "scripts": { "policy": { "/tmp/**": "rw-" }, "~/bin/deploy.sh": { "sha256": "abc123...", "policy": { "~/deploy/**": "rwx" } } } }, "myagent": { "policy": { "~/workspace/**": "rwx" } } } }Policy semantics:
policy— the path→perm rule map inside each agent block. Glob→perm; longest-match-wins using expanded pattern lengths."---"means deny all."/**": "r--"is a permissive catch-all. No rule match = implicit"---"(fail-closed).agents["*"]— base policy applied to every agent. Put org-wide rules here. Can include bothpolicy(path rules) andscripts(per-script overrides).agents["myagent"]— per-agent overrides merged on top ofagents["*"].policyrules are shallow-merged (agent wins on collision).scriptsentries are deep-merged: the basesha256is always preserved and cannot be overridden by an agent block.scripts{}— per-argv[0]hash-verified policy overlay; fires only when the exec target matches a named script key. sha256 mismatch is a hard exec denial. Script override rules are emitted after base rules in the OS sandbox profile (last-match-wins) so they can reach inside broadly denied subtrees.scripts["policy"]— shared path rules merged as the base for every per-script entry in the samescriptsblock. Only takes effect when a script key matches — ascriptsblock with onlyscripts["policy"]and no named script keys has no effect.policyblock adds/narrows rules for that script;sha256is optional integrity check."rwx","rw-","r--","-w-","r-x","--x","---".Design decision — no
deny[], nodefault, noagents["policy"]:"---"rules and a"/**"catch-all are sufficient — no separate deny mechanism needed.defaultwas dropped: it is syntactic sugar for"/**": "perm".agents["policy"](a previous iteration's reserved base key) was removed in favour ofagents["*"], which uses the natural wildcard semantics already established by exec allowlists. No new reserved keywords.Security hardening included:
"---"rules (restrictive base)--ro-bind-trymounts — base already denies by not mounting"---"/"--x"rules emit--tmpfsoverlay to hide the path/tmp--tmpfs /tmponly in permissive mode; restrictive base omits it"---"rule on file path--tmpfs(dirs only) and warns once; tool-layer enforcement still applies/tmp(allow … /private/tmp)emitted only when a rule grants accessSYSTEM_BASELINE_EXEC_PATHSemitted when catch-all lacksxbit — without this,"/**": "r--"silently breaks all subprocess executiondenyops for removed bits so narrowing overrides actually reduce accessmach*,ipc*,network*intentionally unrestricted — feature targets filesystem only"---"mid-path wildcards skipped entirely at OS layer. Tool layer enforces precisely.~/.ssh/**) correctly beat shorter absolute rules (/home/user/**)safeRealpathfollows parent directory chain for new (not-yet-existing) files before the policy checkresolveArgv0env-prefixNAME=valuetokens skipped when resolving script argv0 — prevents sha256 bypass viaFOO=1 /script.shresolveArgv0env look-through"/usr/bin/env" /script.sh) stripped correctly — bare\S+regex stopped at embedded spaces in quoted pathsresolveArgv0env -S-S CMD,-S=CMD,-SCMD; recursion depth capped at 8resolveArgv0symlink keysrealpathSyncso/usr/bin/python(→python3.12) matches correctlypolicy,scripts["policy"], and per-scriptpolicyto/**with a one-time diagnosticloadAccessPolicyFileuses a singlestatSyncper call; re-parses only when the file changesconsole.errorwhen bwrap is unavailable on Linux, or on Windows with permissions configuredTesting
Unit tests across
access-policy.test.ts,access-policy-file.test.ts,exec-sandbox-bwrap.test.ts,exec-sandbox-seatbelt.test.ts(bwrap/seatbelt tests skipped on non-Linux/non-macOS).Covers: direct reads/writes, tilde expansion, path normalization, variable expansion, interpreter indirection, symlink attacks, SBPL deny ordering, bwrap mount ordering,
"---"rule mount suppression, narrowing under permissive base, seatbelt permissive exec baseline,/tmpconditional baseline, pattern specificity, per-script hash verification, agent wildcard merge, bare-dir auto-expand (top-level + per-script), validation dedup, env-prefix argv0 resolution,env -Srecursion all three forms, quoted env argv0 look-through, symlink script key matching, mtime cache,scripts["policy"]shared rules,scripts["policy"]reserved key skip.Known limitations / follow-ups
**/abcspecificity — length-based longest-match uses raw pattern string length, not semantic depth. Avoid mid-path**in rules; use explicit paths or suffix globs instead./home/*/secrets/**cannot be enforced precisely by bwrap/Seatbelt. Non-deny rules use the longest concrete prefix as an approximate OS target for r/w; exec is omitted from the approximation. Tool-layer enforcement is precise. Use trailing-**patterns for full OS enforcement.bash script.shbypasses exec bit — readable scripts can be run via interpreter regardless ofxbit. Use"---"to truly block execution.r--andr-xbind mounts are equivalent at the Linux mount level."---"rule on a file path (bwrap) —--tmpfsonly accepts directories; file-specific"---"rules are skipped at the OS layer with a one-time warning. Deny the parent directory instead for OS-level enforcement.scripts["policy"]requires a matching script entry — shared script rules only apply when an exec target matches a named key inscripts. Ascriptsblock with onlyscripts["policy"]and no named entries has no effect.on-missmode is a future phase.AI-assisted (Claude) · Regression tests added · Fix reviewed and understood