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
57 changes: 57 additions & 0 deletions src/auth/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,63 @@ export function isAuthorizedGitHubSessionLogin(env: Env, login: string): boolean
return allowedLogins.has(login.toLowerCase());
}

/** #4889 hosted per-repo admin mode. When ON, the global ADMIN_GITHUB_LOGINS allowlist stops granting
* fleet-wide maintainer trust at the review/queue exemption sites — each consults the live per-repo GitHub
* permission instead ({@link isPerTenantAdmin}). OFF (the default) keeps self-host's existing
* global-allowlist behavior byte-identical. Truthy convention matches isOpsEnabled (ops-wire.ts). */
export function isPerRepoAdminModeEnabled(env: { LOOPOVER_PER_REPO_ADMIN?: string | undefined }): boolean {
return /^(1|true|yes|on)$/i.test((env.LOOPOVER_PER_REPO_ADMIN ?? "").trim());
}

/** Injectable seam for {@link isPerTenantAdmin}'s live lookup (tests; production callers omit it). Matches
* getRepositoryCollaboratorPermission's shape (src/github/app.ts). */
export type PerTenantAdminPermissionFetch = (
env: Env,
installationId: number,
repoFullName: string,
login: string,
) => Promise<string | null>;

/**
* #4889: whether `login` holds admin trust for THIS repo — the hosted replacement for a bare
* `parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(login)` at the exemption sites.
*
* - Per-repo admin mode OFF (self-host default): exact ADMIN_GITHUB_LOGINS membership, unchanged semantics.
* - Mode ON (hosted): GitHub's real-time collaborator permission on `repoFullName` — `admin`/`maintain`
* passes, anything else denies. Fail-CLOSED: no installation to ask through, a lookup error, or an
* unknown collaborator all deny — an API blip must never silently grant fleet-operator trust (#4889's
* explicit safety guardrail). The repo-owner shortcut stays at the call sites (it predates and is
* independent of the allowlist this replaces).
*/
export async function isPerTenantAdmin(
env: Env,
installationId: number | null,
repoFullName: string,
login: string,
getPermission?: PerTenantAdminPermissionFetch,
): Promise<boolean> {
const normalized = login.trim().toLowerCase();
if (!normalized) return false;
if (!isPerRepoAdminModeEnabled(env)) return parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(normalized);
if (installationId === null) return false;
let permission: string | null;
try {
const fetchPermission = getPermission ?? (await import("../github/app")).getRepositoryCollaboratorPermission;
permission = await fetchPermission(env, installationId, repoFullName, normalized);
} catch (error) {
console.log(
JSON.stringify({
event: "per_tenant_admin_check_failed",
repoFullName,
login: normalized,
message: error instanceof Error ? error.message.slice(0, 150) : String(error).slice(0, 150),
}),
);
return false;
}
return permission === "admin" || permission === "maintain";
}

