diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b6298d78..1b34046ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to GitNexus will be documented in this file. ## [Unreleased] +### Fixed + +- **Hook db-lock probe no longer strands unkillable `lsof`/`ps` orphans** — the probe's `lsof`/`ps` subprocesses are now wrapped in a self-tested coreutils `timeout`/`gtimeout` (`timeout -k 1 …`), so a hook SIGKILLed by the runner's 10s timeout can no longer leave `lsof` running forever (orphan lifetime bounded at ~3s); `acquireHookSlot` now also gates the probe itself, capping concurrent probes at 3 per repo. Opt out with `GITNEXUS_HOOK_TIMEOUT_PATH=disabled`. (#2163) + ### Changed - Migrated from KuzuDB to LadybugDB v0.15 (`@ladybugdb/core`, `@ladybugdb/wasm-core`) - Renamed all internal paths from `kuzu` to `lbug` (storage: `.gitnexus/kuzu` → `.gitnexus/lbug`) diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index c3ec2ecf5..5274ef4c0 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -276,21 +276,34 @@ function handlePreToolUse(input) { const pattern = extractPattern(toolName, toolInput); if (!pattern || pattern.length < 3) return; - if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB, so the CLI augment would - // contend on the lock. Stay silent for strict hook runners (issue #1913); - // surface the reason only when diagnostics are explicitly requested. + + // Acquire the per-repo slot BEFORE the DB-owner probe (#2163): the probe + // itself spawns lsof/ps, so it must be bounded by the same ≤3-per-repo cap + // as the augment, or concurrent sessions fan out unbounded probe + // subprocesses. Keep the acquire right after the cheap guards above — + // moving it earlier would churn slot files on tool calls that never probe. + const release = acquireHookSlot(gitNexusDir); + if (!release) { + // Normal skip path: all per-repo hook slots are held by concurrent + // sessions. Stay silent for strict hook runners (issue #1913); surface + // the reason only when diagnostics are explicitly requested. if (isDebugEnabled()) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + process.stderr.write('[GitNexus] augment skipped: hook slots saturated\n'); } return; } - const release = acquireHookSlot(gitNexusDir); - if (!release) return; - let result = ''; try { + if (hasGitNexusServerOwner(gitNexusDir)) { + // Normal skip path: the MCP server owns the DB, so the CLI augment would + // contend on the lock. Stay silent for strict hook runners (issue #1913); + // surface the reason only when diagnostics are explicitly requested. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } + return; + } const child = runGitNexusCli(['augment', '--', pattern], cwd, 7000); if (!child.error && child.status === 0) { result = extractAugmentContext(child.stderr || ''); diff --git a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs index 752c114a7..5c67804b1 100644 --- a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs +++ b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs @@ -11,6 +11,22 @@ * * Fail-open on most errors; fail-closed only on lsof ETIMEDOUT (Unix) or * PowerShell ETIMEDOUT (Windows), matching the hook contract. + * + * Unix subprocess containment contract (#2163): + * - lsof/ps are wrapped in coreutils `timeout`/`gtimeout` when a working + * wrapper is found (`timeout -k 1 lsof ...`). If this hook process + * is itself SIGKILLed (e.g. by the runner's 10s hook timeout) the wrapper + * survives, SIGTERMs its child at the budget (2s lsof / 1s ps) and SIGKILLs + * it 1s later — orphan lifetime is bounded at ~3s instead of unbounded. + * - GITNEXUS_HOOK_TIMEOUT_PATH: the sentinel value `disabled` switches the + * wrapper off deterministically; any other value is adopted only when it + * exists AND passes a one-shot `-k` self-test — otherwise resolution FALLS + * THROUGH to the built-in candidate list (first self-test pass wins), so + * no malformed value of any shape can silently disable orphan containment. + * - The gitnexus server is lazy-open + sticky-hold: an idle MCP server holds + * ZERO lbug fds until the repo's first MCP query, then keeps the fd open. + * A probe before that first query is therefore always false — a known, + * pre-existing race, not a bug in this probe. */ const fs = require('fs'); @@ -46,6 +62,80 @@ function resolveHookBinary(tool) { return tool; } +// Sentinel: +// undefined = not resolved yet (resolve lazily, on first lsof/ps fallback) +// string = self-tested coreutils timeout/gtimeout path (use as wrapper) +// null = no usable wrapper (disabled, none found, or self-test failed) +let unixGuardTimeoutCache; + +/** + * Resolve a coreutils `timeout`/`gtimeout` binary to wrap lsof/ps with + * (#2163). Dead code on Windows (the win32 dispatch returns earlier). + * + * GITNEXUS_HOOK_TIMEOUT_PATH semantics: the sentinel `disabled` turns the + * wrapper off; any other value is only a CANDIDATE — an existing file path + * is tried first, but it must pass the `-k` self-test to be adopted. On any + * failure (non-existent path, directory, non-executable file, wrapper + * without `-k` support, …) resolution falls through to the built-in + * candidates below, tried in order, first self-test pass wins. This is + * strictly stronger than the sibling GITNEXUS_HOOK_LSOF_PATH / + * GITNEXUS_HOOK_PS_PATH overrides (which only check existence): no bad env + * value of ANY shape can silently disable orphan containment. + * + * Lazy self-test: candidates are probed only when the lsof/ps fallback is + * first reached, and the result is memoized. A candidate is adopted only + * when `timeout -k 1 1 /bin/sh -c :` exits 0. This rejects wrappers that do + * not support the coreutils `-k` flag — busybox <1.34, toybox, broken + * symlinks — which would otherwise exit with a usage error without ever + * running lsof, silently converting the lsof-ETIMEDOUT fail-closed contract + * into fail-open (#1492 regression). Only when EVERY candidate fails does + * the probe fall back to the unwrapped status quo (memoized null). + * busybox ≥1.34 passes the test and is fully usable (capability, not + * identity, decides). + */ +function passesGuardSelfTest(guard) { + try { + const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', ':'], { + encoding: 'utf-8', + timeout: 3000, + stdio: ['ignore', 'ignore', 'ignore'], + windowsHide: true, + }); + return !selfTest.error && selfTest.status === 0; + } catch { + return false; + } +} + +function resolveUnixGuardTimeout() { + if (unixGuardTimeoutCache !== undefined) return unixGuardTimeoutCache; + unixGuardTimeoutCache = null; + const fromEnv = process.env.GITNEXUS_HOOK_TIMEOUT_PATH; + const trimmed = fromEnv ? String(fromEnv).trim() : ''; + if (trimmed === 'disabled') return unixGuardTimeoutCache; + const candidates = []; + if (trimmed && fs.existsSync(trimmed)) candidates.push(trimmed); + for (const builtin of [ + '/usr/bin/timeout', + '/bin/timeout', + '/opt/homebrew/bin/gtimeout', + '/usr/local/bin/gtimeout', + ]) { + try { + if (fs.existsSync(builtin)) candidates.push(builtin); + } catch { + /* ignore */ + } + } + for (const candidate of candidates) { + if (passesGuardSelfTest(candidate)) { + unixGuardTimeoutCache = candidate; + break; + } + } + return unixGuardTimeoutCache; +} + function resolveWindowsPowerShellPath() { const fromEnv = process.env.GITNEXUS_HOOK_POWERSHELL_PATH; if (fromEnv && String(fromEnv).trim() && fs.existsSync(String(fromEnv).trim())) { @@ -188,20 +278,46 @@ function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { } function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { + const guard = resolveUnixGuardTimeout(); const lsofPath = resolveHookBinary('lsof'); - const lsof = spawnSync(lsofPath, ['-nP', '-t', '--', dbPathAbs], { + // The spawnSync timeouts below (lsof 1000ms / ps 500ms) are deliberately + // SHORTER than the wrapper budgets (2s / 1s): on the supervised path Node's + // SIGTERM always fires first, so `error.code === 'ETIMEDOUT'` and the + // fail-closed contract are untouched. The wrapper only matters once this + // hook process has been SIGKILLed and can no longer deliver that SIGTERM. + const [lsofCmd, lsofArgs] = guard + ? [guard, ['-k', '1', '2', lsofPath, '-nP', '-t', '--', dbPathAbs]] + : [lsofPath, ['-nP', '-t', '--', dbPathAbs]]; + const lsof = spawnSync(lsofCmd, lsofArgs, { encoding: 'utf-8', timeout: 1000, stdio: ['ignore', 'pipe', 'ignore'], windowsHide: true, }); if (lsof.error) return lsof.error.code === 'ETIMEDOUT'; + // Guard-mediated deaths map to "unresponsive holder" (fail-closed). Three + // result shapes, verified against coreutils 9.1: + // - signal-death: when `-k` escalates to SIGKILL, coreutils timeout + // SELF-RAISES the signal, so spawnSync reports {status: null, signal} + // with no .error (spawnSync's own ETIMEDOUT was handled above). The + // same shape appears when this hook is frozen >2s (SIGSTOP, laptop + // suspend) and the guard expires while it sleeps. By construction, a + // guard-wrapped probe that died by signal without spawnSync ETIMEDOUT + // is a budget/kill outcome. + // - 124: budget expired and the child exited after the plain SIGTERM. + // - 137: NOT the coreutils -k path — only exit-code-propagating wrappers, + // or a child SIGKILLed externally (e.g. the OOM killer). + if (guard && lsof.status === null && lsof.signal) return true; + if (guard && (lsof.status === 124 || lsof.status === 137)) return true; const pids = (lsof.stdout || '').split(/\s+/).filter(Boolean); const psPath = resolveHookBinary('ps'); for (const pid of pids) { if (Number(pid) === myPid) continue; - const ps = spawnSync(psPath, ['-p', pid, '-o', 'command='], { + const [psCmd, psArgs] = guard + ? [guard, ['-k', '1', '1', psPath, '-p', pid, '-o', 'command=']] + : [psPath, ['-p', pid, '-o', 'command=']]; + const ps = spawnSync(psCmd, psArgs, { encoding: 'utf-8', timeout: 500, stdio: ['ignore', 'pipe', 'ignore'], @@ -211,6 +327,11 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { if (ps.error.code === 'ETIMEDOUT') return true; continue; } + // Same guard-mediated-death mapping as the lsof call above (signal-death + // from the -k escalation or a frozen hook; 124 budget expiry; 137 only + // for exit-code-propagating wrappers / external SIGKILL). + if (guard && ps.status === null && ps.signal) return true; + if (guard && (ps.status === 124 || ps.status === 137)) return true; if (isGitNexusServerCommand(ps.stdout || '')) return true; } return false; diff --git a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs index 0d837fb2c..bd72c7b55 100755 --- a/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs +++ b/gitnexus/hooks/antigravity/gitnexus-antigravity-hook.cjs @@ -284,18 +284,32 @@ function buildAfterToolContext(input) { } function runAugment(gitNexusDir, cwd, pattern) { - if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB. Stay silent for strict - // hook runners (issue #1913); surface the reason only under GITNEXUS_DEBUG. + // Acquire the per-repo slot BEFORE the DB-owner probe (#2163): the probe + // itself spawns lsof/ps, so it must be bounded by the same ≤3-per-repo cap + // as the augment, or concurrent sessions fan out unbounded probe + // subprocesses. The cheap guards (extractPattern, gitNexusDir lookup) run in + // buildAfterToolContext before this — moving the acquire any earlier would + // churn slot files on tool calls that never probe. + const release = acquireHookSlot(gitNexusDir); + if (!release) { + // Normal skip path: all per-repo hook slots are held by concurrent + // sessions. Stay silent for strict hook runners (issue #1913); surface + // the reason only under GITNEXUS_DEBUG. if (isDebugEnabled()) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + process.stderr.write('[GitNexus] augment skipped: hook slots saturated\n'); } return ''; } - const release = acquireHookSlot(gitNexusDir); - if (!release) return ''; - const cliPath = resolveCliPath(); try { + if (hasGitNexusServerOwner(gitNexusDir)) { + // Normal skip path: the MCP server owns the DB. Stay silent for strict + // hook runners (issue #1913); surface the reason only under GITNEXUS_DEBUG. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } + return ''; + } + const cliPath = resolveCliPath(); const child = runGitNexusCli(cliPath, ['augment', '--', pattern], cwd, 7000); if (!child.error && child.status === 0) { return extractAugmentContext(child.stderr || ''); diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index 40d0b08df..c37895f1d 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -259,22 +259,35 @@ function handlePreToolUse(input) { const pattern = extractPattern(toolName, toolInput); if (!pattern || pattern.length < 3) return; - if (hasGitNexusServerOwner(gitNexusDir)) { - // Normal skip path: the MCP server owns the DB, so the CLI augment would - // contend on the lock. Stay silent for strict hook runners (issue #1913); - // surface the reason only when diagnostics are explicitly requested. + + // Acquire the per-repo slot BEFORE the DB-owner probe (#2163): the probe + // itself spawns lsof/ps, so it must be bounded by the same ≤3-per-repo cap + // as the augment, or concurrent sessions fan out unbounded probe + // subprocesses. Keep the acquire right after the cheap guards above — + // moving it earlier would churn slot files on tool calls that never probe. + const release = acquireHookSlot(gitNexusDir); + if (!release) { + // Normal skip path: all per-repo hook slots are held by concurrent + // sessions. Stay silent for strict hook runners (issue #1913); surface + // the reason only when diagnostics are explicitly requested. if (isDebugEnabled()) { - process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + process.stderr.write('[GitNexus] augment skipped: hook slots saturated\n'); } return; } - const release = acquireHookSlot(gitNexusDir); - if (!release) return; - - const cliPath = resolveCliPath(); let result = ''; try { + if (hasGitNexusServerOwner(gitNexusDir)) { + // Normal skip path: the MCP server owns the DB, so the CLI augment would + // contend on the lock. Stay silent for strict hook runners (issue #1913); + // surface the reason only when diagnostics are explicitly requested. + if (isDebugEnabled()) { + process.stderr.write('[GitNexus] augment skipped: MCP server owns DB\n'); + } + return; + } + const cliPath = resolveCliPath(); const child = runGitNexusCli(cliPath, ['augment', '--', pattern], cwd, 7000); if (!child.error && child.status === 0) { result = extractAugmentContext(child.stderr || ''); diff --git a/gitnexus/hooks/claude/hook-db-lock-probe.cjs b/gitnexus/hooks/claude/hook-db-lock-probe.cjs index 752c114a7..5c67804b1 100644 --- a/gitnexus/hooks/claude/hook-db-lock-probe.cjs +++ b/gitnexus/hooks/claude/hook-db-lock-probe.cjs @@ -11,6 +11,22 @@ * * Fail-open on most errors; fail-closed only on lsof ETIMEDOUT (Unix) or * PowerShell ETIMEDOUT (Windows), matching the hook contract. + * + * Unix subprocess containment contract (#2163): + * - lsof/ps are wrapped in coreutils `timeout`/`gtimeout` when a working + * wrapper is found (`timeout -k 1 lsof ...`). If this hook process + * is itself SIGKILLed (e.g. by the runner's 10s hook timeout) the wrapper + * survives, SIGTERMs its child at the budget (2s lsof / 1s ps) and SIGKILLs + * it 1s later — orphan lifetime is bounded at ~3s instead of unbounded. + * - GITNEXUS_HOOK_TIMEOUT_PATH: the sentinel value `disabled` switches the + * wrapper off deterministically; any other value is adopted only when it + * exists AND passes a one-shot `-k` self-test — otherwise resolution FALLS + * THROUGH to the built-in candidate list (first self-test pass wins), so + * no malformed value of any shape can silently disable orphan containment. + * - The gitnexus server is lazy-open + sticky-hold: an idle MCP server holds + * ZERO lbug fds until the repo's first MCP query, then keeps the fd open. + * A probe before that first query is therefore always false — a known, + * pre-existing race, not a bug in this probe. */ const fs = require('fs'); @@ -46,6 +62,80 @@ function resolveHookBinary(tool) { return tool; } +// Sentinel: +// undefined = not resolved yet (resolve lazily, on first lsof/ps fallback) +// string = self-tested coreutils timeout/gtimeout path (use as wrapper) +// null = no usable wrapper (disabled, none found, or self-test failed) +let unixGuardTimeoutCache; + +/** + * Resolve a coreutils `timeout`/`gtimeout` binary to wrap lsof/ps with + * (#2163). Dead code on Windows (the win32 dispatch returns earlier). + * + * GITNEXUS_HOOK_TIMEOUT_PATH semantics: the sentinel `disabled` turns the + * wrapper off; any other value is only a CANDIDATE — an existing file path + * is tried first, but it must pass the `-k` self-test to be adopted. On any + * failure (non-existent path, directory, non-executable file, wrapper + * without `-k` support, …) resolution falls through to the built-in + * candidates below, tried in order, first self-test pass wins. This is + * strictly stronger than the sibling GITNEXUS_HOOK_LSOF_PATH / + * GITNEXUS_HOOK_PS_PATH overrides (which only check existence): no bad env + * value of ANY shape can silently disable orphan containment. + * + * Lazy self-test: candidates are probed only when the lsof/ps fallback is + * first reached, and the result is memoized. A candidate is adopted only + * when `timeout -k 1 1 /bin/sh -c :` exits 0. This rejects wrappers that do + * not support the coreutils `-k` flag — busybox <1.34, toybox, broken + * symlinks — which would otherwise exit with a usage error without ever + * running lsof, silently converting the lsof-ETIMEDOUT fail-closed contract + * into fail-open (#1492 regression). Only when EVERY candidate fails does + * the probe fall back to the unwrapped status quo (memoized null). + * busybox ≥1.34 passes the test and is fully usable (capability, not + * identity, decides). + */ +function passesGuardSelfTest(guard) { + try { + const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', ':'], { + encoding: 'utf-8', + timeout: 3000, + stdio: ['ignore', 'ignore', 'ignore'], + windowsHide: true, + }); + return !selfTest.error && selfTest.status === 0; + } catch { + return false; + } +} + +function resolveUnixGuardTimeout() { + if (unixGuardTimeoutCache !== undefined) return unixGuardTimeoutCache; + unixGuardTimeoutCache = null; + const fromEnv = process.env.GITNEXUS_HOOK_TIMEOUT_PATH; + const trimmed = fromEnv ? String(fromEnv).trim() : ''; + if (trimmed === 'disabled') return unixGuardTimeoutCache; + const candidates = []; + if (trimmed && fs.existsSync(trimmed)) candidates.push(trimmed); + for (const builtin of [ + '/usr/bin/timeout', + '/bin/timeout', + '/opt/homebrew/bin/gtimeout', + '/usr/local/bin/gtimeout', + ]) { + try { + if (fs.existsSync(builtin)) candidates.push(builtin); + } catch { + /* ignore */ + } + } + for (const candidate of candidates) { + if (passesGuardSelfTest(candidate)) { + unixGuardTimeoutCache = candidate; + break; + } + } + return unixGuardTimeoutCache; +} + function resolveWindowsPowerShellPath() { const fromEnv = process.env.GITNEXUS_HOOK_POWERSHELL_PATH; if (fromEnv && String(fromEnv).trim() && fs.existsSync(String(fromEnv).trim())) { @@ -188,20 +278,46 @@ function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { } function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { + const guard = resolveUnixGuardTimeout(); const lsofPath = resolveHookBinary('lsof'); - const lsof = spawnSync(lsofPath, ['-nP', '-t', '--', dbPathAbs], { + // The spawnSync timeouts below (lsof 1000ms / ps 500ms) are deliberately + // SHORTER than the wrapper budgets (2s / 1s): on the supervised path Node's + // SIGTERM always fires first, so `error.code === 'ETIMEDOUT'` and the + // fail-closed contract are untouched. The wrapper only matters once this + // hook process has been SIGKILLed and can no longer deliver that SIGTERM. + const [lsofCmd, lsofArgs] = guard + ? [guard, ['-k', '1', '2', lsofPath, '-nP', '-t', '--', dbPathAbs]] + : [lsofPath, ['-nP', '-t', '--', dbPathAbs]]; + const lsof = spawnSync(lsofCmd, lsofArgs, { encoding: 'utf-8', timeout: 1000, stdio: ['ignore', 'pipe', 'ignore'], windowsHide: true, }); if (lsof.error) return lsof.error.code === 'ETIMEDOUT'; + // Guard-mediated deaths map to "unresponsive holder" (fail-closed). Three + // result shapes, verified against coreutils 9.1: + // - signal-death: when `-k` escalates to SIGKILL, coreutils timeout + // SELF-RAISES the signal, so spawnSync reports {status: null, signal} + // with no .error (spawnSync's own ETIMEDOUT was handled above). The + // same shape appears when this hook is frozen >2s (SIGSTOP, laptop + // suspend) and the guard expires while it sleeps. By construction, a + // guard-wrapped probe that died by signal without spawnSync ETIMEDOUT + // is a budget/kill outcome. + // - 124: budget expired and the child exited after the plain SIGTERM. + // - 137: NOT the coreutils -k path — only exit-code-propagating wrappers, + // or a child SIGKILLed externally (e.g. the OOM killer). + if (guard && lsof.status === null && lsof.signal) return true; + if (guard && (lsof.status === 124 || lsof.status === 137)) return true; const pids = (lsof.stdout || '').split(/\s+/).filter(Boolean); const psPath = resolveHookBinary('ps'); for (const pid of pids) { if (Number(pid) === myPid) continue; - const ps = spawnSync(psPath, ['-p', pid, '-o', 'command='], { + const [psCmd, psArgs] = guard + ? [guard, ['-k', '1', '1', psPath, '-p', pid, '-o', 'command=']] + : [psPath, ['-p', pid, '-o', 'command=']]; + const ps = spawnSync(psCmd, psArgs, { encoding: 'utf-8', timeout: 500, stdio: ['ignore', 'pipe', 'ignore'], @@ -211,6 +327,11 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { if (ps.error.code === 'ETIMEDOUT') return true; continue; } + // Same guard-mediated-death mapping as the lsof call above (signal-death + // from the -k escalation or a frozen hook; 124 budget expiry; 137 only + // for exit-code-propagating wrappers / external SIGKILL). + if (guard && ps.status === null && ps.signal) return true; + if (guard && (ps.status === 124 || ps.status === 137)) return true; if (isGitNexusServerCommand(ps.stdout || '')) return true; } return false; diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index 54193d837..12f38e9c9 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -842,10 +842,514 @@ describe('Cross-platform DB lock probe (source)', () => { expect(p).toContain('GITNEXUS_HOOK_LSOF_PATH'); expect(p).toContain('GITNEXUS_HOOK_POWERSHELL_PATH'); expect(p).toContain('GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS'); + // #2163: lsof/ps orphan containment via a self-tested coreutils + // timeout/gtimeout wrapper. + expect(p).toContain('GITNEXUS_HOOK_TIMEOUT_PATH'); + expect(p).toContain('resolveUnixGuardTimeout'); + }); + } + + // T5 (#2163): the two probe copies were only kept in sync by convention + // (setup.ts copies the canonical gitnexus/hooks/claude/ file; the plugin + // ships its own). Enforce byte-parity in CI, mirroring the + // resolve-analyze-cmd.cjs parity test. `.gitattributes` pins `eol=lf` + // repo-wide, so the byte comparison is safe on the Windows lane too. + it('keeps the two hook-db-lock-probe.cjs copies byte-identical', () => { + expect(fs.readFileSync(CJS_HOOK_DB_PROBE, 'utf-8')).toBe( + fs.readFileSync(PLUGIN_HOOK_DB_PROBE, 'utf-8'), + ); + }); +}); + +// ─── Source: hook slot must gate the DB-owner probe (#2163) ────────── + +describe('Hook slot gates the DB-owner probe (source order, #2163)', () => { + const ANTIGRAVITY_HOOK = path.resolve( + __dirname, + '..', + '..', + 'hooks', + 'antigravity', + 'gitnexus-antigravity-hook.cjs', + ); + + // T1: pin cheap guards → acquireHookSlot → probe. The probe spawns lsof/ps, + // so it must sit BEHIND the per-repo slot cap; and the acquire must stay + // AFTER the cheap gating (extractPattern), or every tool call churns slot + // files. The antigravity adapter splits the cheap gating (extractPattern in + // buildAfterToolContext) from probe+augment (runAugment), so its slice + // spans both functions to express the same call-order contract. + for (const [label, hookPath, sliceStart, sliceEnd] of [ + ['CJS', CJS_HOOK, 'function handlePreToolUse', 'function handlePostToolUse'], + ['Plugin', PLUGIN_HOOK, 'function handlePreToolUse', 'function handlePostToolUse'], + [ + 'Antigravity', + ANTIGRAVITY_HOOK, + 'function buildAfterToolContext', + 'function buildStaleIndexHint', + ], + ] as const) { + it(`${label}: extractPattern → acquireHookSlot → hasGitNexusServerOwner`, () => { + const source = fs.readFileSync(hookPath, 'utf-8'); + const start = source.indexOf(sliceStart); + const end = source.indexOf(sliceEnd); + expect(start).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + const slice = source.slice(start, end); + const patternIdx = slice.indexOf('extractPattern('); + const acquireIdx = slice.indexOf('acquireHookSlot('); + const probeIdx = slice.indexOf('hasGitNexusServerOwner('); + expect(patternIdx).toBeGreaterThanOrEqual(0); + expect(acquireIdx).toBeGreaterThan(patternIdx); + expect(probeIdx).toBeGreaterThan(acquireIdx); }); } }); +// ─── Behavior: slot-gated probe + wrapper-reaped orphans (#2163) ───── + +describe.skipIf(process.platform === 'win32')( + 'DB-owner probe is gated behind the hook slot (behavior, #2163)', + () => { + for (const [label, hookPath] of [ + ['CJS', CJS_HOOK], + ['Plugin', PLUGIN_HOOK], + ] as const) { + it(`${label}: when all slots are full, the lsof probe never runs`, async () => { + const { spawn } = await import('child_process'); + const lockDir = path.join(gitNexusDir, '.hook-locks'); + fs.mkdirSync(lockDir, { recursive: true }); + // REQUIRED: the probe's first guard is + // `if (!fs.existsSync(dbPath)) return false;` — without a real lbug + // file the probe never reaches lsof even before the fix and this + // test would pass vacuously. + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + const lsofMarkerPath = path.join(os.tmpdir(), `gn-hook-slotgate-${process.pid}-${label}`); + fs.rmSync(lsofMarkerPath, { force: true }); + const binDir = createHookToolDir({ + lsofMarkerPath, + lsofOutput: '12345\n', + psOutput: 'node /tmp/node_modules/.bin/gitnexus mcp\n', + }); + + // Fill all 3 slots with live sleeper PIDs (same pattern as the + // concurrency-guard integration tests above). + const sleepers = [0, 1, 2].map(() => + spawn(process.execPath, ['-e', 'setTimeout(()=>{},60000)'], { + stdio: 'ignore', + detached: false, + }), + ); + const writtenLocks: string[] = []; + try { + for (let i = 0; i < sleepers.length; i++) { + const p = path.join(lockDir, `slot-${i}.lock`); + fs.writeFileSync(p, String(sleepers[i].pid)); + writtenLocks.push(p); + } + + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + // Force the Linux /proc scan to fall through to lsof + // immediately. Must be '1' — do NOT "simplify" to '0': the + // current parser (`Number(raw && String(raw).trim())`) + // treats '0' as falsy and falls back to the 1200ms default. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + }, + }, + ); + + expect(result.stdout.trim()).toBe(''); + // Core assertion: the probe (and therefore its lsof child) never + // ran — the slot gate now sits in front of it. Before the fix the + // probe ran un-gated and the marker existed. + expect(fs.existsSync(lsofMarkerPath)).toBe(false); + } finally { + for (const child of sleepers) { + try { + child.kill(); + } catch { + /* ignore */ + } + } + for (const p of writtenLocks) { + try { + fs.unlinkSync(p); + } catch { + /* ignore */ + } + } + try { + fs.rmdirSync(lockDir); + } catch { + /* ignore */ + } + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(lsofMarkerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + } + + // F5-3 (#2165 review): behavior-level slot-gate coverage for the + // ANTIGRAVITY adapter (the loop above only covers CJS/Plugin; the + // antigravity copy was pinned at source level only). The source adapter + // requires sibling helpers that live in hooks/claude/ — it is designed to + // be installed by copy (see the antigravity e2e suite) — so stage adapter + // + helpers into a temp dir and spawn that copy directly. + it('Antigravity: when all slots are full, the lsof probe never runs', async () => { + const { spawn } = await import('child_process'); + const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-antigravity-stage-')); + const antigravitySrc = path.resolve( + __dirname, + '..', + '..', + 'hooks', + 'antigravity', + 'gitnexus-antigravity-hook.cjs', + ); + const claudeHooksDir = path.resolve(__dirname, '..', '..', 'hooks', 'claude'); + const stagedHook = path.join(stageDir, 'gitnexus-antigravity-hook.cjs'); + fs.copyFileSync(antigravitySrc, stagedHook); + for (const helper of [ + 'hook-lock.cjs', + 'hook-db-lock-probe.cjs', + 'resolve-analyze-cmd.cjs', + 'win-rm-list-json.ps1', + ]) { + fs.copyFileSync(path.join(claudeHooksDir, helper), path.join(stageDir, helper)); + } + + const lockDir = path.join(gitNexusDir, '.hook-locks'); + fs.mkdirSync(lockDir, { recursive: true }); + // REQUIRED: without a real lbug file the probe never reaches lsof even + // before the fix and this test would pass vacuously. + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + const lsofMarkerPath = path.join(os.tmpdir(), `gn-hook-slotgate-${process.pid}-antigravity`); + fs.rmSync(lsofMarkerPath, { force: true }); + const binDir = createHookToolDir({ + lsofMarkerPath, + lsofOutput: '12345\n', + psOutput: 'node /tmp/node_modules/.bin/gitnexus mcp\n', + }); + + const sleepers = [0, 1, 2].map(() => + spawn(process.execPath, ['-e', 'setTimeout(()=>{},60000)'], { + stdio: 'ignore', + detached: false, + }), + ); + const writtenLocks: string[] = []; + try { + for (let i = 0; i < sleepers.length; i++) { + const p = path.join(lockDir, `slot-${i}.lock`); + fs.writeFileSync(p, String(sleepers[i].pid)); + writtenLocks.push(p); + } + + const result = runHook( + stagedHook, + { + hook_event_name: 'AfterTool', + tool_name: 'search_file_content', + tool_input: { pattern: 'validateUser' }, + tool_response: { llmContent: '...' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + // '1', NOT '0' — see the CJS/Plugin slot-gate test above. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + }, + }, + ); + + // Guards against a vacuous pass: if the staged copy crashes (e.g. a + // future sibling require missing from the staging list), stdout is + // empty and the marker absent for the wrong reason. + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe(''); + // Core assertion: the probe (and therefore its lsof child) never ran — + // runAugment bailed at the slot gate before hasGitNexusServerOwner. + expect(fs.existsSync(lsofMarkerPath)).toBe(false); + } finally { + for (const child of sleepers) { + try { + child.kill(); + } catch { + /* ignore */ + } + } + for (const p of writtenLocks) { + try { + fs.unlinkSync(p); + } catch { + /* ignore */ + } + } + try { + fs.rmdirSync(lockDir); + } catch { + /* ignore */ + } + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(lsofMarkerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + fs.rmSync(stageDir, { recursive: true, force: true }); + } + }); + }, +); + +describe.skipIf(process.platform !== 'linux')( + 'Orphaned lsof is reaped by the timeout wrapper (#2163)', + () => { + // T3 — minimal reproduction of the incident mechanism: the hook process + // is SIGKILLed (modeling Claude Code's 10s hook timeout) while a slow, + // SIGTERM-immune lsof child is still running. Before the fix nothing can + // signal that child anymore (and spawnSync's own SIGTERM is ignored + // anyway), so it survives its full 30s sleep → test red regardless of + // race timing. After the fix the coreutils `timeout -k 1` wrapper + // outlives the hook and SIGKILLs the child within ~3s — making this also + // a direct regression test for the wrapper's `-k` capability. + it('CJS: SIGKILLed hook leaves no immortal lsof child', async () => { + const { spawn } = await import('child_process'); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + const pidFile = path.join(os.tmpdir(), `gn-hook-lsofpid-${process.pid}`); + fs.rmSync(pidFile, { force: true }); + const binDir = createHookToolDir({ + lsofPidFile: pidFile, + lsofSleepMs: 30000, + lsofIgnoreSigterm: true, + }); + let lsofPid = 0; + let hookChild: ReturnType | null = null; + + const isFakeLsofAlive = () => { + try { + process.kill(lsofPid, 0); + } catch { + return false; // ESRCH — reaped + } + // PID-reuse guard: only count it alive while the cmdline still + // points at our fake lsof. + try { + return fs.readFileSync(`/proc/${lsofPid}/cmdline`, 'utf-8').includes(binDir); + } catch { + return false; + } + }; + + try { + hookChild = spawn(process.execPath, [CJS_HOOK], { + stdio: ['pipe', 'ignore', 'ignore'], + env: { + ...hookEnv(binDir), + // '1', NOT '0' — see the slot-gate test above. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + // Hermeticity: hookEnv() spreads process.env, so a stray + // GITNEXUS_HOOK_TIMEOUT_PATH=disabled left in a developer shell + // would turn the wrapper off and fake-red this test. Empty string + // falls through to the built-in candidates (the path under test). + GITNEXUS_HOOK_TIMEOUT_PATH: '', + }, + }); + hookChild.stdin!.end( + JSON.stringify({ + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }), + ); + + // The fake lsof writes its PID as its FIRST statement; poll tightly. + const spawnDeadline = Date.now() + 8000; + while (Date.now() < spawnDeadline) { + try { + const raw = fs.readFileSync(pidFile, 'utf-8').trim(); + if (raw) { + lsofPid = Number.parseInt(raw, 10); + break; + } + } catch { + /* not written yet */ + } + await new Promise((r) => setTimeout(r, 10)); + } + expect(lsofPid).toBeGreaterThan(0); + + // Kill the hook while its lsof child is alive. + hookChild.kill('SIGKILL'); + + const reapDeadline = Date.now() + 5000; + let alive = isFakeLsofAlive(); + while (alive && Date.now() < reapDeadline) { + await new Promise((r) => setTimeout(r, 100)); + alive = isFakeLsofAlive(); + } + expect(alive).toBe(false); + } finally { + if (lsofPid > 0) { + try { + process.kill(lsofPid, 'SIGKILL'); + } catch { + /* already gone */ + } + } + try { + hookChild?.kill('SIGKILL'); + } catch { + /* ignore */ + } + // The hook claims a slot before probing now; it died holding it. + const lockDir = path.join(gitNexusDir, '.hook-locks'); + try { + for (const f of fs.readdirSync(lockDir)) fs.unlinkSync(path.join(lockDir, f)); + } catch { + /* ignore */ + } + try { + fs.rmdirSync(lockDir); + } catch { + /* ignore */ + } + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(pidFile, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }, 30000); + + // F3 (#2165 review): GITNEXUS_HOOK_TIMEOUT_PATH pointing at an EXISTING + // but unusable path (here: a directory) must not silently disable orphan + // containment. Before the fix, fs.existsSync() accepted the directory as + // THE candidate, its self-test failed, and the wrapper was memoized off — + // no fall-through — so the SIGTERM-immune lsof below survived its full + // 30s sleep. After the fix the env candidate merely goes first in the + // candidate list; failing its self-test falls through to the built-in + // coreutils guard, which still reaps the orphan within ~3s. + it('CJS: env guard pointing at a directory falls through to a working built-in guard', async () => { + const { spawn } = await import('child_process'); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + const pidFile = path.join(os.tmpdir(), `gn-hook-lsofpid-dirguard-${process.pid}`); + fs.rmSync(pidFile, { force: true }); + const guardDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-guard-dir-')); + const binDir = createHookToolDir({ + lsofPidFile: pidFile, + lsofSleepMs: 30000, + lsofIgnoreSigterm: true, + }); + let lsofPid = 0; + let hookChild: ReturnType | null = null; + + const isFakeLsofAlive = () => { + try { + process.kill(lsofPid, 0); + } catch { + return false; // ESRCH — reaped + } + try { + return fs.readFileSync(`/proc/${lsofPid}/cmdline`, 'utf-8').includes(binDir); + } catch { + return false; + } + }; + + try { + hookChild = spawn(process.execPath, [CJS_HOOK], { + stdio: ['pipe', 'ignore', 'ignore'], + env: { + ...hookEnv(binDir), + // '1', NOT '0' — see the slot-gate test above. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + // Exists but is a directory — spawning it fails the lazy + // self-test, forcing the fall-through path under test. + GITNEXUS_HOOK_TIMEOUT_PATH: guardDir, + }, + }); + hookChild.stdin!.end( + JSON.stringify({ + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }), + ); + + const spawnDeadline = Date.now() + 8000; + while (Date.now() < spawnDeadline) { + try { + const raw = fs.readFileSync(pidFile, 'utf-8').trim(); + if (raw) { + lsofPid = Number.parseInt(raw, 10); + break; + } + } catch { + /* not written yet */ + } + await new Promise((r) => setTimeout(r, 10)); + } + expect(lsofPid).toBeGreaterThan(0); + + // Kill the hook while its lsof child is alive (the incident topology). + hookChild.kill('SIGKILL'); + + const reapDeadline = Date.now() + 5000; + let alive = isFakeLsofAlive(); + while (alive && Date.now() < reapDeadline) { + await new Promise((r) => setTimeout(r, 100)); + alive = isFakeLsofAlive(); + } + expect(alive).toBe(false); + } finally { + if (lsofPid > 0) { + try { + process.kill(lsofPid, 'SIGKILL'); + } catch { + /* already gone */ + } + } + try { + hookChild?.kill('SIGKILL'); + } catch { + /* ignore */ + } + const lockDir = path.join(gitNexusDir, '.hook-locks'); + try { + for (const f of fs.readdirSync(lockDir)) fs.unlinkSync(path.join(lockDir, f)); + } catch { + /* ignore */ + } + try { + fs.rmdirSync(lockDir); + } catch { + /* ignore */ + } + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(pidFile, { force: true }); + fs.rmSync(guardDir, { recursive: true, force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }, 30000); + }, +); + // ─── Integration: PreToolUse augmentation filtering (#1492) ───────── describe('PreToolUse augmentation filtering (integration)', () => { @@ -1268,6 +1772,216 @@ describe.skipIf(process.platform === 'win32')( } }); + // T6 (#2163): with the timeout wrapper explicitly disabled the probe must + // degrade to EXACTLY the pre-wrapper behavior — lsof ETIMEDOUT stays + // fail-closed and the augment is silently skipped. Uses the `disabled` + // sentinel, NOT a bogus path: an invalid GITNEXUS_HOOK_TIMEOUT_PATH falls + // through to the real candidate list by design. CI lane note: the sibling + // wrapped ETIMEDOUT tests above exercise GNU /usr/bin/timeout on the + // Linux lane, and on macos-latest hit BSD /usr/bin/timeout (macOS ≥13, + // `-k`-compatible) or Homebrew gtimeout — a de-facto BSD-wrapper + // regression test. + it(`${label}: ETIMEDOUT lsof with wrapper disabled → identical fail-closed skip`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-nowrap-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + lsofSleepMs: 5000, + psOutput: '', + }); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + GITNEXUS_DEBUG: '1', + GITNEXUS_HOOK_TIMEOUT_PATH: 'disabled', + }, + }, + ); + expect(result.stdout.trim()).toBe(''); + expect(result.status).toBe(0); + expect(result.stderr).toContain('[GitNexus] augment skipped'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + + // T7 (#2163): a wrapper that fails the `-k` self-test (busybox <1.34, + // toybox, broken symlink…) must be REJECTED — resolution falls through + // to the built-in candidates (or, with none usable, to the unwrapped + // status quo); either way ETIMEDOUT stays fail-closed. Adopted blindly, + // the bad wrapper would exit with a usage error before ever running + // lsof — empty stdout, status≠0, no ETIMEDOUT — silently flipping the + // fail-closed contract to fail-open. + it(`${label}: bad wrapper (no -k support) is rejected by the self-test → still fail-closed`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-badwrap-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + lsofSleepMs: 5000, + psOutput: '', + }); + // Models busybox <1.34: `-k` unsupported → usage error, exit 2. + const badTimeout = path.join(binDir, 'bad-timeout'); + fs.writeFileSync( + badTimeout, + `#!/usr/bin/env node\nprocess.stderr.write('usage: timeout [-t SECS] [-s SIG] PROG ARGS\\n');\nprocess.exit(2);\n`, + { mode: 0o755 }, + ); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + GITNEXUS_DEBUG: '1', + GITNEXUS_HOOK_TIMEOUT_PATH: badTimeout, + }, + }, + ); + expect(result.stdout.trim()).toBe(''); + expect(result.status).toBe(0); + expect(result.stderr).toContain('[GitNexus] augment skipped'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + + // F5-1 (#2165 review): pin the LIVE arm of the 124 mapping — a guard + // that passes the `-k` self-test and then reports coreutils budget + // expiry (exit 124) must map to "unresponsive holder" → fail-closed + // skip. The fake guard distinguishes the self-test invocation + // (`-k 1 1 /bin/sh -c :`) from a real wrap by the /bin/sh argv token. + it(`${label}: guard exit 124 (budget expiry) → fail-closed skip`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-guard124-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + gitnexusStderr: '[GitNexus] 1 related symbol found:\n\nvalidateUser (src/auth.ts)\n', + lsofOutput: '', + psOutput: '', + }); + const fakeGuard = path.join(binDir, 'guard-exit-124'); + fs.writeFileSync( + fakeGuard, + `#!/usr/bin/env node\nif (process.argv.includes('/bin/sh')) process.exit(0);\nprocess.exit(124);\n`, + { mode: 0o755 }, + ); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + GITNEXUS_DEBUG: '1', + GITNEXUS_HOOK_TIMEOUT_PATH: fakeGuard, + // '1', NOT '0' — see the slot-gate test above. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + }, + }, + ); + expect(result.stdout.trim()).toBe(''); + expect(result.status).toBe(0); + expect(result.stderr).toContain('[GitNexus] augment skipped'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + + // F5-2 (#2165 review): a guard-wrapped probe that dies BY SIGNAL with + // no spawnSync .error must fail closed. coreutils timeout SELF-RAISES + // the signal when `-k` escalates to SIGKILL, so spawnSync sees + // {status: null, signal: 'SIGKILL'} — NOT exit 137. The same shape + // appears when the hook is frozen >2s (SIGSTOP / laptop suspend) and + // resumes after the guard expired. Before the F1 patch this shape fell + // through every check → empty stdout → fail-open, silently inverting + // this call's fail-closed contract. + it(`${label}: guard signal-death (status null + signal, no error) → fail-closed skip`, () => { + const markerPath = path.join(os.tmpdir(), `gn-hook-guardsig-${process.pid}-${label}`); + const lbugPath = path.join(gitNexusDir, 'lbug'); + fs.writeFileSync(lbugPath, ''); + fs.rmSync(markerPath, { force: true }); + const binDir = createHookToolDir({ + gitnexusMarkerPath: markerPath, + gitnexusStderr: '[GitNexus] 1 related symbol found:\n\nvalidateUser (src/auth.ts)\n', + lsofOutput: '', + psOutput: '', + }); + const fakeGuard = path.join(binDir, 'guard-sigkill'); + fs.writeFileSync( + fakeGuard, + `#!/usr/bin/env node\nif (process.argv.includes('/bin/sh')) process.exit(0);\nprocess.kill(process.pid, 'SIGKILL');\n`, + { mode: 0o755 }, + ); + try { + const result = runHook( + hookPath, + { + hook_event_name: 'PreToolUse', + tool_name: 'Grep', + tool_input: { pattern: 'validateUser' }, + cwd: tmpDir, + }, + undefined, + { + env: { + ...hookEnv(binDir), + GITNEXUS_DEBUG: '1', + GITNEXUS_HOOK_TIMEOUT_PATH: fakeGuard, + // '1', NOT '0' — see the slot-gate test above. + GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS: '1', + }, + }, + ); + expect(result.stdout.trim()).toBe(''); + expect(result.status).toBe(0); + expect(result.stderr).toContain('[GitNexus] augment skipped'); + expect(fs.existsSync(markerPath)).toBe(false); + } finally { + fs.rmSync(lbugPath, { force: true }); + fs.rmSync(markerPath, { force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + it(`${label}: non-GitNexus ps line → augment runs`, () => { const markerPath = path.join(os.tmpdir(), `gn-hook-other-${process.pid}-${label}`); const lbugPath = path.join(gitNexusDir, 'lbug'); diff --git a/gitnexus/test/utils/hook-test-helpers.ts b/gitnexus/test/utils/hook-test-helpers.ts index 1baa94ea6..9fd69d5c6 100644 --- a/gitnexus/test/utils/hook-test-helpers.ts +++ b/gitnexus/test/utils/hook-test-helpers.ts @@ -92,6 +92,12 @@ export function createHookToolDir(options: { psOutput?: string; psOutputByPid?: Record; lsofSleepMs?: number; + /** Fake lsof writes this marker file as soon as it starts — proves whether the probe reached the lsof fallback at all (#2163). */ + lsofMarkerPath?: string; + /** Fake lsof writes its own PID here as its FIRST statement, minimizing detection latency for orphan-reaping tests (#2163). */ + lsofPidFile?: string; + /** Fake lsof traps SIGTERM as a no-op before sleeping — models an unkillable/D-state lsof that only SIGKILL can end (#2163). */ + lsofIgnoreSigterm?: boolean; }) { const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-hook-bin-')); const gitnexusStderr = JSON.stringify(options.gitnexusStderr ?? ''); @@ -105,10 +111,21 @@ export function createHookToolDir(options: { options.lsofOutputLines != null ? options.lsofOutputLines.join('\n') + (options.lsofOutputLines.length ? '\n' : '') : (options.lsofOutput ?? ''); + // Composable prologue: pidFile write MUST stay the first statement (see the + // option docs above); SIGTERM trap MUST be installed before any sleep. + const lsofPrologue = + `#!/usr/bin/env node\nconst fs = require('fs');\n` + + (options.lsofPidFile != null + ? `fs.writeFileSync(${JSON.stringify(options.lsofPidFile)}, String(process.pid));\n` + : '') + + (options.lsofMarkerPath != null + ? `fs.writeFileSync(${JSON.stringify(options.lsofMarkerPath)}, 'called');\n` + : '') + + (options.lsofIgnoreSigterm ? `process.on('SIGTERM', () => {});\n` : ''); const lsofBody = options.lsofSleepMs != null - ? `#!/usr/bin/env node\nsetTimeout(() => {}, ${Number(options.lsofSleepMs)});\n` - : `#!/usr/bin/env node\nprocess.stdout.write(${JSON.stringify(lsofOutput)});\nprocess.exit(0);\n`; + ? `${lsofPrologue}setTimeout(() => {}, ${Number(options.lsofSleepMs)});\n` + : `${lsofPrologue}process.stdout.write(${JSON.stringify(lsofOutput)});\nprocess.exit(0);\n`; writeExecutable(path.join(binDir, 'lsof'), lsofBody); const psBody =