diff --git a/src/api/routes.ts b/src/api/routes.ts index e8a4e96060..68dfaba75f 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -104,6 +104,7 @@ import { refreshInstallationHealth, refreshInstallationHealthForInstallation, } from "../github/backfill"; +import { getRepositoryCollaboratorPermission } from "../github/app"; import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot } from "../gittensor/api"; import { fetchPublicContributorProfile, fetchPublicRepoStats } from "../github/public"; import { @@ -1820,7 +1821,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 requireRepoMaintainer(c, fullName); + const gate = await requireRepoKeyWriteAccess(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); @@ -1837,7 +1838,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 requireRepoMaintainer(c, fullName); + const gate = await requireRepoKeyWriteAccess(c, fullName); if (gate instanceof Response) return gate; const actor = gate.identity?.kind === "session" ? gate.identity.actor : null; await deleteRepositoryAiKey(c.env, fullName, actor); @@ -3984,6 +3985,39 @@ 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"]); + +/** + * 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. + */ +async function requireRepoKeyWriteAccess(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 + const summary = await loadControlPanelRoleSummary(c.env, gate.identity.actor); + if (summary.roles.includes("operator")) return gate; // operators manage any repo + const repo = await getRepository(c.env, fullName); + const installationId = repo?.installationId ?? null; + let permission: string | null = null; + if (installationId !== null) { + try { + permission = await getRepositoryCollaboratorPermission(c.env, installationId, fullName, gate.identity.actor); + } catch { + /* v8 ignore next -- defensive: a GitHub permission-check failure fails closed (→ 403 below) */ + permission = null; + } + } + if (!permission || !REPO_KEY_WRITE_PERMISSIONS.has(permission)) { + return c.json({ error: "insufficient_repo_permission" }, 403); + } + return gate; +} + async function skippedPrAuditRepoScope( c: ProtectedRouteContext, identity: AuthIdentity, diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index 39f22042b2..a584ef41d3 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/test/unit/routes-ai-byok.test.ts @@ -1,9 +1,18 @@ -import { afterEach, describe, expect, it, vi } 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, upsertInstallation, upsertRepositorySettings, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { getRepositorySettings, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositorySettings, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import { getRepositoryCollaboratorPermission } from "../../src/github/app"; import { createTestEnv } from "../helpers/d1"; +// The secret-key write gate resolves real GitHub push permission via the installation; mock just that +// call (leave the rest of github/app real) so the per-repo write check is deterministic in tests. +vi.mock("../../src/github/app", async (importOriginal) => ({ + ...(await importOriginal()), + getRepositoryCollaboratorPermission: vi.fn(), +})); +const mockedPermission = vi.mocked(getRepositoryCollaboratorPermission); + const SECRET = "routes-byok-encryption-secret-at-least-32b"; const REPO = "acme/widgets"; @@ -106,6 +115,7 @@ describe("maintainer route authz (session-scoped)", () => { // Role resolution (loadControlPanelRoleSummary) makes a miner-detection fetch; stub it so session role // derivation is deterministic in tests. afterEach(() => vi.unstubAllGlobals()); + beforeEach(() => mockedPermission.mockReset()); function stubMinerFetch() { vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { if (input.toString().includes("gittensor.io")) return Response.json([]); @@ -134,17 +144,73 @@ describe("maintainer route authz (session-scoped)", () => { expect(await res.json()).toMatchObject({ aiReviewMode: "advisory", aiReviewProvider: "anthropic" }); }); - it("allows the repo owner via session to set a BYOK key", async () => { + it("allows the repo owner (admin permission) via session to set a BYOK key", async () => { const app = createApp(); const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "" }); await seedRepo(env, "repo-owner", "owned-repo", 201); stubMinerFetch(); + mockedPermission.mockResolvedValue("admin"); // real GitHub write access const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 201 }); const res = await app.request(`${OWNED}/ai-key`, { method: "POST", headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, body: JSON.stringify({ provider: "anthropic", key: "sk-ant-owner-key-4242" }) }, env); expect(res.status).toBe(200); expect(await res.json()).toMatchObject({ configured: true, provider: "anthropic", last4: "4242" }); }); + it("forbids a read-only collaborator (in scope via a PR, but no push) from writing the BYOK key", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "" }); + await seedRepo(env, "repo-owner", "owned-repo", 201); + // "reader" authored a PR as COLLABORATOR → in maintainer scope, but only has read permission. + await upsertPullRequestFromGitHub(env, "repo-owner/owned-repo", { number: 5, title: "tweak", state: "open", user: { login: "reader" }, author_association: "COLLABORATOR", head: { sha: "a1", ref: "f" }, base: { ref: "main" }, labels: [] }); + stubMinerFetch(); + mockedPermission.mockResolvedValue("read"); + const { token } = await createSessionForGitHubUser(env, { login: "reader", id: 777 }); + const json = { cookie: `gittensory_session=${token}`, "content-type": "application/json" }; + const post = await app.request(`${OWNED}/ai-key`, { method: "POST", headers: json, body: JSON.stringify({ provider: "anthropic", key: "sk-ant-reader-key-9999" }) }, env); + expect(post.status).toBe(403); + expect(await post.json()).toMatchObject({ error: "insufficient_repo_permission" }); + // DELETE is gated the same way. + expect((await app.request(`${OWNED}/ai-key`, { method: "DELETE", headers: { cookie: `gittensory_session=${token}` } }, env)).status).toBe(403); + // The read-only collaborator can still READ the secret-free status (GET is not write-gated). + expect((await app.request(`${OWNED}/ai-key`, { headers: { cookie: `gittensory_session=${token}` } }, env)).status).toBe(200); + }); + + it("allows an operator to set the BYOK key without a per-repo push check", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "ops-admin" }); + await seedRepo(env, "repo-owner", "owned-repo", 201); + stubMinerFetch(); + const { token } = await createSessionForGitHubUser(env, { login: "ops-admin", id: 9 }); + const res = await app.request(`${OWNED}/ai-key`, { method: "POST", headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, body: JSON.stringify({ provider: "openai", key: "sk-openai-operator-key-123" }) }, env); + expect(res.status).toBe(200); + expect(mockedPermission).not.toHaveBeenCalled(); // operators skip the push check + }); + + it("fails closed when GitHub reports no write access (permission 'none')", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "" }); + await seedRepo(env, "repo-owner", "owned-repo", 201); + stubMinerFetch(); + mockedPermission.mockResolvedValue("none"); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 201 }); + const res = await app.request(`${OWNED}/ai-key`, { method: "POST", headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, body: JSON.stringify({ provider: "anthropic", key: "sk-ant-owner-key-4242" }) }, env); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ error: "insufficient_repo_permission" }); + }); + + it("fails closed when the repo has no installation to verify permission against", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "" }); + await seedRepo(env, "repo-owner", "owned-repo", 201); + // Keep the repo "installed" (so the owner stays in scope) but drop the installation id. + await env.DB.prepare("UPDATE repositories SET installation_id = NULL WHERE full_name = ?").bind("repo-owner/owned-repo").run(); + stubMinerFetch(); + const { token } = await createSessionForGitHubUser(env, { login: "repo-owner", id: 201 }); + const res = await app.request(`${OWNED}/ai-key`, { method: "POST", headers: { cookie: `gittensory_session=${token}`, "content-type": "application/json" }, body: JSON.stringify({ provider: "anthropic", key: "sk-ant-owner-key-4242" }) }, env); + expect(res.status).toBe(403); + expect(mockedPermission).not.toHaveBeenCalled(); + }); + it("forbids a session with no role for the repo on every AI route (403)", async () => { const app = createApp(); const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, ADMIN_GITHUB_LOGINS: "" });