/** Parse a GitHub-login allowlist env (e.g. ADMIN_GITHUB_LOGINS) into a lowercased Set. Splits on whitespace OR
* commas so every caller agrees on the same parse (#audit-3.13). */
export function parseGitHubLoginList(value: string | undefined): Set<string> {
Expand Down
5 changes: 5 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,11 @@ declare global {
* byte-identical to today. NOTE: this is read-only OBSERVABILITY only; the auto-tune / config-mutation
* self-improve loop (src/review/auto-apply.ts) is deliberately NOT wired here — see ops-wire.ts. */
LOOPOVER_REVIEW_OPS?: string;
/** #4889 hosted per-repo admin mode: when truthy, the global ADMIN_GITHUB_LOGINS allowlist stops granting
* fleet-wide maintainer trust at the review/queue exemption sites — each consults GitHub's real-time
* per-repo collaborator permission instead (isPerTenantAdmin, src/auth/security.ts), failing CLOSED on
* any API error. Default OFF — unset/false keeps self-host's global-allowlist behavior byte-identical. */
LOOPOVER_PER_REPO_ADMIN?: string;
/** Self-heal: when truthy, an hourly watchdog scans the SAME acting-autonomy repo set the scheduled regate
* sweep covers for a repo whose sweep marker hasn't advanced despite having open PRs to regate, emits a
* structured `sweep_liveness_stale` log (Sentry-visible), and re-enqueues a targeted `agent-regate-sweep`
Expand Down
44 changes: 31 additions & 13 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ import {
preparePrPacketWithAgent,
} from "../services/agent-orchestrator";
import {
isAuthorizedGitHubSessionLogin,
isPerRepoAdminModeEnabled,
isPerTenantAdmin,
parseGitHubLoginList,
} from "../auth/security";
import {
Expand Down Expand Up @@ -2687,7 +2688,8 @@ async function maybeCloseForContributorCapOnOpen(
* repoFullName.split("/")[0] is never undefined for any non-empty repoFullName (every real caller's). */
const repoOwner = repoFullName.split("/")[0] ?? "";
const authorIsOwner = pr.authorLogin.toLowerCase() === repoOwner.toLowerCase();
const authorIsAdmin = parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(pr.authorLogin.toLowerCase());
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
const authorIsAdmin = await isPerTenantAdmin(env, installationId, repoFullName, pr.authorLogin);
const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin);
if (authorIsOwner || authorIsAdmin || authorIsAutomationBot) return false;
// #ignore-authors-parity: a manifest ignore_authors match (e.g. "release-please*") means the bot treats
Expand Down Expand Up @@ -2937,7 +2939,8 @@ async function runAgentMaintenancePlanAndExecute(
// admin login (not the literal repo owner) gets the identical never-auto-closed exemption everywhere.
const authorIsAdmin =
authorLogin.length > 0 &&
parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(authorLogin.toLowerCase());
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
(await isPerTenantAdmin(env, installationId, repoFullName, authorLogin));
const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin);

// Linked-issue HARD-RULE close (#linked-issue-hard-rules): when the repo enabled any rule, a body that links
Expand Down Expand Up @@ -5351,7 +5354,8 @@ async function maybeCloseIssueOverContributorCap(

const repoOwner = repoOwnerLoginFromFullName(repoFullName);
const authorIsOwner = authorLogin.toLowerCase() === repoOwner.toLowerCase();
const authorIsAdmin = parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(authorLogin.toLowerCase());
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
const authorIsAdmin = await isPerTenantAdmin(env, args.installationId, repoFullName, authorLogin);
const authorIsAutomationBot = isProtectedAutomationAuthor(authorLogin);
if (authorIsOwner || authorIsAdmin || authorIsAutomationBot) return;

Expand Down Expand Up @@ -6606,7 +6610,8 @@ async function handleIssueWebhookEvent(
const repoOwner = repoOwnerLoginFromFullName(payload.repository.full_name);
const authorLogin = issue.authorLogin;
const authorIsOwner = authorLogin.toLowerCase() === repoOwner.toLowerCase();
const authorIsAdmin = parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(authorLogin.toLowerCase());
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
const authorIsAdmin = await isPerTenantAdmin(env, installationId, payload.repository.full_name, authorLogin);
const authorIsAutomationBot = isProtectedAutomationAuthor(authorLogin);
const accountAgeThresholdDays = issueSettings.accountAgeThresholdDays;
if (
Expand Down Expand Up @@ -7947,7 +7952,14 @@ async function maybePostVisualFollowupComment(
const visualConfig = await resolveVisualCaptureConfig(env, repoFullName);
if (!visualConfig.bugAnalysis) return;
const { owner } = repoParts(repoFullName);
const notifyLogins = resolveVisualFollowupNotifyLogins(visualConfig.bugAnalysisNotify, owner, parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS));
// #4889: live per-repo permissions cannot be ENUMERATED (the API answers per-login queries only), so in
// per-repo admin mode the notify set carries no allowlist contribution — the repo owner + the repo's own
// configured bugAnalysisNotify list remain the notify surface.
const notifyLogins = resolveVisualFollowupNotifyLogins(
visualConfig.bugAnalysisNotify,
owner,
isPerRepoAdminModeEnabled(env) ? new Set<string>() : parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS),
);
const body = buildVisualFollowupComment(priorAdvisory.findings, notifyLogins);
if (!body) return;
await createOrUpdateVisualFollowupComment(env, installationId, repoFullName, pullNumber, body);
Expand Down Expand Up @@ -9491,7 +9503,8 @@ async function maybePublishPrPublicSurface(
const authorIsExemptFromFreeze =
author !== null &&
(author.toLowerCase() === repoOwnerLoginFromFullName(repoFullName).toLowerCase() ||
parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(author.toLowerCase()) ||
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
(await isPerTenantAdmin(env, installationId, repoFullName, author)) ||
isProtectedAutomationAuthor(author));
const isFrozenForManualReview =
webhook.forceAiReview !== true &&
Expand Down Expand Up @@ -13058,7 +13071,8 @@ async function maybeThrottleReviewNagPing(
// malformed/synthetic payload from ever matching an empty commenter login as "the owner".
const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : "";
if (commenter.toLowerCase() === repoOwner.toLowerCase()) return false;
if (parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(commenter.toLowerCase())) return false;
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
if (await isPerTenantAdmin(env, installationId, repoFullName, commenter)) return false;
// NOTE: no separate isProtectedAutomationAuthor(commenter) check here — every entry in that set (e.g.
// "dependabot[bot]") already ends in "[bot]" and was rejected by the bot-suffix guard above, so it would be
// unreachable dead code at this point (unlike the PR-webhook maintenance path, which checks a PR's stored
Expand Down Expand Up @@ -13244,7 +13258,8 @@ async function maybeThrottleMonitoredMentions(

const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : "";
if (commenter.toLowerCase() === repoOwner.toLowerCase()) return false;
if (parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(commenter.toLowerCase())) return false;
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
if (await isPerTenantAdmin(env, installationId, repoFullName, commenter)) return false;
if (isAutoCloseExempt(commenter, settings.autoCloseExemptLogins)) return false;

const mentionedLogin = monitoredLogins.find((login) => bodyMentionsLogin(body, login));
Expand Down Expand Up @@ -14331,7 +14346,8 @@ async function maybeProcessAgentCommandFeedbackReaction(
deliveryId,
})
: undefined;
const authorization = authorizeFeedbackActor(env, {
const authorization = await authorizeFeedbackActor(env, {
installationId: getInstallationId(payload),
actor,
repoFullName,
pullRequestAuthor,
Expand Down Expand Up @@ -14388,15 +14404,16 @@ function reactionVote(
return null;
}

function authorizeFeedbackActor(
async function authorizeFeedbackActor(
env: Env,
args: {
actor: string;
repoFullName: string;
installationId: number | null;
pullRequestAuthor?: string | null | undefined;
officialAuthorDetection?: OfficialGittensorMinerDetection | undefined;
},
): { authorized: boolean; reason: string; actorKind: "maintainer" | "author" } {
): Promise<{ authorized: boolean; reason: string; actorKind: "maintainer" | "author" }> {
const [owner] = args.repoFullName.split("/");
if (owner && owner.toLowerCase() === args.actor.toLowerCase()) {
return {
Expand All @@ -14405,7 +14422,8 @@ function authorizeFeedbackActor(
actorKind: "maintainer",
};
}
if (isAuthorizedGitHubSessionLogin(env, args.actor)) {
// #4889: per-repo admin mode swaps the global-allowlist operator grant for the live per-repo permission.
if (await isPerTenantAdmin(env, args.installationId, args.repoFullName, args.actor)) {
return {
authorized: true,
reason: "operator_feedback",
Expand Down
15 changes: 11 additions & 4 deletions src/queue/review-evasion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { getRepositoryCollaboratorPermission } from "../github/app";
import { ensurePullRequestLabel } from "../github/labels";
import { fetchPullRequestFreshness, pullRequestFreshnessDetail, type PullRequestFreshness } from "../github/pr-freshness";
import { closePullRequest, createIssueComment, getLastCloserLogin, getLastReopenerLogin, reopenPullRequest } from "../github/pr-actions";
import { parseGitHubLoginList } from "../auth/security";
import { isPerRepoAdminModeEnabled, isPerTenantAdmin, parseGitHubLoginList } from "../auth/security";
import { isAutoCloseExempt } from "../settings/auto-close-exempt";
import { resolveAutonomy } from "../settings/autonomy";
import { isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness } from "../settings/agent-execution";
Expand Down Expand Up @@ -254,7 +254,8 @@ async function closeDraftDodgeAttemptIfBlocked(
// actuation path's trusted-operator definition.
const authorIsAdmin =
draftDodgeAuthorLogin.length > 0 &&
parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(draftDodgeAuthorLogin);
// #4889: per-repo admin mode swaps the global-allowlist grant for the live per-repo permission.
(await isPerTenantAdmin(env, installationId, repoFullName, draftDodgeAuthorLogin));
if (
block &&
block.headSha === pr.headSha &&
Expand Down Expand Up @@ -369,7 +370,11 @@ async function recloseDisallowedReopenIfNeeded(
const repoOwner = repoFullName.includes("/")
? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase()
: "";
const admins = parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS); // unified parse: whitespace OR comma (#audit-3.13)
// #4889: in per-repo admin mode the global allowlist stops granting — the live collaborator check below
// (admin/maintain/write) is the sole permission source; self-host keeps the allowlist shortcut unchanged.
const admins = isPerRepoAdminModeEnabled(env)
? new Set<string>()
: parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS); // unified parse: whitespace OR comma (#audit-3.13)
const hasMaintainerPermission = async (login: string): Promise<boolean> => {
if (login === repoOwner || admins.has(login)) return true;
const permission = await getRepositoryCollaboratorPermission(
Expand Down Expand Up @@ -549,7 +554,9 @@ async function hasMaintainerOrOwnerPermission(env: Env, installationId: number,
// repository match on that exact format before any review-evasion handler runs.
/* v8 ignore next */
const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase() : "";
if (login === repoOwner || parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(login)) return true;
// #4889: in per-repo admin mode the global allowlist stops granting — the live collaborator check below
// is the sole permission source; self-host keeps the allowlist shortcut unchanged.
if (login === repoOwner || (!isPerRepoAdminModeEnabled(env) && parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(login))) return true;
const permission = await getRepositoryCollaboratorPermission(env, installationId, repoFullName, login).catch(() => null);
return permission === "admin" || permission === "maintain" || permission === "write";
}
Expand Down
6 changes: 4 additions & 2 deletions src/review/linked-issue-label-propagation-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from "../github/backfill";
import { createInstallationToken, getRepositoryCollaboratorPermission } from "../github/app";
import { githubRateLimitAdmissionKeyForToken, type GitHubRateLimitAdmissionKey } from "../github/client";
import { parseGitHubLoginList } from "../auth/security";
import { isPerRepoAdminModeEnabled, parseGitHubLoginList } from "../auth/security";
import { errorMessage } from "../utils/json";
import type { LinkedIssueLabelPropagationMapping } from "../types";

Expand Down Expand Up @@ -56,7 +56,9 @@ async function isRepoMaintainerLogin(env: Env, installationId: number, repoFullN
// pattern + rationale in `hasMaintainerOrOwnerPermission`, `src/queue/processors.ts`).
/* v8 ignore next */
const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase() : "";
if (login === repoOwner || parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(login)) return "maintainer";
// #4889: in per-repo admin mode the global allowlist stops granting — the live collaborator check below
// is the sole permission source; self-host keeps the allowlist shortcut unchanged.
if (login === repoOwner || (!isPerRepoAdminModeEnabled(env) && parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(login))) return "maintainer";
let permission: Awaited<ReturnType<typeof getRepositoryCollaboratorPermission>>;
try {
permission = await getRepositoryCollaboratorPermission(env, installationId, repoFullName, login);
Expand Down
Loading