Skip to content

fix(review): detect cross-line split credentials in gate secret scan (#2454)#2887

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/secret-scan-cross-line-gate
Closed

fix(review): detect cross-line split credentials in gate secret scan (#2454)#2887
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/secret-scan-cross-line-gate

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

The unconditional secret_leak hard blocker scanned added diff lines in isolation. A credential split across two adjacent added string literals (e.g. const a = "AKIA…"; + const b = "REST";) evaded every per-line regex — exactly the #2454 evasion class REES enrichment already closes, but the gate path did not.

This PR adds scanPrDiffForSecretKinds and wires secretLeakFinding through it:

#2877 review blocker fix

Closed #2877 skipped the cross-line join whenever any scanner kind matched the line — including soft heuristics (bittensor_key, seed_or_mnemonic) that secretLeakFinding filters out. A split credential whose second line also contained coldkey = "…" would pass unblocked.

Fix: only skip the join when the line matched a gate-blocking kind on its own. Export GATE_BLOCKING_SECRET_KINDS as the single source of truth shared by the walker and the secretLeakFinding filter.

Test plan

Related

…SONbored#2454)

The unconditional secret_leak hard blocker scanned added lines in isolation,
so credentials split across adjacent added string literals evaded every
per-line regex while REES enrichment already joined them (JSONbored#2454).

Add scanPrDiffForSecretKinds with bounded cross-line literal join, hunk/context
boundaries, consecutive-run generic assignment detection, and in-hunk +++
content handling. Wire secretLeakFinding through the new walker.

Only skip the cross-line join when the current line matched a gate-blocking
kind on its own — a soft heuristic alone (e.g. coldkey:) must not suppress
joining literals that complete a split concrete credential (JSONbored#2877 review
blocker). Export GATE_BLOCKING_SECRET_KINDS as the single source of truth
shared between the diff walker and secretLeakFinding filter.

Supersedes closed JSONbored#2877.

Co-authored-by: Cursor <cursoragent@cursor.com>
@RealDiligent RealDiligent requested a review from JSONbored as a code owner July 4, 2026 04:10
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1e62742). Learn more about missing BASE report.
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/review/secrets-scan.ts 97.10% 1 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (97.14%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2887   +/-   ##
=======================================
  Coverage        ?   96.03%           
=======================================
  Files           ?      259           
  Lines           ?    28458           
  Branches        ?    10342           
=======================================
  Hits            ?    27329           
  Misses          ?      492           
  Partials        ?      637           
Files with missing lines Coverage Δ
src/review/safety.ts 100.00% <100.00%> (ø)
src/review/secrets-scan.ts 97.82% <97.10%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 4, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown

Caution

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

🛑 Gittensory review result - fixes required

Review updated: 2026-07-04 04:14:37 UTC

4 files · 1 AI reviewer · no blockers · readiness 80/100 · CI failing · unstable

🛑 Suggested Action - Fix Blockers

  • Touches a guarded path — held for manual review

Review summary
The change correctly moves the hard-blocking secret gate onto a diff-aware scanner and shares the blocking-kind set between the walker and `secretLeakFinding`. The per-line and adjacent-literal paths are covered well, including the regression where a non-blocking heuristic on the second line must not suppress the cross-line credential join. The main remaining risk is maintainability/perf in the generic-assignment run scanner rather than a visible correctness break.

Nits — 6 non-blocking
  • nit: `src/review/secrets-scan.ts:166` calls `hasGenericSecretAssignment(addedRun.join("\n"))` after every added line, so a large consecutive-added hunk repeatedly rescans the whole run; keep a bounded rolling window or scan only when a candidate keyword/value shape is present.
  • nit: `src/review/secrets-scan.ts:196` only joins the previous line's last literal with the current line's first literal, so the comment should spell out that only adjacent edge literals are supported rather than implying all adjacent string literals on consecutive lines are considered.
  • nit: `test/unit/safety-wiring.test.ts:182` has an internally contradictory assertion that first expects the redaction marker in the path and then expects the same marker not to be present; that pre-existing test shape is confusing and should be cleaned up while this file is being touched.
  • In `src/review/secrets-scan.ts:166`, cap `addedRun` to the minimum lines needed for split assignment detection or track candidate state instead of joining the entire consecutive run on each iteration.
  • In `src/review/secrets-scan.ts:196`, add a test for lines with multiple string literals if the intended contract is broader than previous-last/current-first joining.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

CI checks failing

  • codecov/patch — 97.14% of diff hit (target 99.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2877
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 (1 linked issue).
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: 118 registered-repo PR(s), 11 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 118 PR(s), 0 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
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: 118 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
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

@gittensory-orb gittensory-orb Bot added the gittensor Gittensor contributor context label Jul 4, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown

Gittensory is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs are re-reviewed automatically, so an inaccurate close may be reopened, but that does not guarantee it can merge (e.g. if conflicts or failing CI remain).

@gittensory-orb gittensory-orb Bot closed this Jul 4, 2026
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.5x multiplier. gittensor Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant