From 3df96b085c666b33d3775aa4e77219ec1ed828a0 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 15 Oct 2026 17:31:19 +0000 Subject: [PATCH] Filter uninspectable network roots from implicit MXC read grants (#54363) ## What changed On Windows, omit network roots whose filesystem identity cannot be inspected when expanding implicit `:root` read access. Retain inspectable shares and roots containing the policy or command working directory, and apply the same filtering during policy inspection and command launch. Keep explicit grants unchanged and continue expanding `:root` write and deny rules across all executor volumes. ## Testing Extend policy tests to cover filtered read roots, explicit grant precedence, and approved-command write access across all volumes. GitOrigin-RevId: c4d7c8ce2c9138cdb4518385e2cd05f66518628e --- codex-rs/mxc-sandbox/Cargo.toml | 1 + codex-rs/mxc-sandbox/src/lib.rs | 10 ++++++-- codex-rs/mxc-sandbox/src/policy.rs | 29 +++++++++++++++------ codex-rs/mxc-sandbox/src/policy_tests.rs | 21 ++++++++++------ codex-rs/mxc-sandbox/src/windows.rs | 32 ++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 17 deletions(-) diff --git a/codex-rs/mxc-sandbox/Cargo.toml b/codex-rs/mxc-sandbox/Cargo.toml index 49b5c16e66..7c81efc845 100644 --- a/codex-rs/mxc-sandbox/Cargo.toml +++ b/codex-rs/mxc-sandbox/Cargo.toml @@ -36,6 +36,7 @@ windows-sys = { workspace = true, features = [ "Win32_Storage_FileSystem", "Win32_System_Com", "Win32_System_SystemInformation", + "Win32_System_WindowsProgramming", "Win32_UI_Shell", ] } diff --git a/codex-rs/mxc-sandbox/src/lib.rs b/codex-rs/mxc-sandbox/src/lib.rs index 5dc92bdeb4..678ceb8ea2 100644 --- a/codex-rs/mxc-sandbox/src/lib.rs +++ b/codex-rs/mxc-sandbox/src/lib.rs @@ -135,11 +135,17 @@ pub fn prepare_file_system_policy( if policy.has_full_disk_write_access() { return Ok(policy); } - let volumes = windows::volume_roots(policy_cwd.as_path(), command_cwd)? + let volumes = windows::volume_roots(policy_cwd.as_path(), command_cwd)?; + let read_volumes = windows::implicit_read_roots(&volumes, policy_cwd.as_path(), command_cwd)?; + let volumes = volumes .into_iter() .map(PathUri::from_host_native_path) .collect::>>()?; - Ok(policy::materialize_volume_roots(policy, &volumes)?) + Ok(policy::materialize_volume_roots( + policy, + &volumes, + &read_volumes, + )?) } /// Entry point dispatched before ordinary Codex CLI parsing. diff --git a/codex-rs/mxc-sandbox/src/policy.rs b/codex-rs/mxc-sandbox/src/policy.rs index 6fcaaceb06..1bb5486303 100644 --- a/codex-rs/mxc-sandbox/src/policy.rs +++ b/codex-rs/mxc-sandbox/src/policy.rs @@ -6,6 +6,7 @@ use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; +use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::FileSystemSpecialPath; @@ -138,7 +139,11 @@ pub(super) fn build_request( .iter() .map(PathUri::from_host_native_path) .collect::>>()?; - let fs = materialize_volume_roots(policy.clone(), &volumes)?; + #[cfg(windows)] + let read_volumes = crate::windows::implicit_read_roots(volume_roots, cwd, command_cwd)?; + #[cfg(not(windows))] + let read_volumes = volumes.clone(); + let fs = materialize_volume_roots(policy.clone(), &volumes, &read_volumes)?; let roots = if full_disk_write { Vec::new() } else { @@ -342,22 +347,30 @@ fn prune_unavailable_volume_roots(paths: &mut NativePaths, volume_roots: &HashSe }); } -// Bind :root to every executor volume before canonical precedence and carveout -// resolution. This is purely lexical; native volume enumeration happens later. +// Bind :root before canonical precedence and carveout resolution. Only implicit +// reads use the filtered volumes; writes and denies retain every executor volume. pub(super) fn materialize_volume_roots( mut fs: FileSystemSandboxPolicy, volume_roots: &[PathUri], + implicit_read_roots: &[PathUri], ) -> Result { let mut entries = Vec::new(); for entry in fs.entries { match &entry.path { FileSystemPath::Special { value: FileSystemSpecialPath::Root, - } => entries.extend(volume_roots.iter().map(|path| { - let mut entry = entry.clone(); - entry.path = path.clone().into(); - entry - })), + } => { + let roots = if entry.access == FileSystemAccessMode::Read { + implicit_read_roots + } else { + volume_roots + }; + entries.extend(roots.iter().map(|path| { + let mut entry = entry.clone(); + entry.path = path.clone().into(); + entry + })); + } FileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { .. } diff --git a/codex-rs/mxc-sandbox/src/policy_tests.rs b/codex-rs/mxc-sandbox/src/policy_tests.rs index df4934b2b8..90a030dc04 100644 --- a/codex-rs/mxc-sandbox/src/policy_tests.rs +++ b/codex-rs/mxc-sandbox/src/policy_tests.rs @@ -40,9 +40,10 @@ fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> { let volumes = [ PathUri::parse("file:///C:/")?, PathUri::parse("file:///D:/")?, + PathUri::parse("file:///R:/")?, ]; - let readonly = PathUri::parse("file:///D:/foobar")?; - let writable = PathUri::parse("file:///D:/writable")?; + let readonly = PathUri::parse("file:///R:/foobar")?; + let writable = PathUri::parse("file:///R:/writable")?; let mut fs = FileSystemSandboxPolicy::read_only(); fs.entries.extend([ FileSystemSandboxEntry::new(volumes[1].clone().into(), Read), @@ -52,7 +53,7 @@ fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> { ]); for access in [Read, Write, Deny] { fs.entries[0].access = access; - let resolved = materialize_volume_roots(fs.clone(), &volumes)?; + let resolved = materialize_volume_roots(fs.clone(), &volumes, &volumes[..2])?; for volume in &volumes { let cwd = volume.join("work")?; let context = FileSystemSandboxPolicyContext { @@ -62,9 +63,15 @@ fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> { temporary_directories: None, }; assert_eq!( - [&volumes[0], &volumes[1], &readonly, &writable] + [&volumes[0], &volumes[1], &volumes[2], &readonly, &writable] .map(|path| resolved.resolve_access(path, &context)), - [access, access, Read, Write] + [ + access, + access, + if access == Read { Deny } else { access }, + Read, + Write + ] ); } } @@ -76,7 +83,7 @@ fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> { Read, )); assert!(matches!( - materialize_volume_roots(fs, &volumes), + materialize_volume_roots(fs, &volumes, &volumes), Err(PolicyError::UnsupportedSymbolicPath) )); Ok(()) @@ -114,7 +121,7 @@ fn approved_command_expands_volume_roots_without_retaining_original_grants() -> ]); let approved = policy.for_approved_command(&context); assert_eq!(approved.resolve_access(&formerly_readonly, &context), Deny); - let translated = materialize_volume_roots(approved, &volumes)?; + let translated = materialize_volume_roots(approved, &volumes, &volumes[..1])?; assert_eq!( [ &volumes[0], diff --git a/codex-rs/mxc-sandbox/src/windows.rs b/codex-rs/mxc-sandbox/src/windows.rs index 56af7bbbb3..70c38b6185 100644 --- a/codex-rs/mxc-sandbox/src/windows.rs +++ b/codex-rs/mxc-sandbox/src/windows.rs @@ -1,21 +1,28 @@ //! Resolve executor-local state and launch the decoded MXC helper request. use std::collections::HashMap; +use std::os::windows::ffi::OsStrExt; +use std::path::Path; use std::path::PathBuf; use anyhow::Result; use anyhow::ensure; +use codex_utils_path_uri::PathUri; use windows_sys::Win32::Foundation::RPC_E_CHANGED_MODE; +use windows_sys::Win32::Storage::FileSystem::GetDriveTypeW; use windows_sys::Win32::Storage::FileSystem::GetLogicalDrives; use windows_sys::Win32::System::Com::COINIT_MULTITHREADED; use windows_sys::Win32::System::Com::CoInitializeEx; use windows_sys::Win32::System::Com::CoTaskMemFree; use windows_sys::Win32::System::Com::CoUninitialize; use windows_sys::Win32::System::SystemInformation::GetWindowsDirectoryW; +use windows_sys::Win32::System::WindowsProgramming::DRIVE_REMOTE; use windows_sys::Win32::UI::Shell::FOLDERID_ProgramData; use windows_sys::Win32::UI::Shell::FOLDERID_ProgramFiles; use windows_sys::Win32::UI::Shell::FOLDERID_ProgramFilesX86; use windows_sys::Win32::UI::Shell::SHGetKnownFolderPath; +use wxc_common::filesystem_object::ExistingObjectComparison; +use wxc_common::filesystem_object::compare_existing_filesystem_objects; struct ComUninitializeGuard; @@ -85,6 +92,31 @@ pub(super) fn volume_roots( Ok(volumes) } +// Omit only implicit network roots whose object identity MXC cannot inspect. +// Keep healthy shares and cwd volumes; explicit grants follow the normal path. +pub(super) fn implicit_read_roots( + volume_roots: &[PathBuf], + policy_cwd: &Path, + command_cwd: &Path, +) -> std::io::Result> { + volume_roots + .iter() + .filter(|root| { + let wide = root + .as_os_str() + .encode_wide() + .chain([0]) + .collect::>(); + let drive_type = unsafe { GetDriveTypeW(wide.as_ptr()) }; + drive_type != DRIVE_REMOTE + || policy_cwd.starts_with(root) + || command_cwd.starts_with(root) + || compare_existing_filesystem_objects(root, root) == ExistingObjectComparison::Same + }) + .map(PathUri::from_host_native_path) + .collect() +} + fn platform_read_roots() -> Result> { use std::os::windows::ffi::OsStringExt;