From eb784b92d46ad1ab545a76960fcb448bfa271db6 Mon Sep 17 00:00:00 2001
From: David Wiesen <iceweasel@openai.com>
Date: Mon, 11 May 2026 09:13:41 -0700
Subject: [PATCH] fix(windows-sandbox): stop denying extra writable root .git

---
 codex-rs/windows-sandbox-rs/src/allow.rs | 78 +++++++++++++++++++-----
 1 file changed, 64 insertions(+), 14 deletions(-)

diff --git a/codex-rs/windows-sandbox-rs/src/allow.rs b/codex-rs/windows-sandbox-rs/src/allow.rs
index 273dc8c4f2..5d91aa5f4f 100644
--- a/codex-rs/windows-sandbox-rs/src/allow.rs
+++ b/codex-rs/windows-sandbox-rs/src/allow.rs
@@ -42,6 +42,7 @@ pub fn compute_allow_paths(
         let add_writable_root =
             |root: PathBuf,
              policy_cwd: &Path,
+             protect_git_metadata: bool,
              add_allow: &mut dyn FnMut(PathBuf),
              add_deny: &mut dyn FnMut(PathBuf)| {
                 let candidate = if root.is_absolute() {
@@ -52,7 +53,12 @@ pub fn compute_allow_paths(
                 let canonical = canonicalize(&candidate).unwrap_or(candidate);
                 add_allow(canonical.clone());
 
-                for protected_subdir in [".git", ".codex", ".agents"] {
+                let protected_subdirs: &[&str] = if protect_git_metadata {
+                    &[".git", ".codex", ".agents"]
+                } else {
+                    &[".codex", ".agents"]
+                };
+                for protected_subdir in protected_subdirs {
                     let protected_entry = canonical.join(protected_subdir);
                     if protected_entry.exists() {
                         add_deny(protected_entry);
@@ -63,6 +69,7 @@ pub fn compute_allow_paths(
         add_writable_root(
             command_cwd.to_path_buf(),
             policy_cwd,
+            true,
             &mut add_allow_path,
             &mut add_deny_path,
         );
@@ -72,6 +79,7 @@ pub fn compute_allow_paths(
                 add_writable_root(
                     root.clone().into(),
                     policy_cwd,
+                    false,
                     &mut add_allow_path,
                     &mut add_deny_path,
                 );
@@ -117,12 +125,16 @@ mod tests {
 
         let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
 
-        assert!(paths
-            .allow
-            .contains(&dunce::canonicalize(&command_cwd).unwrap()));
-        assert!(paths
-            .allow
-            .contains(&dunce::canonicalize(&extra_root).unwrap()));
+        assert!(
+            paths
+                .allow
+                .contains(&dunce::canonicalize(&command_cwd).unwrap())
+        );
+        assert!(
+            paths
+                .allow
+                .contains(&dunce::canonicalize(&extra_root).unwrap())
+        );
         assert!(paths.deny.is_empty(), "no deny paths expected");
     }
 
@@ -145,12 +157,16 @@ mod tests {
 
         let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &env_map);
 
-        assert!(paths
-            .allow
-            .contains(&dunce::canonicalize(&command_cwd).unwrap()));
-        assert!(!paths
-            .allow
-            .contains(&dunce::canonicalize(&temp_dir).unwrap()));
+        assert!(
+            paths
+                .allow
+                .contains(&dunce::canonicalize(&command_cwd).unwrap())
+        );
+        assert!(
+            !paths
+                .allow
+                .contains(&dunce::canonicalize(&temp_dir).unwrap())
+        );
         assert!(paths.deny.is_empty(), "no deny paths expected");
     }
 
@@ -238,6 +254,37 @@ mod tests {
         assert_eq!(expected_deny, paths.deny);
     }
 
+    #[test]
+    fn does_not_deny_git_dir_inside_additional_writable_root() {
+        let tmp = TempDir::new().expect("tempdir");
+        let command_cwd = tmp.path().join("workspace");
+        let extra_root = tmp.path().join("extra");
+        let extra_git_dir = extra_root.join(".git");
+        let _ = fs::create_dir_all(&command_cwd);
+        let _ = fs::create_dir_all(&extra_git_dir);
+
+        let policy = SandboxPolicy::WorkspaceWrite {
+            writable_roots: vec![AbsolutePathBuf::try_from(extra_root.as_path()).unwrap()],
+            network_access: false,
+            exclude_tmpdir_env_var: true,
+            exclude_slash_tmp: false,
+        };
+
+        let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
+        let expected_allow: HashSet<PathBuf> = [
+            dunce::canonicalize(&command_cwd).unwrap(),
+            dunce::canonicalize(&extra_root).unwrap(),
+        ]
+        .into_iter()
+        .collect();
+
+        assert_eq!(expected_allow, paths.allow);
+        assert!(
+            paths.deny.is_empty(),
+            "additional writable roots should not deny .git"
+        );
+    }
+
     #[test]
     fn skips_protected_subdirs_when_missing() {
         let tmp = TempDir::new().expect("tempdir");
@@ -253,6 +300,9 @@ mod tests {
 
         let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
         assert_eq!(paths.allow.len(), 1);
-        assert!(paths.deny.is_empty(), "no deny when protected dirs are absent");
+        assert!(
+            paths.deny.is_empty(),
+            "no deny when protected dirs are absent"
+        );
     }
 }