Merge 64435bee22 into sapling-pr-archive-bolinfest

codex-rs/windows-sandbox-rs/src/allow.rs

@@ -5,6 +5,8 @@ use std::collections::HashSet;
 use std::path::Path;
 use std::path::PathBuf;
 
+const PROTECTED_WRITE_SUBDIRS: &[&str] = &[".git", ".codex", ".agents"];
+
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct AllowDenyPaths {
     pub allow: HashSet<PathBuf>,
@@ -44,19 +46,11 @@ pub fn compute_allow_paths(
              policy_cwd: &Path,
              add_allow: &mut dyn FnMut(PathBuf),
              add_deny: &mut dyn FnMut(PathBuf)| {
-                let candidate = if root.is_absolute() {
-                    root
-                } else {
-                    policy_cwd.join(root)
-                };
-                let canonical = canonicalize(&candidate).unwrap_or(candidate);
+                let canonical = canonical_writable_root(root, policy_cwd);
                 add_allow(canonical.clone());
 
-                for protected_subdir in [".git", ".codex", ".agents"] {
-                    let protected_entry = canonical.join(protected_subdir);
-                    if protected_entry.exists() {
-                        add_deny(protected_entry);
-                    }
+                for protected_entry in protected_child_deny_paths_for_canonical_root(&canonical) {
+                    add_deny(protected_entry);
                 }
             };
 
@@ -81,6 +75,40 @@ pub fn compute_allow_paths(
     AllowDenyPaths { allow, deny }
 }
 
+pub(crate) fn protected_child_deny_paths_for_roots<I, P>(
+    roots: I,
+    policy_cwd: &Path,
+) -> HashSet<PathBuf>
+where
+    I: IntoIterator<Item = P>,
+    P: AsRef<Path>,
+{
+    roots
+        .into_iter()
+        .flat_map(|root| {
+            let canonical = canonical_writable_root(root.as_ref().to_path_buf(), policy_cwd);
+            protected_child_deny_paths_for_canonical_root(&canonical)
+        })
+        .collect()
+}
+
+fn canonical_writable_root(root: PathBuf, policy_cwd: &Path) -> PathBuf {
+    let candidate = if root.is_absolute() {
+        root
+    } else {
+        policy_cwd.join(root)
+    };
+    canonicalize(&candidate).unwrap_or(candidate)
+}
+
+fn protected_child_deny_paths_for_canonical_root(canonical: &Path) -> Vec<PathBuf> {
+    PROTECTED_WRITE_SUBDIRS
+        .iter()
+        .map(|protected_subdir| canonical.join(protected_subdir))
+        .filter(|protected_entry| protected_entry.exists())
+        .collect()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

codex-rs/windows-sandbox-rs/src/setup.rs

@@ -35,6 +35,8 @@ use windows_sys::Win32::Security::CheckTokenMembership;
 use windows_sys::Win32::Security::FreeSid;
 use windows_sys::Win32::Security::SECURITY_NT_AUTHORITY;
 
+use crate::allow::protected_child_deny_paths_for_roots;
+
 pub const SETUP_VERSION: u32 = 5;
 pub const OFFLINE_USERNAME: &str = "CodexSandboxOffline";
 pub const ONLINE_USERNAME: &str = "CodexSandboxOnline";
@@ -168,7 +170,7 @@ fn run_setup_refresh_inner(
         return Ok(());
     }
     let (read_roots, write_roots) = build_payload_roots(&request, &overrides);
-    let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths);
+    let deny_write_paths = build_payload_deny_write_paths(&request, &overrides, &write_roots);
     let network_identity =
         SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced);
     let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity);
@@ -715,7 +717,7 @@ pub fn run_elevated_setup(
         )
     })?;
     let (read_roots, write_roots) = build_payload_roots(&request, &overrides);
-    let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths);
+    let deny_write_paths = build_payload_deny_write_paths(&request, &overrides, &write_roots);
     let network_identity =
         SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced);
     let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity);
@@ -789,7 +791,8 @@ fn build_payload_roots(
 
 fn build_payload_deny_write_paths(
     request: &SandboxSetupRequest<'_>,
-    explicit_deny_write_paths: Option<Vec<PathBuf>>,
+    overrides: &SetupRootOverrides,
+    write_roots: &[PathBuf],
 ) -> Vec<PathBuf> {
     let allow_deny_paths: AllowDenyPaths = compute_allow_paths(
         request.policy,
@@ -797,10 +800,13 @@ fn build_payload_deny_write_paths(
         request.command_cwd,
         request.env_map,
     );
-    let mut deny_write_paths: Vec<PathBuf> = explicit_deny_write_paths
+    let mut deny_write_paths: Vec<PathBuf> = overrides
+        .deny_write_paths
+        .as_deref()
         .unwrap_or_default()
-        .into_iter()
+        .iter()
         .map(|path| {
+            let path = path.to_path_buf();
             if path.exists() {
                 dunce::canonicalize(&path).unwrap_or(path)
             } else {
@@ -809,6 +815,12 @@ fn build_payload_deny_write_paths(
         })
         .collect();
     deny_write_paths.extend(allow_deny_paths.deny);
+    if overrides.write_roots.is_some() {
+        deny_write_paths.extend(protected_child_deny_paths_for_roots(
+            write_roots,
+            request.policy_cwd,
+        ));
+    }
     deny_write_paths
 }
 
@@ -1406,8 +1418,14 @@ mod tests {
             proxy_enforced: false,
         };
 
+        let overrides = super::SetupRootOverrides {
+            write_roots: Some(vec![command_cwd.clone(), extra_write_root.clone()]),
+            deny_write_paths: Some(vec![explicit_deny.clone()]),
+            ..Default::default()
+        };
+        let (_, write_roots) = super::build_payload_roots(&request, &overrides);
         let deny_write_paths =
-            super::build_payload_deny_write_paths(&request, Some(vec![explicit_deny.clone()]));
+            super::build_payload_deny_write_paths(&request, &overrides, &write_roots);
 
         assert_eq!(
             [