Conversation
This was referenced Apr 27, 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: 23dc42cae4
ℹ️ 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
+1324
to
+1325
| approval_policy == AskForApproval::Never | ||
| && matches!(permission_profile, PermissionProfile::Disabled) |
Contributor
There was a problem hiding this comment.
has_yolo_permissions now only treats PermissionProfile::Disabled as full-access, but managed profiles with unrestricted filesystem and enabled network are also projected as danger-full-access in the rest of the UI. In sessions with approval_policy = Never and a custom managed-unrestricted profile, this change drops the YOLO indicator in the session header even though permissions are still effectively full access, creating inconsistent and misleading permission state display.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Addressed in cac068aaea. has_yolo_permissions now treats both PermissionProfile::Disabled and managed unrestricted filesystem plus enabled network as YOLO, while preserving the distinction for external sandboxing. Covered by yolo_mode_includes_managed_full_access_profiles.
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
f1f1786 to
c247fed
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
3e7f657 to
dbcbf3b
Compare
bolinfest
force-pushed
the
pr19774
branch
3 times, most recently
from
926d327 to
f1a729b
Compare
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
72111af to
cac068a
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
c543c8d to
0519638
Compare
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
347c6f7 to
cb0186b
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
86504e2 to
536ab2d
Compare
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
68fb758 to
bbec1f1
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
333c2c3 to
95aae38
Compare
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
4797158 to
ea8ca7d
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
030cb28 to
5b19acc
Compare
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
53baea0 to
4b9080c
Compare
bolinfest
force-pushed
the
pr19773
branch
2 times, most recently
from
7c94f87 to
80686c3
Compare
bolinfest
added a commit
that referenced
this pull request
Apr 27, 2026
## Why This continues the permissions migration by making legacy config default resolution produce the canonical `PermissionProfile` first. The legacy `SandboxPolicy` projection should stay available at compatibility boundaries, but config loading should not create a legacy policy just to immediately convert it back into a profile. Specifically, when `default_permissions` is not specified in `config.toml`, instead of creating a `SandboxPolicy` in `codex-rs/core/src/config/mod.rs` and then trying to derive a `PermissionProfile` from it, we use `derive_permission_profile()` to create a more faithful `PermissionProfile` using the values of `ConfigToml` directly. This also keeps the existing behavior of `sandbox_workspace_write` and extra writable roots after #19841 replaced `:cwd` with `:project_roots`. Legacy workspace-write defaults are represented as symbolic `:project_roots` write access plus symbolic project-root metadata carveouts. Extra absolute writable roots are still added directly and continue to get concrete metadata protections for paths that exist under those roots. The platform sandboxes differ when a symbolic project-root subpath does not exist yet. * **Seatbelt** can encode literal/subpath exclusions directly, so macOS emits project-root metadata subpath policies even if `.git`, `.agents`, or `.codex` do not exist. * **bwrap** has to materialize bind-mount targets. Binding `/dev/null` to a missing `.git` can create a host-visible placeholder that changes Git repo discovery. Binding missing `.agents` would not affect Git discovery, but it would still create a host-visible project metadata placeholder from an automatic compatibility carveout. Linux therefore skips only missing automatic `.git` and `.agents` read-only metadata masks; missing `.codex` remains protected so first-time project config creation goes through the protected-path approval flow. User-authored `read` and `none` subpath rules keep normal bwrap behavior, and `none` can still mask the first missing component to prevent creation under writable roots. ## What Changed - Adds profile-native helpers for legacy workspace-write semantics, including `PermissionProfile::workspace_write_with()`, `FileSystemSandboxPolicy::workspace_write()`, and `FileSystemSandboxPolicy::with_additional_legacy_workspace_writable_roots()`. - Makes `FileSystemSandboxPolicy::workspace_write()` the single legacy workspace-write constructor so both `from_legacy_sandbox_policy()` and `From<&SandboxPolicy>` include the project-root metadata carveouts. - Removes the no-carveout `legacy_workspace_write_base_policy()` path and the `prune_read_entries_under_writable_roots()` cleanup that was only needed by that split construction. - Adds `ConfigToml::derive_permission_profile()` for legacy sandbox-mode fallback resolution; named `default_permissions` profiles continue through the permissions profile pipeline instead of being reconstructed from `sandbox_mode`. - Updates `Config::load()` to start from the derived profile, validate that it still has a legacy compatibility projection, and apply additional writable roots directly to managed workspace-write filesystem policies. - Updates Linux bwrap argument construction so missing automatic `.git`/`.agents` symbolic project-root read-only carveouts are skipped before emitting bind args; missing `.codex`, user-authored `read`/`none` subpath rules, and existing missing writable-root behavior are preserved. - Adds coverage that legacy workspace-write config produces symbolic project-root metadata carveouts, extra legacy workspace writable roots still protect existing metadata paths such as `.git`, and bwrap skips missing `.git`/`.agents` project-root carveouts while preserving missing `.codex` and user-authored missing subpath rules. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19772). * #19776 * #19775 * #19774 * #19773 * __->__ #19772
bolinfest
force-pushed
the
pr19774
branch
2 times, most recently
from
1e8a11f to
0676603
Compare
This was referenced Apr 28, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
SessionConfiguredEventis the internal event that tells clients what permissions are active for a session. Emitting bothsandbox_policyandpermission_profileleaves two possible authorities and forces every consumer to decide which one to honor. At this point in the migration, the profile is expressive enough to represent managed, disabled, and external sandbox enforcement, so the internal event can be profile-only.The wire compatibility concern is older serialized events or rollout data that only contain
sandbox_policy; those still need to deserialize.What Changed
sandbox_policyfromSessionConfiguredEventand makespermission_profilerequired.sandbox_policyare upgraded to a cwd-anchoredPermissionProfile.sandboxresponse field from the active thread snapshot instead of fromSessionConfiguredEvent.PermissionProfile::Disabledand managed unrestricted filesystem plus enabled network as full-access, while still preserving the distinction between no sandbox and external sandboxing.Verification
cargo test -p codex-protocol session_configured_event --libcargo test -p codex-protocol serialize_event --libcargo test -p codex-exec session_configured --libcargo test -p codex-app-server thread_response_permission_profile_preserves_enforcement --libcargo test -p codex-core session_configured_reports_permission_profile_for_external_sandbox --libcargo test -p codex-tui session_configured --libcargo test -p codex-tui yolo_mode_includes_managed_full_access_profiles --libStack created with Sapling. Best reviewed with ReviewStack.