safety: honor filesystem policy carveouts in apply_patch by bolinfest · Pull Request #13445 · openai/codex

bolinfest · 2026-03-04T08:57:08Z

Why

apply_patch safety approval was still checking writable paths through the legacy SandboxPolicy projection.

That can hide explicit none carveouts when a split filesystem policy projects back to compatibility ExternalSandbox, which leaves one more approval path that can auto-approve writes inside paths that are intentionally blocked.

What changed

passed turn.file_system_sandbox_policy into assess_patch_safety
changed writable-path checks to derive effective access from FileSystemSandboxPolicy instead of the legacy SandboxPolicy
made those checks reject explicit unreadable roots before considering broad write access or writable roots
added regression coverage showing that an ExternalSandbox compatibility projection still asks for approval when the split filesystem policy blocks a subpath

Verification

cargo test -p codex-core safety::tests::
cargo test -p codex-core test_sandbox_config_parsing
cargo clippy -p codex-core --all-targets -- -D warnings

Stack created with Sapling. Best reviewed with ReviewStack.

chatgpt-codex-connector · 2026-03-04T09:07:18Z

💡 Codex Review

codex/codex-rs/core/src/config/mod.rs

Lines 1898 to 1902 in d7e45d6

    
           && !matches!( 
        
               permission_config_syntax, 
        
               Some(PermissionConfigSyntax::Legacy) 
        
           ) 
        
           && cfg.default_permissions.is_none()

Preserve backward compatibility for [permissions.network]

This validation path now treats any non-empty [permissions] table as profile syntax, which causes legacy configs that only define [permissions.network] to fail with a missing default_permissions error. Because PermissionsToml now flattens entries, the old network table is interpreted as a profile key instead of network settings, so existing user configs that previously loaded will now be rejected at startup. Please keep a compatibility path (or explicit migration handling) for the legacy network table before enforcing default_permissions.

codex/codex-rs/core/src/config/mod.rs

Lines 1851 to 1852 in d7e45d6

    
           let configured_network_proxy_config = 
        
               network_proxy_config_from_network(cfg.network.as_ref());

Continue applying legacy network proxy config during migration

