diff --git a/scripts/pre-commit/pnpm-audit-prod.mjs b/scripts/pre-commit/pnpm-audit-prod.mjs index 98d232be60fa..c21f0b80c2b8 100644 --- a/scripts/pre-commit/pnpm-audit-prod.mjs +++ b/scripts/pre-commit/pnpm-audit-prod.mjs @@ -47,6 +47,8 @@ const AUDIT_ADVISORY_VERSION_OVERRIDES = [ }, ]; +class AdvisoryRequestTimeoutError extends Error {} + /** @typedef {{ write: (chunk: string) => boolean }} AuditOutput */ /** * @typedef {object} PnpmAuditOptions @@ -753,7 +755,9 @@ export async function withAdvisoryRequestTimeout({ label, timeoutMs, run }) { /** @type {Promise} */ const timeoutPromise = new Promise((_resolve, reject) => { timeout = setTimeout(() => { - const error = new Error(`${label} exceeded timeout of ${resolvedTimeoutMs}ms`); + const error = new AdvisoryRequestTimeoutError( + `${label} exceeded timeout of ${resolvedTimeoutMs}ms`, + ); controller.abort(error); reject(error); }, resolvedTimeoutMs); @@ -849,35 +853,44 @@ export async function fetchBulkAdvisories({ timeoutMs = resolveBulkAdvisoryRequestTimeoutMs(), }) { const url = `${registryBaseUrl}${BULK_ADVISORY_PATH}`; - return await withAdvisoryRequestTimeout({ - label: "Bulk advisory request", - timeoutMs, - run: async ({ signal, timeoutPromise }) => { - const response = await fetchImpl(url, { - method: "POST", - headers: { - accept: "application/json", - "content-type": "application/json", - }, - body: JSON.stringify(payload), - signal, - }); + const request = async () => + await withAdvisoryRequestTimeout({ + label: "Bulk advisory request", + timeoutMs, + run: async ({ signal, timeoutPromise }) => { + const response = await fetchImpl(url, { + method: "POST", + headers: { + accept: "application/json", + "content-type": "application/json", + }, + body: JSON.stringify(payload), + signal, + }); - if (!response.ok) { - const bodyText = await readBoundedBulkAdvisoryErrorText(response, undefined, { + if (!response.ok) { + const bodyText = await readBoundedBulkAdvisoryErrorText(response, undefined, { + timeoutPromise, + }); + throw new Error( + `Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`, + ); + } + + return await readBulkAdvisoryJson(response, responseBodyMaxBytes, { + signal, timeoutPromise, }); - throw new Error( - `Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`, - ); - } - - return await readBulkAdvisoryJson(response, responseBodyMaxBytes, { - signal, - timeoutPromise, - }); - }, - }); + }, + }); + try { + return await request(); + } catch (error) { + if (!(error instanceof AdvisoryRequestTimeoutError)) { + throw error; + } + } + return await request(); } /** @param {PnpmAuditOptions} [options] */ diff --git a/test/scripts/dependency-vulnerability-gate.test.ts b/test/scripts/dependency-vulnerability-gate.test.ts index 79588e345856..b86b15c50c6a 100644 --- a/test/scripts/dependency-vulnerability-gate.test.ts +++ b/test/scripts/dependency-vulnerability-gate.test.ts @@ -84,7 +84,8 @@ function publishedAdvisory(range = ">= 0.8.0, < 1.0.0") { function withPublicUpstream(npmFetch: typeof fetch, advisories: unknown[] = []): typeof fetch { return async (input, init) => { - const url = new URL(String(input)); + const url = + input instanceof URL ? input : new URL(input instanceof Request ? input.url : input); if (url.pathname.endsWith("/advisories/bulk")) { return npmFetch(input, init); } @@ -255,7 +256,9 @@ describe("dependency-vulnerability-gate", () => { blocks: false, }, ]; - it.each(releaseLocks.flatMap((lockfile) => npmCases.map((entry) => ({ lockfile, ...entry }))))( + it.each( + releaseLocks.flatMap((lockfile) => npmCases.map((entry) => Object.assign({ lockfile }, entry))), + )( "$lockfile: $name stays isolated from the safe product version", async ({ lockfile, location, metadata, severity, title, graph, blocks }) => { await withLockfiles(async (rootDir) => { @@ -351,7 +354,8 @@ describe("dependency-vulnerability-gate", () => { it("writes partial coverage without claiming an unavailable upstream check passed", async () => { await withLockfiles(async (rootDir) => { const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => { - const url = new URL(String(input)); + const url = + input instanceof URL ? input : new URL(input instanceof Request ? input.url : input); return url.pathname.endsWith("/advisories/bulk") ? Response.json({}) : new Response("unavailable", { status: 503 }); @@ -457,20 +461,59 @@ describe("dependency-vulnerability-gate", () => { }); }); - it("propagates release-tool advisory transport failures", async () => { + it("recovers a release-tool advisory timeout through the shared fetch boundary", async () => { await withLockfiles(async (rootDir) => { await writeNpmLock(rootDir, releaseLocks[0], { - "node_modules/tool-only": { version: "1.0.0" }, + "node_modules/tool-only": { version: "1.0.0", dev: true }, }); + vi.stubEnv("OPENCLAW_PNPM_AUDIT_BULK_TIMEOUT_MS", "5"); + let calls = 0; + try { + const report = await runDependencyVulnerabilityGate({ + rootDir, + fetchImpl: withPublicUpstream(async (_url, init) => { + if (!requestPayload(init)["tool-only"]) { + return new Response("{}"); + } + calls += 1; + if (calls === 2) { + return new Response("{}"); + } + return await new Promise((_resolve, reject) => { + const signal = init?.signal; + signal?.addEventListener("abort", () => reject(signal.reason as Error), { + once: true, + }); + }); + }), + }); + expect(calls).toBe(2); + expect(report.blockers).toEqual([]); + } finally { + vi.unstubAllEnvs(); + } + }); + }); + + it("propagates release-tool advisory HTTP failures without retrying", async () => { + await withLockfiles(async (rootDir) => { + await writeNpmLock(rootDir, releaseLocks[0], { + "node_modules/tool-only": { version: "1.0.0", dev: true }, + }); + let calls = 0; await expect( runDependencyVulnerabilityGate({ rootDir, - fetchImpl: async (_url, init) => - requestPayload(init)["tool-only"] - ? new Response("fixture unavailable", { status: 503 }) - : new Response("{}"), + fetchImpl: withPublicUpstream(async (_url, init) => { + if (!requestPayload(init)["tool-only"]) { + return new Response("{}"); + } + calls += 1; + return new Response("fixture unavailable", { status: 503 }); + }), }), ).rejects.toThrow("Bulk advisory request failed (503"); + expect(calls).toBe(1); }); }); diff --git a/test/scripts/pnpm-audit-prod.test.ts b/test/scripts/pnpm-audit-prod.test.ts index 7084f231f645..4b04872bc085 100644 --- a/test/scripts/pnpm-audit-prod.test.ts +++ b/test/scripts/pnpm-audit-prod.test.ts @@ -3,7 +3,7 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { toErrorObject as toLintErrorObject } from "@openclaw/normalization-core/error-coercion"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { collectAllResolvedPackagesFromLockfile, collectProdResolvedPackagesFromLockfile, @@ -313,30 +313,119 @@ snapshots: }, ); - it("aborts stalled bulk advisory requests", async () => { - let signal: AbortSignal | undefined; + it("retries a timed-out bulk advisory request with a fresh request lifecycle", async () => { + const signals: AbortSignal[] = []; + const fetchImpl = vi.fn(((_url, init) => { + const signal = init?.signal; + if (signal) { + signals.push(signal); + } + if (signals.length === 2) { + return Promise.resolve(new Response("{}", { status: 200 })); + } + return new Promise((_resolve, reject) => { + signal?.addEventListener( + "abort", + () => reject(toLintErrorObject(signal.reason, "Non-Error rejection")), + { once: true }, + ); + }); + }) as typeof fetch); + + await expect( + fetchBulkAdvisories({ + payload: { axios: ["1.0.0"] }, + timeoutMs: 5, + fetchImpl, + }), + ).resolves.toEqual({}); + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(signals).toHaveLength(2); + expect(signals[0]).not.toBe(signals[1]); + expect(signals[0]?.aborted).toBe(true); + expect(signals[1]?.aborted).toBe(false); + }); + + it("stops after two timed-out bulk advisory requests", async () => { + const signals: AbortSignal[] = []; + const fetchImpl = vi.fn(((_url, init) => { + const signal = init?.signal; + if (signal) { + signals.push(signal); + } + return new Promise((_resolve, reject) => { + signal?.addEventListener( + "abort", + () => reject(toLintErrorObject(signal.reason, "Non-Error rejection")), + { once: true }, + ); + }); + }) as typeof fetch); const request = fetchBulkAdvisories({ payload: { axios: ["1.0.0"] }, timeoutMs: 5, - fetchImpl: ((_url, init) => { - signal = init?.signal ?? undefined; - return new Promise((_resolve, reject) => { - signal?.addEventListener( - "abort", - () => - reject( - toLintErrorObject(signal?.reason ?? new Error("aborted"), "Non-Error rejection"), - ), - { - once: true, - }, - ); - }); - }) as typeof fetch, + fetchImpl, }); await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u); - expect(signal?.aborted).toBe(true); + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(signals).toHaveLength(2); + expect(signals[0]).not.toBe(signals[1]); + expect(signals.every((signal) => signal.aborted)).toBe(true); + }); + + it("does not retry an untagged error with the timeout message", async () => { + const fetchImpl = vi.fn(async () => { + throw new Error("Bulk advisory request exceeded timeout of 5ms"); + }); + + await expect( + fetchBulkAdvisories({ + payload: { axios: ["1.0.0"] }, + timeoutMs: 5, + fetchImpl, + }), + ).rejects.toThrow("Bulk advisory request exceeded timeout of 5ms"); + expect(fetchImpl).toHaveBeenCalledOnce(); + }); + + it.each([ + { + caseName: "HTTP failures", + responseBodyMaxBytes: undefined, + response: () => + new Response("registry failure", { status: 500, statusText: "Internal Error" }), + expectedError: /Bulk advisory request failed \(500 Internal Error\)/u, + }, + { + caseName: "invalid JSON", + responseBodyMaxBytes: undefined, + response: () => new Response("{", { status: 200 }), + expectedError: /JSON/u, + }, + { + caseName: "empty bodies", + responseBodyMaxBytes: undefined, + response: () => new Response("", { status: 200 }), + expectedError: /Bulk advisory response body was empty/u, + }, + { + caseName: "oversized bodies", + responseBodyMaxBytes: 4, + response: () => new Response("12345", { status: 200 }), + expectedError: /Bulk advisory response body exceeded 4 bytes/u, + }, + ])("does not retry $caseName", async ({ responseBodyMaxBytes, response, expectedError }) => { + const fetchImpl = vi.fn(async () => response()); + + await expect( + fetchBulkAdvisories({ + payload: { axios: ["1.0.0"] }, + ...(responseBodyMaxBytes ? { responseBodyMaxBytes } : {}), + fetchImpl, + }), + ).rejects.toThrow(expectedError); + expect(fetchImpl).toHaveBeenCalledOnce(); }); it("clamps oversized bulk advisory request timers before scheduling", async () => { @@ -366,43 +455,49 @@ snapshots: }); it("cancels stalled successful bulk advisory response bodies on request timeout", async () => { - let cancelled = false; - const body = new ReadableStream({ - pull() { - return new Promise(() => {}); - }, - cancel() { - cancelled = true; - }, - }); + let cancellations = 0; const request = fetchBulkAdvisories({ payload: { axios: ["1.0.0"] }, timeoutMs: 5, - fetchImpl: async () => new Response(body, { status: 200 }), + fetchImpl: async () => + new Response( + new ReadableStream({ + pull() { + return new Promise(() => {}); + }, + cancel() { + cancellations += 1; + }, + }), + { status: 200 }, + ), }); await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u); - expect(cancelled).toBe(true); + expect(cancellations).toBe(2); }); it("cancels stalled failed bulk advisory response bodies on request timeout", async () => { - let cancelled = false; - const body = new ReadableStream({ - pull() { - return new Promise(() => {}); - }, - cancel() { - cancelled = true; - }, - }); + let cancellations = 0; const request = fetchBulkAdvisories({ payload: { axios: ["1.0.0"] }, timeoutMs: 5, - fetchImpl: async () => new Response(body, { status: 500, statusText: "Internal Error" }), + fetchImpl: async () => + new Response( + new ReadableStream({ + pull() { + return new Promise(() => {}); + }, + cancel() { + cancellations += 1; + }, + }), + { status: 500, statusText: "Internal Error" }, + ), }); await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u); - expect(cancelled).toBe(true); + expect(cancellations).toBe(2); }); it("bounds successful bulk advisory response bodies", async () => { @@ -491,7 +586,9 @@ snapshots: const exitCode = await runPnpmAuditProd({ rootDir: tempDir, fetchImpl: async (input) => { - expect(String(input)).toMatch(/\/-\/npm\/v1\/security\/advisories\/bulk$/u); + const url = + input instanceof URL ? input.href : input instanceof Request ? input.url : input; + expect(url).toMatch(/\/-\/npm\/v1\/security\/advisories\/bulk$/u); return new Response( JSON.stringify( blocked