fix(orb): bind enrollment admin checks to immutable GitHub account ids

migrations/0077_orb_github_installation_account_id.sql

@@ -0,0 +1,3 @@
+-- Store the immutable GitHub account id for the Orb App installation owner. Logins can be renamed and
+-- eventually reused, so OAuth self-enrollment must bind the admin check to this stable id as well.
+ALTER TABLE orb_github_installations ADD COLUMN account_id INTEGER;

src/orb/app-auth.ts

@@ -29,6 +29,7 @@ export interface OrbAppInstallation {
   id: number;
   accountLogin: string | null;
   accountType: string | null;
+  accountId: number | null;
   repositorySelection: string | null;
 }
 
@@ -43,9 +44,9 @@ export async function listOrbAppInstallations(env: Env): Promise<OrbAppInstallat
       const body = await response.text();
       throw new Error(`Failed to list Orb App installations (${response.status}): ${body.slice(0, 200)}`);
     }
-    const rows = (await response.json()) as Array<{ id?: number; account?: { login?: string; type?: string } | null; repository_selection?: string }>;
+    const rows = (await response.json()) as Array<{ id?: number; account?: { login?: string; type?: string; id?: number } | null; repository_selection?: string }>;
     for (const row of rows) {
-      if (row.id) installs.push({ id: row.id, accountLogin: row.account?.login ?? null, accountType: row.account?.type ?? null, repositorySelection: row.repository_selection ?? null });
+      if (row.id) installs.push({ id: row.id, accountLogin: row.account?.login ?? null, accountType: row.account?.type ?? null, accountId: row.account?.id ?? null, repositorySelection: row.repository_selection ?? null });
     }
     if (rows.length < 100) break; // short page → last page
     /* v8 ignore next 2 -- runaway-loop backstop: a single App would need 1000+ installs (>10 pages) to reach this */

src/orb/installations.ts

@@ -17,14 +17,14 @@ export async function upsertOrbInstallation(env: Env, eventName: string, payload
     case "created":
     case "new_permissions_accepted":
       await env.DB.prepare(
-        `INSERT INTO orb_github_installations (installation_id, account_login, account_type, repository_selection, last_event_at)
-         VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP)
+        `INSERT INTO orb_github_installations (installation_id, account_login, account_type, account_id, repository_selection, last_event_at)
+         VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP)
          ON CONFLICT(installation_id) DO UPDATE SET
-           account_login = excluded.account_login, account_type = excluded.account_type,
+           account_login = excluded.account_login, account_type = excluded.account_type, account_id = excluded.account_id,
            repository_selection = excluded.repository_selection,
            suspended_at = NULL, removed_at = NULL, last_event_at = CURRENT_TIMESTAMP`,
       )
-        .bind(inst.id, inst.account?.login ?? null, inst.account?.type ?? null, inst.repository_selection ?? null)
+        .bind(inst.id, inst.account?.login ?? null, inst.account?.type ?? null, inst.account?.id ?? null, inst.repository_selection ?? null)
         .run();
       return;
     case "deleted":
@@ -51,14 +51,14 @@ export async function backfillOrbInstallations(env: Env): Promise<{ backfilled:
   const installs = await listOrbAppInstallations(env);
   for (const inst of installs) {
     await env.DB.prepare(
-      `INSERT INTO orb_github_installations (installation_id, account_login, account_type, repository_selection, last_event_at)
-       VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP)
+      `INSERT INTO orb_github_installations (installation_id, account_login, account_type, account_id, repository_selection, last_event_at)
+       VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP)
        ON CONFLICT(installation_id) DO UPDATE SET
-         account_login = excluded.account_login, account_type = excluded.account_type,
+         account_login = excluded.account_login, account_type = excluded.account_type, account_id = excluded.account_id,
          repository_selection = excluded.repository_selection, suspended_at = NULL, removed_at = NULL,
          last_event_at = CURRENT_TIMESTAMP`,
     )
-      .bind(inst.id, inst.accountLogin, inst.accountType, inst.repositorySelection)
+      .bind(inst.id, inst.accountLogin, inst.accountType, inst.accountId, inst.repositorySelection)
       .run();
   }
   return { backfilled: installs.length };

src/orb/oauth.ts

@@ -16,6 +16,7 @@ import type { Context } from "hono";
 import { isOrbBrokerEnabled, issueOrbEnrollment } from "./broker";
 
 type GitHubUser = { login: string; id?: number };
+type GitHubOrgMembership = { role?: string; state?: string; organization?: { id?: number } };
 
 /** Exchange the OAuth code for the maintainer's access token using the ORB App's OAuth credentials. Null when the
  *  credentials aren't configured or GitHub returns no token. */
