Skip to content

feat(fs): path-scoped RWX permissions for file and exec tools#42653

Closed
subrih wants to merge 29 commits into
openclaw:mainfrom
subrih:feat/fs-rwx-permissions
Closed

feat(fs): path-scoped RWX permissions for file and exec tools#42653
subrih wants to merge 29 commits into
openclaw:mainfrom
subrih:feat/fs-rwx-permissions

Conversation

@subrih

@subrih subrih commented Mar 11, 2026

Copy link
Copy Markdown

Problem

The exec allowlist controls which binaries an agent can run but cannot restrict which paths those binaries touch. A permitted /bin/ls on ~/workspace is 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.json sidecar that defines per-agent RWX rules enforced at two layers:

  • Tool layer — read/write/edit/exec tools resolve symlinks (safeRealpath) then check the resolved path against the active policy before executing
  • OS layer (macOS) — exec commands are wrapped with sandbox-exec using a generated Seatbelt (SBPL) profile; on Linux with bwrap installed, bubblewrap bind-mount sandboxing is used instead. On Linux without bwrap or 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 both policy (path rules) and scripts (per-script overrides).
  • agents["myagent"] — per-agent overrides merged on top of agents["*"]. policy rules are shallow-merged (agent wins on collision). scripts entries are deep-merged: the base sha256 is 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 same scripts block. Only takes effect when a script key matches — a scripts block with only scripts["policy"] and no named script keys has no effect.
  • Per-script entries: policy block adds/narrows rules for that script; sha256 is optional integrity check.
  • Permission strings: "rwx", "rw-", "r--", "-w-", "r-x", "--x", "---".

