fix(queue): scan patch-less copied/changed/unchanged files for secret scan - #5951
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-15 04:58:50 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5951 +/- ##
=======================================
Coverage 95.16% 95.16%
=======================================
Files 589 589
Lines 46768 46779 +11
Branches 14933 14936 +3
=======================================
+ Hits 44506 44517 +11
Misses 1511 1511
Partials 751 751
Flags with carried forward coverage won't be shown. Click here to find out more.
|
f2b69fe to
8ec5025
Compare
Closes #5947
PR body
Summary
shouldAttemptPatchLessSecretScansilently treated GitHub PR file statusescopied,changed, andunchangedas "never scan" — identical toremoved—even though those statuses can carry committed content. A patch-less file with
one of those statuses was neither enriched via the content-fetch fallback nor
marked
secretScanIncomplete, so it contributed zero content to thesecret_leakhard blocker with no fail-closed advisory either. That contradictsthis module's documented guarantee.
Root cause
GitHub's
diff_entrystatus isadded | removed | modified | renamed | copied | changed | unchanged. The function handled the first four and fell through toreturn status === "added"for everything else, which is alwaysfalseforthe remaining three. Downstream (
maybeAddSecretLeakFinding→enrichSecretScanFilesWithPatchFallback→buildSecretScanDiff) only marksincomplete for files that were attempted and exceeded the size cap — so a
never-attempted file was silently dropped from both the scan and the advisory.
Change
src/queue/patchless-secret-scan.tscopied,changed, andunchangedlikemodified: attempt thebase/head multiset scan when
baseShais present.unchanged, chose defense-in-depth attempt-when-baseSha (option a)rather than unconditional incomplete: matching
modifiedkeeps the scancheap when content truly matches (
addedLines.length === 0), while stillcovering merge-commit contexts where content can differ. Fail-closed still
applies if the fetch/size-cap path marks incomplete.
removedandrenamedbranches unchanged. Constants unchanged. Existingenrich path already treats non-
added/non-renamedstatuses asmodified-style once
shouldAttemptreturns true — no enrich rewrite needed.Tests (
test/unit/patchless-secret-scan.test.ts)status === "copied"assertion (nowtruewith baseSha;also covers null/blank baseSha →
false).changedandunchanged(with/without baseSha) plus anunknown-status →
falsebranch.it.eachenrichment regressions:copied/changed/unchangedpatch-lessfiles with a committed secret synthesize a scan patch and fire
secret_leak;without baseSha they stay unscanned (no silent incomplete either — same as
modified).maybeAddSecretLeakFindingend-to-end: a patch-lesscopied-status filewith a real
ghp_token is flaggedsecret_leak, not silently dropped andnot marked "could not be fully scanned".
copied/changed/unchangedcost 2 fetches wheneligible (0 when not).
Coverage on
src/queue/patchless-secret-scan.ts: 100% statements / branches /functions / lines.
Links