From 665e5f45ab91d43ec3a49487fb9be90109dd2875 Mon Sep 17 00:00:00 2001 From: chess Date: Wed, 2 Sep 2026 20:54:17 +0000 Subject: [PATCH] Clean up Windows sandbox resources on app uninstall (#42375) ## What changed - Record the authenticated sandbox owner and observe package uninstall events across service restarts and session changes. - During uninstall, serialize setup and cleanup, disable sandbox accounts, stop their processes, and remove sandbox directories, firewall and WFP rules, hidden-user entries, accounts, and the sandbox group. - Restrict desktop-owned directory cleanup to pinned paths and perform it while impersonating the authenticated owner. - Detect missing or disabled sandbox accounts after interrupted cleanup and reprovision them before restoring network access. GitOrigin-RevId: 7d63fff7ddcff3eb01d018653146df12044bd277 --- codex-rs/Cargo.lock | 3 + codex-rs/windows-sandbox-rs/Cargo.toml | 1 + .../src/bin/setup_main/win.rs | 93 +++--- .../src/bin/setup_main/win/sandbox_users.rs | 23 +- .../src/elevated/runner_client.rs | 10 +- codex-rs/windows-sandbox-rs/src/hide_users.rs | 40 +++ codex-rs/windows-sandbox-rs/src/identity.rs | 21 ++ codex-rs/windows-sandbox-rs/src/lib.rs | 13 + .../windows-sandbox-rs/src/setup_mutex.rs | 83 ++++++ .../src/uninstall_windows.rs | 72 +++++ .../src/uninstall_windows/firewall.rs | 34 +++ .../src/uninstall_windows/principals.rs | 80 +++++ .../src/uninstall_windows/processes.rs | 220 ++++++++++++++ codex-rs/windows-sandbox-rs/src/wfp.rs | 37 ++- codex-rs/windows-sandbox-rs/src/wfp_setup.rs | 13 +- codex-rs/windows-sandbox-rs/src/winutil.rs | 51 ++++ codex-rs/windows-sandbox-service/Cargo.toml | 11 + .../src/installation_record.rs | 139 +++++++++ codex-rs/windows-sandbox-service/src/ipc.rs | 48 ++- .../src/ipc/authentication.rs | 43 ++- .../windows-sandbox-service/src/ipc/home.rs | 4 +- codex-rs/windows-sandbox-service/src/lib.rs | 4 + .../src/package_identity.rs | 2 +- .../src/package_lifecycle.rs | 279 ++++++++++++++++++ .../windows-sandbox-service/src/service.rs | 122 ++++++-- 25 files changed, 1349 insertions(+), 97 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/setup_mutex.rs create mode 100644 codex-rs/windows-sandbox-rs/src/uninstall_windows.rs create mode 100644 codex-rs/windows-sandbox-rs/src/uninstall_windows/firewall.rs create mode 100644 codex-rs/windows-sandbox-rs/src/uninstall_windows/principals.rs create mode 100644 codex-rs/windows-sandbox-rs/src/uninstall_windows/processes.rs create mode 100644 codex-rs/windows-sandbox-service/src/installation_record.rs create mode 100644 codex-rs/windows-sandbox-service/src/package_lifecycle.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ce9b1f2a12..02103c6c49 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4982,8 +4982,11 @@ dependencies = [ "codex-config", "codex-core", "codex-windows-sandbox", + "serde", + "serde_json", "tokio", "toml 0.9.11+spec-1.1.0", + "windows 0.58.0", "windows-sys 0.52.0", ] diff --git a/codex-rs/windows-sandbox-rs/Cargo.toml b/codex-rs/windows-sandbox-rs/Cargo.toml index 2e9aedefd4..401d9faa6c 100644 --- a/codex-rs/windows-sandbox-rs/Cargo.toml +++ b/codex-rs/windows-sandbox-rs/Cargo.toml @@ -74,6 +74,7 @@ features = [ "Win32_System_Memory", "Win32_System_Kernel", "Win32_System_Console", + "Win32_System_RemoteDesktop", "Win32_Storage_FileSystem", "Win32_System_Diagnostics_ToolHelp", "Win32_NetworkManagement_NetManagement", 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 5baa3d676a..f80efe4d12 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 @@ -11,6 +11,7 @@ use codex_windows_sandbox::SETUP_VERSION; use codex_windows_sandbox::SetupErrorCode; use codex_windows_sandbox::SetupErrorReport; use codex_windows_sandbox::SetupFailure; +use codex_windows_sandbox::acquire_sandbox_setup_lock; use codex_windows_sandbox::add_deny_write_ace; use codex_windows_sandbox::convert_string_sid_to_sid; use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance; @@ -18,6 +19,7 @@ use codex_windows_sandbox::ensure_allow_write_aces; use codex_windows_sandbox::extract_setup_failure; use codex_windows_sandbox::hide_newly_created_users; use codex_windows_sandbox::install_wfp_filters; +use codex_windows_sandbox::local_user_flags; use codex_windows_sandbox::log_note; use codex_windows_sandbox::log_writer; use codex_windows_sandbox::open_directory_no_reparse; @@ -27,6 +29,7 @@ use codex_windows_sandbox::resolve_sid; use codex_windows_sandbox::sandbox_bin_dir; use codex_windows_sandbox::sandbox_dir; use codex_windows_sandbox::sandbox_secrets_dir; +use codex_windows_sandbox::set_local_user_flags; use codex_windows_sandbox::setup_error_path; use codex_windows_sandbox::setup_log_writer; use codex_windows_sandbox::string_from_sid_bytes; @@ -52,6 +55,7 @@ use std::sync::mpsc; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE; use windows_sys::Win32::Security::ACL; use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW; use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W; @@ -75,6 +79,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; use windows_sys::Win32::Storage::FileSystem::WRITE_DAC; +use windows_sys::Win32::System::Threading::INFINITE; const DENY_ACCESS: i32 = 3; #[cfg(test)] @@ -655,15 +660,26 @@ fn run_read_acl_only(payload: &Payload, log: &mut dyn Write) -> Result<()> { Ok(()) } -fn provision_and_hide_sandbox_users( - payload: &Payload, - log: &mut dyn Write, - sbx_dir: &Path, -) -> Result<()> { +fn provision_sandbox(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Result<()> { + let _setup_lock = acquire_sandbox_setup_lock(INFINITE)?; + let mut repairing_disabled_accounts = false; + for username in [&payload.offline_username, &payload.online_username] { + if local_user_flags(username)?.is_some_and(|flags| flags & UF_ACCOUNTDISABLE != 0) { + repairing_disabled_accounts = true; + } + } + // Interrupted cleanup can leave one account missing and the other disabled. Keep any + // replacement disabled too until this repair has restored the network restrictions. + let new_user_flags = if repairing_disabled_accounts { + UF_ACCOUNTDISABLE + } else { + 0 + }; let provision_result = provision_sandbox_users( &payload.codex_home, &payload.offline_username, &payload.online_username, + new_user_flags, log, payload.mode, ); @@ -681,6 +697,36 @@ fn provision_and_hide_sandbox_users( payload.online_username.clone(), ]; hide_newly_created_users(&users, sbx_dir); + let offline_sid = resolve_sid(&payload.offline_username).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSidResolveFailed, + format!( + "resolve SID for offline user {} failed: {err}", + payload.offline_username + ), + )) + })?; + let offline_sid_str = string_from_sid_bytes(&offline_sid).map_err(anyhow::Error::msg)?; + configure_offline_sandbox_network(payload, &offline_sid_str, log)?; + let wfp_result = install_wfp_filters( + &payload.codex_home, + &payload.offline_username, + payload.otel.as_ref(), + |message| { + let _ = log_line(log, message); + }, + ); + if repairing_disabled_accounts { + // Ordinary setup keeps its best-effort WFP behavior. Recovery must not reopen logons + // after cleanup removed protections unless restoring those protections succeeded. + wfp_result?; + for username in [&payload.offline_username, &payload.online_username] { + let flags = local_user_flags(username)?.ok_or_else(|| { + anyhow::anyhow!("sandbox user {username} disappeared during repair") + })?; + set_local_user_flags(username, flags & !UF_ACCOUNTDISABLE)?; + } + } Ok(()) } @@ -714,14 +760,6 @@ fn configure_offline_sandbox_network( format!("ensure offline outbound block failed: {err}"), ))); } - install_wfp_filters( - &payload.codex_home, - &payload.offline_username, - payload.otel.as_ref(), - |message| { - let _ = log_line(log, message); - }, - ); Ok(()) } @@ -794,17 +832,7 @@ fn lock_sandbox_bin_dir(payload: &Payload, sandbox_group_sid: &[u8]) -> Result<( } fn run_provision_only(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Result<()> { - provision_and_hide_sandbox_users(payload, log, sbx_dir)?; - let offline_sid = resolve_sid(&payload.offline_username).map_err(|err| { - anyhow::Error::new(SetupFailure::new( - SetupErrorCode::HelperSidResolveFailed, - format!( - "resolve SID for offline user {} failed: {err}", - payload.offline_username - ), - )) - })?; - let offline_sid_str = string_from_sid_bytes(&offline_sid).map_err(anyhow::Error::msg)?; + provision_sandbox(payload, log, sbx_dir)?; let sandbox_group_sid = resolve_sandbox_users_group_sid().map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -813,8 +841,6 @@ 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)?; lock_persistent_sandbox_dirs(payload, &sandbox_group_sid)?; log_note("setup provisioning binary completed", Some(sbx_dir)); @@ -824,18 +850,8 @@ fn run_provision_only(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Result<()> { let refresh_only = payload.refresh_only; if !refresh_only { - provision_and_hide_sandbox_users(payload, log, sbx_dir)?; + provision_sandbox(payload, log, sbx_dir)?; } - let offline_sid = resolve_sid(&payload.offline_username).map_err(|err| { - anyhow::Error::new(SetupFailure::new( - SetupErrorCode::HelperSidResolveFailed, - format!( - "resolve SID for offline user {} failed: {err}", - payload.offline_username - ), - )) - })?; - let offline_sid_str = string_from_sid_bytes(&offline_sid).map_err(anyhow::Error::msg)?; let sandbox_group_sid = resolve_sandbox_users_group_sid().map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -853,9 +869,6 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?; let mut refresh_errors: Vec = Vec::new(); - if !refresh_only { - configure_offline_sandbox_network(payload, &offline_sid_str, log)?; - } // Deny-read ACEs must be present before the sandboxed command starts. Apply // them synchronously here instead of delegating them to the background diff --git a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/sandbox_users.rs b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/sandbox_users.rs index f3bc3f4a74..698d7e1cc7 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/sandbox_users.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win/sandbox_users.rs @@ -63,6 +63,7 @@ pub(super) fn provision_sandbox_users( codex_home: &Path, offline_username: &str, online_username: &str, + new_user_flags: u32, log: &mut dyn Write, mode: SetupMode, ) -> Result<()> { @@ -80,8 +81,8 @@ pub(super) fn provision_sandbox_users( )?; let offline_password = random_password(); let online_password = random_password(); - ensure_sandbox_user(offline_username, &offline_password, log)?; - ensure_sandbox_user(online_username, &online_password, log)?; + ensure_sandbox_user(offline_username, &offline_password, new_user_flags, log)?; + ensure_sandbox_user(online_username, &online_password, new_user_flags, log)?; write_secrets( codex_home, offline_username, @@ -93,13 +94,23 @@ pub(super) fn provision_sandbox_users( Ok(()) } -pub fn ensure_sandbox_user(username: &str, password: &str, log: &mut dyn Write) -> Result<()> { - ensure_local_user(username, password, log)?; +pub fn ensure_sandbox_user( + username: &str, + password: &str, + new_user_flags: u32, + log: &mut dyn Write, +) -> Result<()> { + ensure_local_user(username, password, new_user_flags, log)?; ensure_local_group_member(SANDBOX_USERS_GROUP, username)?; Ok(()) } -pub fn ensure_local_user(name: &str, password: &str, log: &mut dyn Write) -> Result<()> { +pub fn ensure_local_user( + name: &str, + password: &str, + new_user_flags: u32, + log: &mut dyn Write, +) -> Result<()> { let name_w = to_wide(OsStr::new(name)); let pwd_w = to_wide(OsStr::new(password)); unsafe { @@ -110,7 +121,7 @@ pub fn ensure_local_user(name: &str, password: &str, log: &mut dyn Write) -> Res usri1_priv: USER_PRIV_USER, usri1_home_dir: std::ptr::null_mut(), usri1_comment: std::ptr::null_mut(), - usri1_flags: UF_SCRIPT | UF_DONT_EXPIRE_PASSWD, + usri1_flags: UF_SCRIPT | UF_DONT_EXPIRE_PASSWD | new_user_flags, usri1_script_path: std::ptr::null_mut(), }; let status = NetUserAdd( 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 d04245c8ae..308a824237 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs @@ -31,6 +31,7 @@ use std::time::Instant; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS; use windows_sys::Win32::Foundation::DuplicateHandle; +use windows_sys::Win32::Foundation::ERROR_ACCOUNT_DISABLED; use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; @@ -97,7 +98,10 @@ pub(crate) struct RunnerTransport { } fn is_refreshable_windows_error(code: u32) -> bool { - matches!(code, ERROR_LOGON_FAILURE | ERROR_NO_SUCH_LOGON_SESSION) + matches!( + code, + ERROR_ACCOUNT_DISABLED | ERROR_LOGON_FAILURE | ERROR_NO_SUCH_LOGON_SESSION + ) } fn command_targets_windows_apps(command: &[String]) -> bool { @@ -455,6 +459,7 @@ mod tests { use crate::ipc_framed::ErrorPayload; use crate::ipc_framed::ErrorStage; use pretty_assertions::assert_eq; + use windows_sys::Win32::Foundation::ERROR_ACCOUNT_DISABLED; use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; @@ -463,6 +468,7 @@ mod tests { fn refreshable_sandbox_creds_error_recognizes_credential_and_child_start_failures() { assert_eq!( [ + ERROR_ACCOUNT_DISABLED, ERROR_LOGON_FAILURE, ERROR_NO_SUCH_LOGON_SESSION, ERROR_NOT_FOUND, @@ -472,7 +478,7 @@ mod tests { anyhow::Error::new(RunnerLogonError { code }).context("runner launch failed"); is_refreshable_sandbox_creds_error(&err, &[]) }), - [true, true, false] + [true, true, true, false] ); assert_eq!( diff --git a/codex-rs/windows-sandbox-rs/src/hide_users.rs b/codex-rs/windows-sandbox-rs/src/hide_users.rs index b919440022..c036a2c4d5 100644 --- a/codex-rs/windows-sandbox-rs/src/hide_users.rs +++ b/codex-rs/windows-sandbox-rs/src/hide_users.rs @@ -5,6 +5,9 @@ use anyhow::anyhow; use std::ffi::OsStr; use std::path::Path; use std::path::PathBuf; +use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND; +use windows_sys::Win32::Foundation::ERROR_PATH_NOT_FOUND; +use windows_sys::Win32::Foundation::ERROR_SUCCESS; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_HIDDEN; use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_SYSTEM; @@ -13,11 +16,14 @@ use windows_sys::Win32::Storage::FileSystem::INVALID_FILE_ATTRIBUTES; use windows_sys::Win32::Storage::FileSystem::SetFileAttributesW; use windows_sys::Win32::System::Registry::HKEY; use windows_sys::Win32::System::Registry::HKEY_LOCAL_MACHINE; +use windows_sys::Win32::System::Registry::KEY_SET_VALUE; use windows_sys::Win32::System::Registry::KEY_WRITE; use windows_sys::Win32::System::Registry::REG_DWORD; use windows_sys::Win32::System::Registry::REG_OPTION_NON_VOLATILE; use windows_sys::Win32::System::Registry::RegCloseKey; use windows_sys::Win32::System::Registry::RegCreateKeyExW; +use windows_sys::Win32::System::Registry::RegDeleteValueW; +use windows_sys::Win32::System::Registry::RegOpenKeyExW; use windows_sys::Win32::System::Registry::RegSetValueExW; const USERLIST_KEY_PATH: &str = @@ -35,6 +41,40 @@ pub fn hide_newly_created_users(usernames: &[String], log_base: &Path) { } } +pub(crate) fn unhide_sandbox_users(usernames: &[&str]) -> anyhow::Result<()> { + let mut key: HKEY = 0; + match unsafe { + RegOpenKeyExW( + HKEY_LOCAL_MACHINE, + to_wide(USERLIST_KEY_PATH).as_ptr(), + /*uloptions*/ 0, + KEY_SET_VALUE, + &mut key, + ) + } { + ERROR_FILE_NOT_FOUND | ERROR_PATH_NOT_FOUND => return Ok(()), + ERROR_SUCCESS => {} + error => return Err(anyhow!("open sandbox hidden-user registry key: {error}")), + } + + let mut errors = Vec::new(); + for username in usernames { + let status = unsafe { RegDeleteValueW(key, to_wide(username).as_ptr()) }; + match status { + ERROR_SUCCESS | ERROR_FILE_NOT_FOUND => {} + error => errors.push(format!("remove hidden sandbox user {username}: {error}")), + } + } + unsafe { + RegCloseKey(key); + } + if errors.is_empty() { + Ok(()) + } else { + Err(anyhow!(errors.join("; "))) + } +} + /// Best-effort: hides the current sandbox user's profile directory once it exists. /// /// Windows only creates profile directories when that user first logs in. diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index 6bcc285749..b2efe43ce2 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -1,6 +1,8 @@ use crate::dpapi; use crate::logging::debug_log; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; +use crate::setup::OFFLINE_USERNAME; +use crate::setup::ONLINE_USERNAME; use crate::setup::SandboxNetworkIdentity; use crate::setup::SandboxUserRecord; use crate::setup::SandboxUsersFile; @@ -12,6 +14,7 @@ use crate::setup::run_elevated_setup_with_proxy_settings; use crate::setup::run_setup_refresh_with_overrides_and_proxy_settings; use crate::setup::sandbox_users_path; use crate::setup::setup_marker_path; +use crate::winutil::local_user_flags; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; @@ -21,6 +24,7 @@ use std::collections::HashMap; use std::fs; use std::path::Path; use std::path::PathBuf; +use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE; #[derive(Debug, Clone)] struct SandboxIdentity { @@ -217,6 +221,23 @@ pub fn require_logon_sandbox_creds( } }; + if identity.is_some() { + // Cleanup may also have removed the group, so repair missing or disabled accounts before ACL + // refresh can fail, not only after a later logon reports ERROR_ACCOUNT_DISABLED. + for username in [OFFLINE_USERNAME, ONLINE_USERNAME] { + let needs_repair = match local_user_flags(username) { + Ok(Some(flags)) => flags & UF_ACCOUNTDISABLE != 0, + Ok(None) => true, + Err(_) => false, + }; + if needs_repair { + setup_reason = Some("sandbox account is missing or disabled".to_string()); + identity = None; + break; + } + } + } + if identity.is_none() { if let Some(reason) = &setup_reason { crate::logging::log_note( diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 9c6dd779e3..1f6d236da5 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -104,6 +104,8 @@ mod winutil; mod workspace_acl; mod deny_read_resolver; +#[cfg(target_os = "windows")] +mod uninstall_windows; #[cfg(target_os = "windows")] mod conpty; @@ -129,6 +131,9 @@ mod setup_error; #[cfg(target_os = "windows")] mod setup_launch; +#[cfg(target_os = "windows")] +mod setup_mutex; + #[cfg(target_os = "windows")] mod spawn_prep; @@ -354,6 +359,8 @@ pub use setup_error::setup_error_path; #[cfg(target_os = "windows")] pub use setup_error::write_setup_error_report; #[cfg(target_os = "windows")] +pub use setup_mutex::acquire_sandbox_setup_lock; +#[cfg(target_os = "windows")] pub use stdio_bridge::forward_sandbox_session_stdio; #[cfg(target_os = "windows")] #[doc(hidden)] @@ -381,6 +388,8 @@ pub use unified_exec::spawn_windows_sandbox_session_for_level; #[cfg(target_os = "windows")] pub use unified_exec::spawn_windows_sandbox_session_legacy; #[cfg(target_os = "windows")] +pub use uninstall_windows::clean_up_packaged_windows_sandbox; +#[cfg(target_os = "windows")] pub use wfp::install_wfp_filters_for_account; #[cfg(target_os = "windows")] pub use wfp_setup::install_wfp_filters; @@ -397,10 +406,14 @@ pub use winutil::SANDBOX_USERS_GROUP; #[cfg(target_os = "windows")] pub use winutil::ensure_sandbox_users_group; #[cfg(target_os = "windows")] +pub use winutil::local_user_flags; +#[cfg(target_os = "windows")] pub use winutil::quote_windows_arg; #[cfg(target_os = "windows")] pub use winutil::resolve_sid; #[cfg(target_os = "windows")] +pub use winutil::set_local_user_flags; +#[cfg(target_os = "windows")] pub use winutil::string_from_sid_bytes; #[cfg(target_os = "windows")] pub use winutil::to_wide; diff --git a/codex-rs/windows-sandbox-rs/src/setup_mutex.rs b/codex-rs/windows-sandbox-rs/src/setup_mutex.rs new file mode 100644 index 0000000000..dcb8f6f0c1 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/setup_mutex.rs @@ -0,0 +1,83 @@ +//! Serializes sandbox account and network changes across setup and uninstall. +//! The acquiring thread must hold the lock until those changes finish. + +use std::io; +use std::marker::PhantomData; +use std::os::windows::io::AsRawHandle; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::ptr::null_mut; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Foundation::HLOCAL; +use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Foundation::WAIT_ABANDONED; +use windows_sys::Win32::Foundation::WAIT_OBJECT_0; +use windows_sys::Win32::Foundation::WAIT_TIMEOUT; +use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW; +use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; +use windows_sys::Win32::System::Threading::CreateMutexW; +use windows_sys::Win32::System::Threading::ReleaseMutex; +use windows_sys::Win32::System::Threading::WaitForSingleObject; + +use crate::winutil::to_wide; + +/// Holds the machine-wide setup lock on the thread that acquired it. +#[must_use] +pub struct SandboxSetupLock { + handle: OwnedHandle, + _thread_bound: PhantomData<*mut ()>, +} + +impl Drop for SandboxSetupLock { + fn drop(&mut self) { + unsafe { + ReleaseMutex(self.handle.as_raw_handle() as HANDLE); + } + } +} + +/// Waits up to `timeout_ms` for exclusive access by LocalSystem or an elevated administrator. +pub fn acquire_sandbox_setup_lock(timeout_ms: u32) -> Result { + let sddl = to_wide("D:P(A;;GA;;;SY)(A;;GA;;;BA)"); + let mut descriptor = null_mut(); + if unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + /*stringsdrevision*/ 1, + &mut descriptor, + null_mut(), + ) + } == 0 + { + return Err(io::Error::last_os_error()).context("create sandbox setup mutex security"); + } + let attributes = SECURITY_ATTRIBUTES { + nLength: size_of::() as u32, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + let name = to_wide(r"Global\CodexSandboxSetup"); + let handle = unsafe { + CreateMutexW(&attributes, /*binitialowner*/ 0, name.as_ptr()) + }; + let result = if handle == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(unsafe { OwnedHandle::from_raw_handle(handle as _) }) + }; + unsafe { LocalFree(descriptor as HLOCAL) }; + let handle = result.context("open sandbox setup mutex")?; + match unsafe { WaitForSingleObject(handle.as_raw_handle() as HANDLE, timeout_ms) } { + // A crashed owner releases the lock; setup still repairs any incomplete account changes. + WAIT_OBJECT_0 | WAIT_ABANDONED => Ok(SandboxSetupLock { + handle, + _thread_bound: PhantomData, + }), + WAIT_TIMEOUT => bail!("timed out waiting for sandbox setup mutex after {timeout_ms} ms"), + _ => Err(io::Error::last_os_error()).context("wait for sandbox setup mutex"), + } +} diff --git a/codex-rs/windows-sandbox-rs/src/uninstall_windows.rs b/codex-rs/windows-sandbox-rs/src/uninstall_windows.rs new file mode 100644 index 0000000000..ff5b7797c2 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/uninstall_windows.rs @@ -0,0 +1,72 @@ +//! Stops sandbox work before removing its protections, then continues independent cleanup. + +use std::path::Path; + +use anyhow::Result; +use anyhow::anyhow; + +use crate::setup::OFFLINE_USERNAME; +use crate::setup::ONLINE_USERNAME; + +mod firewall; +mod principals; +mod processes; + +/// Removes sandbox resources created for one authenticated packaged installation. +/// Keep a supplied home and its ancestors pinned until `clean_up_desktop` starts. +/// That callback removes user-owned desktop files while the sandbox accounts remain disabled. +pub fn clean_up_packaged_windows_sandbox( + codex_home: Option<&Path>, + clean_up_desktop: impl FnOnce() -> Result<()>, +) -> Result<()> { + let _setup_lock = crate::setup_mutex::acquire_sandbox_setup_lock(/*timeout_ms*/ 5_000)?; + let mut errors = Vec::new(); + let mut users = principals::DisabledSandboxUsers::default(); + if let Err(error) = users.disable().and_then(|()| processes::stop(&users)) { + errors.push(format!("{error:#}")); + // No network protections have been removed, so failed preparation can restore these flags. + if let Err(error) = users.restore() { + errors.push(format!("{error:#}")); + } + return Err(anyhow!(errors.join("; "))); + } + + if let Some(codex_home) = codex_home { + for directory in [ + crate::setup::sandbox_dir(codex_home), + crate::setup::sandbox_secrets_dir(codex_home), + crate::setup::sandbox_bin_dir(codex_home), + ] { + match std::fs::remove_dir_all(&directory) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + errors.push(format!("remove {}: {error}", directory.display())); + } + } + } + } + + if let Err(error) = clean_up_desktop() { + errors.push(format!("{error:#}")); + } + + for result in [ + crate::wfp::remove_wfp_filters(), + firewall::cleanup_firewall_rules(), + principals::remove_sandbox_principal("CodexSandboxUsers"), + crate::hide_users::unhide_sandbox_users(&[OFFLINE_USERNAME, ONLINE_USERNAME]), + // Keep accounts disabled and setup locked until shared cleanup and account deletion finish. + principals::remove_sandbox_principal(OFFLINE_USERNAME), + principals::remove_sandbox_principal(ONLINE_USERNAME), + ] { + if let Err(error) = result { + errors.push(format!("{error:#}")); + } + } + if errors.is_empty() { + Ok(()) + } else { + Err(anyhow!(errors.join("; "))) + } +} diff --git a/codex-rs/windows-sandbox-rs/src/uninstall_windows/firewall.rs b/codex-rs/windows-sandbox-rs/src/uninstall_windows/firewall.rs new file mode 100644 index 0000000000..876405b58e --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/uninstall_windows/firewall.rs @@ -0,0 +1,34 @@ +//! Removes known sandbox firewall rules while preserving unrelated rules. + +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2; +use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2; +use windows::Win32::System::Com::CLSCTX_INPROC_SERVER; +use windows::Win32::System::Com::CoCreateInstance; +use windows::core::BSTR; + +pub(super) fn cleanup_firewall_rules() -> Result<()> { + let policy: INetFwPolicy2 = unsafe { + CoCreateInstance(&NetFwPolicy2, /*punkouter*/ None, CLSCTX_INPROC_SERVER) + .context("access firewall policy for sandbox uninstall")? + }; + let rules = unsafe { policy.Rules() }.context("access sandbox firewall rules")?; + let mut errors = Vec::new(); + for name in [ + "codex_sandbox_offline_block_outbound", + "codex_sandbox_offline_block_loopback_tcp", + "codex_sandbox_offline_block_loopback_udp", + "codex_sandbox_offline_allow_loopback_proxy", + ] { + if let Err(error) = unsafe { rules.Remove(&BSTR::from(name)) } { + errors.push(format!("remove sandbox firewall rule {name}: {error}")); + } + } + if errors.is_empty() { + Ok(()) + } else { + Err(anyhow!(errors.join("; "))) + } +} diff --git a/codex-rs/windows-sandbox-rs/src/uninstall_windows/principals.rs b/codex-rs/windows-sandbox-rs/src/uninstall_windows/principals.rs new file mode 100644 index 0000000000..8d52e606af --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/uninstall_windows/principals.rs @@ -0,0 +1,80 @@ +//! Disables sandbox accounts for cleanup and removes sandbox principals. +//! Callers must restore original flags if preparation for cleanup fails. + +use std::ptr::null; + +use anyhow::Result; +use anyhow::bail; +use windows_sys::Win32::NetworkManagement::NetManagement as network; + +use crate::setup::OFFLINE_USERNAME; +use crate::setup::ONLINE_USERNAME; +use crate::winutil::local_user_flags; +use crate::winutil::resolve_sid; +use crate::winutil::set_local_user_flags; +use crate::winutil::to_wide; + +#[derive(Default)] +pub(super) struct DisabledSandboxUsers { + users: Vec, +} + +struct SandboxUser { + name: &'static str, + original_flags: u32, + sid: Vec, +} + +impl DisabledSandboxUsers { + pub(super) fn disable(&mut self) -> Result<()> { + for name in [OFFLINE_USERNAME, ONLINE_USERNAME] { + let Some(original_flags) = local_user_flags(name)? else { + continue; + }; + let sid = resolve_sid(name)?; + + self.users.push(SandboxUser { + name, + original_flags, + sid, + }); + set_local_user_flags(name, original_flags | network::UF_ACCOUNTDISABLE)?; + } + Ok(()) + } + + pub(super) fn sids(&self) -> impl Iterator { + self.users.iter().map(|user| user.sid.as_slice()) + } + + pub(super) fn restore(&self) -> Result<()> { + let mut errors = Vec::new(); + for user in &self.users { + if let Err(error) = set_local_user_flags(user.name, user.original_flags) + && error + .downcast_ref::() + .and_then(std::io::Error::raw_os_error) + != Some(network::NERR_UserNotFound as i32) + { + errors.push(format!("{error:#}")); + } + } + if !errors.is_empty() { + bail!("{}", errors.join("; ")); + } + Ok(()) + } +} + +pub(super) fn remove_sandbox_principal(name: &str) -> Result<()> { + let name_wide = to_wide(name); + let status = if name == "CodexSandboxUsers" { + unsafe { network::NetLocalGroupDel(null(), name_wide.as_ptr()) } + } else { + unsafe { network::NetUserDel(null(), name_wide.as_ptr()) } + }; + match status { + network::NERR_Success | network::NERR_GroupNotFound | network::NERR_UserNotFound => Ok(()), + status => bail!("remove local sandbox principal {name}: {status}"), + } +} diff --git a/codex-rs/windows-sandbox-rs/src/uninstall_windows/processes.rs b/codex-rs/windows-sandbox-rs/src/uninstall_windows/processes.rs new file mode 100644 index 0000000000..300d17579b --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/uninstall_windows/processes.rs @@ -0,0 +1,220 @@ +//! Stops sandbox-account processes and waits for their logon tokens to be released. + +use std::ffi::c_void; +use std::io; +use std::os::windows::io::AsRawHandle; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::ptr::null_mut; +use std::time::Duration; +use std::time::Instant; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER; +use windows_sys::Win32::Foundation::ERROR_NO_SUCH_LOGON_SESSION; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Foundation::LUID; +use windows_sys::Win32::Foundation::STATUS_SUCCESS; +use windows_sys::Win32::Foundation::WAIT_OBJECT_0; +use windows_sys::Win32::Foundation::WAIT_TIMEOUT; +use windows_sys::Win32::Security::Authentication::Identity::LsaEnumerateLogonSessions; +use windows_sys::Win32::Security::Authentication::Identity::LsaFreeReturnBuffer; +use windows_sys::Win32::Security::Authentication::Identity::LsaGetLogonSessionData; +use windows_sys::Win32::Security::Authentication::Identity::LsaNtStatusToWinError; +use windows_sys::Win32::Security::Authentication::Identity::SECURITY_LOGON_SESSION_DATA; +use windows_sys::Win32::Security::EqualSid; +use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::System::RemoteDesktop::WTS_CURRENT_SERVER_HANDLE; +use windows_sys::Win32::System::RemoteDesktop::WTS_PROCESS_INFOW; +use windows_sys::Win32::System::RemoteDesktop::WTSEnumerateProcessesW; +use windows_sys::Win32::System::RemoteDesktop::WTSFreeMemory; +use windows_sys::Win32::System::Threading::OpenProcess; +use windows_sys::Win32::System::Threading::OpenProcessToken; +use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; +use windows_sys::Win32::System::Threading::PROCESS_SYNCHRONIZE; +use windows_sys::Win32::System::Threading::PROCESS_TERMINATE; +use windows_sys::Win32::System::Threading::TerminateProcess; +use windows_sys::Win32::System::Threading::WaitForSingleObject; + +use super::principals::DisabledSandboxUsers; +use crate::token::get_user_sid_bytes; + +pub(super) fn stop(users: &DisabledSandboxUsers) -> Result<()> { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + let processes = sandbox_processes(users)?; + for process in &processes { + let handle = process.as_raw_handle() as HANDLE; + if unsafe { + TerminateProcess(handle, /*uexitcode*/ 1) + } == 0 + { + let error = io::Error::last_os_error(); + if unsafe { + WaitForSingleObject(handle, /*dwmilliseconds*/ 0) + } != WAIT_OBJECT_0 + { + return Err(error).context("stop sandbox process before uninstall"); + } + } + } + for process in &processes { + let remaining = deadline.saturating_duration_since(Instant::now()); + // The five-second deadline keeps milliseconds within a DWORD. + match unsafe { + WaitForSingleObject( + process.as_raw_handle() as HANDLE, + remaining.as_millis() as u32, + ) + } { + WAIT_OBJECT_0 => {} + WAIT_TIMEOUT => bail!("sandbox process did not exit before the uninstall deadline"), + _ => { + return Err(io::Error::last_os_error()) + .context("wait for sandbox process exit"); + } + } + } + drop(processes); + + // A token can outlive its process or exist before a runner starts. Account disable does + // not revoke it, so release our handles and check for tokens before removing protections. + if !has_sandbox_logon_session(users)? { + return Ok(()); + } + if Instant::now() >= deadline { + bail!("sandbox processes or logon tokens remain after the uninstall deadline"); + } + // Repeat to catch descendants created during the previous snapshot. + std::thread::sleep( + Duration::from_millis(50).min(deadline.saturating_duration_since(Instant::now())), + ); + } +} + +fn sandbox_processes(users: &DisabledSandboxUsers) -> Result> { + let mut process_info = null_mut(); + let mut count = 0; + if unsafe { + WTSEnumerateProcessesW( + WTS_CURRENT_SERVER_HANDLE, + /*reserved*/ 0, + /*version*/ 1, + &mut process_info, + &mut count, + ) + } == 0 + { + return Err(io::Error::last_os_error()) + .context("enumerate sandbox processes for uninstall"); + } + let process_info = ProcessList(process_info); + let mut handles = Vec::new(); + if count == 0 { + return Ok(handles); + } + for process in unsafe { std::slice::from_raw_parts(process_info.0, count as usize) } { + if !is_sandbox_user(users, process.pUserSid) { + continue; + } + let handle = unsafe { + OpenProcess( + PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE | PROCESS_SYNCHRONIZE, + /*binherithandle*/ 0, + process.ProcessId, + ) + }; + if handle == 0 { + let error = io::Error::last_os_error(); + if error.raw_os_error() == Some(ERROR_INVALID_PARAMETER as i32) { + continue; + } + return Err(error).context("open sandbox process for uninstall"); + } + let handle = unsafe { OwnedHandle::from_raw_handle(handle as *mut c_void) }; + let mut token = 0; + if unsafe { OpenProcessToken(handle.as_raw_handle() as HANDLE, TOKEN_QUERY, &mut token) } + == 0 + { + let error = io::Error::last_os_error(); + if unsafe { + WaitForSingleObject(handle.as_raw_handle() as HANDLE, /*dwmilliseconds*/ 0) + } == WAIT_OBJECT_0 + { + continue; + } + return Err(error).context("identify sandbox process before uninstall"); + } + let token = unsafe { OwnedHandle::from_raw_handle(token as *mut c_void) }; + let sid = unsafe { get_user_sid_bytes(token.as_raw_handle() as HANDLE) }?; + // A PID can be reused after enumeration. Check the opened process before terminating it. + if users.sids().any(|user_sid| user_sid == sid.as_slice()) { + handles.push(handle); + } + } + Ok(handles) +} + +fn has_sandbox_logon_session(users: &DisabledSandboxUsers) -> Result { + let mut count = 0; + let mut logons = null_mut(); + let status = unsafe { LsaEnumerateLogonSessions(&mut count, &mut logons) }; + if status != STATUS_SUCCESS { + bail!("enumerate sandbox logon sessions for uninstall: {status:#x}"); + } + let logons = LsaBuffer(logons.cast()); + if count == 0 { + return Ok(false); + } + for logon in unsafe { std::slice::from_raw_parts(logons.0.cast::(), count as usize) } { + // LocalSystem uses the reserved logon ID 0:0x3e7 and has no normal logon data. + if logon.LowPart == 0x3e7 && logon.HighPart == 0 { + continue; + } + let mut data = null_mut(); + let status = unsafe { LsaGetLogonSessionData(logon, &mut data) }; + if status != STATUS_SUCCESS { + if unsafe { LsaNtStatusToWinError(status) } == ERROR_NO_SUCH_LOGON_SESSION { + continue; + } + bail!("read sandbox logon session for uninstall: {status:#x}"); + } + // Keep protections when LSA returns no session data. + if data.is_null() { + return Ok(true); + } + let data = LsaBuffer(data.cast()); + let data = unsafe { &*data.0.cast::() }; + // LSA also lists sessions without a user SID, even before sandbox accounts exist. + // Only sessions identified as sandbox users are evidence of remaining sandbox tokens. + if is_sandbox_user(users, data.Sid) { + return Ok(true); + } + } + Ok(false) +} + +fn is_sandbox_user(users: &DisabledSandboxUsers, sid: *mut c_void) -> bool { + !sid.is_null() + && users + .sids() + .any(|user_sid| unsafe { EqualSid(sid, user_sid.as_ptr() as *mut c_void) } != 0) +} + +struct ProcessList(*mut WTS_PROCESS_INFOW); + +impl Drop for ProcessList { + fn drop(&mut self) { + unsafe { WTSFreeMemory(self.0.cast()) }; + } +} + +struct LsaBuffer(*mut c_void); + +impl Drop for LsaBuffer { + fn drop(&mut self) { + unsafe { LsaFreeReturnBuffer(self.0) }; + } +} diff --git a/codex-rs/windows-sandbox-rs/src/wfp.rs b/codex-rs/windows-sandbox-rs/src/wfp.rs index 9291b6eb99..d992bcdd93 100644 --- a/codex-rs/windows-sandbox-rs/src/wfp.rs +++ b/codex-rs/windows-sandbox-rs/src/wfp.rs @@ -9,6 +9,8 @@ use std::ptr::null_mut; use windows_sys::Win32::Foundation::FWP_E_ALREADY_EXISTS; use windows_sys::Win32::Foundation::FWP_E_FILTER_NOT_FOUND; use windows_sys::Win32::Foundation::FWP_E_NOT_FOUND; +use windows_sys::Win32::Foundation::FWP_E_PROVIDER_NOT_FOUND; +use windows_sys::Win32::Foundation::FWP_E_SUBLAYER_NOT_FOUND; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::LocalFree; @@ -43,7 +45,9 @@ use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmEngineO use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmFilterAdd0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmFilterDeleteByKey0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmProviderAdd0; +use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmProviderDeleteByKey0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmSubLayerAdd0; +use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmSubLayerDeleteByKey0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmTransactionAbort0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmTransactionBegin0; use windows_sys::Win32::NetworkManagement::WindowsFilteringPlatform::FwpmTransactionCommit0; @@ -75,9 +79,10 @@ const SUBLAYER_KEY: GUID = GUID::from_u128(0xe65054fd_4d32_4c7c_95ef_621f0cf6431 /// Installs the persistent Codex WFP filters for `account`. /// /// This is intended to run from the already-elevated setup helper. Callers -/// should treat any returned error as non-fatal to the rest of setup. +/// may continue ordinary setup after an error, but must restore these filters +/// before re-enabling accounts left disabled by interrupted cleanup. pub fn install_wfp_filters_for_account(account: &str) -> Result { - let engine = Engine::open()?; + let engine = Engine::open(INFINITE)?; let mut transaction = engine.begin_transaction()?; ensure_provider(engine.handle)?; ensure_sublayer(engine.handle)?; @@ -94,20 +99,44 @@ pub fn install_wfp_filters_for_account(account: &str) -> Result { Ok(installed_filter_count) } +pub(crate) fn remove_wfp_filters() -> Result<()> { + // Leave time for other cleanup if a WFP policy writer holds the transaction lock. + let engine = Engine::open(/*transaction_wait_timeout_ms*/ 1_000)?; + let mut transaction = engine.begin_transaction()?; + for spec in FILTER_SPECS { + delete_filter_if_present(engine.handle, &spec.key)?; + } + for (result, operation, missing) in [ + ( + unsafe { FwpmSubLayerDeleteByKey0(engine.handle, &SUBLAYER_KEY) }, + "FwpmSubLayerDeleteByKey0", + FWP_E_SUBLAYER_NOT_FOUND as u32, + ), + ( + unsafe { FwpmProviderDeleteByKey0(engine.handle, &PROVIDER_KEY) }, + "FwpmProviderDeleteByKey0", + FWP_E_PROVIDER_NOT_FOUND as u32, + ), + ] { + ensure_success_or(result, operation, &[missing, FWP_E_NOT_FOUND as u32])?; + } + transaction.commit() +} + /// Owns an open WFP engine handle and closes it on drop. struct Engine { handle: HANDLE, } impl Engine { - fn open() -> Result { + fn open(transaction_wait_timeout_ms: u32) -> Result { let session_name = to_wide(OsStr::new(SESSION_NAME)); let mut session: FWPM_SESSION0 = unsafe { zeroed() }; session.displayData = FWPM_DISPLAY_DATA0 { name: session_name.as_ptr() as *mut _, description: null_mut(), }; - session.txnWaitTimeoutInMSec = INFINITE; + session.txnWaitTimeoutInMSec = transaction_wait_timeout_ms; let mut handle = HANDLE::default(); let result = unsafe { diff --git a/codex-rs/windows-sandbox-rs/src/wfp_setup.rs b/codex-rs/windows-sandbox-rs/src/wfp_setup.rs index 688878583b..b3d6c4d339 100644 --- a/codex-rs/windows-sandbox-rs/src/wfp_setup.rs +++ b/codex-rs/windows-sandbox-rs/src/wfp_setup.rs @@ -128,7 +128,8 @@ pub fn install_wfp_filters( offline_username: &str, otel: Option<&StatsigMetricsSettings>, mut log: F, -) where +) -> Result<()> +where F: FnMut(&str), { let metric = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { @@ -147,9 +148,7 @@ pub fn install_wfp_filters( } Ok(Err(err)) => { let error = err.to_string(); - log(&format!( - "WFP setup failed for {offline_username}: {error}; continuing elevated setup" - )); + log(&format!("WFP setup failed for {offline_username}: {error}")); WfpSetupMetric { outcome: WfpSetupMetricOutcome::Failure, target_account: offline_username.to_string(), @@ -160,7 +159,7 @@ pub fn install_wfp_filters( Err(panic_payload) => { let error = panic_payload_to_string(panic_payload); log(&format!( - "WFP setup panicked for {offline_username}: {error}; continuing elevated setup" + "WFP setup panicked for {offline_username}: {error}" )); WfpSetupMetric { outcome: WfpSetupMetricOutcome::Failure, @@ -172,4 +171,8 @@ pub fn install_wfp_filters( }; emit_wfp_setup_metric_safely(codex_home, otel, offline_username, &metric, &mut log); + match metric.error { + Some(error) => Err(anyhow::anyhow!(error)), + None => Ok(()), + } } diff --git a/codex-rs/windows-sandbox-rs/src/winutil.rs b/codex-rs/windows-sandbox-rs/src/winutil.rs index 704cce5ec6..502d9024bf 100644 --- a/codex-rs/windows-sandbox-rs/src/winutil.rs +++ b/codex-rs/windows-sandbox-rs/src/winutil.rs @@ -1,3 +1,4 @@ +use anyhow::Context; use anyhow::Result; use std::ffi::OsStr; use std::os::windows::ffi::OsStrExt; @@ -7,7 +8,13 @@ use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::LocalFree; use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_INFO_1; use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success; +use windows_sys::Win32::NetworkManagement::NetManagement::NERR_UserNotFound; +use windows_sys::Win32::NetworkManagement::NetManagement::NetApiBufferFree; use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAdd; +use windows_sys::Win32::NetworkManagement::NetManagement::NetUserGetInfo; +use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo; +use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1; +use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1008; use windows_sys::Win32::Security::Authorization::ConvertSidToStringSidW; use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW; use windows_sys::Win32::Security::CopySid; @@ -135,6 +142,50 @@ const SID_AUTHENTICATED_USERS: &str = "S-1-5-11"; const SID_EVERYONE: &str = "S-1-1-0"; const SID_SYSTEM: &str = "S-1-5-18"; +pub fn local_user_flags(name: &str) -> Result> { + let name_wide = to_wide(name); + let mut buffer = std::ptr::null_mut(); + let status = unsafe { + NetUserGetInfo( + std::ptr::null(), + name_wide.as_ptr(), + /*level*/ 1, + &mut buffer, + ) + }; + if status == NERR_UserNotFound { + return Ok(None); + } + if status != NERR_Success { + return Err(std::io::Error::from_raw_os_error(status as i32)) + .with_context(|| format!("read local sandbox user {name}")); + } + let flags = unsafe { (*buffer.cast::()).usri1_flags }; + unsafe { NetApiBufferFree(buffer.cast()) }; + Ok(Some(flags)) +} + +pub fn set_local_user_flags(name: &str, flags: u32) -> Result<()> { + let name_wide = to_wide(name); + let info = USER_INFO_1008 { + usri1008_flags: flags, + }; + let status = unsafe { + NetUserSetInfo( + std::ptr::null(), + name_wide.as_ptr(), + /*level*/ 1008, + (&raw const info).cast(), + std::ptr::null_mut(), + ) + }; + if status != NERR_Success { + return Err(std::io::Error::from_raw_os_error(status as i32)) + .with_context(|| format!("set local sandbox user {name} flags")); + } + Ok(()) +} + pub fn ensure_sandbox_users_group() -> Result> { const ERROR_ALIAS_EXISTS: u32 = 1379; const NERR_GROUP_EXISTS: u32 = 2223; diff --git a/codex-rs/windows-sandbox-service/Cargo.toml b/codex-rs/windows-sandbox-service/Cargo.toml index 87793e0309..29deeddfb5 100644 --- a/codex-rs/windows-sandbox-service/Cargo.toml +++ b/codex-rs/windows-sandbox-service/Cargo.toml @@ -22,9 +22,18 @@ codex-cloud-config = { workspace = true } codex-config = { workspace = true } codex-core = { workspace = true } codex-windows-sandbox = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } tokio = { workspace = true, features = ["macros", "rt", "sync", "time"] } toml = { workspace = true } +[target.'cfg(windows)'.dependencies.windows] +version = "0.58" +features = [ + "ApplicationModel", + "Win32_System_WinRT", +] + [target.'cfg(windows)'.dependencies.windows-sys] version = "0.52" features = [ @@ -40,6 +49,8 @@ features = [ "Win32_System_IO", "Win32_System_Memory", "Win32_System_Pipes", + "Win32_System_Registry", + "Win32_System_RemoteDesktop", "Win32_System_Services", "Win32_System_SystemServices", "Win32_System_Threading", diff --git a/codex-rs/windows-sandbox-service/src/installation_record.rs b/codex-rs/windows-sandbox-service/src/installation_record.rs new file mode 100644 index 0000000000..769626bab8 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/installation_record.rs @@ -0,0 +1,139 @@ +//! Persists the authenticated sandbox owner across service restarts and package updates. + +use std::ffi::OsString; +use std::io; +use std::mem::size_of; +use std::os::windows::ffi::OsStringExt; +use std::path::Path; +use std::path::PathBuf; +use std::ptr; + +use anyhow::Context; +use anyhow::Result; +use anyhow::ensure; +use codex_windows_sandbox::to_wide; +use codex_windows_sandbox::validate_local_directory_path; +use serde::Deserialize; +use serde::Serialize; +use windows_sys::Win32::Foundation as foundation; +use windows_sys::Win32::System::Registry as registry; +use windows_sys::Win32::UI::Shell::GetUserProfileDirectoryW; + +// Package updates can replace the service key, so keep this record outside it. +const INSTALLATION_KEY: &str = r"SOFTWARE\OpenAI\Codex\WindowsSandboxService"; +const INSTALLATION_VALUE: &str = "ProvisionedInstallation"; +const MAX_VALUE_UNITS: usize = 4096; +const DESKTOP_INSTALLATION_MARKER: &str = ".desktop-created"; + +#[derive(Clone, Deserialize, Serialize)] +pub(crate) struct DesktopInstallation { + pub(crate) created_codex_home: bool, + pub(crate) cache_home: PathBuf, +} + +#[derive(Clone, Deserialize, Serialize)] +pub(crate) struct InstallationRecord { + pub(crate) user_sid: String, + pub(crate) codex_home: PathBuf, + pub(crate) session_id: u32, + #[serde(default)] + pub(crate) desktop_installation: Option, +} + +// Read this app-owned marker while impersonating the authenticated user. The +// desktop writes it only when creating the home. Cache cleanup does not need ownership. +pub(crate) fn read_desktop_installation( + home: &Path, + user_token: foundation::HANDLE, +) -> Result { + let mut length = 0; + unsafe { GetUserProfileDirectoryW(user_token, ptr::null_mut(), &mut length) }; + let mut profile = vec![0_u16; length as usize]; + if unsafe { GetUserProfileDirectoryW(user_token, profile.as_mut_ptr(), &mut length) } == 0 { + return Err(io::Error::last_os_error()).context("read sandbox owner's profile directory"); + } + let cache_home = + PathBuf::from(OsString::from_wide(&profile[..length as usize - 1])).join(".cache"); + validate_local_directory_path(&cache_home)?; + Ok(DesktopInstallation { + created_codex_home: home.join(DESKTOP_INSTALLATION_MARKER).is_file(), + cache_home, + }) +} + +pub(crate) fn load() -> Result> { + let mut value = [0_u16; MAX_VALUE_UNITS]; + let mut value_length = std::mem::size_of_val(&value) as u32; + let status = unsafe { + registry::RegGetValueW( + registry::HKEY_LOCAL_MACHINE, + to_wide(INSTALLATION_KEY).as_ptr(), + to_wide(INSTALLATION_VALUE).as_ptr(), + registry::RRF_RT_REG_SZ, + ptr::null_mut(), + value.as_mut_ptr().cast(), + &mut value_length, + ) + }; + match status { + foundation::ERROR_FILE_NOT_FOUND | foundation::ERROR_PATH_NOT_FOUND => return Ok(None), + foundation::ERROR_SUCCESS => {} + status => { + return Err(io::Error::from_raw_os_error(status as i32)) + .context("read protected sandbox installation record"); + } + } + ensure!( + value_length as usize <= std::mem::size_of_val(&value) + && value_length.is_multiple_of(size_of::() as u32), + "sandbox installation record has an invalid length" + ); + let value = value[..value_length as usize / size_of::()] + .strip_suffix(&[0]) + .context("sandbox installation record is not null-terminated")?; + let value = String::from_utf16(value).context("decode sandbox installation record")?; + let record = serde_json::from_str(&value).context("parse sandbox installation record")?; + Ok(Some(record)) +} + +pub(crate) fn save(record: &InstallationRecord) -> Result<()> { + let value = to_wide( + serde_json::to_string(record).context("serialize protected sandbox installation record")?, + ); + ensure!( + value.len() <= MAX_VALUE_UNITS, + "sandbox installation record exceeds its size limit" + ); + let status = unsafe { + registry::RegSetKeyValueW( + registry::HKEY_LOCAL_MACHINE, + to_wide(INSTALLATION_KEY).as_ptr(), + to_wide(INSTALLATION_VALUE).as_ptr(), + registry::REG_SZ, + value.as_ptr().cast(), + (value.len() * size_of::()) as u32, + ) + }; + if status == foundation::ERROR_SUCCESS { + Ok(()) + } else { + Err(io::Error::from_raw_os_error(status as i32)) + .context("persist protected sandbox installation record") + } +} + +pub(crate) fn remove() -> Result<()> { + let status = unsafe { + registry::RegDeleteKeyW( + registry::HKEY_LOCAL_MACHINE, + to_wide(INSTALLATION_KEY).as_ptr(), + ) + }; + match status { + foundation::ERROR_SUCCESS + | foundation::ERROR_FILE_NOT_FOUND + | foundation::ERROR_PATH_NOT_FOUND => Ok(()), + status => Err(io::Error::from_raw_os_error(status as i32)) + .context("remove protected sandbox installation record"), + } +} diff --git a/codex-rs/windows-sandbox-service/src/ipc.rs b/codex-rs/windows-sandbox-service/src/ipc.rs index 101628b3ff..22c34ece14 100644 --- a/codex-rs/windows-sandbox-service/src/ipc.rs +++ b/codex-rs/windows-sandbox-service/src/ipc.rs @@ -21,9 +21,8 @@ use codex_windows_sandbox::sandbox_setup_is_complete_with_settings; use codex_windows_sandbox::string_from_sid_bytes; use codex_windows_sandbox::to_wide; use codex_windows_sandbox::write_provisioning_frame; -use home::OwnedHandle; -#[cfg(test)] -use home::pin_existing_ancestors; +pub(crate) use home::OwnedHandle; +pub(crate) use home::pin_existing_ancestors; #[cfg(test)] use request::ProvisioningRequest; use request::validate_request; @@ -43,6 +42,8 @@ use windows_sys::Win32::Security::Authorization as authorization; use windows_sys::Win32::Storage::FileSystem as filesystem; use windows_sys::Win32::System::Pipes as pipes; +use crate::installation_record::InstallationRecord; + pub(crate) const PIPE_NAME: &str = codex_windows_sandbox::SANDBOX_PROVISIONING_PIPE_NAME; const MAX_REQUEST_BYTES: usize = 4096; @@ -64,7 +65,12 @@ enum PipeConnection { Disconnected, } -pub(crate) fn run(shutdown: Arc, on_ready: impl FnOnce() -> Result<()>) -> Result<()> { +pub(crate) fn run( + shutdown: Arc, + on_ready: impl FnOnce() -> Result<()>, + on_authenticated_user: impl Fn(&InstallationRecord, OwnedHandle) -> Result<()>, + on_session_change: impl Fn() -> Result<()>, +) -> Result<()> { let sandbox_sid = ensure_sandbox_users_group()?; let sid_string = string_from_sid_bytes(&sandbox_sid).map_err(anyhow::Error::msg)?; let sddl = pipe_security_descriptor(&sid_string); @@ -109,12 +115,15 @@ pub(crate) fn run(shutdown: Arc, on_ready: impl FnOnce() -> Result<( on_ready().context("publish provisioning listener readiness")?; while !shutdown.load(Ordering::Acquire) { - if accept_pipe_connection(pipe.0)? == PipeConnection::Disconnected { - continue; - } + let connection = accept_pipe_connection(pipe.0)?; if shutdown.load(Ordering::Acquire) { break; } + // Session-change wakeups close the pipe immediately and can arrive disconnected. + on_session_change().context("restore the signed-in user's uninstall listener")?; + if connection == PipeConnection::Disconnected { + continue; + } let authorized_process = match crate::package_identity::authorize_client_process(pipe.0) { Ok(process) => process, @@ -123,7 +132,13 @@ pub(crate) fn run(shutdown: Arc, on_ready: impl FnOnce() -> Result<( continue; } }; - let result = handle_request(pipe.0, &authorized_process, &sandbox_sid, &shutdown); + let result = handle_request( + pipe.0, + &authorized_process, + &sandbox_sid, + &shutdown, + &on_authenticated_user, + ); let response = match result { Ok(response) => response, Err(error) if error.is::() => { @@ -241,6 +256,7 @@ fn handle_request( authorized_process: &crate::package_identity::AuthorizedClientProcess, sandbox_sid: &[u8], shutdown: &AtomicBool, + on_authenticated_user: &dyn Fn(&InstallationRecord, OwnedHandle) -> Result<()>, ) -> Result { let deadline = Instant::now() + REQUEST_IDLE_TIMEOUT; let mut request = [0_u8; MAX_REQUEST_BYTES]; @@ -322,7 +338,22 @@ fn handle_request( return Err(error) .context("requested sandbox settings violate administrator-controlled machine policy"); } + // A policy-rejected request must not choose the uninstall owner. Use the + // token already authenticated above instead of impersonating the pipe again. + let previous = crate::installation_record::load()?.filter(|record| { + record.user_sid == identity.user_sid && record.codex_home == identity.codex_home + }); + let installation = InstallationRecord { + codex_home: identity.codex_home.clone(), + user_sid: identity.user_sid, + session_id: identity.session_id, + desktop_installation: previous + .and_then(|record| record.desktop_installation) + .or(identity.desktop_installation), + }; + on_authenticated_user(&installation, identity.token)?; if sandbox_setup_is_complete_with_settings(&identity.codex_home, &request.settings) { + crate::service::record_provisioned_user(&installation)?; return Ok(SandboxProvisioningResponse::Ok); } let helper = std::env::current_exe() @@ -352,6 +383,7 @@ fn handle_request( &retained_handles, ) { Ok(()) => { + crate::service::record_provisioned_user(&installation)?; crate::service::log_information( crate::service::EVENT_PROVISIONING_SUCCEEDED, "Codex sandbox provisioning completed successfully.", diff --git a/codex-rs/windows-sandbox-service/src/ipc/authentication.rs b/codex-rs/windows-sandbox-service/src/ipc/authentication.rs index a94451fafc..ca7c761d53 100644 --- a/codex-rs/windows-sandbox-service/src/ipc/authentication.rs +++ b/codex-rs/windows-sandbox-service/src/ipc/authentication.rs @@ -3,6 +3,7 @@ use anyhow::Context; use anyhow::Result; use anyhow::bail; +use codex_windows_sandbox::string_from_sid_bytes; use std::ffi::c_void; use std::mem::size_of; use std::path::Path; @@ -20,6 +21,10 @@ use super::request::ProvisioningRequest; pub(super) struct ClientIdentity { pub(super) account: String, pub(super) codex_home: PathBuf, + pub(super) user_sid: String, + pub(super) session_id: u32, + pub(super) token: OwnedHandle, + pub(super) desktop_installation: Option, // Retained by both the service and helper throughout provisioning. pub(super) directory_handles: Vec, } @@ -38,7 +43,7 @@ pub(super) fn authenticate_client( .context("impersonate provisioning client"); } - let (identity, token) = authenticate_impersonated_client( + let identity = authenticate_impersonated_client( authorized_process, sandbox_sid, &request.codex_home, @@ -47,7 +52,7 @@ pub(super) fn authenticate_client( &identity.codex_home, &request.settings, &request.listeners, - token.0, + identity.token.0, ); Ok((identity, policy_result)) }) @@ -60,7 +65,7 @@ fn authenticate_impersonated_client( authorized_process: &crate::package_identity::AuthorizedClientProcess, sandbox_sid: &[u8], requested_home: &Path, -) -> Result<(ClientIdentity, OwnedHandle)> { +) -> Result { let mut raw_token = 0; if unsafe { threading::OpenThreadToken( @@ -138,16 +143,34 @@ fn authenticate_impersonated_client( let sid = unsafe { ptr::read_unaligned(user.as_ptr().cast::()) } .User .Sid; + let sid_length = unsafe { security::GetLengthSid(sid) }; + if sid_length == 0 { + return Err(std::io::Error::last_os_error()).context("size provisioning client SID"); + } + let user_sid = string_from_sid_bytes(unsafe { + std::slice::from_raw_parts(sid.cast::(), sid_length as usize) + }) + .map_err(anyhow::Error::msg)?; let account = account_name(sid)?; let (codex_home, handles) = prepare_codex_home(requested_home)?; - Ok(( - ClientIdentity { - account, - codex_home, - directory_handles: handles, - }, + let desktop_installation = + crate::installation_record::read_desktop_installation(&codex_home, token.0) + .inspect_err(|_| { + crate::service::log_error( + crate::service::EVENT_SERVICE_FAILED, + "unable to read desktop directory ownership; preserving desktop directories", + ); + }) + .ok(); + Ok(ClientIdentity { + account, + codex_home, + user_sid, + session_id: session, token, - )) + desktop_installation, + directory_handles: handles, + }) } fn account_name(sid: *mut c_void) -> Result { diff --git a/codex-rs/windows-sandbox-service/src/ipc/home.rs b/codex-rs/windows-sandbox-service/src/ipc/home.rs index aacd1c882b..97f48c78a7 100644 --- a/codex-rs/windows-sandbox-service/src/ipc/home.rs +++ b/codex-rs/windows-sandbox-service/src/ipc/home.rs @@ -33,7 +33,7 @@ impl std::fmt::Display for UnsupportedHomeDrive { impl std::error::Error for UnsupportedHomeDrive {} -pub(super) struct OwnedHandle(pub(super) HANDLE); +pub(crate) struct OwnedHandle(pub(crate) HANDLE); impl Drop for OwnedHandle { fn drop(&mut self) { @@ -138,7 +138,7 @@ pub(super) fn prepare_codex_home(requested: &Path) -> Result<(PathBuf, Vec) -> Result<()> { +pub(crate) fn pin_existing_ancestors(path: &Path, handles: &mut Vec) -> Result<()> { let mut current = PathBuf::new(); for component in path.components() { current.push(component.as_os_str()); diff --git a/codex-rs/windows-sandbox-service/src/lib.rs b/codex-rs/windows-sandbox-service/src/lib.rs index aa89704ee0..67177a3783 100644 --- a/codex-rs/windows-sandbox-service/src/lib.rs +++ b/codex-rs/windows-sandbox-service/src/lib.rs @@ -1,5 +1,7 @@ use anyhow::Result; +#[cfg(windows)] +mod installation_record; #[cfg(windows)] mod ipc; #[cfg(windows)] @@ -7,6 +9,8 @@ mod machine_policy; #[cfg(windows)] mod package_identity; #[cfg(windows)] +mod package_lifecycle; +#[cfg(windows)] mod service; #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/codex-rs/windows-sandbox-service/src/package_identity.rs b/codex-rs/windows-sandbox-service/src/package_identity.rs index 5fbd790af9..53a12410b7 100644 --- a/codex-rs/windows-sandbox-service/src/package_identity.rs +++ b/codex-rs/windows-sandbox-service/src/package_identity.rs @@ -165,7 +165,7 @@ fn package_family_name( )) } -fn token_user(token: HANDLE) -> Result> { +pub(crate) fn token_user(token: HANDLE) -> Result> { let mut length = 0; unsafe { security::GetTokenInformation(token, security::TokenUser, ptr::null_mut(), 0, &mut length) diff --git a/codex-rs/windows-sandbox-service/src/package_lifecycle.rs b/codex-rs/windows-sandbox-service/src/package_lifecycle.rs new file mode 100644 index 0000000000..be3b6b8b36 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/package_lifecycle.rs @@ -0,0 +1,279 @@ +//! Restores owner-scoped uninstall notifications and ties file cleanup to pinned directories. + +use std::cell::RefCell; +use std::io; +use std::path::PathBuf; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; + +use anyhow::Context; +use anyhow::Result; +use anyhow::ensure; +use codex_windows_sandbox::clean_up_packaged_windows_sandbox; +use codex_windows_sandbox::string_from_sid_bytes; +use windows::ApplicationModel::Package; +use windows::ApplicationModel::PackageCatalog; +use windows::ApplicationModel::PackageUninstallingEventArgs; +use windows::Foundation::EventRegistrationToken; +use windows::Foundation::TypedEventHandler; +use windows::Win32::System::WinRT::RO_INIT_MULTITHREADED; +use windows::Win32::System::WinRT::RoInitialize; +use windows::Win32::System::WinRT::RoUninitialize; +use windows::core::HSTRING; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Security as security; +use windows_sys::Win32::System::RemoteDesktop::WTS_CURRENT_SERVER_HANDLE; +use windows_sys::Win32::System::RemoteDesktop::WTSEnumerateSessionsW; +use windows_sys::Win32::System::RemoteDesktop::WTSFreeMemory; +use windows_sys::Win32::System::RemoteDesktop::WTSQueryUserToken; + +use crate::installation_record::DesktopInstallation; +use crate::installation_record::InstallationRecord; +use crate::ipc::OwnedHandle; + +struct UserInstallation { + codex_home: Option, + directory_handles: Vec, + desktop_installation: Option, + user_token: OwnedHandle, + catalog: PackageCatalog, + token: EventRegistrationToken, +} + +pub(crate) struct PackageLifecycle { + package_name: HSTRING, + uninstalling: Arc, + installation: RefCell>, +} + +impl PackageLifecycle { + pub(crate) fn new(uninstalling: Arc) -> Result { + unsafe { RoInitialize(RO_INIT_MULTITHREADED) } + .context("initialize the sandbox service Windows Runtime apartment")?; + Ok(Self { + package_name: Package::Current()?.Id()?.FullName()?, + uninstalling, + installation: RefCell::default(), + }) + } + + pub(crate) fn watch_authenticated_user( + &self, + installation: &InstallationRecord, + user_token: OwnedHandle, + ) -> Result<()> { + if self.installation.borrow().is_some() { + return Ok(()); + } + + with_owner_impersonation(user_token.0, || { + let mut directory_handles = Vec::new(); + let codex_home = match crate::ipc::pin_existing_ancestors( + &installation.codex_home, + &mut directory_handles, + ) { + Ok(()) => Some(installation.codex_home.clone()), + Err(error) => { + directory_handles.clear(); + crate::service::log_error( + crate::service::EVENT_SERVICE_FAILED, + &format!( + "skipping sandbox file cleanup because the home could not be pinned: {error:#}" + ), + ); + None + } + }; + let catalog = PackageCatalog::OpenForCurrentUser() + .context("open the authenticated user's package catalog")?; + let package_name = self.package_name.clone(); + let uninstalling = Arc::clone(&self.uninstalling); + let token = catalog + .PackageUninstalling(&TypedEventHandler::< + PackageCatalog, + PackageUninstallingEventArgs, + >::new(move |_, event| { + if let Some(event) = event + && event.Package()?.Id()?.FullName()? == package_name + { + uninstalling.store(!event.IsComplete()?, Ordering::Release); + } + Ok(()) + })) + .context("subscribe to authenticated package uninstall notifications")?; + self.installation.replace(Some(UserInstallation { + codex_home, + directory_handles, + desktop_installation: installation.desktop_installation.clone(), + user_token, + catalog, + token, + })); + Ok(()) + }) + } + + pub(crate) fn restore_logged_in_owner(&self, recorded_session_id: u32) -> Result<()> { + let Err(error) = self.restore_authenticated_user(recorded_session_id) else { + return Ok(()); + }; + + // Session IDs can change while the service is stopped; the owner SID is durable. + let mut sessions = std::ptr::null_mut(); + let mut count = 0; + if unsafe { + WTSEnumerateSessionsW( + WTS_CURRENT_SERVER_HANDLE, + /*reserved*/ 0, + /*version*/ 1, + &mut sessions, + &mut count, + ) + } == 0 + { + return Err(io::Error::last_os_error()).context("list current Windows sessions"); + } + let restored = (0..count).any(|index| { + let session_id = unsafe { (*sessions.add(index as usize)).SessionId }; + session_id != recorded_session_id && self.restore_authenticated_user(session_id).is_ok() + }); + unsafe { WTSFreeMemory(sessions.cast()) }; + if restored { Ok(()) } else { Err(error) } + } + + pub(crate) fn restore_authenticated_user(&self, session_id: u32) -> Result<()> { + if self.installation.borrow().is_some() { + return Ok(()); + } + let Some(record) = crate::installation_record::load()? else { + return Ok(()); + }; + + let mut raw_token = 0; + if unsafe { WTSQueryUserToken(session_id, &mut raw_token) } == 0 { + return Err(io::Error::last_os_error()).context("open the logged-in user's token"); + } + let token = crate::ipc::OwnedHandle(raw_token); + let user = crate::package_identity::token_user(token.0)?; + let sid = unsafe { std::ptr::read_unaligned(user.as_ptr().cast::()) } + .User + .Sid; + let sid_length = unsafe { security::GetLengthSid(sid) }; + if sid_length == 0 { + return Err(io::Error::last_os_error()).context("read the logged-in user's SID"); + } + let user_sid = string_from_sid_bytes(unsafe { + std::slice::from_raw_parts(sid.cast::(), sid_length as usize) + }) + .map_err(anyhow::Error::msg)?; + ensure!( + user_sid == record.user_sid, + "logged-in user does not match the recorded sandbox owner" + ); + + self.watch_authenticated_user(&record, token)?; + if session_id != record.session_id { + crate::installation_record::save(&crate::installation_record::InstallationRecord { + session_id, + ..record + })?; + } + Ok(()) + } + + pub(crate) fn clean_up(&self) -> Result<()> { + let mut installation = self.installation.borrow_mut(); + let installation = installation + .as_mut() + .context("the authenticated package installation was not recorded")?; + crate::service::log_information( + crate::service::EVENT_CLEANUP_STARTED, + "sandbox uninstall cleanup started", + ); + // A partial uninstall must not let a later install inherit stale directory ownership. + crate::installation_record::remove()?; + let codex_home = installation.codex_home.clone(); + clean_up_packaged_windows_sandbox(codex_home.as_deref(), || { + let Some(desktop) = &installation.desktop_installation else { + return Ok(()); + }; + // The marker is user-writable. It must never authorize deletion as LocalSystem. + with_owner_impersonation(installation.user_token.0, || { + let mut errors = Vec::new(); + let mut record_error = |result: io::Result<()>| { + if let Err(error) = result + && error.kind() != io::ErrorKind::NotFound + { + errors.push(error.to_string()); + } + }; + if let Some(home) = &codex_home + && desktop.created_codex_home + { + // Release the home itself so it can be deleted; keep its ancestors pinned. + installation.directory_handles.pop(); + record_error(std::fs::remove_dir_all(home)); + } + // The cache may have been created after provisioning. Pin it only for cleanup. + let mut cache_directory_handles = Vec::new(); + if desktop.cache_home.is_dir() { + match crate::ipc::pin_existing_ancestors( + &desktop.cache_home, + &mut cache_directory_handles, + ) { + Ok(()) => record_error(std::fs::remove_dir_all( + desktop.cache_home.join("codex-runtimes"), + )), + Err(error) => errors.push(error.to_string()), + } + } + ensure!( + errors.is_empty(), + "remove desktop directories: {}", + errors.join("; ") + ); + Ok(()) + }) + }) + .context("remove packaged Windows sandbox resources")?; + crate::service::log_information( + crate::service::EVENT_CLEANUP_FINISHED, + "sandbox uninstall cleanup finished", + ); + Ok(()) + } +} + +fn with_owner_impersonation( + user_token: HANDLE, + operation: impl FnOnce() -> Result<()>, +) -> Result<()> { + if unsafe { security::ImpersonateLoggedOnUser(user_token) } == 0 { + return Err(io::Error::last_os_error()).context("impersonate the sandbox owner"); + } + let result = operation(); + if unsafe { security::RevertToSelf() } == 0 { + crate::service::log_error( + crate::service::EVENT_SERVICE_FAILED, + &format!( + "unable to revert sandbox-owner impersonation: {}", + io::Error::last_os_error() + ), + ); + // Continuing as the user would make later machine cleanup unsafe. + std::process::abort(); + } + result +} + +impl Drop for PackageLifecycle { + fn drop(&mut self) { + if let Some(installation) = self.installation.get_mut().take() { + let _ = installation + .catalog + .RemovePackageUninstalling(installation.token); + } + unsafe { RoUninitialize() }; + } +} diff --git a/codex-rs/windows-sandbox-service/src/service.rs b/codex-rs/windows-sandbox-service/src/service.rs index 929b9bb717..40bcb86276 100644 --- a/codex-rs/windows-sandbox-service/src/service.rs +++ b/codex-rs/windows-sandbox-service/src/service.rs @@ -2,6 +2,7 @@ use std::ffi::c_void; use std::io; +use std::mem::size_of; use std::ptr; use std::sync::Arc; use std::sync::OnceLock; @@ -19,10 +20,13 @@ use windows_sys::Win32::System::EventLog::EVENTLOG_ERROR_TYPE; use windows_sys::Win32::System::EventLog::EVENTLOG_INFORMATION_TYPE; use windows_sys::Win32::System::EventLog::RegisterEventSourceW; use windows_sys::Win32::System::EventLog::ReportEventW; +use windows_sys::Win32::System::RemoteDesktop::WTSSESSION_NOTIFICATION; use windows_sys::Win32::System::Services::RegisterServiceCtrlHandlerExW; +use windows_sys::Win32::System::Services::SERVICE_ACCEPT_SESSIONCHANGE; use windows_sys::Win32::System::Services::SERVICE_ACCEPT_SHUTDOWN; use windows_sys::Win32::System::Services::SERVICE_ACCEPT_STOP; use windows_sys::Win32::System::Services::SERVICE_CONTROL_INTERROGATE; +use windows_sys::Win32::System::Services::SERVICE_CONTROL_SESSIONCHANGE; use windows_sys::Win32::System::Services::SERVICE_CONTROL_SHUTDOWN; use windows_sys::Win32::System::Services::SERVICE_CONTROL_STOP; use windows_sys::Win32::System::Services::SERVICE_RUNNING; @@ -40,25 +44,33 @@ pub(crate) const SERVICE_NAME: &str = "CodexSandboxService"; const EVENT_SERVICE_STARTED: u32 = 1000; const EVENT_SERVICE_STOP_REQUESTED: u32 = 1001; const EVENT_SERVICE_STOPPED: u32 = 1002; -const EVENT_SERVICE_FAILED: u32 = 1003; +pub(crate) const EVENT_SERVICE_FAILED: u32 = 1003; pub(crate) const EVENT_PROVISIONING_SUCCEEDED: u32 = 2000; pub(crate) const EVENT_PROVISIONING_FAILED: u32 = 2001; pub(crate) const EVENT_REQUEST_REJECTED: u32 = 2002; +pub(crate) const EVENT_CLEANUP_STARTED: u32 = 3002; +pub(crate) const EVENT_CLEANUP_FINISHED: u32 = 3003; const MAX_EVENT_MESSAGE_UNITS: usize = 1024; static SERVICE_STATE: OnceLock = OnceLock::new(); struct ServiceState { shutdown: Arc, + uninstalling: Arc, status_handle: OnceLock, current_status: AtomicU32, + changed_session: AtomicU32, + stop_requested: AtomicBool, } pub(crate) fn run() -> Result<()> { let state = ServiceState { shutdown: Arc::new(AtomicBool::new(false)), + uninstalling: Arc::new(AtomicBool::new(false)), status_handle: OnceLock::new(), current_status: AtomicU32::new(SERVICE_START_PENDING), + changed_session: AtomicU32::new(u32::MAX), + stop_requested: AtomicBool::new(false), }; SERVICE_STATE .set(state) @@ -91,10 +103,15 @@ pub(crate) fn run() -> Result<()> { #[cfg(debug_assertions)] pub(crate) fn run_foreground() -> Result<()> { crate::package_identity::enable_foreground_mode(); - crate::ipc::run(Arc::new(AtomicBool::new(false)), || { - eprintln!("{SERVICE_NAME} listening on {}", crate::ipc::PIPE_NAME); - Ok(()) - }) + crate::ipc::run( + Arc::new(AtomicBool::new(false)), + || { + eprintln!("{SERVICE_NAME} listening on {}", crate::ipc::PIPE_NAME); + Ok(()) + }, + |_, _| Ok(()), + || Ok(()), + ) } unsafe extern "system" fn service_main(_argument_count: u32, _arguments: *mut *mut u16) { @@ -137,16 +154,55 @@ fn service_main_inner(state: &ServiceState) -> Result<()> { .map_err(|_| anyhow::anyhow!("the service status handle was already registered"))?; state.report_status(SERVICE_START_PENDING, NO_ERROR)?; - crate::ipc::run(Arc::clone(&state.shutdown), || { - state.report_status(SERVICE_RUNNING, NO_ERROR)?; - log_information( - EVENT_SERVICE_STARTED, - "The Codex sandbox service is running.", - ); - Ok(()) - }) + let package_lifecycle = + crate::package_lifecycle::PackageLifecycle::new(Arc::clone(&state.uninstalling))?; + crate::ipc::run( + Arc::clone(&state.shutdown), + || { + state.report_status(SERVICE_RUNNING, NO_ERROR)?; + if let Some(record) = crate::installation_record::load()? + && let Err(error) = package_lifecycle.restore_logged_in_owner(record.session_id) + { + log_error( + EVENT_SERVICE_FAILED, + &format!("unable to restore package uninstall listener: {error:#}"), + ); + } + log_information( + EVENT_SERVICE_STARTED, + "The Codex sandbox service is running.", + ); + Ok(()) + }, + |installation, user_token| { + if let Err(error) = package_lifecycle.watch_authenticated_user(installation, user_token) + { + log_error( + EVENT_SERVICE_FAILED, + &format!("unable to observe package uninstall: {error:#}"), + ); + } + Ok(()) + }, + || { + let session = state.changed_session.swap(u32::MAX, Ordering::AcqRel); + if session == u32::MAX { + return Ok(()); + } + if let Err(error) = package_lifecycle.restore_authenticated_user(session) { + log_error( + EVENT_SERVICE_FAILED, + &format!("unable to restore package uninstall listener: {error:#}"), + ); + } + Ok(()) + }, + ) .context("run the sandbox provisioning broker")?; + if state.stop_requested.load(Ordering::Acquire) && state.uninstalling.load(Ordering::Acquire) { + package_lifecycle.clean_up()?; + } log_information( EVENT_SERVICE_STOPPED, "The Codex sandbox service has stopped.", @@ -157,7 +213,7 @@ fn service_main_inner(state: &ServiceState) -> Result<()> { unsafe extern "system" fn service_control_handler( control: u32, _event_type: u32, - _event_data: *mut c_void, + event_data: *mut c_void, _context: *mut c_void, ) -> u32 { let Some(state) = SERVICE_STATE.get() else { @@ -166,19 +222,22 @@ unsafe extern "system" fn service_control_handler( match control { SERVICE_CONTROL_STOP | SERVICE_CONTROL_SHUTDOWN => { + if control == SERVICE_CONTROL_STOP { + state.stop_requested.store(true, Ordering::Release); + } if !state.shutdown.swap(true, Ordering::AcqRel) { if let Err(error) = state.report_status(SERVICE_STOP_PENDING, NO_ERROR) { eprintln!("unable to report service shutdown: {error:#}"); } + log_information( + EVENT_SERVICE_STOP_REQUESTED, + "The Codex sandbox service was asked to stop.", + ); std::thread::spawn(move || { crate::ipc::wake(crate::ipc::PIPE_NAME, || { state.current_status.load(Ordering::Acquire) == SERVICE_STOPPED }); }); - log_information( - EVENT_SERVICE_STOP_REQUESTED, - "The Codex sandbox service was asked to stop.", - ); } NO_ERROR } @@ -193,6 +252,22 @@ unsafe extern "system" fn service_control_handler( } NO_ERROR } + SERVICE_CONTROL_SESSIONCHANGE => { + if !event_data.is_null() { + let event = unsafe { &*event_data.cast::() }; + if event.cbSize as usize >= size_of::() { + state + .changed_session + .store(event.dwSessionId, Ordering::Release); + std::thread::spawn(move || { + crate::ipc::wake(crate::ipc::PIPE_NAME, || { + state.current_status.load(Ordering::Acquire) == SERVICE_STOPPED + }); + }); + } + } + NO_ERROR + } _ => ERROR_CALL_NOT_IMPLEMENTED, } } @@ -201,6 +276,15 @@ pub(crate) fn log_information(event_id: u32, message: &str) { log_event(EVENTLOG_INFORMATION_TYPE, event_id, message); } +pub(crate) fn record_provisioned_user( + installation: &crate::installation_record::InstallationRecord, +) -> Result<()> { + if SERVICE_STATE.get().is_some() { + crate::installation_record::save(installation)?; + } + Ok(()) +} + pub(crate) fn log_error(event_id: u32, message: &str) { log_event(EVENTLOG_ERROR_TYPE, event_id, message); } @@ -262,7 +346,7 @@ impl ServiceState { dwServiceType: SERVICE_WIN32_OWN_PROCESS, dwCurrentState: current_status, dwControlsAccepted: if current_status == SERVICE_RUNNING { - SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN + SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN | SERVICE_ACCEPT_SESSIONCHANGE } else { 0 },