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, 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 d9bc89e84..d1b7e5791 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -8,7 +8,7 @@ const fs = require("fs"); const os = require("os"); const path = require("path"); -const { ROOT, SCRIPTS, run, runCapture } = require("./runner"); +const { ROOT, SCRIPTS, run, runCapture, shellQuote } = require("./runner"); const { getDefaultOllamaModel, getLocalProviderBaseUrl, @@ -84,10 +84,6 @@ function step(n, total, msg) { console.log(` ${"─".repeat(50)}`); } -function shellQuote(value) { - return `'${String(value).replace(/'/g, `'\\''`)}'`; -} - function getInstalledOpenshellVersion(versionOutput = null) { const output = String(versionOutput ?? runCapture("openshell -V", { ignoreError: true })).trim(); const match = output.match(/openshell\s+([0-9]+\.[0-9]+\.[0-9]+)/i); @@ -447,9 +443,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 @@ -711,12 +707,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") { @@ -735,7 +731,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") { @@ -755,7 +751,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..240294bda 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) { @@ -166,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/lib/runner.js b/bin/lib/runner.js index e643394f0..d0ca4ceea 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..220f1aa1c 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,43 @@ 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 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 = path.join(os.tmpdir(), `nemoclaw-env-${Date.now()}`); + 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 "${envTmp}" ${name}:/home/ubuntu/nemoclaw/.env`); - fs.unlinkSync(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 ${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 +277,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 +295,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 +308,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 +351,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 +436,7 @@ const [cmd, ...args] = process.argv.slice(2); // Sandbox-scoped commands: nemoclaw const sandbox = registry.getSandbox(cmd); if (sandbox) { + validateName(cmd, "sandbox name"); const action = args[0] || "connect"; const actionArgs = args.slice(1); 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/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/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 024b564f6..ffe064fc0 100644 --- a/test/runner.test.js +++ b/test/runner.test.js @@ -81,4 +81,130 @@ describe("runner helpers", () => { assert.equal(calls[0][2].env.OPENSHELL_CLUSTER_IMAGE, "ghcr.io/nvidia/openshell/cluster:0.0.12"); assert.equal(calls[0][2].env.PATH, "/usr/local/bin:/usr/bin"); }); + + 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 /'"); + 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/); + }); + }); + + 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 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 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", () => { + 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"); + }); + }); });