diff --git a/src/orb/oauth.ts b/src/orb/oauth.ts index d5d3216ed2..88d23bdfae 100644 --- a/src/orb/oauth.ts +++ b/src/orb/oauth.ts @@ -68,9 +68,25 @@ export async function verifyInstallationAdmin( } async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise { - const token = await exchangeOrbOAuthCode(c.env, code); + // A thrown network error (DNS failure, or a timeout past timeoutFetch's own retry budget) from any of the three + // GitHub calls below degrades to the same clean landing page a bad HTTP *response* already produces, instead of + // escaping handleOrbOAuthCallback as an uncaught framework 500. Mirrors the failure-doesn't-escape convention in + // webhook.ts/relay.ts/ingest.ts. The calls stay separate (not one body-wide wrapper) so a throw is never confused + // with a DB/broker fault after identity is established. + const identityError = () => c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "We couldn't reach GitHub to verify your identity — try the install again."), 400); + let token: string | null; + try { + token = await exchangeOrbOAuthCode(c.env, code); + } catch { + return identityError(); + } if (!token) return c.html(landingPage(c.env, "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); + let user: GitHubUser | null; + try { + user = await fetchOrbOAuthUser(token); + } catch { + return identityError(); + } if (!user) return c.html(landingPage(c.env, "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, account_id, registered, self_enrollment_disabled, suspended_at, removed_at FROM orb_github_installations WHERE installation_id = ?") .bind(installationId) @@ -79,7 +95,12 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, // 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. 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); + let isAdmin: boolean; + try { + isAdmin = await verifyInstallationAdmin(token, user.login, user.id, install.account_login, install.account_type, install.account_id); + } catch { + return identityError(); + } if (!isAdmin) return c.html(landingPage(c.env, "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(c.env, "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(c.env, "Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403); diff --git a/test/integration/orb-oauth.test.ts b/test/integration/orb-oauth.test.ts index 305f57face..b620687110 100644 --- a/test/integration/orb-oauth.test.ts +++ b/test/integration/orb-oauth.test.ts @@ -222,3 +222,55 @@ describe("maintainer self-enrollment via the OAuth callback", () => { expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=0", {}, brokeredEnv())).text()).toContain("LoopOver Orb connected"); // installationId > 0 false }); }); + +describe("self-enrollment degrades a thrown GitHub network error to the clean identity page (not an uncaught 500)", () => { + 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(); + }; + // Each of the three network calls resolves through the module's default timeoutFetch, so spying it lets us reject + // exactly one URL while the earlier calls succeed — reaching the specific call under test before it throws. + const rejectAt = (throwUrlFragment: string) => + vi.spyOn(githubClientModule, "timeoutFetch").mockImplementation((async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes(throwUrlFragment)) throw new Error("network down"); + if (url.includes("/login/oauth/access_token")) return Response.json({ access_token: "ghu_x" }); + if (url.includes("api.github.com/user/memberships/orgs/")) return Response.json({ role: "admin", state: "active", organization: { id: 20 } }); + if (url.endsWith("api.github.com/user")) return Response.json({ login: "alice", id: 7 }); + return new Response("nf", { status: 404 }); + }) as typeof githubClientModule.timeoutFetch); + afterEach(() => vi.restoreAllMocks()); + + it("a thrown code-exchange fetch degrades to the identity page (400), not an uncaught exception", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 600, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + rejectAt("/login/oauth/access_token"); + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=600", {}, e); + expect(res.status).toBe(400); + expect(await res.text()).toContain("Couldn't verify your GitHub identity"); + }); + + it("a thrown /user read degrades to the identity page (400)", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 601, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 }); + rejectAt("api.github.com/user"); // exchange succeeds first, then the /user read throws + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=601", {}, e); + expect(res.status).toBe(400); + expect(await res.text()).toContain("Couldn't verify your GitHub identity"); + expect(await db(e).prepare("SELECT 1 AS x FROM orb_enrollments WHERE installation_id=601").first()).toBeUndefined(); // never reached enrollment + }); + + it("a thrown org-membership check degrades to the identity page (400), leaving the install unregistered", async () => { + const e = brokeredEnv(); + await seedInstall(e, { installation_id: 602, account_login: "acme", account_type: "Organization", account_id: 20, registered: 0 }); + rejectAt("/user/memberships/orgs/"); // exchange + /user succeed, then the admin check throws + const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=602", {}, e); + expect(res.status).toBe(400); + expect(await res.text()).toContain("Couldn't verify your GitHub identity"); + const row = await db(e).prepare("SELECT registered FROM orb_github_installations WHERE installation_id=602").first<{ registered: number }>(); + expect(row?.registered).toBe(0); // a throw at the gate never auto-registers + }); +});