fix(browse): capture daemon stdout/stderr to browse-daemon.log + Windows polyfill spawn fixes (re-derived from #2461)

The detached daemon's stdout/stderr were wired to 'ignore' on every
platform, so every console.error('[browse] FATAL: ...') from a Chromium
crash, uncaughtException, or unhandledRejection was discarded at the OS
level — a crash-and-respawn looked identical to every other dropped
session, with nothing on disk recording why. Both spawn paths now redirect
to <stateDir>/browse-daemon.log (append mode, accumulates across respawns):
the Unix path via an fd from openDaemonLogSink(), the Windows path by
opening the fd INSIDE the node -e launcher string (an fd opened in cli.ts
would not cross the spawn boundary). Unwritable state dir falls back to
'ignore' rather than failing the launch.

Capturing daemon output is what surfaced the PR's second fix, still valid
on current main: bun-polyfill.cjs's Bun.spawn/spawnSync called Node's
child_process with a bare command name, which Windows can't resolve without
PATHEXT lookup ("spawn bun ENOENT" from the terminal-agent respawn path).
Routed through cross-spawn on win32 (now a direct dependency; already in
the tree transitively via @modelcontextprotocol/sdk) — the PR verified
empirically that shell:true does NOT neutralize cmd.exe metacharacters
reachable via `$B skill run` arg passthrough, and that Node refuses .cmd
spawns without a shell (CVE-2024-27980), so cross-spawn's combined PATHEXT
resolution + argument escaping is the only correct shape. The PR's third
fix (resolveDisconnectCause throwing "browser?.process is not a function")
already landed on main via the #2085 typeof guard — not re-applied.

F6 log hygiene (daemon-log-hygiene.test.ts): needle tests pin the log
wiring on both spawn paths (and that stdio 'ignore','ignore','ignore'
never returns), that bun-polyfill stays on cross-spawn with no shell:true,
that NO console.* call in src/ passes a token value (interpolated or bare
arg), and that the page-content carrier modules (tab-session, buffers,
content-security, activity) stay console-free — so neither AUTH_TOKEN nor
unsanitized page-derived strings can reach browse-daemon.log.

Tests: daemon-log-hygiene + bun-polyfill + windows-spawn-hide +
cli-setsid-daemonize 21 pass; stop-dead-daemon + busy-daemon-iron-rule
(exercises a REAL daemon boot through the new log-fd wiring) 10 pass.

Re-derived from PR #2461 by @phuttimatebenchanakatkul.

Co-authored-by: phuttimatebenchanakatkul <phuttimatebenchanakatkul@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

browse/src/bun-polyfill.cjs

@@ -11,7 +11,43 @@
 'use strict';
 
 const http = require('http');
-const { spawnSync, spawn } = require('child_process');
+const { spawnSync: nodeSpawnSync, spawn: nodeSpawn } = require('child_process');
+// Node's spawn on Windows without shell:true only matches an EXACT
+// executable name — no PATHEXT resolution the way a real shell (or
+// Bun.spawn, which this file exists to polyfill) does. A bare command
+// name like 'bun' (no .exe/.cmd) then fails ENOENT even though `bun`
+// works fine typed at a prompt (confirmed live in #2461: this is what
+// produced "[browse] FATAL uncaught exception: spawn bun ENOENT" from
+// terminal-agent-control.ts's respawn path, once daemon output was
+// actually being captured to a file instead of silently discarded).
+//
+// Two things this is NOT fixed with, both tried and rejected in #2461:
+//
+// 1. shell:true + array args. This file is also reached (via server.ts →
+//    write-commands.ts/meta-commands.ts → cookie-import-browser.ts/
+//    browser-skill-commands.ts) by calls that pass genuinely variable
+//    content — browser-skill-commands.ts spreads `...opts.skillArgs`,
+//    sourced from `$B skill run <name> --arg k=v`'s passthrough CLI args,
+//    into the spawned argv. shell:true on Windows routes through cmd.exe,
+//    and Node's own array-arg handling for that combination does NOT
+//    neutralize cmd.exe metacharacters (& | ^ % < >) — verified in #2461 by
+//    directly spawning a resolved .cmd path with an arg containing
+//    `& echo INJECTED > proof.txt`: the file was created. Hand-rolled
+//    double-quote-only escaping doesn't close that either.
+//
+// 2. Resolve the .exe/.cmd path ourselves and spawn it with NO shell.
+//    Works for .exe targets, but Node refuses (EINVAL) to spawn a
+//    .cmd/.bat file without shell:true — deliberately, as part of Node's
+//    CVE-2024-27980 fix for implicit unsafe .cmd execution. bun's own
+//    Windows install (npm global) is exactly a .cmd shim, so this path is
+//    not optional to support.
+//
+// cross-spawn (previously a transitive dep, now direct) is the established
+// library for precisely this problem: PATHEXT resolution AND correct
+// Windows/cmd.exe argument escaping together. #2461 verified the injection
+// payload above reaches the child as a single literal argument while
+// normal resolution (`bun --version`) still works.
+const crossSpawn = require('cross-spawn');
 
 globalThis.Bun = {
   serve(options) {
@@ -66,7 +102,8 @@ globalThis.Bun = {
 
   spawnSync(cmd, options = {}) {
     const [command, ...args] = cmd;
-    const result = spawnSync(command, args, {
+    const spawnSyncFn = process.platform === 'win32' ? crossSpawn.sync : nodeSpawnSync;
+    const result = spawnSyncFn(command, args, {
       stdio: [
         options.stdin || 'pipe',
         options.stdout === 'pipe' ? 'pipe' : 'ignore',
@@ -92,7 +129,8 @@ globalThis.Bun = {
   spawn(cmd, options = {}) {
     const [command, ...args] = cmd;
     const stdio = options.stdio || ['pipe', 'pipe', 'pipe'];
-    const proc = spawn(command, args, {
+    const spawnFn = process.platform === 'win32' ? crossSpawn : nodeSpawn;
+    const proc = spawnFn(command, args, {
       stdio,
       env: options.env,
       cwd: options.cwd,

browse/src/cli.ts

@@ -365,6 +365,29 @@ function raiseHeadedWindowMacOS(): void {
 }
 
 // ─── Server Lifecycle ──────────────────────────────────────────
+// The detached daemon's stdout/stderr used to be wired to 'ignore' on every
+// platform, so console.error('[browse] FATAL: ...') from a Chromium crash,
+// an uncaughtException, or an unhandledRejection (see server.ts's handlers
+// and browser-manager.ts's handleChromiumDisconnect) went nowhere — not to
+// a file, not to the terminal, discarded at the OS level (#2461). That made
+// a crash-and-respawn indistinguishable from any other cause of a dropped
+// session: nothing on disk ever recorded WHY. Redirect both streams to
+// <stateDir>/browse-daemon.log — append mode, so it accumulates across the
+// daemon's full lifetime and every respawn stays visible in one place.
+//
+// F6 log hygiene: nothing that reaches the daemon's stdout/stderr may carry
+// an auth token or unsanitized page-derived strings —
+// browse/test/daemon-log-hygiene.test.ts pins this with needle tests.
+function openDaemonLogSink(): number | 'ignore' {
+  try {
+    return fs.openSync(path.join(config.stateDir, 'browse-daemon.log'), 'a');
+  } catch {
+    // stateDir not writable (permissions, disk full) — fall back to the
+    // previous behavior rather than fail the whole launch over logging.
+    return 'ignore';
+  }
+}
+
 async function startServer(extraEnv?: Record<string, string>): Promise<ServerState> {
   ensureStateDir(config);
 
@@ -393,10 +416,18 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
     // with { detached: true } instead, which is the gold standard for Windows
     // process independence. Credit: PR #191 by @fqueiro.
     const extraEnvStr = JSON.stringify({ BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...(extraEnv || {}) });
+    // The daemon's real process is spawned inside the launcher's own
+    // `node -e` invocation, not in cli.ts's process — so the log file has
+    // to be opened from inside the launcher string too; an fd opened here
+    // in cli.ts wouldn't cross the spawn boundary. Falls back to 'ignore'
+    // the same way openDaemonLogSink() does if the state dir isn't writable.
+    const daemonLogPathStr = JSON.stringify(path.join(config.stateDir, 'browse-daemon.log'));
     const launcherCode =
       `const{spawn}=require('child_process');` +
+      `const fs=require('fs');` +
+      `let logFd;try{logFd=fs.openSync(${daemonLogPathStr},'a');}catch(e){logFd='ignore';}` +
       `spawn(process.execPath,[${JSON.stringify(NODE_SERVER_SCRIPT)}],` +
-      `{detached:true,windowsHide:true,stdio:['ignore','ignore','ignore'],env:Object.assign({},process.env,` +
+      `{detached:true,windowsHide:true,stdio:['ignore',logFd,logFd],env:Object.assign({},process.env,` +
       `${extraEnvStr})}).unref()`;
     Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'], windowsHide: true });
   } else {
@@ -411,10 +442,11 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
     // which calls setsid() so the server becomes its own session leader
     // (PPID=1, STAT=Ss) and survives the spawning shell's exit. Mirrors
     // the Windows path's rationale — same root cause, different OS API.
+    const daemonLogFd = openDaemonLogSink();
     nodeSpawn('bun', ['run', SERVER_SCRIPT], {
       detached: true,
       windowsHide: true,
-      stdio: ['ignore', 'ignore', 'ignore'],
+      stdio: ['ignore', daemonLogFd, daemonLogFd],
       env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...extraEnv },
     }).unref();
   }

browse/test/daemon-log-hygiene.test.ts

@@ -0,0 +1,95 @@
+/**
+ * #2461 daemon crash log + F6 log hygiene needles.
+ *
+ * The detached daemon's stdout/stderr now land in <stateDir>/browse-daemon.log
+ * (both spawn paths) instead of 'ignore'. That makes crashes diagnosable —
+ * and makes it load-bearing that NOTHING secret or page-derived reaches the
+ * daemon's console streams:
+ *
+ *   - No console.* call anywhere in src/ may pass a token VALUE (AUTH_TOKEN,
+ *     state.token, attachToken, INTERNAL_TOKEN, setup keys). Names like
+ *     tokenInfo.clientId are fine — the needle targets expressions whose
+ *     value IS a token.
+ *   - The page-content carrier modules (tab-session, buffers,
+ *     content-security, activity) stay console-free, so raw page-derived
+ *     strings can't be echoed into the log unsanitized.
+ *
+ * Source-level, same style as windows-spawn-hide.test.ts.
+ */
+
+import { describe, expect, test } from 'bun:test';
+import * as fs from 'fs';
+import * as path from 'path';
+
+const SRC_DIR = path.join(import.meta.dir, '../src');
+const SRC = (f: string) => fs.readFileSync(path.join(SRC_DIR, f), 'utf-8');
+
+describe('#2461 daemon log wiring', () => {
+  test('both daemon spawn paths capture stdout/stderr to browse-daemon.log', () => {
+    const cli = SRC('cli.ts');
+    // Unix path: fd from openDaemonLogSink wired into stdio.
+    expect(cli).toContain("stdio: ['ignore', daemonLogFd, daemonLogFd]");
+    expect(cli).toMatch(/openDaemonLogSink/);
+    // Windows path: the fd must be opened INSIDE the node -e launcher (an fd
+    // opened in cli.ts wouldn't cross the spawn boundary).
+    expect(cli).toContain("stdio:['ignore',logFd,logFd]");
+    expect(cli).toContain('browse-daemon.log');
+    // The old fully-discarded wiring must not come back on either daemon path.
+    expect(cli).not.toContain("stdio:['ignore','ignore','ignore']");
+  });
+
+  test('log sink is append-mode (accumulates across respawns)', () => {
+    const cli = SRC('cli.ts');
+    expect(cli).toMatch(/openSync\(path\.join\(config\.stateDir, 'browse-daemon\.log'\), 'a'\)/);
+    expect(cli).toMatch(/openSync\(\$\{daemonLogPathStr\},'a'\)/);
+  });
+
+  test('bun-polyfill routes Windows spawns through cross-spawn (ENOENT + cmd.exe injection fix)', () => {
+    const polyfill = SRC('bun-polyfill.cjs');
+    expect(polyfill).toContain("require('cross-spawn')");
+    expect(polyfill).toMatch(/process\.platform === 'win32' \? crossSpawn\.sync : nodeSpawnSync/);
+    expect(polyfill).toMatch(/process\.platform === 'win32' \? crossSpawn : nodeSpawn/);
+    // The rejected-for-cause alternative must not creep back in: shell:true
+    // on Windows routes through cmd.exe and does NOT neutralize & | ^ % < >.
+    // (Strip comments — the header documents WHY shell:true was rejected.)
+    const code = polyfill.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, '');
+    expect(code).not.toMatch(/shell:\s*true/);
+  });
+});
+
+describe('F6 log hygiene: nothing secret or page-derived reaches daemon console', () => {
+  const files = fs.readdirSync(SRC_DIR).filter((f) => f.endsWith('.ts') || f.endsWith('.cjs'));
+
+  test('no console.* call passes a token value', () => {
+    const offenders: string[] = [];
+    for (const file of files) {
+      const content = SRC(file);
+      for (const [idx, line] of content.split('\n').entries()) {
+        if (!/console\.(log|error|warn|info)\(/.test(line)) continue;
+        // Interpolated token values: ${...token} / ${...Token} — the
+        // expression ENDS in token, i.e. the value IS the token. Names like
+        // ${tokenInfo.clientId} don't match.
+        if (/\$\{[^}]*[tT]oken\s*\}/.test(line)) {
+          offenders.push(`${file}:${idx + 1}: ${line.trim().slice(0, 120)}`);
+          continue;
+        }
+        // Bare token args: console.log('x', token) / (..., authToken)
+        if (/console\.(log|error|warn|info)\([^)]*[^a-zA-Z_.][tT]oken\s*[,)]/.test(line)) {
+          offenders.push(`${file}:${idx + 1}: ${line.trim().slice(0, 120)}`);
+        }
+      }
+    }
+    expect(offenders).toEqual([]);
+  });
+
+  test('page-content carrier modules are console-free', () => {
+    // Page-derived strings flow through these modules. Keeping them
+    // console-free guarantees raw page content can't be echoed into
+    // browse-daemon.log without passing an egress sanitizer first.
+    for (const file of ['tab-session.ts', 'buffers.ts', 'content-security.ts', 'activity.ts']) {
+      const content = SRC(file);
+      const calls = content.match(/console\.(log|error|warn|info)\(/g) || [];
+      expect({ file, count: calls.length }).toEqual({ file, count: 0 });
+    }
+  });
+});

bun.lock

@@ -7,6 +7,7 @@
       "dependencies": {
         "@huggingface/transformers": "^4.1.0",
         "@ngrok/ngrok": "^1.7.0",
+        "cross-spawn": "^7.0.6",
         "diff": "^9.0.0",
         "html-to-docx": "1.8.0",
         "marked": "^18.0.2",

package.json

@@ -53,6 +53,7 @@
   "dependencies": {
     "@huggingface/transformers": "^4.1.0",
     "@ngrok/ngrok": "^1.7.0",
+    "cross-spawn": "^7.0.6",
     "diff": "^9.0.0",
     "html-to-docx": "1.8.0",
     "marked": "^18.0.2",