From 5d96933682844f00cf36d3bbcd461c284aee6e98 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 25 Jun 2026 02:55:03 -0700 Subject: [PATCH] feat(orb): maintainer OAuth self-enrollment (install-admin verified) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let a maintainer self-issue their brokered enrollment secret via the Orb App's OAuth callback — without the operator manually issuing it — while CLOSING the privilege-escalation hole that made the operator-issued path the only option. GitHub redirects to /v1/orb/oauth/callback with an OAuth code + installation_id; the handler now: exchanges the code (Orb App credentials) → identifies the user (GET /user) → VERIFIES the user is an admin of the installation's account → checks registered=1 → issues a one-time secret, recording the maintainer identity. The admin check is the gate: installation_id is an attacker-controllable query param, so a stolen code + a victim's installation_id must never enroll the victim's install. For an Org install the user must be an ACTIVE org ADMIN (their own /user/memberships/orgs role, requires the read:org scope); for a User install they must be the account owner. installation_id is bound server-side in the enrollment and read back (never from a request) at token-exchange time. No request input is echoed into the markup; the secret is shown once and never logged. issueOrbEnrollment now records the maintainer login + github id (the orb_enrollments columns already existed; no migration). The operator-issued path is unchanged (maintainer optional). Adversarially verified (6 properties). Advances #1255. (Configure the Orb App OAuth scopes to read:user + read:org for the org-admin check.) --- src/orb/broker.ts | 18 +++-- src/orb/oauth.ts | 110 ++++++++++++++++++++++--- test/integration/orb-oauth.test.ts | 126 ++++++++++++++++++++++++++++- 3 files changed, 237 insertions(+), 17 deletions(-) diff --git a/src/orb/broker.ts b/src/orb/broker.ts index 03ab99551f..0bbd35d610 100644 --- a/src/orb/broker.ts +++ b/src/orb/broker.ts @@ -18,19 +18,25 @@ export function isOrbBrokerEnabled(env: Env): boolean { export type IssueResult = { enrollId: string; secret: string } | { error: "installation_not_found" | "installation_not_registered" }; -/** Operator-only: mint a one-time enrollment secret for a REGISTERED install. Returns the plaintext secret ONCE - * (stored only hashed) for the operator to hand to the container's config. */ -export async function issueOrbEnrollment(env: Env, installationId: number): Promise { +/** Mint a one-time enrollment secret for a REGISTERED install. Returns the plaintext secret ONCE (stored only + * hashed). Issued by the operator (internal endpoint) OR by a maintainer who proved install-admin via OAuth — + * in the latter case the maintainer's GitHub identity is recorded for audit. installation_id is bound here and + * read back (never from the request) at token-exchange time, so a secret can never mint a token for another install. */ +export async function issueOrbEnrollment( + env: Env, + installationId: number, + maintainer?: { login: string; githubId?: number | null | undefined }, +): Promise { const install = await env.DB.prepare("SELECT registered FROM orb_github_installations WHERE installation_id = ?").bind(installationId).first<{ registered: number }>(); if (!install) return { error: "installation_not_found" }; if (install.registered !== 1) return { error: "installation_not_registered" }; const enrollId = createOpaqueToken("orbenr"); const secret = createOpaqueToken("orbsec"); await env.DB.prepare( - `INSERT INTO orb_enrollments (enroll_id, installation_id, secret_hash, state, authorized_at, enrolled_at) - VALUES (?, ?, ?, 'enrolled', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`, + `INSERT INTO orb_enrollments (enroll_id, installation_id, maintainer_login, maintainer_github_id, secret_hash, state, authorized_at, enrolled_at) + VALUES (?, ?, ?, ?, ?, 'enrolled', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`, ) - .bind(enrollId, installationId, await hashToken(secret)) + .bind(enrollId, installationId, maintainer?.login ?? null, maintainer?.githubId ?? null, await hashToken(secret)) .run(); return { enrollId, secret }; } diff --git a/src/orb/oauth.ts b/src/orb/oauth.ts index ef12789770..4fcf37157b 100644 --- a/src/orb/oauth.ts +++ b/src/orb/oauth.ts @@ -1,16 +1,91 @@ -// Gittensory Orb central GitHub App (#1255) — the post-install / OAuth landing endpoint. GitHub redirects here -// after a maintainer installs or updates the Orb App (the App's Callback URL, with OAuth-during-install ON). For -// now it confirms the connection so the install flow lands on a real page instead of a 401; the full OAuth -// code-exchange + container enrollment (the token-broker) layers onto this same endpoint next. Token-EXEMPT — -// GitHub drives the redirect with no API token (see requiresApiToken). No request input is echoed into the -// markup, so there is no injection surface. +// Gittensory Orb central GitHub App (#1255) — the post-install / OAuth landing + maintainer SELF-ENROLLMENT. +// GitHub redirects here after a maintainer installs/authorizes the Orb App (the App's Callback URL, OAuth-during- +// install ON) with an OAuth `code` + the `installation_id`. The maintainer can then self-issue their brokered +// enrollment secret WITHOUT the operator — but ONLY after we prove, server-side, that they are an ADMIN of the +// account the installation belongs to. +// +// SECURITY: the admin-of-installation check is what closes the privilege-escalation hole. `installation_id` is an +// attacker-controllable query param, so a stolen OAuth code paired with a VICTIM's installation_id must NEVER +// enroll the victim's install. We require: a valid OAuth code (single-use, GitHub-issued) → the authenticated +// user → that user is an admin of the install's account (org admin, or the user account owner) → the install is +// registered=1. installation_id is then bound server-side in the enrollment (read back at token-exchange, never +// from a request). No request input is echoed into the markup (no injection surface). import type { Context } from "hono"; +import { isOrbBrokerEnabled, issueOrbEnrollment } from "./broker"; -function landingPage(heading: string, message: string): string { - return `${heading}

