diff --git a/.changeset/humble-newts-greet.md b/.changeset/humble-newts-greet.md new file mode 100644 index 000000000..8e2cf1294 --- /dev/null +++ b/.changeset/humble-newts-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4251 +--- +**The npm-audit CI gate now retries a slow registry instead of failing on one bad moment, and reports a clear timeout error instead of a misleading JSON parse error when it does fail.** A timed-out audit call previously surfaced as `Unexpected end of JSON input` and made exactly one attempt with no retry, so any single transport hiccup against npm's registry failed a required gate. It now retries up to 3 times with backoff before failing, and any failure names the real cause. (#4250, #4260) diff --git a/scripts/npm-audit-baseline.cjs b/scripts/npm-audit-baseline.cjs index c12e182c8..a9e434f8b 100644 --- a/scripts/npm-audit-baseline.cjs +++ b/scripts/npm-audit-baseline.cjs @@ -24,7 +24,74 @@ const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); -const AUDIT_TIMEOUT_MS = 180_000; +// Per-attempt timeout. 60s comfortably covers the worst *working* latency +// measured against this endpoint (43.41s, see #4260) while staying well +// under the old single-shot 180s budget once multiplied by AUDIT_MAX_ATTEMPTS. +const AUDIT_ATTEMPT_TIMEOUT_MS = 60_000; +// Bounded retry, not unbounded -- a registry-side outage must still fail the +// gate eventually (see #4260's own caveat: silently disarming a required +// security check on a transport error is worse than failing it). +const AUDIT_MAX_ATTEMPTS = 3; +// Exponential backoff base between attempts (2s, 4s for attempts 1->2, 2->3). +const AUDIT_BACKOFF_BASE_MS = 2_000; + +/** + * Synchronous sleep (execFileSync-based retry logic is itself synchronous, + * so backoff between attempts must be too). Uses Atomics.wait on a throwaway + * SharedArrayBuffer, the standard Node pattern for a blocking sleep with no + * external dependency. + */ +function sleepSyncMs(ms) { + const sab = new SharedArrayBuffer(4); + Atomics.wait(new Int32Array(sab), 0, 0, ms); +} + +/** + * True when `error` represents a child process execFileSync killed via its + * `timeout` option (e.g. AUDIT_ATTEMPT_TIMEOUT_MS firing against a + * slow/degraded npm registry) rather than one that exited normally with a + * non-zero code. Node sets `killed: true` and `signal` to the kill signal in + * that case (see the child_process docs for execFileSync's `timeout` + * option); a normal "npm audit found advisories" exit has neither set. The + * distinction matters because a killed process's stdout is truncated + * mid-write, not complete JSON -- treating it as recoverable JSON produces a + * misleading `Unexpected end of JSON input` instead of naming the real + * cause. + */ +function isTimeoutKill(error) { + return Boolean(error && (error.killed === true || error.signal)); +} + +/** + * Builds the standard timeout-kill error message, shared by every caller + * that classifies a killed npm audit process after exhausting retries (see + * isTimeoutKill, runNpmAuditWithRetry) -- kept in one place so the message + * can't independently drift between callers, per this repo's Generative Fix + * Divergence anti-pattern. + * + * Includes whatever `error.stderr` execFileSync captured before the last + * kill (Node buffers stdout/stderr from a killed child in-memory and + * attaches them to the thrown error; without surfacing it here that data + * was simply discarded, giving zero diagnostic signal into WHY npm was + * still running when the timeout fired). + */ +function buildTimeoutKillError(cwd, error, attempts = 1) { + const stderr = error && error.stderr + ? (Buffer.isBuffer(error.stderr) ? error.stderr.toString('utf-8') : String(error.stderr)) + : ''; + const stderrSuffix = stderr.trim() + ? `\n\nCaptured stderr before the last kill:\n${stderr.trim().slice(0, 2000)}` + : '\n\n(no stderr was captured before the last kill)'; + const attemptsPhrase = attempts > 1 + ? `after ${attempts} attempts (each up to ${AUDIT_ATTEMPT_TIMEOUT_MS}ms, with exponential backoff between)` + : `after ${AUDIT_ATTEMPT_TIMEOUT_MS}ms`; + return new Error( + `npm audit timed out ${attemptsPhrase} in ${cwd} -- this is not a JSON ` + + `parse failure, npm audit did not finish. The npm registry's advisories endpoint ` + + `may be degraded; check https://status.npmjs.org before assuming a code regression.` + + stderrSuffix, + ); +} const AUDIT_DIFF_REASON = Object.freeze({ OK_NO_NEW_VULNERABILITIES: 'ok_no_new_vulnerabilities', @@ -69,6 +136,92 @@ function evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }) { }; } +/** + * Runs `npm audit --json` in `cwd` with bounded retry + exponential + * backoff. A per-attempt timeout-kill (see isTimeoutKill) is retried rather + * than failed immediately -- #4260: the audit backend has real, independent + * latency variance from the rest of the registry (measured: 43.41s vs 0.20s + * for a plain registry fetch, and two outright non-responses in the same + * window), and a single 180s attempt with no retry meant any one bad moment + * failed a REQUIRED gate on a transport hiccup, not a real advisory. A + * non-timeout failure (e.g. a genuine non-zero exit with recoverable stdout, + * or an unrecoverable error) is NOT retried -- only a confirmed timeout-kill + * is, since retrying a deterministic failure wastes the budget without + * changing the outcome. + * + * Exhausting all attempts still fails the gate (see #4260's own caveat: + * silently skipping a required security check on a transport error is a + * worse failure mode than occasionally re-running CI). + */ +function runNpmAuditWithRetry(cwd, args, { execFileSyncImpl = execFileSync, sleepImpl = sleepSyncMs } = {}) { + const isWindows = process.platform === 'win32'; + const npmCandidates = isWindows ? ['npm.cmd', 'npm'] : ['npm']; + let lastTimeoutError = null; + + for (let attempt = 1; attempt <= AUDIT_MAX_ATTEMPTS; attempt += 1) { + let out; + let timedOutThisAttempt = false; + let lastErr = null; + + for (const npmCmd of npmCandidates) { + try { + out = execFileSyncImpl(npmCmd, args, { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: AUDIT_ATTEMPT_TIMEOUT_MS, + shell: isWindows, + }); + lastErr = null; + break; + } catch (e) { + if (isTimeoutKill(e)) { + timedOutThisAttempt = true; + lastErr = e; + break; // do not burn the alt npm-candidate slot on a timeout; go to the next retry attempt instead + } + // `npm audit` exits non-zero when advisories are present; the JSON is + // still on stdout in that case. Recover and let the caller classify — + // but only when stdout actually CARRIES the JSON: an audit that was + // killed or aborted can exit non-zero with EMPTY stdout, and accepting + // the empty string here surfaces as a bare `SyntaxError: Unexpected + // end of JSON input` at the parse below, hiding the captured error + // (observed on CI 2026-09-03, both lanes; cause undetermined). + const recovered = e && typeof e.stdout !== 'undefined' && e.stdout !== null + ? (Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout)) + : ''; + if (recovered.trim()) { + out = recovered; + lastErr = null; + break; + } + lastErr = e; + } + } + + if (timedOutThisAttempt) { + lastTimeoutError = lastErr; + if (attempt < AUDIT_MAX_ATTEMPTS) { + sleepImpl(AUDIT_BACKOFF_BASE_MS * (2 ** (attempt - 1))); + continue; + } + throw buildTimeoutKillError(cwd, lastTimeoutError, attempt); + } + + if (lastErr) throw lastErr; // non-timeout failure: fail immediately, no retry + + const parsed = JSON.parse(out); + if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { + return parsed; + } + throw new Error(`Unexpected npm audit JSON shape in ${cwd}: missing metadata.vulnerabilities`); + } + // Unreachable (the loop always returns or throws), but keep a fallback + // throw so a future refactor mistake fails loudly instead of returning + // undefined. + throw buildTimeoutKillError(cwd, lastTimeoutError, AUDIT_MAX_ATTEMPTS); +} + /** * Runs `npm audit --package-lock-only --omit=dev --json` in `cwd` and * returns the parsed JSON, or `null` if `cwd` has no package.json or no @@ -80,63 +233,23 @@ function evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }) { * extracted package.json + package-lock.json (see extractBaselineTree), * without a second full `npm ci`. */ -function runPackageLockAudit(cwd) { +function runPackageLockAudit(cwd, opts = {}) { if (!fs.existsSync(path.join(cwd, 'package.json'))) return null; if (!fs.existsSync(path.join(cwd, 'package-lock.json'))) return null; - const isWindows = process.platform === 'win32'; - const npmCandidates = isWindows ? ['npm.cmd', 'npm'] : ['npm']; - const args = ['audit', '--package-lock-only', '--omit=dev', '--json']; - let out; - let lastErr = null; - for (const npmCmd of npmCandidates) { - try { - out = execFileSync( - npmCmd, - args, - { - cwd, - encoding: 'utf-8', - stdio: ['ignore', 'pipe', 'pipe'], - timeout: AUDIT_TIMEOUT_MS, - shell: isWindows, - }, - ); - lastErr = null; - break; - } catch (e) { - // `npm audit` exits non-zero when advisories are present; the JSON is - // still on stdout in that case. Recover and let the caller classify — - // but only when stdout actually CARRIES the JSON: an audit that was - // killed or aborted can exit non-zero with EMPTY stdout, and accepting - // the empty string here surfaces as a bare `SyntaxError: Unexpected - // end of JSON input` at the parse below, hiding the captured error - // (observed on CI 2026-09-03, both lanes; cause undetermined). - const recovered = e && typeof e.stdout !== 'undefined' && e.stdout !== null - ? (Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout)) - : ''; - if (recovered.trim()) { - out = recovered; - lastErr = null; - break; - } - lastErr = e; - } - } - if (!out || !out.trim()) { - const detail = lastErr - ? [lastErr.stdout, lastErr.stderr, String(lastErr.message)].filter(Boolean).join('\n').slice(0, 500) - : '(no error captured)'; - throw new Error( - `npm audit --json produced no output in ${cwd}. ` + - `Detail from the failed invocation:\n${detail}` - ); - } - if (lastErr) throw lastErr; - const parsed = JSON.parse(out); - if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { - return parsed; - } - throw new Error(`Unexpected npm audit JSON shape in ${cwd}: missing metadata.vulnerabilities`); + return runNpmAuditWithRetry(cwd, ['audit', '--package-lock-only', '--omit=dev', '--json'], opts); +} + +/** + * Runs `npm audit --omit=dev --json` against the REAL installed node_modules + * tree in `cwd` (unlike runPackageLockAudit's --package-lock-only mode, + * which only needs package-lock.json on disk). Returns `null` if `cwd` has + * no package.json or no node_modules/ (not an auditable/installed tree -- + * callers treat this as "skip", not an error). + */ +function runInstalledTreeAudit(cwd, opts = {}) { + if (!fs.existsSync(path.join(cwd, 'package.json'))) return null; + if (!fs.existsSync(path.join(cwd, 'node_modules'))) return null; + return runNpmAuditWithRetry(cwd, ['audit', '--omit=dev', '--json'], opts); } /** @@ -249,7 +362,15 @@ module.exports = { diffNewVulnerablePackages, evaluateAuditDiff, runPackageLockAudit, + runInstalledTreeAudit, + runNpmAuditWithRetry, extractBaselineTree, resolveBaselineRef, + isTimeoutKill, + buildTimeoutKillError, + sleepSyncMs, + AUDIT_ATTEMPT_TIMEOUT_MS, + AUDIT_MAX_ATTEMPTS, + AUDIT_BACKOFF_BASE_MS, NULL_SHA, }; diff --git a/tests/npm-audit-baseline.test.cjs b/tests/npm-audit-baseline.test.cjs index 51328b3d4..e9d2329d0 100644 --- a/tests/npm-audit-baseline.test.cjs +++ b/tests/npm-audit-baseline.test.cjs @@ -20,8 +20,13 @@ const { diffNewVulnerablePackages, evaluateAuditDiff, runPackageLockAudit, + runInstalledTreeAudit, + runNpmAuditWithRetry, extractBaselineTree, resolveBaselineRef, + isTimeoutKill, + buildTimeoutKillError, + AUDIT_BACKOFF_BASE_MS, NULL_SHA, } = require('../scripts/npm-audit-baseline.cjs'); @@ -294,3 +299,204 @@ describe('runPackageLockAudit', () => { } }); }); + +// ─── runInstalledTreeAudit (filesystem-only skip conditions, no registry) ── + +describe('runInstalledTreeAudit', () => { + test('missing package.json -> null', () => { + const dir = createTempDir('gsd-audit-installed-empty-'); + try { + assert.strictEqual(runInstalledTreeAudit(dir), null); + } finally { + cleanup(dir); + } + }); + + test('package.json present but no node_modules -> null', () => { + const dir = createTempDir('gsd-audit-installed-nomodules-'); + try { + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"nomodules"}'); + assert.strictEqual(runInstalledTreeAudit(dir), null); + } finally { + cleanup(dir); + } + }); +}); + +// ─── isTimeoutKill ─────────────────────────────────────────────────────────── + +describe('isTimeoutKill', () => { + test('killed: true -> true', () => { + assert.strictEqual(isTimeoutKill({ killed: true }), true); + }); + + test('signal set (e.g. SIGTERM) -> true', () => { + assert.strictEqual(isTimeoutKill({ signal: 'SIGTERM' }), true); + }); + + test('both killed and signal set -> true', () => { + assert.strictEqual(isTimeoutKill({ killed: true, signal: 'SIGTERM' }), true); + }); + + test('neither killed nor signal set (normal non-zero exit) -> false', () => { + assert.strictEqual(isTimeoutKill({ killed: false, signal: null, status: 1, stdout: '{}' }), false); + }); + + test('killed: false explicitly -> false', () => { + assert.strictEqual(isTimeoutKill({ killed: false }), false); + }); + + test('null/undefined error -> false, does not throw', () => { + assert.strictEqual(isTimeoutKill(null), false); + assert.strictEqual(isTimeoutKill(undefined), false); + }); + + test('plain object with no killed/signal keys at all -> false', () => { + assert.strictEqual(isTimeoutKill({}), false); + }); +}); + +// ─── buildTimeoutKillError ─────────────────────────────────────────────────── + +describe('buildTimeoutKillError', () => { + test('default attempts (1) uses singular ms-based phrasing, not "N attempts"', () => { + const err = buildTimeoutKillError('/some/dir', { stderr: 'some stderr text' }); + assert.match(err.message, /npm audit timed out after \d+ms/); + assert.doesNotMatch(err.message, /attempts/); + assert.match(err.message, /some stderr text/); + }); + + test('attempts > 1 uses plural "N attempts" phrasing with backoff mention', () => { + const err = buildTimeoutKillError('/some/dir', { stderr: '' }, 3); + assert.match(err.message, /npm audit timed out after 3 attempts/); + assert.match(err.message, /exponential backoff/); + }); + + test('no error object at all still produces a message, no crash', () => { + const err = buildTimeoutKillError('/some/dir', undefined); + assert.match(err.message, /npm audit timed out after \d+ms/); + assert.match(err.message, /no stderr was captured/); + }); +}); + +// ─── runPackageLockAudit -- timeout-kill classification (#4250) ───────────── + +describe('runPackageLockAudit — timeout-kill retry classification (#4250, #4260)', () => { + function makeFixtureDir(t) { + const dir = createTempDir('gsd-audit-baseline-timeout-'); + t.after(() => cleanup(dir)); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"fixture"}'); + fs.writeFileSync(path.join(dir, 'package-lock.json'), '{"lockfileVersion":3}'); + return dir; + } + + function makeKilledError() { + return Object.assign(new Error('command timed out'), { + killed: true, + signal: 'SIGTERM', + stdout: '{"auditReportVersion":2,"vulnerabi', // deliberately truncated, non-empty + stderr: 'npm http fetch GET 200 https://registry.npmjs.org/-/npm/v1/security/advisories/bulk (attempt 1) 178234ms', + }); + } + + test('a timeout-killed execFileSync call on every attempt throws a clear timeout error after exhausting retries, not a JSON parse error', (t) => { + const dir = makeFixtureDir(t); + const execFileSyncImpl = () => { throw makeKilledError(); }; + const sleepImpl = () => {}; + + assert.throws( + () => runPackageLockAudit(dir, { execFileSyncImpl, sleepImpl }), + (err) => { + assert.match(err.message, /npm audit timed out after \d+ attempts/); + assert.match(err.message, /status\.npmjs\.org/); + assert.doesNotMatch(err.message, /Unexpected end of JSON input/); + assert.match(err.message, /Captured stderr before the last kill/); + assert.match(err.message, /npm http fetch GET/); + return true; + }, + ); + }); + + test('a timeout-killed call with no captured stderr still produces a clear message (no crash on missing stderr)', (t) => { + const dir = makeFixtureDir(t); + const killedError = Object.assign(new Error('command timed out'), { + killed: true, + signal: 'SIGTERM', + // no stdout, no stderr at all + }); + const execFileSyncImpl = () => { throw killedError; }; + const sleepImpl = () => {}; + + assert.throws( + () => runPackageLockAudit(dir, { execFileSyncImpl, sleepImpl }), + (err) => { + assert.match(err.message, /npm audit timed out after \d+ attempts/); + assert.match(err.message, /no stderr was captured/); + return true; + }, + ); + }); + + test('retry recovers: timeouts on the first attempts followed by a successful final attempt succeeds', (t) => { + const dir = makeFixtureDir(t); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: { high: 0 } }, vulnerabilities: {} }); + let calls = 0; + const execFileSyncImpl = () => { + calls += 1; + if (calls < 3) throw makeKilledError(); + return completeJson; + }; + const sleepImpl = () => {}; + + const result = runPackageLockAudit(dir, { execFileSyncImpl, sleepImpl }); + assert.deepStrictEqual(result.metadata.vulnerabilities, { high: 0 }); + assert.strictEqual(calls, 3); + }); + + test('a normal non-zero exit with complete stdout JSON still recovers correctly (no regression)', (t) => { + const dir = makeFixtureDir(t); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: { high: 1 } }, vulnerabilities: { foo: {} } }); + const nonZeroExitError = Object.assign(new Error('npm audit found vulnerabilities'), { + status: 1, + stdout: completeJson, + }); + const execFileSyncImpl = () => { throw nonZeroExitError; }; + + const result = runPackageLockAudit(dir, { execFileSyncImpl }); + assert.deepStrictEqual(result.metadata.vulnerabilities, { high: 1 }); + }); + + test('a real successful call (no throw) still works via the injected impl', (t) => { + const dir = makeFixtureDir(t); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: {} }, vulnerabilities: {} }); + const execFileSyncImpl = () => completeJson; + + const result = runPackageLockAudit(dir, { execFileSyncImpl }); + assert.deepStrictEqual(result.metadata.vulnerabilities, {}); + }); +}); + +// ─── runNpmAuditWithRetry — backoff timing (#4260) ────────────────────────── + +describe('runNpmAuditWithRetry — backoff timing (#4260)', () => { + test('a timeout-then-recover attempt sequence sleeps once with the base backoff value', (t) => { + const dir = createTempDir('gsd-audit-baseline-backoff-'); + t.after(() => cleanup(dir)); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: {} }, vulnerabilities: {} }); + let calls = 0; + const execFileSyncImpl = () => { + calls += 1; + if (calls === 1) { + throw Object.assign(new Error('command timed out'), { killed: true, signal: 'SIGTERM' }); + } + return completeJson; + }; + const sleepCalls = []; + const sleepImpl = (ms) => sleepCalls.push(ms); + + const parsed = runNpmAuditWithRetry(dir, ['audit', '--json'], { execFileSyncImpl, sleepImpl }); + + assert.deepStrictEqual(parsed.metadata.vulnerabilities, {}); + assert.deepStrictEqual(sleepCalls, [AUDIT_BACKOFF_BASE_MS * 1]); + }); +}); diff --git a/tests/npm-integrity-gate.test.cjs b/tests/npm-integrity-gate.test.cjs index 76156601d..7cde6ff77 100644 --- a/tests/npm-integrity-gate.test.cjs +++ b/tests/npm-integrity-gate.test.cjs @@ -201,86 +201,38 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); const fs = require('node:fs'); -const { execFileSync } = require('node:child_process'); const { evaluateAuditDiff, runPackageLockAudit, + runInstalledTreeAudit, extractBaselineTree, resolveBaselineRef, + AUDIT_ATTEMPT_TIMEOUT_MS, + AUDIT_MAX_ATTEMPTS, + AUDIT_BACKOFF_BASE_MS, } = require('../scripts/npm-audit-baseline.cjs'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, createTempDir } = require('./helpers.cjs'); const ROOT = path.resolve(__dirname, '..'); const SDK = path.join(ROOT, 'sdk'); -const AUDIT_TIMEOUT_MS = 180_000; -const TEST_TIMEOUT_MS = AUDIT_TIMEOUT_MS + 30_000; -function auditProductionVulns(cwd) { - if (!fs.existsSync(path.join(cwd, 'package.json'))) { - return null; // signal "skip" to caller - } - if (!fs.existsSync(path.join(cwd, 'node_modules'))) { - return null; // signal "skip" to caller - } - const isWindows = process.platform === 'win32'; - const npmCandidates = isWindows ? ['npm.cmd', 'npm'] : ['npm']; - const args = ['audit', '--omit=dev', '--json']; - let out; - let lastErr = null; - for (const npmCmd of npmCandidates) { - try { - out = execFileSync( - npmCmd, - args, - { - cwd, - encoding: 'utf-8', - stdio: ['ignore', 'pipe', 'pipe'], - timeout: AUDIT_TIMEOUT_MS, - shell: isWindows, - } - ); - lastErr = null; - break; - } catch (e) { - // `npm audit` exits non-zero when advisories are present; the JSON is - // still on stdout in that case. Recover and let the assertion classify — - // but only when stdout actually CARRIES the JSON. A killed or aborted - // audit exits non-zero with EMPTY stdout (npm writes plain-text errors - // to stderr); accepting the empty string here used to surface as - // `SyntaxError: Unexpected end of JSON input` at the parse below, hiding - // the real cause. Keep the candidate loop going and let the explicit - // empty-output throw below name npm's stderr instead. - const recovered = e && typeof e.stdout !== 'undefined' && e.stdout !== null - ? (Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout)) - : ''; - if (recovered.trim()) { - out = recovered; - lastErr = null; - break; - } - lastErr = e; - } - } - if (!out || !out.trim()) { - const detail = lastErr - ? [lastErr.stdout, lastErr.stderr, String(lastErr.message)].filter(Boolean).join('\n').slice(0, 500) - : '(no error captured)'; - throw new Error( - `npm audit --json produced no output. ` + - `Detail from the failed invocation:\n${detail}` - ); - } - if (lastErr) throw lastErr; - const parsed = JSON.parse(out); - // `null` is reserved for the "node_modules missing → skip" signal above. - // Any other unexpected JSON shape is a real failure of the audit harness - // (npm changed its output format, audit aborted before metadata, etc.) — - // throw so the test fails loudly instead of skipping silently. - if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { - return parsed; - } - throw new Error(`Unexpected npm audit JSON shape in ${cwd}: missing metadata.vulnerabilities`); +// Worst case for ONE retry-audit call: every attempt times out, with +// exponential backoff between each (AUDIT_MAX_ATTEMPTS - 1 gaps) -- computed +// with the SAME formula scripts/npm-audit-baseline.cjs uses internally, so +// this can't independently drift from the real backoff schedule. +let totalBackoffMs = 0; +for (let attempt = 1; attempt < AUDIT_MAX_ATTEMPTS; attempt += 1) { + totalBackoffMs += AUDIT_BACKOFF_BASE_MS * (2 ** (attempt - 1)); +} +const SINGLE_AUDIT_WORST_CASE_MS = (AUDIT_ATTEMPT_TIMEOUT_MS * AUDIT_MAX_ATTEMPTS) + totalBackoffMs; +// checkTreeAgainstBaseline makes up to TWO sequential retry-audit calls +// (auditProductionVulns for HEAD, runPackageLockAudit for the baseline) -- +// budget for both exhausting retries simultaneously, or node:test's own +// timeout fires first and masks buildTimeoutKillError's clear message. +const TEST_TIMEOUT_MS = (SINGLE_AUDIT_WORST_CASE_MS * 2) + 30_000; + +function auditProductionVulns(cwd, opts = {}) { + return runInstalledTreeAudit(cwd, opts); } describe('#3588: npm audit --omit=dev introduces no NEW advisories vs baseline (#4196)', () => { @@ -329,5 +281,71 @@ describe('#3588: npm audit --omit=dev introduces no NEW advisories vs baseline ( checkTreeAgainstBaseline(t, SDK, 'sdk', 'sdk/ is not an auditable npm package or sdk/node_modules/ is missing'); }); }); + +describe('auditProductionVulns — timeout-kill retry classification (#4250, #4260)', () => { + function makeFixtureDir(t) { + const dir = createTempDir('gsd-audit-baseline-timeout-'); + t.after(() => cleanup(dir)); + fs.mkdirSync(path.join(dir, 'node_modules'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"fixture"}'); + return dir; + } + + function makeKilledError() { + return Object.assign(new Error('command timed out'), { + killed: true, + signal: 'SIGTERM', + stdout: '{"auditReportVersion":2,"vulnerabi', // deliberately truncated, non-empty + stderr: 'npm http fetch GET 200 https://registry.npmjs.org/-/npm/v1/security/advisories/bulk (attempt 1) 178234ms', + }); + } + + test('a timeout-killed execFileSync call on every attempt throws a clear timeout error after exhausting retries, not a JSON parse error', (t) => { + const dir = makeFixtureDir(t); + const execFileSyncImpl = () => { throw makeKilledError(); }; + const sleepImpl = () => {}; + + assert.throws( + () => auditProductionVulns(dir, { execFileSyncImpl, sleepImpl }), + (err) => { + assert.match(err.message, /npm audit timed out after \d+ attempts/); + assert.match(err.message, /status\.npmjs\.org/); + assert.doesNotMatch(err.message, /Unexpected end of JSON input/); + assert.match(err.message, /Captured stderr before the last kill/); + assert.match(err.message, /npm http fetch GET/); + return true; + }, + ); + }); + + test('retry recovers: timeouts on the first attempts followed by a successful final attempt succeeds', (t) => { + const dir = makeFixtureDir(t); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: { high: 0 } }, vulnerabilities: {} }); + let calls = 0; + const execFileSyncImpl = () => { + calls += 1; + if (calls < 3) throw makeKilledError(); + return completeJson; + }; + const sleepImpl = () => {}; + + const result = auditProductionVulns(dir, { execFileSyncImpl, sleepImpl }); + assert.deepStrictEqual(result.metadata.vulnerabilities, { high: 0 }); + assert.strictEqual(calls, 3); + }); + + test('a normal non-zero exit with complete stdout JSON still recovers correctly (no regression)', (t) => { + const dir = makeFixtureDir(t); + const completeJson = JSON.stringify({ metadata: { vulnerabilities: { high: 1 } }, vulnerabilities: { foo: {} } }); + const nonZeroExitError = Object.assign(new Error('npm audit found vulnerabilities'), { + status: 1, + stdout: completeJson, + }); + const execFileSyncImpl = () => { throw nonZeroExitError; }; + + const result = auditProductionVulns(dir, { execFileSyncImpl }); + assert.deepStrictEqual(result.metadata.vulnerabilities, { high: 1 }); + }); +}); }); }