Files
codex/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs
viyatb-oaiandCodex 46f30d0282 feat(sandbox): add Windows deny-read parity (#18202)
## Why

The split filesystem policy stack already supports exact and glob
`access = none` read restrictions on macOS and Linux. Windows still
needed subprocess handling for those deny-read policies without claiming
enforcement from a backend that cannot provide it.

## Key finding

The unelevated restricted-token backend cannot safely enforce deny-read
overlays. Its `WRITE_RESTRICTED` token model is authoritative for write
checks, not read denials, so this PR intentionally fails that backend
closed when deny-read overrides are present instead of claiming
unsupported enforcement.

## What changed

This PR adds the Windows deny-read enforcement layer and makes the
backend split explicit:

- Resolves Windows deny-read filesystem policy entries into concrete ACL
targets.
- Preserves exact missing paths so they can be materialized and denied
before an enforceable sandboxed process starts.
- Snapshot-expands existing glob matches into ACL targets for Windows
subprocess enforcement.
- Honors `glob_scan_max_depth` when expanding Windows deny-read globs.
- Plans both the configured lexical path and the canonical target for
existing paths so reparse-point aliases are covered.
- Threads deny-read overrides through the elevated/logon-user Windows
sandbox backend and unified exec.
- Applies elevated deny-read ACLs synchronously before command launch
rather than delegating them to the background read-grant helper.
- Reconciles persistent deny-read ACEs per sandbox principal so policy
changes do not leave stale deny-read ACLs behind.
- Fails closed on the unelevated restricted-token backend when deny-read
overrides are present, because its `WRITE_RESTRICTED` token model is not
authoritative for read denials.

## Landed prerequisites

These prerequisite PRs are already on `main`:

1. #15979 `feat(permissions): add glob deny-read policy support`
2. #18096 `feat(sandbox): add glob deny-read platform enforcement`
3. #17740 `feat(config): support managed deny-read requirements`

This PR targets `main` directly and contains only the Windows deny-read
enforcement layer.

## Implementation notes

- Exact deny-read paths remain enforceable on the elevated path even
when they do not exist yet: Windows materializes the missing path before
applying the deny ACE, so the sandboxed command cannot create and read
it during the same run.
- Existing exact deny paths are preserved lexically until the ACL
planner, which then adds the canonical target as a second ACL target
when needed. That keeps both the configured alias and the resolved
object covered.
- Windows ACLs do not consume Codex glob syntax directly, so glob
deny-read entries are expanded to the concrete matches that exist before
process launch.
- Glob traversal deduplicates directory visits within each pattern walk
to avoid cycles, without collapsing distinct lexical roots that happen
to resolve to the same target.
- Persistent deny-read ACL state is keyed by sandbox principal SID, so
cleanup only removes ACEs owned by the same backend principal.
- Deny-read ACEs are fail-closed on the elevated path: setup aborts if
mandatory deny-read ACL application fails.
- Unelevated restricted-token sessions reject deny-read overrides early
instead of running with a silently unenforceable read policy.

## Verification

- `cargo test -p codex-core
windows_restricted_token_rejects_unreadable_split_carveouts`
- `just fmt`
- `just fix -p codex-core`
- `just fix -p codex-windows-sandbox`
- GitHub Actions rerun is in progress on the pushed head.

---------

Co-authored-by: Codex <noreply@openai.com>
2026-05-11 23:04:28 -07:00

382 lines
13 KiB
Rust

use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::ReadDenyMatcher;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
struct GlobScanPlan {
root: PathBuf,
max_depth: Option<usize>,
}
/// Resolve split filesystem `None` read entries into concrete Windows ACL targets.
///
/// Windows ACLs do not understand Codex filesystem glob patterns directly. Exact
/// unreadable roots can be passed through as-is, including paths that do not
/// exist yet. Glob entries are snapshot-expanded to the files/directories that
/// already exist under their literal scan root; future exact paths are handled
/// later by materializing them before the deny ACE is applied.
pub fn resolve_windows_deny_read_paths(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &AbsolutePathBuf,
) -> Result<Vec<AbsolutePathBuf>, String> {
let mut paths = Vec::new();
let mut seen = HashSet::new();
for path in file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd.as_path()) {
push_absolute_path(&mut paths, &mut seen, path.into_path_buf())?;
}
let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd.as_path());
if unreadable_globs.is_empty() {
return Ok(paths);
}
let glob_policy = FileSystemSandboxPolicy::restricted(
unreadable_globs
.iter()
.map(|pattern| FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: pattern.clone(),
},
access: FileSystemAccessMode::None,
})
.collect(),
);
let Some(matcher) = ReadDenyMatcher::try_new(&glob_policy, cwd.as_path())? else {
return Ok(paths);
};
for pattern in unreadable_globs {
let mut seen_scan_dirs = HashSet::new();
let scan_plan = glob_scan_plan(&pattern, file_system_sandbox_policy.glob_scan_max_depth);
collect_existing_glob_matches(
&scan_plan.root,
&matcher,
&mut paths,
&mut seen,
&mut seen_scan_dirs,
scan_plan.max_depth,
/*depth*/ 0,
)?;
}
Ok(paths)
}
fn collect_existing_glob_matches(
path: &Path,
matcher: &ReadDenyMatcher,
paths: &mut Vec<AbsolutePathBuf>,
seen_paths: &mut HashSet<PathBuf>,
seen_scan_dirs: &mut HashSet<PathBuf>,
max_depth: Option<usize>,
depth: usize,
) -> Result<(), String> {
if !path.exists() {
return Ok(());
}
if matcher.is_read_denied(path) {
push_absolute_path(paths, seen_paths, path.to_path_buf())?;
}
let Ok(metadata) = path.metadata() else {
return Ok(());
};
if !metadata.is_dir() {
return Ok(());
}
// Canonical directory keys keep recursive scans from following a symlink or
// junction cycle forever while preserving the original matched path for the
// ACL layer.
let scan_key = dunce::canonicalize(path).unwrap_or_else(|_| path.to_path_buf());
if !seen_scan_dirs.insert(scan_key) {
return Ok(());
}
if max_depth.is_some_and(|max_depth| depth >= max_depth) {
return Ok(());
}
let Ok(entries) = std::fs::read_dir(path) else {
return Ok(());
};
for entry in entries.flatten() {
collect_existing_glob_matches(
&entry.path(),
matcher,
paths,
seen_paths,
seen_scan_dirs,
max_depth,
depth + 1,
)?;
}
Ok(())
}
fn push_absolute_path(
paths: &mut Vec<AbsolutePathBuf>,
seen: &mut HashSet<PathBuf>,
path: PathBuf,
) -> Result<(), String> {
let absolute_path = AbsolutePathBuf::from_absolute_path(dunce::simplified(&path))
.map_err(|err| err.to_string())?;
if seen.insert(absolute_path.to_path_buf()) {
paths.push(absolute_path);
}
Ok(())
}
fn glob_scan_plan(pattern: &str, configured_max_depth: Option<usize>) -> GlobScanPlan {
// Start scanning at the deepest literal directory prefix before the first
// glob metacharacter. For example, `C:\repo\**\*.env` only scans `C:\repo`
// instead of the current directory or drive root.
let first_glob = pattern
.char_indices()
.find(|(_, ch)| matches!(ch, '*' | '?' | '['))
.map(|(index, _)| index)
.unwrap_or(pattern.len());
let literal_prefix = &pattern[..first_glob];
let Some(separator_index) = literal_prefix.rfind(['/', '\\']) else {
return GlobScanPlan {
root: PathBuf::from("."),
max_depth: effective_glob_scan_max_depth(pattern, configured_max_depth),
};
};
let pattern_suffix = &pattern[separator_index + 1..];
let is_drive_root_separator = separator_index > 0
&& literal_prefix
.as_bytes()
.get(separator_index - 1)
.is_some_and(|ch| *ch == b':');
if separator_index == 0 || is_drive_root_separator {
return GlobScanPlan {
root: PathBuf::from(&literal_prefix[..=separator_index]),
max_depth: effective_glob_scan_max_depth(pattern_suffix, configured_max_depth),
};
}
GlobScanPlan {
root: PathBuf::from(literal_prefix[..separator_index].to_string()),
max_depth: effective_glob_scan_max_depth(pattern_suffix, configured_max_depth),
}
}
fn effective_glob_scan_max_depth(
pattern_suffix: &str,
configured_max_depth: Option<usize>,
) -> Option<usize> {
let components = pattern_suffix
.split(['/', '\\'])
.filter(|component| !component.is_empty())
.collect::<Vec<_>>();
if components.contains(&"**") {
return configured_max_depth;
}
Some(configured_max_depth.map_or(components.len(), |max_depth| {
max_depth.min(components.len())
}))
}
#[cfg(test)]
mod tests {
use super::glob_scan_plan;
use super::resolve_windows_deny_read_paths;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::path::PathBuf;
use tempfile::TempDir;
#[cfg(unix)]
use std::os::unix::fs::symlink;
fn unreadable_glob_entry(pattern: String) -> FileSystemSandboxEntry {
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern { pattern },
access: FileSystemAccessMode::None,
}
}
fn unreadable_path_entry(path: PathBuf) -> FileSystemSandboxEntry {
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::from_absolute_path(path).expect("absolute path"),
},
access: FileSystemAccessMode::None,
}
}
#[test]
fn scan_root_uses_literal_prefix_before_glob() {
assert_eq!(
glob_scan_plan("/tmp/work/**/*.env", /*configured_max_depth*/ None).root,
PathBuf::from("/tmp/work")
);
assert_eq!(
glob_scan_plan(
r"C:\Users\dev\repo\**\*.env",
/*configured_max_depth*/ None,
)
.root,
PathBuf::from(r"C:\Users\dev\repo")
);
assert_eq!(
glob_scan_plan(r"C:\*.env", /*configured_max_depth*/ None).root,
PathBuf::from(r"C:\")
);
}
#[test]
fn scan_depth_is_bounded_for_non_recursive_globs() {
assert_eq!(
glob_scan_plan("/tmp/work/*.env", /*configured_max_depth*/ None).max_depth,
Some(1)
);
assert_eq!(
glob_scan_plan("/tmp/work/*/*.env", /*configured_max_depth*/ None).max_depth,
Some(2)
);
assert_eq!(
glob_scan_plan("/tmp/work/**/*.env", /*configured_max_depth*/ None).max_depth,
None
);
}
#[test]
fn configured_depth_caps_recursive_glob_scans() {
assert_eq!(
glob_scan_plan("/tmp/work/**/*.env", Some(2)).max_depth,
Some(2)
);
assert_eq!(
glob_scan_plan("/tmp/work/*/*.env", Some(1)).max_depth,
Some(1)
);
}
#[test]
fn exact_missing_paths_are_preserved() {
let tmp = TempDir::new().expect("tempdir");
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
let missing = tmp.path().join("missing.env");
let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_path_entry(missing)]);
assert_eq!(
resolve_windows_deny_read_paths(&policy, &cwd).expect("resolve"),
vec![
AbsolutePathBuf::from_absolute_path(
dunce::canonicalize(tmp.path())
.expect("canonical tempdir")
.join("missing.env")
)
.expect("absolute missing")
]
);
}
#[test]
fn glob_patterns_expand_to_existing_matches() {
let tmp = TempDir::new().expect("tempdir");
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
let root_env = tmp.path().join(".env");
let nested_env = tmp.path().join("app").join(".env");
let notes = tmp.path().join("app").join("notes.txt");
std::fs::create_dir_all(notes.parent().expect("parent")).expect("create parent");
std::fs::write(&root_env, "secret").expect("write root env");
std::fs::write(&nested_env, "secret").expect("write nested env");
std::fs::write(&notes, "notes").expect("write notes");
let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry(format!(
"{}/**/*.env",
tmp.path().display()
))]);
let actual: HashSet<PathBuf> = resolve_windows_deny_read_paths(&policy, &cwd)
.expect("resolve")
.into_iter()
.map(AbsolutePathBuf::into_path_buf)
.collect();
let expected = [root_env, nested_env].into_iter().collect();
assert_eq!(actual, expected);
}
#[test]
fn invalid_glob_patterns_fail_before_expansion() {
let tmp = TempDir::new().expect("tempdir");
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry(format!(
"{}/**/[z-a]",
tmp.path().display()
))]);
let err = resolve_windows_deny_read_paths(&policy, &cwd).expect_err("invalid glob");
assert!(
err.contains("invalid deny-read glob pattern"),
"unexpected error: {err}"
);
assert!(err.contains("invalid range"), "unexpected error: {err}");
}
#[test]
fn non_recursive_globs_do_not_expand_nested_matches() {
let tmp = TempDir::new().expect("tempdir");
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
let root_env = tmp.path().join(".env");
let nested_env = tmp.path().join("app").join(".env");
std::fs::create_dir_all(nested_env.parent().expect("parent")).expect("create parent");
std::fs::write(&root_env, "secret").expect("write root env");
std::fs::write(&nested_env, "secret").expect("write nested env");
let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry(format!(
"{}/*.env",
tmp.path().display()
))]);
assert_eq!(
resolve_windows_deny_read_paths(&policy, &cwd).expect("resolve"),
vec![AbsolutePathBuf::from_absolute_path(root_env).expect("absolute root env")]
);
}
#[cfg(unix)]
#[test]
fn aliased_glob_roots_each_preserve_their_lexical_matches() {
let tmp = TempDir::new().expect("tempdir");
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("absolute cwd");
let target = tmp.path().join("target");
let alias_a = tmp.path().join("alias-a");
let alias_b = tmp.path().join("alias-b");
let secret = target.join("secret.env");
std::fs::create_dir_all(&target).expect("create target");
std::fs::write(&secret, "secret").expect("write secret");
symlink(&target, &alias_a).expect("create alias a");
symlink(&target, &alias_b).expect("create alias b");
let policy = FileSystemSandboxPolicy::restricted(vec![
unreadable_glob_entry(format!("{}/**/*.env", alias_a.display())),
unreadable_glob_entry(format!("{}/**/*.env", alias_b.display())),
]);
let actual: HashSet<PathBuf> = resolve_windows_deny_read_paths(&policy, &cwd)
.expect("resolve")
.into_iter()
.map(AbsolutePathBuf::into_path_buf)
.collect();
let expected = [alias_a.join("secret.env"), alias_b.join("secret.env")]
.into_iter()
.collect();
assert_eq!(actual, expected);
}
}