Skip to content

Commit 028079e

Browse files
authored
feat(review): capture bounded raw context in configured-blocker fired events, permanently excluding secret_leak (#8130) (#8135)
1 parent 66a026f commit 028079e

3 files changed

Lines changed: 118 additions & 2 deletions

File tree

src/queue/processors.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10323,7 +10323,11 @@ async function maybePublishPrPublicSurface(
1032310323
// #8104: record RuleFiredEvent for every configured gate blocker except linked_issue_scope_mismatch
1032410324
// (#8101). Same advisory+policy as evaluateGateCheck above so the filter stays in lock-step.
1032510325
if (evaluation) {
10326-
await recordConfiguredGateBlockerSignals(env, advisory, gatePolicy, repoFullName, pr.number);
10326+
// #8130: thread the SAME memoized diff the AI review consumed so ai_consensus_defect/ai_review_split
10327+
// fired events capture the raw context their detection evaluated (never re-fetched).
10328+
await recordConfiguredGateBlockerSignals(env, advisory, gatePolicy, repoFullName, pr.number, {
10329+
aiReviewDiff: buildAiReviewDiff(await getReviewFiles()),
10330+
});
1032710331
}
1032810332
// Deterministic content/registry surface lane (#1255) — flag-gated + per-repo allowlist, byte-identical when
1032910333
// off (evaluateWithSurfaceLane returns the generic evaluation unchanged and resolves no files). A metagraphed

src/rules/advisory.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1053,19 +1053,44 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli
10531053
return false;
10541054
}
10551055

1056+
// #8130: codes whose raw evaluated content must NEVER be captured into fired-event metadata. `secret_leak`
1057+
// is permanently excluded by design: capturing the diff that triggered it would store the leaked credential
1058+
// itself in the calibration audit trail — a real security regression, not an acceptable tradeoff for
1059+
// backtest coverage. A future sensitive code gets ADDED here deliberately (with this reasoning re-applied);
1060+
// per #8130's Boundaries, extending raw-context capture to secret_leak requires an explicit,
1061+
// maintainer-reviewed redaction design first, never a quiet edit.
1062+
export const RAW_CONTEXT_EXCLUDED_CODES = new Set<string>(["secret_leak"]);
1063+
1064+
// #8130: mirror of src/services/ai-review.ts's own `input.diff.slice(0, 120000)` bound — the SAME number, so
1065+
// the captured corpus reflects exactly what the AI reviewer saw. Keep the two in sync by hand.
1066+
export const RAW_CONTEXT_MAX_DIFF_CHARS = 120000;
1067+
10561068
/**
10571069
* Record a {@link RuleFiredEvent} for every finding that `isConfiguredGateBlocker` would put into
10581070
* `configuredBlockers`, excluding `linked_issue_scope_mismatch` (#8104 / complements #8101). Call from the
10591071
* env-bearing gate path immediately after {@link evaluateGateCheck} with the SAME advisory + policy so the
10601072
* filter matches `evaluateGateCheckCore`'s own. Best-effort: a SignalStore failure never throws and never
10611073
* affects the gate verdict.
1074+
*
1075+
* #8130: non-excluded codes also capture the raw context their detection actually evaluated, so their
1076+
* corpora can backtest logic/detection changes rather than only thresholds:
1077+
* • `ai_consensus_defect`/`ai_review_split` — the AI review's own diff (`context.aiReviewDiff`, threaded
1078+
* from the caller that already holds it; bounded to {@link RAW_CONTEXT_MAX_DIFF_CHARS}).
1079+
* • every other non-excluded code — audited individually (#8130): none of them evaluates raw diff content
1080+
* (`missing_linked_issue` reads the PR's linkage state, `duplicate_pr_risk` reads sibling-PR overlap,
1081+
* `pre_merge_check_required`/`cla_check_unresolved` read check-run conclusions, `manifest_missing_tests`
1082+
* reads changed paths vs the manifest's expectations, the review-thread code reads unresolved-thread
1083+
* state) — so the detection's own recorded `detail` string, which narrates exactly that evaluated
1084+
* signal, is captured as `rawSignal` (same bound).
1085+
* • `RAW_CONTEXT_EXCLUDED_CODES` (`secret_leak`) — confidence only, never raw content.
10621086
*/
10631087
export async function recordConfiguredGateBlockerSignals(
10641088
env: Env,
10651089
advisoryResult: Advisory,
10661090
policy: GateCheckPolicy,
10671091
repoFullName: string,
10681092
prNumber: number,
1093+
context: { aiReviewDiff?: string } = {},
10691094
): Promise<void> {
10701095
const effective = applyMergeReadinessGate(policy);
10711096
const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective));
@@ -1075,13 +1100,22 @@ export async function recordConfiguredGateBlockerSignals(
10751100
await Promise.all(
10761101
configuredBlockers.map((finding) => {
10771102
if (finding.code === "linked_issue_scope_mismatch") return Promise.resolve();
1103+
const metadata: Record<string, unknown> = {};
1104+
if (finding.confidence !== undefined) metadata.confidence = finding.confidence;
1105+
if (!RAW_CONTEXT_EXCLUDED_CODES.has(finding.code)) {
1106+
if (AI_JUDGMENT_BLOCKER_CODES.has(finding.code)) {
1107+
if (context.aiReviewDiff !== undefined) metadata.diff = context.aiReviewDiff.slice(0, RAW_CONTEXT_MAX_DIFF_CHARS);
1108+
} else if (finding.detail) {
1109+
metadata.rawSignal = finding.detail.slice(0, RAW_CONTEXT_MAX_DIFF_CHARS);
1110+
}
1111+
}
10781112
return store
10791113
.recordRuleFired({
10801114
ruleId: finding.code,
10811115
targetKey,
10821116
outcome: finding.severity ?? "blocker",
10831117
occurredAt,
1084-
...(finding.confidence !== undefined ? { metadata: { confidence: finding.confidence } } : {}),
1118+
...(Object.keys(metadata).length > 0 ? { metadata } : {}),
10851119
})
10861120
.catch(() => undefined);
10871121
}),

test/unit/configured-gate-blocker-signals.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
22
import {
3+
RAW_CONTEXT_MAX_DIFF_CHARS,
34
recordConfiguredGateBlockerSignals,
45
type GateCheckPolicy,
56
} from "../../src/rules/advisory";
@@ -165,3 +166,80 @@ describe("recordConfiguredGateBlockerSignals (#8104)", () => {
165166
).resolves.toBeUndefined();
166167
});
167168
});
169+
170+
// ── #8130: bounded raw context in fired-event metadata (secret_leak permanently excluded) ───────────────────
171+
172+
describe("recordConfiguredGateBlockerSignals — raw context capture (#8130)", () => {
173+
it("SECURITY: secret_leak's fired event NEVER carries diff or rawSignal, even with confidence and detail present", async () => {
174+
const env = createTestEnv();
175+
await recordConfiguredGateBlockerSignals(
176+
env,
177+
advisory([finding({ code: "secret_leak", severity: "critical", confidence: 0.99, detail: "AKIA... committed in config.ts" })]),
178+
{},
179+
"owner/repo",
180+
7,
181+
{ aiReviewDiff: "+const key = 'AKIA-REAL-SECRET';" },
182+
);
183+
const [fired] = (await createSignalStore(env).queryRuleHistory("secret_leak", 0)).fired;
184+
expect(fired!.metadata).toEqual({ confidence: 0.99 });
185+
expect(fired!.metadata).not.toHaveProperty("diff");
186+
expect(fired!.metadata).not.toHaveProperty("rawSignal");
187+
});
188+
189+
it("captures the AI review's diff (bounded to RAW_CONTEXT_MAX_DIFF_CHARS) for ai_consensus_defect", async () => {
190+
const env = createTestEnv();
191+
const oversized = "d".repeat(RAW_CONTEXT_MAX_DIFF_CHARS + 5000);
192+
await recordConfiguredGateBlockerSignals(
193+
env,
194+
advisory([finding({ code: "ai_consensus_defect", confidence: 0.95 })]),
195+
blockAi,
196+
"owner/repo",
197+
7,
198+
{ aiReviewDiff: oversized },
199+
);
200+
const [fired] = (await createSignalStore(env).queryRuleHistory("ai_consensus_defect", 0)).fired;
201+
expect((fired!.metadata as { diff: string }).diff).toHaveLength(RAW_CONTEXT_MAX_DIFF_CHARS);
202+
expect((fired!.metadata as { confidence: number }).confidence).toBe(0.95);
203+
});
204+
205+
it("records no diff key for an AI code when the caller has no diff to thread", async () => {
206+
const env = createTestEnv();
207+
await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "ai_review_split", confidence: 0.9 })]), blockAi, "owner/repo", 7);
208+
const [fired] = (await createSignalStore(env).queryRuleHistory("ai_review_split", 0)).fired;
209+
expect(fired!.metadata).toEqual({ confidence: 0.9 });
210+
});
211+
212+
it("captures a non-diff-based code's own evaluated signal (its detail) as rawSignal — the audited fallback", async () => {
213+
const env = createTestEnv();
214+
await recordConfiguredGateBlockerSignals(
215+
env,
216+
advisory([finding({ code: "missing_linked_issue", detail: "No linked issue reference found in the PR body." })]),
217+
blockLinked,
218+
"owner/repo",
219+
7,
220+
{ aiReviewDiff: "+irrelevant" },
221+
);
222+
const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired;
223+
expect(fired!.metadata).toEqual({ rawSignal: "No linked issue reference found in the PR body." });
224+
});
225+
226+
it("records no metadata at all for a non-diff code with no confidence and an empty detail", async () => {
227+
const env = createTestEnv();
228+
await recordConfiguredGateBlockerSignals(env, advisory([finding({ code: "missing_linked_issue", detail: "" })]), blockLinked, "owner/repo", 7);
229+
const [fired] = (await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired;
230+
expect(fired!.metadata).toBeUndefined();
231+
});
232+
233+
it("still skips linked_issue_scope_mismatch entirely (#8101's own site records it)", async () => {
234+
const env = createTestEnv();
235+
await recordConfiguredGateBlockerSignals(
236+
env,
237+
advisory([finding({ code: "linked_issue_scope_mismatch" }), finding({ code: "missing_linked_issue" })]),
238+
{ ...blockLinked, linkedIssueSatisfactionGateMode: "block" },
239+
"owner/repo",
240+
7,
241+
);
242+
expect((await createSignalStore(env).queryRuleHistory("linked_issue_scope_mismatch", 0)).fired).toEqual([]);
243+
expect((await createSignalStore(env).queryRuleHistory("missing_linked_issue", 0)).fired).toHaveLength(1);
244+
});
245+
});

0 commit comments

Comments
 (0)