mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
2a320fedb53bc0ce89d2887e5b8c2896bc079ca7
128
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  iceweasel-oaiandGitHub
|
18fe1d9fe3 |
[codex] Preserve proxy state for filesystem sandbox helpers (#29671)
## Why Filesystem helpers intentionally run with a minimal environment that excludes proxy variables. After filesystem operations started using the Windows sandbox wrapper, the wrapper derived an empty proxy configuration from that helper environment and compared it with the persistent sandbox setup marker. When the marker contained proxy ports, every filesystem operation appeared to require a firewall update, which could launch elevated setup, show a UAC or loader dialog, and fail operations such as `apply_patch` with error 1223. Filesystem helpers do not use network access, so they should preserve the proxy/firewall state established by normal sandboxed process launches. ## What changed - Add an explicit Windows sandbox proxy-settings mode for reconciling or preserving persistent proxy state. - Use preserve mode for filesystem helpers while normal process launches continue to reconcile proxy settings from their environment. - Carry the selected proxy state consistently through setup validation, elevated setup, and non-elevated ACL refreshes. - Cover wrapper argument propagation and marker-derived proxy preservation. ## Validation - `cargo build -p codex-cli --bin codex` - `just test -p codex-windows-sandbox preserving_proxy_settings_uses_the_existing_marker` - `just test -p codex-windows-sandbox windows_wrapper_args_round_trip` - `just test -p codex-windows-sandbox setup_request_prefers_explicit_proxy_settings` - `just test -p codex-sandboxing transform_for_direct_spawn_windows` - `just test -p codex-exec-server fs_sandbox::tests` - Ran the same sandboxed `fs/writeFile` reproduction against published `0.142.0-alpha.6` and the new CLI. The published CLI launched elevated setup and failed with `ShellExecuteExW ... 1223`; the new CLI completed without elevation. Related to #28359. |
||
|
iceweasel-oaiandGitHub
|
afbb69a2fb |
[codex] Fix Windows sandbox runtime ACL refresh (#28943)
## Why Codex Desktop repairs sandbox-user read/execute access for binaries copied to `%LOCALAPPDATA%\OpenAI\Codex\bin`, but Computer Use launches its bundled Node runtime from `%LOCALAPPDATA%\OpenAI\Codex\runtimes`. On fresh Windows installations, `CodexSandboxUsers` may therefore be unable to execute the bundled Node binary. The command runner starts, but `CreateProcessAsUserW` fails with error 5 (`ACCESS_DENIED`), causing the Node REPL to exit before Computer Use can discover applications. This is a follow-up to #21564, which added the original runtime `bin` ACL repair. ## What changed - Expand the Codex Desktop runtime ACL roots from only `bin` to both `bin` and `runtimes`. - Apply the existing inherited read/execute ACL repair to each runtime directory when it exists. - Rename the setup helper to reflect that it now handles multiple runtime paths. ## Validation - `cargo fmt -- --check` - `just test -p codex-windows-sandbox` was run: 113 tests passed and five environment-dependent legacy execution tests failed because `CreateRestrictedToken` returned error 87. |
||
|
iceweasel-oaiandGitHub
|
ef75171f18 |
Run fs helper through Windows sandbox wrapper (#28359)
## Why This is the final PR in the Windows fs-helper sandbox stack and contains the actual bug fix. The exec-server filesystem helper is a direct-spawn path: it asks `SandboxManager` for a `SandboxExecRequest`, then launches the returned argv itself. That works on macOS and Linux because the transformed argv is already a self-contained sandbox wrapper. On Windows, the transformed request carried `WindowsRestrictedToken` metadata, but the direct-spawn fs-helper runner still launched the helper argv directly. That means Windows filesystem built-ins backed by the fs-helper could run with the parent Codex process permissions instead of the configured Windows sandbox. This PR makes the direct-spawn transform produce a self-contained Windows wrapper argv before fs-helper launches it. ## What Changed - Added `SandboxManager::transform_for_direct_spawn()` for callers that launch the returned argv themselves. - Wrapped Windows restricted-token direct-spawn requests with `codex.exe --run-as-windows-sandbox` and then marked the outer request as unsandboxed, matching the macOS/Linux wrapper argv shape. - Updated `exec-server/src/fs_sandbox.rs` to use the direct-spawn transform for fs-helper launches. - Materialized the inner `codex.exe --codex-run-as-fs-helper` executable into `.sandbox-bin` so the sandboxed user can run it. - Carried runtime workspace roots through `FileSystemSandboxContext` as `PathUri` values so `:workspace_roots` policies resolve correctly without sending native client paths over exec-server JSON. - Preserved wrapper setup identity environment needed by Windows sandbox setup without changing the serialized inner helper environment. ## Verification - `just bazel-lock-update` - `just bazel-lock-check` - `just test -p codex-sandboxing transform_for_direct_spawn_windows` - `just test -p codex-exec-server fs_sandbox::tests` - `just fix -p codex-windows-sandbox -p codex-sandboxing -p codex-exec-server -p codex-core -p codex-file-system` Local note: `just fmt` completed Rust formatting, but this workstation still fails the non-Rust formatter phases because uv cannot open its cache and the local buildifier/dotslash path is missing. |
||
|
iceweasel-oaiandGitHub
|
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. |
||
|
iceweasel-oaiandGitHub
|
c0b36d234a |
recover stale Windows sandbox credentials (#27944)
## Why The elevated Windows sandbox persists dedicated sandbox account credentials so later commands can launch without reprovisioning. If those persisted credentials drift from the actual Windows account password, `CreateProcessWithLogonW` fails with `ERROR_LOGON_FAILURE` and Codex currently surfaces that as a hard runner launch failure. This change makes that failure self-healing. When Windows specifically rejects the sandbox login, Codex now treats the persisted sandbox credentials as stale, regenerates them through the existing setup path, and retries the runner launch once. ## What Changed - Preserve `CreateProcessWithLogonW` failures as a typed runner logon error so callers can distinguish `ERROR_LOGON_FAILURE` from unrelated launch failures. - Add a sandbox credential refresh helper that deletes the persisted `sandbox_users.json` record and reuses `require_logon_sandbox_creds()` to reprovision credentials through the established setup flow. - Retry elevated runner startup after stale-credential failures in both the legacy elevated capture path and unified exec elevated backend. - Add focused tests for stale logon failure detection and persisted sandbox user file removal. ## Validation - `git diff --check` - `cargo test -p codex-windows-sandbox` |
||
|
iceweasel-oaiandGitHub
|
c3415f76c9 |
Extract shared Windows sandbox session runner (#28357)
## Why This is the first PR in a stack for the Windows fs-helper sandbox fix. Before changing fs-helper behavior, this pulls the reusable Windows sandbox session launch pieces out of the debug CLI path so later PRs can call the same backend selection and stdio forwarding logic. Keeping this as a pure refactor makes the later security fix easier to review: `codex sandbox windows` should continue to launch the same elevated or restricted-token backend, just through shared APIs in `windows-sandbox-rs` instead of code local to `cli/src/debug_sandbox.rs`. ## What Changed - Added `WindowsSandboxSessionRequest` and `spawn_windows_sandbox_session_for_level()` in `windows-sandbox-rs` to share the elevated-vs-legacy session launch decision. - Moved the Windows sandbox stdio forwarding helpers from `cli/src/debug_sandbox.rs` into `windows-sandbox-rs/src/stdio_bridge.rs`. - Updated `codex sandbox windows` to call the shared session launcher and stdio bridge. - Added unit coverage for the moved stdio forwarding helpers. ## Verification - `just bazel-lock-update` - `just bazel-lock-check` - `just test -p codex-windows-sandbox stdio_bridge::tests` - `just fix -p codex-windows-sandbox -p codex-sandboxing -p codex-exec-server -p codex-arg0 -p codex-core -p codex-file-system` - The new `stdio_bridge` tests also passed as part of `just test -p codex-windows-sandbox` on the stack tip. That full local run still fails in pre-existing legacy session integration tests with `CreateRestrictedToken failed: 87` on this workstation. |
||
 Adam Perry @ OpenAIandGitHub
|
740c4f269d |
build: run buildifier from just fmt (#28125)
## Intent Keep Bazel and Starlark files consistently formatted without requiring contributors to install or version buildifier themselves. ## Implementation - Add a SHA-256-pinned, cross-platform DotSlash manifest for buildifier v8.5.1. - Run buildifier from the shared `just fmt` and `just fmt-check` driver, with Windows-safe explicit DotSlash invocation. - Provision DotSlash in formatting CI and contributor devcontainers, and document the source-build prerequisite. - Apply the initial mechanical buildifier formatting baseline. |
||
|
iceweasel-oaiandGitHub
|
0b2e7b5eb1 |
Improve Windows sandbox setup refresh diagnostics (#26471)
## Why Users have been seeing opaque Windows sandbox setup refresh failures such as `windows sandbox: spawn setup refresh`, including reports in #24391 and #21208. The setup refresh path already runs the Windows sandbox setup helper, but it was not using the same structured `setup_error.json` reporting path that elevated setup uses. As a result, when the helper exited non-zero, Codex only surfaced a generic refresh status instead of the helper's `SetupFailure` code and message. ## What changed - Clear stale `setup_error.json` before non-elevated setup refresh launches the helper. - When the refresh helper exits non-zero, read the helper-written report through the existing `report_helper_failure` path. - Keep a parent-side launch diagnostic for cases where the helper never starts, including the helper path, cwd, sandbox log path, and spawn error. - Clear the setup error report after a successful refresh. - Add regression coverage for report consumption and stale-report avoidance. ## Verification - `cargo test -p codex-windows-sandbox setup::tests::` |
||
 pakrym-oaiandGitHub
|
555f8caeff |
[codex] Fix Windows sandbox build script lint (#26445)
## Why The Windows ARM64 Cargo clippy job on `main` is failing because workspace lints deny `clippy::expect_used`, and the `codex-windows-sandbox` build script used `expect()` while reading `CARGO_MANIFEST_DIR`. ## What changed `codex-rs/windows-sandbox-rs/build.rs` now returns `Result<(), String>` from `main()` and converts a missing `CARGO_MANIFEST_DIR` into an explicit build-script error. The non-Windows early return and Windows linker argument behavior are unchanged. ## Verification - `just clippy -p codex-windows-sandbox -- -D warnings` - `just test -p codex-windows-sandbox` |
||
 AbhinavandGitHub
|
0ed2735d19 |
Use Windows setup marker as completion signal (#26074)
# Why When an organization requires the elevated Windows sandbox, Codex launches an elevated helper to provision users, configure firewall and ACL rules, and lock persistent sandbox directories. We observed that closing the helper after setup started could leave the machine partially initialized while the TUI still announced **Sandbox ready**. Model-only turns continued to work, but the first shell command retried setup and failed with Windows cancellation error `1223`. This was not an enforcement bypass; command execution continued to fail closed. The issue was a false readiness signal: `setup_marker.json` was written during user provisioning, before the remaining setup stages had completed. # What Treat `setup_marker.json` as the commit record for Windows sandbox setup: 1. Before full or provisioning setup begins, remove the existing marker and create the final marker path with a protected ACL. 2. Keep the marker empty and therefore invalid while setup is in progress. Sandbox users cannot read, modify, or replace it. 3. Run every synchronous setup stage. 4. After setup succeeds, write the valid marker contents without changing its ACL. 5. After the helper exits successfully, verify the existing readiness check before enabling the sandbox. If setup is canceled or fails, the marker remains invalid and Codex reports setup as incomplete instead of announcing readiness. Refresh-only and read-ACL-only helper runs continue to leave the marker untouched. The setup version remains `5` to avoid forcing all existing Windows users through elevated setup again. # Verification - Added coverage confirming sandbox users cannot read or modify the setup marker after elevated setup. - Added coverage confirming a successful helper exit without complete setup artifacts is rejected. - Ran `just test -p codex-windows-sandbox`. |
||
|
iceweasel-oaiandGitHub
|
b2344d8fbc |
[codex] Restore setup helper UAC manifest (#25949)
## Why #23764 removed Windows resource stamping from `codex-windows-sandbox`, but it also removed the setup helper's UAC manifest. That manifest was doing more than cosmetic version metadata: Microsoft documents `requestedExecutionLevel level="asInvoker"` as the setting that makes an executable run at the same permission level as the process that started it: https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests#trustinfo In the reported session, `codex-windows-sandbox-setup.exe` was launched for a non-elevated setup refresh and `CreateProcess` failed with `os error 740` (`The requested operation requires elevation`). Restoring an explicit `asInvoker` manifest records the helper's intended default launch contract: normal launches inherit the caller's token, and elevation only happens through the code paths that request it explicitly. The setup helper has two launch modes: - setup refresh uses a normal `Command::new(...)` spawn and should never trigger UAC - full setup explicitly uses `ShellExecuteExW` with the `runas` verb when elevation is required Restoring `asInvoker` keeps refresh non-elevated by default while preserving the explicit elevated path for full setup. ## What changed - Restored a minimal `codex-windows-sandbox-setup.manifest` containing only `requestedExecutionLevel level="asInvoker"`. - Added a small build script that passes setup-helper-scoped manifest linker args for MSVC and the Windows GNU/LLVM target used by Bazel. - Wired the manifest into Bazel build-script data. This does not restore `winres`, `FileDescription`, `ProductName`, or package-wide resource stamping, so other Codex binaries that link `codex-windows-sandbox` do not inherit metadata from this package. ## Verification - `cargo fmt -p codex-windows-sandbox` - `cargo build -p codex-windows-sandbox --bin codex-windows-sandbox-setup` - `cargo build -p codex-windows-sandbox --bin codex-command-runner` - `cargo build -p codex-windows-sandbox --lib` - Build-script output simulation for `CARGO_CFG_TARGET_ENV=msvc` emits `/MANIFEST:EMBED` and `/MANIFESTINPUT:<manifest>`. - Build-script output simulation for `CARGO_CFG_TARGET_ENV=gnu` + `CARGO_CFG_TARGET_ABI=llvm` emits `-Wl,-Xlink=/manifest:embed` and `-Wl,-Xlink=/manifestinput:<manifest>`. - Inspected the built binaries and confirmed: - `codex-windows-sandbox-setup.exe` contains `requestedExecutionLevel` / `asInvoker` - `codex-command-runner.exe` does not contain those manifest strings - Windows `VersionInfo` remains blank for `FileDescription` / `ProductName` - `just test -p codex-windows-sandbox` ran through Nextest, with 114 passing, 2 skipped, and 1 existing Windows sandbox failure: `unified_exec::tests::legacy_non_tty_cmd_emits_output` fails with `CreateRestrictedToken failed: 87`. |
||
|
iceweasel-oaiandGitHub
|
cb9178e8b3 |
Add Windows sandbox provisioning setup command (#24831)
## Why Some Windows users do not have local admin access, so they cannot complete the elevated portion of the Windows sandbox setup when Codex first needs it. This adds an alpha provisioning path that an admin or IT deployment script can run ahead of time for the Codex user. The intended managed-deployment shape is: ```powershell codex sandbox setup --elevated --user "$env:COMPUTERNAME\Alice" --codex-home "C:\Users\Alice\.codex" ``` `--elevated` is treated as the requested sandbox setup level, not as proof that the process is elevated. The Windows sandbox setup orchestration still checks that the caller is actually elevated before launching the helper without a UAC prompt. ## What changed - Added `codex sandbox setup --elevated` with explicit user selection via either `--current-user` or `--user ... --codex-home ...`. - Moved the CLI implementation into `cli/src/sandbox_setup.rs` instead of growing `cli/src/main.rs`. - Added a Windows sandbox `ProvisionOnly` helper mode that runs the elevation-required provisioning work without requiring a workspace cwd or runtime sandbox policy. - Reused the existing elevated helper path for creating/updating sandbox users, configuring firewall/WFP rules, and applying sandbox directory ACLs. - Persisted `windows.sandbox = "elevated"` into the target `CODEX_HOME` so the desktop app does not show the initial sandbox setup banner after pre-provisioning succeeds. ## Validation - `cargo fmt -p codex-windows-sandbox -p codex-core -p codex-cli` - `cargo test -p codex-cli sandbox_setup --target-dir target\sandbox-setup-check` - `cargo test -p codex-windows-sandbox payload_accepts_provision_only_mode --target-dir target\sandbox-setup-check` - `git diff --check` - Manual Windows alpha flow with a standard local user (`Mandi Lavida`): ran the new setup command from an admin shell, verified the target `.codex` contents, sandbox marker/secrets, ACLs, firewall rules, and desktop startup without the sandbox setup banner once experimental network proxy requirements were disabled. ## Notes This intentionally does not solve later elevated update coordination for IT-managed deployments. The setup command can still apply provisioning updates when run again, but a broader coordination/process story is out of scope for this alpha. |
||
 Michael BolinandGitHub
|
bcf2b55957 |
windows-sandbox: fix capture cancellation test roots (#24974)
## Why The Windows Bazel job on `main` started failing after #24108 because one Windows-only capture test still passed `cwd.as_path()` to `run_windows_sandbox_capture`. That helper now expects the explicit `workspace_roots` slice introduced by #24108, so the Windows test target no longer compiled. ## What Changed - Updates `legacy_capture_cancellation_is_not_reported_as_timeout` to pass `workspace_roots_for(cwd.as_path()).as_slice()`, matching the adjacent capture test and the new runner signature. ## Verification - GitHub Actions CI is the important validation for this Windows-only compile path. - Created quickly to get Windows CI running while the separate Ubuntu `compact_resume_fork` timeout is still under investigation. |
||
|
Michael BolinandGitHub
|
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. |
||
 
|
3cf737e4e3 |
fix: cancel Windows sandbox on network denial (#19880)
## Why When Guardian or the sandbox network proxy detects and denies a network attempt, core cancels the associated execution through `ExecExpiration`. The Windows sandbox capture path was only forwarding the timeout component of that expiration state. As a result, a sandboxed Windows command whose network attempt had already been denied could keep running until its timeout elapsed rather than terminating promptly in response to the denial. This change closes that cancellation-propagation gap for Windows sandbox execution. ## What changed - Added `WindowsSandboxCancellationToken` as the cancellation hook exposed to Windows capture backends. - Extracted the cancellation token from `ExecExpiration` in core and passed it to both the direct and elevated Windows sandbox capture paths alongside the existing timeout. - Updated direct capture to poll for either process exit, timeout, or cancellation and to terminate cancelled processes without reporting them as timed out. - Updated elevated capture to watch for cancellation and send the existing `Terminate` IPC frame to the elevated runner. The watcher parks for 50 ms between checks to bound response latency without a tight busy wait. - Added Windows regression coverage for a long-running PowerShell command: cancellation ends capture before its timeout and does not set `timed_out`. - Added a visible skip diagnostic when that PowerShell-dependent regression test cannot execute, and consolidated the duplicated expiration-policy branch identified in review. ## Security This improves enforcement after a denied network attempt has been attributed to a Windows sandboxed execution: the command no longer remains alive simply because Windows capture lost the cancellation signal. This PR does not claim to make Windows offline mode an airtight no-network or no-exfiltration boundary. It does not introduce AppContainer or change how network denial is detected; it makes an already-detected denial promptly stop the affected sandboxed command. ## Validation ### Commands run - `just fmt` - `cargo test -p codex-windows-sandbox` - `cargo test -p codex-core network_denial` - `cargo clippy -p codex-core -p codex-windows-sandbox --tests --no-deps -- -D warnings` - `just argument-comment-lint -p codex-windows-sandbox -p codex-core` The new capture regression is `cfg(target_os = "windows")`, so Windows CI is the execution coverage for that test path. The local macOS test runs validate the host-runnable crate and core network-denial behavior. --------- Co-authored-by: Codex <noreply@openai.com> |
||
|
iceweasel-oaiandGitHub
|
9826581e7b |
Attach Windows sandbox log to feedback reports (#24623)
## Why Windows sandbox diagnostics are currently hard to recover from `/feedback` even though they are often the most useful artifact when debugging sandbox behavior. Now that sandbox logging uses daily rolling files, feedback can safely include the current day's sandbox log without uploading the old ever-growing legacy `sandbox.log`. ## What changed - Add a `codex-windows-sandbox` helper that resolves the current daily sandbox log from `codex_home`. - When feedback is submitted with logs enabled on Windows, app-server attaches today's sandbox log if it exists. - Upload the attachment under the stable filename `windows-sandbox.log`, independent of the dated on-disk filename. - Keep existing raw `extra_log_files` behavior unchanged for rollout and desktop log attachments. ## Verification - `cargo fmt -p codex-app-server -p codex-windows-sandbox` - `cargo test -p codex-windows-sandbox current_log_file_path_for_codex_home_uses_sandbox_dir` - `cargo test -p codex-app-server windows_sandbox_log_attachment_uses_current_log` - Manual CLI/TUI `/feedback` test confirmed Sentry received `windows-sandbox.log`. |
||
|
Michael BolinandGitHub
|
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-oaiandGitHub
|
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 BolinandGitHub
|
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 |
||
|
iceweasel-oaiandGitHub
|
8253ae4e5c |
Remove Windows sandbox resource stamping (#23764)
## Why The `codex-windows-sandbox` crate was embedding Windows resource metadata through a package-level `build.rs`. Because that package also exposes the `codex_windows_sandbox` library, downstream binaries that link the library could inherit `FileDescription` / `ProductName` values of `codex-windows-sandbox`. That made ordinary Codex binaries, including the long-lived `codex.exe` app-server sidecar, appear as `codex-windows-sandbox` in Windows UI surfaces such as Task Manager / file properties. We do not rely on this metadata enough to justify a larger bin-only resource split, so this removes the resource stamping entirely. ## What changed - Removed the `windows-sandbox-rs` build script that invoked `winres`. - Removed the setup manifest that was only consumed by that build script. - Removed the `winres` build dependency and corresponding `Cargo.lock` / `MODULE.bazel.lock` entries. - Removed the now-unused Bazel build-script data. ## Verification - `cargo build -p codex-windows-sandbox --bins` - `cargo build -p codex-cli --bin codex` - `bazel mod deps --lockfile_mode=update` via Bazelisk, with local remote-cache-disabling flags because `bazel` is not installed on PATH here - `bazel mod deps --lockfile_mode=error` via Bazelisk, with the same local flags - Verified rebuilt `codex.exe`, `codex-command-runner.exe`, and `codex-windows-sandbox-setup.exe` now have blank `FileDescription` / `ProductName` fields. - `cargo test -p codex-windows-sandbox` still fails on two legacy Windows sandbox tests with `CreateRestrictedToken failed: 87` and the follow-on poisoned test lock; 85 passed, 2 ignored. |
||
|
Michael BolinandGitHub
|
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 BolinandGitHub
|
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 BolinandGitHub
|
61aae56571 |
windows-sandbox: share bundled helper lookup (#23735)
## Summary Follow-up to #23636 review feedback: the Windows sandbox had two copies of the same bundled-helper lookup order, one for `codex-command-runner.exe` in `helper_materialization.rs` and one for `codex-windows-sandbox-setup.exe` in `setup.rs`. This PR centralizes that lookup in `helper_materialization::bundled_executable_path_for_exe()` and has setup reuse it for `codex-windows-sandbox-setup.exe`. The lookup behavior is unchanged: direct sibling first, package-root `codex-resources/` when running from `bin/`, then legacy sibling `codex-resources/`. ## Test plan - `cargo test -p codex-windows-sandbox` ## Notes I also attempted `cargo check -p codex-windows-sandbox --target x86_64-pc-windows-gnullvm`, but this local host is missing `x86_64-w64-mingw32-clang`. |
||
|
Michael BolinandGitHub
|
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 |
||
|
Michael BolinandGitHub
|
110b30d545 |
install: consume Codex package archives (#23636)
## Summary Standalone installs should exercise the same canonical package archive layout that release builds produce, rather than unpacking npm platform packages and reconstructing a parallel install tree. This updates `install.sh` and `install.ps1` to prefer `codex-package-<target>.tar.gz` plus `codex-package_SHA256SUMS` introduced in https://github.com/openai/codex/pull/23635, authenticate the checksum manifest against GitHub release metadata, verify the selected package archive against the authenticated manifest, and install the package archive directly. ## Compatibility Notes Package installs still leave a compatibility command at `current/codex` for managed daemon flows, while visible command shims point at `bin/codex` inside the package layout. Recent releases that predate package archives still publish per-platform npm artifacts, so both installers keep a legacy platform npm fallback for those versions and verify those archives against release metadata directly. Releases old enough to publish only the single root `codex-npm-<version>.tgz` archive are intentionally out of scope. The installers fail clearly when neither package archives nor per-platform npm archives are present. On Windows, the runtime helper lookups now recognize package-layout installs where `codex.exe` runs from `bin/`, so `codex-command-runner.exe` and `codex-windows-sandbox-setup.exe` resolve from the top-level `codex-resources/` directory. The direct-sibling and older sibling-resource fallbacks are preserved. ## Test plan - `sh -n scripts/install/install.sh` - `bash -n scripts/install/install.sh` - `pwsh -NoProfile -Command '$tokens=$null; $errors=$null; $null = [System.Management.Automation.Language.Parser]::ParseFile("scripts/install/install.ps1", [ref]$tokens, [ref]$errors); if ($errors.Count) { $errors | Format-List *; exit 1 }'` - `HOME="$home_dir" CODEX_HOME="$tmp_dir/codex-home" CODEX_INSTALL_DIR="$bin_dir" PATH="$bin_dir:$PATH" sh scripts/install/install.sh --release 0.125.0` - Verified the 0.125.0 isolated install leaves the visible command pointed at `current/codex` and includes the legacy `codex-resources/rg` payload. - `cargo test -p codex-windows-sandbox` - `just fix -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/23636). * #23638 * #23637 * __->__ #23636 |
||
|
Michael BolinandGitHub
|
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 |
||
|
viyatb-oaiandGitHub
|
3c76081876 |
Make deny canonical for filesystem permission entries (#23493)
## Why Filesystem permission profiles used `none` for deny-read entries, which is less direct than the action the entry actually represents. This change makes `deny` the canonical filesystem permission spelling while preserving compatibility for older configs that still send `none`. ## What changed - rename `FileSystemAccessMode::None` to `Deny` - serialize and generate schemas with `deny` as the canonical value - retain `none` only as a legacy input alias for temporary config compatibility - update filesystem glob diagnostics and regression coverage to use the canonical spelling - refresh config and app-server schema fixtures to match the new wire shape ## Validation - `cargo test -p codex-protocol` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-core config_toml_deserializes_permission_profiles --lib` - `cargo test -p codex-core read_write_glob_patterns_still_reject_non_subpath_globs --lib` Earlier in the session, a broad `cargo test -p codex-core` run reached unrelated pre-existing failures in timing/snapshot/git-info tests under this environment; the targeted surfaces touched by this PR passed cleanly. |
||
|
iceweasel-oaiandGitHub
|
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. |
||
 starr-openaiandGitHub
|
7c11c14efc |
Fix Windows sandbox clippy clones (#22687)
## Summary - remove two redundant `PathBuf` clones in Windows sandbox setup tests - fix current `rust-ci-full` Windows clippy failures on `main` ## Validation - `just fmt` - attempted on `dev`: `cargo clippy --target x86_64-pc-windows-msvc --tests --profile dev --timings -- -D warnings` - blocked by missing MSVC cross toolchain on the Linux devbox (`lib.exe` / MSVC C toolchain unavailable) - live failure evidence: main `rust-ci-full` runs 25880209898 and 25879137967 failed on `windows-sandbox-rs/src/bin/setup_main/win.rs` with `clippy::redundant_clone` at the two edited callsites |
||
|
iceweasel-oaiandGitHub
|
c9edb26755 |
windows-sandbox: fail elevated setup when firewall policy is ineffective (#22353)
## Why Elevated Windows sandbox setup currently assumes that the firewall rules it writes will take effect. On managed Windows hosts, local firewall policy changes can be ignored or only partially apply across the active profiles, which means setup can appear to succeed without providing the expected network isolation. ## What changed - Query `INetFwPolicy2::LocalPolicyModifyState` before configuring the elevated sandbox firewall rules. - Fail setup when Windows reports that local firewall policy edits are ineffective or only apply to some current profiles. - Surface that condition with a dedicated `helper_firewall_policy_ineffective` setup error code so support and IT-facing diagnostics can distinguish it from COM access failures. - Add focused coverage for effective policy, group-policy override, and partial-profile coverage cases. ## Testing - `cargo test -p codex-windows-sandbox --bin codex-windows-sandbox-setup` |
||
 AdrianandGitHub
|
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> |
||
|
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> |
||
 bbrown-oaiandGitHub
|
31b233c7c6 |
codex-otel: add configurable trace metadata (#21556)
Add Codex config for static trace span attributes and structured W3C tracestate field upserts. The config flows through OtelSettings so callers can attach trace metadata without touching every span call site. Apply span attributes with an SDK span processor so every exported trace span carries the configured metadata. Model tracestate as nested member fields so configured keys can be upserted while unrelated propagated state in the same member is preserved. Validate configured tracestate before installing provider-global state, including header-unsafe values the SDK does not reject by itself. This keeps Codex from propagating malformed trace context from config. Update the config schema, public docs, and OTLP loopback coverage for config parsing, span export, propagation, and invalid-header rejection. |
||
|
|
54ef99a365 |
Disable empty Cargo test targets (#21584)
## Summary `cargo test` has entails both running standard Rust tests and doctests. It turns out that the doctest discovery is fairly slow, and it's a cost you pay even for crates that don't include any doctests. This PR disables doctests with `doctest = false` for crates that lack any doctests. For the collection of crates below, this speeds up test execution by >4x. E.g., before this PR: ``` Benchmark 1: cargo test -p codex-utils-absolute-path -p codex-utils-cache -p codex-utils-cli -p codex-utils-home-dir -p codex-utils-output-truncation -p codex-utils-path -p codex-utils-string -p codex-utils-template -p codex-utils-elapsed -p codex-utils-json-to-toml Time (mean ± σ): 1.849 s ± 4.455 s [User: 0.752 s, System: 1.367 s] Range (min … max): 0.418 s … 14.529 s 10 runs ``` And after: ``` Benchmark 1: cargo test -p codex-utils-absolute-path -p codex-utils-cache -p codex-utils-cli -p codex-utils-home-dir -p codex-utils-output-truncation -p codex-utils-path -p codex-utils-string -p codex-utils-template -p codex-utils-elapsed -p codex-utils-json-to-toml Time (mean ± σ): 428.6 ms ± 6.9 ms [User: 187.7 ms, System: 219.7 ms] Range (min … max): 418.0 ms … 436.8 ms 10 runs ``` For a single crate, with >2x speedup, before: ``` Benchmark 1: cargo test -p codex-utils-string Time (mean ± σ): 491.1 ms ± 9.0 ms [User: 229.8 ms, System: 234.9 ms] Range (min … max): 480.9 ms … 512.0 ms 10 runs ``` And after: ``` Benchmark 1: cargo test -p codex-utils-string Time (mean ± σ): 213.9 ms ± 4.3 ms [User: 112.8 ms, System: 84.0 ms] Range (min … max): 206.8 ms … 221.0 ms 13 runs ``` Co-authored-by: Codex <noreply@openai.com> |
||
|
iceweasel-oaiandGitHub
|
163eac9306 |
Grant sandbox users access to desktop runtime bin (#21564)
## Why Codex desktop copies bundled Windows binaries out of `WindowsApps` into a LocalAppData runtime cache before launching `codex.exe`. Sandboxed commands can then need to execute helpers from that cache, but the sandbox user group may not have read/execute access to the runtime bin directory. This makes the Windows sandbox refresh path repair that access directly so the packaged desktop runtime remains usable from sandboxed sessions. ## What changed - Added `setup_runtime_bin` to locate `%LOCALAPPDATA%\OpenAI\Codex\bin`, matching the desktop bundled-binaries destination path, with the same `USERPROFILE\AppData\Local` fallback shape. - During refresh setup, check whether `CodexSandboxUsers` already has read/execute access to the runtime bin directory. - If access is missing, grant `CodexSandboxUsers` `OI/CI/RX` inheritance on that directory. - If the runtime bin directory does not exist, no-op cleanly. ## Verification - `cargo build -p codex-windows-sandbox --bin codex-windows-sandbox-setup` - `cargo test -p codex-windows-sandbox --bin codex-windows-sandbox-setup` - Manual Windows ACL exercise against the installed packaged runtime bin: - existing inherited `CodexSandboxUsers:(I)(OI)(CI)(RX)` no-ops without changing SDDL - after disabling inheritance and removing the group ACE, setup adds `CodexSandboxUsers:(OI)(CI)(RX)` - with `LOCALAPPDATA` pointed at a fake location without `OpenAI\Codex\bin`, setup exits successfully and does not create the directory - restored the real runtime bin with inherited ACLs and confirmed the final SDDL matched the baseline exactly |
||
|
iceweasel-oaiandGitHub
|
123e78b97b |
[codex] Fix Windows sandbox git safe.directory for worktrees (#21409)
## Why Windows sandboxed commands run as a sandbox user, while workspace repositories are usually owned by the real user. The sandbox compensates by injecting a temporary Git `safe.directory` entry into the child environment. That injection was still broken for linked worktrees because the helper followed the `.git` file's `gitdir:` pointer and injected the internal `.git/worktrees/...` location. Git's dubious-ownership check expects the worktree root instead, so sandboxed Git commands still failed in worktree-based Codex checkouts. ## What changed - Treat any `.git` marker, directory or file, as the worktree root for `safe.directory` injection. - Keep the safe-directory logic in `windows-sandbox-rs/src/sandbox_utils.rs` and have the one-shot elevated path reuse it. - Add regression coverage for both normal `.git` directories and gitfile-based worktrees. ## Validation - `cargo test -p codex-windows-sandbox sandbox_utils::tests` - `cargo test -p codex-windows-sandbox` built and ran; the new `sandbox_utils` tests passed, while two pre-existing legacy sandbox tests failed locally with `Access is denied`: `session::tests::legacy_non_tty_cmd_emits_output` and `spawn_prep::tests::legacy_spawn_env_applies_offline_network_rewrite`. |
||
|
iceweasel-oaiandGitHub
|
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 BolinandGitHub
|
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-oaiandGitHub
|
8121710ffe |
install WFP filters for Windows sandbox setup (#20101)
## Summary This PR installs a first wave of WFP (Windows Filtering Platform) filters that reduce the surface area of network egress vulnerabilities for the Windows Sandbox. - Add persistent Windows Filtering Platform provider, sublayer, and filters for the Windows sandbox offline account. - Install WFP filters during elevated full setup, log failures non-fatally, and emit setup metrics when analytics are enabled. - Bump the Windows sandbox setup version so existing users rerun full setup and receive the new filters. ## What WFP is Windows Filtering Platform (WFP) is the low-level Windows networking policy engine underneath things like Windows Firewall. It lets privileged code install persistent filtering rules at specific network stack layers, with conditions like "only traffic from this Windows account" or "only this remote port," and an action like block. In this change, we create a Codex-owned persistent WFP provider and sublayer, then install block filters scoped to the Windows sandbox's offline user account via `ALE_USER_ID`. That means the filters are targeted at sandboxed processes running as that account, rather than globally affecting the host. ## Initial filter set We are starting with 12 concrete WFP filters across a few high-value bypass surfaces. The table below describes the filter families rather than one filter per row: | Area | Concrete filters | Purpose | | --- | --- | --- | | ICMP | 4 filters: ICMP v4/v6 on `ALE_AUTH_CONNECT` and `ALE_RESOURCE_ASSIGNMENT` | Block direct ping-style network reachability checks from the offline account. | | DNS | 2 filters: remote port `53` on `ALE_AUTH_CONNECT_V4/V6` | Block direct DNS queries that bypass our intended proxy/offline path. | | DNS-over-TLS | 2 filters: remote port `853` on `ALE_AUTH_CONNECT_V4/V6` | Block encrypted DNS attempts that could bypass ordinary DNS interception. | | SMB / NetBIOS | 4 filters: remote ports `445` and `139` on `ALE_AUTH_CONNECT_V4/V6` | Block Windows file-sharing/network share traffic from sandboxed processes. | For IPv4/IPv6 coverage, the port-based filters are installed on both `ALE_AUTH_CONNECT_V4` and `ALE_AUTH_CONNECT_V6`. ICMP also gets both connect-layer and resource-assignment-layer coverage because ICMP traffic is shaped differently from ordinary TCP/UDP port traffic. ## Validation - `cargo fmt -p codex-windows-sandbox` (completed with existing stable-rustfmt warnings about `imports_granularity = Item`) - `cargo test -p codex-windows-sandbox wfp::tests` - `cargo test -p codex-windows-sandbox` (fails in existing legacy PowerShell sandbox tests because `Microsoft.PowerShell.Utility` could not be loaded; WFP tests passed before that failure) |
||
|
iceweasel-oaiandGitHub
|
06f3b4836a |
[codex] Fix elevated Windows sandbox named-pipe access (#20270)
## Summary - add elevated-only token constructors that include the current token user SID in the restricted SID list - switch the elevated Windows command runner to use those constructors - leave the unelevated restricted-token path unchanged ## Why Windows named pipes created by tools like Ninja use the platform's default named-pipe ACL when no explicit security descriptor is provided. In the elevated sandbox, the pipe owner has access, but the write-restricted token can still fail its restricted-SID access check because the sandbox user SID was not in the restricting SID set. That causes child processes to exit successfully while Ninja never receives the expected pipe completion/close behavior and hangs. Including the elevated sandbox user's SID in the restricting SID list lets the restricted check succeed for these owner-scoped pipe objects without broadening the unelevated sandbox to the real signed-in user. ## Impact - fixes the minimal Ninja hang repro in the elevated Windows sandbox - preserves the existing unelevated sandbox behavior and write protections - keeps the change scoped to the elevated runner rather than changing shared token semantics - this does not affect file-writes for the sandbox because the sandbox users themselves do not receive any additional permissions over what the capability SIDs already have. In fact we don't even explicitly grant the sandbox user ACLs anywhere. ## Validation - `cargo build -p codex-windows-sandbox --quiet` - verified the stock `ninja.exe` minimal repro exits normally on host and in the elevated sandbox - verified the same repro still hangs in the unelevated sandbox, which is the intended scope of this change |
||
|
iceweasel-oaiandGitHub
|
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 |
||
|
iceweasel-oaiandGitHub
|
5cac3f896d |
Fix Windows pseudoconsole attribute handling for sandboxed PTY sessions (#20042)
## Summary Fix the Windows sandbox PTY spawn path to pass the pseudoconsole handle value directly into `UpdateProcThreadAttribute`. ## Why Sandboxed `unified_exec` PTY sessions on Windows were failing during child process startup with `0xc0000142` (`STATUS_DLL_INIT_FAILED`). In practice this showed up as PowerShell DLL init popups when the sandboxed background-terminal path tried to launch an interactive shell. The root cause was that we were passing a pointer to a local `isize` variable instead of the pseudoconsole handle value in the form Windows expects for `PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE`. ## Validation - `cargo build -p codex-windows-sandbox --bins` - Reproduced the real sandboxed `codex exec` flow with `windows.sandbox_private_desktop=true` - Verified a `tty=true` interactive session launched through the normal PowerShell wrapper, printed `READY`, accepted follow-up stdin, and exited cleanly - Confirmed no new `0xc0000142` / `Application Popup` events appeared after the successful repro |
||
 Dylan HurdandGitHub
|
3d10ba9f36 |
chore(cli) deprecate --full-auto (#20133)
## Summary Starts the process of getting rid of `--full-auto`, with some concessions: 1. Fully removes the command from the tui, since it just resolves to the default permissions there, and encourages users to use the one-time trust flow if they're not in a trusted repo. 2. Marks the command as deprecated in `codex exec`, in case users are actively relying on this. We'll remove in an upcoming n+X release. 3. Cleans up some of the `codex sandbox` cli logic, to keep supporting legacy sandbox policies for now. This isn't the cleanest setup, but I think it is worthwhile to warn users for one release before hard-removing it. ## Testing - [x] Updated unit tests |
||
|
Michael BolinandGitHub
|
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 |
||
|
Dylan HurdandGitHub
|
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. |
||
|
|
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-oaiandGitHub
|
867820ac7e |
do not attempt ACLs on installed codex dir (#19214)
We used to attempt a read-ACL on the same dir as `codex.exe` to grant the sandbox user the ability to invoke `codex-command-runner.exe`. That worked for the CLI case but it always fails for the installed desktop app. We have another solution already in place that copies `codex-command-runner.exe` to `CODEX_HOME/.sandbox-bin` so we don't even need this anymore. It causes a scary looking error in the logs that is a non-issue and is therefore confusing |
||
|
iceweasel-oaiandGitHub
|
d169bb541e |
use a version-specific suffix for command runner binary in .sandbox-bin (#19180)
we copy `codex-command-runner.exe` into `CODEX_HOME/.sandbox-bin/` so that it can be executed by the sandbox user. We also detect if that version is stale and copy a new one in if so. This can fail when you are running multiple versions of the app - the file in `.sandbox-bin` can look stale because it comes from another app build. This change allows us to have multiple versions in there for different CLI versions, and it fallsback to a `size+mtime` hash in the filename for dev builds that don't report a real CLI version. |
||
|
iceweasel-oaiandGitHub
|
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. |