From ab18c97cd4f3178b0a764e909f7a06b22b4a0d50 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Mar 2026 01:11:18 -0700 Subject: [PATCH 01/15] fix: support split carveouts in windows elevated sandbox --- codex-rs/core/README.md | 17 +- codex-rs/core/src/exec.rs | 221 ++++++++++++++++-- codex-rs/core/src/exec_tests.rs | 184 ++++++++++++++- codex-rs/core/src/sandboxing/mod.rs | 4 + codex-rs/core/src/tasks/user_shell.rs | 1 + .../tools/runtimes/shell/unix_escalation.rs | 2 + .../windows-sandbox-rs/src/elevated_impl.rs | 24 +- codex-rs/windows-sandbox-rs/src/identity.rs | 29 ++- .../windows-sandbox-rs/src/setup_main_win.rs | 34 +++ .../src/setup_orchestrator.rs | 55 ++++- 10 files changed, 530 insertions(+), 41 deletions(-) diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index c154ef463dd..0dad9a47ad0 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -81,6 +81,12 @@ backend-managed system read roots required for basic execution, such as `C:\Windows`, `C:\Program Files`, `C:\Program Files (x86)`, and `C:\ProgramData`. When it is `false`, those extra system roots are omitted. +The elevated Windows sandbox also supports: + +- legacy `ReadOnly` and `WorkspaceWrite` behavior +- split filesystem policies that need exact readable roots, exact writable + roots, or extra read-only carveouts under writable roots + The unelevated restricted-token backend still supports the legacy full-read Windows model for legacy `ReadOnly` and `WorkspaceWrite` behavior. It also supports a narrow split-filesystem subset: full-read split policies whose @@ -88,12 +94,11 @@ writable roots still match the legacy `WorkspaceWrite` root set, but add extra read-only carveouts under those writable roots. New `[permissions]` / split filesystem policies remain supported on Windows -only when they round-trip through the legacy `SandboxPolicy` model without -changing semantics. Policies that would require direct read restriction, -explicit unreadable carveouts, reopened writable descendants under read-only -carveouts, different writable root sets, or split carveout support in the -elevated setup/runner backend still fail closed instead of running with weaker -enforcement. +only when they can be enforced directly by the selected Windows backend or +round-trip through the legacy `SandboxPolicy` model without changing semantics. +Policies that would require direct explicit unreadable carveouts (`none`) or +reopened writable descendants under read-only carveouts still fail closed +instead of running with weaker enforcement. ### All Platforms diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index a4399ddc46b..0a3e3ce6cde 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -104,6 +104,18 @@ pub(crate) struct WindowsRestrictedTokenFilesystemOverlay { pub(crate) additional_deny_write_paths: Vec, } +/// Resolved filesystem overrides for the elevated Windows sandbox backend. +/// +/// Unlike the restricted-token path, elevated setup can provision explicit read +/// and write roots up front, so we pass the exact split-policy roots and any +/// extra deny-write carveouts into the setup/refresh path directly. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct WindowsElevatedFilesystemOverrides { + pub(crate) read_roots_override: Option>, + pub(crate) write_roots_override: Option>, + pub(crate) additional_deny_write_paths: Vec, +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum ExecCapturePolicy { /// Shell-like execs keep the historical output cap and timeout behavior. @@ -311,6 +323,15 @@ pub fn build_exec_request( exec_req.windows_sandbox_level, ) .map_err(CodexErr::UnsupportedOperation)?; + exec_req.windows_elevated_filesystem_overrides = resolve_windows_elevated_filesystem_overrides( + exec_req.sandbox, + &exec_req.sandbox_policy, + &exec_req.file_system_sandbox_policy, + exec_req.network_sandbox_policy, + sandbox_cwd, + exec_req.windows_sandbox_level, + ) + .map_err(CodexErr::UnsupportedOperation)?; Ok(exec_req) } @@ -334,6 +355,7 @@ pub(crate) async fn execute_exec_request( file_system_sandbox_policy, network_sandbox_policy, windows_restricted_token_filesystem_overlay, + windows_elevated_filesystem_overrides, arg0, } = exec_request; let _ = _sandbox_policy_from_env; @@ -359,6 +381,7 @@ pub(crate) async fn execute_exec_request( sandbox_policy, &file_system_sandbox_policy, windows_restricted_token_filesystem_overlay.as_ref(), + windows_elevated_filesystem_overrides.as_ref(), network_sandbox_policy, stdout_stream, after_spawn, @@ -439,6 +462,7 @@ async fn exec_windows_sandbox( params: ExecParams, sandbox_policy: &SandboxPolicy, windows_restricted_token_filesystem_overlay: Option<&WindowsRestrictedTokenFilesystemOverlay>, + windows_elevated_filesystem_overrides: Option<&WindowsElevatedFilesystemOverrides>, ) -> Result { use crate::config::find_codex_home; use codex_windows_sandbox::run_windows_sandbox_capture_elevated; @@ -490,6 +514,13 @@ async fn exec_windows_sandbox( .collect::>() }) .unwrap_or_default(); + let elevated_read_roots_override = windows_elevated_filesystem_overrides + .and_then(|overrides| overrides.read_roots_override.clone()); + let elevated_write_roots_override = windows_elevated_filesystem_overrides + .and_then(|overrides| overrides.write_roots_override.clone()); + let elevated_deny_write_paths = windows_elevated_filesystem_overrides + .map(|overrides| overrides.additional_deny_write_paths.clone()) + .unwrap_or_default(); let spawn_res = tokio::task::spawn_blocking(move || { if use_elevated { run_windows_sandbox_capture_elevated( @@ -501,6 +532,9 @@ async fn exec_windows_sandbox( env, timeout_ms, windows_sandbox_private_desktop, + elevated_read_roots_override.as_deref(), + elevated_write_roots_override.as_deref(), + &elevated_deny_write_paths, ) } else { run_windows_sandbox_capture_with_extra_deny_write_paths( @@ -827,6 +861,7 @@ async fn exec( _sandbox_policy: &SandboxPolicy, _file_system_sandbox_policy: &FileSystemSandboxPolicy, _windows_restricted_token_filesystem_overlay: Option<&WindowsRestrictedTokenFilesystemOverlay>, + _windows_elevated_filesystem_overrides: Option<&WindowsElevatedFilesystemOverrides>, network_sandbox_policy: NetworkSandboxPolicy, stdout_stream: Option, after_spawn: Option>, @@ -837,6 +872,7 @@ async fn exec( params, _sandbox_policy, _windows_restricted_token_filesystem_overlay, + _windows_elevated_filesystem_overrides, ) .await; } @@ -905,15 +941,27 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( sandbox_policy_cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, ) -> Option { - resolve_windows_restricted_token_filesystem_overlay( - sandbox, - sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, - sandbox_policy_cwd, - windows_sandbox_level, - ) - .err() + if windows_sandbox_level == WindowsSandboxLevel::Elevated { + resolve_windows_elevated_filesystem_overrides( + sandbox, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd, + windows_sandbox_level, + ) + .err() + } else { + resolve_windows_restricted_token_filesystem_overlay( + sandbox, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd, + windows_sandbox_level, + ) + .err() + } } pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( @@ -924,7 +972,9 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( sandbox_policy_cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, ) -> std::result::Result, String> { - if sandbox != SandboxType::WindowsRestrictedToken { + if sandbox != SandboxType::WindowsRestrictedToken + || windows_sandbox_level == WindowsSandboxLevel::Elevated + { return Ok(None); } @@ -951,13 +1001,6 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( )); } - if windows_sandbox_level != WindowsSandboxLevel::RestrictedToken { - return Err( - "windows elevated sandbox backend cannot enforce split filesystem permissions directly; refusing to run unsandboxed" - .to_string(), - ); - } - if !file_system_sandbox_policy.has_full_disk_read_access() { return Err( "windows unelevated restricted-token sandbox cannot enforce split filesystem read restrictions directly; refusing to run unsandboxed" @@ -1054,6 +1097,150 @@ fn normalize_windows_overlay_path(path: &Path) -> std::result::Result std::result::Result, String> { + if sandbox != SandboxType::WindowsRestrictedToken + || windows_sandbox_level != WindowsSandboxLevel::Elevated + { + return Ok(None); + } + + if !should_use_windows_restricted_token_sandbox( + sandbox, + sandbox_policy, + file_system_sandbox_policy, + ) { + return Err(format!( + "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, legacy_policy={sandbox_policy:?}; refusing to run unsandboxed", + file_system_sandbox_policy.kind, + )); + } + + if !file_system_sandbox_policy + .get_unreadable_roots_with_cwd(sandbox_policy_cwd) + .is_empty() + { + return Err( + "windows elevated sandbox cannot enforce unreadable split filesystem carveouts directly; refusing to run unsandboxed" + .to_string(), + ); + } + + let split_writable_roots = + file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + if has_reopened_writable_descendant(&split_writable_roots) { + return Err( + "windows elevated sandbox cannot reopen writable descendants under read-only carveouts directly; refusing to run unsandboxed" + .to_string(), + ); + } + + let needs_direct_runtime_enforcement = file_system_sandbox_policy + .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd); + let normalize_path = |path: PathBuf| dunce::canonicalize(&path).unwrap_or(path); + let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + let legacy_readable_root_set: BTreeSet = sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(codex_utils_absolute_path::AbsolutePathBuf::into_path_buf) + .map(&normalize_path) + .collect(); + let legacy_root_paths: BTreeSet = legacy_writable_roots + .iter() + .map(|root| normalize_path(root.root.to_path_buf())) + .collect(); + let split_readable_roots: Vec = file_system_sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(codex_utils_absolute_path::AbsolutePathBuf::into_path_buf) + .map(&normalize_path) + .collect(); + let split_readable_root_set: BTreeSet = split_readable_roots.iter().cloned().collect(); + let split_root_paths: Vec = split_writable_roots + .iter() + .map(|root| normalize_path(root.root.to_path_buf())) + .collect(); + let split_root_path_set: BTreeSet = split_root_paths.iter().cloned().collect(); + + let matches_legacy_read_access = file_system_sandbox_policy.has_full_disk_read_access() + == sandbox_policy.has_full_disk_read_access(); + let read_roots_override = if matches_legacy_read_access + && (file_system_sandbox_policy.has_full_disk_read_access() + || split_readable_root_set == legacy_readable_root_set) + { + None + } else { + Some(split_readable_roots) + }; + + let write_roots_override = if split_root_path_set == legacy_root_paths { + None + } else { + Some(split_root_paths) + }; + + let additional_deny_write_paths = if needs_direct_runtime_enforcement { + let mut deny_paths = BTreeSet::new(); + for writable_root in &split_writable_roots { + let legacy_root = legacy_writable_roots + .iter() + .find(|candidate| candidate.root == writable_root.root); + for read_only_subpath in &writable_root.read_only_subpaths { + let already_denied_by_legacy = legacy_root.is_some_and(|legacy_root| { + legacy_root + .read_only_subpaths + .iter() + .any(|candidate| candidate == read_only_subpath) + }); + if !already_denied_by_legacy { + deny_paths.insert(normalize_path(read_only_subpath.to_path_buf())); + } + } + } + deny_paths.into_iter().collect() + } else { + Vec::new() + }; + + if read_roots_override.is_none() + && write_roots_override.is_none() + && additional_deny_write_paths.is_empty() + { + return Ok(None); + } + + Ok(Some(WindowsElevatedFilesystemOverrides { + read_roots_override, + write_roots_override, + additional_deny_write_paths, + })) +} + +fn has_reopened_writable_descendant( + writable_roots: &[codex_protocol::protocol::WritableRoot], +) -> bool { + writable_roots.iter().any(|writable_root| { + writable_root + .read_only_subpaths + .iter() + .any(|read_only_subpath| { + writable_roots.iter().any(|candidate| { + candidate.root.as_path() != writable_root.root.as_path() + && candidate + .root + .as_path() + .starts_with(read_only_subpath.as_path()) + }) + }) + }) +} + /// Consumes the output of a child process according to the configured capture /// policy. async fn consume_output( diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 074db95e1cb..944e7fb6cb6 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -472,6 +472,36 @@ fn windows_restricted_token_allows_legacy_workspace_write_policies() { ); } +#[test] +fn windows_elevated_allows_legacy_restricted_read_policies() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let docs = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path( + temp_dir.path().join("docs"), + ) + .expect("absolute docs"); + std::fs::create_dir_all(docs.as_path()).expect("create docs"); + let policy = SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::Restricted { + readable_roots: vec![docs], + include_platform_defaults: false, + }, + network_access: false, + }; + let file_system_policy = FileSystemSandboxPolicy::from(&policy); + + assert_eq!( + unsupported_windows_restricted_token_sandbox_reason( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + temp_dir.path(), + WindowsSandboxLevel::Elevated, + ), + None + ); +} + #[test] fn windows_restricted_token_rejects_split_only_filesystem_policies() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); @@ -614,7 +644,43 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { } #[test] -fn windows_elevated_rejects_split_write_read_carveouts() { +fn windows_elevated_supports_split_restricted_read_roots() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let docs = temp_dir.path().join("docs"); + std::fs::create_dir_all(&docs).expect("create docs"); + let policy = SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + network_access: false, + }; + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Path { + path: codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) + .expect("absolute docs"), + }, + access: codex_protocol::permissions::FileSystemAccessMode::Read, + }, + ]); + + assert_eq!( + resolve_windows_elevated_filesystem_overrides( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + temp_dir.path(), + WindowsSandboxLevel::Elevated, + ), + Ok(Some(WindowsElevatedFilesystemOverrides { + read_roots_override: Some(vec![docs]), + write_roots_override: None, + additional_deny_write_paths: vec![], + })) + ); +} + +#[test] +fn windows_elevated_supports_split_write_read_carveouts() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); let docs = temp_dir.path().join("docs"); std::fs::create_dir_all(&docs).expect("create docs"); @@ -622,8 +688,110 @@ fn windows_elevated_rejects_split_write_read_carveouts() { writable_roots: vec![], read_only_access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Root, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Read, + }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Path { + path: codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) + .expect("absolute docs"), + }, + access: codex_protocol::permissions::FileSystemAccessMode::Read, + }, + ]); + + assert_eq!( + resolve_windows_elevated_filesystem_overrides( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + temp_dir.path(), + WindowsSandboxLevel::Elevated, + ), + Ok(Some(WindowsElevatedFilesystemOverrides { + read_roots_override: None, + write_roots_override: None, + additional_deny_write_paths: vec![docs], + })) + ); +} + +#[test] +fn windows_elevated_rejects_unreadable_split_carveouts() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let blocked = temp_dir.path().join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Root, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Read, + }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Path { + path: codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&blocked) + .expect("absolute blocked"), + }, + access: codex_protocol::permissions::FileSystemAccessMode::None, + }, + ]); + + assert_eq!( + unsupported_windows_restricted_token_sandbox_reason( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + temp_dir.path(), + WindowsSandboxLevel::Elevated, + ), + Some( + "windows elevated sandbox cannot enforce unreadable split filesystem carveouts directly; refusing to run unsandboxed" + .to_string() + ) + ); +} + +#[test] +fn windows_elevated_rejects_reopened_writable_descendants() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let docs = temp_dir.path().join("docs"); + let nested = docs.join("nested"); + std::fs::create_dir_all(&nested).expect("create nested"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ codex_protocol::permissions::FileSystemSandboxEntry { @@ -645,6 +813,13 @@ fn windows_elevated_rejects_split_write_read_carveouts() { }, access: codex_protocol::permissions::FileSystemAccessMode::Read, }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Path { + path: codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&nested) + .expect("absolute nested"), + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, ]); assert_eq!( @@ -657,7 +832,7 @@ fn windows_elevated_rejects_split_write_read_carveouts() { WindowsSandboxLevel::Elevated, ), Some( - "windows elevated sandbox backend cannot enforce split filesystem permissions directly; refusing to run unsandboxed" + "windows elevated sandbox cannot reopen writable descendants under read-only carveouts directly; refusing to run unsandboxed" .to_string() ) ); @@ -725,6 +900,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()> &SandboxPolicy::new_read_only_policy(), &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()), None, + None, NetworkSandboxPolicy::Restricted, None, None, diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 26f4f733c68..8637cd90820 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -11,6 +11,7 @@ use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; use crate::exec::StdoutStream; +use crate::exec::WindowsElevatedFilesystemOverrides; use crate::exec::WindowsRestrictedTokenFilesystemOverlay; use crate::exec::execute_exec_request; #[cfg(target_os = "macos")] @@ -49,6 +50,7 @@ pub struct ExecRequest { pub network_sandbox_policy: NetworkSandboxPolicy, pub(crate) windows_restricted_token_filesystem_overlay: Option, + pub(crate) windows_elevated_filesystem_overrides: Option, pub arg0: Option, } @@ -83,6 +85,7 @@ impl ExecRequest { file_system_sandbox_policy, network_sandbox_policy, windows_restricted_token_filesystem_overlay: None, + windows_elevated_filesystem_overrides: None, arg0, } } @@ -132,6 +135,7 @@ impl ExecRequest { file_system_sandbox_policy, network_sandbox_policy, windows_restricted_token_filesystem_overlay: None, + windows_elevated_filesystem_overrides: None, arg0, } } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index e118d380953..b2f92fbdb51 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -176,6 +176,7 @@ pub(crate) async fn execute_user_shell_command( file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), windows_restricted_token_filesystem_overlay: None, + windows_elevated_filesystem_overrides: None, arg0: None, }; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 4e107232d0e..685fb4c0731 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -138,6 +138,7 @@ pub(super) async fn try_run_zsh_fork( file_system_sandbox_policy, network_sandbox_policy, windows_restricted_token_filesystem_overlay: _windows_restricted_token_filesystem_overlay, + windows_elevated_filesystem_overrides: _windows_elevated_filesystem_overrides, arg0, } = sandbox_exec_request; let ParsedShellCommand { script, login, .. } = extract_shell_script(&command)?; @@ -911,6 +912,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), network_sandbox_policy: self.network_sandbox_policy, windows_restricted_token_filesystem_overlay: None, + windows_elevated_filesystem_overrides: None, arg0: self.arg0.clone(), }, /*stdout_stream*/ None, diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index f6c4563e171..1929946ae4d 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -1,7 +1,5 @@ mod windows_impl { use crate::acl::allow_null_device; - use crate::allow::compute_allow_paths; - use crate::allow::AllowDenyPaths; use crate::cap::load_or_create_cap_sids; use crate::env::ensure_non_interactive_pager; use crate::env::inherit_path_env; @@ -211,21 +209,31 @@ mod windows_impl { mut env_map: HashMap, timeout_ms: Option, use_private_desktop: bool, + read_roots_override: Option<&[PathBuf]>, + write_roots_override: Option<&[PathBuf]>, + deny_write_paths_override: &[PathBuf], ) -> Result { let policy = parse_policy(policy_json_or_preset)?; normalize_null_device_env(&mut env_map); ensure_non_interactive_pager(&mut env_map); inherit_path_env(&mut env_map); inject_git_safe_directory(&mut env_map, cwd, None); - let current_dir = cwd.to_path_buf(); // Use a temp-based log dir that the sandbox user can write. let sandbox_base = codex_home.join(".sandbox"); ensure_codex_home_exists(&sandbox_base)?; let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); log_start(&command, logs_base_dir); - let sandbox_creds = - require_logon_sandbox_creds(&policy, sandbox_policy_cwd, cwd, &env_map, codex_home)?; + let sandbox_creds = require_logon_sandbox_creds( + &policy, + sandbox_policy_cwd, + cwd, + &env_map, + codex_home, + read_roots_override, + write_roots_override, + deny_write_paths_override, + )?; let sandbox_sid = resolve_sid(&sandbox_creds.username).map_err(|err: anyhow::Error| { io::Error::new(io::ErrorKind::PermissionDenied, err.to_string()) })?; @@ -256,9 +264,6 @@ mod windows_impl { } }; - let AllowDenyPaths { allow: _, deny: _ } = - compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map); - // Deny/allow ACEs are now applied during setup; avoid per-command churn. unsafe { allow_null_device(psid_to_use); } @@ -505,6 +510,9 @@ mod stub { _env_map: HashMap, _timeout_ms: Option, _use_private_desktop: bool, + _read_roots_override: Option<&[PathBuf]>, + _write_roots_override: Option<&[PathBuf]>, + _deny_write_paths_override: &[PathBuf], ) -> Result { bail!("Windows sandbox is only available on Windows") } diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index b9d5e29ed7a..52cd0e1a88e 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -3,7 +3,8 @@ use crate::logging::debug_log; use crate::policy::SandboxPolicy; use crate::setup::gather_read_roots; use crate::setup::gather_write_roots; -use crate::setup::run_elevated_setup; +use crate::setup::run_elevated_setup_with_overrides; +use crate::setup::run_setup_refresh_with_overrides; use crate::setup::sandbox_users_path; use crate::setup::setup_marker_path; use crate::setup::SandboxUserRecord; @@ -17,6 +18,7 @@ use base64::Engine; use std::collections::HashMap; use std::fs; use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone)] struct SandboxIdentity { @@ -128,10 +130,17 @@ pub fn require_logon_sandbox_creds( command_cwd: &Path, env_map: &HashMap, codex_home: &Path, + read_roots_override: Option<&[PathBuf]>, + write_roots_override: Option<&[PathBuf]>, + deny_write_paths_override: &[PathBuf], ) -> Result { let sandbox_dir = crate::setup::sandbox_dir(codex_home); - let needed_read = gather_read_roots(command_cwd, policy, codex_home); - let needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map); + let needed_read = read_roots_override + .map(|roots| roots.to_vec()) + .unwrap_or_else(|| gather_read_roots(command_cwd, policy, codex_home)); + let needed_write = write_roots_override + .map(|roots| roots.to_vec()) + .unwrap_or_else(|| gather_write_roots(policy, policy_cwd, command_cwd, env_map)); // NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the // restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for // granting the sandbox group access to this directory without granting the capability SID. @@ -163,7 +172,7 @@ pub fn require_logon_sandbox_creds( } else { crate::logging::log_note("sandbox setup required", Some(&sandbox_dir)); } - run_elevated_setup( + run_elevated_setup_with_overrides( policy, policy_cwd, command_cwd, @@ -171,11 +180,21 @@ pub fn require_logon_sandbox_creds( codex_home, Some(needed_read.clone()), Some(needed_write.clone()), + Some(deny_write_paths_override.to_vec()), )?; identity = select_identity(policy, codex_home)?; } // Always refresh ACLs (non-elevated) for current roots via the setup binary. - crate::setup::run_setup_refresh(policy, policy_cwd, command_cwd, env_map, codex_home)?; + run_setup_refresh_with_overrides( + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + Some(needed_read), + Some(needed_write), + Some(deny_write_paths_override.to_vec()), + )?; let identity = identity.ok_or_else(|| { anyhow!( "Windows sandbox setup is missing or out of date; rerun the sandbox setup with elevation" diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 86c1d104e08..2b4054863a1 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -6,6 +6,7 @@ use anyhow::Context; use anyhow::Result; use base64::engine::general_purpose::STANDARD as BASE64; use base64::Engine; +use codex_windows_sandbox::add_deny_write_ace; use codex_windows_sandbox::canonicalize_path; use codex_windows_sandbox::convert_string_sid_to_sid; use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance; @@ -84,6 +85,8 @@ struct Payload { command_cwd: PathBuf, read_roots: Vec, write_roots: Vec, + #[serde(default)] + deny_write_paths: Vec, real_user: String, #[serde(default)] mode: SetupMode, @@ -623,6 +626,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | FILE_DELETE_CHILD; let mut grant_tasks: Vec = Vec::new(); + let mut seen_deny_paths: HashSet = HashSet::new(); let mut seen_write_roots: HashSet = HashSet::new(); let canonical_command_cwd = canonicalize_path(&payload.command_cwd); @@ -740,6 +744,36 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( } }); + for path in &payload.deny_write_paths { + if !path.exists() || !seen_deny_paths.insert(path.clone()) { + continue; + } + + let canonical_path = canonicalize_path(path); + let deny_psid = if canonical_path.starts_with(&canonical_command_cwd) { + workspace_psid + } else { + cap_psid + }; + + match unsafe { add_deny_write_ace(path, deny_psid) } { + Ok(true) => { + log_line( + log, + &format!("applied deny ACE to protect {}", path.display()), + )?; + } + Ok(false) => {} + Err(err) => { + refresh_errors.push(format!("deny ACE failed on {}: {err}", path.display())); + log_line( + log, + &format!("deny ACE failed on {}: {err}", path.display()), + )?; + } + } + } + lock_sandbox_dir( &sandbox_bin_dir(&payload.codex_home), &payload.real_user, diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 3296d09c437..32e9d836383 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -85,7 +85,7 @@ pub fn run_setup_refresh( env_map: &HashMap, codex_home: &Path, ) -> Result<()> { - run_setup_refresh_inner( + run_setup_refresh_with_overrides( policy, policy_cwd, command_cwd, @@ -93,6 +93,30 @@ pub fn run_setup_refresh( codex_home, /*read_roots_override*/ None, /*write_roots_override*/ None, + /*deny_write_paths_override*/ None, + ) +} + +#[allow(clippy::too_many_arguments)] +pub fn run_setup_refresh_with_overrides( + policy: &SandboxPolicy, + policy_cwd: &Path, + command_cwd: &Path, + env_map: &HashMap, + codex_home: &Path, + read_roots_override: Option>, + write_roots_override: Option>, + deny_write_paths_override: Option>, +) -> Result<()> { + run_setup_refresh_inner( + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + read_roots_override, + write_roots_override, + deny_write_paths_override, ) } @@ -114,9 +138,11 @@ pub fn run_setup_refresh_with_extra_read_roots( codex_home, Some(read_roots), Some(Vec::new()), + None, ) } +#[allow(clippy::too_many_arguments)] fn run_setup_refresh_inner( policy: &SandboxPolicy, policy_cwd: &Path, @@ -125,6 +151,7 @@ fn run_setup_refresh_inner( codex_home: &Path, read_roots_override: Option>, write_roots_override: Option>, + deny_write_paths_override: Option>, ) -> Result<()> { // Skip in danger-full-access. if matches!( @@ -150,6 +177,7 @@ fn run_setup_refresh_inner( command_cwd: command_cwd.to_path_buf(), read_roots, write_roots, + deny_write_paths: deny_write_paths_override.unwrap_or_default(), real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: true, }; @@ -390,6 +418,8 @@ struct ElevationPayload { command_cwd: PathBuf, read_roots: Vec, write_roots: Vec, + #[serde(default)] + deny_write_paths: Vec, real_user: String, #[serde(default)] refresh_only: bool, @@ -581,6 +611,28 @@ pub fn run_elevated_setup( codex_home: &Path, read_roots_override: Option>, write_roots_override: Option>, +) -> Result<()> { + run_elevated_setup_with_overrides( + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + read_roots_override, + write_roots_override, + None, + ) +} + +pub fn run_elevated_setup_with_overrides( + policy: &SandboxPolicy, + policy_cwd: &Path, + command_cwd: &Path, + env_map: &HashMap, + codex_home: &Path, + read_roots_override: Option>, + write_roots_override: Option>, + deny_write_paths_override: Option>, ) -> Result<()> { // Ensure the shared sandbox directory exists before we send it to the elevated helper. let sbx_dir = sandbox_dir(codex_home); @@ -607,6 +659,7 @@ pub fn run_elevated_setup( command_cwd: command_cwd.to_path_buf(), read_roots, write_roots, + deny_write_paths: deny_write_paths_override.unwrap_or_default(), real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: false, }; From 01617c0c1d3560aa707d34c86899434823470d92 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 17 Mar 2026 19:34:43 -0700 Subject: [PATCH 02/15] fix: restack elevated windows sandbox support --- codex-rs/core/src/exec_tests.rs | 6 ++- codex-rs/windows-sandbox-rs/src/identity.rs | 1 + .../src/setup_orchestrator.rs | 46 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 944e7fb6cb6..d505a1c53f9 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -648,6 +648,7 @@ fn windows_elevated_supports_split_restricted_read_roots() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); let docs = temp_dir.path().join("docs"); std::fs::create_dir_all(&docs).expect("create docs"); + let expected_docs = dunce::canonicalize(&docs).expect("canonical docs"); let policy = SandboxPolicy::ReadOnly { access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, network_access: false, @@ -672,7 +673,7 @@ fn windows_elevated_supports_split_restricted_read_roots() { WindowsSandboxLevel::Elevated, ), Ok(Some(WindowsElevatedFilesystemOverrides { - read_roots_override: Some(vec![docs]), + read_roots_override: Some(vec![expected_docs]), write_roots_override: None, additional_deny_write_paths: vec![], })) @@ -684,6 +685,7 @@ fn windows_elevated_supports_split_write_read_carveouts() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); let docs = temp_dir.path().join("docs"); std::fs::create_dir_all(&docs).expect("create docs"); + let expected_docs = dunce::canonicalize(&docs).expect("canonical docs"); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], read_only_access: codex_protocol::protocol::ReadOnlyAccess::FullAccess, @@ -725,7 +727,7 @@ fn windows_elevated_supports_split_write_read_carveouts() { Ok(Some(WindowsElevatedFilesystemOverrides { read_roots_override: None, write_roots_override: None, - additional_deny_write_paths: vec![docs], + additional_deny_write_paths: vec![expected_docs], })) ); } diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index 52cd0e1a88e..73bd63e5ba1 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -124,6 +124,7 @@ fn select_identity(policy: &SandboxPolicy, codex_home: &Path) -> Result, codex_home: &Path) -> V #[cfg(test)] mod tests { + use super::build_payload_roots; use super::gather_legacy_full_read_roots; use super::gather_read_roots; use super::profile_read_roots; @@ -733,6 +735,7 @@ mod tests { use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::PathBuf; @@ -871,6 +874,49 @@ mod tests { assert!(roots.contains(&expected_writable)); } + #[test] + fn build_payload_roots_preserves_restricted_read_policy_when_no_override_is_needed() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let policy_cwd = tmp.path().join("policy-cwd"); + let command_cwd = tmp.path().join("workspace"); + let readable_root = tmp.path().join("docs"); + fs::create_dir_all(&policy_cwd).expect("create policy cwd"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&readable_root).expect("create readable root"); + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![AbsolutePathBuf::from_absolute_path(&readable_root) + .expect("absolute readable root")], + }, + network_access: false, + }; + + let (read_roots, write_roots) = build_payload_roots( + &policy, + &policy_cwd, + &command_cwd, + &HashMap::new(), + &codex_home, + None, + None, + ); + let expected_helper = + dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); + let expected_cwd = dunce::canonicalize(&command_cwd).expect("canonical workspace"); + let expected_readable = + dunce::canonicalize(&readable_root).expect("canonical readable root"); + + assert_eq!(write_roots, Vec::::new()); + assert!(read_roots.contains(&expected_helper)); + assert!(read_roots.contains(&expected_cwd)); + assert!(read_roots.contains(&expected_readable)); + assert!(canonical_windows_platform_default_roots() + .into_iter() + .all(|path| !read_roots.contains(&path))); + } + #[test] fn full_read_roots_preserve_legacy_platform_defaults() { let tmp = TempDir::new().expect("tempdir"); From c7b5c9089c2e0083143ce0ef60cd9f15100de799 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 24 Mar 2026 15:33:45 -0700 Subject: [PATCH 03/15] fix: refresh elevated branch exec test callsites --- codex-rs/core/src/exec_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index d505a1c53f9..59f70631a83 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -270,6 +270,7 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> { &SandboxPolicy::DangerFullAccess, &FileSystemSandboxPolicy::unrestricted(), /*windows_restricted_token_filesystem_overlay*/ None, + /*windows_elevated_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -310,6 +311,7 @@ async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_p &SandboxPolicy::DangerFullAccess, &FileSystemSandboxPolicy::unrestricted(), /*windows_restricted_token_filesystem_overlay*/ None, + /*windows_elevated_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, From da72d73e45fa4b523aff327f497c773ecc22768d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 25 Mar 2026 00:52:54 -0700 Subject: [PATCH 04/15] fix: refresh windows elevated sandbox debug callsite --- codex-rs/cli/src/debug_sandbox.rs | 3 +++ codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 03226cced44..9f79412bd6f 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -172,6 +172,9 @@ async fn run_command_under_sandbox( env_map, /*timeout_ms*/ None, config.permissions.windows_sandbox_private_desktop, + /*read_roots_override*/ None, + /*write_roots_override*/ None, + /*deny_write_paths_override*/ &[], ) } else { run_windows_sandbox_capture( diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 3d5824b2a1c..b361cc9c173 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -138,7 +138,7 @@ pub fn run_setup_refresh_with_extra_read_roots( codex_home, Some(read_roots), Some(Vec::new()), - None, + /*deny_write_paths_override*/ None, ) } @@ -620,7 +620,7 @@ pub fn run_elevated_setup( codex_home, read_roots_override, write_roots_override, - None, + /*deny_write_paths_override*/ None, ) } From fc4fbd2aba389d2f44778731c4eee4b3dd902247 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 26 Mar 2026 00:07:09 -0700 Subject: [PATCH 05/15] fix: preserve elevated windows helper read roots --- .../src/setup_orchestrator.rs | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index b361cc9c173..e365f624a91 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -688,11 +688,10 @@ fn build_payload_roots( gather_write_roots(policy, policy_cwd, command_cwd, env_map) }; let write_roots = filter_sensitive_write_roots(write_roots, codex_home); - let mut read_roots = if let Some(roots) = read_roots_override { - canonical_existing(&roots) - } else { - gather_read_roots(command_cwd, policy, codex_home) - }; + let mut read_roots = gather_read_roots(command_cwd, policy, codex_home); + if let Some(roots) = read_roots_override { + read_roots.extend(canonical_existing(&roots)); + } let write_root_set: HashSet = write_roots.iter().cloned().collect(); read_roots.retain(|root| !write_root_set.contains(root)); (read_roots, write_roots) @@ -917,6 +916,46 @@ mod tests { .all(|path| !read_roots.contains(&path))); } + #[test] + fn build_payload_roots_preserves_helper_roots_when_read_override_is_provided() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let policy_cwd = tmp.path().join("policy-cwd"); + let command_cwd = tmp.path().join("workspace"); + let readable_root = tmp.path().join("docs"); + fs::create_dir_all(&policy_cwd).expect("create policy cwd"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&readable_root).expect("create readable root"); + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + }; + + let (read_roots, write_roots) = build_payload_roots( + &policy, + &policy_cwd, + &command_cwd, + &HashMap::new(), + &codex_home, + Some(vec![readable_root.clone()]), + None, + ); + let expected_helper = + dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); + let expected_readable = + dunce::canonicalize(&readable_root).expect("canonical readable root"); + + assert_eq!(write_roots, Vec::::new()); + assert!(read_roots.contains(&expected_helper)); + assert!(read_roots.contains(&expected_readable)); + assert!(canonical_windows_platform_default_roots() + .into_iter() + .all(|path| read_roots.contains(&path))); + } + #[test] fn full_read_roots_preserve_legacy_platform_defaults() { let tmp = TempDir::new().expect("tempdir"); From e8bbcb4d2ba14925cb8a72e707810a9ca68e11fb Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 12:21:47 -0700 Subject: [PATCH 06/15] fix: normalize elevated windows deny carveout matching Co-authored-by: Codex --- codex-rs/core/src/exec.rs | 22 +++++++++++------ .../windows-sandbox-rs/src/setup_main_win.rs | 2 +- .../src/setup_orchestrator.rs | 24 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index fac96160e5f..90a9707632e 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1193,15 +1193,23 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( let additional_deny_write_paths = if needs_direct_runtime_enforcement { let mut deny_paths = BTreeSet::new(); for writable_root in &split_writable_roots { - let legacy_root = legacy_writable_roots - .iter() - .find(|candidate| candidate.root == writable_root.root); + let writable_root_path = normalize_path(writable_root.root.to_path_buf()); + let legacy_root = legacy_writable_roots.iter().find(|candidate| { + normalize_path(candidate.root.to_path_buf()) == writable_root_path + }); for read_only_subpath in &writable_root.read_only_subpaths { + let read_only_subpath_suffix = read_only_subpath + .as_path() + .strip_prefix(writable_root.root.as_path()) + .ok(); let already_denied_by_legacy = legacy_root.is_some_and(|legacy_root| { - legacy_root - .read_only_subpaths - .iter() - .any(|candidate| candidate == read_only_subpath) + legacy_root.read_only_subpaths.iter().any(|candidate| { + candidate + .as_path() + .strip_prefix(legacy_root.root.as_path()) + .ok() + == read_only_subpath_suffix + }) }); if !already_denied_by_legacy { deny_paths.insert(normalize_path(read_only_subpath.to_path_buf())); diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 04da934bbaa..4a8965d27e8 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -6,12 +6,12 @@ use anyhow::Context; use anyhow::Result; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; -use codex_windows_sandbox::add_deny_write_ace; use codex_windows_sandbox::LOG_FILE_NAME; use codex_windows_sandbox::SETUP_VERSION; use codex_windows_sandbox::SetupErrorCode; use codex_windows_sandbox::SetupErrorReport; use codex_windows_sandbox::SetupFailure; +use codex_windows_sandbox::add_deny_write_ace; use codex_windows_sandbox::canonicalize_path; use codex_windows_sandbox::convert_string_sid_to_sid; use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance; diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index d751ccf83f6..08fff3a50f4 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -812,8 +812,8 @@ fn filter_sensitive_write_roots(mut roots: Vec, codex_home: &Path) -> V #[cfg(test)] mod tests { - use super::build_payload_roots; use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS; + use super::build_payload_roots; use super::gather_legacy_full_read_roots; use super::gather_read_roots; use super::loopback_proxy_port_from_url; @@ -1122,8 +1122,10 @@ mod tests { let policy = SandboxPolicy::ReadOnly { access: ReadOnlyAccess::Restricted { include_platform_defaults: false, - readable_roots: vec![AbsolutePathBuf::from_absolute_path(&readable_root) - .expect("absolute readable root")], + readable_roots: vec![ + AbsolutePathBuf::from_absolute_path(&readable_root) + .expect("absolute readable root"), + ], }, network_access: false, }; @@ -1149,9 +1151,11 @@ mod tests { assert!(read_roots.contains(&expected_helper)); assert!(read_roots.contains(&expected_cwd)); assert!(read_roots.contains(&expected_readable)); - assert!(canonical_windows_platform_default_roots() - .into_iter() - .all(|path| !read_roots.contains(&path))); + assert!( + canonical_windows_platform_default_roots() + .into_iter() + .all(|path| !read_roots.contains(&path)) + ); } #[test] @@ -1195,9 +1199,11 @@ mod tests { assert_eq!(write_roots, Vec::::new()); assert!(read_roots.contains(&expected_helper)); assert!(read_roots.contains(&expected_readable)); - assert!(canonical_windows_platform_default_roots() - .into_iter() - .all(|path| read_roots.contains(&path))); + assert!( + canonical_windows_platform_default_roots() + .into_iter() + .all(|path| read_roots.contains(&path)) + ); } #[test] From 5e92cf6bc73262842bdb84a30c8c5626d7d5687c Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 12:30:40 -0700 Subject: [PATCH 07/15] fix: borrow elevated windows setup root overrides Co-authored-by: Codex --- .../windows-sandbox-rs/src/setup_orchestrator.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 08fff3a50f4..cb6c0a0913e 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -162,7 +162,7 @@ fn run_setup_refresh_inner( ) { return Ok(()); } - let (read_roots, write_roots) = build_payload_roots(&request, overrides); + let (read_roots, write_roots) = build_payload_roots(&request, &overrides); let network_identity = SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced); let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); @@ -734,7 +734,7 @@ pub fn run_elevated_setup( format!("failed to create sandbox dir {}: {err}", sbx_dir.display()), ) })?; - let (read_roots, write_roots) = build_payload_roots(&request, overrides); + let (read_roots, write_roots) = build_payload_roots(&request, &overrides); let network_identity = SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced); let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); @@ -763,9 +763,9 @@ pub fn run_elevated_setup( fn build_payload_roots( request: &SandboxSetupRequest<'_>, - overrides: SetupRootOverrides, + overrides: &SetupRootOverrides, ) -> (Vec, Vec) { - let write_roots = if let Some(roots) = overrides.write_roots { + let write_roots = if let Some(roots) = overrides.write_roots.as_deref() { canonical_existing(&roots) } else { gather_write_roots( @@ -777,7 +777,7 @@ fn build_payload_roots( }; let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home); let mut read_roots = gather_read_roots(request.command_cwd, request.policy, request.codex_home); - if let Some(roots) = overrides.read_roots { + if let Some(roots) = overrides.read_roots.as_deref() { read_roots.extend(canonical_existing(&roots)); } let write_root_set: HashSet = write_roots.iter().cloned().collect(); @@ -1139,7 +1139,7 @@ mod tests { codex_home: &codex_home, proxy_enforced: false, }, - super::SetupRootOverrides::default(), + &super::SetupRootOverrides::default(), ); let expected_helper = dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); @@ -1185,7 +1185,7 @@ mod tests { codex_home: &codex_home, proxy_enforced: false, }, - super::SetupRootOverrides { + &super::SetupRootOverrides { read_roots: Some(vec![readable_root.clone()]), write_roots: None, deny_write_paths: None, From af6075af7c41e646e35ef82a7e1c7fef5db70166 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 12:33:40 -0700 Subject: [PATCH 08/15] docs: clarify elevated windows read root overrides Co-authored-by: Codex --- codex-rs/core/src/exec.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 90a9707632e..cc7ba7ba2a2 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -108,7 +108,9 @@ pub(crate) struct WindowsRestrictedTokenFilesystemOverlay { /// /// Unlike the restricted-token path, elevated setup can provision explicit read /// and write roots up front, so we pass the exact split-policy roots and any -/// extra deny-write carveouts into the setup/refresh path directly. +/// extra deny-write carveouts into the setup/refresh path directly. Read-root +/// overrides are layered on top of the baseline helper/platform roots from +/// `gather_read_roots()`. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct WindowsElevatedFilesystemOverrides { pub(crate) read_roots_override: Option>, From 2b7e9acde179a519c49ab044161dcf101068690e Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 12:41:00 -0700 Subject: [PATCH 09/15] fix: remove needless borrows in windows setup overrides Co-authored-by: Codex --- codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index cb6c0a0913e..d565ef5cf01 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -766,7 +766,7 @@ fn build_payload_roots( overrides: &SetupRootOverrides, ) -> (Vec, Vec) { let write_roots = if let Some(roots) = overrides.write_roots.as_deref() { - canonical_existing(&roots) + canonical_existing(roots) } else { gather_write_roots( request.policy, @@ -778,7 +778,7 @@ fn build_payload_roots( let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home); let mut read_roots = gather_read_roots(request.command_cwd, request.policy, request.codex_home); if let Some(roots) = overrides.read_roots.as_deref() { - read_roots.extend(canonical_existing(&roots)); + read_roots.extend(canonical_existing(roots)); } let write_root_set: HashSet = write_roots.iter().cloned().collect(); read_roots.retain(|root| !write_root_set.contains(root)); From 2b052557d92acc86502170a08f320830449843b3 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 31 Mar 2026 10:19:29 -0700 Subject: [PATCH 10/15] refactor: merge windows sandbox filesystem overrides Co-authored-by: Codex --- codex-rs/core/src/exec.rs | 111 +++++++++--------- codex-rs/core/src/exec_tests.rs | 26 ++-- codex-rs/core/src/sandboxing/mod.rs | 13 +- codex-rs/core/src/tasks/user_shell.rs | 3 +- .../tools/runtimes/shell/unix_escalation.rs | 6 +- 5 files changed, 75 insertions(+), 84 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index cc7ba7ba2a2..363bebf0e47 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -93,29 +93,18 @@ pub struct ExecParams { pub arg0: Option, } -/// Extra filesystem deny-write carveouts for the non-elevated Windows -/// restricted-token backend. +/// Resolved filesystem overrides for the Windows sandbox backends. /// -/// These are applied on top of the legacy `WorkspaceWrite` allow set, so we -/// can support a narrow split-policy subset without changing legacy Windows -/// sandbox semantics. +/// The unelevated restricted-token backend only consumes extra deny-write +/// carveouts on top of the legacy `WorkspaceWrite` allow set. The elevated +/// backend can also consume explicit read and write roots during setup/refresh. +/// Read-root overrides are layered on top of the baseline helper/platform roots +/// from `gather_read_roots()`. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct WindowsRestrictedTokenFilesystemOverlay { - pub(crate) additional_deny_write_paths: Vec, -} - -/// Resolved filesystem overrides for the elevated Windows sandbox backend. -/// -/// Unlike the restricted-token path, elevated setup can provision explicit read -/// and write roots up front, so we pass the exact split-policy roots and any -/// extra deny-write carveouts into the setup/refresh path directly. Read-root -/// overrides are layered on top of the baseline helper/platform roots from -/// `gather_read_roots()`. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct WindowsElevatedFilesystemOverrides { +pub(crate) struct WindowsSandboxFilesystemOverrides { pub(crate) read_roots_override: Option>, pub(crate) write_roots_override: Option>, - pub(crate) additional_deny_write_paths: Vec, + pub(crate) additional_deny_write_paths: Vec, } #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -313,25 +302,27 @@ pub fn build_exec_request( }) .map(|request| ExecRequest::from_sandbox_exec_request(request, options)) .map_err(CodexErr::from)?; - exec_req.windows_restricted_token_filesystem_overlay = - resolve_windows_restricted_token_filesystem_overlay( - exec_req.sandbox, - &exec_req.sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - exec_req.windows_sandbox_level, - ) + exec_req.windows_sandbox_filesystem_overrides = + if exec_req.windows_sandbox_level == WindowsSandboxLevel::Elevated { + resolve_windows_elevated_filesystem_overrides( + exec_req.sandbox, + &exec_req.sandbox_policy, + &exec_req.file_system_sandbox_policy, + exec_req.network_sandbox_policy, + sandbox_cwd, + exec_req.windows_sandbox_level, + ) + } else { + resolve_windows_restricted_token_filesystem_overlay( + exec_req.sandbox, + &exec_req.sandbox_policy, + &exec_req.file_system_sandbox_policy, + exec_req.network_sandbox_policy, + sandbox_cwd, + exec_req.windows_sandbox_level, + ) + } .map_err(CodexErr::UnsupportedOperation)?; - exec_req.windows_elevated_filesystem_overrides = resolve_windows_elevated_filesystem_overrides( - exec_req.sandbox, - &exec_req.sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - exec_req.windows_sandbox_level, - ) - .map_err(CodexErr::UnsupportedOperation)?; Ok(exec_req) } @@ -354,8 +345,7 @@ pub(crate) async fn execute_exec_request( sandbox_policy: _sandbox_policy_from_env, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay, - windows_elevated_filesystem_overrides, + windows_sandbox_filesystem_overrides, arg0, } = exec_request; let _ = _sandbox_policy_from_env; @@ -380,8 +370,7 @@ pub(crate) async fn execute_exec_request( sandbox, sandbox_policy, &file_system_sandbox_policy, - windows_restricted_token_filesystem_overlay.as_ref(), - windows_elevated_filesystem_overrides.as_ref(), + windows_sandbox_filesystem_overrides.as_ref(), network_sandbox_policy, stdout_stream, after_spawn, @@ -461,8 +450,7 @@ fn record_windows_sandbox_spawn_failure( async fn exec_windows_sandbox( params: ExecParams, sandbox_policy: &SandboxPolicy, - windows_restricted_token_filesystem_overlay: Option<&WindowsRestrictedTokenFilesystemOverlay>, - windows_elevated_filesystem_overrides: Option<&WindowsElevatedFilesystemOverrides>, + windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>, ) -> Result { use crate::config::find_codex_home; use codex_windows_sandbox::run_windows_sandbox_capture_elevated; @@ -509,7 +497,7 @@ async fn exec_windows_sandbox( // proxy-enforced sessions must use that backend even when the configured mode is // the default restricted-token sandbox. let use_elevated = proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated); - let additional_deny_write_paths = windows_restricted_token_filesystem_overlay + let additional_deny_write_paths = windows_sandbox_filesystem_overrides .map(|overlay| { overlay .additional_deny_write_paths @@ -518,12 +506,18 @@ async fn exec_windows_sandbox( .collect::>() }) .unwrap_or_default(); - let elevated_read_roots_override = windows_elevated_filesystem_overrides + let elevated_read_roots_override = windows_sandbox_filesystem_overrides .and_then(|overrides| overrides.read_roots_override.clone()); - let elevated_write_roots_override = windows_elevated_filesystem_overrides + let elevated_write_roots_override = windows_sandbox_filesystem_overrides .and_then(|overrides| overrides.write_roots_override.clone()); - let elevated_deny_write_paths = windows_elevated_filesystem_overrides - .map(|overrides| overrides.additional_deny_write_paths.clone()) + let elevated_deny_write_paths = windows_sandbox_filesystem_overrides + .map(|overrides| { + overrides + .additional_deny_write_paths + .iter() + .map(AbsolutePathBuf::to_path_buf) + .collect::>() + }) .unwrap_or_default(); let spawn_res = tokio::task::spawn_blocking(move || { if use_elevated { @@ -867,8 +861,7 @@ async fn exec( _sandbox: SandboxType, _sandbox_policy: &SandboxPolicy, _file_system_sandbox_policy: &FileSystemSandboxPolicy, - _windows_restricted_token_filesystem_overlay: Option<&WindowsRestrictedTokenFilesystemOverlay>, - _windows_elevated_filesystem_overrides: Option<&WindowsElevatedFilesystemOverrides>, + _windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>, network_sandbox_policy: NetworkSandboxPolicy, stdout_stream: Option, after_spawn: Option>, @@ -878,8 +871,7 @@ async fn exec( return exec_windows_sandbox( params, _sandbox_policy, - _windows_restricted_token_filesystem_overlay, - _windows_elevated_filesystem_overrides, + _windows_sandbox_filesystem_overrides, ) .await; } @@ -978,7 +970,7 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, -) -> std::result::Result, String> { +) -> std::result::Result, String> { if sandbox != SandboxType::WindowsRestrictedToken || windows_sandbox_level == WindowsSandboxLevel::Elevated { @@ -1090,7 +1082,9 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( return Ok(None); } - Ok(Some(WindowsRestrictedTokenFilesystemOverlay { + Ok(Some(WindowsSandboxFilesystemOverrides { + read_roots_override: None, + write_roots_override: None, additional_deny_write_paths: additional_deny_write_paths .into_iter() .map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string())) @@ -1111,7 +1105,7 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, -) -> std::result::Result, String> { +) -> std::result::Result, String> { if sandbox != SandboxType::WindowsRestrictedToken || windows_sandbox_level != WindowsSandboxLevel::Elevated { @@ -1218,7 +1212,10 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( } } } - deny_paths.into_iter().collect() + deny_paths + .into_iter() + .map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string())) + .collect::>()? } else { Vec::new() }; @@ -1230,7 +1227,7 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( return Ok(None); } - Ok(Some(WindowsElevatedFilesystemOverrides { + Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override, write_roots_override, additional_deny_write_paths, diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index c8260159dc0..51a300c63f2 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -269,8 +269,7 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> { SandboxType::None, &SandboxPolicy::DangerFullAccess, &FileSystemSandboxPolicy::unrestricted(), - /*windows_restricted_token_filesystem_overlay*/ None, - /*windows_elevated_filesystem_overrides*/ None, + /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -310,8 +309,7 @@ async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_p SandboxType::None, &SandboxPolicy::DangerFullAccess, &FileSystemSandboxPolicy::unrestricted(), - /*windows_restricted_token_filesystem_overlay*/ None, - /*windows_elevated_filesystem_overrides*/ None, + /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -649,7 +647,9 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { &cwd, WindowsSandboxLevel::RestrictedToken, ), - Ok(Some(WindowsRestrictedTokenFilesystemOverlay { + Ok(Some(WindowsSandboxFilesystemOverrides { + read_roots_override: None, + write_roots_override: None, additional_deny_write_paths: expected_deny_write_paths, })) ); @@ -684,7 +684,7 @@ fn windows_elevated_supports_split_restricted_read_roots() { temp_dir.path(), WindowsSandboxLevel::Elevated, ), - Ok(Some(WindowsElevatedFilesystemOverrides { + Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override: Some(vec![expected_docs]), write_roots_override: None, additional_deny_write_paths: vec![], @@ -736,10 +736,13 @@ fn windows_elevated_supports_split_write_read_carveouts() { temp_dir.path(), WindowsSandboxLevel::Elevated, ), - Ok(Some(WindowsElevatedFilesystemOverrides { + Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override: None, write_roots_override: None, - additional_deny_write_paths: vec![expected_docs], + additional_deny_write_paths: vec![ + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(expected_docs) + .expect("absolute docs"), + ], })) ); } @@ -913,11 +916,10 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()> SandboxType::None, &SandboxPolicy::new_read_only_policy(), &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()), - None, - None, + /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Restricted, - None, - None, + /*stdout_stream*/ None, + /*after_spawn*/ None, ) .await?; assert!(output.timed_out); diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 8637cd90820..d922731c1c5 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -11,8 +11,7 @@ use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; use crate::exec::StdoutStream; -use crate::exec::WindowsElevatedFilesystemOverrides; -use crate::exec::WindowsRestrictedTokenFilesystemOverlay; +use crate::exec::WindowsSandboxFilesystemOverrides; use crate::exec::execute_exec_request; #[cfg(target_os = "macos")] use crate::spawn::CODEX_SANDBOX_ENV_VAR; @@ -48,9 +47,7 @@ pub struct ExecRequest { pub sandbox_policy: SandboxPolicy, pub file_system_sandbox_policy: FileSystemSandboxPolicy, pub network_sandbox_policy: NetworkSandboxPolicy, - pub(crate) windows_restricted_token_filesystem_overlay: - Option, - pub(crate) windows_elevated_filesystem_overrides: Option, + pub(crate) windows_sandbox_filesystem_overrides: Option, pub arg0: Option, } @@ -84,8 +81,7 @@ impl ExecRequest { sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay: None, - windows_elevated_filesystem_overrides: None, + windows_sandbox_filesystem_overrides: None, arg0, } } @@ -134,8 +130,7 @@ impl ExecRequest { sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay: None, - windows_elevated_filesystem_overrides: None, + windows_sandbox_filesystem_overrides: None, arg0, } } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 71134448a40..db1cb8afb6e 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -175,8 +175,7 @@ pub(crate) async fn execute_user_shell_command( sandbox_policy: sandbox_policy.clone(), file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), - windows_restricted_token_filesystem_overlay: None, - windows_elevated_filesystem_overrides: None, + windows_sandbox_filesystem_overrides: None, arg0: None, }; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 3f1a4de66b5..cb579b160ed 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -130,8 +130,7 @@ pub(super) async fn try_run_zsh_fork( sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay: _windows_restricted_token_filesystem_overlay, - windows_elevated_filesystem_overrides: _windows_elevated_filesystem_overrides, + windows_sandbox_filesystem_overrides: _windows_sandbox_filesystem_overrides, arg0, } = sandbox_exec_request; let ParsedShellCommand { script, login, .. } = extract_shell_script(&command)?; @@ -714,8 +713,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { sandbox_policy: self.sandbox_policy.clone(), file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), network_sandbox_policy: self.network_sandbox_policy, - windows_restricted_token_filesystem_overlay: None, - windows_elevated_filesystem_overrides: None, + windows_sandbox_filesystem_overrides: None, arg0: self.arg0.clone(), }, /*stdout_stream*/ None, From d8c64cbeb5b149ddf94a3ecde73c655a64e11f1b Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 6 Apr 2026 11:26:15 -0700 Subject: [PATCH 11/15] fix: address windows sandbox clippy Co-authored-by: Codex --- codex-rs/windows-sandbox-rs/src/identity.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index e86c80d1fb4..12b02105456 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -143,10 +143,10 @@ pub fn require_logon_sandbox_creds( ) -> Result { let sandbox_dir = crate::setup::sandbox_dir(codex_home); let needed_read = read_roots_override - .map(|roots| roots.to_vec()) + .map(<[PathBuf]>::to_vec) .unwrap_or_else(|| gather_read_roots(command_cwd, policy, codex_home)); let needed_write = write_roots_override - .map(|roots| roots.to_vec()) + .map(<[PathBuf]>::to_vec) .unwrap_or_else(|| gather_write_roots(policy, policy_cwd, command_cwd, env_map)); let network_identity = SandboxNetworkIdentity::from_policy(policy, proxy_enforced); let desired_offline_proxy_settings = From 998580478e457073ff65957b8538da6f4fb505cd Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 7 Apr 2026 12:28:37 -0700 Subject: [PATCH 12/15] fix: preserve elevated Windows read overrides Co-authored-by: Codex --- .../src/setup_orchestrator.rs | 69 +++++++++++++++++-- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 6369956f7f8..01d9e34f4a1 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -776,10 +776,22 @@ fn build_payload_roots( ) }; let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home); - let mut read_roots = gather_read_roots(request.command_cwd, request.policy, request.codex_home); - if let Some(roots) = overrides.read_roots.as_deref() { - read_roots.extend(canonical_existing(roots)); - } + let mut read_roots = if let Some(roots) = overrides.read_roots.as_deref() { + // An explicit override is the split policy's complete readable set. Keep only the + // helper/platform roots the elevated setup needs; do not re-add legacy cwd/full-read roots. + let mut read_roots = gather_helper_read_roots(request.codex_home); + if request.policy.include_platform_defaults() { + read_roots.extend( + WINDOWS_PLATFORM_DEFAULT_READ_ROOTS + .iter() + .map(PathBuf::from), + ); + } + read_roots.extend(roots.iter().cloned()); + canonical_existing(&read_roots) + } else { + gather_read_roots(request.command_cwd, request.policy, request.codex_home) + }; let write_root_set: HashSet = write_roots.iter().cloned().collect(); read_roots.retain(|root| !write_root_set.contains(root)); (read_roots, write_roots) @@ -1193,11 +1205,13 @@ mod tests { ); let expected_helper = dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); + let expected_cwd = dunce::canonicalize(&command_cwd).expect("canonical workspace"); let expected_readable = dunce::canonicalize(&readable_root).expect("canonical readable root"); assert_eq!(write_roots, Vec::::new()); assert!(read_roots.contains(&expected_helper)); + assert!(!read_roots.contains(&expected_cwd)); assert!(read_roots.contains(&expected_readable)); assert!( canonical_windows_platform_default_roots() @@ -1206,6 +1220,53 @@ mod tests { ); } + #[test] + fn build_payload_roots_replaces_full_read_policy_when_read_override_is_provided() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let policy_cwd = tmp.path().join("policy-cwd"); + let command_cwd = tmp.path().join("workspace"); + let readable_root = tmp.path().join("docs"); + fs::create_dir_all(&policy_cwd).expect("create policy cwd"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&readable_root).expect("create readable root"); + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: false, + }; + + let (read_roots, write_roots) = build_payload_roots( + &super::SandboxSetupRequest { + policy: &policy, + policy_cwd: &policy_cwd, + command_cwd: &command_cwd, + env_map: &HashMap::new(), + codex_home: &codex_home, + proxy_enforced: false, + }, + &super::SetupRootOverrides { + read_roots: Some(vec![readable_root.clone()]), + write_roots: None, + deny_write_paths: None, + }, + ); + let expected_helper = + dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); + let expected_cwd = dunce::canonicalize(&command_cwd).expect("canonical workspace"); + let expected_readable = + dunce::canonicalize(&readable_root).expect("canonical readable root"); + + assert_eq!(write_roots, Vec::::new()); + assert!(read_roots.contains(&expected_helper)); + assert!(!read_roots.contains(&expected_cwd)); + assert!(read_roots.contains(&expected_readable)); + assert!( + canonical_windows_platform_default_roots() + .into_iter() + .all(|path| !read_roots.contains(&path)) + ); + } + #[test] fn full_read_roots_preserve_legacy_platform_defaults() { let tmp = TempDir::new().expect("tempdir"); From e435cc462c753ce8465f98c18735f2b08cd06205 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 10:26:50 -0700 Subject: [PATCH 13/15] fix: align Windows sandbox filesystem overrides Co-authored-by: Codex --- codex-rs/core/src/exec.rs | 93 ++++++++++--------- codex-rs/core/src/exec_tests.rs | 22 ++++- .../windows-sandbox-rs/src/setup_main_win.rs | 11 ++- 3 files changed, 80 insertions(+), 46 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 654b8c892c0..a16f8843669 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -100,7 +100,7 @@ pub struct ExecParams { /// carveouts on top of the legacy `WorkspaceWrite` allow set. The elevated /// backend can also consume explicit read and write roots during setup/refresh. /// Read-root overrides are layered on top of the baseline helper/platform roots -/// from `gather_read_roots()`. +/// that the elevated setup path needs to launch the sandboxed command. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct WindowsSandboxFilesystemOverrides { pub(crate) read_roots_override: Option>, @@ -108,6 +108,16 @@ pub(crate) struct WindowsSandboxFilesystemOverrides { pub(crate) additional_deny_write_paths: Vec, } +fn windows_sandbox_uses_elevated_backend( + sandbox_level: WindowsSandboxLevel, + proxy_enforced: bool, +) -> bool { + // Windows firewall enforcement is tied to the logon-user sandbox identities, so + // proxy-enforced sessions must use that backend even when the configured mode is + // the default restricted-token sandbox. + proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated) +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum ExecCapturePolicy { /// Shell-like execs keep the historical output cap and timeout behavior. @@ -303,27 +313,30 @@ pub fn build_exec_request( }) .map(|request| ExecRequest::from_sandbox_exec_request(request, options)) .map_err(CodexErr::from)?; - exec_req.windows_sandbox_filesystem_overrides = - if exec_req.windows_sandbox_level == WindowsSandboxLevel::Elevated { - resolve_windows_elevated_filesystem_overrides( - exec_req.sandbox, - &exec_req.sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - exec_req.windows_sandbox_level, - ) - } else { - resolve_windows_restricted_token_filesystem_overlay( - exec_req.sandbox, - &exec_req.sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - exec_req.windows_sandbox_level, - ) - } - .map_err(CodexErr::UnsupportedOperation)?; + let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend( + exec_req.windows_sandbox_level, + exec_req.network.is_some(), + ); + exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend { + resolve_windows_elevated_filesystem_overrides( + exec_req.sandbox, + &exec_req.sandbox_policy, + &exec_req.file_system_sandbox_policy, + exec_req.network_sandbox_policy, + sandbox_cwd, + use_windows_elevated_backend, + ) + } else { + resolve_windows_restricted_token_filesystem_overrides( + exec_req.sandbox, + &exec_req.sandbox_policy, + &exec_req.file_system_sandbox_policy, + exec_req.network_sandbox_policy, + sandbox_cwd, + exec_req.windows_sandbox_level, + ) + } + .map_err(CodexErr::UnsupportedOperation)?; Ok(exec_req) } @@ -492,13 +505,10 @@ async fn exec_windows_sandbox( let command_path = command.first().cloned(); let sandbox_level = windows_sandbox_level; let proxy_enforced = network.is_some(); - // Windows firewall enforcement is tied to the logon-user sandbox identities, so - // proxy-enforced sessions must use that backend even when the configured mode is - // the default restricted-token sandbox. - let use_elevated = proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated); + let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced); let additional_deny_write_paths = windows_sandbox_filesystem_overrides - .map(|overlay| { - overlay + .map(|overrides| { + overrides .additional_deny_write_paths .iter() .map(AbsolutePathBuf::to_path_buf) @@ -880,11 +890,11 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd, - windows_sandbox_level, + windows_sandbox_level == WindowsSandboxLevel::Elevated, ) .err() } else { - resolve_windows_restricted_token_filesystem_overlay( + resolve_windows_restricted_token_filesystem_overrides( sandbox, sandbox_policy, file_system_sandbox_policy, @@ -896,7 +906,7 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( } } -pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( +pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( sandbox: SandboxType, sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, @@ -955,11 +965,11 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); let legacy_root_paths: BTreeSet = legacy_writable_roots .iter() - .map(|root| normalize_windows_overlay_path(root.root.as_path())) + .map(|root| normalize_windows_override_path(root.root.as_path())) .collect::>()?; let split_root_paths: BTreeSet = split_writable_roots .iter() - .map(|root| normalize_windows_overlay_path(root.root.as_path())) + .map(|root| normalize_windows_override_path(root.root.as_path())) .collect::>()?; if legacy_root_paths != split_root_paths { @@ -988,9 +998,9 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( let mut additional_deny_write_paths = BTreeSet::new(); for split_root in &split_writable_roots { - let split_root_path = normalize_windows_overlay_path(split_root.root.as_path())?; + let split_root_path = normalize_windows_override_path(split_root.root.as_path())?; let Some(legacy_root) = legacy_writable_roots.iter().find(|candidate| { - normalize_windows_overlay_path(candidate.root.as_path()) + normalize_windows_override_path(candidate.root.as_path()) .is_ok_and(|candidate_path| candidate_path == split_root_path) }) else { return Err( @@ -1005,8 +1015,9 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( .iter() .any(|candidate| candidate == read_only_subpath) { - additional_deny_write_paths - .insert(normalize_windows_overlay_path(read_only_subpath.as_path())?); + additional_deny_write_paths.insert(normalize_windows_override_path( + read_only_subpath.as_path(), + )?); } } } @@ -1025,7 +1036,7 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overlay( })) } -fn normalize_windows_overlay_path(path: &Path) -> std::result::Result { +fn normalize_windows_override_path(path: &Path) -> std::result::Result { AbsolutePathBuf::from_absolute_path(dunce::simplified(path)) .map(AbsolutePathBuf::into_path_buf) .map_err(|err| err.to_string()) @@ -1037,11 +1048,9 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &Path, - windows_sandbox_level: WindowsSandboxLevel, + use_windows_elevated_backend: bool, ) -> std::result::Result, String> { - if sandbox != SandboxType::WindowsRestrictedToken - || windows_sandbox_level != WindowsSandboxLevel::Elevated - { + if sandbox != SandboxType::WindowsRestrictedToken || !use_windows_elevated_backend { return Ok(None); } diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 8c70253a928..13edf89fc1c 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -414,6 +414,22 @@ fn windows_restricted_token_runs_for_legacy_restricted_policies() { ); } +#[test] +fn windows_proxy_enforcement_uses_elevated_backend() { + assert!(!windows_sandbox_uses_elevated_backend( + WindowsSandboxLevel::RestrictedToken, + /*proxy_enforced*/ false, + )); + assert!(windows_sandbox_uses_elevated_backend( + WindowsSandboxLevel::RestrictedToken, + /*proxy_enforced*/ true, + )); + assert!(windows_sandbox_uses_elevated_backend( + WindowsSandboxLevel::Elevated, + /*proxy_enforced*/ false, + )); +} + #[test] fn windows_restricted_token_rejects_network_only_restrictions() { let policy = SandboxPolicy::ExternalSandbox { @@ -648,7 +664,7 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { ]; assert_eq!( - resolve_windows_restricted_token_filesystem_overlay( + resolve_windows_restricted_token_filesystem_overrides( SandboxType::WindowsRestrictedToken, &policy, &file_system_policy, @@ -691,7 +707,7 @@ fn windows_elevated_supports_split_restricted_read_roots() { &file_system_policy, NetworkSandboxPolicy::Restricted, temp_dir.path(), - WindowsSandboxLevel::Elevated, + /*use_windows_elevated_backend*/ true, ), Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override: Some(vec![expected_docs]), @@ -743,7 +759,7 @@ fn windows_elevated_supports_split_write_read_carveouts() { &file_system_policy, NetworkSandboxPolicy::Restricted, temp_dir.path(), - WindowsSandboxLevel::Elevated, + /*use_windows_elevated_backend*/ true, ), Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override: None, diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index a49c8ff0fc1..f1f92842b97 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -763,10 +763,19 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( }); for path in &payload.deny_write_paths { - if !path.exists() || !seen_deny_paths.insert(path.clone()) { + if !seen_deny_paths.insert(path.clone()) { continue; } + // Deny ACEs attach to filesystem objects; if the read-only carveout does not + // exist during setup, the sandbox could otherwise create it later under a + // writable parent and bypass the carveout. Materialize missing carveouts as + // directories so the deny-write ACL is present before the command starts. + if !path.exists() { + std::fs::create_dir_all(path) + .with_context(|| format!("failed to create deny-write path {}", path.display()))?; + } + let canonical_path = canonicalize_path(path); let deny_psid = if canonical_path.starts_with(&canonical_command_cwd) { workspace_psid From 6c2128ca6cafe9aa74d33c9be524b594794c0794 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 10:36:18 -0700 Subject: [PATCH 14/15] docs: clarify Windows deny-write carveout setup Co-authored-by: Codex --- codex-rs/windows-sandbox-rs/src/setup_main_win.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index f1f92842b97..4956651f94f 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -767,10 +767,14 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( continue; } - // Deny ACEs attach to filesystem objects; if the read-only carveout does not - // exist during setup, the sandbox could otherwise create it later under a - // writable parent and bypass the carveout. Materialize missing carveouts as - // directories so the deny-write ACL is present before the command starts. + // These are explicit read-only carveouts from the transformed sandbox policy, not + // optional workspace sentinels such as `.codex` or `.agents` (those are protected + // best-effort below and still skip missing directories). + // + // Deny ACEs attach to filesystem objects; if a policy carveout does not exist during + // setup, the sandbox could otherwise create it later under a writable parent and + // bypass the carveout. Materialize missing carveouts as directories so the deny-write + // ACL is present before the command starts. if !path.exists() { std::fs::create_dir_all(path) .with_context(|| format!("failed to create deny-write path {}", path.display()))?; From 2d0421613f4031d477af222685b06ad932fcd8c4 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 10:43:51 -0700 Subject: [PATCH 15/15] docs: specify read-only carveout materialization Co-authored-by: Codex --- codex-rs/windows-sandbox-rs/src/setup_main_win.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 4956651f94f..6619f60c42a 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -767,9 +767,12 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( continue; } - // These are explicit read-only carveouts from the transformed sandbox policy, not - // optional workspace sentinels such as `.codex` or `.agents` (those are protected - // best-effort below and still skip missing directories). + // These are explicit read-only-under-a-writable-root carveouts from the transformed + // sandbox policy; they are not deny-read paths. + // + // They are also not optional workspace sentinels such as `.codex` or `.agents`: those + // are protected best-effort below and still skip missing directories so we do not leave + // empty protection artifacts behind in a workspace. // // Deny ACEs attach to filesystem objects; if a policy carveout does not exist during // setup, the sandbox could otherwise create it later under a writable parent and