diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index adb205eefab..0ad2f36382a 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -174,6 +174,9 @@ async fn run_command_under_sandbox( timeout_ms: None, use_private_desktop: config.permissions.windows_sandbox_private_desktop, proxy_enforced: false, + read_roots_override: None, + write_roots_override: None, + deny_write_paths_override: &[], }, ) } else { diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index d8ea2ac9843..63d9cba53bf 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -59,6 +59,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 @@ -66,12 +72,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 8a5572a36ed..a16f8843669 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -94,17 +94,30 @@ 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 +/// that the elevated setup path needs to launch the sandboxed command. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct WindowsRestrictedTokenFilesystemOverlay { +pub(crate) struct WindowsSandboxFilesystemOverrides { + pub(crate) read_roots_override: Option>, + pub(crate) write_roots_override: Option>, 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. @@ -300,8 +313,21 @@ 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( + 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, @@ -309,7 +335,8 @@ pub fn build_exec_request( sandbox_cwd, exec_req.windows_sandbox_level, ) - .map_err(CodexErr::UnsupportedOperation)?; + } + .map_err(CodexErr::UnsupportedOperation)?; Ok(exec_req) } @@ -331,7 +358,7 @@ pub(crate) async fn execute_exec_request( sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay, + windows_sandbox_filesystem_overrides, arg0, } = exec_request; @@ -355,7 +382,7 @@ pub(crate) async fn execute_exec_request( sandbox, &sandbox_policy, &file_system_sandbox_policy, - windows_restricted_token_filesystem_overlay.as_ref(), + windows_sandbox_filesystem_overrides.as_ref(), network_sandbox_policy, stdout_stream, after_spawn, @@ -435,7 +462,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_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>, ) -> Result { use crate::config::find_codex_home; use codex_windows_sandbox::run_windows_sandbox_capture_elevated; @@ -478,13 +505,23 @@ 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 additional_deny_write_paths = windows_restricted_token_filesystem_overlay - .map(|overlay| { - overlay + let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced); + let additional_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 elevated_read_roots_override = windows_sandbox_filesystem_overrides + .and_then(|overrides| overrides.read_roots_override.clone()); + let elevated_write_roots_override = windows_sandbox_filesystem_overrides + .and_then(|overrides| overrides.write_roots_override.clone()); + let elevated_deny_write_paths = windows_sandbox_filesystem_overrides + .map(|overrides| { + overrides .additional_deny_write_paths .iter() .map(AbsolutePathBuf::to_path_buf) @@ -504,6 +541,9 @@ async fn exec_windows_sandbox( timeout_ms, use_private_desktop: windows_sandbox_private_desktop, proxy_enforced, + read_roots_override: elevated_read_roots_override.as_deref(), + write_roots_override: elevated_write_roots_override.as_deref(), + deny_write_paths_override: &elevated_deny_write_paths, }, ) } else { @@ -764,7 +804,7 @@ async fn exec( _sandbox: SandboxType, _sandbox_policy: &SandboxPolicy, _file_system_sandbox_policy: &FileSystemSandboxPolicy, - _windows_restricted_token_filesystem_overlay: Option<&WindowsRestrictedTokenFilesystemOverlay>, + _windows_sandbox_filesystem_overrides: Option<&WindowsSandboxFilesystemOverrides>, network_sandbox_policy: NetworkSandboxPolicy, stdout_stream: Option, after_spawn: Option>, @@ -774,7 +814,7 @@ async fn exec( return exec_windows_sandbox( params, _sandbox_policy, - _windows_restricted_token_filesystem_overlay, + _windows_sandbox_filesystem_overrides, ) .await; } @@ -843,26 +883,40 @@ 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 == WindowsSandboxLevel::Elevated, + ) + .err() + } else { + resolve_windows_restricted_token_filesystem_overrides( + 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( +pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( sandbox: SandboxType, sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, -) -> std::result::Result, String> { - if sandbox != SandboxType::WindowsRestrictedToken { +) -> std::result::Result, String> { + if sandbox != SandboxType::WindowsRestrictedToken + || windows_sandbox_level == WindowsSandboxLevel::Elevated + { return Ok(None); } @@ -889,13 +943,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" @@ -918,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 { @@ -951,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( @@ -968,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(), + )?); } } } @@ -978,7 +1026,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())) @@ -986,12 +1036,165 @@ 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()) } +pub(crate) fn resolve_windows_elevated_filesystem_overrides( + sandbox: SandboxType, + sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + use_windows_elevated_backend: bool, +) -> std::result::Result, String> { + if sandbox != SandboxType::WindowsRestrictedToken || !use_windows_elevated_backend { + 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 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 + .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())); + } + } + } + deny_paths + .into_iter() + .map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string())) + .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(WindowsSandboxFilesystemOverrides { + 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 cf00d70501f..13edf89fc1c 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -278,7 +278,7 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> { SandboxType::None, &SandboxPolicy::DangerFullAccess, &FileSystemSandboxPolicy::unrestricted(), - /*windows_restricted_token_filesystem_overlay*/ None, + /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -318,7 +318,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_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Enabled, /*stdout_stream*/ None, /*after_spawn*/ None, @@ -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 { @@ -481,6 +497,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"); @@ -618,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, @@ -626,23 +672,168 @@ 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, })) ); } #[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 expected_docs = dunce::canonicalize(&docs).expect("canonical 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(), + /*use_windows_elevated_backend*/ true, + ), + Ok(Some(WindowsSandboxFilesystemOverrides { + read_roots_override: Some(vec![expected_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"); + let expected_docs = dunce::canonicalize(&docs).expect("canonical docs"); + 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(&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(), + /*use_windows_elevated_backend*/ true, + ), + Ok(Some(WindowsSandboxFilesystemOverrides { + read_roots_override: None, + write_roots_override: None, + additional_deny_write_paths: vec![ + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(expected_docs) + .expect("absolute 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: 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 { @@ -664,6 +855,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!( @@ -676,7 +874,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() ) ); @@ -744,7 +942,7 @@ 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, + /*windows_sandbox_filesystem_overrides*/ None, NetworkSandboxPolicy::Restricted, /*stdout_stream*/ None, /*after_spawn*/ None, diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 0427de7c766..f41fd5a0c2c 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -10,7 +10,7 @@ ExecRequest for execution. use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::exec::StdoutStream; -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; @@ -47,8 +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_sandbox_filesystem_overrides: Option, pub arg0: Option, } @@ -82,7 +81,7 @@ impl ExecRequest { sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay: None, + windows_sandbox_filesystem_overrides: None, arg0, } } @@ -131,7 +130,7 @@ impl ExecRequest { sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - windows_restricted_token_filesystem_overlay: 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 744d8d797c2..83c92ead060 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -176,7 +176,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_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 b52fd8c863b..1af3807eb1a 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -132,7 +132,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_sandbox_filesystem_overrides: _windows_sandbox_filesystem_overrides, arg0, } = sandbox_exec_request; let ParsedShellCommand { script, login, .. } = extract_shell_script(&command)?; @@ -715,7 +715,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_sandbox_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 87ff706e754..327425bd079 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::path::Path; +use std::path::PathBuf; pub struct ElevatedSandboxCaptureRequest<'a> { pub policy_json_or_preset: &'a str, @@ -11,13 +12,14 @@ pub struct ElevatedSandboxCaptureRequest<'a> { pub timeout_ms: Option, pub use_private_desktop: bool, pub proxy_enforced: bool, + pub read_roots_override: Option<&'a [PathBuf]>, + pub write_roots_override: Option<&'a [PathBuf]>, + pub deny_write_paths_override: &'a [PathBuf], } mod windows_impl { use super::ElevatedSandboxCaptureRequest; use crate::acl::allow_null_device; - use crate::allow::AllowDenyPaths; - use crate::allow::compute_allow_paths; use crate::cap::load_or_create_cap_sids; use crate::env::ensure_non_interactive_pager; use crate::env::inherit_path_env; @@ -235,13 +237,15 @@ mod windows_impl { timeout_ms, use_private_desktop, proxy_enforced, + read_roots_override, + write_roots_override, + deny_write_paths_override, } = request; 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)?; @@ -254,6 +258,9 @@ mod windows_impl { cwd, &env_map, codex_home, + read_roots_override, + write_roots_override, + deny_write_paths_override, proxy_enforced, )?; let sandbox_sid = resolve_sid(&sandbox_creds.username).map_err(|err: anyhow::Error| { @@ -291,9 +298,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); } diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index 70ec48fe667..12b02105456 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -5,6 +5,7 @@ use crate::setup::gather_read_roots; use crate::setup::gather_write_roots; use crate::setup::offline_proxy_settings_from_env; use crate::setup::run_elevated_setup; +use crate::setup::run_setup_refresh_with_overrides; use crate::setup::sandbox_users_path; use crate::setup::setup_marker_path; use crate::setup::SandboxNetworkIdentity; @@ -19,6 +20,7 @@ use base64::Engine; use std::collections::HashMap; use std::fs; use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone)] struct SandboxIdentity { @@ -127,17 +129,25 @@ fn select_identity( })) } +#[allow(clippy::too_many_arguments)] pub fn require_logon_sandbox_creds( policy: &SandboxPolicy, policy_cwd: &Path, command_cwd: &Path, env_map: &HashMap, codex_home: &Path, + read_roots_override: Option<&[PathBuf]>, + write_roots_override: Option<&[PathBuf]>, + deny_write_paths_override: &[PathBuf], proxy_enforced: bool, ) -> 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(<[PathBuf]>::to_vec) + .unwrap_or_else(|| gather_read_roots(command_cwd, policy, codex_home)); + let needed_write = write_roots_override + .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 = offline_proxy_settings_from_env(env_map, network_identity); @@ -187,20 +197,28 @@ pub fn require_logon_sandbox_creds( proxy_enforced, }, crate::setup::SetupRootOverrides { - read_roots: Some(needed_read), - write_roots: Some(needed_write), + read_roots: Some(needed_read.clone()), + write_roots: Some(needed_write.clone()), + deny_write_paths: Some(deny_write_paths_override.to_vec()), }, )?; identity = select_identity(network_identity, 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, - proxy_enforced, + run_setup_refresh_with_overrides( + crate::setup::SandboxSetupRequest { + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced, + }, + crate::setup::SetupRootOverrides { + read_roots: Some(needed_read), + write_roots: Some(needed_write), + deny_write_paths: Some(deny_write_paths_override.to_vec()), + }, )?; let identity = identity.ok_or_else(|| { anyhow!( 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 e3f200ca711..6619f60c42a 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -11,6 +11,7 @@ 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; @@ -85,6 +86,7 @@ struct Payload { read_roots: Vec, write_roots: Vec, #[serde(default)] + deny_write_paths: Vec, proxy_ports: Vec, #[serde(default)] allow_local_binding: bool, @@ -642,6 +644,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); @@ -759,6 +762,52 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( } }); + for path in &payload.deny_write_paths { + if !seen_deny_paths.insert(path.clone()) { + continue; + } + + // 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 + // 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 + } 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 99a11e63174..01d9e34f4a1 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -92,6 +92,7 @@ pub struct SandboxSetupRequest<'a> { pub struct SetupRootOverrides { pub read_roots: Option>, pub write_roots: Option>, + pub deny_write_paths: Option>, } pub fn run_setup_refresh( @@ -115,6 +116,13 @@ pub fn run_setup_refresh( ) } +pub fn run_setup_refresh_with_overrides( + request: SandboxSetupRequest<'_>, + overrides: SetupRootOverrides, +) -> Result<()> { + run_setup_refresh_inner(request, overrides) +} + pub fn run_setup_refresh_with_extra_read_roots( policy: &SandboxPolicy, policy_cwd: &Path, @@ -138,6 +146,7 @@ pub fn run_setup_refresh_with_extra_read_roots( SetupRootOverrides { read_roots: Some(read_roots), write_roots: Some(Vec::new()), + deny_write_paths: None, }, ) } @@ -153,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); @@ -165,6 +174,7 @@ fn run_setup_refresh_inner( command_cwd: request.command_cwd.to_path_buf(), read_roots, write_roots, + deny_write_paths: overrides.deny_write_paths.unwrap_or_default(), proxy_ports: offline_proxy_settings.proxy_ports, allow_local_binding: offline_proxy_settings.allow_local_binding, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), @@ -434,6 +444,7 @@ struct ElevationPayload { read_roots: Vec, write_roots: Vec, #[serde(default)] + deny_write_paths: Vec, proxy_ports: Vec, #[serde(default)] allow_local_binding: bool, @@ -723,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); @@ -735,6 +746,7 @@ pub fn run_elevated_setup( command_cwd: request.command_cwd.to_path_buf(), read_roots, write_roots, + deny_write_paths: overrides.deny_write_paths.unwrap_or_default(), proxy_ports: offline_proxy_settings.proxy_ports, allow_local_binding: offline_proxy_settings.allow_local_binding, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), @@ -751,10 +763,10 @@ 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 { - canonical_existing(&roots) + let write_roots = if let Some(roots) = overrides.write_roots.as_deref() { + canonical_existing(roots) } else { gather_write_roots( request.policy, @@ -764,8 +776,19 @@ fn build_payload_roots( ) }; let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home); - let mut read_roots = if let Some(roots) = overrides.read_roots { - 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) }; @@ -802,6 +825,7 @@ fn filter_sensitive_write_roots(mut roots: Vec, codex_home: &Path) -> V #[cfg(test)] mod tests { 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; @@ -1097,6 +1121,152 @@ 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( + &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::default(), + ); + 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 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( + &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 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");