github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 04:23:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Split Windows sandbox cleanup into preparation and completion phases (#45178)

Browse Source 
## What changed

Expose `prepare_packaged_windows_sandbox_cleanup` to disable sandbox accounts and stop their processes before returning a `PreparedWindowsSandboxCleanup` guard that retains the setup lock. Its `finish` method removes resources and protections; dropping it only releases the lock, leaving accounts disabled and protections intact.

Keep `clean_up_packaged_windows_sandbox` as a wrapper around both phases, and extract the service's existing package cleanup logic into a dedicated module.

GitOrigin-RevId: a32663171bce027f771f738d71b0c46404edbedc
This commit is contained in:
zm-oai
2026-09-13 06:15:35 +00:00
committed by copyberry
parent c379459bba
commit a4c61afff2
4 changed files with 153 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/windows-sandbox-rs/src/lib.rs
+4
View File
@@ -414,8 +414,12 @@ 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::PreparedWindowsSandboxCleanup;
#[cfg(target_os = "windows")]
pub use uninstall_windows::clean_up_packaged_windows_sandbox;
#[cfg(target_os = "windows")]
pub use uninstall_windows::prepare_packaged_windows_sandbox_cleanup;
#[cfg(target_os = "windows")]
pub use wfp::install_wfp_filters_for_account;
#[cfg(target_os = "windows")]
pub use wfp_setup::install_wfp_filters;
codex-rs/windows-sandbox-rs/src/uninstall_windows.rs
+76 -33
View File
@@ -1,4 +1,4 @@
//! Stops sandbox work before removing its protections, then continues independent cleanup.
//! Stops sandbox work under the setup lock before removing its resources and protections.
use std::path::Path;
@@ -19,7 +19,30 @@ 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)?;
prepare_packaged_windows_sandbox_cleanup()?.finish(codex_home, clean_up_desktop)
}
/// Holds the setup lock after sandbox accounts are disabled, their processes
/// have exited, and their logon tokens have been released.
///
/// Keep this guard on the thread that acquired the setup lock. Callers must not
/// re-enable the accounts or create new sandbox logons before `finish`.
///
/// Dropping the guard only releases the lock. It does not re-enable accounts,
/// remove protections, or undo work performed between the two phases.
#[must_use = "finish cleanup or leave the sandbox accounts disabled with their protections intact"]
pub struct PreparedWindowsSandboxCleanup {
_setup_lock: crate::setup_mutex::SandboxSetupLock,
}
/// Disables sandbox accounts, stops their processes, and waits for their logon
/// tokens to be released.
///
/// No resources or protections are removed here. On failure, restoration of the
/// original account flags is attempted before the setup lock is released; any
/// restoration failure is included in the returned error.
pub fn prepare_packaged_windows_sandbox_cleanup() -> Result<PreparedWindowsSandboxCleanup> {
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)) {
@@ -31,42 +54,62 @@ pub fn clean_up_packaged_windows_sandbox(
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()));
Ok(PreparedWindowsSandboxCleanup {
_setup_lock: setup_lock,
})
}
impl PreparedWindowsSandboxCleanup {
/// Removes resources while retaining the setup lock and disabled accounts.
///
/// Keep a supplied home and its ancestors pinned until `clean_up_desktop`
/// starts. Independent cleanup steps continue after an error, as in
/// `clean_up_packaged_windows_sandbox`.
pub fn finish(
self,
codex_home: Option<&Path>,
clean_up_desktop: impl FnOnce() -> Result<()>,
) -> Result<()> {
let _prepared = self;
let mut errors = Vec::new();
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 {
if let Err(error) = clean_up_desktop() {
errors.push(format!("{error:#}"));
}
}
if errors.is_empty() {
Ok(())
} else {
Err(anyhow!(errors.join("; ")))
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("; ")))
}
}
}
codex-rs/windows-sandbox-service/src/package_lifecycle.rs
+3 -56
View File
@@ -10,7 +10,6 @@ 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;
@@ -32,6 +31,8 @@ use crate::installation_record::DesktopInstallation;
use crate::installation_record::InstallationRecord;
use crate::ipc::OwnedHandle;
mod cleanup;
struct UserInstallation {
codex_home: Option<PathBuf>,
directory_handles: Vec<OwnedHandle>,
@@ -187,61 +188,7 @@ impl PackageLifecycle {
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(())
cleanup::clean_up(installation)
}
}
codex-rs/windows-sandbox-service/src/package_lifecycle/cleanup.rs
+70
View File
@@ -0,0 +1,70 @@
//! Removes one authenticated owner's sandbox resources during package uninstall.
//! Desktop paths are deleted only while impersonating the owner; native cleanup keeps its order.
use std::io;
use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use codex_windows_sandbox::clean_up_packaged_windows_sandbox;
use super::UserInstallation;
use super::with_owner_impersonation;
pub(super) fn clean_up(installation: &mut UserInstallation) -> Result<()> {
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(())
}