From b4e9f698aa259a051ed8b5c8c6f1c6438395145a Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sat, 11 Jul 2026 08:07:59 -0700 Subject: [PATCH] fix(commands): authorize intent routing before AI classification Requires the commenter be authorized for at least one INTENT_ROUTABLE_COMMANDS entry before calling classifyGittensoryIntent (a cost-bearing local-AI call), and changes maybeThrottleIntentRouting to treat commandRateLimitPolicy === "off" as throttled (blocking) instead of not-throttled (previously allowing unmetered spend when a repo hasn't configured rate limiting at all). Adds regression coverage for the policy-off fail path, the confirmed-miner own-PR authorization path, and the pullRequestAuthor null fallback when neither the cached PR row nor the webhook issue carry a user login. --- src/queue/processors.ts | 70 +++++++++++++------ test/unit/queue-5.test.ts | 139 +++++++++++++++++++++++++++++++++++++- 2 files changed, 188 insertions(+), 21 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 57ffe9ec3..0be0623a4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -150,6 +150,7 @@ import { import { buildMaintainerQueueDigest, buildPublicAgentCommandComment, + INTENT_ROUTABLE_COMMANDS, type GittensoryMentionCommandName, isAiCostBearingCommand, isAuthorizedCommandActor, @@ -12317,10 +12318,10 @@ const INTENT_ROUTING_RATE_LIMIT_EVENT_TYPE = "github_app.intent_routing_invocati * Dedicated rate limit for the intent-classification router (#4596): every unrecognized-verb mention with * non-trivial trailing text that reaches the classifier consumes ONE tick here, using the SAME AI-cost-bearing * ceiling (`commandRateLimitAiMaxPerWindow`) and "off"/"hold" policy switch as every other AI-cost-bearing - * command -- kept as its OWN counter (not folded into any single command's bucket via + * command, with hold policy required before the cost-bearing classifier can run -- kept as its OWN counter (not folded into any single command's bucket via * `maybeThrottleGittensoryCommand`) because an unrecognized-verb mention isn't attributable to any one command * until AFTER classification runs, and a "no match" classification must still count for budget-ledger - * consistency (req 5) even though it never becomes a real command dispatch. Fails OPEN on any throttle: this + * consistency (req 5) even though it never becomes a real command dispatch. Fails OPEN when policy is off or any throttle trips: this * only ever skips the classifier call itself, never blocks the existing did-you-mean fallback it would * otherwise replace -- a contributor still gets a reply either way. */ @@ -12336,7 +12337,7 @@ async function maybeThrottleIntentRouting( ): Promise { /* v8 ignore next -- resolveRepositorySettings always resolves a concrete "off"/"hold"; the undefined side is defensive against the field's optional TS type. */ const policy = args.settings.commandRateLimitPolicy ?? "off"; - if (policy === "off") return false; + if (policy === "off") return true; const targetKey = `${args.repoFullName}#${args.issueNumber}#intent-routing`; const redeliverySinceIso = new Date(Date.now() - COMMAND_RATE_LIMIT_REDELIVERY_WINDOW_MS).toISOString(); @@ -12490,15 +12491,50 @@ async function maybeProcessGittensoryMentionCommand( ), ]); + const pullRequestAuthor = + cachedPullRequest?.authorLogin ?? issue.user?.login ?? null; + // Intent-classification router (#4596): an unrecognized-verb mention with real trailing text (e.g. "why is - // this stuck?") gets ONE chance to be re-routed to an existing Q&A command BEFORE authorization/rate-limit/ - // dispatch run, so the rest of this handler proceeds completely normally for whatever it resolves to -- the - // matched command's OWN authorization, rate limit, and rendering all apply unchanged, exactly as if the - // contributor had typed the exact verb. A no-match (or anything not enabled/available) leaves `command` - // untouched and the existing did-you-mean fallback renders exactly as it always has. + // this stuck?") gets ONE chance to be re-routed to an existing Q&A command. Because the classifier is a + // cost-bearing local-AI call, it is only attempted after the actor is authorized for at least one command the + // router is allowed to pick and after the dedicated hold-policy throttle admits the request. A no-match (or + // anything not enabled/available/authorized/throttled) leaves `command` untouched and the existing did-you-mean + // fallback renders exactly as it always has. + const needsIntentMinerDetection = + command.name === "help" && + command.unrecognizedText && + settings.advisoryAiRouting?.intentRouting === true && + INTENT_ROUTABLE_COMMANDS.some((commandName) => + commandAuthorizationNeedsMinerDetection({ + policy: settings.commandAuthorization, + commandName, + commenterLogin: commenter, + commenterAssociation, + pullRequestAuthorLogin: pullRequestAuthor, + }), + ); + let official = + pullRequestAuthor && needsIntentMinerDetection + ? await getCachedOfficialMinerDetection(env, pullRequestAuthor, { + targetKey: `${repoFullName}#${issue.number}`, + deliveryId, + }) + : undefined; let interpretedFrom: { question: string; matchedCommand: GittensoryMentionCommandName } | undefined; if (command.name === "help" && command.unrecognizedText && settings.advisoryAiRouting?.intentRouting === true) { - const throttled = await maybeThrottleIntentRouting(env, { deliveryId, repoFullName, issueNumber: issue.number, commenter, settings }); + const authorizedForIntentRouting = INTENT_ROUTABLE_COMMANDS.some((commandName) => + isAuthorizedCommandActor({ + commandName, + commenterLogin: commenter, + commenterAssociation, + pullRequestAuthorLogin: pullRequestAuthor, + officialAuthorDetection: official, + commandAuthorizationPolicy: settings.commandAuthorization, + }).authorized, + ); + const throttled = authorizedForIntentRouting + ? await maybeThrottleIntentRouting(env, { deliveryId, repoFullName, issueNumber: issue.number, commenter, settings }) + : true; if (!throttled) { const classification = await classifyGittensoryIntent(env, { text: command.unrecognizedText, @@ -12533,8 +12569,6 @@ async function maybeProcessGittensoryMentionCommand( agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun, }); - const pullRequestAuthor = - cachedPullRequest?.authorLogin ?? issue.user?.login ?? null; const needsMinerDetection = commandAuthorizationNeedsMinerDetection({ policy: settings.commandAuthorization, commandName: command.name, @@ -12542,14 +12576,12 @@ async function maybeProcessGittensoryMentionCommand( commenterAssociation, pullRequestAuthorLogin: pullRequestAuthor, }); - const official = - pullRequestAuthor && - (needsMinerDetection || command.name === "miner-context") - ? await getCachedOfficialMinerDetection(env, pullRequestAuthor, { - targetKey: `${repoFullName}#${issue.number}`, - deliveryId, - }) - : undefined; + if (pullRequestAuthor && !official && (needsMinerDetection || command.name === "miner-context")) { + official = await getCachedOfficialMinerDetection(env, pullRequestAuthor, { + targetKey: `${repoFullName}#${issue.number}`, + deliveryId, + }); + } const authorization = isAuthorizedCommandActor({ commandName: command.name, commenterLogin: commenter, diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index 76e3b1575..f465cbaf0 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -1144,13 +1144,13 @@ describe("queue processors", () => { }); } - function mentionPayload(issueNumber: number, body: string) { + function mentionPayload(issueNumber: number, body: string, options: { commenter?: string } = {}) { return { 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: issueNumber, title: "Rate limit target", state: "open", pull_request: {}, user: { login: "oktofeesh1" }, author_association: "NONE" }, - comment: { id: 1, body, user: { login: "maintainer", type: "User" }, author_association: "OWNER" }, + comment: { id: issueNumber, body, user: { login: options.commenter ?? "maintainer", type: "User" }, author_association: "OWNER" }, }; } @@ -1605,6 +1605,7 @@ describe("queue processors", () => { GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI_ADVISORY: { run: async () => ({ response: '{"command": "blockers"}' }) } as unknown as Ai, }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 309, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); const seen = { comments: [] as string[] }; // advisoryAiRouting is config-as-code only -- enable intentRouting the real way, through the repo's @@ -1637,6 +1638,7 @@ describe("queue processors", () => { GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI_ADVISORY: { run: async () => ({ response: '{"command": "ask"}' }) } as unknown as Ai, }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 315, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); const seen = { comments: [] as string[] }; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -1679,11 +1681,47 @@ describe("queue processors", () => { expect(seen.comments[0]).not.toContain("Gittensory readiness blockers"); }); + it("#4596: SECURITY: unauthorized commenters cannot spend intent-routing AI before command authorization", async () => { + const advisoryRun = vi.fn(async () => ({ response: '{"command": "blockers"}' })); + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI_ADVISORY: { run: advisoryRun } as unknown as Ai, + }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 316, title: "Unauthorized intent routing", state: "open", 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 intentRouting: true\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: "none" }); + if (url.includes("/issues/316/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 }); + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "intent-routing-unauthorized-no-ai", + eventName: "issue_comment", + payload: mentionPayload(316, "@gittensory why is this stuck?", { commenter: "driveby" }), + }); + expect(advisoryRun).not.toHaveBeenCalled(); + expect(seen.comments).toHaveLength(0); + const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.intent_routing_invocation'").first<{ n: number }>(); + expect(invocations?.n).toBe(0); + }); + it("#4596: falls through to the existing did-you-mean hint end-to-end when intentRouting is on but the classifier finds no confident match", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI_ADVISORY: { run: async () => ({ response: '{"command": null}' }) } as unknown as Ai, }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 311, title: "Rate limit target", state: "open", user: { login: "oktofeesh1" }, author_association: "NONE", labels: [], body: "" }); const seen = { comments: [] as string[] }; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -1802,6 +1840,103 @@ describe("queue processors", () => { const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.intent_routing_invocation'").first<{ n: number }>(); expect(invocations?.n).toBe(1); // only ONE invocation recorded despite two processing passes }); + + it("#5107: REGRESSION: leaves the classifier unmetered-spend-blocked when commandRateLimitPolicy is left at its off default", async () => { + const advisoryRun = vi.fn(async () => ({ response: '{"command": "blockers"}' })); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI_ADVISORY: { run: advisoryRun } as unknown as Ai }); + // Deliberately NO upsertRepositorySettings commandRateLimitPolicy override -- stays at its "off" default. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 317, title: "Rate limit target", state: "open", 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 intentRouting: true\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: "maintain" }); + if (url.includes("/issues/317/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/317/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 }); + }); + await processJob(env, { type: "github-webhook", deliveryId: "intent-routing-policy-off", eventName: "issue_comment", payload: mentionPayload(317, "@gittensory why is this stuck?") }); + expect(advisoryRun).not.toHaveBeenCalled(); // an unconfigured rate limit must not allow unmetered AI spend + expect(seen.comments).toHaveLength(1); // fails open to the plain did-you-mean fallback + expect(seen.comments[0]).not.toContain("Interpreted"); + const invocations = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'github_app.intent_routing_invocation'").first<{ n: number }>(); + expect(invocations?.n).toBe(0); // short-circuited at the policy gate, before the counter is even touched + }); + + it("#5107: a confirmed Gittensor miner is authorized for intent-routing on their OWN PR (not a maintainer/collaborator)", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI_ADVISORY: { run: async () => ({ response: '{"command": "blockers"}' }) } as unknown as Ai, + }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", commandRateLimitPolicy: "hold", commandRateLimitAiMaxPerWindow: 5, commandRateLimitWindowHours: 24 }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 319, title: "Rate limit target", state: "open", user: { login: "own-pr-miner" }, author_association: "NONE", labels: [], body: "" }); + await upsertOfficialMinerDetection(env, "own-pr-miner", { status: "confirmed", snapshot: queueMinerSnapshot("own-pr-miner") }, 60_000); + 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 intentRouting: true\n", { status: 200 }); + } + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + // No repo permission at all -- the ONLY route to authorization is the confirmed_miner role on their own PR. + if (url.includes("/collaborators/") && url.includes("/permission")) return new Response("not found", { status: 404 }); + if (url.includes("/issues/319/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/319/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 }); + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "intent-routing-confirmed-miner-own-pr", + eventName: "issue_comment", + payload: mentionPayload(319, "@gittensory why is this stuck?", { commenter: "own-pr-miner" }), + }); + expect(seen.comments).toHaveLength(1); + expect(seen.comments[0]).toContain('Interpreted "why is this stuck?" as `@gittensory blockers`'); + }); + }); + + it("#5107: resolves pullRequestAuthor to null, without breaking command dispatch, when neither the cached PR row nor the webhook issue carry a user login", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // Deliberately no upsertPullRequestFromGitHub call -- the cached PR lookup stays null/uncached. + 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("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "maintain" }); + if (url.includes("/issues/318/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/318/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 }); + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "pull-request-author-null-fallback", + eventName: "issue_comment", + 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" } }, + // Deliberately no `user` field on the issue -- combined with no cached PR row, this exercises the + // pullRequestAuthor `cachedPullRequest?.authorLogin ?? issue.user?.login ?? null` fallback's null arm. + issue: { number: 318, title: "No author anywhere", state: "open", pull_request: {}, author_association: "NONE" }, + comment: { id: 318, body: "@gittensory help", user: { login: "maintainer", type: "User" }, author_association: "OWNER" }, + }, + }); + expect(seen.comments).toHaveLength(1); // command dispatch still completes normally }); it("denies a maintainer Q&A command from an org member without real repo permission (#788)", async () => {