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,