From 81aee8c86ba2549e53f82ea56f163c3e450fb55a Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 14 Oct 2026 16:25:04 +0000 Subject: [PATCH] Preserve Windows startup variables and MXC child environment defaults (#54027) ## Why Windows MXC needs valid host startup variables, while an empty child environment must remain empty for the SDK to select Windows user-profile defaults. ## What changed - Replace caller overrides of `SystemDrive` and `LOCALAPPDATA` case-insensitively with nonempty Unicode host values, failing before launch if either is unavailable. - Track whether the original child environment was empty, ignoring launcher transport keys. Supply host `SystemRoot` for launcher bootstrap in that case, then clear the bootstrap environment before the SDK selects defaults. ## Testing Extend transport tests to cover host-value replacement and preservation of empty child environments. Strengthen the PowerShell fallback RPC test to require `SystemDrive`, `LOCALAPPDATA`, `USERPROFILE`, and `PATH` in the child. GitOrigin-RevId: a097e1079069ef897ebb5ba5e63546328c5faeaa --- .../tests/exec_process/windows_sandbox.rs | 2 +- codex-rs/mxc-sandbox/README.md | 13 ++++++++-- codex-rs/mxc-sandbox/src/lib.rs | 6 +++++ codex-rs/mxc-sandbox/src/policy_tests.rs | 4 ++++ codex-rs/mxc-sandbox/src/transport.rs | 24 ++++++++++++++++--- codex-rs/mxc-sandbox/src/transport_tests.rs | 9 +++++++ 6 files changed, 52 insertions(+), 6 deletions(-) diff --git a/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs b/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs index 3a5ceb9e3d..f34aa493ea 100644 --- a/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs +++ b/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs @@ -48,7 +48,7 @@ async fn powershell_alias_falls_back_without_primary_runtime_over_rpc( .into_owned(), "-NoProfile".to_owned(), "-Command".to_owned(), - "Write-Output 'fallback-ok'".to_owned(), + "if (!$env:SystemDrive -or !$env:LOCALAPPDATA -or !$env:USERPROFILE -or !$env:PATH) { throw 'missing Windows defaults' }; Write-Output 'fallback-ok'".to_owned(), ], cwd, env_policy: None, diff --git a/codex-rs/mxc-sandbox/README.md b/codex-rs/mxc-sandbox/README.md index 0f1f75117e..c67c0bbb62 100644 --- a/codex-rs/mxc-sandbox/README.md +++ b/codex-rs/mxc-sandbox/README.md @@ -45,6 +45,13 @@ request: the canonical `PermissionProfile`, policy cwd, proxy context, and exact argv. The helper inherits command cwd, environment, and stdio from the shared process path; policy cwd can differ from command cwd. +Windows MXC reserves `SystemDrive` and `LOCALAPPDATA` for the host's validated, +nonempty values in both wrapper and explicit child environments. Caller +overrides are replaced case-insensitively; unavailable host values fail before +launch with a diagnostic naming the variable. +Windows may virtualize the child's `LOCALAPPDATA` into a sandbox directory +beneath the host profile. + The request uses launcher-only environment chunks to leave Windows' command-line budget to the workload. Payloads are limited to 1,000,000 UTF-8 bytes and chunks to 4096 bytes; the helper removes them before native process creation. Use @@ -70,8 +77,10 @@ to 4096 bytes; the helper removes them before native process creation. Use command are not implicitly granted. - The native API represents paths and environment values as Unicode strings. Non-Unicode values fail instead of undergoing lossy conversion. -- An empty child environment uses the Windows user-profile defaults. Nonempty - environments remain explicit. +- An empty child environment still uses the Windows user-profile defaults. + Supplying only two keys would turn it into an incomplete explicit environment, + so the wrapper's bootstrap values are removed before the SDK selects defaults. + Nonempty environments remain explicit except for these two host-owned values. - The upstream runner terminates remaining descendants when the foreground process exits, as well as on cancellation. Both existing Windows backends preserve descendants after normal exit, so detached servers currently lose diff --git a/codex-rs/mxc-sandbox/src/lib.rs b/codex-rs/mxc-sandbox/src/lib.rs index e19d67d08c..5dc92bdeb4 100644 --- a/codex-rs/mxc-sandbox/src/lib.rs +++ b/codex-rs/mxc-sandbox/src/lib.rs @@ -18,6 +18,7 @@ use anyhow::Result; use anyhow::ensure; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_protocol::models::PermissionProfile; +use codex_windows_sandbox::environment_transport; use serde::Deserialize; use serde::Serialize; @@ -46,6 +47,8 @@ struct MxcCommand { sandbox_policy_cwd: PathBuf, managed_network: Option, command: Vec, + #[serde(default)] + default_child_env: bool, } /// Inputs used to build an MXC helper invocation. @@ -80,6 +83,9 @@ pub fn create_command_args(args: CreateMxcCommandArgsParams<'_>) -> Result MxcCommand { sandbox_policy_cwd: cwd.to_owned(), managed_network: None, command: vec!["program.exe".to_owned(), "--arg".to_owned()], + default_child_env: false, } } @@ -173,6 +174,7 @@ fn wrapper_preserves_exact_argv_and_separate_command_cwd() -> Result<()> { "quotes\" and slash\\".to_owned(), ]; let mut env = HashMap::new(); + codex_windows_sandbox::environment_transport::encode("stale", &mut env)?; let wrapped = create_command_args(CreateMxcCommandArgsParams { command: argv.clone(), permission_profile: &profile, @@ -182,6 +184,8 @@ fn wrapper_preserves_exact_argv_and_separate_command_cwd() -> Result<()> { })?; assert_eq!(wrapped, vec![crate::CODEX_WINDOWS_MXC_ARG1]); let parsed = crate::transport::decode(&mut env)?; + assert!(parsed.default_child_env); + assert!(env.is_empty()); assert_eq!( ( &parsed.permissions, diff --git a/codex-rs/mxc-sandbox/src/transport.rs b/codex-rs/mxc-sandbox/src/transport.rs index 9ce957ed61..5cb5d1c954 100644 --- a/codex-rs/mxc-sandbox/src/transport.rs +++ b/codex-rs/mxc-sandbox/src/transport.rs @@ -1,5 +1,5 @@ -//! Bounded launcher-only environment transport for policies too large for -//! Windows command lines. Every transport variable is removed before spawn. +//! Bounded launcher transport. Windows startup variables are host-owned; +//! an original-empty marker preserves the SDK's user-profile defaults. use std::collections::HashMap; @@ -11,6 +11,19 @@ use codex_windows_sandbox::environment_transport; use crate::MxcCommand; pub(super) fn encode(command: &MxcCommand, env: &mut HashMap) -> Result<()> { + // SDK profile defaults depend on the launcher's SystemRoot. + #[cfg(windows)] + for key in ["SystemDrive", "LOCALAPPDATA"] + .into_iter() + .chain(command.default_child_env.then_some("SystemRoot")) + { + let value = std::env::var(key) + .ok() + .filter(|value| !value.is_empty()) + .with_context(|| format!("MXC requires nonempty Unicode host {key}"))?; + env.retain(|name, _| !name.eq_ignore_ascii_case(key)); + env.insert(key.to_owned(), value); + } let payload = serde_json::to_string(command)?; environment_transport::encode(&payload, env) } @@ -18,7 +31,12 @@ pub(super) fn encode(command: &MxcCommand, env: &mut HashMap) -> #[cfg(any(windows, test))] pub(super) fn decode(env: &mut HashMap) -> Result { let payload = environment_transport::decode_and_scrub(env)?; - serde_json::from_str(&payload).context("invalid MXC launcher request") + let command: MxcCommand = + serde_json::from_str(&payload).context("invalid MXC launcher request")?; + if command.default_child_env { + env.clear(); + } + Ok(command) } #[cfg(test)] diff --git a/codex-rs/mxc-sandbox/src/transport_tests.rs b/codex-rs/mxc-sandbox/src/transport_tests.rs index 3a92b96198..984cfb04cd 100644 --- a/codex-rs/mxc-sandbox/src/transport_tests.rs +++ b/codex-rs/mxc-sandbox/src/transport_tests.rs @@ -19,6 +19,7 @@ fn command(args: Vec) -> MxcCommand { sandbox_policy_cwd: PathBuf::from("workspace"), managed_network: None, command: args, + default_child_env: false, } } @@ -31,6 +32,10 @@ fn large_unicode_launch_roundtrips_and_never_reaches_child_environment() -> Resu ]; let expected_env = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); let mut env = expected_env.clone(); + #[cfg(windows)] + env.extend( + ["systemdrive", "LocalAppData"].map(|key| (key.to_owned(), "caller-only".to_owned())), + ); environment_transport::encode("stale", &mut env)?; let request = command(args); encode(&request, &mut env)?; @@ -42,6 +47,10 @@ fn large_unicode_launch_roundtrips_and_never_reaches_child_environment() -> Resu serde_json::to_value(decode(&mut env)?)?, serde_json::to_value(request)? ); + #[cfg(windows)] + for key in ["SystemDrive", "LOCALAPPDATA"] { + assert_eq!(env.remove(key), Some(std::env::var(key)?)); + } assert_eq!(env, expected_env); Ok(()) }