From 71525befa4566b40db6784f970559e4b0eea7f69 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 18:44:56 -0700 Subject: [PATCH 1/6] fix(security): harden low-risk code scanning findings --- .../search_assets/modules/SearchEngine.js | 24 ++++++++--- .../scripts/slack-channel-guard.js | 17 +++++++- nemoclaw/src/onboard/config.ts | 14 ++++--- src/commands/internal/installer/plan.ts | 2 +- src/lib/actions/dev/npm-link-or-shim.ts | 5 ++- src/lib/actions/sandbox/rebuild.ts | 7 ++-- src/lib/adapters/openshell/client.test.ts | 4 +- src/lib/adapters/openshell/client.ts | 9 ++-- src/lib/cli/nemoclaw-oclif-command.ts | 4 +- src/lib/diagnostics/debug.ts | 11 +++-- src/lib/onboard.ts | 10 ++--- src/lib/onboard/preflight.ts | 8 +++- src/lib/onboard/summary.test.ts | 4 +- src/lib/onboard/summary.ts | 2 +- src/lib/security/redact.ts | 19 +++++++++ src/lib/state/onboard-session.ts | 5 ++- src/lib/tunnel/services.ts | 41 +++++++++++++++---- 17 files changed, 135 insertions(+), 51 deletions(-) diff --git a/docs/_ext/search_assets/modules/SearchEngine.js b/docs/_ext/search_assets/modules/SearchEngine.js index a1a5bea3415..b32c48baed5 100644 --- a/docs/_ext/search_assets/modules/SearchEngine.js +++ b/docs/_ext/search_assets/modules/SearchEngine.js @@ -495,19 +495,33 @@ class SearchEngine { return this.getDocumentAudience(doc); } + /** + * Extract safe Lunr query terms from user input. + */ + getSafeSearchTerms(query) { + const matches = String(query || '').toLowerCase().match(/[a-z0-9][a-z0-9._-]{0,63}/g); + return matches ? matches.slice(0, 10) : []; + } + /** * Perform search with multiple strategies */ performMultiStrategySearch(query) { + const terms = this.getSafeSearchTerms(query); + if (terms.length === 0) return []; + + const phrase = terms.join(' '); + const wildcardTerms = terms.map(term => `${term}*`).join(' '); + const fuzzyTerms = terms.map(term => `${term}~2`).join(' '); const strategies = [ // Exact phrase search with wildcards - `"${query}" ${query}*`, + `"${phrase}" ${wildcardTerms}`, // Fuzzy search with wildcards - `${query}* ${query}~2`, + `${wildcardTerms} ${fuzzyTerms}`, // Individual terms with boost - query.split(/\s+/).map(term => `${term}*`).join(' '), - // Fallback: just the query - query + wildcardTerms, + // Fallback: sanitized terms only + phrase ]; let allResults = []; diff --git a/nemoclaw-blueprint/scripts/slack-channel-guard.js b/nemoclaw-blueprint/scripts/slack-channel-guard.js index b524638930e..1ab1cff9d9f 100644 --- a/nemoclaw-blueprint/scripts/slack-channel-guard.js +++ b/nemoclaw-blueprint/scripts/slack-channel-guard.js @@ -37,6 +37,21 @@ 'An API error occurred: invalid_auth', ]; + function mentionsSlackHost(value) { + var tokens = String(value || '').split(/\s+/); + for (var i = 0; i < tokens.length; i++) { + var candidate = tokens[i].replace(/^[<("']+|[>),."']+$/g, ''); + try { + var parsed = new URL(candidate); + var host = parsed.hostname.toLowerCase(); + if (host === 'slack.com' || host.endsWith('.slack.com')) return true; + } catch (_e) { + if (/^(?:[a-z0-9-]+\.)*slack\.com(?::\d+)?$/i.test(candidate)) return true; + } + } + return false; + } + function isSlackRejection(reason) { if (!reason) return false; @@ -63,7 +78,7 @@ // servers, the error comes from the HTTP client (CONNECT tunnel // failure), not from @slack/ code. The stack won't contain @slack/ // but the error message or URL may reference the Slack hostname. - if (msg.indexOf('slack.com') !== -1) { + if (mentionsSlackHost(msg)) { return true; } diff --git a/nemoclaw/src/onboard/config.ts b/nemoclaw/src/onboard/config.ts index cb36e15295a..ac59a05eebe 100644 --- a/nemoclaw/src/onboard/config.ts +++ b/nemoclaw/src/onboard/config.ts @@ -1,7 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + writeFileSync, + unlinkSync, +} from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; @@ -138,10 +145,7 @@ function ensureConfigDir(): void { try { mkdirSync(configDir, { recursive: true }); } catch { - configDir = join(tmpdir(), ".nemoclaw"); - if (!existsSync(configDir)) { - mkdirSync(configDir, { recursive: true }); - } + configDir = mkdtempSync(join(tmpdir(), "nemoclaw-config-")); } } configDirCreated = true; diff --git a/src/commands/internal/installer/plan.ts b/src/commands/internal/installer/plan.ts index ece5c17ecfc..1a118a8f9c3 100644 --- a/src/commands/internal/installer/plan.ts +++ b/src/commands/internal/installer/plan.ts @@ -44,6 +44,6 @@ export default class InternalInstallerPlanCommand extends NemoClawCommand { }); if (flags.json) this.logJson(plan); - else console.log(`Installer plan: ref '${plan.installRef}', version '${plan.installerVersion}'`); + else console.log("Installer plan built. Re-run with --json for redacted details."); } } diff --git a/src/lib/actions/dev/npm-link-or-shim.ts b/src/lib/actions/dev/npm-link-or-shim.ts index c39300f7268..90ce993e917 100644 --- a/src/lib/actions/dev/npm-link-or-shim.ts +++ b/src/lib/actions/dev/npm-link-or-shim.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { randomUUID } from "node:crypto"; import { spawnSync, type SpawnSyncReturns } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; @@ -116,7 +117,7 @@ export function runNpmLinkOrShim( ((dir: string) => path.join( dir, - `nemoclaw.tmp.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}`, + `nemoclaw.tmp.${process.pid}.${Date.now()}.${randomUUID()}`, )); const run = deps.run ?? defaultRun; const commandPath = deps.commandPath ?? defaultCommandPath; @@ -124,7 +125,7 @@ export function runNpmLinkOrShim( if (env.NEMOCLAW_INSTALLING) return { status: 0 }; if (!isExecutable(binPath)) { - logError(`[nemoclaw] cannot expose CLI: ${binPath} is missing or not executable`); + logError("[nemoclaw] cannot expose CLI: launcher is missing or not executable"); return { status: 0 }; } diff --git a/src/lib/actions/sandbox/rebuild.ts b/src/lib/actions/sandbox/rebuild.ts index e70dff0cd2a..27ce3dc11fe 100644 --- a/src/lib/actions/sandbox/rebuild.ts +++ b/src/lib/actions/sandbox/rebuild.ts @@ -43,6 +43,7 @@ import * as nim from "../../inference/nim"; import * as policies from "../../policy"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import * as sandboxVersion from "../../sandbox/version"; +import { redact } from "../../security/redact"; import type { Session } from "../../state/onboard-session"; import * as onboardSession from "../../state/onboard-session"; import * as registry from "../../state/registry"; @@ -60,7 +61,7 @@ const agentRuntime = require("../../../../bin/lib/agent-runtime"); * Emit timestamped rebuild diagnostics when verbose rebuild logging is enabled. */ function _rebuildLog(msg: string) { - console.error(` ${D}[rebuild ${new Date().toISOString()}] ${msg}${R}`); + console.error(` ${D}[rebuild ${new Date().toISOString()}] ${redact(msg)}${R}`); } /** @@ -122,7 +123,7 @@ function preflightHermesProviderCredentials( nonEmptyString(process.env[hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV]) || nonEmptyString(process.env.NEMOCLAW_PROVIDER_KEY); log( - `Hermes Provider rebuild preflight: OpenShell provider missing; ${hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV} env=${envKey ? "present" : "missing"}`, + `Hermes Provider rebuild preflight: OpenShell provider missing; API key env=${envKey ? "present" : "missing"}`, ); if (envKey) { try { @@ -145,7 +146,7 @@ function preflightHermesProviderCredentials( console.error(" Hermes Provider credentials must be stored in OpenShell, not host-side files."); if (authMethod === "api_key") { console.error( - ` Export ${hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV} and rerun rebuild, or re-run ${CLI_NAME} onboard to register it.`, + ` Export the Hermes Provider API key and rerun rebuild, or re-run ${CLI_NAME} onboard to register it.`, ); } else { console.error(` Re-run ${CLI_NAME} onboard interactively to authorize Hermes Provider and register it with OpenShell.`); diff --git a/src/lib/adapters/openshell/client.test.ts b/src/lib/adapters/openshell/client.test.ts index bd3101e616c..d4bcaad1fa0 100644 --- a/src/lib/adapters/openshell/client.test.ts +++ b/src/lib/adapters/openshell/client.test.ts @@ -201,7 +201,7 @@ describe("openshell helpers", () => { exit: exitWithCode, }), ).toThrow("exit:1"); - expect(errors).toEqual([" Failed to start openshell status: spawn EACCES"]); + expect(errors).toEqual([" Failed to start OpenShell command: spawn EACCES"]); }); it("treats capture spawn failures as fatal errors", () => { @@ -218,7 +218,7 @@ describe("openshell helpers", () => { exit: exitWithCode, }), ).toThrow("exit:1"); - expect(errors).toEqual([" Failed to start openshell status: spawn ENOENT"]); + expect(errors).toEqual([" Failed to start OpenShell command: spawn ENOENT"]); }); it("reads the installed openshell version through the capture helper", () => { diff --git a/src/lib/adapters/openshell/client.ts b/src/lib/adapters/openshell/client.ts index d4b57975453..5e50370b7f6 100644 --- a/src/lib/adapters/openshell/client.ts +++ b/src/lib/adapters/openshell/client.ts @@ -75,13 +75,12 @@ export function versionGte(left = "0.0.0", right = "0.0.0"): boolean { } function handleSpawnError( - binary: string, - args: string[], + _binary: string, + _args: string[], error: Error, opts: OpenshellSpawnOptions, ): never { - const command = [binary, ...args].join(" "); - (opts.errorLine ?? console.error)(` Failed to start ${command}: ${error.message}`); + (opts.errorLine ?? console.error)(` Failed to start OpenShell command: ${error.message}`); return (opts.exit ?? ((code) => process.exit(code)))(1); } @@ -135,7 +134,7 @@ export function runOpenshellCommand( } if (result.status !== 0 && !opts.ignoreError) { (opts.errorLine ?? console.error)( - ` Command failed (exit ${result.status}): openshell ${args.join(" ")}`, + ` OpenShell command failed (exit ${result.status})`, ); return (opts.exit ?? ((code) => process.exit(code)))(result.status || 1); } diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index a780875d636..031a979e77b 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,8 @@ import { Command, Flags } from "@oclif/core"; +import { redactForLog } from "../security/redact"; + export type CommandExitResult = { exitCode?: number | null; message?: string | null; @@ -21,7 +23,7 @@ export abstract class NemoClawCommand extends Command { }; protected logJson(json: unknown): void { - console.log(JSON.stringify(json, null, 2)); + console.log(JSON.stringify(redactForLog(json), null, 2)); } protected setExitCode(code: number): void { diff --git a/src/lib/diagnostics/debug.ts b/src/lib/diagnostics/debug.ts index 14a8a6c12ce..5ef9f0c2bff 100644 --- a/src/lib/diagnostics/debug.ts +++ b/src/lib/diagnostics/debug.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { platform, tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; @@ -314,8 +314,9 @@ function collectSandboxInternals( section("Sandbox Internals"); - // Generate temporary SSH config - const sshConfigPath = join(tmpdir(), `nemoclaw-ssh-${String(Date.now())}`); + // Generate temporary SSH config in a private directory. + const sshConfigDir = mkdtempSync(join(tmpdir(), "nemoclaw-ssh-")); + const sshConfigPath = join(sshConfigDir, "config"); try { const sshResult = spawnSync("openshell", ["sandbox", "ssh-config", sandboxName], { timeout: TIMEOUT_MS, @@ -358,9 +359,7 @@ function collectSandboxInternals( ]); } } finally { - if (existsSync(sshConfigPath)) { - unlinkSync(sshConfigPath); - } + rmSync(sshConfigDir, { force: true, recursive: true }); } } diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 5edbca63c0a..9890f689f08 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -1554,7 +1554,7 @@ async function replaceNamedCredential( saveCredential(envName, key); process.env[envName] = key; console.log(""); - console.log(` ${envName} staged. Onboarding will register it with the OpenShell gateway.`); + console.log(" Credential staged. Onboarding will register it with the OpenShell gateway."); console.log(""); return key; } @@ -1965,7 +1965,7 @@ function verifyCompatibleEndpointSandboxSmoke(options: { !providerDetails.includes(options.credentialEnv) ) { console.warn( - ` ⚠ Gateway provider '${options.provider}' did not report ${options.credentialEnv}.`, + ` ⚠ Gateway provider '${options.provider}' did not report the selected credential binding.`, ); } @@ -5427,12 +5427,12 @@ async function createSandbox( .filter(Boolean); for (const serverId of serverIds) { if (!DISCORD_SNOWFLAKE_RE.test(serverId)) { - console.warn(` Warning: Discord server ID '${serverId}' does not look like a snowflake.`); + console.warn(" Warning: configured Discord server ID does not look like a snowflake."); } } for (const userId of userIds) { if (!DISCORD_SNOWFLAKE_RE.test(userId)) { - console.warn(` Warning: Discord user ID '${userId}' does not look like a snowflake.`); + console.warn(" Warning: configured Discord user ID does not look like a snowflake."); } } const requireMention = process.env.DISCORD_REQUIRE_MENTION !== "0"; @@ -6572,7 +6572,7 @@ async function setupNim( if (isNonInteractive()) { if (!resolveHermesNousApiKey()) { console.error( - ` ${HERMES_NOUS_API_KEY_CREDENTIAL_ENV} (or NEMOCLAW_PROVIDER_KEY) is required for Hermes Provider Nous API Key in non-interactive mode.`, + " Hermes Provider Nous API Key is required in non-interactive mode.", ); process.exit(1); } diff --git a/src/lib/onboard/preflight.ts b/src/lib/onboard/preflight.ts index 26a3407630f..8f6f8c6a3c1 100644 --- a/src/lib/onboard/preflight.ts +++ b/src/lib/onboard/preflight.ts @@ -1286,7 +1286,13 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes } // Success: busybox nslookup prints "Name:" and "Address:" lines. - if (/\bName:\s*registry\.npmjs\.org\b/.test(output) && /\bAddress:\s*\d/.test(output)) { + const outputLines = output.split(/\r?\n/).map((line) => line.trim()); + const hasRegistryName = outputLines.some((line) => { + const fields = line.split(/\s+/); + return fields[0] === "Name:" && fields[1] === "registry.npmjs.org"; + }); + const hasAddress = outputLines.some((line) => line.startsWith("Address:")); + if (hasRegistryName && hasAddress) { return { ok: true }; } diff --git a/src/lib/onboard/summary.test.ts b/src/lib/onboard/summary.test.ts index 448a7aa5c59..5858e1d92e6 100644 --- a/src/lib/onboard/summary.test.ts +++ b/src/lib/onboard/summary.test.ts @@ -23,8 +23,8 @@ describe("onboard summary helpers", () => { assert.ok(summary.includes("gemini-api"), "summary includes provider"); assert.ok(summary.includes("gemini-2.5-flash"), "summary includes model"); assert.ok( - summary.includes("GEMINI_API_KEY (staged for OpenShell gateway registration)"), - "summary shows API key env var + staging state", + summary.includes("configured for OpenShell gateway registration"), + "summary shows API key staging state without printing env var names", ); assert.ok(summary.includes("enabled"), "summary includes web-search enabled"); assert.ok(summary.includes("telegram, slack"), "summary lists enabled channels"); diff --git a/src/lib/onboard/summary.ts b/src/lib/onboard/summary.ts index ea742d9a92a..eec5bb1fc5f 100644 --- a/src/lib/onboard/summary.ts +++ b/src/lib/onboard/summary.ts @@ -96,7 +96,7 @@ export function formatOnboardConfigSummary({ ? " Nous API key: host-managed; sandbox receives inference placeholder only" : " Nous OAuth: host-managed; sandbox receives inference placeholder only" : credentialEnv - ? ` API key: ${credentialEnv} (staged for OpenShell gateway registration)` + ? " API key: configured for OpenShell gateway registration" : ` API key: (not required for ${provider ?? "this provider"})`; const noteLines = (Array.isArray(notes) ? notes : []) .filter((note) => typeof note === "string" && note.length > 0) diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index 5cc2e7b445e..7a029de8335 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -137,3 +137,22 @@ export function redactUrl(value: unknown): string | null { return redactSensitiveText(value); } } + +function isSensitiveKey(key: string): boolean { + return /(?:api[_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key); +} + +export function redactForLog(value: unknown, seen: WeakSet = new WeakSet()): unknown { + if (typeof value === "string") return redactFull(value); + if (value === null || typeof value !== "object") return value; + if (seen.has(value)) return "[Circular]"; + seen.add(value); + + if (Array.isArray(value)) return value.map((entry) => redactForLog(entry, seen)); + + const redacted: Record = {}; + for (const [key, entry] of Object.entries(value as Record)) { + redacted[key] = isSensitiveKey(key) ? "" : redactForLog(entry, seen); + } + return redacted; +} diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index eb8c226acb3..72e89a8acc3 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -7,6 +7,7 @@ * step-level progress tracking and file-based locking. */ +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; @@ -331,7 +332,7 @@ export function createSession(overrides: Partial = {}): Session { const now = new Date().toISOString(); return { version: SESSION_VERSION, - sessionId: overrides.sessionId ?? `${Date.now()}-${Math.random().toString(36).slice(2, 10)}`, + sessionId: overrides.sessionId ?? `${Date.now()}-${randomUUID()}`, resumable: true, status: "in_progress", mode: overrides.mode ?? "interactive", @@ -440,7 +441,7 @@ export function saveSession(session: Session): Session { ensureSessionDir(); const tmpFile = path.join( SESSION_DIR, - `.onboard-session.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`, + `.onboard-session.${process.pid}.${Date.now()}.${randomUUID()}.tmp`, ); fs.writeFileSync(tmpFile, JSON.stringify(normalized, null, 2), { mode: 0o600 }); fs.renameSync(tmpFile, SESSION_FILE); diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index fad391b781c..8b2d2fc989a 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -5,6 +5,7 @@ import { execFileSync, execSync, spawn, spawnSync } from "node:child_process"; import { chmodSync, closeSync, + constants, existsSync, fchmodSync, mkdirSync, @@ -131,6 +132,23 @@ function commandLineNamesCloudflared(commandLine: string): boolean { .some((token) => basename(token) === "cloudflared"); } +function extractTryCloudflareUrl(log: string): string | null { + for (const rawToken of log.split(/\s+/)) { + const candidate = rawToken.replace(/^[<("']+|[>),."']+$/g, ""); + try { + const url = new URL(candidate); + if (url.protocol !== "https:") continue; + if (url.hostname === "trycloudflare.com" || url.hostname.endsWith(".trycloudflare.com")) { + url.hash = ""; + return url.toString(); + } + } catch { + // Not a URL token. + } + } + return null; +} + export function readCloudflaredState(pidDir: string): CloudflaredState { const pidFile = join(pidDir, "cloudflared.pid"); if (!existsSync(pidFile)) return { kind: "stopped" }; @@ -156,7 +174,15 @@ export function readCloudflaredState(pidDir: string): CloudflaredState { } function writePid(pidDir: string, name: string, pid: number): void { - writeFileSync(join(pidDir, `${name}.pid`), String(pid)); + const pidFile = join(pidDir, `${name}.pid`); + const flags = constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | (constants.O_NOFOLLOW ?? 0); + const fd = openSync(pidFile, flags, 0o600); + try { + fchmodSync(fd, 0o600); + writeFileSync(fd, String(pid)); + } finally { + closeSync(fd); + } } function removePid(pidDir: string, name: string): void { @@ -318,9 +344,9 @@ export function showStatus(opts: ServiceOptions = {}): void { const logFile = join(pidDir, "cloudflared.log"); if (state.kind === "running" && existsSync(logFile)) { const log = readFileSync(logFile, "utf-8"); - const match = /https:\/\/[a-z0-9-]*\.trycloudflare\.com/.exec(log); - if (match) { - info(`Public URL: ${match[0]}`); + const publicUrl = extractTryCloudflareUrl(log); + if (publicUrl) { + info(`Public URL: ${publicUrl}`); } } } @@ -549,7 +575,7 @@ export async function startAll(opts: ServiceOptions = {}): Promise { for (let i = 0; i < 15; i++) { if (existsSync(logFile)) { const log = readFileSync(logFile, "utf-8"); - if (/https:\/\/[a-z0-9-]*\.trycloudflare\.com/.test(log)) { + if (extractTryCloudflareUrl(log)) { break; } } @@ -563,10 +589,7 @@ export async function startAll(opts: ServiceOptions = {}): Promise { const cfLogFile = join(pidDir, "cloudflared.log"); if (isRunning(pidDir, "cloudflared") && existsSync(cfLogFile)) { const log = readFileSync(cfLogFile, "utf-8"); - const match = /https:\/\/[a-z0-9-]*\.trycloudflare\.com/.exec(log); - if (match) { - tunnelUrl = match[0]; - } + tunnelUrl = extractTryCloudflareUrl(log) ?? ""; } const bannerLines = [ From a6208fd7e7bdb1f36534a09a6f7d3ca22c855e09 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 19:09:29 -0700 Subject: [PATCH 2/6] test(security): cover log redaction helpers --- src/lib/cli/nemoclaw-oclif-command.test.ts | 14 ++++++ src/lib/security/redact.test.ts | 58 ++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 src/lib/security/redact.test.ts diff --git a/src/lib/cli/nemoclaw-oclif-command.test.ts b/src/lib/cli/nemoclaw-oclif-command.test.ts index f99979064da..aaf6f819ad4 100644 --- a/src/lib/cli/nemoclaw-oclif-command.test.ts +++ b/src/lib/cli/nemoclaw-oclif-command.test.ts @@ -19,6 +19,10 @@ class TestCommand extends NemoClawCommand { public fail(lines: readonly string[], code?: number): void { this.failWithLines(lines, code); } + + public json(value: unknown): void { + this.logJson(value); + } } function makeCommand(): TestCommand { @@ -54,4 +58,14 @@ describe("NemoClawCommand", () => { expect(process.exitCode).toBe(9); expect(error.mock.calls).toEqual([["line 1"], ["line 2"]]); }); + + it("redacts sensitive JSON output before logging", () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + makeCommand().json({ provider: "build", apiKey: "nvapi-" + "a".repeat(24) }); + + expect(log).toHaveBeenCalledWith( + JSON.stringify({ provider: "build", apiKey: "" }, null, 2), + ); + }); }); diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts new file mode 100644 index 00000000000..5237375210d --- /dev/null +++ b/src/lib/security/redact.test.ts @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { redactForLog } from "./redact.js"; + +describe("redactForLog", () => { + it("redacts sensitive object keys recursively while preserving safe fields", () => { + const result = redactForLog({ + provider: "openai", + apiKey: "sk-" + "a".repeat(24), + nested: { + model: "gpt-4o", + refreshToken: "refresh-token-value", + }, + items: [ + { name: "safe" }, + { credentialEnv: "NVIDIA_API_KEY" }, + ], + }); + + expect(result).toEqual({ + provider: "openai", + apiKey: "", + nested: { + model: "gpt-4o", + refreshToken: "", + }, + items: [ + { name: "safe" }, + { credentialEnv: "" }, + ], + }); + }); + + it("redacts known secret patterns inside otherwise safe strings", () => { + const result = redactForLog({ + message: "upstream returned Authorization: Bearer abcdefghijklmnop", + url: "https://example.test/path?access_token=abcdefghijklmnop", + }); + + expect(result).toEqual({ + message: "upstream returned Authorization: Bearer ", + url: "https://example.test/path?access_token=", + }); + }); + + it("does not recurse forever on circular objects", () => { + const input: Record = { name: "root" }; + input.self = input; + + expect(redactForLog(input)).toEqual({ + name: "root", + self: "[Circular]", + }); + }); +}); From 213da1d34ab5f8e882142950b68c83ac67ba3b2b Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 19:16:13 -0700 Subject: [PATCH 3/6] test(security): cover host validation helpers --- src/lib/tunnel/services.test.ts | 58 ++++++++++++++++++++++++++++++++- test/nemoclaw-start.test.ts | 21 ++++++++++-- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/lib/tunnel/services.test.ts b/src/lib/tunnel/services.test.ts index 5c937cd2e23..0c96b6f3442 100644 --- a/src/lib/tunnel/services.test.ts +++ b/src/lib/tunnel/services.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import childProcess, { type SpawnSyncReturns } from "node:child_process"; -import { mkdtempSync, writeFileSync, existsSync, rmSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { tmpdir } from "node:os"; @@ -12,6 +12,7 @@ import { getServiceStatuses, readCloudflaredState, showStatus, + startAll, stopAll, } from "../../../dist/lib/tunnel/services"; @@ -169,6 +170,61 @@ describe("showStatus", () => { }); }); +describe("startAll", () => { + let tmpDir: string; + let pidDir: string; + let originalPath: string | undefined; + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-start-test-")); + pidDir = join(tmpDir, "pids"); + originalPath = process.env.PATH; + }); + + afterEach(() => { + process.env.PATH = originalPath; + const pid = readCloudflaredState(pidDir); + if (pid.kind === "running") { + try { + process.kill(pid.pid, "SIGTERM"); + } catch { + // Process may have already exited. + } + } + rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("writes a private PID file and surfaces only real trycloudflare hosts", async () => { + const binDir = join(tmpDir, "bin"); + mkdirSync(binDir, { recursive: true }); + const fakeCloudflared = join(binDir, "cloudflared"); + writeFileSync( + fakeCloudflared, + [ + "#!/usr/bin/env sh", + "echo 'https://attacker.trycloudflare.com.evil.test'", + "echo 'https://good.trycloudflare.com/route#secret-fragment'", + "sleep 20", + ].join("\n"), + ); + chmodSync(fakeCloudflared, 0o700); + process.env.PATH = `${binDir}:${originalPath ?? ""}`; + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await startAll({ pidDir, dashboardPort: 12345 }); + + const pidFile = join(pidDir, "cloudflared.pid"); + expect(readFileSync(pidFile, "utf-8")).toMatch(/^\d+$/); + expect(statSync(pidFile).mode & 0o777).toBe(0o600); + const output = logSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(output).toContain("https://good.trycloudflare.com/route"); + expect(output).not.toContain("evil.test"); + expect(output).not.toContain("secret-fragment"); + }); +}); + // #2604: readCloudflaredState is the shared source of truth used by both // showStatus and the doctor's cloudflared check. Tests below exercise each // branch of the discriminated union. diff --git a/test/nemoclaw-start.test.ts b/test/nemoclaw-start.test.ts index 793b0fd333d..3191d82a782 100644 --- a/test/nemoclaw-start.test.ts +++ b/test/nemoclaw-start.test.ts @@ -1201,15 +1201,32 @@ const cases = [ new Error('token_revoked'), Object.assign(new Error('stack path'), { stack: 'at @slack/web-api' }), new Error('CONNECT failed for slack.com'), + new Error('CONNECT failed for https://hooks.slack.com/services/T/B/C'), ]; for (const err of cases) process.emit('unhandledRejection', err, {}); setImmediate(function () { console.log('cases=' + cases.length); }); `); expect(result.status).toBe(0); - expect(result.stdout).toContain("cases=4"); - expect((result.stderr.match(/provider failed to start/g) || []).length).toBe(4); + expect(result.stdout).toContain("cases=5"); + expect((result.stderr.match(/provider failed to start/g) || []).length).toBe(5); expect(result.stderr).toContain("caught by safety net, gateway continues"); }); + + it("does not classify arbitrary hosts containing slack.com as Slack errors", () => { + const result = runSlackGuardHarness(` +let downstreamCalled = false; +process.on('unhandledRejection', function () { + downstreamCalled = true; +}); +process.emit('unhandledRejection', new Error('CONNECT failed for https://slack.com.evil.example'), {}); +setImmediate(function () { + console.log('downstream=' + downstreamCalled); +}); +`); + expect(result.status).toBe(0); + expect(result.stdout).toContain("downstream=true"); + expect(result.stderr).not.toContain("provider failed to start"); + }); }); describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { From 01eba0333764fc05c7fa644bc6503dcf9350ca78 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 18:44:56 -0700 Subject: [PATCH 4/6] fix(security): harden low-risk code scanning findings --- .../search_assets/modules/SearchEngine.js | 24 ++++++++--- .../scripts/slack-channel-guard.js | 17 +++++++- nemoclaw/src/onboard/config.ts | 14 ++++--- src/commands/internal/installer/plan.ts | 2 +- src/lib/actions/dev/npm-link-or-shim.ts | 5 ++- src/lib/actions/sandbox/rebuild.ts | 7 ++-- src/lib/adapters/openshell/client.test.ts | 4 +- src/lib/adapters/openshell/client.ts | 9 ++-- src/lib/cli/nemoclaw-oclif-command.ts | 4 +- src/lib/diagnostics/debug.ts | 11 +++-- src/lib/onboard.ts | 10 ++--- src/lib/onboard/preflight.ts | 26 +++++++++--- src/lib/onboard/summary.test.ts | 4 +- src/lib/onboard/summary.ts | 2 +- src/lib/security/redact.ts | 19 +++++++++ src/lib/state/onboard-session.ts | 5 ++- src/lib/tunnel/services.ts | 41 +++++++++++++++---- 17 files changed, 148 insertions(+), 56 deletions(-) diff --git a/docs/_ext/search_assets/modules/SearchEngine.js b/docs/_ext/search_assets/modules/SearchEngine.js index a1a5bea3415..b32c48baed5 100644 --- a/docs/_ext/search_assets/modules/SearchEngine.js +++ b/docs/_ext/search_assets/modules/SearchEngine.js @@ -495,19 +495,33 @@ class SearchEngine { return this.getDocumentAudience(doc); } + /** + * Extract safe Lunr query terms from user input. + */ + getSafeSearchTerms(query) { + const matches = String(query || '').toLowerCase().match(/[a-z0-9][a-z0-9._-]{0,63}/g); + return matches ? matches.slice(0, 10) : []; + } + /** * Perform search with multiple strategies */ performMultiStrategySearch(query) { + const terms = this.getSafeSearchTerms(query); + if (terms.length === 0) return []; + + const phrase = terms.join(' '); + const wildcardTerms = terms.map(term => `${term}*`).join(' '); + const fuzzyTerms = terms.map(term => `${term}~2`).join(' '); const strategies = [ // Exact phrase search with wildcards - `"${query}" ${query}*`, + `"${phrase}" ${wildcardTerms}`, // Fuzzy search with wildcards - `${query}* ${query}~2`, + `${wildcardTerms} ${fuzzyTerms}`, // Individual terms with boost - query.split(/\s+/).map(term => `${term}*`).join(' '), - // Fallback: just the query - query + wildcardTerms, + // Fallback: sanitized terms only + phrase ]; let allResults = []; diff --git a/nemoclaw-blueprint/scripts/slack-channel-guard.js b/nemoclaw-blueprint/scripts/slack-channel-guard.js index b524638930e..1ab1cff9d9f 100644 --- a/nemoclaw-blueprint/scripts/slack-channel-guard.js +++ b/nemoclaw-blueprint/scripts/slack-channel-guard.js @@ -37,6 +37,21 @@ 'An API error occurred: invalid_auth', ]; + function mentionsSlackHost(value) { + var tokens = String(value || '').split(/\s+/); + for (var i = 0; i < tokens.length; i++) { + var candidate = tokens[i].replace(/^[<("']+|[>),."']+$/g, ''); + try { + var parsed = new URL(candidate); + var host = parsed.hostname.toLowerCase(); + if (host === 'slack.com' || host.endsWith('.slack.com')) return true; + } catch (_e) { + if (/^(?:[a-z0-9-]+\.)*slack\.com(?::\d+)?$/i.test(candidate)) return true; + } + } + return false; + } + function isSlackRejection(reason) { if (!reason) return false; @@ -63,7 +78,7 @@ // servers, the error comes from the HTTP client (CONNECT tunnel // failure), not from @slack/ code. The stack won't contain @slack/ // but the error message or URL may reference the Slack hostname. - if (msg.indexOf('slack.com') !== -1) { + if (mentionsSlackHost(msg)) { return true; } diff --git a/nemoclaw/src/onboard/config.ts b/nemoclaw/src/onboard/config.ts index cb36e15295a..ac59a05eebe 100644 --- a/nemoclaw/src/onboard/config.ts +++ b/nemoclaw/src/onboard/config.ts @@ -1,7 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + writeFileSync, + unlinkSync, +} from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; @@ -138,10 +145,7 @@ function ensureConfigDir(): void { try { mkdirSync(configDir, { recursive: true }); } catch { - configDir = join(tmpdir(), ".nemoclaw"); - if (!existsSync(configDir)) { - mkdirSync(configDir, { recursive: true }); - } + configDir = mkdtempSync(join(tmpdir(), "nemoclaw-config-")); } } configDirCreated = true; diff --git a/src/commands/internal/installer/plan.ts b/src/commands/internal/installer/plan.ts index ece5c17ecfc..1a118a8f9c3 100644 --- a/src/commands/internal/installer/plan.ts +++ b/src/commands/internal/installer/plan.ts @@ -44,6 +44,6 @@ export default class InternalInstallerPlanCommand extends NemoClawCommand { }); if (flags.json) this.logJson(plan); - else console.log(`Installer plan: ref '${plan.installRef}', version '${plan.installerVersion}'`); + else console.log("Installer plan built. Re-run with --json for redacted details."); } } diff --git a/src/lib/actions/dev/npm-link-or-shim.ts b/src/lib/actions/dev/npm-link-or-shim.ts index c39300f7268..90ce993e917 100644 --- a/src/lib/actions/dev/npm-link-or-shim.ts +++ b/src/lib/actions/dev/npm-link-or-shim.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { randomUUID } from "node:crypto"; import { spawnSync, type SpawnSyncReturns } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; @@ -116,7 +117,7 @@ export function runNpmLinkOrShim( ((dir: string) => path.join( dir, - `nemoclaw.tmp.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}`, + `nemoclaw.tmp.${process.pid}.${Date.now()}.${randomUUID()}`, )); const run = deps.run ?? defaultRun; const commandPath = deps.commandPath ?? defaultCommandPath; @@ -124,7 +125,7 @@ export function runNpmLinkOrShim( if (env.NEMOCLAW_INSTALLING) return { status: 0 }; if (!isExecutable(binPath)) { - logError(`[nemoclaw] cannot expose CLI: ${binPath} is missing or not executable`); + logError("[nemoclaw] cannot expose CLI: launcher is missing or not executable"); return { status: 0 }; } diff --git a/src/lib/actions/sandbox/rebuild.ts b/src/lib/actions/sandbox/rebuild.ts index e70dff0cd2a..27ce3dc11fe 100644 --- a/src/lib/actions/sandbox/rebuild.ts +++ b/src/lib/actions/sandbox/rebuild.ts @@ -43,6 +43,7 @@ import * as nim from "../../inference/nim"; import * as policies from "../../policy"; import { parseLiveSandboxNames } from "../../runtime-recovery"; import * as sandboxVersion from "../../sandbox/version"; +import { redact } from "../../security/redact"; import type { Session } from "../../state/onboard-session"; import * as onboardSession from "../../state/onboard-session"; import * as registry from "../../state/registry"; @@ -60,7 +61,7 @@ const agentRuntime = require("../../../../bin/lib/agent-runtime"); * Emit timestamped rebuild diagnostics when verbose rebuild logging is enabled. */ function _rebuildLog(msg: string) { - console.error(` ${D}[rebuild ${new Date().toISOString()}] ${msg}${R}`); + console.error(` ${D}[rebuild ${new Date().toISOString()}] ${redact(msg)}${R}`); } /** @@ -122,7 +123,7 @@ function preflightHermesProviderCredentials( nonEmptyString(process.env[hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV]) || nonEmptyString(process.env.NEMOCLAW_PROVIDER_KEY); log( - `Hermes Provider rebuild preflight: OpenShell provider missing; ${hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV} env=${envKey ? "present" : "missing"}`, + `Hermes Provider rebuild preflight: OpenShell provider missing; API key env=${envKey ? "present" : "missing"}`, ); if (envKey) { try { @@ -145,7 +146,7 @@ function preflightHermesProviderCredentials( console.error(" Hermes Provider credentials must be stored in OpenShell, not host-side files."); if (authMethod === "api_key") { console.error( - ` Export ${hermesProviderAuth.HERMES_NOUS_API_KEY_CREDENTIAL_ENV} and rerun rebuild, or re-run ${CLI_NAME} onboard to register it.`, + ` Export the Hermes Provider API key and rerun rebuild, or re-run ${CLI_NAME} onboard to register it.`, ); } else { console.error(` Re-run ${CLI_NAME} onboard interactively to authorize Hermes Provider and register it with OpenShell.`); diff --git a/src/lib/adapters/openshell/client.test.ts b/src/lib/adapters/openshell/client.test.ts index bd3101e616c..d4bcaad1fa0 100644 --- a/src/lib/adapters/openshell/client.test.ts +++ b/src/lib/adapters/openshell/client.test.ts @@ -201,7 +201,7 @@ describe("openshell helpers", () => { exit: exitWithCode, }), ).toThrow("exit:1"); - expect(errors).toEqual([" Failed to start openshell status: spawn EACCES"]); + expect(errors).toEqual([" Failed to start OpenShell command: spawn EACCES"]); }); it("treats capture spawn failures as fatal errors", () => { @@ -218,7 +218,7 @@ describe("openshell helpers", () => { exit: exitWithCode, }), ).toThrow("exit:1"); - expect(errors).toEqual([" Failed to start openshell status: spawn ENOENT"]); + expect(errors).toEqual([" Failed to start OpenShell command: spawn ENOENT"]); }); it("reads the installed openshell version through the capture helper", () => { diff --git a/src/lib/adapters/openshell/client.ts b/src/lib/adapters/openshell/client.ts index d4b57975453..5e50370b7f6 100644 --- a/src/lib/adapters/openshell/client.ts +++ b/src/lib/adapters/openshell/client.ts @@ -75,13 +75,12 @@ export function versionGte(left = "0.0.0", right = "0.0.0"): boolean { } function handleSpawnError( - binary: string, - args: string[], + _binary: string, + _args: string[], error: Error, opts: OpenshellSpawnOptions, ): never { - const command = [binary, ...args].join(" "); - (opts.errorLine ?? console.error)(` Failed to start ${command}: ${error.message}`); + (opts.errorLine ?? console.error)(` Failed to start OpenShell command: ${error.message}`); return (opts.exit ?? ((code) => process.exit(code)))(1); } @@ -135,7 +134,7 @@ export function runOpenshellCommand( } if (result.status !== 0 && !opts.ignoreError) { (opts.errorLine ?? console.error)( - ` Command failed (exit ${result.status}): openshell ${args.join(" ")}`, + ` OpenShell command failed (exit ${result.status})`, ); return (opts.exit ?? ((code) => process.exit(code)))(result.status || 1); } diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index a780875d636..031a979e77b 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,8 @@ import { Command, Flags } from "@oclif/core"; +import { redactForLog } from "../security/redact"; + export type CommandExitResult = { exitCode?: number | null; message?: string | null; @@ -21,7 +23,7 @@ export abstract class NemoClawCommand extends Command { }; protected logJson(json: unknown): void { - console.log(JSON.stringify(json, null, 2)); + console.log(JSON.stringify(redactForLog(json), null, 2)); } protected setExitCode(code: number): void { diff --git a/src/lib/diagnostics/debug.ts b/src/lib/diagnostics/debug.ts index 14a8a6c12ce..5ef9f0c2bff 100644 --- a/src/lib/diagnostics/debug.ts +++ b/src/lib/diagnostics/debug.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { platform, tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; @@ -314,8 +314,9 @@ function collectSandboxInternals( section("Sandbox Internals"); - // Generate temporary SSH config - const sshConfigPath = join(tmpdir(), `nemoclaw-ssh-${String(Date.now())}`); + // Generate temporary SSH config in a private directory. + const sshConfigDir = mkdtempSync(join(tmpdir(), "nemoclaw-ssh-")); + const sshConfigPath = join(sshConfigDir, "config"); try { const sshResult = spawnSync("openshell", ["sandbox", "ssh-config", sandboxName], { timeout: TIMEOUT_MS, @@ -358,9 +359,7 @@ function collectSandboxInternals( ]); } } finally { - if (existsSync(sshConfigPath)) { - unlinkSync(sshConfigPath); - } + rmSync(sshConfigDir, { force: true, recursive: true }); } } diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 5edbca63c0a..9890f689f08 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -1554,7 +1554,7 @@ async function replaceNamedCredential( saveCredential(envName, key); process.env[envName] = key; console.log(""); - console.log(` ${envName} staged. Onboarding will register it with the OpenShell gateway.`); + console.log(" Credential staged. Onboarding will register it with the OpenShell gateway."); console.log(""); return key; } @@ -1965,7 +1965,7 @@ function verifyCompatibleEndpointSandboxSmoke(options: { !providerDetails.includes(options.credentialEnv) ) { console.warn( - ` ⚠ Gateway provider '${options.provider}' did not report ${options.credentialEnv}.`, + ` ⚠ Gateway provider '${options.provider}' did not report the selected credential binding.`, ); } @@ -5427,12 +5427,12 @@ async function createSandbox( .filter(Boolean); for (const serverId of serverIds) { if (!DISCORD_SNOWFLAKE_RE.test(serverId)) { - console.warn(` Warning: Discord server ID '${serverId}' does not look like a snowflake.`); + console.warn(" Warning: configured Discord server ID does not look like a snowflake."); } } for (const userId of userIds) { if (!DISCORD_SNOWFLAKE_RE.test(userId)) { - console.warn(` Warning: Discord user ID '${userId}' does not look like a snowflake.`); + console.warn(" Warning: configured Discord user ID does not look like a snowflake."); } } const requireMention = process.env.DISCORD_REQUIRE_MENTION !== "0"; @@ -6572,7 +6572,7 @@ async function setupNim( if (isNonInteractive()) { if (!resolveHermesNousApiKey()) { console.error( - ` ${HERMES_NOUS_API_KEY_CREDENTIAL_ENV} (or NEMOCLAW_PROVIDER_KEY) is required for Hermes Provider Nous API Key in non-interactive mode.`, + " Hermes Provider Nous API Key is required in non-interactive mode.", ); process.exit(1); } diff --git a/src/lib/onboard/preflight.ts b/src/lib/onboard/preflight.ts index 0823183baa3..ef2c90e1635 100644 --- a/src/lib/onboard/preflight.ts +++ b/src/lib/onboard/preflight.ts @@ -1321,12 +1321,26 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes // begins with `Server: ... / Address: :53` — proves only that we // reached *something* claiming to be a resolver, not that we got an // answer. Real success requires either an actual `Name:`+`Address:` - // resolution pair OR an NXDOMAIN response body. - const probeAnswered = - /^Server:\s*\S/im.test(output) && - ((/\bName:\s*\S/i.test(output) && /\bAddress:\s*\d/.test(output)) || - /server can't find\b.*NXDOMAIN/i.test(output)); - if (probeAnswered) { + // resolution pair OR an NXDOMAIN response body. Keep this line-based so + // CodeQL does not treat partial host regexes as URL validation. + const outputLines = output.split(/\r?\n/).map((line) => line.trim()); + const hasResolverHeader = outputLines.some((line) => { + const fields = line.split(/\s+/); + return fields[0] === "Server:" && Boolean(fields[1]); + }); + const hasResolvedName = outputLines.some((line) => { + const fields = line.split(/\s+/); + return fields[0] === "Name:" && Boolean(fields[1]); + }); + const hasAddress = outputLines.some((line) => { + const fields = line.split(/\s+/); + return fields[0] === "Address:" && /^\d/.test(fields[1] ?? ""); + }); + const hasNxdomainAnswer = outputLines.some((line) => { + const lower = line.toLowerCase(); + return lower.includes("server can't find") && lower.includes("nxdomain"); + }); + if (hasResolverHeader && ((hasResolvedName && hasAddress) || hasNxdomainAnswer)) { return { ok: true }; } diff --git a/src/lib/onboard/summary.test.ts b/src/lib/onboard/summary.test.ts index 448a7aa5c59..5858e1d92e6 100644 --- a/src/lib/onboard/summary.test.ts +++ b/src/lib/onboard/summary.test.ts @@ -23,8 +23,8 @@ describe("onboard summary helpers", () => { assert.ok(summary.includes("gemini-api"), "summary includes provider"); assert.ok(summary.includes("gemini-2.5-flash"), "summary includes model"); assert.ok( - summary.includes("GEMINI_API_KEY (staged for OpenShell gateway registration)"), - "summary shows API key env var + staging state", + summary.includes("configured for OpenShell gateway registration"), + "summary shows API key staging state without printing env var names", ); assert.ok(summary.includes("enabled"), "summary includes web-search enabled"); assert.ok(summary.includes("telegram, slack"), "summary lists enabled channels"); diff --git a/src/lib/onboard/summary.ts b/src/lib/onboard/summary.ts index ea742d9a92a..eec5bb1fc5f 100644 --- a/src/lib/onboard/summary.ts +++ b/src/lib/onboard/summary.ts @@ -96,7 +96,7 @@ export function formatOnboardConfigSummary({ ? " Nous API key: host-managed; sandbox receives inference placeholder only" : " Nous OAuth: host-managed; sandbox receives inference placeholder only" : credentialEnv - ? ` API key: ${credentialEnv} (staged for OpenShell gateway registration)` + ? " API key: configured for OpenShell gateway registration" : ` API key: (not required for ${provider ?? "this provider"})`; const noteLines = (Array.isArray(notes) ? notes : []) .filter((note) => typeof note === "string" && note.length > 0) diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index 5cc2e7b445e..7a029de8335 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -137,3 +137,22 @@ export function redactUrl(value: unknown): string | null { return redactSensitiveText(value); } } + +function isSensitiveKey(key: string): boolean { + return /(?:api[_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key); +} + +export function redactForLog(value: unknown, seen: WeakSet = new WeakSet()): unknown { + if (typeof value === "string") return redactFull(value); + if (value === null || typeof value !== "object") return value; + if (seen.has(value)) return "[Circular]"; + seen.add(value); + + if (Array.isArray(value)) return value.map((entry) => redactForLog(entry, seen)); + + const redacted: Record = {}; + for (const [key, entry] of Object.entries(value as Record)) { + redacted[key] = isSensitiveKey(key) ? "" : redactForLog(entry, seen); + } + return redacted; +} diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index eb8c226acb3..72e89a8acc3 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -7,6 +7,7 @@ * step-level progress tracking and file-based locking. */ +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; @@ -331,7 +332,7 @@ export function createSession(overrides: Partial = {}): Session { const now = new Date().toISOString(); return { version: SESSION_VERSION, - sessionId: overrides.sessionId ?? `${Date.now()}-${Math.random().toString(36).slice(2, 10)}`, + sessionId: overrides.sessionId ?? `${Date.now()}-${randomUUID()}`, resumable: true, status: "in_progress", mode: overrides.mode ?? "interactive", @@ -440,7 +441,7 @@ export function saveSession(session: Session): Session { ensureSessionDir(); const tmpFile = path.join( SESSION_DIR, - `.onboard-session.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`, + `.onboard-session.${process.pid}.${Date.now()}.${randomUUID()}.tmp`, ); fs.writeFileSync(tmpFile, JSON.stringify(normalized, null, 2), { mode: 0o600 }); fs.renameSync(tmpFile, SESSION_FILE); diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index fad391b781c..8b2d2fc989a 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -5,6 +5,7 @@ import { execFileSync, execSync, spawn, spawnSync } from "node:child_process"; import { chmodSync, closeSync, + constants, existsSync, fchmodSync, mkdirSync, @@ -131,6 +132,23 @@ function commandLineNamesCloudflared(commandLine: string): boolean { .some((token) => basename(token) === "cloudflared"); } +function extractTryCloudflareUrl(log: string): string | null { + for (const rawToken of log.split(/\s+/)) { + const candidate = rawToken.replace(/^[<("']+|[>),."']+$/g, ""); + try { + const url = new URL(candidate); + if (url.protocol !== "https:") continue; + if (url.hostname === "trycloudflare.com" || url.hostname.endsWith(".trycloudflare.com")) { + url.hash = ""; + return url.toString(); + } + } catch { + // Not a URL token. + } + } + return null; +} + export function readCloudflaredState(pidDir: string): CloudflaredState { const pidFile = join(pidDir, "cloudflared.pid"); if (!existsSync(pidFile)) return { kind: "stopped" }; @@ -156,7 +174,15 @@ export function readCloudflaredState(pidDir: string): CloudflaredState { } function writePid(pidDir: string, name: string, pid: number): void { - writeFileSync(join(pidDir, `${name}.pid`), String(pid)); + const pidFile = join(pidDir, `${name}.pid`); + const flags = constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | (constants.O_NOFOLLOW ?? 0); + const fd = openSync(pidFile, flags, 0o600); + try { + fchmodSync(fd, 0o600); + writeFileSync(fd, String(pid)); + } finally { + closeSync(fd); + } } function removePid(pidDir: string, name: string): void { @@ -318,9 +344,9 @@ export function showStatus(opts: ServiceOptions = {}): void { const logFile = join(pidDir, "cloudflared.log"); if (state.kind === "running" && existsSync(logFile)) { const log = readFileSync(logFile, "utf-8"); - const match = /https:\/\/[a-z0-9-]*\.trycloudflare\.com/.exec(log); - if (match) { - info(`Public URL: ${match[0]}`); + const publicUrl = extractTryCloudflareUrl(log); + if (publicUrl) { + info(`Public URL: ${publicUrl}`); } } } @@ -549,7 +575,7 @@ export async function startAll(opts: ServiceOptions = {}): Promise { for (let i = 0; i < 15; i++) { if (existsSync(logFile)) { const log = readFileSync(logFile, "utf-8"); - if (/https:\/\/[a-z0-9-]*\.trycloudflare\.com/.test(log)) { + if (extractTryCloudflareUrl(log)) { break; } } @@ -563,10 +589,7 @@ export async function startAll(opts: ServiceOptions = {}): Promise { const cfLogFile = join(pidDir, "cloudflared.log"); if (isRunning(pidDir, "cloudflared") && existsSync(cfLogFile)) { const log = readFileSync(cfLogFile, "utf-8"); - const match = /https:\/\/[a-z0-9-]*\.trycloudflare\.com/.exec(log); - if (match) { - tunnelUrl = match[0]; - } + tunnelUrl = extractTryCloudflareUrl(log) ?? ""; } const bannerLines = [ From 9d3b20e8f4782945a8da44faaeaa4f3fbcbfbaaf Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 19:09:29 -0700 Subject: [PATCH 5/6] test(security): cover log redaction helpers --- src/lib/cli/nemoclaw-oclif-command.test.ts | 14 ++++++ src/lib/security/redact.test.ts | 58 ++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 src/lib/security/redact.test.ts diff --git a/src/lib/cli/nemoclaw-oclif-command.test.ts b/src/lib/cli/nemoclaw-oclif-command.test.ts index f99979064da..aaf6f819ad4 100644 --- a/src/lib/cli/nemoclaw-oclif-command.test.ts +++ b/src/lib/cli/nemoclaw-oclif-command.test.ts @@ -19,6 +19,10 @@ class TestCommand extends NemoClawCommand { public fail(lines: readonly string[], code?: number): void { this.failWithLines(lines, code); } + + public json(value: unknown): void { + this.logJson(value); + } } function makeCommand(): TestCommand { @@ -54,4 +58,14 @@ describe("NemoClawCommand", () => { expect(process.exitCode).toBe(9); expect(error.mock.calls).toEqual([["line 1"], ["line 2"]]); }); + + it("redacts sensitive JSON output before logging", () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + makeCommand().json({ provider: "build", apiKey: "nvapi-" + "a".repeat(24) }); + + expect(log).toHaveBeenCalledWith( + JSON.stringify({ provider: "build", apiKey: "" }, null, 2), + ); + }); }); diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts new file mode 100644 index 00000000000..5237375210d --- /dev/null +++ b/src/lib/security/redact.test.ts @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { redactForLog } from "./redact.js"; + +describe("redactForLog", () => { + it("redacts sensitive object keys recursively while preserving safe fields", () => { + const result = redactForLog({ + provider: "openai", + apiKey: "sk-" + "a".repeat(24), + nested: { + model: "gpt-4o", + refreshToken: "refresh-token-value", + }, + items: [ + { name: "safe" }, + { credentialEnv: "NVIDIA_API_KEY" }, + ], + }); + + expect(result).toEqual({ + provider: "openai", + apiKey: "", + nested: { + model: "gpt-4o", + refreshToken: "", + }, + items: [ + { name: "safe" }, + { credentialEnv: "" }, + ], + }); + }); + + it("redacts known secret patterns inside otherwise safe strings", () => { + const result = redactForLog({ + message: "upstream returned Authorization: Bearer abcdefghijklmnop", + url: "https://example.test/path?access_token=abcdefghijklmnop", + }); + + expect(result).toEqual({ + message: "upstream returned Authorization: Bearer ", + url: "https://example.test/path?access_token=", + }); + }); + + it("does not recurse forever on circular objects", () => { + const input: Record = { name: "root" }; + input.self = input; + + expect(redactForLog(input)).toEqual({ + name: "root", + self: "[Circular]", + }); + }); +}); From fa0cceb85fe37257cdf4b4ee4004d462e4b41321 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 16 May 2026 19:16:13 -0700 Subject: [PATCH 6/6] test(security): cover host validation helpers --- src/lib/tunnel/services.test.ts | 58 ++++++++++++++++++++++++++++++++- test/nemoclaw-start.test.ts | 21 ++++++++++-- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/lib/tunnel/services.test.ts b/src/lib/tunnel/services.test.ts index 5c937cd2e23..0c96b6f3442 100644 --- a/src/lib/tunnel/services.test.ts +++ b/src/lib/tunnel/services.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import childProcess, { type SpawnSyncReturns } from "node:child_process"; -import { mkdtempSync, writeFileSync, existsSync, rmSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { tmpdir } from "node:os"; @@ -12,6 +12,7 @@ import { getServiceStatuses, readCloudflaredState, showStatus, + startAll, stopAll, } from "../../../dist/lib/tunnel/services"; @@ -169,6 +170,61 @@ describe("showStatus", () => { }); }); +describe("startAll", () => { + let tmpDir: string; + let pidDir: string; + let originalPath: string | undefined; + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-start-test-")); + pidDir = join(tmpDir, "pids"); + originalPath = process.env.PATH; + }); + + afterEach(() => { + process.env.PATH = originalPath; + const pid = readCloudflaredState(pidDir); + if (pid.kind === "running") { + try { + process.kill(pid.pid, "SIGTERM"); + } catch { + // Process may have already exited. + } + } + rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("writes a private PID file and surfaces only real trycloudflare hosts", async () => { + const binDir = join(tmpDir, "bin"); + mkdirSync(binDir, { recursive: true }); + const fakeCloudflared = join(binDir, "cloudflared"); + writeFileSync( + fakeCloudflared, + [ + "#!/usr/bin/env sh", + "echo 'https://attacker.trycloudflare.com.evil.test'", + "echo 'https://good.trycloudflare.com/route#secret-fragment'", + "sleep 20", + ].join("\n"), + ); + chmodSync(fakeCloudflared, 0o700); + process.env.PATH = `${binDir}:${originalPath ?? ""}`; + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await startAll({ pidDir, dashboardPort: 12345 }); + + const pidFile = join(pidDir, "cloudflared.pid"); + expect(readFileSync(pidFile, "utf-8")).toMatch(/^\d+$/); + expect(statSync(pidFile).mode & 0o777).toBe(0o600); + const output = logSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(output).toContain("https://good.trycloudflare.com/route"); + expect(output).not.toContain("evil.test"); + expect(output).not.toContain("secret-fragment"); + }); +}); + // #2604: readCloudflaredState is the shared source of truth used by both // showStatus and the doctor's cloudflared check. Tests below exercise each // branch of the discriminated union. diff --git a/test/nemoclaw-start.test.ts b/test/nemoclaw-start.test.ts index 793b0fd333d..3191d82a782 100644 --- a/test/nemoclaw-start.test.ts +++ b/test/nemoclaw-start.test.ts @@ -1201,15 +1201,32 @@ const cases = [ new Error('token_revoked'), Object.assign(new Error('stack path'), { stack: 'at @slack/web-api' }), new Error('CONNECT failed for slack.com'), + new Error('CONNECT failed for https://hooks.slack.com/services/T/B/C'), ]; for (const err of cases) process.emit('unhandledRejection', err, {}); setImmediate(function () { console.log('cases=' + cases.length); }); `); expect(result.status).toBe(0); - expect(result.stdout).toContain("cases=4"); - expect((result.stderr.match(/provider failed to start/g) || []).length).toBe(4); + expect(result.stdout).toContain("cases=5"); + expect((result.stderr.match(/provider failed to start/g) || []).length).toBe(5); expect(result.stderr).toContain("caught by safety net, gateway continues"); }); + + it("does not classify arbitrary hosts containing slack.com as Slack errors", () => { + const result = runSlackGuardHarness(` +let downstreamCalled = false; +process.on('unhandledRejection', function () { + downstreamCalled = true; +}); +process.emit('unhandledRejection', new Error('CONNECT failed for https://slack.com.evil.example'), {}); +setImmediate(function () { + console.log('downstream=' + downstreamCalled); +}); +`); + expect(result.status).toBe(0); + expect(result.stdout).toContain("downstream=true"); + expect(result.stderr).not.toContain("provider failed to start"); + }); }); describe("nemoclaw-start auto-pair client whitelisting (#117)", () => {