refactor(cli): make runner helpers argv-only by default

src/lib/deploy.ts

@@ -59,8 +59,8 @@ export interface DeployExecutionOptions {
   getCredential: (key: string) => string | null;
   validateName: (value: string, label: string) => string;
   shellQuote: (value: string) => string;
-  run: (command: string, opts?: { ignoreError?: boolean }) => void;
-  runInteractive: (command: string) => void;
+  run: (command: readonly string[], opts?: { ignoreError?: boolean }) => void;
+  runInteractive: (command: readonly string[]) => void;
   execFileSync: (file: string, args: string[], opts?: ExecLikeOptions) => string;
   spawnSync: (file: string, args: string[], opts?: ExecLikeOptions) => void;
   log: (message?: string) => void;
@@ -264,7 +264,6 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
   }
 
   const name = validateName(instanceName, "instance name");
-  const qname = shellQuote(name);
   const gpu = env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1";
   const brevProvider = String(env.NEMOCLAW_BREV_PROVIDER || "gcp")
     .trim()
@@ -324,12 +323,12 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
 
   if (!exists) {
     log(`  Creating Brev instance '${name}' (${gpu}, provider=${brevProvider})...`);
-    run(`brev create ${qname} --type ${shellQuote(gpu)} --provider ${shellQuote(brevProvider)}`);
+    run(["brev", "create", name, "--type", gpu, "--provider", brevProvider]);
   } else {
     log(`  Brev instance '${name}' already exists.`);
   }
 
