Preserve Windows root reads with deny-read rules (#40441)

## What changed

- Detect readable symbolic `:root` entries separately from effective full-disk access so narrower deny-read rules do not disable the Windows sandbox's broad-read setup.
- Add explicit readable roots to that setup while omitting the filesystem root itself, and prevent read roots at or below denied paths from being granted access.

## Testing

- Extend the elevated Windows sandbox integration test to verify that an allowed bundled skill under the user profile remains readable while exact and glob-based secrets stay denied.

GitOrigin-RevId: 8f6f267b78718b9e8609a9205c489f253f478491
This commit is contained in:
zm-oai
2026-08-24 17:22:38 +00:00
committed by copyberry
parent 3470b807b4
commit a12c4f4c01
3 changed files with 56 additions and 11 deletions

View File

@@ -273,12 +273,18 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh
let workspace = TempDir::new()?;
let cwd = dunce::canonicalize(workspace.path())?.abs();
let glob_secret = cwd.join("secret.env");
let exact_secret = cwd.join("exact-secret.txt");
let public = cwd.join("public.txt");
let user_profile = TempDir::new_in(dirs::home_dir().context("resolve user profile")?)?;
let _user_profile_guard = EnvVarGuard::set("USERPROFILE", user_profile.path().as_os_str());
let exact_secret = user_profile.path().join("exact-secret.txt");
std::fs::write(&exact_secret, "exact secret\n")?;
let bundled_skill_dir = user_profile.path().join(".codex/plugins/cache");
std::fs::create_dir_all(&bundled_skill_dir)?;
let bundled_skill = bundled_skill_dir.join("SKILL.md");
let setup_marker = codex_home.path().join(".sandbox").join("setup_marker.json");
std::fs::write(&glob_secret, "glob secret\n")?;
std::fs::write(&exact_secret, "exact secret\n")?;
std::fs::write(&public, "public ok\n")?;
std::fs::write(&bundled_skill, "bundled skill ok\n")?;
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
@@ -303,7 +309,7 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: exact_secret.into(),
path: exact_secret.clone().abs().into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -323,16 +329,19 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh
"cmd.exe".to_string(),
"/D".to_string(),
"/C".to_string(),
format!(
"(type secret.env 1>NUL 2>NUL && echo GLOB-READ || echo GLOB-DENIED) & (type exact-secret.txt 1>NUL 2>NUL && echo EXACT-READ || echo EXACT-DENIED) & (type \"{}\" 1>NUL 2>NUL && echo MARKER-READ-ALLOWED || echo MARKER-READ-DENIED) & (echo tampered > \"{}\" 2>NUL && echo MARKER-WRITE-ALLOWED || echo MARKER-WRITE-DENIED) & type public.txt",
setup_marker.display(),
setup_marker.display()
),
"(type secret.env 1>NUL 2>NUL && echo GLOB-READ || echo GLOB-DENIED) & (type %SETUP_MARKER% 1>NUL 2>NUL && echo MARKER-READ-ALLOWED || echo MARKER-READ-DENIED) & (echo tampered > %SETUP_MARKER% 2>NUL && echo MARKER-WRITE-ALLOWED || echo MARKER-WRITE-DENIED) & type public.txt & type %BUNDLED_SKILL% & (type %EXACT_SECRET% 1>NUL 2>NUL && echo EXACT-READ || echo EXACT-DENIED)".to_string(),
],
cwd: cwd.clone(),
expiration: 10_000.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
env: [
("BUNDLED_SKILL", &bundled_skill),
("EXACT_SECRET", &exact_secret),
("SETUP_MARKER", &setup_marker),
]
.into_iter()
.map(|(name, path)| (name.to_string(), format!("\"{}\"", path.display())))
.collect(),
network: None,
network_environment_id: None,
sandbox_permissions: SandboxPermissions::UseDefault,
@@ -371,6 +380,7 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh
stdout.text.contains("public ok"),
"allowed reads should still work: {stdout:?}"
);
assert!(stdout.text.contains("bundled skill ok"));
assert!(
stdout.text.contains("MARKER-READ-DENIED"),
"sandboxed command should not read setup readiness: {stdout:?}"

View File

@@ -4,6 +4,7 @@ use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath::Root;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashMap;
@@ -106,6 +107,16 @@ impl ResolvedWindowsSandboxPermissions {
self.file_system.has_full_disk_read_access()
}
pub(crate) fn has_symbolic_root_read_access(&self, cwd: &Path) -> bool {
self.file_system.entries.iter().any(|entry| {
matches!(&entry.path, FileSystemPath::Special { value: Root })
&& entry.access.can_read()
}) && cwd
.ancestors()
.last()
.is_some_and(|root| self.file_system.can_read_path_with_cwd(root, cwd))
}
pub(crate) fn include_platform_defaults(&self) -> bool {
self.file_system.include_platform_defaults()
}

View File

@@ -575,6 +575,12 @@ fn gather_full_read_roots_for_permissions(
.into_iter()
.map(|root| root.root),
);
roots.extend(
permissions
.readable_roots_for_cwd(command_cwd)
.into_iter()
.filter(|root| root.parent().is_some() || !command_cwd.starts_with(root)),
);
canonical_existing(&roots)
}
@@ -584,7 +590,7 @@ pub(crate) fn gather_read_roots(
env_map: &HashMap<String, String>,
codex_home: &Path,
) -> Vec<PathBuf> {
if permissions.has_full_disk_read_access() {
if permissions.has_symbolic_root_read_access(command_cwd) {
return gather_full_read_roots_for_permissions(
command_cwd,
permissions,
@@ -1174,7 +1180,25 @@ fn build_payload_roots(
read_roots = filter_user_profile_root_exclusions(read_roots);
read_roots = filter_ssh_config_dependency_roots(read_roots);
let write_root_set: HashSet<PathBuf> = write_roots.iter().cloned().collect();
read_roots.retain(|root| !write_root_set.contains(root));
let deny_read_keys: Vec<String> = overrides
.deny_read_paths
.as_deref()
.unwrap_or_default()
.iter()
.map(|path| canonical_path_key(path))
.collect();
read_roots.retain(|root| {
if write_root_set.contains(root) {
return false;
}
if deny_read_keys.is_empty() {
return true;
}
let root_key = canonical_path_key(root);
!deny_read_keys
.iter()
.any(|denied| Path::new(&root_key).starts_with(denied))
});
(read_roots, write_roots)
}