Skip to content

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

Merged
viyatb-oai merged 15 commits intomainfrom
pr13453
Mar 8, 2026
Merged

linux-sandbox: honor split filesystem policies in bwrap#13453
viyatb-oai merged 15 commits intomainfrom
pr13453

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Mar 4, 2026

Why

After #13449, the Linux helper could receive split filesystem and network policies, but the bubblewrap mount builder still reconstructed filesystem access from the legacy SandboxPolicy.

That loses explicit unreadable carveouts under writable roots, and it also mishandles Root read access paired with explicit deny carveouts. In those cases bubblewrap could still expose paths that the split filesystem policy intentionally blocked.

What changed

  • switched bubblewrap mount generation to consume FileSystemSandboxPolicy directly at the implementation boundary; legacy SandboxPolicy configs still flow through the existing FileSystemSandboxPolicy::from(&sandbox_policy) bridge before reaching bwrap
  • kept the Linux helper and preflight path on the split filesystem policy all the way into bwrap
  • re-applied explicit unreadable carveouts after readable and writable mounts so blocked subpaths still win under bubblewrap
  • masked denied directories with --tmpfs plus --remount-ro and denied files with --ro-bind-data, preserving the backing fd until exec
  • added comments in the unreadable-root masking block to explain why the mount order and directory/file split are intentional
  • updated Linux helper call sites and tests for the split-policy bwrap path

Verification

  • added protocol coverage for root carveouts staying scoped
  • added core coverage that root-write plus deny carveouts still requires a platform sandbox
  • added bwrap unit coverage for reapplying blocked carveouts after writable binds
  • added Linux integration coverage for explicit split-policy carveouts under bubblewrap
  • validated the final branch state with cargo test -p codex-linux-sandbox, cargo clippy -p codex-linux-sandbox --all-targets -- -D warnings, and the PR CI reruns

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link
Contributor

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

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: 0fbe89feee

ℹ️ 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".

@bolinfest bolinfest force-pushed the pr13452 branch 2 times, most recently from 444942a to d3e5080 Compare March 6, 2026 19:04
@bolinfest bolinfest force-pushed the pr13453 branch 2 times, most recently from 4688783 to e1a0e6a Compare March 6, 2026 20:02
@bolinfest bolinfest force-pushed the pr13452 branch 2 times, most recently from a8119da to b3a21ce Compare March 6, 2026 21:03
@bolinfest bolinfest force-pushed the pr13453 branch 2 times, most recently from f9f81ce to 5da322b Compare March 6, 2026 22:33
viyatb-oai added a commit that referenced this pull request Mar 7, 2026
## 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>
viyatb-oai added a commit that referenced this pull request Mar 7, 2026
## 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>
viyatb-oai added a commit that referenced this pull request Mar 7, 2026
## 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`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13445).
* #13453
* #13452
* #13451
* #13449
* #13448
* __->__ #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
viyatb-oai added a commit that referenced this pull request Mar 8, 2026
## Why

After `#13440` and `#13445`, macOS Seatbelt policy generation was still
deriving filesystem and network behavior from the legacy `SandboxPolicy`
projection.

That projection loses explicit unreadable carveouts and conflates split
network decisions, so the generated Seatbelt policy could still be wider
than the split policy that Codex had already computed.

## What changed

- added Seatbelt entrypoints that accept `FileSystemSandboxPolicy` and
`NetworkSandboxPolicy` directly
- built read and write policy stanzas from access roots plus excluded
subpaths so explicit unreadable carveouts survive into the generated
Seatbelt policy
- switched network policy generation to consult `NetworkSandboxPolicy`
directly
- failed closed when managed-network or proxy-constrained sessions do
not yield usable loopback proxy endpoints
- updated the macOS callers and test helpers that now need to carry the
split policies explicitly

## Verification

- added regression coverage in `core/src/seatbelt.rs` for unreadable
carveouts under both full-disk and scoped-readable policies
- 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/13448).
* #13453
* #13452
* #13451
* #13449
* __->__ #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
viyatb-oai added a commit that referenced this pull request Mar 8, 2026
## Why

The Linux sandbox helper still only accepted the legacy `SandboxPolicy`
payload.

That meant the runtime could compute split filesystem and network
policies, but the helper would immediately collapse them back to the
compatibility projection before applying seccomp or staging the
bubblewrap inner command.

## What changed

- added hidden `--file-system-sandbox-policy` and
`--network-sandbox-policy` flags alongside the legacy `--sandbox-policy`
flag so the helper can migrate incrementally
- updated the core-side Landlock wrapper to pass the split policies
explicitly when launching `codex-linux-sandbox`
- added helper-side resolution logic that accepts either the legacy
policy alone or a complete split-policy pair and normalizes that into
one effective configuration
- switched Linux helper network decisions to use `NetworkSandboxPolicy`
directly
- added `FromStr` support for the split policy types so the helper can
parse them from CLI JSON

## Verification

- added helper coverage in `linux-sandbox/src/linux_run_main_tests.rs`
for split-policy flags and policy resolution
- added CLI argument coverage in `core/src/landlock.rs`
- 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/13449).
* #13453
* #13452
* #13451
* __->__ #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
viyatb-oai added a commit that referenced this pull request Mar 8, 2026
## Why

After the split-policy plumbing landed, additional-permissions widening
still rebuilt filesystem access through the legacy projection in a few
places.

That can erase explicit deny entries and make the runtime treat a policy
as fully writable even when it still has blocked subpaths, which in turn
can skip the platform sandbox when it is still needed.

## What changed

- preserved explicit deny entries when merging additional read and write
permissions into `FileSystemSandboxPolicy`
- switched platform-sandbox selection to rely on
`FileSystemSandboxPolicy::has_full_disk_write_access()` instead of ad
hoc root-write checks
- kept the widened policy path in `core/src/exec.rs` and
`core/src/sandboxing/mod.rs` aligned so denied subpaths survive both
policy merging and sandbox selection
- added regression coverage for root-write policies that still carry
carveouts

## Verification

- added regression coverage in `core/src/sandboxing/mod.rs` showing that
root write plus carveouts still requires the platform sandbox
- 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/13451).
* #13453
* #13452
* __->__ #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
viyatb-oai added a commit that referenced this pull request Mar 8, 2026
## Why

A restricted filesystem policy that grants `:root` read or write access
but also carries explicit deny entries should still behave like scoped
access with carveouts, not like unrestricted disk access.

Without that distinction, later platform backends cannot preserve
blocked subpaths under root-level permissions because the protocol layer
reports the policy as fully unrestricted.

## What changed

- taught `FileSystemSandboxPolicy` to treat root access plus explicit
deny entries as scoped access rather than full-disk access
- derived readable and writable roots from the filesystem root when root
access is combined with carveouts, while preserving the denied paths as
read-only subpaths
- added protocol coverage for root-write policies with carveouts and a
core sandboxing regression so those policies still require platform
sandboxing

## Verification

- added protocol coverage in `protocol/src/permissions.rs` and
`protocol/src/protocol.rs` for root access with explicit carveouts
- added platform-sandbox regression coverage in
`core/src/sandboxing/mod.rs`
- 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/13452).
* #13453
* __->__ #13452
* #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
Base automatically changed from pr13452 to main March 8, 2026 05:15
@viyatb-oai
Copy link
Collaborator

@codex review

Copy link
Contributor

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

Choose a reason for hiding this comment

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

💡 Codex Review

if readable_roots.iter().any(|root| root == Path::new("/")) {

P1 Badge Enforce deny carveouts when root read access is present

In create_filesystem_args, a restricted split policy that includes Root read plus None carveouts still matches this / check and mounts --ro-bind / /. Deny carveouts are only re-applied from writable roots later, so carveouts outside writable roots are never enforced and remain readable. This means split filesystem deny rules are not fully honored.

ℹ️ 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".

charley-oai pushed a commit that referenced this pull request Mar 8, 2026
## Why

After the split-policy plumbing landed, additional-permissions widening
still rebuilt filesystem access through the legacy projection in a few
places.

That can erase explicit deny entries and make the runtime treat a policy
as fully writable even when it still has blocked subpaths, which in turn
can skip the platform sandbox when it is still needed.

## What changed

- preserved explicit deny entries when merging additional read and write
permissions into `FileSystemSandboxPolicy`
- switched platform-sandbox selection to rely on
`FileSystemSandboxPolicy::has_full_disk_write_access()` instead of ad
hoc root-write checks
- kept the widened policy path in `core/src/exec.rs` and
`core/src/sandboxing/mod.rs` aligned so denied subpaths survive both
policy merging and sandbox selection
- added regression coverage for root-write policies that still carry
carveouts

## Verification

- added regression coverage in `core/src/sandboxing/mod.rs` showing that
root write plus carveouts still requires the platform sandbox
- 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/13451).
* #13453
* #13452
* __->__ #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
charley-oai pushed a commit that referenced this pull request Mar 8, 2026
## Why

A restricted filesystem policy that grants `:root` read or write access
but also carries explicit deny entries should still behave like scoped
access with carveouts, not like unrestricted disk access.

Without that distinction, later platform backends cannot preserve
blocked subpaths under root-level permissions because the protocol layer
reports the policy as fully unrestricted.

## What changed

- taught `FileSystemSandboxPolicy` to treat root access plus explicit
deny entries as scoped access rather than full-disk access
- derived readable and writable roots from the filesystem root when root
access is combined with carveouts, while preserving the denied paths as
read-only subpaths
- added protocol coverage for root-write policies with carveouts and a
core sandboxing regression so those policies still require platform
sandboxing

## Verification

- added protocol coverage in `protocol/src/permissions.rs` and
`protocol/src/protocol.rs` for root access with explicit carveouts
- added platform-sandbox regression coverage in
`core/src/sandboxing/mod.rs`
- 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/13452).
* #13453
* __->__ #13452
* #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
@viyatb-oai viyatb-oai merged commit dcc4d7b into main Mar 8, 2026
51 of 53 checks passed
@viyatb-oai viyatb-oai deleted the pr13453 branch March 8, 2026 07:46
@github-actions github-actions bot locked and limited conversation to collaborators Mar 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants