Address PR feedback: redact TUI/console log path too (#2028)

Move redactSecrets to the log/warn/error entry points so both the
recorded history (served over /system/logs) and the TUI/console output
are masked — previously only the stored entry was redacted while the raw
message still printed to the terminal. Add assertions for both the
console and TUI-target output paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-12 14:28:00 -07:00
parent f1b6a6340c
commit 2ffebef022
2 changed files with 56 additions and 5 deletions

View File

@@ -366,4 +366,52 @@ describe("DashboardLogSink system-log history", () => {
expect(stored).not.toContain("sk-abcdef0123456789abcdef");
expect(seen[0]).toBe(stored);
});
it("redacts secrets on the TUI/console output path too", () => {
const secret = "sk-abcdef0123456789abcdef";
// Non-TTY sink → forwards to console.*; assert the raw secret never prints.
const consoleSpies = {
log: vi.spyOn(console, "log").mockImplementation(() => {}),
warn: vi.spyOn(console, "warn").mockImplementation(() => {}),
error: vi.spyOn(console, "error").mockImplementation(() => {}),
};
try {
const sink = new DashboardLogSink();
sink.log(`token=${secret}`);
sink.warn(`token=${secret}`, "engine");
sink.error(`Authorization: Bearer ${secret}`);
const allPrinted = [
...consoleSpies.log.mock.calls,
...consoleSpies.warn.mock.calls,
...consoleSpies.error.mock.calls,
]
.flat()
.join(" ");
expect(allPrinted).not.toContain(secret);
expect(allPrinted).toContain("[REDACTED]");
} finally {
consoleSpies.log.mockRestore();
consoleSpies.warn.mockRestore();
consoleSpies.error.mockRestore();
}
});
it("redacts secrets forwarded to the TUI target", () => {
const secret = "ghp_abcdef0123456789ABCDEF";
const lines: string[] = [];
const tuiTarget = {
running: true,
log: (m: string) => lines.push(m),
warn: (m: string) => lines.push(m),
error: (m: string) => lines.push(m),
};
const sink = new DashboardLogSink(tuiTarget);
sink.log(`leaked ${secret}`);
sink.error(`boom ${secret}`, "engine");
const joined = lines.join(" ");
expect(joined).not.toContain(secret);
expect(joined).toContain("[REDACTED]");
});
});

View File

@@ -119,11 +119,8 @@ export class DashboardLogSink {
}
private record(level: LogEntry["level"], message: string, prefix?: string): void {
// Redact before storing/broadcasting: the System panel serves this history
// over /system/logs + /system/logs/stream and into diagnostics/bug reports,
// so any secret that reaches a log line would otherwise be resurfaceable to
// a dashboard client. Mask before it enters the ring buffer or listeners.
const entry: LogEntry = { timestamp: new Date(), level, message: redactSecrets(message), prefix };
// `message` is already redacted by the public log/warn/error entry points.
const entry: LogEntry = { timestamp: new Date(), level, message, prefix };
this.history.push(entry);
for (const listener of this.entryListeners) {
try {
@@ -150,6 +147,10 @@ export class DashboardLogSink {
log(message: string, prefix?: string): void {
if (this.silenced) return;
// Redact once at the entry point so BOTH the recorded history (served over
// /system/logs) and the TUI/console output are masked — a secret must not
// leak on either path.
message = redactSecrets(message);
this.record("info", message, prefix);
const line = prefix ? `[${prefix}] ${message}` : message;
if (this.tui && this.isTTY) {
@@ -163,6 +164,7 @@ export class DashboardLogSink {
warn(message: string, prefix?: string): void {
if (this.silenced) return;
message = redactSecrets(message);
this.record("warn", message, prefix);
const line = prefix ? `[${prefix}] ${message}` : message;
if (this.tui && this.isTTY) {
@@ -176,6 +178,7 @@ export class DashboardLogSink {
error(message: string, prefix?: string): void {
if (this.silenced) return;
message = redactSecrets(message);
this.record("error", message, prefix);
const line = prefix ? `[${prefix}] ${message}` : message;
if (this.tui && this.isTTY) {