mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 18:28:15 +01:00
Stop macOS EPERM marking a finished QA capture as incomplete (#4170)
* Stop macOS EPERM marking a finished QA capture as incomplete On macOS the cleanup kill of an exited command's process group can get EPERM while the group is being torn down, and the capture then recorded a successful command as incomplete with exit code 2. On EPERM, check the group directly (kill with signal 0, then a ps listing that ignores zombie and exiting entries) and treat it as gone only when no live member remains. A live member or a failed check keeps the fail-closed result. Co-authored-by: vilano93 <244097767+vilano93@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the EPERM group check to macOS so Linux still fails closed Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Count exiting macOS group members as gone after a refused group kill On macOS, ps shows a process that is exiting with a trailing E flag (for example RE or UE), so a check on the leading letter alone read a mid-exit member as live and kept the capture incomplete. Count a member as gone when its state starts with Z or carries E; X means traced on macOS, so it stays live. Linux is unchanged and still fails closed. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Document the macOS EPERM exception in QA deadline cleanup Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: vilano93 <244097767+vilano93@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
4 files changed
+90
-2
No files matched your search
@@ -0,0 +1 @@
|
||||
- **QA captures on macOS no longer record a finished command as incomplete.** After a command exited, the cleanup kill of its process group could get `EPERM` from macOS while the group was being torn down, and `gstack-qa-evidence capture` then recorded `status: incomplete`, `exitCode: 2`. On macOS, an `EPERM` now makes the capture check the group directly and count it as gone only when every remaining member is a zombie or is exiting (`ps` state starting with `Z` or carrying the `E` flag); a live member, or a check that cannot run, still fails closed, and `EPERM` on other platforms fails closed as before. The macOS teardown state is inferred from XNU and `ps`, not measured. Reported with measurements by @vilano93 (#4166).
|
||||
@@ -73,6 +73,10 @@ including later report Writes and cleanup; it is not a completed-session measure
|
||||
or new-session descendants are outside that guarantee, so detached probes are
|
||||
unsupported. Force-killing the guard itself with SIGKILL also prevents its POSIX
|
||||
cleanup handler from running.
|
||||
- A cleanup kill that fails with anything other than "no such process" is a guard
|
||||
error (2), with one macOS exception: an EPERM while the group is being torn
|
||||
down counts as clean only when every remaining member is a zombie or exiting
|
||||
(ps state Z or flag E). A live member, or a failed check, still returns 2.
|
||||
- On Windows, dedicated nested Jobs contain the probe worker and its descendants,
|
||||
including children whose immediate parent exits. Failure to initialize this
|
||||
containment prevents the command from starting.
|
||||
|
||||
+13
-2
@@ -1,7 +1,7 @@
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { spawn } from 'node:child_process';
|
||||
import { spawn, spawnSync } from 'node:child_process';
|
||||
import { bunIsolationArgs } from './bun-isolation';
|
||||
import { constants as osConstants } from 'node:os';
|
||||
import { initializeWindowsReviewJob } from './claude-code-windows-job';
|
||||
@@ -120,6 +120,16 @@ export interface QaCommandCapture {
|
||||
spawn?: { cwd?: string; env?: Record<string, string> };
|
||||
}
|
||||
|
||||
function processGroupEmpty(pgid: number): boolean {
|
||||
try { process.kill(-pgid, 0); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ESRCH') return true; }
|
||||
const listing = spawnSync('ps', ['-A', '-o', 'pgid=,stat='], { encoding: 'utf8', timeout: 5_000 });
|
||||
if (listing.status !== 0) return false;
|
||||
return !listing.stdout.split('\n').some(line => {
|
||||
const [group, state = ''] = line.trim().split(/\s+/);
|
||||
return Number(group) === pgid && !state.startsWith('Z') && !state.includes('E');
|
||||
});
|
||||
}
|
||||
|
||||
export async function runQaDeadlineCommand(file: string, command: string, args: string[], emit: Emit, capture?: QaCommandCapture): Promise<number> {
|
||||
if (!['linux', 'darwin', 'win32'].includes(process.platform)) throw new QaDeadlineError('Process containment is unavailable on this platform');
|
||||
let status = qaDeadlineStatus(readQaDeadline(file));
|
||||
@@ -152,7 +162,8 @@ export async function runQaDeadlineCommand(file: string, command: string, args:
|
||||
if (process.platform === 'win32') child.kill('SIGKILL');
|
||||
else process.kill(-child.pid, 'SIGKILL');
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code !== 'ESRCH') outcome = 2;
|
||||
const code = (error as NodeJS.ErrnoException).code;
|
||||
if (code !== 'ESRCH' && !(code === 'EPERM' && process.platform === 'darwin' && processGroupEmpty(child.pid))) outcome = 2;
|
||||
}
|
||||
};
|
||||
const finish = (code: number, signal: NodeJS.Signals | null = null, completed = false) => {
|
||||
|
||||
@@ -569,3 +569,75 @@ test('materialize accepts a single limits string as a one-item list', () => {
|
||||
expect(report.status, report.stderr).toBe(0);
|
||||
expect(JSON.parse(fs.readFileSync(path.join(f.root, 'evidence.json'), 'utf8')).limits).toEqual(['One probe only.']);
|
||||
});
|
||||
|
||||
function refusedGroupKill(f: { root: string }, platform: NodeJS.Platform) {
|
||||
const preload = path.join(f.root, 'refused-group-kill.ts');
|
||||
const refused = path.join(f.root, 'refused-group-kill');
|
||||
const group = path.join(f.root, 'refused-group');
|
||||
fs.writeFileSync(preload, `import { appendFileSync, writeFileSync } from 'node:fs';
|
||||
if (process.argv[1] === ${JSON.stringify(CLI)}) {
|
||||
Object.defineProperty(process, 'platform', { value: ${JSON.stringify(platform)} });
|
||||
const kill = process.kill.bind(process);
|
||||
process.kill = (pid, signal) => {
|
||||
if (pid >= 0) return kill(pid, signal);
|
||||
appendFileSync(${JSON.stringify(refused)}, String(signal ?? 'SIGTERM') + '\\n');
|
||||
writeFileSync(${JSON.stringify(group)}, String(-pid));
|
||||
throw Object.assign(new Error('kill EPERM'), { code: 'EPERM', errno: -1, syscall: 'kill' });
|
||||
};
|
||||
}`);
|
||||
return { preload, refused, group };
|
||||
}
|
||||
|
||||
test.skipIf(process.platform === 'win32')('a refused group kill after the command exits still records a complete capture once the group is empty', () => {
|
||||
const f = fixture();
|
||||
const stub = refusedGroupKill(f, 'darwin');
|
||||
const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'],
|
||||
{ cwd: f.root, encoding: 'utf8', timeout: 10_000 });
|
||||
expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL');
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
expect(receipt(result.stdout)).toMatchObject({ status: 'complete', exitCode: 0 });
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('a refused group kill off macOS fails closed even once the group is empty', () => {
|
||||
const f = fixture();
|
||||
const stub = refusedGroupKill(f, 'linux');
|
||||
const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'],
|
||||
{ cwd: f.root, encoding: 'utf8', timeout: 10_000 });
|
||||
expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL');
|
||||
expect(result.status).not.toBe(0);
|
||||
expect(receipt(result.stdout)).toMatchObject({ status: 'incomplete', exitCode: 2 });
|
||||
});
|
||||
|
||||
test.skipIf(process.platform === 'win32')('a refused group kill fails closed while the group still has a live member', () => {
|
||||
const f = fixture();
|
||||
const stub = refusedGroupKill(f, 'darwin');
|
||||
const marker = path.join(f.root, 'member.pid');
|
||||
const program = `const member = require('node:child_process').spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); require('node:fs').writeFileSync(${JSON.stringify(marker)}, String(member.pid)); member.unref(); console.log('{}');`;
|
||||
let pid = 0;
|
||||
try {
|
||||
const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', program],
|
||||
{ cwd: f.root, encoding: 'utf8', timeout: 10_000 });
|
||||
pid = Number(fs.readFileSync(marker, 'utf8'));
|
||||
expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL');
|
||||
expect(() => process.kill(pid, 0)).not.toThrow();
|
||||
expect(result.status).not.toBe(0);
|
||||
expect(receipt(result.stdout)).toMatchObject({ status: 'incomplete', exitCode: 2 });
|
||||
} finally {
|
||||
if (pid > 0) try { process.kill(pid, 'SIGKILL'); } catch {}
|
||||
}
|
||||
});
|
||||
|
||||
for (const [state, status, exitCode] of [['RE', 'complete', 0], ['UE', 'complete', 0], ['R', 'incomplete', 2], ['S', 'incomplete', 2], ['X', 'incomplete', 2], ['RX', 'incomplete', 2]] as const) {
|
||||
test.skipIf(process.platform === 'win32')(`a refused macOS group kill reads a listed member in ps state ${state} as ${status}`, () => {
|
||||
const f = fixture();
|
||||
const stub = refusedGroupKill(f, 'darwin');
|
||||
const bin = path.join(f.root, 'bin');
|
||||
fs.mkdirSync(bin);
|
||||
fs.writeFileSync(path.join(bin, 'ps'), `#!/bin/sh\nprintf '%s ${state}\\n' "$(cat ${JSON.stringify(stub.group)})"\n`, { mode: 0o755 });
|
||||
const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'],
|
||||
{ cwd: f.root, encoding: 'utf8', timeout: 10_000, env: { ...process.env, PATH: `${bin}${path.delimiter}${process.env.PATH}` } });
|
||||
expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL');
|
||||
expect(result.status, result.stderr).toBe(exitCode);
|
||||
expect(receipt(result.stdout)).toMatchObject({ status, exitCode });
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user