feat(review): capture bounded raw context in configured-blocker fired events, permanently excluding secret_leak - #8135
Conversation
… events, permanently excluding secret_leak (JSONbored#8130)
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8135 +/- ##
==========================================
- Coverage 92.02% 90.37% -1.66%
==========================================
Files 760 99 -661
Lines 77394 26010 -51384
Branches 23392 5108 -18284
==========================================
- Hits 71225 23506 -47719
+ Misses 5061 2233 -2828
+ Partials 1108 271 -837
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-23 00:30:52 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
Closes #8130.
Extends #8104's generic
recordConfiguredGateBlockerSignals(src/rules/advisory.ts) so every non-excluded configured-blocker code captures the raw context its detection actually evaluated — making those corpora rich enough to backtest logic/detection changes, not just thresholds — while permanently excludingsecret_leakby design.RAW_CONTEXT_EXCLUDED_CODES— the exported, named constant the issue mandates (secret_leakinside), with the full security reasoning attached: capturing the diff that triggered asecret_leakfinding would store the leaked credential itself in the calibration audit trail. The comment also carries the issue's Boundaries note (additions require an explicit, maintainer-reviewed redaction design — never a quiet edit).RAW_CONTEXT_MAX_DIFF_CHARS = 120000— mirrorssrc/services/ai-review.ts's owninput.diff.slice(0, 120000)bound exactly (the same number, with a keep-in-sync comment), never a newly invented bound.ai_consensus_defect/ai_review_split): the metadata gains the AI review's own diff, threaded from the one caller (src/queue/processors.ts) via the already-memoizedgetReviewFiles()+ the samebuildAiReviewDiffthe review consumed — never re-fetched or re-derived.missing_linked_issuereads linkage state,duplicate_pr_riskreads sibling-PR overlap,pre_merge_check_required/cla_check_unresolvedread check-run conclusions,manifest_missing_testsreads changed paths vs manifest expectations, the review-thread code reads unresolved-thread state — none evaluates raw diff content, so each captures its detection's own recordeddetail(the narrative of exactly that evaluated signal) asrawSignal, same bound.linked_issue_scope_mismatchremains skipped entirely (calibration: wire linked_issue_scope_mismatch into the shared signal-tracking module (fired + reversal) #8101's own site records it — unchanged), and the calibration: wire recordRuleFired/recordHumanOverride for the remaining isConfiguredGateBlocker codes (excludes linked_issue_scope_mismatch, covered by #8101) #8104 behavior for confidence capture is preserved byte-for-byte.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥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.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test/unit/configured-gate-blocker-signals.test.ts— 14/14 green (8 pre-existing calibration: wire recordRuleFired/recordHumanOverride for the remaining isConfiguredGateBlocker codes (excludes linked_issue_scope_mismatch, covered by #8101) #8104 cases unchanged + 6 new calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130 cases: the security negative assertingsecret_leak's event carries NOdiff/rawSignaleven with confidence, detail, and a threaded diff all present; the AI-code diff capture with the truncation branch at the exact bound; the no-diff-threaded branch; the auditedrawSignalfallback; the empty-detail/no-metadata branch; thelinked_issue_scope_mismatchskip) — plusnpm run engine-parity:drift-check(ok: the advisory.ts↔gate-advisory.ts twin pair still agrees; the added recorder context is host-side-only, the same posture calibration: wire recordRuleFired/recordHumanOverride for the remaining isConfiguredGateBlocker codes (excludes linked_issue_scope_mismatch, covered by #8101) #8104 itself established). Real CI re-runs typecheck and the sharded suite here.Safety
UI Evidencesection 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.The three unchecked Safety boxes are N/A: no auth/session/UI change. Note the change is itself security-motivated in part: the
secret_leakexclusion exists precisely so no leaked credential can ever enter the calibration audit trail, and that property is pinned by a dedicated negative test.UI Evidence
N/A — no UI change.
Notes
metadatakey at all (rather than an empty object) when nothing is captured — preserving calibration: wire recordRuleFired/recordHumanOverride for the remaining isConfiguredGateBlocker codes (excludes linked_issue_scope_mismatch, covered by #8101) #8104's exact optional-property behavior for that case.