Skip to content

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

Closed
RealDiligent wants to merge 3 commits into
JSONbored:mainfrom
RealDiligent:fix/secret-scan-patchless-files-v3
Closed

fix(review): scan patch-less PR files for leaked secrets#3473
RealDiligent wants to merge 3 commits into
JSONbored:mainfrom
RealDiligent:fix/secret-scan-patchless-files-v3

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Problem

The unconditional secret_leak hard blocker scans only + lines from buildSecretScanDiff. GitHub omits inline patch for binary/large changed files (>~1 MB), so those files produced header-only entries with no scannable body.

Fix

  • Extract patch-less enrichment into src/queue/patchless-secret-scan.ts with dedicated unit tests.
  • Fetch head/base content via Contents API when inline patch is absent.
  • Fail closed when content exceeds the 512KB cap or cannot be fetched completely.

Pre-PR validation (local)

  • npm run typecheck
  • test/unit/patchless-secret-scan.test.ts — 41 tests pass
  • patchless-secret-scan.ts — 100% statements/branches/lines locally

CI

  • All checks green
  • codecov/patch >= 99%

RealDiligent and others added 3 commits July 5, 2026 16:10
)

Extract patch-less secret-scan enrichment into patchless-secret-scan.ts with
full unit coverage. When GitHub omits inline diff for binary/large files,
fetch head/base content via Contents API and synthesize scannable + lines
before the unconditional secret_leak hard blocker runs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 5, 2026 08:42
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 08:45:49 UTC

3 files · 1 AI reviewer · 3 blockers · readiness 73/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
  • AI reviewers agree on a likely critical defect: src/queue/patchless-secret-scan.ts:121 passes exactly SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS to getFileContent, but the existing fetchFullFileContents pattern requests max+1 before checking length
  • if the fetcher truncates to the requested cap, a >512KB patch-less file is scanned as a complete 512KB prefix instead of producing the fail-closed incomplete finding, so change the head/base fetches to request SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1 and keep the existing > cap check. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The change wires a patch-less-file fallback into the deterministic secret scan and adds broad unit coverage for added/modified/renamed files, which is the right hot path to fix. The current implementation does not faithfully reproduce GitHub added-line semantics for modified files and also loses the over-cap sentinel pattern used by the existing content-fetch path, so a reachable patch-less PR can be scanned incompletely while the gate treats it as complete.

Blockers

  • src/queue/patchless-secret-scan.ts:121 passes exactly SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS to getFileContent, but the existing fetchFullFileContents pattern requests max+1 before checking length; if the fetcher truncates to the requested cap, a >512KB patch-less file is scanned as a complete 512KB prefix instead of producing the fail-closed incomplete finding, so change the head/base fetches to request SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1 and keep the existing > cap check.
  • src/queue/patchless-secret-scan.ts:9 implements modified-file additions as a multiset difference, which misses a real added duplicate secret line when the same line already exists in base; use an actual line diff/LCS-style added-line extraction or another existing diff helper so patch-less fallback scans the same + lines GitHub would have emitted.
Nits — 6 non-blocking
  • test/unit/patchless-secret-scan.test.ts only proves oversized behavior with a fake fetcher that returns 512001 chars even though the production fetcher may cap at the requested max; add a regression test where getFileContent returns exactly the requested limit for oversized content after the code requests max+1.
  • src/queue/patchless-secret-scan.ts:27 synthesizes patches without hunk headers, which works for the current secret scanner but is a narrow implicit contract; a short comment tying this to buildSecretScanDiff/secretLeakFinding would make future scanner changes less fragile.
  • In src/queue/patchless-secret-scan.ts, mirror src/review/review-grounding.ts by fetching SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1 and marking incomplete when the returned content exceeds the cap.
  • Replace addedLinesForSecretScan in src/queue/patchless-secret-scan.ts with a sequence-aware diff helper and add a test for base containing one credential line while head contains that same line twice.
  • Add a wiring test in test/unit/patchless-secret-scan.test.ts that asserts makeGithubFileFetcher/getFileContent is called with 512001, not 512000.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • src/queue/patchless-secret-scan.ts:121 passes exactly SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS to getFileContent, but the existing fetchFullFileContents pattern requests max+1 before checking length; if the fetcher truncates to the requested cap, a >512KB patch-less file is scanned as a complete 512KB prefix instead of producing the fail-closed incomplete finding, so change the head/base fetches to request SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1 and keep the existing > cap check.
  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 3 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 138 registered-repo PR(s), 18 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 138 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 138 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Explain no-issue PR.
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (No linked issue detected; Maintainer requires a linked issue; AI reviewers agree on a likely critical defect: src/queue/patchless-secret-scan.ts:121 passes exactly SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS to getFileContent, but the existing fetchFullFileContents pattern requests max+1 before checking length; if the fetcher truncates to the requested cap, a >512KB patch-less file is scanned as a complete 512KB prefix instead of producing the fail-closed incomplete finding, so change the head/base fetches to request SECRET_SCAN_PATCH_FALLBACK_MAX_CHARS + 1 and keep the existing > cap check.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.03%. Comparing base (4de33af) to head (d674d79).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3473      +/-   ##
==========================================
+ Coverage   93.01%   93.03%   +0.01%     
==========================================
  Files         297      298       +1     
  Lines       31013    31091      +78     
  Branches    11310    11338      +28     
==========================================
+ Hits        28846    28924      +78     
  Misses       1512     1512              
  Partials      655      655              
Files with missing lines Coverage Δ
src/queue/patchless-secret-scan.ts 100.00% <100.00%> (ø)
src/queue/processors.ts 93.12% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant