From c8c1ee5da8af5c79ec8433ee88d3d4ebdcfd80fd Mon Sep 17 00:00:00 2001 From: malsamiri-oai Date: Thu, 24 Sep 2026 19:01:17 +0000 Subject: [PATCH] Transport large Windows sandbox launch payloads through the environment (#47919) ## Why Expanded recursive deny lists can exceed the Windows command-line limit, preventing sandbox wrappers and setup helpers from launching. ## What changed - Use chunked environment transport for wrapper and non-elevating setup payloads above 24,000 UTF-16 code units, preserving the existing argument protocol for smaller requests. - Share the transport with the MXC launcher, supporting payloads up to 16 MiB with Unicode-safe chunking and validation. - Keep transport variables out of the workload environment and remove inherited chunks before launching setup helpers. ## Testing Add coverage for transport boundaries, Unicode round trips, malformed payload rejection, environment cleanup, and large payloads crossing Windows process creation. Extend the elevated sandbox integration test with 1,000 nested denied files to verify read restrictions and `AGENTS.md` discovery with a payload exceeding the command-line limit. GitOrigin-RevId: 8ef92fdc7d534143f0eac76036a2173a6f585d08 --- codex-rs/core/tests/suite/windows_sandbox.rs | 38 +++- codex-rs/mxc-sandbox/src/transport.rs | 98 +-------- codex-rs/mxc-sandbox/src/transport_tests.rs | 41 +--- codex-rs/sandboxing/src/manager.rs | 2 +- .../src/environment_transport.rs | 197 ++++++++++++++++++ .../src/environment_transport_tests.rs | 146 +++++++++++++ .../src/launch_environment.rs | 38 ++++ .../src/launch_environment_tests.rs | 16 ++ codex-rs/windows-sandbox-rs/src/lib.rs | 5 + codex-rs/windows-sandbox-rs/src/setup.rs | 11 +- .../src/setup_provisioning.rs | 15 +- codex-rs/windows-sandbox-rs/src/wrapper.rs | 20 +- .../windows-sandbox-rs/src/wrapper_tests.rs | 69 +++++- 13 files changed, 549 insertions(+), 147 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/environment_transport.rs create mode 100644 codex-rs/windows-sandbox-rs/src/environment_transport_tests.rs create mode 100644 codex-rs/windows-sandbox-rs/src/launch_environment.rs create mode 100644 codex-rs/windows-sandbox-rs/src/launch_environment_tests.rs diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index 6587102826..b9d7b085a8 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -655,7 +655,7 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial(codex_home)] -async fn windows_elevated_unified_exec_enforces_managed_deny_reads() -> anyhow::Result<()> { +async fn windows_elevated_unified_exec_enforces_large_recursive_deny_reads() -> anyhow::Result<()> { let codex_home = codex_home_for_windows_sandbox_test("windows-elevated-tool-runtime-deny-read-codex-home")?; let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str()); @@ -711,6 +711,15 @@ async fn windows_elevated_unified_exec_enforces_managed_deny_reads() -> anyhow:: .expect("set managed deny-read permission profile"); }) .with_workspace_setup(|cwd, _fs| async move { + let nested = cwd.join("nested"); + std::fs::create_dir_all(&nested)?; + for index in 0..1_000 { + std::fs::write(nested.join(format!("bulk-{index}.env")), "bulk secret\n")?; + } + std::fs::write( + cwd.join("AGENTS.md"), + "Preserve the large payload integration fixture.\n", + )?; std::fs::write( cwd.join("secret.env"), "glob secret should remain private\n", @@ -724,9 +733,24 @@ async fn windows_elevated_unified_exec_enforces_managed_deny_reads() -> anyhow:: }); let harness = TestCodexHarness::with_builder(builder).await?; + let config = &harness.test().config; + let paths = codex_windows_sandbox::resolve_windows_deny_read_paths( + &config.permissions.file_system_sandbox_policy(), + &config.cwd, + ) + .map_err(anyhow::Error::msg)?; + assert!( + paths.len() >= 1_002, + "recursive deny fixture was not fully expanded" + ); + // The deny list alone exceeds CreateProcessW's limit, so both wrapper and + // setup-refresh must transport the full request outside argv. + assert!(serde_json::to_string(&paths)?.encode_utf16().count() > 32_767); + let command = concat!( "(type secret.env 1>NUL 2>NUL && echo GLOB-READ || echo GLOB-DENIED) & ", "(type exact-secret.txt 1>NUL 2>NUL && echo EXACT-READ || echo EXACT-DENIED) & ", + "(type nested\\bulk-999.env 1>NUL 2>NUL && echo BULK-READ || echo BULK-DENIED) & ", "type public.txt" ); let call_id = "windows-managed-deny-read-exec-command"; @@ -766,6 +790,18 @@ async fn windows_elevated_unified_exec_enforces_managed_deny_reads() -> anyhow:: .await?; let output = harness.function_call_stdout(call_id).await; + assert!( + harness.request_bodies().await.iter().any(|body| body + .to_string() + .contains("Preserve the large payload integration fixture.")), + "sandboxed AGENTS.md discovery must load the fixture instructions" + ); + assert!( + output.contains("BULK-DENIED") + && !output.contains("BULK-READ") + && !output.contains("bulk secret"), + "large recursive deny list must remain enforced: {output:?}" + ); assert!( output.contains("GLOB-DENIED"), "exec_command should reject glob-denied reads: {output:?}" diff --git a/codex-rs/mxc-sandbox/src/transport.rs b/codex-rs/mxc-sandbox/src/transport.rs index 24bdb0a7e5..9ce957ed61 100644 --- a/codex-rs/mxc-sandbox/src/transport.rs +++ b/codex-rs/mxc-sandbox/src/transport.rs @@ -6,110 +6,18 @@ use std::collections::HashMap; #[cfg(any(windows, test))] use anyhow::Context; use anyhow::Result; -use anyhow::ensure; +use codex_windows_sandbox::environment_transport; use crate::MxcCommand; -const PREFIX: &str = "CODEX_MXC_LAUNCH_"; -const COUNT: &str = "CODEX_MXC_LAUNCH_COUNT"; -const LENGTH: &str = "CODEX_MXC_LAUNCH_BYTES"; -const CHUNK_BYTES: usize = 4096; -const MAX_CHUNKS: usize = 256; -const MAX_BYTES: usize = 1_000_000; - -fn is_transport_key(key: &str) -> bool { - key.as_bytes() - .get(..PREFIX.len()) - .is_some_and(|start| start.eq_ignore_ascii_case(PREFIX.as_bytes())) -} - pub(super) fn encode(command: &MxcCommand, env: &mut HashMap) -> Result<()> { let payload = serde_json::to_string(command)?; - ensure!( - payload.len() <= MAX_BYTES, - "MXC launcher payload exceeds {MAX_BYTES} bytes" - ); - let mut chunks = Vec::new(); - let mut start = 0; - while start < payload.len() { - let mut end = (start + CHUNK_BYTES).min(payload.len()); - while !payload.is_char_boundary(end) { - end -= 1; - } - chunks.push(payload[start..end].to_owned()); - start = end; - } - ensure!( - chunks.len() <= MAX_CHUNKS, - "MXC launcher payload has too many chunks" - ); - env.retain(|key, _| !is_transport_key(key)); - env.insert(COUNT.to_owned(), chunks.len().to_string()); - env.insert(LENGTH.to_owned(), payload.len().to_string()); - for (index, chunk) in chunks.into_iter().enumerate() { - env.insert(format!("{PREFIX}{index}"), chunk); - } - Ok(()) + environment_transport::encode(&payload, env) } #[cfg(any(windows, test))] pub(super) fn decode(env: &mut HashMap) -> Result { - let mut encoded = HashMap::new(); - let mut duplicate = false; - let mut oversized = false; - env.retain(|key, value| { - if !is_transport_key(key) { - return true; - } - if encoded.len() >= MAX_CHUNKS + 2 - || key.len() > PREFIX.len() + 32 - || value.len() > CHUNK_BYTES - { - oversized = true; - return false; - } - duplicate |= encoded - .insert(key.to_ascii_uppercase(), std::mem::take(value)) - .is_some(); - false - }); - ensure!(!oversized, "oversized MXC launcher environment"); - ensure!(!duplicate, "duplicate MXC launcher environment variables"); - let count: usize = encoded - .get(COUNT) - .context("missing MXC launcher chunk count")? - .parse()?; - let length: usize = encoded - .get(LENGTH) - .context("missing MXC launcher payload length")? - .parse()?; - ensure!( - (1..=MAX_CHUNKS).contains(&count) && length <= MAX_BYTES, - "invalid MXC launcher payload size" - ); - ensure!( - encoded.len() == count + 2, - "unexpected MXC launcher environment variables" - ); - let mut payload = String::with_capacity(length); - for index in 0..count { - let chunk = encoded - .get(&format!("{PREFIX}{index}")) - .context("missing MXC launcher chunk")?; - ensure!( - !chunk.is_empty() && chunk.len() <= CHUNK_BYTES, - "invalid MXC launcher chunk size" - ); - ensure!( - payload.len() + chunk.len() <= length, - "MXC launcher payload length mismatch" - ); - payload.push_str(chunk); - } - ensure!( - payload.len() == length, - "MXC launcher payload length mismatch" - ); + let payload = environment_transport::decode_and_scrub(env)?; serde_json::from_str(&payload).context("invalid MXC launcher request") } diff --git a/codex-rs/mxc-sandbox/src/transport_tests.rs b/codex-rs/mxc-sandbox/src/transport_tests.rs index 1c8a520963..3a92b96198 100644 --- a/codex-rs/mxc-sandbox/src/transport_tests.rs +++ b/codex-rs/mxc-sandbox/src/transport_tests.rs @@ -5,15 +5,11 @@ use std::path::PathBuf; use anyhow::Result; use codex_protocol::models::PermissionProfile; +use codex_windows_sandbox::environment_transport; use pretty_assertions::assert_eq; use crate::MxcCommand; -use super::CHUNK_BYTES; -use super::COUNT; -use super::LENGTH; -use super::MAX_BYTES; -use super::PREFIX; use super::decode; use super::encode; @@ -35,13 +31,12 @@ 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(); - env.insert("codex_mxc_launch_999".to_owned(), "spoofed".to_owned()); + environment_transport::encode("stale", &mut env)?; let request = command(args); encode(&request, &mut env)?; - assert!(!env.contains_key("codex_mxc_launch_999")); assert!( env.values() - .all(|value| value.encode_utf16().count() < 8192) + .all(|value| value.encode_utf16().count() < 32_767) ); assert_eq!( serde_json::to_value(decode(&mut env)?)?, @@ -50,33 +45,3 @@ fn large_unicode_launch_roundtrips_and_never_reaches_child_environment() -> Resu assert_eq!(env, expected_env); Ok(()) } - -#[test] -fn malformed_transport_is_rejected_and_removed() -> Result<()> { - let expected_env = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); - let mut valid = expected_env.clone(); - encode(&command(vec!["program.exe".to_owned()]), &mut valid)?; - for (key, value) in [ - (COUNT.to_owned(), "257".to_owned()), - (LENGTH.to_owned(), "0".to_owned()), - (format!("{PREFIX}0"), "x".repeat(CHUNK_BYTES + 1)), - (COUNT.to_ascii_lowercase(), "1".to_owned()), - ] { - let mut env = valid.clone(); - env.insert(key, value); - assert!(decode(&mut env).is_err()); - assert_eq!(env, expected_env); - } - valid.remove(&format!("{PREFIX}0")); - assert!(decode(&mut valid).is_err()); - assert_eq!(valid, expected_env); - Ok(()) -} - -#[test] -fn oversized_payload_fails_before_modifying_launcher_environment() { - let expected = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); - let mut env = expected.clone(); - assert!(encode(&command(vec!["x".repeat(MAX_BYTES)]), &mut env).is_err()); - assert_eq!(env, expected); -} diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 82b9888cfc..783cef5e04 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -665,7 +665,7 @@ fn wrap_windows_sandbox_exec_request_for_direct_spawn( inner_command, &native_cwd, workspace_roots, - &request.env, + &mut request.env, &request.permission_profile, request.windows_sandbox_level, proxy_enforced, diff --git a/codex-rs/windows-sandbox-rs/src/environment_transport.rs b/codex-rs/windows-sandbox-rs/src/environment_transport.rs new file mode 100644 index 0000000000..e6088174cf --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/environment_transport.rs @@ -0,0 +1,197 @@ +//! Bounded launcher-only environment transport for payloads too large for +//! Windows command lines. + +use std::collections::HashMap; +use std::ffi::OsStr; + +use anyhow::Context; +use anyhow::Result; +use anyhow::ensure; + +const PREFIX: &str = "CODEX_SANDBOX_LAUNCH_"; +const COUNT: &str = "CODEX_SANDBOX_LAUNCH_COUNT"; +const LENGTH: &str = "CODEX_SANDBOX_LAUNCH_BYTES"; +const CHUNK_BYTES: usize = 16 * 1024; +const MAX_BYTES: usize = 16 * 1024 * 1024; +const MAX_CHUNKS: usize = MAX_BYTES / (CHUNK_BYTES - 3) + 1; + +/// Whether `key` belongs to the shared sandbox launch transport namespace. +pub fn is_key(key: &OsStr) -> bool { + environment_key_starts_with(key, PREFIX) +} + +/// Split a UTF-8 payload into bounded environment variables. +/// +/// Existing transport variables are removed only after the complete payload +/// has passed validation. +pub fn encode(payload: &str, env: &mut HashMap) -> Result<()> { + ensure!( + !payload.is_empty() && payload.len() <= MAX_BYTES, + "sandbox launch payload must contain 1..={MAX_BYTES} bytes" + ); + ensure!( + !payload.contains('\0'), + "sandbox launch payload contains a NUL" + ); + let mut chunks = Vec::new(); + let mut start = 0; + while start < payload.len() { + let mut end = (start + CHUNK_BYTES).min(payload.len()); + while !payload.is_char_boundary(end) { + end -= 1; + } + chunks.push(payload[start..end].to_owned()); + start = end; + } + ensure!( + chunks.len() <= MAX_CHUNKS, + "sandbox launch payload has too many chunks" + ); + env.retain(|key, _| !is_key(key.as_ref())); + env.insert(COUNT.to_owned(), chunks.len().to_string()); + env.insert(LENGTH.to_owned(), payload.len().to_string()); + for (index, chunk) in chunks.into_iter().enumerate() { + env.insert(format!("{PREFIX}{index}"), chunk); + } + Ok(()) +} + +/// Reconstruct a payload without inspecting unrelated non-Unicode variables. +pub fn decode, V: AsRef>( + env: impl IntoIterator, +) -> Result { + let mut encoded = HashMap::new(); + for (key, value) in env { + let key = key.as_ref(); + if !is_key(key) { + continue; + } + let key = canonical_key(key)?; + let value = value + .as_ref() + .to_str() + .context("non-Unicode sandbox launch environment value")?; + ensure!( + encoded.len() < MAX_CHUNKS + 2 + && key.len() <= PREFIX.len() + 32 + && value.len() <= CHUNK_BYTES, + "oversized sandbox launch environment" + ); + ensure!( + encoded.insert(key, value.to_owned()).is_none(), + "duplicate sandbox launch environment variables" + ); + } + let count: usize = encoded + .get(COUNT) + .context("missing sandbox launch chunk count")? + .parse()?; + let length: usize = encoded + .get(LENGTH) + .context("missing sandbox launch payload length")? + .parse()?; + ensure!( + (1..=MAX_CHUNKS).contains(&count) && length <= MAX_BYTES, + "invalid sandbox launch payload size" + ); + ensure!( + encoded.len() == count + 2, + "unexpected sandbox launch environment variables" + ); + let mut payload = String::with_capacity(length); + for index in 0..count { + let chunk = encoded + .get(&format!("{PREFIX}{index}")) + .context("missing sandbox launch chunk")?; + ensure!(!chunk.is_empty(), "invalid sandbox launch chunk size"); + ensure!( + payload.len() + chunk.len() <= length, + "sandbox launch payload length mismatch" + ); + payload.push_str(chunk); + } + ensure!( + payload.len() == length, + "sandbox launch payload length mismatch" + ); + Ok(payload) +} + +/// Remove every transport variable from `env`, even when decoding fails. +pub fn decode_and_scrub(env: &mut HashMap) -> Result { + let payload = decode(env.iter()); + env.retain(|key, _| !is_key(key.as_ref())); + payload +} + +fn canonical_key(key: &OsStr) -> Result { + if environment_key_eq(key, COUNT) { + return Ok(COUNT.to_owned()); + } + if environment_key_eq(key, LENGTH) { + return Ok(LENGTH.to_owned()); + } + let key = key + .to_str() + .context("non-Unicode sandbox launch environment key")?; + Ok(format!( + "{PREFIX}{}", + key.chars().skip(PREFIX.len()).collect::() + )) +} + +#[cfg(windows)] +fn environment_key_eq(key: &OsStr, expected: &str) -> bool { + use std::os::windows::ffi::OsStrExt; + #[link(name = "kernel32")] + unsafe extern "system" { + fn CompareStringOrdinal( + left: *const u16, + left_len: i32, + right: *const u16, + right_len: i32, + ignore_case: i32, + ) -> i32; + } + let key: Vec = key.encode_wide().collect(); + let expected: Vec = expected.encode_utf16().collect(); + key.len() == expected.len() + && unsafe { + CompareStringOrdinal( + key.as_ptr(), + key.len() as i32, + expected.as_ptr(), + expected.len() as i32, + /*ignore_case*/ 1, + ) == 2 + } +} + +#[cfg(not(windows))] +fn environment_key_eq(key: &OsStr, expected: &str) -> bool { + key.as_encoded_bytes() + .eq_ignore_ascii_case(expected.as_bytes()) +} + +#[cfg(windows)] +fn environment_key_starts_with(key: &OsStr, prefix: &str) -> bool { + use std::ffi::OsString; + use std::os::windows::ffi::OsStrExt; + use std::os::windows::ffi::OsStringExt; + let key_prefix: Vec = key + .encode_wide() + .take(prefix.encode_utf16().count()) + .collect(); + environment_key_eq(&OsString::from_wide(&key_prefix), prefix) +} + +#[cfg(not(windows))] +fn environment_key_starts_with(key: &OsStr, prefix: &str) -> bool { + key.as_encoded_bytes() + .get(..prefix.len()) + .is_some_and(|start| start.eq_ignore_ascii_case(prefix.as_bytes())) +} + +#[cfg(test)] +#[path = "environment_transport_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/environment_transport_tests.rs b/codex-rs/windows-sandbox-rs/src/environment_transport_tests.rs new file mode 100644 index 0000000000..5cfe7f0f14 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/environment_transport_tests.rs @@ -0,0 +1,146 @@ +//! Shared transport roundtrips, validation, and environment cleanup. + +use std::collections::HashMap; + +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn unicode_payload_roundtrips_and_stale_chunks_are_replaced() { + let payload = "a🧊".repeat(500_000); + let expected = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); + let mut env = expected.clone(); + env.insert( + format!("{}999", PREFIX.to_ascii_lowercase()), + "spoofed".to_owned(), + ); + encode(&payload, &mut env).unwrap(); + assert!(!env.contains_key(&format!("{}999", PREFIX.to_ascii_lowercase()))); + assert_eq!(decode_and_scrub(&mut env).unwrap(), payload); + assert_eq!(env, expected); +} + +#[test] +fn malformed_transport_is_rejected_and_scrubbed() { + let expected = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); + let mut valid = expected.clone(); + encode("payload", &mut valid).unwrap(); + for (key, value) in [ + (COUNT.to_owned(), (MAX_CHUNKS + 1).to_string()), + (LENGTH.to_owned(), "0".to_owned()), + (LENGTH.to_owned(), (MAX_BYTES + 1).to_string()), + (format!("{PREFIX}0"), "x".repeat(CHUNK_BYTES + 1)), + (COUNT.to_ascii_lowercase(), "1".to_owned()), + (format!("{PREFIX}unexpected"), "x".to_owned()), + ] { + let mut env = valid.clone(); + env.insert(key, value); + assert!(decode_and_scrub(&mut env).is_err()); + assert_eq!(env, expected); + } + valid.remove(&format!("{PREFIX}0")); + assert!(decode_and_scrub(&mut valid).is_err()); + assert_eq!(valid, expected); + assert!(decode(HashMap::::new()).is_err()); +} + +#[test] +fn rejected_payload_preserves_environment() { + let expected = HashMap::from([("CUSTOM".to_owned(), "value".to_owned())]); + for payload in [ + String::new(), + "x".repeat(MAX_BYTES + 1), + "bad\0payload".into(), + ] { + let mut env = expected.clone(); + assert!(encode(&payload, &mut env).is_err()); + assert_eq!(env, expected); + } +} + +#[cfg(any(windows, unix))] +#[test] +fn native_environment_ignores_unrelated_non_unicode_and_rejects_invalid_transport() { + use std::ffi::OsString; + #[cfg(unix)] + use std::os::unix::ffi::OsStringExt; + #[cfg(windows)] + use std::os::windows::ffi::OsStringExt; + #[cfg(windows)] + let invalid = OsString::from_wide(&[0xd800]); + #[cfg(unix)] + let invalid = OsString::from_vec(vec![0xff]); + let mut env = vec![ + (OsString::from(COUNT), OsString::from("1")), + (OsString::from(LENGTH), OsString::from("2")), + (OsString::from(format!("{PREFIX}0")), OsString::from("ok")), + (OsString::from("unrelated"), invalid.clone()), + (invalid.clone(), OsString::from("unrelated")), + ]; + assert_eq!(decode(env.clone()).unwrap(), "ok"); + env.push((OsString::from(format!("{PREFIX}0")), invalid.clone())); + assert!(decode(env).is_err()); + let mut invalid_key = OsString::from(PREFIX); + invalid_key.push(invalid); + assert!(decode([(invalid_key, OsString::from("bad"))]).is_err()); +} + +#[cfg(windows)] +#[test] +fn windows_equivalent_names_are_scrubbed_decoded_and_deduplicated() { + let alias_prefix = PREFIX.to_ascii_lowercase(); + let alias_count = format!("{alias_prefix}count"); + let alias_length = format!("{alias_prefix}bytes"); + let mut env = HashMap::from([ + (alias_count.clone(), "1".to_owned()), + (alias_length.clone(), "2".to_owned()), + (format!("{alias_prefix}0"), "ok".to_owned()), + ]); + assert_eq!(decode(env.clone()).unwrap(), "ok"); + env.insert(COUNT.to_owned(), "1".to_owned()); + assert!(decode(env).is_err()); + + let mut stale = HashMap::from([ + (alias_count, "1".to_owned()), + (alias_length, "2".to_owned()), + (format!("{alias_prefix}999"), "stale".to_owned()), + ]); + encode("ok", &mut stale).unwrap(); + assert_eq!(stale.len(), 3); +} + +// Exercise a real Unicode Windows environment block, without provisioning or ACL changes. +#[cfg(windows)] +#[test] +fn large_payload_survives_windows_process_creation() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + let payload = r#"C:\fixture\秘密\file-😀.txt\""#.repeat(150_000); + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + command + .arg("--exact") + .arg("environment_transport::tests::environment_child"); + let mut env = HashMap::new(); + encode(&payload, &mut env).unwrap(); + // Command preserves the spelling of a Windows-equivalent inherited key. + command.env(LENGTH.replace('S', "s"), "stale"); + command.env("unrelated", OsString::from_wide(&[0xd800])); + let output = command.envs(env).arg("--nocapture").output().unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(String::from_utf8_lossy(&output.stdout).contains("transport-child-ok")); +} + +#[cfg(windows)] +#[test] +fn environment_child() { + if std::env::var_os(LENGTH).is_none() { + return; + } + let payload = r#"C:\fixture\秘密\file-😀.txt\""#.repeat(150_000); + assert_eq!(decode(std::env::vars_os()).unwrap(), payload); + println!("transport-child-ok"); +} diff --git a/codex-rs/windows-sandbox-rs/src/launch_environment.rs b/codex-rs/windows-sandbox-rs/src/launch_environment.rs new file mode 100644 index 0000000000..7af153722d --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/launch_environment.rs @@ -0,0 +1,38 @@ +//! Invocation-scoped transport for Windows sandbox payloads that exceed argv. +//! Chunks stay below the per-variable limit; no process-global environment is mutated. + +#[cfg(windows)] +use anyhow::Result; + +#[cfg(windows)] +use crate::environment_transport; + +#[cfg(windows)] +pub(crate) const ARG: &str = "--launch-payload-env"; + +/// Keep small launches compatible with the existing argv protocol. +pub(crate) fn needs_environment(payload: &str) -> bool { + payload.encode_utf16().count() > 24_000 +} + +#[cfg(windows)] +pub(crate) fn configure_command(command: &mut std::process::Command, payload: &str) -> Result<()> { + // A wrapper may itself carry a payload. Never inherit its chunks into setup. + for (key, _) in std::env::vars_os() { + if environment_transport::is_key(&key) { + command.env_remove(key); + } + } + if needs_environment(payload) { + let mut env = std::collections::HashMap::new(); + environment_transport::encode(payload, &mut env)?; + command.arg(ARG).envs(env); + } else { + command.arg(payload); + } + Ok(()) +} + +#[cfg(test)] +#[path = "launch_environment_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/launch_environment_tests.rs b/codex-rs/windows-sandbox-rs/src/launch_environment_tests.rs new file mode 100644 index 0000000000..d2dc558e77 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/launch_environment_tests.rs @@ -0,0 +1,16 @@ +//! Select the launch transport using Windows command-line UTF-16 units. + +use super::needs_environment; +use pretty_assertions::assert_eq; + +#[test] +fn only_large_payloads_use_environment_transport() { + for (payload, expected) in [ + ("a".repeat(24_000), false), + ("a".repeat(24_001), true), + ("🧊".repeat(12_000), false), + ("🧊".repeat(12_001), true), + ] { + assert_eq!(needs_environment(&payload), expected); + } +} diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index fa9c07d640..84767d1bc6 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -5,6 +5,11 @@ #[cfg(any(target_os = "windows", test))] mod ssh_config_dependencies; +#[doc(hidden)] +pub mod environment_transport; +#[cfg(any(windows, test))] +mod launch_environment; + use std::fmt; use std::sync::Arc; diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index ad9ed0dcd3..c40a56c996 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -387,8 +387,8 @@ fn run_setup_refresh_payload(b64: &str, codex_home: &Path) -> Result<()> { }; // Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC. let mut cmd = Command::new(&exe); - cmd.arg(b64) - .stdin(Stdio::null()) + crate::launch_environment::configure_command(&mut cmd, b64)?; + cmd.stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); let cwd = std::env::current_dir().unwrap_or_else(|_| codex_home.to_path_buf()); @@ -977,17 +977,16 @@ fn run_setup_exe_payload( }; if !needs_elevation { + let mut command = Command::new(&exe); + crate::launch_environment::configure_command(&mut command, payload_b64)?; let status = if retained_handles.is_empty() { - Command::new(&exe) - .arg(payload_b64) + command .creation_flags(0x08000000) // CREATE_NO_WINDOW .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) .status() } else { - let mut command = Command::new(&exe); - command.arg(payload_b64); crate::setup_launch::spawn_with_retained_handles(&mut command, retained_handles) .and_then(|mut child| child.wait()) } diff --git a/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs b/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs index 6b003f09d6..f60b1d9d5c 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_provisioning.rs @@ -211,8 +211,9 @@ fn spawn_read_acl_helper(payload: &Payload, _log: &mut dyn Write) -> Result<()> let payload_json = serde_json::to_vec(&read_payload)?; let payload_b64 = BASE64.encode(payload_json); let exe = std::env::current_exe().context("locate setup helper")?; - Command::new(&exe) - .arg(payload_b64) + let mut command = Command::new(&exe); + crate::launch_environment::configure_command(&mut command, &payload_b64)?; + command .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -514,6 +515,16 @@ fn real_main(setup_mode: &mut Option) -> Result<()> { ))); } let payload_b64 = args.remove(1); + let payload_b64 = if payload_b64 == crate::launch_environment::ARG { + crate::environment_transport::decode(std::env::vars_os()).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperRequestArgsFailed, + format!("failed to read payload environment: {err}"), + )) + })? + } else { + payload_b64 + }; let payload_json = BASE64.decode(payload_b64).map_err(|err| { anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperRequestArgsFailed, diff --git a/codex-rs/windows-sandbox-rs/src/wrapper.rs b/codex-rs/windows-sandbox-rs/src/wrapper.rs index e6566f77b8..c449505f45 100644 --- a/codex-rs/windows-sandbox-rs/src/wrapper.rs +++ b/codex-rs/windows-sandbox-rs/src/wrapper.rs @@ -2,7 +2,7 @@ //! //! This gives direct-spawn callers an argv-shaped Windows sandbox launcher, //! analogous to the macOS seatbelt and Linux sandbox wrapper paths. The wrapper -//! parses sandbox metadata from argv, launches the requested inner command in a +//! parses sandbox metadata from argv or a launch environment, launches the inner command in a //! Windows sandbox session, and forwards stdio to that inner command. use std::collections::HashMap; @@ -53,7 +53,7 @@ pub fn create_windows_sandbox_command_args_for_permission_profile( command: Vec, command_cwd: &AbsolutePathBuf, workspace_roots: &[AbsolutePathBuf], - env_map: &HashMap, + env_map: &mut HashMap, permission_profile: &PermissionProfile, windows_sandbox_level: WindowsSandboxLevel, proxy_enforced: bool, @@ -66,6 +66,8 @@ pub fn create_windows_sandbox_command_args_for_permission_profile( deny_write_paths_override: &[AbsolutePathBuf], codex_home: &Path, ) -> Result> { + // Launcher-only variables must not enter the serialized workload environment. + env_map.retain(|key, _| !crate::environment_transport::is_key(key.as_ref())); let permission_profile_json = serde_json::to_string(permission_profile) .unwrap_or_else(|err| panic!("failed to serialize permission profile: {err}")); let env_json = serde_json::to_string(env_map) @@ -241,6 +243,14 @@ pub fn create_windows_sandbox_command_args_for_permission_profile( } args.push("--".to_string()); args.extend(command); + let payload = serde_json::to_string(&args[1..])?; + if crate::launch_environment::needs_environment(&payload) { + crate::environment_transport::encode(&payload, env_map)?; + return Ok(vec![ + CODEX_WINDOWS_SANDBOX_ARG1.to_string(), + crate::launch_environment::ARG.to_string(), + ]); + } Ok(args) } @@ -275,6 +285,12 @@ pub fn run_windows_sandbox_wrapper_main() -> ! { } async fn run_windows_sandbox_wrapper_args(args: Vec) -> Result { + let args = if args == [crate::launch_environment::ARG] { + serde_json::from_str(&crate::environment_transport::decode(std::env::vars_os())?) + .context("failed to parse Windows sandbox launch arguments")? + } else { + args + }; let request = parse_windows_sandbox_wrapper_args(args)?; run_windows_sandbox_wrapper_request(request).await } diff --git a/codex-rs/windows-sandbox-rs/src/wrapper_tests.rs b/codex-rs/windows-sandbox-rs/src/wrapper_tests.rs index 2531654f8e..e68299e068 100644 --- a/codex-rs/windows-sandbox-rs/src/wrapper_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/wrapper_tests.rs @@ -26,6 +26,71 @@ use super::WRITE_ROOTS_JSON_FLAG; use super::create_windows_sandbox_command_args_for_permission_profile; use super::parse_windows_sandbox_wrapper_args; +#[test] +fn large_deny_list_uses_environment_without_changing_the_request() { + let temp = tempfile::tempdir().expect("tempdir"); + let cwd = AbsolutePathBuf::from_absolute_path(temp.path()).expect("absolute command cwd"); + let paths = (0..1000) + .map(|index| { + AbsolutePathBuf::from_absolute_path(format!(r"C:\private\nested\file-{index}.txt")) + .unwrap() + }) + .collect::>(); + let expected_env = HashMap::from([("Path".to_string(), r"C:\Windows\System32".to_string())]); + let mut env = expected_env.clone(); + crate::environment_transport::encode("must not reach workload", &mut env).unwrap(); + let profile = PermissionProfile::read_only(); + let command = vec![ + "codex.exe".to_string(), + "--codex-run-as-fs-helper".to_string(), + ]; + let args = create_windows_sandbox_command_args_for_permission_profile( + command.clone(), + &cwd, + std::slice::from_ref(&cwd), + &mut env, + &profile, + WindowsSandboxLevel::RestrictedToken, + /*proxy_enforced*/ true, + /*network_proxy_restricting_sid*/ Some("S-1-5-21-100-200-300-400"), + crate::WindowsSandboxProxySettingsMode::Preserve, + /*read_roots_override*/ None, + /*read_roots_include_platform_defaults*/ true, + /*write_roots_override*/ None, + &paths, + &paths, + temp.path(), + ) + .unwrap(); + assert_eq!( + args, + vec![CODEX_WINDOWS_SANDBOX_ARG1, crate::launch_environment::ARG] + ); + let decoded = crate::environment_transport::decode(env).unwrap(); + let parsed = + parse_windows_sandbox_wrapper_args(serde_json::from_str(&decoded).unwrap()).unwrap(); + let desktop = crate::desktop::LaunchDesktop::open_private(&parsed.private_desktop_name) + .expect("wrapper must receive a live private desktop"); + drop(desktop); + assert_eq!(parsed.deny_read_paths_override, paths); + assert_eq!(parsed.deny_write_paths_override, paths); + assert_eq!(parsed.env_map, expected_env); + assert_eq!(parsed.permission_profile, profile); + assert_eq!(parsed.command, command); + assert_eq!(parsed.command_cwd, cwd); + assert_eq!(parsed.workspace_roots, vec![cwd]); + assert_eq!(parsed.proxy_enforced, true); + assert_eq!(parsed.read_roots_include_platform_defaults, true); + assert_eq!( + parsed.network_proxy_restricting_sid.as_deref(), + Some("S-1-5-21-100-200-300-400") + ); + assert_eq!( + parsed.proxy_settings_mode, + crate::WindowsSandboxProxySettingsMode::Preserve + ); +} + #[test] fn windows_wrapper_args_round_trip() { let temp = tempfile::tempdir().expect("tempdir"); @@ -36,7 +101,7 @@ fn windows_wrapper_args_round_trip() { AbsolutePathBuf::from_absolute_path(Path::new(r"D:\other-workspace")) .expect("absolute workspace root"), ]; - let env = HashMap::from([("Path".to_string(), r"C:\Windows\System32".to_string())]); + let mut env = HashMap::from([("Path".to_string(), r"C:\Windows\System32".to_string())]); let permission_profile = PermissionProfile::read_only(); let read_roots_override = vec![PathBuf::from(r"C:\read")]; let write_roots_override = vec![PathBuf::from(r"C:\write")]; @@ -56,7 +121,7 @@ fn windows_wrapper_args_round_trip() { ], &command_cwd, workspace_roots.as_slice(), - &env, + &mut env, &permission_profile, WindowsSandboxLevel::RestrictedToken, /*proxy_enforced*/ true,