diff --git a/codex-rs/network-proxy/tests/windows_stable_ingress.rs b/codex-rs/network-proxy/tests/windows_stable_ingress.rs index 510fb5c764..2aa408732c 100644 --- a/codex-rs/network-proxy/tests/windows_stable_ingress.rs +++ b/codex-rs/network-proxy/tests/windows_stable_ingress.rs @@ -13,6 +13,7 @@ use codex_network_proxy::NetworkProxyConfig; use codex_network_proxy::NetworkProxyState; use codex_network_proxy::build_config_state; use codex_windows_sandbox::ConsoleMode; +use codex_windows_sandbox::LaunchDesktop; use codex_windows_sandbox::LocalSid; use codex_windows_sandbox::create_process_as_user; use codex_windows_sandbox::create_readonly_token_with_caps_and_user_from; @@ -447,7 +448,9 @@ fn run_restricted_child_blocking( /*logs_base_dir*/ None, /*stdio*/ None, /*console_mode*/ ConsoleMode::Inherit, - /*use_private_desktop*/ false, + LaunchDesktop::prepare( + /*use_private_desktop*/ false, /*logs_base_dir*/ None, + )?, )? }; let process = unsafe { diff --git a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs index 41d1e5c8ca..d4d061e46c 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/command_runner/win.rs @@ -20,6 +20,7 @@ use codex_windows_sandbox::ErrorStage; use codex_windows_sandbox::ExitPayload; use codex_windows_sandbox::FramedMessage; use codex_windows_sandbox::IPC_PROTOCOL_VERSION; +use codex_windows_sandbox::LaunchDesktop; use codex_windows_sandbox::LocalSid; use codex_windows_sandbox::Message; use codex_windows_sandbox::OutputPayload; @@ -295,6 +296,15 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { } let effective_cwd = effective_cwd(&req.cwd, Some(log_dir.as_path())); + let desktop = if req.use_private_desktop { + LaunchDesktop::open_private( + req.private_desktop_name + .as_deref() + .context("runner: missing parent-owned private desktop")?, + )? + } else { + LaunchDesktop::prepare(/*use_private_desktop*/ false, Some(log_dir.as_path()))? + }; let mut conpty_owner = None; let mut hpc_handle: Option = None; @@ -305,8 +315,7 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { &req.command, &effective_cwd, &req.env, - req.use_private_desktop, - Some(log_dir.as_path()), + desktop, )?; let job = conpty .job() @@ -348,7 +357,7 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { } else { ConsoleMode::Inherit }, - req.use_private_desktop, + desktop, Some(log_dir.as_path()), )?; let pi = spawned_pipes.process; diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index 475a6c0f54..a8bb27a13c 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -106,8 +106,7 @@ pub fn spawn_conpty_process_as_user( argv: &[String], cwd: &Path, env_map: &HashMap, - use_private_desktop: bool, - logs_base_dir: Option<&Path>, + desktop: LaunchDesktop, ) -> Result<(PROCESS_INFORMATION, ConptyInstance)> { let cmdline_str = argv .iter() @@ -122,7 +121,6 @@ pub fn spawn_conpty_process_as_user( si.StartupInfo.hStdInput = INVALID_HANDLE_VALUE; si.StartupInfo.hStdOutput = INVALID_HANDLE_VALUE; si.StartupInfo.hStdError = INVALID_HANDLE_VALUE; - let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; si.StartupInfo.lpDesktop = desktop.startup_info_desktop(); let job = Arc::new(JobObject::create().context("create process job")?); diff --git a/codex-rs/windows-sandbox-rs/src/desktop.rs b/codex-rs/windows-sandbox-rs/src/desktop.rs index 918278828d..0dcc60a4cb 100644 --- a/codex-rs/windows-sandbox-rs/src/desktop.rs +++ b/codex-rs/windows-sandbox-rs/src/desktop.rs @@ -1,20 +1,40 @@ +use crate::allow::compute_allow_paths_for_permissions; +use crate::deny_read_acl::plan_deny_read_acl_paths; use crate::logging; +use crate::path_normalization::canonicalize_path; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; +use crate::setup::SandboxSetupRequest; +use crate::setup::SetupRootOverrides; +use crate::setup::build_payload_deny_write_paths; +use crate::setup::build_payload_roots; +use crate::setup::gather_read_roots; +use crate::spawn_prep::LegacySessionSecurity; use crate::token::get_current_token_for_restriction; use crate::token::get_logon_sid_bytes; +use crate::token::get_user_sid_bytes; use crate::winutil::format_last_error; +use crate::winutil::resolve_sid; +use crate::winutil::sid_bytes_from_string; +use crate::winutil::string_from_sid_bytes; use crate::winutil::to_wide; use anyhow::Result; use rand::Rng; use rand::SeedableRng; use rand::rngs::SmallRng; +use std::collections::BTreeSet; +use std::collections::HashMap; use std::ffi::c_void; use std::path::Path; +use std::path::PathBuf; use std::ptr; +use std::sync::Mutex; +use std::sync::OnceLock; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::ERROR_SUCCESS; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW; use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W; use windows_sys::Win32::Security::Authorization::GRANT_ACCESS; use windows_sys::Win32::Security::Authorization::SE_WINDOW_OBJECT; @@ -24,6 +44,8 @@ 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::PSECURITY_DESCRIPTOR; +use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::StationsAndDesktops::CloseDesktop; use windows_sys::Win32::System::StationsAndDesktops::CreateDesktopW; use windows_sys::Win32::System::StationsAndDesktops::DESKTOP_CREATEMENU; @@ -39,6 +61,9 @@ use windows_sys::Win32::System::StationsAndDesktops::DESKTOP_SWITCHDESKTOP; use windows_sys::Win32::System::StationsAndDesktops::DESKTOP_WRITE_DAC; use windows_sys::Win32::System::StationsAndDesktops::DESKTOP_WRITE_OWNER; use windows_sys::Win32::System::StationsAndDesktops::DESKTOP_WRITEOBJECTS; +use windows_sys::Win32::System::StationsAndDesktops::OpenDesktopW; + +const PRIVATE_DESKTOP_PREFIX: &str = "CodexSandboxDesktop-"; const DESKTOP_ALL_ACCESS: u32 = DESKTOP_READOBJECTS | DESKTOP_CREATEWINDOW @@ -54,12 +79,123 @@ const DESKTOP_ALL_ACCESS: u32 = DESKTOP_READOBJECTS | DESKTOP_WRITE_DAC | DESKTOP_WRITE_OWNER; +const DESKTOP_PARTICIPANT_ACCESS: u32 = + DESKTOP_ALL_ACCESS & !(DESKTOP_WRITE_DAC | DESKTOP_WRITE_OWNER | DESKTOP_DELETE); + +static SHARED_PRIVATE_DESKTOPS: OnceLock>> = + OnceLock::new(); + +#[derive(Clone, PartialEq, Eq, Hash)] +pub(crate) struct DesktopPolicy { + uses_write_capabilities: bool, + capability_sids: BTreeSet>, + network_enabled: bool, + network_proxy_restricting_sid: Option>, + // None denotes the legacy backend's unrestricted reads and different desktop ACLs. + read_roots: Option>, + write_roots: BTreeSet, + deny_read_paths: BTreeSet, + deny_write_paths: BTreeSet, +} + +impl DesktopPolicy { + pub(crate) fn elevated( + request: SandboxSetupRequest<'_>, + mut overrides: SetupRootOverrides, + capability_sids: &[String], + network_proxy_restricting_sid: Option<&str>, + ) -> Result { + // Match the complete read override passed by credential setup to the ACL helper. + overrides.read_roots.get_or_insert_with(|| { + gather_read_roots( + request.command_cwd, + request.permissions, + request.env_map, + request.codex_home, + ) + }); + let (read_roots, write_roots) = build_payload_roots(&request, &overrides); + Ok(Self { + uses_write_capabilities: request + .permissions + .uses_write_capabilities_for_cwd(request.command_cwd, request.env_map), + capability_sids: capability_sids + .iter() + .map(|sid| sid_bytes_from_string(sid)) + .collect::>()?, + network_enabled: request.permissions.network_policy().is_enabled(), + network_proxy_restricting_sid: network_proxy_restricting_sid + .map(sid_bytes_from_string) + .transpose()?, + read_roots: Some(read_roots.into_iter().collect()), + write_roots: write_roots.into_iter().collect(), + deny_read_paths: plan_deny_read_acl_paths( + overrides.deny_read_paths.as_deref().unwrap_or_default(), + ) + .into_iter() + .collect(), + deny_write_paths: build_payload_deny_write_paths(&request, overrides.deny_write_paths) + .into_iter() + .map(|path| canonicalize_path(&path)) + .collect(), + }) + } +} + pub struct LaunchDesktop { _private_desktop: Option, startup_name: Vec, } impl LaunchDesktop { + pub(crate) fn prepare_legacy( + use_private_desktop: bool, + permissions: &ResolvedWindowsSandboxPermissions, + cwd: &Path, + env: &HashMap, + security: &LegacySessionSecurity, + additional_deny_write_paths: &[PathBuf], + logs_base_dir: Option<&Path>, + ) -> Result { + if !use_private_desktop { + return Self::prepare(/*use_private_desktop*/ false, logs_base_dir); + } + let sandbox_sid = unsafe { get_user_sid_bytes(security.h_token)? }; + let sandbox_sid = string_from_sid_bytes(&sandbox_sid).map_err(anyhow::Error::msg)?; + let paths = compute_allow_paths_for_permissions(permissions, cwd, env); + let policy = DesktopPolicy { + uses_write_capabilities: security.readonly_sid.is_none(), + capability_sids: security + .readonly_sid_str + .iter() + .chain(security.write_root_sids.iter().map(|root| &root.sid_str)) + .map(|sid| sid_bytes_from_string(sid)) + .collect::>()?, + network_enabled: permissions.network_policy().is_enabled(), + network_proxy_restricting_sid: None, + read_roots: None, + write_roots: paths.allow.into_iter().collect(), + deny_read_paths: BTreeSet::new(), + deny_write_paths: paths + .deny + .into_iter() + .chain(additional_deny_write_paths.iter().cloned()) + .map(|path| canonicalize_path(&path)) + .collect(), + }; + let mut desktops = SHARED_PRIVATE_DESKTOPS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .map_err(|_| anyhow::anyhow!("shared private desktop cache was poisoned"))?; + let desktop = match desktops.entry((sandbox_sid, policy)) { + std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), + std::collections::hash_map::Entry::Vacant(entry) => { + entry.insert(PrivateDesktop::create(logs_base_dir)?) + } + }; + Self::open_private(&desktop.name) + } + pub fn prepare(use_private_desktop: bool, logs_base_dir: Option<&Path>) -> Result { if use_private_desktop { let private_desktop = PrivateDesktop::create(logs_base_dir)?; @@ -76,11 +212,132 @@ impl LaunchDesktop { } } + /// Opens the caller-owned private desktop without creating one or falling back to Default. + pub fn open_private(name: &str) -> Result { + if !name + .strip_prefix(PRIVATE_DESKTOP_PREFIX) + .is_some_and(|nonce| { + !nonce.is_empty() + && nonce.len() <= 32 + && nonce.bytes().all(|byte| byte.is_ascii_hexdigit()) + }) + { + anyhow::bail!("invalid private desktop name"); + } + let name_wide = to_wide(name); + let handle = unsafe { + OpenDesktopW( + name_wide.as_ptr(), + /*dwflags*/ 0, + /*finherit*/ 0, + DESKTOP_PARTICIPANT_ACCESS, + ) + }; + if handle == 0 { + anyhow::bail!("OpenDesktopW failed: {}", unsafe { GetLastError() }); + } + Ok(Self { + _private_desktop: Some(PrivateDesktop { + handle, + name: name.to_owned(), + }), + startup_name: to_wide(format!("Winsta0\\{name}")), + }) + } + pub fn startup_info_desktop(&self) -> *mut u16 { self.startup_name.as_ptr() as *mut u16 } } +/// Reuses a private desktop only for the same sandbox account and effective permissions. +pub(crate) fn shared_private_desktop_for_user( + sandbox_username: &str, + policy: &DesktopPolicy, + logs_base_dir: Option<&Path>, +) -> Result { + let sandbox_sid = + string_from_sid_bytes(&resolve_sid(sandbox_username)?).map_err(anyhow::Error::msg)?; + let mut desktops = SHARED_PRIVATE_DESKTOPS + .get_or_init(|| Mutex::new(HashMap::new())) + .lock() + .map_err(|_| anyhow::anyhow!("shared private desktop cache was poisoned"))?; + let key = (sandbox_sid.clone(), policy.clone()); + if let Some(desktop) = desktops.get(&key) { + return Ok(desktop.name.clone()); + } + + let owner_user_sid = unsafe { + let token = get_current_token_for_restriction()?; + let sid = get_user_sid_bytes(token); + CloseHandle(token); + sid? + }; + let owner_user_sid = string_from_sid_bytes(&owner_user_sid).map_err(anyhow::Error::msg)?; + // CreateProcessWithLogonW shares the caller's logon SID with the sandbox account. + // Grant ACL-management rights to the caller's user SID instead. + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createprocesswithlogonw + let sddl = to_wide(format!( + "D:P(A;;0x{DESKTOP_ALL_ACCESS:x};;;{owner_user_sid})(A;;0x{DESKTOP_PARTICIPANT_ACCESS:x};;;{sandbox_sid})" + )); + let mut security_descriptor: PSECURITY_DESCRIPTOR = ptr::null_mut(); + if unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + /*stringsdrevision*/ 1, + &mut security_descriptor, + ptr::null_mut(), + ) + } == 0 + { + anyhow::bail!( + "ConvertStringSecurityDescriptorToSecurityDescriptorW failed: {}", + unsafe { GetLastError() } + ); + } + + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: security_descriptor, + bInheritHandle: 0, + }; + let mut rng = SmallRng::from_entropy(); + let name = format!("{PRIVATE_DESKTOP_PREFIX}{:032x}", rng.r#gen::()); + let name_wide = to_wide(&name); + let handle = unsafe { + CreateDesktopW( + name_wide.as_ptr(), + ptr::null(), + ptr::null_mut(), + /*dwflags*/ 0, + DESKTOP_ALL_ACCESS, + &attributes, + ) + }; + let error = unsafe { GetLastError() }; + unsafe { + LocalFree(security_descriptor as HLOCAL); + } + if handle == 0 { + logging::debug_log( + &format!("CreateDesktopW failed for shared private desktop: {error}"), + logs_base_dir, + ); + anyhow::bail!("CreateDesktopW failed for shared private desktop: {error}"); + } + + // Retain ownership across runner exits and idle gaps; different policies stay on separate + // desktops so GUI hooks do not automatically cross those policies. + desktops.insert( + key, + PrivateDesktop { + handle, + name: name.clone(), + }, + ); + Ok(name) +} + struct PrivateDesktop { handle: isize, name: String, @@ -194,3 +451,7 @@ impl Drop for PrivateDesktop { } } } + +#[cfg(test)] +#[path = "desktop_tests.rs"] +mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/desktop_tests.rs b/codex-rs/windows-sandbox-rs/src/desktop_tests.rs new file mode 100644 index 0000000000..2c8f35c961 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/desktop_tests.rs @@ -0,0 +1,242 @@ +use super::DesktopPolicy; +use super::LaunchDesktop; +use super::PRIVATE_DESKTOP_PREFIX; +use super::shared_private_desktop_for_user; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; +use crate::setup::SandboxSetupRequest; +use crate::setup::SetupRootOverrides; +use crate::spawn_prep::legacy_session_capability_roots; +use crate::spawn_prep::prepare_legacy_session_security; +use anyhow::Result; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use rand::Rng; +use rand::SeedableRng; +use rand::rngs::SmallRng; +use std::collections::HashMap; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::path::Path; +use tempfile::TempDir; +use windows_sys::Win32::NetworkManagement::NetManagement::DNLEN; +use windows_sys::Win32::NetworkManagement::NetManagement::UNLEN; +use windows_sys::Win32::Security::Authentication::Identity::GetUserNameExW; +use windows_sys::Win32::Security::Authentication::Identity::NameSamCompatible; + +#[test] +fn private_desktop_rejects_interactive_and_injected_names() { + for name in [ + "Default", + r"Winsta0\Default", + "CodexSandboxDesktop-", + r"CodexSandboxDesktop-abcd\Default", + "CodexSandboxDesktop-abcd\0Default", + "CodexSandboxDesktop-0123456789abcdef0123456789abcdef0", + ] { + assert!(LaunchDesktop::open_private(name).is_err(), "{name:?}"); + } +} + +#[test] +fn opening_missing_private_desktop_does_not_create_or_fall_back() { + let name = format!( + "{PRIVATE_DESKTOP_PREFIX}{:032x}", + SmallRng::from_entropy().r#gen::(), + ); + assert!(LaunchDesktop::open_private(&name).is_err()); +} + +#[test] +fn shared_desktop_survives_launch_handles_and_concurrent_requests() -> Result<()> { + let policy = DesktopPolicy { + uses_write_capabilities: false, + capability_sids: Default::default(), + network_enabled: false, + network_proxy_restricting_sid: None, + read_roots: Some(Default::default()), + write_roots: Default::default(), + deny_read_paths: Default::default(), + deny_write_paths: Default::default(), + }; + let account = current_account_name()?; + let names = std::thread::scope(|scope| { + let workers = [(); 4].map(|_| { + let account = &account; + let policy = &policy; + scope.spawn(move || { + shared_private_desktop_for_user(account, policy, /*logs_base_dir*/ None) + }) + }); + workers + .into_iter() + .map(|worker| worker.join().expect("desktop request panicked")) + .collect::>>() + })?; + let name = names[0].clone(); + assert_eq!(names, vec![name.clone(); 4]); + drop(LaunchDesktop::open_private(&name)?); + assert_eq!( + shared_private_desktop_for_user( + &account.to_ascii_lowercase(), + &policy, + /*logs_base_dir*/ None + )?, + name, + ); + drop(LaunchDesktop::open_private(&name)?); + Ok(()) +} + +#[test] +fn shared_desktop_reuses_only_equivalent_permissions() -> Result<()> { + let temp = TempDir::new()?; + let workspace = temp.path().join("workspace"); + let readable = temp.path().join("readable"); + let writable = temp.path().join("writable"); + for path in [&workspace, &readable, &writable] { + std::fs::create_dir(path)?; + } + let codex_home = temp.path().join("codex-home"); + let permissions = workspace_permissions(&workspace)?; + let env = HashMap::new(); + let account = current_account_name()?; + let sids = ["S-1-5-21-10-20-30-40".into(), "S-1-5-21-10-20-30-41".into()]; + let proxy_sid = "S-1-5-21-50-60-70-80"; + let overrides = || SetupRootOverrides { + read_roots: Some(vec![readable.clone()]), + write_roots: Some(vec![workspace.clone(), writable.clone()]), + ..Default::default() + }; + let desktop = |overrides, capability_sids: &[String], network_proxy_restricting_sid| { + let policy = DesktopPolicy::elevated( + SandboxSetupRequest { + permissions: &permissions, + command_cwd: &workspace, + env_map: &env, + codex_home: &codex_home, + proxy_enforced: true, + }, + overrides, + capability_sids, + Some(network_proxy_restricting_sid), + )?; + shared_private_desktop_for_user(&account, &policy, /*logs_base_dir*/ None) + }; + let name = desktop(overrides(), &sids, proxy_sid)?; + assert_eq!( + desktop( + SetupRootOverrides { + read_roots: Some(vec![readable.clone(), readable.clone()]), + write_roots: Some(vec![ + writable.join("..").join("writable"), + workspace.clone(), + writable.clone(), + ]), + ..Default::default() + }, + &[sids[1].clone(), sids[0].clone(), sids[1].clone()], + proxy_sid, + )?, + name, + ); + for (reason, changed) in [ + ( + "read roots", + SetupRootOverrides { + read_roots: Some(vec![workspace.clone()]), + ..overrides() + }, + ), + ( + "write roots", + SetupRootOverrides { + write_roots: Some(vec![workspace.clone()]), + ..overrides() + }, + ), + ( + "deny reads", + SetupRootOverrides { + deny_read_paths: Some(vec![readable.clone()]), + ..overrides() + }, + ), + ( + "deny writes", + SetupRootOverrides { + deny_write_paths: Some(vec![writable.clone()]), + ..overrides() + }, + ), + ] { + let changed_name = desktop(changed, &sids, proxy_sid)?; + assert_ne!(changed_name, name, "{reason}"); + drop(LaunchDesktop::open_private(&changed_name)?); + } + assert_ne!(desktop(overrides(), &sids[..1], proxy_sid)?, name); + assert_ne!(desktop(overrides(), &sids, "S-1-5-21-50-60-70-81")?, name,); + assert_eq!(desktop(overrides(), &sids, proxy_sid)?, name); + Ok(()) +} + +#[test] +fn legacy_desktop_reuses_only_equivalent_permissions() -> Result<()> { + let temp = TempDir::new()?; + let workspace = temp.path().join("workspace"); + std::fs::create_dir(&workspace)?; + let codex_home = temp.path().join("codex-home"); + let permissions = workspace_permissions(&workspace)?; + let env = HashMap::new(); + let security = prepare_legacy_session_security( + /*uses_write_capabilities*/ true, + &codex_home, + &workspace, + legacy_session_capability_roots(&permissions, &workspace, &env, &codex_home), + )?; + let _token = unsafe { OwnedHandle::from_raw_handle(security.h_token as *mut _) }; + let desktop = |deny_write_paths| { + LaunchDesktop::prepare_legacy( + /*use_private_desktop*/ true, + &permissions, + &workspace, + &env, + &security, + deny_write_paths, + /*logs_base_dir*/ None, + ) + }; + let first = desktop(&[])?; + let name = first.startup_name.clone(); + drop(first); + assert_eq!(desktop(&[])?.startup_name, name); + assert_ne!( + desktop(std::slice::from_ref(&workspace))?.startup_name, + name + ); + assert_eq!(desktop(&[])?.startup_name, name); + Ok(()) +} + +fn current_account_name() -> Result { + // Bazel does not provide USERDOMAIN or USERNAME in the test environment. + let mut account = [0; (DNLEN + UNLEN + 2) as usize]; + let mut length = account.len() as u32; + if unsafe { GetUserNameExW(NameSamCompatible, account.as_mut_ptr(), &mut length) } == 0 { + return Err(std::io::Error::last_os_error().into()); + } + Ok(String::from_utf16(&account[..length as usize])?) +} + +fn workspace_permissions(workspace: &Path) -> Result { + ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots( + &PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ), + &[AbsolutePathBuf::from_absolute_path(workspace)?], + ) +} diff --git a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs index fca07f73c3..5363ae6686 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs @@ -26,7 +26,7 @@ use std::path::PathBuf; const MAX_FRAME_LEN: usize = 8 * 1024 * 1024; /// Protocol version shared by the parent process and elevated command runner. -pub const IPC_PROTOCOL_VERSION: u8 = 5; +pub const IPC_PROTOCOL_VERSION: u8 = 6; /// Length-prefixed, JSON-encoded frame. #[derive(Debug, Serialize, Deserialize, Clone)] @@ -74,6 +74,8 @@ pub struct SpawnRequest { pub stdin_open: bool, #[serde(default)] pub use_private_desktop: bool, + /// Private desktop kept alive by the parent across command runners. + pub private_desktop_name: Option, } /// Ack from runner after it spawns the child process. @@ -231,7 +233,8 @@ mod tests { timeout_ms: Some(1000), tty: false, stdin_open: false, - use_private_desktop: false, + use_private_desktop: true, + private_desktop_name: Some("CodexSandboxDesktop-1234".to_string()), }), }, }; @@ -249,6 +252,10 @@ mod tests { }; assert_eq!(PermissionProfile::read_only(), payload.permission_profile); assert_eq!(workspace_roots, payload.workspace_roots); + assert_eq!( + Some("CodexSandboxDesktop-1234"), + payload.private_desktop_name.as_deref() + ); assert_eq!( Some("S-1-5-21-100-200-300-400"), payload.network_proxy_restricting_sid.as_deref() diff --git a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs index 90ad2dbc93..50c2843dbf 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs @@ -1,3 +1,5 @@ +use crate::desktop::DesktopPolicy; +use crate::desktop::shared_private_desktop_for_user; use crate::identity::SandboxCreds; use crate::ipc_framed::ErrorPayload; use crate::ipc_framed::ErrorStage; @@ -312,8 +314,12 @@ pub(crate) fn spawn_runner_transport( cwd: &Path, sandbox_creds: &SandboxCreds, log_dir: Option<&Path>, - spawn_request: SpawnRequest, + mut spawn_request: SpawnRequest, + desktop_policy: Option<&DesktopPolicy>, ) -> Result { + spawn_request.private_desktop_name = desktop_policy + .map(|policy| shared_private_desktop_for_user(&sandbox_creds.username, policy, log_dir)) + .transpose()?; let (pipe_in_name, pipe_out_name) = pipe_pair(); let h_pipe_in = create_named_pipe(&pipe_in_name, PIPE_ACCESS_OUTBOUND, &sandbox_creds.username)?; diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 62f4d2a455..702884acbd 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -57,6 +57,7 @@ mod windows_impl { use codex_utils_absolute_path::AbsolutePathBuf; use std::fs::File; use std::path::Path; + use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; @@ -185,6 +186,28 @@ mod windows_impl { } (|| -> Result { + let desktop_policy = use_private_desktop + .then(|| { + crate::desktop::DesktopPolicy::elevated( + crate::setup::SandboxSetupRequest { + permissions: &permissions, + command_cwd: cwd, + env_map: &env_map, + codex_home, + proxy_enforced, + }, + crate::setup::SetupRootOverrides { + read_roots: read_roots_override.map(<[PathBuf]>::to_vec), + read_roots_include_platform_defaults, + write_roots: write_roots_override.map(<[PathBuf]>::to_vec), + deny_read_paths: Some(deny_read_paths_override.clone()), + deny_write_paths: Some(deny_write_paths_override.clone()), + }, + &cap_sids, + network_proxy_restricting_sid.as_deref(), + ) + }) + .transpose()?; let spawn_request = SpawnRequest { command: command.clone(), cwd: cwd.to_path_buf(), @@ -199,6 +222,7 @@ mod windows_impl { tty: false, stdin_open: false, use_private_desktop, + private_desktop_name: None, }; let transport = retry_runner_spawn_once( sandbox_creds, @@ -210,6 +234,7 @@ mod windows_impl { &sandbox_creds, logs_base_dir, spawn_request.clone(), + desktop_policy.as_ref(), ) }, || { diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index e79ab4692a..0ca4dcafe8 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -576,7 +576,16 @@ mod windows_impl { )?; let (stdin_pair, stdout_pair, stderr_pair) = unsafe { setup_stdio_pipes()? }; let ((in_r, in_w), (out_r, out_w), (err_r, err_w)) = (stdin_pair, stdout_pair, stderr_pair); - let spawn_res = unsafe { + let spawn_res = crate::LaunchDesktop::prepare_legacy( + use_private_desktop, + &permissions, + ¤t_dir, + &env_map, + &security, + &additional_deny_write_paths, + logs_base_dir, + ) + .and_then(|desktop| unsafe { create_process_as_user( security.h_token, &command, @@ -585,9 +594,9 @@ mod windows_impl { logs_base_dir, Some((in_r, out_w, err_w)), ConsoleMode::Inherit, - use_private_desktop, + desktop, ) - }; + }); let created = match spawn_res { Ok(v) => v, Err(err) => { diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index c2f873f0e1..aa87ddba49 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -97,12 +97,11 @@ pub unsafe fn create_process_as_user( logs_base_dir: Option<&Path>, stdio: Option<(HANDLE, HANDLE, HANDLE)>, console_mode: ConsoleMode, - use_private_desktop: bool, + desktop: LaunchDesktop, ) -> Result { let cmdline_str = argv_to_command_line(argv); let mut cmdline: Vec = to_wide(&cmdline_str); let env_block = make_env_block(env_map); - let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; let job = Arc::new(JobObject::create().context("create process job")?); let mut pi: PROCESS_INFORMATION = std::mem::zeroed(); let cwd_wide = to_wide(cwd); @@ -229,7 +228,7 @@ pub fn spawn_process_with_pipes( stdin_mode: StdinMode, stderr_mode: StderrMode, console_mode: ConsoleMode, - use_private_desktop: bool, + desktop: LaunchDesktop, logs_base_dir: Option<&Path>, ) -> Result { let mut in_r: HANDLE = 0; @@ -273,7 +272,7 @@ pub fn spawn_process_with_pipes( logs_base_dir, stdio, console_mode, - use_private_desktop, + desktop, ) }; let created = match spawn_result { diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 890755d01c..5daaa8d60d 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -1143,7 +1143,7 @@ pub fn run_elevated_provisioning_setup( run_setup_exe(&payload, /*needs_elevation*/ false, codex_home) } -fn build_payload_roots( +pub(crate) fn build_payload_roots( request: &SandboxSetupRequest<'_>, overrides: &SetupRootOverrides, ) -> (Vec, Vec) { @@ -1202,7 +1202,7 @@ fn build_payload_roots( (read_roots, write_roots) } -fn build_payload_deny_write_paths( +pub(crate) fn build_payload_deny_write_paths( request: &SandboxSetupRequest<'_>, explicit_deny_write_paths: Option>, ) -> Vec { diff --git a/codex-rs/windows-sandbox-rs/src/token.rs b/codex-rs/windows-sandbox-rs/src/token.rs index 688ec529c4..94b2f5d7c9 100644 --- a/codex-rs/windows-sandbox-rs/src/token.rs +++ b/codex-rs/windows-sandbox-rs/src/token.rs @@ -276,7 +276,7 @@ pub unsafe fn get_logon_sid_bytes(h_token: HANDLE) -> Result> { Err(anyhow!("Logon SID not present on token")) } -unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result> { +pub(crate) unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result> { let mut needed: u32 = 0; GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed); if needed == 0 { diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs index 138b0be790..0d7d2ca19a 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs @@ -3,6 +3,7 @@ use super::windows_common::make_runner_resizer; use super::windows_common::start_runner_pipe_writer; use super::windows_common::start_runner_stdin_writer; use super::windows_common::start_runner_stdout_reader; +use crate::desktop::DesktopPolicy; use crate::identity::SandboxCreds; use crate::identity::refresh_logon_sandbox_creds; use crate::ipc_framed::EmptyPayload; @@ -96,10 +97,47 @@ async fn spawn_runner_transport_task( request: RunnerTransportRequest, ) -> Result { tokio::task::spawn_blocking(move || -> Result<_> { + let desktop_policy = request + .spawn_request + .use_private_desktop + .then(|| { + DesktopPolicy::elevated( + crate::setup::SandboxSetupRequest { + permissions: &request.permissions, + command_cwd: &request.cwd, + env_map: &request.env_map, + codex_home: &request.codex_home, + proxy_enforced: request.proxy_enforced, + }, + crate::setup::SetupRootOverrides { + read_roots: request.read_roots_override.clone(), + read_roots_include_platform_defaults: request + .read_roots_include_platform_defaults, + write_roots: request.write_roots_override.clone(), + deny_read_paths: Some(request.deny_read_paths_override.clone()), + deny_write_paths: Some(request.deny_write_paths_override.clone()), + }, + &request.spawn_request.cap_sids, + request + .spawn_request + .network_proxy_restricting_sid + .as_deref(), + ) + }) + .transpose()?; spawn_runner_transport_with_retry( sandbox_creds, &request, - spawn_runner_transport, + |codex_home, cwd, sandbox_creds, log_dir, spawn_request| { + spawn_runner_transport( + codex_home, + cwd, + sandbox_creds, + log_dir, + spawn_request, + desktop_policy.as_ref(), + ) + }, refresh_logon_sandbox_creds, ) }) @@ -177,6 +215,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil tty, stdin_open, use_private_desktop, + private_desktop_name: None, }, read_roots_override: read_roots_override.map(<[PathBuf]>::to_vec), read_roots_include_platform_defaults, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs index a5187eb15c..e8025109e0 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated_tests.rs @@ -81,6 +81,7 @@ fn retry_uses_original_unified_exec_request_and_stops_after_second_failure() { tty: true, stdin_open: true, use_private_desktop: true, + private_desktop_name: None, }, read_roots_override: Some(vec![PathBuf::from(r"C:\workspace\read")]), read_roots_include_platform_defaults: true, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index bcbb4368c3..08ede4a2b8 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -10,7 +10,9 @@ use crate::process::StderrMode; use crate::process::StdinMode; use crate::process::read_handle_loop; use crate::process::spawn_process_with_pipes; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use crate::spawn_prep::LegacyAclSids; +use crate::spawn_prep::LegacySessionSecurity; use crate::spawn_prep::SpawnPrepOptions; use crate::spawn_prep::allow_null_device_for_workspace_write; use crate::spawn_prep::apply_legacy_session_acl_rules; @@ -61,7 +63,9 @@ struct LegacyProcessHandles { #[allow(clippy::too_many_arguments)] fn spawn_legacy_process( - h_token: HANDLE, + security: &LegacySessionSecurity, + permissions: &ResolvedWindowsSandboxPermissions, + additional_deny_write_paths: &[std::path::PathBuf], command: &[String], cwd: &Path, env_map: &HashMap, @@ -73,15 +77,19 @@ fn spawn_legacy_process( writer_rx: mpsc::Receiver>, logs_base_dir: Option<&Path>, ) -> Result { + let h_token = security.h_token; + let launch_desktop = LaunchDesktop::prepare_legacy( + use_private_desktop, + permissions, + cwd, + env_map, + security, + additional_deny_write_paths, + logs_base_dir, + )?; let (pi, job, output_join, writer_handle, hpc, conpty_owner, desktop) = if tty { - let (pi, mut conpty) = spawn_conpty_process_as_user( - h_token, - command, - cwd, - env_map, - use_private_desktop, - logs_base_dir, - )?; + let (pi, mut conpty) = + spawn_conpty_process_as_user(h_token, command, cwd, env_map, launch_desktop)?; let job = conpty .job() .ok_or_else(|| anyhow::anyhow!("spawned ConPTY is missing its process job"))?; @@ -106,7 +114,7 @@ fn spawn_legacy_process( }, StderrMode::Separate, ConsoleMode::Inherit, - use_private_desktop, + launch_desktop, logs_base_dir, )?; let stdout_join = spawn_output_reader(pipe_handles.stdout_read, stdout_tx); @@ -389,7 +397,9 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( token_handle, desktop, } = match spawn_legacy_process( - security.h_token, + &security, + &common.permissions, + &additional_deny_write_paths, &command, cwd, &env_map, diff --git a/codex-rs/windows-sandbox-rs/src/winutil.rs b/codex-rs/windows-sandbox-rs/src/winutil.rs index 45378b4bb5..aa281d9a6d 100644 --- a/codex-rs/windows-sandbox-rs/src/winutil.rs +++ b/codex-rs/windows-sandbox-rs/src/winutil.rs @@ -178,7 +178,7 @@ fn well_known_sid_str(name: &str) -> Option<&'static str> { } } -fn sid_bytes_from_string(sid_str: &str) -> Result> { +pub(crate) fn sid_bytes_from_string(sid_str: &str) -> Result> { let sid_w = to_wide(OsStr::new(sid_str)); let mut psid: *mut std::ffi::c_void = std::ptr::null_mut(); if unsafe { ConvertStringSidToSidW(sid_w.as_ptr(), &mut psid) } == 0 {