Skip to content

Commit 16dc507

Browse files
authored
fix(review): raise the patch-less secret-scan cap from 512KB to 4MB (#8605)
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.
1 parent f4f001d commit 16dc507

2 files changed

Lines changed: 21 additions & 11 deletions

File tree

src/queue/patchless-secret-scan.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,17 @@ import type { FileFetcher } from "../review/review-grounding";
22
import { mapWithConcurrency } from "./map-with-concurrency";
33
import type { AdvisoryFinding, PullRequestFileRecord } from "../types";
44

5-
/** Per-file cap when synthesizing a patch for GitHub's patch-less (binary/large) PR files. */
6-
export const SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS = 512_000;
5+
/** Per-file cap when synthesizing a patch for GitHub's patch-less (binary/large) PR files. The fetcher behind
6+
* this (grounding-wire.ts's makeGithubFileFetcher) requests the `application/vnd.github.raw+json` media type
7+
* specifically to bypass the Contents API's ~1MB base64-JSON envelope ceiling, so this cap is the real limit,
8+
* not GitHub's. Raised from 512_000 (2026-07-25): a repo whose regenerated OpenAPI/JSON-schema artifacts
9+
* routinely exceed 512KB (observed live: metagraphed's openapi.json at ~1.9MB, api-components.schema.json at
10+
* ~514KB) was hitting the fail-closed `secretScanIncomplete` block on every such PR even though the file is
11+
* deterministically generated from already-scanned source in the same diff. Raising this cap only EXPANDS scan
12+
* coverage (more content becomes fetchable-and-scannable instead of being marked incomplete-and-blocked) --
13+
* it never reduces detection on anything previously caught. 4MB gives real headroom above the largest
14+
* observed case without being unbounded; fetch count/concurrency stay capped separately below. */
15+
export const SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS = 4_000_000;
716
/** Fetch probe limit passed to {@link FileFetcher.getFileContent}: the grounding fetcher returns `maxChars+1`
817
* bytes when the file exceeds `maxChars - 1`, so `content.length > SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS` reliably
918
* detects truncation instead of scanning a clipped prefix. Mirrors review-grounding's `+ 1` probe. */

test/unit/patchless-secret-scan.test.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
addedLinesForSecretScan,
55
enrichSecretScanFilesWithPatchFallback,
66
incompletePatchLessSecretScanFinding,
7+
SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS,
78
SECRET_SCAN_PATCH_FALLBACK_MAX_FETCHES,
89
markEligiblePatchLessFilesIncomplete,
910
patchlessSecretScanInternals,
@@ -389,7 +390,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => {
389390
});
390391

391392
it("marks a renamed file incomplete when base content exceeds the scan cap", async () => {
392-
const oversized = "x".repeat(512_001);
393+
const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1);
393394
const fetcher: FileFetcher = {
394395
async getFileContent(path, ref) {
395396
if (path === "old-secrets.env" && ref === "base-sha") return oversized;
@@ -420,7 +421,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => {
420421
});
421422

422423
it("marks a renamed file incomplete when head content exceeds the scan cap", async () => {
423-
const oversized = "x".repeat(512_001);
424+
const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1);
424425
const fetcher: FileFetcher = {
425426
async getFileContent(path, ref) {
426427
if (path === "secrets.env" && ref === "head-sha") return oversized;
@@ -536,7 +537,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => {
536537
});
537538

538539
it("marks a modified file incomplete when base content exceeds the scan cap", async () => {
539-
const oversized = "x".repeat(512_001);
540+
const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1);
540541
const fetcher: FileFetcher = {
541542
async getFileContent(path, ref) {
542543
if (path !== "src/config.ts") return null;
@@ -821,8 +822,8 @@ describe("enrichSecretScanFilesWithPatchFallback", () => {
821822
expect(secretLeakFinding(buildSecretScanDiff(enriched))).toBeNull();
822823
});
823824

824-
it("scans patch-less content at the exact 512KB cap without marking incomplete", async () => {
825-
const atCap = "x".repeat(512_000);
825+
it("scans patch-less content at the exact cap without marking incomplete", async () => {
826+
const atCap = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS);
826827
const fetcher: FileFetcher = {
827828
async getFileContent(path, ref) {
828829
if (path === "large.env" && ref === "head-sha") return atCap;
@@ -876,7 +877,7 @@ describe("enrichSecretScanFilesWithPatchFallback", () => {
876877
});
877878

878879
it("marks a patch-less file incomplete when fetched content exceeds the scan cap", async () => {
879-
const oversized = "x".repeat(512_001);
880+
const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1);
880881
const fetcher: FileFetcher = {
881882
async getFileContent(path, ref) {
882883
if (path === "secrets.env" && ref === "head-sha") return oversized;
@@ -1060,8 +1061,8 @@ describe("patchlessSecretScanInternals", () => {
10601061

10611062
it("covers helper boundaries for synthetic patches and content limits", () => {
10621063
expect(syntheticSecretScanPatch(["a", "b"])).toBe("+a\n+b");
1063-
expect(isOverSecretScanContentLimit("x".repeat(512_000))).toBe(false);
1064-
expect(isOverSecretScanContentLimit("x".repeat(512_001))).toBe(true);
1064+
expect(isOverSecretScanContentLimit("x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS))).toBe(false);
1065+
expect(isOverSecretScanContentLimit("x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1))).toBe(true);
10651066
const incomplete = markPatchLessSecretScanIncomplete({
10661067
path: "secrets.env",
10671068
} as Parameters<typeof markPatchLessSecretScanIncomplete>[0]);
@@ -1457,7 +1458,7 @@ describe("maybeAddSecretLeakFinding patch-less fallback wiring", () => {
14571458
it("blocks when patch-less enrichment cannot fully scan an oversized file", async () => {
14581459
const env = createTestEnv();
14591460
const adv = advisory();
1460-
const oversized = "x".repeat(512_001);
1461+
const oversized = "x".repeat(SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1);
14611462
const files = [
14621463
{
14631464
repoFullName: "acme/widgets",

0 commit comments

Comments
 (0)