-  run("brev refresh", { ignoreError: true });
+  run(["brev", "refresh"], { ignoreError: true });
 
   stdoutWrite("  Waiting for Brev instance readiness ");
   for (let i = 0; i < 60; i++) {
@@ -411,10 +410,24 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
     const remoteDir = `${remoteHome}/nemoclaw`;
 
     log("  Syncing NemoClaw to VM...");
-    run(`ssh ${sshOpts} ${qname} 'mkdir -p ${shellQuote(remoteDir)}'`);
-    run(
-      `rsync -az --delete --exclude node_modules --exclude .git --exclude dist --exclude .venv -e "ssh ${sshOpts}" "${rootDir}/" ${qname}:${shellQuote(`${remoteDir}/`)}`,
-    );
+    run(["ssh", ...sshArgs, name, `mkdir -p ${shellQuote(remoteDir)}`]);
+    run([
+      "rsync",
+      "-az",
+      "--delete",
+      "--exclude",
+      "node_modules",
+      "--exclude",
+      ".git",
+      "--exclude",
+      "dist",
+      "--exclude",
+      ".venv",
+      "-e",
+      `ssh ${sshOpts}`,
+      `${rootDir}/`,
+      `${name}:${remoteDir}/`,
+    ]);
 
     const envLines = buildDeployEnvLines({
       env,
@@ -427,8 +440,8 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
     const envTmp = path.join(envDir, "env");
     fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 });
     try {
-      run(`scp -q ${sshOpts} ${shellQuote(envTmp)} ${qname}:${shellQuote(`${remoteDir}/.env`)}`);
-      run(`ssh -q ${sshOpts} ${qname} 'chmod 600 ${shellQuote(`${remoteDir}/.env`)}'`);
+      run(["scp", "-q", ...sshArgs, envTmp, `${name}:${remoteDir}/.env`]);
+      run(["ssh", "-q", ...sshArgs, name, `chmod 600 ${shellQuote(`${remoteDir}/.env`)}`]);
     } finally {
       try {
         fs.unlinkSync(envTmp);
@@ -443,9 +456,13 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
     }
 
     log("  Running setup...");
-    runInteractive(
-      `ssh -t ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/install.sh --non-interactive --yes-i-accept-third-party-software'`,
-    );
+    runInteractive([
+      "ssh",
+      "-t",
+      ...sshArgs,
+      name,
+      `cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/install.sh --non-interactive --yes-i-accept-third-party-software`,
+    ]);
 
     if (
       !skipStartServices &&
@@ -454,9 +471,12 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
         credentials.SLACK_BOT_TOKEN)
     ) {
       log("  Starting services...");
-      run(
-        `ssh ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/start-services.sh'`,
-      );
+      run([
+        "ssh",
+        ...sshArgs,
+        name,
+        `cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/start-services.sh`,
+      ]);
     }
 
     if (skipStartServices) {
@@ -474,9 +494,13 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise<void>
     log("");
     log("  Connecting to sandbox...");
     log("");
-    runInteractive(
-      `ssh -t ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && openshell sandbox connect ${shellQuote(sandboxName)}'`,
-    );
+    runInteractive([
+      "ssh",
+      "-t",
+      ...sshArgs,
+      name,
+      `cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && openshell sandbox connect ${shellQuote(sandboxName)}`,
+    ]);
   } finally {
     fs.rmSync(khDir, { recursive: true, force: true });
   }

src/lib/onboard.ts

@@ -26,7 +26,7 @@ const LOCAL_INFERENCE_TIMEOUT_SECS = envInt("NEMOCLAW_LOCAL_INFERENCE_TIMEOUT",
  *  Covers CSI (color, erase, cursor), OSC, and C1 two-byte escapes per ECMA-48. */
 const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g;
 const runner: typeof import("./runner") = require("./runner");
-const { ROOT, SCRIPTS, redact, run, runCapture, runFile, shellQuote, validateName } = runner;
+const { ROOT, SCRIPTS, redact, run, runShell, runCapture, runFile, shellQuote, validateName } = runner;
 const errnoUtils: typeof import("./errno") = require("./errno");
 const { isErrnoException } = errnoUtils;
 
@@ -233,7 +233,7 @@ const BACK_TO_SELECTION = "__NEMOCLAW_BACK_TO_SELECTION__";
 function verifyGatewayContainerRunning() {
   const containerName = `openshell-cluster-${GATEWAY_NAME}`;
   const result = run(
-    `docker inspect --type container --format '{{.State.Running}}' ${containerName}`,
+    ["docker", "inspect", "--type", "container", "--format", "{{.State.Running}}", containerName],
     { ignoreError: true, suppressOutput: true },
   );
   if (result.status === 0 && String(result.stdout || "").trim() === "true") {
@@ -1912,11 +1912,7 @@ function destroyGateway() {
   }
   // openshell gateway destroy doesn't remove Docker volumes, which leaves
   // corrupted cluster state that breaks the next gateway start. Clean them up.
-  // Shell required: pipe (|), && chaining, || fallback.
-  run(
-    `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm || true`,
-    { ignoreError: true },
-  );
+  removeGatewayClusterVolumes();
 }
 
 function getGatewayClusterContainerState(): string {
@@ -1966,6 +1962,24 @@ function buildGatewayClusterExecArgv(script: string): string[] {
   return ["docker", "exec", getGatewayClusterContainerName(), "sh", "-lc", script];
 }
 
+function getGatewayClusterVolumeNames(): string[] {
+  return runCapture(["docker", "volume", "ls", "-q", "--filter", `name=${getGatewayClusterContainerName()}`], {
+    ignoreError: true,
+  })
+    .split(/\r?\n/)
+    .map((line) => line.trim())
+    .filter(Boolean);
+}
+
+function removeGatewayClusterVolumes(opts: { suppressOutput?: boolean } = {}): void {
+  const names = getGatewayClusterVolumeNames();
+  if (names.length === 0) return;
+  run(["docker", "volume", "rm", ...names], {
+    ignoreError: true,
+    ...(opts.suppressOutput ? { suppressOutput: true } : {}),
+  });
+}
+
 function hostCommandExists(commandName: string): boolean {
   return !!runCapture(["sh", "-c", 'command -v "$1"', "--", commandName], {
     ignoreError: true,
@@ -2556,10 +2570,7 @@ async function preflight(): Promise<ReturnType<typeof nim.detectGpu>> {
         suppressOutput: true,
       });
       if (postInspectResult.status !== 0) {
-        run(
-          `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm 2>/dev/null || true`,
-          { ignoreError: true, suppressOutput: true },
-        );
+        removeGatewayClusterVolumes({ suppressOutput: true });
         registry.clearAll();
         console.log("  ✓ Orphaned gateway container removed");
       } else {
@@ -2597,7 +2608,7 @@ async function preflight(): Promise<ReturnType<typeof nim.detectGpu>> {
           console.log(
             `  Cleaning up orphaned SSH port-forward on port ${port} (PID ${portCheck.pid})...`,
           );
-          run(`kill ${portCheck.pid} 2>/dev/null || true`, { ignoreError: true });
+          run(["kill", String(portCheck.pid)], { ignoreError: true });
           sleep(1);
           portCheck = await checkPortAvailable(port);
           if (portCheck.ok) {
@@ -4621,7 +4632,7 @@ async function setupNim(gpu: ReturnType<typeof nim.detectGpu>): Promise<{
           // because WSL2 relays IPv4-only sockets to the Windows host.
           // Shell required: backgrounding (&), env var prefix, output redirection.
           const ollamaEnv = isWsl() ? "" : `OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} `;
-          run(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true });
+          runShell(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true });
           sleep(2);
           if (!isWsl()) printOllamaExposureWarning();
         }
@@ -4702,11 +4713,11 @@ async function setupNim(gpu: ReturnType<typeof nim.detectGpu>): Promise<{
           run(["brew", "install", "ollama"], { ignoreError: true });
         } else {
           console.log("  Installing Ollama via official installer...");
-          run("set -o pipefail; curl -fsSL https://ollama.com/install.sh | sh");
+          runShell("set -o pipefail; curl -fsSL https://ollama.com/install.sh | sh");
         }
         console.log("  Starting Ollama...");
         // Shell required: backgrounding (&), env var prefix, output redirection.
-        run(`OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} ollama serve > /dev/null 2>&1 &`, {
+        runShell(`OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} ollama serve > /dev/null 2>&1 &`, {
           ignoreError: true,
         });
         sleep(2);

src/lib/runner-argv.test.ts

@@ -46,9 +46,10 @@ describe("run with argv array", () => {
     expect(result).toContain("rm");
   });
 
-  it("still works with string commands (legacy path)", () => {
-    const result = runner.run("echo hello", { suppressOutput: true });
-    expect(result.status).toBe(0);
+  it("rejects string commands", () => {
+    expect(() => runner.run("echo hello", { suppressOutput: true })).toThrow(
+      /argv array instead/,
+    );
   });
 
   it("surfaces ENOENT error for missing executables", () => {
@@ -62,6 +63,39 @@ describe("run with argv array", () => {
   });
 });
 
+describe("runShell", () => {
+  it("runs an explicit shell command string", () => {
+    const result = runner.runShell("echo hello", { suppressOutput: true });
+    expect(result.status).toBe(0);
+  });
+});
+
+describe("runInteractive with argv array", () => {
+  it("executes an interactive argv command", () => {
+    const result = runner.runInteractive(["echo", "hello"], { suppressOutput: true });
+    expect(result.status).toBe(0);
+  });
+
+  it("rejects string commands", () => {
+    expect(() => runner.runInteractive("echo hello", { suppressOutput: true })).toThrow(
+      /argv array instead/,
+    );
+  });
+
+  it("rejects shell: true to prevent security bypass", () => {
+    expect(() => runner.runInteractive(["echo", "hello"], { shell: true })).toThrow(
+      /shell option is forbidden/,
+    );
+  });
+});
+
+describe("runInteractiveShell", () => {
+  it("runs an explicit interactive shell command string", () => {
+    const result = runner.runInteractiveShell("echo hello", { suppressOutput: true });
+    expect(result.status).toBe(0);
+  });
+});
+
 describe("runCapture with argv array", () => {
   it("captures stdout from a simple command", () => {
     const output = runner.runCapture(["echo", "hello world"]);

src/lib/runner.ts

@@ -80,32 +80,41 @@ function spawnAndHandle(
 }
 
 /**
- * Run a command, streaming stdout/stderr (redacted) to the terminal.
+ * Run a program directly with argv-style arguments, bypassing shell parsing.
  * Exits the process on failure unless opts.ignoreError is true.
  *
- * Accepts two forms:
- *   run("bash -c string")  — legacy: passes the string to bash for interpretation
- *   run(["docker", "rm", name])  — safe: calls spawnSync(exe, args) with no shell
- *
- * When an argv array is passed, the shell option is forbidden to prevent
- * callers from accidentally re-enabling shell interpretation.
+ * Shell-string execution is intentionally unsupported here. If a caller truly
+ * needs shell parsing, it must opt in explicitly via runShell().
  */
-function run(cmd: string | readonly string[], opts: RunnerOptions = {}): SpawnResult {
-  if (Array.isArray(cmd)) {
-    return runArrayCmd(cmd, opts);
+function run(cmd: readonly string[], opts: RunnerOptions = {}): SpawnResult {
+  if (!Array.isArray(cmd)) {
+    throw new Error("run no longer accepts shell strings; pass an argv array instead");
   }
+  return runArrayCmd(cmd, opts);
+}
+
+/**
+ * Run an explicit shell command string through bash -c.
+ * Exits the process on failure unless opts.ignoreError is true.
+ */
+function runShell(cmd: string, opts: RunnerOptions = {}): SpawnResult {
   const shellCmd = String(cmd);
   const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"];
   return spawnAndHandle("bash", ["-c", shellCmd], opts, stdio, shellCmd);
 }
 
 /**
  * Internal: execute an argv array via spawnSync with no shell.
- * Shared by run() and kept separate for clarity.
+ * Shared by run() and runInteractive() and kept separate for clarity.
  */
-function runArrayCmd(cmd: readonly string[], opts: RunnerOptions = {}): SpawnResult {
+function runArrayCmd(
+  cmd: readonly string[],
+  opts: RunnerOptions = {},
+  defaultStdio: RunnerOptions["stdio"] = ["ignore", "pipe", "pipe"],
+  callerName = "run",
+): SpawnResult {
   if (cmd.length === 0) {
-    throw new Error("run: argv array must not be empty");
+    throw new Error(`${callerName}: argv array must not be empty`);
   }
 
   const exe = cmd[0];
@@ -114,10 +123,10 @@ function runArrayCmd(cmd: readonly string[], opts: RunnerOptions = {}): SpawnRes
 
   // Guard: re-enabling shell interpretation defeats the purpose of argv arrays.
   if (spawnOpts.shell) {
-    throw new Error("run: shell option is forbidden when passing an argv array");
+    throw new Error(`${callerName}: shell option is forbidden when passing an argv array`);
   }
 
-  const stdio = stdioCfg ?? ["ignore", "pipe", "pipe"];
+  const stdio = stdioCfg ?? defaultStdio;
 
   const result = spawnSync(exe, args, {
     ...spawnOpts,
@@ -145,10 +154,21 @@ function runArrayCmd(cmd: readonly string[], opts: RunnerOptions = {}): SpawnRes
 }
 
 /**
- * Run a shell command interactively (stdin inherited) while capturing and redacting stdout/stderr.
+ * Run a program directly with argv-style arguments while inheriting stdin.
+ * Exits the process on failure unless opts.ignoreError is true.
+ */
+function runInteractive(cmd: readonly string[], opts: RunnerOptions = {}): SpawnResult {
+  if (!Array.isArray(cmd)) {
+    throw new Error("runInteractive no longer accepts shell strings; pass an argv array instead");
+  }
+  return runArrayCmd(cmd, opts, ["inherit", "pipe", "pipe"], "runInteractive");
+}
+
+/**
+ * Run an explicit shell command string interactively (stdin inherited).
  * Exits the process on failure unless opts.ignoreError is true.
  */
-function runInteractive(cmd: string, opts: RunnerOptions = {}): SpawnResult {
+function runInteractiveShell(cmd: string, opts: RunnerOptions = {}): SpawnResult {
   const stdio = opts.stdio ?? ["inherit", "pipe", "pipe"];
   return spawnAndHandle("bash", ["-c", cmd], opts, stdio, cmd);
 }
@@ -259,9 +279,11 @@ export {
   SCRIPTS,
   redact,
   run,
+  runShell,
   runCapture,
   runFile,
   runInteractive,
+  runInteractiveShell,
   shellQuote,
   validateName,
 };