fix: remove unused _writableRoots arg to exec() function (#762)

I suspect this was done originally so that `execForSandbox()` had a
consistent signature for both the `SandboxType.NONE` and
`SandboxType.MACOS_SEATBELT` cases, but that is not really necessary and
turns out to make the upcoming Landlock support a bit more complicated
to implement, so I had Codex remove it and clean up the call sites.
This commit is contained in:
Michael Bolin
2025-04-30 14:08:27 -07:00
committed by GitHub
parent e6fe8d6fa1
commit 033d379eca
6 changed files with 11 additions and 10 deletions

View File

@@ -45,9 +45,6 @@ export function exec(
// This is a temporary measure to understand what are the common base commands // This is a temporary measure to understand what are the common base commands
// until we start persisting and uploading rollouts // until we start persisting and uploading rollouts
const execForSandbox =
sandbox === SandboxType.MACOS_SEATBELT ? execWithSeatbelt : rawExec;
const opts: SpawnOptions = { const opts: SpawnOptions = {
timeout: timeoutInMillis || DEFAULT_TIMEOUT_MS, timeout: timeoutInMillis || DEFAULT_TIMEOUT_MS,
...(requiresShell(cmd) ? { shell: true } : {}), ...(requiresShell(cmd) ? { shell: true } : {}),
@@ -59,7 +56,12 @@ export function exec(
os.tmpdir(), os.tmpdir(),
...additionalWritableRoots, ...additionalWritableRoots,
]; ];
return execForSandbox(cmd, opts, writableRoots, abortSignal); if (sandbox === SandboxType.MACOS_SEATBELT) {
return execWithSeatbelt(cmd, opts, writableRoots, abortSignal);
}
// SandboxType.NONE (or any other) falls back to the raw exec implementation
return rawExec(cmd, opts, abortSignal);
} }
export function execApplyPatch( export function execApplyPatch(

View File

@@ -72,7 +72,7 @@ export function execWithSeatbelt(
"--", "--",
...cmd, ...cmd,
]; ];
return exec(fullCommand, opts, writableRoots, abortSignal); return exec(fullCommand, opts, abortSignal);
} }
const READ_ONLY_SEATBELT_POLICY = ` const READ_ONLY_SEATBELT_POLICY = `

View File

@@ -20,7 +20,6 @@ import * as os from "os";
export function exec( export function exec(
command: Array<string>, command: Array<string>,
options: SpawnOptions, options: SpawnOptions,
_writableRoots: ReadonlyArray<string>,
abortSignal?: AbortSignal, abortSignal?: AbortSignal,
): Promise<ExecResult> { ): Promise<ExecResult> {
// Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows) // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows)

View File

@@ -14,7 +14,7 @@ describe("exec cancellation", () => {
const cmd = ["node", "-e", "setTimeout(() => console.log('late'), 5000);"]; const cmd = ["node", "-e", "setTimeout(() => console.log('late'), 5000);"];
const start = Date.now(); const start = Date.now();
const promise = rawExec(cmd, {}, [], abortController.signal); const promise = rawExec(cmd, {}, abortController.signal);
// Abort almost immediately. // Abort almost immediately.
abortController.abort(); abortController.abort();
@@ -38,7 +38,7 @@ describe("exec cancellation", () => {
const cmd = ["node", "-e", "console.log('finished')"]; const cmd = ["node", "-e", "console.log('finished')"];
const result = await rawExec(cmd, {}, [], abortController.signal); const result = await rawExec(cmd, {}, abortController.signal);
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
expect(result.stdout.trim()).toBe("finished"); expect(result.stdout.trim()).toBe("finished");

View File

@@ -10,7 +10,7 @@ describe("rawExec invalid command handling", () => {
it("resolves with nonzero exit code when executable is missing", async () => { it("resolves with nonzero exit code when executable is missing", async () => {
const cmd = ["definitely-not-a-command-1234567890"]; const cmd = ["definitely-not-a-command-1234567890"];
const result = await rawExec(cmd, {}, []); const result = await rawExec(cmd, {});
expect(result.exitCode).not.toBe(0); expect(result.exitCode).not.toBe(0);
expect(result.stderr.length).toBeGreaterThan(0); expect(result.stderr.length).toBeGreaterThan(0);

View File

@@ -33,7 +33,7 @@ describe("rawExec abort kills entire process group", () => {
// - prints the PID of the `sleep` // - prints the PID of the `sleep`
// - waits for `sleep` to exit // - waits for `sleep` to exit
const { stdout, exitCode } = await (async () => { const { stdout, exitCode } = await (async () => {
const p = rawExec(cmd, {}, [], abortController.signal); const p = rawExec(cmd, {}, abortController.signal);
// Give Bash a tiny bit of time to start and print the PID. // Give Bash a tiny bit of time to start and print the PID.
await new Promise((r) => setTimeout(r, 100)); await new Promise((r) => setTimeout(r, 100));