diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 8583da841d..58b2d8edb7 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -10311,7 +10311,11 @@ async function maybePublishPrPublicSurface( // #8104: record RuleFiredEvent for every configured gate blocker except linked_issue_scope_mismatch // (#8101). Same advisory+policy as evaluateGateCheck above so the filter stays in lock-step. if (evaluation) { - await recordConfiguredGateBlockerSignals(env, advisory, gatePolicy, repoFullName, pr.number); + // #8130: thread the SAME memoized diff the AI review consumed so ai_consensus_defect/ai_review_split + // fired events capture the raw context their detection evaluated (never re-fetched). + await recordConfiguredGateBlockerSignals(env, advisory, gatePolicy, repoFullName, pr.number, { + aiReviewDiff: buildAiReviewDiff(await getReviewFiles()), + }); } // Deterministic content/registry surface lane (#1255) — flag-gated + per-repo allowlist, byte-identical when // off (evaluateWithSurfaceLane returns the generic evaluation unchanged and resolves no files). A metagraphed diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 0ca64eb47e..e1fae729c8 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -1053,12 +1053,36 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli return false; } +// #8130: codes whose raw evaluated content must NEVER be captured into fired-event metadata. `secret_leak` +// is permanently excluded by design: capturing the diff that triggered it would store the leaked credential +// itself in the calibration audit trail — a real security regression, not an acceptable tradeoff for +// backtest coverage. A future sensitive code gets ADDED here deliberately (with this reasoning re-applied); +// per #8130's Boundaries, extending raw-context capture to secret_leak requires an explicit, +// maintainer-reviewed redaction design first, never a quiet edit. +export const RAW_CONTEXT_EXCLUDED_CODES = new Set(["secret_leak"]); + +// #8130: mirror of src/services/ai-review.ts's own `input.diff.slice(0, 120000)` bound — the SAME number, so +// the captured corpus reflects exactly what the AI reviewer saw. Keep the two in sync by hand. +export const RAW_CONTEXT_MAX_DIFF_CHARS = 120000; + /** * Record a {@link RuleFiredEvent} for every finding that `isConfiguredGateBlocker` would put into * `configuredBlockers`, excluding `linked_issue_scope_mismatch` (#8104 / complements #8101). Call from the * env-bearing gate path immediately after {@link evaluateGateCheck} with the SAME advisory + policy so the * filter matches `evaluateGateCheckCore`'s own. Best-effort: a SignalStore failure never throws and never * affects the gate verdict. + * + * #8130: non-excluded codes also capture the raw context their detection actually evaluated, so their + * corpora can backtest logic/detection changes rather than only thresholds: + * • `ai_consensus_defect`/`ai_review_split` — the AI review's own diff (`context.aiReviewDiff`, threaded + * from the caller that already holds it; bounded to {@link RAW_CONTEXT_MAX_DIFF_CHARS}). + * • every other non-excluded code — audited individually (#8130): none of them evaluates raw diff content + * (`missing_linked_issue` reads the PR's linkage state, `duplicate_pr_risk` reads sibling-PR overlap, + * `pre_merge_check_required`/`cla_check_unresolved` read check-run conclusions, `manifest_missing_tests` + * reads changed paths vs the manifest's expectations, the review-thread code reads unresolved-thread + * state) — so the detection's own recorded `detail` string, which narrates exactly that evaluated + * signal, is captured as `rawSignal` (same bound). + * • `RAW_CONTEXT_EXCLUDED_CODES` (`secret_leak`) — confidence only, never raw content. */ export async function recordConfiguredGateBlockerSignals( env: Env, @@ -1066,6 +1090,7 @@ export async function recordConfiguredGateBlockerSignals( policy: GateCheckPolicy, repoFullName: string, prNumber: number, + context: { aiReviewDiff?: string } = {}, ): Promise { const effective = applyMergeReadinessGate(policy); const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective)); @@ -1075,13 +1100,22 @@ export async function recordConfiguredGateBlockerSignals( await Promise.all( configuredBlockers.map((finding) => { if (finding.code === "linked_issue_scope_mismatch") return Promise.resolve(); + const metadata: Record = {}; + if (finding.confidence !== undefined) metadata.confidence = finding.confidence; + if (!RAW_CONTEXT_EXCLUDED_CODES.has(finding.code)) { + if (AI_JUDGMENT_BLOCKER_CODES.has(finding.code)) { + if (context.aiReviewDiff !== undefined) metadata.diff = context.aiReviewDiff.slice(0, RAW_CONTEXT_MAX_DIFF_CHARS); + } else if (finding.detail) { + metadata.rawSignal = finding.detail.slice(0, RAW_CONTEXT_MAX_DIFF_CHARS); + } + } return store .recordRuleFired({ ruleId: finding.code, targetKey, outcome: finding.severity ?? "blocker", occurredAt, - ...(finding.confidence !== undefined ? { metadata: { confidence: finding.confidence } } : {}), + ...(Object.keys(metadata).length > 0 ? { metadata } : {}), }) .catch(() => undefined); }), diff --git a/test/unit/configured-gate-blocker-signals.test.ts b/test/unit/configured-gate-blocker-signals.test.ts index 912e3ba90c..80fde2c0c3 100644 --- a/test/unit/configured-gate-blocker-signals.test.ts +++ b/test/unit/configured-gate-blocker-signals.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { + RAW_CONTEXT_MAX_DIFF_CHARS, recordConfiguredGateBlockerSignals, type GateCheckPolicy, } from "../../src/rules/advisory"; @@ -165,3 +166,80 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => { ).resolves.toBeUndefined(); }); }); + +// ── #8130: bounded raw context in fired-event metadata (secret_leak permanently excluded) ─────────────────── + +describe("recordConfiguredGateBlockerSignals — raw context capture (#8130)", () => { + it("SECURITY: secret_leak's fired event NEVER carries diff or rawSignal, even with confidence and detail present", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "secret_leak", severity: "critical", confidence: 0.99, detail: "AKIA... committed in config.ts" })]), + {}, + "owner/repo", + 7, + { aiReviewDiff: "+const key = 'AKIA-REAL-SECRET';" }, + ); + const [fired] = (await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired; + expect(fired!.metadata).toEqual({ confidence: 0.99 }); + expect(fired!.metadata).not.toHaveProperty("diff"); + expect(fired!.metadata).not.toHaveProperty("rawSignal"); + }); + + it("captures the AI review's diff (bounded to RAW_CONTEXT_MAX_DIFF_CHARS) for ai_consensus_defect", async () => { + const env = createTestEnv(); + const oversized = "d".repeat(RAW_CONTEXT_MAX_DIFF_CHARS + 5000); + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "ai_consensus_defect", confidence: 0.95 })]), + blockAi, + "owner/repo", + 7, + { aiReviewDiff: oversized }, + ); + const [fired] = (await createSignalStore(env).queryRuleHistory("ai_consensus_defect", 0)).fired; + expect((fired!.metadata as { diff: string }).diff).toHaveLength(RAW_CONTEXT_MAX_DIFF_CHARS); + expect((fired!.metadata as { confidence: number }).confidence).toBe(0.95); + }); + + it("records no diff key for an AI code when the caller has no diff to thread", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "ai_review_split", confidence: 0.9 })]), blockAi, "owner/repo", 7); + const [fired] = (await createSignalStore(env).queryRuleHistory("ai_review_split", 0)).fired; + expect(fired!.metadata).toEqual({ confidence: 0.9 }); + }); + + it("captures a non-diff-based code's own evaluated signal (its detail) as rawSignal — the audited fallback", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "missing_linked_issue", detail: "No linked issue reference found in the PR body." })]), + blockLinked, + "owner/repo", + 7, + { aiReviewDiff: "+irrelevant" }, + ); + const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired; + expect(fired!.metadata).toEqual({ rawSignal: "No linked issue reference found in the PR body." }); + }); + + it("records no metadata at all for a non-diff code with no confidence and an empty detail", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "missing_linked_issue", detail: "" })]), blockLinked, "owner/repo", 7); + const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired; + expect(fired!.metadata).toBeUndefined(); + }); + + it("still skips linked_issue_scope_mismatch entirely (#8101's own site records it)", async () => { + const env = createTestEnv(); + await recordConfiguredGateBlockerSignals( + env, + advisory([finding({ code: "linked_issue_scope_mismatch" }), finding({ code: "missing_linked_issue" })]), + { ...blockLinked, linkedIssueSatisfactionGateMode: "block" }, + "owner/repo", + 7, + ); + expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]); + expect((await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired).toHaveLength(1); + }); +});