Conversation
This was referenced Apr 27, 2026
bolinfest
added a commit
that referenced
this pull request
Apr 27, 2026
## Why Several execution paths still converted profile-backed permissions into `SandboxPolicy` and then rebuilt runtime permissions from that legacy shape. Those round trips are unnecessary after the preceding PRs and can lose split filesystem semantics. Core approval and escalation should carry the resolved profile directly. ## What Changed - Removes `sandbox_policy` from `ResolvedPermissionProfile`; the resolved permission object now carries the canonical `PermissionProfile` directly. - Updates exec-policy fallback, shell/unified-exec interception, escalation reruns, and related tests to pass profiles instead of legacy policies. - Removes legacy additional-permission merge helpers that built an effective `SandboxPolicy` before rebuilding runtime permissions. - Keeps legacy projections only at compatibility boundaries that still require `SandboxPolicy`, not in core permission computation. ## Verification - `cargo test -p codex-core direct_write_roots` - `cargo test -p codex-core runtime_roots_to_legacy_projection` - `cargo test -p codex-app-server requested_permissions_trust_project_uses_permission_profile_intent` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19394). * #19737 * #19736 * #19735 * #19734 * #19395 * __->__ #19394
d3d674b to
4a813f1
Compare
viyatb-oai
approved these changes
Apr 27, 2026
This was referenced Apr 27, 2026
95ba6f8 to
fc3c6bf
Compare
be50c03 to
19884cb
Compare
bolinfest
added a commit
that referenced
this pull request
Apr 27, 2026
## Why The remaining migration work still needs `SandboxPolicy` at a few compatibility boundaries, but those projections should come from one canonical path. Keeping ad hoc legacy projections scattered through app-server, CLI, and config code makes it easy for behavior to drift as `PermissionProfile` gains fidelity that the legacy enum cannot represent. ## What Changed - Adds `Permissions::legacy_sandbox_policy(cwd)` and `Config::legacy_sandbox_policy()` as the compatibility projection from the canonical `PermissionProfile`. - Adds `Permissions::can_set_legacy_sandbox_policy()` so legacy inputs are checked after they are converted into profile semantics. - Updates app-server command handling, Windows sandbox setup, session configuration, and sandbox summaries to use the centralized projection helper. - Leaves `SandboxPolicy` in place only for boundary inputs/outputs that still speak the legacy abstraction. ## Verification - `cargo check -p codex-config -p codex-core -p codex-sandboxing -p codex-app-server -p codex-cli -p codex-tui` - `cargo test -p codex-tui permissions_selection_history_snapshot_full_access_to_default -- --nocapture` - `cargo test -p codex-tui permissions_selection_sends_approvals_reviewer_in_override_turn_context -- --nocapture` - `bazel test //codex-rs/tui:tui-unit-tests-bin --test_arg=permissions_selection_history_snapshot_full_access_to_default --test_output=errors` - `bazel test //codex-rs/tui:tui-unit-tests-bin --test_arg=permissions_selection_sends_approvals_reviewer_in_override_turn_context --test_output=errors` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19734). * #19737 * #19736 * #19735 * __->__ #19734
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.
Why
Requirements are the other place where the old sandbox abstraction can still constrain runtime behavior. If
allowed_sandbox_modesvalidates throughSandboxPolicy, profile-backed configurations can be rejected or lossy even though the runtime can now carry and enforce the richer profile directly.What Changed
ConfigRequirements::sandbox_policywithConfigRequirements::permission_profile.allowed_sandbox_modesinto aConstrained<PermissionProfile>by classifying profiles into the existing sandbox-mode requirement buckets.PermissionProfileinstead of projecting through a strict legacy bridge./debug-configrequirement-source output to report the profile-backed requirement source.Verification
cargo check -p codex-config -p codex-core -p codex-sandboxing -p codex-app-server -p codex-cli -p codex-tuicargo test -p codex-configcargo test -p codex-core config::config_loader_testscargo test -p codex-core config::tests::requirementscargo test -p codex-core derive_sandbox_policycargo test -p codex-core permission_profile_overridecargo test -p codex-tui debug_config_output_lists_requirement_sourcesStack created with Sapling. Best reviewed with ReviewStack.