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
67 changes: 42 additions & 25 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ import {
import {
loadLinkedIssueHardRules,
resolveLinkedIssueHardRule,
resolveLinkedIssueHasOpenReference,
} from "../review/linked-issue-hard-rules";
import { DEFAULT_UNLINKED_ISSUE_GUARDRAIL } from "../review/unlinked-issue-guardrail-config";
import { resolveUnlinkedIssueMatchHold } from "../review/unlinked-issue-guardrail";
Expand Down Expand Up @@ -1628,20 +1629,22 @@ async function sweepRepoRegate(
const others = openPullRequests.filter(
(other) => other.number !== pr.number,
);
// Thread linked-issue authors so the re-gate sweep applies the self-authored-linked-issue block too — without
// this a self-authored PR re-gated by the sweep escapes a block the main webhook path applies. (#self-authored-parity)
const linkedIssueAuthorLogins = await resolveLinkedIssueAuthorLogins(
// Thread linked-issue authors + the open-reference check so the re-gate sweep applies the same
// self-authored-linked-issue block AND stale-issue-link countermeasure the main webhook path applies —
// without this a self-authored or stale-link-gaming PR re-gated by the sweep escapes both. (#self-authored-parity, #unlinked-issue-guardrail-followup)
const { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue } = await resolveLinkedIssueAdvisoryContext(
env,
sweepInstallationId,
repoFullName,
pr.linkedIssues,
settings.selfAuthoredLinkedIssueGateMode === "block",
settings,
);
const advisory = buildPullRequestAdvisory(repo, pr, {
otherOpenPullRequests: others,
requireLinkedIssue,
duplicateWinnerEnabled,
linkedIssueAuthorLogins,
confirmedNoOpenLinkedIssue,
});
const gate = evaluateGateCheck(
advisory,
Expand Down Expand Up @@ -2991,16 +2994,10 @@ async function reReviewStoredPullRequest(
))
)
return;
const [cachedOtherOpenPullRequests, linkedIssueAuthorLogins] =
const [cachedOtherOpenPullRequests, { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue }] =
await Promise.all([
listOtherOpenPullRequests(env, repoFullName, prNumber),
resolveLinkedIssueAuthorLogins(
env,
installationId,
repoFullName,
pr.linkedIssues,
settings.selfAuthoredLinkedIssueGateMode === "block",
),
resolveLinkedIssueAdvisoryContext(env, installationId, repoFullName, pr.linkedIssues, settings),
]);
// #dup-winner / audit #15: drop any cached-open duplicate sibling already closed on GitHub before the advisory
// (and the disposition below) elect the cluster winner, so the real lowest-OPEN PR is never demoted+auto-closed.
Expand All @@ -3015,6 +3012,7 @@ async function reReviewStoredPullRequest(
otherOpenPullRequests,
requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings),
duplicateWinnerEnabled: env.GITTENSORY_DUPLICATE_WINNER === "true",
confirmedNoOpenLinkedIssue,
linkedIssueAuthorLogins,
});
await persistAdvisory(env, advisory);
Expand Down Expand Up @@ -5378,19 +5376,14 @@ async function processGitHubWebhook(
});
return;
}
// Resolve settings first so the self-authored live-fetch fallback only fires when its gate is in block mode.
// Resolve settings first so the self-authored + open-reference live-fetch fallbacks only fire when their
// respective gates are in block mode.
const settings = await resolveRepositorySettings(env, repoFullName);
const [repo, cachedOtherOpenPullRequests, linkedIssueAuthorLogins] =
const [repo, cachedOtherOpenPullRequests, { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue }] =
await Promise.all([
getRepository(env, repoFullName),
listOtherOpenPullRequests(env, repoFullName, pr.number),
resolveLinkedIssueAuthorLogins(
env,
installationId,
repoFullName,
pr.linkedIssues,
settings.selfAuthoredLinkedIssueGateMode === "block",
),
resolveLinkedIssueAdvisoryContext(env, installationId, repoFullName, pr.linkedIssues, settings),
]);
// #dup-winner / audit #15: drop any cached-open duplicate sibling already closed on GitHub before the
// advisory (and the disposition) elect the cluster winner, so the real lowest-OPEN PR is never auto-closed.
Expand All @@ -5405,6 +5398,7 @@ async function processGitHubWebhook(
otherOpenPullRequests,
requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings),
duplicateWinnerEnabled: env.GITTENSORY_DUPLICATE_WINNER === "true",
confirmedNoOpenLinkedIssue,
linkedIssueAuthorLogins,
});
await persistAdvisory(env, advisory);
Expand Down Expand Up @@ -5914,6 +5908,27 @@ export async function resolveLinkedIssueAuthorLogins(
);
}

