fix: preserve split filesystem semantics in linux sandbox#14173
Merged
viyatb-oai merged 6 commits intomainfrom Mar 12, 2026
Merged
fix: preserve split filesystem semantics in linux sandbox#14173viyatb-oai merged 6 commits intomainfrom
viyatb-oai merged 6 commits intomainfrom
Conversation
8b2b59c to
a9fb72f
Compare
ef8cb44 to
b6118e0
Compare
This was referenced Mar 10, 2026
b6118e0 to
971fab4
Compare
a9fb72f to
94ee51b
Compare
971fab4 to
d3aa8db
Compare
94ee51b to
1660ba2
Compare
celia-oai
reviewed
Mar 11, 2026
a6ed2e6 to
d4f22d3
Compare
1660ba2 to
9c6dc97
Compare
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
Contributor
There was a problem hiding this comment.
💡 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".
d4f22d3 to
441353d
Compare
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
1ed6507 to
781ba3c
Compare
…ons-linux-parity # Conflicts: # codex-rs/linux-sandbox/README.md # codex-rs/linux-sandbox/src/linux_run_main.rs
celia-oai
approved these changes
Mar 12, 2026
Collaborator
celia-oai
left a comment
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
cwdresolution by comparing semantics instead of raw legacy structs.--use-legacy-landlockfor split-only shapes that legacy Landlock cannot enforce.Example
Given a split filesystem policy like:
this PR makes Linux enforce the intended result under bubblewrap:
/codestays writable/code/.gitstays read-only/code/secretsstays denied/code/secrets/tmpcan still be reopened as writable if explicitly allowedBefore 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.