diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 64d1010b8c..8d13bef8db 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1542,14 +1542,16 @@ async function maybeProcessGateOverrideCommand(env: Env, deliveryId: string, pay return true; } - const actorAssociation = await resolveRealRepoPermissionAssociation(env, installationId, repoFullName, actor); - const pullRequestAuthor = pr.authorLogin ?? issue.user?.login ?? null; - const authorization = isAuthorizedCommandActor({ + const { authorization } = await authorizePrActionActor({ + env, + deliveryId, + installationId, + repoFullName, + issue, + actor, commandName: "gate-override" as GittensoryMentionCommandName, - commenterLogin: actor, - commenterAssociation: actorAssociation, - pullRequestAuthorLogin: pullRequestAuthor, - commandAuthorizationPolicy: settings.commandAuthorization, + settings, + pr, }); if (!authorization.authorized) { await recordAuditEvent(env, { @@ -1570,11 +1572,7 @@ async function maybeProcessGateOverrideCommand(env: Env, deliveryId: string, pay return true; } - const [repo, otherOpenPullRequests] = await Promise.all([getRepository(env, repoFullName), listOtherOpenPullRequests(env, repoFullName, pr.number)]); - const advisory = buildPullRequestAdvisory(repo, pr, { - otherOpenPullRequests, - requireLinkedIssue: settings.requireLinkedIssue || settings.linkedIssueGateMode !== "off", - }); + const { advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, pr, settings); const safeReason = sanitizePublicComment((command.reason ?? "").trim() || "No reason provided."); await createOrUpdateOverriddenGateCheckRun(env, installationId, repoFullName, advisory, { actor, reason: safeReason }); await recordAuditEvent(env, { @@ -1663,23 +1661,17 @@ async function maybeProcessPrPanelRetrigger(env: Env, deliveryId: string, payloa return true; } - const actorAssociation = await resolveRealRepoPermissionAssociation(env, installationId, repoFullName, actor); - const pullRequestAuthor = pr.authorLogin ?? issue.user?.login ?? null; - const needsMinerDetection = commandAuthorizationNeedsMinerDetection({ - policy: settings.commandAuthorization, - commandName: "review-now", - commenterLogin: actor, - commenterAssociation: actorAssociation, - pullRequestAuthorLogin: pullRequestAuthor, - }); - const official = pullRequestAuthor && needsMinerDetection ? await getCachedOfficialMinerDetection(env, pullRequestAuthor, { targetKey: `${repoFullName}#${issue.number}`, deliveryId }) : undefined; - const authorization = isAuthorizedCommandActor({ + const { authorization } = await authorizePrActionActor({ + env, + deliveryId, + installationId, + repoFullName, + issue, + actor, commandName: "review-now", - commenterLogin: actor, - commenterAssociation: actorAssociation, - pullRequestAuthorLogin: pullRequestAuthor, - officialAuthorDetection: official, - commandAuthorizationPolicy: settings.commandAuthorization, + settings, + pr, + needsMinerDetection: true, }); if (!authorization.authorized) { await recordPrPanelRetriggerSkip(env, deliveryId, repoFullName, `${repoFullName}#${pr.number}`, actor, authorization.reason); @@ -1693,14 +1685,7 @@ async function maybeProcessPrPanelRetrigger(env: Env, deliveryId: string, payloa return true; } - const [repo, otherOpenPullRequests] = await Promise.all([ - getRepository(env, repoFullName), - listOtherOpenPullRequests(env, repoFullName, pr.number), - ]); - const advisory = buildPullRequestAdvisory(repo, pr, { - otherOpenPullRequests, - requireLinkedIssue: settings.requireLinkedIssue || settings.linkedIssueGateMode !== "off", - }); + const { repo, advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, pr, settings); await persistAdvisory(env, advisory); await recordAuditEvent(env, { eventType: "github_app.pr_panel_retriggered", @@ -1731,6 +1716,65 @@ async function resolveRealRepoPermissionAssociation(env: Env, installationId: nu return null; } +// #824 the SINGLE real-permission authorization gate for @gittensory action commands (gate-override, the +// PR-panel retrigger, and the agent-layer write actions to come in #778/#769). It resolves the actor's REAL +// repo permission via resolveRealRepoPermissionAssociation — never the spoofable author_association (the #788 +// hazard) — then runs isAuthorizedCommandActor. Every action command authorizes through here, so no future +// command can accidentally fall back to a weaker check. Returns the decision; the caller owns the +// command-specific deny/allow handling. +async function authorizePrActionActor(args: { + env: Env; + deliveryId: string; + installationId: number; + repoFullName: string; + issue: NonNullable; + actor: string | null; + commandName: GittensoryMentionCommandName; + settings: RepositorySettings; + pr: PullRequestRecord; + needsMinerDetection?: boolean; +}): Promise<{ authorization: ReturnType; actorAssociation: string | null; pullRequestAuthor: string | null }> { + const actorAssociation = await resolveRealRepoPermissionAssociation(args.env, args.installationId, args.repoFullName, args.actor); + const pullRequestAuthor = args.pr.authorLogin ?? args.issue.user?.login ?? null; + const official = + args.needsMinerDetection && + pullRequestAuthor && + commandAuthorizationNeedsMinerDetection({ + policy: args.settings.commandAuthorization, + commandName: args.commandName, + commenterLogin: args.actor, + commenterAssociation: actorAssociation, + pullRequestAuthorLogin: pullRequestAuthor, + }) + ? await getCachedOfficialMinerDetection(args.env, pullRequestAuthor, { targetKey: `${args.repoFullName}#${args.issue.number}`, deliveryId: args.deliveryId }) + : undefined; + const authorization = isAuthorizedCommandActor({ + commandName: args.commandName, + commenterLogin: args.actor, + commenterAssociation: actorAssociation, + pullRequestAuthorLogin: pullRequestAuthor, + officialAuthorDetection: official, + commandAuthorizationPolicy: args.settings.commandAuthorization, + }); + return { authorization, actorAssociation, pullRequestAuthor }; +} + +// #824 the common "load the PR's repo context + build its advisory" step every authorized action command runs +// before its mutation. Identical across gate-override and the PR-panel retrigger. +async function buildAuthorizedPrActionAdvisory( + env: Env, + repoFullName: string, + pr: PullRequestRecord, + settings: RepositorySettings, +): Promise<{ repo: Awaited>; advisory: ReturnType }> { + const [repo, otherOpenPullRequests] = await Promise.all([getRepository(env, repoFullName), listOtherOpenPullRequests(env, repoFullName, pr.number)]); + const advisory = buildPullRequestAdvisory(repo, pr, { + otherOpenPullRequests, + requireLinkedIssue: settings.requireLinkedIssue || settings.linkedIssueGateMode !== "off", + }); + return { repo, advisory }; +} + function isCheckedPrPanelRetrigger(body: string | null | undefined): boolean { if (!body?.includes(PR_PANEL_COMMENT_MARKER) || !body.includes(PR_PANEL_RETRIGGER_MARKER)) return false; return checkedMarkerRegex(PR_PANEL_RETRIGGER_MARKER).test(body); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 4a3c9730e0..d35edc2a7f 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1487,6 +1487,82 @@ describe("queue processors", () => { expect(usageEvents).toEqual(expect.arrayContaining([expect.objectContaining({ surface: "github_app", eventName: "pr_panel_retriggered", outcome: "completed" })])); }); + it("reruns the panel when a confirmed-miner PR author checks the rerun task (#824 miner-detection path)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "off", + includeMaintainerAuthors: true, + // review-now allows a confirmed miner, so a confirmed-miner PR author can retrigger their own panel. + commandAuthorization: { default: ["maintainer", "collaborator", "confirmed_miner"], commands: { "review-now": ["maintainer", "confirmed_miner"] } }, + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 48, + title: "Miner self-rerun", + state: "open", + user: { login: "contributor" }, + author_association: "CONTRIBUTOR", + head: { sha: "panel480" }, + labels: [], + body: "Validation: npm test", + }); + const checkedPanel = ["", "", "- [x] Re-run Gittensory review"].join("\n"); + const calls = { minerList: 0, permission: 0, commentPatches: 0 }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") { + calls.minerList += 1; + return Response.json([{ uid: 7, githubUsername: "contributor", githubId: "123", totalPrs: 4, totalMergedPrs: 3, totalOpenPrs: 1, totalClosedPrs: 0, totalOpenIssues: 0, totalClosedIssues: 0, totalSolvedIssues: 0, totalValidSolvedIssues: 0, isEligible: true, credibility: 1, eligibleRepoCount: 1 }]); + } + if (url === "https://api.gittensor.io/miners/123") return Response.json({ repositories: [{ repositoryFullName: "JSONbored/gittensory", totalPrs: "4", totalMergedPrs: "3", totalOpenPrs: "1", totalClosedPrs: "0", totalOpenIssues: "0", totalClosedIssues: "0", isEligible: true, credibility: "1.000000" }] }); + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + if (url.endsWith("/users/contributor")) return Response.json({ login: "contributor", public_repos: 2, followers: 1 }); + if (url.includes("/users/contributor/repos")) return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + // The confirmed-miner author has NO repo write/admin — authorized via confirmed_miner, not maintainer. + if (url.includes("/collaborators/contributor/permission")) { + calls.permission += 1; + return Response.json({ permission: "none" }); + } + if (url.includes("/issues/48/comments") && method === "GET") return Response.json([{ id: 778, body: checkedPanel, user: { login: "gittensory[bot]", type: "Bot" } }]); + if (url.includes("/issues/comments/778") && method === "PATCH") { + calls.commentPatches += 1; + return Response.json({ id: 778 }); + } + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "panel-retrigger-miner", + eventName: "issue_comment", + payload: { + action: "edited", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: 48, title: "Miner self-rerun", state: "open", user: { login: "contributor" }, pull_request: {} }, + comment: { id: 778, body: checkedPanel, user: { login: "gittensory[bot]", type: "Bot" } }, + sender: { login: "contributor", type: "User" }, + }, + }); + + // The confirmed-miner detection WAS fetched (the #824 helper's miner-detection path) and the panel retriggered. + expect(calls.minerList).toBeGreaterThanOrEqual(1); + expect(calls.permission).toBe(1); + expect(calls.commentPatches).toBe(1); + const audit = await env.DB.prepare("select actor, outcome from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_panel_retriggered", "JSONbored/gittensory#48") + .first<{ actor: string; outcome: string }>(); + expect(audit).toMatchObject({ actor: "contributor", outcome: "completed" }); + }); + it("skips PR panel reruns from users without repository write permission", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123);