Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 85 additions & 15 deletions scripts/lib/reviewed-npm-audit.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> {
if (typeof value !== "object" || value === null || Array.isArray(value)) {
Expand Down Expand Up @@ -269,6 +280,67 @@ 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<string, unknown>;
result: NpmAuditCommandResult;
}> {
const wait = input.wait ?? waitSynchronously;
const warn = input.warn ?? console.warn;
const attemptCount = NPM_AUDIT_RETRY_DELAYS_MS.length + 1;
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 {
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 (reason=${npmAuditRetryReason(lastResult)})`,
),
result: lastResult,
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

export function vulnerabilityCounts(report: Record<string, unknown>): Record<Severity, number> {
const metadata = asRecord(report.metadata, "npm audit report metadata");
const vulnerabilities = asRecord(
Expand Down Expand Up @@ -577,23 +649,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<string, unknown> = {};
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,
Expand Down
6 changes: 6 additions & 0 deletions test/reviewed-npm-audit-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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"');
});

Expand Down
183 changes: 181 additions & 2 deletions test/reviewed-npm-audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
parseAuditReport,
provenanceSidecarPath,
readAuditExceptionRegistry,
runNpmAuditWithRetry,
runReviewedNpmAudit,
vulnerabilityCounts,
} from "../scripts/lib/reviewed-npm-audit.mts";
Expand Down Expand Up @@ -167,6 +168,180 @@ 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[] = [];
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: () => responses[attempts++]!,
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: "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),
warn: () => {},
});

expect(attempts).toBe(3);
expect(delays).toEqual([1_000, 2_000]);
expect(audit.report).toBeUndefined();
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", () => {
withInstalledGraph({ parent: "2.0.0", "vulnerable-package": "1.0.0" }, (directory) => {
const result = evaluateAuditPolicy({
Expand Down Expand Up @@ -435,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<string, unknown>;
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({
Expand Down
Loading