diff --git a/packages/gittensory-engine/src/settings/command-authorization.ts b/packages/gittensory-engine/src/settings/command-authorization.ts index dbb8c6e250..af9cacd334 100644 --- a/packages/gittensory-engine/src/settings/command-authorization.ts +++ b/packages/gittensory-engine/src/settings/command-authorization.ts @@ -129,15 +129,26 @@ export function evaluateCommandAuthorization(args: { * `pr_author` weren't in the allowed-roles list at all, so a repo that hasn't turned on rate limiting * never grants contributor chat access no matter what `chat`'s configured roles say. */ commandRateLimitPolicy?: "off" | "hold" | undefined; + /** #5092: ALSO required (must be `true`) for a bare `pr_author` match to authorize a command in + * {@link PR_AUTHOR_RATE_LIMITED_COMMANDS} -- the per-PR rate-limit counter (`repoFullName#issueNumber#command`) + * never resets or checks PR state, so without this a contributor could keep a fresh allowance forever by + * reopening/reusing a closed PR or spamming cheap draft PRs. Caller-computed (e.g. `pr.state === "open" && + * !pr.isDraft`) so this function doesn't need to know GitHub's own state-string conventions. Unset/`false` + * denies exactly like a missing rate-limit policy -- maintainers/collaborators are unaffected regardless + * (this bounds the less-trusted pr_author tier, not already-trusted roles). */ + pullRequestOpenAndNotDraft?: boolean | undefined; }): CommandAuthorizationDecision { const allowedRoles = commandAuthorizationAllowedRoles(args.policy, args.commandName); const roles = actorRoles(args); const matchedRole = roles.find((role) => allowedRoles.includes(role)) ?? null; - const prAuthorRateLimitGated = - matchedRole === "pr_author" && - PR_AUTHOR_RATE_LIMITED_COMMANDS.has(normalizeCommandName(args.commandName)) && - args.commandRateLimitPolicy !== "hold"; - if (matchedRole && !prAuthorRateLimitGated) { + const prAuthorGatedCommand = matchedRole === "pr_author" && PR_AUTHOR_RATE_LIMITED_COMMANDS.has(normalizeCommandName(args.commandName)); + if (prAuthorGatedCommand && args.commandRateLimitPolicy !== "hold") { + return { authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null, allowedRoles }; + } + if (prAuthorGatedCommand && args.pullRequestOpenAndNotDraft !== true) { + return { authorized: false, reason: "pr_author_requires_open_pr", actorKind: "author", matchedRole: null, allowedRoles }; + } + if (matchedRole) { return { authorized: true, reason: authorizationReason(matchedRole), @@ -146,9 +157,6 @@ export function evaluateCommandAuthorization(args: { allowedRoles, }; } - if (prAuthorRateLimitGated) { - return { authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null, allowedRoles }; - } const ownPrAuthor = isSameLogin(args.commenterLogin, args.pullRequestAuthorLogin); if (ownPrAuthor && allowedRoles.includes("confirmed_miner")) { return { diff --git a/src/github/commands.ts b/src/github/commands.ts index 701526d9fe..25950b9eb5 100644 --- a/src/github/commands.ts +++ b/src/github/commands.ts @@ -385,6 +385,9 @@ export function isAuthorizedCommandActor(args: { /** #5084: required (must be `"hold"`) for a PR author to be authorized for `chat` -- see * PR_AUTHOR_RATE_LIMITED_COMMANDS in settings/command-authorization.ts. */ commandRateLimitPolicy?: "off" | "hold" | undefined; + /** #5092: ALSO required (must be `true`) for a PR author to be authorized for `chat` -- the caller-computed + * `pr.state === "open" && !pr.isDraft`, since the per-PR rate-limit counter never checks PR state on its own. */ + pullRequestOpenAndNotDraft?: boolean | undefined; }): { authorized: boolean; reason: string; actorKind: "maintainer" | "author" | "none" } { const decision = evaluateCommandAuthorization({ policy: args.commandAuthorizationPolicy, @@ -394,6 +397,7 @@ export function isAuthorizedCommandActor(args: { pullRequestAuthorLogin: args.pullRequestAuthorLogin, minerStatus: args.officialAuthorDetection?.status, commandRateLimitPolicy: args.commandRateLimitPolicy, + pullRequestOpenAndNotDraft: args.pullRequestOpenAndNotDraft, }); return { authorized: decision.authorized, reason: decision.reason, actorKind: decision.actorKind }; } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index eb54607c51..399b861de6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -12561,6 +12561,10 @@ async function maybeProcessGittensoryMentionCommand( officialAuthorDetection: official, commandAuthorizationPolicy: settings.commandAuthorization, commandRateLimitPolicy: settings.commandRateLimitPolicy, + // #5092: the per-PR rate-limit counter below never checks PR state on its own (a closed/merged PR keeps + // its own counter forever; a brand-new PR gets a fresh one) -- without this, a contributor could keep a + // fresh chat allowance indefinitely by reopening/reusing a closed PR or spamming cheap draft PRs. + pullRequestOpenAndNotDraft: cachedPullRequest?.state === "open" && cachedPullRequest?.isDraft !== true, }); if (!authorization.authorized) { await recordAuditEvent(env, { diff --git a/src/settings/command-authorization.ts b/src/settings/command-authorization.ts index 0539401882..f4039ee945 100644 --- a/src/settings/command-authorization.ts +++ b/src/settings/command-authorization.ts @@ -129,15 +129,26 @@ export function evaluateCommandAuthorization(args: { * `pr_author` weren't in the allowed-roles list at all, so a repo that hasn't turned on rate limiting * never grants contributor chat access no matter what `chat`'s configured roles say. */ commandRateLimitPolicy?: "off" | "hold" | undefined; + /** #5092: ALSO required (must be `true`) for a bare `pr_author` match to authorize a command in + * {@link PR_AUTHOR_RATE_LIMITED_COMMANDS} -- the per-PR rate-limit counter (`repoFullName#issueNumber#command`) + * never resets or checks PR state, so without this a contributor could keep a fresh allowance forever by + * reopening/reusing a closed PR or spamming cheap draft PRs. Caller-computed (e.g. `pr.state === "open" && + * !pr.isDraft`) so this function doesn't need to know GitHub's own state-string conventions. Unset/`false` + * denies exactly like a missing rate-limit policy -- maintainers/collaborators are unaffected regardless + * (this bounds the less-trusted pr_author tier, not already-trusted roles). */ + pullRequestOpenAndNotDraft?: boolean | undefined; }): CommandAuthorizationDecision { const allowedRoles = commandAuthorizationAllowedRoles(args.policy, args.commandName); const roles = actorRoles(args); const matchedRole = roles.find((role) => allowedRoles.includes(role)) ?? null; - const prAuthorRateLimitGated = - matchedRole === "pr_author" && - PR_AUTHOR_RATE_LIMITED_COMMANDS.has(normalizeCommandName(args.commandName)) && - args.commandRateLimitPolicy !== "hold"; - if (matchedRole && !prAuthorRateLimitGated) { + const prAuthorGatedCommand = matchedRole === "pr_author" && PR_AUTHOR_RATE_LIMITED_COMMANDS.has(normalizeCommandName(args.commandName)); + if (prAuthorGatedCommand && args.commandRateLimitPolicy !== "hold") { + return { authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null, allowedRoles }; + } + if (prAuthorGatedCommand && args.pullRequestOpenAndNotDraft !== true) { + return { authorized: false, reason: "pr_author_requires_open_pr", actorKind: "author", matchedRole: null, allowedRoles }; + } + if (matchedRole) { return { authorized: true, reason: authorizationReason(matchedRole), @@ -146,9 +157,6 @@ export function evaluateCommandAuthorization(args: { allowedRoles, }; } - if (prAuthorRateLimitGated) { - return { authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null, allowedRoles }; - } const ownPrAuthor = isSameLogin(args.commenterLogin, args.pullRequestAuthorLogin); if (ownPrAuthor && allowedRoles.includes("confirmed_miner")) { return { diff --git a/test/unit/command-authorization-engine.test.ts b/test/unit/command-authorization-engine.test.ts index 3ded817fba..dfcb572321 100644 --- a/test/unit/command-authorization-engine.test.ts +++ b/test/unit/command-authorization-engine.test.ts @@ -195,33 +195,52 @@ describe("repo command authorization policy", () => { it("#5084: a chat pr_author match is only granted when commandRateLimitPolicy is \"hold\" for the repo", () => { // No rate-limit policy passed at all (the undefined branch) -- denied, with a distinct reason from the // generic denials so an operator can tell "rate limiting isn't on" apart from "not authorized at all". + // pullRequestOpenAndNotDraft: true throughout, so this test isolates the rate-limit gate specifically. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null }); // Explicitly "off" (not just unset) -- same denial, covering both falsy branches of the `!== "hold"` check. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "off" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "off", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting" }); // "hold" -- the PR's own author is authorized. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: true, reason: "allowed_pr_author", actorKind: "author", matchedRole: "pr_author" }); // A confirmed miner acting on their OWN PR matches pr_author first (chat's roles list has pr_author, not // confirmed_miner) -- so a miner is gated by the SAME rate-limit requirement as any other PR author, not // the separate confirmed_miner exception "review" gets. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting" }); expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: true, reason: "allowed_pr_author", matchedRole: "pr_author" }); // A commenter on someone ELSE's PR is still denied outright -- pr_author never matches for a non-author, // rate limiting or not. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "other", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "other", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "not_maintainer_or_pr_author" }); }); + it("#5092: a chat pr_author match is ALSO only granted when the PR is open and not draft", () => { + // commandRateLimitPolicy: "hold" throughout, so this test isolates the PR-state gate specifically. The + // per-PR rate-limit counter (repoFullName#issueNumber#command) never checks PR state on its own, so + // without this a contributor could keep a fresh chat allowance forever by reopening/reusing a closed PR + // or spamming cheap draft PRs. + const base = { commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" as const }; + // Unset (the undefined branch) -- denied, distinct reason from the rate-limit denial. + expect(evaluateCommandAuthorization(base)).toMatchObject({ authorized: false, reason: "pr_author_requires_open_pr", actorKind: "author", matchedRole: null }); + // Explicitly false (not just unset) -- same denial, covering both falsy branches of the `!== true` check. + expect(evaluateCommandAuthorization({ ...base, pullRequestOpenAndNotDraft: false })).toMatchObject({ authorized: false, reason: "pr_author_requires_open_pr" }); + // Open and not draft -- authorized. + expect(evaluateCommandAuthorization({ ...base, pullRequestOpenAndNotDraft: true })).toMatchObject({ authorized: true, reason: "allowed_pr_author", matchedRole: "pr_author" }); + // Maintainers/collaborators are completely unaffected by PR state -- the check only bounds the + // less-trusted pr_author tier, never already-trusted roles. + expect(evaluateCommandAuthorization({ commandName: "chat", commenterAssociation: "OWNER" })).toMatchObject({ authorized: true, reason: "maintainer_invocation" }); + expect(evaluateCommandAuthorization({ commandName: "chat", commenterAssociation: "COLLABORATOR" })).toMatchObject({ authorized: true, reason: "collaborator_invocation" }); + }); + it("#5084: a maintainer's yml override restating chat's own default (incl. pr_author) is not clamped away", () => { const restated = normalizeCommandAuthorizationPolicy({ commands: { chat: ["collaborator", "pr_author"] } }); expect(restated.warnings).not.toContain("Ignored author command authorization roles for maintainer-only command: chat."); diff --git a/test/unit/command-authorization.test.ts b/test/unit/command-authorization.test.ts index b27531005c..f016b9b15b 100644 --- a/test/unit/command-authorization.test.ts +++ b/test/unit/command-authorization.test.ts @@ -166,33 +166,52 @@ describe("repo command authorization policy", () => { it("#5084: a chat pr_author match is only granted when commandRateLimitPolicy is \"hold\" for the repo", () => { // No rate-limit policy passed at all (the undefined branch) -- denied, with a distinct reason from the // generic denials so an operator can tell "rate limiting isn't on" apart from "not authorized at all". + // pullRequestOpenAndNotDraft: true throughout, so this test isolates the rate-limit gate specifically. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting", actorKind: "author", matchedRole: null }); // Explicitly "off" (not just unset) -- same denial, covering both falsy branches of the `!== "hold"` check. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "off" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "off", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting" }); // "hold" -- the PR's own author is authorized. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: true, reason: "allowed_pr_author", actorKind: "author", matchedRole: "pr_author" }); // A confirmed miner acting on their OWN PR matches pr_author first (chat's roles list has pr_author, not // confirmed_miner) -- so a miner is gated by the SAME rate-limit requirement as any other PR author, not // the separate confirmed_miner exception "review" gets. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "pr_author_requires_rate_limiting" }); expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "miner", pullRequestAuthorLogin: "miner", minerStatus: "confirmed", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: true, reason: "allowed_pr_author", matchedRole: "pr_author" }); // A commenter on someone ELSE's PR is still denied outright -- pr_author never matches for a non-author, // rate limiting or not. expect( - evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "other", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" }), + evaluateCommandAuthorization({ commandName: "chat", commenterLogin: "other", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold", pullRequestOpenAndNotDraft: true }), ).toMatchObject({ authorized: false, reason: "not_maintainer_or_pr_author" }); }); + it("#5092: a chat pr_author match is ALSO only granted when the PR is open and not draft", () => { + // commandRateLimitPolicy: "hold" throughout, so this test isolates the PR-state gate specifically. The + // per-PR rate-limit counter (repoFullName#issueNumber#command) never checks PR state on its own, so + // without this a contributor could keep a fresh chat allowance forever by reopening/reusing a closed PR + // or spamming cheap draft PRs. + const base = { commandName: "chat", commenterLogin: "author", pullRequestAuthorLogin: "author", commandRateLimitPolicy: "hold" as const }; + // Unset (the undefined branch) -- denied, distinct reason from the rate-limit denial. + expect(evaluateCommandAuthorization(base)).toMatchObject({ authorized: false, reason: "pr_author_requires_open_pr", actorKind: "author", matchedRole: null }); + // Explicitly false (not just unset) -- same denial, covering both falsy branches of the `!== true` check. + expect(evaluateCommandAuthorization({ ...base, pullRequestOpenAndNotDraft: false })).toMatchObject({ authorized: false, reason: "pr_author_requires_open_pr" }); + // Open and not draft -- authorized. + expect(evaluateCommandAuthorization({ ...base, pullRequestOpenAndNotDraft: true })).toMatchObject({ authorized: true, reason: "allowed_pr_author", matchedRole: "pr_author" }); + // Maintainers/collaborators are completely unaffected by PR state -- the check only bounds the + // less-trusted pr_author tier, never already-trusted roles. + expect(evaluateCommandAuthorization({ commandName: "chat", commenterAssociation: "OWNER" })).toMatchObject({ authorized: true, reason: "maintainer_invocation" }); + expect(evaluateCommandAuthorization({ commandName: "chat", commenterAssociation: "COLLABORATOR" })).toMatchObject({ authorized: true, reason: "collaborator_invocation" }); + }); + it("#5084: a maintainer's yml override restating chat's own default (incl. pr_author) is not clamped away", () => { const restated = normalizeCommandAuthorizationPolicy({ commands: { chat: ["collaborator", "pr_author"] } }); expect(restated.warnings).not.toContain("Ignored author command authorization roles for maintainer-only command: chat."); diff --git a/test/unit/github-commands.test.ts b/test/unit/github-commands.test.ts index 2444aa9560..de7cae171d 100644 --- a/test/unit/github-commands.test.ts +++ b/test/unit/github-commands.test.ts @@ -309,7 +309,7 @@ describe("GitHub mention commands", () => { ).toMatchObject({ authorized: false, reason: "not_maintainer_or_pr_author" }); }); - it("#5084: threads commandRateLimitPolicy through to a chat pr_author authorization decision", () => { + it("#5084/#5092: threads commandRateLimitPolicy and pullRequestOpenAndNotDraft through to a chat pr_author authorization decision", () => { expect( isAuthorizedCommandActor({ commandName: "chat", @@ -326,6 +326,16 @@ describe("GitHub mention commands", () => { pullRequestAuthorLogin: "oktofeesh1", commandRateLimitPolicy: "hold", }), + ).toMatchObject({ authorized: false, reason: "pr_author_requires_open_pr" }); + expect( + isAuthorizedCommandActor({ + commandName: "chat", + commenterLogin: "oktofeesh1", + commenterAssociation: "NONE", + pullRequestAuthorLogin: "oktofeesh1", + commandRateLimitPolicy: "hold", + pullRequestOpenAndNotDraft: true, + }), ).toMatchObject({ authorized: true, reason: "allowed_pr_author" }); }); diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index 5f9dcb9bdf..76e3b15759 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -1374,6 +1374,77 @@ describe("queue processors", () => { expect(skipped?.detail).toBe("pr_author_requires_rate_limiting"); }); + it("#5092: a contributor's OWN closed PR does not authorize chat, even with commandRateLimitPolicy: hold (the per-PR counter never resets, so a closed PR must not keep granting access)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI_ADVISORY: { run: async () => ({ response: "The PR is blocked because CI is failing." }) } as unknown as Ai, + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 332, title: "Contributor chat target (closed)", state: "closed", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[] }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("raw.githubusercontent.com") && url.includes(".gittensory.yml")) { + return new Response("settings:\n advisoryAiRouting:\n chatQa: true\n commandRateLimitPolicy: hold\n", { status: 200 }); + } + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "read" }); + if (url.includes("/issues/332/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/332/comments") && method === "POST") { + seen.comments.push(String(JSON.parse(String(init?.body ?? "{}")).body ?? "")); + return Response.json({ id: seen.comments.length }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + const authorPayload = { + action: "created" as const, + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + // A closed PR can still receive comments on GitHub -- the webhook's own issue.state reflects that. + issue: { number: 332, title: "Contributor chat target (closed)", state: "closed", pull_request: {}, user: { login: "oktofeesh1" }, author_association: "NONE" }, + comment: { id: 3, body: "@gittensory chat why is this blocked?", user: { login: "oktofeesh1", type: "User" }, author_association: "NONE" }, + }; + await processJob(env, { type: "github-webhook", deliveryId: "contributor-chat-closed-pr", eventName: "issue_comment", payload: authorPayload }); + expect(seen.comments).toHaveLength(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = 'github_app.agent_command_skipped'").first<{ detail: string }>(); + expect(skipped?.detail).toBe("pr_author_requires_open_pr"); + }); + + it("#5092: a contributor's OWN draft PR does not authorize chat, even with commandRateLimitPolicy: hold (drafts are cheap to open and must not be a quota-farming vector)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI_ADVISORY: { run: async () => ({ response: "The PR is blocked because CI is failing." }) } as unknown as Ai, + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 333, title: "Contributor chat target (draft)", state: "open", draft: true, user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); + const seen = { comments: [] as string[] }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("raw.githubusercontent.com") && url.includes(".gittensory.yml")) { + return new Response("settings:\n advisoryAiRouting:\n chatQa: true\n commandRateLimitPolicy: hold\n", { status: 200 }); + } + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "read" }); + if (url.includes("/issues/333/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/333/comments") && method === "POST") { + seen.comments.push(String(JSON.parse(String(init?.body ?? "{}")).body ?? "")); + return Response.json({ id: seen.comments.length }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + const authorPayload = { + action: "created" as const, + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + issue: { number: 333, title: "Contributor chat target (draft)", state: "open", pull_request: {}, user: { login: "oktofeesh1" }, author_association: "NONE" }, + comment: { id: 4, body: "@gittensory chat why is this blocked?", user: { login: "oktofeesh1", type: "User" }, author_association: "NONE" }, + }; + await processJob(env, { type: "github-webhook", deliveryId: "contributor-chat-draft-pr", eventName: "issue_comment", payload: authorPayload }); + expect(seen.comments).toHaveLength(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = 'github_app.agent_command_skipped'").first<{ detail: string }>(); + expect(skipped?.detail).toBe("pr_author_requires_open_pr"); + }); + it("#5063: posts a FRESH, separate reply comment for each chat invocation (never edits a shared comment), each linking back to its own triggering comment", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),