mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(browse): read Windows process start times in-process; drop PowerShell from two flaky test paths (GSTA-139 S5)
The terminal agent's identity probe launched powershell.exe Get-CimInstance with a 2 s timeout on every boot (twice when started standalone). On loaded Windows runners a launch takes ~0.94 s at p50 and up to 10.8 s, so the agent missed the watchdog test's 5 s window and a stalled launch returned an empty identity. Under Bun the start time now comes from GetProcessTimes in the same /Date(ms)/ form PowerShell's CreationDate serialises to, so records, the Node daemon (which keeps the PowerShell reader) and earlier releases agree. The kill-time command-line check still uses PowerShell. path-security-realpath builds its 8.3 alias with GetShortPathNameW instead of a PowerShell launch that stalled for 30 s and was reported as '8.3 disabled'. cookie-import-native-job prints the Restart Manager receipt when its owner assertion fails. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
4b8c03bea9
commit
d4f0ea3c31
4 files changed
+191
-14
No files matched your search
@@ -24,9 +24,10 @@ import { atomicWriteSync } from '../../lib/fs-atomic';
|
||||
import { readPidCmdline, readPidStartTime } from './xvfb';
|
||||
import { emptyBunConfig, findWindowsBunExe, type BunExeLookup } from './bun-child-config';
|
||||
|
||||
function agentProcessInfo(pid: number): { startTime: string; commandLine: string } {
|
||||
if (!Number.isSafeInteger(pid) || pid <= 0) return { startTime: '', commandLine: '' };
|
||||
if (process.platform !== 'win32') return { startTime: readPidStartTime(pid), commandLine: readPidCmdline(pid) };
|
||||
const FILETIME_UNIX_EPOCH_MS = 11_644_473_600_000n;
|
||||
let win32Kernel: any;
|
||||
|
||||
function windowsProcessQuery(pid: number): { startTime: string; commandLine: string } {
|
||||
const script = `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}' | Select-Object CreationDate,CommandLine | ConvertTo-Json -Compress)`;
|
||||
try {
|
||||
const result = spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', script], { encoding: 'utf8', windowsHide: true, timeout: 2000 });
|
||||
@@ -36,8 +37,40 @@ function agentProcessInfo(pid: number): { startTime: string; commandLine: string
|
||||
} catch { return { startTime: '', commandLine: '' }; }
|
||||
}
|
||||
|
||||
export const win32CreationTimeSource = {
|
||||
read(pid: number): string | undefined {
|
||||
if (!process.versions.bun) return;
|
||||
try {
|
||||
const { dlopen, FFIType, ptr } = require('bun:ffi');
|
||||
win32Kernel ??= dlopen('kernel32.dll', {
|
||||
OpenProcess: { args: [FFIType.u32, FFIType.i32, FFIType.u32], returns: FFIType.u64 },
|
||||
GetProcessTimes: { args: [FFIType.u64, FFIType.ptr, FFIType.ptr, FFIType.ptr, FFIType.ptr], returns: FFIType.i32 },
|
||||
CloseHandle: { args: [FFIType.u64], returns: FFIType.i32 },
|
||||
}).symbols;
|
||||
const handle = win32Kernel.OpenProcess(0x1000, 0, pid);
|
||||
if (!handle) return;
|
||||
try {
|
||||
const times = new BigUint64Array(4);
|
||||
if (!win32Kernel.GetProcessTimes(handle, ptr(times, 0), ptr(times, 8), ptr(times, 16), ptr(times, 24)) || times[0] === 0n) return;
|
||||
return `/Date(${times[0] / 10_000n - FILETIME_UNIX_EPOCH_MS})/`;
|
||||
} finally {
|
||||
win32Kernel.CloseHandle(handle);
|
||||
}
|
||||
} catch { return; }
|
||||
},
|
||||
};
|
||||
|
||||
function agentProcessInfo(pid: number): { startTime: string; commandLine: string } {
|
||||
if (!Number.isSafeInteger(pid) || pid <= 0) return { startTime: '', commandLine: '' };
|
||||
if (process.platform !== 'win32') return { startTime: readPidStartTime(pid), commandLine: readPidCmdline(pid) };
|
||||
const query = windowsProcessQuery(pid);
|
||||
return { startTime: win32CreationTimeSource.read(pid) ?? query.startTime, commandLine: query.commandLine };
|
||||
}
|
||||
|
||||
export function readAgentStartTime(pid: number): string {
|
||||
return agentProcessInfo(pid).startTime;
|
||||
if (!Number.isSafeInteger(pid) || pid <= 0) return '';
|
||||
if (process.platform !== 'win32') return readPidStartTime(pid);
|
||||
return win32CreationTimeSource.read(pid) ?? windowsProcessQuery(pid).startTime;
|
||||
}
|
||||
|
||||
const pendingAgentExits = new Set<any>();
|
||||
|
||||
@@ -999,9 +999,9 @@ function safeNativeEnvelope(output: string): object {
|
||||
|
||||
function assertOwnerReceipt(receipt: { available: boolean; owners: { pid: number; creationMatched: boolean; isTestHost: boolean }[] }, pid: number) {
|
||||
const before = JSON.stringify(receipt);
|
||||
expect(receipt.available).toBe(true);
|
||||
expect(Array.isArray(receipt.owners)).toBe(true);
|
||||
expect(receipt.owners.some(owner => owner.pid === pid && owner.creationMatched && owner.isTestHost)).toBe(true);
|
||||
expect(receipt.available, before).toBe(true);
|
||||
expect(Array.isArray(receipt.owners), before).toBe(true);
|
||||
expect(receipt.owners.some(owner => owner.pid === pid && owner.creationMatched && owner.isTestHost), before).toBe(true);
|
||||
expect(JSON.stringify(receipt)).toBe(before);
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { dlopen, FFIType, ptr } from 'bun:ffi';
|
||||
|
||||
const VALIDATOR = path.resolve(import.meta.dir, '../src/path-security.ts');
|
||||
const WIN = process.platform === 'win32';
|
||||
@@ -43,11 +44,16 @@ function validate(tempDir: string, checks: Array<['output' | 'read', string]>):
|
||||
}
|
||||
|
||||
function shortPath(dir: string): string {
|
||||
const result = Bun.spawnSync(['powershell', '-NoProfile', '-NonInteractive', '-Command',
|
||||
`(New-Object -ComObject Scripting.FileSystemObject).GetFolder($env:GSTACK_LONG_DIR).ShortPath`], {
|
||||
env: { ...process.env, GSTACK_LONG_DIR: dir }, stdout: 'pipe', stderr: 'pipe', timeout: 30_000,
|
||||
});
|
||||
return result.stdout.toString().trim();
|
||||
const kernel = dlopen('kernel32.dll', { GetShortPathNameW: { args: [FFIType.ptr, FFIType.ptr, FFIType.u32], returns: FFIType.u32 } });
|
||||
try {
|
||||
const input = Buffer.from(dir + '\0', 'utf16le');
|
||||
const output = Buffer.alloc(65536);
|
||||
const length = kernel.symbols.GetShortPathNameW(ptr(input), ptr(output), 32768);
|
||||
if (length === 0 || length >= 32768) throw new Error(`GetShortPathNameW failed for ${dir} (returned ${length})`);
|
||||
return output.subarray(0, length * 2).toString('utf16le');
|
||||
} finally {
|
||||
kernel.close();
|
||||
}
|
||||
}
|
||||
|
||||
describe('#3104 one realpath for safe dirs and targets', () => {
|
||||
|
||||
@@ -4,7 +4,10 @@ import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { isProcessAlive } from '../src/error-handling';
|
||||
import { spawnTerminalAgent } from '../src/terminal-agent-control';
|
||||
import {
|
||||
isAgentRecordGone, isOurAgent, readAgentStartTime, spawnTerminalAgent, stopAgentByRecord, win32CreationTimeSource, writeAgentRecord,
|
||||
type AgentRecord,
|
||||
} from '../src/terminal-agent-control';
|
||||
|
||||
// REGRESSION TEST for the Windows terminal-agent leak.
|
||||
//
|
||||
@@ -117,8 +120,10 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
|
||||
}
|
||||
return originalProbe(command, options);
|
||||
}) as typeof Bun.spawnSync);
|
||||
const powershellCalls: string[] = [];
|
||||
const windowsProbe = spyOn(childProcess, 'spawnSync').mockImplementation(((command: string, args: string[], options: any) => {
|
||||
if (command === 'powershell.exe') {
|
||||
powershellCalls.push(args.join(' '));
|
||||
const owner = args.join(' ').includes(`ProcessId = ${process.pid}'`);
|
||||
return { status: 0, stdout: JSON.stringify(owner ? { CreationDate: 'fixture-owner-start', CommandLine: 'test-owner' } : null), stderr: '' };
|
||||
}
|
||||
@@ -143,7 +148,13 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
|
||||
// Owner-PID lifetime tie (#2019): the agent polls this and exits when
|
||||
// its owning browse server dies, so it can't be adopted by PID 1.
|
||||
expect(captured.env.BROWSE_OWNER_PID).toBe(String(process.pid));
|
||||
expect(captured.env.BROWSE_OWNER_START_TIME).toBe('fixture-owner-start');
|
||||
if (process.platform === 'win32') {
|
||||
expect(captured.env.BROWSE_OWNER_START_TIME).toMatch(/^\/Date\(\d+\)\/$/);
|
||||
expect(captured.env.BROWSE_OWNER_START_TIME).toBe(readAgentStartTime(process.pid));
|
||||
expect(powershellCalls).toEqual([]);
|
||||
} else {
|
||||
expect(captured.env.BROWSE_OWNER_START_TIME).toBe('fixture-owner-start');
|
||||
}
|
||||
// Detached background daemon — must not inherit a terminal either.
|
||||
expect(captured.stdio).toEqual(['ignore', 'ignore', 'ignore']);
|
||||
await exited;
|
||||
@@ -174,3 +185,130 @@ describe('process liveness probe (Windows terminal-agent leak)', () => {
|
||||
expect(windowMs).toBeGreaterThanOrEqual(tick * guardMax);
|
||||
});
|
||||
});
|
||||
|
||||
describe('win32 terminal-agent identity (GSTA-139)', () => {
|
||||
const pid = 2147483645;
|
||||
const gen = 'win32-identity-generation';
|
||||
const birth = '/Date(1761600000123)/';
|
||||
const ownedCommand = `"C:\\bun\\bun.exe" run terminal-agent.ts --agent-gen=${gen}`;
|
||||
const record = (startTime = birth): AgentRecord => ({ pid, gen, startTime, startedAt: 0, ownerPid: pid, ownerStartTime: birth });
|
||||
type PowerShell = { status: number | null; creationDate?: string; commandLine?: string };
|
||||
|
||||
function onWin32(scenario: { ffi: string | undefined; powershell: PowerShell }, run: (calls: { powershell: number; signals: unknown[] }) => void): void {
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')!;
|
||||
const calls = { powershell: 0, signals: [] as unknown[] };
|
||||
let live = true;
|
||||
const kill = spyOn(process, 'kill').mockImplementation(((target: number, signal: unknown) => {
|
||||
expect(target).toBe(pid);
|
||||
if (!live) throw Object.assign(new Error('gone'), { code: 'ESRCH' });
|
||||
if (signal !== 0) { calls.signals.push(signal); live = false; }
|
||||
return true;
|
||||
}) as typeof process.kill);
|
||||
const read = spyOn(win32CreationTimeSource, 'read').mockImplementation((target: number) => (target === pid && live ? scenario.ffi : undefined));
|
||||
const probe = spyOn(childProcess, 'spawnSync').mockImplementation(((command: string) => {
|
||||
expect(command).toBe('powershell.exe');
|
||||
calls.powershell++;
|
||||
const { status, creationDate, commandLine } = scenario.powershell;
|
||||
return status === 0
|
||||
? { status, stdout: JSON.stringify({ CreationDate: creationDate, CommandLine: commandLine }), stderr: '' }
|
||||
: { status, stdout: '', stderr: '', error: Object.assign(new Error('spawnSync powershell.exe ETIMEDOUT'), { code: 'ETIMEDOUT' }) };
|
||||
}) as typeof childProcess.spawnSync);
|
||||
Object.defineProperty(process, 'platform', { ...platform, value: 'win32' });
|
||||
try {
|
||||
run(calls);
|
||||
} finally {
|
||||
Object.defineProperty(process, 'platform', platform);
|
||||
probe.mockRestore();
|
||||
read.mockRestore();
|
||||
kill.mockRestore();
|
||||
}
|
||||
}
|
||||
|
||||
test('the start time is read in-process, so a PowerShell launch killed at its timeout cannot empty it', () => {
|
||||
onWin32({ ffi: birth, powershell: { status: null } }, calls => {
|
||||
expect(readAgentStartTime(pid)).toBe(birth);
|
||||
expect(calls.powershell).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
test('without the in-process reader (Node daemon) the start time falls back to the PowerShell CreationDate', () => {
|
||||
onWin32({ ffi: undefined, powershell: { status: 0, creationDate: birth, commandLine: ownedCommand } }, calls => {
|
||||
expect(readAgentStartTime(pid)).toBe(birth);
|
||||
expect(calls.powershell).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
test('kill-time ownership still requires the generation in the PowerShell command line', () => {
|
||||
onWin32({ ffi: birth, powershell: { status: 0, creationDate: birth, commandLine: ownedCommand } }, calls => {
|
||||
expect(isOurAgent(record())).toBe(true);
|
||||
expect(stopAgentByRecord(record(), 0)).toBe(true);
|
||||
expect(calls.signals).toEqual(['SIGTERM']);
|
||||
});
|
||||
for (const powershell of [{ status: 0, creationDate: birth, commandLine: 'bun run unrelated.ts' }, { status: null }]) {
|
||||
onWin32({ ffi: birth, powershell }, calls => {
|
||||
expect(isOurAgent(record())).toBe(false);
|
||||
expect(isAgentRecordGone(record())).toBe(false);
|
||||
expect(stopAgentByRecord(record(), 0)).toBe(false);
|
||||
expect(calls.signals).toEqual([]);
|
||||
});
|
||||
}
|
||||
onWin32({ ffi: '/Date(1761600000124)/', powershell: { status: 0, creationDate: birth, commandLine: ownedCommand } }, calls => {
|
||||
expect(isAgentRecordGone(record())).toBe(true);
|
||||
expect(calls.signals).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
test('an unreadable start time is unknown: never signalled and never treated as gone', () => {
|
||||
onWin32({ ffi: undefined, powershell: { status: null } }, calls => {
|
||||
expect(readAgentStartTime(pid)).toBe('');
|
||||
expect(isOurAgent(record())).toBe(false);
|
||||
expect(isAgentRecordGone(record())).toBe(false);
|
||||
expect(stopAgentByRecord(record(), 0)).toBe(false);
|
||||
expect(calls.signals).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
test.skipIf(process.platform !== 'win32')('the in-process start time equals the CreationDate earlier releases recorded', () => {
|
||||
const child = Bun.spawn([process.execPath, '-e', 'setInterval(() => {}, 1000)'], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
try {
|
||||
for (const target of [process.pid, child.pid]) {
|
||||
const script = `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${target}' | Select-Object CreationDate | ConvertTo-Json -Compress)`;
|
||||
const legacy = childProcess.spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', script], { encoding: 'utf8', windowsHide: true, timeout: 60_000 });
|
||||
expect(legacy.status).toBe(0);
|
||||
const recorded = JSON.parse(legacy.stdout).CreationDate;
|
||||
expect(recorded).toMatch(/^\/Date\(\d+\)\/$/);
|
||||
expect(win32CreationTimeSource.read(target)).toBe(recorded);
|
||||
expect(readAgentStartTime(target)).toBe(recorded);
|
||||
}
|
||||
} finally {
|
||||
child.kill();
|
||||
}
|
||||
}, 150_000);
|
||||
|
||||
test.skipIf(process.platform !== 'win32')('a live agent whose identity cannot be read blocks a second agent', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-unknown-'));
|
||||
const script = path.join(tmpDir, 'fake-agent.ts');
|
||||
fs.writeFileSync(script, '// no-op\n');
|
||||
const live = Bun.spawn([process.execPath, '-e', 'setInterval(() => {}, 1000)'], { stdio: ['ignore', 'ignore', 'ignore'] });
|
||||
writeAgentRecord(tmpDir, { pid: live.pid, gen, startTime: birth, startedAt: 0, ownerPid: process.pid, ownerStartTime: birth });
|
||||
const original = childProcess.spawnSync;
|
||||
const probe = spyOn(childProcess, 'spawnSync').mockImplementation(((command: string, args: string[], options: any) => (
|
||||
command === 'powershell.exe' ? { status: null, stdout: '', stderr: '' } : original(command, args, options)
|
||||
)) as typeof childProcess.spawnSync);
|
||||
const read = spyOn(win32CreationTimeSource, 'read').mockImplementation(() => undefined);
|
||||
const origSpawn = (Bun as any).spawn;
|
||||
let spawned = 0;
|
||||
(Bun as any).spawn = () => { spawned++; throw new Error('a second agent must not start'); };
|
||||
try {
|
||||
expect(spawnTerminalAgent({ stateFile: path.join(tmpDir, 'state.json'), serverPort: 12345, ownerPid: process.pid, cwd: tmpDir, scriptPath: script })).toBeNull();
|
||||
expect(spawned).toBe(0);
|
||||
expect(isProcessAlive(live.pid)).toBe(true);
|
||||
} finally {
|
||||
(Bun as any).spawn = origSpawn;
|
||||
read.mockRestore();
|
||||
probe.mockRestore();
|
||||
live.kill();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5 });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user