diff --git a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs index 0ee822f404..a7e6590453 100644 --- a/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs +++ b/codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs @@ -496,19 +496,6 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> { } } } - // Deny-read ACEs are applied after read grants so the DACL ends with - // explicit deny entries that take precedence over the broad read allowlist. - match unsafe { apply_deny_read_acls(&payload.deny_read_paths, sandbox_group_psid) } { - Ok(applied) => { - if !applied.is_empty() { - log_line(log, &format!("applied {} deny-read ACLs", applied.len()))?; - } - } - Err(err) => { - refresh_errors.push(format!("apply deny-read ACLs failed: {err}")); - log_line(log, &format!("apply deny-read ACLs failed: {err}"))?; - } - } unsafe { if !sandbox_group_psid.is_null() { LocalFree(sandbox_group_psid as HLOCAL); @@ -633,13 +620,21 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( ); } - // The read ACL helper also applies deny-read ACEs, so it must run whenever - // deny-read paths are present even if no new read roots need to be granted. - if payload.read_roots.is_empty() && payload.deny_read_paths.is_empty() { + // Deny-read ACEs must be present before the sandboxed command starts. Apply + // them synchronously here instead of delegating them to the background + // helper used for read grants. + let applied_deny_read_paths = + unsafe { apply_deny_read_acls(&payload.deny_read_paths, sandbox_group_psid) } + .context("apply deny-read ACLs")?; + if !applied_deny_read_paths.is_empty() { log_line( log, - "no read roots or deny-read paths; skipping read ACL helper", + &format!("applied {} deny-read ACLs", applied_deny_read_paths.len()), )?; + } + + if payload.read_roots.is_empty() { + log_line(log, "no read roots to grant; skipping read ACL helper")?; } else { match read_acl_mutex_exists() { Ok(true) => { 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 13eb5a8141..4a8c76338b 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs @@ -51,8 +51,8 @@ pub fn resolve_windows_deny_read_paths( return Ok(paths); }; - let mut seen_scan_dirs = HashSet::new(); for pattern in unreadable_globs { + let mut seen_scan_dirs = HashSet::new(); let scan_plan = glob_scan_plan(&pattern); collect_existing_glob_matches( &scan_plan.root, @@ -194,6 +194,9 @@ mod tests { 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 }, @@ -312,4 +315,34 @@ mod tests { 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 = 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); + } } diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 9e01430307..3b6e47086e 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -821,14 +821,9 @@ fn build_payload_deny_write_paths( } fn build_payload_deny_read_paths(explicit_deny_read_paths: Option>) -> Vec { - // Preserve missing exact deny paths so the Windows helper can materialize - // and deny them before the sandboxed process runs. Existing paths are - // canonicalized here to make the elevated helper operate on stable targets. - explicit_deny_read_paths - .unwrap_or_default() - .into_iter() - .map(|path| canonicalize_path(&path)) - .collect() + // Keep the configured spelling here so the ACL layer can plan both the + // lexical path and any existing canonical target for reparse-point aliases. + explicit_deny_read_paths.unwrap_or_default() } fn expand_user_profile_root(roots: Vec) -> Vec { @@ -1472,7 +1467,7 @@ mod tests { } #[test] - fn build_payload_deny_read_paths_keeps_missing_paths_and_canonicalizes_existing_paths() { + fn build_payload_deny_read_paths_preserves_explicit_paths() { let tmp = TempDir::new().expect("tempdir"); let existing = tmp.path().join("secret.env"); let missing = tmp.path().join("future.env"); @@ -1480,10 +1475,7 @@ mod tests { assert_eq!( super::build_payload_deny_read_paths(Some(vec![existing.clone(), missing.clone()])), - vec![ - dunce::canonicalize(existing).expect("canonical existing"), - missing - ] + vec![existing, missing] ); } }