fix(queue): shouldAttemptPatchLessSecretScan silently skips copied/changed/unchanged file statuses, bypassing the fail-closed secret scan guarantee

[JSONbored]

Context

shouldAttemptPatchLessSecretScan in src/queue/patchless-secret-scan.ts (lines 50-58) decides which patch-less (GitHub omitted the inline patch field — binary/large files) PR files get the secret-scan content-fetch fallback, or, failing that, get marked secretScanIncomplete so the fail-closed advisory fires:

export 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";
}

GitHub's Pull Request Files API status field can legitimately be added | removed | modified | renamed | copied | changed | unchanged (GitHub's own diff_entry OpenAPI schema), and this codebase passes file.status straight through from the API response unfiltered (PullRequestFileRecord.status: string in src/types.ts). For any status this function doesn't explicitly handle — concretely copied, changed, and unchanged — it falls through to the final line, return status === "added", which evaluates to false. That's silently identical to the explicit removed case, i.e. "never scan," even though a copied or changed file can carry genuinely new committed content.

Traced downstream through src/queue/processors.ts (maybeAddSecretLeakFinding) and src/review/review-diff.ts (buildSecretScanDiff): a patch-less file with one of these three statuses is neither enriched with fetched content (shouldAttemptPatchLessSecretScan returning false means it's never a scan candidate) nor marked secretScanIncomplete (the fail-closed marking only applies to files that were attempted and exceeded the size cap) — so it silently contributes zero content to the secret_leak hard-blocker scan, with no fail-closed advisory either. This contradicts the module's own doc comments, which describe an unconditional, fail-closed guarantee ("the unconditional secret_leak hard blocker can still inspect committed credentials"; "content over the per-file cap is marked incomplete so the gate fails closed instead of scanning a truncated prefix"). A committed secret in a patch-less copied/changed/unchanged-status file bypasses both layers of protection entirely, silently.

test/unit/patchless-secret-scan.test.ts (~line 972) explicitly asserts shouldAttemptPatchLessSecretScan({}, "copied", "base-sha") returns false — the current behavior is pinned by an existing test, but nothing in the test suite or source comments acknowledges that this defeats the module's own documented fail-closed guarantee for those three status values.

Requirements

• shouldAttemptPatchLessSecretScan must treat copied and changed statuses the same as modified (attempt the scan when baseSha is present) — both can introduce new committed content relative to base.
• unchanged should either (a) also attempt the scan for defense-in-depth (it's cheap relative to the fail-closed alternative and GitHub's own docs describe unchanged as usable in certain merge-commit contexts where content can still differ from what the local diff view assumes), or (b) explicitly be marked as a secretScanIncomplete fail-closed case rather than silently treated as "never scan" — pick whichever the surrounding module's fail-closed philosophy favors (re-read the doc comments at the top of patchless-secret-scan.ts before deciding; the existing precedent is "when genuinely uncertain, mark incomplete and fail closed" rather than "assume safe").
• Update the existing pinned test at test/unit/patchless-secret-scan.test.ts:972 (status === "copied" case) to reflect the corrected behavior — this is a deliberate behavior change, not a preserved invariant.
• Do not change the removed (line 51) or renamed (line 53) branches — those are correctly handled today.
• Do not change SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS, SECRET_SCAN_PATCH_FALLBACK_MAX_FETCHES, or any other constant in this file.

Deliverables

• shouldAttemptPatchLessSecretScan no longer silently treats copied/changed (and, per the decision above, unchanged) statuses identically to removed — each either attempts the fallback scan or is explicitly marked secretScanIncomplete.
• The pinned test at test/unit/patchless-secret-scan.test.ts:972 is updated to assert the corrected behavior for status: "copied".
• New regression tests for status: "changed" and status: "unchanged" asserting the corrected (non-silently-skipped) behavior.
• A test tracing the fix through to maybeAddSecretLeakFinding/buildSecretScanDiff (or the closest existing integration test covering that path) confirming a copied-status file with a genuine secret in its fetched content is now either scanned or explicitly flagged incomplete, not silently dropped.

Test Coverage Requirements

Aim for 99%+ Codecov patch coverage (100% including the new branch/invariant) on the touched lines in src/queue/patchless-secret-scan.ts. This is a fix for a real fail-closed-guarantee bypass on a security-critical scanner, so all regression tests above are required, not just incidental line coverage.

Expected Outcome

A patch-less PR file with GitHub API status copied, changed, or unchanged is no longer silently treated as "never scan, never flag incomplete" — it now either goes through the content-fetch fallback scan or is explicitly marked secretScanIncomplete so the fail-closed advisory fires, matching this module's own documented guarantee that the secret_leak hard blocker can inspect (or explicitly refuse to silently skip) every committed credential.

Links & Resources

• src/queue/patchless-secret-scan.ts (shouldAttemptPatchLessSecretScan, lines 50-58; module-level fail-closed doc comments)
• src/queue/processors.ts (maybeAddSecretLeakFinding — downstream consumer)
• src/review/review-diff.ts (buildSecretScanDiff — where the resulting content/incompleteness ultimately affects the secret_leak hard blocker)
• src/types.ts (PullRequestFileRecord.status: string — confirms status is passed through unfiltered from GitHub's API)
• test/unit/patchless-secret-scan.test.ts (existing pinned test at ~line 972, to be updated as part of this fix)

[linear-code]

JSO-4089