From f0207d3ea03bbda0949f1f57a697cc40ab7d83f0 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 28 May 2026 22:56:38 +0200 Subject: [PATCH] fix(security): bound prod audit registry responses --- scripts/pre-commit/pnpm-audit-prod.mjs | 135 ++++++++++++++++++++++--- test/scripts/pnpm-audit-prod.test.ts | 43 ++++++++ 2 files changed, 163 insertions(+), 15 deletions(-) diff --git a/scripts/pre-commit/pnpm-audit-prod.mjs b/scripts/pre-commit/pnpm-audit-prod.mjs index 35833c32f12..890cd9f2a84 100644 --- a/scripts/pre-commit/pnpm-audit-prod.mjs +++ b/scripts/pre-commit/pnpm-audit-prod.mjs @@ -9,6 +9,8 @@ const DEFAULT_REGISTRY = "https://registry.npmjs.org"; const BULK_ADVISORY_PATH = "/-/npm/v1/security/advisories/bulk"; const MIN_SEVERITY = "high"; export const BULK_ADVISORY_ERROR_BODY_MAX_CHARS = 4096; +export const BULK_ADVISORY_RESPONSE_BODY_MAX_BYTES = 8 * 1024 * 1024; +export const BULK_ADVISORY_REQUEST_TIMEOUT_MS = 60_000; const SEVERITY_RANK = { info: 0, low: 1, @@ -677,6 +679,92 @@ function resolveRegistryBaseUrl() { return configured.replace(/\/+$/u, ""); } +function parsePositiveIntegerEnv(name, fallback) { + const raw = process.env[name]?.trim(); + if (!raw) { + return fallback; + } + const parsed = Number.parseInt(raw, 10); + if (!Number.isSafeInteger(parsed) || parsed < 1 || String(parsed) !== raw) { + throw new Error(`${name} must be a positive integer`); + } + return parsed; +} + +function resolveBulkAdvisoryRequestTimeoutMs() { + return parsePositiveIntegerEnv( + "OPENCLAW_PNPM_AUDIT_BULK_TIMEOUT_MS", + BULK_ADVISORY_REQUEST_TIMEOUT_MS, + ); +} + +function resolveBulkAdvisoryResponseBodyMaxBytes() { + return parsePositiveIntegerEnv( + "OPENCLAW_PNPM_AUDIT_BULK_RESPONSE_MAX_BYTES", + BULK_ADVISORY_RESPONSE_BODY_MAX_BYTES, + ); +} + +async function withBulkAdvisoryTimeout({ label, timeoutMs, run }) { + const controller = new AbortController(); + let timeout; + try { + return await Promise.race([ + run(controller.signal), + new Promise((_resolve, reject) => { + timeout = setTimeout(() => { + const error = new Error(`${label} exceeded timeout of ${timeoutMs}ms`); + controller.abort(error); + reject(error); + }, timeoutMs); + }), + ]); + } finally { + if (timeout) { + clearTimeout(timeout); + } + } +} + +async function readBoundedResponseText(response, maxBytes, label) { + const contentLength = Number.parseInt(response.headers?.get?.("content-length") ?? "", 10); + if (Number.isFinite(contentLength) && contentLength > maxBytes) { + throw Object.assign(new Error(`${label} exceeded ${maxBytes} bytes`), { code: "ETOOBIG" }); + } + + if (!response.body) { + return ""; + } + + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + const chunks = []; + let totalBytes = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) { + const tail = decoder.decode(); + if (tail) { + chunks.push(tail); + } + break; + } + + totalBytes += value.byteLength; + if (totalBytes > maxBytes) { + await reader.cancel().catch(() => undefined); + throw Object.assign(new Error(`${label} exceeded ${maxBytes} bytes`), { code: "ETOOBIG" }); + } + chunks.push(decoder.decode(value, { stream: true })); + } + } finally { + reader.releaseLock(); + } + + return chunks.join(""); +} + export async function readBoundedBulkAdvisoryErrorText( response, maxChars = BULK_ADVISORY_ERROR_BODY_MAX_CHARS, @@ -716,29 +804,46 @@ export async function readBoundedBulkAdvisoryErrorText( return truncated ? `${text}\n[truncated]` : text; } +async function readBulkAdvisoryJson(response, maxBytes) { + const text = await readBoundedResponseText(response, maxBytes, "Bulk advisory response body"); + if (!text.trim()) { + throw new Error("Bulk advisory response body was empty"); + } + return JSON.parse(text); +} + export async function fetchBulkAdvisories({ payload, fetchImpl = fetch, registryBaseUrl = resolveRegistryBaseUrl(), + responseBodyMaxBytes = resolveBulkAdvisoryResponseBodyMaxBytes(), + timeoutMs = resolveBulkAdvisoryRequestTimeoutMs(), }) { const url = `${registryBaseUrl}${BULK_ADVISORY_PATH}`; - const response = await fetchImpl(url, { - method: "POST", - headers: { - accept: "application/json", - "content-type": "application/json", + return await withBulkAdvisoryTimeout({ + label: "Bulk advisory request", + timeoutMs, + run: async (signal) => { + 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); + throw new Error( + `Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`, + ); + } + + return await readBulkAdvisoryJson(response, responseBodyMaxBytes); }, - body: JSON.stringify(payload), }); - - if (!response.ok) { - const bodyText = await readBoundedBulkAdvisoryErrorText(response); - throw new Error( - `Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`, - ); - } - - return response.json(); } export async function runPnpmAuditProd({ diff --git a/test/scripts/pnpm-audit-prod.test.ts b/test/scripts/pnpm-audit-prod.test.ts index a5a30a30c76..026106df576 100644 --- a/test/scripts/pnpm-audit-prod.test.ts +++ b/test/scripts/pnpm-audit-prod.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest"; import { collectProdResolvedPackagesFromLockfile, createBulkAdvisoryPayload, + fetchBulkAdvisories, filterFindingsBySeverity, parseSnapshotKey, readBoundedBulkAdvisoryErrorText, @@ -231,6 +232,48 @@ snapshots: expect(text.length).toBeLessThan(4200); }); + it("aborts stalled bulk advisory requests", async () => { + let signal: AbortSignal | undefined; + 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(signal?.reason ?? new Error("aborted")), { + once: true, + }); + }); + }) as typeof fetch, + }); + + await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u); + expect(signal?.aborted).toBe(true); + }); + + it("bounds successful bulk advisory response bodies", async () => { + const request = fetchBulkAdvisories({ + payload: { axios: ["1.0.0"] }, + responseBodyMaxBytes: 4, + fetchImpl: async () => + new Response("{}", { + status: 200, + headers: { "content-length": "5" }, + }), + }); + + await expect(request).rejects.toThrow(/Bulk advisory response body exceeded 4 bytes/u); + }); + + it("fails closed on empty successful bulk advisory response bodies", async () => { + const request = fetchBulkAdvisories({ + payload: { axios: ["1.0.0"] }, + fetchImpl: async () => new Response("", { status: 200 }), + }); + + await expect(request).rejects.toThrow(/Bulk advisory response body was empty/u); + }); + it("returns a failing exit code when bulk advisories include high severity findings", async () => { const tempDir = await mkdtemp(path.join(tmpdir(), "openclaw-audit-prod-")); await writeFile(