From d401de5ea76125fd40682ad9c27a61db8291b78a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:50:03 -0700 Subject: [PATCH] fix(review): raise the patch-less secret-scan cap from 512KB to 4MB Live diagnosis on the self-hosted ORB box: metagraphed's secret_leak gate false-positive rate climbed from 38% to 56% over 6 hours, holding mergeable PRs. Root cause wasn't a pattern-match false positive -- it was the fail-closed "content exceeded the scan cap" block on regenerated OpenAPI/JSON-schema artifacts. metagraphed's openapi.json is ~1.9MB and api-components.schema.json ~514KB, both well past the old 512,000-char cap; three recent PRs (#8106, #8095, #8005) were all held for this reason and all merged anyway once manually verified clean. The underlying fetcher (grounding-wire.ts's makeGithubFileFetcher) already requests the raw+json media type specifically to bypass GitHub's Contents API ~1MB base64-JSON envelope ceiling, so the real limit was always this local constant, not GitHub's. Raising it only expands scan coverage -- more content becomes fetchable-and-scannable instead of being marked incomplete-and-blocked -- so this can't weaken detection on anything the old cap already caught. 4MB gives headroom above the largest observed real case (loopover's own openapi.json is already at 525KB, past the old cap too). Fetch count/concurrency stay capped separately, unaffected. --- src/queue/patchless-secret-scan.ts | 13 +++++++++++-- test/unit/patchless-secret-scan.test.ts | 19 ++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/queue/patchless-secret-scan.ts b/src/queue/patchless-secret-scan.ts index e64dbb6495..64b1dcd903 100644 --- a/src/queue/patchless-secret-scan.ts +++ b/src/queue/patchless-secret-scan.ts @@ -2,8 +2,17 @@ import type { FileFetcher } from "../review/review-grounding"; import { mapWithConcurrency } from "./map-with-concurrency"; import type { AdvisoryFinding, PullRequestFileRecord } from "../types"; -/** Per-file cap when synthesizing a patch for GitHub's patch-less (binary/large) PR files. */ -export const SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS = 512_000; +/** Per-file cap when synthesizing a patch for GitHub's patch-less (binary/large) PR files. The fetcher behind + * this (grounding-wire.ts's makeGithubFileFetcher) requests the `application/vnd.github.raw+json` media type + * specifically to bypass the Contents API's ~1MB base64-JSON envelope ceiling, so this cap is the real limit, + * not GitHub's. Raised from 512_000 (2026-07-25): a repo whose regenerated OpenAPI/JSON-schema artifacts + * routinely exceed 512KB (observed live: metagraphed's openapi.json at ~1.9MB, api-components.schema.json at + * ~514KB) was hitting the fail-closed `secretScanIncomplete` block on every such PR even though the file is + * deterministically generated from already-scanned source in the same diff. Raising this cap only EXPANDS scan + * coverage (more content becomes fetchable-and-scannable instead of being marked incomplete-and-blocked) -- + * it never reduces detection on anything previously caught. 4MB gives real headroom above the largest + * observed case without being unbounded; fetch count/concurrency stay capped separately below. */ +export const SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS = 4_000_000; /** Fetch probe limit passed to {@link FileFetcher.getFileContent}: the grounding fetcher returns `maxChars+1` * bytes when the file exceeds `maxChars - 1`, so `content.length > SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS` reliably * detects truncation instead of scanning a clipped prefix. Mirrors review-grounding's `+ 1` probe. */ diff --git a/test/unit/patchless-secret-scan.test.ts b/test/unit/patchless-secret-scan.test.ts index 81031d05df..b47397a3d1 100644 --- a/test/unit/patchless-secret-scan.test.ts +++ b/test/unit/patchless-secret-scan.test.ts @@ -4,6 +4,7 @@ import { addedLinesForSecretScan, enrichSecretScanFilesWithPatchFallback, incompletePatchLessSecretScanFinding, + SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS, SECRET_SCAN_PATCH_FALLBACK_MAX_FETCHES, markEligiblePatchLessFilesIncomplete, patchlessSecretScanInternals, @@ -389,7 +390,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => { }); it("marks a renamed file incomplete when base content exceeds the scan cap", async () => { - const oversized = "x".repeat(512_001); + const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1); const fetcher: FileFetcher = { async getFileContent(path, ref) { if (path === "old-secrets.env" && ref === "base-sha") return oversized; @@ -420,7 +421,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => { }); it("marks a renamed file incomplete when head content exceeds the scan cap", async () => { - const oversized = "x".repeat(512_001); + const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1); const fetcher: FileFetcher = { async getFileContent(path, ref) { if (path === "secrets.env" && ref === "head-sha") return oversized; @@ -536,7 +537,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => { }); it("marks a modified file incomplete when base content exceeds the scan cap", async () => { - const oversized = "x".repeat(512_001); + const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1); const fetcher: FileFetcher = { async getFileContent(path, ref) { if (path !== "src/config.ts") return null; @@ -821,8 +822,8 @@ describe("enrichSecretScanFilesWithPatchFallback", () => { expect(secretLeakFinding(buildSecretScanDiff(enriched))).toBeNull(); }); - it("scans patch-less content at the exact 512KB cap without marking incomplete", async () => { - const atCap = "x".repeat(512_000); + it("scans patch-less content at the exact cap without marking incomplete", async () => { + const atCap = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS); const fetcher: FileFetcher = { async getFileContent(path, ref) { if (path === "large.env" && ref === "head-sha") return atCap; @@ -876,7 +877,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => { }); it("marks a patch-less file incomplete when fetched content exceeds the scan cap", async () => { - const oversized = "x".repeat(512_001); + const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1); const fetcher: FileFetcher = { async getFileContent(path, ref) { if (path === "secrets.env" && ref === "head-sha") return oversized; @@ -1060,8 +1061,8 @@ describe("patchlessSecretScanInternals", () => { it("covers helper boundaries for synthetic patches and content limits", () => { expect(syntheticSecretScanPatch(["a", "b"])).toBe("+a\n+b"); - expect(isOverSecretScanContentLimit("x".repeat(512_000))).toBe(false); - expect(isOverSecretScanContentLimit("x".repeat(512_001))).toBe(true); + expect(isOverSecretScanContentLimit("x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS))).toBe(false); + expect(isOverSecretScanContentLimit("x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1))).toBe(true); const incomplete = markPatchLessSecretScanIncomplete({ path: "secrets.env", } as Parameters[0]); @@ -1457,7 +1458,7 @@ describe("maybeAddSecretLeakFinding patch-less fallback wiring", () => { it("blocks when patch-less enrichment cannot fully scan an oversized file", async () => { const env = createTestEnv(); const adv = advisory(); - const oversized = "x".repeat(512_001); + const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1); const files = [ { repoFullName: "acme/widgets",