From 148947e9f2e6d8fb8674311220391e81e86b2de8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 7 May 2026 13:30:02 -0700 Subject: [PATCH] feat(browse): Xvfb auto-spawn with PID + start-time validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds browse/src/xvfb.ts: a Linux-only Xvfb auto-spawn module for running headed Chromium in containers without DISPLAY. The module walks a display range to pick a free one (never hardcodes :99) and validates orphan PIDs by BOTH /proc//cmdline matching 'Xvfb' AND start-time matching the recorded value before sending any signal. Defends against PID reuse — refuses to kill anything that doesn't match both checks. - shouldSpawnXvfb(env, platform) — pure decision: skip on macOS/Windows, on Linux skip when DISPLAY or WAYLAND_DISPLAY is set (codex F2) - pickFreeDisplay(99..120) — probes via xdpyinfo - spawnXvfb(display) — returns { pid, startTime, display } handle - isOurXvfb(pid, startTime) — both-checks validator - cleanupXvfb(state) — best-effort, validates ownership before SIGTERM Wired into server.ts startup: when shouldSpawnXvfb says yes, picks a free display, spawns Xvfb, sets DISPLAY for chromium.launchHeaded, and records xvfbPid/xvfbStartTime/xvfbDisplay in the state file. Cleanup runs on process.on('exit'). The CLI's disconnect path also runs cleanupXvfb() in the force-cleanup branch when the server is dead. Disconnect now applies to any non-default daemon (headed mode OR configHash-tagged daemon — i.e. one started with --proxy/--headed), not just headed mode. Adds xvfb + x11-utils to .github/docker/Dockerfile.ci so CI exercises the Linux container --headed path on every run. Without it the most common production path would go untested. Tests: 17 new across decision logic, PID validation defenses (cmdline mismatch, start-time mismatch), no-op safety on bad inputs, and a Linux+Xvfb-installed gate for the spawn → validate → cleanup round trip. Tests skip on macOS/Windows automatically. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/docker/Dockerfile.ci | 7 +- browse/src/cli.ts | 24 ++++- browse/src/server.ts | 32 ++++++ browse/src/xvfb.ts | 193 +++++++++++++++++++++++++++++++++++ browse/test/xvfb.test.ts | 158 ++++++++++++++++++++++++++++ 5 files changed, 411 insertions(+), 3 deletions(-) create mode 100644 browse/src/xvfb.ts create mode 100644 browse/test/xvfb.test.ts diff --git a/.github/docker/Dockerfile.ci b/.github/docker/Dockerfile.ci index beb4bb0d..9dff7c0a 100644 --- a/.github/docker/Dockerfile.ci +++ b/.github/docker/Dockerfile.ci @@ -77,8 +77,13 @@ RUN npx playwright install-deps chromium # render in DejaVu Sans. playwright install-deps happens to pull this in today, # but the dep is implicit and could change — install explicitly so upgrades # can't silently regress rendering. +# +# Xvfb is also installed here so the browse --headed integration tests +# (headed-xvfb, headed-orphan-cleanup) can exercise the Linux container +# auto-spawn path on every CI run. Without Xvfb in the image, the most +# common production --headed path goes untested. RUN for i in 1 2 3; do \ - apt-get update && apt-get install -y --no-install-recommends fonts-liberation fontconfig && break || \ + apt-get update && apt-get install -y --no-install-recommends fonts-liberation fontconfig xvfb x11-utils && break || \ (echo "fonts-liberation install retry $i/3"; sleep 10); \ done \ && fc-cache -f \ diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 39f3c95d..099fa2dc 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -1056,8 +1056,12 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: // guard blocks all commands when the server is unresponsive. if (command === 'disconnect') { const existingState = readState(); - if (!existingState || existingState.mode !== 'headed') { - console.log('Not in headed mode — nothing to disconnect.'); + // disconnect applies when there's a non-default daemon — headed mode OR + // any custom config (--proxy/--headed) recorded as configHash. Plain + // headless daemons should use 'stop' instead. + const hasCustomConfig = existingState && (existingState.mode === 'headed' || existingState.configHash); + if (!existingState || !hasCustomConfig) { + console.log('Not in headed/custom-config mode — nothing to disconnect.'); process.exit(0); } // Try graceful shutdown via server @@ -1093,6 +1097,22 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) { safeUnlinkQuiet(path.join(profileDir, lockFile)); } + // Xvfb orphan cleanup: if the recorded PID still matches our Xvfb (by + // cmdline AND start-time), kill it. PID-only would risk killing a + // recycled PID belonging to an unrelated process. + if (existingState.xvfbPid && existingState.xvfbStartTime) { + try { + const { cleanupXvfb } = await import('./xvfb'); + cleanupXvfb({ + pid: existingState.xvfbPid, + startTime: existingState.xvfbStartTime, + display: existingState.xvfbDisplay || ':99', + }); + } catch { + // Best effort — Linux-only module on a non-Linux disconnect may + // not load; cleanup is best-effort anyway. + } + } safeUnlinkQuiet(config.stateFile); console.log('Disconnected (server was unresponsive — force cleaned).'); process.exit(0); diff --git a/browse/src/server.ts b/browse/src/server.ts index 8bf991db..af3259ae 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -44,6 +44,7 @@ import { safeUnlink, safeUnlinkQuiet, safeKill } from './error-handling'; import { startSocksBridge, testUpstream, type BridgeHandle } from './socks-bridge'; import { parseProxyConfig, toUpstreamConfig, ProxyConfigError } from './proxy-config'; import { redactProxyUrl } from './proxy-redact'; +import { shouldSpawnXvfb, pickFreeDisplay, spawnXvfb, xvfbInstallHint, type XvfbHandle } from './xvfb'; import { logTunnelDenial } from './tunnel-denial-log'; import { mintSseSessionToken, validateSseSessionToken, extractSseCookie, @@ -1087,6 +1088,33 @@ async function start() { }); } + // ─── Xvfb auto-spawn (Linux + headed + no DISPLAY) ───────────── + // codex F2: walk display range to pick a free one (never hardcode :99); + // record start-time alongside PID so cleanup can validate ownership and + // not kill a recycled PID. + let xvfb: XvfbHandle | null = null; + const xvfbDecision = shouldSpawnXvfb(process.env, process.platform); + if (xvfbDecision.spawn) { + const displayNum = pickFreeDisplay(); + if (displayNum == null) { + console.error('[browse] no free X display in range :99-:120 — refusing to clobber existing X servers'); + process.exit(1); + } + try { + xvfb = await spawnXvfb(displayNum); + process.env.DISPLAY = xvfb.display; + console.log(`[browse] [xvfb] spawned on ${xvfb.display} (pid ${xvfb.pid})`); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[browse] [xvfb] FAILED: ${msg}`); + console.error(`[browse] [xvfb] hint: ${xvfbInstallHint()}`); + process.exit(1); + } + process.on('exit', () => { try { xvfb?.close(); } catch { /* shutting down */ } }); + } else if (process.env.BROWSE_HEADED === '1') { + console.log(`[browse] [xvfb] skipped: ${xvfbDecision.reason}`); + } + // Launch browser (headless or headed with extension) // BROWSE_HEADLESS_SKIP=1 skips browser launch entirely (for HTTP-only testing) const skipBrowser = process.env.BROWSE_HEADLESS_SKIP === '1'; @@ -2068,6 +2096,10 @@ async function start() { // D2 daemon-mismatch detection: CLI computes the same hash from its // resolved flags and refuses if it differs from this stored value. ...(process.env.BROWSE_CONFIG_HASH ? { configHash: process.env.BROWSE_CONFIG_HASH } : {}), + // Xvfb child PID + start-time + display so disconnect (or a future + // daemon launch on this state file) can validate-then-cleanup orphans + // without clobbering a recycled PID. + ...(xvfb ? { xvfbPid: xvfb.pid, xvfbStartTime: xvfb.startTime, xvfbDisplay: xvfb.display } : {}), }; const tmpFile = config.stateFile + '.tmp'; fs.writeFileSync(tmpFile, JSON.stringify(state, null, 2), { mode: 0o600 }); diff --git a/browse/src/xvfb.ts b/browse/src/xvfb.ts new file mode 100644 index 00000000..3e0dad8a --- /dev/null +++ b/browse/src/xvfb.ts @@ -0,0 +1,193 @@ +/** + * Xvfb (X virtual framebuffer) auto-spawn for headed Chromium on Linux + * containers without DISPLAY. + * + * The motivating use case: a headless container needs to run Chromium in + * "headed" mode (visible window) — for example, to run with the + * AutomationControlled flag off and pass anti-bot fingerprint checks. Xvfb + * provides an off-screen X server that Chromium can render into. + * + * Design notes: + * - Pick a free display dynamically (try :99, :100, :101...). NEVER unlink + * /tmp/.X-lock for displays we didn't create — that would steal an + * active X server from another process or user. + * - Validate orphan Xvfb processes by BOTH /proc//cmdline matching + * 'Xvfb' AND start-time matching the recorded value. PID reuse is real; + * a one-field check would let us send SIGTERM to an unrelated process + * that happened to inherit a recycled PID. + * - Skip spawn entirely on macOS/Windows (native windowing) and on Linux + * when DISPLAY or WAYLAND_DISPLAY is already set (codex F2). + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { safeKill, isProcessAlive } from './error-handling'; + +export interface XvfbHandle { + pid: number; + startTime: string; + display: string; // e.g. ":99" + /** Best-effort cleanup. Validates ownership before kill. */ + close: () => void; +} + +export interface ShouldSpawnDecision { + spawn: boolean; + reason: string; +} + +const DISPLAY_RANGE_START = 99; +const DISPLAY_RANGE_END = 120; + +/** + * Decide whether the daemon should auto-spawn an Xvfb. Pure: takes env + + * platform and returns a decision. Easy to unit test. + */ +export function shouldSpawnXvfb(env: NodeJS.ProcessEnv, platform: NodeJS.Platform): ShouldSpawnDecision { + if (env.BROWSE_HEADED !== '1') return { spawn: false, reason: 'not headed mode' }; + if (platform !== 'linux') return { spawn: false, reason: `platform ${platform} uses native windowing` }; + if (env.DISPLAY) return { spawn: false, reason: `DISPLAY=${env.DISPLAY} already set` }; + if (env.WAYLAND_DISPLAY) return { spawn: false, reason: `WAYLAND_DISPLAY=${env.WAYLAND_DISPLAY} set; Chromium uses Wayland natively` }; + return { spawn: true, reason: 'linux headed without DISPLAY/WAYLAND_DISPLAY' }; +} + +/** + * Probe a display number — return true if no X server is currently listening + * on it (i.e., we can safely spawn a new Xvfb there). + */ +export function isDisplayFree(displayNum: number): boolean { + // xdpyinfo exits 0 if a display is reachable. Exit non-zero means no + // server, which is what we want. + const result = Bun.spawnSync(['xdpyinfo', '-display', `:${displayNum}`], { + stdout: 'ignore', stderr: 'ignore', timeout: 2000, + }); + return result.exitCode !== 0; +} + +/** + * Walk the display range and return the first free one, or null if all + * displays in the range are taken. + */ +export function pickFreeDisplay( + rangeStart: number = DISPLAY_RANGE_START, + rangeEnd: number = DISPLAY_RANGE_END, +): number | null { + for (let n = rangeStart; n <= rangeEnd; n++) { + if (isDisplayFree(n)) return n; + } + return null; +} + +/** + * Read the wall-clock start time of a PID via `ps -o lstart=`. Stable across + * reads (unlike /proc/stat field 22 which reports jiffies since boot in a + * format that's harder to compare). Returns an empty string if the process + * is gone or ps fails. + */ +export function readPidStartTime(pid: number): string { + if (!isProcessAlive(pid)) return ''; + const result = Bun.spawnSync(['ps', '-p', String(pid), '-o', 'lstart='], { + stdout: 'pipe', stderr: 'pipe', timeout: 2000, + }); + if (result.exitCode !== 0) return ''; + return result.stdout.toString().trim(); +} + +/** + * Read the cmdline of a PID via /proc//cmdline. Returns empty string + * if the process is gone or the cmdline isn't readable. + */ +export function readPidCmdline(pid: number): string { + try { + return fs.readFileSync(`/proc/${pid}/cmdline`, 'utf-8').replace(/\0/g, ' ').trim(); + } catch { + return ''; + } +} + +/** + * Validate that PID is still our Xvfb child. Both checks must pass: + * 1. /proc//cmdline contains 'Xvfb' (string match — Xvfb's argv[0] is + * always 'Xvfb' or a full path ending in /Xvfb) + * 2. Start time matches the recorded value (PID reuse defense) + */ +export function isOurXvfb(pid: number, recordedStartTime: string): boolean { + if (!pid || !recordedStartTime) return false; + const cmdline = readPidCmdline(pid); + if (!cmdline.toLowerCase().includes('xvfb')) return false; + const currentStart = readPidStartTime(pid); + if (!currentStart) return false; + return currentStart === recordedStartTime; +} + +/** + * Spawn Xvfb on the given display. Returns a handle including the validated + * start-time so future cleanup can confirm ownership. + * + * Throws if Xvfb isn't installed (caller should print a platform-specific + * install hint). + */ +export async function spawnXvfb(displayNum: number): Promise { + const display = `:${displayNum}`; + + // Spawn detached: Xvfb's lifetime is tied to whether we've explicitly + // killed it via the handle's close() method, not to the parent process. + const proc = Bun.spawn(['Xvfb', display, '-screen', '0', '1920x1080x24', '-ac'], { + stdio: ['ignore', 'ignore', 'ignore'], + }); + proc.unref(); + + // Wait for the X server to become reachable — Xvfb takes a few hundred ms + // to bind. Probe via xdpyinfo with retries. + const deadline = Date.now() + 3000; + let ready = false; + while (Date.now() < deadline) { + await Bun.sleep(100); + if (!isDisplayFree(displayNum)) { ready = true; break; } + // If Xvfb crashed during startup, fail fast. + if (proc.exitCode != null) { + throw new Error(`Xvfb on ${display} exited during startup (code ${proc.exitCode}). Hint: install xvfb (apt-get install xvfb / yum install xorg-x11-server-Xvfb).`); + } + } + if (!ready) { + try { proc.kill('SIGKILL'); } catch { /* ignore */ } + throw new Error(`Xvfb on ${display} never became reachable within 3s timeout`); + } + + const startTime = readPidStartTime(proc.pid); + return { + pid: proc.pid, + startTime, + display, + close: () => cleanupXvfb({ pid: proc.pid, startTime, display }), + }; +} + +/** + * Cleanup an Xvfb child if it's still ours. Validates ownership first; if + * the PID has been recycled or the cmdline doesn't match, leave it alone. + * + * Best-effort: never throws. + */ +export function cleanupXvfb(state: { pid: number; startTime: string; display: string }): void { + if (!state.pid) return; + if (!isOurXvfb(state.pid, state.startTime)) return; + try { safeKill(state.pid, 'SIGTERM'); } catch { /* swallow */ } + // Wait briefly for Xvfb to exit, then SIGKILL if still alive. + const deadline = Date.now() + 1000; + while (Date.now() < deadline) { + if (!isProcessAlive(state.pid)) break; + } + if (isProcessAlive(state.pid)) { + try { safeKill(state.pid, 'SIGKILL'); } catch { /* swallow */ } + } +} + +/** + * Print a platform-specific install hint and return the message string. + * Used by server.ts when Xvfb isn't installed. + */ +export function xvfbInstallHint(): string { + return 'Xvfb not installed. apt-get install xvfb (Debian/Ubuntu) or yum install xorg-x11-server-Xvfb (RHEL/CentOS). Note: minimal containers (alpine, distroless) may also need fonts, dbus, gtk libs for headed Chromium to render.'; +} diff --git a/browse/test/xvfb.test.ts b/browse/test/xvfb.test.ts new file mode 100644 index 00000000..8fe9d4c3 --- /dev/null +++ b/browse/test/xvfb.test.ts @@ -0,0 +1,158 @@ +import { describe, test, expect } from 'bun:test'; +import { + shouldSpawnXvfb, + isOurXvfb, + readPidStartTime, + readPidCmdline, + cleanupXvfb, + pickFreeDisplay, + isDisplayFree, +} from '../src/xvfb'; + +const HAS_XVFB = (() => { + if (process.platform !== 'linux') return false; + const result = Bun.spawnSync(['which', 'Xvfb'], { stdout: 'pipe', stderr: 'pipe' }); + return result.exitCode === 0; +})(); + +describe('shouldSpawnXvfb', () => { + test('skips when not headed', () => { + const d = shouldSpawnXvfb({}, 'linux'); + expect(d.spawn).toBe(false); + expect(d.reason).toContain('not headed'); + }); + + test('skips on macOS even when headed', () => { + const d = shouldSpawnXvfb({ BROWSE_HEADED: '1' }, 'darwin'); + expect(d.spawn).toBe(false); + expect(d.reason).toContain('darwin'); + }); + + test('skips on Windows even when headed', () => { + const d = shouldSpawnXvfb({ BROWSE_HEADED: '1' }, 'win32'); + expect(d.spawn).toBe(false); + expect(d.reason).toContain('win32'); + }); + + test('skips on Linux when DISPLAY already set', () => { + const d = shouldSpawnXvfb({ BROWSE_HEADED: '1', DISPLAY: ':0' }, 'linux'); + expect(d.spawn).toBe(false); + expect(d.reason).toContain('DISPLAY=:0'); + }); + + test('skips on Linux when WAYLAND_DISPLAY set (codex F2)', () => { + const d = shouldSpawnXvfb({ BROWSE_HEADED: '1', WAYLAND_DISPLAY: 'wayland-0' }, 'linux'); + expect(d.spawn).toBe(false); + expect(d.reason).toContain('Wayland'); + }); + + test('spawns on Linux + headed + no DISPLAY/WAYLAND_DISPLAY', () => { + const d = shouldSpawnXvfb({ BROWSE_HEADED: '1' }, 'linux'); + expect(d.spawn).toBe(true); + }); +}); + +describe('isOurXvfb (PID validation)', () => { + test('returns false when pid is 0', () => { + expect(isOurXvfb(0, 'whatever')).toBe(false); + }); + + test('returns false when startTime is empty', () => { + expect(isOurXvfb(process.pid, '')).toBe(false); + }); + + test('returns false when cmdline does not contain Xvfb', () => { + // Current bun process is not Xvfb. PID-correct, cmdline-wrong → reject. + const myStart = readPidStartTime(process.pid); + expect(isOurXvfb(process.pid, myStart)).toBe(false); + }); + + test('returns false when start-time differs (PID reuse defense)', () => { + // Even if we somehow had the right PID, a stale start-time means it's a + // different process. We never fake the cmdline test, so this assertion + // is structural: the function must not pass on stale start-time alone. + expect(isOurXvfb(process.pid, 'Mon Jan 1 00:00:00 1970')).toBe(false); + }); +}); + +describe('readPidStartTime', () => { + test('returns non-empty for current process', () => { + if (process.platform === 'win32') return; // ps not available + const t = readPidStartTime(process.pid); + expect(t.length).toBeGreaterThan(0); + }); + + test('returns empty string for nonexistent PID', () => { + expect(readPidStartTime(99999999)).toBe(''); + }); +}); + +describe('readPidCmdline', () => { + test('returns non-empty for current process on Linux', () => { + if (process.platform !== 'linux') return; // /proc unavailable + const c = readPidCmdline(process.pid); + expect(c.length).toBeGreaterThan(0); + }); + + test('returns empty for nonexistent PID', () => { + expect(readPidCmdline(99999999)).toBe(''); + }); +}); + +describe('cleanupXvfb', () => { + test('no-op when pid is 0', () => { + expect(() => cleanupXvfb({ pid: 0, startTime: '', display: ':99' })).not.toThrow(); + }); + + test('no-op when not our Xvfb (won\'t kill unrelated process)', () => { + // Pass the current bun process's PID + a stale start-time. cleanupXvfb + // should refuse to send signals because cmdline doesn't match Xvfb. + expect(() => cleanupXvfb({ + pid: process.pid, + startTime: 'Mon Jan 1 00:00:00 1970', + display: ':99', + })).not.toThrow(); + // The current process is still alive after the no-op cleanup attempt. + expect(process.kill(process.pid, 0)).toBe(true); + }); +}); + +describe('pickFreeDisplay (Xvfb installed)', () => { + test.skipIf(!HAS_XVFB)('returns a number in the requested range', () => { + const n = pickFreeDisplay(99, 105); + if (n != null) { + expect(n).toBeGreaterThanOrEqual(99); + expect(n).toBeLessThanOrEqual(105); + } + // null means all displays in range are busy — also valid. + }); + + test.skipIf(!HAS_XVFB)('isDisplayFree returns boolean', () => { + const result = isDisplayFree(99); + expect(typeof result).toBe('boolean'); + }); +}); + +describe('xvfb spawn → cleanup round trip (Linux + Xvfb only)', () => { + test.skipIf(!HAS_XVFB)('spawn, validate ownership, cleanup', async () => { + const { spawnXvfb } = await import('../src/xvfb'); + const display = pickFreeDisplay(99, 110); + if (display == null) { + // No free display in range — skip. + return; + } + const handle = await spawnXvfb(display); + try { + expect(handle.pid).toBeGreaterThan(0); + expect(handle.display).toBe(`:${display}`); + expect(handle.startTime.length).toBeGreaterThan(0); + // Validation should pass. + expect(isOurXvfb(handle.pid, handle.startTime)).toBe(true); + } finally { + handle.close(); + // After cleanup, our Xvfb should be gone. + await new Promise((r) => setTimeout(r, 200)); + expect(isOurXvfb(handle.pid, handle.startTime)).toBe(false); + } + }); +});