computerd: let the caller choose the exec shell

.changeset/selectable-exec-shell.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/computerd": minor
+---
+
+The exec runner now takes an optional shell naming the interpreter each command runs under, and computerd reads the same value from EXEC_SHELL. Both default to /bin/sh, so existing behavior is unchanged. On a Debian-family image /bin/sh is dash, where bash-only syntax such as the PIPESTATUS array is a parse error that aborts the command rather than a missing feature, and that array is how a caller recovers the real exit status of a pipeline whose output it filters. Repointing /bin/sh in the image was the only previous workaround, which changes echo semantics for every other script in that image and is unavailable when the image is prebuilt.

packages/computerd/README.md

@@ -113,10 +113,13 @@ Additional environment variables:
 
 ```sh
 EXEC_LOG_MAX_BYTES=1048576        # cap the in-memory exec log buffer (bytes)
+EXEC_SHELL=/usr/bin/bash          # interpreter exec runs commands under (default /bin/sh)
 RPC_CLIENT_SECRET=<secret>        # require Authorization: Bearer <secret> on every route but /health
 COMPUTER_VAR_NODE_ENV=production  # forwarded into exec as NODE_ENV
 ```
 
+`EXEC_SHELL` must be an absolute path. It exists because `/bin/sh` is `dash` on a Debian-family image, where bash-only syntax is a parse error that aborts the command rather than a missing feature: `${PIPESTATUS[@]}`, arrays, `[[ ... ]]`, and process substitution all fail that way. `PIPESTATUS` is the usual way to recover the real exit status of a pipeline whose output is filtered — a command redacting a credential through `sed`, for instance — so a caller that needs it can select an interpreter that has it without repointing `/bin/sh` for every other script in the image.
+
 `FUSE_MOUNT=auto` is the friendly default: if `/dev/fuse` (or macFUSE) is available `computerd` mounts a real FUSE filesystem, otherwise it transparently falls back to the userspace shim. Pin the value (`fuse` / `macfuse` / `shim` / `none`) when a test needs to assert a specific code path.
 
 ## `FUSE_MOUNT=shim` — userspace dev shim

packages/computerd/src/cli/computerd.ts

@@ -636,12 +636,23 @@ async function main(): Promise<void> {
     }
     logMaxBytesOverride = parsed;
   }
+  // EXEC_SHELL picks the interpreter exec runs commands under, for images
+  // whose /bin/sh cannot be repointed. Default lives in the Runner.
+  const shellEnv = process.env.EXEC_SHELL;
+  let shellOverride: string | undefined;
+  if (shellEnv !== undefined && shellEnv !== "") {
+    if (!shellEnv.startsWith("/")) {
+      throw new Error(`EXEC_SHELL must be an absolute path; got ${JSON.stringify(shellEnv)}`);
+    }
+    shellOverride = shellEnv;
+  }
   const runner = new Runner({
     db,
     // When we have a mount (real FUSE or the shim) point spawned
     // children at it so writes from exec flow through the VFS.
     ...(fuse !== undefined ? { cwd: mountPoint } : {}),
     ...(logMaxBytesOverride !== undefined ? { logMaxBytes: logMaxBytesOverride } : {}),
+    ...(shellOverride !== undefined ? { shell: shellOverride } : {}),
   });
   // Heartbeat events are computerd-local and must not cross the RPC boundary.
   // Wrap the runner so every exec/get stream drops heartbeat events before

packages/computerd/src/exec/runner.test.ts

@@ -1,3 +1,5 @@
+import { existsSync } from "node:fs";
+
 import { Database, initializeSchema, WorkspaceFilesystem } from "@cloudflare/dofs";
 import { SQLiteTestStorage } from "@cloudflare/dofs/testing";
 import { expect, test } from "vitest";
@@ -517,3 +519,74 @@ test("a spawned command sees the allowlisted environment, not the daemon's", asy
     dispose();
   }
 });
