fix(browser): redact WebSocket headers from connection logs (#2901)

## Summary

- Redact WebSocket header values and URL credentials (userinfo and query
parameters) before logging Puppeteer connection options.
- Redact `--wsHeaders` / `--ws-headers` and connection endpoint
arguments in the daemon startup log, while passing the original
arguments to the spawned daemon.
- Keep the redaction helpers in `src/utils/logger.ts` rather than
expanding the `BrowserManager` public API.
- Verify the actual `ensureBrowser()` path sends unmodified credentials
to `puppeteer.connect`, alongside focused URL/daemon logging regression
tests.

## Scope

This is the logging-safety piece split out of #2764. It does not add
environment-variable transport or change `wsHeaders` precedence or CLI
serialization.

## Validation (Butcher; PR-pinned devtools-frontend revision)

- `npm run format` — passed
- `npm run build` — passed
- `node scripts/test.js --test-name-pattern='preserves
WebSocket|sensitive connection logging' tests/browser.test.ts
tests/utils/logger.test.ts` — passed (7 selected tests)
- `git diff --check` — passed

Updated in c5b9bdc.

---------

Signed-off-by: Robert McConnell <robert@mcc0nnell.org>

src/BrowserManager.ts

@@ -16,7 +16,11 @@ import type {
   LaunchOptions,
 } from './third_party/index.js';
 import {Mutex, puppeteer} from './third_party/index.js';
-import {logger, puppeteerLogger} from './utils/logger.js';
+import {
+  logger,
+  puppeteerLogger,
+  redactConnectOptionsForLogging,
+} from './utils/logger.js';
 import {isAllowedUrl} from './utils/url.js';
 
 export interface BrowserManagerOptions {
@@ -410,7 +414,10 @@ export class BrowserManager {
       );
     }
 
-    logger?.('Connecting Puppeteer to ', JSON.stringify(connectOptions));
+    logger?.(
+      'Connecting Puppeteer to ',
+      JSON.stringify(redactConnectOptionsForLogging(connectOptions)),
+    );
     try {
       const connected = await puppeteer.connect(connectOptions);
       logger?.('Connected Puppeteer');

src/daemon/client.ts

@@ -12,7 +12,11 @@ import path from 'node:path';
 import type {CallToolResult} from '../third_party/index.js';
 import {PipeTransport} from '../third_party/index.js';
 import {getTempFilePath} from '../utils/files.js';
-import {logger, puppeteerLogger} from '../utils/logger.js';
+import {
+  logger,
+  puppeteerLogger,
+  redactDaemonArgsForLogging,
+} from '../utils/logger.js';
 
 import type {
   DaemonMessage,
@@ -133,7 +137,7 @@ export async function startDaemon(
     fs.unlinkSync(pidFilePath);
   }
 
-  logger?.('Starting daemon...', ...mcpArgs);
+  logger?.('Starting daemon...', ...redactDaemonArgsForLogging(mcpArgs));
   const child = spawn(process.execPath, [DAEMON_SCRIPT_PATH, ...mcpArgs], {
     detached: true,
     stdio: 'ignore',

src/utils/logger.ts

@@ -52,6 +52,85 @@ export function escapeForLog(value: string): string {
   );
 }
 
+/**
+ * Redact credentials from URLs used in connection and diagnostic logs.
+ * The URL passed to Puppeteer is never modified.
+ */
+export function redactUrlForLogging(value: string): string {
+  try {
+    const url = new URL(value);
+    url.username = '';
+    url.password = '';
+    if (url.search) {
+      url.search = '?REDACTED';
+    }
+    return url.toString();
+  } catch {
+    // Unparseable URLs might still contain credentials. Fail closed.
+    return '[REDACTED]';
+  }
+}
+
+/**
+ * Return a redacted copy of the Puppeteer connection options for logging.
+ * Keep the actual headers and connection URLs intact for Puppeteer.
+ */
+export function redactConnectOptionsForLogging<
+  T extends {
+    headers?: Record<string, string>;
+    browserWSEndpoint?: string;
+    browserURL?: string;
+  },
+>(options: T): T {
+  return {
+    ...options,
+    ...(options.headers
+      ? {
+          headers: Object.fromEntries(
+            Object.keys(options.headers).map(name => [name, '[REDACTED]']),
+          ),
+        }
+      : {}),
+    ...(options.browserWSEndpoint
+      ? {browserWSEndpoint: redactUrlForLogging(options.browserWSEndpoint)}
+      : {}),
+    ...(options.browserURL
+      ? {browserURL: redactUrlForLogging(options.browserURL)}
+      : {}),
+  };
+}
+
+/**
+ * Filter sensitive CLI arguments in the daemon startup log only.
+ * Spawn still receives the original arguments unchanged.
+ */
+export function redactDaemonArgsForLogging(args: readonly string[]): string[] {
+  const headersFlag = /^--ws-?headers$/i;
+  const endpointFlag = /^--(?:ws-?endpoint|browser-?url)$/i;
+  return args.map((arg, index) => {
+    const previous = args[index - 1] ?? '';
+    if (headersFlag.test(previous)) {
+      return '[REDACTED]';
+    }
+    if (endpointFlag.test(previous)) {
+      return redactUrlForLogging(arg);
+    }
+    const equalsAt = arg.indexOf('=');
+    if (equalsAt === -1) {
+      return arg;
+    }
+    const flag = arg.slice(0, equalsAt);
+    const value = arg.slice(equalsAt + 1);
+    if (headersFlag.test(flag)) {
+      return `${flag}=[REDACTED]`;
+    }
+    if (endpointFlag.test(flag)) {
+      return `${flag}=${redactUrlForLogging(value)}`;
+    }
+    return arg;
+  });
+}
+
 export const logger: Logger = (...args: unknown[]) => {
   if (logFileStream) {
     logFileStream.write(

tests/browser.test.ts

@@ -67,6 +67,34 @@ describe('browser', () => {
   });
 
   describe('BrowserManager', () => {
+    it('preserves WebSocket connection credentials for Puppeteer while redacting logs', async () => {
+      const pptrBrowser = createMockPuppeteerBrowser();
+      const connectStub = sinon
+        .stub(puppeteer, 'connect')
+        .resolves(pptrBrowser);
+      const endpoint =
+        'wss://user:password@browser.example/devtools/browser/test?token=WS_SECRET_123';
+      const headers = {
+        Authorization: 'Bearer HEADER_SECRET_456',
+        'X-Test': 'visible',
+      };
+      const manager = new BrowserManager(
+        createMockParsedArguments({wsEndpoint: endpoint, wsHeaders: headers}),
+      );
+
+      await manager.ensureBrowser();
+
+      sinon.assert.calledOnceWithMatch(connectStub, {
+        browserWSEndpoint: endpoint,
+        wsOptions: {headers},
+      });
+      assert.deepStrictEqual(
+        connectStub.firstCall.args[0].wsOptions?.headers,
+        headers,
+      );
+      await manager.close();
+    });
+
     it('launches a browser when no connect options are set and closes it on close()', async () => {
       const pptrBrowser = createMockPuppeteerBrowser();
       const launchStub = sinon.stub(puppeteer, 'launch').resolves(pptrBrowser);

tests/utils/logger.test.ts

@@ -11,6 +11,9 @@ import util from 'node:util';
 
 import {
   escapeForLog,
+  redactConnectOptionsForLogging,
+  redactDaemonArgsForLogging,
+  redactUrlForLogging,
   puppeteerLogger,
   saveLogsToFile,
 } from '../../src/utils/logger.js';
@@ -93,3 +96,71 @@ describe('escapeForLog', () => {
     }
   });
 });
+
+describe('sensitive connection logging', () => {
+  it('removes URL userinfo and query parameters, failing closed on invalid URLs', () => {
+    assert.strictEqual(
+      redactUrlForLogging('wss://user:pass@example.test/browser?token=SECRET'),
+      'wss://example.test/browser?REDACTED',
+    );
+    assert.strictEqual(
+      redactUrlForLogging('http://example.test/json/version?key=SECRET'),
+      'http://example.test/json/version?REDACTED',
+    );
+    assert.strictEqual(redactUrlForLogging('not a URL:SECRET'), '[REDACTED]');
+  });
+
+  it('redacts URLs and headers without changing the connection options', () => {
+    const options = {
+      browserWSEndpoint: 'wss://admin:password@example.test/ws?key=URL_SECRET',
+      browserURL: 'http://example.test/json?token=OTHER_SECRET',
+      headers: {
+        Authorization: 'Bearer HEADER_SECRET',
+        'X-Token': 'ALSO_SECRET',
+      },
+    };
+    const sanitized = redactConnectOptionsForLogging(options);
+    assert.deepStrictEqual(sanitized.headers, {
+      Authorization: '[REDACTED]',
+      'X-Token': '[REDACTED]',
+    });
+    for (const secret of [
+      'password',
+      'URL_SECRET',
+      'OTHER_SECRET',
+      'HEADER_SECRET',
+      'ALSO_SECRET',
+    ]) {
+      assert.ok(!JSON.stringify(sanitized).includes(secret));
+    }
+    assert.strictEqual(options.headers.Authorization, 'Bearer HEADER_SECRET');
+    assert.match(options.browserWSEndpoint, /URL_SECRET/);
+  });
+
+  it('redacts daemon startup flags without modifying the arguments passed to spawn', () => {
+    const args = [
+      '--wsHeaders',
+      '{"Authorization":"Bearer HEADER_SECRET"}',
+      '--wsEndpoint=wss://user:password@example.test/ws?token=URL_SECRET',
+      '--browserUrl',
+      'http://example.test?key=OTHER_SECRET',
+      '--logFile',
+      '/tmp/example.log',
+      '--ws-headers={"X-Api-Key":"DASH_SECRET"}',
+    ];
+    const sanitized = redactDaemonArgsForLogging(args);
+    assert.deepStrictEqual(sanitized, [
+      '--wsHeaders',
+      '[REDACTED]',
+      '--wsEndpoint=wss://example.test/ws?REDACTED',
+      '--browserUrl',
+      'http://example.test/?REDACTED',
+      '--logFile',
+      '/tmp/example.log',
+      '--ws-headers=[REDACTED]',
+    ]);
+    assert.match(args[1], /HEADER_SECRET/);
+    assert.ok(!JSON.stringify(sanitized).includes('SECRET'));
+    assert.ok(!JSON.stringify(sanitized).includes('password'));
+  });
+});