Skip to content

fix: preserve split filesystem semantics in linux sandbox#14173

Merged
viyatb-oai merged 6 commits intomainfrom
codex/viyatb/permissions-linux-parity
Mar 12, 2026
Merged

fix: preserve split filesystem semantics in linux sandbox#14173
viyatb-oai merged 6 commits intomainfrom
codex/viyatb/permissions-linux-parity

Conversation

@viyatb-oai
Copy link
Copy Markdown
Collaborator

@viyatb-oai viyatb-oai commented Mar 10, 2026

Stack

fix: fail closed for unsupported split windows sandboxing #14172
-> fix: preserve split filesystem semantics in linux sandbox #14173
fix: align core approvals with split sandbox policies #14171
refactor: centralize filesystem permissions precedence #14174

Summary

  • Preserve Linux split filesystem carveouts in bubblewrap by applying mount masks in the right order, so narrower rules still win under broader writable roots.
  • Preserve unreadable ancestors of writable roots by masking them first and then rebinding the narrower writable descendants.
  • Stop rejecting legacy-plus-split Linux configs that are sandbox-equivalent after cwd resolution by comparing semantics instead of raw legacy structs.
  • Fail closed when callers provide partial split policies, mismatched legacy-plus-split policies, or force --use-legacy-landlock for split-only shapes that legacy Landlock cannot enforce.
  • Add Linux regressions for overlapping writable, read-only, and denied paths, and document the supported split-policy enforcement path.

Example

Given a split filesystem policy like:

[permissions.dev.filesystem]
":root" = "read"
"/code" = "write"
"/code/.git" = "read"
"/code/secrets" = "none"
"/code/secrets/tmp" = "write"

this PR makes Linux enforce the intended result under bubblewrap:

  • /code stays writable
  • /code/.git stays read-only
  • /code/secrets stays denied
  • /code/secrets/tmp can still be reopened as writable if explicitly allowed

Before this, Linux could lose one of those carveouts depending on mount order or legacy-policy fallback. This PR keeps the split-policy semantics intact and rejects configurations that legacy Landlock cannot represent safely.

@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-linux-parity branch from 8b2b59c to a9fb72f Compare March 10, 2026 05:15
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-core-consumers branch from ef8cb44 to b6118e0 Compare March 10, 2026 05:15
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-core-consumers branch from b6118e0 to 971fab4 Compare March 10, 2026 05:58
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-linux-parity branch from a9fb72f to 94ee51b Compare March 10, 2026 05:58
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-core-consumers branch from 971fab4 to d3aa8db Compare March 10, 2026 06:06
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-linux-parity branch from 94ee51b to 1660ba2 Compare March 10, 2026 06:06
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-core-consumers branch 2 times, most recently from a6ed2e6 to d4f22d3 Compare March 12, 2026 01:12
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-linux-parity branch from 1660ba2 to 9c6dc97 Compare March 12, 2026 01:13
@viyatb-oai viyatb-oai marked this pull request as ready for review March 12, 2026 01:16
viyatb-oai added a commit that referenced this pull request Mar 12, 2026
## Stack

   fix: fail closed for unsupported split windows sandboxing #14172
   fix: preserve split filesystem semantics in linux sandbox #14173
   fix: align core approvals with split sandbox policies #14171
-> refactor: centralize filesystem permissions precedence #14174

## Summary
- add a shared per-path split filesystem precedence helper in
`FileSystemSandboxPolicy`
- derive readable, writable, and unreadable roots from the same
most-specific resolution rules
- add regression coverage for nested `write` / `read` / `none` carveouts
and legacy bridge enforcement detection

## Testing
- cargo test -p codex-protocol
- cargo clippy -p codex-protocol --tests -- -D warnings
Copy link
Copy Markdown
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: 1ed650702a

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

@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-core-consumers branch from d4f22d3 to 441353d Compare March 12, 2026 01:44
Base automatically changed from codex/viyatb/permissions-core-consumers to main March 12, 2026 02:23
viyatb-oai added a commit that referenced this pull request Mar 12, 2026
## Stack

   fix: fail closed for unsupported split windows sandboxing #14172
   fix: preserve split filesystem semantics in linux sandbox #14173
-> fix: align core approvals with split sandbox policies #14171
   refactor: centralize filesystem permissions precedence #14174

## Why This PR Exists

This PR is intentionally narrower than the title may suggest.

Most of the original split-permissions migration already landed in the
earlier `#13434 -> #13453` stack. In particular:

