windows-sandbox: add profile-native elevated APIs by bolinfest · Pull Request #23714 · openai/codex

bolinfest · 2026-05-20T16:29:46Z

Why

This is the next step after #23167 in the Windows sandbox PermissionProfile migration. The elevated Windows backend still exposed policy-string entry points, which forced callers to pass a compatibility SandboxPolicy before the command-runner IPC could receive a profile.

Adding profile-native APIs first keeps the core switch in the next PR small: reviewers can see that the Windows crate can prepare elevated setup, capability SIDs, and runner IPC from a resolved PermissionProfile without changing core behavior yet.

What

Adds ElevatedSandboxProfileCaptureRequest and run_windows_sandbox_capture_for_permission_profile_elevated for one-shot elevated capture.
Adds spawn_windows_sandbox_session_elevated_for_permission_profile for unified exec sessions.
Factors elevated spawn prep through prepare_elevated_spawn_context_for_permissions, so both new APIs operate from ResolvedWindowsSandboxPermissions directly.
Keeps the existing legacy policy-string APIs as adapters for callers that have not moved yet.

Verification

cargo test -p codex-windows-sandbox

Stack created with Sapling. Best reviewed with ReviewStack.

chatgpt-codex-connector · 2026-05-20T16:39:28Z

💡 Codex Review

codex/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs

