fix(review): scan patch-less PR files for leaked secrets

src/queue/processors.ts

@@ -375,7 +375,9 @@ import {
   buildReviewGroundingText,
   checkSummaryText as checkFailureSummaryText,
   isGroundingEnabled,
+  makeGithubFileFetcher,
 } from "../review/grounding-wire";
+import type { FileFetcher } from "../review/review-grounding";
 import {
   attributeReviewRagTelemetry,
   buildReviewRagContextWithMetrics,
@@ -5467,6 +5469,9 @@ export function buildAiReviewDiff(
  * Build the complete inline patch corpus for deterministic secret scanning. Unlike {@link buildAiReviewDiff},
  * this is intentionally unbudgeted and does not reorder files or drop hunks: security controls must inspect
  * every raw patch GitHub returned instead of the lossy AI-review prompt view.
+ *
+ * GitHub omits inline `patch` for binary/large files; {@link enrichSecretScanFilesWithPatchFallback} recovers
+ * scannable `+` lines for those files before this runs (see {@link maybeAddSecretLeakFinding}).
  */
 export function buildSecretScanDiff(
   files: Awaited<ReturnType<typeof listPullRequestFiles>>,
@@ -5483,6 +5488,180 @@ export function buildSecretScanDiff(
     .trim();
 }
 
+/** Per-file cap when synthesizing a patch for GitHub's patch-less (binary/large) PR files. */
+const SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS = 512_000;
+/** Bound concurrent Contents API reads during patch-less secret-scan enrichment. */
+const SECRET_SCAN_PATCH_FALLBACK_MAX_CONCURRENT = 4;
+
+/** Lines present in `head` but not in `base` (multiset), for scanning only the additions on a modified file. */
+export function addedLinesForSecretScan(base: string, head: string): string[] {
+  const baseCounts = new Map<string, number>();
+  for (const line of base.split("\n")) {
+    baseCounts.set(line, (baseCounts.get(line) ?? 0) + 1);
+  }
+  const added: string[] = [];
+  for (const line of head.split("\n")) {
+    const remaining = baseCounts.get(line) ?? 0;
+    if (remaining > 0) {
+      baseCounts.set(line, remaining - 1);
+    } else {
+      added.push(line);
+    }
+  }
+  return added;
+}
+
+function syntheticSecretScanPatch(lines: readonly string[]): string {
+  return lines.map((line) => `+${line}`).join("\n");
+}
+
+function isOverSecretScanContentLimit(content: string): boolean {
+  return content.length > SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS;
+}
+
+function markPatchLessSecretScanIncomplete<T extends { payload?: Record<string, unknown> }>(file: T): T {
+  return {
+    ...file,
+    payload: { ...file.payload, secretScanIncomplete: true },
+  };
+}
+
+function shouldAttemptPatchLessSecretScan(
+  file: { previousFilename?: string | null | undefined },
+  status: string,
+  baseSha?: string | null | undefined,
+): boolean {
+  if (status === "removed") return false;
+  if (status === "modified") return Boolean(baseSha?.trim());
+  if (status === "renamed") return Boolean(baseSha?.trim() && file.previousFilename?.trim());
+  return status === "added";
+}
+
+function markEligiblePatchLessFilesIncomplete(
+  files: Awaited<ReturnType<typeof listPullRequestFiles>>,
+  baseSha?: string | null | undefined,
+): Awaited<ReturnType<typeof listPullRequestFiles>> {
+  return files.map((file) => {
+    const existingPatch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
+    if (existingPatch) return file;
+    const status = file.status ?? "modified";
+    if (!shouldAttemptPatchLessSecretScan(file, status, baseSha)) return file;
+    return markPatchLessSecretScanIncomplete(file);
+  });
+}
+
+/** @internal Exported for patch-less secret-scan unit tests only. */
+export const secretScanPatchFallbackInternals = {
+  markEligiblePatchLessFilesIncomplete,
+  shouldAttemptPatchLessSecretScan,
+  syntheticSecretScanPatch,
+  isOverSecretScanContentLimit,
+  markPatchLessSecretScanIncomplete,
+};
+
+export function incompletePatchLessSecretScanFinding(
+  files: Awaited<ReturnType<typeof listPullRequestFiles>>,
+): AdvisoryFinding | null {
+  const paths = files
+    .filter((file) => file.payload?.secretScanIncomplete === true)
+    .map((file) => file.path);
+  if (paths.length === 0) return null;
+  return {
+    code: "secret_leak",
+    severity: "critical",
+    title: `Patch-less file(s) could not be fully scanned for secrets (${paths.length})`,
+    detail: `GitHub omitted inline diff for: ${paths.join(", ")}. Fetched content exceeded the ${SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS}-char scan cap or could not be retrieved completely, so leaked-secret verification is incomplete. Shrink the change, split the file, or ensure the diff is reviewable before merge.`,
+    action: "Ensure patch-less files are within scan limits or split the change so secrets can be verified.",
+  };
+}
+
+async function mapPatchLessSecretScanFilesWithConcurrency<T, R>(
+  items: T[],
+  limit: number,
+  mapper: (item: T) => Promise<R>,
+): Promise<R[]> {
+  const results: R[] = new Array(items.length);
+  let nextIndex = 0;
+  const workers = Array.from({ length: Math.min(limit, items.length) }, async () => {
+    while (nextIndex < items.length) {
+      const index = nextIndex;
+      nextIndex += 1;
+      results[index] = await mapper(items[index]!);
+    }
+  });
+  await Promise.all(workers);
+  return results;
+}
+
+/** When GitHub omits inline `patch` (binary/large files), fetch post-change content and synthesize `+` lines so
+ *  the unconditional `secret_leak` hard blocker can still inspect committed credentials. Added files scan only
+ *  genuinely new lines; modified/renamed files multiset-diff against base when `baseSha` is known. Unfetchable
+ *  or baseline-unknown content leaves the file header-only so pre-existing secrets are not mis-flagged; content
+ *  over the per-file cap is marked incomplete so the gate fails closed instead of scanning a truncated prefix.
+ */
+export async function enrichSecretScanFilesWithPatchFallback(
+  files: Awaited<ReturnType<typeof listPullRequestFiles>>,
+  args: {
+    headSha?: string | null | undefined;
+    baseSha?: string | null | undefined;
+    fetcher: FileFetcher;
+  },
+): Promise<Awaited<ReturnType<typeof listPullRequestFiles>>> {
+  const headSha = args.headSha?.trim();
+  if (!headSha) return files;
+  return mapPatchLessSecretScanFilesWithConcurrency(
+    files,
+    SECRET_SCAN_PATCH_FALLBACK_MAX_CONCURRENT,
+    async (file) => {
+      const status = file.status ?? "modified";
+      const existingPatch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
+      if (existingPatch) return file;
+      const needsFetch = shouldAttemptPatchLessSecretScan(file, status, args.baseSha);
+      if (!needsFetch) return file;
+      try {
+        const headContent = await args.fetcher.getFileContent(
+          file.path,
+          headSha,
+          SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS,
+        );
+        if (headContent == null) return markPatchLessSecretScanIncomplete(file);
+        if (isOverSecretScanContentLimit(headContent)) return markPatchLessSecretScanIncomplete(file);
+        let addedLines: string[];
+        if (status === "added") {
+          addedLines = headContent.split("\n");
+        } else if (status === "renamed") {
+          const baseSha = args.baseSha!.trim();
+          const previousPath = file.previousFilename!.trim();
+          const baseContent = await args.fetcher.getFileContent(
+            previousPath,
+            baseSha,
+            SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS,
+          );
+          if (baseContent == null) return markPatchLessSecretScanIncomplete(file);
+          if (isOverSecretScanContentLimit(baseContent)) return markPatchLessSecretScanIncomplete(file);
+          addedLines = addedLinesForSecretScan(baseContent, headContent);
+        } else {
+          const baseContent = await args.fetcher.getFileContent(
+            file.path,
+            args.baseSha!.trim(),
+            SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS,
+          );
+          if (baseContent == null) return markPatchLessSecretScanIncomplete(file);
+          if (isOverSecretScanContentLimit(baseContent)) return markPatchLessSecretScanIncomplete(file);
+          addedLines = addedLinesForSecretScan(baseContent, headContent);
+        }
+        if (addedLines.length === 0) return file;
+        return {
+          ...file,
+          payload: { ...file.payload, patch: syntheticSecretScanPatch(addedLines) },
+        };
+      } catch {
+        return markPatchLessSecretScanIncomplete(file);
+      }
+    },
+  );
+}
+
 /**
  * Run the opt-in AI maintainer review and fold it into the gate + panel. Mutates `advisory.findings`
  * with a dual-model consensus defect (when `aiReviewMode: block` and the free Workers-AI pair agrees with
@@ -6034,6 +6213,9 @@ export async function maybeAddSecretLeakFinding(
     repoFullName: string;
     pullNumber: number;
     files: Awaited<ReturnType<typeof listPullRequestFiles>> | null;
+    installationId?: number | null | undefined;
+    headSha?: string | null | undefined;
+    baseSha?: string | null | undefined;
   },
 ): Promise<void> {
   // UNCONDITIONAL (#audit-3.4): a CONCRETE, real-format committed credential (github_token, aws_access_key, …)
@@ -6044,7 +6226,31 @@ export async function maybeAddSecretLeakFinding(
     const files =
       args.files ??
       (await listPullRequestFiles(env, args.repoFullName, args.pullNumber));
-    const finding = secretLeakFinding(buildSecretScanDiff(files));
+    let scanFiles = files;
+    if (args.headSha) {
+      try {
+        const fetcher = await makeGithubFileFetcher(env, args.repoFullName, args.installationId);
+        scanFiles = await enrichSecretScanFilesWithPatchFallback(files, {
+          headSha: args.headSha,
+          baseSha: args.baseSha,
+          fetcher,
+        });
+      } catch (error) {
+        console.error(
+          JSON.stringify({
+            level: "error",
+            event: "secret_scan_patch_fallback_failed",
+            repository: args.repoFullName,
+            pullNumber: args.pullNumber,
+            error: errorMessage(error),
+          }),
+        );
+        scanFiles = markEligiblePatchLessFilesIncomplete(files, args.baseSha);
+      }
+    }
+    const incompleteFinding = incompletePatchLessSecretScanFinding(scanFiles);
+    if (incompleteFinding) args.advisory.findings.push(incompleteFinding);
+    const finding = secretLeakFinding(buildSecretScanDiff(scanFiles));
     if (finding) args.advisory.findings.push(finding);
   } catch (error) {
     /* v8 ignore next -- fail-safe: a file-load error never destabilizes the gate. */
@@ -7442,6 +7648,9 @@ async function maybePublishPrPublicSurface(
       repoFullName,
       pullNumber: pr.number,
       files: await getReviewFiles(),
+      installationId,
+      headSha: advisory.headSha,
+      baseSha: webhook.baseSha ?? null,
     });
 
     // Lockfile-tamper-risk scan (#2563): opt-in via `lockfileIntegrityGateMode` (default off — the scan is