${heading}

${message}

Open the dashboard
`; +type GitHubUser = { login: string; 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. */ +export async function exchangeOrbOAuthCode(env: Env, code: string, fetchImpl: typeof fetch = fetch): Promise { + if (!env.ORB_GITHUB_CLIENT_ID || !env.ORB_GITHUB_CLIENT_SECRET) return null; + const res = await fetchImpl("https://github.com/login/oauth/access_token", { + method: "POST", + headers: { accept: "application/json", "content-type": "application/json" }, + body: JSON.stringify({ client_id: env.ORB_GITHUB_CLIENT_ID, client_secret: env.ORB_GITHUB_CLIENT_SECRET, code }), + }); + const body = (await res.json().catch(() => ({}))) as { access_token?: string }; + return body.access_token ?? null; +} + +/** Identify the authenticated maintainer (GET /user with their token). Null on any non-OK / loginless response. */ +export async function fetchOrbOAuthUser(token: string, fetchImpl: typeof fetch = fetch): Promise { + const res = await fetchImpl("https://api.github.com/user", { + headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json", "user-agent": "gittensory/0.1" }, + }); + const user = (await res.json().catch(() => ({}))) as GitHubUser; + return res.ok && user.login ? user : null; +} + +/** CRITICAL admin-of-installation check — the gate that closes the privilege-escalation hole. The maintainer must + * be an ADMIN of the account the installation belongs to: for a User install they must BE that account owner; + * for an Org install they must be an ACTIVE org ADMIN (checked against their OWN membership, requires read:org). + * Anything else (member, non-member, unknown account, API error) → false. */ +export async function verifyInstallationAdmin( + token: string, + userLogin: string, + accountLogin: string | null, + accountType: string | null, + fetchImpl: typeof fetch = fetch, +): Promise { + if (!accountLogin) return false; + if (accountType !== "Organization") { + return 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"; +} + +async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise { + 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 FROM orb_github_installations WHERE installation_id = ?") + .bind(installationId) + .first<{ account_login: string | null; account_type: string | null; registered: number }>(); + 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); + if (install.registered !== 1) return c.html(landingPage("Not enabled yet", "This installation isn't enabled for brokered self-host yet — ask the operator to register it, then retry."), 403); + const isAdmin = await verifyInstallationAdmin(token, user.login, install.account_login, install.account_type); + 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); + const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null }); + /* v8 ignore next -- defensive: the existence + registered=1 checks above already passed, so issueOrbEnrollment + (which re-checks the same) cannot return an error here; kept so a future refactor degrades safely. */ + if ("error" in result) return c.html(landingPage("Couldn't issue an enrollment", "Please retry, or contact the operator."), 409); + return c.html(secretPage(result.secret)); } -export function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Response { +export async function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Promise { + const code = c.req.query("code"); + const installationId = Number(c.req.query("installation_id")); + // Self-enrollment: a maintainer authorized with an OAuth code + an installation_id, and the broker is enabled. + if (code && Number.isInteger(installationId) && installationId > 0 && isOrbBrokerEnabled(c.env)) { + return handleOrbEnrollment(c, code, installationId); + } const updated = c.req.query("setup_action") === "update"; return c.html( updated @@ -18,3 +93,20 @@ export function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Response : landingPage("Gittensory Orb connected", "Your repositories are linked. Their review activity now flows to the global Gittensory dashboard."), ); } + +function shell(heading: string, inner: string): string { + return `${heading}

${heading}

