mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
sandbox: remove dead seatbelt helper and update tests
This commit is contained in:
@@ -139,7 +139,6 @@ pub use project_doc::discover_project_doc_paths;
|
||||
pub use project_doc::read_project_docs;
|
||||
mod rollout;
|
||||
pub(crate) mod safety;
|
||||
pub mod seatbelt;
|
||||
mod session_rollout_init_error;
|
||||
pub mod shell;
|
||||
pub(crate) mod shell_snapshot;
|
||||
|
||||
@@ -1,48 +0,0 @@
|
||||
#![cfg(target_os = "macos")]
|
||||
|
||||
use crate::spawn::CODEX_SANDBOX_ENV_VAR;
|
||||
use crate::spawn::SpawnChildRequest;
|
||||
use crate::spawn::StdioPolicy;
|
||||
use crate::spawn::spawn_child_async;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
|
||||
use codex_sandboxing::seatbelt::create_seatbelt_command_args_for_policies;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use tokio::process::Child;
|
||||
|
||||
pub async fn spawn_command_under_seatbelt(
|
||||
command: Vec<String>,
|
||||
command_cwd: AbsolutePathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
sandbox_policy_cwd: &AbsolutePathBuf,
|
||||
stdio_policy: StdioPolicy,
|
||||
network: Option<&NetworkProxy>,
|
||||
mut env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
let args = create_seatbelt_command_args_for_policies(
|
||||
command,
|
||||
&FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_policy_cwd),
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
sandbox_policy_cwd,
|
||||
/*enforce_managed_network*/ false,
|
||||
network,
|
||||
);
|
||||
let arg0 = None;
|
||||
env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string());
|
||||
spawn_child_async(SpawnChildRequest {
|
||||
program: PathBuf::from(MACOS_PATH_TO_SEATBELT_EXECUTABLE),
|
||||
args,
|
||||
arg0,
|
||||
cwd: command_cwd,
|
||||
network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy),
|
||||
network,
|
||||
stdio_policy,
|
||||
env,
|
||||
})
|
||||
.await
|
||||
}
|
||||
@@ -1,8 +1,5 @@
|
||||
#![cfg(target_os = "macos")]
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::string::ToString;
|
||||
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::process_exec_tool_call;
|
||||
@@ -17,6 +14,7 @@ use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::get_platform_sandbox;
|
||||
use core_test_support::PathExt;
|
||||
use std::collections::HashMap;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn skip_test() -> bool {
|
||||
@@ -29,14 +27,18 @@ fn skip_test() -> bool {
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput> {
|
||||
async fn run_test_cmd<I, S>(tmp: TempDir, command: I) -> Result<ExecToolCallOutput>
|
||||
where
|
||||
I: IntoIterator<Item = S>,
|
||||
S: Into<String>,
|
||||
{
|
||||
let sandbox_type = get_platform_sandbox(/*windows_sandbox_enabled*/ false)
|
||||
.expect("should be able to get sandbox type");
|
||||
assert_eq!(sandbox_type, SandboxType::MacosSeatbelt);
|
||||
let cwd = tmp.path().abs();
|
||||
|
||||
let params = ExecParams {
|
||||
command: cmd.iter().map(ToString::to_string).collect(),
|
||||
command: command.into_iter().map(Into::into).collect(),
|
||||
cwd: cwd.clone(),
|
||||
expiration: 1000.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
@@ -129,6 +131,37 @@ async fn exit_command_not_found_is_ok() {
|
||||
run_test_cmd(tmp, cmd).await.unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn openpty_works_under_real_exec_seatbelt_path() {
|
||||
if skip_test() {
|
||||
return;
|
||||
}
|
||||
|
||||
let python = match which::which("python3") {
|
||||
Ok(path) => path,
|
||||
Err(_) => {
|
||||
eprintln!("python3 not found in PATH, skipping test.");
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let tmp = TempDir::new().expect("should be able to create temp dir");
|
||||
let cmd = vec![
|
||||
python.to_string_lossy().into_owned(),
|
||||
"-c".to_string(),
|
||||
r#"import os
|
||||
|
||||
master, slave = os.openpty()
|
||||
os.write(slave, b"ping")
|
||||
assert os.read(master, 4) == b"ping""#
|
||||
.to_string(),
|
||||
];
|
||||
|
||||
let output = run_test_cmd(tmp, cmd).await.unwrap();
|
||||
assert_eq!(output.stdout.text, "");
|
||||
assert_eq!(output.stderr.text, "");
|
||||
}
|
||||
|
||||
/// Writing a file fails and should be considered a sandbox error
|
||||
#[tokio::test]
|
||||
async fn write_file_fails_as_sandbox_error() {
|
||||
@@ -139,7 +172,7 @@ async fn write_file_fails_as_sandbox_error() {
|
||||
let tmp = TempDir::new().expect("should be able to create temp dir");
|
||||
let path = tmp.path().join("test.txt");
|
||||
let cmd = vec![
|
||||
"/user/bin/touch",
|
||||
"/usr/bin/touch",
|
||||
path.to_str().expect("should be able to get path"),
|
||||
];
|
||||
|
||||
|
||||
@@ -85,7 +85,6 @@ mod rmcp_client;
|
||||
mod rollout_list_find;
|
||||
mod safety_check_downgrade;
|
||||
mod search_tool;
|
||||
mod seatbelt;
|
||||
mod shell_command;
|
||||
mod shell_serialization;
|
||||
mod shell_snapshot;
|
||||
|
||||
@@ -1,316 +0,0 @@
|
||||
#![cfg(target_os = "macos")]
|
||||
|
||||
//! 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::seatbelt::spawn_command_under_seatbelt;
|
||||
use codex_core::spawn::CODEX_SANDBOX_ENV_VAR;
|
||||
use codex_core::spawn::StdioPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
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.as_path().try_into().unwrap()],
|
||||
read_only_access: Default::default(),
|
||||
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.as_path().try_into().unwrap()],
|
||||
read_only_access: Default::default(),
|
||||
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::new_read_only_policy();
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn openpty_works_under_seatbelt() {
|
||||
if std::env::var(CODEX_SANDBOX_ENV_VAR) == Ok("seatbelt".to_string()) {
|
||||
eprintln!("{CODEX_SANDBOX_ENV_VAR} is set to 'seatbelt', skipping test.");
|
||||
return;
|
||||
}
|
||||
|
||||
if which::which("python3").is_err() {
|
||||
eprintln!("python3 not found in PATH, skipping test.");
|
||||
return;
|
||||
}
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
let command_cwd = AbsolutePathBuf::current_dir().expect("getcwd");
|
||||
let sandbox_cwd = command_cwd.clone();
|
||||
|
||||
let mut child = spawn_command_under_seatbelt(
|
||||
vec![
|
||||
"python3".to_string(),
|
||||
"-c".to_string(),
|
||||
r#"import os
|
||||
|
||||
master, slave = os.openpty()
|
||||
os.write(slave, b"ping")
|
||||
assert os.read(master, 4) == b"ping""#
|
||||
.to_string(),
|
||||
],
|
||||
command_cwd,
|
||||
&policy,
|
||||
&sandbox_cwd,
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
/*network*/ None,
|
||||
HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("should be able to spawn python under seatbelt");
|
||||
|
||||
let status = child
|
||||
.wait()
|
||||
.await
|
||||
.expect("should be able to wait for child process");
|
||||
assert!(status.success(), "python exited with {status:?}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn java_home_finds_runtime_under_seatbelt() {
|
||||
if std::env::var(CODEX_SANDBOX_ENV_VAR) == Ok("seatbelt".to_string()) {
|
||||
eprintln!("{CODEX_SANDBOX_ENV_VAR} is set to 'seatbelt', skipping test.");
|
||||
return;
|
||||
}
|
||||
|
||||
let java_home_path = Path::new("/usr/libexec/java_home");
|
||||
if !java_home_path.exists() {
|
||||
eprintln!("/usr/libexec/java_home is not present, skipping test.");
|
||||
return;
|
||||
}
|
||||
|
||||
let baseline_output = tokio::process::Command::new(java_home_path)
|
||||
.env_remove("JAVA_HOME")
|
||||
.output()
|
||||
.await
|
||||
.expect("should be able to invoke java_home outside seatbelt");
|
||||
if !baseline_output.status.success() {
|
||||
eprintln!(
|
||||
"java_home exited with {:?} outside seatbelt, skipping test",
|
||||
baseline_output.status
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
let command_cwd = AbsolutePathBuf::current_dir().expect("getcwd");
|
||||
let sandbox_cwd = command_cwd.clone();
|
||||
|
||||
let mut env: HashMap<String, String> = std::env::vars().collect();
|
||||
env.remove("JAVA_HOME");
|
||||
env.remove(CODEX_SANDBOX_ENV_VAR);
|
||||
|
||||
let child = spawn_command_under_seatbelt(
|
||||
vec![java_home_path.to_string_lossy().to_string()],
|
||||
command_cwd,
|
||||
&policy,
|
||||
&sandbox_cwd,
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
/*network*/ None,
|
||||
env,
|
||||
)
|
||||
.await
|
||||
.expect("should be able to spawn java_home under seatbelt");
|
||||
|
||||
let output = child
|
||||
.wait_with_output()
|
||||
.await
|
||||
.expect("should be able to wait for java_home child");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"java_home under seatbelt exited with {:?}, stderr: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
assert!(
|
||||
!stdout.trim().is_empty(),
|
||||
"java_home stdout unexpectedly empty under seatbelt"
|
||||
);
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
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"),
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
/// Note that `path` must be absolute.
|
||||
async fn touch(path: &Path, policy: &SandboxPolicy) -> bool {
|
||||
assert!(path.is_absolute(), "Path must be absolute: {path:?}");
|
||||
let command_cwd = AbsolutePathBuf::current_dir().expect("getcwd");
|
||||
let sandbox_cwd = command_cwd.clone();
|
||||
let mut child = spawn_command_under_seatbelt(
|
||||
vec![
|
||||
"/usr/bin/touch".to_string(),
|
||||
path.to_string_lossy().to_string(),
|
||||
],
|
||||
command_cwd,
|
||||
policy,
|
||||
&sandbox_cwd,
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
/*network*/ None,
|
||||
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()
|
||||
}
|
||||
@@ -19,17 +19,67 @@ async fn spawn_command_under_sandbox(
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
use codex_core::seatbelt::spawn_command_under_seatbelt;
|
||||
spawn_command_under_seatbelt(
|
||||
command,
|
||||
command_cwd,
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::build_exec_request;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use std::process::Stdio;
|
||||
|
||||
let codex_linux_sandbox_exe = None;
|
||||
let exec_request = build_exec_request(
|
||||
ExecParams {
|
||||
command,
|
||||
cwd: command_cwd,
|
||||
expiration: 1000.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env,
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
},
|
||||
sandbox_policy,
|
||||
&FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, sandbox_cwd),
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
sandbox_cwd,
|
||||
stdio_policy,
|
||||
/*network*/ None,
|
||||
env,
|
||||
&codex_linux_sandbox_exe,
|
||||
/*use_legacy_landlock*/ false,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| io::Error::other(err.to_string()))?;
|
||||
|
||||
let (program, args) = exec_request
|
||||
.command
|
||||
.split_first()
|
||||
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "command args are empty"))?;
|
||||
|
||||
let mut child = tokio::process::Command::new(program);
|
||||
if let Some(arg0) = exec_request.arg0.as_deref() {
|
||||
child.arg0(arg0);
|
||||
}
|
||||
child.args(args);
|
||||
child.current_dir(exec_request.cwd);
|
||||
child.env_clear();
|
||||
child.envs(exec_request.env);
|
||||
|
||||
match stdio_policy {
|
||||
StdioPolicy::RedirectForShellTool => {
|
||||
child.stdin(Stdio::null());
|
||||
child.stdout(Stdio::piped()).stderr(Stdio::piped());
|
||||
}
|
||||
StdioPolicy::Inherit => {
|
||||
child
|
||||
.stdin(Stdio::inherit())
|
||||
.stdout(Stdio::inherit())
|
||||
.stderr(Stdio::inherit());
|
||||
}
|
||||
}
|
||||
|
||||
child.kill_on_drop(true).spawn()
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
|
||||
Reference in New Issue
Block a user