Conversation
This was referenced Apr 27, 2026
1e864f3 to
f1f1786
Compare
6487630 to
54173b9
Compare
c247fed to
ec9a1d7
Compare
eddf071 to
c9269a0
Compare
72111af to
cac068a
Compare
6c92cc4 to
4e3c8ec
Compare
347c6f7 to
cb0186b
Compare
b392db1 to
b77b4d5
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
1e8a11f to
0676603
Compare
5772009 to
7a2d356
Compare
This was referenced Apr 28, 2026
bolinfest
added a commit
that referenced
this pull request
Apr 28, 2026
## Why `ThreadSessionState` is the TUI's cached view of an app-server session. To make `PermissionProfile` the canonical runtime permissions model, cached thread sessions need to always have a profile instead of treating the profile as an optional supplement to a legacy `sandbox` response field. The main compatibility concern is older app-server v2 lifecycle responses that only include `sandbox` and omit `permissionProfile`: - `thread/start` -> `ThreadStartResponse.sandbox` - `thread/resume` -> `ThreadResumeResponse.sandbox` - `thread/fork` -> `ThreadForkResponse.sandbox` Those responses must still hydrate correctly when the TUI is pointed at an older app-server. This PR converts the legacy `sandbox` value into a `PermissionProfile` immediately at response ingestion time, using the response `cwd`, so cached sessions do not carry an optional profile that can later reinterpret cwd-bound grants against a different thread cwd. This fallback is intentionally boundary compatibility. The follow-up PRs in this stack continue the cleanup by making `SessionConfiguredEvent` profile-only, deriving sandbox projections from snapshots only when an API still needs them, and then removing `sandbox_policy` from `ThreadSessionState`. ## What Changed - Makes `ThreadSessionState.permission_profile` required. - Converts legacy app-server response `sandbox` values into a `PermissionProfile` at ingestion time using the response cwd. - Ensures `thread/read` hydration does not reuse a primary session profile that may be anchored to a different cwd; it uses the active widget permission settings for the read thread fallback instead of reusing cached primary-session permissions. - Keeps the app-server request path unchanged: embedded sessions send profiles, while remote sessions continue using legacy sandbox overrides for compatibility. ## Verification - `cargo test -p codex-tui thread_read --lib` - `cargo test -p codex-tui permission_settings_sync_preserves_active_profile_only_rules --lib` - `cargo test -p codex-tui resume_response_restores_turns_from_thread_items --lib` - `cargo test -p codex-tui thread_session_state::tests --lib` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19773). * #19900 * #19899 * #19776 * #19775 * #19774 * __->__ #19773
5895cd8 to
fac5fcd
Compare
viyatb-oai
approved these changes
Apr 28, 2026
bolinfest
added a commit
that referenced
this pull request
Apr 28, 2026
## Why `SessionConfiguredEvent` is the internal event that tells clients what permissions are active for a session. Emitting both `sandbox_policy` and `permission_profile` leaves 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 - Removes `sandbox_policy` from `SessionConfiguredEvent` and makes `permission_profile` required. - Adds custom deserialization so old payloads with only `sandbox_policy` are upgraded to a cwd-anchored `PermissionProfile`. - Updates core event emission and TUI session handling to sync permissions from the profile directly. - Updates app-server response construction to derive the legacy `sandbox` response field from the active thread snapshot instead of from `SessionConfiguredEvent`. - Updates yolo-mode display logic to treat both `PermissionProfile::Disabled` and 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 --lib` - `cargo test -p codex-protocol serialize_event --lib` - `cargo test -p codex-exec session_configured --lib` - `cargo test -p codex-app-server thread_response_permission_profile_preserves_enforcement --lib` - `cargo test -p codex-core session_configured_reports_permission_profile_for_external_sandbox --lib` - `cargo test -p codex-tui session_configured --lib` - `cargo test -p codex-tui yolo_mode_includes_managed_full_access_profiles --lib` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19774). * #19900 * #19899 * #19776 * #19775 * __->__ #19774
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
ThreadConfigSnapshotis used by app-server and thread metadata code as a stable view of active runtime settings. Keeping bothsandbox_policyandpermission_profilein the snapshot duplicates permission state and makes it possible for the legacy projection to drift from the canonical profile.The legacy
sandboxvalue is still needed at app-server compatibility boundaries, so this PR derives it on demand from the snapshot profile and cwd instead of storing it.What Changed
ThreadConfigSnapshot.sandbox_policy.ThreadConfigSnapshot::sandbox_policy()as a compatibility projection frompermission_profilepluscwd.Verification
cargo test -p codex-app-server thread_response_permission_profile_preserves_enforcement --libcargo test -p codex-core dispatch_reclaims_stale_global_lock_and_starts_consolidation --libStack created with Sapling. Best reviewed with ReviewStack.