From f11d0dd0120ed936629acb57385090e5317e3903 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 9 Sep 2026 22:31:00 +0000 Subject: [PATCH] Prevent filesystem-root read denies in the Windows sandbox (#44327) ## Why The elevated Windows sandbox requires effective `:root` read access and cannot safely enforce policies that deny reads at the filesystem root. ## What changed - Validate root read access before elevated setup, service provisioning, and setup refresh, rejecting root-denying globs before expansion. - Reject filesystem-root deny-read ACL targets after canonicalization and check the opened handle before applying an ACL, so aliases cannot bypass the root guard. - Read and update deny ACLs through the same handle, and allow an existing deny ACE to be recognized when write access to the ACL is unavailable. ## Testing Add regression coverage for default root denial, root-denying globs, allowed root reads with narrower deny rules, handle-based root detection, and reading existing deny ACEs without `WRITE_DAC`. GitOrigin-RevId: b904e7cdbc09ee0147e1818c898b0382b5820c1b --- .../windows_sandbox_processor.rs | 9 +- codex-rs/core/tests/suite/windows_sandbox.rs | 43 ++++++- codex-rs/windows-sandbox-rs/src/acl.rs | 90 +++++++++++++- codex-rs/windows-sandbox-rs/src/acl_tests.rs | 40 ++++++ codex-rs/windows-sandbox-rs/src/audit.rs | 5 +- .../windows-sandbox-rs/src/deny_read_acl.rs | 51 ++++++-- codex-rs/windows-sandbox-rs/src/desktop.rs | 2 +- .../src/resolved_permissions.rs | 117 +++++++++++++++++- codex-rs/windows-sandbox-rs/src/setup.rs | 65 +++++++++- 9 files changed, 392 insertions(+), 30 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs index 6e454e13f7..70e0e6efa8 100644 --- a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs @@ -123,14 +123,17 @@ impl WindowsSandboxRequestProcessor { let service_setup_request = setup_request.clone(); let service_setup_start = Instant::now(); tokio::task::spawn_blocking(move || -> anyhow::Result<()> { - if codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots( + let Ok(permissions) = codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots( &service_setup_request.permission_profile, &service_setup_request.workspace_roots, - ).is_err() { + ) else { // The existing setup path can still succeed for completed // provisioning without resolving the current profile. return Ok(()); - } + }; + permissions.validate_elevated_filesystem_policy( + &service_setup_request.command_cwd, + )?; // The shared setup path below handles helper fallback and // refreshes workspace ACLs after provisioning. codex_windows_sandbox::provision_windows_sandbox_via_service( diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index 2276d33567..3525654b15 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -3,6 +3,9 @@ use codex_core::exec::ExecCapturePolicy; use codex_core::exec::ExecParams; use codex_core::exec::process_exec_tool_call; use codex_core::sandboxing::SandboxPermissions; +use codex_core::windows_sandbox::WindowsSandboxSetupMode; +use codex_core::windows_sandbox::WindowsSandboxSetupRequest; +use codex_core::windows_sandbox::run_windows_sandbox_setup; use codex_core::windows_sandbox::sandbox_setup_is_complete; use codex_features::Feature; use codex_protocol::config_types::WindowsSandboxLevel; @@ -229,13 +232,12 @@ fn windows_sandbox_cli_preserves_managed_deny_reads_across_launches() -> anyhow: \n\ [permissions.managed-deny-test.filesystem]\n\ \":minimal\" = \"read\"\n\ - \"{}\" = \"read\"\n\ + \":root\" = \"read\"\n\ \"{}\" = \"write\"\n\ \"{}\" = \"deny\"\n\ \n\ [permissions.managed-deny-test.network]\n\ enabled = false\n", - escape_toml_path(&fixture_root), escape_toml_path(&work), escape_toml_path(&denied), ), @@ -262,6 +264,43 @@ fn windows_sandbox_cli_preserves_managed_deny_reads_across_launches() -> anyhow: Ok(()) } +#[tokio::test] +#[serial(codex_home)] +async fn windows_elevated_setup_rejects_default_root_deny() -> anyhow::Result<()> { + let codex_home = codex_home_for_windows_sandbox_test("windows-elevated-root-deny-codex-home")?; + let workspace = TempDir::new()?; + let cwd = dunce::canonicalize(workspace.path())?.abs(); + let file_system_sandbox_policy = + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Write, + missing_path_behavior: None, + }]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + + let err = run_windows_sandbox_setup(WindowsSandboxSetupRequest { + mode: WindowsSandboxSetupMode::Elevated, + permission_profile, + workspace_roots: vec![cwd.clone()], + command_cwd: cwd.to_path_buf(), + env_map: HashMap::new(), + codex_home: codex_home.path().to_path_buf(), + }) + .await + .expect_err("elevated setup should reject default root deny"); + + assert_eq!( + err.to_string(), + "elevated Windows sandbox requires effective `:root` read access" + ); + Ok(()) +} + #[tokio::test] #[serial(codex_home)] async fn windows_restricted_token_rejects_exact_and_glob_deny_read_policy() -> anyhow::Result<()> { diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index 4dbbb155ce..064b7fe6fb 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/codex-rs/windows-sandbox-rs/src/acl.rs @@ -1,9 +1,14 @@ use crate::winutil::resolve_sid; use crate::winutil::sid_bytes_from_string; use crate::winutil::to_wide; +use anyhow::Context; use anyhow::Result; use anyhow::anyhow; +use anyhow::ensure; use std::ffi::c_void; +use std::fs::OpenOptions; +use std::os::windows::fs::OpenOptionsExt; +use std::os::windows::io::AsRawHandle; use std::path::Path; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::ERROR_SUCCESS; @@ -49,8 +54,10 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; +use windows_sys::Win32::Storage::FileSystem::GetFinalPathNameByHandleW; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; +use windows_sys::Win32::Storage::FileSystem::VOLUME_NAME_NONE; use windows_sys::Win32::Storage::FileSystem::WRITE_DAC; use windows_sys::Win32::Storage::FileSystem::WRITE_OWNER; const SE_KERNEL_OBJECT: u32 = 6; @@ -740,11 +747,60 @@ impl DenyAceKind { } } +unsafe fn deny_ace_already_present( + handle: &std::fs::File, + path: &Path, + psid: *mut c_void, + kind: DenyAceKind, +) -> Result { + let (mut p_dacl, mut p_sd) = (std::ptr::null_mut(), std::ptr::null_mut()); + let code = GetSecurityInfo( + handle.as_raw_handle() as _, + 1, + DACL_SECURITY_INFORMATION, + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut p_dacl, + std::ptr::null_mut(), + &mut p_sd, + ); + let result = + acl_api_result(path, "GetSecurityInfo", code).map(|()| kind.already_present(p_dacl, psid)); + if !p_sd.is_null() { + LocalFree(p_sd as HLOCAL); + } + result +} + unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Result { + let handle = match OpenOptions::new() + .access_mode(READ_CONTROL | WRITE_DAC) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(path) + { + Ok(handle) => handle, + Err(write_error) => { + let read_handle = OpenOptions::new() + .access_mode(READ_CONTROL) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(path) + .with_context(|| format!("open deny ACL target {}", path.display()))?; + if matches!(kind, DenyAceKind::Read) { + ensure_handle_is_not_filesystem_root(&read_handle, path)?; + } + if deny_ace_already_present(&read_handle, path, psid, kind)? { + return Ok(false); + } + return Err(write_error).context("open deny ACL target for update"); + } + }; + if matches!(kind, DenyAceKind::Read) { + ensure_handle_is_not_filesystem_root(&handle, path)?; + } let mut p_sd: *mut c_void = std::ptr::null_mut(); let mut p_dacl: *mut ACL = std::ptr::null_mut(); - let code = GetNamedSecurityInfoW( - to_wide(path).as_ptr(), + let code = GetSecurityInfo( + handle.as_raw_handle() as _, 1, DACL_SECURITY_INFORMATION, std::ptr::null_mut(), @@ -758,7 +814,7 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res LocalFree(p_sd as HLOCAL); } return Err(anyhow!( - "GetNamedSecurityInfoW failed for {}: {code}", + "GetSecurityInfo failed for {}: {code}", path.display() )); } @@ -782,8 +838,8 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res let result = if let Err(err) = acl_api_result(path, "SetEntriesInAclW", code2) { Err(err) } else { - let code3 = SetNamedSecurityInfoW( - to_wide(path).as_ptr() as *mut u16, + let code3 = SetSecurityInfo( + handle.as_raw_handle() as _, 1, DACL_SECURITY_INFORMATION, std::ptr::null_mut(), @@ -791,7 +847,7 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res p_new_dacl, std::ptr::null_mut(), ); - acl_api_result(path, "SetNamedSecurityInfoW", code3).map(|()| true) + acl_api_result(path, "SetSecurityInfo", code3).map(|()| true) }; if !p_new_dacl.is_null() { LocalFree(p_new_dacl as HLOCAL); @@ -804,6 +860,28 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res result } +fn ensure_handle_is_not_filesystem_root(handle: &std::fs::File, path: &Path) -> Result<()> { + let mut buffer = [0_u16; 2]; + let length = unsafe { + GetFinalPathNameByHandleW( + handle.as_raw_handle() as _, + buffer.as_mut_ptr(), + buffer.len() as u32, + VOLUME_NAME_NONE, + ) + }; + if length == 0 { + return Err(std::io::Error::last_os_error()) + .with_context(|| format!("resolve deny-read ACL target {}", path.display())); + } + ensure!( + length != 1 || buffer[0] != b'\\' as u16, + "refusing to apply a deny-read ACE to filesystem root {}", + path.display() + ); + Ok(()) +} + /// Adds a deny ACE to prevent reads for the given SID on the target path. /// /// `SetEntriesInAclW` places newly-created deny ACEs before allow ACEs, which diff --git a/codex-rs/windows-sandbox-rs/src/acl_tests.rs b/codex-rs/windows-sandbox-rs/src/acl_tests.rs index 96ad375ceb..597df03ced 100644 --- a/codex-rs/windows-sandbox-rs/src/acl_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/acl_tests.rs @@ -1,6 +1,13 @@ use super::acl_api_result; +use super::deny_ace_already_present; +use super::ensure_handle_is_not_filesystem_root; +use crate::token::LocalSid; use pretty_assertions::assert_eq; +use std::fs::OpenOptions; +use std::os::windows::fs::OpenOptionsExt; use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED; +use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; +use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; #[test] fn deny_ace_update_failure_is_an_error() { @@ -13,3 +20,36 @@ fn deny_ace_update_failure_is_an_error() { r"SetNamedSecurityInfoW failed for C:\world-writable: 5" ); } + +#[test] +fn deny_read_root_check_uses_the_open_handle() { + let cwd = std::env::current_dir().expect("current directory"); + let root = cwd.ancestors().last().expect("filesystem root"); + let root_directory = OpenOptions::new() + .access_mode(READ_CONTROL) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(root) + .expect("open filesystem root"); + let diagnostic_path = std::path::Path::new(r"C:\not-a-root"); + + let error = ensure_handle_is_not_filesystem_root(&root_directory, diagnostic_path) + .expect_err("classification must follow the root handle, not the diagnostic path"); + + assert_eq!( + error.to_string(), + r"refusing to apply a deny-read ACE to filesystem root C:\not-a-root" + ); +} + +#[test] +fn existing_deny_ace_is_visible_without_write_dac() { + let target = tempfile::NamedTempFile::new().expect("temporary file"); + let sid = LocalSid::from_string("S-1-5-21-10-20-30-40").expect("test SID"); + let path = target.path(); + let psid = sid.as_ptr(); + assert!(unsafe { super::add_deny_read_ace(path, psid) }.expect("add deny ACE")); + let already_present = + unsafe { deny_ace_already_present(target.as_file(), path, psid, super::DenyAceKind::Read) } + .expect("read existing deny ACE"); + assert!(already_present); +} diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index e4451f1781..5a9fb5a3a9 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -262,7 +262,10 @@ fn apply_capability_denies_for_world_writable_for_permissions( let cap_path = cap_sid_file(codex_home); let caps = load_or_create_cap_sids(codex_home)?; std::fs::write(&cap_path, serde_json::to_string(&caps)?)?; - if !permissions.is_enforceable_by_windows_sandbox() { + if permissions + .validate_elevated_filesystem_policy(cwd) + .is_err() + { return Ok(()); } let (active_sids, workspace_roots): (Vec, Vec) = diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_acl.rs b/codex-rs/windows-sandbox-rs/src/deny_read_acl.rs index dd3bc350e4..fd1dd1b7f3 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_acl.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_acl.rs @@ -1,6 +1,5 @@ use crate::acl::add_deny_read_ace; use crate::acl::revoke_ace; -use crate::path_normalization::canonicalize_path; use anyhow::Context; use anyhow::Result; use std::collections::HashSet; @@ -14,17 +13,33 @@ use std::path::PathBuf; /// canonical target. The lexical path covers the path users configured and lets /// missing exact denies be materialized later; the canonical path also covers /// an existing reparse-point target so a sandbox cannot read the same object -/// through the resolved location. -pub fn plan_deny_read_acl_paths(paths: &[PathBuf]) -> Vec { +/// through the resolved location. Filesystem roots are rejected after this +/// canonicalization so aliases cannot install a root deny ACE. +pub fn plan_deny_read_acl_paths(paths: &[PathBuf]) -> Result> { let mut planned = Vec::new(); let mut seen = HashSet::new(); for path in paths { push_planned_path(&mut planned, &mut seen, path.to_path_buf()); - if path.exists() { - push_planned_path(&mut planned, &mut seen, canonicalize_path(path)); - } + let canonical = match dunce::canonicalize(path) { + Ok(canonical) => canonical, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => { + return Err(err) + .with_context(|| format!("canonicalize deny-read path {}", path.display())); + } + }; + push_planned_path(&mut planned, &mut seen, canonical); } - planned + if let Some(root) = planned + .iter() + .find(|path| path.is_absolute() && path.parent().is_none()) + { + anyhow::bail!( + "refusing to apply a deny-read ACE to filesystem root {}", + root.display() + ); + } + Ok(planned) } fn push_planned_path(planned: &mut Vec, seen: &mut HashSet, path: PathBuf) { @@ -49,7 +64,7 @@ pub(crate) fn lexical_path_key(path: &Path) -> String { /// # Safety /// Caller must pass a valid SID pointer for the sandbox principal being denied. pub unsafe fn apply_deny_read_acls(paths: &[PathBuf], psid: *mut c_void) -> Result> { - let planned = plan_deny_read_acl_paths(paths); + let planned = plan_deny_read_acl_paths(paths)?; let mut applied = Vec::new(); let mut seen = HashSet::new(); let mut added_in_this_call: Vec = Vec::new(); @@ -93,7 +108,7 @@ mod tests { let missing = tmp.path().join("future-secret.env"); assert_eq!( - plan_deny_read_acl_paths(std::slice::from_ref(&missing)), + plan_deny_read_acl_paths(std::slice::from_ref(&missing)).expect("plan deny-read ACL"), vec![missing] ); } @@ -105,6 +120,7 @@ mod tests { std::fs::write(&existing, "secret").expect("write secret"); let planned: HashSet = plan_deny_read_acl_paths(std::slice::from_ref(&existing)) + .expect("plan deny-read ACL") .into_iter() .collect(); let expected: HashSet = [ @@ -116,4 +132,21 @@ mod tests { assert_eq!(planned, expected); } + + #[test] + fn plan_rejects_file_system_root() { + let cwd = std::env::current_dir().expect("current directory"); + let root = cwd.ancestors().last().expect("filesystem root"); + + let err = plan_deny_read_acl_paths(&[root.to_path_buf()]) + .expect_err("filesystem root must not receive a deny-read ACE"); + + assert_eq!( + err.to_string(), + format!( + "refusing to apply a deny-read ACE to filesystem root {}", + root.display() + ) + ); + } } diff --git a/codex-rs/windows-sandbox-rs/src/desktop.rs b/codex-rs/windows-sandbox-rs/src/desktop.rs index 0dcc60a4cb..d8e7d19adf 100644 --- a/codex-rs/windows-sandbox-rs/src/desktop.rs +++ b/codex-rs/windows-sandbox-rs/src/desktop.rs @@ -131,7 +131,7 @@ impl DesktopPolicy { write_roots: write_roots.into_iter().collect(), deny_read_paths: plan_deny_read_acl_paths( overrides.deny_read_paths.as_deref().unwrap_or_default(), - ) + )? .into_iter() .collect(), deny_write_paths: build_payload_deny_write_paths(&request, overrides.deny_write_paths) diff --git a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs index 9834370ec0..392dd5fc0d 100644 --- a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs +++ b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs @@ -6,6 +6,7 @@ use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath::Root; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::permissions::ReadDenyMatcher; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; use std::path::Path; @@ -99,8 +100,20 @@ impl ResolvedWindowsSandboxPermissions { self.network } - pub(crate) fn is_enforceable_by_windows_sandbox(&self) -> bool { - matches!(self.file_system.kind, FileSystemSandboxKind::Restricted) + /// Rejects filesystem policies that the elevated Windows sandbox cannot + /// enforce safely. + pub fn validate_elevated_filesystem_policy(&self, cwd: &Path) -> Result<()> { + let root = cwd + .ancestors() + .last() + .ok_or_else(|| anyhow::anyhow!("command cwd has no filesystem root"))?; + let root_is_denied = ReadDenyMatcher::try_new_for_local_paths(&self.file_system, cwd) + .map_err(anyhow::Error::msg)? + .is_some_and(|matcher| matcher.is_local_path_read_denied(root)); + if !self.file_system.can_read_local_path_with_cwd(root, cwd) || root_is_denied { + anyhow::bail!("elevated Windows sandbox requires effective `:root` read access"); + } + Ok(()) } pub(crate) fn has_full_disk_read_access(&self) -> bool { @@ -223,6 +236,16 @@ mod tests { vec![AbsolutePathBuf::from_absolute_path(root).expect("absolute workspace root")] } + fn permission_profile_with_entries(entries: Vec) -> PermissionProfile { + PermissionProfile::Managed { + file_system: ManagedFileSystemPermissions::Restricted { + entries, + glob_scan_max_depth: None, + }, + network: NetworkSandboxPolicy::Restricted, + } + } + #[test] fn permission_profile_workspace_write_uses_windows_temp_env_vars() { let tmp = TempDir::new().expect("tempdir"); @@ -464,6 +487,96 @@ mod tests { ); } + #[test] + fn elevated_filesystem_policy_requires_effective_root_read_access() { + let tmp = TempDir::new().expect("tempdir"); + let cwd = tmp.path().join("workspace"); + let denied_path = tmp.path().join("private"); + std::fs::create_dir_all(&cwd).expect("create cwd"); + + let default_root_denied = ResolvedWindowsSandboxPermissions::try_from_permission_profile( + &permission_profile_with_entries(vec![FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + FileSystemAccessMode::Write, + )]), + ) + .expect("managed permission profile"); + let root_read_with_carveout = + ResolvedWindowsSandboxPermissions::try_from_permission_profile( + &permission_profile_with_entries(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + FileSystemPath::Path { + path: AbsolutePathBuf::from_absolute_path(&denied_path) + .expect("absolute denied path") + .into(), + }, + FileSystemAccessMode::Deny, + ), + ]), + ) + .expect("managed permission profile"); + let root = cwd.ancestors().last().expect("filesystem root"); + let root_read_with_explicit_path = + ResolvedWindowsSandboxPermissions::try_from_permission_profile( + &permission_profile_with_entries(vec![FileSystemSandboxEntry::new( + FileSystemPath::Path { + path: AbsolutePathBuf::from_absolute_path(root) + .expect("absolute root") + .into(), + }, + FileSystemAccessMode::Read, + )]), + ) + .expect("managed permission profile"); + let root_read_with_root_deny_glob = + ResolvedWindowsSandboxPermissions::try_from_permission_profile( + &permission_profile_with_entries(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: root.join("**").display().to_string(), + }, + FileSystemAccessMode::Deny, + ), + ]), + ) + .expect("managed permission profile"); + + assert!( + default_root_denied + .validate_elevated_filesystem_policy(&cwd) + .is_err() + ); + assert!( + root_read_with_carveout + .validate_elevated_filesystem_policy(&cwd) + .is_ok() + ); + assert!( + root_read_with_explicit_path + .validate_elevated_filesystem_policy(&cwd) + .is_ok() + ); + assert!( + root_read_with_root_deny_glob + .validate_elevated_filesystem_policy(&cwd) + .is_err() + ); + } + #[test] fn token_mode_rejects_full_disk_write_entries() { let tmp = TempDir::new().expect("tempdir"); diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 7eb6279088..1e84b6fef4 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -235,6 +235,7 @@ pub fn run_setup_refresh( else { return Ok(()); }; + permissions.validate_elevated_filesystem_policy(command_cwd)?; let deny_read_paths = setup_refresh_deny_read_paths(permission_profile, workspace_roots, command_cwd)?; run_setup_refresh_inner( @@ -278,6 +279,7 @@ pub fn run_setup_refresh_with_extra_read_roots( else { return Ok(()); }; + permissions.validate_elevated_filesystem_policy(command_cwd)?; let deny_read_paths = setup_refresh_deny_read_paths(permission_profile, workspace_roots, command_cwd)?; let mut read_roots = gather_read_roots(command_cwd, &permissions, env_map, codex_home); @@ -325,9 +327,9 @@ fn run_setup_refresh_inner( overrides: SetupRootOverrides, offline_proxy_settings_override: Option<&OfflineProxySettings>, ) -> Result<()> { - if !request.permissions.is_enforceable_by_windows_sandbox() { - anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup"); - } + request + .permissions + .validate_elevated_filesystem_policy(request.command_cwd)?; let (read_roots, write_roots) = build_payload_roots(&request, &overrides); let deny_read_paths = build_payload_deny_read_paths(overrides.deny_read_paths); let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths); @@ -1052,9 +1054,9 @@ fn run_elevated_setup_inner( request: SandboxSetupRequest<'_>, offline_proxy_settings_override: Option<&OfflineProxySettings>, ) -> Result<()> { - if !request.permissions.is_enforceable_by_windows_sandbox() { - anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup"); - } + request + .permissions + .validate_elevated_filesystem_policy(request.command_cwd)?; // Ensure the shared sandbox directory exists before we send it to the elevated helper. let sbx_dir = sandbox_dir(request.codex_home); std::fs::create_dir_all(&sbx_dir).map_err(|err| { @@ -1409,6 +1411,7 @@ mod tests { use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::permissions::project_roots_glob_pattern; @@ -1764,6 +1767,56 @@ mod tests { ); } + #[test] + fn setup_refresh_rejects_root_globs_before_expansion() { + let tmp = TempDir::new().expect("tempdir"); + let command_cwd = tmp.path().join("workspace"); + let codex_home = tmp.path().join("codex-home"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + let root = command_cwd.ancestors().last().expect("filesystem root"); + let mut file_system = FileSystemSandboxPolicy::read_only(); + file_system.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: root.join("**").display().to_string(), + }, + FileSystemAccessMode::Deny, + )); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system, + NetworkSandboxPolicy::Restricted, + ); + let workspace_roots = workspace_roots_for(&command_cwd); + let env_map = HashMap::new(); + let errors = [ + super::run_setup_refresh( + &permission_profile, + &workspace_roots, + &command_cwd, + &env_map, + &codex_home, + /*proxy_enforced*/ false, + ) + .expect_err("root glob must be rejected before expansion"), + super::run_setup_refresh_with_extra_read_roots( + &permission_profile, + &workspace_roots, + &command_cwd, + &env_map, + &codex_home, + Vec::new(), + /*proxy_enforced*/ false, + ) + .expect_err("root glob must be rejected before expansion"), + ]; + + let expected = + "elevated Windows sandbox requires effective `:root` read access".to_string(); + assert_eq!( + errors.map(|err| err.to_string()), + [expected.clone(), expected] + ); + } + #[test] fn setup_refresh_rejects_invalid_deny_read_globs() { let tmp = TempDir::new().expect("tempdir");