diff --git a/browse/src/xvfb.ts b/browse/src/xvfb.ts index 3e0dad8a6..17269c78d 100644 --- a/browse/src/xvfb.ts +++ b/browse/src/xvfb.ts @@ -58,11 +58,18 @@ export function shouldSpawnXvfb(env: NodeJS.ProcessEnv, platform: NodeJS.Platfor */ 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; + // server, which is what we want. xdpyinfo ships in x11-utils, which some + // images with Xvfb still lack (first Linux CI run: ENOENT) — fall back to + // the X socket/lock files, the same signal X servers themselves use. + try { + const result = Bun.spawnSync(['xdpyinfo', '-display', `:${displayNum}`], { + stdout: 'ignore', stderr: 'ignore', timeout: 2000, + }); + return result.exitCode !== 0; + } catch { + return !fs.existsSync(`/tmp/.X11-unix/X${displayNum}`) + && !fs.existsSync(`/tmp/.X${displayNum}-lock`); + } } /** @@ -106,16 +113,34 @@ export function readPidCmdline(pid: number): string { } } +/** + * Read argv[0] of a PID via /proc//cmdline (NUL-separated). Returns + * empty string if the process is gone or the cmdline isn't readable. + */ +export function readPidArgv0(pid: number): string { + try { + const raw = fs.readFileSync(`/proc/${pid}/cmdline`, 'utf-8'); + return raw.split('\0', 1)[0] ?? ''; + } 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) + * 1. argv[0]'s basename IS the Xvfb binary. A substring match over the + * whole cmdline is identity-kill poison: any process whose ARGUMENTS + * mention xvfb (the test runner executing xvfb.test.ts, an editor with + * the file open) would pass and become killable. First Linux CI run + * caught exactly that — the suite identified itself as our 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 argv0 = readPidArgv0(pid); + if (!argv0) return false; + const base = argv0.split('/').pop() ?? ''; + if (base.toLowerCase() !== 'xvfb') return false; const currentStart = readPidStartTime(pid); if (!currentStart) return false; return currentStart === recordedStartTime; diff --git a/browse/test/xvfb.test.ts b/browse/test/xvfb.test.ts index 8fe9d4c30..dab242a8d 100644 --- a/browse/test/xvfb.test.ts +++ b/browse/test/xvfb.test.ts @@ -63,10 +63,28 @@ describe('isOurXvfb (PID validation)', () => { test('returns false when cmdline does not contain Xvfb', () => { // Current bun process is not Xvfb. PID-correct, cmdline-wrong → reject. + // NOTE: this very suite's argv CONTAINS "xvfb.test.ts" — a substring + // match over the whole cmdline identified the test runner as our Xvfb + // on the first Linux CI run. Identity rests on argv[0]'s basename. const myStart = readPidStartTime(process.pid); expect(isOurXvfb(process.pid, myStart)).toBe(false); }); + test('a process whose ARGUMENTS mention xvfb is not ours (argv0 identity)', async () => { + // sh's $0 trick plants "xvfb" in the child's args while argv[0] stays sh. + // Killing this process because its arguments mention xvfb is the exact + // sibling-kill class the identity check exists to prevent. + const child = Bun.spawn(['/bin/sh', '-c', 'sleep 2', 'xvfb-lookalike-arg']); + try { + const start = readPidStartTime(child.pid); + // On non-Linux, /proc is absent and both reads return '' → false either way. + expect(isOurXvfb(child.pid, start || 'recorded')).toBe(false); + } finally { + child.kill(); + await child.exited; + } + }); + 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