From 404967fe250c6a4eefb864253e694fa818738ba4 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Thu, 9 Apr 2026 08:53:19 -0700 Subject: [PATCH] Keep real user on Windows sandbox-created objects --- .../src/elevated/command_runner_win.rs | 23 +++++++++++++++--- .../src/elevated/ipc_framed.rs | 2 ++ .../windows-sandbox-rs/src/elevated_impl.rs | 24 +++++++++++++++++++ codex-rs/windows-sandbox-rs/src/lib.rs | 2 ++ codex-rs/windows-sandbox-rs/src/token.rs | 21 ++++++++++++++-- 5 files changed, 67 insertions(+), 5 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 9cb59e18dc..7b2e6b6c7b 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 @@ -27,7 +27,7 @@ use codex_windows_sandbox::StdinMode; use codex_windows_sandbox::allow_null_device; use codex_windows_sandbox::convert_string_sid_to_sid; 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_workspace_write_token_with_caps_and_default_dacl_sids_from; use codex_windows_sandbox::decode_bytes; use codex_windows_sandbox::encode_bytes; use codex_windows_sandbox::get_current_token_for_restriction; @@ -208,8 +208,16 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { if cap_psids.is_empty() { anyhow::bail!("runner: empty capability SID list"); } + let mut real_user_psid: Option<*mut c_void> = None; + if let Some(real_user_sid) = &req.real_user_sid { + let Some(psid) = (unsafe { convert_string_sid_to_sid(real_user_sid) }) else { + anyhow::bail!("ConvertStringSidToSidW failed for real user SID"); + }; + real_user_psid = Some(psid); + } let base = unsafe { get_current_token_for_restriction()? }; + let default_dacl_extra_sids: Vec<*mut c_void> = real_user_psid.iter().copied().collect(); let token_res: Result<(HANDLE, *mut c_void)> = unsafe { match &policy { SandboxPolicy::ReadOnly { .. } => { @@ -217,8 +225,12 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { .map(|h_token| (h_token, cap_psids[0])) } SandboxPolicy::WorkspaceWrite { .. } => { - create_workspace_write_token_with_caps_from(base, &cap_psids) - .map(|h_token| (h_token, cap_psids[0])) + create_workspace_write_token_with_caps_and_default_dacl_sids_from( + base, + &cap_psids, + &default_dacl_extra_sids, + ) + .map(|h_token| (h_token, cap_psids[0])) } SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { unreachable!() @@ -237,6 +249,11 @@ fn spawn_ipc_process(req: &SpawnRequest) -> Result { LocalFree(psid as HLOCAL); } } + if let Some(psid) = real_user_psid + && !psid.is_null() + { + LocalFree(psid as HLOCAL); + } } let effective_cwd = effective_cwd(&req.cwd, Some(log_dir.as_path())); diff --git a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs index 3f3425eb0e..e4c44fe44c 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/ipc_framed.rs @@ -58,6 +58,8 @@ pub struct SpawnRequest { pub codex_home: PathBuf, pub real_codex_home: PathBuf, pub cap_sids: Vec, + #[serde(default)] + pub real_user_sid: Option, pub timeout_ms: Option, pub tty: bool, #[serde(default)] diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 87ff706e75..54e8d93b2b 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -220,6 +220,29 @@ mod windows_impl { } } + fn resolve_real_user_sid_for_runner(log_dir: Option<&Path>) -> Option { + let real_user = match std::env::var("USERNAME") { + Ok(username) => username, + Err(err) => { + log_note( + &format!("runner launch: USERNAME unavailable: {err}"), + log_dir, + ); + return None; + } + }; + match resolve_sid(&real_user).and_then(|sid| string_from_sid_bytes(&sid)) { + Ok(sid) => Some(sid), + Err(err) => { + log_note( + &format!("runner launch: resolve SID for real user {real_user} failed: {err}"), + log_dir, + ); + None + } + } + } + /// Launches the command runner under the sandbox user and captures its output. #[allow(clippy::too_many_arguments)] pub fn run_windows_sandbox_capture( @@ -416,6 +439,7 @@ mod windows_impl { codex_home: sandbox_base.clone(), real_codex_home: codex_home.to_path_buf(), cap_sids, + real_user_sid: resolve_real_user_sid_for_runner(logs_base_dir), timeout_ms, tty: false, stdin_open: false, diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index dd8f23d00f..96a04b93ff 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -172,6 +172,8 @@ pub use token::create_readonly_token_with_cap_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_default_dacl_sids_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 7d7a6c5c41..d8b5c54e70 100644 --- a/codex-rs/windows-sandbox-rs/src/token.rs +++ b/codex-rs/windows-sandbox-rs/src/token.rs @@ -312,7 +312,22 @@ 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, and include extra SIDs in the token's default DACL. +/// +/// The extra SIDs are not added to the token for access checks; they only keep objects created by +/// sandboxed children manageable by their expected owner (for example, the interactive user). +/// +/// # Safety +/// Caller must close the returned token handle; all SID pointers must be valid for this call. +pub unsafe fn create_workspace_write_token_with_caps_and_default_dacl_sids_from( + base_token: HANDLE, + psid_capabilities: &[*mut c_void], + default_dacl_extra_sids: &[*mut c_void], +) -> Result { + create_token_with_caps_from(base_token, psid_capabilities, default_dacl_extra_sids) } /// Create a restricted token that includes all provided capability SIDs. @@ -323,12 +338,13 @@ 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, &[]) } unsafe fn create_token_with_caps_from( base_token: HANDLE, psid_capabilities: &[*mut c_void], + default_dacl_extra_sids: &[*mut c_void], ) -> Result { if psid_capabilities.is_empty() { return Err(anyhow!("no capability SIDs provided")); @@ -372,6 +388,7 @@ unsafe fn create_token_with_caps_from( dacl_sids.push(psid_logon); dacl_sids.push(psid_everyone); dacl_sids.extend_from_slice(psid_capabilities); + dacl_sids.extend_from_slice(default_dacl_extra_sids); set_default_dacl(new_token, &dacl_sids)?; enable_single_privilege(new_token, "SeChangeNotifyPrivilege")?;