fix(api): require real GitHub write access for BYOK key writes (closes #674)

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<Response | { identity: AuthIdentity | null }> {
+  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,

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<typeof import("../../src/github/app")>()),
+  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: "" });