diff --git a/codex-rs/core/src/windows_sandbox.rs b/codex-rs/core/src/windows_sandbox.rs index 45212e1d4..22825752b 100644 --- a/codex-rs/core/src/windows_sandbox.rs +++ b/codex-rs/core/src/windows_sandbox.rs @@ -52,6 +52,19 @@ pub fn sandbox_setup_is_complete(_codex_home: &Path) -> bool { false } +#[cfg(target_os = "windows")] +pub fn elevated_setup_failure_details(err: &anyhow::Error) -> Option<(String, String)> { + let failure = codex_windows_sandbox::extract_setup_failure(err)?; + let code = failure.code.as_str().to_string(); + let message = codex_windows_sandbox::sanitize_setup_metric_tag_value(&failure.message); + Some((code, message)) +} + +#[cfg(not(target_os = "windows"))] +pub fn elevated_setup_failure_details(_err: &anyhow::Error) -> Option<(String, String)> { + None +} + #[cfg(target_os = "windows")] pub fn run_elevated_setup( policy: &SandboxPolicy, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 87f232ed2..4e9cfb3fc 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1632,10 +1632,27 @@ impl App { } } Err(err) => { + let mut code_tag: Option = None; + let mut message_tag: Option = None; + if let Some((code, message)) = + codex_core::windows_sandbox::elevated_setup_failure_details( + &err, + ) + { + code_tag = Some(code); + message_tag = Some(message); + } + let mut tags: Vec<(&str, &str)> = Vec::new(); + if let Some(code) = code_tag.as_deref() { + tags.push(("code", code)); + } + if let Some(message) = message_tag.as_deref() { + tags.push(("message", message)); + } otel_manager.counter( "codex.windows_sandbox.elevated_setup_failure", 1, - &[], + &tags, ); tracing::error!( error = %err, diff --git a/codex-rs/windows-sandbox-rs/src/firewall.rs b/codex-rs/windows-sandbox-rs/src/firewall.rs index c75214bb3..b4939fd42 100644 --- a/codex-rs/windows-sandbox-rs/src/firewall.rs +++ b/codex-rs/windows-sandbox-rs/src/firewall.rs @@ -21,6 +21,9 @@ use windows::Win32::System::Com::CoUninitialize; use windows::Win32::System::Com::CLSCTX_INPROC_SERVER; use windows::Win32::System::Com::COINIT_APARTMENTTHREADED; +use codex_windows_sandbox::SetupErrorCode; +use codex_windows_sandbox::SetupFailure; + // This is the stable identifier we use to find/update the rule idempotently. // It intentionally does not change between installs. const OFFLINE_BLOCK_RULE_NAME: &str = "codex_sandbox_offline_block_outbound"; @@ -33,16 +36,27 @@ pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Resul let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; if hr.is_err() { - return Err(anyhow::anyhow!("CoInitializeEx failed: {hr:?}")); + 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(|e| anyhow::anyhow!("CoCreateInstance NetFwPolicy2: {e:?}"))?; - let rules = policy - .Rules() - .map_err(|e| anyhow::anyhow!("INetFwPolicy2::Rules: {e:?}"))?; + .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:?}"), + )) + })?; // Block all outbound IP protocols for this user. ensure_block_rule( @@ -75,14 +89,28 @@ fn ensure_block_rule( ) -> Result<()> { let name = BSTR::from(internal_name); let rule: INetFwRule3 = match unsafe { rules.Item(&name) } { - Ok(existing) => existing - .cast() - .map_err(|e| anyhow::anyhow!("cast existing firewall rule to INetFwRule3: {e:?}"))?, + Ok(existing) => existing.cast().map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("cast existing firewall rule to INetFwRule3 failed: {err:?}"), + )) + })?, Err(_) => { let new_rule: INetFwRule3 = - unsafe { CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER) } - .map_err(|e| anyhow::anyhow!("CoCreateInstance NetFwRule: {e:?}"))?; - unsafe { new_rule.SetName(&name) }.map_err(|e| anyhow::anyhow!("SetName: {e:?}"))?; + unsafe { CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER) }.map_err( + |err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("CoCreateInstance NetFwRule failed: {err:?}"), + )) + }, + )?; + unsafe { new_rule.SetName(&name) }.map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetName failed: {err:?}"), + )) + })?; // Set all properties before adding the rule so we don't leave half-configured rules. configure_rule( &new_rule, @@ -91,7 +119,12 @@ fn ensure_block_rule( local_user_spec, offline_sid, )?; - unsafe { rules.Add(&new_rule) }.map_err(|e| anyhow::anyhow!("Rules::Add: {e:?}"))?; + unsafe { rules.Add(&new_rule) }.map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("Rules::Add failed: {err:?}"), + )) + })?; new_rule } }; @@ -117,29 +150,66 @@ fn configure_rule( ) -> Result<()> { unsafe { rule.SetDescription(&BSTR::from(friendly_desc)) - .map_err(|e| anyhow::anyhow!("SetDescription: {e:?}"))?; - rule.SetDirection(NET_FW_RULE_DIR_OUT) - .map_err(|e| anyhow::anyhow!("SetDirection: {e:?}"))?; - rule.SetAction(NET_FW_ACTION_BLOCK) - .map_err(|e| anyhow::anyhow!("SetAction: {e:?}"))?; - rule.SetEnabled(VARIANT_TRUE) - .map_err(|e| anyhow::anyhow!("SetEnabled: {e:?}"))?; - rule.SetProfiles(NET_FW_PROFILE2_ALL.0) - .map_err(|e| anyhow::anyhow!("SetProfiles: {e:?}"))?; - rule.SetProtocol(protocol) - .map_err(|e| anyhow::anyhow!("SetProtocol: {e:?}"))?; + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetDescription failed: {err:?}"), + )) + })?; + rule.SetDirection(NET_FW_RULE_DIR_OUT).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetDirection failed: {err:?}"), + )) + })?; + rule.SetAction(NET_FW_ACTION_BLOCK).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetAction failed: {err:?}"), + )) + })?; + rule.SetEnabled(VARIANT_TRUE).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetEnabled failed: {err:?}"), + )) + })?; + rule.SetProfiles(NET_FW_PROFILE2_ALL.0).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetProfiles failed: {err:?}"), + )) + })?; + rule.SetProtocol(protocol).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetProtocol failed: {err:?}"), + )) + })?; rule.SetLocalUserAuthorizedList(&BSTR::from(local_user_spec)) - .map_err(|e| anyhow::anyhow!("SetLocalUserAuthorizedList: {e:?}"))?; + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetLocalUserAuthorizedList failed: {err:?}"), + )) + })?; } // Read-back verification: ensure we actually wrote the expected SID scope. - let actual = unsafe { rule.LocalUserAuthorizedList() } - .map_err(|e| anyhow::anyhow!("LocalUserAuthorizedList (read-back): {e:?}"))?; + let actual = unsafe { rule.LocalUserAuthorizedList() }.map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleVerifyFailed, + format!("LocalUserAuthorizedList (read-back) failed: {err:?}"), + )) + })?; let actual_str = actual.to_string(); if !actual_str.contains(offline_sid) { - anyhow::bail!( - "offline firewall rule user scope mismatch: expected SID {offline_sid}, got {actual_str}" - ); + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleVerifyFailed, + format!( + "offline firewall rule user scope mismatch: expected SID {offline_sid}, got {actual_str}" + ), + ))); } Ok(()) } diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 62fda6765..fc062d6fd 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -16,6 +16,9 @@ mod setup; #[cfg(target_os = "windows")] mod elevated_impl; +#[cfg(target_os = "windows")] +mod setup_error; + #[cfg(target_os = "windows")] pub use acl::allow_null_device; #[cfg(target_os = "windows")] @@ -67,6 +70,20 @@ pub use setup::sandbox_secrets_dir; #[cfg(target_os = "windows")] pub use setup::SETUP_VERSION; #[cfg(target_os = "windows")] +pub use setup_error::extract_failure as extract_setup_failure; +#[cfg(target_os = "windows")] +pub use setup_error::sanitize_tag_value as sanitize_setup_metric_tag_value; +#[cfg(target_os = "windows")] +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_error::SetupErrorCode; +#[cfg(target_os = "windows")] +pub use setup_error::SetupErrorReport; +#[cfg(target_os = "windows")] +pub use setup_error::SetupFailure; +#[cfg(target_os = "windows")] pub use token::convert_string_sid_to_sid; #[cfg(target_os = "windows")] pub use token::create_readonly_token_with_cap_from; diff --git a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs index 5847363ef..5ab39c3ce 100644 --- a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs +++ b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs @@ -39,6 +39,8 @@ use codex_windows_sandbox::sandbox_dir; use codex_windows_sandbox::sandbox_secrets_dir; use codex_windows_sandbox::string_from_sid_bytes; use codex_windows_sandbox::to_wide; +use codex_windows_sandbox::SetupErrorCode; +use codex_windows_sandbox::SetupFailure; use codex_windows_sandbox::SETUP_VERSION; pub const SANDBOX_USERS_GROUP: &str = "CodexSandboxUsers"; @@ -122,9 +124,10 @@ pub fn ensure_local_user(name: &str, password: &str, log: &mut File) -> Result<( ); if upd != NERR_Success { super::log_line(log, &format!("NetUserSetInfo failed for {name} code {upd}"))?; - return Err(anyhow::anyhow!( - "failed to create/update user {name}, code {status}/{upd}" - )); + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUserCreateOrUpdateFailed, + format!("failed to create/update user {name}, code {status}/{upd}"), + ))); } } @@ -174,7 +177,10 @@ pub fn ensure_local_group(name: &str, comment: &str, log: &mut File) -> Result<( log, &format!("NetLocalGroupAdd failed for {name} code {status} parm_err={parm_err}"), )?; - anyhow::bail!("failed to create local group {name}, code {status}"); + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersGroupCreateFailed, + format!("failed to create local group {name}, code {status}"), + ))); } } Ok(()) @@ -394,11 +400,37 @@ fn write_secrets( online_pwd: &str, ) -> Result<()> { let sandbox_dir = sandbox_dir(codex_home); - std::fs::create_dir_all(&sandbox_dir)?; + std::fs::create_dir_all(&sandbox_dir).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersFileWriteFailed, + format!( + "failed to create sandbox dir {}: {err}", + sandbox_dir.display() + ), + )) + })?; let secrets_dir = sandbox_secrets_dir(codex_home); - std::fs::create_dir_all(&secrets_dir)?; - let offline_blob = dpapi_protect(offline_pwd.as_bytes())?; - let online_blob = dpapi_protect(online_pwd.as_bytes())?; + std::fs::create_dir_all(&secrets_dir).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersFileWriteFailed, + format!( + "failed to create secrets dir {}: {err}", + secrets_dir.display() + ), + )) + })?; + let offline_blob = dpapi_protect(offline_pwd.as_bytes()).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperDpapiProtectFailed, + format!("dpapi protect failed for offline user: {err}"), + )) + })?; + let online_blob = dpapi_protect(online_pwd.as_bytes()).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperDpapiProtectFailed, + format!("dpapi protect failed for online user: {err}"), + )) + })?; let users = SandboxUsersFile { version: SETUP_VERSION, offline: SandboxUserRecord { @@ -420,7 +452,35 @@ fn write_secrets( }; let users_path = secrets_dir.join("sandbox_users.json"); let marker_path = sandbox_dir.join("setup_marker.json"); - std::fs::write(users_path, serde_json::to_vec_pretty(&users)?)?; - std::fs::write(marker_path, serde_json::to_vec_pretty(&marker)?)?; + let users_json = serde_json::to_vec_pretty(&users).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersFileWriteFailed, + format!("serialize sandbox users failed: {err}"), + )) + })?; + std::fs::write(&users_path, users_json).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUsersFileWriteFailed, + format!( + "write sandbox users file {} failed: {err}", + users_path.display() + ), + )) + })?; + let marker_json = serde_json::to_vec_pretty(&marker).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSetupMarkerWriteFailed, + format!("serialize setup marker failed: {err}"), + )) + })?; + std::fs::write(&marker_path, marker_json).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSetupMarkerWriteFailed, + format!( + "write setup marker file {} failed: {err}", + marker_path.display() + ), + )) + })?; Ok(()) } diff --git a/codex-rs/windows-sandbox-rs/src/setup_error.rs b/codex-rs/windows-sandbox-rs/src/setup_error.rs new file mode 100644 index 000000000..21d260b4d --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/setup_error.rs @@ -0,0 +1,296 @@ +use anyhow::Context; +use anyhow::Result; +use serde::Deserialize; +use serde::Serialize; +use std::fs; +use std::io::ErrorKind; +use std::path::Path; +use std::path::PathBuf; + +/// These represent the most common failures for the elevated sandbox setup. +/// +/// Codes are used as metric tags. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SetupErrorCode { + // Orchestrator (run in CLI) failures. + /// Failed to create `codex_home/.sandbox` in the orchestrator. + OrchestratorSandboxDirCreateFailed, + /// Failed to determine whether the current process is elevated. + OrchestratorElevationCheckFailed, + /// Failed to serialize the elevation payload before launching the helper. + OrchestratorPayloadSerializeFailed, + /// Failed to launch the setup helper process (spawn or ShellExecuteExW). + OrchestratorHelperLaunchFailed, + /// Helper exited non-zero and no structured report was available. + OrchestratorHelperExitNonzero, + /// Helper exited non-zero and reading `setup_error.json` failed. + OrchestratorHelperReportReadFailed, + // Helper (elevated process) failures. + /// Helper failed while validating or decoding the request payload. + HelperRequestArgsFailed, + /// Helper failed to create `codex_home/.sandbox`. + HelperSandboxDirCreateFailed, + /// Helper failed to open or write the setup log. + HelperLogFailed, + /// Helper failed in the provisioning phase (fallback bucket). + HelperUserProvisionFailed, + /// Helper failed to create the sandbox users local group. + HelperUsersGroupCreateFailed, + /// Helper failed to create or update a sandbox user account. + HelperUserCreateOrUpdateFailed, + /// Helper failed to protect user passwords with DPAPI. + HelperDpapiProtectFailed, + /// Helper failed to write the sandbox users secrets file. + HelperUsersFileWriteFailed, + /// Helper failed to write the setup marker file. + HelperSetupMarkerWriteFailed, + /// Helper failed to resolve a SID or convert it to a PSID. + HelperSidResolveFailed, + /// Helper failed to load or convert capability SIDs. + HelperCapabilitySidFailed, + /// Helper failed to initialize COM for firewall configuration. + HelperFirewallComInitFailed, + /// Helper failed to access firewall policy or rule collections. + HelperFirewallPolicyAccessFailed, + /// Helper failed to create, update, or add the firewall rule. + HelperFirewallRuleCreateOrAddFailed, + /// Helper failed to verify the configured firewall rule scope. + HelperFirewallRuleVerifyFailed, + /// Helper failed to spawn the read-ACL helper process. + HelperReadAclHelperSpawnFailed, + /// Helper failed to lock down sandbox directories via ACLs. + HelperSandboxLockFailed, + /// Helper failed for an unmapped or unexpected reason. + HelperUnknownError, +} + +impl SetupErrorCode { + pub fn as_str(self) -> &'static str { + match self { + Self::OrchestratorSandboxDirCreateFailed => "orchestrator_sandbox_dir_create_failed", + Self::OrchestratorElevationCheckFailed => "orchestrator_elevation_check_failed", + Self::OrchestratorPayloadSerializeFailed => "orchestrator_payload_serialize_failed", + Self::OrchestratorHelperLaunchFailed => "orchestrator_helper_launch_failed", + Self::OrchestratorHelperExitNonzero => "orchestrator_helper_exit_nonzero", + Self::OrchestratorHelperReportReadFailed => "orchestrator_helper_report_read_failed", + Self::HelperRequestArgsFailed => "helper_request_args_failed", + Self::HelperSandboxDirCreateFailed => "helper_sandbox_dir_create_failed", + Self::HelperLogFailed => "helper_log_failed", + Self::HelperUserProvisionFailed => "helper_user_provision_failed", + Self::HelperUsersGroupCreateFailed => "helper_users_group_create_failed", + Self::HelperUserCreateOrUpdateFailed => "helper_user_create_or_update_failed", + Self::HelperDpapiProtectFailed => "helper_dpapi_protect_failed", + Self::HelperUsersFileWriteFailed => "helper_users_file_write_failed", + Self::HelperSetupMarkerWriteFailed => "helper_setup_marker_write_failed", + Self::HelperSidResolveFailed => "helper_sid_resolve_failed", + Self::HelperCapabilitySidFailed => "helper_capability_sid_failed", + Self::HelperFirewallComInitFailed => "helper_firewall_com_init_failed", + Self::HelperFirewallPolicyAccessFailed => "helper_firewall_policy_access_failed", + Self::HelperFirewallRuleCreateOrAddFailed => { + "helper_firewall_rule_create_or_add_failed" + } + Self::HelperFirewallRuleVerifyFailed => "helper_firewall_rule_verify_failed", + Self::HelperReadAclHelperSpawnFailed => "helper_read_acl_helper_spawn_failed", + Self::HelperSandboxLockFailed => "helper_sandbox_lock_failed", + Self::HelperUnknownError => "helper_unknown_error", + } + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SetupErrorReport { + pub code: SetupErrorCode, + pub message: String, +} + +#[derive(Debug)] +pub struct SetupFailure { + pub code: SetupErrorCode, + pub message: String, +} + +impl SetupFailure { + pub fn new(code: SetupErrorCode, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + pub fn from_report(report: SetupErrorReport) -> Self { + Self::new(report.code, report.message) + } + + pub fn metric_message(&self) -> String { + sanitize_tag_value(&self.message) + } +} + +impl std::fmt::Display for SetupFailure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}: {}", self.code.as_str(), self.message) + } +} + +impl std::error::Error for SetupFailure {} + +pub fn failure(code: SetupErrorCode, message: impl Into) -> anyhow::Error { + anyhow::Error::new(SetupFailure::new(code, message)) +} + +pub fn extract_failure(err: &anyhow::Error) -> Option<&SetupFailure> { + err.downcast_ref::() +} + +pub fn setup_error_path(codex_home: &Path) -> PathBuf { + codex_home.join(".sandbox").join("setup_error.json") +} + +pub fn clear_setup_error_report(codex_home: &Path) -> Result<()> { + let path = setup_error_path(codex_home); + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(err) if err.kind() == ErrorKind::NotFound => Ok(()), + Err(err) => Err(err).with_context(|| format!("remove {}", path.display())), + } +} + +pub fn write_setup_error_report(codex_home: &Path, report: &SetupErrorReport) -> Result<()> { + let sandbox_dir = codex_home.join(".sandbox"); + fs::create_dir_all(&sandbox_dir) + .with_context(|| format!("create sandbox dir {}", sandbox_dir.display()))?; + let path = setup_error_path(codex_home); + let json = serde_json::to_vec_pretty(report)?; + fs::write(&path, json).with_context(|| format!("write {}", path.display()))?; + Ok(()) +} + +pub fn read_setup_error_report(codex_home: &Path) -> Result> { + let path = setup_error_path(codex_home); + let bytes = match fs::read(&path) { + Ok(bytes) => bytes, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err).with_context(|| format!("read {}", path.display())), + }; + let report = serde_json::from_slice::(&bytes) + .with_context(|| format!("parse {}", path.display()))?; + Ok(Some(report)) +} + +/// Sanitize a tag value to comply with metric tag validation rules: +/// only ASCII alphanumeric, '.', '_', '-', and '/' are allowed. +pub fn sanitize_tag_value(value: &str) -> String { + const MAX_LEN: usize = 256; + let redacted = redact_home_paths(value); + let sanitized: String = redacted + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || matches!(ch, '.' | '_' | '-' | '/') { + ch + } else { + '_' + } + }) + .collect(); + let trimmed = sanitized.trim_matches('_'); + if trimmed.is_empty() { + return "unspecified".to_string(); + } + if trimmed.len() <= MAX_LEN { + trimmed.to_string() + } else { + trimmed[..MAX_LEN].to_string() + } +} + +fn redact_home_paths(value: &str) -> String { + let mut usernames: Vec = Vec::new(); + if let Ok(username) = std::env::var("USERNAME") { + if !username.trim().is_empty() { + usernames.push(username); + } + } + if let Ok(user) = std::env::var("USER") { + if !user.trim().is_empty() && !usernames.iter().any(|v| v.eq_ignore_ascii_case(&user)) { + usernames.push(user); + } + } + + redact_username_segments(value, &usernames) +} + +fn redact_username_segments(value: &str, usernames: &[String]) -> String { + if usernames.is_empty() { + return value.to_string(); + } + + let mut segments: Vec = Vec::new(); + let mut separators: Vec = Vec::new(); + let mut current = String::new(); + + for ch in value.chars() { + if ch == '\\' || ch == '/' { + segments.push(std::mem::take(&mut current)); + separators.push(ch); + } else { + current.push(ch); + } + } + segments.push(current); + + for segment in &mut segments { + let matches = if cfg!(windows) { + usernames + .iter() + .any(|name| segment.eq_ignore_ascii_case(name)) + } else { + usernames.iter().any(|name| segment == name) + }; + if matches { + *segment = "".to_string(); + } + } + + let mut out = String::new(); + for (idx, segment) in segments.iter().enumerate() { + out.push_str(segment); + if let Some(sep) = separators.get(idx) { + out.push(*sep); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn sanitize_tag_value_redacts_username_segments() { + let usernames = vec!["Alice".to_string(), "Bob".to_string()]; + let msg = "failed to write C:\\Users\\Alice\\file.txt; fallback D:\\Profiles\\Bob\\x"; + let redacted = redact_username_segments(msg, &usernames); + assert_eq!( + redacted, + "failed to write C:\\Users\\\\file.txt; fallback D:\\Profiles\\\\x" + ); + } + + #[test] + fn sanitize_tag_value_leaves_unknown_segments() { + let usernames = vec!["Alice".to_string()]; + let msg = "failed to write E:\\data\\file.txt"; + let redacted = redact_username_segments(msg, &usernames); + assert_eq!(redacted, msg); + } + + #[test] + fn sanitize_tag_value_redacts_multiple_occurrences() { + let usernames = vec!["Alice".to_string()]; + let msg = "C:\\Users\\Alice\\a and C:\\Users\\Alice\\b"; + let redacted = redact_username_segments(msg, &usernames); + assert_eq!(redacted, "C:\\Users\\\\a and C:\\Users\\\\b"); + } +} 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 39c8a95d4..686802878 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -9,6 +9,7 @@ use base64::Engine; use codex_windows_sandbox::convert_string_sid_to_sid; use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance; 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::load_or_create_cap_sids; use codex_windows_sandbox::log_note; @@ -17,6 +18,10 @@ use codex_windows_sandbox::sandbox_dir; use codex_windows_sandbox::sandbox_secrets_dir; use codex_windows_sandbox::string_from_sid_bytes; use codex_windows_sandbox::to_wide; +use codex_windows_sandbox::write_setup_error_report; +use codex_windows_sandbox::SetupErrorCode; +use codex_windows_sandbox::SetupErrorReport; +use codex_windows_sandbox::SetupFailure; use codex_windows_sandbox::LOG_FILE_NAME; use codex_windows_sandbox::SETUP_VERSION; use serde::Deserialize; @@ -89,7 +94,12 @@ enum SetupMode { fn log_line(log: &mut File, msg: &str) -> Result<()> { let ts = chrono::Utc::now().to_rfc3339(); - writeln!(log, "[{ts}] {msg}")?; + writeln!(log, "[{ts}] {msg}").map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperLogFailed, + format!("failed to write setup log line: {err}"), + )) + })?; Ok(()) } @@ -349,29 +359,74 @@ pub fn main() -> Result<()> { fn real_main() -> Result<()> { let mut args = std::env::args().collect::>(); if args.len() != 2 { - anyhow::bail!("expected payload argument"); + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperRequestArgsFailed, + "expected payload argument", + ))); } let payload_b64 = args.remove(1); - let payload_json = BASE64 - .decode(payload_b64) - .context("failed to decode payload b64")?; - let payload: Payload = - serde_json::from_slice(&payload_json).context("failed to parse payload json")?; + let payload_json = BASE64.decode(payload_b64).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperRequestArgsFailed, + format!("failed to decode payload b64: {err}"), + )) + })?; + let payload: Payload = serde_json::from_slice(&payload_json).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperRequestArgsFailed, + format!("failed to parse payload json: {err}"), + )) + })?; if payload.version != SETUP_VERSION { - anyhow::bail!("setup version mismatch"); + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperRequestArgsFailed, + format!( + "setup version mismatch: expected {SETUP_VERSION}, got {}", + payload.version + ), + ))); } let sbx_dir = sandbox_dir(&payload.codex_home); - std::fs::create_dir_all(&sbx_dir)?; + std::fs::create_dir_all(&sbx_dir).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSandboxDirCreateFailed, + format!("failed to create sandbox dir {}: {err}", sbx_dir.display()), + )) + })?; let log_path = sbx_dir.join(LOG_FILE_NAME); let mut log = File::options() .create(true) .append(true) .open(&log_path) - .context("open log")?; + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperLogFailed, + format!("open log {} failed: {err}", log_path.display()), + )) + })?; let result = run_setup(&payload, &mut log, &sbx_dir); if let Err(err) = &result { let _ = log_line(&mut log, &format!("setup error: {err:?}")); log_note(&format!("setup error: {err:?}"), Some(sbx_dir.as_path())); + let failure = extract_setup_failure(err) + .map(|f| SetupFailure::new(f.code, f.message.clone())) + .unwrap_or_else(|| { + SetupFailure::new(SetupErrorCode::HelperUnknownError, err.to_string()) + }); + let report = SetupErrorReport { + code: failure.code, + message: failure.message.clone(), + }; + if let Err(write_err) = write_setup_error_report(&payload.codex_home, &report) { + let _ = log_line( + &mut log, + &format!("setup error report write failed: {write_err}"), + ); + log_note( + &format!("setup error report write failed: {write_err}"), + Some(sbx_dir.as_path()), + ); + } } result } @@ -446,32 +501,77 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( let refresh_only = payload.refresh_only; if refresh_only { } else { - provision_sandbox_users( + let provision_result = provision_sandbox_users( &payload.codex_home, &payload.offline_username, &payload.online_username, log, - )?; + ); + if let Err(err) = provision_result { + if extract_setup_failure(&err).is_some() { + return Err(err); + } + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperUserProvisionFailed, + format!("provision sandbox users failed: {err}"), + ))); + } let users = vec![ payload.offline_username.clone(), payload.online_username.clone(), ]; hide_newly_created_users(&users, sbx_dir); } - let offline_sid = resolve_sid(&payload.offline_username)?; + 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()?; - let sandbox_group_psid = sid_bytes_to_psid(&sandbox_group_sid)?; + let sandbox_group_sid = resolve_sandbox_users_group_sid().map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSidResolveFailed, + format!("resolve sandbox users group SID failed: {err}"), + )) + })?; + let sandbox_group_psid = sid_bytes_to_psid(&sandbox_group_sid).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSidResolveFailed, + format!("convert sandbox users group SID to PSID failed: {err}"), + )) + })?; - let caps = load_or_create_cap_sids(&payload.codex_home)?; + let caps = load_or_create_cap_sids(&payload.codex_home).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperCapabilitySidFailed, + format!("load or create capability SIDs failed: {err}"), + )) + })?; let cap_psid = unsafe { - convert_string_sid_to_sid(&caps.workspace) - .ok_or_else(|| anyhow::anyhow!("convert capability SID failed"))? + convert_string_sid_to_sid(&caps.workspace).ok_or_else(|| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperCapabilitySidFailed, + format!("convert capability SID {} failed", caps.workspace), + )) + })? }; let mut refresh_errors: Vec = Vec::new(); if !refresh_only { - firewall::ensure_offline_outbound_block(&offline_sid_str, log)?; + let firewall_result = firewall::ensure_offline_outbound_block(&offline_sid_str, log); + if let Err(err) = firewall_result { + if extract_setup_failure(&err).is_some() { + return Err(err); + } + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("ensure offline outbound block failed: {err}"), + ))); + } } if payload.read_roots.is_empty() { @@ -482,14 +582,26 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( log_line(log, "read ACL helper already running; skipping spawn")?; } Ok(false) => { - spawn_read_acl_helper(payload, log)?; + spawn_read_acl_helper(payload, log).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperReadAclHelperSpawnFailed, + format!("spawn read ACL helper failed: {err}"), + )) + })?; } Err(err) => { log_line( log, &format!("read ACL mutex check failed: {err}; spawning anyway"), )?; - spawn_read_acl_helper(payload, log)?; + spawn_read_acl_helper(payload, log).map_err(|spawn_err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperReadAclHelperSpawnFailed, + format!( + "spawn read ACL helper failed after mutex error {err}: {spawn_err}" + ), + )) + })?; } } } @@ -615,14 +727,32 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( &sandbox_group_sid, GRANT_ACCESS, log, - )?; + ) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSandboxLockFailed, + format!( + "lock sandbox dir {} failed: {err}", + sandbox_dir(&payload.codex_home).display() + ), + )) + })?; lock_sandbox_dir( &sandbox_secrets_dir(&payload.codex_home), &payload.real_user, &sandbox_group_sid, DENY_ACCESS, log, - )?; + ) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperSandboxLockFailed, + format!( + "lock sandbox secrets dir {} failed: {err}", + sandbox_secrets_dir(&payload.codex_home).display() + ), + )) + })?; let legacy_users = sandbox_dir(&payload.codex_home).join("sandbox_users.json"); if legacy_users.exists() { let _ = std::fs::remove_file(&legacy_users); diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 19e20fc9d..74137379e 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -13,6 +13,11 @@ use crate::allow::compute_allow_paths; use crate::allow::AllowDenyPaths; use crate::logging::log_note; use crate::policy::SandboxPolicy; +use crate::setup_error::clear_setup_error_report; +use crate::setup_error::failure; +use crate::setup_error::read_setup_error_report; +use crate::setup_error::SetupErrorCode; +use crate::setup_error::SetupFailure; use anyhow::anyhow; use anyhow::Context; use anyhow::Result; @@ -308,7 +313,30 @@ fn find_setup_exe() -> PathBuf { PathBuf::from("codex-windows-sandbox-setup.exe") } -fn run_setup_exe(payload: &ElevationPayload, needs_elevation: bool) -> Result<()> { +fn report_helper_failure( + codex_home: &Path, + cleared_report: bool, + exit_code: Option, +) -> anyhow::Error { + let exit_detail = format!("setup helper exited with status {exit_code:?}"); + if !cleared_report { + return failure(SetupErrorCode::OrchestratorHelperExitNonzero, exit_detail); + } + match read_setup_error_report(codex_home) { + Ok(Some(report)) => anyhow::Error::new(SetupFailure::from_report(report)), + Ok(None) => failure(SetupErrorCode::OrchestratorHelperExitNonzero, exit_detail), + Err(err) => failure( + SetupErrorCode::OrchestratorHelperReportReadFailed, + format!("{exit_detail}; failed to read setup_error.json: {err}"), + ), + } +} + +fn run_setup_exe( + payload: &ElevationPayload, + needs_elevation: bool, + codex_home: &Path, +) -> Result<()> { use windows_sys::Win32::System::Threading::GetExitCodeProcess; use windows_sys::Win32::System::Threading::WaitForSingleObject; use windows_sys::Win32::System::Threading::INFINITE; @@ -316,8 +344,25 @@ fn run_setup_exe(payload: &ElevationPayload, needs_elevation: bool) -> Result<() use windows_sys::Win32::UI::Shell::SEE_MASK_NOCLOSEPROCESS; use windows_sys::Win32::UI::Shell::SHELLEXECUTEINFOW; let exe = find_setup_exe(); - let payload_json = serde_json::to_string(payload)?; + let payload_json = serde_json::to_string(payload).map_err(|err| { + failure( + SetupErrorCode::OrchestratorPayloadSerializeFailed, + format!("failed to serialize elevation payload: {err}"), + ) + })?; let payload_b64 = BASE64_STANDARD.encode(payload_json.as_bytes()); + let cleared_report = match clear_setup_error_report(codex_home) { + Ok(()) => true, + Err(err) => { + log_note( + &format!( + "setup orchestrator: failed to clear setup_error.json before launch: {err}" + ), + Some(&sandbox_dir(codex_home)), + ); + false + } + }; if !needs_elevation { let status = Command::new(&exe) @@ -327,13 +372,27 @@ fn run_setup_exe(payload: &ElevationPayload, needs_elevation: bool) -> Result<() .stdout(Stdio::null()) .stderr(Stdio::null()) .status() - .context("failed to launch setup helper")?; + .map_err(|err| { + failure( + SetupErrorCode::OrchestratorHelperLaunchFailed, + format!("failed to launch setup helper (non-elevated): {err}"), + ) + })?; if !status.success() { - return Err(anyhow!( - "setup helper exited with status {:?}", - status.code() + return Err(report_helper_failure( + codex_home, + cleared_report, + status.code(), )); } + if let Err(err) = clear_setup_error_report(codex_home) { + log_note( + &format!( + "setup orchestrator: failed to clear setup_error.json after success: {err}" + ), + Some(&sandbox_dir(codex_home)), + ); + } return Ok(()); } @@ -351,9 +410,10 @@ fn run_setup_exe(payload: &ElevationPayload, needs_elevation: bool) -> Result<() sei.nShow = 0; // SW_HIDE let ok = unsafe { ShellExecuteExW(&mut sei) }; if ok == 0 || sei.hProcess == 0 { - return Err(anyhow!( - "ShellExecuteExW failed to launch setup helper: {}", - unsafe { GetLastError() } + let last_error = unsafe { GetLastError() }; + return Err(failure( + SetupErrorCode::OrchestratorHelperLaunchFailed, + format!("ShellExecuteExW failed to launch setup helper: {last_error}"), )); } unsafe { @@ -362,9 +422,19 @@ fn run_setup_exe(payload: &ElevationPayload, needs_elevation: bool) -> Result<() GetExitCodeProcess(sei.hProcess, &mut code); CloseHandle(sei.hProcess); if code != 0 { - return Err(anyhow!("setup helper exited with status {}", code)); + return Err(report_helper_failure( + codex_home, + cleared_report, + Some(code as i32), + )); } } + if let Err(err) = clear_setup_error_report(codex_home) { + log_note( + &format!("setup orchestrator: failed to clear setup_error.json after success: {err}"), + Some(&sandbox_dir(codex_home)), + ); + } Ok(()) } @@ -379,7 +449,12 @@ pub fn run_elevated_setup( ) -> Result<()> { // Ensure the shared sandbox directory exists before we send it to the elevated helper. let sbx_dir = sandbox_dir(codex_home); - std::fs::create_dir_all(&sbx_dir)?; + std::fs::create_dir_all(&sbx_dir).map_err(|err| { + failure( + SetupErrorCode::OrchestratorSandboxDirCreateFailed, + format!("failed to create sandbox dir {}: {err}", sbx_dir.display()), + ) + })?; let (read_roots, write_roots) = build_payload_roots( policy, policy_cwd, @@ -399,8 +474,13 @@ pub fn run_elevated_setup( real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: false, }; - let needs_elevation = !is_elevated()?; - run_setup_exe(&payload, needs_elevation) + 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) } fn build_payload_roots(