From ad97b46407744076b9b4ef29710ffef8ff04384f Mon Sep 17 00:00:00 2001 From: Jason Ma Date: Thu, 28 May 2026 10:04:38 +0800 Subject: [PATCH 1/2] fix(cli): add sudo hint to debug dmesg-restricted message (#4366) When `kernel.dmesg_restrict=1` and the user runs `nemoclaw debug` as non-root, the Kernel Messages section explained why the section was skipped but did not tell the user how to include kernel logs anyway. Extend `dmesgRestrictedMessage` to append a "Re-run with `sudo nemoclaw debug`" hint so users and triagers see a concrete next step, matching the spec in the bug's Suggested Fix. The TTY-aware `sudo -n dmesg` fallback from the same Suggested Fix is intentionally out of scope for this minimum-viable change. Export `dmesgRestrictedMessage` so the wording can be pinned by a unit test. Fixes #4366. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jason Ma --- src/lib/diagnostics/debug.test.ts | 15 +++++++++++++++ src/lib/diagnostics/debug.ts | 7 +++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/lib/diagnostics/debug.test.ts b/src/lib/diagnostics/debug.test.ts index 6900a97ec6..053c1547b1 100644 --- a/src/lib/diagnostics/debug.test.ts +++ b/src/lib/diagnostics/debug.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; // Import from compiled dist/ so coverage is attributed correctly. import { createTarball, + dmesgRestrictedMessage, getDebugCompletionMessages, isDmesgPermissionDeniedOutput, isDmesgRestrictedForCurrentUser, @@ -147,3 +148,17 @@ describe("isDmesgPermissionDeniedOutput", () => { expect(isDmesgPermissionDeniedOutput("docker: Permission denied")).toBe(false); }); }); + +describe("dmesgRestrictedMessage (#4366)", () => { + it("explains why kernel messages were skipped", () => { + const msg = dmesgRestrictedMessage("kernel.dmesg_restrict=1 prevents non-root access"); + expect(msg).toContain("kernel messages skipped"); + expect(msg).toContain("kernel.dmesg_restrict=1 prevents non-root access"); + }); + + it("includes a 'sudo nemoclaw debug' hint so users can re-run with kernel logs", () => { + const msg = dmesgRestrictedMessage("some-reason"); + expect(msg).toMatch(/sudo nemoclaw debug/); + expect(msg.toLowerCase()).toMatch(/re-?run/); + }); +}); diff --git a/src/lib/diagnostics/debug.ts b/src/lib/diagnostics/debug.ts index cde7ea9a8a..0561e244b0 100644 --- a/src/lib/diagnostics/debug.ts +++ b/src/lib/diagnostics/debug.ts @@ -154,8 +154,11 @@ export function isDmesgPermissionDeniedOutput(output: string): boolean { return /\b(dmesg|kernel buffer|kernel logs?)\b/i.test(output); } -function dmesgRestrictedMessage(reason: string): string { - return ` (kernel messages skipped: dmesg access is restricted for this user; ${reason})`; +export function dmesgRestrictedMessage(reason: string): string { + return [ + ` (kernel messages skipped: dmesg access is restricted for this user; ${reason}.`, + " Re-run with `sudo nemoclaw debug` to include kernel logs in this report.)", + ].join("\n"); } function collectDmesg(collectDir: string): void { From b56f12aaebbe68dd41b47b9311458dfc5c0728ce Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Fri, 29 May 2026 15:09:48 -0400 Subject: [PATCH 2/2] fix(cli): preserve debug options in dmesg-restricted sudo hint (#4366) Address PR review advisor feedback: the previous hint hardcoded `sudo nemoclaw debug` regardless of the user's invocation, which would nudge a user who ran `debug --quick` into the broader privileged collector. - Make `dmesgRestrictedMessage` accept `DebugOptions` and build the rerun command via new `buildDmesgRerunCommand` helper that preserves `--quick` and `--output` (with shell-safe single-quoting). - Plumb `opts` through `collectKernelMessages` -> `collectDmesg`. - Add a sensitive-data caution to the recovery message so users review privileged diagnostics before sharing. - Add option-aware regression tests covering the rerun-command builder and message wording. Refs: #4366 Signed-off-by: Julie Yaunches --- src/lib/diagnostics/debug.test.ts | 51 +++++++++++++++++++++++++++++++ src/lib/diagnostics/debug.ts | 35 ++++++++++++++++----- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/lib/diagnostics/debug.test.ts b/src/lib/diagnostics/debug.test.ts index 053c1547b1..273fca77f1 100644 --- a/src/lib/diagnostics/debug.test.ts +++ b/src/lib/diagnostics/debug.test.ts @@ -7,6 +7,7 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; // Import from compiled dist/ so coverage is attributed correctly. import { + buildDmesgRerunCommand, createTarball, dmesgRestrictedMessage, getDebugCompletionMessages, @@ -161,4 +162,54 @@ describe("dmesgRestrictedMessage (#4366)", () => { expect(msg).toMatch(/sudo nemoclaw debug/); expect(msg.toLowerCase()).toMatch(/re-?run/); }); + + it("warns that privileged diagnostics may contain sensitive data", () => { + const msg = dmesgRestrictedMessage("some-reason"); + expect(msg.toLowerCase()).toMatch(/sensitive/); + }); + + it("preserves --quick in the rerun hint when the user invoked debug --quick", () => { + const msg = dmesgRestrictedMessage("some-reason", { quick: true }); + expect(msg).toContain("sudo nemoclaw debug --quick"); + }); + + it("preserves --output in the rerun hint when the user supplied an output path", () => { + const msg = dmesgRestrictedMessage("some-reason", { output: "/tmp/out.tgz" }); + expect(msg).toContain("sudo nemoclaw debug --output '/tmp/out.tgz'"); + }); + + it("preserves both --quick and --output together", () => { + const msg = dmesgRestrictedMessage("some-reason", { + quick: true, + output: "/tmp/out.tgz", + }); + expect(msg).toContain("sudo nemoclaw debug --quick --output '/tmp/out.tgz'"); + }); + + it("falls back to bare 'sudo nemoclaw debug' when no options are supplied", () => { + const msg = dmesgRestrictedMessage("some-reason"); + expect(msg).toMatch(/`sudo nemoclaw debug`/); + }); +}); + +describe("buildDmesgRerunCommand (#4366)", () => { + it("returns the bare command when no options are set", () => { + expect(buildDmesgRerunCommand()).toBe("sudo nemoclaw debug"); + }); + + it("appends --quick when opts.quick is true", () => { + expect(buildDmesgRerunCommand({ quick: true })).toBe("sudo nemoclaw debug --quick"); + }); + + it("appends a single-quoted --output path", () => { + expect(buildDmesgRerunCommand({ output: "/tmp/out.tgz" })).toBe( + "sudo nemoclaw debug --output '/tmp/out.tgz'", + ); + }); + + it("escapes single quotes inside the output path", () => { + expect(buildDmesgRerunCommand({ output: "/tmp/o'ut.tgz" })).toBe( + "sudo nemoclaw debug --output '/tmp/o'\\''ut.tgz'", + ); + }); }); diff --git a/src/lib/diagnostics/debug.ts b/src/lib/diagnostics/debug.ts index 0561e244b0..c45485e30b 100644 --- a/src/lib/diagnostics/debug.ts +++ b/src/lib/diagnostics/debug.ts @@ -154,14 +154,34 @@ export function isDmesgPermissionDeniedOutput(output: string): boolean { return /\b(dmesg|kernel buffer|kernel logs?)\b/i.test(output); } -export function dmesgRestrictedMessage(reason: string): string { +/** + * Build the option-aware re-run command for the dmesg-restricted hint. + * + * Preserves the user's original invocation flags (`--quick`, `--output`) so the + * hint nudges them back into the same scoped diagnostic instead of a broader + * privileged collector. See issue #4366. + */ +export function buildDmesgRerunCommand(opts: DebugOptions = {}): string { + const parts = ["sudo", "nemoclaw", "debug"]; + if (opts.quick) parts.push("--quick"); + if (opts.output) { + // Single-quote the path and escape embedded single quotes for shell safety. + const escaped = opts.output.replace(/'/g, "'\\''"); + parts.push("--output", `'${escaped}'`); + } + return parts.join(" "); +} + +export function dmesgRestrictedMessage(reason: string, opts: DebugOptions = {}): string { + const rerun = buildDmesgRerunCommand(opts); return [ ` (kernel messages skipped: dmesg access is restricted for this user; ${reason}.`, - " Re-run with `sudo nemoclaw debug` to include kernel logs in this report.)", + ` Re-run with \`${rerun}\` to include kernel logs in this report.`, + " Note: privileged diagnostics and kernel logs may contain sensitive data; review before sharing.)", ].join("\n"); } -function collectDmesg(collectDir: string): void { +function collectDmesg(collectDir: string, opts: DebugOptions = {}): void { if (!commandExists("dmesg")) { writeCollectedMessage(collectDir, "dmesg", " (dmesg not found, skipping)"); return; @@ -173,6 +193,7 @@ function collectDmesg(collectDir: string): void { "dmesg", dmesgRestrictedMessage( `${DMESG_RESTRICT_PATH}=1 prevents non-root users from reading kernel logs`, + opts, ), ); return; @@ -189,7 +210,7 @@ function collectDmesg(collectDir: string): void { writeCollectedMessage( collectDir, "dmesg", - dmesgRestrictedMessage("the dmesg command denied access to kernel logs"), + dmesgRestrictedMessage("the dmesg command denied access to kernel logs", opts), ); return; } @@ -489,7 +510,7 @@ function collectKernel(collectDir: string): void { } } -function collectKernelMessages(collectDir: string): void { +function collectKernelMessages(collectDir: string, opts: DebugOptions = {}): void { section("Kernel Messages"); if (isMacOS) { collectShell( @@ -498,7 +519,7 @@ function collectKernelMessages(collectDir: string): void { 'log show --last 5m --predicate "eventType == logEvent" --style compact 2>/dev/null | tail -100', ); } else { - collectDmesg(collectDir); + collectDmesg(collectDir, opts); } } @@ -590,7 +611,7 @@ export function runDebug(opts: DebugOptions = {}): void { collectKernel(collectDir); } - collectKernelMessages(collectDir); + collectKernelMessages(collectDir, opts); let tarballOk = true; if (output) {