diff --git a/lib/gstack-memory-helpers.ts b/lib/gstack-memory-helpers.ts index 91a786665..e0b619040 100644 --- a/lib/gstack-memory-helpers.ts +++ b/lib/gstack-memory-helpers.ts @@ -126,25 +126,88 @@ export function canonicalizeRemote(url: string | null | undefined): string { // ── Public: secretScanFile (gitleaks wrapper) ───────────────────────────── let _gitleaksAvailability: boolean | null = null; +let _gitleaksWarned = false; -function gitleaksAvailable(): boolean { - if (_gitleaksAvailability !== null) return _gitleaksAvailability; +// Probe budgets. The first is short because the common answers (a real +// gitleaks, or ENOENT) are both immediate; the second is generous because by +// then we know the box is busy, not that the binary is absent. +const GITLEAKS_PROBE_MS = 2_000; +const GITLEAKS_RETRY_MS = 10_000; +let _probeMs = GITLEAKS_PROBE_MS; +let _retryMs = GITLEAKS_RETRY_MS; + +/** + * Probe outcome. "slow" is the load case: the binary may well be installed, + * the machine just did not get around to answering. It is deliberately NOT + * folded into "absent" — see gitleaksAvailable(). + */ +type GitleaksProbe = "ok" | "absent" | "slow"; + +function probeGitleaks(timeoutMs: number): GitleaksProbe { try { execFileSync("gitleaks", ["version"], { env: process.env, stdio: "ignore", - timeout: 2_000, + timeout: timeoutMs, }); + return "ok"; + } catch (err) { + const e = err as NodeJS.ErrnoException & { killed?: boolean; signal?: string }; + if (e?.code === "ENOENT") return "absent"; + // execFileSync kills the child when the budget runs out: `killed` with a + // SIGTERM on POSIX, ETIMEDOUT on runtimes that surface errno instead. + if (e?.killed === true || e?.signal === "SIGTERM" || e?.code === "ETIMEDOUT") { + return "slow"; + } + // Present but unusable (non-zero exit, EACCES). Practically the same as + // absent, and equally permanent for this process. + return "absent"; + } +} + +/** + * Is gitleaks usable? Answers are cached for the process — EXCEPT a timeout. + * + * Caching a timeout was a fail-open bug: one busy moment (observed under the + * 7-way sharded test runner, where spawning a shell script took over 2s) set + * availability to false for the whole run, and every later file was ingested + * unscanned behind a single stderr line. A missing binary is a fact and stays + * cached; a slow answer is a condition and gets retried on the next file. + */ +function gitleaksAvailable(): boolean { + if (_gitleaksAvailability !== null) return _gitleaksAvailability; + + let probe = probeGitleaks(_probeMs); + if (probe === "slow") probe = probeGitleaks(_retryMs); + + if (probe === "ok") { _gitleaksAvailability = true; - } catch { - _gitleaksAvailability = false; - // Only warn once per process — Lane E will vendor the binary. + return true; + } + + if (probe === "slow") { + // No cache write: leave the question open for the next call. + if (!_gitleaksWarned) { + _gitleaksWarned = true; + process.stderr.write( + "[gstack-memory-helpers] gitleaks did not answer in " + + `${Math.round((_probeMs + _retryMs) / 1000)}s (machine under load); ` + + "this file goes unscanned and the probe retries on the next one.\n" + ); + } + return false; + } + + _gitleaksAvailability = false; + // Only warn once per process — Lane E will vendor the binary. + if (!_gitleaksWarned) { + _gitleaksWarned = true; process.stderr.write( "[gstack-memory-helpers] gitleaks not in PATH; secret scanning disabled. " + "Run /setup-gbrain to install (or `brew install gitleaks`).\n" ); } - return _gitleaksAvailability; + return false; } /** @@ -513,4 +576,20 @@ function logErrorContext(entry: ErrorContextEntry): void { // Test-only export for resetting the gitleaks availability cache between tests. export function _resetGitleaksAvailabilityCache(): void { _gitleaksAvailability = null; + _gitleaksWarned = false; + _probeMs = GITLEAKS_PROBE_MS; + _retryMs = GITLEAKS_RETRY_MS; +} + +// Test-only: shrink the probe budgets so the slow path can be exercised +// without a multi-second sleep in the suite. Reset restores the defaults. +export function _setGitleaksProbeTimeouts(first: number, second: number): void { + _probeMs = first; + _retryMs = second; +} + +// Test-only: read the cache without triggering a probe. `null` means the +// question is still open — which is the whole point of the timeout path. +export function _gitleaksCacheState(): boolean | null { + return _gitleaksAvailability; } diff --git a/test/gstack-memory-helpers.test.ts b/test/gstack-memory-helpers.test.ts index 48a3dbd81..1b06a4303 100644 --- a/test/gstack-memory-helpers.test.ts +++ b/test/gstack-memory-helpers.test.ts @@ -23,6 +23,8 @@ import { withErrorContext, detectEngineTier, _resetGitleaksAvailabilityCache, + _setGitleaksProbeTimeouts, + _gitleaksCacheState, } from "../lib/gstack-memory-helpers"; // ── canonicalizeRemote ───────────────────────────────────────────────────── @@ -153,8 +155,140 @@ exit 2 expect(result.scanner).toBe("gitleaks"); expect(result.findings).toEqual([]); const calls = readFileSync(log, "utf-8").trim().split("\n"); + // Under load the first probe can expire and retry, so assert the shape: + // one or more `version` probes, then the scan. Pinning calls[1] made a + // busy machine look like a broken scanner. expect(calls[0]).toBe("version"); - expect(calls[1]).toContain("detect --no-git --source"); + expect(calls.at(-1)).toContain("detect --no-git --source"); + expect(calls.slice(0, -1).every((c) => c === "version")).toBe(true); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + rmSync(dir, { recursive: true, force: true }); + } + }); + + // ── probe timeout vs missing binary ────────────────────────────────────── + // + // A timeout used to be cached as "gitleaks is absent", which turned one busy + // moment into an entire run of unscanned files behind a single stderr line. + // These pin the two outcomes apart. Budgets are shrunk via the test-only + // hook so a sleeping fake costs milliseconds, not seconds. + + /** + * Fake gitleaks. With a `marker` path, the FIRST `version` call hangs far + * past any budget and later calls answer instantly; with an empty marker it + * hangs every time. Timing is expressed as "hangs forever" vs "immediate" + * rather than as a race between a short sleep and a short budget — a race is + * exactly the flake being fixed here. + */ + function fakeGitleaks(binDir: string, log: string, marker: string): void { + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, "gitleaks"), + `#!/bin/sh +printf '%s\\n' "$*" >> "${log}" +if [ "$1" = "version" ]; then + if [ -n "${marker}" ] && [ -f "${marker}" ]; then + exit 0 + fi + if [ -n "${marker}" ]; then + touch "${marker}" + fi + sleep 30 + exit 0 +fi +if [ "$1" = "detect" ]; then + echo '[]' + exit 0 +fi +exit 2 +`, + "utf-8", + ); + chmodSync(join(binDir, "gitleaks"), 0o755); + } + + function withFakeOnPath(binDir: string, fn: () => T): T { + const oldPath = process.env.PATH; + process.env.PATH = `${binDir}:${oldPath || ""}`; + try { + return fn(); + } finally { + if (oldPath === undefined) delete process.env.PATH; + else process.env.PATH = oldPath; + } + } + + const versionProbes = (log: string): number => + existsSync(log) + ? readFileSync(log, "utf-8").trim().split("\n").filter((c) => c === "version").length + : 0; + + it("retries a slow probe instead of declaring gitleaks missing", () => { + const dir = mkdtempSync(join(tmpdir(), "gstack-test-")); + const binDir = join(dir, "bin"); + const log = join(dir, "calls.log"); + const file = join(dir, "clean.txt"); + writeFileSync(file, "no secrets here\n"); + fakeGitleaks(binDir, log, join(dir, "hung-once")); + try { + // Budgets are picked so neither outcome can hinge on machine speed: 3s is + // ample for a shell to start and log even on a loaded box (yet the hung + // `sleep 30` still cannot answer within it), and the 30s retry cannot + // expire against a fake that exits immediately. The first draft used + // 1s/5s and flaked under the 7-way shard runner — the very failure mode + // this file is about. + _setGitleaksProbeTimeouts(3_000, 30_000); + const result = withFakeOnPath(binDir, () => secretScanFile(file)); + expect(result.scanner).toBe("gitleaks"); + expect(versionProbes(log)).toBe(2); + expect(_gitleaksCacheState()).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("does not cache a timed-out probe, so the next file tries again", () => { + const dir = mkdtempSync(join(tmpdir(), "gstack-test-")); + const binDir = join(dir, "bin"); + const log = join(dir, "calls.log"); + const file = join(dir, "clean.txt"); + writeFileSync(file, "no secrets here\n"); + // Empty marker: EVERY call hangs, so both budgets expire. + fakeGitleaks(binDir, log, ""); + try { + // Short on purpose, and safe to be short: the fake hangs for 30s, so the + // probe times out at ANY budget — load cannot flip this outcome the way + // it can in the retry case above. 800ms only has to cover writing one + // line to the log. + _setGitleaksProbeTimeouts(800, 800); + const first = withFakeOnPath(binDir, () => secretScanFile(file)); + expect(first.scanner).toBe("missing"); + // The question stays open: nothing was learned about the binary. + expect(_gitleaksCacheState()).toBeNull(); + + const before = versionProbes(log); + withFakeOnPath(binDir, () => secretScanFile(file)); + expect(versionProbes(log)).toBeGreaterThan(before); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("caches an absent binary, so it is probed once per process", () => { + const dir = mkdtempSync(join(tmpdir(), "gstack-test-")); + const binDir = join(dir, "empty-bin"); + mkdirSync(binDir, { recursive: true }); + const file = join(dir, "clean.txt"); + writeFileSync(file, "no secrets here\n"); + const oldPath = process.env.PATH; + try { + // Nothing named gitleaks anywhere on PATH -> ENOENT, a permanent fact. + process.env.PATH = binDir; + const result = secretScanFile(file); + expect(result.scanner).toBe("missing"); + expect(_gitleaksCacheState()).toBe(false); } finally { if (oldPath === undefined) delete process.env.PATH; else process.env.PATH = oldPath;