From e0c1adf55c213a6554380c888ff76bf393def11f Mon Sep 17 00:00:00 2001 From: galuis116 Date: Sat, 25 Jul 2026 18:55:22 -0700 Subject: [PATCH] fix(orb): authorize feedback votes against the answer's real command policy authorizeFeedbackActor called isAuthorizedCommandActor without commandName or policy context, so every feedback vote was evaluated under the unrelated "preflight" default. A PR author authorized for chat (pr_author + hold rate-limit) was then denied with pr_author_not_confirmed_miner, and custom commandAuthorization overrides were silently ignored. Thread the answer's command name, commandAuthorization, commandRateLimitPolicy, and open/non-draft PR state through, matching every other command-auth call site in processors.ts. Closes #8682 --- src/queue/processors.ts | 19 +++++++ test/unit/queue-4.test.ts | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index f4623c4de7..dd8e979336 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -14580,12 +14580,20 @@ async function maybeProcessAgentCommandFeedbackReaction( deliveryId, }) : undefined; + // #8682: feedback votes must be authorized against the SAME command policy that authorized the answer, + // not isAuthorizedCommandActor's `"preflight"` default. Load live settings so commandAuthorization / + // commandRateLimitPolicy overrides reach the check; PR open/non-draft mirrors the chat call site (#5092). + const settings = await resolveRepositorySettings(env, repoFullName); const authorization = await authorizeFeedbackActor(env, { installationId: getInstallationId(payload), actor, repoFullName, pullRequestAuthor, officialAuthorDetection: official, + commandName: command as LoopOverMentionCommandName, + commandAuthorizationPolicy: settings.commandAuthorization, + commandRateLimitPolicy: settings.commandRateLimitPolicy, + pullRequestOpenAndNotDraft: cachedPullRequest?.state === "open" && cachedPullRequest?.isDraft !== true, }); if (!authorization.authorized) { await recordAuditEvent(env, { @@ -14646,6 +14654,13 @@ async function authorizeFeedbackActor( installationId: number | null; pullRequestAuthor?: string | null | undefined; officialAuthorDetection?: OfficialGittensorMinerDetection | undefined; + // #8682: the command whose answer is being voted on, plus the live policy/rate-limit/PR-state context + // every other isAuthorizedCommandActor call site in this file already threads through. Omitting these + // silently falls back to `"preflight"` and ignores repo commandAuthorization overrides. + commandName?: LoopOverMentionCommandName | undefined; + commandAuthorizationPolicy?: RepositorySettings["commandAuthorization"] | undefined; + commandRateLimitPolicy?: RepositorySettings["commandRateLimitPolicy"] | undefined; + pullRequestOpenAndNotDraft?: boolean | undefined; }, ): Promise<{ authorized: boolean; reason: string; actorKind: "maintainer" | "author" }> { const [owner] = args.repoFullName.split("/"); @@ -14665,10 +14680,14 @@ async function authorizeFeedbackActor( }; } const authorAuthorization = isAuthorizedCommandActor({ + commandName: args.commandName, commenterLogin: args.actor, commenterAssociation: null, pullRequestAuthorLogin: args.pullRequestAuthor, officialAuthorDetection: args.officialAuthorDetection, + commandAuthorizationPolicy: args.commandAuthorizationPolicy, + commandRateLimitPolicy: args.commandRateLimitPolicy, + pullRequestOpenAndNotDraft: args.pullRequestOpenAndNotDraft, }); return { authorized: authorAuthorization.authorized, diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index a9d94eb0ea..85cfa543c5 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -7888,6 +7888,108 @@ describe("queue processors", () => { expect(summary.commands).toEqual([expect.objectContaining({ command: "blockers", usefulnessRate: 1 })]); }); + it("#8682: a non-miner PR author can vote on their own chat answer when commandRateLimitPolicy is hold", async () => { + // Without command context, authorizeFeedbackActor defaults to "preflight" (confirmed_miner in the + // default roles) and denies with pr_author_not_confirmed_miner. Wiring commandName:"chat" + hold + // rate-limit + open non-draft PR must authorize via chat's pr_author role. + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 77, + title: "Contributor chat", + state: "open", + draft: false, + user: { login: "contributor1" }, + author_association: "NONE", + head: { sha: "chatsha" }, + labels: [], + body: "x", + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + settings: { commandRateLimitPolicy: "hold" }, + }); + await upsertAgentCommandAnswer(env, commandAnswer("answer-chat-author", "chat", { responseCommentId: 9101 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "feedback-chat-pr-author", + eventName: "reaction", + payload: { + action: "created", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: 77, title: "Contributor chat", state: "open", pull_request: {}, user: { login: "contributor1" }, author_association: "NONE" }, + comment: { id: 9101, body: commandAnswerBody("answer-chat-author", "chat"), user: { login: "loopover-orb[bot]", type: "Bot" } }, + reaction: { id: 41, content: "+1", user: { login: "contributor1", type: "User" } }, + }, + }); + + const feedbackAudit = await env.DB.prepare( + "select event_type, detail from audit_events where event_type in (?, ?) order by created_at", + ) + .bind("github_app.agent_command_feedback_recorded", "github_app.agent_command_feedback_denied") + .all<{ event_type: string; detail: string | null }>(); + expect(feedbackAudit.results).toEqual([ + expect.objectContaining({ event_type: "github_app.agent_command_feedback_recorded" }), + ]); + expect(feedbackAudit.results.some((row) => row.detail === "pr_author_not_confirmed_miner")).toBe(false); + const stored = await env.DB.prepare("select count(*) as n from github_agent_command_feedback where answer_id = ?").bind("answer-chat-author").first<{ n: number }>(); + expect(stored?.n).toBe(1); + }); + + it("#8682: a custom commandAuthorization override for the answer's command is honored on feedback votes", async () => { + // next-action's shipped default has no pr_author role (falls through to confirmed_miner). A repo that + // explicitly widens next-action to include pr_author must have that override applied to feedback too — + // previously ignored because authorizeFeedbackActor never threaded the policy object. + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 77, + title: "Override feedback", + state: "open", + draft: false, + user: { login: "contributor2" }, + author_association: "NONE", + head: { sha: "ovsha" }, + labels: [], + body: "x", + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + settings: { + commandAuthorization: { + default: ["maintainer", "collaborator", "confirmed_miner"], + commands: { "next-action": ["maintainer", "collaborator", "pr_author"] }, + }, + }, + }); + await upsertAgentCommandAnswer(env, commandAnswer("answer-next-override", "next-action", { responseCommentId: 9102 })); + + await processJob(env, { + type: "github-webhook", + deliveryId: "feedback-next-override", + eventName: "reaction", + payload: { + action: "created", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: 77, title: "Override feedback", state: "open", pull_request: {}, user: { login: "contributor2" }, author_association: "NONE" }, + comment: { id: 9102, body: commandAnswerBody("answer-next-override", "next-action"), user: { login: "loopover-orb[bot]", type: "Bot" } }, + reaction: { id: 42, content: "-1", user: { login: "contributor2", type: "User" } }, + }, + }); + + const feedbackAudit = await env.DB.prepare( + "select event_type, detail from audit_events where event_type in (?, ?) order by created_at", + ) + .bind("github_app.agent_command_feedback_recorded", "github_app.agent_command_feedback_denied") + .all<{ event_type: string; detail: string | null }>(); + expect(feedbackAudit.results).toEqual([ + expect.objectContaining({ event_type: "github_app.agent_command_feedback_recorded" }), + ]); + const stored = await env.DB.prepare("select count(*) as n from github_agent_command_feedback where answer_id = ?") + .bind("answer-next-override") + .first<{ n: number }>(); + expect(stored?.n).toBe(1); + }); + it("rejects copied feedback markers that do not match the stored answer context", async () => { const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "maintainer" }); await upsertAgentCommandAnswer(env, commandAnswer("answer-bound", "preflight", { responseCommentId: 9001 }));