diff --git a/bin/lib/runner.js b/bin/lib/runner.js index 3b09e4fb8ba..7ce41fc0c90 100644 --- a/bin/lib/runner.js +++ b/bin/lib/runner.js @@ -13,36 +13,50 @@ if (dockerHost) { process.env.DOCKER_HOST = dockerHost.dockerHost; } +/** + * Run a shell command via bash, streaming stdout/stderr (redacted) to the terminal. + * Exits the process on failure unless opts.ignoreError is true. + */ function run(cmd, opts = {}) { - const stdio = opts.stdio ?? ["ignore", "inherit", "inherit"]; + const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"]; const result = spawnSync("bash", ["-c", cmd], { ...opts, stdio, cwd: ROOT, env: { ...process.env, ...opts.env }, }); + writeRedactedResult(result, stdio); if (result.status !== 0 && !opts.ignoreError) { - console.error(` Command failed (exit ${result.status}): ${cmd.slice(0, 80)}`); + console.error(` Command failed (exit ${result.status}): ${redact(cmd).slice(0, 80)}`); process.exit(result.status || 1); } return result; } +/** + * Run a shell command interactively (stdin inherited) while capturing and redacting stdout/stderr. + * Exits the process on failure unless opts.ignoreError is true. + */ function runInteractive(cmd, opts = {}) { - const stdio = opts.stdio ?? "inherit"; + const stdio = opts.stdio ?? ["inherit", "pipe", "pipe"]; const result = spawnSync("bash", ["-c", cmd], { ...opts, stdio, cwd: ROOT, env: { ...process.env, ...opts.env }, }); + writeRedactedResult(result, stdio); if (result.status !== 0 && !opts.ignoreError) { - console.error(` Command failed (exit ${result.status}): ${cmd.slice(0, 80)}`); + console.error(` Command failed (exit ${result.status}): ${redact(cmd).slice(0, 80)}`); process.exit(result.status || 1); } return result; } +/** + * Run a shell command and return its stdout as a trimmed string. + * Throws a redacted error on failure, or returns '' when opts.ignoreError is true. + */ function runCapture(cmd, opts = {}) { try { return execSync(cmd, { @@ -54,7 +68,98 @@ function runCapture(cmd, opts = {}) { }).trim(); } catch (err) { if (opts.ignoreError) return ""; - throw err; + throw redactError(err); + } +} + +/** + * Redact known secret patterns from a string to prevent accidental leaks + * in CLI log and error output. Covers NVIDIA API keys, bearer tokens, + * generic API key assignments, and base64-style long tokens. + */ +const SECRET_PATTERNS = [ + /nvapi-[A-Za-z0-9_-]{10,}/g, + /nvcf-[A-Za-z0-9_-]{10,}/g, + /ghp_[A-Za-z0-9_-]{10,}/g, + /(?<=Bearer\s+)[A-Za-z0-9_.+/=-]{10,}/gi, + /(?<=(?:_KEY|API_KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL)[=: ]['"]?)[A-Za-z0-9_.+/=-]{10,}/gi, +]; + +/** + * Partially redact a matched secret string: keep the first 4 chars and replace + * the rest with asterisks (capped at 20 asterisks). + */ +function redactMatch(match) { + return match.slice(0, 4) + "*".repeat(Math.min(match.length - 4, 20)); +} + +/** + * Redact credentials from a URL string: clears url.password and blanks + * known auth-style query params (auth, sig, signature, token, access_token). + * Returns the original value unchanged if it cannot be parsed as a URL. + */ +function redactUrl(value) { + if (typeof value !== "string" || value.length === 0) return value; + try { + const url = new URL(value); + if (url.password) { + url.password = "****"; + } + for (const key of [...url.searchParams.keys()]) { + if (/(^|[-_])(?:signature|sig|token|auth|access_token)$/i.test(key)) { + url.searchParams.set(key, "****"); + } + } + return url.toString(); + } catch { + return value; + } +} + +/** + * Redact known secret patterns and authenticated URLs from a string. + * Non-string values are returned unchanged. + */ +function redact(str) { + if (typeof str !== "string") return str; + let out = str.replace(/https?:\/\/[^\s'"]+/g, redactUrl); + for (const pat of SECRET_PATTERNS) { + out = out.replace(pat, redactMatch); + } + return out; +} + +/** + * Redact sensitive fields on an error object before surfacing it to callers. + * NOTE: this mutates the original error instance in place. + */ +function redactError(err) { + if (!err || typeof err !== "object") return err; + const originalMessage = typeof err.message === "string" ? err.message : null; + if (typeof err.message === "string") err.message = redact(err.message); + if (typeof err.cmd === "string") err.cmd = redact(err.cmd); + if (typeof err.stdout === "string") err.stdout = redact(err.stdout); + if (typeof err.stderr === "string") err.stderr = redact(err.stderr); + if (Array.isArray(err.output)) { + err.output = err.output.map((value) => (typeof value === "string" ? redact(value) : value)); + } + if (originalMessage && typeof err.stack === "string") { + err.stack = err.stack.replaceAll(originalMessage, err.message); + } + return err; +} + +/** + * Write redacted stdout/stderr from a spawnSync result to the parent process streams. + * No-op when stdio is 'inherit' or not an array. + */ +function writeRedactedResult(result, stdio) { + if (!result || stdio === "inherit" || !Array.isArray(stdio)) return; + if (stdio[1] === "pipe" && result.stdout) { + process.stdout.write(redact(result.stdout.toString())); + } + if (stdio[2] === "pipe" && result.stderr) { + process.stderr.write(redact(result.stderr.toString())); } } @@ -85,4 +190,13 @@ function validateName(name, label = "name") { return name; } -module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote, validateName }; +module.exports = { + ROOT, + SCRIPTS, + redact, + run, + runCapture, + runInteractive, + shellQuote, + validateName, +}; diff --git a/test/runner.test.js b/test/runner.test.js index 9b4e2d0f7d0..120c532c201 100644 --- a/test/runner.test.js +++ b/test/runner.test.js @@ -6,7 +6,7 @@ import childProcess from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { runCapture } from "../bin/lib/runner"; @@ -39,7 +39,7 @@ describe("runner helpers", () => { // @ts-expect-error — intentional partial mock for testing childProcess.spawnSync = (...args) => { calls.push(args); - return { status: 0 }; + return { status: 0, stdout: "", stderr: "" }; }; try { @@ -53,8 +53,8 @@ describe("runner helpers", () => { } expect(calls).toHaveLength(2); - expect(calls[0][2].stdio).toEqual(["ignore", "inherit", "inherit"]); - expect(calls[1][2].stdio).toBe("inherit"); + expect(calls[0][2].stdio).toEqual(["ignore", "pipe", "pipe"]); + expect(calls[1][2].stdio).toEqual(["inherit", "pipe", "pipe"]); }); }); @@ -83,7 +83,7 @@ describe("runner env merging", () => { // @ts-expect-error — intentional partial mock for testing childProcess.spawnSync = (...args) => { calls.push(args); - return { status: 0 }; + return { status: 0, stdout: "", stderr: "" }; }; try { @@ -170,7 +170,233 @@ describe("validateName", () => { }); }); +describe("redact", () => { + it("masks NVIDIA API keys", () => { + const { redact } = require(runnerPath); + expect(redact("key is nvapi-abc123XYZ_def456")).toBe("key is nvap******************"); + }); + + it("masks NVCF keys", () => { + const { redact } = require(runnerPath); + expect(redact("nvcf-abcdef1234567890")).toBe("nvcf*****************"); + }); + + it("masks bearer tokens", () => { + const { redact } = require(runnerPath); + expect(redact("Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.payload")).toBe( + "Authorization: Bearer eyJh********************", + ); + }); + + it("masks key assignments in commands", () => { + const { redact } = require(runnerPath); + expect(redact("export NVIDIA_API_KEY=nvapi-realkey12345")).toContain("nvap"); + expect(redact("export NVIDIA_API_KEY=nvapi-realkey12345")).not.toContain("realkey12345"); + }); + + it("masks variables ending in _KEY", () => { + const { redact } = require(runnerPath); + const output = redact('export SERVICE_KEY="supersecretvalue12345"'); + expect(output).not.toContain("supersecretvalue12345"); + expect(output).toContain('export SERVICE_KEY="supe'); + }); + + it("masks bare GitHub personal access tokens", () => { + const { redact } = require(runnerPath); + const output = redact("token ghp_abcdefghijklmnopqrstuvwxyz1234567890"); + expect(output).toContain("ghp_"); + expect(output).not.toContain("abcdefghijklmnopqrstuvwxyz1234567890"); + }); + + it("masks bearer tokens case-insensitively", () => { + const { redact } = require(runnerPath); + expect(redact("authorization: bearer someBearerToken")).toContain("some****"); + expect(redact("authorization: bearer someBearerToken")).not.toContain("someBearerToken"); + expect(redact("AUTHORIZATION: BEARER someBearerToken")).toContain("some****"); + expect(redact("AUTHORIZATION: BEARER someBearerToken")).not.toContain("someBearerToken"); + }); + + it("masks bearer tokens with repeated spacing", () => { + const { redact } = require(runnerPath); + const output = redact("Authorization: Bearer someBearerToken"); + expect(output).toContain("some****"); + expect(output).not.toContain("someBearerToken"); + }); + + it("masks quoted assignment values", () => { + const { redact } = require(runnerPath); + const output = redact('API_KEY="secret123abc"'); + expect(output).not.toContain("secret123abc"); + expect(output).toContain('API_KEY="sec'); + }); + + it("masks multiple secrets in one string", () => { + const { redact } = require(runnerPath); + const output = redact("nvapi-firstkey12345 nvapi-secondkey67890"); + expect(output).not.toContain("firstkey12345"); + expect(output).not.toContain("secondkey67890"); + expect(output).toContain("nvap"); + expect(output).toContain(" "); + }); + + it("masks URL credentials and auth query parameters", () => { + const { redact } = require(runnerPath); + const output = redact( + "https://alice:secret@example.com/v1/models?auth=abc123456789&sig=def987654321&keep=yes", + ); + expect(output).toBe("https://alice:****@example.com/v1/models?auth=****&sig=****&keep=yes"); + }); + + it("masks auth-style query parameters case-insensitively", () => { + const { redact } = require(runnerPath); + const output = redact("https://example.com?Signature=secret123456&AUTH=anothersecret123"); + expect(output).toBe("https://example.com/?Signature=****&AUTH=****"); + }); + + it("leaves non-secret strings untouched", () => { + const { redact } = require(runnerPath); + expect(redact("docker run --name my-sandbox")).toBe("docker run --name my-sandbox"); + expect(redact("openshell sandbox list")).toBe("openshell sandbox list"); + }); + + it("handles non-string input gracefully", () => { + const { redact } = require(runnerPath); + expect(redact(null)).toBe(null); + expect(redact(undefined)).toBe(undefined); + expect(redact(42)).toBe(42); + }); +}); + describe("regression guards", () => { + it("runCapture redacts secrets before rethrowing errors", () => { + const originalExecSync = childProcess.execSync; + childProcess.execSync = () => { + throw new Error( + 'command failed: export SERVICE_KEY="supersecretvalue12345" ghp_abcdefghijklmnopqrstuvwxyz1234567890', + ); + }; + + try { + delete require.cache[require.resolve(runnerPath)]; + const { runCapture } = require(runnerPath); + + let error; + try { + runCapture("echo nope"); + } catch (err) { + error = err; + } + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain("ghp_"); + expect(error.message).not.toContain("supersecretvalue12345"); + expect(error.message).not.toContain("abcdefghijklmnopqrstuvwxyz1234567890"); + } finally { + childProcess.execSync = originalExecSync; + delete require.cache[require.resolve(runnerPath)]; + } + }); + + it("runCapture redacts execSync error cmd/output fields", () => { + const originalExecSync = childProcess.execSync; + childProcess.execSync = () => { + const err = /** @type {any} */ (new Error("command failed")); + err.cmd = "echo nvapi-aaaabbbbcccc1111 && echo ghp_abcdefghijklmnopqrstuvwxyz123456"; + err.output = ["stdout: nvapi-aaaabbbbcccc1111", "stderr: PASSWORD=secret123456"]; + throw err; + }; + + try { + delete require.cache[require.resolve(runnerPath)]; + const { runCapture } = require(runnerPath); + + let error; + try { + runCapture("echo nope"); + } catch (err) { + error = /** @type {any} */ (err); + } + + expect(error).toBeDefined(); + expect(error).toBeInstanceOf(Error); + expect(error.cmd).not.toContain("nvapi-aaaabbbbcccc1111"); + expect(error.cmd).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz123456"); + expect(Array.isArray(error.output)).toBe(true); + expect(error.output[0]).not.toContain("nvapi-aaaabbbbcccc1111"); + expect(error.output[1]).not.toContain("secret123456"); + expect(error.output[0]).toContain("****"); + expect(error.output[1]).toContain("****"); + } finally { + childProcess.execSync = originalExecSync; + delete require.cache[require.resolve(runnerPath)]; + } + }); + + it("run redacts captured child output before printing on failure", () => { + const originalSpawnSync = childProcess.spawnSync; + const originalExit = process.exit; + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = () => ({ + status: 1, + stdout: "token ghp_abcdefghijklmnopqrstuvwxyz1234567890\n", + stderr: 'export SERVICE_KEY="supersecretvalue12345"\n', + }); + process.exit = (code) => { + throw new Error(`exit:${code}`); + }; + + try { + delete require.cache[require.resolve(runnerPath)]; + const { run } = require(runnerPath); + expect(() => run("echo fail")).toThrow("exit:1"); + expect(stdoutSpy).toHaveBeenCalledWith("token ghp_********************\n"); + expect(stderrSpy).toHaveBeenCalledWith('export SERVICE_KEY="supe*****************"\n'); + expect(errorSpy).toHaveBeenCalledWith(" Command failed (exit 1): echo fail"); + } finally { + childProcess.spawnSync = originalSpawnSync; + process.exit = originalExit; + stdoutSpy.mockRestore(); + stderrSpy.mockRestore(); + errorSpy.mockRestore(); + delete require.cache[require.resolve(runnerPath)]; + } + }); + + it("runInteractive keeps stdin inherited while redacting captured output", () => { + const originalSpawnSync = childProcess.spawnSync; + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const calls = []; + + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = (...args) => { + calls.push(args); + return { + status: 0, + stdout: "visit https://alice:secret@example.com/?token=abc123456789\n", + stderr: "", + }; + }; + + try { + delete require.cache[require.resolve(runnerPath)]; + const { runInteractive } = require(runnerPath); + runInteractive("echo interactive"); + expect(calls[0][2].stdio).toEqual(["inherit", "pipe", "pipe"]); + expect(stdoutSpy).toHaveBeenCalledWith("visit https://alice:****@example.com/?token=****\n"); + expect(stderrSpy).not.toHaveBeenCalled(); + } finally { + childProcess.spawnSync = originalSpawnSync; + stdoutSpy.mockRestore(); + stderrSpy.mockRestore(); + delete require.cache[require.resolve(runnerPath)]; + } + }); + it("nemoclaw.js does not use execSync", () => { const src = fs.readFileSync( path.join(import.meta.dirname, "..", "bin", "nemoclaw.js"),