mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Expand MXC volume grants and resolve deny globs (#44289)
## Why MXC volume-root grants are nonrecursive, and filesystem deny globs were rejected before launch. ## What changed - Expand volume-root grants to include existing immediate children, preserving read-only carveouts and denies. Children created directly under a volume root after policy construction are not granted access. - Normalize filesystem object aliases before and after expansion so generated grants preserve tighter access restrictions. - Skip unavailable volumes and uninspectable generated children. - Resolve deny globs into concrete file and directory paths using the Windows sandbox resolver. ## Testing Add regression tests for read-only children and aliases, normalized root access, uninspectable children, and deny glob expansion alongside explicit denies. GitOrigin-RevId: 3242e9a30d16581d6fe5b7b6e39e0e0f71ca6ef4
This commit is contained in:
Generated
+2
@@ -4065,11 +4065,13 @@ dependencies = [
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"codex-windows-sandbox",
|
||||
"learning_mode_windows",
|
||||
"pretty_assertions",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
"tracelogging",
|
||||
"windows-sys 0.61.2",
|
||||
"wxc_common",
|
||||
]
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ anyhow = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
codex-windows-sandbox = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
wxc_common = { workspace = true }
|
||||
|
||||
@@ -27,6 +28,13 @@ appcontainer_common = { workspace = true }
|
||||
learning_mode_windows = { workspace = true }
|
||||
# 1.2.4 imports OneCore_apiset, which the GNU Windows toolchain does not ship.
|
||||
tracelogging = "=1.2.3"
|
||||
windows-sys = { version = "0.61", features = [
|
||||
"Win32_Foundation",
|
||||
"Win32_Storage_FileSystem",
|
||||
"Win32_System_Com",
|
||||
"Win32_System_SystemInformation",
|
||||
"Win32_UI_Shell",
|
||||
] }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -2,16 +2,24 @@
|
||||
//! access. Denies and read-only carveouts remain separate kernel policy lists.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::protocol::FileSystemPath;
|
||||
use codex_protocol::protocol::FileSystemSpecialPath;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_windows_sandbox::resolve_windows_deny_read_paths;
|
||||
use thiserror::Error;
|
||||
use wxc_common::cmdline::CommandLineContext;
|
||||
use wxc_common::cmdline::CommandLineError;
|
||||
use wxc_common::cmdline::cmdline_from_argv_for_context;
|
||||
use wxc_common::filesystem_object::ExistingObjectComparison;
|
||||
use wxc_common::filesystem_object::compare_existing_filesystem_objects;
|
||||
use wxc_common::filesystem_object::normalize_object_conflicts;
|
||||
use wxc_common::logger::Logger;
|
||||
use wxc_common::logger::Mode;
|
||||
use wxc_common::models::ContainerPolicy;
|
||||
use wxc_common::models::ExecutionRequest;
|
||||
use wxc_common::models::FallbackPolicy;
|
||||
@@ -37,12 +45,18 @@ pub enum PolicyError {
|
||||
RelativeCommandCwd,
|
||||
#[error("MXC symbolic filesystem roots are not implemented")]
|
||||
SymbolicRoots,
|
||||
#[error("MXC does not support filesystem deny glob patterns")]
|
||||
DenyGlobs,
|
||||
#[error("MXC requires a Unicode command working directory")]
|
||||
NonUnicodeCommandCwd,
|
||||
#[error("MXC requires Unicode filesystem policy paths")]
|
||||
NonUnicodePolicyPath,
|
||||
#[error("{0}")]
|
||||
PolicyResolution(String),
|
||||
#[error("enumerate MXC volume {path}: {source}")]
|
||||
EnumerateVolume {
|
||||
path: PathBuf,
|
||||
#[source]
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error(transparent)]
|
||||
Path(#[from] std::io::Error),
|
||||
#[error(transparent)]
|
||||
@@ -53,6 +67,8 @@ pub fn build_request(
|
||||
command: &MxcCommand,
|
||||
command_cwd: &Path,
|
||||
env: Vec<String>,
|
||||
volume_roots: &[PathBuf],
|
||||
_platform_read_roots: &[PathBuf],
|
||||
) -> Result<ExecutionRequest, PolicyError> {
|
||||
if command.command.is_empty() {
|
||||
return Err(PolicyError::EmptyCommand);
|
||||
@@ -76,10 +92,6 @@ pub fn build_request(
|
||||
{
|
||||
return Err(PolicyError::SymbolicRoots);
|
||||
}
|
||||
if !fs.get_unreadable_globs_with_cwd(cwd).is_empty() {
|
||||
return Err(PolicyError::DenyGlobs);
|
||||
}
|
||||
|
||||
let roots = fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd);
|
||||
let mut write = collect_paths(roots.iter().map(|root| root.root.to_path_buf()))?;
|
||||
let mut read = collect_paths(
|
||||
@@ -88,10 +100,13 @@ pub fn build_request(
|
||||
.filter(|path| !fs.can_write_local_path_with_cwd(path.as_path(), cwd))
|
||||
.map(|path| path.to_path_buf()),
|
||||
)?;
|
||||
let absolute_cwd = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
.map_err(|error| PolicyError::PolicyResolution(error.to_string()))?;
|
||||
let deny = collect_paths(
|
||||
fs.get_unreadable_roots_with_cwd(cwd)
|
||||
resolve_windows_deny_read_paths(&fs, &absolute_cwd)
|
||||
.map_err(PolicyError::PolicyResolution)?
|
||||
.into_iter()
|
||||
.map(|path| path.to_path_buf()),
|
||||
.map(AbsolutePathBuf::into_path_buf),
|
||||
)?;
|
||||
let carveouts = collect_paths(roots.into_iter().flat_map(|root| {
|
||||
let protected = root
|
||||
@@ -112,7 +127,30 @@ pub fn build_request(
|
||||
.map(|(key, path)| (key.clone(), path.clone())),
|
||||
);
|
||||
write.retain(|key, _| !carveouts.contains_key(key) && !deny.contains_key(key));
|
||||
read.retain(|key, _| !deny.contains_key(key));
|
||||
read.retain(|key, _| !write.contains_key(key) && !deny.contains_key(key));
|
||||
let volume_roots = volume_roots
|
||||
.iter()
|
||||
.map(PathUri::from_host_native_path)
|
||||
.collect::<std::io::Result<HashSet<_>>>()?;
|
||||
prune_unavailable_volume_roots(&mut write, &volume_roots);
|
||||
prune_unavailable_volume_roots(&mut read, &volume_roots);
|
||||
// Normalize pre-expansion aliases so generated children inherit
|
||||
// any tightened access applied to their volume root.
|
||||
let normalized = normalize_policy(ContainerPolicy {
|
||||
readwrite_paths: unicode_paths(write)?,
|
||||
readonly_paths: unicode_paths(read)?,
|
||||
denied_paths: unicode_paths(deny)?,
|
||||
..Default::default()
|
||||
})?;
|
||||
let write = collect_paths(normalized.readwrite_paths.into_iter().map(PathBuf::from))?;
|
||||
let read = collect_paths(normalized.readonly_paths.into_iter().map(PathBuf::from))?;
|
||||
let deny = collect_paths(normalized.denied_paths.into_iter().map(PathBuf::from))?;
|
||||
// If expansion skips an unavailable writable volume, its generated read-only
|
||||
// carveouts remain below. Pruning them requires retaining their root provenance,
|
||||
// so we accept that rare normalization failure for now.
|
||||
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));
|
||||
@@ -131,7 +169,7 @@ pub fn build_request(
|
||||
default: egress_default,
|
||||
..Default::default()
|
||||
};
|
||||
Ok(ExecutionRequest {
|
||||
let mut request = ExecutionRequest {
|
||||
script_code: cmdline_from_argv_for_context(
|
||||
&command.command,
|
||||
CommandLineContext::WindowsCreateProcess,
|
||||
@@ -169,7 +207,77 @@ pub fn build_request(
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
})
|
||||
};
|
||||
// Reconcile aliases introduced by expansion. MXC/PSEC separately enforces
|
||||
// restrictions on junction-resolved targets at access time.
|
||||
request.policy = normalize_policy(request.policy)?;
|
||||
Ok(request)
|
||||
}
|
||||
|
||||
fn normalize_policy(mut policy: ContainerPolicy) -> Result<ContainerPolicy, PolicyError> {
|
||||
let mut logger = Logger::new(Mode::Buffer);
|
||||
if let Some(normalized) =
|
||||
normalize_object_conflicts(&policy, &mut logger).map_err(PolicyError::PolicyResolution)?
|
||||
{
|
||||
policy = normalized;
|
||||
}
|
||||
Ok(policy)
|
||||
}
|
||||
|
||||
fn prune_unavailable_volume_roots(paths: &mut NativePaths, volume_roots: &HashSet<PathUri>) {
|
||||
paths.retain(|key, path| {
|
||||
if !volume_roots.contains(key) && path.parent().is_some() {
|
||||
return true;
|
||||
}
|
||||
match std::fs::read_dir(path) {
|
||||
Ok(_) => true,
|
||||
Err(error) => !inaccessible_volume(&error),
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// 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
|
||||
// restrictions on junction-resolved targets at access time.
|
||||
fn expand_volume_roots(
|
||||
paths: NativePaths,
|
||||
volume_roots: &HashSet<PathUri>,
|
||||
) -> Result<NativePaths, PolicyError> {
|
||||
let mut expanded = NativePaths::new();
|
||||
for (key, path) in paths {
|
||||
if volume_roots.contains(&key) || path.parent().is_none() {
|
||||
match std::fs::read_dir(&path).and_then(Iterator::collect::<std::io::Result<Vec<_>>>) {
|
||||
Ok(entries) => {
|
||||
for entry in entries {
|
||||
let child = entry.path();
|
||||
if child.to_str().is_none() {
|
||||
continue;
|
||||
}
|
||||
// A self-comparison is MXC's public object-identity probe.
|
||||
// Generated grants can be skipped; explicit paths fail closed below.
|
||||
if compare_existing_filesystem_objects(&child, &child)
|
||||
!= ExistingObjectComparison::Same
|
||||
{
|
||||
continue;
|
||||
}
|
||||
expanded.insert(PathUri::from_host_native_path(&child)?, child);
|
||||
}
|
||||
}
|
||||
// GetLogicalDrives includes disconnected and empty removable
|
||||
// drives. They must not prevent commands on available volumes.
|
||||
Err(error) if inaccessible_volume(&error) => continue,
|
||||
Err(error) => {
|
||||
return Err(PolicyError::EnumerateVolume {
|
||||
path,
|
||||
source: error,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
expanded.insert(key, path);
|
||||
}
|
||||
Ok(expanded)
|
||||
}
|
||||
|
||||
fn collect_paths(paths: impl IntoIterator<Item = PathBuf>) -> Result<NativePaths, PolicyError> {
|
||||
@@ -179,6 +287,33 @@ fn collect_paths(paths: impl IntoIterator<Item = PathBuf>) -> Result<NativePaths
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn inaccessible_volume(error: &std::io::Error) -> bool {
|
||||
if matches!(
|
||||
error.kind(),
|
||||
std::io::ErrorKind::PermissionDenied
|
||||
| std::io::ErrorKind::NotFound
|
||||
| std::io::ErrorKind::NetworkUnreachable
|
||||
| std::io::ErrorKind::HostUnreachable
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use windows_sys::Win32::Foundation::ERROR_BAD_NETPATH;
|
||||
use windows_sys::Win32::Foundation::ERROR_CONNECTION_UNAVAIL;
|
||||
use windows_sys::Win32::Foundation::ERROR_DEVICE_NOT_CONNECTED;
|
||||
use windows_sys::Win32::Foundation::ERROR_NOT_READY;
|
||||
matches!(
|
||||
error.raw_os_error().map(|code| code as u32),
|
||||
Some(ERROR_NOT_READY | ERROR_DEVICE_NOT_CONNECTED | ERROR_BAD_NETPATH)
|
||||
) || error.raw_os_error() == Some(ERROR_CONNECTION_UNAVAIL as i32)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
fn unicode_paths(paths: NativePaths) -> Result<Vec<String>, PolicyError> {
|
||||
let mut paths = paths
|
||||
.into_values()
|
||||
|
||||
@@ -61,7 +61,7 @@ fn native_grants_preserve_denies_and_read_only_carveouts() -> Result<()> {
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(&command(&profile, root), root, Vec::new())?;
|
||||
let request = build_request(&command(&profile, root), root, Vec::new(), &[], &[])?;
|
||||
let mut expected_read = [
|
||||
root.join(".agents"),
|
||||
root.join(".codex"),
|
||||
@@ -95,21 +95,171 @@ fn native_grants_preserve_denies_and_read_only_carveouts() -> Result<()> {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_deny_globs_fail_before_launch() -> Result<()> {
|
||||
let root = tempfile::tempdir()?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.secret".to_owned(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
)]);
|
||||
fn volume_expansion_does_not_turn_read_only_child_writable() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let canonical = canonical_root(&temp)?;
|
||||
let root = canonical.as_path();
|
||||
let readonly = root.join("protected");
|
||||
let writable = root.join("work");
|
||||
std::fs::create_dir(&readonly)?;
|
||||
std::fs::create_dir(&writable)?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(root, FileSystemAccessMode::Write)?,
|
||||
entry(&readonly, FileSystemAccessMode::Read)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let error =
|
||||
build_request(&command(&profile, root.path()), root.path(), Vec::new()).unwrap_err();
|
||||
let request = build_request(
|
||||
&command(&profile, root),
|
||||
root,
|
||||
Vec::new(),
|
||||
&[root.to_owned()],
|
||||
&[],
|
||||
)?;
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"MXC does not support filesystem deny glob patterns"
|
||||
request.policy.readwrite_paths,
|
||||
vec![root.to_str().unwrap(), writable.to_str().unwrap()]
|
||||
);
|
||||
assert_eq!(
|
||||
request.policy.readonly_paths,
|
||||
vec![
|
||||
root.join(".agents").to_str().unwrap(),
|
||||
root.join(".codex").to_str().unwrap(),
|
||||
root.join(".git").to_str().unwrap(),
|
||||
readonly.to_str().unwrap()
|
||||
]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn volume_expansion_does_not_turn_read_only_alias_writable() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = canonical_root(&temp)?;
|
||||
let readonly = root.join("protected");
|
||||
let alias = root.join("alias");
|
||||
std::fs::write(&readonly, "protected")?;
|
||||
std::fs::hard_link(&readonly, &alias)?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&root, FileSystemAccessMode::Write)?,
|
||||
entry(&readonly, FileSystemAccessMode::Read)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(
|
||||
&command(&profile, &root),
|
||||
&root,
|
||||
Vec::new(),
|
||||
std::slice::from_ref(&root),
|
||||
&[],
|
||||
)?;
|
||||
let alias = alias.display().to_string();
|
||||
assert_eq!(
|
||||
(
|
||||
request.policy.readwrite_paths.contains(&alias),
|
||||
request.policy.readonly_paths.contains(&alias),
|
||||
),
|
||||
(false, true)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn volume_expansion_uses_normalized_root_access() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = canonical_root(&temp)?;
|
||||
let volume = root.join("volume");
|
||||
let child = volume.join("child");
|
||||
let alias = root.join("alias");
|
||||
std::fs::create_dir_all(&child)?;
|
||||
std::os::unix::fs::symlink(&volume, &alias)?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&volume, FileSystemAccessMode::Write)?,
|
||||
entry(&alias, FileSystemAccessMode::Read)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(
|
||||
&command(&profile, &root),
|
||||
&root,
|
||||
Vec::new(),
|
||||
std::slice::from_ref(&volume),
|
||||
&[],
|
||||
)?;
|
||||
let child = child.display().to_string();
|
||||
assert_eq!(
|
||||
(
|
||||
request.policy.readwrite_paths.contains(&child),
|
||||
request.policy.readonly_paths.contains(&child),
|
||||
),
|
||||
(false, true)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn volume_expansion_skips_uninspectable_children() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = canonical_root(&temp)?;
|
||||
let uninspectable = root.join("loop");
|
||||
std::os::unix::fs::symlink(&uninspectable, &uninspectable)?;
|
||||
let denied = root.join("secret");
|
||||
std::fs::write(&denied, "secret")?;
|
||||
let fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&root, FileSystemAccessMode::Write)?,
|
||||
entry(&denied, FileSystemAccessMode::Deny)?,
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(
|
||||
&command(&profile, &root),
|
||||
&root,
|
||||
Vec::new(),
|
||||
std::slice::from_ref(&root),
|
||||
&[],
|
||||
)?;
|
||||
assert_eq!(
|
||||
(request.policy.readwrite_paths, request.policy.denied_paths),
|
||||
(
|
||||
vec![root.to_str().unwrap().to_owned()],
|
||||
vec![denied.to_str().unwrap().to_owned()],
|
||||
)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = canonical_root(&temp)?;
|
||||
let nested = root.join("nested");
|
||||
std::fs::create_dir(&nested)?;
|
||||
let file = nested.join("file.secret");
|
||||
let directory = nested.join("directory.secret");
|
||||
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 fs = FileSystemSandboxPolicy::restricted(vec![
|
||||
entry(&root, FileSystemAccessMode::Write)?,
|
||||
entry(&missing, FileSystemAccessMode::Deny)?,
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.secret".to_owned(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
),
|
||||
]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let request = build_request(&command(&profile, &root), &nested, Vec::new(), &[], &[])?;
|
||||
let mut expected = [file, directory, missing];
|
||||
expected.sort();
|
||||
assert_eq!(
|
||||
request.policy.denied_paths,
|
||||
expected.map(|path| path.to_str().unwrap().to_owned())
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -125,8 +275,14 @@ fn relative_working_directories_fail_before_launch() -> Result<()> {
|
||||
(Path::new("relative"), root.path(), "policy"),
|
||||
(root.path(), Path::new("relative"), "command"),
|
||||
] {
|
||||
let error =
|
||||
build_request(&command(&profile, policy_cwd), command_cwd, Vec::new()).unwrap_err();
|
||||
let error = build_request(
|
||||
&command(&profile, policy_cwd),
|
||||
command_cwd,
|
||||
Vec::new(),
|
||||
&[],
|
||||
&[],
|
||||
)
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
format!("MXC requires an absolute {kind} working directory")
|
||||
@@ -162,7 +318,8 @@ fn non_unicode_paths_fail_before_launch() -> Result<()> {
|
||||
)?]);
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted);
|
||||
let error = build_request(&command(&profile, &root), command_cwd, Vec::new()).unwrap_err();
|
||||
let error = build_request(&command(&profile, &root), command_cwd, Vec::new(), &[], &[])
|
||||
.unwrap_err();
|
||||
assert_eq!(error.to_string(), expected);
|
||||
}
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user