diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index c01537e895..a91aacb237 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -97,9 +97,9 @@ pub struct ExecParams { /// Resolved filesystem overrides for the Windows sandbox backends. /// -/// 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. +/// The unelevated restricted-token backend consumes explicit write roots and +/// extra deny-write carveouts on top of the legacy `WorkspaceWrite` shape. The +/// elevated backend can also consume explicit read roots during setup/refresh. /// Read-root overrides are layered on top of the baseline helper roots that the /// elevated setup path needs to launch the sandboxed command. Split policies /// that opt into platform defaults carry that explicitly with the override. @@ -617,11 +617,11 @@ async fn exec_windows_sandbox( .collect::>() }) .unwrap_or_default(); - let elevated_read_roots_override = windows_sandbox_filesystem_overrides + let read_roots_override = windows_sandbox_filesystem_overrides .and_then(|overrides| overrides.read_roots_override.clone()); - let elevated_read_roots_include_platform_defaults = windows_sandbox_filesystem_overrides + let read_roots_include_platform_defaults = windows_sandbox_filesystem_overrides .is_some_and(|overrides| overrides.read_roots_include_platform_defaults); - let elevated_write_roots_override = windows_sandbox_filesystem_overrides + let 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| { @@ -645,10 +645,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(), - read_roots_include_platform_defaults: - elevated_read_roots_include_platform_defaults, - write_roots_override: elevated_write_roots_override.as_deref(), + read_roots_override: read_roots_override.as_deref(), + read_roots_include_platform_defaults, + write_roots_override: write_roots_override.as_deref(), deny_write_paths_override: &elevated_deny_write_paths, }, ) @@ -661,6 +660,7 @@ async fn exec_windows_sandbox( &cwd, env, timeout_ms, + write_roots_override.as_deref(), &additional_deny_write_paths, windows_sandbox_private_desktop, ) @@ -1085,12 +1085,18 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( .map(|root| normalize_windows_override_path(root.root.as_path())) .collect::>()?; - if legacy_root_paths != split_root_paths { + let write_roots_override = if legacy_root_paths == split_root_paths { + None + } else if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) + && !split_root_paths.iter().any(|path| path.parent().is_none()) + { + Some(split_root_paths.iter().cloned().collect::>()) + } else { return Err( "windows unelevated restricted-token sandbox cannot enforce split writable root sets directly; refusing to run unsandboxed" .to_string(), ); - } + }; for writable_root in &split_writable_roots { for read_only_subpath in &writable_root.read_only_subpaths { @@ -1112,22 +1118,18 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( let mut additional_deny_write_paths = BTreeSet::new(); for split_root in &split_writable_roots { let split_root_path = normalize_windows_override_path(split_root.root.as_path())?; - let Some(legacy_root) = legacy_writable_roots.iter().find(|candidate| { + let legacy_root = legacy_writable_roots.iter().find(|candidate| { normalize_windows_override_path(candidate.root.as_path()) .is_ok_and(|candidate_path| candidate_path == split_root_path) - }) else { - return Err( - "windows unelevated restricted-token sandbox cannot enforce split writable root sets directly; refusing to run unsandboxed" - .to_string(), - ); - }; + }); for read_only_subpath in &split_root.read_only_subpaths { - if !legacy_root - .read_only_subpaths - .iter() - .any(|candidate| candidate == read_only_subpath) - { + if !legacy_root.is_some_and(|legacy_root| { + legacy_root + .read_only_subpaths + .iter() + .any(|candidate| candidate == read_only_subpath) + }) { additional_deny_write_paths.insert(normalize_windows_override_path( read_only_subpath.as_path(), )?); @@ -1135,14 +1137,14 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( } } - if additional_deny_write_paths.is_empty() { + if write_roots_override.is_none() && additional_deny_write_paths.is_empty() { return Ok(None); } Ok(Some(WindowsSandboxFilesystemOverrides { read_roots_override: None, read_roots_include_platform_defaults: false, - write_roots_override: None, + write_roots_override, additional_deny_write_paths: additional_deny_write_paths .into_iter() .map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string())) diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index f875fc5f34..ab8bf521dc 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -664,6 +664,56 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { ); } +#[test] +fn windows_restricted_token_supports_extra_split_writable_root_override() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let cwd = dunce::canonicalize(temp_dir.path()) + .expect("canonicalize temp dir") + .abs(); + let extra_root = cwd.join("extra-root"); + std::fs::create_dir_all(extra_root.as_path()).expect("create extra root"); + let policy = SandboxPolicy::WorkspaceWrite { + 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::Path { path: cwd.clone() }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Path { + path: extra_root.clone(), + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, + ]); + + assert_eq!( + resolve_windows_restricted_token_filesystem_overrides( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + &cwd, + WindowsSandboxLevel::RestrictedToken, + ), + Ok(Some(WindowsSandboxFilesystemOverrides { + read_roots_override: None, + read_roots_include_platform_defaults: false, + write_roots_override: Some(vec![cwd.to_path_buf(), extra_root.to_path_buf()]), + additional_deny_write_paths: vec![], + })) + ); +} + #[test] fn windows_elevated_supports_split_restricted_read_roots() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 4279f76080..a9efc9b653 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -254,6 +254,7 @@ mod windows_impl { use super::acl::revoke_ace; use super::allow::AllowDenyPaths; use super::allow::compute_allow_paths; + use super::allow::protected_child_deny_paths_for_roots; use super::cap::load_or_create_cap_sids; use super::cap::workspace_cap_sid_for_cwd; use super::logging::log_failure; @@ -339,6 +340,7 @@ mod windows_impl { cwd, env_map, timeout_ms, + None, &[], use_private_desktop, ) @@ -353,6 +355,7 @@ mod windows_impl { cwd: &Path, mut env_map: HashMap, timeout_ms: Option, + write_roots_override: Option<&[PathBuf]>, additional_deny_write_paths: &[PathBuf], use_private_desktop: bool, ) -> Result { @@ -421,8 +424,18 @@ mod windows_impl { } let persist_aces = is_workspace_write; - let AllowDenyPaths { allow, mut deny } = - compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map); + let AllowDenyPaths { allow, mut deny } = if let Some(write_roots) = write_roots_override { + AllowDenyPaths { + allow: write_roots + .iter() + .filter(|path| path.exists()) + .cloned() + .collect(), + deny: protected_child_deny_paths_for_roots(write_roots, sandbox_policy_cwd), + } + } else { + compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map) + }; for path in additional_deny_write_paths { if path.exists() { deny.insert(path.clone());