From 154fe5f0f4faec53c9697ae55d47ab369b6d3adb Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Sat, 25 Jul 2026 23:01:18 -0700 Subject: [PATCH 1/3] ci(security): retry incomplete npm audits Signed-off-by: Apurv Kumaria --- scripts/lib/reviewed-npm-audit.mts | 102 ++++++++++++-- test/reviewed-npm-audit-workflow.test.ts | 6 + test/reviewed-npm-audit.test.ts | 170 +++++++++++++++++++++++ 3 files changed, 263 insertions(+), 15 deletions(-) diff --git a/scripts/lib/reviewed-npm-audit.mts b/scripts/lib/reviewed-npm-audit.mts index f980d34ae8..7298657801 100755 --- a/scripts/lib/reviewed-npm-audit.mts +++ b/scripts/lib/reviewed-npm-audit.mts @@ -91,6 +91,17 @@ const GRAPH_ID = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/u; const MAX_EXCEPTION_LIFETIME_DAYS = 30; const GHSA_ID_IN_URL = /GHSA(?:-[23456789cfghjmpqrvwx]{4}){3}/gi; +const NPM_AUDIT_ATTEMPT_TIMEOUT_MS = 45_000; +const NPM_AUDIT_RETRY_DELAYS_MS = [1_000, 2_000] as const; + +type NpmAuditCommandResult = Readonly<{ + error?: Error; + status: number | null; + stderr: string; + stdout: string; +}>; + +type NpmAuditRetryReason = "empty-output" | "incomplete-report" | "invalid-json" | "timeout"; function asRecord(value: unknown, label: string): Record { if (typeof value !== "object" || value === null || Array.isArray(value)) { @@ -269,6 +280,69 @@ export function parseAuditReport(result: { return report; } +function waitSynchronously(delayMs: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, delayMs); +} + +function npmAuditRetryReason(result: NpmAuditCommandResult): NpmAuditRetryReason { + if (result.error) return "timeout"; + if (!result.stdout.trim()) return "empty-output"; + try { + JSON.parse(result.stdout); + } catch { + return "invalid-json"; + } + return "incomplete-report"; +} + +export function runNpmAuditWithRetry( + input: Readonly<{ + run: () => NpmAuditCommandResult; + wait?: (delayMs: number) => void; + warn?: (message: string) => void; + }>, +): Readonly<{ + failure?: Error; + report?: Record; + result: NpmAuditCommandResult; +}> { + const wait = input.wait ?? waitSynchronously; + const warn = input.warn ?? console.warn; + const attemptCount = NPM_AUDIT_RETRY_DELAYS_MS.length + 1; + let lastFailure: Error | undefined; + let lastResult: NpmAuditCommandResult | undefined; + + for (let attempt = 1; attempt <= attemptCount; attempt += 1) { + const result = input.run(); + if (result.error && (result.error as NodeJS.ErrnoException).code !== "ETIMEDOUT") { + throw result.error; + } + lastResult = result; + try { + if (result.error) { + throw new Error(`npm audit exceeded its ${NPM_AUDIT_ATTEMPT_TIMEOUT_MS} ms timeout`); + } + return { report: parseAuditReport(result), result }; + } catch (error) { + lastFailure = error instanceof Error ? error : new Error(String(error)); + const delayMs = NPM_AUDIT_RETRY_DELAYS_MS[attempt - 1]; + if (delayMs === undefined) break; + warn( + `npm audit scan incomplete on attempt ${attempt}/${attemptCount}; retrying in ${delayMs} ms (reason=${npmAuditRetryReason(result)})`, + ); + wait(delayMs); + } + } + + if (!lastResult) throw new Error("npm audit retry loop completed without running the scanner"); + return { + failure: new Error( + `npm audit scan remained incomplete after ${attemptCount} attempts: ${lastFailure?.message ?? "unknown scanner failure"}`, + ), + result: lastResult, + }; +} + export function vulnerabilityCounts(report: Record): Record { const metadata = asRecord(report.metadata, "npm audit report metadata"); const vulnerabilities = asRecord( @@ -577,23 +651,21 @@ export function runReviewedNpmAudit( } const exceptionRegistry = readAuditExceptionRegistry(options.exceptionFile); const startedAt = new Date().toISOString(); - const result = spawnSync("npm", ["audit", "--omit=dev", "--json"], { - cwd: options.directory, - encoding: "utf-8", - env: { ...process.env, NPM_CONFIG_UPDATE_NOTIFIER: "false" }, - maxBuffer: 64 * 1024 * 1024, - stdio: ["ignore", "pipe", "pipe"], + const audit = runNpmAuditWithRetry({ + run: () => + spawnSync("npm", ["audit", "--omit=dev", "--json"], { + cwd: options.directory, + encoding: "utf-8", + env: { ...process.env, NPM_CONFIG_UPDATE_NOTIFIER: "false" }, + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + timeout: NPM_AUDIT_ATTEMPT_TIMEOUT_MS, + }), }); const finishedAt = new Date().toISOString(); - if (result.error) throw result.error; - if (options.reportFile) fs.writeFileSync(options.reportFile, result.stdout); - let report: Record = {}; - let auditFailure: Error | undefined; - try { - report = parseAuditReport(result); - } catch (error) { - auditFailure = error instanceof Error ? error : new Error(String(error)); - } + const auditFailure = audit.failure; + const report = audit.report ?? {}; + if (options.reportFile) fs.writeFileSync(options.reportFile, audit.result.stdout); if (options.provenance && options.reportFile) { const provenance = buildAuditProvenance({ failure: auditFailure?.message, diff --git a/test/reviewed-npm-audit-workflow.test.ts b/test/reviewed-npm-audit-workflow.test.ts index e69328126b..f3036a1b3b 100644 --- a/test/reviewed-npm-audit-workflow.test.ts +++ b/test/reviewed-npm-audit-workflow.test.ts @@ -129,6 +129,10 @@ describe("trusted reviewed npm audit workflow (#5896)", () => { path.join(REPO_ROOT, "scripts", "audit-reviewed-npm-graph.mts"), "utf8", ); + const helper = fs.readFileSync( + path.join(REPO_ROOT, "scripts", "lib", "reviewed-npm-audit.mts"), + "utf8", + ); expect(action).toContain('node-version: "22.23.1"'); expect(action).toContain("npm install --global npm@10.9.4"); @@ -139,6 +143,8 @@ describe("trusted reviewed npm audit workflow (#5896)", () => { ); expect(action).not.toContain("run: node --experimental-strip-types scripts/"); expect(driver).toContain("resolveTrustedAuditConfigPath(TRUSTED_REPO_ROOT)"); + expect(helper).toContain("const NPM_AUDIT_ATTEMPT_TIMEOUT_MS = 45_000"); + expect(helper).toContain("timeout: NPM_AUDIT_ATTEMPT_TIMEOUT_MS"); expect(driver).not.toContain('resolveTargetPath(\n "ci/reviewed-npm-audit.json"'); }); diff --git a/test/reviewed-npm-audit.test.ts b/test/reviewed-npm-audit.test.ts index 08380110d8..cd0e633723 100644 --- a/test/reviewed-npm-audit.test.ts +++ b/test/reviewed-npm-audit.test.ts @@ -17,6 +17,7 @@ import { parseAuditReport, provenanceSidecarPath, readAuditExceptionRegistry, + runNpmAuditWithRetry, runReviewedNpmAudit, vulnerabilityCounts, } from "../scripts/lib/reviewed-npm-audit.mts"; @@ -167,6 +168,175 @@ describe("reviewed npm audit gate", () => { ).toThrow(/vulnerability report|vulnerability count/); }); + it("retries scan-incomplete npm responses with bounded backoff", () => { + const completeReport = { + metadata: { + vulnerabilities: { info: 0, low: 0, moderate: 0, high: 0, critical: 0 }, + }, + }; + const sensitiveStderr = + "request failed for https://audit-user:secret-token@registry.example/\n\u001b[31mstderr detail"; + const sensitiveErrorBody = { + summary: "request failed for https://json-user:json-secret@registry.example/", + detail: "first line\n\u001b[31msecond line", + }; + const responses = [ + { status: 1, stderr: sensitiveStderr, stdout: "" }, + { + status: 1, + stderr: "", + stdout: JSON.stringify({ error: sensitiveErrorBody }), + }, + { status: 0, stderr: "", stdout: JSON.stringify(completeReport) }, + ]; + const delays: number[] = []; + const warnings: string[] = []; + let attempt = 0; + + const audit = runNpmAuditWithRetry({ + run: () => responses[attempt++] as (typeof responses)[number], + wait: (delayMs) => delays.push(delayMs), + warn: (message) => warnings.push(message), + }); + + expect(attempt).toBe(3); + expect(delays).toEqual([1_000, 2_000]); + expect(warnings).toEqual([ + "npm audit scan incomplete on attempt 1/3; retrying in 1000 ms (reason=empty-output)", + "npm audit scan incomplete on attempt 2/3; retrying in 2000 ms (reason=incomplete-report)", + ]); + const warningOutput = warnings.join("\n"); + expect(warningOutput).not.toContain("audit-user"); + expect(warningOutput).not.toContain("secret-token"); + expect(warningOutput).not.toContain("json-user"); + expect(warningOutput).not.toContain("json-secret"); + expect(warningOutput).not.toContain("registry.example"); + expect(warningOutput).not.toContain("\u001b"); + expect(audit.failure).toBeUndefined(); + expect(audit.report).toEqual(completeReport); + expect(audit.result).toEqual(responses[2]); + }); + + it("does not retry a complete blocking vulnerability report", () => { + let attempts = 0; + const report = highFindingReport(); + + const audit = runNpmAuditWithRetry({ + run: () => { + attempts += 1; + return { status: 1, stderr: "", stdout: JSON.stringify(report) }; + }, + wait: () => { + throw new Error("complete reports must not back off"); + }, + warn: () => { + throw new Error("complete reports must not emit retry warnings"); + }, + }); + + expect(attempts).toBe(1); + expect(audit.failure).toBeUndefined(); + expect(audit.report).toEqual(report); + withInstalledGraph({ parent: "2.0.0", "vulnerable-package": "1.0.0" }, (directory) => { + expect( + evaluateAuditPolicy({ + directory, + exceptionPolicy: EMPTY_POLICY, + exceptionPolicySha256: "a".repeat(64), + graph: "test-graph", + report, + threshold: "high", + }).status, + ).toBe("blocked"); + }); + }); + + it("retries a timed-out scanner process within the same budget", () => { + const delays: number[] = []; + let attempts = 0; + + const audit = runNpmAuditWithRetry({ + run: () => { + attempts += 1; + if (attempts < 3) { + return { + error: Object.assign(new Error("spawnSync npm ETIMEDOUT"), { code: "ETIMEDOUT" }), + status: null, + stderr: "", + stdout: "", + }; + } + return { + status: 0, + stderr: "", + stdout: JSON.stringify({ + metadata: { + vulnerabilities: { info: 0, low: 0, moderate: 0, high: 0, critical: 0 }, + }, + }), + }; + }, + wait: (delayMs) => delays.push(delayMs), + warn: () => {}, + }); + + expect(attempts).toBe(3); + expect(delays).toEqual([1_000, 2_000]); + expect(audit.failure).toBeUndefined(); + }); + + it("does not retry non-timeout scanner spawn errors", () => { + const waits: number[] = []; + const warnings: string[] = []; + const spawnError = Object.assign(new Error("spawnSync npm ENOENT"), { code: "ENOENT" }); + let attempts = 0; + + expect(() => + runNpmAuditWithRetry({ + run: () => { + attempts += 1; + return { + error: spawnError, + status: null, + stderr: "npm was not found", + stdout: "", + }; + }, + wait: (delayMs) => waits.push(delayMs), + warn: (message) => warnings.push(message), + }), + ).toThrow("spawnSync npm ENOENT"); + + expect(attempts).toBe(1); + expect(waits).toEqual([]); + expect(warnings).toEqual([]); + }); + + it("fails closed after the scan-incomplete retry budget is exhausted", () => { + const delays: number[] = []; + let attempts = 0; + + const audit = runNpmAuditWithRetry({ + run: () => { + attempts += 1; + return { + status: 1, + stderr: "", + stdout: JSON.stringify({ error: { summary: "", detail: "" } }), + }; + }, + wait: (delayMs) => delays.push(delayMs), + warn: () => {}, + }); + + expect(attempts).toBe(3); + expect(delays).toEqual([1_000, 2_000]); + expect(audit.report).toBeUndefined(); + expect(audit.failure?.message).toMatch( + /scan remained incomplete after 3 attempts.*metadata must be an object.*"summary":""/s, + ); + }); + it("accepts one exact blocking advisory and its propagated meta-vulnerability", () => { withInstalledGraph({ parent: "2.0.0", "vulnerable-package": "1.0.0" }, (directory) => { const result = evaluateAuditPolicy({ From 5ad2ca420fb6d38eaecff81da7fdad818d19718e Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Sun, 26 Jul 2026 00:13:22 -0700 Subject: [PATCH 2/3] test(security): linearize npm audit retry setup Signed-off-by: Apurv Kumaria --- test/reviewed-npm-audit.test.ts | 37 +++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/test/reviewed-npm-audit.test.ts b/test/reviewed-npm-audit.test.ts index cd0e633723..916939b8e9 100644 --- a/test/reviewed-npm-audit.test.ts +++ b/test/reviewed-npm-audit.test.ts @@ -253,29 +253,26 @@ describe("reviewed npm audit gate", () => { it("retries a timed-out scanner process within the same budget", () => { const delays: number[] = []; + const timeoutResult = { + error: Object.assign(new Error("spawnSync npm ETIMEDOUT"), { code: "ETIMEDOUT" }), + status: null, + stderr: "", + stdout: "", + }; + const completeResult = { + status: 0, + stderr: "", + stdout: JSON.stringify({ + metadata: { + vulnerabilities: { info: 0, low: 0, moderate: 0, high: 0, critical: 0 }, + }, + }), + }; + const responses = [timeoutResult, timeoutResult, completeResult]; let attempts = 0; const audit = runNpmAuditWithRetry({ - run: () => { - attempts += 1; - if (attempts < 3) { - return { - error: Object.assign(new Error("spawnSync npm ETIMEDOUT"), { code: "ETIMEDOUT" }), - status: null, - stderr: "", - stdout: "", - }; - } - return { - status: 0, - stderr: "", - stdout: JSON.stringify({ - metadata: { - vulnerabilities: { info: 0, low: 0, moderate: 0, high: 0, critical: 0 }, - }, - }), - }; - }, + run: () => responses[attempts++]!, wait: (delayMs) => delays.push(delayMs), warn: () => {}, }); From feb812c77f20cf60464eebc82ac944d2f43f2aaa Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Sun, 26 Jul 2026 00:18:30 -0700 Subject: [PATCH 3/3] fix(security): redact terminal npm audit failures Signed-off-by: Apurv Kumaria --- scripts/lib/reviewed-npm-audit.mts | 6 ++---- test/reviewed-npm-audit.test.ts | 24 ++++++++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/scripts/lib/reviewed-npm-audit.mts b/scripts/lib/reviewed-npm-audit.mts index 7298657801..8c1bde6736 100755 --- a/scripts/lib/reviewed-npm-audit.mts +++ b/scripts/lib/reviewed-npm-audit.mts @@ -309,7 +309,6 @@ export function runNpmAuditWithRetry( const wait = input.wait ?? waitSynchronously; const warn = input.warn ?? console.warn; const attemptCount = NPM_AUDIT_RETRY_DELAYS_MS.length + 1; - let lastFailure: Error | undefined; let lastResult: NpmAuditCommandResult | undefined; for (let attempt = 1; attempt <= attemptCount; attempt += 1) { @@ -323,8 +322,7 @@ export function runNpmAuditWithRetry( throw new Error(`npm audit exceeded its ${NPM_AUDIT_ATTEMPT_TIMEOUT_MS} ms timeout`); } return { report: parseAuditReport(result), result }; - } catch (error) { - lastFailure = error instanceof Error ? error : new Error(String(error)); + } catch { const delayMs = NPM_AUDIT_RETRY_DELAYS_MS[attempt - 1]; if (delayMs === undefined) break; warn( @@ -337,7 +335,7 @@ export function runNpmAuditWithRetry( if (!lastResult) throw new Error("npm audit retry loop completed without running the scanner"); return { failure: new Error( - `npm audit scan remained incomplete after ${attemptCount} attempts: ${lastFailure?.message ?? "unknown scanner failure"}`, + `npm audit scan remained incomplete after ${attemptCount} attempts (reason=${npmAuditRetryReason(lastResult)})`, ), result: lastResult, }; diff --git a/test/reviewed-npm-audit.test.ts b/test/reviewed-npm-audit.test.ts index 916939b8e9..0f6d00da94 100644 --- a/test/reviewed-npm-audit.test.ts +++ b/test/reviewed-npm-audit.test.ts @@ -318,8 +318,13 @@ describe("reviewed npm audit gate", () => { attempts += 1; return { status: 1, - stderr: "", - stdout: JSON.stringify({ error: { summary: "", detail: "" } }), + stderr: "registry-token=terminal-stderr-secret", + stdout: JSON.stringify({ + error: { + summary: "registry-token=terminal-summary-secret", + detail: "authorization: bearer terminal-detail-secret", + }, + }), }; }, wait: (delayMs) => delays.push(delayMs), @@ -329,9 +334,12 @@ describe("reviewed npm audit gate", () => { expect(attempts).toBe(3); expect(delays).toEqual([1_000, 2_000]); expect(audit.report).toBeUndefined(); - expect(audit.failure?.message).toMatch( - /scan remained incomplete after 3 attempts.*metadata must be an object.*"summary":""/s, + expect(audit.failure?.message).toBe( + "npm audit scan remained incomplete after 3 attempts (reason=incomplete-report)", ); + expect(audit.failure?.message).not.toContain("terminal-stderr-secret"); + expect(audit.failure?.message).not.toContain("terminal-summary-secret"); + expect(audit.failure?.message).not.toContain("terminal-detail-secret"); }); it("accepts one exact blocking advisory and its propagated meta-vulnerability", () => { @@ -602,11 +610,15 @@ describe("reviewed npm audit provenance", () => { reportFile: reportPath, threshold: "high", }), - ).toThrow(/ECONNREFUSED/); + ).toThrow("npm audit scan remained incomplete after 3 attempts (reason=incomplete-report)"); const sidecar = JSON.parse( fs.readFileSync(path.join(tempRoot, "graph.provenance.json"), "utf-8"), ) as Record; - expect(sidecar.failure).toMatch(/ECONNREFUSED/); + expect(sidecar.failure).toBe( + "npm audit scan remained incomplete after 3 attempts (reason=incomplete-report)", + ); + expect(sidecar.failure).not.toContain("ECONNREFUSED"); + expect(sidecar.failure).not.toContain("registry unreachable"); expect(sidecar.advisoryIds).toEqual([]); expect(sidecar.rawReportPath).toBe("graph.json"); expect(sidecar.registry).toEqual({