From decb0cc953897e877108e4727c75a2180403752c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 28 Apr 2025 12:13:52 -0700 Subject: [PATCH] fix: tighten up check for /usr/bin/sandbox-exec --- .../src/utils/agent/handle-exec-command.ts | 28 +++++++++++-------- .../src/utils/agent/sandbox/macos-seatbelt.ts | 10 ++++++- codex-rs/core/src/exec.rs | 4 ++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 6cb48016ad..63f8d09229 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -11,8 +11,8 @@ import { exec, execApplyPatch } from "./exec.js"; import { ReviewDecision } from "./review.js"; import { isLoggingEnabled, log } from "../logger/log.js"; import { SandboxType } from "./sandbox/interface.js"; -import { access } from "fs/promises"; -import { execFile } from "node:child_process"; +import { PATH_TO_SEATBELT_EXECUTABLE } from "./sandbox/macos-seatbelt.js"; +import fs from "fs/promises"; // --------------------------------------------------------------------------- // Session‑level cache of commands that the user has chosen to always approve. @@ -218,7 +218,7 @@ async function execCommand( let { workdir } = execInput; if (workdir) { try { - await access(workdir); + await fs.access(workdir); } catch (e) { log(`EXEC workdir=${workdir} not found, use process.cwd() instead`); workdir = process.cwd(); @@ -275,14 +275,18 @@ async function execCommand( * Return `true` if the `sandbox-exec` binary can be located. This intentionally does **not** * spawn the binary – we only care about its presence. */ -export const isSandboxExecAvailable = (): Promise => - new Promise((res) => - execFile( - "command", - ["-v", "sandbox-exec"], - { signal: AbortSignal.timeout(200) }, - (err) => res(!err), // exit 0 ⇒ found - ), +const isSandboxExecAvailable: Promise = fs + .access(PATH_TO_SEATBELT_EXECUTABLE, fs.constants.X_OK) + .then( + () => true, + (err) => { + if (!["ENOENT", "ACCESS", "EPERM"].includes(err.code)) { + log( + `Unexpected error for \`stat ${PATH_TO_SEATBELT_EXECUTABLE}\`: ${err.message}`, + ); + } + return false; + }, ); async function getSandbox(runInSandbox: boolean): Promise { @@ -295,7 +299,7 @@ async function getSandbox(runInSandbox: boolean): Promise { // instance, inside certain CI images). Attempting to spawn a missing // binary makes Node.js throw an *uncaught* `ENOENT` error further down // the stack which crashes the whole CLI. - if (await isSandboxExecAvailable()) { + if (await isSandboxExecAvailable) { return SandboxType.MACOS_SEATBELT; } else { throw new Error( diff --git a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts index 934056d9af..a01e2c63ee 100644 --- a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts +++ b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts @@ -12,6 +12,14 @@ function getCommonRoots() { ]; } +/** + * When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin` + * to defend against an attacker trying to inject a malicious version on the + * PATH. If /usr/bin/sandbox-exec has been tampered with, then the attacker + * already has root access. + */ +export const PATH_TO_SEATBELT_EXECUTABLE = "/usr/bin/sandbox-exec"; + export function execWithSeatbelt( cmd: Array, opts: SpawnOptions, @@ -57,7 +65,7 @@ export function execWithSeatbelt( ); const fullCommand = [ - "sandbox-exec", + PATH_TO_SEATBELT_EXECUTABLE, "-p", fullPolicy, ...policyTemplateParams, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 4ce07acf78..0edc96305b 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -35,6 +35,8 @@ const TIMEOUT_CODE: i32 = 64; const MACOS_SEATBELT_READONLY_POLICY: &str = include_str!("seatbelt_readonly_policy.sbpl"); +const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; + #[derive(Deserialize, Debug, Clone)] pub struct ExecParams { pub command: Vec, @@ -186,7 +188,7 @@ pub fn create_seatbelt_command( }; let mut seatbelt_command: Vec = vec![ - "sandbox-exec".to_string(), + MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string(), "-p".to_string(), full_policy.to_string(), ];