+
+// The interpreter is a per-consumer choice, not a property of the image.
+//
+// A caller that redacts credentials through a pipe -- `git push "$URL" 2>&1 |
+// sed -E 's#//[^@]*@#//***@#'` -- gets the pipeline's last exit status, so a
+// failed push reads as success. PIPESTATUS is the usual recovery, and under
+// dash it is a parse error that aborts the command rather than a missing
+// feature, which is worse than the problem it was reached for.
+const hasBash = existsSync("/usr/bin/bash");
+
+test("defaults to /bin/sh when no shell is given", async () => {
+  const { runner, dispose } = fixture();
+  try {
+    const handle = runner.exec("printf '%s' \"$0\"");
+    const events = await drain(handle.events);
+    const stdout = events
+      .filter((event) => event.name === "stdout")
+      .map((event) => decode(event.value as Uint8Array))
+      .join("");
+    expect(stdout).toBe("/bin/sh");
+  } finally {
+    dispose();
+  }
+});
+
+test.skipIf(!hasBash)("runs commands under an explicitly chosen shell", async () => {
+  const { runner, dispose } = fixture({ shell: "/usr/bin/bash" });
+  try {
+    const handle = runner.exec("printf '%s' \"$0\"");
+    const events = await drain(handle.events);
+    const stdout = events
+      .filter((event) => event.name === "stdout")
+      .map((event) => decode(event.value as Uint8Array))
+      .join("");
+    expect(stdout).toBe("/usr/bin/bash");
+  } finally {
+    dispose();
+  }
+});
+
+test.skipIf(!hasBash)("a chosen shell resolves PIPESTATUS instead of aborting", async () => {
+  const { runner, dispose } = fixture({ shell: "/usr/bin/bash" });
+  try {
+    // false | true leaves $? as true's 0 while the first pipeline stage's real
+    // failure survives in the PIPESTATUS array. The trailing marker proves the
+    // command was not aborted: under dash the expansion is fatal and "after"
+    // never prints. The expansion is assembled from parts so its braces are
+    // not linted as a JavaScript template placeholder.
+    const first = ['"$', "{PIPESTATUS[0]}", '"'].join("");
+    const handle = runner.exec(`false | true; printf '[%s]' ${first}; printf "after"`);
+    const events = await drain(handle.events);
+    const stdout = events
+      .filter((event) => event.name === "stdout")
+      .map((event) => decode(event.value as Uint8Array))
+      .join("");
+    expect(stdout).toBe("[1]after");
+  } finally {
+    dispose();
+  }
+});
+
+test("rejects a shell that is not an absolute path", () => {
+  const storage = new SQLiteTestStorage();
+  const db = new Database(storage);
+  initializeSchema(db, () => Date.now());
+  try {
+    expect(() => new Runner({ db, shell: "bash" })).toThrow(/absolute path/);
+  } finally {
+    storage.close?.();
+  }
+});

packages/computerd/src/exec/runner.ts

@@ -61,6 +61,7 @@ const DEFAULTS = {
   // After SIGTERM, give the child this long to exit on its own
   // before sending SIGKILL.
   killGraceMs: 5_000,
+  shell: "/bin/sh",
 } as const;
 
 export interface RunnerInit extends RunnerOptions {
@@ -80,6 +81,7 @@ export class Runner {
     sweepIntervalMs: number;
     defaultTimeoutMs: number;
     heartbeatIntervalMs: number;
+    shell: string;
     now: () => number;
   };
   private readonly records = new Map<string, ExecRecord>();
@@ -96,8 +98,14 @@ export class Runner {
       sweepIntervalMs: init.sweepIntervalMs ?? DEFAULTS.sweepIntervalMs,
       defaultTimeoutMs: init.defaultTimeoutMs ?? DEFAULTS.defaultTimeoutMs,
       heartbeatIntervalMs: init.heartbeatIntervalMs ?? 0,
+      shell: init.shell ?? DEFAULTS.shell,
       now: init.now ?? Date.now,
     };
+    // Fail here rather than letting every exec() surface a bare ENOENT from
+    // spawn, which names the interpreter but not the misconfiguration.
+    if (!this.opts.shell.startsWith("/")) {
+      throw new Error(`shell must be an absolute path; got ${JSON.stringify(this.opts.shell)}`);
+    }
     initializeExecSchema(this.db);
     if (init.resetSchema !== false) clearExecState(this.db);
   }
@@ -131,7 +139,7 @@ export class Runner {
     // status pipe. If cwd lives inside computerd's own FUSE mount, the
     // child's chdir issues a FUSE LOOKUP that computerd can't service
     // (its event loop is stuck in uv_spawn), and the whole
-    // process deadlocks. Have /bin/sh do the chdir instead: by
+    // process deadlocks. Have the shell do the chdir instead: by
     // the time the shell runs its `cd`, computerd's event loop is back
     // and can answer the FUSE callback normally.
     //
@@ -143,7 +151,7 @@ export class Runner {
     // (`spawn("/bin/sh", ["-c", command])`) was already a shell-
     // owned exec, so this is no change in process shape.
     const wrapped = cwd !== undefined ? `cd ${shellQuote(cwd)} && ${command}` : command;
-    const child = spawn("/bin/sh", ["-c", wrapped], {
+    const child = spawn(this.opts.shell, ["-c", wrapped], {
       env,
       stdio: [options.stdin === undefined ? "ignore" : "pipe", "pipe", "pipe"],
     });

packages/computerd/src/exec/types.ts

@@ -62,6 +62,13 @@ export interface RunnerOptions {
   // Emit a heartbeat event every this many milliseconds while a child
   // process is alive. When unset or zero, no heartbeat events are emitted.
   heartbeatIntervalMs?: number;
+  // Absolute path to the interpreter each command runs under. Defaults to
+  // /bin/sh, which on a Debian-family image is dash: bash-only syntax a
+  // caller may reach for, notably ${PIPESTATUS[@]}, is a parse error there
+  // and aborts the command rather than degrading. Not inferred from SHELL,
+  // which names the caller's login shell and is deliberately absent from the
+  // env allowlist.
+  shell?: string;
   // Test seam: replaces Date.now() for retention math and log ts.
   now?: () => number;
 }