From 61506432d60f83485fb7440633e1d3d194f1bf71 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Tue, 3 Nov 2026 21:55:40 +0000 Subject: [PATCH] Expand Windows deny-read globs without invoking ripgrep (#59250) ## What changed Use the `ignore` crate to snapshot matching files and directories in-process, replacing the external `rg` scan and fallback walker. Remove scanner discovery from MXC sandbox dependency checks. Keep case-insensitive matching and configured scan depth limits. Skip directory links below the scan root while following explicitly linked roots, and continue past missing or inaccessible entries. ## Testing Update resolver tests to cover child directory links, linked scan roots, dangling links, and literal brackets in glob patterns. GitOrigin-RevId: e7f02d4db1fd42958adc212bb54055710fa2e536 --- codex-rs/Cargo.lock | 2 +- codex-rs/exec-server/Cargo.toml | 1 - .../src/sandbox_integrity/AGENTS.md | 5 - .../exec-server/src/sandbox_integrity/mxc.rs | 25 +- codex-rs/mxc-sandbox/src/lib.rs | 12 +- codex-rs/windows-sandbox-rs/Cargo.toml | 1 + .../src/deny_read_resolver.rs | 256 ++++++------------ .../src/deny_read_resolver_access_tests.rs | 74 ++--- .../src/deny_read_walker.rs | 151 ----------- .../src/deny_read_walker_tests.rs | 17 -- codex-rs/windows-sandbox-rs/src/lib.rs | 2 - 11 files changed, 111 insertions(+), 435 deletions(-) delete mode 100644 codex-rs/windows-sandbox-rs/src/deny_read_walker.rs delete mode 100644 codex-rs/windows-sandbox-rs/src/deny_read_walker_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4e0b6e98a4..02632ab011 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3326,7 +3326,6 @@ dependencies = [ "tracing-subscriber", "url", "uuid", - "which 8.0.0", "windows-sys 0.61.2", "wiremock", ] @@ -5636,6 +5635,7 @@ dependencies = [ "dirs-next", "dunce", "glob", + "ignore", "pretty_assertions", "rand 0.8.6", "serde", diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 6f9a1e7af8..aee56d1d75 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -85,7 +85,6 @@ tempfile = { workspace = true } [target.'cfg(windows)'.dependencies] codex-mxc-sandbox = { workspace = true } -which = { workspace = true } windows-sys = { workspace = true, features = [ "Win32_Foundation", "Win32_Security", diff --git a/codex-rs/exec-server/src/sandbox_integrity/AGENTS.md b/codex-rs/exec-server/src/sandbox_integrity/AGENTS.md index adf9af66e2..07d4bbe390 100644 --- a/codex-rs/exec-server/src/sandbox_integrity/AGENTS.md +++ b/codex-rs/exec-server/src/sandbox_integrity/AGENTS.md @@ -30,11 +30,6 @@ negatives. Backend glob/symlink nuances and writable hard-link aliases are not fully modeled. The separate glob scan can differ from enforcement's snapshot. Metrics describe observed checker outcomes, not accuracy or missed violations. -MXC scanner discovery currently uses `which`, while enforcement uses -`Command`. Their search-order and environment differences can select different -executables or cause only one scan to use the internal walker. This is an accepted -telemetry limitation for this phase; changing production scanner selection is deferred. - These are accepted limitations of this phase, not requests to duplicate backend implementations. Review against this scope. Regressions in explicit policy handling, executor context, or reporting remain actionable; this is not a blanket diff --git a/codex-rs/exec-server/src/sandbox_integrity/mxc.rs b/codex-rs/exec-server/src/sandbox_integrity/mxc.rs index 8d4c7eefa1..27924c2770 100644 --- a/codex-rs/exec-server/src/sandbox_integrity/mxc.rs +++ b/codex-rs/exec-server/src/sandbox_integrity/mxc.rs @@ -13,29 +13,10 @@ pub(super) const NAME: &str = "windows_mxc"; pub(super) fn critical_dependencies( request: &SandboxExecRequest, - policy: &FileSystemSandboxPolicy, - cwd: &AbsolutePathBuf, + _policy: &FileSystemSandboxPolicy, + _cwd: &AbsolutePathBuf, ) -> io::Result> { - let command_cwd = request.cwd.to_abs_path()?; - let mut dependencies = codex_and_launcher_dependencies(request); - if !policy - .get_unreadable_globs_with_cwd(cwd.as_path()) - .is_empty() - { - let path = request - .env - .iter() - .find(|(key, _)| key.eq_ignore_ascii_case("PATH")) - .map(|(_, value)| value); - if let Ok(scanner) = which::which_in( - codex_mxc_sandbox::GLOB_SCAN_PROGRAM, - path, - command_cwd.as_path(), - ) { - dependencies.push(("glob_scanner", Ok(scanner))); - } - } - Ok(dependencies) + Ok(codex_and_launcher_dependencies(request)) } pub(super) fn prepare_policy( diff --git a/codex-rs/mxc-sandbox/src/lib.rs b/codex-rs/mxc-sandbox/src/lib.rs index fde6ddf1a9..d42edba8ba 100644 --- a/codex-rs/mxc-sandbox/src/lib.rs +++ b/codex-rs/mxc-sandbox/src/lib.rs @@ -107,9 +107,6 @@ pub fn is_available() -> bool { } } -#[cfg(windows)] -pub use codex_windows_sandbox::GLOB_SCAN_PROGRAM; - /// Resolve configured symbols and snapshot deny globs for policy inspection. /// Reuses MXC's resolution without adding native grants or protections. #[cfg(windows)] @@ -126,13 +123,8 @@ pub fn prepare_file_system_policy( .map(|(key, value)| format!("{key}={value}")) .collect::>(); let mut policy = policy::materialize_temporary_paths(policy, &env_entries)?; - let denied = codex_windows_sandbox::resolve_windows_deny_read_paths_in_environment( - &policy, - policy_cwd, - env, - command_cwd, - ) - .map_err(anyhow::Error::msg)?; + let denied = codex_windows_sandbox::resolve_windows_deny_read_paths(&policy, policy_cwd) + .map_err(anyhow::Error::msg)?; policy = policy.with_expanded_deny_globs(denied); if policy.has_full_disk_write_access() { return Ok(policy); diff --git a/codex-rs/windows-sandbox-rs/Cargo.toml b/codex-rs/windows-sandbox-rs/Cargo.toml index a798fb828d..8c7858782e 100644 --- a/codex-rs/windows-sandbox-rs/Cargo.toml +++ b/codex-rs/windows-sandbox-rs/Cargo.toml @@ -40,6 +40,7 @@ codex-utils-string = { workspace = true } codex-otel = { workspace = true } dunce = "1.0" glob = { workspace = true } +ignore = { workspace = true } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "3" diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs index 340b0614bb..24e5c16884 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs @@ -6,20 +6,10 @@ use codex_protocol::permissions::ReadDenyMatcher; use codex_protocol::permissions::ResolvedFileSystemPermissions; use codex_protocol::permissions::windows_deny_read_glob_scan; use codex_utils_absolute_path::AbsolutePathBuf; -use std::collections::HashMap; +use ignore::overrides::OverrideBuilder; use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; -use std::process::Command; - -#[path = "deny_read_walker.rs"] -mod walker; - -use walker::DirectoryScanMode; -use walker::collect_existing_glob_directory_matches; - -/// External scanner used by deny-glob expansion and its dependency inventory. -pub const GLOB_SCAN_PROGRAM: &str = "rg"; #[derive(Debug, Eq, PartialEq)] struct GlobScanPlan { @@ -33,8 +23,9 @@ struct GlobScanPlan { /// 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. +/// already exist under their literal scan root. Directory links below that root +/// are not traversed; explicitly linked scan roots are followed. Future exact +/// paths are materialized later before the deny ACE is applied. pub fn resolve_windows_deny_read_paths( file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &AbsolutePathBuf, @@ -44,7 +35,6 @@ pub fn resolve_windows_deny_read_paths( &file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd), file_system_sandbox_policy.glob_scan_max_depth, cwd, - |_| {}, ) } @@ -58,26 +48,6 @@ pub fn expand_windows_deny_read_paths( &permissions.unreadable_globs, permissions.glob_scan_max_depth, cwd, - |_| {}, - ) -} - -/// Resolve deny paths with the launcher's scanner in an explicit command environment. -/// The supplied environment replaces inheritance, including when it is empty. -pub fn resolve_windows_deny_read_paths_in_environment( - file_system_sandbox_policy: &FileSystemSandboxPolicy, - cwd: &AbsolutePathBuf, - env: &HashMap, - command_cwd: &Path, -) -> Result, String> { - expand_deny_read_paths( - &file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd), - &file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd), - file_system_sandbox_policy.glob_scan_max_depth, - cwd, - |command| { - command.env_clear().envs(env).current_dir(command_cwd); - }, ) } @@ -86,7 +56,6 @@ fn expand_deny_read_paths( unreadable_globs: &[String], glob_scan_max_depth: Option, cwd: &AbsolutePathBuf, - configure: impl Fn(&mut Command), ) -> Result, String> { let mut paths = Vec::new(); let mut seen = HashSet::new(); @@ -123,105 +92,58 @@ fn expand_deny_read_paths( continue; } - let directory_scan_mode = if let Some(file_paths) = ripgrep_files(&scan_plan, &configure)? { - for path in file_paths { - if matcher.is_local_path_read_denied(&path) { - push_absolute_path(&mut paths, &mut seen, path)?; - } - } - DirectoryScanMode::DirectoriesOnly - } else { - // Rebuild the complete accessible snapshot with the policy matcher - // when ripgrep is missing or reports an incomplete traversal. Never - // apply ACLs from a failed scan's partial stdout. - DirectoryScanMode::IncludeAllFiles - }; + // Snapshot matching entries without following directory links below the scan root. + let mut filter = OverrideBuilder::new(&scan_plan.root); + filter.allow_unclosed_class(/*yes*/ true); + filter + .case_insensitive(/*yes*/ true) + .map_err(|err| err.to_string())?; + for glob in &scan_plan.globs { + filter.add(glob).map_err(|err| err.to_string())?; + } + let filter = filter.build().map_err(|err| err.to_string())?; - collect_existing_glob_directory_matches( - &scan_plan.root, - &matcher, - &mut paths, - &mut seen, - scan_plan.max_depth, - directory_scan_mode, - )?; + for entry in ignore::WalkBuilder::new(&scan_plan.root) + .standard_filters(/*yes*/ false) + .follow_links(/*yes*/ false) + .max_depth(scan_plan.max_depth) + .overrides(filter) + .build() + { + let entry = entry.and_then(|entry| { + if entry.path_is_symlink() && matcher.is_local_path_read_denied(entry.path()) { + std::fs::metadata(entry.path()).map_err(ignore::Error::Io)?; + } + Ok(entry) + }); + let entry = match entry { + Ok(entry) => entry, + Err(err) + if err.io_error().is_some_and(|err| { + matches!( + err.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied + ) + }) => + { + continue; + } + Err(err) => { + return Err(format!( + "failed to enumerate unreadable glob paths under {}: {err}", + scan_plan.root.display() + )); + } + }; + if matcher.is_local_path_read_denied(entry.path()) { + push_absolute_path(&mut paths, &mut seen, entry.into_path())?; + } + } } Ok(paths) } -fn ripgrep_files( - scan_plan: &GlobScanPlan, - configure: &impl Fn(&mut Command), -) -> Result>, String> { - let mut command = codex_utils_process::background_command(GLOB_SCAN_PROGRAM); - configure(&mut command); - command - .arg("--files") - .arg("--hidden") - .arg("--no-ignore") - .arg("--glob-case-insensitive") - .arg("--null"); - if let Some(max_depth) = scan_plan.max_depth { - command.arg("--max-depth").arg(max_depth.to_string()); - } - for glob in &scan_plan.globs { - command.arg("--glob").arg(glob); - } - command.arg("--").arg(&scan_plan.root); - - let output = match command.output() { - Ok(output) => output, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), - Err(err) => { - return Err(format!( - "failed to run bundled ripgrep for unreadable glob scan under {}: {err}", - scan_plan.root.display() - )); - } - }; - if !output.status.success() { - if output.status.code() == Some(1) && output.stderr.is_empty() { - return Ok(Some(Vec::new())); - } - if output.status.code() == Some(2) { - // Ripgrep uses exit 2 for traversal errors, including protected - // Windows directories. Its output may be incomplete. The caller - // must enumerate again using the matcher-backed walker; policy - // syntax has already been validated by - // ReadDenyMatcher::try_new_for_local_paths. - return Ok(None); - } - - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(format!( - "ripgrep unreadable glob scan failed under {}: {stderr}", - scan_plan.root.display() - )); - } - - output - .stdout - .split(|byte| *byte == b'\0') - .filter(|path| !path.is_empty()) - .map(|path| { - let path = std::str::from_utf8(path).map_err(|err| { - format!( - "ripgrep returned a non-UTF-8 path under {}: {err}", - scan_plan.root.display() - ) - })?; - let path = PathBuf::from(path); - Ok(if path.is_absolute() { - path - } else { - scan_plan.root.join(path) - }) - }) - .collect::, _>>() - .map(Some) -} - fn push_absolute_path( paths: &mut Vec, seen: &mut HashSet, @@ -271,45 +193,16 @@ fn glob_scan_plan(pattern: &str, configured_max_depth: Option) -> GlobSca GlobScanPlan { root: PathBuf::from(scan.root), max_depth: scan.max_depth, - globs: vec![ripgrep_glob(scan.pattern_suffix)], + globs: vec![file_filter_glob(scan.pattern_suffix)], } } -fn ripgrep_glob(pattern: &str) -> String { +fn file_filter_glob(pattern: &str) -> String { let pattern = pattern.replace('\\', "/"); - let mut escaped = String::with_capacity(pattern.len()); - let mut chars = pattern.chars(); - - while let Some(ch) = chars.next() { - if ch != '[' { - escaped.push(ch); - continue; - } - - let mut class = String::new(); - let mut closed = false; - for class_ch in chars.by_ref() { - if class_ch == ']' { - closed = true; - break; - } - class.push(class_ch); - } - - if closed { - escaped.push('['); - escaped.push_str(&class); - escaped.push(']'); - } else { - escaped.push_str(r"\["); - escaped.push_str(&class); - } - } - - if escaped.starts_with("**/") { - escaped + if pattern.starts_with("**/") { + pattern } else { - format!("**/{escaped}") + format!("**/{pattern}") } } @@ -320,10 +213,10 @@ mod access_tests; #[cfg(test)] mod tests { use super::GlobScanPlan; + use super::file_filter_glob; use super::glob_scan_plan; use super::glob_scan_plans; use super::resolve_windows_deny_read_paths; - use super::ripgrep_glob; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -568,18 +461,6 @@ mod tests { tmp.path().display() ))]); - if let Some(paths) = super::ripgrep_files( - &GlobScanPlan { - root: tmp.path().to_path_buf(), - max_depth: None, - globs: vec!["**/*.env".to_string()], - }, - &|_| {}, - ) - .expect("case-insensitive ripgrep scan") - { - assert!(paths.contains(&uppercase_env)); - } let actual: HashSet = resolve_windows_deny_read_paths(&policy, &cwd) .expect("resolve") .into_iter() @@ -684,10 +565,27 @@ mod tests { } #[test] - fn ripgrep_globs_normalize_windows_separators_and_unclosed_classes() { - assert_eq!(ripgrep_glob(r"**\*.env"), "**/*.env"); - assert_eq!(ripgrep_glob("nested/*.env"), "**/nested/*.env"); - assert_eq!(ripgrep_glob("[*.env"), r"**/\[*.env"); + fn file_filter_globs_normalize_windows_separators_and_unclosed_classes() { + assert_eq!(file_filter_glob(r"**\*.env"), "**/*.env"); + assert_eq!(file_filter_glob("nested/*.env"), "**/nested/*.env"); + assert_eq!(file_filter_glob("[*.env"), "**/[*.env"); + + let tmp = TempDir::new().expect("tempdir"); + let cwd = AbsolutePathBuf::from_absolute_path(tmp.path()).expect("cwd"); + for (name, pattern) in [ + ("[]secret.env", "**/[]*.env"), + ("[[secret.env", "**/[[*.env"), + ] { + let secret = cwd.join(name); + std::fs::write(&secret, "secret").expect("write literal bracket name"); + let policy = FileSystemSandboxPolicy::restricted(vec![unreadable_glob_entry( + pattern.to_owned(), + )]); + assert_eq!( + resolve_windows_deny_read_paths(&policy, &cwd).expect("valid literal bracket glob"), + vec![secret], + ); + } } #[test] diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs index f29bf0e061..8ce1a19d8d 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs @@ -1,11 +1,8 @@ use super::resolve_windows_deny_read_paths; -use super::walker::DirectoryScanMode; -use super::walker::collect_existing_glob_directory_matches; 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 pretty_assertions::assert_eq; use std::collections::HashSet; @@ -98,7 +95,7 @@ fn protected_directory_does_not_discard_accessible_deny_matches() { #[cfg(windows)] { // CI enables backup privileges, which let directory enumeration bypass - // this fixture's ACL. Restrict a separate process, including its rg child, + // this fixture's ACL. Restrict a separate process, // rather than changing the shared test process's privileges. The parent // owns ACL setup and cleanup, which the restricted child cannot perform. let codex_home = TempDir::new().expect("restricted codex home"); @@ -179,7 +176,7 @@ fn assert_accessible_deny_matches(root: &Path) { } #[test] -fn native_walk_preserves_canonical_matches_through_directory_links_and_cycles() { +fn glob_scans_skip_child_directory_links_but_follow_linked_roots() { let temp = TempDir::new().expect("tempdir"); let cwd = AbsolutePathBuf::from_absolute_path(temp.path()).expect("cwd"); let walk = temp.path().join("walk"); @@ -190,7 +187,9 @@ fn native_walk_preserves_canonical_matches_through_directory_links_and_cycles() std::fs::write(target.join("public.txt"), "public").expect("public"); let alias = walk.join("alias"); let cycle = target.join("cycle"); - for (link, destination) in [(&alias, &target), (&cycle, &walk)] { + let dangling = walk.join("broken.env"); + let missing = temp.path().join("missing"); + for (link, destination) in [(&alias, &target), (&cycle, &walk), (&dangling, &missing)] { #[cfg(unix)] std::os::unix::fs::symlink(destination, link).expect("directory symlink"); #[cfg(windows)] @@ -205,45 +204,26 @@ fn native_walk_preserves_canonical_matches_through_directory_links_and_cycles() .success() ); } - let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::GlobPattern { - pattern: target - .canonicalize() - .expect("canonical target") - .join("**") - .join("*.env") - .display() - .to_string(), - }, - access: FileSystemAccessMode::Deny, - missing_path_behavior: None, - }]); - let matcher = ReadDenyMatcher::try_new_for_local_paths(&policy, cwd.as_path()) - .expect("valid matcher") - .expect("deny restriction"); - assert!( - matcher.is_local_path_read_denied_with_canonical_path( - &alias.join("secret.env"), - &target - .canonicalize() - .expect("canonical target") - .join("secret.env"), - ) - ); - let mut paths = Vec::new(); - collect_existing_glob_directory_matches( - &walk, - &matcher, - &mut paths, - &mut HashSet::new(), - /*max_depth*/ None, - DirectoryScanMode::IncludeAllFiles, - ) - .expect("walk through directory alias and cycle"); - assert_eq!( - paths, - vec![ - AbsolutePathBuf::from_absolute_path(alias.join("secret.env")).expect("lexical target") - ] - ); + for (root, expected) in [ + (&walk, Vec::new()), + ( + &alias, + vec![ + AbsolutePathBuf::from_absolute_path(alias.join("secret.env")) + .expect("linked secret"), + ], + ), + ] { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: root.join("**").join("*.env").display().to_string(), + }, + access: FileSystemAccessMode::Deny, + missing_path_behavior: None, + }]); + assert_eq!( + resolve_windows_deny_read_paths(&policy, &cwd).expect("resolve directory links"), + expected, + ); + } } diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs b/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs deleted file mode 100644 index 08656ff783..0000000000 --- a/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs +++ /dev/null @@ -1,151 +0,0 @@ -//! Matcher-backed snapshot traversal for Windows deny-read ACL targets. -//! -//! The accessible-directory behavior is the same as the original native -//! resolver: disappearing and inaccessible branches cannot be enumerated, but -//! a matching directory is retained before reading its children. This does not -//! grant access to a skipped branch. Unexpected I/O errors abort the snapshot. -//! Canonical parents are reused only within this walk; links are resolved anew. - -use super::push_absolute_path; -use codex_protocol::permissions::ReadDenyMatcher; -use codex_utils_absolute_path::AbsolutePathBuf; -use std::collections::HashSet; -use std::fs::FileType; -use std::io; -use std::path::Path; -use std::path::PathBuf; - -#[derive(Clone, Copy)] -pub(super) enum DirectoryScanMode { - DirectoriesOnly, - IncludeAllFiles, -} - -pub(super) fn collect_existing_glob_directory_matches( - root: &Path, - matcher: &ReadDenyMatcher, - paths: &mut Vec, - seen_paths: &mut HashSet, - max_depth: Option, - mode: DirectoryScanMode, -) -> Result<(), String> { - let Some(metadata) = accessible_entry(root.metadata(), root)? else { - return Ok(()); - }; - let canonical = accessible_entry(root.canonicalize(), root)?; - let mut walker = GlobWalker { - matcher, - paths, - seen_paths, - seen_directories: HashSet::new(), - max_depth, - }; - walker.visit( - root, - canonical.as_deref(), - metadata.file_type(), - /*depth*/ 0, - mode, - ) -} - -struct GlobWalker<'a> { - matcher: &'a ReadDenyMatcher, - paths: &'a mut Vec, - seen_paths: &'a mut HashSet, - seen_directories: HashSet, - max_depth: Option, -} - -impl GlobWalker<'_> { - fn visit( - &mut self, - path: &Path, - canonical: Option<&Path>, - file_type: FileType, - depth: usize, - mode: DirectoryScanMode, - ) -> Result<(), String> { - let denied = match canonical { - Some(canonical) => self - .matcher - .is_local_path_read_denied_with_canonical_path(path, canonical), - None => self.matcher.is_local_path_read_denied(path), - }; - if denied { - push_absolute_path(self.paths, self.seen_paths, path)?; - } - if !file_type.is_dir() - || self.max_depth.is_some_and(|limit| depth >= limit) - || !self - .seen_directories - .insert(canonical.unwrap_or(path).to_path_buf()) - { - return Ok(()); - } - let Some(entries) = accessible_entry(std::fs::read_dir(path), path)? else { - return Ok(()); - }; - for entry in entries { - let Some(entry) = accessible_entry(entry, path)? else { - continue; - }; - let entry_path = entry.path(); - let Some(file_type) = accessible_entry(entry.file_type(), &entry_path)? else { - continue; - }; - if file_type.is_symlink() { - // Name-surrogate reparse points (including Windows junctions) - // cannot inherit the parent's canonical location. Resolve the - // target, then retain lexical matches and detect target cycles. - let target = accessible_entry(entry_path.canonicalize(), &entry_path)?; - let Some(metadata) = accessible_entry(entry_path.metadata(), &entry_path)? else { - continue; - }; - self.visit( - &entry_path, - target.as_deref(), - metadata.file_type(), - depth + 1, - DirectoryScanMode::IncludeAllFiles, - )?; - } else if file_type.is_dir() || matches!(mode, DirectoryScanMode::IncludeAllFiles) { - // Directory entries already carry their type on Windows. A - // normal child has the resolved parent's location plus its - // enumerated name; do not stat/canonicalize every ordinary file. - self.visit( - &entry_path, - canonical - .map(|parent| parent.join(entry.file_name())) - .as_deref(), - file_type, - depth + 1, - mode, - )?; - } - } - Ok(()) - } -} - -fn accessible_entry(result: io::Result, path: &Path) -> Result, String> { - match result { - Ok(value) => Ok(Some(value)), - Err(err) - if matches!( - err.kind(), - io::ErrorKind::NotFound | io::ErrorKind::PermissionDenied - ) => - { - Ok(None) - } - Err(err) => Err(format!( - "failed to enumerate unreadable glob paths under {}: {err}", - path.display() - )), - } -} - -#[cfg(test)] -#[path = "deny_read_walker_tests.rs"] -mod tests; diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_walker_tests.rs b/codex-rs/windows-sandbox-rs/src/deny_read_walker_tests.rs deleted file mode 100644 index 8836197762..0000000000 --- a/codex-rs/windows-sandbox-rs/src/deny_read_walker_tests.rs +++ /dev/null @@ -1,17 +0,0 @@ -use super::accessible_entry; -use pretty_assertions::assert_eq; -use std::io; -use std::path::Path; - -#[test] -fn unexpected_io_errors_abort_the_snapshot() { - let error = io::Error::other("directory read interrupted"); - - assert_eq!( - accessible_entry::<()>(Err(error), Path::new("scan-root")), - Err( - "failed to enumerate unreadable glob paths under scan-root: directory read interrupted" - .to_string() - ) - ); -} diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 2dc203ebd2..ed2917bf71 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -265,10 +265,8 @@ pub use conpty::spawn_conpty_process_as_user; pub use deny_read_acl::apply_deny_read_acls; #[cfg(target_os = "windows")] pub use deny_read_acl::plan_deny_read_acl_paths; -pub use deny_read_resolver::GLOB_SCAN_PROGRAM; pub use deny_read_resolver::expand_windows_deny_read_paths; pub use deny_read_resolver::resolve_windows_deny_read_paths; -pub use deny_read_resolver::resolve_windows_deny_read_paths_in_environment; #[cfg(target_os = "windows")] pub use deny_read_state::sync_persistent_deny_read_acls; #[cfg(target_os = "windows")]