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
10 changes: 6 additions & 4 deletions packages/loopover-engine/src/advisory/gate-advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ import { REVIEW_THREAD_BLOCKER_CODE } from "../review/review-thread-findings.js"
import { labelMatchesPattern } from "../scoring/label-match.js";

// Kept byte-identical with the GATE_DECISION_TWIN_PAIR copy in src/rules/advisory.ts
// (checkGateDecisionVersionBump enforces this). The mnemonics/seed-phrases/cohort/miner-|human-originated/
// bare-raw-trust/bare-rankings terms were ported from sanitizePublicComment's own fix for the same leak class
// (#7074) -- `raw\s+trust\s+scores?` stays ahead of bare `raw\s+trust` so the compound still matches first.
// (checkGateDecisionForbiddenTermsParity now diffs the two regex bodies byte-for-byte, #8697). The
// mnemonics/seed-phrases/cohort/miner-|human-originated/bare-raw-trust/bare-rankings terms were ported from
// sanitizePublicComment's own fix for the same leak class (#7074) -- `raw\s+trust\s+scores?` stays ahead of
// bare `raw\s+trust` so the compound still matches first.
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?|payouts?|farming|estimated\s+scores?|raw\s+trust\s+scores?|raw\s+trust|trust\s+scores?|score\s+estimates?|reward\s+estimates?|wallets?|hotkeys?|coldkeys?|mnemonics?|seed\s?phrases?|cohorts?|miner[-_\s]?originated|human[-_\s]?originated|rankings?|reviewability|scoreability|private\s+signals?)\b/gi;
/\b(?:rewards?|payouts?|farming|estimated\s+scores?|raw\s+trust\s+scores?|raw\s+trust|trust\s+scores?|score\s+estimates?|reward\s+estimates?|wallets?|hotkeys?|coldkeys?|mnemonics?|seed\s?phrases?|cohorts?|miner[-_\s]?originated|human[-_\s]?originated|rankings?|reviewability|scoreability|private\s+signals?|likely_duplicate|reviewability\s*\d)\b/gi;

