diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 4e58e4de96..58ab110223 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -30,7 +30,8 @@ use crate::rpc::invalid_request; const FS_HELPER_ENV_ALLOWLIST: &[&str] = &["PATH", "TMPDIR", "TMP", "TEMP"]; #[cfg(target_os = "windows")] -const WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST: &[&str] = &["USERNAME", "USERPROFILE"]; +#[path = "fs_sandbox_windows.rs"] +mod windows; #[cfg(debug_assertions)] const FS_HELPER_BAZEL_BWRAP_ENV_ALLOWLIST: &[&str] = &[ "CARGO_BIN_EXE_bwrap", @@ -169,7 +170,7 @@ impl FileSystemSandboxRunner { #[cfg(target_os = "windows")] if command.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken { let request_file = - wrap_windows_sandbox_command(&mut command, &helper_paths.windows_wrapper)?; + windows::wrap_sandbox_exec_request(&mut command, &helper_paths.windows_wrapper)?; return Ok(PreparedFsHelperCommand::with_windows_wrapper_request_file( command, request_file, @@ -188,7 +189,7 @@ struct FsHelperLaunchPaths { struct PreparedFsHelperCommand { request: SandboxExecRequest, #[cfg(target_os = "windows")] - _windows_wrapper_request_file: Option, + _windows_wrapper_request_file: Option, } impl PreparedFsHelperCommand { @@ -203,7 +204,7 @@ impl PreparedFsHelperCommand { #[cfg(target_os = "windows")] fn with_windows_wrapper_request_file( request: SandboxExecRequest, - request_file: WindowsSandboxWrapperRequestFile, + request_file: windows::WindowsSandboxWrapperRequestFile, ) -> Self { Self { request, @@ -421,132 +422,6 @@ fn spawn_child_command( command.spawn().map_err(io_error) } -#[cfg(target_os = "windows")] -fn wrap_windows_sandbox_command( - request: &mut SandboxExecRequest, - helper: &AbsolutePathBuf, -) -> Result { - let codex_home = codex_utils_home_dir::find_codex_home().map_err(|err| { - internal_error(format!( - "windows fs sandbox helper failed to resolve CODEX_HOME: {err}" - )) - })?; - let sandboxed_env = request.env.clone(); - let wrapper_request = - codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile( - std::mem::take(&mut request.command), - request.cwd.clone(), - sandboxed_env, - request.permission_profile.clone(), - request.windows_sandbox_level, - request.windows_sandbox_private_desktop, - codex_home.to_path_buf(), - ); - let request_file = - WindowsSandboxWrapperRequestFile::create(codex_home.as_path(), &wrapper_request)?; - let mut args = codex_windows_sandbox::create_windows_sandbox_command_args_for_request_file( - request_file.path.as_path(), - ); - request.command = Vec::with_capacity(1 + args.len()); - request - .command - .push(helper.as_path().to_string_lossy().into_owned()); - request.command.append(&mut args); - request.sandbox = codex_sandboxing::SandboxType::None; - request.arg0 = None; - add_windows_wrapper_setup_env(&mut request.env); - Ok(request_file) -} - -#[cfg(target_os = "windows")] -fn add_windows_wrapper_setup_env(env: &mut HashMap) { - add_windows_wrapper_setup_env_from_vars(env, std::env::vars_os()); -} - -#[cfg(target_os = "windows")] -fn add_windows_wrapper_setup_env_from_vars( - env: &mut HashMap, - vars: impl IntoIterator, -) { - for (key, value) in vars { - let key = key.to_string_lossy().into_owned(); - if !WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST - .iter() - .any(|allowed| key.eq_ignore_ascii_case(allowed)) - { - continue; - } - if env - .keys() - .any(|existing| existing.eq_ignore_ascii_case(&key)) - { - continue; - } - env.insert(key, value.to_string_lossy().into_owned()); - } -} - -#[cfg(target_os = "windows")] -struct WindowsSandboxWrapperRequestFile { - path: std::path::PathBuf, -} - -#[cfg(target_os = "windows")] -impl WindowsSandboxWrapperRequestFile { - fn create( - codex_home: &std::path::Path, - request: &codex_windows_sandbox::WindowsSandboxWrapperRequest, - ) -> Result { - let request_dir = windows_sandbox_wrapper_request_dir(codex_home); - std::fs::create_dir_all(&request_dir).map_err(|err| { - internal_error(format!( - "failed to create windows fs sandbox wrapper request dir {}: {err}", - request_dir.display() - )) - })?; - codex_windows_sandbox::ensure_current_user_cleanup_access(&request_dir).map_err(|err| { - internal_error(format!( - "failed to grant cleanup access to windows fs sandbox wrapper request dir {}: {err}", - request_dir.display() - )) - })?; - let path = request_dir.join(format!( - "fs-helper-wrapper-request-{}.json", - uuid::Uuid::new_v4() - )); - let request_json = serde_json::to_vec(request).map_err(json_error)?; - let mut file = std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&path) - .map_err(|err| { - internal_error(format!( - "failed to create windows fs sandbox wrapper request file {}: {err}", - path.display() - )) - })?; - std::io::Write::write_all(&mut file, &request_json).map_err(|err| { - internal_error(format!( - "failed to write windows fs sandbox wrapper request file {}: {err}", - path.display() - )) - })?; - Ok(Self { path }) - } -} - -#[cfg(target_os = "windows")] -fn windows_sandbox_wrapper_request_dir(codex_home: &std::path::Path) -> std::path::PathBuf { - codex_windows_sandbox::sandbox_secrets_dir(codex_home).join("wrapper-requests") -} - -#[cfg(target_os = "windows")] -impl Drop for WindowsSandboxWrapperRequestFile { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.path); - } -} - fn io_error(err: std::io::Error) -> JSONRPCErrorError { internal_error(err.to_string()) } @@ -577,15 +452,11 @@ mod tests { use super::FileSystemSandboxRunner; use super::FsHelperLaunchPaths; use super::add_helper_runtime_permissions; - #[cfg(target_os = "windows")] - use super::add_windows_wrapper_setup_env_from_vars; use super::helper_env; use super::helper_env_from_vars; use super::helper_env_key_is_allowed; use super::helper_read_roots; use super::sandbox_cwd; - #[cfg(target_os = "windows")] - use super::windows_sandbox_wrapper_request_dir; #[test] fn helper_permissions_enable_minimal_reads_for_restricted_profile() { @@ -725,43 +596,6 @@ mod tests { ); } - #[cfg(target_os = "windows")] - #[test] - fn windows_wrapper_setup_env_preserves_only_setup_identity() { - let mut env = HashMap::from([("PATH".to_string(), r"C:\Windows\System32".to_string())]); - - add_windows_wrapper_setup_env_from_vars( - &mut env, - [ - ("USERNAME", "alice"), - ("USERPROFILE", r"C:\Users\alice"), - ("OPENAI_API_KEY", "secret"), - ] - .map(|(key, value)| (OsString::from(key), OsString::from(value))), - ); - - assert_eq!( - env, - HashMap::from([ - ("PATH".to_string(), r"C:\Windows\System32".to_string()), - ("USERNAME".to_string(), "alice".to_string()), - ("USERPROFILE".to_string(), r"C:\Users\alice".to_string()), - ]) - ); - } - - #[cfg(target_os = "windows")] - #[test] - fn windows_wrapper_request_dir_uses_sandbox_secrets() { - let codex_home = std::env::temp_dir().join("codex-home"); - let sandbox_dir = codex_windows_sandbox::sandbox_dir(&codex_home); - let secrets_dir = codex_windows_sandbox::sandbox_secrets_dir(&codex_home); - let request_dir = windows_sandbox_wrapper_request_dir(&codex_home); - - assert!(!request_dir.starts_with(sandbox_dir)); - assert!(request_dir.starts_with(secrets_dir)); - } - #[test] fn sandbox_exec_request_carries_helper_env() { let Some((path_key, path)) = std::env::vars_os().find(|(key, _)| { diff --git a/codex-rs/exec-server/src/fs_sandbox_windows.rs b/codex-rs/exec-server/src/fs_sandbox_windows.rs new file mode 100644 index 0000000000..184cf78494 --- /dev/null +++ b/codex-rs/exec-server/src/fs_sandbox_windows.rs @@ -0,0 +1,212 @@ +use std::collections::HashMap; +use std::io::Write; + +use codex_app_server_protocol::JSONRPCErrorError; +use codex_sandboxing::SandboxExecRequest; +use codex_utils_absolute_path::AbsolutePathBuf; + +use crate::rpc::internal_error; + +const WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST: &[&str] = &["USERNAME", "USERPROFILE"]; + +pub(super) fn wrap_sandbox_exec_request( + request: &mut SandboxExecRequest, + helper: &AbsolutePathBuf, +) -> Result { + let codex_home = codex_utils_home_dir::find_codex_home().map_err(|err| { + internal_error(format!( + "windows fs sandbox helper failed to resolve CODEX_HOME: {err}" + )) + })?; + let sandboxed_env = request.env.clone(); + let wrapper_request = + codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile( + std::mem::take(&mut request.command), + request.cwd.clone(), + sandboxed_env, + request.permission_profile.clone(), + request.windows_sandbox_level, + request.windows_sandbox_private_desktop, + codex_home.to_path_buf(), + ); + let request_file = + WindowsSandboxWrapperRequestFile::create(codex_home.as_path(), &wrapper_request)?; + let mut args = codex_windows_sandbox::create_windows_sandbox_command_args_for_request_file( + request_file.path.as_path(), + ); + request.command = Vec::with_capacity(1 + args.len()); + request + .command + .push(helper.as_path().to_string_lossy().into_owned()); + request.command.append(&mut args); + request.sandbox = codex_sandboxing::SandboxType::None; + request.arg0 = None; + add_wrapper_setup_env(&mut request.env); + Ok(request_file) +} + +fn add_wrapper_setup_env(env: &mut HashMap) { + add_wrapper_setup_env_from_vars(env, std::env::vars_os()); +} + +fn add_wrapper_setup_env_from_vars( + env: &mut HashMap, + vars: impl IntoIterator, +) { + for (key, value) in vars { + let key = key.to_string_lossy().into_owned(); + if !WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST + .iter() + .any(|allowed| key.eq_ignore_ascii_case(allowed)) + { + continue; + } + if env + .keys() + .any(|existing| existing.eq_ignore_ascii_case(&key)) + { + continue; + } + env.insert(key, value.to_string_lossy().into_owned()); + } +} + +pub(super) struct WindowsSandboxWrapperRequestFile { + path: std::path::PathBuf, +} + +impl WindowsSandboxWrapperRequestFile { + fn create( + codex_home: &std::path::Path, + request: &codex_windows_sandbox::WindowsSandboxWrapperRequest, + ) -> Result { + let request_dir = wrapper_request_dir(codex_home); + std::fs::create_dir_all(&request_dir).map_err(|err| { + internal_error(format!( + "failed to create windows fs sandbox wrapper request dir {}: {err}", + request_dir.display() + )) + })?; + codex_windows_sandbox::ensure_current_user_cleanup_access(&request_dir).map_err(|err| { + internal_error(format!( + "failed to grant cleanup access to windows fs sandbox wrapper request dir {}: {err}", + request_dir.display() + )) + })?; + let path = request_dir.join(format!( + "fs-helper-wrapper-request-{}.json", + uuid::Uuid::new_v4() + )); + let request_json = serde_json::to_vec(request).map_err(|err| { + internal_error(format!( + "failed to encode or decode fs sandbox helper message: {err}" + )) + })?; + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .map_err(|err| { + internal_error(format!( + "failed to create windows fs sandbox wrapper request file {}: {err}", + path.display() + )) + })?; + file.write_all(&request_json).map_err(|err| { + internal_error(format!( + "failed to write windows fs sandbox wrapper request file {}: {err}", + path.display() + )) + })?; + Ok(Self { path }) + } +} + +fn wrapper_request_dir(codex_home: &std::path::Path) -> std::path::PathBuf { + codex_windows_sandbox::sandbox_secrets_dir(codex_home).join("wrapper-requests") +} + +impl Drop for WindowsSandboxWrapperRequestFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::ffi::OsString; + + use codex_protocol::config_types::WindowsSandboxLevel; + use codex_protocol::models::PermissionProfile; + use codex_protocol::permissions::NetworkSandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + + use super::WindowsSandboxWrapperRequestFile; + use super::add_wrapper_setup_env_from_vars; + use super::wrapper_request_dir; + + #[test] + fn wrapper_setup_env_preserves_only_setup_identity() { + let mut env = HashMap::from([("PATH".to_string(), r"C:\Windows\System32".to_string())]); + + add_wrapper_setup_env_from_vars( + &mut env, + [ + ("USERNAME", "alice"), + ("USERPROFILE", r"C:\Users\alice"), + ("OPENAI_API_KEY", "secret"), + ] + .map(|(key, value)| (OsString::from(key), OsString::from(value))), + ); + + assert_eq!( + env, + HashMap::from([ + ("PATH".to_string(), r"C:\Windows\System32".to_string()), + ("USERNAME".to_string(), "alice".to_string()), + ("USERPROFILE".to_string(), r"C:\Users\alice".to_string()), + ]) + ); + } + + #[test] + fn wrapper_request_dir_uses_sandbox_secrets() { + let codex_home = std::env::temp_dir().join("codex-home"); + let sandbox_dir = codex_windows_sandbox::sandbox_dir(&codex_home); + let secrets_dir = codex_windows_sandbox::sandbox_secrets_dir(&codex_home); + let request_dir = wrapper_request_dir(&codex_home); + + assert!(!request_dir.starts_with(sandbox_dir)); + assert!(request_dir.starts_with(secrets_dir)); + } + + #[test] + fn wrapper_request_file_is_removed_on_drop() { + let codex_home = tempfile::TempDir::new().expect("codex home"); + let command_cwd = + AbsolutePathBuf::from_absolute_path(codex_home.path()).expect("absolute command cwd"); + let request = + codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile( + vec!["helper.exe".to_string()], + command_cwd, + HashMap::new(), + PermissionProfile::External { + network: NetworkSandboxPolicy::Restricted, + }, + WindowsSandboxLevel::RestrictedToken, + /*windows_sandbox_private_desktop*/ false, + codex_home.path().to_path_buf(), + ); + + let request_file = WindowsSandboxWrapperRequestFile::create(codex_home.path(), &request) + .expect("create wrapper request file"); + let path = request_file.path.clone(); + assert!(path.exists()); + + drop(request_file); + + assert!(!path.exists()); + } +} diff --git a/codex-rs/exec-server/tests/windows_file_system_sandbox.rs b/codex-rs/exec-server/tests/windows_file_system_sandbox.rs index fda7341bfd..03033db5dc 100644 --- a/codex-rs/exec-server/tests/windows_file_system_sandbox.rs +++ b/codex-rs/exec-server/tests/windows_file_system_sandbox.rs @@ -305,7 +305,7 @@ async fn elevated_fs_helper_rejects_unenforceable_reopened_writable_descendant() #[serial(codex_home)] async fn elevated_fs_helper_allows_writable_path_when_setup_is_available() -> Result<()> { if !process_is_elevated() { - eprintln!("skipping elevated fs-helper test: test process is not elevated"); + eprintln!("skipping elevated fs-helper write: test process is not elevated"); return Ok(()); } diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index ca4ac7d428..3c8326a332 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/codex-rs/windows-sandbox-rs/src/acl.rs @@ -492,28 +492,8 @@ pub unsafe fn add_allow_ace(path: &Path, psid: *mut c_void) -> Result { } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn current_user_cleanup_access_grants_delete_rights() -> Result<()> { - let temp_dir = tempfile::TempDir::new()?; - let request_dir = temp_dir.path().join("wrapper-requests"); - std::fs::create_dir(&request_dir)?; - - ensure_current_user_cleanup_access(&request_dir)?; - - let real_user = std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()); - let real_user_sid = resolve_sid(&real_user)?; - assert!(path_mask_allows( - &request_dir, - &[real_user_sid.as_ptr() as *mut c_void], - DELETE | FILE_DELETE_CHILD, - /*require_all_bits*/ true, - )?); - Ok(()) - } -} +#[path = "acl_tests.rs"] +mod tests; /// Adds a deny ACE to prevent write/append/delete for the given SID on the target path. /// diff --git a/codex-rs/windows-sandbox-rs/src/acl_tests.rs b/codex-rs/windows-sandbox-rs/src/acl_tests.rs new file mode 100644 index 0000000000..1d84966d72 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/acl_tests.rs @@ -0,0 +1,121 @@ +use std::ffi::c_void; +use std::path::Path; + +use anyhow::Result; +use anyhow::anyhow; +use windows_sys::Win32::Foundation::CloseHandle; +use windows_sys::Win32::Foundation::ERROR_SUCCESS; +use windows_sys::Win32::Foundation::HLOCAL; +use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Security::ACL; +use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W; +use windows_sys::Win32::Security::Authorization::SetEntriesInAclW; +use windows_sys::Win32::Security::Authorization::SetSecurityInfo; +use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID; +use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN; +use windows_sys::Win32::Security::Authorization::TRUSTEE_W; +use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; +use windows_sys::Win32::Security::PROTECTED_DACL_SECURITY_INFORMATION; +use windows_sys::Win32::Storage::FileSystem::CreateFileW; +use windows_sys::Win32::Storage::FileSystem::DELETE; +use windows_sys::Win32::Storage::FileSystem::FILE_DELETE_CHILD; +use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; +use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; + +use crate::acl::ensure_current_user_cleanup_access; +use crate::acl::path_mask_allows; +use crate::winutil::resolve_sid; +use crate::winutil::to_wide; + +const CONTAINER_INHERIT_ACE: u32 = 0x2; +const OBJECT_INHERIT_ACE: u32 = 0x1; + +#[test] +fn current_user_cleanup_access_grants_delete_rights() -> Result<()> { + let temp_dir = tempfile::TempDir::new()?; + let request_dir = temp_dir.path().join("wrapper-requests"); + std::fs::create_dir(&request_dir)?; + + let real_user = std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()); + let real_user_sid = resolve_sid(&real_user)?; + let real_user_sid = real_user_sid.as_ptr() as *mut c_void; + set_delete_less_allow_ace_for_test(&request_dir, real_user_sid)?; + assert!(!path_mask_allows( + &request_dir, + &[real_user_sid], + DELETE | FILE_DELETE_CHILD, + /*require_all_bits*/ true, + )?); + + ensure_current_user_cleanup_access(&request_dir)?; + + assert!(path_mask_allows( + &request_dir, + &[real_user_sid], + DELETE | FILE_DELETE_CHILD, + /*require_all_bits*/ true, + )?); + Ok(()) +} + +fn set_delete_less_allow_ace_for_test(path: &Path, psid: *mut c_void) -> Result<()> { + unsafe { + let trustee = TRUSTEE_W { + pMultipleTrustee: std::ptr::null_mut(), + MultipleTrusteeOperation: 0, + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_UNKNOWN, + ptstrName: psid as *mut u16, + }; + let mut explicit: EXPLICIT_ACCESS_W = std::mem::zeroed(); + explicit.grfAccessPermissions = + FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE; + explicit.grfAccessMode = 2; // SET_ACCESS + explicit.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE; + explicit.Trustee = trustee; + let mut p_new_dacl: *mut ACL = std::ptr::null_mut(); + let code = SetEntriesInAclW(1, &explicit, std::ptr::null_mut(), &mut p_new_dacl); + if code != ERROR_SUCCESS { + return Err(anyhow!("SetEntriesInAclW failed: {code}")); + } + + let desired = 0x00020000 | 0x00040000; // READ_CONTROL | WRITE_DAC + let h = CreateFileW( + to_wide(path).as_ptr(), + desired, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + std::ptr::null_mut(), + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + 0, + ); + if h == 0 || h == INVALID_HANDLE_VALUE { + LocalFree(p_new_dacl as HLOCAL); + return Err(anyhow!("CreateFileW failed for {}", path.display())); + } + let code = SetSecurityInfo( + h, + 1, + DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + p_new_dacl, + std::ptr::null_mut(), + ); + CloseHandle(h); + if !p_new_dacl.is_null() { + LocalFree(p_new_dacl as HLOCAL); + } + if code != ERROR_SUCCESS { + return Err(anyhow!("SetSecurityInfo failed: {code}")); + } + } + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/wrapper.rs b/codex-rs/windows-sandbox-rs/src/wrapper.rs index 00ecfbb3ab..cef78aa4c7 100644 --- a/codex-rs/windows-sandbox-rs/src/wrapper.rs +++ b/codex-rs/windows-sandbox-rs/src/wrapper.rs @@ -91,18 +91,11 @@ async fn run_windows_sandbox_wrapper_args(args: Vec) -> Result { run_windows_sandbox_wrapper_request(request).await } -async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperRequest) -> Result { - if !request.codex_home.is_absolute() { - bail!( - "windows sandbox wrapper codex_home must be absolute: {}", - request.codex_home.display() - ); - } - if request.command.is_empty() { - bail!("missing sandboxed command in windows sandbox wrapper request"); - } - - let env = request.env_map; +async fn run_windows_sandbox_wrapper_request( + mut request: WindowsSandboxWrapperRequest, +) -> Result { + validate_windows_sandbox_wrapper_request(&request)?; + let env_map = std::mem::take(&mut request.env_map); let workspace_roots = vec![request.command_cwd.clone()]; let spawned = match request.windows_sandbox_level { WindowsSandboxLevel::Elevated => { @@ -120,7 +113,7 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque request.codex_home.as_path(), request.command, request.command_cwd.as_path(), - env, + env_map, /*timeout_ms*/ None, overrides.read_roots_override.as_deref(), overrides.read_roots_include_platform_defaults, @@ -148,7 +141,7 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque request.codex_home.as_path(), request.command, request.command_cwd.as_path(), - env, + env_map, /*timeout_ms*/ None, &overrides.additional_deny_read_paths, &overrides.additional_deny_write_paths, @@ -163,6 +156,19 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque Ok(crate::stdio_bridge::forward_sandbox_session_stdio(spawned).await) } +fn validate_windows_sandbox_wrapper_request(request: &WindowsSandboxWrapperRequest) -> Result<()> { + if !request.codex_home.is_absolute() { + bail!( + "windows sandbox wrapper codex_home must be absolute: {}", + request.codex_home.display() + ); + } + if request.command.is_empty() { + bail!("missing sandboxed command in windows sandbox wrapper request"); + } + Ok(()) +} + fn parse_windows_sandbox_wrapper_args(args: Vec) -> Result { let mut args = args.into_iter(); let Some(flag) = args.next() else { @@ -191,11 +197,24 @@ fn next_flag_value(args: &mut impl Iterator, flag: &str) -> Resul #[cfg(test)] mod tests { + use std::sync::Mutex; + use codex_protocol::permissions::NetworkSandboxPolicy; use pretty_assertions::assert_eq; + use tempfile::TempDir; + use tokio::runtime::Builder; use super::*; + static WRAPPER_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn current_thread_runtime() -> tokio::runtime::Runtime { + Builder::new_current_thread() + .enable_all() + .build() + .expect("build tokio runtime") + } + #[test] fn windows_wrapper_args_reference_request_file_only() -> Result<()> { let args = create_windows_sandbox_command_args_for_request_file(Path::new( @@ -256,4 +275,41 @@ mod tests { assert_eq!(request.windows_sandbox_private_desktop, true); Ok(()) } + + #[test] + fn windows_wrapper_request_env_reaches_sandboxed_command() -> Result<()> { + let _guard = WRAPPER_PROCESS_TEST_LOCK + .lock() + .expect("wrapper process test lock poisoned"); + let runtime = current_thread_runtime(); + runtime.block_on(async move { + let workspace = TempDir::new()?; + let workspace = AbsolutePathBuf::from_absolute_path(workspace.path())?; + let codex_home = TempDir::new()?; + let request = create_windows_sandbox_wrapper_request_for_permission_profile( + vec![ + r"C:\Windows\System32\cmd.exe".to_string(), + "/d".to_string(), + "/s".to_string(), + "/c".to_string(), + r#"if "%CODEX_WRAPPER_REQUEST_ENV%"=="from-request" (exit /b 0) else (exit /b 7)"# + .to_string(), + ], + workspace, + HashMap::from([( + "CODEX_WRAPPER_REQUEST_ENV".to_string(), + "from-request".to_string(), + )]), + PermissionProfile::read_only(), + WindowsSandboxLevel::RestrictedToken, + /*windows_sandbox_private_desktop*/ true, + codex_home.path().to_path_buf(), + ); + + let exit_code = run_windows_sandbox_wrapper_request(request).await?; + + assert_eq!(exit_code, 0); + Ok(()) + }) + } }