feat(core[exec]): Stronger SandboxVerdict Classifier & Exec Flow

[imajes]

Why

• The previous approach treated many non‑zero exits as sandbox denials, making normal command failures look like sandbox errors.
• SIGSYS mapping was inconsistent (hardcoded to 12 while Linux uses 31), and magic integers made maintenance hard.

Pytest exit codes (context for "Why")

• 0 — All tests collected and passed.
• 1 — Tests ran but some failed (normal failing test signal).
• 2 — Execution interrupted by user (Ctrl‑C).
• 3 — Internal pytest error while executing tests.
• 4 — Command‑line usage/config error (bad CLI/options).
• 5 — No tests were collected. Intentionally non‑zero so CI fails when your test discovery/config is broken (e.g., wrong -k/-m filters, path patterns, or missing files).

Implication for Codex classification: these are typical app/tool outcomes, not sandbox denials. Treat 0/2/4/5 as LikelyNotSandbox; 1 and generic >0 without hints as Unknown (not sandbox). Only upgrade to LikelySandbox on clear evidence (e.g., SIGSYS, explicit sandbox denial text).

What

• Introduced a conservative SandboxVerdict classifier with three outcomes:
  • LikelySandbox · LikelyNotSandbox · Unknown
• Rewired process_exec_tool_call to:
  • Preserve timeouts as Timeout and non‑SIGSYS signals as Signal.
  • Defer SIGSYS classification until stderr is available.
  • Return Denied only on strong sandbox evidence.
• Removed legacy is_likely_sandbox_denied (no remaining callers).
• Replaced magic numbers with named constants for clarity and correctness.
• Improved debug logging context.

Details

• New classifier

  fn sandbox_verdict(
      sandbox_type: SandboxType,
      status: ExitStatus,
      stderr: &str,
  ) -> SandboxVerdict

• Signals

  • SIGSYS → LikelySandbox (via status.signal() or exit code 128 + SIGSYS).
  • Common app/user signals → LikelyNotSandbox:
    • SIGINT_CODE = 2, SIGABRT_CODE = 6, SIGBUS_CODE = 7, SIGFPE_CODE = 8, SIGKILL_CODE = 9, SIGSEGV_CODE = 11, SIGPIPE_CODE = 13, SIGTERM_CODE = 15.

• Exit codes

  • LikelyNotSandbox: 0, 2, 124 (timeout wrapper), 127 (not found), 64..=78 (BSD sysexits), and 128 + {signals above}.
  • EXIT_NOT_EXECUTABLE_CODE = 126 → Unknown unless stderr has explicit sandbox hints.
  • Otherwise Unknown unless hints indicate sandbox.
  • Stderr hints (case‑insensitive; conservative):
    • "sandbox: deny", "not permitted by sandbox", "bad system call", "seccomp".

• Platform constants

  • SIGSYS_CODE: macOS = 12, Linux = 31 (no libc deps).

• Exec flow changes

  • Defer SIGSYS to inspect stderr; otherwise return Signal immediately.
  • After building ExecToolCallOutput, compute verdict:
    • On LikelySandbox → Denied { output }.
    • Else if deferred signal → Signal.
    • Else return Ok(exec_output).

• Logging

  tracing::debug!(
      target: "codex_core::exec",
      exit_code, ?pending_signal, verdict=?verdict,
      "exec SandboxClassification",
  );

Removed

• is_likely_sandbox_denied shim removed; note left explaining how to synthesize an ExitStatus if ever needed again.

Refactors / Clippy

• sandbox_verdict now takes ExitStatus by value (small Copy type) to satisfy clippy::trivially-copy-pass-by-ref.
• Replaced raw lists of integers with named constants and a computed set for 128 + signal codes.

Tests