The runtime network proxy config now reads only cfg.network, so a legacy [permissions.network] block is silently ignored when the rest of the config still resolves via legacy sandbox settings (for example with sandbox_mode set). In that case startup succeeds but proxy enforcement unexpectedly changes because enabled, proxy_url, and domain settings from the existing config are dropped. The loader should continue honoring the old location (or fail fast with a targeted migration error) instead of silently disabling it.

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…guage in config.toml (#13434) ## Why `SandboxPolicy` currently mixes together three separate concerns: - parsing layered config from `config.toml` - representing filesystem sandbox state - carrying basic network policy alongside filesystem choices That makes the existing config awkward to extend and blocks the new TOML proposal where `[permissions]` becomes a table of named permission profiles selected by `default_permissions`. (The idea is that if `default_permissions` is not specified, we assume the user is opting into the "traditional" way to configure the sandbox.) This PR adds the config-side plumbing for those profiles while still projecting back to the legacy `SandboxPolicy` shape that the current macOS and Linux sandbox backends consume. It also tightens the filesystem profile model so scoped entries only exist for `:project_roots`, and so nested keys must stay within a project root instead of using `.` or `..` traversal. This drops support for the short-lived `[permissions.network]` in `config.toml` because now that would be interpreted as a profile named `network` within `[permissions]`. ## What Changed - added `PermissionsToml`, `PermissionProfileToml`, `FilesystemPermissionsToml`, and `FilesystemPermissionToml` so config can parse named profiles under `[permissions.<profile>.filesystem]` - added top-level `default_permissions` selection, validation for missing or unknown profiles, and compilation from a named profile into split `FileSystemSandboxPolicy` and `NetworkSandboxPolicy` values - taught config loading to choose between the legacy `sandbox_mode` path and the profile-based path without breaking legacy users - introduced `codex-protocol::permissions` for the split filesystem and network sandbox types, and stored those alongside the legacy projected `sandbox_policy` in runtime `Permissions` - modeled `FileSystemSpecialPath` so only `ProjectRoots` can carry a nested `subpath`, matching the intended config syntax instead of allowing invalid states for other special paths - restricted scoped filesystem maps to `:project_roots`, with validation that nested entries are non-empty descendant paths and cannot use `.` or `..` to escape the project root - kept existing runtime consumers working by projecting `FileSystemSandboxPolicy` back into `SandboxPolicy`, with an explicit error for profiles that request writes outside the workspace root - loaded proxy settings from top-level `[network]` - regenerated `core/config.schema.json` ## Verification - added config coverage for profile deserialization, `default_permissions` selection, top-level `[network]` loading, network enablement, rejection of writes outside the workspace root, rejection of nested entries for non-`:project_roots` special paths, and rejection of parent-directory traversal in `:project_roots` maps - added protocol coverage for the legacy bridge rejecting non-workspace writes ## Docs - update the Codex config docs on developers.openai.com/codex to document named `[permissions.<profile>]` entries, `default_permissions`, scoped `:project_roots` syntax, the descendant-path restriction for nested `:project_roots` entries, and top-level `[network]` proxy configuration --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13434). * #13453 * #13452 * #13451 * #13449 * #13448 * #13445 * #13440 * #13439 * __->__ #13434

## Why `#13434` introduces split `FileSystemSandboxPolicy` and `NetworkSandboxPolicy`, but the runtime still made most execution-time sandbox decisions from the legacy `SandboxPolicy` projection. That projection loses information about combinations like unrestricted filesystem access with restricted network access. In practice, that means the runtime can choose the wrong platform sandbox behavior or set the wrong network-restriction environment for a command even when config has already separated those concerns. This PR carries the split policies through the runtime so sandbox selection, process spawning, and exec handling can consult the policy that actually matters. ## What changed - threaded `FileSystemSandboxPolicy` and `NetworkSandboxPolicy` through `TurnContext`, `ExecRequest`, sandbox attempts, shell escalation state, unified exec, and app-server exec overrides - updated sandbox selection in `core/src/sandboxing/mod.rs` and `core/src/exec.rs` to key off `FileSystemSandboxPolicy.kind` plus `NetworkSandboxPolicy`, rather than inferring behavior only from the legacy `SandboxPolicy` - updated process spawning in `core/src/spawn.rs` and the platform wrappers to use `NetworkSandboxPolicy` when deciding whether to set `CODEX_SANDBOX_NETWORK_DISABLED` - kept additional-permissions handling and legacy `ExternalSandbox` compatibility projections aligned with the split policies, including explicit user-shell execution and Windows restricted-token routing - updated callers across `core`, `app-server`, and `linux-sandbox` to pass the split policies explicitly ## Verification - added regression coverage in `core/tests/suite/user_shell_cmd.rs` to verify `RunUserShellCommand` does not inherit `CODEX_SANDBOX_NETWORK_DISABLED` from the active turn - added coverage in `core/src/exec.rs` for Windows restricted-token sandbox selection when the legacy projection is `ExternalSandbox` - updated Linux sandbox coverage in `linux-sandbox/tests/suite/landlock.rs` to exercise the split-policy exec path - verified the current PR state with `just clippy` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13439). * #13453 * #13452 * #13451 * #13449 * #13448 * #13445 * #13440 * __->__ #13439 --------- Co-authored-by: viyatb-oai <viyatb@openai.com>

## Why `#13434` and `#13439` introduce split filesystem and network policies, but the only code that could answer basic filesystem questions like "is access effectively unrestricted?" or "which roots are readable and writable for this cwd?" still lived on the legacy `SandboxPolicy` path. That would force later backends to either keep projecting through `SandboxPolicy` or duplicate path-resolution logic. This PR moves those queries onto `FileSystemSandboxPolicy` itself so later runtime and platform changes can consume the split policy directly. ## What changed - added `FileSystemSandboxPolicy` helpers for full-read/full-write checks, platform-default reads, readable roots, writable roots, and explicit unreadable roots resolved against a cwd - added a shared helper for the default read-only carveouts under writable roots so the legacy and split-policy paths stay aligned - added protocol coverage for full-access detection and derived readable, writable, and unreadable roots ## Verification - added protocol coverage in `protocol/src/protocol.rs` and `protocol/src/permissions.rs` for full-root access and derived filesystem roots - verified the current PR state with `just clippy` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13440). * #13453 * #13452 * #13451 * #13449 * #13448 * #13445 * __->__ #13440 * #13439 --------- Co-authored-by: viyatb-oai <viyatb@openai.com>

bolinfest changed the base branch from main to pr13442 March 4, 2026 08:57

bolinfest force-pushed the pr13442 branch from 6c55884 to 9634623 Compare March 4, 2026 09:08

bolinfest force-pushed the pr13445 branch from d7e45d6 to a06222f Compare March 4, 2026 09:08

bolinfest force-pushed the pr13442 branch from 9634623 to 65344a6 Compare March 4, 2026 09:10

bolinfest force-pushed the pr13445 branch from a06222f to 3244ffd Compare March 4, 2026 09:10

This was referenced Mar 4, 2026

seatbelt: honor split filesystem sandbox policies #13448

Merged

linux-sandbox: plumb split sandbox policies through helper #13449

Merged

bolinfest force-pushed the pr13442 branch from 65344a6 to fab10a6 Compare March 4, 2026 09:47

bolinfest force-pushed the pr13445 branch from 3244ffd to 1434bba Compare March 4, 2026 09:47

This was referenced Mar 4, 2026

sandboxing: preserve denied paths when widening permissions #13451

Merged

linux-sandbox: honor split filesystem policies in bwrap #13453

Merged

protocol: keep root carveouts sandboxed #13452

Merged

bolinfest changed the base branch from pr13442 to pr13440 March 6, 2026 18:12

bolinfest force-pushed the pr13445 branch from 1434bba to 0e95cf5 Compare March 6, 2026 18:12

bolinfest force-pushed the pr13440 branch from c83a867 to a7c157b Compare March 6, 2026 19:04

bolinfest force-pushed the pr13445 branch from 0e95cf5 to a6a9651 Compare March 6, 2026 19:04

bolinfest force-pushed the pr13440 branch from a7c157b to 1d1e0ba Compare March 6, 2026 19:35

bolinfest force-pushed the pr13445 branch 3 times, most recently from b9614d6 to da1d510 Compare March 6, 2026 20:19

bolinfest force-pushed the pr13440 branch 2 times, most recently from e915ef6 to ebe6f77 Compare March 6, 2026 20:27

bolinfest force-pushed the pr13445 branch 2 times, most recently from e919dba to b1aa31b Compare March 6, 2026 21:03

bolinfest force-pushed the pr13440 branch from 36eee0e to a8d36af Compare March 6, 2026 21:32

bolinfest force-pushed the pr13440 branch from bee47a2 to c58266b Compare March 6, 2026 23:02

bolinfest force-pushed the pr13445 branch from dc40ece to 2e6098c Compare March 6, 2026 23:02

bolinfest force-pushed the pr13440 branch from c58266b to cf4dd5b Compare March 6, 2026 23:17

bolinfest force-pushed the pr13445 branch from 2e6098c to b2ddc1c Compare March 6, 2026 23:17

bolinfest force-pushed the pr13440 branch from cf4dd5b to 05e9b52 Compare March 6, 2026 23:20

bolinfest force-pushed the pr13445 branch 2 times, most recently from 4b6d18c to f04febf Compare March 6, 2026 23:22

bolinfest force-pushed the pr13440 branch from 05e9b52 to 7380c3f Compare March 6, 2026 23:44

bolinfest force-pushed the pr13445 branch from f04febf to 4a4c7aa Compare March 6, 2026 23:44

bolinfest force-pushed the pr13440 branch from 7380c3f to d06aaab Compare March 6, 2026 23:54

bolinfest force-pushed the pr13445 branch 2 times, most recently from 25dfa21 to b8b3565 Compare March 7, 2026 00:15

bolinfest force-pushed the pr13440 branch from d06aaab to 20a29b2 Compare March 7, 2026 00:15

bolinfest added 3 commits March 6, 2026 16:23

sandboxing: plumb split sandbox policies through runtime

a065a40

protocol: derive effective file access from filesystem policies

8a70b18

safety: honor filesystem policy carveouts in apply_patch

fd42996

bolinfest force-pushed the pr13445 branch from b8b3565 to fd42996 Compare March 7, 2026 00:25

bolinfest force-pushed the pr13440 branch from 20a29b2 to 8a70b18 Compare March 7, 2026 00:25

Base automatically changed from pr13440 to main March 7, 2026 03:49

viyatb-oai added 2 commits March 6, 2026 22:14

fix(core): respect split filesystem carveouts in apply_patch approval

42fd9d2

Merge origin/main into pr13445

fe2e567

viyatb-oai approved these changes Mar 7, 2026

View reviewed changes

fix(core): scope apply_patch carveout handling

74f1327

viyatb-oai enabled auto-merge (squash) March 7, 2026 07:24

viyatb-oai merged commit 5ceff65 into main Mar 7, 2026
51 of 53 checks passed

viyatb-oai deleted the pr13445 branch March 7, 2026 08:01

github-actions bot locked and limited conversation to collaborators Mar 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

safety: honor filesystem policy carveouts in apply_patch#13445

safety: honor filesystem policy carveouts in apply_patch#13445
viyatb-oai merged 6 commits intomainfrom
pr13445

bolinfest commented Mar 4, 2026 •

edited by viyatb-oai

Loading

Uh oh!

chatgpt-codex-connector bot commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bolinfest commented Mar 4, 2026 • edited by viyatb-oai Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

Verification

Uh oh!

chatgpt-codex-connector bot commented Mar 4, 2026

💡 Codex Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bolinfest commented Mar 4, 2026 •

edited by viyatb-oai

Loading