Skip to content

Commit 06f3b48

Browse files
[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
1 parent 31f8813 commit 06f3b48

3 files changed

Lines changed: 92 additions & 10 deletions

File tree

codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use codex_windows_sandbox::SpawnRequest;
2828
use codex_windows_sandbox::StderrMode;
2929
use codex_windows_sandbox::StdinMode;
3030
use codex_windows_sandbox::allow_null_device;
31-
use codex_windows_sandbox::create_readonly_token_with_caps_from;
32-
use codex_windows_sandbox::create_workspace_write_token_with_caps_from;
31+
use codex_windows_sandbox::create_readonly_token_with_caps_and_user_from;
32+
use codex_windows_sandbox::create_workspace_write_token_with_caps_and_user_from;
3333
use codex_windows_sandbox::decode_bytes;
3434
use codex_windows_sandbox::encode_bytes;
3535
use codex_windows_sandbox::get_current_token_for_restriction;
@@ -242,10 +242,10 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result<IpcSpawnedProcess> {
242242
let h_token = OwnedWinHandle::new(unsafe {
243243
match &policy {
244244
SandboxPolicy::ReadOnly { .. } => {
245-
create_readonly_token_with_caps_from(base.raw(), &cap_psid_ptrs)
245+
create_readonly_token_with_caps_and_user_from(base.raw(), &cap_psid_ptrs)
246246
}
247247
SandboxPolicy::WorkspaceWrite { .. } => {
248-
create_workspace_write_token_with_caps_from(base.raw(), &cap_psid_ptrs)
248+
create_workspace_write_token_with_caps_and_user_from(base.raw(), &cap_psid_ptrs)
249249
}
250250
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
251251
unreachable!()

codex-rs/windows-sandbox-rs/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,12 @@ pub use token::convert_string_sid_to_sid;
208208
#[cfg(target_os = "windows")]
209209
pub use token::create_readonly_token_with_cap_from;
210210
#[cfg(target_os = "windows")]
211+
pub use token::create_readonly_token_with_caps_and_user_from;
212+
#[cfg(target_os = "windows")]
211213
pub use token::create_readonly_token_with_caps_from;
212214
#[cfg(target_os = "windows")]
215+
pub use token::create_workspace_write_token_with_caps_and_user_from;
216+
#[cfg(target_os = "windows")]
213217
pub use token::create_workspace_write_token_with_caps_from;
214218
#[cfg(target_os = "windows")]
215219
pub use token::get_current_token_for_restriction;

codex-rs/windows-sandbox-rs/src/token.rs

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use windows_sys::Win32::Security::SetTokenInformation;
2626

2727
use windows_sys::Win32::Security::TokenDefaultDacl;
2828
use windows_sys::Win32::Security::TokenGroups;
29+
use windows_sys::Win32::Security::TokenUser;
2930
use windows_sys::Win32::Security::ACL;
3031
use windows_sys::Win32::Security::SID_AND_ATTRIBUTES;
3132
use windows_sys::Win32::Security::TOKEN_ADJUST_DEFAULT;
@@ -35,6 +36,7 @@ use windows_sys::Win32::Security::TOKEN_ASSIGN_PRIMARY;
3536
use windows_sys::Win32::Security::TOKEN_DUPLICATE;
3637
use windows_sys::Win32::Security::TOKEN_PRIVILEGES;
3738
use windows_sys::Win32::Security::TOKEN_QUERY;
39+
use windows_sys::Win32::Security::TOKEN_USER;
3840
use windows_sys::Win32::System::Threading::GetCurrentProcess;
3941

4042
const DISABLE_MAX_PRIVILEGE: u32 = 0x01;
@@ -250,6 +252,44 @@ pub unsafe fn get_logon_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
250252

251253
Err(anyhow!("Logon SID not present on token"))
252254
}
255+
256+
unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
257+
let mut needed: u32 = 0;
258+
GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed);
259+
if needed == 0 {
260+
return Err(anyhow!("TokenUser size query returned 0"));
261+
}
262+
let mut user_buf: Vec<u8> = vec![0u8; needed as usize];
263+
let ok = GetTokenInformation(
264+
h_token,
265+
TokenUser,
266+
user_buf.as_mut_ptr() as *mut c_void,
267+
needed,
268+
&mut needed,
269+
);
270+
if ok == 0 || (needed as usize) < std::mem::size_of::<TOKEN_USER>() {
271+
return Err(anyhow!(
272+
"GetTokenInformation(TokenUser) failed: {}",
273+
GetLastError()
274+
));
275+
}
276+
let token_user: TOKEN_USER = std::ptr::read_unaligned(user_buf.as_ptr() as *const TOKEN_USER);
277+
let sid_len = GetLengthSid(token_user.User.Sid);
278+
if sid_len == 0 {
279+
return Err(anyhow!("GetLengthSid(TokenUser) failed: {}", GetLastError()));
280+
}
281+
let mut user_sid_bytes = vec![0u8; sid_len as usize];
282+
if CopySid(
283+
sid_len,
284+
user_sid_bytes.as_mut_ptr() as *mut c_void,
285+
token_user.User.Sid,
286+
) == 0
287+
{
288+
return Err(anyhow!("CopySid(TokenUser) failed: {}", GetLastError()));
289+
}
290+
Ok(user_sid_bytes)
291+
}
292+
253293
unsafe fn enable_single_privilege(h_token: HANDLE, name: &str) -> Result<()> {
254294
let mut luid = LUID {
255295
LowPart: 0,
@@ -300,7 +340,7 @@ pub unsafe fn create_readonly_token_with_cap_from(
300340
base_token: HANDLE,
301341
psid_capability: *mut c_void,
302342
) -> Result<(HANDLE, *mut c_void)> {
303-
let new_token = create_token_with_caps_from(base_token, &[psid_capability])?;
343+
let new_token = create_token_with_caps_from(base_token, &[psid_capability], &[])?;
304344
Ok((new_token, psid_capability))
305345
}
306346

@@ -312,7 +352,23 @@ pub unsafe fn create_workspace_write_token_with_caps_from(
312352
base_token: HANDLE,
313353
psid_capabilities: &[*mut c_void],
314354
) -> Result<HANDLE> {
315-
create_token_with_caps_from(base_token, psid_capabilities)
355+
create_token_with_caps_from(base_token, psid_capabilities, &[])
356+
}
357+
358+
/// Create a restricted token that includes all provided capability SIDs plus the token user SID.
359+
///
360+
/// This is intended for the elevated sandbox backend, where the token user is the dedicated
361+
/// sandbox account rather than the real signed-in user.
362+
///
363+
/// # Safety
364+
/// Caller must close the returned token handle; base_token must be a valid primary token.
365+
pub unsafe fn create_workspace_write_token_with_caps_and_user_from(
366+
base_token: HANDLE,
367+
psid_capabilities: &[*mut c_void],
368+
) -> Result<HANDLE> {
369+
let mut user_sid_bytes = get_user_sid_bytes(base_token)?;
370+
let psid_user = user_sid_bytes.as_mut_ptr() as *mut c_void;
371+
create_token_with_caps_from(base_token, psid_capabilities, &[psid_user])
316372
}
317373

318374
/// Create a restricted token that includes all provided capability SIDs.
@@ -323,12 +379,29 @@ pub unsafe fn create_readonly_token_with_caps_from(
323379
base_token: HANDLE,
324380
psid_capabilities: &[*mut c_void],
325381
) -> Result<HANDLE> {
326-
create_token_with_caps_from(base_token, psid_capabilities)
382+
create_token_with_caps_from(base_token, psid_capabilities, &[])
383+
}
384+
385+
/// Create a restricted token that includes all provided capability SIDs plus the token user SID.
386+
///
387+
/// This is intended for the elevated sandbox backend, where the token user is the dedicated
388+
/// sandbox account rather than the real signed-in user.
389+
///
390+
/// # Safety
391+
/// Caller must close the returned token handle; base_token must be a valid primary token.
392+
pub unsafe fn create_readonly_token_with_caps_and_user_from(
393+
base_token: HANDLE,
394+
psid_capabilities: &[*mut c_void],
395+
) -> Result<HANDLE> {
396+
let mut user_sid_bytes = get_user_sid_bytes(base_token)?;
397+
let psid_user = user_sid_bytes.as_mut_ptr() as *mut c_void;
398+
create_token_with_caps_from(base_token, psid_capabilities, &[psid_user])
327399
}
328400

329401
unsafe fn create_token_with_caps_from(
330402
base_token: HANDLE,
331403
psid_capabilities: &[*mut c_void],
404+
extra_restricting_sids: &[*mut c_void],
332405
) -> Result<HANDLE> {
333406
if psid_capabilities.is_empty() {
334407
return Err(anyhow!("no capability SIDs provided"));
@@ -338,14 +411,19 @@ unsafe fn create_token_with_caps_from(
338411
let mut everyone = world_sid()?;
339412
let psid_everyone = everyone.as_mut_ptr() as *mut c_void;
340413

341-
// Exact order: Capabilities..., Logon, Everyone
414+
// Exact order: Capabilities..., ExtraRestricting..., Logon, Everyone
342415
let mut entries: Vec<SID_AND_ATTRIBUTES> =
343-
vec![std::mem::zeroed(); psid_capabilities.len() + 2];
416+
vec![std::mem::zeroed(); psid_capabilities.len() + extra_restricting_sids.len() + 2];
344417
for (i, psid) in psid_capabilities.iter().enumerate() {
345418
entries[i].Sid = *psid;
346419
entries[i].Attributes = 0;
347420
}
348-
let logon_idx = psid_capabilities.len();
421+
let extras_idx = psid_capabilities.len();
422+
for (i, psid) in extra_restricting_sids.iter().enumerate() {
423+
entries[extras_idx + i].Sid = *psid;
424+
entries[extras_idx + i].Attributes = 0;
425+
}
426+
let logon_idx = extras_idx + extra_restricting_sids.len();
349427
entries[logon_idx].Sid = psid_logon;
350428
entries[logon_idx].Attributes = 0;
351429
entries[logon_idx + 1].Sid = psid_everyone;

0 commit comments

Comments
 (0)