• Unit tests in core/src/exec.rs:
  • not_sandbox_obvious_codes: 127 / 124 / 2 → LikelyNotSandbox.
  • sandbox_sigsys_codepath: 128 + SIGSYS → LikelySandbox.
  • sandbox_stderr_hint: 126 + "Sandbox: deny …" → LikelySandbox.
  • unknown_generic_failure: 1 → Unknown.
• Existing integration/suite tests already cover timeout/signal/Denied and OK paths.

Manual Test Plan

• macOS seatbelt

  • Run a command that exits 126 and prints "Sandbox: deny …" to stderr → Denied.
  • Run a command that exits 127 or 2 → not Denied.

• Linux seccomp

  • Simulate bad syscall exit 128 + 31 → Denied.
  • 130 / 141 / 143 (SIGINT / SIGPIPE / SIGTERM) → not Denied; surfaced as normal failure or Signal.

• Confirm debug line appears with target codex_core::exec and message exec SandboxClassification.

Behavioral Changes

• Normal non‑zero exits (e.g., exit 1) are no longer misclassified as sandbox errors.
• Timeouts and non‑SIGSYS signals retain previous semantics.
• Denials are now attributed only on strong evidence.

Risk / Rollout

• Scope limited to core/src/exec.rs; no new dependencies.
• macOS and Linux only; no *BSD branches.
• Low risk: adds conservative classification and clearer logging; removes unused shim.

Follow‑ups (optional)

• Add a macOS‑specific stderr hint "sandbox-exec:" if we want to treat seatbelt launcher failures as LikelySandbox.
• Expand tests to include a couple more 128 + signal codes to lock in "not sandbox" classification.
• Run just fix -p codex-core and optionally a full workspace test pass once reviewers approve.

---

External (non‑OpenAI) Pull Request Requirements

Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md

Note: ✅ I didn't create the issue first as I was solving the bug I was experiencing in real time. Apologies for that.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

[imajes]

@pakrym-oai, @bolinfest -- this fixes issues introduced by #3576. The approach afforded by is_likely_sandbox_denied isn't quite enough, as it is trapping pretty much any command run in the sandbox that doesn't return 0, essentially.

This is true for things like pytest, which makes use of varied exit codes, but i would imagine anything run like e.g. bash -lc ... as that'll have varying exit codes also.

It's not easy to know whether an exit code is because of the sandbox or the process it invoked, but i think there are some heuristics we can leverage. This PR provides a heuristic classifier, which essentially looks for the class of outcomes that most likely indicates a sandbox error, and then pretty much passes through the rest -- because they're not likely to be sandbox specific.

This came about because in the last few days, i've noticed a significant number of failed in sandbox type error states where it didn't have output. When i added some tracing for the exact output from the command run, i saw that it was valid, but the exit code was non zero.

Happy to talk more, edit, etc -- but i've been running this all afternoon with great success. Obviously it'll be good to get wider feedback, but it is working so far!

[imajes]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

[chatgpt-codex-connector]

Avoid treating generic “permission denied” output as sandbox denial

The new stderr_hints_sandbox function classifies any stderr containing phrases like "permission denied" or "operation not permitted" as LikelySandbox, and process_exec_tool_call turns that verdict into a SandboxErr::Denied. Many common commands emit those exact strings for reasons unrelated to the sandbox (e.g., git clone failing with Permission denied (publickey) or an SSH server refusing credentials). When such a failure happens under seatbelt/seccomp, the code will now misreport it as a sandbox denial even though the sandbox played no role. This defeats the goal of distinguishing real sandbox denials from normal command failures and will send users down the wrong debugging path. The heuristic should be narrowed or combined with other evidence before upgrading to Denied.

Useful? React with 👍 / 👎.

[imajes]

Well, it's a first step. I agree that there's no such thing as a perfect heuristic; i wonder if we should adopt a weighted approach which assigns each of these "signals" with a weight, and then calculate the probability that it's a sandbox error.

That sounds good, but it becomes super complex. Either way, a permission denied (publickey) is likely a sandbox fail as it'll require an escalation to the user to fix.