Skip to content

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

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)#2898
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

@RealDiligent RealDiligent requested a review from JSONbored as a code owner July 4, 2026 04:23
…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 force-pushed the fix/secret-scan-cross-line-gate branch from cc600c5 to 75625f8 Compare July 4, 2026 04:48
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.03%. Comparing base (f7b5abc) to head (75625f8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2898   +/-   ##
=======================================
  Coverage   96.02%   96.03%           
=======================================
  Files         259      259           
  Lines       28406    28469   +63     
  Branches    10339    10354   +15     
=======================================
+ Hits        27278    27341   +63     
  Misses        491      491           
  Partials      637      637           
Files with missing lines Coverage Δ
src/review/safety.ts 100.00% <100.00%> (ø)
src/review/secrets-scan.ts 100.00% <100.00%> (ø)
🚀 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 - reject/close recommended

Review updated: 2026-07-04 05:03:27 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 80/100 · CI green · unknown

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: src/review/secrets-scan.ts:197 rescans `addedRun.join("\n")` after every added line, so a large generated all-added hunk triggers quadratic string building and regex scanning on the unconditional secret gate path
  • track only the previous/current generic-assignment window or scan the run once at boundaries instead. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.
  • Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Review summary
The change correctly moves the gate from a flat added-line blob scan to a diff-aware scanner and covers the intended adjacent-literal credential split, including the #2877 soft-heuristic case. The notable risk in the visible implementation is that the generic-assignment run check repeatedly rejoins and rescans the entire consecutive added-line run on every added line, which turns large all-added hunks into quadratic work on the unconditional gate path. The shared gate-blocking kind set is a good correctness improvement because the join suppression and final `secretLeakFinding` filter now use the same source of truth.

Blockers

  • src/review/secrets-scan.ts:197 rescans `addedRun.join("\n")` after every added line, so a large generated all-added hunk triggers quadratic string building and regex scanning on the unconditional secret gate path; track only the previous/current generic-assignment window or scan the run once at boundaries instead.
Nits — 5 non-blocking
  • nit: src/review/secrets-scan.ts:182 treats a literal blank line in the synthetic diff as a file-section boundary, so confirm `buildSecretScanDiff` always inserts blank lines only between file sections and never emits raw blank patch lines outside `+`/`-`/space diff prefixes.
  • nit: test/unit/secrets-scan.test.ts:309 asserts the optimization path by exact returned-kind order, which couples the test to `Set` insertion behavior rather than the externally relevant absence of duplicate findings.
  • src/review/secrets-scan.ts:197 should defer `hasGenericSecretAssignment(addedRun.join("\n"))` until a boundary reset, or maintain a bounded rolling two-line buffer for split generic assignments so runtime stays linear in diff size.
  • test/unit/secrets-scan.test.ts should add a large consecutive-added-lines regression that verifies the scanner handles generated hunks without repeatedly rescanning the full prefix.
  • test/unit/secrets-scan.test.ts:309 can assert `toEqual(["generic_secret_assignment"])` only after documenting that stable output ordering is part of the scanner contract; otherwise prefer `toContain` plus a duplicate-count assertion.

Why this is blocked

  • src/review/secrets-scan.ts:197 rescans `addedRun.join("\n")` after every added line, so a large generated all-added hunk triggers quadratic string building and regex scanning on the unconditional secret gate path; track only the previous/current generic-assignment window or scan the run once at boundaries instead.
  • Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.
Signal Result Evidence
Code review ❌ 2 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 ❌ 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: 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 (AI reviewers agree on a likely critical defect: src/review/secrets-scan.ts:197 rescans `addedRun.join("\n")` after every added line, so a large generated all-added hunk triggers quadratic string building and regex scanning on the unconditional secret gate path; track only the previous/current generic-assignment window or scan the run once at boundaries instead.; Possible leaked secret in the diff (generic_secret_assignment)). 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