mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
bb72e151e50cfd4f6282ab86418188af3c83b5df
14
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
fbbe7706d6 |
Add hidden Windows sandbox wrapper entrypoint (#28358)
## Why This is the second PR in the Windows fs-helper sandbox stack. The fs-helper path needs a Windows sandbox launcher that has the same argv-shaped contract as macOS `sandbox-exec` and `codex-linux-sandbox`, but this PR only introduces that hidden launcher. It does not route fs-helper through it yet. The hidden launcher still needs to be policy-complete before later direct-spawn callers use it. In particular, it has to carry the same Windows sandbox policy details that the existing spawn paths already understand: proxy enforcement, read/write root overrides, and deny-read/deny-write overrides. ## What Changed - Added the hidden `codex.exe --run-as-windows-sandbox` arg1 dispatch path. - Added `windows-sandbox-rs/src/wrapper.rs`, which parses the wrapper argv, launches the requested command through the shared Windows sandbox session runner from PR1, and forwards stdio. - Added `create_windows_sandbox_command_args_for_permission_profile()` so later direct-spawn callers can build the wrapper argv consistently. - Made the wrapper argv round-trip the full Windows sandbox policy surface it needs later: workspace roots, environment, permission profile, sandbox level, private desktop, proxy enforcement, read/write root overrides, and deny-read/deny-write overrides. - Carried `proxy_enforced` through the shared Windows session request so proxy-managed executions continue to use the offline/elevated sandbox identity. - Added wrapper argument round-trip coverage for the full policy fields. ## Verification - `just test -p codex-windows-sandbox windows_wrapper_args_round_trip` - `just test -p codex-arg0` - `just test -p codex-core exec::tests::windows_` - `just fix -p codex-windows-sandbox -p codex-core -p codex-cli` Local note: the full `just fmt` command still fails on this workstation in non-Rust formatter setup (`uv` cache access denied and missing `dotslash`/buildifier), but the Rust formatter phase completed. |
||
|
|
986c60467b |
windows-sandbox: pass workspace roots to runner (#24108)
## Why #23813 switches the Windows sandbox runner path to `PermissionProfile`, but it still left one runtime anchor for resolving symbolic `:workspace_roots` entries. That is not enough once a turn has multiple effective workspace roots: exact entries and deny globs under `:workspace_roots` need to be materialized for every runtime root before the command runner chooses token mode or builds ACL plans. ## What Changed - Replaces the Windows runner/setup `permission_profile_cwd` plumbing with `workspace_roots: Vec<AbsolutePathBuf>`. - Resolves Windows-local `PermissionProfile` data with `materialize_project_roots_with_workspace_roots(...)` instead of the single-cwd helper. - Threads `Config::effective_workspace_roots()` through core execution, unified exec, TUI setup/read-grant flows, app-server setup, app-server `command/exec`, and `debug sandbox` on Windows. - Preserves those workspace roots through the zsh-fork escalation executor instead of rebuilding them from `sandbox_policy_cwd`. - Makes `ExecRequest::new(...)` and the remaining `build_exec_request(...)` helper path take `windows_sandbox_workspace_roots` explicitly so new call sites cannot silently fall back to `vec![cwd]`. - Clarifies the `debug sandbox` non-Windows comment: remaining cwd-dependent resolution still uses `sandbox_policy_cwd`, while `:workspace_roots` entries are already materialized from config roots. - Updates elevated runner IPC `SpawnRequest` to send `workspace_roots` and bumps the framed IPC protocol version to `3` for the payload shape change. - Adds Windows-local resolver coverage for expanding exact and glob `:workspace_roots` entries across multiple roots, plus core helper coverage proving explicit roots are preserved. ## Verification - `cargo check -p codex-windows-sandbox -p codex-core -p codex-tui -p codex-cli -p codex-app-server` - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-core windows_sandbox` - `cargo test -p codex-core unix_escalation` - `cargo test -p codex-app-server windows_sandbox` - `cargo test -p codex-tui windows_sandbox` - `cargo test -p codex-cli debug_sandbox` - `just test -p codex-core unified_exec` - `just test -p codex-core build_exec_request_preserves_windows_workspace_roots` - `env -u CODEX_NETWORK_PROXY_ACTIVE -u CODEX_NETWORK_ALLOW_LOCAL_BINDING just test -p codex-app-server --lib command_exec` - `just test -p codex-windows-sandbox` - `just test -p codex-exec sandbox` - `just fix -p codex-core -p codex-app-server -p codex-windows-sandbox` A local macOS cross-check with `cargo check --target x86_64-pc-windows-msvc ...` did not reach crate Rust code because native dependencies require Windows SDK headers (`windows.h` / `assert.h`) in this environment; Windows CI remains the real target validation. Two local targeted filters compile but do not run assertions on macOS: `env -u CODEX_NETWORK_PROXY_ACTIVE -u CODEX_NETWORK_ALLOW_LOCAL_BINDING just test -p codex-app-server --lib command_exec_processor` matched zero tests, and `just test -p codex-linux-sandbox landlock` matched zero tests because the landlock suite is Linux-only. |
||
|
|
0a6bc4e687 |
windows-sandbox: remove SandboxPolicy runner plumbing (#23813)
## Why The Windows sandbox runner still carried the old `SandboxPolicy` compatibility path even though core now computes `PermissionProfile`. That meant Windows command-runner execution could only see the legacy projection, so profile-only filesystem rules such as deny globs were not part of the runner input. ## What Changed - Removed the Windows-local `SandboxPolicy` parser/export and deleted `windows-sandbox-rs/src/policy.rs`. - Changed restricted-token capture/session setup, elevated setup, world-writable audit, read-root grant, and command-runner session APIs to accept `PermissionProfile` plus the profile cwd. - Bumped the elevated command-runner IPC protocol to version 2 because `SpawnRequest` now carries `permission_profile` / `permission_profile_cwd` instead of the legacy `policy_json_or_preset` / `sandbox_policy_cwd` fields. - Updated core exec, unified exec, debug-sandbox, TUI setup/grant flows, and app-server setup to pass the actual effective `PermissionProfile`. - Left regression coverage asserting the old IPC policy fields are absent and the runner serializes tagged `PermissionProfile` JSON. ## Verification - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-core windows_sandbox` - `cargo test -p codex-app-server request_processors::windows_sandbox_processor` - `just fix -p codex-windows-sandbox -p codex-core -p codex-app-server -p codex-cli -p codex-tui` - `just fix -p codex-cli -p codex-tui` - `just fix -p codex-windows-sandbox -p codex-tui` - `rg "\\bSandboxPolicy\\b" codex-rs/windows-sandbox-rs` returned no matches. Note: `cargo test -p codex-cli` was attempted but did not reach crate tests because local disk filled while compiling dependencies (`No space left on device`). The targeted clippy pass compiled the affected CLI/TUI surfaces afterward. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23813). * #24108 * __->__ #23813 |
||
|
|
c9ff067e31 |
windows-sandbox: add profile-native elevated APIs (#23714)
## 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 |
||
|
|
896ee672cc |
windows-sandbox: feed setup from resolved permissions (#23167)
## 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 |
||
|
|
e1ec0eee5f |
windows-sandbox: drive write roots from resolved permissions (#22923)
## 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 |
||
|
|
2b4898cc47 |
windows-sandbox: add resolved permissions helper (#22896)
## 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 |
||
|
|
b1c13b6fe5 |
Simplify legacy Windows sandbox ACL persistence (#22569)
## Why The legacy Windows sandbox still carried a `persist_aces` mode switch, even though the only path that meaningfully applies filesystem ACEs today is `workspace-write`, which already uses the persistent behavior. Legacy read-only sessions rely on the read-only capability SID rather than per-command filesystem ACE mutation, so the temporary cleanup branch had become conceptual overhead without a corresponding behavioral need. Removing that split makes the ACL lifecycle match the current sandbox model more directly and trims the guard/revocation plumbing from the legacy launcher paths. ## What changed - Removed the `persist_aces` parameter from legacy ACL preparation. - Made legacy deny-read handling always use the persistent reconciliation path. - Dropped guard tracking and post-exit ACE revocation from both capture and unified-exec legacy flows. - Kept workspace `.codex` / `.agents` protection tied directly to `WorkspaceWrite` instead of an intermediate persistence flag. ## Verification - `cargo fmt -p codex-windows-sandbox` - `git diff --check` - `cargo test -p codex-windows-sandbox` - 85 passed, 2 ignored, 2 (unrelated) failed locally. |
||
|
|
3d2a0b5517 |
[codex] Scope Windows sandbox write-root capability SIDs (#21479)
## Summary - fix by scoping Windows workspace-write capability SIDs to active effective write roots - build legacy/elevated tokens from only the active effective write roots - align setup/audit deny ACL handling with active root-specific SIDs ## Testing - just fmt - git diff --check --cached - just argument-comment-lint - cargo check -p codex-windows-sandbox --locked (blocked by libwebrtc -> libyuv fetch: CONNECT tunnel failed, response 403) |
||
|
|
46f30d0282 |
feat(sandbox): add Windows deny-read parity (#18202)
## Why The split filesystem policy stack already supports exact and glob `access = none` read restrictions on macOS and Linux. Windows still needed subprocess handling for those deny-read policies without claiming enforcement from a backend that cannot provide it. ## Key finding The unelevated restricted-token backend cannot safely enforce deny-read overlays. Its `WRITE_RESTRICTED` token model is authoritative for write checks, not read denials, so this PR intentionally fails that backend closed when deny-read overrides are present instead of claiming unsupported enforcement. ## What changed This PR adds the Windows deny-read enforcement layer and makes the backend split explicit: - Resolves Windows deny-read filesystem policy entries into concrete ACL targets. - Preserves exact missing paths so they can be materialized and denied before an enforceable sandboxed process starts. - Snapshot-expands existing glob matches into ACL targets for Windows subprocess enforcement. - Honors `glob_scan_max_depth` when expanding Windows deny-read globs. - Plans both the configured lexical path and the canonical target for existing paths so reparse-point aliases are covered. - Threads deny-read overrides through the elevated/logon-user Windows sandbox backend and unified exec. - Applies elevated deny-read ACLs synchronously before command launch rather than delegating them to the background read-grant helper. - Reconciles persistent deny-read ACEs per sandbox principal so policy changes do not leave stale deny-read ACLs behind. - Fails closed on the unelevated restricted-token backend when deny-read overrides are present, because its `WRITE_RESTRICTED` token model is not authoritative for read denials. ## Landed prerequisites These prerequisite PRs are already on `main`: 1. #15979 `feat(permissions): add glob deny-read policy support` 2. #18096 `feat(sandbox): add glob deny-read platform enforcement` 3. #17740 `feat(config): support managed deny-read requirements` This PR targets `main` directly and contains only the Windows deny-read enforcement layer. ## Implementation notes - Exact deny-read paths remain enforceable on the elevated path even when they do not exist yet: Windows materializes the missing path before applying the deny ACE, so the sandboxed command cannot create and read it during the same run. - Existing exact deny paths are preserved lexically until the ACL planner, which then adds the canonical target as a second ACL target when needed. That keeps both the configured alias and the resolved object covered. - Windows ACLs do not consume Codex glob syntax directly, so glob deny-read entries are expanded to the concrete matches that exist before process launch. - Glob traversal deduplicates directory visits within each pattern walk to avoid cycles, without collapsing distinct lexical roots that happen to resolve to the same target. - Persistent deny-read ACL state is keyed by sandbox principal SID, so cleanup only removes ACEs owned by the same backend principal. - Deny-read ACEs are fail-closed on the elevated path: setup aborts if mandatory deny-read ACL application fails. - Unelevated restricted-token sessions reject deny-read overrides early instead of running with a silently unenforceable read policy. ## Verification - `cargo test -p codex-core windows_restricted_token_rejects_unreadable_split_carveouts` - `just fmt` - `just fix -p codex-core` - `just fix -p codex-windows-sandbox` - GitHub Actions rerun is in progress on the pushed head. --------- Co-authored-by: Codex <noreply@openai.com> |
||
|
|
cecca5ae06 |
Improve Windows process management edge cases (#19211)
## Summary Some improvements to Windows process-management issues from https://github.com/openai/codex/pull/15578 - bound the elevated runner pipe-connect handshake instead of waiting forever on blocking pipe connects - terminate the spawned runner if that handshake fails, so timeout/error paths do not leave a stray `codex-command-runner.exe` - loop on partial `WriteFile` results when forwarding stdin in the elevated runner, so input is not silently truncated - fix the concrete HANDLE/SID cleanup paths in the runner setup code - keep draining driver-backed stdout/stderr after exit until the backend closes, instead of dropping the tail after a fixed 200ms grace period - reuse `LocalSid` for SID ownership and add more explanatory comments around the ownership/concurrency-sensitive code paths ## Why The original PR fixed a lot of Windows session plumbing, but there were still a few sharp process-lifecycle edges: - some elevated runner handshakes could block forever - the new timeout path could still orphan the spawned runner process - stdin forwarding still assumed a single `WriteFile` consumed the whole buffer - a few raw HANDLE/SID error paths still leaked - driver-backed output could still lose the last chunk of stdout/stderr on slower backends ## Validation - `cargo fmt -p codex-windows-sandbox -p codex-utils-pty` - `cargo test -p codex-utils-pty` - `cargo test -p codex-windows-sandbox finish_driver_spawn` - `cargo test -p codex-windows-sandbox runner_` Ran a local test matrix of unified-exec and shell_tool tests, all passing |
||
|
|
789f387982 |
permissions: remove legacy read-only access modes (#19449)
## Why `ReadOnlyAccess` was a transitional legacy shape on `SandboxPolicy`: `FullAccess` meant the historical read-only/workspace-write modes could read the full filesystem, while `Restricted` tried to carry partial readable roots. The partial-read model now belongs in `FileSystemSandboxPolicy` and `PermissionProfile`, so keeping it on `SandboxPolicy` makes every legacy projection reintroduce lossy read-root bookkeeping and creates unnecessary noise in the rest of the permissions migration. This PR makes the legacy policy model narrower and explicit: `SandboxPolicy::ReadOnly` and `SandboxPolicy::WorkspaceWrite` represent the old full-read sandbox modes only. Split readable roots, deny-read globs, and platform-default/minimal read behavior stay in the runtime permissions model. ## What changed - Removes `ReadOnlyAccess` from `codex_protocol::protocol::SandboxPolicy`, including the generated `access` and `readOnlyAccess` API fields. - Updates legacy policy/profile conversions so restricted filesystem reads are represented only by `FileSystemSandboxPolicy` / `PermissionProfile` entries. - Keeps app-server v2 compatible with legacy `fullAccess` read-access payloads by accepting and ignoring that no-op shape, while rejecting legacy `restricted` read-access payloads instead of silently widening them to full-read legacy policies. - Carries Windows sandbox platform-default read behavior with an explicit override flag instead of depending on `ReadOnlyAccess::Restricted`. - Refreshes generated app-server schema/types and updates tests/docs for the simplified legacy policy shape. ## Verification - `cargo check -p codex-app-server-protocol --tests` - `cargo check -p codex-windows-sandbox --tests` - `cargo test -p codex-app-server-protocol sandbox_policy_` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19449). * #19395 * #19394 * #19393 * #19392 * #19391 * __->__ #19449 |
||
|
|
3a451b6321 |
use long-lived sessions for codex sandbox windows (#18953)
`codex sandbox windows` previously did a one-shot spawn for all commands. This change uses the `unified_exec` session to spawn long-lived processes instead, and implements a simple bridge to forward stdin to the spawned session and stdout/stderr from the spawned session back to the caller. It also fixes a bug with the new shared spawn context code where the "no-network env" was being applied to both elevated and unelevated sandbox spawns. It should only be applied for the unelevated sandbox because the elevated one uses firewall rules instead of an env-based network suppression strategy. |
||
|
|
8612714aa6 |
Add Windows sandbox unified exec runtime support (#15578)
## Summary This is the runtime/foundation half of the Windows sandbox unified-exec work. - add Windows sandbox `unified_exec` session support in `windows-sandbox-rs` for both: - the legacy restricted-token backend - the elevated runner backend - extend the PTY/process runtime so driver-backed sessions can support: - stdin streaming - stdout/stderr separation - exit propagation - PTY resize hooks - add Windows sandbox runtime coverage in `codex-windows-sandbox` / `codex-utils-pty` This PR does **not** enable Windows sandbox `UnifiedExec` for product callers yet because hooking this up to app-server comes in the next PR. Windows sandbox advertising is intentionally kept aligned with `main`, so sandboxed Windows callers still fall back to `ShellCommand`. This PR isolates the runtime/session layer so it can be reviewed independently from product-surface enablement. --------- Co-authored-by: jif-oai <jif@openai.com> Co-authored-by: Codex <noreply@openai.com> |