// Shared per-call-site resolver for buildPullRequestAdvisory's linked-issue-derived context
// (#unlinked-issue-guardrail-followup). Every gate-evaluating call site (the main webhook path, the cron
// sweep, the heavy re-review pass, and authorized PR actions) already threads `linkedIssueAuthorLogins` the
// same way; bundling the new open-reference check into the SAME resolver keeps all of them in parity rather
// than risking only some remembering to add it. The live open-reference fetch is skipped entirely (resolves
// `true` with no network call) unless `linkedIssueGateMode` is actually "block" -- the only mode where
// whether a citation is open can change the gate's outcome.
export async function resolveLinkedIssueAdvisoryContext(
env: Env,
installationId: number | null | undefined,
repoFullName: string,
linkedIssues: number[],
settings: Pick<RepositorySettings, "selfAuthoredLinkedIssueGateMode" | "linkedIssueGateMode">,
): Promise<{ linkedIssueAuthorLogins: (string | null)[]; confirmedNoOpenLinkedIssue: boolean }> {
const [linkedIssueAuthorLogins, hasOpenReference] = await Promise.all([
resolveLinkedIssueAuthorLogins(env, installationId, repoFullName, linkedIssues, settings.selfAuthoredLinkedIssueGateMode === "block"),
settings.linkedIssueGateMode === "block" ? resolveLinkedIssueHasOpenReference({ env, repoFullName, linkedIssues, installationId }) : Promise.resolve(true),
]);
return { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue: !hasOpenReference };
}

export function shouldCollectSlopEvidence(
settings: Pick<RepositorySettings, "slopGateMode" | "mergeReadinessGateMode">,
): boolean {
Expand Down Expand Up @@ -9908,19 +9923,21 @@ export async function buildAuthorizedPrActionAdvisory(
getRepository(env, repoFullName),
listOtherOpenPullRequests(env, repoFullName, pr.number),
]);
// Mirror the main webhook path: thread linked-issue authors so an authorized PR action (gate-override / panel
// retrigger) honors the self-authored-linked-issue block too. installationId comes from the repo record. (#self-authored-parity)
const linkedIssueAuthorLogins = await resolveLinkedIssueAuthorLogins(
// Mirror the main webhook path: thread linked-issue authors + the open-reference check so an authorized PR
// action (gate-override / panel retrigger) honors the same self-authored-linked-issue block AND stale-
// issue-link countermeasure. installationId comes from the repo record. (#self-authored-parity, #unlinked-issue-guardrail-followup)
const { linkedIssueAuthorLogins, confirmedNoOpenLinkedIssue } = await resolveLinkedIssueAdvisoryContext(
env,
repo?.installationId ?? null,
repoFullName,
pr.linkedIssues,
settings.selfAuthoredLinkedIssueGateMode === "block",
settings,
);
const advisory = buildPullRequestAdvisory(repo, pr, {
otherOpenPullRequests,
requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings),
duplicateWinnerEnabled: env.GITTENSORY_DUPLICATE_WINNER === "true",
confirmedNoOpenLinkedIssue,
linkedIssueAuthorLogins,
});
return { repo, advisory };
Expand Down
49 changes: 48 additions & 1 deletion src/review/linked-issue-hard-rules.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { fetchLinkedIssueFacts } from "../github/backfill";
import { fetchLinkedIssueFacts, type LinkedIssueFactsFetch } from "../github/backfill";
import { githubRateLimitAdmissionKeyForToken } from "../github/client";
import { createInstallationToken } from "../github/app";
import { extractLinkedIssueNumbersWithOverflow } from "../db/repositories";
import { resolveRepositorySettings } from "../settings/repository-settings";
import { DEFAULT_LINKED_ISSUE_HARD_RULES } from "./linked-issue-hard-rules-config";
Expand Down Expand Up @@ -184,3 +185,49 @@ export async function resolveLinkedIssueHardRule(args: {
}
return evaluateLinkedIssueHardRules({ issues: issueFacts, config: args.config, repoOwner: args.repoOwner, prAuthorLogin: args.prAuthorLogin });
}

// ── Stale/fabricated-link countermeasure for the "must link an issue" HARD gate (#unlinked-issue-guardrail-
// followup) ──────────────────────────────────────────────────────────────────────────────────────────────
//
// `pr.linkedIssues` (extractLinkedIssueNumbersWithOverflow) is a pure body-text regex match — it never checks
// whether the cited issue is actually OPEN. So a repo running `linkedIssueGateMode: "block"` (requires a
// linked issue to merge) can be satisfied by a contributor citing an already-CLOSED or fabricated issue
// number, which defeats the whole point of requiring a link. This pair of functions gives the gate a
// verified, fail-open "is at least one citation a real, currently open issue" signal to use INSTEAD of bare
// presence, without changing what `pr.linkedIssues` itself means anywhere else it's used (duplicate-winner
// overlap, label propagation, scoring, etc. all keep reading raw presence).

/**
* PURE evaluator. `true` means "treat the presence check as satisfied" — either a linked issue is CONFIRMED
* open, or at least one fetch was ambiguous (`fetch_error`) and we can't rule out a real open issue behind
* it. `false` — the only case this whole mechanism exists to catch — means EVERY fetched result conclusively
* resolved to NOT an open issue (found-but-closed, or a confirmed 404), with zero ambiguity. An empty input
* (nothing was fetched, e.g. the caller didn't need to check) fails open to `true` — the caller is
* responsible for handling "no linked issues at all" separately (that's the existing bare-presence check).
*/
export function hasVerifiableOpenLinkedIssueReference(fetchResults: LinkedIssueFactsFetch[]): boolean {
if (fetchResults.length === 0) return true;
if (fetchResults.some((result) => result.status === "found" && result.facts.state === "open")) return true;
return fetchResults.some((result) => result.status === "fetch_error");
}

/**
* Orchestrate the live per-issue fetch for {@link hasVerifiableOpenLinkedIssueReference}. Mints its own
* installation token (falling back to the public token, exactly like fetchLinkedIssueFacts's own
* hasProvenAccess discipline degrades a public-token 404 to `fetch_error` rather than a confirmed miss) so
* callers only need an `installationId`, mirroring `resolveLinkedIssueAuthorLogins`'s lazy-token pattern.
* Fail-safe: a token-mint failure still proceeds on the public token rather than skipping the check.
*/
export async function resolveLinkedIssueHasOpenReference(args: {
env: Env;
repoFullName: string;
linkedIssues: number[];
installationId?: number | null | undefined;
}): Promise<boolean> {
if (args.linkedIssues.length === 0) return true;
const ciToken = args.installationId ? await createInstallationToken(args.env, args.installationId).catch(() => undefined) : undefined;
const token = ciToken ?? args.env.GITHUB_PUBLIC_TOKEN;
const admissionKey = githubRateLimitAdmissionKeyForToken(args.env, token, args.installationId);
const fetchResults = await Promise.all(args.linkedIssues.map((issueNumber) => fetchLinkedIssueFacts(args.env, args.repoFullName, issueNumber, token, admissionKey)));
return hasVerifiableOpenLinkedIssueReference(fetchResults);
}
19 changes: 16 additions & 3 deletions src/rules/advisory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,15 @@ export function buildPullRequestAdvisory(
* surface a `self_authored_linked_issue` finding when the PR author also opened the linked issue. Absent
* or empty ⇒ the finding is never raised (fail-open: unknown issue authorship stays advisory-only). */
linkedIssueAuthorLogins?: (string | null | undefined)[];
/** Same-account issue-avoidance countermeasure (#unlinked-issue-guardrail-followup): `pr.linkedIssues` is
* populated by a pure body-text regex that never checks whether the cited issue is actually OPEN, so a
* contributor can satisfy `linkedIssueGateMode: "block"` by citing an already-CLOSED (or fabricated)
* issue number. When the caller has live-verified that NONE of this PR's linked issue numbers resolve to
* a confirmed-open issue, it sets this true and `missing_linked_issue` fires exactly as if nothing were
* linked at all. Absent/false ⇒ byte-identical to today (presence alone still satisfies the requirement)
* — this is fail-open by construction: the caller only ever sets it true after a live check confirms
* every reference is dead, never on ambiguity. */
confirmedNoOpenLinkedIssue?: boolean;
} = {},
): Advisory {
const repoFullName = pr?.repoFullName ?? repo?.fullName ?? "unknown/unknown";
Expand All @@ -215,7 +224,7 @@ export function buildPullRequestAdvisory(
action: "Re-deliver the webhook or wait for the next sync.",
});
} else {
addPullRequestFindings(repo, pr, findings, context.otherOpenPullRequests ?? [], Boolean(context.requireLinkedIssue), Boolean(context.duplicateWinnerEnabled), context.linkedIssueAuthorLogins ?? []);
addPullRequestFindings(repo, pr, findings, context.otherOpenPullRequests ?? [], Boolean(context.requireLinkedIssue), Boolean(context.duplicateWinnerEnabled), context.linkedIssueAuthorLogins ?? [], Boolean(context.confirmedNoOpenLinkedIssue));
}
return advisory("pull_request", targetKey, repoFullName, findings, "Pull request advisory generated.", pr?.number, undefined, pr?.headSha ?? undefined);
}
Expand Down Expand Up @@ -675,6 +684,7 @@ function addPullRequestFindings(
requireLinkedIssue: boolean,
duplicateWinnerEnabled: boolean,
linkedIssueAuthorLogins: (string | null | undefined)[],
confirmedNoOpenLinkedIssue: boolean,
): void {
if (pr.state !== "open") {
findings.push({
Expand All @@ -684,12 +694,15 @@ function addPullRequestFindings(
detail: `The pull request state is ${pr.state}.`,
});
}
if (pr.linkedIssues.length === 0 && requireLinkedIssue) {
const noLinkedIssueCited = pr.linkedIssues.length === 0;
if ((noLinkedIssueCited || confirmedNoOpenLinkedIssue) && requireLinkedIssue) {
findings.push({
code: "missing_linked_issue",
severity: "warning",
title: "No linked issue detected",
detail: "No closing reference or linked issue number was found in the PR metadata/body.",
detail: noLinkedIssueCited
? "No closing reference or linked issue number was found in the PR metadata/body."
: "The PR cites an issue number, but it could not be verified as a currently open issue.",
action: "If this PR is intended to solve an issue, link it explicitly in the PR body.",
});
} else {
Expand Down
Loading
Loading