fix(review): reduce secret-scan false positives and surface finding locations

[JSONbored]

Summary

• Fixes a false-positive hard block from the generic_secret_assignment rule: a captured value made
  entirely of lowercase words joined by hyphens (2+ segments, e.g. installation-token — the test-fixture
  mock token used 351+ times across this repo's own test suite) is now treated as a non-secret. Real
  secrets/tokens are essentially always alphanumeric, mixed-case, or base64/hex, never a pure
  lowercase-hyphenated phrase, so this exclusion stays narrow (requires ≥1 hyphen) and doesn't broaden into
  excluding arbitrary single lowercase words. This is the confirmed root cause of PR fix(review): key the durable CI-state cache on resolved required contexts #3036's wrongful
  hard-block (test/unit/queue.test.ts's Response.json({ token: "installation-token" }) fixture).
• Fixes the second part of fix(review): reduce false positives in the gate's hard-blocking secret scan #3041: a hard-blocking secret_leak finding previously carried no file:line
  location at all, forcing a maintainer to re-derive it from the whole diff. Added
  scanDiffForSecretsWithLocations in src/review/secrets-scan.ts, which walks a buildSecretScanDiff-shaped
  diff line by line (tracking file headers, hunk headers, and +/-/context line types) and returns each
  pattern hit with its file path and 1-based line number in the new/post-change file (or line: 0 for a
  secret-shaped filename on an added/renamed file header, preserving the previous header-scanning behavior).
  secretLeakFinding in src/review/safety.ts now calls this directly on the raw diff (no more
  pre-filtering to an added-only text blob) and appends up to 5 path:line locations to the finding's
  detail, noting how many more were omitted beyond that cap.
• The pattern-matching logic (SECRET_PATTERNS + hasGenericSecretAssignment) was factored into one shared
  matchedKindsIn helper that both scanForSecrets (unchanged signature/behavior) and the new
  scanDiffForSecretsWithLocations delegate to, so there is exactly one place the pattern list is applied.
• Out of scope (per the issue, explicitly "Consider"-worded): an owner/maintainer override/acknowledgment
  path for a confirmed false positive. That's a separate, larger design decision; left as a follow-up rather
  than built here.

Fixes #3041

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally — both changed files are at 100%: src/review/secrets-scan.ts 55/55
  lines, 34/34 branches, 8/8 functions; src/review/safety.ts 19/19 lines, 12/12 branches, 7/7
  functions. Full suite: 8630 passed, 7 skipped.
• npm run test:workers
• npm run build:mcp
• npm run test:mcp-pack
• npm run ui:openapi:check
• npm run ui:lint
• npm run ui:typecheck
• npm run ui:build
• npm audit --audit-level=moderate — 0 vulnerabilities
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Also ran (not template-listed, but part of this repo's full local gate): npm run db:migrations:check,
npm run db:schema-drift:check, npm run cf-typegen:check, npm run selfhost:validate-observability,
npm run ui:test, npm run ui:version-audit, npm run rees:test (677 passed) — all green.

If any required check was skipped, explain why:

• npm run selfhost:env-reference:check (part of the local test:ci aggregator, but not part of the
  validate-code GitHub Actions job) fails on a clean origin/main checkout with no changes from this PR —
  apps/gittensory-ui/src/lib/selfhost-env-reference.ts drifted from a prior, unrelated merge. Verified via
  git stash that this failure is present before any of this PR's changes are applied. This PR touches
  neither the self-host env-var surface nor that generated file, so regenerating it here would be out of
  scope; flagging as a pre-existing, separate issue rather than fixing it in this PR.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
• API/OpenAPI/MCP behavior is updated and tested where needed.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
• Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

This is a deterministic, security-relevant scanner change, so extra care was taken to verify it doesn't
regress detection of real secrets:

• All pre-existing test/unit/secrets-scan.test.ts and test/unit/safety-wiring.test.ts tests pass
  unmodified — including every format-specific pattern (github_token, github_pat, aws_access_key,
  slack_token, google_api_key, gitlab_token, npm_token, stripe_secret_key, sendgrid_key,
  huggingface_token, jwt, private_key_block) and the existing generic_secret_assignment positive case
  (sk_live_ + mixed-case/digit value), none of which are touched by the new exclusion.
• New regression guard: a mixed-case/digit-bearing fake secret (same shape as the existing positive test)
  still flags after the fix, proving the new lowercase-hyphenated-compound exclusion is narrow and doesn't
  broaden past its intended shape.
• New test proving a single lowercase word with no hyphen is unaffected by the new exclusion (still flags),
  confirming the regex specifically requires 2+ hyphen-joined segments.
• The format-specific SECRET_PATTERNS regexes (github_token, aws_access_key, etc.) are completely
  untouched — the fix only changes isPlaceholderSecretValue, which exclusively gates the
  generic_secret_assignment heuristic path.
• secretLeakFinding's existing "scans only added lines / added-or-renamed file headers, never
  removed/context lines" behavior is preserved exactly — now implemented via
  scanDiffForSecretsWithLocations's own line-type handling instead of a pre-filter — and all of that
  behavior's original tests pass unmodified, plus a new explicit test that a removed line's secret-shaped
  content never appears in the finding.

UI Evidence

N/A — backend-only change (src/review/**), no UI/frontend/docs/extension surface touched.

Notes

• Root cause and fix scope were pre-investigated and confirmed empirically against PR fix(review): key the durable CI-state cache on resolved required contexts #3036's real diff
  before this PR was written.

[superagent-security]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.72%. Comparing base (ee4e8ef) to head (fbc78b0).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3178   +/-   ##
=======================================
  Coverage   94.72%   94.72%           
=======================================
  Files         269      269           
  Lines       29619    29658   +39     
  Branches    10806    10814    +8     
=======================================
+ Hits        28056    28095   +39     
  Misses        917      917           
  Partials      646      646           

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.

[loopover-orb]

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-04 19:13:48 UTC}

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

⏸️ Suggested Action - Manual Review

• 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 secret scanning to a structured diff walk so `secret_leak` findings can include file:line locations while preserving the prior added-line-only behavior. The lowercase-hyphenated placeholder exclusion is applied at the captured value source, and the tests cover the real `scanForSecrets`/`secretLeakFinding` paths rather than fabricating an unreachable payload. I do not see a reachable correctness blocker in the provided diff.

Nits — 6 non-blocking

• nit: src/review/secrets-scan.ts:169 treats any non-header/non-removed line as a new-file context line, so malformed hunk headers like the existing test fixture `@​@​` or metadata lines inside a patch can shift reported locations; consider only incrementing for `line.startsWith(" ")` and leaving separators/metadata neutral.
• nit: src/review/secrets-scan.ts:72 has a nested-looking lowercase-hyphen pattern; it is delimiter-bounded by hyphens, but a short comment or a split-based helper would make the intentional non-ReDoS shape clearer for future reviewers.
• nit: test/unit/safety-wiring.test.ts:514 covers line attribution on a normal hunk, but there is no direct unit coverage for `scanDiffForSecretsWithLocations` returning multiple kinds or deduping repeated locations through `secretLeakFinding`.
• src/review/secrets-scan.ts:169: change the fallback increment to only count real context lines, e.g. `if (line.startsWith(" ")) currentNewLine += 1;`, so separators and patch metadata cannot perturb later locations in malformed or hand-built diffs.
• src/review/secrets-scan.ts:72: consider replacing the regex with `value.includes("-") && value.split("-").every((part) => /^[a-z]+$/.test(part))` to make the narrow lowercase-compound exclusion self-evident.
• PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

• 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	❌ 1 blocker	1 reviewer
Linked issue	✅ Linked	#3041
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	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 60 registered-repo PR(s), 50 merged, 442 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 60 PR(s), 442 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• 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: 60 PR(s), 442 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• No action.

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