${inner}
`; +} + +function landingPage(heading: string, message: string): string { + return shell(heading, `

${message}

Open the dashboard`); +} + +/** Show the freshly-issued enrollment secret ONCE. The secret is a generated opaque token (no user input), safe + * to embed; it is never logged. */ +function secretPage(secret: string): string { + return shell( + "Your enrollment secret", + `

Set this as ORB_ENROLLMENT_SECRET in your self-host .env, then restart the container. It is shown once — store it now.

${secret}
`, + ); +} diff --git a/test/integration/orb-oauth.test.ts b/test/integration/orb-oauth.test.ts index 945dbe08ac..5e2265bcf6 100644 --- a/test/integration/orb-oauth.test.ts +++ b/test/integration/orb-oauth.test.ts @@ -1,6 +1,9 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createApp } from "../../src/api/routes"; -import { createTestEnv } from "../helpers/d1"; +import { exchangeOrbOAuthCode, fetchOrbOAuthUser, verifyInstallationAdmin } from "../../src/orb/oauth"; +import { createTestEnv, type TestD1Database } from "../helpers/d1"; + +const asFetch = (fn: (url: string) => Promise): typeof fetch => ((url: RequestInfo | URL) => fn(String(url))) as typeof fetch; describe("GET /v1/orb/oauth/callback (post-install landing)", () => { const app = createApp(); @@ -32,3 +35,122 @@ describe("GET /v1/orb/oauth/callback (post-install landing)", () => { expect([400, 413]).toContain(res.status); // reached the (exempt) ingest handler, failed only on the empty body }); }); + +describe("verifyInstallationAdmin (the privilege-escalation gate)", () => { + it("a USER-account install: only the account owner is an admin (case-insensitive)", async () => { + const f = asFetch(async () => Response.json({})); + expect(await verifyInstallationAdmin("t", "Alice", "alice", "User", f)).toBe(true); + expect(await verifyInstallationAdmin("t", "mallory", "alice", "User", f)).toBe(false); + }); + it("an ORG install: an ACTIVE org admin passes; a member, a pending admin, and an API error all fail", async () => { + expect(await verifyInstallationAdmin("t", "alice", "acme", "Organization", asFetch(async () => Response.json({ role: "admin", state: "active" })))).toBe(true); + expect(await verifyInstallationAdmin("t", "bob", "acme", "Organization", asFetch(async () => Response.json({ role: "member", state: "active" })))).toBe(false); + expect(await verifyInstallationAdmin("t", "carol", "acme", "Organization", asFetch(async () => Response.json({ role: "admin", state: "pending" })))).toBe(false); + expect(await verifyInstallationAdmin("t", "mallory", "acme", "Organization", asFetch(async () => new Response("no", { status: 403 })))).toBe(false); + expect(await verifyInstallationAdmin("t", "alice", "acme", "Organization", asFetch(async () => new Response("not-json", { status: 200 })))).toBe(false); // json() rejects → {} → not admin + }); + it("a missing account login is never admin", async () => { + expect(await verifyInstallationAdmin("t", "alice", null, "Organization", asFetch(async () => Response.json({})))).toBe(false); + }); +}); + +describe("exchangeOrbOAuthCode + fetchOrbOAuthUser", () => { + it("exchange returns null without client credentials, the token otherwise, null on a tokenless body", async () => { + expect(await exchangeOrbOAuthCode({} as Env, "c")).toBeNull(); + const env = { ORB_GITHUB_CLIENT_ID: "id", ORB_GITHUB_CLIENT_SECRET: "sec" } as Env; + expect(await exchangeOrbOAuthCode(env, "c", asFetch(async () => Response.json({ access_token: "ghu_x" })))).toBe("ghu_x"); + expect(await exchangeOrbOAuthCode(env, "c", asFetch(async () => Response.json({})))).toBeNull(); + expect(await exchangeOrbOAuthCode(env, "c", asFetch(async () => new Response("not-json")))).toBeNull(); // json() rejects → {} → null + }); + it("user fetch returns the user on ok, null on a non-ok / loginless response", async () => { + expect(await fetchOrbOAuthUser("t", asFetch(async () => Response.json({ login: "alice", id: 1 })))).toEqual({ login: "alice", id: 1 }); + expect(await fetchOrbOAuthUser("t", asFetch(async () => new Response("no", { status: 401 })))).toBeNull(); + }); +}); + +describe("maintainer self-enrollment via the OAuth callback", () => { + const app = createApp(); + const db = (e: Env) => e.DB as unknown as TestD1Database; + const brokeredEnv = () => createTestEnv({ ORB_BROKER_ENABLED: "true", ORB_GITHUB_CLIENT_ID: "id", ORB_GITHUB_CLIENT_SECRET: "sec" }); + const seedInstall = (e: Env, cols: Record) => { + const keys = Object.keys(cols); + return db(e).prepare(`INSERT INTO orb_github_installations (${keys.join(", ")}) VALUES (${keys.map(() => "?").join(", ")})`).bind(...keys.map((k) => cols[k] as string | number)).run(); + }; + const stubGitHub = (over: { token?: string; user?: unknown; membership?: unknown } = {}) => + vi.stubGlobal("fetch", asFetch(async (url) => { + if (url.includes("/login/oauth/access_token")) return Response.json({ access_token: over.token ?? "ghu_x" }); + if (url.includes("api.github.com/user/memberships/orgs/")) return Response.json(over.membership ?? { role: "admin", state: "active" }); + if (url.endsWith("api.github.com/user")) return Response.json(over.user ?? { login: "alice", id: 7 }); + return new Response("nf", { status: 404 }); + })); + afterEach(() => vi.unstubAllGlobals()); + + it("an org ADMIN self-enrolls a registered install → a one-time secret + recorded maintainer identity", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 500, account_login: "acme", account_type: "Organization", registered: 1 }); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=500", {}, e); + expect(res.status).toBe(200); + const html = await res.text(); + expect(html).toContain("Your enrollment secret"); + expect(html).toMatch(/orbsec_/); + const row = await db(e).prepare("SELECT maintainer_login, maintainer_github_id FROM orb_enrollments WHERE installation_id=500").first<{ maintainer_login: string; maintainer_github_id: number }>(); + expect(row).toMatchObject({ maintainer_login: "alice", maintainer_github_id: 7 }); + }); + + it("a NON-admin is refused (403) and NO enrollment is created — the escalation gate", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 501, account_login: "acme", account_type: "Organization", registered: 1 }); + stubGitHub({ membership: { role: "member", state: "active" } }); + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=501", {}, e); + expect(res.status).toBe(403); + expect(await res.text()).toContain("Admin access required"); + expect(await db(e).prepare("SELECT 1 AS x FROM orb_enrollments WHERE installation_id=501").first()).toBeUndefined(); + }); + + it("an UNREGISTERED install is refused (403)", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 502, account_login: "acme", account_type: "Organization", registered: 0 }); + stubGitHub(); + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=502", {}, e); + expect(res.status).toBe(403); + expect(await res.text()).toContain("Not enabled yet"); + }); + + it("an UNKNOWN install is 404", async () => { + const e = brokeredEnv(); + stubGitHub(); + expect((await app.request("/v1/orb/oauth/callback?code=abc&installation_id=999", {}, e)).status).toBe(404); + }); + + it("a USER-account owner self-enrolls (a login-only identity stores a null github id)", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 504, account_login: "alice", account_type: "User", registered: 1 }); + stubGitHub({ user: { login: "alice" } }); // no id → user.id ?? null + expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=504", {}, e)).text()).toContain("Your enrollment secret"); + const row = await db(e).prepare("SELECT maintainer_login, maintainer_github_id FROM orb_enrollments WHERE installation_id=504").first<{ maintainer_login: string; maintainer_github_id: number | null }>(); + expect(row).toMatchObject({ maintainer_login: "alice", maintainer_github_id: null }); + }); + + it("a failed code exchange → 400; the broker being OFF falls through to the landing page", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 505, account_login: "acme", account_type: "Organization", registered: 1 }); + vi.stubGlobal("fetch", asFetch(async () => Response.json({}))); // no access_token + expect((await app.request("/v1/orb/oauth/callback?code=abc&installation_id=505", {}, e)).status).toBe(400); + const off = createTestEnv({ ORB_GITHUB_CLIENT_ID: "id", ORB_GITHUB_CLIENT_SECRET: "sec" }); // broker OFF + expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=505", {}, off)).text()).toContain("Gittensory Orb connected"); + }); + + it("a failed /user read → 400", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 506, account_login: "acme", account_type: "Organization", registered: 1 }); + vi.stubGlobal("fetch", asFetch(async (url) => (url.endsWith("api.github.com/user") ? new Response("no", { status: 401 }) : Response.json({ access_token: "x" })))); + expect((await app.request("/v1/orb/oauth/callback?code=abc&installation_id=506", {}, e)).status).toBe(400); + }); + + it("a code with a non-numeric or non-positive installation_id is NOT an enrollment → landing page", async () => { + stubGitHub(); + expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=nope", {}, brokeredEnv())).text()).toContain("Gittensory Orb connected"); // Number.isInteger false + expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=0", {}, brokeredEnv())).text()).toContain("Gittensory Orb connected"); // installationId > 0 false + }); +});