Design decision — no deny[], no default, no agents["policy"]:

  • "---" rules and a "/**" catch-all are sufficient — no separate deny mechanism needed.
  • default was dropped: it is syntactic sugar for "/**": "perm".
  • agents["policy"] (a previous iteration's reserved base key) was removed in favour of agents["*"], which uses the natural wildcard semantics already established by exec allowlists. No new reserved keywords.

Security hardening included:

Area Detail
bwrap "---" rules (restrictive base) No --ro-bind-try mounts — base already denies by not mounting
bwrap narrowing rules (permissive base) "---" / "--x" rules emit --tmpfs overlay to hide the path
bwrap mount ordering Rules sorted by concrete path length ascending so specific binds override broader ones
bwrap /tmp --tmpfs /tmp only in permissive mode; restrictive base omits it
bwrap "---" rule on file path Skips --tmpfs (dirs only) and warns once; tool-layer enforcement still applies
Seatbelt /tmp (allow … /private/tmp) emitted only when a rule grants access
Seatbelt permissive base exec SYSTEM_BASELINE_EXEC_PATHS emitted when catch-all lacks x bit — without this, "/**": "r--" silently breaks all subprocess execution
Seatbelt script-override narrowing Override block emits deny ops for removed bits so narrowing overrides actually reduce access
Seatbelt network/IPC in restrictive mode mach*, ipc*, network* intentionally unrestricted — feature targets filesystem only
Mid-path wildcard OS approximation Non-deny rules emit longest concrete prefix for r/w at OS layer; exec bit intentionally omitted (too broad). "---" mid-path wildcards skipped entirely at OS layer. Tool layer enforces precisely.
Pattern specificity Expanded pattern lengths used for longest-match so tilde rules (~/.ssh/**) correctly beat shorter absolute rules (/home/user/**)
Symlink resolution safeRealpath follows parent directory chain for new (not-yet-existing) files before the policy check
Allowlist fail-closed Allowlist-miss check preserved on Linux/Windows where OS sandbox may be absent
resolveArgv0 env-prefix Leading NAME=value tokens skipped when resolving script argv0 — prevents sha256 bypass via FOO=1 /script.sh
resolveArgv0 env look-through Quoted argv0 ("/usr/bin/env" /script.sh) stripped correctly — bare \S+ regex stopped at embedded spaces in quoted paths
resolveArgv0 env -S All three GNU forms handled: -S CMD, -S=CMD, -SCMD; recursion depth capped at 8
resolveArgv0 symlink keys Script config keys resolved via realpathSync so /usr/bin/python (→ python3.12) matches correctly
Bare dir auto-expand Validator auto-expands bare directory paths in policy, scripts["policy"], and per-script policy to /** with a one-time diagnostic
mtime cache loadAccessPolicyFile uses a single statSync per call; re-parses only when the file changes
Platform warnings One-time console.error when bwrap is unavailable on Linux, or on Windows with permissions configured

Testing

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, /tmp conditional 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 -S recursion 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

  • **/abc specificity — length-based longest-match uses raw pattern string length, not semantic depth. Avoid mid-path ** in rules; use explicit paths or suffix globs instead.
  • Mid-path wildcards at OS layer — patterns like /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.sh bypasses exec bit — readable scripts can be run via interpreter regardless of x bit. Use "---" to truly block execution.
  • bwrap exec-bit not enforced at OS layerr-- and r-x bind mounts are equivalent at the Linux mount level.
  • "---" rule on a file path (bwrap)--tmpfs only 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 in scripts. A scripts block with only scripts["policy"] and no named entries has no effect.
  • No approval/ask flow for policy misses — fail-closed; interactive on-miss mode is a future phase.
  • Glob/find reveals filenames — directory listing syscalls are not blocked even when file content is denied.
  • Windows and Linux-without-bwrap — exec runs unconfined (warned once per process).
  • A2A/MCP cross-agent delegation — uses the calling agent's policy; no automatic narrowing.

AI-assisted (Claude) · Regression tests added · Fix reviewed and understood

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: XL labels Mar 11, 2026
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from cd9a026 to 087d4a0 Compare March 12, 2026 14:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/agents/bash-tools.exec-host-gateway.ts Outdated
Comment thread src/agents/pi-tools.read.ts Outdated
Comment thread src/agents/bash-tools.exec-runtime.ts Outdated
@greptile-apps

greptile-apps Bot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This 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 sandbox-exec via SBPL profiles, Linux bwrap mount namespaces). The access policy is loaded from ~/.openclaw/access-policy.json, supports per-agent overrides with a wildcard "*" base, per-script sha256 integrity checks, and fails closed when the file is malformed. The implementation is generally well-engineered with careful attention to TOCTOU windows, symlink safety, and cache mutation risks.

Issues found:

  • Seatbelt baseline exec override (logic bug): In generateSeatbeltProfile, SYSTEM_BASELINE_EXEC_PATHS allows are emitted before user-defined rules. Since SBPL uses last-match-wins, a policy like "/**": "r--" causes the user catch-all rule to emit (deny process-exec* (subpath "/")) after the baseline (allow process-exec* (subpath "/bin")) — completely suppressing the baseline. The design comment explicitly says this baseline is "required when permissive base denies exec" to prevent "/**": "r--" from breaking all subprocess execution, but the current emission order means the baseline has no effect.

  • mergeAccessPolicy !override branch returns base without deep-copying (style): The !base branch deep-copies the result to prevent validateAccessPolicyConfig → autoExpandBareDir from mutating the cached agents["*"] entry. The !override branch returns base directly, which is safe in the current call-graph but creates a subtle footgun for any future caller that passes a cached reference as base with a falsy override.

Confidence Score: 3/5

  • This PR introduces a meaningful security feature but contains a logic bug in the macOS Seatbelt profile generator that causes the intended baseline exec allowances to be silently overridden, making a common policy pattern ("/**": "r--") behave differently than documented.
  • The overall design is solid — fail-closed semantics, TOCTOU mitigation, symlink resolution, cache mutation protection, and thorough test coverage are all present. However, the Seatbelt baseline exec emission order bug means any user relying on "/**": "r--" will silently get all exec denied at the OS layer, contrary to the stated design intent. This is a functional correctness issue in the macOS sandboxing path that should be fixed before the feature lands in production.
  • src/infra/exec-sandbox-seatbelt.ts requires the most attention due to the SBPL rule ordering bug. src/infra/access-policy-file.ts warrants a minor defensive-copy fix in the !override branch of mergeAccessPolicy.

Comments Outside Diff (2)

  1. src/infra/exec-sandbox-seatbelt.ts, line 206-223 (link)

    Baseline exec allows are overridden by the user's /** catch-all rule

    SYSTEM_BASELINE_EXEC_PATHS are emitted at lines 220–222 to ensure the sandboxed shell can still run common tools even when "/**": "r--" denies exec globally. However, user-defined path rules are emitted after the baseline (lines 277–299), and SBPL uses last-match-wins semantics.

    For a policy containing "/**": "r--", the user rules section emits:

    (deny process-exec* (subpath "/"))  ; from deniedOps("r--") for the /** rule
    

    Because subpath "/" matches everything (including /bin/sh, /usr/bin/grep, etc.) and this deny appears after the baseline allow process-exec* entries, the deny wins. The net result is that exec is completely blocked at the OS level — the baseline allowances become dead code.

    A minimal reproduction:

    { "policy": { "/**": "r--", "~/workspace/**": "rw-" } }

    Expected: /bin/sh and /usr/bin/* are exec-allowed (baseline intent).
    Actual: all process-exec* is denied; the sandbox shell cannot spawn any subprocess.

    The fix is to emit the baseline exec allows after user-defined rules so they take precedence, or to filter out (deny process-exec* (subpath "/")) from the /** user-rule emission when the catch-all already covers this via the global (deny process-exec* (subpath "/")) at the top.

  2. src/infra/access-policy-file.ts, line 76-78 (link)

    !override branch returns base without a defensive deep-copy

    The !base branch (lines 44–75) goes to considerable effort to return a deep copy of override specifically to prevent validateAccessPolicyConfig → autoExpandBareDir from mutating the cached agents["*"] entry in _fileCache. The accompanying comment explicitly explains the reasoning.

    The !override branch, however, returns base directly:

    if (!override) {
      return base;
    }

    In the current production call-path (resolveAccessPolicyForAgent), base is already a deep copy by the time this branch can be reached, so no cache corruption occurs today. But mergeAccessPolicy is exported, and any future caller that passes a live cached reference as base with a falsy override will receive that same reference back. A subsequent validateAccessPolicyConfig call on the returned value would then mutate the cache, causing the same silent corruption the !base copy was designed to prevent.

    Consider returning a defensive copy here too, mirroring the !base branch:

    if (!override) {
      // Return a deep copy for the same reason as the !base branch above —
      // prevents validateAccessPolicyConfig → autoExpandBareDir from mutating
      // the caller's original object.
      return {
        ...base,
        policy: base.policy ? { ...base.policy } : undefined,
        // scripts deep-copy omitted for brevity; mirror the !base block if needed.
      };
    }
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/infra/exec-sandbox-seatbelt.ts
Line: 206-223

Comment:
**Baseline exec allows are overridden by the user's `/**` catch-all rule**

`SYSTEM_BASELINE_EXEC_PATHS` are emitted at lines 220–222 to ensure the sandboxed shell can still run common tools even when `"/**": "r--"` denies exec globally. However, user-defined path rules are emitted *after* the baseline (lines 277–299), and SBPL uses **last-match-wins** semantics.

For a policy containing `"/**": "r--"`, the user rules section emits:
```
(deny process-exec* (subpath "/"))  ; from deniedOps("r--") for the /** rule
```

Because `subpath "/"` matches everything (including `/bin/sh`, `/usr/bin/grep`, etc.) and this `deny` appears *after* the baseline `allow process-exec*` entries, the deny wins. The net result is that exec is **completely blocked** at the OS level — the baseline allowances become dead code.

A minimal reproduction:
```json
{ "policy": { "/**": "r--", "~/workspace/**": "rw-" } }
```

Expected: `/bin/sh` and `/usr/bin/*` are exec-allowed (baseline intent).  
Actual: all `process-exec*` is denied; the sandbox shell cannot spawn any subprocess.

The fix is to emit the baseline exec allows **after** user-defined rules so they take precedence, or to filter out `(deny process-exec* (subpath "/"))` from the `/**` user-rule emission when the catch-all already covers this via the global `(deny process-exec* (subpath "/"))` at the top.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/infra/access-policy-file.ts
Line: 76-78

Comment:
**`!override` branch returns `base` without a defensive deep-copy**

The `!base` branch (lines 44–75) goes to considerable effort to return a deep copy of `override` specifically to prevent `validateAccessPolicyConfig → autoExpandBareDir` from mutating the cached `agents["*"]` entry in `_fileCache`. The accompanying comment explicitly explains the reasoning.

The `!override` branch, however, returns `base` directly:

```typescript
if (!override) {
  return base;
}
```

In the current production call-path (`resolveAccessPolicyForAgent`), `base` is already a deep copy by the time this branch can be reached, so no cache corruption occurs today. But `mergeAccessPolicy` is exported, and any future caller that passes a live cached reference as `base` with a falsy `override` will receive that same reference back. A subsequent `validateAccessPolicyConfig` call on the returned value would then mutate the cache, causing the same silent corruption the `!base` copy was designed to prevent.

Consider returning a defensive copy here too, mirroring the `!base` branch:

```typescript
if (!override) {
  // Return a deep copy for the same reason as the !base branch above —
  // prevents validateAccessPolicyConfig → autoExpandBareDir from mutating
  // the caller's original object.
  return {
    ...base,
    policy: base.policy ? { ...base.policy } : undefined,
    // scripts deep-copy omitted for brevity; mirror the !base block if needed.
  };
}
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 3c941ea

Comment thread src/agents/bash-tools.exec-runtime.ts Outdated
Comment thread src/agents/bash-tools.exec-host-gateway.ts Outdated
Comment thread src/infra/access-policy.ts Outdated
Comment thread src/infra/exec-sandbox-bwrap.ts Outdated
Comment thread CHANGELOG.md
@subrih subrih force-pushed the feat/fs-rwx-permissions branch 3 times, most recently from da1d317 to 6a950d5 Compare March 12, 2026 16:39

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/infra/exec-sandbox-bwrap.ts
Comment thread src/infra/exec-sandbox-seatbelt.ts
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from 6a950d5 to 0c2ef0f Compare March 12, 2026 16:58
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

We were unable to download your code in a timely manner.
ℹ️ 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".

@subrih subrih force-pushed the feat/fs-rwx-permissions branch from 0c2ef0f to 6c89437 Compare March 12, 2026 17:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/agents/pi-tools.read.ts Outdated
Comment thread src/agents/bash-tools.exec-runtime.ts Outdated
Comment thread src/infra/exec-sandbox-bwrap.ts
@subrih subrih force-pushed the feat/fs-rwx-permissions branch 2 times, most recently from aa719d0 to 84d6151 Compare March 12, 2026 18:05

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/agents/pi-tools.read.ts Outdated
Comment thread src/infra/exec-sandbox-bwrap.ts Outdated
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from 84d6151 to d98ee91 Compare March 12, 2026 19:09

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/agents/bash-tools.exec-runtime.ts Outdated
Comment thread src/infra/exec-sandbox-bwrap.ts Outdated
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from d98ee91 to f274358 Compare March 12, 2026 20:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/infra/access-policy.ts
Comment thread src/infra/exec-sandbox-seatbelt.ts Outdated
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from f274358 to e9db2b5 Compare March 12, 2026 20:49
@subrih subrih marked this pull request as ready for review March 12, 2026 20:57
@subrih subrih marked this pull request as draft March 12, 2026 20:57

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/infra/access-policy.ts Outdated
Comment thread src/infra/access-policy.ts Outdated
Comment thread src/infra/access-policy.ts
@subrih subrih force-pushed the feat/fs-rwx-permissions branch from e9db2b5 to ddd0468 Compare March 12, 2026 21:14

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/infra/access-policy.ts Outdated
Comment thread src/agents/pi-tools.read.ts Outdated
Comment thread src/infra/exec-approvals-analysis.ts Outdated
subrih added 28 commits March 14, 2026 16:19
… seatbelt mid-path wildcards, O_EXCL profile files
…arg, PATHEXT on win32, bwrap test skip on non-Linux
…rators, skip Unix-only resolveArgv0 tests on Windows
…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/agents/pi-tools.ts
Comment on lines +388 to 390
permissions: fsPolicy.permissions,
workspaceRoot,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +203 to +205
const pa = patternToPath(a, homeDir);
const pb = patternToPath(b, homeDir);
return (pa?.length ?? 0) - (pb?.length ?? 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Path-scoped RWX permissions for exec and file tools

1 participant