Reserve missing .git without creating it

Author: evawong-oai

codex-rs/linux-sandbox/src/bwrap.rs

@@ -515,6 +515,13 @@ fn append_read_only_subpath_args(
         if let Some(first_missing_component) = find_first_non_existent_component(subpath)
             && is_within_allowed_write_paths(&first_missing_component, allowed_write_paths)
         {
+            // Keep missing top level `.git` reserved in the logical policy
+            // without materializing it in the host workspace at sandbox
+            // startup. Existing `.git` paths are remounted read-only by the
+            // branch below once they exist.
+            if first_missing_component.file_name() == Some(std::ffi::OsStr::new(".git")) {
+                return;
+            }
             args.push("--ro-bind".to_string());
             args.push("/dev/null".to_string());
             args.push(path_to_string(&first_missing_component));
@@ -1054,6 +1061,10 @@ mod tests {
             Path::new("/"),
         )
         .expect("bwrap fs args");
+        assert!(
+            !args.args.iter().any(|arg| arg == "/.git"),
+            "missing /.git should stay logical only and should not be materialized in bwrap args",
+        );
         assert_eq!(
             args.args,
             vec![
@@ -1068,9 +1079,10 @@ mod tests {
                 "--bind".to_string(),
                 "/".to_string(),
                 "/".to_string(),
-                // Mask the default protected .codex subpath under that writable
-                // root. Because the root is `/` in this test, the carveout path
-                // appears as `/.codex`.
+                // Keep the missing `/.git` reservation logical only, but mask
+                // the default protected `.codex` subpath under that writable
+                // root. Because the root is `/` in this test, the carveout
+                // path appears as `/.codex`.
                 "--ro-bind".to_string(),
                 "/dev/null".to_string(),
                 "/.codex".to_string(),

codex-rs/protocol/src/permissions.rs

@@ -1113,26 +1113,24 @@ fn default_read_only_subpaths_for_writable_root(
     // (file .git with gitdir pointer), and bare repos when the gitdir is the
     // writable root itself.
     let top_level_git_is_file = top_level_git.as_path().is_file();
-    let top_level_git_is_dir = top_level_git.as_path().is_dir();
-    if top_level_git_is_dir || top_level_git_is_file {
-        if top_level_git_is_file
-            && is_git_pointer_file(&top_level_git)
-            && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
-        {
-            subpaths.push(gitdir);
-        }
-        subpaths.push(top_level_git);
+    if top_level_git_is_file
+        && is_git_pointer_file(&top_level_git)
+        && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
+    {
+        subpaths.push(gitdir);
     }
+    subpaths.push(top_level_git);
 
     let top_level_agents = writable_root.join(".agents");
     if top_level_agents.as_path().is_dir() {
         subpaths.push(top_level_agents);
     }
 
-    // Keep top-level project metadata under .codex read-only to the agent by
-    // default. For the workspace root itself, protect it even before the
-    // directory exists so first-time creation still goes through the
-    // protected-path approval flow.
+    // Keep top level project metadata under .git and .codex read-only to the
+    // agent by default. Existing .git pointer files still protect their
+    // resolved gitdir targets when present. For the workspace root itself,
+    // protect .codex even before it exists so first time creation still goes
+    // through the protected path approval flow.
     let top_level_codex = writable_root.join(".codex");
     if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
         subpaths.push(top_level_codex);
@@ -1290,12 +1288,13 @@ mod tests {
 
     #[cfg(unix)]
     #[test]
-    fn writable_roots_proactively_protect_missing_dot_codex() {
+    fn writable_roots_reserve_missing_project_metadata_paths() {
         let cwd = TempDir::new().expect("tempdir");
         let expected_root = AbsolutePathBuf::from_absolute_path(
             cwd.path().canonicalize().expect("canonicalize cwd"),
         )
         .expect("absolute canonical root");
+        let expected_dot_git = expected_root.join(".git");
         let expected_dot_codex = expected_root.join(".codex");
 
         let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
@@ -1308,6 +1307,11 @@ mod tests {
         let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
         assert_eq!(writable_roots.len(), 1);
         assert_eq!(writable_roots[0].root, expected_root);
+        assert!(
+            writable_roots[0]
+                .read_only_subpaths
+                .contains(&expected_dot_git)
+        );
         assert!(
             writable_roots[0]
                 .read_only_subpaths
@@ -1383,6 +1387,13 @@ mod tests {
     #[test]
     fn legacy_workspace_write_projection_accepts_relative_cwd() {
         let relative_cwd = Path::new("workspace");
+        let expected_dot_git = AbsolutePathBuf::from_absolute_path(
+            std::env::current_dir()
+                .expect("current dir")
+                .join(relative_cwd)
+                .join(".git"),
+        )
+        .expect("absolute dot git");
         let expected_dot_codex = AbsolutePathBuf::from_absolute_path(
             std::env::current_dir()
                 .expect("current dir")
@@ -1413,6 +1424,12 @@ mod tests {
                     },
                     access: FileSystemAccessMode::Write,
                 },
+                FileSystemSandboxEntry {
+                    path: FileSystemPath::Path {
+                        path: expected_dot_git,
+                    },
+                    access: FileSystemAccessMode::Read,
+                },
                 FileSystemSandboxEntry {
                     path: FileSystemPath::Path {
                         path: expected_dot_codex,
@@ -1498,6 +1515,7 @@ mod tests {
         let expected_root =
             AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root");
         let expected_blocked = link_blocked.clone();
+        let expected_git = expected_root.join(".git");
         let expected_agents = expected_root.join(".agents");
         let expected_codex = expected_root.join(".codex");
 
@@ -1542,6 +1560,7 @@ mod tests {
                 .read_only_subpaths
                 .contains(&expected_agents)
         );
+        assert!(writable_roots[0].read_only_subpaths.contains(&expected_git));
         assert!(
             writable_roots[0]
                 .read_only_subpaths
@@ -1560,6 +1579,13 @@ mod tests {
         symlink_dir(&decoy, &dot_codex).expect("create .codex symlink");
 
         let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root");
+        let expected_dot_git = AbsolutePathBuf::from_absolute_path(
+            root.as_path()
+                .canonicalize()
+                .expect("canonicalize root")
+                .join(".git"),
+        )
+        .expect("absolute .git path");
         let expected_dot_codex = AbsolutePathBuf::from_absolute_path(
             root.as_path()
                 .canonicalize()
@@ -1580,7 +1606,7 @@ mod tests {
         assert_eq!(writable_roots.len(), 1);
         assert_eq!(
             writable_roots[0].read_only_subpaths,
-            vec![expected_dot_codex]
+            vec![expected_dot_git, expected_dot_codex]
         );
         assert!(
             !writable_roots[0]
@@ -1626,7 +1652,7 @@ mod tests {
         assert_eq!(writable_roots[0].root, expected_root);
         assert_eq!(
             writable_roots[0].read_only_subpaths,
-            vec![expected_linked_private]
+            vec![expected_root.join(".git"), expected_linked_private]
         );
         assert!(
             !writable_roots[0]
@@ -1673,7 +1699,7 @@ mod tests {
         assert_eq!(writable_roots[0].root, expected_root);
         assert_eq!(
             writable_roots[0].read_only_subpaths,
-            vec![expected_linked_private]
+            vec![expected_root.join(".git"), expected_linked_private]
         );
         assert!(
             !writable_roots[0]
@@ -1713,7 +1739,10 @@ mod tests {
         let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
         assert_eq!(writable_roots.len(), 1);
         assert_eq!(writable_roots[0].root, expected_root);
-        assert_eq!(writable_roots[0].read_only_subpaths, vec![expected_alias]);
+        assert_eq!(
+            writable_roots[0].read_only_subpaths,
+            vec![expected_root.join(".git"), expected_alias]
+        );
     }
 
     #[cfg(unix)]
@@ -1751,6 +1780,7 @@ mod tests {
         let expected_root =
             AbsolutePathBuf::from_absolute_path(&link_tmpdir).expect("absolute symlinked tmpdir");
         let expected_blocked = link_blocked.clone();
+        let expected_git = expected_root.join(".git");
         let expected_codex = expected_root.join(".codex");
 
         unsafe {
@@ -1783,6 +1813,7 @@ mod tests {
                 .read_only_subpaths
                 .contains(&expected_blocked)
         );
+        assert!(writable_roots[0].read_only_subpaths.contains(&expected_git));
         assert!(
             writable_roots[0]
                 .read_only_subpaths

codex-rs/protocol/src/protocol.rs

@@ -1287,26 +1287,24 @@ fn default_read_only_subpaths_for_writable_root(
     // (file .git with gitdir pointer), and bare repos when the gitdir is the
     // writable root itself.
     let top_level_git_is_file = top_level_git.as_path().is_file();
-    let top_level_git_is_dir = top_level_git.as_path().is_dir();
-    if top_level_git_is_dir || top_level_git_is_file {
-        if top_level_git_is_file
-            && is_git_pointer_file(&top_level_git)
-            && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
-        {
-            subpaths.push(gitdir);
-        }
-        subpaths.push(top_level_git);
+    if top_level_git_is_file
+        && is_git_pointer_file(&top_level_git)
+        && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
+    {
+        subpaths.push(gitdir);
     }
+    subpaths.push(top_level_git);
 
     let top_level_agents = writable_root.join(".agents");
     if top_level_agents.as_path().is_dir() {
         subpaths.push(top_level_agents);
     }
 
-    // Keep top-level project metadata under .codex read-only to the agent by
-    // default. For the workspace root itself, protect it even before the
-    // directory exists so first-time creation still goes through the
-    // protected-path approval flow.
+    // Keep top level project metadata under .git and .codex read-only to the
+    // agent by default. Existing .git pointer files still protect their
+    // resolved gitdir targets when present. For the workspace root itself,
+    // protect .codex even before it exists so first time creation still goes
+    // through the protected path approval flow.
     let top_level_codex = writable_root.join(".codex");
     if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
         subpaths.push(top_level_codex);
@@ -4284,6 +4282,8 @@ mod tests {
         let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path());
         let expected_secret = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("secret"))
             .expect("canonical secret");
+        let expected_git = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".git"))
+            .expect("canonical .git");
         let expected_agents = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents"))
             .expect("canonical .agents");
         let expected_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
@@ -4328,6 +4328,12 @@ mod tests {
                 .iter()
                 .any(|path| path.as_path() == expected_secret.as_path())
         );
+        assert!(
+            writable_roots[0]
+                .read_only_subpaths
+                .iter()
+                .any(|path| path.as_path() == expected_git.as_path())
+        );
         assert!(
             writable_roots[0]
                 .read_only_subpaths
@@ -4354,8 +4360,12 @@ mod tests {
         let expected_docs_public =
             AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public"))
                 .expect("canonical docs/public");
-        let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
-            .expect("canonical .codex");
+        let mut expected_cwd_reserved_paths = vec![
+            canonical_cwd.join(".git"),
+            canonical_cwd.join(".codex"),
+            expected_docs.to_path_buf(),
+        ];
+        expected_cwd_reserved_paths.sort();
         let policy = FileSystemSandboxPolicy::restricted(vec![
             FileSystemSandboxEntry {
                 path: FileSystemPath::Special {
@@ -4377,14 +4387,11 @@ mod tests {
         assert_eq!(
             sorted_writable_roots(policy.get_writable_roots_with_cwd(cwd.path())),
             vec![
+                (canonical_cwd, expected_cwd_reserved_paths),
                 (
-                    canonical_cwd,
-                    vec![
-                        expected_dot_codex.to_path_buf(),
-                        expected_docs.to_path_buf()
-                    ],
+                    expected_docs_public.to_path_buf(),
+                    vec![expected_docs_public.join(".git").to_path_buf()],
                 ),
-                (expected_docs_public.to_path_buf(), Vec::new()),
             ]
         );
     }
@@ -4395,8 +4402,9 @@ mod tests {
         let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path());
         let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path())
             .expect("canonicalize cwd");
-        let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
-            .expect("canonical .codex");
+        let mut expected_reserved_paths =
+            vec![canonical_cwd.join(".git"), canonical_cwd.join(".codex")];
+        expected_reserved_paths.sort();
         let policy = SandboxPolicy::WorkspaceWrite {
             writable_roots: vec![],
             read_only_access: ReadOnlyAccess::Restricted {
@@ -4413,7 +4421,7 @@ mod tests {
                 FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path())
                     .get_writable_roots_with_cwd(cwd.path())
             ),
-            vec![(canonical_cwd, vec![expected_dot_codex.to_path_buf()])]
+            vec![(canonical_cwd, expected_reserved_paths)]
         );
     }