From 7404a3c8ecfa3e2fb569e3bccc1d5bc95f6d11ce Mon Sep 17 00:00:00 2001 From: Eva Wong Date: Mon, 4 May 2026 10:08:57 -0700 Subject: [PATCH] Allow Windows sandbox Git signal pipes --- codex-rs/windows-sandbox-rs/src/acl.rs | 94 +++++++++++++++++-- .../src/elevated/command_runner_win.rs | 3 + .../windows-sandbox-rs/src/elevated_impl.rs | 2 + codex-rs/windows-sandbox-rs/src/lib.rs | 8 ++ codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 5 + 5 files changed, 102 insertions(+), 10 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index f351dba190..49c5b42dd0 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/codex-rs/windows-sandbox-rs/src/acl.rs @@ -46,6 +46,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; use windows_sys::Win32::Storage::FileSystem::DELETE; +const SE_FILE_OBJECT: u32 = 1; const SE_KERNEL_OBJECT: u32 = 6; const INHERIT_ONLY_ACE: u8 = 0x08; const GENERIC_WRITE_MASK: u32 = 0x4000_0000; @@ -568,19 +569,20 @@ pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) { } } -/// Grants RX to the null device for the given SID to support stdout/stderr redirection. -/// -/// # Safety -/// Caller must ensure `psid` is a valid SID pointer. -pub unsafe fn allow_null_device(psid: *mut c_void) { +unsafe fn allow_opened_object_path( + psid: *mut c_void, + path: &str, + object_type: u32, + flags_and_attributes: u32, +) { let desired = 0x00020000 | 0x00040000; // READ_CONTROL | WRITE_DAC let h = CreateFileW( - to_wide(r"\\\\.\\NUL").as_ptr(), + to_wide(path).as_ptr(), desired, - FILE_SHARE_READ | FILE_SHARE_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, std::ptr::null_mut(), OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, + flags_and_attributes, 0, ); if h == 0 || h == INVALID_HANDLE_VALUE { @@ -590,7 +592,7 @@ pub unsafe fn allow_null_device(psid: *mut c_void) { let mut p_dacl: *mut ACL = std::ptr::null_mut(); let code = GetSecurityInfo( h, - SE_KERNEL_OBJECT as i32, + object_type as i32, DACL_SECURITY_INFORMATION, std::ptr::null_mut(), std::ptr::null_mut(), @@ -617,7 +619,7 @@ pub unsafe fn allow_null_device(psid: *mut c_void) { if code2 == ERROR_SUCCESS { let _ = SetSecurityInfo( h, - SE_KERNEL_OBJECT as i32, + object_type as i32, DACL_SECURITY_INFORMATION, std::ptr::null_mut(), std::ptr::null_mut(), @@ -634,5 +636,77 @@ pub unsafe fn allow_null_device(psid: *mut c_void) { } CloseHandle(h); } + +unsafe fn allow_named_file_object_path(psid: *mut c_void, path: &str, allow_mask: u32) { + let mut p_sd: *mut c_void = std::ptr::null_mut(); + let mut p_dacl: *mut ACL = std::ptr::null_mut(); + let code = GetNamedSecurityInfoW( + to_wide(path).as_ptr(), + SE_FILE_OBJECT as i32, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut p_dacl, + std::ptr::null_mut(), + &mut p_sd, + ); + if code != ERROR_SUCCESS { + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + } + return; + } + let trustee = TRUSTEE_W { + pMultipleTrustee: std::ptr::null_mut(), + MultipleTrusteeOperation: 0, + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_UNKNOWN, + ptstrName: psid as *mut u16, + }; + let mut explicit: EXPLICIT_ACCESS_W = std::mem::zeroed(); + explicit.grfAccessPermissions = allow_mask; + explicit.grfAccessMode = 2; // SET_ACCESS + explicit.grfInheritance = 0; + explicit.Trustee = trustee; + let mut p_new_dacl: *mut ACL = std::ptr::null_mut(); + let code2 = SetEntriesInAclW(1, &explicit, p_dacl, &mut p_new_dacl); + if code2 == ERROR_SUCCESS { + let _ = SetNamedSecurityInfoW( + to_wide(path).as_ptr() as *mut u16, + SE_FILE_OBJECT as i32, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + p_new_dacl, + std::ptr::null_mut(), + ); + if !p_new_dacl.is_null() { + LocalFree(p_new_dacl as HLOCAL); + } + } + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + } +} + +/// Grants access to the null device for the given SID to support stdout/stderr redirection. +/// +/// # Safety +/// Caller must ensure `psid` is a valid SID pointer. +pub unsafe fn allow_null_device(psid: *mut c_void) { + allow_opened_object_path(psid, "\\\\.\\NUL", SE_KERNEL_OBJECT, FILE_ATTRIBUTE_NORMAL); +} + +/// Grants access to the named pipe namespace for the given SID. +/// +/// MSYS and Git for Windows create signal pipes during process startup. Restricted tokens need an +/// explicit allow on the pipe namespace, otherwise those child processes fail during initialization +/// with `ERROR_ACCESS_DENIED`. +/// +/// # Safety +/// Caller must ensure `psid` is a valid SID pointer. +pub unsafe fn allow_named_pipe_device(psid: *mut c_void) { + allow_named_file_object_path(psid, "\\\\.\\pipe\\", FILE_DELETE_CHILD); +} const CONTAINER_INHERIT_ACE: u32 = 0x2; const OBJECT_INHERIT_ACE: u32 = 0x1; 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 80e67044e8..88fb47947b 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 @@ -26,6 +26,7 @@ use codex_windows_sandbox::SpawnReady; use codex_windows_sandbox::SpawnRequest; use codex_windows_sandbox::StderrMode; use codex_windows_sandbox::StdinMode; +use codex_windows_sandbox::allow_named_pipe_device; use codex_windows_sandbox::allow_null_device; 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; @@ -254,8 +255,10 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { // These ACL adjustments need the raw SID values, but ownership stays with `cap_psids`. // We do not manually `LocalFree` anything here; the wrappers handle every return path. allow_null_device(cap_psid_ptrs[0]); + allow_named_pipe_device(cap_psid_ptrs[0]); for psid in &cap_psid_ptrs { allow_null_device(*psid); + allow_named_pipe_device(*psid); } } diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 1dcadbd706..6a7924f7a3 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -23,6 +23,7 @@ pub struct ElevatedSandboxCaptureRequest<'a> { mod windows_impl { use super::ElevatedSandboxCaptureRequest; + use crate::acl::allow_named_pipe_device; use crate::acl::allow_null_device; use crate::cap::load_or_create_cap_sids; use crate::env::ensure_non_interactive_pager; @@ -189,6 +190,7 @@ mod windows_impl { unsafe { allow_null_device(psid_to_use); + allow_named_pipe_device(psid_to_use); } (|| -> Result { diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 8d780f652e..744aeac643 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -79,6 +79,8 @@ mod session; #[cfg(target_os = "windows")] pub use acl::add_deny_write_ace; +#[cfg(target_os = "windows")] +pub use acl::allow_named_pipe_device; #[cfg(target_os = "windows")] pub use acl::allow_null_device; #[cfg(target_os = "windows")] @@ -263,6 +265,7 @@ mod windows_impl { use super::ProtectedMetadataTarget; use super::acl::add_allow_ace; use super::acl::add_deny_write_ace; + use super::acl::allow_named_pipe_device; use super::acl::allow_null_device; use super::acl::revoke_ace; use super::allow::AllowDenyPaths; @@ -431,6 +434,7 @@ mod windows_impl { let mut tmp = bytes; let psid2 = tmp.as_mut_ptr() as *mut c_void; allow_null_device(psid2); + allow_named_pipe_device(psid2); } windows_sys::Win32::Foundation::CloseHandle(base); } @@ -479,8 +483,10 @@ mod windows_impl { } } allow_null_device(psid_generic); + allow_named_pipe_device(psid_generic); if let Some(psid) = psid_workspace { allow_null_device(psid); + allow_named_pipe_device(psid); } } let (stdin_pair, stdout_pair, stderr_pair) = unsafe { setup_stdio_pipes()? }; @@ -664,7 +670,9 @@ mod windows_impl { let _ = add_deny_write_ace(p, psid_generic); } allow_null_device(psid_generic); + allow_named_pipe_device(psid_generic); allow_null_device(psid_workspace); + allow_named_pipe_device(psid_workspace); } Ok(()) diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index 6b46564d4e..00f4a607c1 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -1,5 +1,6 @@ use crate::acl::add_allow_ace; use crate::acl::add_deny_write_ace; +use crate::acl::allow_named_pipe_device; use crate::acl::allow_null_device; use crate::allow::AllowDenyPaths; use crate::allow::compute_allow_paths; @@ -206,6 +207,7 @@ pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) { let mut tmp = bytes; let psid = tmp.as_mut_ptr() as *mut c_void; allow_null_device(psid); + allow_named_pipe_device(psid); } CloseHandle(base); } @@ -249,8 +251,10 @@ pub(crate) fn apply_legacy_session_acl_rules( } } allow_null_device(psid_generic.as_ptr()); + allow_named_pipe_device(psid_generic.as_ptr()); if let Some(psid_workspace) = psid_workspace { allow_null_device(psid_workspace.as_ptr()); + allow_named_pipe_device(psid_workspace.as_ptr()); if persist_aces && matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) { let _ = protect_workspace_codex_dir(current_dir, psid_workspace.as_ptr()); let _ = protect_workspace_agents_dir(current_dir, psid_workspace.as_ptr()); @@ -325,6 +329,7 @@ pub(crate) fn prepare_elevated_spawn_context( unsafe { allow_null_device(psid_to_use.as_ptr()); + allow_named_pipe_device(psid_to_use.as_ptr()); } Ok(ElevatedSpawnContext {