fix(enrichment): bound dependency scans

review-enrichment/src/analyzers/dependency-scan.ts

@@ -10,6 +10,21 @@ interface DepChange {
   to: string;
 }
 
+const MAX_MANIFEST_FILES = 20;
+const MAX_PATCH_LINES_PER_FILE = 500;
+const MAX_DEPENDENCY_QUERIES = 25;
+
+interface ScanLimits {
+  maxManifestFiles?: number;
+  maxPatchLinesPerFile?: number;
+  maxDependencyQueries?: number;
+}
+
+interface ScanOptions {
+  signal?: AbortSignal;
+  limits?: ScanLimits;
+}
+
 // Per-manifest line parsers. Each returns [name, version] for a `+`/`-` diff line, or null. Heuristic (line-based,
 // not a full manifest parse) — good enough to flag the deps a PR adds/bumps without resolving the whole tree.
 const NPM_RE = /^"([^"]+)"\s*:\s*"([\^~>=<\s]*[0-9][^"]*)"/;
@@ -43,16 +58,23 @@ const ECOSYSTEM: Record<string, string> = {
 /** Extract added/changed (not removed) dependency versions from the changed manifests in the diff. Pure. */
 export function extractDependencyChanges(
   files: NonNullable<EnrichRequest["files"]>,
+  limits: ScanLimits = {},
 ): DepChange[] {
   const byKey = new Map<
     string,
     { ecosystem: string; package: string; added?: string; removed?: string }
   >();
+  const maxManifestFiles = limits.maxManifestFiles ?? MAX_MANIFEST_FILES;
+  const maxPatchLinesPerFile =
+    limits.maxPatchLinesPerFile ?? MAX_PATCH_LINES_PER_FILE;
+  let manifestFiles = 0;
   for (const file of files) {
     const manifest = file.path.split("/").pop() ?? file.path;
     const ecosystem = ECOSYSTEM[manifest];
     if (!ecosystem || !file.patch) continue;
-    for (const line of file.patch.split("\n")) {
+    manifestFiles += 1;
+    if (manifestFiles > maxManifestFiles) break;
+    for (const line of file.patch.split("\n", maxPatchLinesPerFile)) {
       const sign = line[0];
       if (
         (sign !== "+" && sign !== "-") ||
@@ -131,11 +153,14 @@ export async function queryOsv(
   name: string,
   version: string,
   fetchImpl: typeof fetch = fetch,
+  signal?: AbortSignal,
 ): Promise<Cve[]> {
+  if (signal?.aborted) return [];
   const response = await fetchImpl("https://api.osv.dev/v1/query", {
     method: "POST",
     headers: { "content-type": "application/json" },
     body: JSON.stringify({ package: { name, ecosystem }, version }),
+    signal,
   });
   if (!response.ok) return [];
   const data = (await response.json()) as { vulns?: OsvVuln[] };
@@ -153,15 +178,21 @@ export async function queryOsv(
 export async function scanDependencies(
   req: EnrichRequest,
   fetchImpl: typeof fetch = fetch,
+  options: ScanOptions = {},
 ): Promise<DependencyFinding[]> {
-  const changes = extractDependencyChanges(req.files ?? []);
+  const changes = extractDependencyChanges(req.files ?? [], options.limits).slice(
+    0,
+    options.limits?.maxDependencyQueries ?? MAX_DEPENDENCY_QUERIES,
+  );
   const findings: DependencyFinding[] = [];
   for (const change of changes) {
+    if (options.signal?.aborted) break;
     const cves = await queryOsv(
       change.ecosystem,
       change.package,
       change.to,
       fetchImpl,
+      options.signal,
     );
     if (cves.length) {
       findings.push({

review-enrichment/src/brief.ts

@@ -15,22 +15,29 @@ import { scanActionPins } from "./analyzers/actions-pin.js";
 import { scanEol } from "./analyzers/eol-check.js";
 import { renderBrief } from "./render.js";
 
-type AnalyzerFn = (req: EnrichRequest) => Promise<unknown>;
+type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise<unknown>;
 
 // The analyzer registry. More land behind this same shape: license (#1475), secret (#1476), static (#1477), history (#1478).
 const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
-  dependency: (req) => scanDependencies(req),
+  dependency: (req, signal) => scanDependencies(req, fetch, { signal }),
   secret: (req) => scanSecrets(req),
   license: (req) => scanLicenses(req),
   installScript: (req) => scanInstallScripts(req),
   actionPin: (req) => scanActionPins(req),
   eol: (req) => scanEol(req),
 };
 
-function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
+function runWithTimeout<T>(
+  run: (signal: AbortSignal) => Promise<T>,
+  ms: number,
+): Promise<T> {
+  const controller = new AbortController();
   return new Promise((resolve, reject) => {
-    const timer = setTimeout(() => reject(new Error("analyzer_timeout")), ms);
-    promise.then(
+    const timer = setTimeout(() => {
+      controller.abort();
+      reject(new Error("analyzer_timeout"));
+    }, ms);
+    run(controller.signal).then(
       (value) => {
         clearTimeout(timer);
         resolve(value);
@@ -58,7 +65,10 @@ export async function buildBrief(req: EnrichRequest): Promise<ReviewBrief> {
   await Promise.all(
     requested.map(async (name) => {
       try {
-        const result = await withTimeout(ANALYZERS[name](req), budgetMs);
+        const result = await runWithTimeout(
+          (signal) => ANALYZERS[name](req, signal),
+          budgetMs,
+        );
         findings[name] = result as never;
         analyzerStatus[name] = "ok";
       } catch {

review-enrichment/test/enrichment.test.ts

@@ -529,6 +529,86 @@ test("buildBrief: action-pin analyzer runs (pure, no network)", async () => {
   }
 });
 
+test("extractDependencyChanges: caps manifest files and patch lines", () => {
+  const changes = extractDependencyChanges(
+    [
+      {
+        path: "package.json",
+        patch: ['+    "first": "1.0.0",', '+    "second": "1.0.0",'].join(
+          "\n",
+        ),
+      },
+      { path: "nested/package.json", patch: '+    "third": "1.0.0",' },
+    ],
+    { maxManifestFiles: 1, maxPatchLinesPerFile: 1 },
+  );
+
+  assert.deepEqual(
+    changes.map((change) => change.package),
+    ["first"],
+  );
+});
+
+test("scanDependencies: caps OSV queries and forwards abort signals", async () => {
+  const seenSignals = [];
+  const files = Array.from({ length: 3 }, (_, index) => ({
+    path: "package.json",
+    patch: `+    "pkg-${index}": "1.0.0",`,
+  }));
+
+  const controller = new AbortController();
+  const findings = await scanDependencies(
+    { repoFullName: "o/r", prNumber: 1, files },
+    async (_url, init) => {
+      seenSignals.push(init.signal);
+      return { ok: true, json: async () => ({ vulns: [] }) };
+    },
+    { signal: controller.signal, limits: { maxDependencyQueries: 2 } },
+  );
+
+  assert.equal(findings.length, 0);
+  assert.equal(seenSignals.length, 2);
+  assert.ok(seenSignals.every((signal) => signal instanceof AbortSignal));
+});
+
+test("buildBrief: timeout aborts dependency scan so OSV work stops", async () => {
+  const realFetch = globalThis.fetch;
+  const signals = [];
+  let fetchCount = 0;
+  globalThis.fetch = async (_url, init) => {
+    fetchCount += 1;
+    signals.push(init.signal);
+    return await new Promise((_resolve, reject) => {
+      init.signal.addEventListener("abort", () => reject(new Error("aborted")), {
+        once: true,
+      });
+    });
+  };
+
+  try {
+    const brief = await buildBrief({
+      repoFullName: "o/r",
+      prNumber: 10,
+      analyzers: ["dependency"],
+      budget: { timeoutMs: 1 },
+      files: Array.from({ length: 5 }, (_, index) => ({
+        path: "package.json",
+        patch: `+    "pkg-${index}": "1.0.0",`,
+      })),
+    });
+
+    assert.equal(brief.partial, true);
+    assert.equal(brief.analyzerStatus.dependency, "degraded");
+    assert.equal(fetchCount, 1);
+    assert.equal(signals.length, 1);
+    assert.equal(signals[0].aborted, true);
+    await new Promise((resolve) => setTimeout(resolve, 10));
+    assert.equal(fetchCount, 1);
+  } finally {
+    globalThis.fetch = realFetch;
+  }
+});
+
 test("extractVersionPins: Dockerfile FROM + .nvmrc + go.mod; latest skipped", () => {
   const pins = extractVersionPins([
     {