From 6aad9731c7e7066cf359d133af9eb5be531237c1 Mon Sep 17 00:00:00 2001 From: sauravdev Date: Sat, 4 Jul 2026 16:10:24 +0530 Subject: [PATCH 01/11] feat(cli): add structured logging with --debug and NEMOCLAW_LOG_LEVEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces src/lib/cli/logger.ts — a singleton Logger class with four levels (error < warn < info < debug) written exclusively to stderr so stdout remains clean for --json consumers. - `NEMOCLAW_LOG_LEVEL=debug` env var sets level at process start - `--debug` flag (inherited by all commands via NemoClawCommand.baseFlags) activates debug level and ISO-8601 timestamp prefixes - `-q / --quiet` flag suppresses info; shows only warn+error - `NEMOCLAW_DEBUG=1` and `DEBUG=*nemoclaw*` also activate debug level NemoClawCommand.init() reads both flags and configures the singleton before any command's run() method executes, so subcommands see the correct level without needing to parse flags themselves. Adds unit tests covering all level transitions and env-var pickup. Gradual migration of existing console.log/console.error call sites to log.info/log.error can be done incrementally on separate branches. Co-Authored-By: Claude Sonnet 4.6 --- src/lib/cli/logger.test.ts | 94 ++++++++++++++++++++ src/lib/cli/logger.ts | 118 ++++++++++++++++++++++++++ src/lib/cli/nemoclaw-oclif-command.ts | 30 +++++++ 3 files changed, 242 insertions(+) create mode 100644 src/lib/cli/logger.test.ts create mode 100644 src/lib/cli/logger.ts diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts new file mode 100644 index 00000000000..01d2ad5697f --- /dev/null +++ b/src/lib/cli/logger.test.ts @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { LogLevel } from "./logger"; + +// Re-import logger fresh for each test to reset singleton state +async function freshLogger() { + vi.resetModules(); + const mod = await import("./logger"); + return mod; +} + +describe("Logger", () => { + let stderrSpy: ReturnType; + + beforeEach(() => { + stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + stderrSpy.mockClear(); + vi.unstubAllEnvs(); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + }); + + it("defaults to info level", async () => { + const { log } = await freshLogger(); + expect(log.level).toBe("info"); + }); + + it("reads NEMOCLAW_LOG_LEVEL from env", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "debug"); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it("suppresses debug messages at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debug("should not appear"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("shows debug messages after setDebug(true)", async () => { + const { log } = await freshLogger(); + log.setDebug(true); + log.debug("visible debug"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible debug")); + }); + + it("quiet mode suppresses info", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.info("suppressed info"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("quiet mode still shows warn", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.warn("visible warning"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible warning")); + }); + + it("error always shown", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.error("critical error"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("critical error")); + }); + + it("error suppressed below error level only for warn+info+debug", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.warn("should be suppressed"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("debugObject emits JSON at debug level", async () => { + const { log } = await freshLogger(); + log.setLevel("debug"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('"key"')); + }); + + it("debugObject suppressed at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/cli/logger.ts b/src/lib/cli/logger.ts new file mode 100644 index 00000000000..4bcb1e2397b --- /dev/null +++ b/src/lib/cli/logger.ts @@ -0,0 +1,118 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Centralized logger for NemoClaw CLI. + * + * Levels (lowest → highest verbosity): + * error < warn < info < debug + * + * Default level: info (errors, warnings, and info messages shown). + * Quiet mode: warn (only warnings and errors shown). + * Debug mode: debug (all messages shown with timestamps). + * + * Configure via: + * NEMOCLAW_LOG_LEVEL=debug nemoclaw ... + * nemoclaw ... --debug (shorthand for debug level) + * nemoclaw ... -q / --quiet (suppresses info, shows warn+error) + */ + +export type LogLevel = "error" | "warn" | "info" | "debug"; + +const LEVEL_RANK: Record = { + error: 0, + warn: 1, + info: 2, + debug: 3, +}; + +function resolveLevel(): LogLevel { + const env = process.env.NEMOCLAW_LOG_LEVEL?.toLowerCase(); + if (env === "error" || env === "warn" || env === "info" || env === "debug") return env; + if (process.env.NEMOCLAW_DEBUG === "1" || process.env.DEBUG?.includes("nemoclaw")) return "debug"; + return "info"; +} + +class Logger { + private _level: LogLevel; + private _quiet: boolean; + private _timestamps: boolean; + + constructor() { + this._level = resolveLevel(); + this._quiet = false; + this._timestamps = this._level === "debug"; + } + + get level(): LogLevel { + return this._level; + } + + setLevel(level: LogLevel): void { + this._level = level; + this._timestamps = level === "debug"; + } + + setQuiet(quiet: boolean): void { + this._quiet = quiet; + if (quiet && LEVEL_RANK[this._level] > LEVEL_RANK["warn"]) { + this._level = "warn"; + } + } + + setDebug(debug: boolean): void { + if (debug) this.setLevel("debug"); + } + + isDebug(): boolean { + return this._level === "debug"; + } + + isQuiet(): boolean { + return this._quiet; + } + + private shouldLog(level: LogLevel): boolean { + return LEVEL_RANK[level] <= LEVEL_RANK[this._level]; + } + + private prefix(level: LogLevel): string { + if (!this._timestamps) return ""; + const ts = new Date().toISOString(); + return `[${ts}] [${level.toUpperCase()}] `; + } + + error(message: string, ...args: unknown[]): void { + if (!this.shouldLog("error")) return; + const parts = [this.prefix("error") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + warn(message: string, ...args: unknown[]): void { + if (!this.shouldLog("warn")) return; + const parts = [this.prefix("warn") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + info(message: string, ...args: unknown[]): void { + if (!this.shouldLog("info")) return; + const parts = [this.prefix("info") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + debug(message: string, ...args: unknown[]): void { + if (!this.shouldLog("debug")) return; + const parts = [this.prefix("debug") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + /** Log a structured object at debug level. Redacts nothing — call only with safe data. */ + debugObject(label: string, obj: unknown): void { + if (!this.shouldLog("debug")) return; + const ts = this._timestamps ? `[${new Date().toISOString()}] [DEBUG] ` : ""; + process.stderr.write(`${ts}${label}: ${JSON.stringify(obj, null, 2)}\n`); + } +} + +/** Singleton logger shared across all NemoClaw modules. */ +export const log = new Logger(); diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index 031a979e77b..61c65131041 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,7 @@ import { Command, Flags } from "@oclif/core"; +import { log } from "./logger"; import { redactForLog } from "../security/redact"; export type CommandExitResult = { @@ -20,8 +21,37 @@ export type CommandExitResult = { export abstract class NemoClawCommand extends Command { static baseFlags = { help: Flags.help({ char: "h" }), + // Hidden logging flags. Universal visible flags would have to be + // documented in every command section of docs/reference/commands.mdx + // (cli-parity gate), so the documented interface is + // NEMOCLAW_LOG_LEVEL/NEMOCLAW_DEBUG; the flags remain as a convenience. + debug: Flags.boolean({ + description: "Enable debug output (equivalent to NEMOCLAW_LOG_LEVEL=debug)", + env: "NEMOCLAW_DEBUG", + default: false, + hidden: true, + exclusive: ["quiet"], + }), + quiet: Flags.boolean({ + description: "Suppress informational output; show only warnings and errors", + default: false, + hidden: true, + exclusive: ["debug"], + }), }; + async init(): Promise { + await super.init(); + // Configure logging from raw argv rather than this.parse(): an early + // parse would trigger oclif's default help on --help and preempt the + // custom help of passthrough commands (e.g. ` agent`). The flags + // are still declared in baseFlags so each command's own parse accepts + // them and enforces the exclusive constraint. Debug wins when both + // appear here; the command's parse rejects that combination anyway. + if (this.argv.includes("--quiet")) log.setQuiet(true); + if (this.argv.includes("--debug")) log.setDebug(true); + } + protected logJson(json: unknown): void { console.log(JSON.stringify(redactForLog(json), null, 2)); } From 32d8bad6a45ebde0fc87bb23f221ebe7c7497aa2 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 11:03:36 -0700 Subject: [PATCH 02/11] fix(cli): harden structured logging controls Signed-off-by: Apurv Kumaria --- docs/reference/commands-nemohermes.mdx | 20 ++ docs/reference/commands.mdx | 20 ++ src/commands/sandbox/agent.test.ts | 27 ++- src/commands/sandbox/exec.test.ts | 21 +- .../simple-global-oclif-adapters.test.ts | 31 ++- src/lib/cli/logger.test.ts | 165 ++++++++++++--- src/lib/cli/logger.ts | 188 ++++++++++++++---- src/lib/cli/nemoclaw-oclif-command.test.ts | 36 +++- src/lib/cli/nemoclaw-oclif-command.ts | 43 ++-- src/lib/security/redact.test.ts | 20 ++ src/lib/security/redact.ts | 10 +- 11 files changed, 499 insertions(+), 82 deletions(-) diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index deebd6cf86a..98b2c0d15cc 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -2140,6 +2140,26 @@ NemoClaw reads the following environment variables to configure service ports, o Set them before running `nemohermes onboard` or any command that starts services. All ports must be non-privileged integers between 1024 and 65535. +### CLI Logging + +The centralized CLI logger writes its output to `stderr` and uses `info` verbosity by default. +These controls affect leveled logger output; they do not suppress command results or command-specific output that has not migrated to the centralized logger. + +| Variable | Accepted values | Effect | +|----------|-----------------|--------| +| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over the other debug environment variables. An invalid, blank, or unset value falls through to the lower-priority selectors. | +| `NEMOCLAW_DEBUG` | `1`, `true`, `y`, or `yes` (case-insensitive) | Enables `debug` logging when `NEMOCLAW_LOG_LEVEL` does not contain a valid value. | +| `DEBUG` | Comma- or whitespace-separated namespace selectors with optional `*` wildcards and `-` exclusions | Enables `debug` logging when a selector matches the NemoClaw namespace and neither of the higher-priority variables selects a level. For example, `DEBUG=nemoclaw` and `DEBUG=*` enable NemoClaw logging, while a matching exclusion such as `-nemoclaw` disables it. | + +The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, then `DEBUG`, followed by the default `info` level. +The `error` level prints errors only, `warn` also prints warnings, `info` also prints informational messages, and `debug` prints all levels with timestamps. + +Commands whose parser owns the base logging options also accept the hidden long-form `--debug` and `--quiet` flags, even though these options do not appear in command help. +The flags are mutually exclusive. +`--debug` overrides the environment-derived threshold and selects `debug`, while `--quiet` caps verbosity at `warn` without increasing an environment-derived `error` threshold. +There is no global `-q` logging shorthand. +Passthrough commands do not consume flags intended for the downstream command as host logging options, so use the environment variables when you need unambiguous host logging around a passthrough invocation. + | Variable | Default | Service | |----------|---------|---------| | `NEMOCLAW_GATEWAY_PORT` | 8080 | OpenShell gateway port | diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 4930992cc36..9b0405858fe 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2599,6 +2599,26 @@ NemoClaw reads the following environment variables to configure service ports, o Set them before running `$$nemoclaw onboard` or any command that starts services. All ports must be non-privileged integers between 1024 and 65535. +### CLI Logging + +The centralized CLI logger writes its output to `stderr` and uses `info` verbosity by default. +These controls affect leveled logger output; they do not suppress command results or command-specific output that has not migrated to the centralized logger. + +| Variable | Accepted values | Effect | +|----------|-----------------|--------| +| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over the other debug environment variables. An invalid, blank, or unset value falls through to the lower-priority selectors. | +| `NEMOCLAW_DEBUG` | `1`, `true`, `y`, or `yes` (case-insensitive) | Enables `debug` logging when `NEMOCLAW_LOG_LEVEL` does not contain a valid value. | +| `DEBUG` | Comma- or whitespace-separated namespace selectors with optional `*` wildcards and `-` exclusions | Enables `debug` logging when a selector matches the NemoClaw namespace and neither of the higher-priority variables selects a level. For example, `DEBUG=nemoclaw` and `DEBUG=*` enable NemoClaw logging, while a matching exclusion such as `-nemoclaw` disables it. | + +The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, then `DEBUG`, followed by the default `info` level. +The `error` level prints errors only, `warn` also prints warnings, `info` also prints informational messages, and `debug` prints all levels with timestamps. + +Commands whose parser owns the base logging options also accept the hidden long-form `--debug` and `--quiet` flags, even though these options do not appear in command help. +The flags are mutually exclusive. +`--debug` overrides the environment-derived threshold and selects `debug`, while `--quiet` caps verbosity at `warn` without increasing an environment-derived `error` threshold. +There is no global `-q` logging shorthand. +Passthrough commands do not consume flags intended for the downstream command as host logging options, so use the environment variables when you need unambiguous host logging around a passthrough invocation. + | Variable | Default | Service | |----------|---------|---------| | `NEMOCLAW_GATEWAY_PORT` | 8080 | OpenShell gateway port | diff --git a/src/commands/sandbox/agent.test.ts b/src/commands/sandbox/agent.test.ts index b56f28c9bfe..c1438491ab5 100644 --- a/src/commands/sandbox/agent.test.ts +++ b/src/commands/sandbox/agent.test.ts @@ -8,6 +8,7 @@ vi.mock("../../lib/actions/sandbox/agent/passthrough", () => ({ runAgentPassthrough: runAgentPassthroughMock, })); +import { log } from "../../lib/cli/logger"; import SandboxAgentCommand from "./agent"; const rootDir = process.cwd(); @@ -21,7 +22,7 @@ describe("SandboxAgentCommand oclif parse path", () => { }); afterEach(() => { - logSpy.mockRestore(); + vi.restoreAllMocks(); }); it("forwards the OpenClaw argv verbatim to runAgentPassthrough", async () => { @@ -31,6 +32,30 @@ describe("SandboxAgentCommand oclif parse path", () => { }); }); + it("forwards downstream logging flags without changing host logging", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await SandboxAgentCommand.run(["alpha", "--debug", "-m", "hi"], rootDir); + + expect(runAgentPassthroughMock).toHaveBeenCalledWith("alpha", { + extraArgs: ["--debug", "-m", "hi"], + }); + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: false }); + expect(configure).not.toHaveBeenCalledWith({ debug: true, quiet: false }); + }); + + it("preserves the option boundary without treating later flags as host flags", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await SandboxAgentCommand.run(["alpha", "--", "--quiet"], rootDir); + + expect(runAgentPassthroughMock).toHaveBeenCalledWith("alpha", { + extraArgs: ["--", "--quiet"], + }); + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: false }); + expect(configure).not.toHaveBeenCalledWith({ debug: false, quiet: true }); + }); + it("passes --help after the sandbox name to agent-aware dispatch (#5790)", async () => { await SandboxAgentCommand.run(["alpha", "--help"], rootDir); expect(runAgentPassthroughMock).toHaveBeenCalledWith("alpha", { diff --git a/src/commands/sandbox/exec.test.ts b/src/commands/sandbox/exec.test.ts index 948a8e4a654..ded2bb8929a 100644 --- a/src/commands/sandbox/exec.test.ts +++ b/src/commands/sandbox/exec.test.ts @@ -1,13 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const execSandboxMock = vi.hoisted(() => vi.fn(async () => {})); vi.mock("../../lib/actions/sandbox/exec", () => ({ execSandbox: execSandboxMock, })); +import { log } from "../../lib/cli/logger"; import SandboxExecCommand from "./exec"; const rootDir = process.cwd(); @@ -17,6 +18,10 @@ describe("SandboxExecCommand oclif parse path", () => { execSandboxMock.mockReset(); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it("forwards everything after -- as the inner command argv", async () => { await SandboxExecCommand.run( ["alpha", "--", "openclaw", "agent", "--agent", "main", "-m", "hi"], @@ -29,6 +34,20 @@ describe("SandboxExecCommand oclif parse path", () => { ); }); + it("does not assign host meaning to logging flags after --", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await SandboxExecCommand.run(["alpha", "--", "agent-cli", "--debug", "--quiet"], rootDir); + + expect(execSandboxMock).toHaveBeenCalledWith("alpha", ["agent-cli", "--debug", "--quiet"], { + workdir: undefined, + tty: null, + timeoutSeconds: undefined, + }); + expect(configure).not.toHaveBeenCalledWith({ debug: true, quiet: false }); + expect(configure).not.toHaveBeenCalledWith({ debug: false, quiet: true }); + }); + it("parses --workdir before -- and keeps the inner command intact", async () => { await SandboxExecCommand.run( ["alpha", "--workdir", "/sandbox/workspace", "--", "ls", "-la"], diff --git a/src/commands/simple-global-oclif-adapters.test.ts b/src/commands/simple-global-oclif-adapters.test.ts index 7d79837b566..b7f0083a4c6 100644 --- a/src/commands/simple-global-oclif-adapters.test.ts +++ b/src/commands/simple-global-oclif-adapters.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => { class GatewayTokenCommandError extends Error { @@ -87,6 +87,7 @@ vi.mock("../lib/uninstall-command", () => ({ })); vi.mock("../lib/core/version", () => ({ getVersion: mocks.getVersion })); +import { log } from "../lib/cli/logger"; import DebugCliCommand from "./debug"; import DeployCliCommand from "./deploy"; import RootHelpCommand from "./root/help"; @@ -110,6 +111,10 @@ describe("simple global oclif adapters", () => { vi.clearAllMocks(); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it("maps debug and deploy parser output to actions", async () => { await DebugCliCommand.run( ["--quick", "--output", "/tmp/debug.tar.gz", "--sandbox", "alpha"], @@ -127,6 +132,18 @@ describe("simple global oclif adapters", () => { expect(mocks.runDeployAction).toHaveBeenCalledWith("gpu-alpha"); }); + it("keeps debug -q scoped to quick diagnostics instead of global quiet mode", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await DebugCliCommand.run(["-q"], rootDir); + + expect(mocks.runDebugCommandWithOptions).toHaveBeenCalledWith( + { quick: true }, + expect.objectContaining({ runDebug: expect.any(Function) }), + ); + expect(configure).not.toHaveBeenCalledWith({ debug: false, quiet: true }); + }); + it("builds debug defaults from the sandbox registry and OpenShell liveness", async () => { mocks.listSandboxes.mockReturnValue({ defaultSandbox: "alpha", @@ -309,4 +326,16 @@ describe("simple global oclif adapters", () => { }), ); }); + + it("forwards uninstall flags without assigning host logging semantics", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await UninstallCliCommand.run(["--yes", "--debug"], rootDir); + + expect(mocks.runUninstallCommand).toHaveBeenCalledWith( + expect.objectContaining({ args: ["--yes", "--debug"] }), + ); + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: false }); + expect(configure).not.toHaveBeenCalledWith({ debug: true, quiet: false }); + }); }); diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts index 01d2ad5697f..9ad37ed87af 100644 --- a/src/lib/cli/logger.test.ts +++ b/src/lib/cli/logger.test.ts @@ -3,35 +3,87 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { LogLevel } from "./logger"; - -// Re-import logger fresh for each test to reset singleton state +// Re-import logger fresh for each test to reset singleton state. async function freshLogger() { vi.resetModules(); - const mod = await import("./logger"); - return mod; + return import("./logger"); } describe("Logger", () => { let stderrSpy: ReturnType; beforeEach(() => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", undefined); + vi.stubEnv("NEMOCLAW_DEBUG", undefined); + vi.stubEnv("DEBUG", undefined); stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); - stderrSpy.mockClear(); - vi.unstubAllEnvs(); }); afterEach(() => { stderrSpy.mockRestore(); + vi.unstubAllEnvs(); }); + function output(): string { + return stderrSpy.mock.calls.map((call: unknown[]) => String(call[0])).join(""); + } + it("defaults to info level", async () => { const { log } = await freshLogger(); expect(log.level).toBe("info"); }); - it("reads NEMOCLAW_LOG_LEVEL from env", async () => { - vi.stubEnv("NEMOCLAW_LOG_LEVEL", "debug"); + it("reads a trimmed case-insensitive NEMOCLAW_LOG_LEVEL", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", " DEBUG "); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it.each([ + "1", + "true", + "y", + "yes", + "TRUE", + ])("enables debug for the supported NEMOCLAW_DEBUG value %s", async (value) => { + vi.stubEnv("NEMOCLAW_DEBUG", value); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it.each([ + "*", + "nemoclaw", + "foo,*nemoclaw*", + "foo nemoclaw*", + ])("enables debug when DEBUG selector %s includes the NemoClaw namespace", async (value) => { + vi.stubEnv("DEBUG", value); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it.each([ + "notnemoclaw", + "foo", + "*,-nemoclaw", + "*nemoclaw*,-nemoclaw*", + ])("does not enable debug when DEBUG selector %s excludes the NemoClaw namespace", async (value) => { + vi.stubEnv("DEBUG", value); + const { log } = await freshLogger(); + expect(log.level).toBe("info"); + }); + + it("gives a valid NEMOCLAW_LOG_LEVEL precedence over debug selectors", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "error"); + vi.stubEnv("NEMOCLAW_DEBUG", "true"); + vi.stubEnv("DEBUG", "*"); + const { log } = await freshLogger(); + expect(log.level).toBe("error"); + }); + + it("falls through an invalid NEMOCLAW_LOG_LEVEL to debug selectors", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "verbose"); + vi.stubEnv("NEMOCLAW_DEBUG", "true"); const { log } = await freshLogger(); expect(log.level).toBe("debug"); }); @@ -47,47 +99,106 @@ describe("Logger", () => { const { log } = await freshLogger(); log.setDebug(true); log.debug("visible debug"); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible debug")); + expect(output()).toContain("visible debug"); }); - it("quiet mode suppresses info", async () => { + it("quiet mode suppresses info and still shows warnings", async () => { const { log } = await freshLogger(); log.setQuiet(true); log.info("suppressed info"); - expect(stderrSpy).not.toHaveBeenCalled(); + log.warn("visible warning"); + expect(output()).toBe("visible warning\n"); + }); + + it("can remove quiet and debug overrides without leaking state", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + expect(log.level).toBe("warn"); + log.setQuiet(false); + expect(log.level).toBe("info"); + log.setDebug(true); + expect(log.level).toBe("debug"); + log.setDebug(false); + expect(log.level).toBe("info"); }); - it("quiet mode still shows warn", async () => { + it("configure resets prior overrides to the current environment baseline", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "error"); const { log } = await freshLogger(); + log.setDebug(true); log.setQuiet(true); + log.configure(); + expect(log.level).toBe("error"); + expect(log.isQuiet()).toBe(false); + }); + + it("quiet overrides environment debug without retaining debug timestamps", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "debug"); + const { log } = await freshLogger(); + log.configure({ quiet: true }); log.warn("visible warning"); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible warning")); + expect(log.level).toBe("warn"); + expect(output()).toBe("visible warning\n"); }); - it("error always shown", async () => { + it("shows only errors at error level", async () => { const { log } = await freshLogger(); - log.setLevel("error" as LogLevel); + log.setLevel("error"); + log.warn("suppressed warning"); log.error("critical error"); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("critical error")); + expect(output()).toBe("critical error\n"); }); - it("error suppressed below error level only for warn+info+debug", async () => { + it("redacts secrets from messages, arguments, labels, and structured values", async () => { + const secret = `nvapi-${"a".repeat(40)}`; const { log } = await freshLogger(); - log.setLevel("error" as LogLevel); - log.warn("should be suppressed"); - expect(stderrSpy).not.toHaveBeenCalled(); + log.setDebug(true); + log.debug(`token=${secret}`, { authorization: `Bearer ${secret}` }); + log.debugObject(`context ${secret}`, { + apiKey: secret, + auth: "opaque-auth-secret", + cookie: "session=opaque-cookie-secret", + "API Key": "opaque-api-secret", + nested: { message: `Bearer ${secret}` }, + url: "https://user:password@example.test/path?access_token=raw-token", + }); + expect(output()).not.toContain(secret); + expect(output()).not.toContain("user:password"); + expect(output()).not.toContain("raw-token"); + expect(output()).not.toContain("opaque-auth-secret"); + expect(output()).not.toContain("opaque-cookie-secret"); + expect(output()).not.toContain("opaque-api-secret"); + expect(output()).toContain(""); }); - it("debugObject emits JSON at debug level", async () => { + it("serializes circular values, BigInt, Error, Map, and Set without throwing", async () => { const { log } = await freshLogger(); - log.setLevel("debug"); - log.debugObject("context", { key: "val" }); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('"key"')); + log.setDebug(true); + const value: Record = { + count: 1n, + error: new Error("failure"), + map: new Map([["token", "secret-value"]]), + set: new Set(["one", "two"]), + }; + value.self = value; + expect(() => log.debugObject("context", value)).not.toThrow(); + expect(output()).toContain('"count": "1"'); + expect(output()).toContain('"self": "[Circular]"'); + expect(output()).toContain('"name": "Error"'); }); - it("debugObject suppressed at info level", async () => { + it("does not let a synchronous stderr failure escape", async () => { + const { log } = await freshLogger(); + stderrSpy.mockImplementation(() => { + throw new Error("closed sink"); + }); + expect(() => log.error("failure")).not.toThrow(); + log.setDebug(true); + expect(() => log.debugObject("context", { ok: true })).not.toThrow(); + }); + + it("suppresses debugObject at info level", async () => { const { log } = await freshLogger(); - log.setLevel("info"); log.debugObject("context", { key: "val" }); expect(stderrSpy).not.toHaveBeenCalled(); }); diff --git a/src/lib/cli/logger.ts b/src/lib/cli/logger.ts index 4bcb1e2397b..5de92c20f92 100644 --- a/src/lib/cli/logger.ts +++ b/src/lib/cli/logger.ts @@ -1,6 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { redact, redactForLog } from "../security/redact"; + /** * Centralized logger for NemoClaw CLI. * @@ -8,17 +10,23 @@ * error < warn < info < debug * * Default level: info (errors, warnings, and info messages shown). - * Quiet mode: warn (only warnings and errors shown). + * Quiet mode: at most warn (only warnings and errors shown). * Debug mode: debug (all messages shown with timestamps). * * Configure via: * NEMOCLAW_LOG_LEVEL=debug nemoclaw ... - * nemoclaw ... --debug (shorthand for debug level) - * nemoclaw ... -q / --quiet (suppresses info, shows warn+error) + * NEMOCLAW_DEBUG=1 nemoclaw ... + * nemoclaw ... --debug + * nemoclaw ... --quiet */ export type LogLevel = "error" | "warn" | "info" | "debug"; +export type LoggerConfig = { + debug?: boolean; + quiet?: boolean; +}; + const LEVEL_RANK: Record = { error: 0, warn: 1, @@ -26,46 +34,150 @@ const LEVEL_RANK: Record = { debug: 3, }; +const TRUE_ENV_VALUES = new Set(["1", "true", "y", "yes"]); +const UNSERIALIZABLE = "[unserializable]"; + +function wildcardMatches(pattern: string, value: string): boolean { + const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replaceAll("*", ".*"); + return new RegExp(`^${escaped}$`, "i").test(value); +} + +function debugNamespaceEnabled(value: string | undefined): boolean { + if (!value) return false; + const selectors = value.split(/[\s,]+/).filter(Boolean); + const matches = (selector: string): boolean => wildcardMatches(selector, "nemoclaw"); + if (selectors.some((selector) => selector.startsWith("-") && matches(selector.slice(1)))) { + return false; + } + return selectors.some((selector) => !selector.startsWith("-") && matches(selector)); +} + function resolveLevel(): LogLevel { - const env = process.env.NEMOCLAW_LOG_LEVEL?.toLowerCase(); + const env = process.env.NEMOCLAW_LOG_LEVEL?.trim().toLowerCase(); if (env === "error" || env === "warn" || env === "info" || env === "debug") return env; - if (process.env.NEMOCLAW_DEBUG === "1" || process.env.DEBUG?.includes("nemoclaw")) return "debug"; + const debugEnv = process.env.NEMOCLAW_DEBUG?.trim().toLowerCase(); + if (debugEnv && TRUE_ENV_VALUES.has(debugEnv)) return "debug"; + if (debugNamespaceEnabled(process.env.DEBUG)) return "debug"; return "info"; } +function normalizeForSerialization(value: unknown, seen = new WeakSet()): unknown { + if (value === null || typeof value === "string" || typeof value === "number") return value; + if (typeof value === "boolean") return value; + if (typeof value === "bigint") return value.toString(); + if (typeof value === "undefined") return "[undefined]"; + if (typeof value === "symbol" || typeof value === "function") return String(value); + + try { + if (seen.has(value)) return "[Circular]"; + seen.add(value); + + if (value instanceof Error) { + const normalized: Record = { + name: value.name, + message: value.message, + }; + if (value.stack) normalized.stack = value.stack; + if (value.cause !== undefined) { + normalized.cause = normalizeForSerialization(value.cause, seen); + } + for (const [key, entry] of Object.entries(value)) { + normalized[key] = normalizeForSerialization(entry, seen); + } + return normalized; + } + + if (value instanceof Date) { + return Number.isNaN(value.getTime()) ? "Invalid Date" : value.toISOString(); + } + if (value instanceof RegExp || value instanceof URL) return String(value); + if (Buffer.isBuffer(value)) return `[Buffer ${value.length} bytes]`; + if (ArrayBuffer.isView(value)) { + return `[${value.constructor.name} ${value.byteLength} bytes]`; + } + if (value instanceof ArrayBuffer) return `[ArrayBuffer ${value.byteLength} bytes]`; + if (value instanceof Map) { + const entries: Record = {}; + for (const [key, entry] of value.entries()) { + entries[String(key)] = normalizeForSerialization(entry, seen); + } + return entries; + } + if (value instanceof Set) { + return [...value].map((entry) => normalizeForSerialization(entry, seen)); + } + if (Array.isArray(value)) { + return value.map((entry) => normalizeForSerialization(entry, seen)); + } + + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [key, normalizeForSerialization(entry, seen)]), + ); + } catch { + return UNSERIALIZABLE; + } +} + +function safeSerialize(value: unknown): string { + try { + const normalized = normalizeForSerialization(value); + const serialized = JSON.stringify(redactForLog(normalized), null, 2); + return serialized ? redact(serialized) : JSON.stringify(UNSERIALIZABLE); + } catch { + return JSON.stringify(UNSERIALIZABLE); + } +} + +function safeText(value: unknown): string { + try { + if (typeof value === "string") return redact(String(redactForLog(value))); + if (value === null || typeof value === "number" || typeof value === "boolean") { + return String(value); + } + if (typeof value === "bigint") return value.toString(); + if (typeof value === "undefined") return "undefined"; + return safeSerialize(value); + } catch { + return UNSERIALIZABLE; + } +} + class Logger { - private _level: LogLevel; - private _quiet: boolean; - private _timestamps: boolean; + private _level: LogLevel = "info"; + private _quiet = false; + private _debug = false; constructor() { - this._level = resolveLevel(); - this._quiet = false; - this._timestamps = this._level === "debug"; + this.configure(); } get level(): LogLevel { + if (this._debug) return "debug"; + if (this._quiet && LEVEL_RANK[this._level] > LEVEL_RANK.warn) return "warn"; return this._level; } + /** Reset to environment defaults, then apply command-line overrides. */ + configure(config: LoggerConfig = {}): void { + this._level = resolveLevel(); + this._quiet = config.quiet === true; + this._debug = config.debug === true; + } + setLevel(level: LogLevel): void { this._level = level; - this._timestamps = level === "debug"; } setQuiet(quiet: boolean): void { this._quiet = quiet; - if (quiet && LEVEL_RANK[this._level] > LEVEL_RANK["warn"]) { - this._level = "warn"; - } } setDebug(debug: boolean): void { - if (debug) this.setLevel("debug"); + this._debug = debug; } isDebug(): boolean { - return this._level === "debug"; + return this.level === "debug"; } isQuiet(): boolean { @@ -73,44 +185,48 @@ class Logger { } private shouldLog(level: LogLevel): boolean { - return LEVEL_RANK[level] <= LEVEL_RANK[this._level]; + return LEVEL_RANK[level] <= LEVEL_RANK[this.level]; } private prefix(level: LogLevel): string { - if (!this._timestamps) return ""; - const ts = new Date().toISOString(); - return `[${ts}] [${level.toUpperCase()}] `; + if (!this.isDebug()) return ""; + return `[${new Date().toISOString()}] [${level.toUpperCase()}] `; + } + + private emit(line: string): void { + try { + process.stderr.write(line); + } catch { + // A diagnostic sink must not turn an otherwise successful command into a failure. + } + } + + private write(level: LogLevel, message: string, args: unknown[]): void { + if (!this.shouldLog(level)) return; + const parts = [this.prefix(level) + safeText(message), ...args.map(safeText)].join(" "); + this.emit(`${parts}\n`); } error(message: string, ...args: unknown[]): void { - if (!this.shouldLog("error")) return; - const parts = [this.prefix("error") + message, ...args.map(String)].join(" "); - process.stderr.write(parts + "\n"); + this.write("error", message, args); } warn(message: string, ...args: unknown[]): void { - if (!this.shouldLog("warn")) return; - const parts = [this.prefix("warn") + message, ...args.map(String)].join(" "); - process.stderr.write(parts + "\n"); + this.write("warn", message, args); } info(message: string, ...args: unknown[]): void { - if (!this.shouldLog("info")) return; - const parts = [this.prefix("info") + message, ...args.map(String)].join(" "); - process.stderr.write(parts + "\n"); + this.write("info", message, args); } debug(message: string, ...args: unknown[]): void { - if (!this.shouldLog("debug")) return; - const parts = [this.prefix("debug") + message, ...args.map(String)].join(" "); - process.stderr.write(parts + "\n"); + this.write("debug", message, args); } - /** Log a structured object at debug level. Redacts nothing — call only with safe data. */ + /** Log a redacted structured value without allowing serialization errors to escape. */ debugObject(label: string, obj: unknown): void { if (!this.shouldLog("debug")) return; - const ts = this._timestamps ? `[${new Date().toISOString()}] [DEBUG] ` : ""; - process.stderr.write(`${ts}${label}: ${JSON.stringify(obj, null, 2)}\n`); + this.emit(`${this.prefix("debug")}${safeText(label)}: ${safeSerialize(obj)}\n`); } } diff --git a/src/lib/cli/nemoclaw-oclif-command.test.ts b/src/lib/cli/nemoclaw-oclif-command.test.ts index aaf6f819ad4..cdf76a685c3 100644 --- a/src/lib/cli/nemoclaw-oclif-command.test.ts +++ b/src/lib/cli/nemoclaw-oclif-command.test.ts @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { afterEach, describe, expect, it, vi } from "vitest"; - -import { NemoClawCommand, type CommandExitResult } from "./nemoclaw-oclif-command"; +import { log } from "./logger"; +import { type CommandExitResult, NemoClawCommand } from "./nemoclaw-oclif-command"; class TestCommand extends NemoClawCommand { static id = "test"; @@ -25,6 +25,15 @@ class TestCommand extends NemoClawCommand { } } +class ParsingTestCommand extends NemoClawCommand { + static id = "parsing-test"; + static flags = {}; + + public async run(): Promise { + await this.parse(ParsingTestCommand); + } +} + function makeCommand(): TestCommand { return Object.create(TestCommand.prototype) as TestCommand; } @@ -32,6 +41,8 @@ function makeCommand(): TestCommand { describe("NemoClawCommand", () => { afterEach(() => { vi.restoreAllMocks(); + vi.unstubAllEnvs(); + log.configure(); process.exitCode = undefined; }); @@ -68,4 +79,25 @@ describe("NemoClawCommand", () => { JSON.stringify({ provider: "build", apiKey: "" }, null, 2), ); }); + + it("applies host logging flags from oclif parser output", async () => { + const configure = vi.spyOn(log, "configure").mockImplementation(() => undefined); + + await ParsingTestCommand.run(["--quiet"], process.cwd()); + await ParsingTestCommand.run(["--debug"], process.cwd()); + + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: true }); + expect(configure).toHaveBeenCalledWith({ debug: true, quiet: false }); + }); + + it("keeps NEMOCLAW_LOG_LEVEL precedence unless a CLI flag overrides it", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "error"); + vi.stubEnv("NEMOCLAW_DEBUG", "true"); + + await ParsingTestCommand.run([], process.cwd()); + expect(log.level).toBe("error"); + + await ParsingTestCommand.run(["--debug"], process.cwd()); + expect(log.level).toBe("debug"); + }); }); diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index 61c65131041..cfee9056011 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -1,10 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { Command, Flags } from "@oclif/core"; - -import { log } from "./logger"; +import { Command, Flags, type Interfaces } from "@oclif/core"; import { redactForLog } from "../security/redact"; +import { log } from "./logger"; export type CommandExitResult = { exitCode?: number | null; @@ -27,7 +26,6 @@ export abstract class NemoClawCommand extends Command { // NEMOCLAW_LOG_LEVEL/NEMOCLAW_DEBUG; the flags remain as a convenience. debug: Flags.boolean({ description: "Enable debug output (equivalent to NEMOCLAW_LOG_LEVEL=debug)", - env: "NEMOCLAW_DEBUG", default: false, hidden: true, exclusive: ["quiet"], @@ -40,16 +38,35 @@ export abstract class NemoClawCommand extends Command { }), }; - async init(): Promise { + protected override async init(): Promise { await super.init(); - // Configure logging from raw argv rather than this.parse(): an early - // parse would trigger oclif's default help on --help and preempt the - // custom help of passthrough commands (e.g. ` agent`). The flags - // are still declared in baseFlags so each command's own parse accepts - // them and enforces the exclusive constraint. Debug wins when both - // appear here; the command's parse rejects that combination anyway. - if (this.argv.includes("--quiet")) log.setQuiet(true); - if (this.argv.includes("--debug")) log.setDebug(true); + // Every invocation starts from the current environment. Raw-argv + // passthrough commands intentionally stop here: only environment-based + // logging configuration applies to them. + log.configure({ debug: false, quiet: false }); + } + + protected override async parse< + F extends Interfaces.OutputFlags, + B extends Interfaces.OutputFlags, + A extends Interfaces.OutputArgs, + >( + options?: Interfaces.Input, + argv?: string[], + ): Promise> { + const parsed = await super.parse(options, argv); + + // Logging flags belong to the host only when a command invokes oclif's + // parser. Commands that deliberately consume raw argv (for example + // `sandbox agent` and `uninstall`) must forward similarly named flags + // without changing host logging. Using parser output also honors `--`: + // downstream flags after the boundary never acquire host meaning. + log.configure({ + debug: parsed.flags.debug === true, + quiet: parsed.flags.quiet === true, + }); + + return parsed; } protected logJson(json: unknown): void { diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index d2b54dbf7a9..b67d77c2566 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -115,6 +115,26 @@ describe("redactForLog", () => { }); }); + it("redacts opaque credentials under auth, cookie, and spaced API-key fields", () => { + expect( + redactForLog({ + auth: "opaque-auth-secret", + cookie: "session=opaque-cookie-secret", + setCookie: "session=opaque-set-cookie-secret", + "API Key": "opaque-api-secret", + headers: { proxyAuth: "Basic opaque-basic-secret" }, + author: "safe author", + }), + ).toEqual({ + auth: "", + cookie: "", + setCookie: "", + "API Key": "", + headers: { proxyAuth: "" }, + author: "safe author", + }); + }); + it("redacts known secret patterns inside otherwise safe strings", () => { const result = redactForLog({ message: "upstream returned Authorization: Bearer abcdefghijklmnop", diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index 595c612f1c7..af36e429ef6 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -245,7 +245,15 @@ export function redactUrl(value: unknown): string | null { } function isSensitiveKey(key: string): boolean { - return /(?:api[_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key); + if (/(?:api[\s_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key)) { + return true; + } + const words = key + .replace(/([a-z0-9])([A-Z])/g, "$1 $2") + .toLowerCase() + .split(/[^a-z0-9]+/) + .filter(Boolean); + return words.includes("auth") || words.includes("cookie"); } export function redactForLog(value: unknown, seen: WeakSet = new WeakSet()): unknown { From 5ac42bef880cdcfc029f4925c0c59b4d32866e88 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 11:25:41 -0700 Subject: [PATCH 03/11] test(cli): strengthen logging boundary assertions Signed-off-by: Apurv Kumaria --- src/commands/sandbox/exec.test.ts | 1 + src/commands/simple-global-oclif-adapters.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/commands/sandbox/exec.test.ts b/src/commands/sandbox/exec.test.ts index ded2bb8929a..ae839b213c9 100644 --- a/src/commands/sandbox/exec.test.ts +++ b/src/commands/sandbox/exec.test.ts @@ -44,6 +44,7 @@ describe("SandboxExecCommand oclif parse path", () => { tty: null, timeoutSeconds: undefined, }); + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: false }); expect(configure).not.toHaveBeenCalledWith({ debug: true, quiet: false }); expect(configure).not.toHaveBeenCalledWith({ debug: false, quiet: true }); }); diff --git a/src/commands/simple-global-oclif-adapters.test.ts b/src/commands/simple-global-oclif-adapters.test.ts index b7f0083a4c6..450f4b73de0 100644 --- a/src/commands/simple-global-oclif-adapters.test.ts +++ b/src/commands/simple-global-oclif-adapters.test.ts @@ -141,6 +141,7 @@ describe("simple global oclif adapters", () => { { quick: true }, expect.objectContaining({ runDebug: expect.any(Function) }), ); + expect(configure).toHaveBeenCalledWith({ debug: false, quiet: false }); expect(configure).not.toHaveBeenCalledWith({ debug: false, quiet: true }); }); From 935aa56080c8b23ce29b6806757a18c45b350846 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 11:41:34 -0700 Subject: [PATCH 04/11] fix(cli): close structured logging redaction gaps Signed-off-by: Apurv Kumaria --- docs/reference/commands-nemohermes.mdx | 6 +- docs/reference/commands.mdx | 6 +- src/lib/cli/logger.test.ts | 72 ++++++++---- src/lib/cli/logger.ts | 24 +--- src/lib/security/credential-filter.test.ts | 4 + src/lib/security/redact.test.ts | 126 ++++++++++++++++++++- src/lib/security/redact.ts | 51 ++++++++- 7 files changed, 234 insertions(+), 55 deletions(-) diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index 98b2c0d15cc..cb6b4c650aa 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -2147,12 +2147,12 @@ These controls affect leveled logger output; they do not suppress command result | Variable | Accepted values | Effect | |----------|-----------------|--------| -| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over the other debug environment variables. An invalid, blank, or unset value falls through to the lower-priority selectors. | +| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over `NEMOCLAW_DEBUG`. An invalid, blank, or unset value falls through to `NEMOCLAW_DEBUG`. | | `NEMOCLAW_DEBUG` | `1`, `true`, `y`, or `yes` (case-insensitive) | Enables `debug` logging when `NEMOCLAW_LOG_LEVEL` does not contain a valid value. | -| `DEBUG` | Comma- or whitespace-separated namespace selectors with optional `*` wildcards and `-` exclusions | Enables `debug` logging when a selector matches the NemoClaw namespace and neither of the higher-priority variables selects a level. For example, `DEBUG=nemoclaw` and `DEBUG=*` enable NemoClaw logging, while a matching exclusion such as `-nemoclaw` disables it. | -The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, then `DEBUG`, followed by the default `info` level. +The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, followed by the default `info` level. The `error` level prints errors only, `warn` also prints warnings, `info` also prints informational messages, and `debug` prints all levels with timestamps. +Use these NemoClaw-specific variables instead of the generic `DEBUG` variable. `DEBUG` is not a NemoClaw logger control and can enable dependency diagnostics that include raw command arguments. Commands whose parser owns the base logging options also accept the hidden long-form `--debug` and `--quiet` flags, even though these options do not appear in command help. The flags are mutually exclusive. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 9b0405858fe..b77736a1839 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2606,12 +2606,12 @@ These controls affect leveled logger output; they do not suppress command result | Variable | Accepted values | Effect | |----------|-----------------|--------| -| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over the other debug environment variables. An invalid, blank, or unset value falls through to the lower-priority selectors. | +| `NEMOCLAW_LOG_LEVEL` | `error`, `warn`, `info`, or `debug` (case-insensitive; surrounding whitespace is ignored) | Sets the logging threshold. A valid value takes precedence over `NEMOCLAW_DEBUG`. An invalid, blank, or unset value falls through to `NEMOCLAW_DEBUG`. | | `NEMOCLAW_DEBUG` | `1`, `true`, `y`, or `yes` (case-insensitive) | Enables `debug` logging when `NEMOCLAW_LOG_LEVEL` does not contain a valid value. | -| `DEBUG` | Comma- or whitespace-separated namespace selectors with optional `*` wildcards and `-` exclusions | Enables `debug` logging when a selector matches the NemoClaw namespace and neither of the higher-priority variables selects a level. For example, `DEBUG=nemoclaw` and `DEBUG=*` enable NemoClaw logging, while a matching exclusion such as `-nemoclaw` disables it. | -The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, then `DEBUG`, followed by the default `info` level. +The environment precedence is `NEMOCLAW_LOG_LEVEL`, then `NEMOCLAW_DEBUG`, followed by the default `info` level. The `error` level prints errors only, `warn` also prints warnings, `info` also prints informational messages, and `debug` prints all levels with timestamps. +Use these NemoClaw-specific variables instead of the generic `DEBUG` variable. `DEBUG` is not a NemoClaw logger control and can enable dependency diagnostics that include raw command arguments. Commands whose parser owns the base logging options also accept the hidden long-form `--debug` and `--quiet` flags, even though these options do not appear in command help. The flags are mutually exclusive. diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts index 9ad37ed87af..7b16a4ba82e 100644 --- a/src/lib/cli/logger.test.ts +++ b/src/lib/cli/logger.test.ts @@ -51,37 +51,20 @@ describe("Logger", () => { expect(log.level).toBe("debug"); }); - it.each([ - "*", - "nemoclaw", - "foo,*nemoclaw*", - "foo nemoclaw*", - ])("enables debug when DEBUG selector %s includes the NemoClaw namespace", async (value) => { - vi.stubEnv("DEBUG", value); - const { log } = await freshLogger(); - expect(log.level).toBe("debug"); - }); - - it.each([ - "notnemoclaw", - "foo", - "*,-nemoclaw", - "*nemoclaw*,-nemoclaw*", - ])("does not enable debug when DEBUG selector %s excludes the NemoClaw namespace", async (value) => { - vi.stubEnv("DEBUG", value); + it("does not treat the framework DEBUG variable as a NemoClaw logging control", async () => { + vi.stubEnv("DEBUG", "*"); const { log } = await freshLogger(); expect(log.level).toBe("info"); }); - it("gives a valid NEMOCLAW_LOG_LEVEL precedence over debug selectors", async () => { + it("gives a valid NEMOCLAW_LOG_LEVEL precedence over NEMOCLAW_DEBUG", async () => { vi.stubEnv("NEMOCLAW_LOG_LEVEL", "error"); vi.stubEnv("NEMOCLAW_DEBUG", "true"); - vi.stubEnv("DEBUG", "*"); const { log } = await freshLogger(); expect(log.level).toBe("error"); }); - it("falls through an invalid NEMOCLAW_LOG_LEVEL to debug selectors", async () => { + it("falls through an invalid NEMOCLAW_LOG_LEVEL to NEMOCLAW_DEBUG", async () => { vi.stubEnv("NEMOCLAW_LOG_LEVEL", "verbose"); vi.stubEnv("NEMOCLAW_DEBUG", "true"); const { log } = await freshLogger(); @@ -154,11 +137,37 @@ describe("Logger", () => { const { log } = await freshLogger(); log.setDebug(true); log.debug(`token=${secret}`, { authorization: `Bearer ${secret}` }); + log.debug("Authorization: Basic opaque-basic-header"); + log.debug("Proxy-Authorization: Digest username=opaque-user, response=opaque-response"); + log.debug("Cookie: session=opaque-cookie-header"); + log.debug("Set-Cookie: session=opaque-set-cookie-header; HttpOnly"); + log.debug("OPENAI_API_KEY", "opaque-split-env-value"); + log.debug("NEMOCLAW_PROVIDER_KEY", "-opaque-leading-dash-value"); + log.debug("author", "safe author argument"); log.debugObject(`context ${secret}`, { apiKey: secret, auth: "opaque-auth-secret", - cookie: "session=opaque-cookie-secret", + API_SERVER_KEY: "opaque-server-key", + NEMOCLAW_PROVIDER_KEY: "opaque-provider-key", + privateKey: "opaque-private-key", + sessionKey: "opaque-session-key", "API Key": "opaque-api-secret", + headers: { + "Proxy-Authorization": "Basic opaque-basic-secret", + Cookie: "session=opaque-cookie-secret", + }, + publicKey: "safe public key", + author: "safe author", + argv: [ + "--password", + "opaque-cli-password", + "--api-key", + "opaque-cli-api-key", + "--public-key", + "safe CLI public key", + "--author", + "safe CLI author", + ], nested: { message: `Bearer ${secret}` }, url: "https://user:password@example.test/path?access_token=raw-token", }); @@ -166,8 +175,27 @@ describe("Logger", () => { expect(output()).not.toContain("user:password"); expect(output()).not.toContain("raw-token"); expect(output()).not.toContain("opaque-auth-secret"); + expect(output()).not.toContain("opaque-server-key"); + expect(output()).not.toContain("opaque-provider-key"); + expect(output()).not.toContain("opaque-private-key"); + expect(output()).not.toContain("opaque-session-key"); + expect(output()).not.toContain("opaque-basic-secret"); + expect(output()).not.toContain("opaque-basic-header"); + expect(output()).not.toContain("opaque-user"); + expect(output()).not.toContain("opaque-response"); expect(output()).not.toContain("opaque-cookie-secret"); + expect(output()).not.toContain("opaque-cookie-header"); + expect(output()).not.toContain("opaque-set-cookie-header"); expect(output()).not.toContain("opaque-api-secret"); + expect(output()).not.toContain("opaque-split-env-value"); + expect(output()).not.toContain("opaque-leading-dash-value"); + expect(output()).not.toContain("opaque-cli-password"); + expect(output()).not.toContain("opaque-cli-api-key"); + expect(output()).toContain("safe public key"); + expect(output()).toContain("safe author"); + expect(output()).toContain("safe author argument"); + expect(output()).toContain("safe CLI public key"); + expect(output()).toContain("safe CLI author"); expect(output()).toContain(""); }); diff --git a/src/lib/cli/logger.ts b/src/lib/cli/logger.ts index 5de92c20f92..1e9b48df456 100644 --- a/src/lib/cli/logger.ts +++ b/src/lib/cli/logger.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { redact, redactForLog } from "../security/redact"; +import { redact, redactForLog, redactLogSequence } from "../security/redact"; /** * Centralized logger for NemoClaw CLI. @@ -37,27 +37,11 @@ const LEVEL_RANK: Record = { const TRUE_ENV_VALUES = new Set(["1", "true", "y", "yes"]); const UNSERIALIZABLE = "[unserializable]"; -function wildcardMatches(pattern: string, value: string): boolean { - const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replaceAll("*", ".*"); - return new RegExp(`^${escaped}$`, "i").test(value); -} - -function debugNamespaceEnabled(value: string | undefined): boolean { - if (!value) return false; - const selectors = value.split(/[\s,]+/).filter(Boolean); - const matches = (selector: string): boolean => wildcardMatches(selector, "nemoclaw"); - if (selectors.some((selector) => selector.startsWith("-") && matches(selector.slice(1)))) { - return false; - } - return selectors.some((selector) => !selector.startsWith("-") && matches(selector)); -} - function resolveLevel(): LogLevel { const env = process.env.NEMOCLAW_LOG_LEVEL?.trim().toLowerCase(); if (env === "error" || env === "warn" || env === "info" || env === "debug") return env; const debugEnv = process.env.NEMOCLAW_DEBUG?.trim().toLowerCase(); if (debugEnv && TRUE_ENV_VALUES.has(debugEnv)) return "debug"; - if (debugNamespaceEnabled(process.env.DEBUG)) return "debug"; return "info"; } @@ -203,7 +187,8 @@ class Logger { private write(level: LogLevel, message: string, args: unknown[]): void { if (!this.shouldLog(level)) return; - const parts = [this.prefix(level) + safeText(message), ...args.map(safeText)].join(" "); + const [safeMessage, ...safeArgs] = redactLogSequence([message, ...args]).map(safeText); + const parts = [this.prefix(level) + safeMessage, ...safeArgs].join(" "); this.emit(`${parts}\n`); } @@ -226,7 +211,8 @@ class Logger { /** Log a redacted structured value without allowing serialization errors to escape. */ debugObject(label: string, obj: unknown): void { if (!this.shouldLog("debug")) return; - this.emit(`${this.prefix("debug")}${safeText(label)}: ${safeSerialize(obj)}\n`); + const [safeLabel, safeObject] = redactLogSequence([label, obj]); + this.emit(`${this.prefix("debug")}${safeText(safeLabel)}: ${safeSerialize(safeObject)}\n`); } } diff --git a/src/lib/security/credential-filter.test.ts b/src/lib/security/credential-filter.test.ts index 22f64914bff..066418e3650 100644 --- a/src/lib/security/credential-filter.test.ts +++ b/src/lib/security/credential-filter.test.ts @@ -34,6 +34,7 @@ describe("isCredentialField", () => { expect(isCredentialField("bearerToken")).toBe(true); expect(isCredentialField("privateKey")).toBe(true); expect(isCredentialField("sessionToken")).toBe(true); + expect(isCredentialField("sessionKey")).toBe(true); // OpenClaw channel token fields (#5027). expect(isCredentialField("botToken")).toBe(true); expect(isCredentialField("appToken")).toBe(true); @@ -43,6 +44,8 @@ describe("isCredentialField", () => { expect(isCredentialField("GITHUB_TOKEN")).toBe(true); expect(isCredentialField("BRAVE_API_KEY")).toBe(true); expect(isCredentialField("OPENAI_API_KEY")).toBe(true); + expect(isCredentialField("API_SERVER_KEY")).toBe(true); + expect(isCredentialField("NEMOCLAW_PROVIDER_KEY")).toBe(true); expect(isCredentialField("DB_PASSWORD")).toBe(true); expect(isCredentialField("SLACK_APP_TOKEN")).toBe(true); // Bare uppercase secret words must also be scrubbed. @@ -77,6 +80,7 @@ describe("isCredentialField", () => { expect(isCredentialField("tokenizer")).toBe(false); expect(isCredentialField("maxTokens")).toBe(false); expect(isCredentialField("X-Request-Id")).toBe(false); + expect(isCredentialField("author")).toBe(false); }); it("does not strip public keys (verification material, not secrets)", () => { diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index b67d77c2566..a306fdf4f34 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from "vitest"; -import { redact, redactForLog, redactUrl } from "./redact.js"; +import { redact, redactForLog, redactFull, redactLogSequence, redactUrl } from "./redact.js"; describe("URL redaction", () => { it.each([ @@ -115,26 +115,140 @@ describe("redactForLog", () => { }); }); - it("redacts opaque credentials under auth, cookie, and spaced API-key fields", () => { + it("uses canonical credential fields for opaque structured values without false positives", () => { expect( redactForLog({ auth: "opaque-auth-secret", - cookie: "session=opaque-cookie-secret", + API_SERVER_KEY: "opaque-server-key", + NEMOCLAW_PROVIDER_KEY: "opaque-provider-key", + privateKey: "opaque-private-key", + sessionKey: "opaque-session-key", setCookie: "session=opaque-set-cookie-secret", "API Key": "opaque-api-secret", - headers: { proxyAuth: "Basic opaque-basic-secret" }, + headers: { + "Proxy-Authorization": "Basic opaque-basic-secret", + Cookie: "session=opaque-cookie-secret", + }, + secretValue: "opaque-secret-value", + tokenValue: "opaque-token-value", + passwordValue: "opaque-password-value", + publicKey: "safe public key", + PUBLIC_KEY: "safe uppercase public key", author: "safe author", + oauth: "safe auth method", }), ).toEqual({ auth: "", - cookie: "", + API_SERVER_KEY: "", + NEMOCLAW_PROVIDER_KEY: "", + privateKey: "", + sessionKey: "", setCookie: "", "API Key": "", - headers: { proxyAuth: "" }, + headers: { + "Proxy-Authorization": "", + Cookie: "", + }, + secretValue: "", + tokenValue: "", + passwordValue: "", + publicKey: "safe public key", + PUBLIC_KEY: "safe uppercase public key", author: "safe author", + oauth: "safe auth method", }); }); + it("redacts opaque CLI values by sequence and inline flag context", () => { + expect( + redactForLog({ + argv: [ + "--password", + "opaque-password", + "--api-key", + "opaque-api-key", + "--private-key=opaque-inline-private-key", + "--session-key", + "opaque-session-key", + "--password", + "-opaque-leading-dash", + "--api-key", + "--opaque-leading-double-dash", + "--public-key", + "safe-public-key", + "--author", + "safe-author", + "--password", + "--verbose", + "safe-tail", + ], + }), + ).toEqual({ + argv: [ + "--password", + "", + "--api-key", + "", + "--private-key=", + "--session-key", + "", + "--password", + "", + "--api-key", + "", + "--public-key", + "safe-public-key", + "--author", + "safe-author", + "--password", + "", + "safe-tail", + ], + }); + + expect( + redactLogSequence([ + "OPENAI_API_KEY", + "opaque-env-value", + "NEMOCLAW_PROVIDER_KEY", + "-opaque-leading-dash-value", + "author", + "safe-author", + ]), + ).toEqual([ + "OPENAI_API_KEY", + "", + "NEMOCLAW_PROVIDER_KEY", + "", + "author", + "safe-author", + ]); + }); + + it("redacts Basic, Digest, proxy-auth, and cookie text without matching safe labels", () => { + const text = [ + "Authorization: Basic opaque-basic-value", + "Proxy-Authorization: Digest username=opaque-user, response=opaque-response", + "Cookie: session=opaque-cookie-value", + "Set-Cookie: session=opaque-set-cookie-value; HttpOnly", + 'headers={"Authorization":"Basic opaque-json-value"}', + "author: safe-author", + ].join("\n"); + + const result = redactFull(text); + for (const secret of [ + "opaque-basic-value", + "opaque-user", + "opaque-response", + "opaque-cookie-value", + "opaque-set-cookie-value", + "opaque-json-value", + ]) { + expect(result).not.toContain(secret); + } + expect(result).toContain("author: safe-author"); + }); + it("redacts known secret patterns inside otherwise safe strings", () => { const result = redactForLog({ message: "upstream returned Authorization: Bearer abcdefghijklmnop", diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index af36e429ef6..86cf52a65ba 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -19,6 +19,7 @@ import type { StdioOptions } from "node:child_process"; */ import { listMessagingCredentialMetadata } from "../messaging/channels"; +import { isCredentialField } from "./credential-filter"; import { SECRET_BLOCK_PATTERNS, SECRET_PATTERNS, TOKEN_PREFIX_PATTERNS } from "./secret-patterns"; const SENSITIVE_ENV_ASSIGNMENT_KEYS = [ @@ -178,6 +179,18 @@ const FULL_REDACT_PATTERNS: [RegExp, string][] = [ /((?:"|')?(?:api[_-]?key|token|secret|password|credential)(?:"|')?\s*[:=]\s*(?:"|')?)[^"',}\s]+((?:"|')?)/gi, "$1$2", ], + [ + /("(?:authorization|proxy-authorization|cookie|set-cookie)"\s*:\s*")((?:(?:basic|bearer|digest)\s+)?)(?:\\.|[^"\\])*"/gi, + '$1$2"', + ], + [ + /('(?:authorization|proxy-authorization|cookie|set-cookie)'\s*:\s*')((?:(?:basic|bearer|digest)\s+)?)(?:\\.|[^'\\])*'/gi, + "$1$2'", + ], + [ + /(\b(?:authorization|proxy-authorization|cookie|set-cookie)\s*:\s*)((?:(?:basic|bearer|digest)\s+)?)[^\r\n]*/gi, + "$1$2", + ], ...TOKEN_PREFIX_PATTERNS.map((p): [RegExp, string] => [ new RegExp(p.source, p.flags), "", @@ -245,6 +258,7 @@ export function redactUrl(value: unknown): string | null { } function isSensitiveKey(key: string): boolean { + if (isCredentialField(key)) return true; if (/(?:api[\s_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key)) { return true; } @@ -256,13 +270,46 @@ function isSensitiveKey(key: string): boolean { return words.includes("auth") || words.includes("cookie"); } +function credentialFlagKey(value: unknown): string | null { + if (typeof value !== "string") return null; + const flag = /^--?([A-Za-z0-9][A-Za-z0-9._-]*)$/.exec(value.trim()); + return flag && isSensitiveKey(flag[1]) ? flag[1] : null; +} + +function credentialContextKey(value: unknown): string | null { + if (typeof value !== "string") return null; + const flag = credentialFlagKey(value); + if (flag) return flag; + const candidate = value.trim().replace(/[:=]$/, "").trim(); + return candidate && isSensitiveKey(candidate) ? candidate : null; +} + +/** Redact opaque values whose credential context is carried by the previous argument. */ +export function redactLogSequence(values: readonly unknown[]): unknown[] { + return values.map((value, index) => + index > 0 && credentialContextKey(values[index - 1]) !== null ? "" : value, + ); +} + +function redactInlineCredentialFlag(value: string): string { + const match = /^(--?)([A-Za-z0-9][A-Za-z0-9._-]*)=(.*)$/s.exec(value); + if (!match || !isSensitiveKey(match[2])) return redactFull(value); + return `${match[1]}${match[2]}=`; +} + export function redactForLog(value: unknown, seen: WeakSet = new WeakSet()): unknown { - if (typeof value === "string") return redactFull(value); + if (typeof value === "string") return redactInlineCredentialFlag(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)); + if (Array.isArray(value)) { + return value.map((entry, index) => + index > 0 && credentialFlagKey(value[index - 1]) !== null + ? "" + : redactForLog(entry, seen), + ); + } const redacted: Record = {}; for (const [key, entry] of Object.entries(value as Record)) { From 8cfa2bb418a90075c367532ca9a1a8f28f66cb60 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 11:53:10 -0700 Subject: [PATCH 05/11] fix(cli): preserve provenance redaction boundaries Signed-off-by: Apurv Kumaria --- src/lib/openclaw/agent-json-provenance.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/openclaw/agent-json-provenance.ts b/src/lib/openclaw/agent-json-provenance.ts index 8a38b91ed60..a22978bf95e 100644 --- a/src/lib/openclaw/agent-json-provenance.ts +++ b/src/lib/openclaw/agent-json-provenance.ts @@ -23,15 +23,15 @@ type WalkEntry = { }; function snippet(value: string, limit = 300): string { - const squashed = value + const sanitized = value .replace(ANSI_OSC_PATTERN, "") .replace(ANSI_CSI_PATTERN, "") .replace(/\r|\u0008/gu, "") - .replace(CONTROL_PATTERN, "") - .replace(/\s+/gu, " ") - .trim(); - const redacted = redactProvenanceDetail(squashed); - return redacted.length <= limit ? redacted : `${redacted.slice(0, limit - 3)}...`; + .replace(CONTROL_PATTERN, ""); + // Preserve line boundaries while redacting so line-oriented header patterns + // cannot consume unrelated details that happen to follow on another line. + const squashed = redactProvenanceDetail(sanitized).replace(/\s+/gu, " ").trim(); + return squashed.length <= limit ? squashed : `${squashed.slice(0, limit - 3)}...`; } function redactProvenanceDetail(value: string): string { From 5db0ca21088656d297a17eb4f733b6ce601c1459 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 12:07:28 -0700 Subject: [PATCH 06/11] fix(cli): preserve redacted diagnostic context Signed-off-by: Apurv Kumaria --- src/lib/security/redact.test.ts | 43 +++++++++++++++++++++++++++++++++ src/lib/security/redact.ts | 29 ++++++++++++++++++++-- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index a306fdf4f34..366ff30398a 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -229,6 +229,13 @@ describe("redactForLog", () => { const text = [ "Authorization: Basic opaque-basic-value", "Proxy-Authorization: Digest username=opaque-user, response=opaque-response", + "Authorization: Basic-Plus opaque-basic-plus", + "Authorization: Bearer+DPoP opaque-bearer-plus", + "Proxy-Authorization: Digest-v2 opaque-digest-v2", + "Authorization=Basic opaque-equals-auth", + "Proxy-Authorization=Digest opaque-equals-proxy", + "Cookie=session=opaque-equals-cookie", + "Set-Cookie=session=opaque-equals-set-cookie", "Cookie: session=opaque-cookie-value", "Set-Cookie: session=opaque-set-cookie-value; HttpOnly", 'headers={"Authorization":"Basic opaque-json-value"}', @@ -240,6 +247,13 @@ describe("redactForLog", () => { "opaque-basic-value", "opaque-user", "opaque-response", + "opaque-basic-plus", + "opaque-bearer-plus", + "opaque-digest-v2", + "opaque-equals-auth", + "opaque-equals-proxy", + "opaque-equals-cookie", + "opaque-equals-set-cookie", "opaque-cookie-value", "opaque-set-cookie-value", "opaque-json-value", @@ -249,6 +263,35 @@ describe("redactForLog", () => { expect(result).toContain("author: safe-author"); }); + it("preserves same-line diagnostics after Basic and Bearer credentials", () => { + expect(redactFull("Authorization: Bearer opaque-bearer-value request failed")).toBe( + "Authorization: Bearer request failed", + ); + }); + + it("redacts folded credential headers without consuming the next diagnostic line", () => { + for (const header of ["Authorization", "Proxy-Authorization", "Cookie", "Set-Cookie"]) { + const result = redactFull(`${header}:\r\n\topaque-folded-value\r\nnext diagnostic`); + expect(result).toBe(`${header}: \r\nnext diagnostic`); + } + expect(redactFull("Authorization:\ropaque-bare-cr\rnext diagnostic")).toBe( + "Authorization: \rnext diagnostic", + ); + }); + + it("fails closed for malformed quoted credential fields", () => { + for (const input of [ + '{"Authorization":"Basic opaque-unterminated', + '{"Cookie":"session=opaque-unterminated', + '{"Authorization": Basic opaque-unquoted}', + ]) { + expect(redactFull(input)).not.toContain("opaque-"); + } + expect(redactFull('{"Authorization":"Basic opaque-complete","status":"kept"}')).toBe( + '{"Authorization":"Basic ","status":"kept"}', + ); + }); + it("redacts known secret patterns inside otherwise safe strings", () => { const result = redactForLog({ message: "upstream returned Authorization: Bearer abcdefghijklmnop", diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index 86cf52a65ba..5bcc21f574a 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -188,9 +188,34 @@ const FULL_REDACT_PATTERNS: [RegExp, string][] = [ "$1$2'", ], [ - /(\b(?:authorization|proxy-authorization|cookie|set-cookie)\s*:\s*)((?:(?:basic|bearer|digest)\s+)?)[^\r\n]*/gi, - "$1$2", + /("(?:authorization|proxy-authorization|cookie|set-cookie)"[ \t]*[:=])(?![ \t]*"(?:\\.|[^"\\])*")[^\r\n]*/gi, + "$1 ", ], + [ + /('(?:authorization|proxy-authorization|cookie|set-cookie)'[ \t]*[:=])(?![ \t]*'(?:\\.|[^'\\])*')[^\r\n]*/gi, + "$1 ", + ], + [ + /(\b(?:authorization|proxy-authorization|cookie|set-cookie)[ \t]*[:=])[^\r\n]*\r(?!\n)[^\r\n]*/gi, + "$1 ", + ], + [ + /(\b(?:authorization|proxy-authorization|cookie|set-cookie)[ \t]*[:=])[^\r\n]*(?:\r?\n[ \t]+[^\r\n]*)+/gi, + "$1 ", + ], + [ + /(\b(?:authorization|proxy-authorization)[ \t]*[:=][ \t]*(?:basic|bearer)[ \t]+)\S+/gi, + "$1", + ], + [ + /(\b(?:authorization|proxy-authorization)[ \t]*[:=][ \t]*digest[ \t]+)[^\r\n]*/gi, + "$1", + ], + [ + /(\b(?:authorization|proxy-authorization)[ \t]*[:=])(?![ \t]*(?:basic|bearer|digest)(?:[ \t]|$))[ \t]*[^\r\n]*/gi, + "$1 ", + ], + [/(\b(?:cookie|set-cookie)[ \t]*[:=][ \t]*)[^\r\n]*/gi, "$1"], ...TOKEN_PREFIX_PATTERNS.map((p): [RegExp, string] => [ new RegExp(p.source, p.flags), "", From a3ab263da332eb1a4e96c50714a1cdd41b5cafb8 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Tue, 7 Jul 2026 12:20:00 -0700 Subject: [PATCH 07/11] fix(cli): constrain credential context labels Signed-off-by: Apurv Kumaria --- src/lib/cli/logger.test.ts | 9 +++++++++ src/lib/security/redact.test.ts | 28 ++++++++++++++++++++++++++++ src/lib/security/redact.ts | 8 +++++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts index 7b16a4ba82e..e195b24f411 100644 --- a/src/lib/cli/logger.test.ts +++ b/src/lib/cli/logger.test.ts @@ -85,6 +85,15 @@ describe("Logger", () => { expect(output()).toContain("visible debug"); }); + it("keeps diagnostic arguments after prose that mentions credentials", async () => { + const { log } = await freshLogger(); + log.setDebug(true); + log.debug("Failed to refresh token, retrying", { attempt: 3 }); + log.debugObject("Token refresh failed", { attempt: 4 }); + expect(output()).toContain('"attempt": 3'); + expect(output()).toContain('"attempt": 4'); + }); + it("quiet mode suppresses info and still shows warnings", async () => { const { log } = await freshLogger(); log.setQuiet(true); diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index 366ff30398a..b797369278a 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -212,16 +212,44 @@ describe("redactForLog", () => { "opaque-env-value", "NEMOCLAW_PROVIDER_KEY", "-opaque-leading-dash-value", + "token", + "opaque-token-label", + "API Key:", + "opaque-api-key-label", + "proxyAuth", + "opaque-proxy-auth-label", + "proxyAuth:", + "opaque-proxy-auth-colon-label", + "public key", + "safe-public-key", "author", "safe-author", + "Failed to refresh token, retrying", + { attempt: 3 }, + "Token refresh failed", + { attempt: 4 }, ]), ).toEqual([ "OPENAI_API_KEY", "", "NEMOCLAW_PROVIDER_KEY", "", + "token", + "", + "API Key:", + "", + "proxyAuth", + "", + "proxyAuth:", + "", + "public key", + "safe-public-key", "author", "safe-author", + "Failed to refresh token, retrying", + { attempt: 3 }, + "Token refresh failed", + { attempt: 4 }, ]); }); diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index 5bcc21f574a..3e83d99dc28 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -301,12 +301,18 @@ function credentialFlagKey(value: unknown): string | null { return flag && isSensitiveKey(flag[1]) ? flag[1] : null; } +const CREDENTIAL_CONTEXT_LABEL_PATTERN = + /^(?:tokens?|secrets?|passwords?|passphrases?|credentials?|auth|authorization|bearer|cookies?|set[ _-]*cookie|proxy[ _-]*(?:auth|authorization)|(?:api|access|refresh|client|bearer|auth|private|signing|session|bot|app|resolved)[ _-]*(?:tokens?|keys?|secrets?|passwords?))$/i; + function credentialContextKey(value: unknown): string | null { if (typeof value !== "string") return null; const flag = credentialFlagKey(value); if (flag) return flag; const candidate = value.trim().replace(/[:=]$/, "").trim(); - return candidate && isSensitiveKey(candidate) ? candidate : null; + return candidate && + (isCredentialField(candidate) || CREDENTIAL_CONTEXT_LABEL_PATTERN.test(candidate)) + ? candidate + : null; } /** Redact opaque values whose credential context is carried by the previous argument. */ From bff0386b7a7bf00a6aa2d6a8998d76885d43e7e9 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 8 Jul 2026 15:45:43 -0700 Subject: [PATCH 08/11] fix(logging): avoid redacting benign structured keys Signed-off-by: Apurv Kumaria --- src/lib/security/redact.test.ts | 9 +++++++++ src/lib/security/redact.ts | 21 +++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index 3d839dfc020..7c123d113d6 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -161,6 +161,11 @@ describe("redactForLog", () => { passRate: 0.9, passCount: 4, passThrough: "enabled", + tokenizer: "cl100k_base", + maxTokens: 1024, + secretary: "safe role", + credentialing: "complete", + passwordless: true, correlationMarker: "reply-correlation-marker-123", }; @@ -214,6 +219,7 @@ describe("redactForLog", () => { sessionKey: "opaque-session-key", setCookie: "session=opaque-set-cookie-secret", "API Key": "opaque-api-secret", + APIKey: "opaque-api-secret-with-acronym", headers: { "Proxy-Authorization": "Basic opaque-basic-secret", Cookie: "session=opaque-cookie-secret", @@ -221,6 +227,7 @@ describe("redactForLog", () => { secretValue: "opaque-secret-value", tokenValue: "opaque-token-value", passwordValue: "opaque-password-value", + credentials: "opaque-credentials-value", publicKey: "safe public key", PUBLIC_KEY: "safe uppercase public key", author: "safe author", @@ -234,6 +241,7 @@ describe("redactForLog", () => { sessionKey: "", setCookie: "", "API Key": "", + APIKey: "", headers: { "Proxy-Authorization": "", Cookie: "", @@ -241,6 +249,7 @@ describe("redactForLog", () => { secretValue: "", tokenValue: "", passwordValue: "", + credentials: "", publicKey: "safe public key", PUBLIC_KEY: "safe uppercase public key", author: "safe author", diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index db252f2a8a5..faa69e3508d 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -291,17 +291,30 @@ export function redactUrl(value: unknown): string | null { return `${parsed.url.toString()}${parsed.suffix}`; } +const SENSITIVE_KEY_WORDS: ReadonlySet = new Set([ + "auth", + "authorization", + "bearer", + "cookie", + "credential", + "credentials", + "password", + "secret", + "token", +]); + function isSensitiveKey(key: string): boolean { if (isCredentialField(key)) return true; - if (/(?:api[\s_-]?key|token|secret|password|credential|authorization|bearer)/i.test(key)) { - return true; - } const words = key + .replace(/([A-Z]+)([A-Z][a-z])/g, "$1 $2") .replace(/([a-z0-9])([A-Z])/g, "$1 $2") .toLowerCase() .split(/[^a-z0-9]+/) .filter(Boolean); - return words.includes("auth") || words.includes("cookie"); + return ( + words.some((word) => SENSITIVE_KEY_WORDS.has(word)) || + (words.includes("api") && words.includes("key")) + ); } function credentialFlagKey(value: unknown): string | null { From 844fcf09d4eed1135df4f7c147826ae62d121ad3 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Wed, 8 Jul 2026 16:02:40 -0700 Subject: [PATCH 09/11] fix(logging): redact combined API key names Signed-off-by: Apurv Kumaria --- src/lib/security/redact.test.ts | 4 ++++ src/lib/security/redact.ts | 1 + 2 files changed, 5 insertions(+) diff --git a/src/lib/security/redact.test.ts b/src/lib/security/redact.test.ts index 7c123d113d6..f815d02d200 100644 --- a/src/lib/security/redact.test.ts +++ b/src/lib/security/redact.test.ts @@ -220,6 +220,8 @@ describe("redactForLog", () => { setCookie: "session=opaque-set-cookie-secret", "API Key": "opaque-api-secret", APIKey: "opaque-api-secret-with-acronym", + apikey: "opaque-run-together-api-secret", + APIKEY: "opaque-uppercase-api-secret", headers: { "Proxy-Authorization": "Basic opaque-basic-secret", Cookie: "session=opaque-cookie-secret", @@ -242,6 +244,8 @@ describe("redactForLog", () => { setCookie: "", "API Key": "", APIKey: "", + apikey: "", + APIKEY: "", headers: { "Proxy-Authorization": "", Cookie: "", diff --git a/src/lib/security/redact.ts b/src/lib/security/redact.ts index faa69e3508d..1fd128b2d2f 100644 --- a/src/lib/security/redact.ts +++ b/src/lib/security/redact.ts @@ -292,6 +292,7 @@ export function redactUrl(value: unknown): string | null { } const SENSITIVE_KEY_WORDS: ReadonlySet = new Set([ + "apikey", "auth", "authorization", "bearer", From 5f39fc8b8c96610fe59be13c60cefb955d6f02ec Mon Sep 17 00:00:00 2001 From: sauravdev Date: Sat, 4 Jul 2026 16:10:24 +0530 Subject: [PATCH 10/11] feat(cli): add structured logging with --debug and NEMOCLAW_LOG_LEVEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces src/lib/cli/logger.ts — a singleton Logger class with four levels (error < warn < info < debug) written exclusively to stderr so stdout remains clean for --json consumers. - `NEMOCLAW_LOG_LEVEL=debug` env var sets level at process start - `--debug` flag (inherited by all commands via NemoClawCommand.baseFlags) activates debug level and ISO-8601 timestamp prefixes - `-q / --quiet` flag suppresses info; shows only warn+error - `NEMOCLAW_DEBUG=1` and `DEBUG=*nemoclaw*` also activate debug level NemoClawCommand.init() reads both flags and configures the singleton before any command's run() method executes, so subcommands see the correct level without needing to parse flags themselves. Adds unit tests covering all level transitions and env-var pickup. Gradual migration of existing console.log/console.error call sites to log.info/log.error can be done incrementally on separate branches. Co-Authored-By: Claude Sonnet 4.6 --- src/lib/cli/logger.test.ts | 94 ++++++++++++++++++++ src/lib/cli/logger.ts | 118 ++++++++++++++++++++++++++ src/lib/cli/nemoclaw-oclif-command.ts | 30 +++++++ 3 files changed, 242 insertions(+) create mode 100644 src/lib/cli/logger.test.ts create mode 100644 src/lib/cli/logger.ts diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts new file mode 100644 index 00000000000..01d2ad5697f --- /dev/null +++ b/src/lib/cli/logger.test.ts @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { LogLevel } from "./logger"; + +// Re-import logger fresh for each test to reset singleton state +async function freshLogger() { + vi.resetModules(); + const mod = await import("./logger"); + return mod; +} + +describe("Logger", () => { + let stderrSpy: ReturnType; + + beforeEach(() => { + stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + stderrSpy.mockClear(); + vi.unstubAllEnvs(); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + }); + + it("defaults to info level", async () => { + const { log } = await freshLogger(); + expect(log.level).toBe("info"); + }); + + it("reads NEMOCLAW_LOG_LEVEL from env", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "debug"); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it("suppresses debug messages at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debug("should not appear"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("shows debug messages after setDebug(true)", async () => { + const { log } = await freshLogger(); + log.setDebug(true); + log.debug("visible debug"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible debug")); + }); + + it("quiet mode suppresses info", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.info("suppressed info"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("quiet mode still shows warn", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.warn("visible warning"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible warning")); + }); + + it("error always shown", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.error("critical error"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("critical error")); + }); + + it("error suppressed below error level only for warn+info+debug", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.warn("should be suppressed"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("debugObject emits JSON at debug level", async () => { + const { log } = await freshLogger(); + log.setLevel("debug"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('"key"')); + }); + + it("debugObject suppressed at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/cli/logger.ts b/src/lib/cli/logger.ts new file mode 100644 index 00000000000..4bcb1e2397b --- /dev/null +++ b/src/lib/cli/logger.ts @@ -0,0 +1,118 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Centralized logger for NemoClaw CLI. + * + * Levels (lowest → highest verbosity): + * error < warn < info < debug + * + * Default level: info (errors, warnings, and info messages shown). + * Quiet mode: warn (only warnings and errors shown). + * Debug mode: debug (all messages shown with timestamps). + * + * Configure via: + * NEMOCLAW_LOG_LEVEL=debug nemoclaw ... + * nemoclaw ... --debug (shorthand for debug level) + * nemoclaw ... -q / --quiet (suppresses info, shows warn+error) + */ + +export type LogLevel = "error" | "warn" | "info" | "debug"; + +const LEVEL_RANK: Record = { + error: 0, + warn: 1, + info: 2, + debug: 3, +}; + +function resolveLevel(): LogLevel { + const env = process.env.NEMOCLAW_LOG_LEVEL?.toLowerCase(); + if (env === "error" || env === "warn" || env === "info" || env === "debug") return env; + if (process.env.NEMOCLAW_DEBUG === "1" || process.env.DEBUG?.includes("nemoclaw")) return "debug"; + return "info"; +} + +class Logger { + private _level: LogLevel; + private _quiet: boolean; + private _timestamps: boolean; + + constructor() { + this._level = resolveLevel(); + this._quiet = false; + this._timestamps = this._level === "debug"; + } + + get level(): LogLevel { + return this._level; + } + + setLevel(level: LogLevel): void { + this._level = level; + this._timestamps = level === "debug"; + } + + setQuiet(quiet: boolean): void { + this._quiet = quiet; + if (quiet && LEVEL_RANK[this._level] > LEVEL_RANK["warn"]) { + this._level = "warn"; + } + } + + setDebug(debug: boolean): void { + if (debug) this.setLevel("debug"); + } + + isDebug(): boolean { + return this._level === "debug"; + } + + isQuiet(): boolean { + return this._quiet; + } + + private shouldLog(level: LogLevel): boolean { + return LEVEL_RANK[level] <= LEVEL_RANK[this._level]; + } + + private prefix(level: LogLevel): string { + if (!this._timestamps) return ""; + const ts = new Date().toISOString(); + return `[${ts}] [${level.toUpperCase()}] `; + } + + error(message: string, ...args: unknown[]): void { + if (!this.shouldLog("error")) return; + const parts = [this.prefix("error") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + warn(message: string, ...args: unknown[]): void { + if (!this.shouldLog("warn")) return; + const parts = [this.prefix("warn") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + info(message: string, ...args: unknown[]): void { + if (!this.shouldLog("info")) return; + const parts = [this.prefix("info") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + debug(message: string, ...args: unknown[]): void { + if (!this.shouldLog("debug")) return; + const parts = [this.prefix("debug") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + /** Log a structured object at debug level. Redacts nothing — call only with safe data. */ + debugObject(label: string, obj: unknown): void { + if (!this.shouldLog("debug")) return; + const ts = this._timestamps ? `[${new Date().toISOString()}] [DEBUG] ` : ""; + process.stderr.write(`${ts}${label}: ${JSON.stringify(obj, null, 2)}\n`); + } +} + +/** Singleton logger shared across all NemoClaw modules. */ +export const log = new Logger(); diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index 031a979e77b..61c65131041 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,7 @@ import { Command, Flags } from "@oclif/core"; +import { log } from "./logger"; import { redactForLog } from "../security/redact"; export type CommandExitResult = { @@ -20,8 +21,37 @@ export type CommandExitResult = { export abstract class NemoClawCommand extends Command { static baseFlags = { help: Flags.help({ char: "h" }), + // Hidden logging flags. Universal visible flags would have to be + // documented in every command section of docs/reference/commands.mdx + // (cli-parity gate), so the documented interface is + // NEMOCLAW_LOG_LEVEL/NEMOCLAW_DEBUG; the flags remain as a convenience. + debug: Flags.boolean({ + description: "Enable debug output (equivalent to NEMOCLAW_LOG_LEVEL=debug)", + env: "NEMOCLAW_DEBUG", + default: false, + hidden: true, + exclusive: ["quiet"], + }), + quiet: Flags.boolean({ + description: "Suppress informational output; show only warnings and errors", + default: false, + hidden: true, + exclusive: ["debug"], + }), }; + async init(): Promise { + await super.init(); + // Configure logging from raw argv rather than this.parse(): an early + // parse would trigger oclif's default help on --help and preempt the + // custom help of passthrough commands (e.g. ` agent`). The flags + // are still declared in baseFlags so each command's own parse accepts + // them and enforces the exclusive constraint. Debug wins when both + // appear here; the command's parse rejects that combination anyway. + if (this.argv.includes("--quiet")) log.setQuiet(true); + if (this.argv.includes("--debug")) log.setDebug(true); + } + protected logJson(json: unknown): void { console.log(JSON.stringify(redactForLog(json), null, 2)); } From 7ff951f2e968127b9e9ccb979e7ba85fcc5fb5e4 Mon Sep 17 00:00:00 2001 From: sauravdev Date: Sat, 4 Jul 2026 16:10:24 +0530 Subject: [PATCH 11/11] feat(cli): add structured logging with --debug and NEMOCLAW_LOG_LEVEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces src/lib/cli/logger.ts — a singleton Logger class with four levels (error < warn < info < debug) written exclusively to stderr so stdout remains clean for --json consumers. - `NEMOCLAW_LOG_LEVEL=debug` env var sets level at process start - `--debug` flag (inherited by all commands via NemoClawCommand.baseFlags) activates debug level and ISO-8601 timestamp prefixes - `-q / --quiet` flag suppresses info; shows only warn+error - `NEMOCLAW_DEBUG=1` and `DEBUG=*nemoclaw*` also activate debug level NemoClawCommand.init() reads both flags and configures the singleton before any command's run() method executes, so subcommands see the correct level without needing to parse flags themselves. Adds unit tests covering all level transitions and env-var pickup. Gradual migration of existing console.log/console.error call sites to log.info/log.error can be done incrementally on separate branches. Co-Authored-By: Claude Sonnet 4.6 --- src/lib/cli/logger.test.ts | 94 ++++++++++++++++++++ src/lib/cli/logger.ts | 118 ++++++++++++++++++++++++++ src/lib/cli/nemoclaw-oclif-command.ts | 30 +++++++ 3 files changed, 242 insertions(+) create mode 100644 src/lib/cli/logger.test.ts create mode 100644 src/lib/cli/logger.ts diff --git a/src/lib/cli/logger.test.ts b/src/lib/cli/logger.test.ts new file mode 100644 index 00000000000..01d2ad5697f --- /dev/null +++ b/src/lib/cli/logger.test.ts @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { LogLevel } from "./logger"; + +// Re-import logger fresh for each test to reset singleton state +async function freshLogger() { + vi.resetModules(); + const mod = await import("./logger"); + return mod; +} + +describe("Logger", () => { + let stderrSpy: ReturnType; + + beforeEach(() => { + stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + stderrSpy.mockClear(); + vi.unstubAllEnvs(); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + }); + + it("defaults to info level", async () => { + const { log } = await freshLogger(); + expect(log.level).toBe("info"); + }); + + it("reads NEMOCLAW_LOG_LEVEL from env", async () => { + vi.stubEnv("NEMOCLAW_LOG_LEVEL", "debug"); + const { log } = await freshLogger(); + expect(log.level).toBe("debug"); + }); + + it("suppresses debug messages at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debug("should not appear"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("shows debug messages after setDebug(true)", async () => { + const { log } = await freshLogger(); + log.setDebug(true); + log.debug("visible debug"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible debug")); + }); + + it("quiet mode suppresses info", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.info("suppressed info"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("quiet mode still shows warn", async () => { + const { log } = await freshLogger(); + log.setQuiet(true); + log.warn("visible warning"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("visible warning")); + }); + + it("error always shown", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.error("critical error"); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining("critical error")); + }); + + it("error suppressed below error level only for warn+info+debug", async () => { + const { log } = await freshLogger(); + log.setLevel("error" as LogLevel); + log.warn("should be suppressed"); + expect(stderrSpy).not.toHaveBeenCalled(); + }); + + it("debugObject emits JSON at debug level", async () => { + const { log } = await freshLogger(); + log.setLevel("debug"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('"key"')); + }); + + it("debugObject suppressed at info level", async () => { + const { log } = await freshLogger(); + log.setLevel("info"); + log.debugObject("context", { key: "val" }); + expect(stderrSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/cli/logger.ts b/src/lib/cli/logger.ts new file mode 100644 index 00000000000..4bcb1e2397b --- /dev/null +++ b/src/lib/cli/logger.ts @@ -0,0 +1,118 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Centralized logger for NemoClaw CLI. + * + * Levels (lowest → highest verbosity): + * error < warn < info < debug + * + * Default level: info (errors, warnings, and info messages shown). + * Quiet mode: warn (only warnings and errors shown). + * Debug mode: debug (all messages shown with timestamps). + * + * Configure via: + * NEMOCLAW_LOG_LEVEL=debug nemoclaw ... + * nemoclaw ... --debug (shorthand for debug level) + * nemoclaw ... -q / --quiet (suppresses info, shows warn+error) + */ + +export type LogLevel = "error" | "warn" | "info" | "debug"; + +const LEVEL_RANK: Record = { + error: 0, + warn: 1, + info: 2, + debug: 3, +}; + +function resolveLevel(): LogLevel { + const env = process.env.NEMOCLAW_LOG_LEVEL?.toLowerCase(); + if (env === "error" || env === "warn" || env === "info" || env === "debug") return env; + if (process.env.NEMOCLAW_DEBUG === "1" || process.env.DEBUG?.includes("nemoclaw")) return "debug"; + return "info"; +} + +class Logger { + private _level: LogLevel; + private _quiet: boolean; + private _timestamps: boolean; + + constructor() { + this._level = resolveLevel(); + this._quiet = false; + this._timestamps = this._level === "debug"; + } + + get level(): LogLevel { + return this._level; + } + + setLevel(level: LogLevel): void { + this._level = level; + this._timestamps = level === "debug"; + } + + setQuiet(quiet: boolean): void { + this._quiet = quiet; + if (quiet && LEVEL_RANK[this._level] > LEVEL_RANK["warn"]) { + this._level = "warn"; + } + } + + setDebug(debug: boolean): void { + if (debug) this.setLevel("debug"); + } + + isDebug(): boolean { + return this._level === "debug"; + } + + isQuiet(): boolean { + return this._quiet; + } + + private shouldLog(level: LogLevel): boolean { + return LEVEL_RANK[level] <= LEVEL_RANK[this._level]; + } + + private prefix(level: LogLevel): string { + if (!this._timestamps) return ""; + const ts = new Date().toISOString(); + return `[${ts}] [${level.toUpperCase()}] `; + } + + error(message: string, ...args: unknown[]): void { + if (!this.shouldLog("error")) return; + const parts = [this.prefix("error") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + warn(message: string, ...args: unknown[]): void { + if (!this.shouldLog("warn")) return; + const parts = [this.prefix("warn") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + info(message: string, ...args: unknown[]): void { + if (!this.shouldLog("info")) return; + const parts = [this.prefix("info") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + debug(message: string, ...args: unknown[]): void { + if (!this.shouldLog("debug")) return; + const parts = [this.prefix("debug") + message, ...args.map(String)].join(" "); + process.stderr.write(parts + "\n"); + } + + /** Log a structured object at debug level. Redacts nothing — call only with safe data. */ + debugObject(label: string, obj: unknown): void { + if (!this.shouldLog("debug")) return; + const ts = this._timestamps ? `[${new Date().toISOString()}] [DEBUG] ` : ""; + process.stderr.write(`${ts}${label}: ${JSON.stringify(obj, null, 2)}\n`); + } +} + +/** Singleton logger shared across all NemoClaw modules. */ +export const log = new Logger(); diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index 031a979e77b..61c65131041 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,7 @@ import { Command, Flags } from "@oclif/core"; +import { log } from "./logger"; import { redactForLog } from "../security/redact"; export type CommandExitResult = { @@ -20,8 +21,37 @@ export type CommandExitResult = { export abstract class NemoClawCommand extends Command { static baseFlags = { help: Flags.help({ char: "h" }), + // Hidden logging flags. Universal visible flags would have to be + // documented in every command section of docs/reference/commands.mdx + // (cli-parity gate), so the documented interface is + // NEMOCLAW_LOG_LEVEL/NEMOCLAW_DEBUG; the flags remain as a convenience. + debug: Flags.boolean({ + description: "Enable debug output (equivalent to NEMOCLAW_LOG_LEVEL=debug)", + env: "NEMOCLAW_DEBUG", + default: false, + hidden: true, + exclusive: ["quiet"], + }), + quiet: Flags.boolean({ + description: "Suppress informational output; show only warnings and errors", + default: false, + hidden: true, + exclusive: ["debug"], + }), }; + async init(): Promise { + await super.init(); + // Configure logging from raw argv rather than this.parse(): an early + // parse would trigger oclif's default help on --help and preempt the + // custom help of passthrough commands (e.g. ` agent`). The flags + // are still declared in baseFlags so each command's own parse accepts + // them and enforces the exclusive constraint. Debug wins when both + // appear here; the command's parse rejects that combination anyway. + if (this.argv.includes("--quiet")) log.setQuiet(true); + if (this.argv.includes("--debug")) log.setDebug(true); + } + protected logJson(json: unknown): void { console.log(JSON.stringify(redactForLog(json), null, 2)); }