From 2ffebef0221e3a45dd3422f22f26fbcb3bfc0fb9 Mon Sep 17 00:00:00 2001
From: gsxdsm <gsxdsm@users.noreply.github.com>
Date: Sun, 12 Jul 2026 14:28:00 -0700
Subject: [PATCH] Address PR feedback: redact TUI/console log path too (#2028)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../dashboard-tui/__tests__/log-sink.test.ts  | 48 +++++++++++++++++++
 .../src/commands/dashboard-tui/log-sink.ts    | 13 +++--
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts b/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts
index e4c1e805ea..2df38bb4ae 100644
--- a/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts
+++ b/packages/cli/src/commands/dashboard-tui/__tests__/log-sink.test.ts
@@ -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]");
+  });
 });
diff --git a/packages/cli/src/commands/dashboard-tui/log-sink.ts b/packages/cli/src/commands/dashboard-tui/log-sink.ts
index d82bb5d1ff..67b32da06e 100644
--- a/packages/cli/src/commands/dashboard-tui/log-sink.ts
+++ b/packages/cli/src/commands/dashboard-tui/log-sink.ts
@@ -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) {