[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
This commit is contained in:
iceweasel-oai
2026-04-30 12:06:11 -07:00
committed by GitHub
Unverified
parent 31f8813e3e
commit 06f3b4836a
3 changed files with 92 additions and 10 deletions
@@ -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<IpcSpawnedProcess> {
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!()
+4
View File
@@ -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;
+84 -6
View File
@@ -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<Vec<u8>> {
Err(anyhow!("Logon SID not present on token"))
}
unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
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<u8> = 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::<TOKEN_USER>() {
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<HANDLE> {
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<HANDLE> {
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<HANDLE> {
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<HANDLE> {
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<HANDLE> {
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<SID_AND_ATTRIBUTES> =
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;