mirror of https://github.com/garrytan/gstack.git
fix(browse): Xvfb identity is argv[0]'s basename, not a cmdline substring
First Linux CI run: isOurXvfb identified the TEST RUNNER as our Xvfb —
the suite's own argv contains 'xvfb.test.ts', the substring match over
the whole cmdline passed, and the start-time check matched because the
pid was real. Any process whose ARGUMENTS mention xvfb (a runner, an
editor) was killable — the sibling-kill class the identity check
exists to prevent. Identity now rests on argv[0]'s basename ('Xvfb'),
with a sh-$0 regression pin. isDisplayFree falls back to the X
socket/lock files when xdpyinfo isn't installed (x11-utils is absent
on some images that ship Xvfb).
This commit is contained in:
parent
bb69c34b4b
commit
4466af6a50
|
|
@ -58,11 +58,18 @@ export function shouldSpawnXvfb(env: NodeJS.ProcessEnv, platform: NodeJS.Platfor
|
||||||
*/
|
*/
|
||||||
export function isDisplayFree(displayNum: number): boolean {
|
export function isDisplayFree(displayNum: number): boolean {
|
||||||
// xdpyinfo exits 0 if a display is reachable. Exit non-zero means no
|
// xdpyinfo exits 0 if a display is reachable. Exit non-zero means no
|
||||||
// server, which is what we want.
|
// server, which is what we want. xdpyinfo ships in x11-utils, which some
|
||||||
const result = Bun.spawnSync(['xdpyinfo', '-display', `:${displayNum}`], {
|
// images with Xvfb still lack (first Linux CI run: ENOENT) — fall back to
|
||||||
stdout: 'ignore', stderr: 'ignore', timeout: 2000,
|
// the X socket/lock files, the same signal X servers themselves use.
|
||||||
});
|
try {
|
||||||
return result.exitCode !== 0;
|
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/<pid>/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:
|
* Validate that PID is still our Xvfb child. Both checks must pass:
|
||||||
* 1. /proc/<pid>/cmdline contains 'Xvfb' (string match — Xvfb's argv[0] is
|
* 1. argv[0]'s basename IS the Xvfb binary. A substring match over the
|
||||||
* always 'Xvfb' or a full path ending in /Xvfb)
|
* 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)
|
* 2. Start time matches the recorded value (PID reuse defense)
|
||||||
*/
|
*/
|
||||||
export function isOurXvfb(pid: number, recordedStartTime: string): boolean {
|
export function isOurXvfb(pid: number, recordedStartTime: string): boolean {
|
||||||
if (!pid || !recordedStartTime) return false;
|
if (!pid || !recordedStartTime) return false;
|
||||||
const cmdline = readPidCmdline(pid);
|
const argv0 = readPidArgv0(pid);
|
||||||
if (!cmdline.toLowerCase().includes('xvfb')) return false;
|
if (!argv0) return false;
|
||||||
|
const base = argv0.split('/').pop() ?? '';
|
||||||
|
if (base.toLowerCase() !== 'xvfb') return false;
|
||||||
const currentStart = readPidStartTime(pid);
|
const currentStart = readPidStartTime(pid);
|
||||||
if (!currentStart) return false;
|
if (!currentStart) return false;
|
||||||
return currentStart === recordedStartTime;
|
return currentStart === recordedStartTime;
|
||||||
|
|
|
||||||
|
|
@ -63,10 +63,28 @@ describe('isOurXvfb (PID validation)', () => {
|
||||||
|
|
||||||
test('returns false when cmdline does not contain Xvfb', () => {
|
test('returns false when cmdline does not contain Xvfb', () => {
|
||||||
// Current bun process is not Xvfb. PID-correct, cmdline-wrong → reject.
|
// 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);
|
const myStart = readPidStartTime(process.pid);
|
||||||
expect(isOurXvfb(process.pid, myStart)).toBe(false);
|
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)', () => {
|
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
|
// 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
|
// different process. We never fake the cmdline test, so this assertion
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue