fix(tooling): timeout transitive manifest packuments

vincentkoc · vincentkoc · commit 845ad1cf71b9 · 2026-06-19T05:55:00.000+02:00
diff --git a/scripts/transitive-manifest-risk-report.mjs b/scripts/transitive-manifest-risk-report.mjs
@@ -6,6 +6,7 @@ import { readFile } from "node:fs/promises";
 import path from "node:path";
 import process from "node:process";
 import YAML from "yaml";
+import { readBoundedResponseText } from "./lib/bounded-response.mjs";
 import { parseReportCliArgs, writeReportArtifact } from "./lib/report-cli-helpers.mjs";
 import {
   collectAllResolvedPackagesFromLockfile,
@@ -24,6 +25,7 @@ const RECENTLY_PUBLISHED_VERSION_TYPE = "recently-published-version";
 const NPM_PACKUMENT_ACCEPT_HEADER = "application/json";
 /** Maximum npm packument response size accepted by the risk scanner. */
 export const NPM_PACKUMENT_RESPONSE_MAX_BYTES = 16 * 1024 * 1024;
+const NPM_PACKUMENT_FETCH_TIMEOUT_MS = 60_000;
 
 function isAllowedPinnedSpec(spec) {
   if (typeof spec !== "string") {
@@ -64,47 +66,12 @@ function isExoticResolvedVersion(version) {
 export async function readBoundedNpmRegistryText(
   response,
   maxBytes = NPM_PACKUMENT_RESPONSE_MAX_BYTES,
+  options = {},
 ) {
-  const contentLength = response.headers?.get?.("content-length");
-  if (contentLength && /^\d+$/u.test(contentLength)) {
-    const parsedContentLength = Number(contentLength);
-    if (Number.isSafeInteger(parsedContentLength) && parsedContentLength > maxBytes) {
-      await response.body?.cancel().catch(() => {});
-      throw new Error(
-        `npm registry response exceeded ${maxBytes} bytes (content-length ${contentLength})`,
-      );
-    }
-  }
-
-  if (!response.body) {
-    return "";
-  }
-
-  const reader = response.body.getReader();
-  const decoder = new TextDecoder();
-  let receivedBytes = 0;
-  let text = "";
-
-  try {
-    while (true) {
-      const { done, value } = await reader.read();
-      if (done) {
-        break;
-      }
-      receivedBytes += value.byteLength;
-      if (receivedBytes > maxBytes) {
-        await reader.cancel().catch(() => {});
-        throw new Error(
-          `npm registry response exceeded ${maxBytes} bytes while reading response body`,
-        );
-      }
-      text += decoder.decode(value, { stream: true });
-    }
-    text += decoder.decode();
-    return text;
-  } finally {
-    reader.releaseLock();
-  }
+  return await readBoundedResponseText(response, "npm registry", maxBytes, {
+    signal: options.signal,
+    formatTooLargeMessage: (_label, bytes) => `npm registry response exceeded ${bytes} bytes`,
+  });
 }
 
 function packageVersionsFromPayload(payload) {
@@ -266,16 +233,19 @@ export async function fetchNpmManifest({
   fetchImpl,
   registryBaseUrl,
   maxBytes = NPM_PACKUMENT_RESPONSE_MAX_BYTES,
+  timeoutMs = NPM_PACKUMENT_FETCH_TIMEOUT_MS,
 }) {
+  const signal = AbortSignal.timeout(timeoutMs);
   const response = await fetchImpl(`${registryBaseUrl}/${encodePackageName(packageName)}`, {
     headers: {
       Accept: NPM_PACKUMENT_ACCEPT_HEADER,
     },
+    signal,
   });
   if (!response.ok) {
     throw new Error(`${response.status} ${response.statusText}`);
   }
-  const packumentText = await readBoundedNpmRegistryText(response, maxBytes);
+  const packumentText = await readBoundedNpmRegistryText(response, maxBytes, { signal });
   const packument = JSON.parse(packumentText);
   const manifest = packument.versions?.[version];
   if (!manifest) {
diff --git a/test/scripts/transitive-manifest-risk-report.test.ts b/test/scripts/transitive-manifest-risk-report.test.ts
@@ -179,7 +179,8 @@ describe("transitive-manifest-risk-report", () => {
   });
 
   it("fetches full npm packuments for the requested manifest version", async () => {
-    const fetchCalls: Array<{ url: string; accept: string | null }> = [];
+    const fetchCalls: Array<{ url: string; accept: string | null; signal: AbortSignal | null }> =
+      [];
     const manifest = await fetchNpmManifest({
       packageName: "@scope/package",
       version: "1.0.0",
@@ -188,6 +189,7 @@ describe("transitive-manifest-risk-report", () => {
         fetchCalls.push({
           url: String(url),
           accept: new Headers(init?.headers).get("accept"),
+          signal: init?.signal instanceof AbortSignal ? init.signal : null,
         });
         return new Response(
           JSON.stringify({
@@ -216,6 +218,7 @@ describe("transitive-manifest-risk-report", () => {
       {
         url: "https://registry.example.test/@scope%2fpackage",
         accept: "application/json",
+        signal: expect.any(AbortSignal),
       },
     ]);
     expect(manifest).toEqual({
@@ -231,6 +234,37 @@ describe("transitive-manifest-risk-report", () => {
     });
   });
 
+  it("cancels stalled npm registry body reads when the request aborts", async () => {
+    const controller = new AbortController();
+    let canceled = false;
+    const response = {
+      headers: new Headers(),
+      body: {
+        getReader() {
+          return {
+            read() {
+              return new Promise<ReadableStreamReadResult<Uint8Array>>(() => {});
+            },
+            async cancel() {
+              canceled = true;
+            },
+            releaseLock() {
+              throw new Error("releaseLock should not run while a read is pending");
+            },
+          };
+        },
+      },
+    } as unknown as Response;
+
+    const readPromise = readBoundedNpmRegistryText(response, 8, {
+      signal: controller.signal,
+    });
+    controller.abort(new Error("npm registry request timed out"));
+
+    await expect(readPromise).rejects.toThrow("npm registry request timed out");
+    expect(canceled).toBe(true);
+  });
+
   it("rejects npm registry bodies that exceed the content-length cap", async () => {
     let canceled = false;
     const response = new Response(
@@ -247,7 +281,7 @@ describe("transitive-manifest-risk-report", () => {
     );
 
     await expect(readBoundedNpmRegistryText(response, 8)).rejects.toThrow(
-      "npm registry response exceeded 8 bytes (content-length 12)",
+      "npm registry response exceeded 8 bytes",
     );
     expect(canceled).toBe(true);
   });
@@ -260,7 +294,7 @@ describe("transitive-manifest-risk-report", () => {
     });
 
     await expect(readBoundedNpmRegistryText(response, 8)).rejects.toThrow(
-      "npm registry response exceeded 8 bytes (content-length 12)",
+      "npm registry response exceeded 8 bytes",
     );
   });
 
@@ -306,7 +340,7 @@ describe("transitive-manifest-risk-report", () => {
     );
 
     await expect(readBoundedNpmRegistryText(response, 8)).rejects.toThrow(
-      "npm registry response exceeded 8 bytes while reading response body",
+      "npm registry response exceeded 8 bytes",
     );
   });
 });
