openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-28 18:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Avoid object-identity preflight for MXC path grants (#57353)

Browse Source 
## Why

MXC's `normalize_object_conflicts` can reject otherwise usable Windows policies when a network share does not support `FileIdInfo` or a protected deny path cannot be opened for metadata.

## What changed

Remove object-conflict normalization before and after volume-root expansion. Send explicit path grants directly to BaseContainer, which enforces path and symlink/junction alias precedence.

Document the resulting limitations:

- Hard-link names retain independent permissions, so an allowed name can access the same file through a less restrictive grant.
- Volume roots expand using their declared access without first reconciling aliases. A writable root can therefore generate more-specific writable grants that override a read-only root alias.

GitOrigin-RevId: de0b2d7b5b98eaa358a97156c02e9f7f85860335
This commit is contained in:
iceweasel-oai
2026-10-27 04:39:28 +00:00
committed by copyberry
parent 9510f36df2
commit 3f74ff28fa
3 changed files with 30 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/mxc-sandbox/README.md
+20
View File
@@ -26,6 +26,26 @@ direct non-loopback egress and general inbound network access.
Win32k calls and desktop handles remain available for PowerShell startup;
clipboard, input-injection, and desktop/system-control restrictions remain.
Filesystem conflicts in the emitted policy are enforced by BaseContainer:
equivalent path spellings and symlink/junction aliases use the strictest access
(`deny` > read-only > read-write), while more-specific paths override ancestor
grants. See the [BaseContainer path-precedence rules](https://github.com/microsoft/mxc/blob/main/docs/schema.md#overlapping-paths-for-windows-basecontainer).
Do not call MXC's shared `normalize_object_conflicts` before or after volume
expansion. Its identity preflight is used by mount-based backends and can reject
an otherwise usable Windows policy when a network share does not support
`FileIdInfo`, or a protected deny path cannot be opened for metadata. Codex sends
explicit path grants to BaseContainer without requiring that additional lookup.
We intentionally accept two limitations:
- Hard-link names retain independent path permissions. An allowed name can read
or modify the same file as a denied or read-only name; we do not tighten all
names to an object-wide restriction.
- Volume-root expansion uses each root's declared access without reconciling
aliases first. For example, a writable `V:\` plus a read-only junction to
`V:\` can produce a writable `V:\projects` grant. That generated, more-specific
grant permits writes through either name despite the read-only root alias.
## Selection and rollout
`windows.sandbox = "mxc"` is strict. The default-off `features.prefer_mxc`
codex-rs/mxc-sandbox/src/policy.rs
+10 -36
View File
@@ -1,5 +1,6 @@
//! Translate canonical permissions into native MXC grants without expanding
//! access. Denies and read-only carveouts remain separate kernel policy lists.
//! Translate canonical permissions into MXC path grants.
//! Denies and read-only carveouts remain separate kernel policy lists.
//! Alias limitations are documented in the crate README.
use std::collections::HashMap;
use std::collections::HashSet;
@@ -18,9 +19,6 @@ use mxc_sdk::mxc_common::cmdline::CommandLineError;
use mxc_sdk::mxc_common::cmdline::cmdline_from_argv_for_context;
use mxc_sdk::mxc_common::filesystem_object::ExistingObjectComparison;
use mxc_sdk::mxc_common::filesystem_object::compare_existing_filesystem_objects;
use mxc_sdk::mxc_common::filesystem_object::normalize_object_conflicts;
use mxc_sdk::mxc_common::logger::Logger;
use mxc_sdk::mxc_common::logger::Mode;
use mxc_sdk::mxc_common::models::BaseProcessUiConfig;
use mxc_sdk::mxc_common::models::ContainerPolicy;
use mxc_sdk::mxc_common::models::ExecutionRequest;
@@ -205,20 +203,10 @@ pub(super) fn build_request(
.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.
// BaseContainer owns alias precedence for the emitted paths. Do not call
// 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.
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)?;
@@ -269,7 +257,7 @@ pub(super) fn build_request(
ports: Vec::new(),
});
}
let mut request = ExecutionRequest {
Ok(ExecutionRequest {
script_code: cmdline_from_argv_for_context(
&command.command,
CommandLineContext::WindowsCreateProcess,
@@ -311,21 +299,7 @@ pub(super) 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>) {
@@ -400,7 +374,7 @@ fn expand_volume_roots(
continue;
}
// A self-comparison is MXC's public object-identity probe.
// Generated grants can be skipped; explicit paths fail closed below.
// Generated grants can be skipped; explicit paths go to BaseContainer.
if compare_existing_filesystem_objects(&child, &child)
!= ExistingObjectComparison::Same
{
codex-rs/mxc-sandbox/src/policy_tests.rs
-66
View File
@@ -322,72 +322,6 @@ fn volume_expansion_does_not_turn_read_only_child_writable() -> Result<()> {
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<()> {