mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Honor home deletion preferences during Windows uninstall (#54708)
## What changed Read `desktop.uninstall_codex_home` from the owner's current `config.toml` before recursively deleting a desktop-created Codex home during Windows sandbox uninstall. - Preserve the home by default for ordinary installations; retain the deletion default for OEM preinstalls when no preference is set. - Let an explicit boolean preference override either default. Preserve the home when configuration is unreadable, malformed, or contains an invalid preference. - Capture OEM metadata before Windows removes it during uninstall, and revoke sandbox permissions on retained homes. ## Testing Add unit tests for missing preferences, explicit overrides, malformed or invalid configuration, and configuration read failures across both installation defaults. GitOrigin-RevId: 46afc2a7b0f4ecf7f447e46b515e50d0d278e4a8
This commit is contained in:
Generated
+1
@@ -5398,6 +5398,7 @@ dependencies = [
|
||||
"codex-windows-sandbox",
|
||||
"pretty_assertions",
|
||||
"serde_json",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"toml 0.9.11+spec-1.1.0",
|
||||
"windows 0.58.0",
|
||||
|
||||
@@ -28,6 +28,7 @@ toml = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
[target.'cfg(windows)'.dependencies.windows]
|
||||
version = "0.58"
|
||||
@@ -35,6 +36,7 @@ features = [
|
||||
"ApplicationModel",
|
||||
"Foundation_Collections",
|
||||
"Management_Deployment",
|
||||
"Storage",
|
||||
"Win32_System_WinRT",
|
||||
]
|
||||
|
||||
|
||||
@@ -17,6 +17,8 @@ mod provisioning;
|
||||
mod registered_runtime;
|
||||
#[cfg(windows)]
|
||||
mod service;
|
||||
#[cfg(any(windows, test))]
|
||||
mod uninstall_policy;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum RunMode {
|
||||
|
||||
@@ -56,6 +56,7 @@ struct UserInstallation {
|
||||
|
||||
pub(crate) struct PackageLifecycle {
|
||||
package_name: HSTRING,
|
||||
preinstalled: bool,
|
||||
uninstalling: Arc<AtomicBool>,
|
||||
installation: RefCell<Option<UserInstallation>>,
|
||||
}
|
||||
@@ -64,8 +65,20 @@ impl PackageLifecycle {
|
||||
pub(crate) fn new(uninstalling: Arc<AtomicBool>) -> Result<Self> {
|
||||
unsafe { RoInitialize(RO_INIT_MULTITHREADED) }
|
||||
.context("initialize the sandbox service Windows Runtime apartment")?;
|
||||
let package = Package::Current()?;
|
||||
// Windows removes OEM metadata on uninstall; capture it while the package is installed.
|
||||
let preinstalled = package
|
||||
.InstalledLocation()
|
||||
.and_then(|location| location.Path())
|
||||
.is_ok_and(|path| {
|
||||
PathBuf::from(path.to_os_string())
|
||||
.join("microsoft.system.package.metadata")
|
||||
.join("Custom.data")
|
||||
.is_file()
|
||||
});
|
||||
Ok(Self {
|
||||
package_name: Package::Current()?.Id()?.FullName()?,
|
||||
package_name: package.Id()?.FullName()?,
|
||||
preinstalled,
|
||||
uninstalling,
|
||||
installation: RefCell::default(),
|
||||
})
|
||||
@@ -265,7 +278,13 @@ impl PackageLifecycle {
|
||||
let installation = installation
|
||||
.as_mut()
|
||||
.context("the authenticated package installation was not recorded")?;
|
||||
cleanup::clean_up(installation, prepared, runtime, &self.uninstalling)
|
||||
cleanup::clean_up(
|
||||
installation,
|
||||
prepared,
|
||||
runtime,
|
||||
&self.uninstalling,
|
||||
/*preinstalled*/ self.preinstalled,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ pub(super) fn clean_up(
|
||||
prepared: &PreparedWindowsSandboxCleanup,
|
||||
runtime: Option<&InstallationRecord>,
|
||||
uninstalling: &AtomicBool,
|
||||
preinstalled: bool,
|
||||
) -> Result<()> {
|
||||
crate::service::log_information(
|
||||
crate::service::EVENT_CLEANUP_STARTED,
|
||||
@@ -73,7 +74,9 @@ pub(super) fn clean_up(
|
||||
}
|
||||
};
|
||||
if let Some(home) = &codex_home {
|
||||
if desktop.created_codex_home {
|
||||
if desktop.created_codex_home
|
||||
&& crate::uninstall_policy::clean_uninstall_enabled(home, preinstalled)
|
||||
{
|
||||
// Release the home itself so it can be deleted; keep its ancestors pinned.
|
||||
if installation.directory_guard.take().is_some() {
|
||||
installation.directory_handles.pop();
|
||||
@@ -84,8 +87,8 @@ pub(super) fn clean_up(
|
||||
);
|
||||
} 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.
|
||||
log_cleanup("preserving codex home");
|
||||
// Retained data must not keep permissions for the deleted sandbox group.
|
||||
record_result(
|
||||
"remove codex home sandbox permissions",
|
||||
resolve_sid("CodexSandboxUsers")
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
//! Reads the owner's current home-deletion preference; existing OEM preinstalls retain their default.
|
||||
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_config::AbsolutePathBufGuard;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
|
||||
pub(crate) fn clean_uninstall_enabled(home: &Path, preinstalled: bool) -> bool {
|
||||
let contents = match std::fs::read_to_string(home.join("config.toml")) {
|
||||
Ok(contents) => contents,
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return preinstalled,
|
||||
Err(_) => return false,
|
||||
};
|
||||
let _guard = AbsolutePathBufGuard::new(home);
|
||||
let Ok(config) = toml::from_str::<ConfigToml>(&contents) else {
|
||||
return false;
|
||||
};
|
||||
config
|
||||
.desktop
|
||||
.as_ref()
|
||||
.and_then(|desktop| desktop.get("uninstall_codex_home"))
|
||||
.map_or(preinstalled, |value| value.as_bool().unwrap_or(false))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "uninstall_policy_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,74 @@
|
||||
//! Verifies local configuration precedence over the preinstalled cleanup default.
|
||||
|
||||
use anyhow::Result;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::clean_uninstall_enabled;
|
||||
|
||||
/// Existing preinstalls keep clean uninstall when no user preference exists.
|
||||
#[test]
|
||||
fn missing_preference_uses_preinstall_default() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
for preinstalled in [false, true] {
|
||||
assert_eq!(
|
||||
clean_uninstall_enabled(home.path(), preinstalled),
|
||||
preinstalled
|
||||
);
|
||||
}
|
||||
for contents in ["", "log_dir = 'logs'", "[desktop]\ntheme = 'dark'"] {
|
||||
std::fs::write(home.path().join("config.toml"), contents)?;
|
||||
for preinstalled in [false, true] {
|
||||
assert_eq!(
|
||||
clean_uninstall_enabled(home.path(), preinstalled),
|
||||
preinstalled
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A user's explicit choice overrides both ordinary and preinstalled defaults.
|
||||
#[test]
|
||||
fn explicit_preference_overrides_preinstall_default() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
for enabled in [false, true] {
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!("log_dir = 'logs'\n[desktop]\nuninstall_codex_home = {enabled}"),
|
||||
)?;
|
||||
for preinstalled in [false, true] {
|
||||
assert_eq!(clean_uninstall_enabled(home.path(), preinstalled), enabled);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Invalid configuration never falls back to destructive cleanup, even on a preload.
|
||||
#[test]
|
||||
fn malformed_or_invalid_preference_preserves_home() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
for contents in [
|
||||
"[desktop",
|
||||
"desktop = false",
|
||||
"[desktop]\nuninstall_codex_home = 'true'",
|
||||
"[desktop]\nuninstall_codex_home = 1",
|
||||
"[desktop]\nuninstall_codex_home = []",
|
||||
] {
|
||||
std::fs::write(home.path().join("config.toml"), contents)?;
|
||||
for preinstalled in [false, true] {
|
||||
assert!(!clean_uninstall_enabled(home.path(), preinstalled));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Failure to read an existing config must not restore the preinstalled deletion default.
|
||||
#[test]
|
||||
fn unreadable_config_preserves_home() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
std::fs::create_dir(home.path().join("config.toml"))?;
|
||||
for preinstalled in [false, true] {
|
||||
assert!(!clean_uninstall_enabled(home.path(), preinstalled));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user