Conversation
This was referenced Apr 17, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc6908e432
ℹ️ 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".
Comment on lines
+56
to
+58
| let permissions = PermissionProfile::from_runtime_permissions( | ||
| &FileSystemSandboxPolicy::from(&sandbox_policy), | ||
| NetworkSandboxPolicy::from(&sandbox_policy), |
Contributor
There was a problem hiding this comment.
FileSystemSandboxContext::new now builds permissions via FileSystemSandboxPolicy::from(&sandbox_policy), which is the cwd-independent projection and does not apply the legacy from_legacy_sandbox_policy adjustments (e.g., workspace-root read-only carveouts like .git/.codex and other cwd-aware normalization). Before this change, those adjustments were applied later in fs_sandbox::run; now callers that construct contexts through new(SandboxPolicy::WorkspaceWrite { .. }) lose those protections and can write files that were previously blocked.
Useful? React with 👍 / 👎.
bolinfest
force-pushed
the
pr18275
branch
2 times, most recently
from
ee37dcf to
af87f21
Compare
bolinfest
force-pushed
the
pr18276
branch
2 times, most recently
from
2d42d10 to
e206a39
Compare
bolinfest
force-pushed
the
pr18275
branch
2 times, most recently
from
9a98ccb to
fee24f3
Compare
bolinfest
force-pushed
the
pr18867
branch
2 times, most recently
from
f0cadb0 to
22370dd
Compare
bolinfest
added a commit
that referenced
this pull request
Apr 22, 2026
## Why #18275 anchors session-scoped `:cwd` and `:project_roots` grants to the request cwd before recording them for reuse. Relative deny glob entries need the same treatment. Without anchoring, a stored session permission can keep a pattern such as `**/*.env` relative, then reinterpret that deny against a later turn cwd. That makes the persisted profile depend on the cwd at reuse time instead of the cwd that was reviewed and approved. ## What changed `intersect_permission_profiles` now materializes retained `FileSystemPath::GlobPattern` entries against the request cwd, matching the existing materialization for cwd-sensitive special paths. Materialized accepted grants are now deduplicated before deny retention runs. This keeps the sticky-grant preapproval shape stable when a repeated request is merged with the stored grant and both `:cwd = write` and the materialized absolute cwd write are present. The preapproval check compares against the same materialized form, so a later request for the same cwd-relative deny glob still matches the stored anchored grant instead of re-prompting or rejecting. Tests cover both the storage path and the preapproval path: a session-scoped `:cwd = write` grant with `**/*.env = none` is stored with both the cwd write and deny glob anchored to the original request cwd, cannot be reused from a later cwd, and remains preapproved when re-requested from the original cwd after merging with the stored grant. ## Verification - `cargo test -p codex-sandboxing policy_transforms` - `cargo test -p codex-core --lib relative_deny_glob_grants_remain_preapproved_after_materialization` - `cargo clippy -p codex-sandboxing --tests -- -D clippy::redundant_clone` - `cargo clippy -p codex-core --lib -- -D clippy::redundant_clone` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18867). * #18288 * #18287 * #18286 * #18285 * #18284 * #18283 * #18282 * #18281 * #18280 * #18279 * #18278 * #18277 * #18276 * __->__ #18867
bolinfest
force-pushed
the
pr18276
branch
2 times, most recently
from
5d75e01 to
9d11c77
Compare
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.
2 participants
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.
Why
The exec-server still needs platform sandbox inputs, but the migration should preserve the
PermissionProfilethat produced them. Keeping only the derived legacy sandbox map would keepSandboxPolicyas the effective abstraction and would make full-disk vs. restricted profiles harder to preserve as the permissions stack starts round-tripping profiles.PermissionProfileentries can also be cwd-sensitive (:cwd,:project_roots, relative globs), so the exec-server must carry the request sandbox cwd instead of resolving those entries against the long-lived exec-server process cwd.What changed
FileSystemSandboxContextnow carriespermissions: PermissionProfileplus an optionalcwd:sandboxPolicy,sandboxPolicyCwd,fileSystemSandboxPolicy, andadditionalPermissionspermissionsandcwdwindowsSandboxLevel,windowsSandboxPrivateDesktop, anduseLegacyLandlockCore turn and apply-patch paths populate the context from the active runtime permissions and request cwd. Exec-server derives platform
SandboxPolicy/FileSystemSandboxPolicyat the filesystem boundary, adds helper runtime reads there, and rejects cwd-dependent profiles that arrive without a cwd.The legacy
FileSystemSandboxContext::new(SandboxPolicy)constructor now preserves the old workspace-write conversion semantics for compatibility tests/callers.Verification
cargo test -p codex-exec-servercargo test -p codex-exec-server sandbox_cwd -- --nocapturecargo test -p codex-exec-server sandbox_context_new_preserves_legacy_workspace_write_read_only_subpaths -- --nocapturecargo test -p codex-core --lib file_system_sandbox_context_uses_active_attempt -- --nocapture