- `#13439` already did the broad runtime plumbing for split filesystem
and network policies.
- `#13445` already moved `apply_patch` safety onto filesystem-policy
semantics.
- `#13448` already switched macOS Seatbelt generation to split policies.
- `#13449` and `#13453` already handled Linux helper and bubblewrap
enforcement.
- `#13440` already introduced the first protocol-side helpers for
deriving effective filesystem access.

The reason this PR still exists is that after the follow-on
`[permissions]` work and the new shared precedence helper in `#14174`, a
few core approval paths were still deciding behavior from the legacy
`SandboxPolicy` projection instead of the split filesystem policy that
actually carries the carveouts.

That means this PR is mostly a cleanup and alignment pass over the
remaining core consumers, not a fresh sandbox backend migration.

## What Is Actually New Here

- make unmatched-command fallback decisions consult
`FileSystemSandboxPolicy` instead of only legacy `DangerFullAccess` /
`ReadOnly` / `WorkspaceWrite` categories
- thread `file_system_sandbox_policy` into the shell, unified-exec, and
intercepted-exec approval paths so they all use the same split-policy
semantics
- keep `apply_patch` safety on the same effective-access rules as the
shared protocol helper, rather than letting it drift through
compatibility projections
- add loader-level regression coverage proving legacy `sandbox_mode`
config still builds split policies and round-trips back without semantic
drift

## What This PR Does Not Do

This PR does not introduce new platform backend enforcement on its own.

- Linux backend parity remains in `#14173`.
- Windows fail-closed handling remains in `#14172`.
- The shared precedence/model changes live in `#14174`.

## Files To Focus On

- `core/src/exec_policy.rs`: unmatched-command fallback and approval
rendering now read the split filesystem policy directly
- `core/src/tools/sandboxing.rs`: default exec-approval requirement keys
off `FileSystemSandboxPolicy.kind`
- `core/src/tools/handlers/shell.rs`: shell approval requests now carry
the split filesystem policy
- `core/src/unified_exec/process_manager.rs`: unified-exec approval
requests now carry the split filesystem policy
- `core/src/tools/runtimes/shell/unix_escalation.rs`: intercepted exec
fallback now uses the same split-policy approval semantics
- `core/src/safety.rs`: `apply_patch` safety keeps using effective
filesystem access rather than legacy sandbox categories
- `core/src/config/config_tests.rs`: new regression coverage for legacy
`sandbox_mode` no-drift behavior through the split-policy loader

## Notes

- `core/src/codex.rs` and `core/src/codex_tests.rs` are just small
fallout updates for `RequestPermissionsResponse.scope`; they are not the
point of the PR.
- If you reviewed the earlier `#13439` / `#13445` stack, the main review
question here is simply: “are there any remaining approval or
patch-safety paths that still reconstruct semantics from legacy
`SandboxPolicy` instead of consuming the split filesystem policy
directly?”

## Testing
- cargo test -p codex-core
legacy_sandbox_mode_config_builds_split_policies_without_drift
- cargo test -p codex-core request_permissions
- cargo test -p codex-core intercepted_exec_policy
- cargo test -p codex-core
restricted_sandbox_requires_exec_approval_on_request
- cargo test -p codex-core
unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts
- cargo test -p codex-core explicit_
- cargo clippy -p codex-core --tests -- -D warnings
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/permissions-linux-parity branch from 1ed6507 to 781ba3c Compare March 12, 2026 03:21
…ons-linux-parity

# Conflicts:
#	codex-rs/linux-sandbox/README.md
#	codex-rs/linux-sandbox/src/linux_run_main.rs
@viyatb-oai viyatb-oai enabled auto-merge (squash) March 12, 2026 16:56
Copy link
Copy Markdown
Collaborator

@celia-oai celia-oai left a comment

Choose a reason for hiding this comment

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

does this remaining edge case makes sense:

- Policy: /repo = write, /repo/a = none, /repo/a/b = write

  Expected result from the policy model:

  - /repo writable
  - /repo/a denied
  - /repo/a/b writable again, because the more specific child write should win over the parent deny

  Why it may still fail on Linux bubblewrap:

  - The sandbox first binds /repo writable.
  - Then it masks /repo/a as unreadable.
  - Later it tries to bind /repo/a/b writable again.
  - But after /repo/a was masked, the path to /repo/a/b may no longer exist inside the sandbox.
  - The branch only recreates parent directories for one class of deny/write overlap, and this case does not seem to go through that path.

@viyatb-oai viyatb-oai merged commit 774965f into main Mar 12, 2026
35 of 36 checks passed
@viyatb-oai viyatb-oai deleted the codex/viyatb/permissions-linux-parity branch March 12, 2026 17:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 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