Merge pull request #7023 from JSONbored/fix/lockfile-drift-osv-per-item-fallback

fix(rees): add per-item OSV fallback to lockfile-drift batch queries

Author: JSONbored

review-enrichment/src/analyzers/lockfile-drift.ts

@@ -54,6 +54,7 @@ const MAX_LOCKFILE_FILES = 12;
 const MAX_PATCH_LINES_PER_FILE = 1200;
 const MAX_OSV_QUERIES = 40;
 const LOCKFILE_OSV_BATCH_CHUNK_SIZE = 10;
+const LOCKFILE_OSV_QUERY_MAX_BYTES = 512 * 1024;
 const VERSION_SAFE_RE = /^[0-9][0-9A-Za-z._+-]*$/;
 const MAX_PACKAGE_LEN = 200;
 const MAX_VERSION_LEN = 100;
@@ -384,6 +385,44 @@ export function extractLockfileChanges(
   return changes;
 }
 
+/** Single-item OSV.dev fallback for a lockfile resolution whose batch chunk failed. Mirrors
+ *  dependency-scan.ts's fetchOsvDirect so a transient/oversized batch response degrades to per-item
+ *  queries instead of silently dropping the whole chunk's findings. */
+async function fetchOsvDirect(
+  ecosystem: string,
+  name: string,
+  version: string,
+  fetchImpl: typeof fetch,
+  signal: AbortSignal | undefined,
+  options: Pick<ScanOptions, "analysis" | "diagnostics" | "limits">,
+): Promise<Cve[]> {
+  if (signal?.aborted) return [];
+  const fetchOptions = {
+    endpointCategory: "osv-query",
+    method: "POST",
+    headers: { "content-type": "application/json" },
+    body: JSON.stringify({ package: { name, ecosystem }, version }),
+    signal,
+    fetchImpl,
+    diagnostics: options.diagnostics,
+    phase: "lockfile-drift",
+    subcall: "osv-query",
+    maxBytes: LOCKFILE_OSV_QUERY_MAX_BYTES,
+    maxCallsPerCategory: options.limits?.maxOsvQueries ?? MAX_OSV_QUERIES,
+  };
+  const response = options.analysis
+    ? await options.analysis.fetchJson<{ vulns?: OsvVuln[] }>(
+        "https://api.osv.dev/v1/query",
+        fetchOptions,
+      )
+    : await boundedFetchJson<{ vulns?: OsvVuln[] }>(
+        "https://api.osv.dev/v1/query",
+        fetchOptions,
+      );
+  if (!response.ok) return [];
+  return toCves(response.data.vulns);
+}
+
 /** Batch-query OSV.dev for lockfile resolutions. Best-effort: returns empty CVE arrays on any failure. */
 export async function queryOsvBatch(
   changes: LockfileChange[],
@@ -422,7 +461,20 @@ export async function queryOsvBatch(
       : await boundedFetchJson<{
         results?: Array<{ vulns?: OsvVuln[] }>;
       }>("https://api.osv.dev/v1/querybatch", fetchOptions);
-    if (!response.ok) continue;
+    if (!response.ok) {
+      for (const change of chunk) {
+        const cves = await fetchOsvDirect(
+          change.ecosystem,
+          change.package,
+          change.to,
+          fetchImpl,
+          signal,
+          options,
+        );
+        results.set(`${change.ecosystem}::${change.package}@${change.to}`, cves);
+      }
+      continue;
+    }
     chunk.forEach((change, index) => {
       results.set(
         `${change.ecosystem}::${change.package}@${change.to}`,

review-enrichment/test/lockfile-drift.test.ts

@@ -1,7 +1,15 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
 
-import { extractLockfileChanges } from "../dist/analyzers/lockfile-drift.js";
+import { extractLockfileChanges, queryOsvBatch } from "../dist/analyzers/lockfile-drift.js";
+import { createAnalysisContext } from "../dist/analysis-context.js";
+
+const jsonResponse = (body, init = {}) =>
+  new Response(JSON.stringify(body), {
+    status: 200,
+    headers: { "content-type": "application/json" },
+    ...init,
+  });
 
 test("extractLockfileChanges matches lockfile basenames case-insensitively", () => {
   const changes = extractLockfileChanges([
@@ -252,3 +260,119 @@ test("extractLockfileChanges skips malformed/partial lockfile hunks rather than
 
   assert.deepEqual(changes, []);
 });
+
+test("queryOsvBatch falls back to per-item OSV queries when a batch chunk fails, instead of dropping the whole chunk", async () => {
+  // Mirrors dependency-scan.ts's own batch-failure fallback (scanDependencyChanges falls back to direct
+  // OSV queries after an oversized batch response, test/analysis-context.test.ts): a failed /v1/querybatch
+  // chunk must degrade to per-change /v1/query calls, not silently drop every finding in the chunk.
+  let batchCalls = 0;
+  const directPackages = [];
+  const fetchImpl = async (url, init = {}) => {
+    if (String(url) === "https://api.osv.dev/v1/querybatch") {
+      batchCalls += 1;
+      return new Response("Internal Server Error", { status: 500 });
+    }
+    assert.equal(String(url), "https://api.osv.dev/v1/query");
+    const body = JSON.parse(String(init.body));
+    directPackages.push(body.package.name);
+    return jsonResponse({
+      vulns:
+        body.package.name === "lodash"
+          ? [
+              {
+                id: "GHSA-lockfile-fallback",
+                summary: "lockfile drift fallback advisory",
+                database_specific: { severity: "HIGH" },
+              },
+            ]
+          : [],
+    });
+  };
+  const changes = [
+    { file: "package-lock.json", line: 5, ecosystem: "npm", package: "lodash", from: "4.17.20", to: "4.17.21" },
+    { file: "package-lock.json", line: 9, ecosystem: "npm", package: "axios", from: "1.6.0", to: "1.6.1" },
+  ];
+
+  const cvesByKey = await queryOsvBatch(changes, fetchImpl);
+
+  assert.equal(batchCalls, 1);
+  assert.deepEqual(directPackages, ["lodash", "axios"]);
+  assert.equal(cvesByKey.size, 2);
+  const lodashCves = cvesByKey.get("npm::lodash@4.17.21");
+  assert.equal(lodashCves?.length, 1);
+  assert.equal(lodashCves?.[0]?.id, "GHSA-lockfile-fallback");
+  assert.deepEqual(cvesByKey.get("npm::axios@1.6.1"), []);
+});
+
+test("queryOsvBatch's per-item fallback still returns empty (never throws) when the direct query also fails", async () => {
+  const fetchImpl = async () => new Response("Internal Server Error", { status: 500 });
+  const changes = [
+    { file: "package-lock.json", line: 5, ecosystem: "npm", package: "lodash", from: "4.17.20", to: "4.17.21" },
+  ];
+
+  const cvesByKey = await queryOsvBatch(changes, fetchImpl);
+
+  assert.deepEqual(cvesByKey.get("npm::lodash@4.17.21"), []);
+});
+
+test("queryOsvBatch's per-item fallback routes through the request-scoped analysis context (cache/metrics), honoring a custom maxOsvQueries limit", async () => {
+  const context = createAnalysisContext({
+    repoFullName: "JSONbored/loopover",
+    prNumber: 1810,
+  });
+  let batchCalls = 0;
+  const directPackages = [];
+  const fetchImpl = async (url, init = {}) => {
+    if (String(url) === "https://api.osv.dev/v1/querybatch") {
+      batchCalls += 1;
+      return new Response("Internal Server Error", { status: 500 });
+    }
+    assert.equal(String(url), "https://api.osv.dev/v1/query");
+    const body = JSON.parse(String(init.body));
+    directPackages.push(body.package.name);
+    return jsonResponse({ vulns: [] });
+  };
+  const changes = [
+    { file: "package-lock.json", line: 5, ecosystem: "npm", package: "lodash", from: "4.17.20", to: "4.17.21" },
+  ];
+
+  const cvesByKey = await queryOsvBatch(changes, fetchImpl, undefined, {
+    analysis: context,
+    limits: { maxOsvQueries: 5 },
+  });
+
+  assert.equal(batchCalls, 1);
+  assert.deepEqual(directPackages, ["lodash"]);
+  assert.deepEqual(cvesByKey.get("npm::lodash@4.17.21"), []);
+  assert.deepEqual(context.snapshotMetrics().externalCallsByCategory, {
+    "osv-querybatch": 1,
+    "osv-query": 1,
+  });
+});
+
+test("queryOsvBatch's per-item fallback stops issuing direct queries once the signal is aborted mid-chunk", async () => {
+  const controller = new AbortController();
+  const directPackages = [];
+  const fetchImpl = async (url, init = {}) => {
+    if (String(url) === "https://api.osv.dev/v1/querybatch") {
+      return new Response("Internal Server Error", { status: 500 });
+    }
+    const body = JSON.parse(String(init.body));
+    directPackages.push(body.package.name);
+    // Abort AFTER the first per-item request is issued but before the fallback loop reaches the
+    // second change -- exercises fetchOsvDirect's own `if (signal?.aborted) return [];` guard.
+    controller.abort();
+    return jsonResponse({ vulns: [] });
+  };
+  const changes = [
+    { file: "package-lock.json", line: 5, ecosystem: "npm", package: "lodash", from: "4.17.20", to: "4.17.21" },
+    { file: "package-lock.json", line: 9, ecosystem: "npm", package: "axios", from: "1.6.0", to: "1.6.1" },
+  ];
+
+  const cvesByKey = await queryOsvBatch(changes, fetchImpl, controller.signal);
+
+  assert.deepEqual(directPackages, ["lodash"]);
+  assert.deepEqual(cvesByKey.get("npm::lodash@4.17.21"), []);
+  assert.equal(cvesByKey.has("npm::axios@1.6.1"), true);
+  assert.deepEqual(cvesByKey.get("npm::axios@1.6.1"), []);
+});