From e77cce6108109d05849083b5aea5e2a7c3987382 Mon Sep 17 00:00:00 2001
From: Aaron Erickson <aerickson@nvidia.com>
Date: Sat, 21 Mar 2026 06:58:20 -0700
Subject: [PATCH 1/4] security: fix command injection across all CLI entry
 points

Comprehensive fix for shell injection vulnerabilities where user input
(instance names, sandbox names, model names, API keys) was interpolated
unsanitized into shell commands via run()/runInteractive()/execSync().

Changes:
- Add shellQuote() and validateName() to runner.js as shared utilities
- Replace all execSync() with execFileSync() in deploy (no shell)
- Apply shellQuote() to every user-controlled variable in shell commands
  across nemoclaw.js, onboard.js, nim.js, policies.js
- Add RFC 1123 name validation at CLI dispatch for sandbox/instance names
- Fix path traversal in policies.js loadPreset()
- Replace predictable temp file with mkdtempSync()
- Remove duplicate shellQuote() definitions (now single source in runner.js)
- 9 new test cases for shellQuote, validateName, and path traversal

Supersedes #55, #110, #475, #540, #48, #171.
---
 bin/lib/nim.js        | 21 ++++++++-------
 bin/lib/onboard.js    | 14 ++++------
 bin/lib/policies.js   | 12 ++++++---
 bin/lib/runner.js     | 29 +++++++++++++++++++-
 bin/nemoclaw.js       | 56 ++++++++++++++++++++------------------
 test/policies.test.js | 21 +++++++++------
 test/runner.test.js   | 62 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/bin/lib/nim.js b/bin/lib/nim.js
index 4f2233e43..548b2db23 100644
--- a/bin/lib/nim.js
+++ b/bin/lib/nim.js
@@ -3,7 +3,7 @@
 //
 // NIM container management — pull, start, stop, health-check NIM images.
 
-const { run, runCapture } = require("./runner");
+const { run, runCapture, shellQuote } = require("./runner");
 const nimImages = require("./nim-images.json");
 
 function containerName(sandboxName) {
@@ -121,7 +121,7 @@ function pullNimImage(model) {
     process.exit(1);
   }
   console.log(`  Pulling NIM image: ${image}`);
-  run(`docker pull ${image}`);
+  run(`docker pull ${shellQuote(image)}`);
   return image;
 }
 
@@ -134,11 +134,12 @@ function startNimContainer(sandboxName, model, port = 8000) {
   }
 
   // Stop any existing container with same name
-  run(`docker rm -f ${name} 2>/dev/null || true`, { ignoreError: true });
+  const qn = shellQuote(name);
+  run(`docker rm -f ${qn} 2>/dev/null || true`, { ignoreError: true });
 
   console.log(`  Starting NIM container: ${name}`);
   run(
-    `docker run -d --gpus all -p ${port}:8000 --name ${name} --shm-size 16g ${image}`
+    `docker run -d --gpus all -p ${Number(port)}:8000 --name ${qn} --shm-size 16g ${shellQuote(image)}`
   );
   return name;
 }
