fix: move general sandbox tests to codex-rs/core/tests/sandbox.rs (#2317)

Previous to this PR, `codex-rs/core/tests/sandbox.rs` contained
integration tests that were specific to Seatbelt. This PR moves those
tests to `codex-rs/core/src/seatbelt.rs` and designates
`codex-rs/core/tests/sandbox.rs` to be used as the home for
cross-platform (well, Mac and Linux...) sandbox tests.

To start, this migrates
`python_multiprocessing_lock_works_under_seatbelt()` from #1823 to the
new `sandbox.rs` because this is the type of thing that should work on
both Mac _and_ Linux, though I still need to do some work to clean up
the test so it works on both platforms.

codex-rs/core/src/seatbelt.rs

@@ -301,59 +301,6 @@ mod tests {
         assert_eq!(expected_args, args);
     }
 
-    #[test]
-    fn seatbelt_base_policy_allows_ipc_posix_sem() {
-        assert!(
-            MACOS_SEATBELT_BASE_POLICY.contains("(allow ipc-posix-sem)"),
-            "base policy should allow ipc-posix-sem"
-        );
-    }
-
-    #[cfg(target_os = "macos")]
-    #[tokio::test]
-    async fn python_multiprocessing_lock_works_under_seatbelt() {
-        use super::spawn_command_under_seatbelt;
-        use crate::spawn::StdioPolicy;
-        use std::collections::HashMap;
-
-        let policy = SandboxPolicy::WorkspaceWrite {
-            writable_roots: vec![],
-            network_access: false,
-            include_default_writable_roots: true,
-        };
-
-        let python_code = r#"import multiprocessing
-from multiprocessing import Lock, Process
-
-def f(lock):
-    with lock:
-        print("Lock acquired in child process")
-
-if __name__ == '__main__':
-    lock = Lock()
-    p = Process(target=f, args=(lock,))
-    p.start()
-    p.join()
-"#;
-
-        let mut child = spawn_command_under_seatbelt(
-            vec![
-                "python3".to_string(),
-                "-c".to_string(),
-                python_code.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 python under seatbelt");
-
-        let status = child.wait().await.expect("should wait for child process");
-        assert!(status.success(), "python exited with {status:?}");
-    }
-
     struct PopulatedTmp {
         root_with_git: PathBuf,
         root_without_git: PathBuf,

codex-rs/core/tests/sandbox.rs

@@ -1,197 +1,49 @@
-#![cfg(target_os = "macos")]
+// TODO(mbolin): Update this test to run on Linux, as well.
-#![expect(clippy::expect_used)]
+// (Should rename the test as part of that work.)
-
+#[cfg(target_os = "macos")]
-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() {
+async fn python_multiprocessing_lock_works_under_seatbelt() {
-    let tmp = TempDir::new().expect("should be able to create temp dir");
+    #![expect(clippy::expect_used)]
-    let test_scenario = create_test_scenario(&tmp);
+    use codex_core::protocol::SandboxPolicy;
+    use codex_core::seatbelt::spawn_command_under_seatbelt;
+    use codex_core::spawn::StdioPolicy;
+    use std::collections::HashMap;
+
     let policy = SandboxPolicy::WorkspaceWrite {
-        writable_roots: vec![test_scenario.repo_parent.clone()],
+        writable_roots: vec![],
         network_access: false,
-        exclude_tmpdir_env_var: true,
+        exclude_tmpdir_env_var: false,
-        exclude_slash_tmp: true,
+        exclude_slash_tmp: false,
     };
 
-    test_scenario
+    let python_code = r#"import multiprocessing
-        .run_test(
+from multiprocessing import Lock, Process
-            &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
+def f(lock):
-/// presence of a .git folder), then the .git folder should be read-only if
+    with lock:
-/// the policy is `WorkspaceWrite`.
+        print("Lock acquired in child process")
-#[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,
-        exclude_tmpdir_env_var: true,
-        exclude_slash_tmp: true,
-    };
 
-    test_scenario
+if __name__ == '__main__':
-        .run_test(
+    lock = Lock()
-            &policy,
+    p = Process(target=f, args=(lock,))
-            TestExpectations {
+    p.start()
-                file_outside_repo_is_writable: false,
+    p.join()
-                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(),
+            "python3".to_string(),
-            path.to_string_lossy().to_string(),
+            "-c".to_string(),
+            python_code.to_string(),
         ],
-        policy,
+        &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");
+    .expect("should be able to spawn python under seatbelt");
-    child
+
-        .wait()
+    let status = child.wait().await.expect("should wait for child process");
-        .await
+    assert!(status.success(), "python exited with {status:?}");
-        .expect("should be able to wait for child process")
-        .success()
 }

codex-rs/core/tests/seatbelt.rs

@@ -0,0 +1,200 @@
+#![cfg(target_os = "macos")]
+#![expect(clippy::expect_used)]
+
+//! Tests for the macOS sandboxing that are specific to Seatbelt.
+//! Tests that apply to both Mac and Linux sandboxing should go in sandbox.rs.
+
+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,
+        exclude_tmpdir_env_var: true,
+        exclude_slash_tmp: 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,
+        exclude_tmpdir_env_var: true,
+        exclude_slash_tmp: 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()
+}