From 23934d31b15b1127c83535f84854e5009f7cd51d Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 29 Oct 2026 04:17:31 +0000 Subject: [PATCH] Skip read-only volume-root expansion in the MXC sandbox (#57985) ## Why MXC read-only volume-root grants already provide recursive access, so enumerating their children is unnecessary. ## What changed Expand only writable volume roots into grants for their immediate children. Document that read-only grants recurse, while new children created during a running command are not implicitly writable. ## Testing Update policy tests to verify that only writable roots expand their children and that narrowing one volume preserves the other volume's grants. GitOrigin-RevId: a7c7348b2f75bc7258fb1f130cfa4d9660e2faa6 --- codex-rs/mxc-sandbox/README.md | 7 +++--- codex-rs/mxc-sandbox/src/policy.rs | 4 ++-- codex-rs/mxc-sandbox/src/policy_tests.rs | 27 ++++++++++-------------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/codex-rs/mxc-sandbox/README.md b/codex-rs/mxc-sandbox/README.md index 3afb89521a..bf4cd0fd0d 100644 --- a/codex-rs/mxc-sandbox/README.md +++ b/codex-rs/mxc-sandbox/README.md @@ -92,9 +92,10 @@ to 4096 bytes; the helper removes them before native process creation. Use the proxy's additional private-network destination checks. Proxy domain rules still apply to proxied traffic; direct DNS remains denied. This default also applies to remote Windows executors and does not enable disabled networking. -- Windows volume-root grants do not recurse. The adapter grants the root and - its immediate children; directories added or newly mounted during a running - command are not implicitly granted. +- Read-only volume-root grants recurse without enumerating children. Writable + volume-root grants do not recurse, so the adapter grants the root and its + immediate children; new children added during a running command are not + implicitly writable. - Default read access excludes network shares unrelated to the policy or command cwd. Explicit read/write grants remain supported. Unrelated shares are not probed for object identity or enumerated for implicit reads. diff --git a/codex-rs/mxc-sandbox/src/policy.rs b/codex-rs/mxc-sandbox/src/policy.rs index c008d4fd59..c05f9eadb0 100644 --- a/codex-rs/mxc-sandbox/src/policy.rs +++ b/codex-rs/mxc-sandbox/src/policy.rs @@ -205,9 +205,9 @@ pub(super) fn build_request( // MXC's object normalizer: its metadata preflight rejects usable SMB grants // and inaccessible deny paths before the native API can evaluate them. // The hard-link and volume-root alias tradeoffs are intentional; see README. + // Read-only roots already grant recursive access; only expand writable roots. let mut write = expand_volume_roots(write, &volume_roots)?; write.retain(|key, _| !carveouts.contains_key(key) && !deny.contains_key(key)); - let mut read = expand_volume_roots(read, &volume_roots)?; // Resolve equal-path write overrides before reaching MXC so the native // API receives one effective access mode for each path identity. read.retain(|key, _| !write.contains_key(key) && !deny.contains_key(key)); @@ -353,7 +353,7 @@ pub(super) fn materialize_volume_roots( Ok(fs) } -// MXC volume-root grants are nonrecursive, so snapshot each root's current +// MXC writable 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 // restrictions on junction-resolved targets at access time. diff --git a/codex-rs/mxc-sandbox/src/policy_tests.rs b/codex-rs/mxc-sandbox/src/policy_tests.rs index 04e364baf0..69ff1709a2 100644 --- a/codex-rs/mxc-sandbox/src/policy_tests.rs +++ b/codex-rs/mxc-sandbox/src/policy_tests.rs @@ -364,7 +364,7 @@ fn volume_expansion_skips_broken_links_and_preserves_other_grants() -> Result<() } #[test] -fn full_access_enumerates_children_of_every_volume() -> Result<()> { +fn only_write_access_expands_volume_children() -> Result<()> { let first = tempfile::tempdir()?; let second = tempfile::tempdir()?; let unmapped = tempfile::tempdir()?; @@ -372,19 +372,16 @@ fn full_access_enumerates_children_of_every_volume() -> Result<()> { 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] { + let mut expected = vec![volumes[0].clone(), volumes[1].clone(), explicit.clone()]; + if access == FileSystemAccessMode::Write { + expected.extend([volumes[0].join("one"), volumes[1].join("two")]); + } + expected.sort(); fs.entries[0].access = access; fs.entries[1].access = access; let profile = @@ -798,13 +795,11 @@ fn narrowing_one_volume_keeps_the_other_volume_root_grant() -> Result<()> { .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(), - ] - ); + let mut expected = vec![volumes[1].to_str().unwrap().to_owned()]; + if root_access == FileSystemAccessMode::Write { + expected.push(volumes[1].join("child").to_str().unwrap().to_owned()); + } + assert_eq!(second_grants, expected); } Ok(()) }