@@ -46,34 +47,37 @@ export async function fetchOrbOAuthUser(token: string, fetchImpl: typeof fetch =
 export async function verifyInstallationAdmin(
   token: string,
   userLogin: string,
+  userId: number | null | undefined,
   accountLogin: string | null,
   accountType: string | null,
+  accountId: number | null,
   fetchImpl: typeof fetch = fetch,
 ): Promise<boolean> {
-  if (!accountLogin) return false;
+  if (!accountLogin || accountId === null) return false;
   if (accountType !== "Organization") {
-    return userLogin.toLowerCase() === accountLogin.toLowerCase();
+    return userId === accountId && userLogin.toLowerCase() === accountLogin.toLowerCase();
   }
   const res = await fetchImpl(`https://api.github.com/user/memberships/orgs/${encodeURIComponent(accountLogin)}`, {
     headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json", "user-agent": "gittensory/0.1" },
   });
   if (!res.ok) return false;
-  const body = (await res.json().catch(() => ({}))) as { role?: string; state?: string };
-  return body.state === "active" && body.role === "admin";
+  const body = (await res.json().catch(() => ({}))) as GitHubOrgMembership;
+  return body.state === "active" && body.role === "admin" && body.organization?.id === accountId;
 }
 
 async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise<Response> {
   const token = await exchangeOrbOAuthCode(c.env, code);
   if (!token) return c.html(landingPage("Couldn't verify your GitHub identity", "The authorization didn't complete — re-run the install from GitHub and try again."), 400);
   const user = await fetchOrbOAuthUser(token);
   if (!user) return c.html(landingPage("Couldn't verify your GitHub identity", "We couldn't read your GitHub account — try the install again."), 400);
-  const install = await c.env.DB.prepare("SELECT account_login, account_type, registered, self_enrollment_disabled, suspended_at, removed_at FROM orb_github_installations WHERE installation_id = ?")
+  const install = await c.env.DB.prepare("SELECT account_login, account_type, account_id, registered, self_enrollment_disabled, suspended_at, removed_at FROM orb_github_installations WHERE installation_id = ?")
     .bind(installationId)
-    .first<{ account_login: string | null; account_type: string | null; registered: number; self_enrollment_disabled: number; suspended_at: string | null; removed_at: string | null }>();
+    .first<{ account_login: string | null; account_type: string | null; account_id: number | null; registered: number; self_enrollment_disabled: number; suspended_at: string | null; removed_at: string | null }>();
   if (!install) return c.html(landingPage("Installation not recognized", "We haven't recorded this installation yet — give it a moment after installing, then retry."), 404);
   // The admin-of-installation check is the authorization gate — it runs BEFORE we reveal or change any state, so a
-  // non-admin learns nothing about the install and can never enroll someone else's.
-  const isAdmin = await verifyInstallationAdmin(token, user.login, install.account_login, install.account_type);
+  // non-admin learns nothing about the install and can never enroll someone else's. It binds to the immutable
+  // GitHub account id (logins can be renamed/reused), so a stale account_login can never grant access.
+  const isAdmin = await verifyInstallationAdmin(token, user.login, user.id, install.account_login, install.account_type, install.account_id);
   if (!isAdmin) return c.html(landingPage("Admin access required", "You must be an admin of this installation's account to enroll it for self-host."), 403);
   if (install.removed_at !== null || install.suspended_at !== null) return c.html(landingPage("Installation not active", "This installation is suspended or uninstalled — re-install the Orb App, then retry."), 403);
   if (install.self_enrollment_disabled === 1) return c.html(landingPage("Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403);

test/integration/orb-installations.test.ts

@@ -2,18 +2,18 @@ import { describe, expect, it } from "vitest";
 import { upsertOrbInstallation } from "../../src/orb/installations";
 import { createTestEnv, type TestD1Database } from "../helpers/d1";
 
-const created = (id: number) => ({ action: "created", installation: { id, account: { login: "acme", type: "Organization" }, repository_selection: "selected" } });
+const created = (id: number) => ({ action: "created", installation: { id, account: { login: "acme", type: "Organization", id: 9001 }, repository_selection: "selected" } });
 const get = (e: Env, id: number) =>
   (e.DB as unknown as TestD1Database)
-    .prepare("SELECT account_login, account_type, repository_selection, registered, suspended_at, removed_at FROM orb_github_installations WHERE installation_id=?")
+    .prepare("SELECT account_login, account_type, account_id, repository_selection, registered, suspended_at, removed_at FROM orb_github_installations WHERE installation_id=?")
     .bind(id)
-    .first<{ account_login: string; account_type: string; repository_selection: string; registered: number; suspended_at: string | null; removed_at: string | null }>();
+    .first<{ account_login: string; account_type: string; account_id: number | null; repository_selection: string; registered: number; suspended_at: string | null; removed_at: string | null }>();
 
 describe("upsertOrbInstallation", () => {
   it("'created' registers the install (registered=0 — the manual-onboarding gate)", async () => {
     const e = createTestEnv();
     await upsertOrbInstallation(e, "installation", created(100));
-    expect(await get(e, 100)).toMatchObject({ account_login: "acme", account_type: "Organization", repository_selection: "selected", registered: 0, suspended_at: null, removed_at: null });
+    expect(await get(e, 100)).toMatchObject({ account_login: "acme", account_type: "Organization", account_id: 9001, repository_selection: "selected", registered: 0, suspended_at: null, removed_at: null });
   });
 
   it("'created' with a minimal installation stores null account/type/selection", async () => {
@@ -36,7 +36,7 @@ describe("upsertOrbInstallation", () => {
     await upsertOrbInstallation(e, "installation", created(102));
     await upsertOrbInstallation(e, "installation", { action: "deleted", installation: { id: 102 } });
     expect((await get(e, 102))?.removed_at).not.toBeNull();
-    await upsertOrbInstallation(e, "installation", { action: "new_permissions_accepted", installation: { id: 102, account: { login: "acme", type: "Organization" }, repository_selection: "all" } });
+    await upsertOrbInstallation(e, "installation", { action: "new_permissions_accepted", installation: { id: 102, account: { login: "acme", type: "Organization", id: 9001 }, repository_selection: "all" } });
     const row = await get(e, 102);
     expect(row?.removed_at).toBeNull();
     expect(row?.repository_selection).toBe("all");