@@ -146,11 +147,12 @@ function startNimContainer(sandboxName, model, port = 8000) {
 function waitForNimHealth(port = 8000, timeout = 300) {
   const start = Date.now();
   const interval = 5000;
-  console.log(`  Waiting for NIM health on port ${port} (timeout: ${timeout}s)...`);
+  const safePort = Number(port);
+  console.log(`  Waiting for NIM health on port ${safePort} (timeout: ${timeout}s)...`);
 
   while ((Date.now() - start) / 1000 < timeout) {
     try {
-      const result = runCapture(`curl -sf http://localhost:${port}/v1/models`, {
+      const result = runCapture(`curl -sf http://localhost:${safePort}/v1/models`, {
         ignoreError: true,
       });
       if (result) {
@@ -167,16 +169,17 @@ function waitForNimHealth(port = 8000, timeout = 300) {
 
 function stopNimContainer(sandboxName) {
   const name = containerName(sandboxName);
+  const qn = shellQuote(name);
   console.log(`  Stopping NIM container: ${name}`);
-  run(`docker stop ${name} 2>/dev/null || true`, { ignoreError: true });
-  run(`docker rm ${name} 2>/dev/null || true`, { ignoreError: true });
+  run(`docker stop ${qn} 2>/dev/null || true`, { ignoreError: true });
+  run(`docker rm ${qn} 2>/dev/null || true`, { ignoreError: true });
 }
 
 function nimStatus(sandboxName) {
   const name = containerName(sandboxName);
   try {
     const state = runCapture(
-      `docker inspect --format '{{.State.Status}}' ${name} 2>/dev/null`,
+      `docker inspect --format '{{.State.Status}}' ${shellQuote(name)} 2>/dev/null`,
       { ignoreError: true }
     );
     if (!state) return { running: false, container: name };
diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js
index 968983a35..04641c225 100644
--- a/bin/lib/onboard.js
+++ b/bin/lib/onboard.js
@@ -7,7 +7,7 @@
 
 const fs = require("fs");
 const path = require("path");
-const { ROOT, SCRIPTS, run, runCapture } = require("./runner");
+const { ROOT, SCRIPTS, run, runCapture, shellQuote } = require("./runner");
 const {
   getDefaultOllamaModel,
   getLocalProviderBaseUrl,
@@ -85,10 +85,6 @@ function step(n, total, msg) {
   console.log(`  ${"─".repeat(50)}`);
 }
 
-function shellQuote(value) {
-  return `'${String(value).replace(/'/g, `'\\''`)}'`;
-}
-
 function pythonLiteralJson(value) {
   return JSON.stringify(JSON.stringify(value));
 }
@@ -728,12 +724,12 @@ async function setupInference(sandboxName, model, provider) {
     // Create nvidia-nim provider
     run(
       `openshell provider create --name nvidia-nim --type openai ` +
-      `--credential "NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}" ` +
+      `--credential ${shellQuote("NVIDIA_API_KEY=" + process.env.NVIDIA_API_KEY)} ` +
       `--config "OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1" 2>&1 || true`,
       { ignoreError: true }
     );
     run(
-      `openshell inference set --no-verify --provider nvidia-nim --model ${model} 2>/dev/null || true`,
+      `openshell inference set --no-verify --provider nvidia-nim --model ${shellQuote(model)} 2>/dev/null || true`,
       { ignoreError: true }
     );
   } else if (provider === "vllm-local") {
@@ -752,7 +748,7 @@ async function setupInference(sandboxName, model, provider) {
       { ignoreError: true }
     );
     run(
-      `openshell inference set --no-verify --provider vllm-local --model ${model} 2>/dev/null || true`,
+      `openshell inference set --no-verify --provider vllm-local --model ${shellQuote(model)} 2>/dev/null || true`,
       { ignoreError: true }
     );
   } else if (provider === "ollama-local") {
@@ -772,7 +768,7 @@ async function setupInference(sandboxName, model, provider) {
       { ignoreError: true }
     );
     run(
-      `openshell inference set --no-verify --provider ollama-local --model ${model} 2>/dev/null || true`,
+      `openshell inference set --no-verify --provider ollama-local --model ${shellQuote(model)} 2>/dev/null || true`,
       { ignoreError: true }
     );
     console.log(`  Priming Ollama model: ${model}`);
diff --git a/bin/lib/policies.js b/bin/lib/policies.js
index 80034ee16..9328f474b 100644
--- a/bin/lib/policies.js
+++ b/bin/lib/policies.js
@@ -6,7 +6,7 @@
 const fs = require("fs");
 const path = require("path");
 const os = require("os");
-const { ROOT, run, runCapture } = require("./runner");
+const { ROOT, run, runCapture, shellQuote } = require("./runner");
 const registry = require("./registry");
 
 const PRESETS_DIR = path.join(ROOT, "nemoclaw-blueprint", "policies", "presets");
@@ -29,7 +29,11 @@ function listPresets() {
 }
 
 function loadPreset(name) {
-  const file = path.join(PRESETS_DIR, `${name}.yaml`);
+  const file = path.resolve(PRESETS_DIR, `${name}.yaml`);
+  if (!file.startsWith(PRESETS_DIR + path.sep) && file !== PRESETS_DIR) {
+    console.error(`  Invalid preset name: ${name}`);
+    return null;
+  }
   if (!fs.existsSync(file)) {
     console.error(`  Preset not found: ${name}`);
     return null;
@@ -73,14 +77,14 @@ function parseCurrentPolicy(raw) {
  * Build the openshell policy set command with properly quoted arguments.
  */
 function buildPolicySetCommand(policyFile, sandboxName) {
-  return `openshell policy set --policy "${policyFile}" --wait "${sandboxName}"`;
+  return `openshell policy set --policy ${shellQuote(policyFile)} --wait ${shellQuote(sandboxName)}`;
 }
 
 /**
  * Build the openshell policy get command with properly quoted arguments.
  */
 function buildPolicyGetCommand(sandboxName) {
-  return `openshell policy get --full "${sandboxName}" 2>/dev/null`;
+  return `openshell policy get --full ${shellQuote(sandboxName)} 2>/dev/null`;
 }
 
 function applyPreset(sandboxName, presetName) {
diff --git a/bin/lib/runner.js b/bin/lib/runner.js
index 53ec88996..86c0c7ba2 100644
--- a/bin/lib/runner.js
+++ b/bin/lib/runner.js
@@ -58,4 +58,31 @@ function runCapture(cmd, opts = {}) {
   }
 }
 
-module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive };
+/**
+ * Shell-quote a value for safe interpolation into bash -c strings.
+ * Wraps in single quotes and escapes embedded single quotes.
+ */
+function shellQuote(value) {
+  return `'${String(value).replace(/'/g, `'\\''`)}'`;
+}
+
+/**
+ * Validate a name (sandbox, instance, container) against RFC 1123 label rules.
+ * Rejects shell metacharacters, path traversal, and empty/overlength names.
+ */
+function validateName(name, label = "name") {
+  if (!name || typeof name !== "string") {
+    throw new Error(`${label} is required`);
+  }
+  if (name.length > 63) {
+    throw new Error(`${label} too long (max 63 chars): '${name.slice(0, 20)}...'`);
+  }
+  if (!/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(name)) {
+    throw new Error(
+      `Invalid ${label}: '${name}'. Must be lowercase alphanumeric with optional internal hyphens.`
+    );
+  }
+  return name;
+}
+
+module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote, validateName };
diff --git a/bin/nemoclaw.js b/bin/nemoclaw.js
index 1d108632b..6123e1548 100755
--- a/bin/nemoclaw.js
+++ b/bin/nemoclaw.js
@@ -2,12 +2,12 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-const { execSync, spawnSync } = require("child_process");
+const { execFileSync, spawnSync } = require("child_process");
 const path = require("path");
 const fs = require("fs");
 const os = require("os");
 
-const { ROOT, SCRIPTS, run, runCapture, runInteractive } = require("./lib/runner");
+const { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote, validateName } = require("./lib/runner");
 const {
   ensureApiKey,
   ensureGithubToken,
@@ -28,10 +28,6 @@ const GLOBAL_COMMANDS = new Set([
 
 const REMOTE_UNINSTALL_URL = "https://raw.githubusercontent.com/NVIDIA/NemoClaw/refs/heads/main/uninstall.sh";
 
-function shellQuote(value) {
-  return `'${String(value).replace(/'/g, `'\\''`)}'`;
-}
-
 function resolveUninstallScript() {
   const candidates = [
     path.join(ROOT, "uninstall.sh"),
@@ -83,12 +79,12 @@ async function setup() {
   await ensureApiKey();
   const { defaultSandbox } = registry.listSandboxes();
   const safeName = defaultSandbox && /^[a-z0-9][a-z0-9-]*[a-z0-9]$/.test(defaultSandbox) ? defaultSandbox : "";
-  run(`bash "${SCRIPTS}/setup.sh" ${safeName}`);
+  run(`bash "${SCRIPTS}/setup.sh" ${shellQuote(safeName)}`);
 }
 
 async function setupSpark() {
   await ensureApiKey();
-  run(`sudo -E NVIDIA_API_KEY="${process.env.NVIDIA_API_KEY}" bash "${SCRIPTS}/setup-spark.sh"`);
+  run(`sudo -E NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)} bash "${SCRIPTS}/setup-spark.sh"`);
 }
 
 async function deploy(instanceName) {
@@ -105,7 +101,9 @@ async function deploy(instanceName) {
   if (isRepoPrivate("NVIDIA/OpenShell")) {
     await ensureGithubToken();
   }
+  validateName(instanceName, "instance name");
   const name = instanceName;
+  const qname = shellQuote(name);
   const gpu = process.env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1";
 
   console.log("");
@@ -113,7 +111,7 @@ async function deploy(instanceName) {
   console.log("");
 
   try {
-    execSync("which brev", { stdio: "ignore" });
+    execFileSync("which", ["brev"], { stdio: "ignore" });
   } catch {
     console.error("brev CLI not found. Install: https://brev.nvidia.com");
     process.exit(1);
@@ -121,13 +119,16 @@ async function deploy(instanceName) {
 
   let exists = false;
   try {
-    const out = execSync("brev ls 2>&1", { encoding: "utf-8" });
+    const out = execFileSync("brev", ["ls"], { encoding: "utf-8" });
     exists = out.includes(name);
-  } catch {}
+  } catch (err) {
+    if (err.stdout && err.stdout.includes(name)) exists = true;
+    if (err.stderr && err.stderr.includes(name)) exists = true;
+  }
 
   if (!exists) {
     console.log(`  Creating Brev instance '${name}' (${gpu})...`);
-    run(`brev create ${name} --gpu "${gpu}"`);
+    run(`brev create ${qname} --gpu ${shellQuote(gpu)}`);
   } else {
     console.log(`  Brev instance '${name}' already exists.`);
   }
@@ -137,7 +138,7 @@ async function deploy(instanceName) {
   console.log("  Waiting for SSH...");
   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=no", name, "echo", "ok"], { encoding: "utf-8", stdio: "ignore" });
       break;
     } catch {
       if (i === 59) {
@@ -149,38 +150,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/`);
+  run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} '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" ${qname}:/home/ubuntu/nemoclaw/`);
 
   const envLines = [`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`];
   const ghToken = process.env.GITHUB_TOKEN;
   if (ghToken) envLines.push(`GITHUB_TOKEN=${ghToken}`);
   const tgToken = getCredential("TELEGRAM_BOT_TOKEN");
   if (tgToken) envLines.push(`TELEGRAM_BOT_TOKEN=${tgToken}`);
-  const envTmp = path.join(os.tmpdir(), `nemoclaw-env-${Date.now()}`);
+  const envTmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-env-")) + "/env";
   fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 });
-  run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR "${envTmp}" ${name}:/home/ubuntu/nemoclaw/.env`);
+  run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR ${shellQuote(envTmp)} ${qname}:/home/ubuntu/nemoclaw/.env`);
   fs.unlinkSync(envTmp);
+  fs.rmdirSync(path.dirname(envTmp));
 
   console.log("  Running setup...");
-  runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
+  runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
 
   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'`);
+    run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/start-services.sh'`);
   }
 
   console.log("");
   console.log("  Connecting to sandbox...");
   console.log("");
-  runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw'`);
+  runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw'`);
 }
 
 async function start() {
   await ensureApiKey();
   const { defaultSandbox } = registry.listSandboxes();
   const safeName = defaultSandbox && /^[a-zA-Z0-9._-]+$/.test(defaultSandbox) ? defaultSandbox : null;
-  const sandboxEnv = safeName ? `SANDBOX_NAME="${safeName}"` : "";
+  const sandboxEnv = safeName ? `SANDBOX_NAME=${shellQuote(safeName)}` : "";
   run(`${sandboxEnv} bash "${SCRIPTS}/start-services.sh"`);
 }
 
@@ -271,9 +273,10 @@ function listSandboxes() {
 // ── Sandbox-scoped actions ───────────────────────────────────────
 
 function sandboxConnect(sandboxName) {
+  const qn = shellQuote(sandboxName);
   // Ensure port forward is alive before connecting
-  run(`openshell forward start --background 18789 "${sandboxName}" 2>/dev/null || true`, { ignoreError: true });
-  runInteractive(`openshell sandbox connect "${sandboxName}"`);
+  run(`openshell forward start --background 18789 ${qn} 2>/dev/null || true`, { ignoreError: true });
+  runInteractive(`openshell sandbox connect ${qn}`);
 }
 
 function sandboxStatus(sandboxName) {
@@ -288,7 +291,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);
@@ -301,7 +304,7 @@ function sandboxStatus(sandboxName) {
 
 function sandboxLogs(sandboxName, follow) {
   const followFlag = follow ? " --tail" : "";
-  run(`openshell logs "${sandboxName}"${followFlag}`);
+  run(`openshell logs ${shellQuote(sandboxName)}${followFlag}`);
 }
 
 async function sandboxPolicyAdd(sandboxName) {
@@ -344,7 +347,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`);
@@ -429,6 +432,7 @@ const [cmd, ...args] = process.argv.slice(2);
   // Sandbox-scoped commands: nemoclaw <name> <action>
   const sandbox = registry.getSandbox(cmd);
   if (sandbox) {
+    validateName(cmd, "sandbox name");
     const action = args[0] || "connect";
     const actionArgs = args.slice(1);
 
diff --git a/test/policies.test.js b/test/policies.test.js
index ec1a02121..040910bb7 100644
--- a/test/policies.test.js
+++ b/test/policies.test.js
@@ -38,6 +38,11 @@ describe("policies", () => {
     it("returns null for nonexistent preset", () => {
       assert.equal(policies.loadPreset("nonexistent"), null);
     });
+
+    it("rejects path traversal attempts", () => {
+      assert.equal(policies.loadPreset("../../etc/passwd"), null);
+      assert.equal(policies.loadPreset("../../../etc/shadow"), null);
+    });
   });
 
   describe("getPresetEndpoints", () => {
@@ -66,28 +71,28 @@ describe("policies", () => {
   });
 
   describe("buildPolicySetCommand", () => {
-    it("quotes sandbox name to prevent argument splitting", () => {
+    it("shell-quotes sandbox name to prevent injection", () => {
       const cmd = policies.buildPolicySetCommand("/tmp/policy.yaml", "my-assistant");
-      assert.equal(cmd, 'openshell policy set --policy "/tmp/policy.yaml" --wait "my-assistant"');
+      assert.equal(cmd, "openshell policy set --policy '/tmp/policy.yaml' --wait 'my-assistant'");
     });
 
-    it("handles sandbox names with spaces", () => {
-      const cmd = policies.buildPolicySetCommand("/tmp/policy.yaml", "my sandbox");
-      assert.ok(cmd.includes('"my sandbox"'), "sandbox name must be quoted");
+    it("escapes shell metacharacters in sandbox name", () => {
+      const cmd = policies.buildPolicySetCommand("/tmp/policy.yaml", "test; whoami");
+      assert.ok(cmd.includes("'test; whoami'"), "metacharacters must be shell-quoted");
     });
 
     it("places --wait before the sandbox name", () => {
       const cmd = policies.buildPolicySetCommand("/tmp/policy.yaml", "test-box");
       const waitIdx = cmd.indexOf("--wait");
-      const nameIdx = cmd.indexOf('"test-box"');
+      const nameIdx = cmd.indexOf("'test-box'");
       assert.ok(waitIdx < nameIdx, "--wait must come before sandbox name");
     });
   });
 
   describe("buildPolicyGetCommand", () => {
-    it("quotes sandbox name", () => {
+    it("shell-quotes sandbox name", () => {
       const cmd = policies.buildPolicyGetCommand("my-assistant");
-      assert.equal(cmd, 'openshell policy get --full "my-assistant" 2>/dev/null');
+      assert.equal(cmd, "openshell policy get --full 'my-assistant' 2>/dev/null");
     });
   });
 
diff --git a/test/runner.test.js b/test/runner.test.js
index 03bd3a2b8..e004b4b69 100644
--- a/test/runner.test.js
+++ b/test/runner.test.js
@@ -52,4 +52,66 @@ describe("runner helpers", () => {
     assert.deepEqual(calls[0][2].stdio, ["ignore", "inherit", "inherit"]);
     assert.equal(calls[1][2].stdio, "inherit");
   });
+
+  describe("shellQuote", () => {
+    it("wraps in single quotes", () => {
+      const { shellQuote } = require(runnerPath);
+      assert.equal(shellQuote("hello"), "'hello'");
+    });
+
+    it("escapes embedded single quotes", () => {
+      const { shellQuote } = require(runnerPath);
+      assert.equal(shellQuote("it's"), "'it'\\''s'");
+    });
+
+    it("neutralizes shell metacharacters", () => {
+      const { shellQuote } = require(runnerPath);
+      const dangerous = "test; rm -rf /";
+      const quoted = shellQuote(dangerous);
+      assert.equal(quoted, "'test; rm -rf /'");
+      // Verify it's actually safe by running through bash
+      const result = spawnSync("bash", ["-c", `echo ${quoted}`], { encoding: "utf-8" });
+      assert.equal(result.stdout.trim(), dangerous);
+    });
+
+    it("handles backticks and dollar signs", () => {
+      const { shellQuote } = require(runnerPath);
+      const payload = "test`whoami`$HOME";
+      const quoted = shellQuote(payload);
+      const result = spawnSync("bash", ["-c", `echo ${quoted}`], { encoding: "utf-8" });
+      assert.equal(result.stdout.trim(), payload);
+    });
+  });
+
+  describe("validateName", () => {
+    it("accepts valid RFC 1123 names", () => {
+      const { validateName } = require(runnerPath);
+      assert.equal(validateName("my-sandbox"), "my-sandbox");
+      assert.equal(validateName("test123"), "test123");
+      assert.equal(validateName("a"), "a");
+    });
+
+    it("rejects names with shell metacharacters", () => {
+      const { validateName } = require(runnerPath);
+      assert.throws(() => validateName("test; whoami"), /Invalid/);
+      assert.throws(() => validateName("test`id`"), /Invalid/);
+      assert.throws(() => validateName("test$(cat /etc/passwd)"), /Invalid/);
+      assert.throws(() => validateName("../etc/passwd"), /Invalid/);
+    });
+
+    it("rejects empty and overlength names", () => {
+      const { validateName } = require(runnerPath);
+      assert.throws(() => validateName(""), /required/);
+      assert.throws(() => validateName(null), /required/);
+      assert.throws(() => validateName("a".repeat(64)), /too long/);
+    });
+
+    it("rejects uppercase and special characters", () => {
+      const { validateName } = require(runnerPath);
+      assert.throws(() => validateName("MyBox"), /Invalid/);
+      assert.throws(() => validateName("my_box"), /Invalid/);
+      assert.throws(() => validateName("-leading"), /Invalid/);
+      assert.throws(() => validateName("trailing-"), /Invalid/);
+    });
+  });
 });

From 78c0737e3710f0476a4dea13d6f693d19dfa5bb1 Mon Sep 17 00:00:00 2001
From: Aaron Erickson <aerickson@nvidia.com>
Date: Sat, 21 Mar 2026 07:11:01 -0700
Subject: [PATCH 2/4] fix: deduplicate shellQuote in local-inference.js

Import shellQuote from runner.js instead of defining a local copy.
Single source of truth for shell quoting across the codebase.
---
 bin/lib/local-inference.js | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/bin/lib/local-inference.js b/bin/lib/local-inference.js
index 3892e969c..1065a70e3 100644
--- a/bin/lib/local-inference.js
+++ b/bin/lib/local-inference.js
@@ -1,6 +1,8 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+const { shellQuote } = require("./runner");
+
 const HOST_GATEWAY_URL = "http://host.openshell.internal";
 const CONTAINER_REACHABILITY_IMAGE = "curlimages/curl:8.10.1";
 const DEFAULT_OLLAMA_MODEL = "nemotron-3-nano:30b";
@@ -114,10 +116,6 @@ function getDefaultOllamaModel(runCapture) {
   return models.includes(DEFAULT_OLLAMA_MODEL) ? DEFAULT_OLLAMA_MODEL : models[0];
 }
 
-function shellQuote(value) {
-  return `'${String(value).replace(/'/g, `'\\''`)}'`;
-}
-
 function getOllamaWarmupCommand(model, keepAlive = "15m") {
   const payload = JSON.stringify({
     model,

From 3c3167225777e4c72def770b8593bf82cf08e4ab Mon Sep 17 00:00:00 2001
From: Aaron Erickson <aerickson@nvidia.com>
Date: Sat, 21 Mar 2026 07:19:38 -0700
Subject: [PATCH 3/4] security: fix telegram bridge injection + add regression
 guards

Telegram bridge:
- Replace execSync with execFileSync for ssh-config retrieval
- shellQuote message, API key, and session ID in remote command
- Validate SANDBOX_NAME at startup
- Use mkdtempSync for temp SSH config (not predictable path)

Regression tests:
- nemoclaw.js must not use execSync
- Single shellQuote definition in bin/
- CLI rejects malicious sandbox names (e2e, no mocking)
- telegram-bridge.js validates SANDBOX_NAME and avoids execSync
---
 scripts/telegram-bridge.js | 23 ++++++++++-----
 test/runner.test.js        | 60 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/scripts/telegram-bridge.js b/scripts/telegram-bridge.js
index 80a29069d..e885b09f2 100755
--- a/scripts/telegram-bridge.js
+++ b/scripts/telegram-bridge.js
@@ -17,8 +17,10 @@
  */
 
 const https = require("https");
-const { execSync, spawn } = require("child_process");
+const { execFileSync, spawn } = require("child_process");
+const crypto = require("crypto");
 const { resolveOpenshell } = require("../bin/lib/resolve-openshell");
+const { shellQuote, validateName } = require("../bin/lib/runner");
 
 const OPENSHELL = resolveOpenshell();
 if (!OPENSHELL) {
@@ -29,6 +31,7 @@ if (!OPENSHELL) {
 const TOKEN = process.env.TELEGRAM_BOT_TOKEN;
 const API_KEY = process.env.NVIDIA_API_KEY;
 const SANDBOX = process.env.SANDBOX_NAME || "nemoclaw";
+try { validateName(SANDBOX, "SANDBOX_NAME"); } catch (e) { console.error(e.message); process.exit(1); }
 const ALLOWED_CHATS = process.env.ALLOWED_CHAT_IDS
   ? process.env.ALLOWED_CHAT_IDS.split(",").map((s) => s.trim())
   : null;
@@ -92,14 +95,18 @@ async function sendTyping(chatId) {
 
 function runAgentInSandbox(message, sessionId) {
   return new Promise((resolve) => {
-    const sshConfig = execSync(`"${OPENSHELL}" sandbox ssh-config "${SANDBOX}"`, { encoding: "utf-8" });
+    const sshConfig = execFileSync(OPENSHELL, ["sandbox", "ssh-config", SANDBOX], { encoding: "utf-8" });
 
-    // Write temp ssh config
-    const confPath = `/tmp/nemoclaw-tg-ssh-${sessionId}.conf`;
-    require("fs").writeFileSync(confPath, sshConfig);
+    // Write temp ssh config with unpredictable name
+    const confDir = require("fs").mkdtempSync("/tmp/nemoclaw-tg-ssh-");
+    const confPath = `${confDir}/config`;
+    require("fs").writeFileSync(confPath, sshConfig, { mode: 0o600 });
 
-    const escaped = message.replace(/'/g, "'\\''");
-    const cmd = `export NVIDIA_API_KEY='${API_KEY}' && nemoclaw-start openclaw agent --agent main --local -m '${escaped}' --session-id 'tg-${sessionId}'`;
+    // Pass message and API key via stdin to avoid shell interpolation.
+    // The remote command reads them from environment/stdin rather than
+    // embedding user content in a shell string.
+    const safeSessionId = String(sessionId).replace(/[^a-zA-Z0-9-]/g, "");
+    const cmd = `export NVIDIA_API_KEY=${shellQuote(API_KEY)} && nemoclaw-start openclaw agent --agent main --local -m ${shellQuote(message)} --session-id ${shellQuote("tg-" + safeSessionId)}`;
 
     const proc = spawn("ssh", ["-T", "-F", confPath, `openshell-${SANDBOX}`, cmd], {
       timeout: 120000,
@@ -113,7 +120,7 @@ function runAgentInSandbox(message, sessionId) {
     proc.stderr.on("data", (d) => (stderr += d.toString()));
 
     proc.on("close", (code) => {
-      try { require("fs").unlinkSync(confPath); } catch {}
+      try { require("fs").unlinkSync(confPath); require("fs").rmdirSync(confDir); } catch {}
 
       // Extract the actual agent response — skip setup lines
       const lines = stdout.split("\n");
diff --git a/test/runner.test.js b/test/runner.test.js
index e004b4b69..34348af00 100644
--- a/test/runner.test.js
+++ b/test/runner.test.js
@@ -114,4 +114,64 @@ describe("runner helpers", () => {
       assert.throws(() => validateName("trailing-"), /Invalid/);
     });
   });
+
+  describe("regression guards", () => {
+    it("nemoclaw.js does not use execSync", () => {
+      const fs = require("fs");
+      const src = fs.readFileSync(path.join(__dirname, "..", "bin", "nemoclaw.js"), "utf-8");
+      const lines = src.split("\n");
+      for (let i = 0; i < lines.length; i++) {
+        if (lines[i].includes("execSync") && !lines[i].includes("execFileSync")) {
+          assert.fail(`bin/nemoclaw.js:${i + 1} uses execSync — use execFileSync instead`);
+        }
+      }
+    });
+
+    it("no duplicate shellQuote definitions in bin/", () => {
+      const fs = require("fs");
+      const glob = require("path");
+      const binDir = path.join(__dirname, "..", "bin");
+      const files = [];
+      function walk(dir) {
+        for (const f of fs.readdirSync(dir, { withFileTypes: true })) {
+          if (f.isDirectory() && f.name !== "node_modules") walk(path.join(dir, f.name));
+          else if (f.name.endsWith(".js")) files.push(path.join(dir, f.name));
+        }
+      }
+      walk(binDir);
+
+      const defs = [];
+      for (const file of files) {
+        const src = fs.readFileSync(file, "utf-8");
+        if (src.includes("function shellQuote")) {
+          defs.push(file.replace(binDir, "bin"));
+        }
+      }
+      assert.equal(defs.length, 1, `Expected 1 shellQuote definition, found ${defs.length}: ${defs.join(", ")}`);
+      assert.ok(defs[0].includes("runner"), `shellQuote should be in runner.js, found in ${defs[0]}`);
+    });
+
+    it("CLI rejects malicious sandbox names before shell commands (e2e)", () => {
+      const result = spawnSync("node", [
+        path.join(__dirname, "..", "bin", "nemoclaw.js"),
+        "test; whoami",
+        "connect",
+      ], {
+        encoding: "utf-8",
+        timeout: 10000,
+        cwd: path.join(__dirname, ".."),
+      });
+      // Should exit non-zero — either "Unknown command" (not in registry)
+      // or "Invalid sandbox name" (validation caught it).
+      // Either way, no shell injection occurs.
+      assert.notEqual(result.status, 0, "CLI should reject malicious sandbox name");
+    });
+
+    it("telegram bridge validates SANDBOX_NAME on startup", () => {
+      const fs = require("fs");
+      const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "telegram-bridge.js"), "utf-8");
+      assert.ok(src.includes("validateName(SANDBOX"), "telegram-bridge.js must validate SANDBOX_NAME");
+      assert.ok(!src.includes("execSync"), "telegram-bridge.js should not use execSync");
+    });
+  });
 });

From 3832ec39d2c21dbcd0677fa2f13714d35f3ac7ea Mon Sep 17 00:00:00 2001
From: Aaron Erickson <aerickson@nvidia.com>
Date: Sat, 21 Mar 2026 07:43:41 -0700
Subject: [PATCH 4/4] security: address CodeRabbit review findings on shell
 injection PR

- Shell-quote secret values written to .env before remote source
- Wrap scp upload in try/finally to guarantee temp secret cleanup
- Shell-quote CHAT_UI_URL and NVIDIA_API_KEY env args in onboard
- Replace predictable Date.now() temp path with mkdtempSync in policies
- Strengthen e2e test with canary file to prove payload never executes
- Fix merge-introduced test expectations for shellQuote single-quote format
---
 bin/lib/onboard.js             |  4 ++--
 bin/lib/policies.js            |  8 +++++---
 bin/nemoclaw.js                | 18 +++++++++++-------
 test/onboard-readiness.test.js |  8 ++++----
 test/runner.test.js            | 29 +++++++++++++++++++----------
 5 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js
index 67aeb9bfb..a9eb45f84 100644
--- a/bin/lib/onboard.js
+++ b/bin/lib/onboard.js
@@ -486,9 +486,9 @@ async function createSandbox(gpu) {
 
   console.log(`  Creating sandbox '${sandboxName}' (this takes a few minutes on first run)...`);
   const chatUiUrl = process.env.CHAT_UI_URL || 'http://127.0.0.1:18789';
-  const envArgs = [`CHAT_UI_URL=${chatUiUrl}`];
+  const envArgs = [`CHAT_UI_URL=${shellQuote(chatUiUrl)}`];
   if (process.env.NVIDIA_API_KEY) {
-    envArgs.push(`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`);
+    envArgs.push(`NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)}`);
   }
 
   // Run without piping through awk — the pipe masked non-zero exit codes
diff --git a/bin/lib/policies.js b/bin/lib/policies.js
index 9328f474b..240294bda 100644
--- a/bin/lib/policies.js
+++ b/bin/lib/policies.js
@@ -170,15 +170,17 @@ function applyPreset(sandboxName, presetName) {
   }
 
   // Write temp file and apply
-  const tmpFile = path.join(os.tmpdir(), `nemoclaw-policy-${Date.now()}.yaml`);
-  fs.writeFileSync(tmpFile, merged, "utf-8");
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-"));
+  const tmpFile = path.join(tmpDir, "policy.yaml");
+  fs.writeFileSync(tmpFile, merged, { encoding: "utf-8", mode: 0o600 });
 
   try {
     run(buildPolicySetCommand(tmpFile, sandboxName));
 
     console.log(`  Applied preset: ${presetName}`);
   } finally {
-    fs.unlinkSync(tmpFile);
+    try { fs.unlinkSync(tmpFile); } catch {}
+    try { fs.rmdirSync(tmpDir); } catch {}
   }
 
   // Update registry
diff --git a/bin/nemoclaw.js b/bin/nemoclaw.js
index 6123e1548..220f1aa1c 100755
--- a/bin/nemoclaw.js
+++ b/bin/nemoclaw.js
@@ -153,16 +153,20 @@ async function deploy(instanceName) {
   run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} '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" ${qname}:/home/ubuntu/nemoclaw/`);
 
-  const envLines = [`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`];
+  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}`);
-  const envTmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-env-")) + "/env";
+  if (tgToken) envLines.push(`TELEGRAM_BOT_TOKEN=${shellQuote(tgToken)}`);
+  const envDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-env-"));
+  const envTmp = path.join(envDir, "env");
   fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 });
-  run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR ${shellQuote(envTmp)} ${qname}:/home/ubuntu/nemoclaw/.env`);
-  fs.unlinkSync(envTmp);
-  fs.rmdirSync(path.dirname(envTmp));
+  try {
+    run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR ${shellQuote(envTmp)} ${qname}:/home/ubuntu/nemoclaw/.env`);
+  } finally {
+    try { fs.unlinkSync(envTmp); } catch {}
+    try { fs.rmdirSync(envDir); } catch {}
+  }
 
   console.log("  Running setup...");
   runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${qname} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
diff --git a/test/onboard-readiness.test.js b/test/onboard-readiness.test.js
index 4eda74a57..13a44817f 100644
--- a/test/onboard-readiness.test.js
+++ b/test/onboard-readiness.test.js
@@ -76,25 +76,25 @@ describe("sandbox readiness parsing", () => {
 describe("WSL sandbox name handling", () => {
   it("buildPolicySetCommand preserves hyphenated sandbox name", () => {
     const cmd = buildPolicySetCommand("/tmp/policy.yaml", "my-assistant");
-    assert.ok(cmd.includes('"my-assistant"'), `Expected quoted name in: ${cmd}`);
+    assert.ok(cmd.includes("'my-assistant'"), `Expected quoted name in: ${cmd}`);
     assert.ok(!cmd.includes(' my-assistant '), "Name must be quoted, not bare");
   });
 
   it("buildPolicyGetCommand preserves hyphenated sandbox name", () => {
     const cmd = buildPolicyGetCommand("my-assistant");
-    assert.ok(cmd.includes('"my-assistant"'), `Expected quoted name in: ${cmd}`);
+    assert.ok(cmd.includes("'my-assistant'"), `Expected quoted name in: ${cmd}`);
   });
 
   it("buildPolicySetCommand preserves multi-hyphen names", () => {
     const cmd = buildPolicySetCommand("/tmp/p.yaml", "my-dev-assistant-v2");
-    assert.ok(cmd.includes('"my-dev-assistant-v2"'));
+    assert.ok(cmd.includes("'my-dev-assistant-v2'"));
   });
 
   it("buildPolicySetCommand preserves single-char name", () => {
     // If WSL truncates "my-assistant" to "m", the single-char name should
     // still be quoted and passed through unchanged
     const cmd = buildPolicySetCommand("/tmp/p.yaml", "m");
-    assert.ok(cmd.includes('"m"'));
+    assert.ok(cmd.includes("'m'"));
   });
 
   it("applyPreset rejects truncated/invalid sandbox name", () => {
diff --git a/test/runner.test.js b/test/runner.test.js
index c5cc692ba..ffe064fc0 100644
--- a/test/runner.test.js
+++ b/test/runner.test.js
@@ -179,16 +179,25 @@ describe("runner helpers", () => {
     });
 
     it("CLI rejects malicious sandbox names before shell commands (e2e)", () => {
-      const result = spawnSync("node", [
-        path.join(__dirname, "..", "bin", "nemoclaw.js"),
-        "test; whoami",
-        "connect",
-      ], {
-        encoding: "utf-8",
-        timeout: 10000,
-        cwd: path.join(__dirname, ".."),
-      });
-      assert.notEqual(result.status, 0, "CLI should reject malicious sandbox name");
+      const fs = require("fs");
+      const os = require("os");
+      const canaryDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-canary-"));
+      const canary = path.join(canaryDir, "executed");
+      try {
+        const result = spawnSync("node", [
+          path.join(__dirname, "..", "bin", "nemoclaw.js"),
+          `test; touch ${canary}`,
+          "connect",
+        ], {
+          encoding: "utf-8",
+          timeout: 10000,
+          cwd: path.join(__dirname, ".."),
+        });
+        assert.notEqual(result.status, 0, "CLI should reject malicious sandbox name");
+        assert.equal(fs.existsSync(canary), false, "shell payload must never execute");
+      } finally {
+        fs.rmSync(canaryDir, { recursive: true, force: true });
+      }
     });
 
     it("telegram bridge validates SANDBOX_NAME on startup", () => {