function sanitizeForCheckRun(text: string): string {
return text.replace(CHECK_RUN_FORBIDDEN_TERMS, "[context]").replace(/\s+/g, " ").trim();
Expand Down Expand Up @@ -707,6 +708,7 @@ function normalizeScore(value: number | null | undefined): number | null {
/** @internal Exported for unit tests of advisory severity wiring. */
export const gateAdvisoryInternals = {
advisory,
sanitizeForCheckRun,
highestSeverity,
conclusionForSeverity,
buildSizeHoldFinding,
Expand Down
64 changes: 64 additions & 0 deletions scripts/check-engine-parity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,67 @@ export function checkGateDecisionTwinPresence({
return { failures, pairChecked: twin };
}

/** The `const` both gate-decision twins declare their shared check-run redaction regex on. */
const FORBIDDEN_TERMS_CONST = "const CHECK_RUN_FORBIDDEN_TERMS =";

/** Pull the `CHECK_RUN_FORBIDDEN_TERMS` regex literal (source + flags) out of a twin file's raw text so the
* two copies can be compared by CONTENT, not just by the const name's presence (#8697). The regex body
* carries no forward slash, so the literal spans from the first `/` after the const to the next unescaped
* `/` and its trailing flags. Returns null when the const declaration or its regex literal isn't found. */
export function extractForbiddenTermsRegex(text: string): string | null {
const constIndex = text.indexOf(FORBIDDEN_TERMS_CONST);
if (constIndex === -1) return null;
const afterConst = text.slice(constIndex + FORBIDDEN_TERMS_CONST.length);
const literal = afterConst.match(/\/((?:\\.|[^/\\])+)\/([a-z]*)/);
if (!literal) return null;
return `/${literal[1]}/${literal[2]}`;
}

/** Content-level drift guard (#8697): both gate-decision twins hand-maintain a `CHECK_RUN_FORBIDDEN_TERMS`
* regex their own comments claim is "byte-identical", but no check ever diffed the bodies -- the engine copy
* had silently dropped `likely_duplicate|reviewability\s*\d`. `checkGateDecisionTwinPresence` only asserts
* the four function-name markers exist, so it never saw this. This diffs the two regex literals directly, so
* a future divergence in the body itself (not just a missing entrypoint) fails CI immediately. */
export function checkGateDecisionForbiddenTermsParity({
root,
readFile = defaultReadFile,
pair = GATE_DECISION_TWIN_PAIR,
}: {
root: string;
readFile?: EngineParityReadFile;
pair?: NamedTwinPair;
}): { failures: string[] } {
let hostText: string;
let engineText: string;
try {
hostText = readFile(root, pair.hostRelative);
engineText = readFile(root, pair.engineRelative);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
return { failures: [`Could not load ${pair.area} twin pair files for regex-body parity: ${message}`] };
}
const failures: string[] = [];
const hostRegex = extractForbiddenTermsRegex(hostText);
const engineRegex = extractForbiddenTermsRegex(engineText);
if (hostRegex === null) {
failures.push(`${pair.hostRelative} is missing a CHECK_RUN_FORBIDDEN_TERMS regex literal to diff.`);
}
if (engineRegex === null) {
failures.push(`${pair.engineRelative} is missing a CHECK_RUN_FORBIDDEN_TERMS regex literal to diff.`);
}
if (hostRegex !== null && engineRegex !== null && hostRegex !== engineRegex) {
failures.push(
[
"CHECK_RUN_FORBIDDEN_TERMS regex body has drifted between the gate-decision twins:",
` • ${pair.hostRelative}: ${hostRegex}`,
` • ${pair.engineRelative}: ${engineRegex}`,
` Make ${pair.engineRelative}'s regex byte-identical to the host copy.`,
].join("\n"),
);
}
return { failures };
}

export function parseEnginePackageVersion(text: string): string | null {
try {
const version = JSON.parse(text).version;
Expand Down Expand Up @@ -635,6 +696,8 @@ export function runEngineParityChecks(options: {
const namedTwinPresence = NAMED_TWIN_PAIRS.map(({ pair, markers }) =>
checkGateDecisionTwinPresence({ root: options.root, readFile, pair, markers }),
);
// Content-level guard on the gate-decision twins' shared redaction regex, beyond mere marker presence (#8697).
const forbiddenTermsParity = checkGateDecisionForbiddenTermsParity({ root: options.root, readFile });
const skew = checkEngineVersionSkew(options);
const pinSync = checkMinerEngineVersionPinSync(options);
let headEngineVersion = options.headEngineVersion;
Expand Down Expand Up @@ -675,6 +738,7 @@ export function runEngineParityChecks(options: {
failures: [
...drift.failures,
...namedTwinPresence.flatMap((result) => result.failures),
...forbiddenTermsParity.failures,
...versionBump.failures,
...skew.failures,
...pinSync.failures,
Expand Down
8 changes: 4 additions & 4 deletions src/rules/advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,10 @@ export function buildIssueAdvisory(repo: RepositoryRecord | null, issue: IssueRe
}

// Kept byte-identical with the GATE_DECISION_TWIN_PAIR copy in packages/loopover-engine/src/advisory/
// gate-advisory.ts (checkGateDecisionVersionBump enforces this). The mnemonics/seed-phrases/cohort/
// miner-|human-originated/bare-raw-trust/bare-rankings terms were ported from sanitizePublicComment's own fix
// for the same leak class (#7074) -- `raw\s+trust\s+scores?` stays ahead of bare `raw\s+trust` so the compound
// still matches first.
// gate-advisory.ts (checkGateDecisionForbiddenTermsParity now diffs the two regex bodies byte-for-byte, #8697).
// The mnemonics/seed-phrases/cohort/miner-|human-originated/bare-raw-trust/bare-rankings terms were ported from
// sanitizePublicComment's own fix for the same leak class (#7074) -- `raw\s+trust\s+scores?` stays ahead of
// bare `raw\s+trust` so the compound still matches first.
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?|payouts?|farming|estimated\s+scores?|raw\s+trust\s+scores?|raw\s+trust|trust\s+scores?|score\s+estimates?|reward\s+estimates?|wallets?|hotkeys?|coldkeys?|mnemonics?|seed\s?phrases?|cohorts?|miner[-_\s]?originated|human[-_\s]?originated|rankings?|reviewability|scoreability|private\s+signals?|likely_duplicate|reviewability\s*\d)\b/gi;

Expand Down
107 changes: 107 additions & 0 deletions test/unit/check-engine-parity-script.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { join } from "node:path";
import { describe, expect, it, vi } from "vitest";
import {
checkEngineParityDrift,
checkGateDecisionForbiddenTermsParity,
checkGateDecisionTwinPresence,
checkGateDecisionVersionBump,
checkEngineVersionSkew,
Expand All @@ -18,6 +19,7 @@ import {
discoverEngineParityPairs,
discoverGateDecisionTwinPair,
enginePackageVersionIncreased,
extractForbiddenTermsRegex,
GATE_DECISION_TWIN_PAIR,
type EngineParityPair,
isEngineStubPair,
Expand Down Expand Up @@ -347,6 +349,111 @@ describe("check-engine-parity script", () => {
});
});

// #8697: strengthen the gate-decision twin guard to diff the CHECK_RUN_FORBIDDEN_TERMS regex BODY, not just
// the four function-name markers -- the engine copy had silently dropped `likely_duplicate|reviewability\s*\d`
// and nothing caught it. These mirror the marker-presence test pattern above with a synthetic divergence.
describe("CHECK_RUN_FORBIDDEN_TERMS regex-body parity (#8697)", () => {
const gateDecisionReadFile = (host: string, engine: string) => (_root: string, relativePath: string) => {
if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return host;
if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return engine;
throw new Error(`unexpected read: ${relativePath}`);
};

it("extracts the regex literal (source + flags) and returns null when it is absent", () => {
const withRegex = String.raw`
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?|likely_duplicate|reviewability\s*\d)\b/gi;
`;
expect(extractForbiddenTermsRegex(withRegex)).toBe("/\\b(?:rewards?|likely_duplicate|reviewability\\s*\\d)\\b/gi");
expect(extractForbiddenTermsRegex("export const OTHER = 1;\n")).toBeNull();
expect(extractForbiddenTermsRegex("const CHECK_RUN_FORBIDDEN_TERMS = buildTerms(list);\n")).toBeNull();
});

it("passes against the real repo now that both twins carry the same regex body", () => {
expect(checkGateDecisionForbiddenTermsParity({ root: process.cwd() }).failures).toEqual([]);
});

it("passes when the two synthetic regex bodies are byte-identical", () => {
const body = String.raw`
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?|likely_duplicate|reviewability\s*\d)\b/gi;
`;
const result = checkGateDecisionForbiddenTermsParity({ root: "/fake", readFile: gateDecisionReadFile(body, body) });
expect(result.failures).toEqual([]);
});

it("fails when the two regex bodies are made to diverge again (synthetic fixture)", () => {
const host = String.raw`
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?|likely_duplicate|reviewability\s*\d)\b/gi;
`;
const engine = String.raw`
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?)\b/gi;
`;
const result = checkGateDecisionForbiddenTermsParity({ root: "/fake", readFile: gateDecisionReadFile(host, engine) });
expect(result.failures.some((failure) => failure.includes("regex body has drifted"))).toBe(true);
// runEngineParityChecks surfaces the same drift end-to-end.
const combined = runEngineParityChecks({
root: "/fake",
readFile: (_root, relativePath) => {
if (relativePath === "packages/loopover-engine/package.json") return JSON.stringify({ version: "0.2.0" });
if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return host;
if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return engine;
throw new Error(`unexpected read: ${relativePath}`);
},
listDir: () => [],
resolveInstalled: () => "0.2.0",
readExpected: () => "0.2.0",
changedFiles: [],
headEngineVersion: "0.2.0",
});
expect(combined.failures.some((failure) => failure.includes("regex body has drifted"))).toBe(true);
});

it("fails when either twin copy is missing its regex literal entirely", () => {
const withRegex = String.raw`
const CHECK_RUN_FORBIDDEN_TERMS =
/\b(?:rewards?)\b/gi;
`;
const withoutRegex = "const CHECK_RUN_FORBIDDEN_TERMS = buildForbiddenTerms();\n";

const engineMissing = checkGateDecisionForbiddenTermsParity({
root: "/fake",
readFile: gateDecisionReadFile(withRegex, withoutRegex),
});
expect(
engineMissing.failures.some((failure) => failure.includes(`${GATE_DECISION_TWIN_PAIR.engineRelative} is missing`)),
).toBe(true);

const hostMissing = checkGateDecisionForbiddenTermsParity({
root: "/fake",
readFile: gateDecisionReadFile(withoutRegex, withRegex),
});
expect(
hostMissing.failures.some((failure) => failure.includes(`${GATE_DECISION_TWIN_PAIR.hostRelative} is missing`)),
).toBe(true);
});

it("reports a load failure when a twin file cannot be read (both Error and non-Error throws)", () => {
const errorThrow = checkGateDecisionForbiddenTermsParity({
root: "/fake",
readFile: () => {
throw new Error("boom");
},
});
expect(errorThrow.failures).toEqual(["Could not load gate-decision twin pair files for regex-body parity: boom"]);

const stringThrow = checkGateDecisionForbiddenTermsParity({
root: "/fake",
readFile: () => {
throw "kaboom";
},
});
expect(stringThrow.failures).toEqual(["Could not load gate-decision twin pair files for regex-body parity: kaboom"]);
});
});

describe("named twin-pair coverage (#4605)", () => {
it("registers the gate-decision, safe-url, diff-file-priority, shares-meaningful-file, and secret-detection pairs", () => {
const areas = NAMED_TWIN_PAIRS.map(({ pair }) => pair.area);
Expand Down
39 changes: 39 additions & 0 deletions test/unit/predicted-gate-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,45 @@ describe("predicted-gate engine module coverage (#2283)", () => {
expect(sanitizePublicComment("open pr count 12 exceeds threshold 10")).toContain("private context");
});

// #8697: CHECK_RUN_FORBIDDEN_TERMS in gate-advisory.ts had silently drifted from its host twin, missing the
// `likely_duplicate` and `reviewability\s*\d` alternatives, so sanitizeForCheckRun leaked those terms. Prove
// the engine copy now redacts them, both directly and through evaluateGateCheck's rendered check-run text.
it("sanitizeForCheckRun redacts likely_duplicate and reviewability<digit> (#8697 twin drift)", () => {
expect(gateAdvisoryInternals.sanitizeForCheckRun("this is likely_duplicate of another PR")).toBe(
"this is [context] of another PR",
);
const scrubbed = gateAdvisoryInternals.sanitizeForCheckRun("reviewability 87 too low");
expect(scrubbed).not.toContain("reviewability");
expect(scrubbed).toContain("[context]");

const evaluation = evaluateGateCheck(
{
id: "a",
targetType: "pull_request",
targetKey: "k",
repoFullName: REPO.fullName,
conclusion: "action_required",
severity: "critical",
title: "t",
summary: "s",
generatedAt: "2026-01-01T00:00:00.000Z",
findings: [
{
code: "duplicate_pr_risk",
severity: "critical",
title: "This PR is likely_duplicate of #123",
detail: "overlaps an existing PR",
action: "reviewability 87 is below the floor",
},
],
},
{ duplicatePrGateMode: "block" },
);
expect(evaluation.conclusion).toBe("failure");
expect(`${evaluation.title} ${evaluation.summary}`).not.toMatch(/likely_duplicate|reviewability/);
expect(evaluation.summary).toContain("[context]");
});

// Regression: this sanitizer's phrase list had no entry for bare "cohort" or standalone
// miner-originated/human-originated/raw-trust (only compound phrases like "raw trust score"), unlike the
// canonical PUBLIC_UNSAFE_TERMS boundary (src/signals/redaction.ts) which treats all of these as unsafe.
Expand Down
Loading