Skip to content

Commit 4269a51

Browse files
committed
fix: quote shell interpolations in deploy and harden SSH
Extract deploy helpers (validateInstanceName, buildSshCommand, buildRsyncCommand) to prevent shell injection via instance names. Replace StrictHostKeyChecking=no with accept-new (TOFU) across all SSH and rsync commands in the deploy path. Signed-off-by: Brian Taylor <brian@briantaylor.xyz> Signed-off-by: Brian Taylor <brian.taylor818@gmail.com>
1 parent df47f67 commit 4269a51

File tree

3 files changed

+145
-8
lines changed

3 files changed

+145
-8
lines changed

bin/lib/deploy.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
//
4+
// Deploy helpers — input validation and shell-safe command builders.
5+
6+
const INSTANCE_NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
7+
8+
function validateInstanceName(name) {
9+
if (!name || !INSTANCE_NAME_RE.test(name)) {
10+
throw new Error(
11+
`Invalid instance name: ${JSON.stringify(name)}. ` +
12+
"Must start with alphanumeric and contain only [a-zA-Z0-9._-]."
13+
);
14+
}
15+
}
16+
17+
function buildSshCommand(host, remoteCmd) {
18+
const args = [
19+
"ssh",
20+
"-o", "StrictHostKeyChecking=accept-new",
21+
"-o", "LogLevel=ERROR",
22+
];
23+
args.push(shellQuote(host));
24+
if (remoteCmd) args.push(shellQuote(remoteCmd));
25+
return args.join(" ");
26+
}
27+
28+
function buildRsyncCommand(sources, host, dest) {
29+
const sshOpt = '"ssh -o StrictHostKeyChecking=accept-new -o LogLevel=ERROR"';
30+
const quotedSources = sources.map(shellQuote).join(" ");
31+
return `rsync -az --delete --exclude node_modules --exclude .git --exclude src -e ${sshOpt} ${quotedSources} ${shellQuote(host + ":" + dest)}`;
32+
}
33+
34+
function shellQuote(s) {
35+
// Simple single-quote wrapping — escape any embedded single quotes
36+
return "'" + s.replace(/'/g, "'\\''") + "'";
37+
}
38+
39+
module.exports = {
40+
INSTANCE_NAME_RE,
41+
validateInstanceName,
42+
buildSshCommand,
43+
buildRsyncCommand,
44+
shellQuote,
45+
};

bin/nemoclaw.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const {
1717
const registry = require("./lib/registry");
1818
const nim = require("./lib/nim");
1919
const policies = require("./lib/policies");
20+
const { validateInstanceName, buildSshCommand, buildRsyncCommand, shellQuote } = require("./lib/deploy");
2021

2122
// ── Global commands ──────────────────────────────────────────────
2223

@@ -62,6 +63,7 @@ async function deploy(instanceName) {
6263
await ensureGithubToken();
6364
}
6465
const name = instanceName;
66+
validateInstanceName(name);
6567
const gpu = process.env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1";
6668

6769
console.log("");
@@ -83,7 +85,7 @@ async function deploy(instanceName) {
8385

8486
if (!exists) {
8587
console.log(` Creating Brev instance '${name}' (${gpu})...`);
86-
run(`brev create ${name} --gpu "${gpu}"`);
88+
run(`brev create ${shellQuote(name)} --gpu "${gpu}"`);
8789
} else {
8890
console.log(` Brev instance '${name}' already exists.`);
8991
}
@@ -93,7 +95,7 @@ async function deploy(instanceName) {
9395
console.log(" Waiting for SSH...");
9496
for (let i = 0; i < 60; i++) {
9597
try {
96-
execSync(`ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no ${name} 'echo ok' 2>/dev/null`, { encoding: "utf-8", stdio: "pipe" });
98+
execSync(`ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=accept-new ${shellQuote(name)} 'echo ok' 2>/dev/null`, { encoding: "utf-8", stdio: "pipe" });
9799
break;
98100
} catch {
99101
if (i === 59) {
@@ -105,8 +107,12 @@ async function deploy(instanceName) {
105107
}
106108

107109
console.log(" Syncing NemoClaw to VM...");
108-
run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'mkdir -p /home/ubuntu/nemoclaw'`);
109-
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/`);
110+
run(buildSshCommand(name, "mkdir -p /home/ubuntu/nemoclaw"));
111+
run(buildRsyncCommand(
112+
[`${ROOT}/scripts`, `${ROOT}/Dockerfile`, `${ROOT}/nemoclaw`, `${ROOT}/nemoclaw-blueprint`, `${ROOT}/bin`, `${ROOT}/package.json`],
113+
name,
114+
"/home/ubuntu/nemoclaw/"
115+
));
110116

111117
const envLines = [`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`];
112118
const ghToken = process.env.GITHUB_TOKEN;
@@ -115,21 +121,21 @@ async function deploy(instanceName) {
115121
if (tgToken) envLines.push(`TELEGRAM_BOT_TOKEN=${tgToken}`);
116122
const envTmp = path.join(os.tmpdir(), `nemoclaw-env-${Date.now()}`);
117123
fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 });
118-
run(`scp -q -o StrictHostKeyChecking=no -o LogLevel=ERROR "${envTmp}" ${name}:/home/ubuntu/nemoclaw/.env`);
124+
run(`scp -q -o StrictHostKeyChecking=accept-new -o LogLevel=ERROR "${envTmp}" ${shellQuote(name)}:/home/ubuntu/nemoclaw/.env`);
119125
fs.unlinkSync(envTmp);
120126

121127
console.log(" Running setup...");
122-
run(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
128+
run(`ssh -t -o StrictHostKeyChecking=accept-new -o LogLevel=ERROR ${shellQuote(name)} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/brev-setup.sh'`);
123129

124130
if (tgToken) {
125131
console.log(" Starting services...");
126-
run(`ssh -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/start-services.sh'`);
132+
run(buildSshCommand(name, "cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && bash scripts/start-services.sh"));
127133
}
128134

129135
console.log("");
130136
console.log(" Connecting to sandbox...");
131137
console.log("");
132-
run(`ssh -t -o StrictHostKeyChecking=no -o LogLevel=ERROR ${name} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw'`);
138+
run(`ssh -t -o StrictHostKeyChecking=accept-new -o LogLevel=ERROR ${shellQuote(name)} 'cd /home/ubuntu/nemoclaw && set -a && . .env && set +a && openshell sandbox connect nemoclaw'`);
133139
}
134140

135141
async function start() {

test/deploy.test.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
const { describe, it } = require("node:test");
5+
const assert = require("node:assert/strict");
6+
7+
const {
8+
validateInstanceName,
9+
buildSshCommand,
10+
buildRsyncCommand,
11+
shellQuote,
12+
} = require("../bin/lib/deploy");
13+
14+
describe("deploy helpers", () => {
15+
describe("validateInstanceName", () => {
16+
it("accepts valid names", () => {
17+
for (const name of ["my-box", "prod.server", "test_01", "a", "A1-b.c_d"]) {
18+
assert.doesNotThrow(() => validateInstanceName(name));
19+
}
20+
});
21+
22+
it("rejects names starting with hyphen", () => {
23+
assert.throws(() => validateInstanceName("-bad"), /Invalid instance name/);
24+
});
25+
26+
it("rejects shell injection", () => {
27+
assert.throws(() => validateInstanceName("foo;rm -rf /"), /Invalid instance name/);
28+
});
29+
30+
it("rejects command substitution", () => {
31+
assert.throws(() => validateInstanceName("$(whoami)"), /Invalid instance name/);
32+
});
33+
34+
it("rejects empty string", () => {
35+
assert.throws(() => validateInstanceName(""), /Invalid instance name/);
36+
});
37+
38+
it("rejects names with spaces", () => {
39+
assert.throws(() => validateInstanceName("foo bar"), /Invalid instance name/);
40+
});
41+
});
42+
43+
describe("buildSshCommand", () => {
44+
it("uses StrictHostKeyChecking=accept-new", () => {
45+
const cmd = buildSshCommand("myhost", "ls");
46+
assert.ok(cmd.includes("StrictHostKeyChecking=accept-new"));
47+
assert.ok(!cmd.includes("StrictHostKeyChecking=no"));
48+
});
49+
50+
it("quotes host and remote command", () => {
51+
const cmd = buildSshCommand("myhost", "echo hello");
52+
assert.ok(cmd.includes("'myhost'"));
53+
assert.ok(cmd.includes("'echo hello'"));
54+
});
55+
56+
it("works without remote command", () => {
57+
const cmd = buildSshCommand("myhost");
58+
assert.ok(cmd.includes("'myhost'"));
59+
assert.ok(cmd.startsWith("ssh "));
60+
});
61+
});
62+
63+
describe("buildRsyncCommand", () => {
64+
it("quotes source paths and destination", () => {
65+
const cmd = buildRsyncCommand(["/tmp/a", "/tmp/b"], "host", "/dest/");
66+
assert.ok(cmd.includes("'/tmp/a'"));
67+
assert.ok(cmd.includes("'/tmp/b'"));
68+
assert.ok(cmd.includes("'host:/dest/'"));
69+
});
70+
71+
it("uses accept-new in ssh option", () => {
72+
const cmd = buildRsyncCommand(["/tmp/a"], "host", "/dest/");
73+
assert.ok(cmd.includes("accept-new"));
74+
});
75+
});
76+
77+
describe("shellQuote", () => {
78+
it("wraps in single quotes", () => {
79+
assert.equal(shellQuote("hello"), "'hello'");
80+
});
81+
82+
it("escapes embedded single quotes", () => {
83+
assert.equal(shellQuote("it's"), "'it'\\''s'");
84+
});
85+
});
86+
});

0 commit comments

Comments
 (0)