refactor(review): extract shared secret-detection module (#4608)

[JSONbored]

Summary

• src/review/secrets-scan.ts (PR-diff hard-block) and src/review/content-lane/security-scan.ts
  (content-lane hard-block) hand-duplicated the same secret-detection primitives with no automated pairing
  between them, despite both living under src/, same build, same deploy. That already caused two
  independent, currently-live drifts (fix(review): stop generic_secret_assignment false-flagging self-naming fixture/enum values #4587, Port missing secret-scan fixes to content-lane/security-scan.ts #4604) even after a same-day commit edited both files for one
  change.
• Extracts SECRET_PATTERNS, GENERIC_SECRET_ASSIGNMENT_PATTERN, hasLongSequentialRun,
  isPlaceholderSecretValue, hasGenericSecretAssignment, and HARD_SECRET_KINDS into one new
  src/review/secret-patterns.ts, imported by secrets-scan.ts, content-lane/security-scan.ts, and
  src/review/safety.ts (which carried its own third hand-copy of HARD_SECRET_KINDS).
• Pure extraction — no behavior change for either hard-block path. The only functional edit is
  canonicalizing GENERIC_SECRET_ASSIGNMENT_PATTERN to one capture-group shape (both callers previously
  used slightly different, but behaviorally equivalent, regex forms).
• review-enrichment/src/analyzers/secret-scan.ts (REES) stays untouched and genuinely separate (standalone
  Railway deploy, deliberately wider rule set). Adds a SECRET_DETECTION_TWIN_PAIR named-pair entry to
  scripts/check-engine-parity.ts (same mechanism as Fix engine-parity diffFilePriority typo; make drift-check recursive/function-aware #4605's SAFE_URL_TWIN_PAIR/
  DIFF_FILE_PRIORITY_TWIN_PAIR) that mechanically drift-checks only the subset REES shares with the new
  module — the isPlaceholderSecretValue algorithm plus the hard-kind names that are exact string matches
  on both sides today — so REES's much wider rule set doesn't false-fail a full-file comparison.

Fixes #4608.

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 a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. (see note below)
• npm run test:workers (N/A — no test/workers/** code path touched)
• npm run build:mcp (N/A — no packages/gittensory-mcp code touched)
• npm run test:mcp-pack (N/A — same reason)
• npm run ui:openapi:check (N/A — no API/schema change)
• npm run ui:lint (N/A — no apps/gittensory-ui files touched)
• npm run ui:typecheck (N/A — same reason)
• npm run ui:build (N/A — same reason)
• npm audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

• Ran a targeted vitest run over the six affected test files (test/unit/secret-patterns.test.ts,
  secrets-scan.test.ts, content-lane-security-scan.test.ts, safety.test.ts, safety-wiring.test.ts,
  check-engine-parity-script.test.ts) with --coverage, instead of the full unsharded npm run test:coverage. coverage/lcov.info confirms all four changed src/** files are at 100% lines/branches:
  secret-patterns.ts 25/25 lines, 20/20 branches; secrets-scan.ts 34/34, 18/18; content-lane/security-scan.ts
  35/35, 20/20; safety.ts 19/19, 14/14. All 210 tests in that scope pass. test:workers/build:mcp/
  test:mcp-pack/ui:* were skipped because this PR touches only src/review/**,
  scripts/check-engine-parity.ts, and test/unit/** — no workers-pool tests, MCP package, or UI/apps code.
  Also ran the real npm run engine-parity:drift-check directly (not just its unit tests) against the live
  repo — passes clean. CI's validate job runs the complete gate on every check above.

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. (N/A — no such changes)
• API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — unchanged)
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes)
• Visible UI changes include a UI Evidence section below with screenshots. (N/A — no visible UI change)
• Public docs/changelogs are updated where needed. (N/A — no changelog edit, as required for a normal PR)

UI Evidence

N/A — backend-only refactor, no visible UI/frontend/docs/extension change.

Notes

• src/review/safety.ts is included even though issue Extract shared secret-detection module across secrets-scan.ts and content-lane/security-scan.ts #4608 names only secrets-scan.ts and
  content-lane/security-scan.ts: it carried its own byte-identical third copy of HARD_SECRET_KINDS as
  part of the same PR-diff hard-block path the issue describes, so leaving it out would have left one of the
  three existing duplicates unfixed.
• private_key_block/aws_access_key are deliberately excluded from the new drift-check's kind-name
  markers: REES already names those two concepts private_key/aws_access_key_id, a pre-existing, harmless,
  out-of-scope naming divergence — including them would false-fail the check on this very PR.

[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.14%. Comparing base (e984eef) to head (545933e).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4678      +/-   ##
==========================================
- Coverage   94.14%   94.14%   -0.01%     
==========================================
  Files         435      436       +1     
  Lines       38566    38541      -25     
  Branches    14060    14052       -8     
==========================================
- Hits        36308    36283      -25     
  Misses       1599     1599              
  Partials      659      659              

Files with missing lines	Coverage Δ
src/review/content-lane/security-scan.ts	100.00% <ø> (ø)
src/review/safety.ts	100.00% <ø> (ø)
src/review/secret-patterns.ts	100.00% <100.00%> (ø)
src/review/secrets-scan.ts	100.00% <ø> (ø)

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.