fix(enrichment): fail closed on dependency scan truncation

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

@@ -25,6 +25,13 @@ interface ScanOptions {
   limits?: ScanLimits;
 }
 
+export class DependencyScanTruncatedError extends Error {
+  constructor(reason: string) {
+    super(`dependency_scan_truncated:${reason}`);
+    this.name = "DependencyScanTruncatedError";
+  }
+}
+
 // 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][^"]*)"/;
@@ -73,8 +80,12 @@ export function extractDependencyChanges(
     const ecosystem = ECOSYSTEM[manifest];
     if (!ecosystem || !file.patch) continue;
     manifestFiles += 1;
-    if (manifestFiles > maxManifestFiles) break;
-    for (const line of file.patch.split("\n", maxPatchLinesPerFile)) {
+    if (manifestFiles > maxManifestFiles)
+      throw new DependencyScanTruncatedError("manifest_file_limit");
+    const patchLines = file.patch.split("\n");
+    if (patchLines.length > maxPatchLinesPerFile)
+      throw new DependencyScanTruncatedError("patch_line_limit");
+    for (const line of patchLines) {
       const sign = line[0];
       if (
         (sign !== "+" && sign !== "-") ||
@@ -180,10 +191,11 @@ export async function scanDependencies(
   fetchImpl: typeof fetch = fetch,
   options: ScanOptions = {},
 ): Promise<DependencyFinding[]> {
-  const changes = extractDependencyChanges(req.files ?? [], options.limits).slice(
-    0,
-    options.limits?.maxDependencyQueries ?? MAX_DEPENDENCY_QUERIES,
-  );
+  const changes = extractDependencyChanges(req.files ?? [], options.limits);
+  const maxDependencyQueries =
+    options.limits?.maxDependencyQueries ?? MAX_DEPENDENCY_QUERIES;
+  if (changes.length > maxDependencyQueries)
+    throw new DependencyScanTruncatedError("dependency_query_limit");
   const findings: DependencyFinding[] = [];
   for (const change of changes) {
     if (options.signal?.aborted) break;

review-enrichment/test/enrichment.test.ts

@@ -1,6 +1,7 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
 import {
+  DependencyScanTruncatedError,
   extractDependencyChanges,
   queryOsv,
   scanDependencies,
@@ -133,6 +134,80 @@ test("scanDependencies: only deps with vulns are returned", async () => {
   assert.equal(findings[0].cves[0].severity, "critical");
 });
 
+test("scanDependencies: truncation fails closed instead of hiding later vulnerable deps", async () => {
+  const files = [
+    {
+      path: "package.json",
+      patch: Array.from(
+        { length: 26 },
+        (_, i) => `+    "pkg-${i}": "1.0.0",`,
+      ).join("\n"),
+    },
+  ];
+  let calls = 0;
+  await assert.rejects(
+    scanDependencies(
+      { repoFullName: "o/r", prNumber: 1, files },
+      async () => {
+        calls += 1;
+        return { ok: true, json: async () => ({ vulns: [] }) };
+      },
+    ),
+    DependencyScanTruncatedError,
+  );
+  assert.equal(calls, 0);
+});
+
+test("scanDependencies: manifest and patch-line truncation fail closed", () => {
+  const manifestFiles = Array.from({ length: 21 }, (_, i) => ({
+    path: `pkg-${i}/package.json`,
+    patch: `+    "pkg-${i}": "1.0.0",`,
+  }));
+  assert.throws(
+    () => extractDependencyChanges(manifestFiles),
+    DependencyScanTruncatedError,
+  );
+
+  assert.throws(
+    () =>
+      extractDependencyChanges([
+        {
+          path: "package.json",
+          patch: Array.from({ length: 501 }, (_, i) =>
+            i === 500 ? '+    "lodash": "4.17.20",' : " context",
+          ).join("\n"),
+        },
+      ]),
+    DependencyScanTruncatedError,
+  );
+});
+
+test("buildBrief: truncated dependency scan marks the brief degraded", async () => {
+  const realFetch = globalThis.fetch;
+  globalThis.fetch = okFetch([]);
+  try {
+    const brief = await buildBrief({
+      repoFullName: "o/r",
+      prNumber: 9,
+      analyzers: ["dependency"],
+      files: [
+        {
+          path: "package.json",
+          patch: Array.from(
+            { length: 26 },
+            (_, i) => `+    "pkg-${i}": "1.0.0",`,
+          ).join("\n"),
+        },
+      ],
+    });
+    assert.equal(brief.partial, true);
+    assert.equal(brief.analyzerStatus.dependency, "degraded");
+    assert.equal(brief.findings.dependency, undefined);
+  } finally {
+    globalThis.fetch = realFetch;
+  }
+});
+
 test("renderBrief: sorts by severity, empty when no findings", () => {
   const empty = renderBrief({});
   assert.equal(empty.promptSection, "");
@@ -597,27 +672,37 @@ 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 },
+test("extractDependencyChanges: rejects truncated manifest files and patch lines", () => {
+  assert.throws(
+    () =>
+      extractDependencyChanges(
+        [
+          { path: "package.json", patch: '+    "first": "1.0.0",' },
+          { path: "nested/package.json", patch: '+    "second": "1.0.0",' },
+        ],
+        { maxManifestFiles: 1 },
+      ),
+    DependencyScanTruncatedError,
   );
 
-  assert.deepEqual(
-    changes.map((change) => change.package),
-    ["first"],
+  assert.throws(
+    () =>
+      extractDependencyChanges(
+        [
+          {
+            path: "package.json",
+            patch: ['+    "first": "1.0.0",', '+    "second": "1.0.0",'].join(
+              "\n",
+            ),
+          },
+        ],
+        { maxPatchLinesPerFile: 1 },
+      ),
+    DependencyScanTruncatedError,
   );
 });
 
-test("scanDependencies: caps OSV queries and forwards abort signals", async () => {
+test("scanDependencies: forwards abort signals without truncation", async () => {
   const seenSignals = [];
   const files = Array.from({ length: 3 }, (_, index) => ({
     path: "package.json",
@@ -631,11 +716,11 @@ test("scanDependencies: caps OSV queries and forwards abort signals", async () =
       seenSignals.push(init.signal);
       return { ok: true, json: async () => ({ vulns: [] }) };
     },
-    { signal: controller.signal, limits: { maxDependencyQueries: 2 } },
+    { signal: controller.signal, limits: { maxDependencyQueries: 3 } },
   );
 
   assert.equal(findings.length, 0);
-  assert.equal(seenSignals.length, 2);
+  assert.equal(seenSignals.length, 3);
   assert.ok(seenSignals.every((signal) => signal instanceof AbortSignal));
 });