Skip to content

core: pass permission profiles to Windows runner#23715

Merged
bolinfest merged 1 commit into
mainfrom
pr23715
May 21, 2026
Merged

core: pass permission profiles to Windows runner#23715
bolinfest merged 1 commit into
mainfrom
pr23715

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented May 20, 2026

Why

This is the functional handoff PR for the Windows sandbox PermissionProfile migration. After #23714, the Windows elevated backend can accept a profile-native request, but core still sent a compatibility SandboxPolicy into the elevated command-runner path. That meant profile-only details such as deny globs had to be translated through side channels instead of being preserved in the runner SpawnRequest.

Passing the real PermissionProfile completes the command-runner handoff while leaving the unelevated restricted-token fallback on the legacy policy-string API.

What

  • Updates one-shot Windows elevated execution in core/src/exec.rs to call run_windows_sandbox_capture_for_permission_profile_elevated.
  • Updates unified exec in core/src/unified_exec/process_manager.rs to call spawn_windows_sandbox_session_elevated_for_permission_profile.
  • Passes request.permission_profile / exec_request.permission_profile and the stored Windows sandbox policy cwd to the elevated backend.
  • Keeps compatibility SandboxPolicy serialization only for the non-elevated restricted-token fallback.

Verification

  • cargo test -p codex-core --test all --no-run

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

let permissions =
ResolvedWindowsSandboxPermissions::try_from_permission_profile(permission_profile)?;
let elevated = prepare_elevated_spawn_context_for_permissions(
permissions,
codex_home,
cwd,

P1 Badge Resolve managed project roots against profile cwd

This path builds ResolvedWindowsSandboxPermissions from permission_profile without anchoring it to permission_profile_cwd, then immediately prepares ACL/capability roots using the process cwd. For managed profiles that use :project_roots (including legacy workspace-write conversions), this rebinds writable roots when cwd != permission_profile_cwd, so the elevated setup can grant/deny the wrong directories and generate mismatched capability SIDs. A common trigger is running a command in a subdirectory while policy resolution is intentionally anchored to the turn sandbox cwd.


let permissions =
ResolvedWindowsSandboxPermissions::try_from_permission_profile(permission_profile)?;

P1 Badge Anchor elevated capture setup to permission profile cwd

The elevated capture path resolves ResolvedWindowsSandboxPermissions directly from permission_profile and then computes sandbox credentials/roots with the command cwd, never materializing project-root permissions against permission_profile_cwd. When those two directories differ (for example, command execution in a nested subdirectory), the setup ACLs and capability SID roots can be derived from the wrong base, causing the elevated runner to enforce a different filesystem scope than the request intended.

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

bolinfest added a commit that referenced this pull request May 20, 2026
## 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
@bolinfest bolinfest force-pushed the pr23715 branch 2 times, most recently from 8ec5126 to dbdaa45 Compare May 20, 2026 17:42
bolinfest added a commit that referenced this pull request May 20, 2026
## 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
@bolinfest bolinfest force-pushed the pr23714 branch 2 times, most recently from fcfb007 to a5ba7c9 Compare May 20, 2026 19:53
bolinfest added a commit that referenced this pull request May 20, 2026
## 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
bolinfest added a commit that referenced this pull request May 20, 2026
## 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 bolinfest force-pushed the pr23714 branch 2 times, most recently from 1c896b2 to ee5e06e Compare May 20, 2026 22:03
@bolinfest bolinfest force-pushed the pr23715 branch 2 times, most recently from 72e7bbd to 009c415 Compare May 21, 2026 00:01
Base automatically changed from pr23714 to main May 21, 2026 00:25
bolinfest added a commit that referenced this pull request May 21, 2026
## 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`












---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23714).
* #23715
* __->__ #23714
@bolinfest bolinfest merged commit 63a72e6 into main May 21, 2026
62 of 78 checks passed
@bolinfest bolinfest deleted the pr23715 branch May 21, 2026 00:57
@github-actions github-actions Bot locked and limited conversation to collaborators May 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants