diff --git a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs index 6dd372375e..dc43abd922 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs @@ -1,4 +1,5 @@ mod firewall; +mod no_reparse_dir; mod read_acl_mutex; use anyhow::Context; @@ -36,6 +37,7 @@ use std::collections::HashSet; use std::ffi::OsStr; use std::ffi::c_void; use std::io::Write; +use std::os::windows::io::AsRawHandle; use std::os::windows::process::CommandExt; use std::path::Path; use std::path::PathBuf; @@ -52,6 +54,7 @@ use windows_sys::Win32::Security::Authorization::GRANT_ACCESS; use windows_sys::Win32::Security::Authorization::SE_FILE_OBJECT; use windows_sys::Win32::Security::Authorization::SetEntriesInAclW; use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW; +use windows_sys::Win32::Security::Authorization::SetSecurityInfo; use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID; use windows_sys::Win32::Security::Authorization::TRUSTEE_W; use windows_sys::Win32::Security::CONTAINER_INHERIT_ACE; @@ -69,6 +72,7 @@ const WRITE_ROOT_ALLOW_MASK: u32 = mod sandbox_users; mod setup_runtime_bin; +use no_reparse_dir::open_or_create_no_reparse; use read_acl_mutex::acquire_read_acl_mutex; use read_acl_mutex::read_acl_mutex_exists; use sandbox_users::commit_setup_marker; @@ -297,9 +301,17 @@ fn lock_sandbox_dir( sandbox_group_access_mode: i32, sandbox_group_mask: u32, real_user_mask: u32, - _log: &mut dyn Write, + setup_mode: SetupMode, ) -> Result<()> { - std::fs::create_dir_all(dir)?; + // ProvisionOnly accepts another user's CODEX_HOME; keep its ACL mutation + // bound to a no-reparse handle without changing full setup behavior. + let directory = match setup_mode { + SetupMode::Full | SetupMode::ReadAclsOnly => { + std::fs::create_dir_all(dir)?; + None + } + SetupMode::ProvisionOnly => Some(open_or_create_no_reparse(dir)?), + }; let system_sid = resolve_sid("SYSTEM")?; let admins_sid = resolve_sid("Administrators")?; let real_sid = resolve_sid(real_user)?; @@ -360,20 +372,37 @@ fn lock_sandbox_dir( "SetEntriesInAclW sandbox dir failed: {set}", )); } - let path_w = to_wide(dir.as_os_str()); - let res = SetNamedSecurityInfoW( - path_w.as_ptr() as *mut u16, - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, - std::ptr::null_mut(), - std::ptr::null_mut(), - new_dacl, - std::ptr::null_mut(), - ); + let (res, api) = match directory.as_ref() { + Some(directory) => ( + SetSecurityInfo( + directory.as_raw_handle() as _, + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + new_dacl, + std::ptr::null_mut(), + ), + "SetSecurityInfo", + ), + None => { + let path_w = to_wide(dir.as_os_str()); + ( + SetNamedSecurityInfoW( + path_w.as_ptr() as *mut u16, + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + new_dacl, + std::ptr::null_mut(), + ), + "SetNamedSecurityInfoW", + ) + } + }; if res != 0 { - return Err(anyhow::anyhow!( - "SetNamedSecurityInfoW sandbox dir failed: {res}", - )); + return Err(anyhow::anyhow!("{api} sandbox dir failed: {res}")); } if !new_dacl.is_null() { LocalFree(new_dacl as HLOCAL); @@ -631,11 +660,7 @@ fn configure_offline_sandbox_network( Ok(()) } -fn lock_persistent_sandbox_dirs( - payload: &Payload, - sandbox_group_sid: &[u8], - log: &mut dyn Write, -) -> Result<()> { +fn lock_persistent_sandbox_dirs(payload: &Payload, sandbox_group_sid: &[u8]) -> Result<()> { lock_sandbox_dir( &sandbox_dir(&payload.codex_home), &payload.real_user, @@ -643,7 +668,7 @@ fn lock_persistent_sandbox_dirs( GRANT_ACCESS, FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE, FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE, - log, + payload.mode, ) .map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -661,7 +686,7 @@ fn lock_persistent_sandbox_dirs( DENY_ACCESS, FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE, FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE, - log, + payload.mode, ) .map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -679,11 +704,7 @@ fn lock_persistent_sandbox_dirs( Ok(()) } -fn lock_sandbox_bin_dir( - payload: &Payload, - sandbox_group_sid: &[u8], - log: &mut dyn Write, -) -> Result<()> { +fn lock_sandbox_bin_dir(payload: &Payload, sandbox_group_sid: &[u8]) -> Result<()> { lock_sandbox_dir( &sandbox_bin_dir(&payload.codex_home), &payload.real_user, @@ -691,7 +712,7 @@ fn lock_sandbox_bin_dir( GRANT_ACCESS, FILE_GENERIC_READ | FILE_GENERIC_EXECUTE, FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE, - log, + payload.mode, ) .map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -726,8 +747,8 @@ fn run_provision_only(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> configure_offline_sandbox_network(payload, &offline_sid_str, log)?; - lock_sandbox_bin_dir(payload, &sandbox_group_sid, log)?; - lock_persistent_sandbox_dirs(payload, &sandbox_group_sid, log)?; + lock_sandbox_bin_dir(payload, &sandbox_group_sid)?; + lock_persistent_sandbox_dirs(payload, &sandbox_group_sid)?; log_note("setup provisioning binary completed", Some(sbx_dir)); Ok(()) } @@ -982,7 +1003,7 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res } } - lock_sandbox_bin_dir(payload, &sandbox_group_sid, log)?; + lock_sandbox_bin_dir(payload, &sandbox_group_sid)?; if refresh_only { log_line( @@ -995,7 +1016,7 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res )?; } if !refresh_only { - lock_persistent_sandbox_dirs(payload, &sandbox_group_sid, log)?; + lock_persistent_sandbox_dirs(payload, &sandbox_group_sid)?; } unsafe { diff --git a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir.rs b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir.rs new file mode 100644 index 0000000000..dd9c100b07 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir.rs @@ -0,0 +1,152 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use anyhow::ensure; +use std::ffi::OsStr; +use std::ffi::c_void; +use std::mem::size_of; +use std::os::windows::ffi::OsStrExt; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::path::Component; +use std::path::Path; +use std::path::Prefix; +use std::ptr; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::NTSTATUS; +use windows_sys::Win32::Foundation::RtlNtStatusToDosError; +use windows_sys::Win32::Foundation::UNICODE_STRING; +use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_NORMAL; +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::WRITE_DAC; +use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; +use windows_sys::Win32::System::IO::IO_STATUS_BLOCK_0; +use windows_sys::Win32::System::Kernel::OBJ_CASE_INSENSITIVE; +use windows_sys::Win32::System::Kernel::OBJ_DONT_REPARSE; + +const FILE_OPEN_IF: u32 = 3; +const FILE_DIRECTORY_FILE: u32 = 1; +const STATUS_REPARSE_POINT_ENCOUNTERED: NTSTATUS = 0xC000_050B_u32 as i32; + +#[repr(C)] +struct ObjectAttributes { + length: u32, + root_directory: HANDLE, + object_name: *const UNICODE_STRING, + attributes: u32, + security_descriptor: *const c_void, + security_quality_of_service: *const c_void, +} + +#[link(name = "ntdll")] +unsafe extern "system" { + fn NtCreateFile( + file_handle: *mut HANDLE, + desired_access: u32, + object_attributes: *const ObjectAttributes, + io_status_block: *mut IO_STATUS_BLOCK, + allocation_size: *const i64, + file_attributes: u32, + share_access: u32, + create_disposition: u32, + create_options: u32, + ea_buffer: *const c_void, + ea_length: u32, + ) -> NTSTATUS; +} + +/// Opens or creates the final directory without following a reparse point in +/// any path component. +/// +/// Parent directories must already exist; only the final directory is created. +/// +/// The returned handle must remain open through any security mutation so the +/// mutation stays bound to the directory that passed this validation. +pub(super) fn open_or_create_no_reparse(path: &Path) -> Result { + ensure!( + path.is_absolute(), + "sandbox ACL path must be absolute: {}", + path.display() + ); + let source_offset = match path.components().next() { + Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::Disk(_)) => 0, + Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_)) => 4, + _ => bail!( + "sandbox ACL path must have a local disk prefix: {}", + path.display() + ), + }; + // NtCreateFile expects an NT namespace path. Convert only the local-disk + // spellings accepted by provisioning, without canonicalizing and following + // the reparse points that OBJ_DONT_REPARSE must reject. + let source: Vec = path.as_os_str().encode_wide().collect(); + let mut nt_path: Vec = OsStr::new("\\??\\").encode_wide().collect(); + for unit in source.into_iter().skip(source_offset) { + nt_path.push(if unit == b'/' as u16 { + b'\\' as u16 + } else { + unit + }); + } + nt_path.push(0); + let name_length = u16::try_from((nt_path.len() - 1) * size_of::()) + .context("sandbox ACL path is too long")?; + let maximum_length = + u16::try_from(nt_path.len() * size_of::()).context("sandbox ACL path is too long")?; + let object_name = UNICODE_STRING { + Length: name_length, + MaximumLength: maximum_length, + Buffer: nt_path.as_mut_ptr(), + }; + let object_attributes = ObjectAttributes { + length: size_of::() as u32, + root_directory: 0, + object_name: &object_name, + attributes: OBJ_CASE_INSENSITIVE as u32 | OBJ_DONT_REPARSE as u32, + security_descriptor: ptr::null(), + security_quality_of_service: ptr::null(), + }; + let mut io_status_block = IO_STATUS_BLOCK { + Anonymous: IO_STATUS_BLOCK_0 { Status: 0 }, + Information: 0, + }; + let mut handle = 0; + let status = unsafe { + NtCreateFile( + &mut handle, + WRITE_DAC, + &object_attributes, + &mut io_status_block, + ptr::null(), + FILE_ATTRIBUTE_NORMAL, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + FILE_OPEN_IF, + FILE_DIRECTORY_FILE, + ptr::null(), + /*ea_length*/ 0, + ) + }; + if status < 0 { + if status == STATUS_REPARSE_POINT_ENCOUNTERED { + bail!( + "sandbox ACL path contains a reparse point: {}", + path.display() + ); + } + let error = unsafe { RtlNtStatusToDosError(status) }; + return Err(std::io::Error::from_raw_os_error(error as i32)) + .with_context(|| format!("open sandbox ACL directory {}", path.display())); + } + ensure!( + handle != 0 && handle != INVALID_HANDLE_VALUE, + "NtCreateFile returned an invalid sandbox ACL directory handle" + ); + Ok(unsafe { OwnedHandle::from_raw_handle(handle as *mut c_void) }) +} + +#[cfg(test)] +#[path = "no_reparse_dir_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir_tests.rs b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir_tests.rs new file mode 100644 index 0000000000..4d8f987f51 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/no_reparse_dir_tests.rs @@ -0,0 +1,64 @@ +use std::fs; +use std::path::Path; +use std::process::Command; + +use anyhow::Result; + +use super::open_or_create_no_reparse; + +fn create_directory_junction(target: &Path, alias: &Path) -> Result<()> { + let output = Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(alias) + .arg(target) + .output()?; + if !output.status.success() { + anyhow::bail!( + "mklink /J failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(()) +} + +#[test] +fn creates_and_opens_plain_directory_leaf() -> Result<()> { + let temporary = tempfile::tempdir()?; + let directory = temporary.path().join(".sandbox-bin"); + + drop(open_or_create_no_reparse(&directory)?); + let _handle = open_or_create_no_reparse(&directory)?; + + assert!(directory.is_dir()); + Ok(()) +} + +#[test] +fn rejects_final_directory_junction() -> Result<()> { + let temporary = tempfile::tempdir()?; + let target = temporary.path().join("target"); + let alias = temporary.path().join(".sandbox-bin"); + fs::create_dir(&target)?; + create_directory_junction(&target, &alias)?; + + let _ = + open_or_create_no_reparse(&alias).expect_err("final directory junction must be rejected"); + fs::remove_dir(&alias)?; + Ok(()) +} + +#[test] +fn rejects_ancestor_directory_junction() -> Result<()> { + let temporary = tempfile::tempdir()?; + let target_home = temporary.path().join("target-home"); + let alias_home = temporary.path().join("linked-home"); + fs::create_dir(&target_home)?; + fs::create_dir(target_home.join(".sandbox-bin"))?; + create_directory_junction(&target_home, &alias_home)?; + + let _ = open_or_create_no_reparse(&alias_home.join(".sandbox-bin")) + .expect_err("ancestor directory junction must be rejected"); + fs::remove_dir(&alias_home)?; + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 3ef968aa57..4a5f403e5b 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -1078,6 +1078,24 @@ pub fn run_elevated_provisioning_setup( real_user: &str, settings: crate::WindowsSandboxProvisioningSettings, ) -> Result<()> { + if !codex_home.is_absolute() + || !matches!( + codex_home.components().next(), + Some(std::path::Component::Prefix(prefix)) + if matches!( + prefix.kind(), + std::path::Prefix::Disk(_) | std::path::Prefix::VerbatimDisk(_) + ) + ) + { + return Err(failure( + SetupErrorCode::OrchestratorSandboxDirCreateFailed, + format!( + "sandbox provisioning CODEX_HOME must be an absolute local disk path: {}", + codex_home.display() + ), + )); + } let sbx_dir = sandbox_dir(codex_home); std::fs::create_dir_all(&sbx_dir).map_err(|err| { failure(