From d1696652a20d18f1e1188b0a1912ab3c1304ffff Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 10 Sep 2026 16:30:11 +0000 Subject: [PATCH] Support symbolic `:root` filesystem policies in MXC (#44580) ## Why MXC rejected filesystem policies containing `:root` or granting full disk write access, preventing these policies from being translated into native grants. ## What changed - Resolve `:root` against every executor volume before applying permission precedence and read-only carveouts. - Grant full disk write access across enumerated volumes while preserving explicit writable paths outside them, such as unmapped shares. - Treat a `:root` deny as the default so narrower explicit grants survive, while retaining explicit path denies. - Continue rejecting unsupported symbolic paths with a more specific error. ## Testing Add regression tests for precedence across drives and working directories, volume grant expansion, explicit grants outside enumerated volumes, root denies with narrower grants, and restrictions on one volume preserving grants on another. GitOrigin-RevId: 94c9fec9e6429913714ebbdeca382c4f1624b5a2 --- codex-rs/mxc-sandbox/src/policy.rs | 78 +++++-- codex-rs/mxc-sandbox/src/policy_tests.rs | 274 +++++++++++++++++++++++ 2 files changed, 337 insertions(+), 15 deletions(-) diff --git a/codex-rs/mxc-sandbox/src/policy.rs b/codex-rs/mxc-sandbox/src/policy.rs index 5d989458fe..46b767794a 100644 --- a/codex-rs/mxc-sandbox/src/policy.rs +++ b/codex-rs/mxc-sandbox/src/policy.rs @@ -7,6 +7,7 @@ use std::path::Path; use std::path::PathBuf; use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::FileSystemSpecialPath; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -43,8 +44,8 @@ pub enum PolicyError { RelativePolicyCwd, #[error("MXC requires an absolute command working directory")] RelativeCommandCwd, - #[error("MXC symbolic filesystem roots are not implemented")] - SymbolicRoots, + #[error("MXC symbolic filesystem paths other than :root are not implemented")] + UnsupportedSymbolicPath, #[error("MXC requires a Unicode command working directory")] NonUnicodeCommandCwd, #[error("MXC requires Unicode filesystem policy paths")] @@ -81,19 +82,30 @@ pub fn build_request( if !command_cwd.is_absolute() { return Err(PolicyError::RelativeCommandCwd); } - let fs = permissions.file_system_sandbox_policy(); - if fs.has_full_disk_write_access() - || fs.entries.iter().any(|entry| match &entry.path { - FileSystemPath::Path { .. } | FileSystemPath::GlobPattern { .. } => false, - FileSystemPath::Special { value } => { - !matches!(value, FileSystemSpecialPath::Unknown { .. }) - } - }) - { - return Err(PolicyError::SymbolicRoots); - } - let roots = fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd); + let policy = permissions.file_system_sandbox_policy(); + let full_disk_write = policy.has_full_disk_write_access(); + let volumes = volume_roots + .iter() + .map(PathUri::from_host_native_path) + .collect::>>()?; + let fs = materialize_volume_roots(policy.clone(), &volumes)?; + let roots = if full_disk_write { + Vec::new() + } else { + fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd) + }; let mut write = collect_paths(roots.iter().map(|root| root.root.to_path_buf()))?; + if full_disk_write { + write.extend(collect_paths(volume_roots.iter().cloned())?); + // Explicit grants may name unmapped shares outside the enumerated volumes. + for entry in &policy.entries { + if entry.access.can_write() + && let FileSystemPath::Path { path } = &entry.path + { + write.insert(path.clone(), path.to_abs_path()?.into_path_buf()); + } + } + } let mut read = collect_paths( fs.get_readable_roots_with_cwd(cwd) .into_iter() @@ -102,8 +114,10 @@ pub fn build_request( )?; let absolute_cwd = AbsolutePathBuf::from_absolute_path(cwd) .map_err(|error| PolicyError::PolicyResolution(error.to_string()))?; + // A symbolic root deny is the default, not a recursive native mask that + // should erase narrower grants. Only materialize grants and carveouts. let deny = collect_paths( - resolve_windows_deny_read_paths(&fs, &absolute_cwd) + resolve_windows_deny_read_paths(&policy, &absolute_cwd) .map_err(PolicyError::PolicyResolution)? .into_iter() .map(AbsolutePathBuf::into_path_buf), @@ -236,6 +250,40 @@ 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. +pub(super) fn materialize_volume_roots( + mut fs: FileSystemSandboxPolicy, + volume_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 + })), + FileSystemPath::Special { + value: + FileSystemSpecialPath::Minimal + | FileSystemSpecialPath::ProjectRoots { .. } + | FileSystemSpecialPath::Tmpdir + | FileSystemSpecialPath::SlashTmp, + } => return Err(PolicyError::UnsupportedSymbolicPath), + FileSystemPath::Path { .. } + | FileSystemPath::GlobPattern { .. } + | FileSystemPath::Special { + value: FileSystemSpecialPath::Unknown { .. }, + } => entries.push(entry), + } + } + fs.entries = entries; + Ok(fs) +} + // MXC volume-root grants are nonrecursive, so snapshot each root's current // immediate children as recursive grants. A child created directly under the // root after policy construction is not granted. MXC/PSEC still enforces diff --git a/codex-rs/mxc-sandbox/src/policy_tests.rs b/codex-rs/mxc-sandbox/src/policy_tests.rs index 49d8093295..5df6230e59 100644 --- a/codex-rs/mxc-sandbox/src/policy_tests.rs +++ b/codex-rs/mxc-sandbox/src/policy_tests.rs @@ -5,17 +5,72 @@ use std::path::PathBuf; use anyhow::Result; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemSandboxPolicyContext; use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxEntry; use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::FileSystemSpecialPath; use codex_protocol::protocol::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use crate::MxcCommand; +use crate::policy::PolicyError; use crate::policy::build_request; +use crate::policy::materialize_volume_roots; + +#[test] +fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> { + use FileSystemAccessMode::Deny; + use FileSystemAccessMode::Read; + use FileSystemAccessMode::Write; + + let volumes = [ + PathUri::parse("file:///C:/")?, + PathUri::parse("file:///D:/")?, + ]; + let readonly = PathUri::parse("file:///D:/foobar")?; + let writable = PathUri::parse("file:///D:/writable")?; + let mut fs = FileSystemSandboxPolicy::read_only(); + fs.entries.extend([ + FileSystemSandboxEntry::new(volumes[1].clone().into(), Read), + FileSystemSandboxEntry::new(readonly.clone().into(), Read), + FileSystemSandboxEntry::new(writable.clone().into(), Read), + FileSystemSandboxEntry::new(writable.clone().into(), Write), + ]); + for access in [Read, Write, Deny] { + fs.entries[0].access = access; + let resolved = materialize_volume_roots(fs.clone(), &volumes)?; + for volume in &volumes { + let cwd = volume.join("work")?; + let context = FileSystemSandboxPolicyContext { + cwd: &cwd, + workspace_roots: std::slice::from_ref(&cwd), + user_home_dir: None, + temporary_directories: None, + }; + assert_eq!( + [&volumes[0], &volumes[1], &readonly, &writable] + .map(|path| resolved.resolve_access(path, &context)), + [access, access, Read, Write] + ); + } + } + fs.entries[0].access = Write; + fs.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + Read, + )); + assert!(matches!( + materialize_volume_roots(fs, &volumes), + Err(PolicyError::UnsupportedSymbolicPath) + )); + Ok(()) +} fn canonical_root(temp: &tempfile::TempDir) -> Result { Ok( @@ -230,6 +285,57 @@ fn volume_expansion_skips_uninspectable_children() -> Result<()> { Ok(()) } +#[test] +fn full_access_enumerates_children_of_every_volume() -> Result<()> { + let first = tempfile::tempdir()?; + let second = tempfile::tempdir()?; + let unmapped = tempfile::tempdir()?; + let explicit = canonical_root(&unmapped)?; + let volumes = [canonical_root(&first)?, canonical_root(&second)?]; + std::fs::create_dir(volumes[0].join("one"))?; + std::fs::create_dir(volumes[1].join("two"))?; + let mut expected = [ + volumes[0].clone(), + volumes[0].join("one"), + volumes[1].clone(), + volumes[1].join("two"), + explicit.clone(), + ]; + expected.sort(); + let mut fs = FileSystemSandboxPolicy::read_only(); + // Like an unmapped UNC share, this grant is outside the supplied volumes. + fs.entries + .push(entry(&explicit, FileSystemAccessMode::Read)?); + for access in [FileSystemAccessMode::Read, FileSystemAccessMode::Write] { + fs.entries[0].access = access; + fs.entries[1].access = access; + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &volumes[0]), + &volumes[0], + Vec::new(), + &volumes, + &[], + )?; + let native = request.policy; + let (granted, other) = if access == FileSystemAccessMode::Write { + (native.readwrite_paths, native.readonly_paths) + } else { + (native.readonly_paths, native.readwrite_paths) + }; + assert_eq!( + granted, + expected + .iter() + .map(|path| path.to_str().unwrap()) + .collect::>() + ); + assert_eq!(other, Vec::::new()); + } + Ok(()) +} + #[test] fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> { let temp = tempfile::tempdir()?; @@ -264,6 +370,174 @@ fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> { Ok(()) } +#[test] +fn root_deny_keeps_only_narrow_explicit_grants() -> Result<()> { + let temp = tempfile::tempdir()?; + let canonical = canonical_root(&temp)?; + let root = canonical.as_path(); + let child = root.join("allowed"); + for access in [FileSystemAccessMode::Read, FileSystemAccessMode::Write] { + let fs = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access, + ), + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Deny, + ), + entry(&child, access)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, root), + root, + Vec::new(), + &[root.to_owned()], + &[], + )?; + let allowed = vec![child.to_str().unwrap().to_owned()]; + let expected = match access { + FileSystemAccessMode::Read => (Vec::new(), allowed), + FileSystemAccessMode::Write => ( + allowed, + [".agents", ".codex", ".git"] + .map(|name| child.join(name).to_str().unwrap().to_owned()) + .to_vec(), + ), + FileSystemAccessMode::Deny => unreachable!(), + }; + assert_eq!( + ( + request.policy.readwrite_paths, + request.policy.readonly_paths + ), + expected + ); + assert_eq!(request.policy.denied_paths, Vec::::new()); + } + Ok(()) +} + +#[test] +fn symbolic_root_preserves_equal_path_precedence_and_denies() -> Result<()> { + let temp = tempfile::tempdir()?; + let base = canonical_root(&temp)?; + let volumes = [ + base.join("first"), + base.join("readonly"), + base.join("denied"), + ]; + for volume in &volumes { + std::fs::create_dir_all(volume.join("child"))?; + } + let fs = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Write, + ), + entry(&volumes[1], FileSystemAccessMode::Read)?, + entry(&volumes[2], FileSystemAccessMode::Deny)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &volumes[0]), + &volumes[0], + Vec::new(), + &volumes, + &[], + )?; + assert_eq!( + request.policy.readwrite_paths, + vec![ + volumes[0].to_str().unwrap().to_owned(), + volumes[0].join("child").to_str().unwrap().to_owned(), + volumes[1].to_str().unwrap().to_owned(), + volumes[1].join("child").to_str().unwrap().to_owned(), + ] + ); + let reads_in_volumes: Vec<_> = request + .policy + .readonly_paths + .into_iter() + .filter(|path| Path::new(path).starts_with(&base)) + .collect(); + assert_eq!( + reads_in_volumes, + volumes[..2] + .iter() + .flat_map(|volume| [".agents", ".codex", ".git"].map(|name| volume + .join(name) + .to_str() + .unwrap() + .to_owned())) + .collect::>() + ); + assert_eq!( + request.policy.denied_paths, + vec![volumes[2].to_str().unwrap()] + ); + Ok(()) +} + +#[test] +fn narrowing_one_volume_keeps_the_other_volume_root_grant() -> Result<()> { + let temp = tempfile::tempdir()?; + let base = canonical_root(&temp)?; + let volumes = [base.join("first"), base.join("second")]; + for volume in &volumes { + std::fs::create_dir_all(volume.join("child"))?; + } + for (root_access, narrowed) in [ + (FileSystemAccessMode::Read, FileSystemAccessMode::Deny), + (FileSystemAccessMode::Write, FileSystemAccessMode::Read), + ] { + let fs = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + root_access, + ), + entry(&volumes[0].join("child"), narrowed)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &volumes[0]), + &volumes[0], + Vec::new(), + &volumes, + &[], + )?; + let granted = if root_access == FileSystemAccessMode::Write { + request.policy.readwrite_paths + } else { + request.policy.readonly_paths + }; + let second_grants: Vec<_> = granted + .into_iter() + .filter(|path| Path::new(path).starts_with(&volumes[1])) + .collect(); + assert_eq!( + second_grants, + vec![ + volumes[1].to_str().unwrap().to_owned(), + volumes[1].join("child").to_str().unwrap().to_owned(), + ] + ); + } + Ok(()) +} + #[test] fn relative_working_directories_fail_before_launch() -> Result<()> { let root = tempfile::tempdir()?;