fix: tighten up check for /usr/bin/sandbox-exec

This commit is contained in:
Michael Bolin
2025-04-28 12:13:52 -07:00
parent 77e2918049
commit decb0cc953
3 changed files with 28 additions and 14 deletions

View File

@@ -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";
// ---------------------------------------------------------------------------
// Sessionlevel 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<boolean> =>
new Promise((res) =>
execFile(
"command",
["-v", "sandbox-exec"],
{ signal: AbortSignal.timeout(200) },
(err) => res(!err), // exit 0 ⇒ found
),
const isSandboxExecAvailable: Promise<boolean> = 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<SandboxType> {
@@ -295,7 +299,7 @@ async function getSandbox(runInSandbox: boolean): Promise<SandboxType> {
// 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(

View File

@@ -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<string>,
opts: SpawnOptions,
@@ -57,7 +65,7 @@ export function execWithSeatbelt(
);
const fullCommand = [
"sandbox-exec",
PATH_TO_SEATBELT_EXECUTABLE,
"-p",
fullPolicy,
...policyTemplateParams,

View File

@@ -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<String>,
@@ -186,7 +188,7 @@ pub fn create_seatbelt_command(
};
let mut seatbelt_command: Vec<String> = vec![
"sandbox-exec".to_string(),
MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string(),
"-p".to_string(),
full_policy.to_string(),
];