Complete Windows sandbox uninstall cleanup (#45799)

## Why

Packaged uninstall could leave sandbox user profiles and desktop-created data behind. Cleanup also needs to handle a service stop before package removal completes without deleting data belonging to an update or reinstall.

## What changed

- Delete sandbox profiles before their accounts, preserving accounts for retry when profile deletion fails. Defer retained runtime accounts and the sandbox group until runtime registrations are removed and profiles unload.
- Allow cleanup while the exact retiring package is still registered, while preserving desktop data when a successor package is present.
- Remove desktop-created Codex homes during registered runtime cleanup, preserve existing CLI data, and prune empty home and cache directories. Keep directory pins through retries and avoid privileged traversal after releasing the home.
- Retry cleanup up to five attempts after a service stop, while retaining shutdown cancellation behavior.
- Report cleanup outcomes in the Windows Event Log and emit final completion after registered runtime cleanup finishes. Remove the empty installation registry parent.

## Testing

Add Windows tests for profile deletion retry and account preservation, service-stop retries versus system shutdown, and PowerShell finalizer parsing and native binding compilation. Extend retained-token coverage to check account SID matching.

GitOrigin-RevId: 1aefa969e6aff9bb8e77e2c9fe9ee772230fc4fb
This commit is contained in:
chess
2026-09-15 21:33:16 +00:00
committed by copyberry
parent 63c09ed212
commit 872fc22f9c
16 changed files with 703 additions and 62 deletions
-1
View File
@@ -5224,7 +5224,6 @@ name = "codex-windows-sandbox-service"
version = "0.0.0"
dependencies = [
"anyhow",
"base64 0.22.1",
"codex-cloud-config",
"codex-config",
"codex-core",
@@ -17,11 +17,13 @@ mod retained_logons;
/// 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.
/// `report` must remain usable after the home and its log files have been removed.
pub fn clean_up_packaged_windows_sandbox(
codex_home: Option<&Path>,
report: impl Fn(&str),
clean_up_desktop: impl FnOnce() -> Result<()>,
) -> Result<()> {
prepare_packaged_windows_sandbox_cleanup()?.finish(codex_home, clean_up_desktop)
prepare_packaged_windows_sandbox_cleanup()?.finish(codex_home, report, clean_up_desktop)
}
/// Holds the setup lock after sandbox accounts are disabled, their processes
@@ -86,9 +88,11 @@ impl PreparedWindowsSandboxCleanup {
/// starts. Independent cleanup steps continue after an error, as in
/// `clean_up_packaged_windows_sandbox`. Retrying keeps the original account
/// identities; it must not adopt replacement accounts created between attempts.
/// `report` records each cleanup outcome without relying on files in the home.
pub fn finish(
&self,
codex_home: Option<&Path>,
report: impl Fn(&str),
clean_up_desktop: impl FnOnce() -> Result<()>,
) -> Result<()> {
self.users.validate_current()?;
@@ -98,38 +102,72 @@ impl PreparedWindowsSandboxCleanup {
if let Err(error) = crate::logging::release_setup_log() {
errors.push(format!("release setup log: {error:#}"));
}
for directory in [
crate::setup::sandbox_dir(codex_home),
crate::setup::sandbox_secrets_dir(codex_home),
crate::setup::sandbox_bin_dir(codex_home),
for (directory, path) in [
(".sandbox", crate::setup::sandbox_dir(codex_home)),
(
".sandbox-secrets",
crate::setup::sandbox_secrets_dir(codex_home),
),
(".sandbox-bin", crate::setup::sandbox_bin_dir(codex_home)),
] {
match std::fs::remove_dir_all(&directory) {
Ok(()) => {}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
match std::fs::remove_dir_all(path) {
Ok(()) => report(&format!("removed {directory}")),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
report(&format!("skipping {directory}: not found"));
}
Err(error) => {
errors.push(format!("remove {}: {error}", directory.display()));
let message = format!("remove {directory}: failed, {error}");
report(&message);
errors.push(message);
}
}
}
} else {
report("skipping sandbox directories: no pinned home");
}
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),
for (operation, result) in [
("remove WFP filters", crate::wfp::remove_wfp_filters()),
("remove firewall rules", firewall::cleanup_firewall_rules()),
(
"remove hidden-user entries",
crate::hide_users::unhide_sandbox_users(&[OFFLINE_USERNAME, ONLINE_USERNAME]),
),
] {
if let Err(error) = result {
errors.push(format!("{error:#}"));
match result {
Ok(()) => report(&format!("{operation}: completed")),
Err(error) => {
let message = format!("{operation}: failed, {error:#}");
report(&message);
errors.push(message);
}
}
}
if let Err(error) = self.users.remove_users(&self._retained_logons, &report) {
errors.push(format!("{error:#}"));
}
// Retry home permission cleanup with the same group; retained runtime users
// and their group are removed by the finalizer after their profiles unload.
if errors.is_empty()
&& !self
.users
.sids()
.any(|sid| self._retained_logons.contains_sid(sid))
{
match principals::remove_sandbox_principal("CodexSandboxUsers") {
Ok(()) => report("remove sandbox group: completed"),
Err(error) => {
report(&format!("remove sandbox group: failed, {error:#}"));
errors.push(format!("{error:#}"));
}
}
} else {
report("retaining sandbox group until cleanup finishes");
}
if errors.is_empty() {
Ok(())
} else {
@@ -1,4 +1,4 @@
//! Disables sandbox accounts for cleanup and removes sandbox principals.
//! Disables sandbox accounts for cleanup and removes their profiles and principals.
//! Callers must restore original flags if preparation for cleanup fails.
use std::ptr::null;
@@ -6,15 +6,22 @@ use std::ptr::null;
use anyhow::Result;
use anyhow::bail;
use anyhow::ensure;
use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND;
use windows_sys::Win32::Foundation::ERROR_SUCCESS;
use windows_sys::Win32::NetworkManagement::NetManagement as network;
use windows_sys::Win32::System::Registry as registry;
use windows_sys::Win32::UI::Shell::DeleteProfileW;
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::string_from_sid_bytes;
use crate::winutil::to_wide;
use super::retained_logons::RetainedLogons;
#[derive(Default)]
pub(super) struct DisabledSandboxUsers {
users: Vec<SandboxUser>,
@@ -80,6 +87,70 @@ impl DisabledSandboxUsers {
}
Ok(())
}
pub(super) fn remove_users(
&self,
retained: &RetainedLogons,
report: impl Fn(&str),
) -> Result<()> {
let mut errors = Vec::new();
for user in &self.users {
if retained.contains_sid(&user.sid) {
report(&format!(
"deferring {} profile and account to runtime cleanup",
user.name
));
continue;
}
match user.remove() {
Ok(()) => report(&format!("removed {} profile and account", user.name)),
Err(error) => {
report(&format!(
"remove {} profile and account: failed, {error:#}",
user.name
));
errors.push(format!("{error:#}"));
}
}
}
ensure!(errors.is_empty(), "{}", errors.join("; "));
Ok(())
}
}
impl SandboxUser {
fn remove(&self) -> Result<()> {
let Self { name, sid, .. } = self;
let sid = string_from_sid_bytes(sid).map_err(anyhow::Error::msg)?;
let profile_key = to_wide(format!(
r"SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\{sid}"
));
let mut key = 0;
let status = unsafe {
registry::RegOpenKeyExW(
registry::HKEY_LOCAL_MACHINE,
profile_key.as_ptr(),
/*uloptions*/ 0,
registry::KEY_READ | registry::KEY_WOW64_64KEY,
&mut key,
)
};
match status {
ERROR_SUCCESS => {
unsafe { registry::RegCloseKey(key) };
// Keep the account's SID available for retry if its profile is still loaded.
if unsafe { DeleteProfileW(to_wide(sid).as_ptr(), null(), null()) } == 0 {
bail!(
"remove sandbox profile for {name}: {}",
std::io::Error::last_os_error()
);
}
}
ERROR_FILE_NOT_FOUND => {}
status => bail!("open sandbox profile for {name}: {status}"),
}
remove_sandbox_principal(name)
}
}
#[cfg(test)]
@@ -108,3 +179,7 @@ pub(super) fn remove_sandbox_principal(name: &str) -> Result<()> {
status => bail!("remove local sandbox principal {name}: {status}"),
}
}
#[cfg(test)]
#[path = "principals_tests.rs"]
mod tests;
@@ -0,0 +1,218 @@
//! Exercises profile deletion and its retry boundary with one disposable local user.
use std::io;
use std::os::windows::ffi::OsStringExt;
use std::os::windows::io::AsRawHandle;
use std::os::windows::io::FromRawHandle;
use std::os::windows::io::OwnedHandle;
use std::path::PathBuf;
use std::ptr::null;
use std::ptr::null_mut;
use std::time::Duration;
use std::time::Instant;
use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use pretty_assertions::assert_eq;
use rand::RngCore;
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND;
use windows_sys::Win32::Foundation::ERROR_SUCCESS;
use windows_sys::Win32::Foundation::WAIT_OBJECT_0;
use windows_sys::Win32::NetworkManagement::NetManagement as network;
use windows_sys::Win32::System::Registry as registry;
use windows_sys::Win32::System::Threading as threading;
use super::SandboxUser;
use crate::winutil::local_user_flags;
use crate::winutil::resolve_sid;
use crate::winutil::set_local_user_flags;
use crate::winutil::string_from_sid_bytes;
use crate::winutil::to_wide;
struct ProfileFixture {
user: SandboxUser,
process: Option<OwnedHandle>,
}
impl ProfileFixture {
fn stop(&mut self) -> Result<()> {
if let Some(process) = self.process.take() {
let handle = process.as_raw_handle() as _;
unsafe {
threading::TerminateProcess(handle, /*uexitcode*/ 0)
};
ensure!(
unsafe {
threading::WaitForSingleObject(handle, /*dwmilliseconds*/ 10_000)
} == WAIT_OBJECT_0,
"owned profile fixture did not exit"
);
drop(process);
// User Profile Service unloads the hive after the last owned process exits.
let sid = string_from_sid_bytes(&self.user.sid).map_err(anyhow::Error::msg)?;
let deadline = Instant::now() + Duration::from_secs(10);
loop {
let mut key = 0;
let status = unsafe {
registry::RegOpenKeyExW(
registry::HKEY_USERS,
to_wide(&sid).as_ptr(),
/*uloptions*/ 0,
registry::KEY_READ,
&mut key,
)
};
if status == ERROR_FILE_NOT_FOUND {
break;
}
ensure!(status == ERROR_SUCCESS, "read owned profile hive: {status}");
unsafe { registry::RegCloseKey(key) };
ensure!(Instant::now() < deadline, "owned profile did not unload");
std::thread::sleep(Duration::from_millis(50));
}
}
Ok(())
}
}
impl Drop for ProfileFixture {
fn drop(&mut self) {
let _ = self.stop();
let result = if self.user.sid.is_empty() {
super::remove_sandbox_principal(self.user.name)
} else {
self.user.remove()
};
if let Err(error) = result {
eprintln!("unable to clean up owned profile fixture: {error:#}");
}
}
}
#[test]
fn profile_cleanup_preserves_account_until_profile_can_be_deleted() -> Result<()> {
// Production names are static. This one unique name lives for this test process only.
let name: &'static str =
Box::leak(format!("CodexPrf{:08x}", rand::rngs::OsRng.next_u32()).into_boxed_str());
let username = to_wide(name);
let password = to_wide(format!("Cdx!a0{:016x}", rand::rngs::OsRng.next_u64()));
let user_info = network::USER_INFO_1 {
usri1_name: username.as_ptr().cast_mut(),
usri1_password: password.as_ptr().cast_mut(),
usri1_priv: network::USER_PRIV_USER,
usri1_flags: network::UF_NORMAL_ACCOUNT
| network::UF_SCRIPT
| network::UF_DONT_EXPIRE_PASSWD,
..unsafe { std::mem::zeroed() }
};
let status = unsafe {
network::NetUserAdd(
null(),
/*level*/ 1,
(&raw const user_info).cast(),
null_mut(),
)
};
if status == ERROR_ACCESS_DENIED {
eprintln!(
"skipping native profile cleanup test: creating its disposable user requires elevation"
);
return Ok(());
}
ensure!(
status == network::NERR_Success,
"create owned profile fixture: {status}"
);
let mut fixture = ProfileFixture {
user: SandboxUser {
name,
original_flags: user_info.usri1_flags,
sid: Vec::new(),
},
process: None,
};
fixture.user.sid = resolve_sid(name)?;
let sid = string_from_sid_bytes(&fixture.user.sid).map_err(anyhow::Error::msg)?;
let profile_key = to_wide(format!(
r"SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\{sid}"
));
let system_root = std::env::var_os("SystemRoot").context("SystemRoot is missing")?;
let executable =
PathBuf::from(&system_root).join(r"System32\WindowsPowerShell\v1.0\powershell.exe");
let executable = to_wide(executable);
let mut command =
to_wide("powershell.exe -NoProfile -NonInteractive -Command Start-Sleep -Seconds 600");
let mut startup: threading::STARTUPINFOW = unsafe { std::mem::zeroed() };
startup.cb = std::mem::size_of_val(&startup) as u32;
let mut process: threading::PROCESS_INFORMATION = unsafe { std::mem::zeroed() };
let created = unsafe {
threading::CreateProcessWithLogonW(
username.as_ptr(),
to_wide(".").as_ptr(),
password.as_ptr(),
threading::LOGON_WITH_PROFILE,
executable.as_ptr(),
command.as_mut_ptr(),
threading::CREATE_NO_WINDOW,
null(),
to_wide(&system_root).as_ptr(),
&startup,
&mut process,
)
};
ensure!(
created != 0,
"start owned profile fixture: {}",
io::Error::last_os_error()
);
fixture.process = Some(unsafe { OwnedHandle::from_raw_handle(process.hProcess as _) });
drop(unsafe { OwnedHandle::from_raw_handle(process.hThread as _) });
let mut path = [0u16; 32768];
let mut length = std::mem::size_of_val(&path) as u32;
let status = unsafe {
registry::RegGetValueW(
registry::HKEY_LOCAL_MACHINE,
profile_key.as_ptr(),
to_wide("ProfileImagePath").as_ptr(),
registry::RRF_RT_REG_SZ | registry::RRF_RT_REG_EXPAND_SZ,
null_mut(),
path.as_mut_ptr().cast(),
&mut length,
)
};
assert_eq!(status, ERROR_SUCCESS);
let path = PathBuf::from(std::ffi::OsString::from_wide(
&path[..length as usize / 2 - 1],
));
assert!(path.is_dir());
set_local_user_flags(name, user_info.usri1_flags | network::UF_ACCOUNTDISABLE)?;
assert!(
fixture.user.remove().is_err(),
"loaded profile must block deletion"
);
assert_eq!(resolve_sid(name)?, fixture.user.sid);
assert!(path.is_dir());
fixture.stop()?;
fixture.user.remove()?;
assert_eq!(local_user_flags(name)?, None);
assert!(!path.exists());
let mut key = 0;
let status = unsafe {
registry::RegOpenKeyExW(
registry::HKEY_LOCAL_MACHINE,
profile_key.as_ptr(),
/*uloptions*/ 0,
registry::KEY_READ,
&mut key,
)
};
if status == ERROR_SUCCESS {
unsafe { registry::RegCloseKey(key) };
}
assert_eq!(status, ERROR_FILE_NOT_FOUND);
fixture.user.remove()?;
Ok(())
}
@@ -21,6 +21,7 @@ pub(super) struct RetainedLogons {
// Keeping each token open prevents its AuthenticationId from being recycled.
_tokens: Vec<OwnedHandle>,
ids: Vec<LUID>,
sids: Vec<Vec<u8>>,
}
impl RetainedLogons {
@@ -30,14 +31,13 @@ impl RetainedLogons {
) -> Result<Self> {
ensure!(tokens.len() <= 2, "too many retained cleanup tokens");
let mut retained = Self::default();
let mut seen_sids = Vec::new();
for token in tokens {
let token = token.try_clone_to_owned()?;
let handle = token.as_raw_handle() as HANDLE;
let sid = unsafe { crate::token::get_user_sid_bytes(handle) }?;
ensure!(
users.sids().any(|candidate| candidate == sid.as_slice())
&& !seen_sids.contains(&sid),
&& !retained.sids.contains(&sid),
"cleanup token does not identify a distinct managed sandbox account"
);
let mut stats: TOKEN_STATISTICS = unsafe { std::mem::zeroed() };
@@ -58,7 +58,7 @@ impl RetainedLogons {
returned as usize == std::mem::size_of::<TOKEN_STATISTICS>(),
"unexpected cleanup token statistics length"
);
seen_sids.push(sid);
retained.sids.push(sid);
retained.ids.push(stats.AuthenticationId);
retained._tokens.push(token);
}
@@ -70,6 +70,10 @@ impl RetainedLogons {
.iter()
.any(|held| held.LowPart == id.LowPart && held.HighPart == id.HighPart)
}
pub(super) fn contains_sid(&self, sid: &[u8]) -> bool {
self.sids.iter().any(|held| held == sid)
}
}
#[cfg(test)]
@@ -34,6 +34,7 @@ fn matches_only_the_complete_retained_authentication_id() {
LowPart: 17,
HighPart: 4,
}],
sids: Vec::new(),
};
let observed = [
LUID {
@@ -102,5 +103,7 @@ fn retained_token_remains_queryable_after_the_borrowed_handle_is_closed() -> Res
assert_eq!(retained._tokens.len(), 1);
let actual_sid = unsafe { get_user_sid_bytes(retained._tokens[0].as_raw_handle() as HANDLE) }?;
assert_eq!(actual_sid, expected_sid);
assert!(retained.contains_sid(&expected_sid));
assert!(!retained.contains_sid(&[]));
Ok(())
}
@@ -18,7 +18,6 @@ workspace = true
[dependencies]
anyhow = { workspace = true }
base64 = { workspace = true }
codex-cloud-config = { workspace = true }
codex-config = { workspace = true }
codex-core = { workspace = true }
@@ -264,7 +264,7 @@ impl PackageLifecycle {
let installation = installation
.as_mut()
.context("the authenticated package installation was not recorded")?;
cleanup::clean_up(installation, prepared, runtime)
cleanup::clean_up(installation, prepared, runtime, &self.uninstalling)
}
}
@@ -2,6 +2,8 @@
//! Owner impersonation, directory pins, and registration-aware cleanup order are preserved.
use std::io;
use std::path::Path;
use std::sync::atomic::AtomicBool;
use anyhow::Context;
use anyhow::Result;
@@ -13,11 +15,15 @@ use codex_windows_sandbox::revoke_ace;
use super::UserInstallation;
use super::with_owner_impersonation;
use crate::installation_record::InstallationRecord;
use crate::service::EVENT_CLEANUP_DETAIL;
use crate::service::log_error;
use crate::service::log_information;
pub(super) fn clean_up(
installation: &mut UserInstallation,
prepared: &PreparedWindowsSandboxCleanup,
runtime: Option<&InstallationRecord>,
uninstalling: &AtomicBool,
) -> Result<()> {
crate::service::log_information(
crate::service::EVENT_CLEANUP_STARTED,
@@ -26,39 +32,58 @@ pub(super) fn clean_up(
let codex_home = installation.codex_home.clone();
// Remove exact grants from the locked cleanup record before native account deletion.
if let Some(record) = runtime {
log_cleanup("removing registered runtime metadata");
crate::registered_runtime::remove_metadata(installation.user_token.0, record)?;
}
let result = prepared.finish(codex_home.as_deref(), || {
// Release once, even when registered cleanup retries its remaining steps.
installation.directory_guard.take();
log_cleanup("removing native sandbox resources");
let mut prune_codex_home = false;
// Once owner-scoped deletion releases the home, retries must not traverse it as SYSTEM.
let sandbox_home = codex_home
.as_deref()
.filter(|_| installation.directory_guard.is_some());
let result = prepared.finish(sandbox_home, log_cleanup, || {
if let Some(record) = runtime
&& super::registered::runtime_owner_has_package(record)?
&& !super::registered::owner_allows_cleanup(uninstalling, record)?
{
// Only the old sandbox resources are repaired on reinstall. Never remove
// the reinstalled app's desktop-created home or runtime cache.
log_cleanup("skipping desktop directories: owner reinstalled the app");
return Ok(());
}
let Some(desktop) = &installation.record.desktop_installation else {
log_cleanup("skipping desktop directories: no desktop installation record");
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());
let mut record_result = |operation: &str, result: io::Result<()>| match result {
Ok(()) => log_cleanup(&format!("{operation}: completed")),
Err(error) if error.kind() == io::ErrorKind::NotFound => {
log_cleanup(&format!("{operation}: skipped, not found"));
}
Err(error) => {
let message = format!("{operation}: failed, {error}");
log_error(EVENT_CLEANUP_DETAIL, &message);
errors.push(message);
}
};
if let Some(home) = &codex_home {
if desktop.created_codex_home && runtime.is_none() {
if 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));
if installation.directory_guard.take().is_some() {
installation.directory_handles.pop();
}
record_result(
"remove desktop-created codex home",
std::fs::remove_dir_all(home),
);
} else {
prune_codex_home = true;
log_cleanup("skipping recursive codex home removal: existing CLI home");
// Preserve CLI data without leaving inherited permissions for the deleted group.
record_error(
record_result(
"remove codex home sandbox permissions",
resolve_sid("CodexSandboxUsers")
.and_then(|mut sid| unsafe {
revoke_ace(home, sid.as_mut_ptr().cast())
@@ -66,6 +91,8 @@ pub(super) fn clean_up(
.map_err(io::Error::other),
);
}
} else {
log_cleanup("skipping codex home: no pinned home");
}
// The cache may have been created after provisioning. Pin it only for cleanup.
let mut cache_directory_handles = Vec::new();
@@ -74,11 +101,25 @@ pub(super) fn clean_up(
&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()),
Ok(()) => {
record_result(
"remove codex runtime cache",
std::fs::remove_dir_all(desktop.cache_home.join("codex-runtimes")),
);
// Release only the cache root; its ancestors must remain pinned.
cache_directory_handles.pop();
remove_empty_directory(&desktop.cache_home, "cache home");
}
Err(error) => {
log_error(
EVENT_CLEANUP_DETAIL,
&format!("skipping runtime cache: could not pin cache home, {error:#}"),
);
errors.push(error.to_string());
}
}
} else {
log_cleanup("skipping cache home: no accessible directory");
}
ensure!(
errors.is_empty(),
@@ -89,9 +130,55 @@ pub(super) fn clean_up(
})
});
result.context("remove packaged Windows sandbox resources")?;
if prune_codex_home && let Some(home) = &codex_home {
// Keep the home pinned through native retries. Empty-root pruning is best effort
// so a failure cannot restart native cleanup through an unpinned home.
if let Err(error) = with_owner_impersonation(installation.user_token.0, || {
if installation.directory_guard.take().is_some() {
installation.directory_handles.pop();
}
remove_empty_directory(home, "codex home");
Ok(())
}) {
log_error(
EVENT_CLEANUP_DETAIL,
&format!("remove empty codex home: failed, {error:#}"),
);
}
}
crate::service::log_information(
crate::service::EVENT_CLEANUP_FINISHED,
"sandbox uninstall cleanup finished",
if runtime.is_some() {
EVENT_CLEANUP_DETAIL
} else {
crate::service::EVENT_CLEANUP_FINISHED
},
if runtime.is_some() {
"native sandbox cleanup finished; registered runtime cleanup pending"
} else {
"sandbox uninstall cleanup finished"
},
);
Ok(())
}
fn remove_empty_directory(path: &Path, target: &str) {
match std::fs::remove_dir(path) {
Ok(()) => log_cleanup(&format!("removed empty {target}")),
Err(error) if error.kind() == io::ErrorKind::NotFound => {
log_cleanup(&format!("skipping empty {target} removal: not found"));
}
Err(error) if error.kind() == io::ErrorKind::DirectoryNotEmpty => {
log_cleanup(&format!("preserving {target}: directory is not empty"));
}
Err(error) => {
log_error(
EVENT_CLEANUP_DETAIL,
&format!("remove empty {target}: failed, {error}"),
);
}
}
}
fn log_cleanup(message: &str) {
log_information(EVENT_CLEANUP_DETAIL, message);
}
@@ -8,6 +8,7 @@ use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use codex_windows_sandbox::prepare_packaged_windows_sandbox_cleanup_with_retained_tokens;
use windows::ApplicationModel::Package;
use windows::ApplicationModel::PackageUninstallingEventArgs;
use windows::Management::Deployment::PackageManager;
use windows::core::HSTRING;
@@ -60,9 +61,7 @@ pub(super) fn clean_up(lifecycle: &PackageLifecycle, record: InstallationRecord)
"interrupted runtime cleanup requires repair"
);
ensure!(
!runtime_owner_has_package(&record)?
|| (lifecycle.uninstalling.load(Ordering::Acquire)
&& !crate::registered_runtime::has_retaining_registration(&record)?),
owner_allows_cleanup(&lifecycle.uninstalling, &record)?,
"registered runtime owner still has the app installed"
);
ensure!(
@@ -88,9 +87,7 @@ pub(super) fn clean_up(lifecycle: &PackageLifecycle, record: InstallationRecord)
prepare_packaged_windows_sandbox_cleanup_with_retained_tokens(&removal.tokens())?;
// A reinstall before this boundary cancels without retiring resources.
ensure!(
(lifecycle.uninstalling.load(Ordering::Acquire)
&& !crate::registered_runtime::has_retaining_registration(&record)?)
|| !runtime_owner_has_package(&record)?,
owner_allows_cleanup(&lifecycle.uninstalling, &record)?,
"owner reinstalled before native cleanup started"
);
crate::installation_record::save_runtime(&record)?;
@@ -100,6 +97,36 @@ pub(super) fn clean_up(lifecycle: &PackageLifecycle, record: InstallationRecord)
removal.commit()
}
pub(super) fn owner_allows_cleanup(
uninstalling: &AtomicBool,
record: &InstallationRecord,
) -> Result<bool> {
let packages = PackageManager::new()?.FindPackagesByUserSecurityIdPackageFamilyName(
&HSTRING::from(&record.user_sid),
&HSTRING::from(&record.runtime()?.package_family),
)?;
let iterator = packages.First()?;
let mut has_current = iterator.HasCurrent()?;
if !has_current {
return Ok(true);
}
if !uninstalling.load(Ordering::Acquire)
|| crate::registered_runtime::has_retaining_registration(record)?
{
return Ok(false);
}
// SCM may stop us before this exact package is removed. A successor version
// belongs to an update or reinstall and must keep its desktop data.
let retiring_package = Package::Current()?.Id()?.FullName()?;
while has_current {
if iterator.Current()?.Id()?.FullName()? != retiring_package {
return Ok(false);
}
has_current = iterator.MoveNext()?;
}
Ok(true)
}
/// The managed registration must not keep the package alive after its owner
/// uninstalls it, but an update to another version of the family is not uninstall.
pub(crate) fn runtime_owner_removed() -> Result<bool> {
@@ -22,6 +22,17 @@ public static class CleanupNative {
[StructLayout(LayoutKind.Sequential)] public struct Priv { public uint count; public Luid luid; public uint attributes; }
[DllImport("userenv.dll",CharSet=CharSet.Unicode,SetLastError=true)] public static extern bool LoadUserProfile(IntPtr t,ref Profile p);
[DllImport("userenv.dll",SetLastError=true)] public static extern bool UnloadUserProfile(IntPtr t,IntPtr p);
[DllImport("userenv.dll",CharSet=CharSet.Unicode,SetLastError=true)] public static extern bool DeleteProfile(string sid,string path,string computer);
[StructLayout(LayoutKind.Sequential)] struct User23 { public IntPtr name,fullName,comment; public uint flags; public IntPtr sid; }
[DllImport("netapi32.dll",CharSet=CharSet.Unicode)] static extern int NetUserGetInfo(string server,string user,int level,out IntPtr info);
[DllImport("netapi32.dll",CharSet=CharSet.Unicode)] static extern int NetUserDel(string server,string user);
[DllImport("netapi32.dll")] static extern int NetApiBufferFree(IntPtr info);
[DllImport("netapi32.dll",CharSet=CharSet.Unicode)] public static extern int NetLocalGroupDel(string server,string group);
[DllImport("advapi32.dll",CharSet=CharSet.Unicode)] static extern IntPtr RegisterEventSource(string server,string source);
[DllImport("advapi32.dll",CharSet=CharSet.Unicode)] static extern bool ReportEvent(IntPtr h,ushort type,ushort category,uint id,IntPtr sid,ushort count,uint size,string[] messages,IntPtr data);
[DllImport("advapi32.dll")] static extern bool DeregisterEventSource(IntPtr h);
public static void Log(string source,ushort type,uint id,string message) { IntPtr h=RegisterEventSource(null,source); if(h==IntPtr.Zero)return; try { ReportEvent(h,type,0,id,IntPtr.Zero,1,0,new[]{message.Substring(0,Math.Min(message.Length,4096))},IntPtr.Zero); } finally { DeregisterEventSource(h); } }
public static void DeleteUser(string user,string sid) { IntPtr p; int status=NetUserGetInfo(null,user,23,out p); if(status==2221)return; if(status!=0)throw new Win32Exception(status); try { User23 info=(User23)Marshal.PtrToStructure(p,typeof(User23)); if(new System.Security.Principal.SecurityIdentifier(info.sid).Value!=sid || (info.flags&2)==0)throw new InvalidOperationException("Sandbox account changed or was re-enabled"); } finally { NetApiBufferFree(p); } status=NetUserDel(null,user); if(status!=0 && status!=2221)throw new Win32Exception(status); }
[DllImport("advapi32.dll",SetLastError=true)] public static extern bool ImpersonateLoggedOnUser(IntPtr t);
[DllImport("advapi32.dll",SetLastError=true)] public static extern bool RevertToSelf();
[DllImport("kernel32.dll")] public static extern bool CloseHandle(IntPtr h);
@@ -40,6 +51,7 @@ $null = [Windows.Management.Deployment.DeploymentProgress,Windows.Management.Dep
$asTask = [System.WindowsRuntimeSystemExtensions].GetMethods() | Where-Object { $_.Name -eq 'AsTask' -and $_.IsGenericMethod -and $_.GetGenericArguments().Count -eq 2 -and $_.GetParameters().Count -eq 1 } | Select-Object -First 1
$await = $asTask.MakeGenericMethod([Windows.Management.Deployment.DeploymentResult],[Windows.Management.Deployment.DeploymentProgress])
$profiles = @()
$removedRegistrations = @{}
try {
[CleanupNative]::Enable('SeBackupPrivilege')
[CleanupNative]::Enable('SeRestorePrivilege')
@@ -79,6 +91,7 @@ try {
$record.codex_home -ne $plan.record.codex_home -or
$record.runtime.package_family -ne $plan.record.runtime.package_family) { return }
foreach ($entry in $profiles) {
if ($removedRegistrations.ContainsKey($entry.sid)) { continue }
foreach ($package in @(Appx\Get-AppxPackage -User $entry.sid -ErrorAction Stop)) {
if ($package.PackageFamilyName -ne $record.runtime.package_family) { continue }
if (![CleanupNative]::ImpersonateLoggedOnUser($entry.token)) { throw 'Cleanup impersonation failed' }
@@ -100,9 +113,35 @@ try {
}
}
foreach ($account in $plan.record.runtime.accounts) {
if ($removedRegistrations.ContainsKey($account.user_sid)) { continue }
foreach ($package in @(Appx\Get-AppxPackage -User $account.user_sid -ErrorAction Stop)) {
if ($package.PackageFamilyName -eq $record.runtime.package_family) { throw 'Runtime registration remains' }
}
$removedRegistrations[$account.user_sid] = $true
}
foreach ($account in $plan.record.runtime.accounts) {
$sid = $account.user_sid
foreach ($entry in $profiles) {
if ($entry.sid -ne $sid -or $entry.profile.handle -eq [IntPtr]::Zero) { continue }
if (![CleanupNative]::UnloadUserProfile($entry.token, $entry.profile.handle)) { throw [ComponentModel.Win32Exception]::new([Runtime.InteropServices.Marshal]::GetLastWin32Error()) }
$entry.profile.handle = [IntPtr]::Zero
}
$profileKey = [Microsoft.Win32.Registry]::LocalMachine.OpenSubKey("SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList\$sid")
if ($profileKey) {
$profileKey.Dispose()
# PowerShell converts $null to an empty string; DeleteProfile requires native NULL.
if (![CleanupNative]::DeleteProfile($sid, [NullString]::Value, [NullString]::Value)) { throw [ComponentModel.Win32Exception]::new([Runtime.InteropServices.Marshal]::GetLastWin32Error()) }
}
[CleanupNative]::DeleteUser(('CodexSandbox' + $account.account), $sid)
[CleanupNative]::Log($plan.service_name, 4, 3004, "Removed $($account.account) sandbox profile and account")
}
$groupSid = $null
try { if ($plan.group_sid) { $groupSid = ([Security.Principal.NTAccount]::new('CodexSandboxUsers')).Translate([Security.Principal.SecurityIdentifier]).Value } }
catch [Security.Principal.IdentityNotMappedException] { }
if ($groupSid) {
if ($groupSid -cne $plan.group_sid) { throw 'Sandbox group was replaced' }
$status = [CleanupNative]::NetLocalGroupDel($null, 'CodexSandboxUsers')
if ($status -ne 0 -and $status -ne 2220) { throw [ComponentModel.Win32Exception]::new($status) }
}
$legacy = [Microsoft.Win32.Registry]::LocalMachine.OpenSubKey($plan.legacy_key, $true)
if (!$legacy) { throw 'Installation parent is missing' }
@@ -114,8 +153,13 @@ try {
$key.Dispose(); $key = $null
$legacy.Flush()
[Microsoft.Win32.Registry]::LocalMachine.DeleteSubKey($plan.key, $false)
if ($legacy.SubKeyCount -eq 0 -and $legacy.ValueCount -eq 0) {
$legacy.Dispose(); $legacy = $null
[Microsoft.Win32.Registry]::LocalMachine.DeleteSubKey($plan.legacy_key, $false)
}
$finished = $true
} catch { [Console]::Error.WriteLine("Runtime package removal deferred: $_") }
[CleanupNative]::Log($plan.service_name, 4, 3003, 'Sandbox uninstall cleanup finished, including registered runtimes, profiles, and accounts')
} catch { [CleanupNative]::Log($plan.service_name, 1, 3004, "Runtime cleanup deferred: $_") }
finally {
if ($key) { $key.Dispose() }
if ($legacy) { $legacy.Dispose() }
@@ -126,6 +170,7 @@ try {
}
} finally {
foreach ($entry in $profiles) {
if ($entry.profile.handle -eq [IntPtr]::Zero) { continue }
while (![CleanupNative]::UnloadUserProfile($entry.token, $entry.profile.handle)) {
Microsoft.PowerShell.Utility\Start-Sleep -Seconds 1
}
@@ -19,7 +19,6 @@ use std::time::Instant;
use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use base64::Engine;
use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS;
use windows_sys::Win32::Foundation::DuplicateHandle;
use windows_sys::Win32::Foundation::HANDLE;
@@ -95,6 +94,16 @@ pub(crate) fn prepare(owner_token: HANDLE, record: &InstallationRecord) -> Resul
}));
tokens.push(token);
}
let group_sid = if tokens.is_empty() {
None
} else {
Some(
codex_windows_sandbox::string_from_sid_bytes(&codex_windows_sandbox::resolve_sid(
codex_windows_sandbox::SANDBOX_USERS_GROUP,
)?)
.map_err(anyhow::Error::msg)?,
)
};
let mut system = [0u16; 32768];
let length = unsafe { GetSystemDirectoryW(system.as_mut_ptr(), system.len() as u32) } as usize;
ensure!(
@@ -113,13 +122,9 @@ pub(crate) fn prepare(owner_token: HANDLE, record: &InstallationRecord) -> Resul
== 0,
"invalid Windows PowerShell executable"
);
let bytes = include_str!("removal.ps1")
.encode_utf16()
.flat_map(u16::to_le_bytes)
.collect::<Vec<_>>();
let encoded = base64::engine::general_purpose::STANDARD.encode(bytes);
let script = include_str!("removal.ps1");
ensure!(
executable.as_os_str().len() + encoded.len() + 64 < 32767,
executable.as_os_str().len() + script.len() * 2 + 64 < 32767,
"cleanup command exceeds the Windows command-line limit"
);
let mut child = Command::new(executable)
@@ -127,8 +132,8 @@ pub(crate) fn prepare(owner_token: HANDLE, record: &InstallationRecord) -> Resul
"-NoLogo",
"-NoProfile",
"-NonInteractive",
"-EncodedCommand",
&encoded,
"-Command",
script,
])
.current_dir(system)
.stdin(Stdio::piped())
@@ -166,6 +171,7 @@ pub(crate) fn prepare(owner_token: HANDLE, record: &InstallationRecord) -> Resul
"record": record,
"targets": targets,
"service_name": codex_windows_sandbox::windows_sandbox_service_name()?,
"group_sid": group_sid,
});
let input = child.stdin.as_mut().context("capture cleanup input")?;
serde_json::to_writer(&mut *input, &plan)?;
@@ -19,6 +19,53 @@ use windows_sys::Win32::System::SystemInformation::GetSystemDirectoryW;
use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW;
use windows_sys::Win32::System::Threading::WaitForSingleObject;
#[test]
fn finalizer_script_parses_and_native_bindings_compile() -> Result<()> {
let executable =
PathBuf::from(std::env::var_os("SystemRoot").context("SystemRoot is missing")?)
.join(r"System32\WindowsPowerShell\v1.0\powershell.exe");
let script = include_str!("removal.ps1");
let native_start = script
.find("Add-Type -TypeDefinition @\"")
.context("find native bindings")?;
let native_end = native_start
+ script[native_start..]
.find("\n\"@")
.context("find binding boundary")?
+ 3;
let validation = format!(
"$ErrorActionPreference = 'Stop'; $tokens = $null; $errors = $null;\n\
$null = [Management.Automation.Language.Parser]::ParseInput([Console]::In.ReadToEnd(), [ref]$tokens, [ref]$errors);\n\
if ($errors.Count) {{ throw ($errors | Out-String) }}\n{}",
&script[native_start..native_end],
);
let mut child = Command::new(executable)
.args([
"-NoLogo",
"-NoProfile",
"-NonInteractive",
"-Command",
&validation,
])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.creation_flags(CREATE_NO_WINDOW)
.spawn()?;
child
.stdin
.take()
.context("capture parser input")?
.write_all(script.as_bytes())?;
let output = child.wait_with_output()?;
ensure!(
output.status.success(),
"finalizer validation failed: {}",
String::from_utf8_lossy(&output.stderr)
);
Ok(())
}
#[test]
fn finalizer_preserves_utf8_plan_and_requires_commit_and_stdin_eof() -> Result<()> {
let mut system = [0u16; 32768];
@@ -51,6 +51,7 @@ 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;
pub(crate) const EVENT_CLEANUP_DETAIL: u32 = 3004;
const MAX_EVENT_MESSAGE_UNITS: usize = 1024;
static SERVICE_STATE: OnceLock<ServiceState> = OnceLock::new();
@@ -121,6 +121,7 @@ pub(super) fn run(state: &ServiceState, package_lifecycle: &PackageLifecycle) ->
/// Retries the current teardown step, never a phase recovered from stored intent.
pub(crate) fn retry_cleanup(mut operation: impl FnMut() -> Result<()>) -> Result<()> {
let mut stop_failures = 0;
loop {
let Err(error) = operation() else {
return Ok(());
@@ -132,7 +133,15 @@ pub(crate) fn retry_cleanup(mut operation: impl FnMut() -> Result<()>) -> Result
EVENT_SERVICE_FAILED,
&format!("sandbox cleanup deferred: {error:#}"),
);
if !wait_for_cleanup_retry(state) {
if state.stop_requested.load(Ordering::Acquire) {
// SCM stop initiates uninstall cleanup rather than cancelling it.
// Four one-second retry waits fit within the 10-second SCM wait hint.
stop_failures += 1;
if stop_failures == 5 {
return Err(error);
}
std::thread::sleep(std::time::Duration::from_secs(1));
} else if !wait_for_cleanup_retry(state) && !state.stop_requested.load(Ordering::Acquire) {
return Err(error);
}
}
@@ -147,3 +156,7 @@ fn wait_for_cleanup_retry(state: &ServiceState) -> bool {
}
!state.shutdown.load(Ordering::Acquire)
}
#[cfg(test)]
#[path = "runtime_lifecycle_tests.rs"]
mod tests;
@@ -0,0 +1,79 @@
//! Verifies teardown retries distinguish SCM stop from system shutdown.
use super::retry_cleanup;
use crate::service::SERVICE_STATE;
use crate::service::ServiceState;
use crate::service::service_control_handler;
use anyhow::Context;
use anyhow::Result;
use pretty_assertions::assert_eq;
use std::io;
use std::ptr;
use std::sync::Arc;
use std::sync::OnceLock;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::AtomicU32;
use std::sync::atomic::Ordering;
use windows_sys::Win32::Foundation::ERROR_SHARING_VIOLATION;
use windows_sys::Win32::Foundation::NO_ERROR;
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_STOPPED;
#[test]
fn cleanup_retries_after_scm_stop_but_not_system_shutdown() -> Result<()> {
// One test owns the process-global state. The pre-set shutdown flag keeps
// the real control handler from calling SCM or spawning a pipe wake thread.
assert!(
SERVICE_STATE
.set(ServiceState {
service_name: "CodexSandboxServiceTests".into(),
pipe_name: r"\\.\pipe\CodexSandboxServiceTests".into(),
shutdown: Arc::new(AtomicBool::new(true)),
uninstalling: Arc::new(AtomicBool::new(false)),
status_handle: OnceLock::new(),
current_status: AtomicU32::new(SERVICE_STOPPED),
stop_requested: AtomicBool::new(false),
})
.is_ok()
);
let state = SERVICE_STATE.get().context("service state is missing")?;
for (control, failures, expected_attempts, expected_success) in [
(SERVICE_CONTROL_SHUTDOWN, 1, 1, false),
(SERVICE_CONTROL_STOP, 1, 2, true),
(SERVICE_CONTROL_STOP, usize::MAX, 5, false),
] {
assert_eq!(
unsafe {
service_control_handler(
control,
/*_event_type*/ 0,
ptr::null_mut(),
ptr::null_mut(),
)
},
NO_ERROR
);
let mut attempts = 0;
let result = retry_cleanup(|| {
attempts += 1;
if attempts <= failures {
return Err(io::Error::from_raw_os_error(ERROR_SHARING_VIOLATION as i32).into());
}
Ok(())
});
assert_eq!(result.is_ok(), expected_success);
assert_eq!(attempts, expected_attempts);
assert!(state.shutdown.load(Ordering::Acquire));
if let Err(error) = result {
assert_eq!(
error
.downcast_ref::<io::Error>()
.context("cleanup did not preserve the I/O error")?
.raw_os_error(),
Some(ERROR_SHARING_VIOLATION as i32)
);
}
}
Ok(())
}