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
This commit is contained in:
iceweasel-oai
2026-10-15 17:36:47 +00:00
committed by copyberry
parent 922510f4d7
commit 3df96b085c
5 changed files with 76 additions and 17 deletions
+1
View File
@@ -36,6 +36,7 @@ windows-sys = { workspace = true, features = [
"Win32_Storage_FileSystem",
"Win32_System_Com",
"Win32_System_SystemInformation",
"Win32_System_WindowsProgramming",
"Win32_UI_Shell",
] }
+8 -2
View File
@@ -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::<std::io::Result<Vec<_>>>()?;
Ok(policy::materialize_volume_roots(policy, &volumes)?)
Ok(policy::materialize_volume_roots(
policy,
&volumes,
&read_volumes,
)?)
}
/// Entry point dispatched before ordinary Codex CLI parsing.
+21 -8
View File
@@ -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::<std::io::Result<Vec<_>>>()?;
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<FileSystemSandboxPolicy, PolicyError> {
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 { .. }
+14 -7
View File
@@ -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],
+32
View File
@@ -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<Vec<PathUri>> {
volume_roots
.iter()
.filter(|root| {
let wide = root
.as_os_str()
.encode_wide()
.chain([0])
.collect::<Vec<_>>();
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<Vec<PathBuf>> {
use std::os::windows::ffi::OsStringExt;