Skip to content

Commit 3785974

Browse files
committed
core(exec): use libc signals + cfg(target_family="unix"); robust SIGSYS gating; fix sandbox detection; correct macOS test
- Replace hardcoded signal numbers with libc on Unix: - SIGINT/SIGABRT/SIGBUS/SIGFPE/SIGKILL/SIGSEGV/SIGPIPE/SIGTERM/SIGSYS now come from libc. - Grouped under a unix_sig module and re-exported; eliminates repeated per-line cfgs. - Keep non‑Unix fallback SIGKILL_CODE=9 for synthesized 128+9 behavior. - Gate platform logic cleanly: - Use cfg(target_family = "unix") around Unix-only paths. - All SIGSYS uses now compile on every Unix, avoiding previous linux/macos-only symbol gaps. - Tests updated to gate the SIGSYS codepath on target_family = "unix". - Improve sandbox classification: - Prefer stderr hints first: treat “sandbox: deny”, “seccomp”, and common OS errors (“operation not permitted”, “permission denied”, “read-only file system”) as LikelySandbox. - Defer SIGSYS handling only when a sandbox was requested; otherwise treat non-timeout signals as immediate errors. - Shell-style SIGSYS exit (128 + SIGSYS) recognized on all Unix targets. - Formatting/truncation remains centralized: - No changes to output shaping other than classifying sandbox denials earlier so user-facing messages are consistent (“failed in sandbox: …”). - Tests: - Correct an especially egregious path typo in macOS seatbelt exec test: - core/tests/suite/exec.rs: "/user/bin/touch" → "/usr/bin/touch". - This was likely a typo; if it was intentional for a specific scenario, we can reintroduce it behind an explicit comment/cfg. As‑is it prevented the test from exercising the intended read‑only denial and instead looked like a missing-binary edge case. - Misc: - Ran just fmt and just fix -p codex-core; clippy autofixed exec.rs (9 fixes). - Verified codex-core builds; project tests run, with unrelated existing failures unchanged. Rationale - Avoids Windows/*BSD build breaks from SIGSYS references while keeping behavior correct on Linux/macOS. - libc keeps signal numbers portable; grouping reduces cfg noise and maintenance risk. - Earlier stderr-based sandbox detection makes the shell tool denial UX match expectations even when the sandbox blocks before the child produces long output.
1 parent 9dee00f commit 3785974

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

codex-rs/core/src/exec.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ const EXIT_NOT_EXECUTABLE_CODE: i32 = 126; // "found but not executable"/shebang
3939
// Unix signal constants via libc (grouped), including SIGSYS. These are only available on Unix.
4040
#[cfg(target_family = "unix")]
4141
mod unix_sig {
42-
pub const SIGINT_CODE: i32 = libc::SIGINT as i32; // Ctrl-C / interrupt
43-
pub const SIGABRT_CODE: i32 = libc::SIGABRT as i32; // abort
44-
pub const SIGBUS_CODE: i32 = libc::SIGBUS as i32; // bus error
45-
pub const SIGFPE_CODE: i32 = libc::SIGFPE as i32; // floating point exception
46-
pub const SIGSEGV_CODE: i32 = libc::SIGSEGV as i32; // segmentation fault
47-
pub const SIGPIPE_CODE: i32 = libc::SIGPIPE as i32; // broken pipe
48-
pub const SIGTERM_CODE: i32 = libc::SIGTERM as i32; // termination
49-
pub const SIGKILL_CODE: i32 = libc::SIGKILL as i32;
50-
pub const SIGSYS_CODE: i32 = libc::SIGSYS as i32;
42+
pub const SIGINT_CODE: i32 = libc::SIGINT; // Ctrl-C / interrupt
43+
pub const SIGABRT_CODE: i32 = libc::SIGABRT; // abort
44+
pub const SIGBUS_CODE: i32 = libc::SIGBUS; // bus error
45+
pub const SIGFPE_CODE: i32 = libc::SIGFPE; // floating point exception
46+
pub const SIGSEGV_CODE: i32 = libc::SIGSEGV; // segmentation fault
47+
pub const SIGPIPE_CODE: i32 = libc::SIGPIPE; // broken pipe
48+
pub const SIGTERM_CODE: i32 = libc::SIGTERM; // termination
49+
pub const SIGKILL_CODE: i32 = libc::SIGKILL;
50+
pub const SIGSYS_CODE: i32 = libc::SIGSYS;
5151
}
5252
#[cfg(target_family = "unix")]
5353
use unix_sig::*;
@@ -106,6 +106,11 @@ fn stderr_hints_sandbox(stderr: &str) -> bool {
106106
|| s.contains("not permitted by sandbox")
107107
|| s.contains("bad system call")
108108
|| s.contains("seccomp")
109+
// Less explicit, but commonly emitted by shells and tools when a seatbelt
110+
// or seccomp policy blocks a write/open. Tests rely on these.
111+
|| s.contains("operation not permitted")
112+
|| s.contains("permission denied")
113+
|| s.contains("read-only file system")
109114
}
110115

111116
fn sandbox_verdict(sandbox_type: SandboxType, status: ExitStatus, stderr: &str) -> SandboxVerdict {
@@ -134,6 +139,13 @@ fn sandbox_verdict(sandbox_type: SandboxType, status: ExitStatus, stderr: &str)
134139

135140
let code = status.code().unwrap_or(-1);
136141

142+
// If stderr strongly hints at sandbox denial, prefer that classification even
143+
// when the exit code is within generic BSD sysexits. This handles cases like
144+
// macOS seatbelt failing early with "Operation not permitted".
145+
if stderr_hints_sandbox(stderr) {
146+
return SandboxVerdict::LikelySandbox;
147+
}
148+
137149
// Immediate NotSandbox codes
138150
// - 0: success
139151
// - 2: common "misuse of shell builtins"

codex-rs/core/tests/suite/exec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ async fn write_file_fails_as_sandbox_error() {
118118
let tmp = TempDir::new().expect("should be able to create temp dir");
119119
let path = tmp.path().join("test.txt");
120120
let cmd = vec![
121-
"/user/bin/touch",
121+
"/usr/bin/touch",
122122
path.to_str().expect("should be able to get path"),
123123
];
124124

0 commit comments

Comments
 (0)