mirror of
https://github.com/openai/codex.git
synced 2026-04-28 16:45:54 +00:00
sandbox: remove dead seatbelt helper and update tests (#17859)
## Why `spawn_command_under_seatbelt()` in `codex-rs/core/src/seatbelt.rs` had fallen out of production use and was only referenced by test-only wrappers. That left us with sandbox tests that could stay green even if the actual seatbelt exec path regressed, because production shell execution now flows through `SandboxManager::transform()` and `ExecRequest::from_sandbox_exec_request()` instead of that helper. Removing the dead helper also exposed one downstream `codex-exec` integration test that still imported it, which broke `just clippy`. ## What Changed - Removed `codex-rs/core/src/seatbelt.rs` and stopped exporting `codex_core::seatbelt`. - Removed the redundant `codex-rs/core/tests/suite/seatbelt.rs` coverage that only exercised the dead helper. - Kept the `openpty` regression check, but moved it into `codex-rs/core/tests/suite/exec.rs` so it now runs through `process_exec_tool_call()`. - Fixed the seatbelt denial test in `codex-rs/core/tests/suite/exec.rs` to use `/usr/bin/touch`, so it actually exercises the sandbox instead of a nonexistent path. - Updated `codex-rs/exec/tests/suite/sandbox.rs` on macOS to build the sandboxed command through `build_exec_request()` and spawn the transformed command, instead of importing the removed helper. - Left the lower-level seatbelt policy coverage in `codex-rs/sandboxing/src/seatbelt_tests.rs`, where the policy generator is still covered directly. ## Verification - `cargo test -p codex-core suite::exec::` - `cargo test -p codex-exec` - `cargo clippy -p codex-exec --tests -- -D warnings`
This commit is contained in:
@@ -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"),
|
||||
];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user