fix: address code review findings on security PR

- Fix setupSpark(): pass NVIDIA_API_KEY via env option, not shell string
- Remove dead sandboxEnv variable in createSandbox(), clarify comment
  on openshell env subcommand protocol
- Add assertSafeName() at dispatch entry before registry lookup,
  blocking metacharacters even for unregistered sandbox names
- Validate NEMOCLAW_GPU with dedicated regex allowing colons/dots,
  instead of stripping chars before assertSafeName
- Remove no-op local `execSync("echo ok")` from SSH wait loop
- Convert waitForNimHealth() curl to runCaptureArgv for consistency

Author: gn00295120

bin/lib/nim.js

@@ -155,7 +155,7 @@ function waitForNimHealth(port = 8000, timeout = 300) {
 
   while ((Date.now() - start) / 1000 < timeout) {
     try {
-      const result = runCapture(`curl -sf http://localhost:${port}/v1/models`, {
+      const result = runCaptureArgv("curl", ["-sf", `http://localhost:${port}/v1/models`], {
         ignoreError: true,
       });
       if (result) {

bin/lib/onboard.js

@@ -190,11 +190,10 @@ 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';
-  // Pass secrets via environment, not command-line arguments
-  const sandboxEnv = { CHAT_UI_URL: chatUiUrl };
-  if (process.env.NVIDIA_API_KEY) {
-    sandboxEnv.NVIDIA_API_KEY = process.env.NVIDIA_API_KEY;
-  }
+  // Note: openshell sandbox create uses `-- env K=V` to set env vars inside
+  // the sandbox. These appear in the process argument list but are required by
+  // the openshell CLI protocol. The values are validated (chatUiUrl is a URL,
+  // NVIDIA_API_KEY starts with "nvapi-") and passed via argv (no shell expansion).
   createArgv.push("--", "env",
     `CHAT_UI_URL=${chatUiUrl}`,
     ...(process.env.NVIDIA_API_KEY ? [`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`] : []),

bin/nemoclaw.js

@@ -44,7 +44,9 @@ async function setup() {
 
 async function setupSpark() {
   await ensureApiKey();
-  run(`sudo -E NVIDIA_API_KEY="${process.env.NVIDIA_API_KEY}" bash "${SCRIPTS}/setup-spark.sh"`);
+  runArgv("sudo", ["-E", "bash", `${SCRIPTS}/setup-spark.sh`], {
+    env: { ...process.env, NVIDIA_API_KEY: process.env.NVIDIA_API_KEY },
+  });
 }
 
 async function deploy(instanceName) {
@@ -64,7 +66,10 @@ async function deploy(instanceName) {
   }
   const name = instanceName;
   const gpu = process.env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1";
-  assertSafeName(gpu.replace(/[:.]/g, "-"), "NEMOCLAW_GPU");
+  if (!/^[a-zA-Z0-9][a-zA-Z0-9_.:-]{0,127}$/.test(gpu)) {
+    console.error(`  Invalid NEMOCLAW_GPU: "${gpu}". Only alphanumerics, hyphens, underscores, dots, and colons are allowed.`);
+    process.exit(1);
+  }
 
   console.log("");
   console.log(`  Deploying NemoClaw to Brev instance: ${name}`);
@@ -96,11 +101,8 @@ async function deploy(instanceName) {
 
   console.log("  Waiting for SSH...");
   for (let i = 0; i < 60; i++) {
-    try {
-      execSync("echo ok", { encoding: "utf-8", stdio: "pipe", timeout: 10000 });
-      const r = spawnSync("ssh", [...sshOpts, name, "echo ok"], { encoding: "utf-8", stdio: "pipe", timeout: 10000 });
-      if (r.status === 0) break;
-    } catch {}
+    const r = spawnSync("ssh", [...sshOpts, name, "echo ok"], { encoding: "utf-8", stdio: "pipe", timeout: 10000 });
+    if (r.status === 0) break;
     if (i === 59) {
       console.error(`  Timed out waiting for SSH to ${name}`);
       process.exit(1);
@@ -345,6 +347,9 @@ const [cmd, ...args] = process.argv.slice(2);
   }
 
   // Sandbox-scoped commands: nemoclaw <name> <action>
+  // Validate the name early — before any lookup or use — to block
+  // shell metacharacters regardless of whether the sandbox exists.
+  assertSafeName(cmd, "sandbox name");
   const sandbox = registry.getSandbox(cmd);
   if (sandbox) {
     const action = args[0] || "connect";