mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
0a6bc4e68759f0086434fd5b4d87c7f5a597d6d1
17
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Bolin
|
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 |
||
|
iceweasel-oai
|
5b1b6a20dd |
[codex] Use rolling files for Windows sandbox logs (#24117)
## Why Windows sandbox diagnostics currently append to a single `sandbox.log` under `CODEX_HOME/.sandbox`. That file never rolls over, which makes it hard to safely include sandbox diagnostics in future feedback reports without risking unbounded growth. ## What changed - Replaced direct append-open sandbox logging with `tracing_appender::rolling::RollingFileAppender`. - Configured sandbox logs to rotate daily using names like `sandbox.YYYY-MM-DD.log`. - Added a conservative `MAX_LOG_FILES` cap of 90 retained matching log files. - Routed the Windows sandbox setup helper through the same rolling writer. - Added helpers for resolving the current daily sandbox log path so future feedback upload work can use the same filename logic. - Updated tests and test diagnostics to read the dated daily log file. This intentionally does not include sandbox logs in `/feedback` yet; scrubbing and attachment behavior can happen in a follow-up. ## Testing - `cargo fmt -p codex-windows-sandbox` - `cargo check -p codex-windows-sandbox` - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-windows-sandbox logging::tests` - `cargo clippy -p codex-windows-sandbox --all-targets -- -D warnings` |
||
|
Michael Bolin
|
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 |
||
|
Michael Bolin
|
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 |
||
|
Michael Bolin
|
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 |
||
|
Michael Bolin
|
729bdf3c8d |
windows-sandbox: send permission profiles to elevated runner (#22918)
## 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 |
||
|
iceweasel-oai
|
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. |
||
|
Adrian
|
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) |
||
|
viyatb-oaiandCodex
|
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> |
||
|
Charlie MarshandCodex
|
7c9731c9af |
Enable --deny-warnings for cargo shear (#21616)
## Summary In https://github.com/openai/codex/pull/21584, we disabled doctests for crates that lack any doctests. We can enforce that property via `cargo shear --deny-warnings`: crates that lack doctests will be flagged if doctests are enabled, and crates with doctests will be flagged if doctests are disabled. A few additional notes: - By adding `--deny-warnings`, `cargo shear` also flagged a number of modules that were not reachable at all. Some of those have been removed. - This PR removes a usage of `windows_modules!` (since `cargo shear` and `rustfmt` couldn't see through it) in favor of simple `#[cfg(target_os = "windows")]` macros. As a consequence, many of these files exhibit churn in this PR, since they weren't being formatted by `rustfmt` at all on main. - Again, to make the code more analyzable, this PR also removes some usages of `#[path = "cwd_junction.rs"]` in favor of a more standard module structure. The bin sidecar structure is still retained, but, e.g., `windows-sandbox-rs/src/bin/command_runner.rs` was moved to `windows-sandbox-rs/src/bin/command_runner/main.rs`, and so on. --------- Co-authored-by: Codex <noreply@openai.com> |
||
|
iceweasel-oai
|
5d5500650b |
Fix Windows PTY teardown by preserving ConPTY ownership (#20685)
## Why On Windows, background terminals could stay visible after their shell process had already exited. The elevated runner waits for the PTY output reader to reach EOF before it sends the final exit message, but the ConPTY helper was reducing ownership down to raw handles too early. That left the pseudoconsole's borrowed pipe handles alive past teardown, so EOF never propagated and the session stayed `running`. ## What changed - change `utils/pty/src/win/conpty.rs` to hand off owned ConPTY resources instead of leaking only raw handles - make `windows-sandbox-rs/src/conpty/mod.rs` keep the pseudoconsole owner and the backing pipe handles together until teardown - update the elevated runner and the legacy unified-exec backend to keep that `ConptyInstance` alive, take only the specific pipe handles they need, and drop the owner at teardown instead of trying to close a detached pseudoconsole handle later ## Testing - desktop app in `Auto-review`: 11 x `cmd /c "ping -n 3 google.com"` all exited cleanly and did not accumulate in the UI - desktop app in `Auto-review`: 5 x `cmd /c "ping -n 30 google.com"` appeared in the UI and drained back out on their own |
||
|
Michael Bolin
|
466798aa83 |
ci: cross-compile Windows Bazel tests (#20585)
## Status This is the Bazel PR-CI cross-compilation follow-up to #20485. It is intentionally split from the Cargo/cargo-xwin release-build PoC so #20485 can stay as the historical release-build exploration. The unrelated async-utils test cleanup has been moved to #20686, so this PR is focused on the Windows Bazel CI path. The intended tradeoff is now explicit in `.github/workflows/bazel.yml`: pull requests get the fast Windows cross-compiled Bazel test leg, while post-merge pushes to `main` run both that fast cross leg and a fully native Windows Bazel test leg. The native main-only job keeps full V8/code-mode coverage and gets a 40-minute timeout because it is less latency-sensitive than PR CI. All other Bazel jobs remain at 30 minutes. ## Why Windows Bazel PR CI currently does the expensive part of the build on Windows. A native Windows Bazel test job on `main` completed in about 28m12s, leaving very little headroom under the 30-minute job timeout and making Windows the slowest PR signal. #20485 showed that Windows cross-compilation can be materially faster for Cargo release builds, but PR CI needs Bazel because Bazel owns our test sharding, flaky-test retries, and integration-test layout. This PR applies the same high-level shape we already use for macOS Bazel CI: compile with remote Linux execution, then run platform-specific tests on the platform runner. The compromise is deliberately signal-aware: code-mode/V8 changes are rare enough that PR CI can accept losing the direct V8/code-mode smoke-test signal temporarily, while `main` still runs the native Windows job post-merge to catch that class of regression. A follow-up PR should investigate making the cross-built Windows gnullvm V8 archive pass the direct V8/code-mode tests so this tradeoff can eventually go away. ## What Changed - Adds a `ci-windows-cross` Bazel config that targets `x86_64-pc-windows-gnullvm`, uses Linux RBE for build actions, and keeps `TestRunner` actions local on the Windows runner. - Adds explicit Windows platform definitions for `windows_x86_64_gnullvm`, `windows_x86_64_msvc`, and a bridge toolchain that lets gnullvm test targets execute under the Windows MSVC host platform. - Updates the Windows Bazel PR test leg to opt into the cross-compile path via `--windows-cross-compile` and `--remote-download-toplevel`. - Adds a `test-windows-native-main` job that runs only for `push` events on `refs/heads/main`, uses the native Windows Bazel path, includes V8/code-mode smoke tests, and has `timeout-minutes: 40`. - Keeps fork/community PRs without `BUILDBUDDY_API_KEY` on the previous local Windows MSVC-host fallback, including `--host_platform=//:local_windows_msvc` and `--jobs=8`. - Preserves the existing integration-test shape on non-gnullvm platforms, while generating Windows-cross wrapper targets only for `windows_gnullvm`. - Resolves `CARGO_BIN_EXE_*` values from runfiles at test runtime, avoiding hard-coded Cargo paths and duplicate test runfiles. - Extends the V8 Bazel patches enough for the `x86_64-pc-windows-gnullvm` target and Linux remote execution path. - Makes the Windows sandbox test cwd derive from `INSTA_WORKSPACE_ROOT` at runtime when Bazel provides it, because cross-compiled binaries may contain Linux compile-time paths. - Keeps the direct V8/code-mode unit smoke tests out of the Windows cross PR path for now while native Windows CI continues to cover them post-merge. ## Command Shape The fast Windows PR test leg invokes the normal Bazel CI wrapper like this: ```shell ./.github/scripts/run-bazel-ci.sh \ --print-failed-action-summary \ --print-failed-test-logs \ --windows-cross-compile \ --remote-download-toplevel \ -- \ test \ --test_tag_filters=-argument-comment-lint \ --test_verbose_timeout_warnings \ --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ -- \ //... \ -//third_party/v8:all \ -//codex-rs/code-mode:code-mode-unit-tests \ -//codex-rs/v8-poc:v8-poc-unit-tests ``` With the BuildBuddy secret available on Windows, the wrapper selects `--config=ci-windows-cross` and appends the important Windows-cross overrides after rc expansion: ```shell --host_platform=//:rbe --shell_executable=/bin/bash --action_env=PATH=/usr/bin:/bin --host_action_env=PATH=/usr/bin:/bin --test_env=PATH=${CODEX_BAZEL_WINDOWS_PATH} ``` The native post-merge Windows job intentionally omits `--windows-cross-compile` and does not exclude the V8/code-mode unit targets: ```shell ./.github/scripts/run-bazel-ci.sh \ --print-failed-action-summary \ --print-failed-test-logs \ -- \ test \ --test_tag_filters=-argument-comment-lint \ --test_verbose_timeout_warnings \ --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ --build_metadata=TAG_windows_native_main=true \ -- \ //... \ -//third_party/v8:all ``` ## Research Notes The existing macOS Bazel CI config already uses the model we want here: build actions run remotely with `--strategy=remote`, but `TestRunner` actions execute on the macOS runner. This PR mirrors that pattern for Windows with `--strategy=TestRunner=local`. The important Bazel detail is that `rules_rs` is already targeting `x86_64-pc-windows-gnullvm` for Windows Bazel PR tests. This PR changes where the build actions execute; it does not switch the Bazel PR test target to Cargo, `cargo-nextest`, or the MSVC release target. Cargo release builds differ from this Bazel path for V8: the normal Windows Cargo release target is MSVC, and `rusty_v8` publishes prebuilt Windows MSVC `.lib.gz` archives. The Bazel PR path targets `windows-gnullvm`; `rusty_v8` does not publish a prebuilt Windows GNU/gnullvm archive, so this PR builds that archive in-tree. That Linux-RBE-built gnullvm archive currently crashes in direct V8/code-mode smoke tests, which is why the workflow keeps native Windows coverage on `main`. The less obvious Bazel detail is test wrapper selection. Bazel chooses the Windows test wrapper (`tw.exe`) from the test action execution platform, not merely from the Rust target triple. The outer `workspace_root_test` therefore declares the default test toolchain and uses the bridge toolchain above so the test action executes on Windows while its inner Rust binary is built for gnullvm. The V8 investigation exposed a Windows-client gotcha: even when an action execution platform is Linux RBE, Bazel can still derive the genrule shell path from the Windows client. That produced remote commands trying to run `C:\Program Files\Git\usr\bin\bash.exe` on Linux workers. The wrapper now passes `--shell_executable=/bin/bash` with `--host_platform=//:rbe` for the Windows cross path. The same Windows-client/Linux-RBE boundary also affected `third_party/v8:binding_cc`: a multiline genrule command can carry CRLF line endings into Linux remote bash, which failed as `$'\r'`. That genrule now keeps the `sed` command on one physical shell line while using an explicit Starlark join so the shell arguments stay readable. ## Verification Local checks included: ```shell bash -n .github/scripts/run-bazel-ci.sh bash -n workspace_root_test_launcher.sh.tpl ruby -e "require %q{yaml}; YAML.load_file(%q{.github/workflows/bazel.yml}); puts %q{ok}" RUNNER_OS=Linux ./scripts/list-bazel-clippy-targets.sh RUNNER_OS=Windows ./scripts/list-bazel-clippy-targets.sh RUNNER_OS=Linux ./tools/argument-comment-lint/list-bazel-targets.sh RUNNER_OS=Windows ./tools/argument-comment-lint/list-bazel-targets.sh ``` The Linux clippy and argument-comment target lists contain zero `*-windows-cross-bin` labels, while the Windows lists still include 47 Windows-cross internal test binaries. CI evidence: - Baseline native Windows Bazel test on `main`: success in about 28m12s, https://github.com/openai/codex/actions/runs/25206257208/job/73907325959 - Green Windows-cross Bazel run on the split PR before adding the main-only native leg: Windows test 9m16s, Windows release verify 5m10s, Windows clippy 4m43s, https://github.com/openai/codex/actions/runs/25231890068 - The latest SHA adds the explicit PR-vs-main tradeoff in `bazel.yml`; CI is rerunning on that focused diff. ## Follow-Up A subsequent PR should investigate making a cross-built Windows binary work with V8/code-mode enabled. Likely options are either making the Linux-RBE-built `windows-gnullvm` V8 archive correct at runtime, or evaluating whether a Bazel MSVC target/toolchain can reuse the same prebuilt MSVC `rusty_v8` archive shape that Cargo release builds already use. |
||
|
iceweasel-oai
|
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 |
||
|
Dylan Hurd
|
32aad7bd13 |
Serialize legacy Windows PowerShell sandbox tests (#19453)
## Why Recent `main` CI had repeated Windows timeouts in the legacy sandbox process tests: - `codex-windows-sandbox session::tests::legacy_capture_powershell_emits_output` failed in runs [24909500958](https://github.com/openai/codex/actions/runs/24909500958), [24908076251](https://github.com/openai/codex/actions/runs/24908076251), [24906197645](https://github.com/openai/codex/actions/runs/24906197645), [24905411571](https://github.com/openai/codex/actions/runs/24905411571), [24903336028](https://github.com/openai/codex/actions/runs/24903336028), and [24898949647](https://github.com/openai/codex/actions/runs/24898949647). - `legacy_tty_powershell_emits_output_and_accepts_input` failed in the same set of runs. - `legacy_non_tty_cmd_emits_output` failed in runs [24909500958](https://github.com/openai/codex/actions/runs/24909500958), [24908076251](https://github.com/openai/codex/actions/runs/24908076251), [24906197645](https://github.com/openai/codex/actions/runs/24906197645), and [24903336028](https://github.com/openai/codex/actions/runs/24903336028). - `legacy_non_tty_powershell_emits_output` failed in runs [24908076251](https://github.com/openai/codex/actions/runs/24908076251), [24906197645](https://github.com/openai/codex/actions/runs/24906197645), and [24903336028](https://github.com/openai/codex/actions/runs/24903336028). These failures were 30s timeouts on Windows x64 and/or arm64 rather than assertion failures. ## Root Cause The active legacy Windows sandbox process tests all exercise host-level resources: sandbox setup, ACL/user state, private desktop process launch, stdio capture, and PowerShell/cmd child cleanup. Running several of these tests concurrently can leave them competing for the same Windows sandbox setup path and process/session resources, which makes command startup or output collection hang under CI load. ## What Changed - Added a shared in-process mutex for the active legacy Windows sandbox process tests. - Held that guard across each legacy cmd/PowerShell process test so those host-resource-heavy cases run one at a time. - Kept the skipped legacy cmd TTY tests unchanged. ## Why This Should Be Reliable The tests still use unique homes and run the real legacy sandbox process path, but they no longer overlap the fragile host-level setup and process/session lifecycle. Serializing just this small group removes the concurrency race without reducing the behavioral coverage of each test. ## Verification - `cargo test -p codex-windows-sandbox` - GitHub Windows CI is the primary validation signal for the affected tests; on this PR, Windows clippy, Windows release, and Windows local Bazel passed after the serialization fix. |
||
|
iceweasel-oaiandviyatb-oai
|
e787358f70 |
check PID of named pipe consumer (#19283)
## Why The elevated Windows command runner currently trusts the first process that connects to its parent-created named pipes. Tightening the pipe ACL already narrows who can reach that boundary, but verifying the connected client PID gives the parent one more fail-closed check: it only accepts the exact runner process it just spawned. ## What changed - validate `GetNamedPipeClientProcessId` after `ConnectNamedPipe` and reject clients whose PID does not match the spawned runner - also did some code de-duplication to route the one-shot elevated capture flow in `windows-sandbox-rs/src/elevated_impl.rs` through `spawn_runner_transport()` so both elevated codepaths use the same pipe bootstrap and PID validation Using the transport unification here also reduces duplication in the elevated Windows IPC bootstrap, so future hardening to the runner handshake only needs to land in one place. ## Validation - `cargo test -p codex-windows-sandbox` - manual testing: one-shot elevated path via `target/debug/codex.exe exec` running a randomized shell command and confirming captured output - manual testing: elevated session path via `target/debug/codex.exe -c 'windows.sandbox="elevated"' sandbox windows -- python -u -c ...` with stdin/stdout round-trips (`READY`, then `GOT:...` for two input lines) --------- Co-authored-by: viyatb-oai <viyatb@openai.com> |
||
|
iceweasel-oai
|
6f6997758a | skip busted tests while I fix them (#18885) | ||
|
|
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> |