Lines 54 to 57 in f9201b1

    
           let elevated = prepare_elevated_spawn_context_for_permissions( 
        
               permissions, 
        
               codex_home, 
        
               cwd,

Resolve elevated session roots with permission_profile_cwd

This profile-native elevated path computes ResolvedWindowsSandboxPermissions from permission_profile and immediately prepares ACL/capability roots using cwd, but never uses permission_profile_cwd for that resolution. For managed profiles that still contain :project_roots entries (including legacy WorkspaceWrite converted via PermissionProfile::from_legacy_sandbox_policy_for_cwd), this rebinds writable roots to the command directory, so commands launched from a subdirectory can lose write access to other intended workspace paths.

codex/codex-rs/windows-sandbox-rs/src/elevated_impl.rs

Lines 114 to 117 in f9201b1

    
           let sandbox_creds = require_logon_sandbox_creds( 
        
               &permissions, 
        
               cwd, 
        
               &env_map,

Use permission_profile_cwd for capture credential setup

The new profile-based capture flow resolves permissions from permission_profile but derives sandbox credentials and writable capability roots from cwd instead of permission_profile_cwd. When the profile uses workspace-root semantics, this narrows effective write roots to the invocation directory, causing elevated exec captures from nested directories to fail writes that were allowed by the original policy cwd.

ℹ️ 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".

## Why The Windows sandbox migration away from the legacy `SandboxPolicy` abstraction needs a small local bridge before IPC and core wiring can move to `PermissionProfile`. Leaf helpers currently branch directly on `WorkspaceWrite`, which spreads legacy assumptions through path planning and token setup code. This PR introduces a Windows-local resolved permissions view so those helpers can ask Windows-specific questions about runtime filesystem/network permissions without matching on the legacy policy enum everywhere. ## What changed - Added `ResolvedWindowsSandboxPermissions` in `windows-sandbox-rs/src/resolved_permissions.rs`, with legacy `SandboxPolicy` constructors for the current call sites. - Moved `allow.rs` writable-root and read-only-subpath planning onto the resolved permissions type. - Preserved Windows `TEMP`/`TMP` writable-root behavior when the effective policy includes writable tmpdir access. - Avoided resolving Unix `:slash_tmp` or parent-process `TMPDIR` while computing Windows writable roots. - Reused the shared allow-path result for setup write-root gathering and routed network-block selection through the resolved abstraction. ## Verification - `cargo test -p codex-windows-sandbox` - `just fix -p codex-windows-sandbox` - GitHub CI restarted on the amended commit; Windows Bazel is the required signal for the Windows-only code paths. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22896). * #23715 * #23714 * #23167 * #22923 * #22918 * __->__ #22896

## Why This is the next PR in the Windows sandbox migration stack after #22896. The bottom PR introduces a Windows-local resolved permissions helper while existing callers still start from legacy `SandboxPolicy`. This PR moves the elevated runner IPC boundary to `PermissionProfile`, which makes the direction of the stack visible without changing the public core call sites yet. Because that changes the CLI-to-command-runner message shape, the framed IPC protocol version is bumped in the same PR so the boundary change is explicit. ## What changed - Replaced elevated IPC `policy_json_or_preset`/`sandbox_policy_cwd` fields with `permission_profile`/`permission_profile_cwd`. - Bumped the elevated command-runner IPC protocol to `IPC_PROTOCOL_VERSION = 2` and switched parent/runner frames to use the shared constant. - Converted the parent elevated paths from the parsed legacy policy into a materialized `PermissionProfile` before sending the runner request. - Added `WindowsSandboxTokenMode` resolution for managed `PermissionProfile` values and made the runner choose read-only vs writable-root capability tokens from that resolved profile. - Rejected disabled, external, unrestricted, and full-disk-write profiles before token selection. - Added IPC JSON coverage for tagged `PermissionProfile` payloads and token-mode unit coverage for the resolved permission helper. ## Verification - `cargo test -p codex-windows-sandbox` - `just fix -p codex-windows-sandbox` - `cargo check -p codex-windows-sandbox --target x86_64-pc-windows-msvc --tests` was attempted locally but blocked before crate type-checking because the macOS compiler environment lacks Windows C headers such as `windows.h` and `assert.h`; GitHub Windows CI is the required verification for the runner path. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22918). * #23715 * #23714 * #23167 * #22923 * __->__ #22918

## Why This is the third PR in the Windows sandbox `SandboxPolicy` -> `PermissionProfile` migration stack. #22896 introduced `ResolvedWindowsSandboxPermissions`, and #22918 moved elevated runner IPC to carry `PermissionProfile`. This PR starts moving the remaining setup/spawn helpers away from asking legacy enum questions like “is this `WorkspaceWrite`?” and toward resolved runtime permission questions like “does this profile require write capability roots?” ## What changed - Added resolved-permissions helpers for network identity and write-capability detection. - Moved setup write-root gathering to operate on `ResolvedWindowsSandboxPermissions`, with the legacy `SandboxPolicy` wrapper left in place for existing call sites. - Updated identity setup, elevated capture setup, and world-writable audit denies to use resolved write roots. - Updated spawn preparation to carry resolved permissions in `SpawnContext` and use them for network blocking, setup write roots, elevated capability SID selection, and legacy capability roots. - Removed a now-unused legacy write-root helper. ## Verification - `cargo test -p codex-windows-sandbox` - `just fix -p codex-windows-sandbox` - Existing stack checks are green on #22896 and #22918; CI has started for this PR. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22923). * #23715 * #23714 * #23167 * __->__ #22923

## Why This is the next step in the Windows sandbox migration away from the legacy `SandboxPolicy` abstraction. #22923 moved write-root and token decisions onto `ResolvedWindowsSandboxPermissions`, but setup and identity still accepted `SandboxPolicy` and converted internally. This PR pushes that conversion outward so the setup path consumes the resolved Windows permission view directly. ## What Changed - Changed `SandboxSetupRequest` to carry `ResolvedWindowsSandboxPermissions` instead of `SandboxPolicy` plus policy cwd. - Updated setup refresh/elevation and identity credential preparation to use resolved permissions for read roots, write roots, network identity, and deny-write payload planning. - Removed the production `allow.rs` legacy wrapper; allow-path computation now takes resolved permissions directly. - Added a permissions-based world-writable audit entry point while keeping the existing legacy wrapper for compatibility. - Updated legacy ACL setup and the core Windows setup bridge to construct resolved permissions at the boundary. - Hardened the Windows sandbox integration test helper staging so Bazel retries can reuse an already-staged helper if a prior sandbox helper process still has the executable open. ## Verification - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-core --test all --no-run` - `just fix -p codex-windows-sandbox` - `just fix -p codex-core` - Attempted `cargo check -p codex-windows-sandbox --target x86_64-pc-windows-gnullvm`, but the local machine is missing `x86_64-w64-mingw32-clang`; Windows CI should cover that target. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23167). * #23715 * #23714 * __->__ #23167

bolinfest requested a review from a team as a code owner May 20, 2026 16:29

bolinfest changed the base branch from main to pr23167 May 20, 2026 16:29

bolinfest requested review from iceweasel-oai and viyatb-oai May 20, 2026 17:03

bolinfest force-pushed the pr23714 branch from f9201b1 to 7d0a05b Compare May 20, 2026 17:08

bolinfest force-pushed the pr23167 branch from ec92703 to 26dc133 Compare May 20, 2026 17:08

bolinfest force-pushed the pr23167 branch from 26dc133 to acc03dd Compare May 20, 2026 17:33

bolinfest force-pushed the pr23714 branch 2 times, most recently from ba9dd22 to 14c55f6 Compare May 20, 2026 17:42

bolinfest force-pushed the pr23167 branch from acc03dd to 3acf07f Compare May 20, 2026 17:42

iceweasel-oai approved these changes May 20, 2026

View reviewed changes

bolinfest force-pushed the pr23714 branch from 14c55f6 to fcfb007 Compare May 20, 2026 19:43

bolinfest force-pushed the pr23167 branch from 3acf07f to 83a0cc1 Compare May 20, 2026 19:43

bolinfest force-pushed the pr23714 branch from fcfb007 to a5ba7c9 Compare May 20, 2026 19:53

bolinfest force-pushed the pr23167 branch from 83a0cc1 to 5d17344 Compare May 20, 2026 19:53

bolinfest force-pushed the pr23714 branch from a5ba7c9 to e7578ef Compare May 20, 2026 20:59

bolinfest force-pushed the pr23167 branch from 5d17344 to 4bab69b Compare May 20, 2026 20:59

bolinfest force-pushed the pr23167 branch from 4bab69b to f82053c Compare May 20, 2026 21:31

bolinfest force-pushed the pr23714 branch from e7578ef to 124bb6e Compare May 20, 2026 21:31

Base automatically changed from pr23167 to main May 20, 2026 21:52

bolinfest force-pushed the pr23714 branch 3 times, most recently from ee5e06e to 53b4266 Compare May 20, 2026 23:39

viyatb-oai approved these changes May 20, 2026

View reviewed changes

windows-sandbox: add profile-native elevated APIs

9574d37

bolinfest force-pushed the pr23714 branch from 53b4266 to 9574d37 Compare May 21, 2026 00:01

bolinfest enabled auto-merge (squash) May 21, 2026 00:05

bolinfest merged commit c9ff067 into main May 21, 2026
47 of 62 checks passed

bolinfest deleted the pr23714 branch May 21, 2026 00:25

github-actions Bot locked and limited conversation to collaborators May 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

windows-sandbox: add profile-native elevated APIs#23714

windows-sandbox: add profile-native elevated APIs#23714
bolinfest merged 1 commit into
mainfrom
pr23714

bolinfest commented May 20, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bolinfest commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Verification

Uh oh!

chatgpt-codex-connector Bot commented May 20, 2026

💡 Codex Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bolinfest commented May 20, 2026 •

edited

Loading