From 06f3b4836ac6e4fecc1d75a09b8a9fa369b16635 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 30 Apr 2026 12:06:11 -0700 Subject: [PATCH] [codex] Fix elevated Windows sandbox named-pipe access (#20270) ## Summary - add elevated-only token constructors that include the current token user SID in the restricted SID list - switch the elevated Windows command runner to use those constructors - leave the unelevated restricted-token path unchanged ## Why Windows named pipes created by tools like Ninja use the platform's default named-pipe ACL when no explicit security descriptor is provided. In the elevated sandbox, the pipe owner has access, but the write-restricted token can still fail its restricted-SID access check because the sandbox user SID was not in the restricting SID set. That causes child processes to exit successfully while Ninja never receives the expected pipe completion/close behavior and hangs. Including the elevated sandbox user's SID in the restricting SID list lets the restricted check succeed for these owner-scoped pipe objects without broadening the unelevated sandbox to the real signed-in user. ## Impact - fixes the minimal Ninja hang repro in the elevated Windows sandbox - preserves the existing unelevated sandbox behavior and write protections - keeps the change scoped to the elevated runner rather than changing shared token semantics - this does not affect file-writes for the sandbox because the sandbox users themselves do not receive any additional permissions over what the capability SIDs already have. In fact we don't even explicitly grant the sandbox user ACLs anywhere. ## Validation - `cargo build -p codex-windows-sandbox --quiet` - verified the stock `ninja.exe` minimal repro exits normally on host and in the elevated sandbox - verified the same repro still hangs in the unelevated sandbox, which is the intended scope of this change --- .../src/elevated/command_runner_win.rs | 8 +- codex-rs/windows-sandbox-rs/src/lib.rs | 4 + codex-rs/windows-sandbox-rs/src/token.rs | 90 +++++++++++++++++-- 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs b/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs index a519a9141..b908e7a4e 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs @@ -28,8 +28,8 @@ use codex_windows_sandbox::SpawnRequest; use codex_windows_sandbox::StderrMode; use codex_windows_sandbox::StdinMode; use codex_windows_sandbox::allow_null_device; -use codex_windows_sandbox::create_readonly_token_with_caps_from; -use codex_windows_sandbox::create_workspace_write_token_with_caps_from; +use codex_windows_sandbox::create_readonly_token_with_caps_and_user_from; +use codex_windows_sandbox::create_workspace_write_token_with_caps_and_user_from; use codex_windows_sandbox::decode_bytes; use codex_windows_sandbox::encode_bytes; use codex_windows_sandbox::get_current_token_for_restriction; @@ -242,10 +242,10 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { let h_token = OwnedWinHandle::new(unsafe { match &policy { SandboxPolicy::ReadOnly { .. } => { - create_readonly_token_with_caps_from(base.raw(), &cap_psid_ptrs) + create_readonly_token_with_caps_and_user_from(base.raw(), &cap_psid_ptrs) } SandboxPolicy::WorkspaceWrite { .. } => { - create_workspace_write_token_with_caps_from(base.raw(), &cap_psid_ptrs) + create_workspace_write_token_with_caps_and_user_from(base.raw(), &cap_psid_ptrs) } SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { unreachable!() diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index fc68f1ab4..01ac2e781 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -208,8 +208,12 @@ pub use token::convert_string_sid_to_sid; #[cfg(target_os = "windows")] pub use token::create_readonly_token_with_cap_from; #[cfg(target_os = "windows")] +pub use token::create_readonly_token_with_caps_and_user_from; +#[cfg(target_os = "windows")] pub use token::create_readonly_token_with_caps_from; #[cfg(target_os = "windows")] +pub use token::create_workspace_write_token_with_caps_and_user_from; +#[cfg(target_os = "windows")] pub use token::create_workspace_write_token_with_caps_from; #[cfg(target_os = "windows")] pub use token::get_current_token_for_restriction; diff --git a/codex-rs/windows-sandbox-rs/src/token.rs b/codex-rs/windows-sandbox-rs/src/token.rs index 7d7a6c5c4..71a4b1dd7 100644 --- a/codex-rs/windows-sandbox-rs/src/token.rs +++ b/codex-rs/windows-sandbox-rs/src/token.rs @@ -26,6 +26,7 @@ use windows_sys::Win32::Security::SetTokenInformation; use windows_sys::Win32::Security::TokenDefaultDacl; use windows_sys::Win32::Security::TokenGroups; +use windows_sys::Win32::Security::TokenUser; use windows_sys::Win32::Security::ACL; use windows_sys::Win32::Security::SID_AND_ATTRIBUTES; use windows_sys::Win32::Security::TOKEN_ADJUST_DEFAULT; @@ -35,6 +36,7 @@ use windows_sys::Win32::Security::TOKEN_ASSIGN_PRIMARY; use windows_sys::Win32::Security::TOKEN_DUPLICATE; use windows_sys::Win32::Security::TOKEN_PRIVILEGES; use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::Security::TOKEN_USER; use windows_sys::Win32::System::Threading::GetCurrentProcess; const DISABLE_MAX_PRIVILEGE: u32 = 0x01; @@ -250,6 +252,44 @@ pub unsafe fn get_logon_sid_bytes(h_token: HANDLE) -> Result> { Err(anyhow!("Logon SID not present on token")) } + +unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result> { + let mut needed: u32 = 0; + GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed); + if needed == 0 { + return Err(anyhow!("TokenUser size query returned 0")); + } + let mut user_buf: Vec = vec![0u8; needed as usize]; + let ok = GetTokenInformation( + h_token, + TokenUser, + user_buf.as_mut_ptr() as *mut c_void, + needed, + &mut needed, + ); + if ok == 0 || (needed as usize) < std::mem::size_of::() { + return Err(anyhow!( + "GetTokenInformation(TokenUser) failed: {}", + GetLastError() + )); + } + let token_user: TOKEN_USER = std::ptr::read_unaligned(user_buf.as_ptr() as *const TOKEN_USER); + let sid_len = GetLengthSid(token_user.User.Sid); + if sid_len == 0 { + return Err(anyhow!("GetLengthSid(TokenUser) failed: {}", GetLastError())); + } + let mut user_sid_bytes = vec![0u8; sid_len as usize]; + if CopySid( + sid_len, + user_sid_bytes.as_mut_ptr() as *mut c_void, + token_user.User.Sid, + ) == 0 + { + return Err(anyhow!("CopySid(TokenUser) failed: {}", GetLastError())); + } + Ok(user_sid_bytes) +} + unsafe fn enable_single_privilege(h_token: HANDLE, name: &str) -> Result<()> { let mut luid = LUID { LowPart: 0, @@ -300,7 +340,7 @@ pub unsafe fn create_readonly_token_with_cap_from( base_token: HANDLE, psid_capability: *mut c_void, ) -> Result<(HANDLE, *mut c_void)> { - let new_token = create_token_with_caps_from(base_token, &[psid_capability])?; + let new_token = create_token_with_caps_from(base_token, &[psid_capability], &[])?; Ok((new_token, psid_capability)) } @@ -312,7 +352,23 @@ pub unsafe fn create_workspace_write_token_with_caps_from( base_token: HANDLE, psid_capabilities: &[*mut c_void], ) -> Result { - create_token_with_caps_from(base_token, psid_capabilities) + create_token_with_caps_from(base_token, psid_capabilities, &[]) +} + +/// Create a restricted token that includes all provided capability SIDs plus the token user SID. +/// +/// This is intended for the elevated sandbox backend, where the token user is the dedicated +/// sandbox account rather than the real signed-in user. +/// +/// # Safety +/// Caller must close the returned token handle; base_token must be a valid primary token. +pub unsafe fn create_workspace_write_token_with_caps_and_user_from( + base_token: HANDLE, + psid_capabilities: &[*mut c_void], +) -> Result { + let mut user_sid_bytes = get_user_sid_bytes(base_token)?; + let psid_user = user_sid_bytes.as_mut_ptr() as *mut c_void; + create_token_with_caps_from(base_token, psid_capabilities, &[psid_user]) } /// Create a restricted token that includes all provided capability SIDs. @@ -323,12 +379,29 @@ pub unsafe fn create_readonly_token_with_caps_from( base_token: HANDLE, psid_capabilities: &[*mut c_void], ) -> Result { - create_token_with_caps_from(base_token, psid_capabilities) + create_token_with_caps_from(base_token, psid_capabilities, &[]) +} + +/// Create a restricted token that includes all provided capability SIDs plus the token user SID. +/// +/// This is intended for the elevated sandbox backend, where the token user is the dedicated +/// sandbox account rather than the real signed-in user. +/// +/// # Safety +/// Caller must close the returned token handle; base_token must be a valid primary token. +pub unsafe fn create_readonly_token_with_caps_and_user_from( + base_token: HANDLE, + psid_capabilities: &[*mut c_void], +) -> Result { + let mut user_sid_bytes = get_user_sid_bytes(base_token)?; + let psid_user = user_sid_bytes.as_mut_ptr() as *mut c_void; + create_token_with_caps_from(base_token, psid_capabilities, &[psid_user]) } unsafe fn create_token_with_caps_from( base_token: HANDLE, psid_capabilities: &[*mut c_void], + extra_restricting_sids: &[*mut c_void], ) -> Result { if psid_capabilities.is_empty() { return Err(anyhow!("no capability SIDs provided")); @@ -338,14 +411,19 @@ unsafe fn create_token_with_caps_from( let mut everyone = world_sid()?; let psid_everyone = everyone.as_mut_ptr() as *mut c_void; - // Exact order: Capabilities..., Logon, Everyone + // Exact order: Capabilities..., ExtraRestricting..., Logon, Everyone let mut entries: Vec = - vec![std::mem::zeroed(); psid_capabilities.len() + 2]; + vec![std::mem::zeroed(); psid_capabilities.len() + extra_restricting_sids.len() + 2]; for (i, psid) in psid_capabilities.iter().enumerate() { entries[i].Sid = *psid; entries[i].Attributes = 0; } - let logon_idx = psid_capabilities.len(); + let extras_idx = psid_capabilities.len(); + for (i, psid) in extra_restricting_sids.iter().enumerate() { + entries[extras_idx + i].Sid = *psid; + entries[extras_idx + i].Attributes = 0; + } + let logon_idx = extras_idx + extra_restricting_sids.len(); entries[logon_idx].Sid = psid_logon; entries[logon_idx].Attributes = 0; entries[logon_idx + 1].Sid = psid_everyone;