diff --git a/src/api/routes.ts b/src/api/routes.ts index 8db71ad86c..19a9e5f04f 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -2030,7 +2030,7 @@ export function createApp() { const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; const decision = c.req.param("decision"); if (decision !== "accept" && decision !== "reject") return c.json({ error: "invalid_decision", detail: "decision must be 'accept' or 'reject'" }, 400); - const gate = await requireRepoMaintainer(c, fullName); + const gate = await requireRepoWriteAccess(c, fullName); /* v8 ignore next -- unauthorized requests are rejected by the auth middleware before reaching the handler. */ if (gate instanceof Response) return gate; const pending = await getPendingAgentAction(c.env, c.req.param("id")); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index ec60cea129..8b3bf69ec0 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -43,6 +43,7 @@ import { } from "../db/repositories"; import { buildNotificationFeed } from "../notifications/service"; import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot } from "../gittensor/api"; +import { getRepositoryCollaboratorPermission } from "../github/app"; import { fetchPublicContributorProfile } from "../github/public"; import { listLatestRegistrySnapshots } from "../registry/sync"; import { getOrCreateScoringModelSnapshot, isTimeDecayEnabled } from "../scoring/model"; @@ -336,6 +337,11 @@ const proposeActionShape = { mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(), closeComment: z.string().max(60000).optional(), }; + +// GitHub permissions that imply real write access to a repo. Cached PR author_association can report +// MEMBER/COLLABORATOR for users without push permission, so write-capable MCP surfaces must verify live. +const REPO_WRITE_PERMISSIONS = new Set(["admin", "maintain", "write"]); + const proposeActionOutputSchema = { created: z.boolean().optional(), action: z @@ -1520,8 +1526,20 @@ export class GittensoryMcp { private async requireRepoManageAccess(repoFullName: string): Promise { if (this.identity.kind !== "session") return; const scope = await this.loadSessionAccessScope(); - if (scope.operator || scope.repositoryFullNames.includes(repoFullName)) return; - throw new Error("Forbidden: maintainer access is required to propose an action on this repository."); + if (scope.operator) return; + + const repo = await getRepository(this.env, repoFullName); + const installationId = repo?.installationId ?? null; + let permission: string | null = null; + if (installationId !== null) { + try { + permission = await getRepositoryCollaboratorPermission(this.env, installationId, repoFullName, this.identity.actor); + } catch { + permission = null; + } + } + if (permission && REPO_WRITE_PERMISSIONS.has(permission)) return; + throw new Error("Forbidden: write access is required to propose an action on this repository."); } // Issue-watch gate (#699 path B). Sessions may only watch repos they can SEE: any gittensory-tracked PUBLIC diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 359dbf4837..e039e342c4 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -1,11 +1,23 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { GittensoryMcp } from "../../src/mcp/server"; -import { createPendingAgentActionIfAbsent, listPendingAgentActions, upsertInstallation, upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; +import { getRepositoryCollaboratorPermission } from "../../src/github/app"; +import { createPendingAgentActionIfAbsent, listPendingAgentActions, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; import type { AuthIdentity } from "../../src/auth/security"; import { createTestEnv } from "../helpers/d1"; +vi.mock("../../src/github/app", async (importOriginal) => ({ + ...(await importOriginal()), + getRepositoryCollaboratorPermission: vi.fn(), +})); +const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission); + +beforeEach(() => { + mockedPermission.mockReset(); + mockedPermission.mockResolvedValue("write"); +}); + async function connect(env: Env, identity?: AuthIdentity) { const server = (identity ? new GittensoryMcp(env, identity) : new GittensoryMcp(env)).createServer(); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); @@ -118,13 +130,33 @@ describe("MCP gittensory_propose_action (#784)", () => { expect(JSON.stringify(result)).toMatch(/not installed/i); }); - it("forbids a session without maintainer access to the repo", async () => { + it("forbids a session without live GitHub write access to the repo", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + mockedPermission.mockResolvedValue("read"); const client = await connect(env, { kind: "session", actor: "rando" } as AuthIdentity); const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); expect(result.isError).toBe(true); - expect(JSON.stringify(result)).toMatch(/maintainer access/i); + expect(JSON.stringify(result)).toMatch(/write access/i); + expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0); + }); + + it("does not trust cached collaborator association without live write permission", async () => { + const env = createTestEnv(); + await upsertInstallation(env, { + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write" }, events: ["pull_request"] }, + repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], + }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "x", state: "open", user: { login: "reader" }, author_association: "COLLABORATOR", head: { sha: "sha" } }); + mockedPermission.mockResolvedValue("read"); + + const client = await connect(env, { kind: "session", actor: "reader" } as AuthIdentity); + const result = await client.callTool({ name: "gittensory_propose_action", arguments: { owner: "owner", repo: "repo", pullNumber: 7, actionClass: "merge" } }); + + expect(result.isError).toBe(true); + expect(JSON.stringify(result)).toMatch(/write access/i); + expect(mockedPermission).toHaveBeenCalledWith(env, 5, "owner/repo", "reader"); expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(0); }); });