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()?;