Skip to content

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

Merged
viyatb-oai merged 10 commits intomainfrom
pr13449
Mar 8, 2026
Merged

linux-sandbox: plumb split sandbox policies through helper#13449
viyatb-oai merged 10 commits intomainfrom
pr13449

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Mar 4, 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

Stack created with Sapling. Best reviewed with ReviewStack.

@chatgpt-codex-connector
Copy link
Contributor

💡 Codex Review

FileSystemSandboxPolicy::from(&effective_policy)

P1 Badge Preserve unreadable roots when adding extra permissions

When additional permissions include any read/write roots, this branch rebuilds file_system_sandbox_policy from the legacy effective_policy instead of extending the existing split policy. The legacy SandboxPolicy projection cannot encode FileSystemAccessMode::None, so explicit deny carveouts are dropped before execution. In profiles that mark sensitive subpaths as none, requesting any extra read/write permission can unintentionally re-open those blocked paths.


run_bwrap_with_proc_fallback(
&sandbox_policy_cwd,
&sandbox_policy,
network_sandbox_policy,

P1 Badge Enforce Linux sandbox from split filesystem policy

Linux bwrap execution still uses the legacy sandbox_policy here, even though a split file_system_sandbox_policy is available. Because legacy SandboxPolicy cannot represent explicit access = none carveouts, those deny rules are silently ignored on Linux for profile-based policies that depend on split filesystem entries. This creates a cross-platform enforcement gap (carveouts can be honored elsewhere but not in Linux command sandboxing).

ℹ️ 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 pr13449 branch 2 times, most recently from 79e535e to c5dc48e Compare March 6, 2026 23:54
@bolinfest bolinfest force-pushed the pr13448 branch 2 times, most recently from 3744e54 to 60e0188 Compare March 7, 2026 00:15
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>
Base automatically changed from pr13448 to main March 8, 2026 00:35
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>
# Conflicts:
#	codex-rs/app-server/src/codex_message_processor.rs
#	codex-rs/core/src/exec.rs
#	codex-rs/core/src/landlock.rs
#	codex-rs/core/src/safety.rs
#	codex-rs/core/src/sandboxing/mod.rs
#	codex-rs/core/src/seatbelt.rs
#	codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
#	codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs
#	codex-rs/protocol/src/protocol.rs
@viyatb-oai viyatb-oai enabled auto-merge (squash) March 8, 2026 01:29
@viyatb-oai viyatb-oai disabled auto-merge March 8, 2026 01:36
@viyatb-oai viyatb-oai merged commit 07a30da into main Mar 8, 2026
31 checks passed
@viyatb-oai viyatb-oai deleted the pr13449 branch March 8, 2026 03:40
@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