Skip to content

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

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)#2877
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:

Mirrors the REES secret-scan.ts join heuristic while keeping secrets-scan.ts self-contained (no cross-package import).

Test plan

  • npm run typecheck
  • npx vitest run test/unit/secrets-scan.test.ts test/unit/safety-wiring.test.ts (68/68)
  • CI (Linux) green including codecov/patch ≥ 99%

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.

Co-authored-by: Cursor <cursoragent@cursor.com>
@RealDiligent RealDiligent requested a review from JSONbored as a code owner July 4, 2026 03:22
@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 4, 2026
@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 (f031ff3) to head (e7e6311).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2877   +/-   ##
=======================================
  Coverage   96.02%   96.03%           
=======================================
  Files         259      259           
  Lines       28360    28421   +61     
  Branches    10312    10325   +13     
=======================================
+ Hits        27233    27294   +61     
  Misses        491      491           
  Partials      636      636           
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 commented Jul 4, 2026

Copy link
Copy Markdown

Caution

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

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-04 03:46:31 UTC

4 files · 1 AI reviewer · 1 blocker · readiness 73/100 · CI green · unstable

🛑 Suggested Action - Reject/Close

  • AI reviewers agree on a likely critical defect: src/review/secrets-scan.ts:164-173 sets `matched` for any scanner kind, so a split credential whose second added line also matches only a non-hard heuristic skips the adjacent-literal join and then gets filtered out in src/review/safety.ts:103, allowing that real introduced secret to pass unblocked. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This PR replaces the old added-line blob scan with a diff-aware scanner that preserves single-line detection while adding bounded adjacent string-literal joins across consecutive added lines. The hunk, context, removed-line, and file-section reset behavior is well covered, and the safety wiring now uses the new scanner. One reachable skip condition still leaves a split credential evasion in the new path because weak scanner matches suppress the join before safety filtering removes them.

Blockers

  • src/review/secrets-scan.ts:164-173 sets `matched` for any scanner kind, so a split credential whose second added line also matches only a non-hard heuristic skips the adjacent-literal join and then gets filtered out in src/review/safety.ts:103, allowing that real introduced secret to pass unblocked.
Nits — 6 non-blocking
  • nit: src/review/secrets-scan.ts:102 duplicates the whole-text secret-kind collection from `scanForSecrets`, so future pattern or generic-assignment changes now have two call sites to keep in lockstep.
  • nit: test/unit/safety-wiring.test.ts:447 calls `secretLeakFinding(diff)` twice in the same assertion path; storing the finding once would make the wiring test easier to read and avoid re-running the scanner.
  • In src/review/secrets-scan.ts:164-173, either run the adjacent-literal join regardless and rely on the `Set` for dedupe, or track a separate match flag that only suppresses the join for already-blocking concrete matches.
  • Add a regression in test/unit/secrets-scan.test.ts where the second added line both contains the completing literal fragment and independently matches a filtered weak kind, then assert `scanPrDiffForSecretKinds` still returns the concrete credential kind.
  • Consider replacing `formatSecretKindsFromText` with a shared internal helper used by both `scanForSecrets` and `scanPrDiffForSecretKinds` so the two scanner entry points cannot drift.
  • 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/review/secrets-scan.ts:164-173 sets `matched` for any scanner kind, so a split credential whose second added line also matches only a non-hard heuristic skips the adjacent-literal join and then gets filtered out in src/review/safety.ts:103, allowing that real introduced secret to pass unblocked.
Signal Result Evidence
Code review ❌ 1 blocker 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: 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
  • 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

@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:164-173 sets `matched` for any scanner kind, so a split credential whose second added line also matches only a non-hard heuristic skips the adjacent-literal join and then gets filtered out in src/review/safety.ts:103, allowing that real introduced secret to pass unblocked.). 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
RealDiligent added a commit to RealDiligent/gittensory that referenced this pull request Jul 4, 2026
…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 added a commit to RealDiligent/gittensory that referenced this pull request Jul 4, 2026
…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 added a commit to RealDiligent/gittensory that referenced this pull request Jul 4, 2026
…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>
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