From 35ebd055eecb5842b0c632aadac598035dc75b62 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Fri, 12 Jun 2026 13:07:07 -0700 Subject: [PATCH] recover stale Windows sandbox credentials --- .../src/elevated/runner_client.rs | 44 +++++++++++- .../windows-sandbox-rs/src/elevated_impl.rs | 34 +++++++-- codex-rs/windows-sandbox-rs/src/identity.rs | 70 ++++++++++++++++++ .../src/unified_exec/backends/elevated.rs | 71 +++++++++++++++---- 4 files changed, 201 insertions(+), 18 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs index 0da7faae4de3..6919face452e 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/runner_client.rs @@ -28,6 +28,7 @@ use std::time::Instant; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS; use windows_sys::Win32::Foundation::DuplicateHandle; +use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; @@ -49,11 +50,29 @@ const RUNNER_SPAWN_READY_POLL_INTERVAL: Duration = Duration::from_millis(50); const RUNNER_ERROR_MODE_FLAGS: u32 = 0x0001 | 0x0002; const WAIT_OBJECT_0: u32 = 0; +#[derive(Debug)] +struct RunnerLogonError { + code: u32, +} + +impl std::fmt::Display for RunnerLogonError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "CreateProcessWithLogonW failed: {}", self.code) + } +} + +impl std::error::Error for RunnerLogonError {} + pub(crate) struct RunnerTransport { pipe_write: File, pipe_read: File, } +pub(crate) fn is_stale_sandbox_creds_error(err: &anyhow::Error) -> bool { + err.downcast_ref::() + .is_some_and(|err| err.code == ERROR_LOGON_FAILURE) +} + impl RunnerTransport { pub(crate) fn send_spawn_request(&mut self, request: SpawnRequest) -> Result<()> { let spawn_request = FramedMessage { @@ -275,12 +294,12 @@ pub(crate) fn spawn_runner_transport( SetErrorMode(previous_error_mode); } if spawn_res == 0 { - let err = unsafe { GetLastError() } as i32; + let err = unsafe { GetLastError() }; unsafe { CloseHandle(h_pipe_in); CloseHandle(h_pipe_out); } - return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {err}")); + return Err(RunnerLogonError { code: err }.into()); } let expected_runner_pid = pi.dwProcessId; @@ -393,3 +412,24 @@ fn wait_for_complete_frame(pipe_read: &File, timeout: Duration) -> Result<()> { std::thread::sleep(RUNNER_SPAWN_READY_POLL_INTERVAL); } } + +#[cfg(test)] +mod tests { + use super::RunnerLogonError; + use super::is_stale_sandbox_creds_error; + use pretty_assertions::assert_eq; + use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE; + use windows_sys::Win32::Foundation::ERROR_NOT_FOUND; + + #[test] + fn stale_sandbox_creds_error_recognizes_logon_failures() { + assert_eq!( + [ERROR_LOGON_FAILURE, ERROR_NOT_FOUND].map(|code| { + let err = + anyhow::Error::new(RunnerLogonError { code }).context("runner launch failed"); + is_stale_sandbox_creds_error(&err) + }), + [true, false] + ); + } +} diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 36e47a539694..5b8bb3769adb 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -30,6 +30,7 @@ mod windows_impl { use crate::env::ensure_non_interactive_pager; use crate::env::inherit_path_env; use crate::env::normalize_null_device_env; + use crate::identity::refresh_logon_sandbox_creds; use crate::identity::require_logon_sandbox_creds; use crate::ipc_framed::EmptyPayload; use crate::ipc_framed::FramedMessage; @@ -43,6 +44,7 @@ mod windows_impl { use crate::logging::log_start; use crate::logging::log_success; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; + use crate::runner_client::is_stale_sandbox_creds_error; use crate::runner_client::spawn_runner_transport; use crate::sandbox_utils::ensure_codex_home_exists; use crate::sandbox_utils::inject_git_safe_directory; @@ -137,7 +139,7 @@ mod windows_impl { let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); log_start(&command, logs_base_dir); - let sandbox_creds = require_logon_sandbox_creds( + let mut sandbox_creds = require_logon_sandbox_creds( &permissions, cwd, &env_map, @@ -192,13 +194,37 @@ mod windows_impl { stdin_open: false, use_private_desktop, }; - let transport = spawn_runner_transport( + let transport = match spawn_runner_transport( codex_home, cwd, &sandbox_creds, logs_base_dir, - spawn_request, - )?; + spawn_request.clone(), + ) { + Ok(transport) => transport, + Err(err) if is_stale_sandbox_creds_error(&err) => { + sandbox_creds = refresh_logon_sandbox_creds( + &permissions, + cwd, + &env_map, + codex_home, + read_roots_override, + read_roots_include_platform_defaults, + write_roots_override, + &deny_read_paths_override, + &deny_write_paths_override, + proxy_enforced, + )?; + spawn_runner_transport( + codex_home, + cwd, + &sandbox_creds, + logs_base_dir, + spawn_request, + )? + } + Err(err) => return Err(err), + }; let (pipe_write, mut pipe_read) = transport.into_files(); let cancel_writer = spawn_cancel_writer(&pipe_write, cancellation)?; diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index 37238190d0cc..fcba538cea01 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -97,6 +97,19 @@ fn load_users(codex_home: &Path) -> Result> { } } +fn remove_sandbox_users_file(codex_home: &Path, reason: &str) -> Result<()> { + let path = sandbox_users_path(codex_home); + debug_log( + &format!("{reason}; deleting {}", path.display()), + Some(codex_home), + ); + match fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(err).with_context(|| format!("delete {}", path.display())), + } +} + fn decode_password(record: &SandboxUserRecord) -> Result { let blob = BASE64_STANDARD .decode(record.password.as_bytes()) @@ -233,3 +246,60 @@ pub fn require_logon_sandbox_creds( password: identity.password, }) } + +#[allow(clippy::too_many_arguments)] +pub(crate) fn refresh_logon_sandbox_creds( + permissions: &ResolvedWindowsSandboxPermissions, + command_cwd: &Path, + env_map: &HashMap, + codex_home: &Path, + read_roots_override: Option<&[PathBuf]>, + read_roots_include_platform_defaults: bool, + write_roots_override: Option<&[PathBuf]>, + deny_read_paths_override: &[PathBuf], + deny_write_paths_override: &[PathBuf], + proxy_enforced: bool, +) -> Result { + remove_sandbox_users_file(codex_home, "sandbox user login failed")?; + require_logon_sandbox_creds( + permissions, + command_cwd, + env_map, + codex_home, + read_roots_override, + read_roots_include_platform_defaults, + write_roots_override, + deny_read_paths_override, + deny_write_paths_override, + proxy_enforced, + ) +} + +#[cfg(test)] +mod tests { + use super::remove_sandbox_users_file; + use crate::setup::sandbox_users_path; + use std::fs; + use tempfile::TempDir; + + #[test] + fn remove_sandbox_users_file_deletes_existing_file() { + let codex_home = TempDir::new().expect("tempdir"); + let users_path = sandbox_users_path(codex_home.path()); + fs::create_dir_all(users_path.parent().expect("sandbox secrets dir")) + .expect("create sandbox secrets dir"); + fs::write(&users_path, "users").expect("write users"); + + remove_sandbox_users_file(codex_home.path(), "stale creds").expect("remove users"); + assert!(!users_path.exists()); + } + + #[test] + fn remove_sandbox_users_file_ignores_missing_file() { + let codex_home = TempDir::new().expect("tempdir"); + let users_path = sandbox_users_path(codex_home.path()); + + remove_sandbox_users_file(codex_home.path(), "stale creds").expect("remove users"); + assert!(!users_path.exists()); + } +} diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs index 0a2af6d7ad48..e1a7a69fbc2d 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs @@ -3,12 +3,16 @@ use super::windows_common::make_runner_resizer; use super::windows_common::start_runner_pipe_writer; use super::windows_common::start_runner_stdin_writer; use super::windows_common::start_runner_stdout_reader; +use crate::identity::SandboxCreds; +use crate::identity::refresh_logon_sandbox_creds; use crate::ipc_framed::EmptyPayload; use crate::ipc_framed::FramedMessage; use crate::ipc_framed::IPC_PROTOCOL_VERSION; use crate::ipc_framed::Message; use crate::ipc_framed::SpawnRequest; use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; +use crate::runner_client::RunnerTransport; +use crate::runner_client::is_stale_sandbox_creds_error; use crate::runner_client::spawn_runner_transport; use crate::spawn_prep::prepare_elevated_spawn_context_for_permissions; use anyhow::Result; @@ -23,6 +27,26 @@ use tokio::sync::broadcast; use tokio::sync::mpsc; use tokio::sync::oneshot; +async fn spawn_runner_transport_task( + codex_home: PathBuf, + cwd: PathBuf, + sandbox_creds: SandboxCreds, + logs_base_dir: Option, + spawn_request: SpawnRequest, +) -> Result { + tokio::task::spawn_blocking(move || -> Result<_> { + spawn_runner_transport( + &codex_home, + &cwd, + &sandbox_creds, + logs_base_dir.as_deref(), + spawn_request, + ) + }) + .await + .map_err(|err| anyhow::anyhow!("runner handshake task failed: {err}"))? +} + #[allow(clippy::too_many_arguments)] pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profile( permission_profile: &PermissionProfile, @@ -55,7 +79,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil workspace_roots, )?; let elevated = prepare_elevated_spawn_context_for_permissions( - permissions, + permissions.clone(), codex_home, cwd, &mut env_map, @@ -83,19 +107,42 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil }; let codex_home = codex_home.to_path_buf(); let cwd = cwd.to_path_buf(); - let sandbox_creds = elevated.sandbox_creds.clone(); + let sandbox_creds = elevated.sandbox_creds; let logs_base_dir = elevated.logs_base_dir.clone(); - let transport = tokio::task::spawn_blocking(move || -> Result<_> { - spawn_runner_transport( - &codex_home, - &cwd, - &sandbox_creds, - logs_base_dir.as_deref(), - spawn_request, - ) - }) + let transport = match spawn_runner_transport_task( + codex_home.clone(), + cwd.clone(), + sandbox_creds, + logs_base_dir.clone(), + spawn_request.clone(), + ) .await - .map_err(|err| anyhow::anyhow!("runner handshake task failed: {err}"))??; + { + Ok(transport) => transport, + Err(err) if is_stale_sandbox_creds_error(&err) => { + let sandbox_creds = refresh_logon_sandbox_creds( + &permissions, + &cwd, + &env_map, + &codex_home, + read_roots_override, + read_roots_include_platform_defaults, + write_roots_override, + &deny_read_paths_override, + &deny_write_paths_override, + /*proxy_enforced*/ false, + )?; + spawn_runner_transport_task( + codex_home, + cwd, + sandbox_creds, + logs_base_dir, + spawn_request, + ) + .await? + } + Err(err) => return Err(err), + }; let (pipe_write, pipe_read) = transport.into_files(); let (writer_tx, writer_rx) = mpsc::channel::>(128);