mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Create missing parent directories for MXC deny paths on Windows (#58168)
## Why MXC requires deny paths to have existing parent directories, even when the denied path itself does not exist. ## What changed Create missing parent directories before expanding deny globs and volume grants on Windows. If creation fails, return a policy resolution error with guidance to create the directory manually or correct the path or permissions. ## Testing Extend policy tests to cover missing deny parents, preserved read-only carveouts during volume expansion, and parent denies expressed as explicit paths or glob patterns. Verify that parent directories are created without creating the denied child. GitOrigin-RevId: d852444bef51e5d5449c4228a31d01c0e6f982dc
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
//! Translate canonical permissions into MXC path grants.
|
||||
//! Denies and read-only carveouts remain separate kernel policy lists.
|
||||
//! Missing deny parents are created before deny globs and volume grants are expanded.
|
||||
//! Alias limitations are documented in the crate README.
|
||||
|
||||
use std::collections::HashMap;
|
||||
@@ -129,6 +130,19 @@ pub(super) fn build_request(
|
||||
return Err(PolicyError::RelativeCommandCwd);
|
||||
}
|
||||
let policy = materialize_temporary_paths(permissions.file_system_sandbox_policy(), &env)?;
|
||||
#[cfg(windows)]
|
||||
for denied in policy.get_unreadable_roots_with_cwd(cwd) {
|
||||
if let Some(parent) = denied.as_path().parent()
|
||||
&& matches!(parent.try_exists(), Ok(false))
|
||||
{
|
||||
std::fs::create_dir_all(parent).map_err(|error| {
|
||||
PolicyError::PolicyResolution(format!(
|
||||
"MXC currently requires deny paths to have existing parent directories. Could not create {}: {error}. Create this directory manually or correct the path/permissions, then retry.",
|
||||
parent.display()
|
||||
))
|
||||
})?;
|
||||
}
|
||||
}
|
||||
let full_disk_write = policy.has_full_disk_write_access();
|
||||
let volumes = volume_roots
|
||||
.iter()
|
||||
|
||||
@@ -290,11 +290,14 @@ fn volume_expansion_does_not_turn_read_only_child_writable() -> Result<()> {
|
||||
let root = canonical.as_path();
|
||||
let readonly = root.join("protected");
|
||||
let writable = root.join("work");
|
||||
let missing = root.join("new");
|
||||
let denied = missing.join("parent").join("secret");
|
||||
std::fs::create_dir(&readonly)?;
|
||||
std::fs::create_dir(&writable)?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(root, FileSystemAccessMode::Write)?,
|
||||
entry(&readonly, FileSystemAccessMode::Read)?,
|
||||
entry(&denied, FileSystemAccessMode::Deny)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
@@ -305,10 +308,14 @@ fn volume_expansion_does_not_turn_read_only_child_writable() -> Result<()> {
|
||||
&[root.to_owned()],
|
||||
&[],
|
||||
)?;
|
||||
assert_eq!(
|
||||
request.policy.readwrite_paths,
|
||||
vec![root.to_str().unwrap(), writable.to_str().unwrap()]
|
||||
);
|
||||
let mut expected_write_paths = vec![root.to_str().unwrap(), writable.to_str().unwrap()];
|
||||
if cfg!(windows) {
|
||||
expected_write_paths.insert(1, missing.to_str().unwrap());
|
||||
assert!(denied.parent().unwrap().is_dir());
|
||||
assert!(!denied.exists());
|
||||
}
|
||||
assert_eq!(request.policy.readwrite_paths, expected_write_paths);
|
||||
assert_eq!(request.policy.denied_paths, vec![denied.to_str().unwrap()]);
|
||||
assert_eq!(
|
||||
request.policy.readonly_paths,
|
||||
vec![
|
||||
@@ -508,7 +515,7 @@ fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> {
|
||||
std::fs::write(&file, "secret")?;
|
||||
std::fs::create_dir(&directory)?;
|
||||
std::fs::write(nested.join("allowed.txt"), "allowed")?;
|
||||
let missing = root.join("explicit.secret");
|
||||
let missing = root.join("missing").join("parent").join("explicit.secret");
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&root, FileSystemAccessMode::Write)?,
|
||||
entry(&missing, FileSystemAccessMode::Deny)?,
|
||||
@@ -522,6 +529,12 @@ fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> {
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(&command(&profile, &root), &nested, Vec::new(), &[], &[])?;
|
||||
#[cfg(windows)]
|
||||
{
|
||||
assert!(missing.parent().unwrap().is_dir());
|
||||
assert!(!missing.exists());
|
||||
}
|
||||
assert_eq!(request.policy.readwrite_paths, vec![root.to_str().unwrap()]);
|
||||
let mut expected = [file, directory, missing];
|
||||
expected.sort();
|
||||
assert_eq!(
|
||||
@@ -531,6 +544,43 @@ fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn denied_parents_are_created_before_glob_expansion() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = canonical_root(&temp)?;
|
||||
let missing = root.join("missing");
|
||||
for parent_deny in [
|
||||
entry(&missing, FileSystemAccessMode::Deny)?,
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/missing".to_owned(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
),
|
||||
] {
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&root, FileSystemAccessMode::Write)?,
|
||||
parent_deny,
|
||||
entry(&missing.join("child"), FileSystemAccessMode::Deny)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(&command(&profile, &root), &root, Vec::new(), &[], &[])?;
|
||||
assert!(missing.is_dir());
|
||||
assert!(!missing.join("child").exists());
|
||||
assert_eq!(
|
||||
request.policy.denied_paths,
|
||||
vec![
|
||||
missing.to_str().unwrap(),
|
||||
missing.join("child").to_str().unwrap()
|
||||
]
|
||||
);
|
||||
std::fs::remove_dir(&missing)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_deny_keeps_only_narrow_explicit_grants() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user