From be520ab93e568dd58f8d7bbbbacb25bc0bea185c Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Sat, 21 Mar 2026 10:41:55 -0700 Subject: [PATCH] Add Windows sandbox uninstall flow --- codex-rs/cli/src/main.rs | 49 +++++++++ codex-rs/windows-sandbox-rs/src/firewall.rs | 59 ++++++++++ codex-rs/windows-sandbox-rs/src/lib.rs | 4 + .../windows-sandbox-rs/src/sandbox_users.rs | 37 +++++++ .../windows-sandbox-rs/src/setup_main_win.rs | 101 ++++++++++++++++++ .../src/setup_orchestrator.rs | 41 +++++++ 6 files changed, 291 insertions(+) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9386398284..5f3638b18c 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -31,6 +31,7 @@ use codex_tui::update_action::UpdateAction; use codex_utils_cli::CliConfigOverrides; use owo_colors::OwoColorize; use std::io::IsTerminal; +use std::io::Write; use std::path::PathBuf; use supports_color::Stream; @@ -254,8 +255,15 @@ enum SandboxCommand { /// Run a command under Windows restricted token (Windows only). Windows(WindowsCommand), + + /// Remove Windows sandbox setup state created by Codex. + #[clap(name = "windows-uninstall")] + WindowsUninstall(WindowsUninstallCommand), } +#[derive(Debug, Parser)] +struct WindowsUninstallCommand {} + #[derive(Debug, Parser)] struct ExecpolicyCommand { #[command(subcommand)] @@ -494,6 +502,40 @@ fn run_execpolicycheck(cmd: ExecPolicyCheckCommand) -> anyhow::Result<()> { cmd.run() } +fn run_windows_sandbox_uninstall() -> anyhow::Result<()> { + #[cfg(target_os = "windows")] + { + let codex_home = find_codex_home()?; + codex_windows_sandbox::run_elevated_teardown(&codex_home)?; + ConfigEditsBuilder::new(&codex_home) + .with_edits([ + codex_core::config::edit::ConfigEdit::ClearPath { + segments: vec!["windows".to_string(), "sandbox".to_string()], + }, + codex_core::config::edit::ConfigEdit::ClearPath { + segments: vec!["windows".to_string(), "sandbox_private_desktop".to_string()], + }, + ]) + .set_feature_enabled(codex_core::features::Feature::WindowsSandbox.key(), false) + .set_feature_enabled( + codex_core::features::Feature::WindowsSandboxElevated.key(), + false, + ) + .clear_legacy_windows_sandbox_keys() + .apply_blocking()?; + writeln!( + std::io::stdout(), + "Windows sandbox state removed from Codex and local sandbox setup cleaned up." + )?; + Ok(()) + } + + #[cfg(not(target_os = "windows"))] + { + anyhow::bail!("`codex sandbox windows-uninstall` is only available on Windows"); + } +} + async fn run_debug_app_server_command(cmd: DebugAppServerCommand) -> anyhow::Result<()> { match cmd.subcommand { DebugAppServerSubcommand::SendMessageV2(cmd) => { @@ -823,6 +865,13 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { ) .await?; } + SandboxCommand::WindowsUninstall(_) => { + reject_remote_mode_for_subcommand( + root_remote.as_deref(), + "sandbox windows-uninstall", + )?; + run_windows_sandbox_uninstall()?; + } }, Some(Subcommand::Debug(DebugCommand { subcommand })) => match subcommand { DebugSubcommand::AppServer(cmd) => { diff --git a/codex-rs/windows-sandbox-rs/src/firewall.rs b/codex-rs/windows-sandbox-rs/src/firewall.rs index b4939fd42b..5e5f2e4bb2 100644 --- a/codex-rs/windows-sandbox-rs/src/firewall.rs +++ b/codex-rs/windows-sandbox-rs/src/firewall.rs @@ -9,6 +9,7 @@ use windows::core::BSTR; use windows::Win32::Foundation::VARIANT_TRUE; use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2; use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule3; +use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRules; use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2; use windows::Win32::NetworkManagement::WindowsFirewall::NetFwRule; use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_ACTION_BLOCK; @@ -78,6 +79,46 @@ pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Resul result } +pub fn remove_offline_outbound_block(log: &mut File) -> Result<()> { + let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; + if hr.is_err() { + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallComInitFailed, + format!("CoInitializeEx failed: {hr:?}"), + ))); + } + + let result = unsafe { + (|| -> Result<()> { + let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallPolicyAccessFailed, + format!("CoCreateInstance NetFwPolicy2 failed: {err:?}"), + )) + })?; + let rules = policy.Rules().map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallPolicyAccessFailed, + format!("INetFwPolicy2::Rules failed: {err:?}"), + )) + })?; + + remove_rule(&rules, OFFLINE_BLOCK_RULE_NAME)?; + log_line( + log, + &format!("firewall rule removed name={OFFLINE_BLOCK_RULE_NAME}"), + )?; + Ok(()) + })() + }; + + unsafe { + CoUninitialize(); + } + result +} + fn ensure_block_rule( rules: &windows::Win32::NetworkManagement::WindowsFirewall::INetFwRules, internal_name: &str, @@ -141,6 +182,24 @@ fn ensure_block_rule( Ok(()) } +fn remove_rule(rules: &INetFwRules, internal_name: &str) -> Result<()> { + let name = BSTR::from(internal_name); + match unsafe { rules.Remove(&name) } { + Ok(()) => Ok(()), + Err(err) => { + let msg = format!("{err:?}"); + if msg.contains("0x80070002") { + Ok(()) + } else { + Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("Rules::Remove failed: {err:?}"), + ))) + } + } + } +} + fn configure_rule( rule: &INetFwRule3, friendly_desc: &str, diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index cb4be52750..3548d9d959 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -58,6 +58,8 @@ pub use acl::fetch_dacl_handle; #[cfg(target_os = "windows")] pub use acl::path_mask_allows; #[cfg(target_os = "windows")] +pub use acl::revoke_ace; +#[cfg(target_os = "windows")] pub use audit::apply_world_writable_scan_and_denies; #[cfg(target_os = "windows")] pub use cap::load_or_create_cap_sids; @@ -106,6 +108,8 @@ pub use process::StdinMode; #[cfg(target_os = "windows")] pub use setup::run_elevated_setup; #[cfg(target_os = "windows")] +pub use setup::run_elevated_teardown; +#[cfg(target_os = "windows")] pub use setup::run_setup_refresh; #[cfg(target_os = "windows")] pub use setup::run_setup_refresh_with_extra_read_roots; diff --git a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs index 5ab39c3cec..96b7b7bb7d 100644 --- a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs +++ b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs @@ -15,10 +15,14 @@ use std::path::PathBuf; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::LocalFree; use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER; +use windows_sys::Win32::NetworkManagement::NetManagement::NERR_GroupNotFound; use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success; +use windows_sys::Win32::NetworkManagement::NetManagement::NERR_UserNotFound; use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAdd; use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAddMembers; +use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupDel; use windows_sys::Win32::NetworkManagement::NetManagement::NetUserAdd; +use windows_sys::Win32::NetworkManagement::NetManagement::NetUserDel; use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo; use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_INFO_1; use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_MEMBERS_INFO_3; @@ -206,6 +210,39 @@ pub fn ensure_local_group_member(group_name: &str, member_name: &str) -> Result< Ok(()) } +pub fn remove_local_user(name: &str, log: &mut File) -> Result<()> { + let name_w = to_wide(OsStr::new(name)); + unsafe { + let status = NetUserDel(std::ptr::null(), name_w.as_ptr()); + if status != NERR_Success && status != NERR_UserNotFound { + super::log_line(log, &format!("NetUserDel failed for {name} code {status}"))?; + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUserCreateOrUpdateFailed, + format!("failed to delete user {name}, code {status}"), + ))); + } + } + Ok(()) +} + +pub fn remove_local_group(name: &str, log: &mut File) -> Result<()> { + let name_w = to_wide(OsStr::new(name)); + unsafe { + let status = NetLocalGroupDel(std::ptr::null(), name_w.as_ptr()); + if status != NERR_Success && status != NERR_GroupNotFound { + super::log_line( + log, + &format!("NetLocalGroupDel failed for {name} code {status}"), + )?; + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersGroupCreateFailed, + format!("failed to delete local group {name}, code {status}"), + ))); + } + } + Ok(()) +} + pub fn resolve_sid(name: &str) -> Result> { if let Some(sid_str) = well_known_sid_str(name) { return sid_bytes_from_string(sid_str); diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 476620cc2e..fafd88f3c9 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -18,6 +18,7 @@ use codex_windows_sandbox::log_note; use codex_windows_sandbox::path_mask_allows; use codex_windows_sandbox::protect_workspace_agents_dir; use codex_windows_sandbox::protect_workspace_codex_dir; +use codex_windows_sandbox::revoke_ace; use codex_windows_sandbox::sandbox_bin_dir; use codex_windows_sandbox::sandbox_dir; use codex_windows_sandbox::sandbox_secrets_dir; @@ -71,9 +72,25 @@ mod sandbox_users; use read_acl_mutex::acquire_read_acl_mutex; use read_acl_mutex::read_acl_mutex_exists; use sandbox_users::provision_sandbox_users; +use sandbox_users::remove_local_group; +use sandbox_users::remove_local_user; use sandbox_users::resolve_sandbox_users_group_sid; use sandbox_users::resolve_sid; use sandbox_users::sid_bytes_to_psid; +use sandbox_users::SANDBOX_USERS_GROUP; + +const USERPROFILE_CLEANUP_ROOTS: &[&str] = &[ + ".ssh", + ".gnupg", + ".aws", + ".azure", + ".kube", + ".docker", + ".config", + ".npm", + ".pki", + ".terraform.d", +]; #[derive(Debug, Clone, Deserialize, Serialize)] struct Payload { @@ -89,6 +106,8 @@ struct Payload { mode: SetupMode, #[serde(default)] refresh_only: bool, + #[serde(default)] + action: SetupAction, } #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Default)] @@ -99,6 +118,14 @@ enum SetupMode { ReadAclsOnly, } +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Default)] +#[serde(rename_all = "kebab-case")] +enum SetupAction { + #[default] + Setup, + Teardown, +} + fn log_line(log: &mut File, msg: &str) -> Result<()> { let ts = chrono::Utc::now().to_rfc3339(); writeln!(log, "[{ts}] {msg}").map_err(|err| { @@ -437,12 +464,86 @@ fn real_main() -> Result<()> { } fn run_setup(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<()> { + if payload.action == SetupAction::Teardown { + return run_teardown(payload, log); + } match payload.mode { SetupMode::ReadAclsOnly => run_read_acl_only(payload, log), SetupMode::Full => run_setup_full(payload, log, sbx_dir), } } +fn revoke_path_for_sid(path: &Path, sid_str: &str) { + if !path.exists() { + return; + } + if let Some(psid) = unsafe { convert_string_sid_to_sid(sid_str) } { + unsafe { + revoke_ace(path, psid); + LocalFree(psid as HLOCAL); + } + } +} + +fn revoke_path_tree_for_sid(path: &Path, sid_str: &str) { + revoke_path_for_sid(path, sid_str); + if let Ok(entries) = std::fs::read_dir(path) { + for entry in entries.flatten() { + let child = entry.path(); + if child.is_dir() { + revoke_path_tree_for_sid(&child, sid_str); + } else { + revoke_path_for_sid(&child, sid_str); + } + } + } +} + +fn remove_path_if_exists(path: &Path, log: &mut File) -> Result<()> { + if !path.exists() { + return Ok(()); + } + if path.is_dir() { + std::fs::remove_dir_all(path) + .map_err(|err| anyhow::anyhow!("remove {} failed: {err}", path.display()))?; + } else { + std::fs::remove_file(path) + .map_err(|err| anyhow::anyhow!("remove {} failed: {err}", path.display()))?; + } + log_line(log, &format!("removed {}", path.display()))?; + Ok(()) +} + +fn run_teardown(payload: &Payload, log: &mut File) -> Result<()> { + log_line(log, "running windows sandbox teardown")?; + + let sandbox_group_sid = resolve_sandbox_users_group_sid().ok(); + let sandbox_group_sid_str = sandbox_group_sid + .as_deref() + .and_then(|sid| string_from_sid_bytes(sid).ok()); + + if let Some(user_profile) = std::env::var_os("USERPROFILE") { + let user_profile = PathBuf::from(user_profile); + if let Some(sandbox_group_sid_str) = sandbox_group_sid_str.as_deref() { + revoke_path_for_sid(&user_profile, sandbox_group_sid_str); + for relative in USERPROFILE_CLEANUP_ROOTS { + revoke_path_tree_for_sid(&user_profile.join(relative), sandbox_group_sid_str); + } + } + } + + firewall::remove_offline_outbound_block(log)?; + remove_local_user(&payload.offline_username, log)?; + remove_local_user(&payload.online_username, log)?; + remove_local_group(SANDBOX_USERS_GROUP, log)?; + + remove_path_if_exists(&sandbox_secrets_dir(&payload.codex_home), log)?; + remove_path_if_exists(&sandbox_bin_dir(&payload.codex_home), log)?; + remove_path_if_exists(&sandbox_dir(&payload.codex_home), log)?; + + Ok(()) +} + fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> { let _read_acl_guard = match acquire_read_acl_mutex()? { Some(guard) => guard, diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 317a4d467c..7b620abbf8 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -152,6 +152,7 @@ fn run_setup_refresh_inner( write_roots, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: true, + action: SetupAction::Setup, }; let json = serde_json::to_vec(&payload)?; let b64 = BASE64_STANDARD.encode(json); @@ -393,6 +394,16 @@ struct ElevationPayload { real_user: String, #[serde(default)] refresh_only: bool, + #[serde(default)] + action: SetupAction, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "kebab-case")] +enum SetupAction { + #[default] + Setup, + Teardown, } fn quote_arg(arg: &str) -> String { @@ -609,6 +620,36 @@ pub fn run_elevated_setup( write_roots, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: false, + action: SetupAction::Setup, + }; + let needs_elevation = !is_elevated().map_err(|err| { + failure( + SetupErrorCode::OrchestratorElevationCheckFailed, + format!("failed to determine elevation state: {err}"), + ) + })?; + run_setup_exe(&payload, needs_elevation, codex_home) +} + +pub fn run_elevated_teardown(codex_home: &Path) -> Result<()> { + let sbx_dir = sandbox_dir(codex_home); + std::fs::create_dir_all(&sbx_dir).map_err(|err| { + failure( + SetupErrorCode::OrchestratorSandboxDirCreateFailed, + format!("failed to create sandbox dir {}: {err}", sbx_dir.display()), + ) + })?; + let payload = ElevationPayload { + version: SETUP_VERSION, + offline_username: OFFLINE_USERNAME.to_string(), + online_username: ONLINE_USERNAME.to_string(), + codex_home: codex_home.to_path_buf(), + command_cwd: codex_home.to_path_buf(), + read_roots: Vec::new(), + write_roots: Vec::new(), + real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), + refresh_only: false, + action: SetupAction::Teardown, }; let needs_elevation = !is_elevated().map_err(|err| { failure(