fix: quote shell interpolations in deploy and harden SSH

bin/lib/deploy.js

@@ -0,0 +1,88 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Deploy helpers — input validation and shell-free command builders.
+//
+// SSH/rsync/scp use runArgv() (argv arrays, no shell) to eliminate command
+// injection at the root cause. shellQuote() is retained for call sites that
+// still need run() (e.g. brev CLI with shell features).
+
+const { runArgv } = require("./runner");
+
+const INSTANCE_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
+
+/**
+ * Validate that name is a safe instance/hostname string.
+ * @param {string} name - Instance name to validate.
+ * @throws {Error} If name is invalid, non-string, or too long.
+ */
+function validateInstanceName(name) {
+  if (!name || typeof name !== "string" || name.length > 253 || !INSTANCE_NAME_RE.test(name)) {
+    throw new Error(
+      `Invalid instance name: ${JSON.stringify(String(name).slice(0, 40))}. ` +
+      "Must be a string, 1-253 chars, start with alphanumeric, and contain only [a-zA-Z0-9._-]."
+    );
+  }
+}
+
+const SSH_OPTS = ["-o", "StrictHostKeyChecking=accept-new", "-o", "LogLevel=ERROR"];
+
+/** @param remoteCmd — executed by the remote shell. Use only constant strings
+ *  or values wrapped in shellQuote(). Never interpolate unsanitized input. */
+function runSsh(host, remoteCmd, opts = {}) {
+  const args = [...SSH_OPTS];
+  if (opts.tty) args.unshift("-t");
+  args.push(host);
+  if (remoteCmd) args.push(remoteCmd);
+  return runArgv("ssh", args, opts);
+}
+
+/**
+ * Copy a file to a remote host via scp using argv arrays (no shell).
+ * @param {string} src - Local source path.
+ * @param {string} destHostPath - Remote destination in host:path format.
+ * @param {object} [opts] - Options forwarded to runArgv.
+ */
+function runScp(src, destHostPath, opts = {}) {
+  const args = ["-q", ...SSH_OPTS, src, destHostPath];
+  return runArgv("scp", args, opts);
+}
+
+/**
+ * Sync files to a remote host via rsync using argv arrays (no shell).
+ * @param {string[]} sources - Local paths to sync.
+ * @param {string} host - Remote hostname (must pass validateInstanceName).
+ * @param {string} dest - Remote destination directory.
+ * @param {object} [opts] - Options forwarded to runArgv.
+ */
+function runRsync(sources, host, dest, opts = {}) {
+  const args = [
+    "-az", "--delete",
+    "--exclude", "node_modules",
+    "--exclude", ".git",
+    "--exclude", "src",
+    "-e", "ssh " + SSH_OPTS.join(" "),
+    ...sources,
+    `${host}:${dest}`,
+  ];
+  return runArgv("rsync", args, opts);
+}
+
+/**
+ * Wrap a string in POSIX single quotes, escaping embedded quotes.
+ * @param {string} s - Value to quote.
+ * @returns {string} Shell-safe single-quoted string.
+ */
+function shellQuote(s) {
+  return "'" + s.replace(/'/g, "'\\''") + "'";
+}
+
+module.exports = {
+  INSTANCE_NAME_RE,
+  validateInstanceName,
+  runSsh,
+  runScp,
+  runRsync,
+  shellQuote,
+  SSH_OPTS,
+};

bin/lib/runner.js

@@ -37,6 +37,75 @@ function run(cmd, opts = {}) {
   return result;
 }
 
+// Env vars that must never be overridden by callers — they enable code
+// execution, library injection, or trust-store hijacking in subprocesses.
+const BLOCKED_ENV_VARS = new Set([
+  "LD_PRELOAD", "LD_LIBRARY_PATH", "DYLD_INSERT_LIBRARIES",
+  "NODE_OPTIONS", "BASH_ENV", "ENV",
+  "GIT_SSH_COMMAND", "SSH_AUTH_SOCK",
+  "DOCKER_HOST", "KUBECONFIG",
+  "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY",
+  "CURL_CA_BUNDLE", "SSL_CERT_FILE", "NODE_EXTRA_CA_CERTS",
+]);
+
+/**
+ * Validate caller-supplied env vars against a blocklist of dangerous keys.
+ * Throws if any blocked key is present; returns a shallow copy otherwise.
+ * @param {object} [callerEnv] - Env overrides from the caller.
+ * @returns {object} Sanitized env entries safe to merge with process.env.
+ */
+function sanitizeEnv(callerEnv) {
+  if (!callerEnv) return {};
+  const blocked = Object.keys(callerEnv).filter((k) => BLOCKED_ENV_VARS.has(k));
+  if (blocked.length > 0) {
+    throw new Error(`runArgv() does not allow overriding: ${blocked.join(", ")}`);
+  }
+  return { ...callerEnv };
+}
+
+/**
+ * Shell-free alternative to run(). Executes prog with an argv array via
+ * spawnSync(prog, args) — no bash, no string interpolation, no injection.
+ * Use this for any command that includes user-controlled values.
+ */
+function runArgv(prog, args, opts = {}) {
+  const { env, ...spawnOpts } = opts;
+  const result = spawnSync(prog, args, {
+    stdio: "inherit",
+    ...spawnOpts,
+    cwd: ROOT,
+    env: { ...process.env, ...sanitizeEnv(env) },
+    shell: false,
+  });
+  if (result.status !== 0 && !opts.ignoreError) {
+    console.error(`  Command failed (exit ${result.status}): ${prog} ${args.join(" ").slice(0, 60)}`);
+    process.exit(result.status || 1);
+  }
+  return result;
+}
+
+/**
+ * Shell-free alternative to runCapture(). Uses execFileSync(prog, args)
+ * with no shell. Returns trimmed stdout.
+ */
+function runCaptureArgv(prog, args, opts = {}) {
+  const { env, ...execOpts } = opts;
+  const { execFileSync } = require("child_process");
+  try {
+    return execFileSync(prog, args, {
+      encoding: "utf-8",
+      stdio: ["pipe", "pipe", "pipe"],
+      ...execOpts,
+      cwd: ROOT,
+      env: { ...process.env, ...sanitizeEnv(env) },
+      shell: false,
+    }).trim();
+  } catch (err) {
+    if (opts.ignoreError) return "";
+    throw err;
+  }
+}
+
 function runCapture(cmd, opts = {}) {
   try {
     return execSync(cmd, {
@@ -52,4 +121,4 @@ function runCapture(cmd, opts = {}) {
   }
 }
 
-module.exports = { ROOT, SCRIPTS, run, runCapture };
+module.exports = { ROOT, SCRIPTS, run, runCapture, runArgv, runCaptureArgv };

bin/nemoclaw.js

@@ -7,7 +7,7 @@ const path = require("path");
 const fs = require("fs");
 const os = require("os");
 
-const { ROOT, SCRIPTS, run, runCapture } = require("./lib/runner");
+const { ROOT, SCRIPTS, run, runCapture, runArgv } = require("./lib/runner");
 const {
   ensureApiKey,
   ensureGithubToken,
@@ -17,6 +17,7 @@ const {
 const registry = require("./lib/registry");
 const nim = require("./lib/nim");
 const policies = require("./lib/policies");
+const { validateInstanceName, runSsh, runScp, runRsync, shellQuote } = require("./lib/deploy");
 
 // ── Global commands ──────────────────────────────────────────────
 
@@ -62,6 +63,7 @@ async function deploy(instanceName) {
     await ensureGithubToken();
   }
   const name = instanceName;
+  validateInstanceName(name);
   const gpu = process.env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1";
 
   console.log("");
@@ -83,17 +85,18 @@ async function deploy(instanceName) {
 
   if (!exists) {
     console.log(`  Creating Brev instance '${name}' (${gpu})...`);
-    run(`brev create ${name} --gpu "${gpu}"`);
+    runArgv("brev", ["create", name, "--gpu", gpu]);
   } else {
     console.log(`  Brev instance '${name}' already exists.`);
   }
 
-  run(`brev refresh`, { ignoreError: true });
+  runArgv("brev", ["refresh"], { ignoreError: true });
 
   console.log("  Waiting for SSH...");
+  const { execFileSync } = require("child_process");
   for (let i = 0; i < 60; i++) {
     try {
-      execSync(`ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no ${name} 'echo ok' 2>/dev/null`, { encoding: "utf-8", stdio: "pipe" });
+      execFileSync("ssh", ["-o", "ConnectTimeout=5", "-o", "StrictHostKeyChecking=accept-new", name, "echo ok"], { encoding: "utf-8", stdio: "pipe" });
       break;
     } catch {
       if (i === 59) {
@@ -105,31 +108,39 @@ async function deploy(instanceName) {
   }
 
   console.log("  Syncing NemoClaw to VM...");
-  run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'mkdir -p /home/ubuntu/nemoclaw'`);
-  run(`rsync -az --delete --exclude node_modules --exclude .git --exclude src -e "ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR" "${ROOT}/scripts" "${ROOT}/Dockerfile" "${ROOT}/nemoclaw" "${ROOT}/nemoclaw-blueprint" "${ROOT}/bin" "${ROOT}/package.json" ${name}:/home/ubuntu/nemoclaw/`);
-
-  const envLines = [`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`];
+  runSsh(name, "mkdir -p /home/ubuntu/nemoclaw");
+  runRsync(
+    [`${ROOT}/scripts`, `${ROOT}/Dockerfile`, `${ROOT}/nemoclaw`, `${ROOT}/nemoclaw-blueprint`, `${ROOT}/bin`, `${ROOT}/package.json`],
+    name,
+    "/home/ubuntu/nemoclaw/"
+  );
+
+  const envLines = [`NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)}`];
   const ghToken = process.env.GITHUB_TOKEN;
-  if (ghToken) envLines.push(`GITHUB_TOKEN=${ghToken}`);
+  if (ghToken) envLines.push(`GITHUB_TOKEN=${shellQuote(ghToken)}`);
   const tgToken = getCredential("TELEGRAM_BOT_TOKEN");
-  if (tgToken) envLines.push(`TELEGRAM_BOT_TOKEN=${tgToken}`);
+  if (tgToken) envLines.push(`TELEGRAM_BOT_TOKEN=${shellQuote(tgToken)}`);
   const envTmp = path.join(os.tmpdir(), `nemoclaw-env-${Date.now()}`);
   fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 });
-  run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR "${envTmp}" ${name}:/home/ubuntu/nemoclaw/.env`);
+  const scpResult = runScp(envTmp, `${name}:/home/ubuntu/nemoclaw/.env`, { ignoreError: true });
   fs.unlinkSync(envTmp);
+  if (scpResult.status !== 0) {
+    console.error(`  Failed to copy .env to ${name}`);
+    process.exit(scpResult.status || 1);
+  }
 
   console.log("  Running setup...");
-  run(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
+  runSsh(name, "cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh", { tty: true });
 
   if (tgToken) {
     console.log("  Starting services...");
-    run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/start-services.sh'`);
+    runSsh(name, "cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/start-services.sh");
   }
 
   console.log("");
   console.log("  Connecting to sandbox...");
   console.log("");
-  run(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw'`);
+  runSsh(name, "cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw", { tty: true });
 }
 
 async function start() {
@@ -188,8 +199,9 @@ function listSandboxes() {
 
 function sandboxConnect(sandboxName) {
   // Ensure port forward is alive before connecting
-  run(`openshell forward start --background 18789 "${sandboxName}" 2>/dev/null || true`, { ignoreError: true });
-  run(`openshell sandbox connect "${sandboxName}"`);
+  run(`openshell forward start --background 18789 ${shellQuote(sandboxName)} 2>/dev/null || true`, { ignoreError: true });
+  run(`openshell sandbox connect ${shellQuote(sandboxName)}`);
+
 }
 
 function sandboxStatus(sandboxName) {
@@ -204,7 +216,7 @@ function sandboxStatus(sandboxName) {
   }
 
   // openshell info
-  run(`openshell sandbox get "${sandboxName}" 2>/dev/null || true`, { ignoreError: true });
+  run(`openshell sandbox get ${shellQuote(sandboxName)} 2>/dev/null || true`, { ignoreError: true });
 
   // NIM health
   const nimStat = nim.nimStatus(sandboxName);
@@ -217,7 +229,7 @@ function sandboxStatus(sandboxName) {
 
 function sandboxLogs(sandboxName, follow) {
   const followFlag = follow ? " --follow" : "";
-  run(`openshell sandbox logs "${sandboxName}"${followFlag}`);
+  run(`openshell sandbox logs ${shellQuote(sandboxName)}${followFlag}`);
 }
 
 async function sandboxPolicyAdd(sandboxName) {
@@ -260,7 +272,7 @@ function sandboxDestroy(sandboxName) {
   nim.stopNimContainer(sandboxName);
 
   console.log(`  Deleting sandbox '${sandboxName}'...`);
-  run(`openshell sandbox delete "${sandboxName}" 2>/dev/null || true`, { ignoreError: true });
+  run(`openshell sandbox delete ${shellQuote(sandboxName)} 2>/dev/null || true`, { ignoreError: true });
 
   registry.removeSandbox(sandboxName);
   console.log(`  ✓ Sandbox '${sandboxName}' destroyed`);