From 91de5d5863db809a8f9454ee64dcd9af3761d061 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Fri, 1 May 2026 10:18:21 -0700 Subject: [PATCH] fix(windows-sandbox): retry default desktop on spawn failure --- codex-rs/windows-sandbox-rs/src/conpty/mod.rs | 56 +++++--- codex-rs/windows-sandbox-rs/src/process.rs | 129 +++++++++++++----- 2 files changed, 136 insertions(+), 49 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index 54d1f34281..96c504eac8 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -7,7 +7,9 @@ //! Windows sandbox flows that need a PTY. use crate::desktop::LaunchDesktop; +use crate::logging; use crate::proc_thread_attr::ProcThreadAttributeList; +use crate::process::DEFAULT_WINDOWS_DESKTOP; use crate::winutil::format_last_error; use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; @@ -29,6 +31,7 @@ use windows_sys::Win32::System::Threading::STARTF_USESTDHANDLES; use windows_sys::Win32::System::Threading::STARTUPINFOEXW; use crate::process::make_env_block; +use crate::process::should_retry_without_private_desktop; /// Owns a ConPTY handle and its backing pipe handles. pub struct ConptyInstance { @@ -106,6 +109,7 @@ pub fn spawn_conpty_process_as_user( si.StartupInfo.hStdOutput = INVALID_HANDLE_VALUE; si.StartupInfo.hStdError = INVALID_HANDLE_VALUE; let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; + let default_desktop = to_wide(DEFAULT_WINDOWS_DESKTOP); si.StartupInfo.lpDesktop = desktop.startup_info_desktop(); let raw = RawConPty::new(/*cols*/ 80, /*rows*/ 24)?; @@ -121,23 +125,43 @@ pub fn spawn_conpty_process_as_user( si.lpAttributeList = attrs.as_mut_ptr(); let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; - let ok = unsafe { - CreateProcessAsUserW( - h_token, - std::ptr::null(), - cmdline.as_mut_ptr(), - std::ptr::null_mut(), - std::ptr::null_mut(), - 0, - EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, - env_block.as_ptr() as *mut c_void, - to_wide(cwd).as_ptr(), - &si.StartupInfo, - &mut pi, - ) - }; - if ok == 0 { + loop { + let ok = unsafe { + CreateProcessAsUserW( + h_token, + std::ptr::null(), + cmdline.as_mut_ptr(), + std::ptr::null_mut(), + std::ptr::null_mut(), + 0, + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, + env_block.as_ptr() as *mut c_void, + to_wide(cwd).as_ptr(), + &si.StartupInfo, + &mut pi, + ) + }; + if ok != 0 { + break; + } let err = unsafe { GetLastError() } as i32; + if should_retry_without_private_desktop(use_private_desktop, err) + && si.StartupInfo.lpDesktop != default_desktop.as_ptr() as *mut u16 + { + logging::debug_log( + &format!( + "CreateProcessAsUserW failed on private desktop: {} ({}); retrying on {} | cwd={} | cmd={}", + err, + format_last_error(err), + DEFAULT_WINDOWS_DESKTOP, + cwd.display(), + cmdline_str, + ), + logs_base_dir, + ); + si.StartupInfo.lpDesktop = default_desktop.as_ptr() as *mut u16; + continue; + } return Err(anyhow::anyhow!( "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={}", err, diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 1571bb8ccf..ef7a39625e 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -4,26 +4,26 @@ use crate::proc_thread_attr::ProcThreadAttributeList; use crate::winutil::argv_to_command_line; use crate::winutil::format_last_error; use crate::winutil::to_wide; -use anyhow::anyhow; use anyhow::Result; +use anyhow::anyhow; use std::collections::HashMap; use std::ffi::c_void; use std::path::Path; use std::ptr; -use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::CloseHandle; -use windows_sys::Win32::Foundation::SetHandleInformation; +use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT; use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::SetHandleInformation; use windows_sys::Win32::Storage::FileSystem::ReadFile; use windows_sys::Win32::System::Console::GetStdHandle; use windows_sys::Win32::System::Console::STD_ERROR_HANDLE; use windows_sys::Win32::System::Console::STD_INPUT_HANDLE; use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE; use windows_sys::Win32::System::Pipes::CreatePipe; -use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT; +use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; use windows_sys::Win32::System::Threading::STARTF_USESTDHANDLES; @@ -36,6 +36,8 @@ pub struct CreatedProcess { _desktop: LaunchDesktop, } +pub(crate) const DEFAULT_WINDOWS_DESKTOP: &str = "Winsta0\\Default"; + pub fn make_env_block(env: &HashMap) -> Vec { let mut items: Vec<(String, String)> = env.iter().map(|(k, v)| (k.clone(), v.clone())).collect(); @@ -72,6 +74,10 @@ unsafe fn ensure_inheritable_stdio(si: &mut STARTUPINFOW) -> Result<()> { Ok(()) } +pub(crate) fn should_retry_without_private_desktop(use_private_desktop: bool, err: i32) -> bool { + use_private_desktop && matches!(err, 5 | 1920) +} + /// # Safety /// Caller must provide a valid primary token handle (`h_token`) with appropriate access, /// and the `argv`, `cwd`, and `env_map` must remain valid for the duration of the call. @@ -88,6 +94,7 @@ pub unsafe fn create_process_as_user( let mut cmdline: Vec = to_wide(&cmdline_str); let env_block = make_env_block(env_map); let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; + let default_desktop = to_wide(DEFAULT_WINDOWS_DESKTOP); let mut pi: PROCESS_INFORMATION = std::mem::zeroed(); let cwd_wide = to_wide(cwd); let env_block_len = env_block.len(); @@ -118,23 +125,42 @@ pub unsafe fn create_process_as_user( let mut attrs = ProcThreadAttributeList::new(/*attr_count*/ 1)?; attrs.set_handle_list(inherited_handles)?; si.lpAttributeList = attrs.as_mut_ptr(); - let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; - let ok = CreateProcessAsUserW( - h_token, - std::ptr::null(), - cmdline.as_mut_ptr(), - std::ptr::null_mut(), - std::ptr::null_mut(), - 1, - creation_flags, - env_block.as_ptr() as *mut c_void, - cwd_wide.as_ptr(), - &si.StartupInfo, - &mut pi, - ); - if ok == 0 { + loop { + let ok = CreateProcessAsUserW( + h_token, + std::ptr::null(), + cmdline.as_mut_ptr(), + std::ptr::null_mut(), + std::ptr::null_mut(), + 1, + creation_flags, + env_block.as_ptr() as *mut c_void, + cwd_wide.as_ptr(), + &si.StartupInfo, + &mut pi, + ); + if ok != 0 { + break; + } let err = GetLastError() as i32; + if should_retry_without_private_desktop(use_private_desktop, err) + && si.StartupInfo.lpDesktop != default_desktop.as_ptr() as *mut u16 + { + logging::debug_log( + &format!( + "CreateProcessAsUserW failed on private desktop: {} ({}); retrying on {} | cwd={} | cmd={}", + err, + format_last_error(err), + DEFAULT_WINDOWS_DESKTOP, + cwd.display(), + cmdline_str, + ), + logs_base_dir, + ); + si.StartupInfo.lpDesktop = default_desktop.as_ptr() as *mut u16; + continue; + } let msg = format!( "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}", err, @@ -161,21 +187,41 @@ pub unsafe fn create_process_as_user( ensure_inheritable_stdio(&mut si)?; let creation_flags = CREATE_UNICODE_ENVIRONMENT; - let ok = CreateProcessAsUserW( - h_token, - std::ptr::null(), - cmdline.as_mut_ptr(), - std::ptr::null_mut(), - std::ptr::null_mut(), - 1, - creation_flags, - env_block.as_ptr() as *mut c_void, - cwd_wide.as_ptr(), - &si, - &mut pi, - ); - if ok == 0 { + loop { + let ok = CreateProcessAsUserW( + h_token, + std::ptr::null(), + cmdline.as_mut_ptr(), + std::ptr::null_mut(), + std::ptr::null_mut(), + 1, + creation_flags, + env_block.as_ptr() as *mut c_void, + cwd_wide.as_ptr(), + &si, + &mut pi, + ); + if ok != 0 { + break; + } let err = GetLastError() as i32; + if should_retry_without_private_desktop(use_private_desktop, err) + && si.lpDesktop != default_desktop.as_ptr() as *mut u16 + { + logging::debug_log( + &format!( + "CreateProcessAsUserW failed on private desktop: {} ({}); retrying on {} | cwd={} | cmd={}", + err, + format_last_error(err), + DEFAULT_WINDOWS_DESKTOP, + cwd.display(), + cmdline_str, + ), + logs_base_dir, + ); + si.lpDesktop = default_desktop.as_ptr() as *mut u16; + continue; + } let msg = format!( "CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}", err, @@ -325,6 +371,23 @@ pub fn spawn_process_with_pipes( }) } +#[cfg(test)] +mod tests { + use super::should_retry_without_private_desktop; + + #[test] + fn private_desktop_retry_handles_known_spawn_errors() { + assert!(should_retry_without_private_desktop(true, 5)); + assert!(should_retry_without_private_desktop(true, 1920)); + } + + #[test] + fn private_desktop_retry_is_disabled_for_other_cases() { + assert!(!should_retry_without_private_desktop(false, 5)); + assert!(!should_retry_without_private_desktop(true, 2)); + } +} + /// Reads a HANDLE until EOF and invokes `on_chunk` for each read. pub fn read_handle_loop(handle: HANDLE, mut on_chunk: F) -> std::thread::JoinHandle<()> where