diff --git a/bin/lib/runner.js b/bin/lib/runner.js index d0ca4ceeac..ae0314a861 100644 --- a/bin/lib/runner.js +++ b/bin/lib/runner.js @@ -8,6 +8,33 @@ const { detectDockerHost } = require("./platform"); const ROOT = path.resolve(__dirname, "..", ".."); const SCRIPTS = path.join(ROOT, "scripts"); +/** + * Redact known secret patterns from a string to prevent credential leaks + * in logs, error messages, and terminal output. + * + * Matches: + * - Environment-style assignments: NVIDIA_API_KEY=sk-... → NVIDIA_API_KEY= + * - NVIDIA API key prefix: nvapi-Abc123... → + * - GitHub PAT prefix: ghp_Abc123... → + * - Bearer tokens: Bearer eyJhb... → Bearer + */ +const SECRET_PATTERNS = [ + { re: /(NVIDIA_API_KEY|API_KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL|_KEY)=\S+/gi, replacement: "$1=" }, + { re: /nvapi-[A-Za-z0-9_-]{10,}/g, replacement: "" }, + { re: /ghp_[A-Za-z0-9]{30,}/g, replacement: "" }, + { re: /(Bearer )\S+/gi, replacement: "$1" }, +]; + +function redactSecrets(str) { + if (typeof str !== "string") return str; + let result = str; + for (const { re, replacement } of SECRET_PATTERNS) { + re.lastIndex = 0; + result = result.replace(re, replacement); + } + return result; +} + const dockerHost = detectDockerHost(); if (dockerHost) { process.env.DOCKER_HOST = dockerHost.dockerHost; @@ -22,7 +49,7 @@ function run(cmd, opts = {}) { env: { ...process.env, ...opts.env }, }); if (result.status !== 0 && !opts.ignoreError) { - console.error(` Command failed (exit ${result.status}): ${cmd.slice(0, 80)}`); + console.error(` Command failed (exit ${result.status}): ${redactSecrets(cmd.slice(0, 80))}`); process.exit(result.status || 1); } return result; @@ -37,7 +64,7 @@ function runInteractive(cmd, opts = {}) { env: { ...process.env, ...opts.env }, }); if (result.status !== 0 && !opts.ignoreError) { - console.error(` Command failed (exit ${result.status}): ${cmd.slice(0, 80)}`); + console.error(` Command failed (exit ${result.status}): ${redactSecrets(cmd.slice(0, 80))}`); process.exit(result.status || 1); } return result; @@ -85,4 +112,4 @@ function validateName(name, label = "name") { return name; } -module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote, validateName }; +module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote, validateName, redactSecrets }; diff --git a/bin/nemoclaw.js b/bin/nemoclaw.js index 2010cfeb22..9f32dab00d 100755 --- a/bin/nemoclaw.js +++ b/bin/nemoclaw.js @@ -97,7 +97,9 @@ async function setup() { async function setupSpark() { await ensureApiKey(); - run(`sudo -E NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)} bash "${SCRIPTS}/setup-spark.sh"`); + run(`sudo -E bash "${SCRIPTS}/setup-spark.sh"`, { + env: { NVIDIA_API_KEY: process.env.NVIDIA_API_KEY }, + }); } async function deploy(instanceName) { diff --git a/research/results.tsv b/research/results.tsv new file mode 100644 index 0000000000..a79fe577c1 --- /dev/null +++ b/research/results.tsv @@ -0,0 +1,4 @@ +experiment track description tests_total tests_pass tests_fail notes +0 baseline initial state — 194 tests, 188 pass, 6 fail 194 188 6 6 failures in installer preflight + uninstall CLI; 30 open issues; coverage: 93% lines, 98% functions, 87% branches +1 track1 fix 3 remaining test failures: uninstall symlink rm, installer sudo heuristic 194 194 0 uninstall.sh: check dir writability not symlink target; scripts/install.sh: check npm prefix writability instead of assuming nodesource needs sudo +2 track2 fix #579/#664: add redactSecrets to runner.js, fix API key exposure in setupSpark/walkthrough/telegram-bridge 203 203 0 added redactSecrets() with 4 patterns (env assignments, nvapi-, ghp_, Bearer); error messages now redacted; setupSpark passes key via env not cmdline; walkthrough.sh uses tmux -e; telegram-bridge uses SSH SendEnv; 9 new tests diff --git a/scripts/install.sh b/scripts/install.sh index 9e3dc1a2e0..5babe63f0a 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -414,10 +414,11 @@ rm -rf "$NEMOCLAW_SRC" mkdir -p "$(dirname "$NEMOCLAW_SRC")" git clone --depth 1 https://github.com/NVIDIA/NemoClaw.git "$NEMOCLAW_SRC" pre_extract_openclaw "$NEMOCLAW_SRC" || warn "Pre-extraction failed — npm install may fail if openclaw tarball is broken" -# Use sudo for npm link when the global prefix requires it (e.g., nodesource), -# but skip sudo if already root (e.g., Docker containers). +# Use sudo for npm link only when the global prefix directory is not writable +# by the current user (e.g., system-managed nodesource installs to /usr). SUDO="" -if [ "$NODE_MGR" = "nodesource" ] && [ "$(id -u)" -ne 0 ]; then +NPM_GLOBAL_PREFIX="$(npm config get prefix 2>/dev/null)" || true +if [ -n "$NPM_GLOBAL_PREFIX" ] && [ ! -w "$NPM_GLOBAL_PREFIX" ] && [ "$(id -u)" -ne 0 ]; then SUDO="sudo" fi (cd "$NEMOCLAW_SRC" && npm install --ignore-scripts && cd nemoclaw && npm install --ignore-scripts && npm run build && cd .. && $SUDO npm link) diff --git a/scripts/telegram-bridge.js b/scripts/telegram-bridge.js index e885b09f2d..9d671c449b 100755 --- a/scripts/telegram-bridge.js +++ b/scripts/telegram-bridge.js @@ -102,13 +102,13 @@ function runAgentInSandbox(message, sessionId) { const confPath = `${confDir}/config`; require("fs").writeFileSync(confPath, sshConfig, { mode: 0o600 }); - // 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. + // Pass API key via SendEnv + ssh config to avoid exposing it in + // process arguments (visible in ps aux / /proc/*/cmdline). 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 cmd = `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], { + const proc = spawn("ssh", ["-T", "-F", confPath, "-o", "SendEnv=NVIDIA_API_KEY", `openshell-${SANDBOX}`, cmd], { + env: { ...process.env, NVIDIA_API_KEY: API_KEY }, timeout: 120000, stdio: ["ignore", "pipe", "pipe"], }); diff --git a/scripts/walkthrough.sh b/scripts/walkthrough.sh index b50e24b062..7754e62f9a 100755 --- a/scripts/walkthrough.sh +++ b/scripts/walkthrough.sh @@ -69,7 +69,7 @@ if ! command -v tmux > /dev/null 2>&1; then echo "" echo " Terminal 2 (Agent):" echo " openshell sandbox connect nemoclaw" - echo " export NVIDIA_API_KEY=$NVIDIA_API_KEY" + echo " export NVIDIA_API_KEY=\$NVIDIA_API_KEY" echo " nemoclaw-start" echo " openclaw agent --agent main --local --session-id live" exit 0 @@ -83,9 +83,10 @@ tmux kill-session -t "$SESSION" 2>/dev/null || true # Create session with TUI on the left tmux new-session -d -s "$SESSION" -x 200 -y 50 "openshell term" -# Split right pane for the agent -tmux split-window -h -t "$SESSION" \ - "openshell sandbox connect nemoclaw -- bash -c 'export NVIDIA_API_KEY=$NVIDIA_API_KEY && nemoclaw-start openclaw agent --agent main --local --session-id live'" +# Split right pane for the agent — pass API key via env to avoid leaking +# the secret in process arguments (visible in ps aux / /proc/*/cmdline). +tmux split-window -h -t "$SESSION" -e "NVIDIA_API_KEY=$NVIDIA_API_KEY" \ + "openshell sandbox connect nemoclaw -- bash -c 'nemoclaw-start openclaw agent --agent main --local --session-id live'" # Even split tmux select-layout -t "$SESSION" even-horizontal diff --git a/test/runner.test.js b/test/runner.test.js index ffe064fc00..b998bba5fc 100644 --- a/test/runner.test.js +++ b/test/runner.test.js @@ -143,6 +143,58 @@ describe("runner helpers", () => { }); }); + describe("redactSecrets", () => { + it("redacts NVIDIA API key assignments", () => { + const { redactSecrets } = require(runnerPath); + assert.equal( + redactSecrets("NVIDIA_API_KEY=nvapi-abc123xyz"), + "NVIDIA_API_KEY=", + ); + }); + + it("redacts nvapi- prefixed tokens in free text", () => { + const { redactSecrets } = require(runnerPath); + const input = "using key nvapi-AbCdEfGhIj1234 for auth"; + assert.ok(!redactSecrets(input).includes("nvapi-AbCdEfGhIj1234")); + assert.ok(redactSecrets(input).includes("")); + }); + + it("redacts GitHub PATs", () => { + const { redactSecrets } = require(runnerPath); + const ghToken = "ghp_" + "a".repeat(36); + assert.equal(redactSecrets(`GITHUB_TOKEN=${ghToken}`), "GITHUB_TOKEN="); + }); + + it("redacts Bearer tokens", () => { + const { redactSecrets } = require(runnerPath); + assert.equal( + redactSecrets("Authorization: Bearer eyJhbGciOiJIUzI1Ni"), + "Authorization: Bearer ", + ); + }); + + it("redacts multiple secrets in one string", () => { + const { redactSecrets } = require(runnerPath); + const input = "NVIDIA_API_KEY=nvapi-secret123456 GITHUB_TOKEN=ghp_" + "b".repeat(36); + const result = redactSecrets(input); + assert.ok(!result.includes("nvapi-secret123456")); + assert.ok(!result.includes("ghp_")); + assert.ok(result.includes("")); + }); + + it("returns non-string values unchanged", () => { + const { redactSecrets } = require(runnerPath); + assert.equal(redactSecrets(null), null); + assert.equal(redactSecrets(undefined), undefined); + assert.equal(redactSecrets(42), 42); + }); + + it("leaves clean strings unchanged", () => { + const { redactSecrets } = require(runnerPath); + assert.equal(redactSecrets("bash setup.sh"), "bash setup.sh"); + }); + }); + describe("regression guards", () => { it("nemoclaw.js does not use execSync", () => { const fs = require("fs"); @@ -200,6 +252,30 @@ describe("runner helpers", () => { } }); + it("setupSpark does not embed API key in command string", () => { + const fs = require("fs"); + const src = fs.readFileSync(path.join(__dirname, "..", "bin", "nemoclaw.js"), "utf-8"); + // Extract the setupSpark function body — between "async function setupSpark" + // and the next top-level "async function" or "function" declaration. + const match = src.match(/async function setupSpark\b[\s\S]*?\n\}/); + assert.ok(match, "setupSpark function must exist"); + const body = match[0]; + assert.ok( + !body.includes("NVIDIA_API_KEY=") || body.includes("env:"), + "setupSpark must pass API key via env option, not in the command string", + ); + }); + + it("walkthrough.sh does not echo raw API key value", () => { + const fs = require("fs"); + const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "walkthrough.sh"), "utf-8"); + // The script should reference the variable name, not expand it unsafely + assert.ok( + !src.includes('echo " export NVIDIA_API_KEY=$NVIDIA_API_KEY"'), + "walkthrough.sh must not echo the raw API key value to terminal", + ); + }); + 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"); diff --git a/uninstall.sh b/uninstall.sh index 60d0567390..a51aa0c882 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -223,7 +223,7 @@ remove_file_with_optional_sudo() { return 0 fi - if [ -w "$path" ] || [ -w "$(dirname "$path")" ]; then + if [ -w "$(dirname "$path")" ]; then rm -f "$path" elif [ "${NEMOCLAW_NON_INTERACTIVE:-}" = "1" ] || [ ! -t 0 ]; then warn "Skipping privileged removal of $path in non-interactive mode."