mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user