Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 35 additions & 1 deletion src/rules/advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1053,19 +1053,44 @@ 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<string>(["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,
advisoryResult: Advisory,
policy: GateCheckPolicy,
repoFullName: string,
prNumber: number,
context: { aiReviewDiff?: string } = {},
): Promise<void> {
const effective = applyMergeReadinessGate(policy);
const configuredBlockers = advisoryResult.findings.filter((finding) => isConfiguredGateBlocker(finding, effective));
Expand All @@ -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<string, unknown> = {};
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);
}),
Expand Down
78 changes: 78 additions & 0 deletions test/unit/configured-gate-blocker-signals.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);
});
});