feat: make .git read-only within a writable root when using Seatbelt

codex-rs/core/src/seatbelt.rs

@@ -4,6 +4,7 @@ use std::path::PathBuf;
 use tokio::process::Child;
 
 use crate::protocol::SandboxPolicy;
+use crate::spawn::CODEX_SANDBOX_ENV_VAR;
 use crate::spawn::StdioPolicy;
 use crate::spawn::spawn_child_async;
 
@@ -20,10 +21,11 @@ pub async fn spawn_command_under_seatbelt(
     sandbox_policy: &SandboxPolicy,
     cwd: PathBuf,
     stdio_policy: StdioPolicy,
-    env: HashMap<String, String>,
+    mut env: HashMap<String, String>,
 ) -> std::io::Result<Child> {
     let args = create_seatbelt_command_args(command, sandbox_policy, &cwd);
     let arg0 = None;
+    env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string());
     spawn_child_async(
         PathBuf::from(MACOS_PATH_TO_SEATBELT_EXECUTABLE),
         args,
@@ -50,16 +52,45 @@ fn create_seatbelt_command_args(
             )
         } else {
             let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
-            let (writable_folder_policies, cli_args): (Vec<String>, Vec<String>) = writable_roots
-                .iter()
-                .enumerate()
-                .map(|(index, root)| {
-                    let param_name = format!("WRITABLE_ROOT_{index}");
-                    let policy: String = format!("(subpath (param \"{param_name}\"))");
-                    let cli_arg = format!("-D{param_name}={}", root.to_string_lossy());
-                    (policy, cli_arg)
-                })
-                .unzip();
+
+            let mut writable_folder_policies: Vec<String> = Vec::new();
+            let mut cli_args: Vec<String> = Vec::new();
+
+            for (index, root) in writable_roots.iter().enumerate() {
+                // Canonicalize to avoid mismatches like /var vs /private/var on macOS.
+                let canonical_root = root.canonicalize().unwrap_or_else(|_| root.clone());
+                let param_name = format!("WRITABLE_ROOT_{index}");
+                cli_args.push(format!(
+                    "-D{param_name}={}",
+                    canonical_root.to_string_lossy()
+                ));
+
+                // For WorkspaceWrite, if the writable root itself looks like a
+                // git repository (i.e., contains a top-level ".git" directory),
+                // then disallow writes specifically under that top-level ".git".
+                // Do NOT block ".git" directories under subdirectories: those
+                // are allowed when the parent of the repo is writable.
+                let policy_component = if let SandboxPolicy::WorkspaceWrite { .. } = sandbox_policy
+                {
+                    let top_level_git = canonical_root.join(".git");
+                    if top_level_git.is_dir() {
+                        let git_param_name = format!("WRITABLE_ROOT_{index}_GIT");
+                        cli_args.push(format!(
+                            "-D{git_param_name}={}",
+                            top_level_git.to_string_lossy()
+                        ));
+                        format!(
+                            "(require-all (subpath (param \"{param_name}\")) (require-not (subpath (param \"{git_param_name}\"))))"
+                        )
+                    } else {
+                        format!("(subpath (param \"{param_name}\"))")
+                    }
+                } else {
+                    format!("(subpath (param \"{param_name}\"))")
+                };
+                writable_folder_policies.push(policy_component);
+            }
+
             if writable_folder_policies.is_empty() {
                 ("".to_string(), Vec::<String>::new())
             } else {
@@ -88,6 +119,7 @@ fn create_seatbelt_command_args(
     let full_policy = format!(
         "{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}"
     );
+
     let mut seatbelt_args: Vec<String> = vec!["-p".to_string(), full_policy];
     seatbelt_args.extend(extra_cli_args);
     seatbelt_args.push("--".to_string());

codex-rs/core/src/spawn.rs

@@ -17,6 +17,8 @@ use crate::protocol::SandboxPolicy;
 /// attributes, so this may change in the future.
 pub const CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR: &str = "CODEX_SANDBOX_NETWORK_DISABLED";
 
+pub const CODEX_SANDBOX_ENV_VAR: &str = "CODEX_SANDBOX";
+
 #[derive(Debug, Clone, Copy)]
 pub enum StdioPolicy {
     RedirectForShellTool,

codex-rs/core/tests/sandbox.rs

@@ -0,0 +1,195 @@
+#![cfg(target_os = "macos")]
+#![expect(clippy::expect_used)]
+
+use std::collections::HashMap;
+use std::path::Path;
+use std::path::PathBuf;
+
+use codex_core::protocol::SandboxPolicy;
+use codex_core::seatbelt::spawn_command_under_seatbelt;
+use codex_core::spawn::CODEX_SANDBOX_ENV_VAR;
+use codex_core::spawn::StdioPolicy;
+use tempfile::TempDir;
+
+struct TestScenario {
+    repo_parent: PathBuf,
+    file_outside_repo: PathBuf,
+    repo_root: PathBuf,
+    file_in_repo_root: PathBuf,
+    file_in_dot_git_dir: PathBuf,
+}
+
+struct TestExpectations {
+    file_outside_repo_is_writable: bool,
+    file_in_repo_root_is_writable: bool,
+    file_in_dot_git_dir_is_writable: bool,
+}
+
+impl TestScenario {
+    async fn run_test(&self, policy: &SandboxPolicy, expectations: TestExpectations) {
+        if std::env::var(CODEX_SANDBOX_ENV_VAR) == Ok("seatbelt".to_string()) {
+            eprintln!("{CODEX_SANDBOX_ENV_VAR} is set to 'seatbelt', skipping test.");
+            return;
+        }
+
+        assert_eq!(
+            touch(&self.file_outside_repo, policy).await,
+            expectations.file_outside_repo_is_writable
+        );
+        assert_eq!(
+            self.file_outside_repo.exists(),
+            expectations.file_outside_repo_is_writable
+        );
+
+        assert_eq!(
+            touch(&self.file_in_repo_root, policy).await,
+            expectations.file_in_repo_root_is_writable
+        );
+        assert_eq!(
+            self.file_in_repo_root.exists(),
+            expectations.file_in_repo_root_is_writable
+        );
+
+        assert_eq!(
+            touch(&self.file_in_dot_git_dir, policy).await,
+            expectations.file_in_dot_git_dir_is_writable
+        );
+        assert_eq!(
+            self.file_in_dot_git_dir.exists(),
+            expectations.file_in_dot_git_dir_is_writable
+        );
+    }
+}
+
+/// If the user has added a workspace root that is not a Git repo root, then
+/// the user has to specify `--skip-git-repo-check` or go through some
+/// interstitial that indicates they are taking on some risk because Git
+/// cannot be used to backup their work before the agent begins.
+///
+/// Because the user has agreed to this risk, we do not try find all .git
+/// folders in the workspace and block them (though we could change our
+/// position on this in the future).
+#[tokio::test]
+async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() {
+    let tmp = TempDir::new().expect("should be able to create temp dir");
+    let test_scenario = create_test_scenario(&tmp);
+    let policy = SandboxPolicy::WorkspaceWrite {
+        writable_roots: vec![test_scenario.repo_parent.clone()],
+        network_access: false,
+        use_exact_writable_roots: true,
+    };
+
+    test_scenario
+        .run_test(
+            &policy,
+            TestExpectations {
+                file_outside_repo_is_writable: true,
+                file_in_repo_root_is_writable: true,
+                file_in_dot_git_dir_is_writable: true,
+            },
+        )
+        .await;
+}
+
+/// When the writable root is the root of a Git repository (as evidenced by the
+/// presence of a .git folder), then the .git folder should be read-only if
+/// the policy is `WorkspaceWrite`.
+#[tokio::test]
+async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() {
+    let tmp = TempDir::new().expect("should be able to create temp dir");
+    let test_scenario = create_test_scenario(&tmp);
+    let policy = SandboxPolicy::WorkspaceWrite {
+        writable_roots: vec![test_scenario.repo_root.clone()],
+        network_access: false,
+        use_exact_writable_roots: true,
+    };
+
+    test_scenario
+        .run_test(
+            &policy,
+            TestExpectations {
+                file_outside_repo_is_writable: false,
+                file_in_repo_root_is_writable: true,
+                file_in_dot_git_dir_is_writable: false,
+            },
+        )
+        .await;
+}
+
+/// Under DangerFullAccess, all writes should be permitted anywhere on disk,
+/// including inside the .git folder.
+#[tokio::test]
+async fn danger_full_access_allows_all_writes() {
+    let tmp = TempDir::new().expect("should be able to create temp dir");
+    let test_scenario = create_test_scenario(&tmp);
+    let policy = SandboxPolicy::DangerFullAccess;
+
+    test_scenario
+        .run_test(
+            &policy,
+            TestExpectations {
+                file_outside_repo_is_writable: true,
+                file_in_repo_root_is_writable: true,
+                file_in_dot_git_dir_is_writable: true,
+            },
+        )
+        .await;
+}
+
+/// Under ReadOnly, writes should not be permitted anywhere on disk.
+#[tokio::test]
+async fn read_only_forbids_all_writes() {
+    let tmp = TempDir::new().expect("should be able to create temp dir");
+    let test_scenario = create_test_scenario(&tmp);
+    let policy = SandboxPolicy::ReadOnly;
+
+    test_scenario
+        .run_test(
+            &policy,
+            TestExpectations {
+                file_outside_repo_is_writable: false,
+                file_in_repo_root_is_writable: false,
+                file_in_dot_git_dir_is_writable: false,
+            },
+        )
+        .await;
+}
+
+fn create_test_scenario(tmp: &TempDir) -> TestScenario {
+    let repo_parent = tmp.path().to_path_buf();
+    let repo_root = repo_parent.join("repo");
+    let dot_git_dir = repo_root.join(".git");
+
+    std::fs::create_dir(&repo_root).expect("should be able to create repo root");
+    std::fs::create_dir(&dot_git_dir).expect("should be able to create .git dir");
+
+    TestScenario {
+        file_outside_repo: repo_parent.join("outside.txt"),
+        repo_parent,
+        file_in_repo_root: repo_root.join("repo_file.txt"),
+        repo_root,
+        file_in_dot_git_dir: dot_git_dir.join("dot_git_file.txt"),
+    }
+}
+
+/// Note that `path` must be absolute.
+async fn touch(path: &Path, policy: &SandboxPolicy) -> bool {
+    assert!(path.is_absolute(), "Path must be absolute: {path:?}");
+    let mut child = spawn_command_under_seatbelt(
+        vec![
+            "/usr/bin/touch".to_string(),
+            path.to_string_lossy().to_string(),
+        ],
+        policy,
+        std::env::current_dir().expect("should be able to get current dir"),
+        StdioPolicy::RedirectForShellTool,
+        HashMap::new(),
+    )
+    .await
+    .expect("should be able to spawn command under seatbelt");
+    child
+        .wait()
+        .await
+        .expect("should be able to wait for child process")
+        .success()
+}