From 289713e18294d8657d0f35269d89d30ba338eefe Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 14 Jun 2026 04:53:50 -0700 Subject: [PATCH] fix(auth): require repo write access for activation --- src/api/routes.ts | 27 ++++---- .../integration/maintainer-activation.test.ts | 69 ++++++++++++++++++- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/src/api/routes.ts b/src/api/routes.ts index 92aa9dfea3..07a3522eb5 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -1818,7 +1818,7 @@ export function createApp() { // mode. Merges onto current settings so unrelated fields are preserved. app.post("/v1/repos/:owner/:repo/activation", async (c) => { const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; - const gate = await requireRepoMaintainer(c, fullName); + const gate = await requireRepoWriteAccess(c, fullName); if (gate instanceof Response) return gate; const current = await getRepositorySettings(c.env, fullName); const updated = await upsertRepositorySettings(c.env, { ...current, ...recommendedAdvisoryActivationSettings() }); @@ -1869,7 +1869,7 @@ export function createApp() { app.post("/v1/repos/:owner/:repo/ai-key", async (c) => { const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; - const gate = await requireRepoKeyWriteAccess(c, fullName); + const gate = await requireRepoWriteAccess(c, fullName); if (gate instanceof Response) return gate; const parsed = repositoryAiKeySchema.safeParse(await c.req.json().catch(() => null)); if (!parsed.success) return c.json({ error: "invalid_ai_key", issues: parsed.error.issues }, 400); @@ -1886,7 +1886,7 @@ export function createApp() { app.delete("/v1/repos/:owner/:repo/ai-key", async (c) => { const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; - const gate = await requireRepoKeyWriteAccess(c, fullName); + const gate = await requireRepoWriteAccess(c, fullName); if (gate instanceof Response) return gate; const actor = gate.identity?.kind === "session" ? gate.identity.actor : null; await deleteRepositoryAiKey(c.env, fullName, actor); @@ -3933,7 +3933,8 @@ function isExtensionScopedSession(identity: AuthIdentity): boolean { // `GittensoryMcp.canAccessRepo` (MCP). Maintainer-of-repo-A grants ZERO access to repo B. // Two maintainer tiers: (a) affiliation (owns/installed the repo, or authored a PR there with a // maintainer association) gates maintainer-DATA reads; (b) verified write/admin/maintain permission, -// resolved live via the installation, additionally gates the SECRET BYOK key writes (`requireRepoKeyWriteAccess`). +// resolved live via the installation, additionally gates repo-visible settings writes and SECRET BYOK key +// writes (`requireRepoWriteAccess`). // Operators (ADMIN_GITHUB_LOGINS) and server-to-server tokens bypass per-repo scope by design. // `canSessionAccessPath` is the coarse path allowlist that runs in the global middleware BEFORE a route // handler; it only decides whether a session may REACH a path — the per-route guards above enforce the @@ -4077,17 +4078,17 @@ async function requireRepoMaintainer(c: ProtectedRouteContext, fullName: string) return { identity }; } -// GitHub permissions that imply real write access to a repo (and thus authority to manage its secret -// BYOK key). "maintain"/"write"/"admin" can push; "triage"/"read"/"none" cannot. -const REPO_KEY_WRITE_PERMISSIONS = new Set(["admin", "maintain", "write"]); +// GitHub permissions that imply real write access to a repo (and thus authority to change repo-visible +// behavior or manage its secret BYOK key). "maintain"/"write"/"admin" can push; "triage"/"read"/"none" cannot. +const REPO_WRITE_PERMISSIONS = new Set(["admin", "maintain", "write"]); /** - * Stricter gate for the secret-bearing BYOK key WRITES (POST/DELETE /ai-key). On top of the maintainer - * gate, a session caller must have real GitHub write access to the repo — resolved via the installation, - * not merely inferred from a PR author_association (which includes org MEMBER / read-only COLLABORATOR). - * Operators and server-to-server tokens are exempt. Fails closed (403) if write access can't be verified. + * Stricter gate for repo-visible settings/secret WRITES. On top of the maintainer gate, a session caller + * must have real GitHub write access to the repo — resolved via the installation, not merely inferred + * from a PR author_association (which includes org MEMBER / read-only COLLABORATOR). Operators and + * server-to-server tokens are exempt. Fails closed (403) if write access can't be verified. */ -async function requireRepoKeyWriteAccess(c: ProtectedRouteContext, fullName: string): Promise { +async function requireRepoWriteAccess(c: ProtectedRouteContext, fullName: string): Promise { const gate = await requireRepoMaintainer(c, fullName); if (gate instanceof Response) return gate; if (gate.identity?.kind !== "session") return gate; // server-to-server token: no per-repo push check @@ -4104,7 +4105,7 @@ async function requireRepoKeyWriteAccess(c: ProtectedRouteContext, fullName: str permission = null; } } - if (!permission || !REPO_KEY_WRITE_PERMISSIONS.has(permission)) { + if (!permission || !REPO_WRITE_PERMISSIONS.has(permission)) { return c.json({ error: "insufficient_repo_permission" }, 403); } return gate; diff --git a/test/integration/maintainer-activation.test.ts b/test/integration/maintainer-activation.test.ts index 5fa312008c..e1f8e8aa00 100644 --- a/test/integration/maintainer-activation.test.ts +++ b/test/integration/maintainer-activation.test.ts @@ -1,14 +1,38 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; import { createSessionForGitHubUser } from "../../src/auth/security"; -import { getRepositorySettings } from "../../src/db/repositories"; +import { getRepositorySettings, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { getRepositoryCollaboratorPermission } from "../../src/github/app"; import { createTestEnv } from "../helpers/d1"; +vi.mock("../../src/github/app", async (importOriginal) => ({ + ...(await importOriginal()), + getRepositoryCollaboratorPermission: vi.fn(), +})); +const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission); + const FULL_NAME = "owner/repo"; const PATH_PREVIEW = "/v1/repos/owner/repo/activation-preview"; const PATH_ACTIVATE = "/v1/repos/owner/repo/activation"; +async function seedRepo(env: Env, owner: string, name: string, installationId: number): Promise { + await upsertInstallation(env, { + installation: { id: installationId, account: { login: owner, id: installationId, type: "User" }, repository_selection: "selected", permissions: { metadata: "read" }, events: ["repository"] }, + }); + await upsertRepositoryFromGitHub(env, { name, full_name: `${owner}/${name}`, private: false, owner: { login: owner } }, installationId); + await env.DB.prepare("UPDATE repositories SET is_registered = 1 WHERE full_name = ?").bind(`${owner}/${name}`).run(); +} + +function stubMinerFetch() { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + if (input.toString().includes("gittensor.io")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); +} + describe("maintainer activation routes", () => { + afterEach(() => vi.unstubAllGlobals()); + beforeEach(() => mockedPermission.mockReset()); it("lets a maintainer preview activation and flip on advisory mode in one action", async () => { const app = createApp(); const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "operator-admin" }); @@ -36,6 +60,47 @@ describe("maintainer activation routes", () => { expect((await afterPreview.json() as { recommendedAction: string | null }).recommendedAction).toBeNull(); }); + + it("forbids read-only repo collaborators from activating advisory checks", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRepo(env, "owner", "repo", 201); + await upsertPullRequestFromGitHub(env, FULL_NAME, { + number: 7, + title: "docs tweak", + state: "open", + user: { login: "reader" }, + author_association: "COLLABORATOR", + head: { sha: "abc123", ref: "docs" }, + base: { ref: "main" }, + labels: [], + }); + stubMinerFetch(); + mockedPermission.mockResolvedValue("read"); + const { token } = await createSessionForGitHubUser(env, { login: "reader", id: 777 }); + const headers = { cookie: `gittensory_session=${token}`, "content-type": "application/json" }; + + const preview = await app.request(PATH_PREVIEW, { headers }, env); + expect(preview.status).toBe(200); + + const activate = await app.request(PATH_ACTIVATE, { method: "POST", headers, body: "{}" }, env); + expect(activate.status).toBe(403); + expect(await activate.json()).toMatchObject({ error: "insufficient_repo_permission" }); + expect((await getRepositorySettings(env, FULL_NAME)).gateCheckMode).toBe("off"); + }); + + it("allows a session with GitHub write permission to activate advisory checks", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "" }); + await seedRepo(env, "owner", "repo", 201); + stubMinerFetch(); + mockedPermission.mockResolvedValue("write"); + const { token } = await createSessionForGitHubUser(env, { login: "owner", id: 201 }); + const response = await app.request(PATH_ACTIVATE, { method: "POST", headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, body: "{}" }, env); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ repoFullName: FULL_NAME, gateCheckMode: "enabled" }); + }); + it("forbids a non-maintainer session from the activation preview", async () => { const app = createApp(); const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "operator-admin" });