fix(orb): revoke-on-rotate and a self-hoster-reachable revoke path for broker enrollments (#9149)

issueOrbEnrollment was a bare INSERT: it never revoked, updated, or even
counted sibling rows for the same installation_id, so re-running the
install flow after a secret leak minted a SECOND simultaneously-valid
secret and left the leaked one working forever. The only revoke path sat
behind INTERNAL_JOB_TOKEN, unreachable by a self-hosting maintainer.

- issueOrbEnrollment takes an optional { rotate: true }: when set, every
  prior live enrollment for the installation is revoked before the new one
  is minted. Defaults to false (append) so a blue/green container swap can
  still rely on two live enrollments briefly overlapping (#9150's sibling
  fix is what makes that overlap safe, not this one).
- The OAuth landing page's callback now recognizes state=<installationId>:
  rotate / :revoke (installation_id has no channel back through a bare
  GitHub login/oauth/authorize bounce other than state) -- re-proving
  admin-of-installation via the SAME verifyInstallationAdmin gate the
  enrollment flow already uses, so a maintainer can rotate or revoke
  without operator involvement. Revoke deliberately skips the
  suspended/self-enrollment-disabled checks: locking down a leaked secret
  should never be blocked by unrelated administrative state.
- The secret page now surfaces how many enrollments are live for the
  installation and links to the rotate/revoke actions; its response sets
  Cache-Control: no-store (it was previously a cacheable GET response
  rendering a plaintext secret).
- The internal installations list and enrollment-issue routes gained the
  same live-enrollment-count visibility and an optional rotate flag, for
  operator-issued parity with the OAuth self-service path.

Author: JSONbored

src/api/routes.ts

@@ -166,6 +166,7 @@ import {
   isOrbBrokerEnabled,
   issueOrbEnrollment,
   issueOrbStoredSecret,
+  ORB_SECRET_TYPE_GITHUB_TOKEN,
   ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL,
   revokeOrbEnrollment,
 } from "../orb/broker";
@@ -4579,15 +4580,22 @@ export function createApp() {
   // records lands at registered=0; only REGISTERED ones count toward the global public counter (getOrbGlobalStats)
   // and are eligible for token brokering. Bearer-gated by the `/v1/internal/*` middleware (INTERNAL_JOB_TOKEN). The
   // list shows pending + registered installs so an operator knows what they're opting in.
+  //
+  // liveEnrollmentCount (#9149): how many enrollment secrets are currently live ('enrolled', not revoked) for
+  // this install — an operator previously had no way to see that a re-enrollment (issueOrbEnrollment was a bare
+  // INSERT, never revoking) had accumulated a second standing credential. A correlated subquery rather than a
+  // JOIN: each installation has at most a handful of enrollment rows, so this stays cheap without reshaping the
+  // one-row-per-installation result set a GROUP BY would require.
   app.get("/v1/internal/orb/installations", async (c) => {
     const rows = await c.env.DB
       .prepare(
         `SELECT installation_id AS installationId, account_login AS accountLogin, account_type AS accountType,
                 repository_selection AS repositorySelection, registered, suspended_at AS suspendedAt,
-                removed_at AS removedAt, first_seen_at AS firstSeenAt, last_event_at AS lastEventAt
+                removed_at AS removedAt, first_seen_at AS firstSeenAt, last_event_at AS lastEventAt,
+                (SELECT COUNT(*) FROM orb_enrollments oe WHERE oe.installation_id = orb_github_installations.installation_id AND oe.state = 'enrolled' AND oe.revoked_at IS NULL) AS liveEnrollmentCount
          FROM orb_github_installations ORDER BY last_event_at DESC`,
       )
-      .all<{ installationId: number; accountLogin: string | null; accountType: string | null; repositorySelection: string | null; registered: number; suspendedAt: string | null; removedAt: string | null; firstSeenAt: string; lastEventAt: string }>();
+      .all<{ installationId: number; accountLogin: string | null; accountType: string | null; repositorySelection: string | null; registered: number; suspendedAt: string | null; removedAt: string | null; firstSeenAt: string; lastEventAt: string; liveEnrollmentCount: number }>();
     return c.json({ installations: (rows.results ?? []).map((r) => ({ ...r, registered: r.registered === 1 })) });
   });
 
@@ -4623,9 +4631,15 @@ export function createApp() {
   // secret issuance path control-plane's hosted provisioning core (#7180/#8066) calls instead, for a credential
   // that already exists (a tenant's Postgres connection string) rather than a GitHub installation to bind.
   // `installationId` is irrelevant to that path (see issueOrbStoredSecret's own header comment for why).
+  //
+  // Also accepts an optional `{ rotate: true }` (#9149): when set, every PRIOR live enrollment for this
+  // installation is revoked before the new one is minted, mirroring the OAuth landing page's `state=<id>:rotate`
+  // flow (oauth.ts) for the operator-issued path. Defaults to false (append, not replace) -- unchanged behavior
+  // for every existing caller, since a blue/green container swap legitimately relies on two live enrollments
+  // briefly overlapping (see #9150's sibling fix, which makes that overlap safe rather than assuming away).
   app.post("/v1/internal/orb/enrollments", async (c) => {
     if (!isOrbBrokerEnabled(c.env)) return c.json({ error: "not_found" }, 404);
-    const payload = (await c.req.json().catch(() => null)) as { installationId?: unknown; secretType?: unknown; secretValue?: unknown } | null;
+    const payload = (await c.req.json().catch(() => null)) as { installationId?: unknown; secretType?: unknown; secretValue?: unknown; rotate?: unknown } | null;
     if (payload?.secretType === ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL) {
       const secretValue = typeof payload.secretValue === "string" ? payload.secretValue : "";
       const result = await issueOrbStoredSecret(c.env, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, secretValue);
@@ -4634,7 +4648,7 @@ export function createApp() {
     }
     const installationId = Number(payload?.installationId);
     if (!Number.isInteger(installationId) || installationId <= 0) return c.json({ error: "installationId required" }, 400);
-    const result = await issueOrbEnrollment(c.env, installationId);
+    const result = await issueOrbEnrollment(c.env, installationId, undefined, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: payload?.rotate === true });
     if ("error" in result) return c.json(result, result.error === "installation_not_found" ? 404 : 409);
     return c.json(result); // { enrollId, secret } — secret shown exactly once
   });

src/orb/broker.ts

@@ -53,16 +53,26 @@ export type IssueResult = { enrollId: string; secret: string } | { error: "insta
  *  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. `secretType` defaults to the only mintable type today; every existing caller is unaffected. */
+ *  install. `secretType` defaults to the only mintable type today; every existing caller is unaffected.
+ *
+ *  `opts.rotate` (#9149): this was previously a bare INSERT, so re-running the enrollment flow (e.g. after a
+ *  secret leak) minted a SECOND simultaneously-valid secret and revoked nothing — the leaked one kept working
+ *  forever. Deliberately NOT the default: a blue/green container swap or a deliberate multi-container setup
+ *  legitimately relies on two live enrollments overlapping for a short window (see #9150's sibling fix, which
+ *  makes that overlap safe rather than assuming it can't happen) — auto-revoking on every issuance would break
+ *  that. `rotate: true` is the caller's explicit "I want this to be the only valid secret now" confirmation
+ *  (the OAuth landing page's `?rotate=1`/`state=<id>:rotate` flow, oauth.ts). */
 export async function issueOrbEnrollment(
   env: Env,
   installationId: number,
   maintainer?: { login: string; githubId?: number | null | undefined },
   secretType: string = ORB_SECRET_TYPE_GITHUB_TOKEN,
+  opts: { rotate?: boolean } = {},
 ): Promise<IssueResult> {
   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" };
+  if (opts.rotate === true) await revokeAllLiveEnrollmentsForInstallation(env, installationId);
   const enrollId = createOpaqueToken("orbenr");
   const secret = createOpaqueToken("orbsec");
   await env.DB.prepare(
@@ -74,6 +84,28 @@ export async function issueOrbEnrollment(
   return { enrollId, secret };
 }
 
+/** Revokes every currently-live ('enrolled', not yet revoked) enrollment for an installation, regardless of
+ *  secret_type — the self-hoster-reachable counterpart to the single-enrollment {@link revokeOrbEnrollment}
+ *  (#9149). Returns the count actually revoked (0 is a valid, non-error result: an install with no live
+ *  enrollment is not a failure, it's just already in the state the caller wanted). Deliberately does NOT gate
+ *  on `registered`/`suspended_at`/`removed_at` the way issuance does — revoking access should work EVEN ON an
+ *  installation the operator has since disabled or suspended, since a maintainer revoking a leaked secret has
+ *  no reason to be blocked by an unrelated administrative state. */
+export async function revokeAllLiveEnrollmentsForInstallation(env: Env, installationId: number): Promise<number> {
+  const result = await env.DB.prepare("UPDATE orb_enrollments SET revoked_at = CURRENT_TIMESTAMP, state = 'revoked' WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL")
+    .bind(installationId)
+    .run();
+  return result.meta.changes;
+}
+
+/** How many enrollments are currently live ('enrolled', not revoked) for an installation (#9149) — the count
+ *  surfaced to an operator (the internal installations list) and to a maintainer (the OAuth secret page) so
+ *  neither has to guess whether a re-enrollment minted an extra standing credential. */
+export async function countLiveEnrollmentsForInstallation(env: Env, installationId: number): Promise<number> {
+  const row = await env.DB.prepare("SELECT COUNT(*) AS c FROM orb_enrollments WHERE installation_id = ? AND state = 'enrolled' AND revoked_at IS NULL").bind(installationId).first<{ c: number }>();
+  return row?.c ?? 0;
+}
+
 export type IssueStoredSecretResult = IssueResult | { error: "secret_value_required" | "encryption_unavailable" };
 
 /** Issues a one-time enrollment secret for a STORED (not minted) credential (#8064) -- e.g. control-plane's

src/orb/oauth.ts

@@ -15,7 +15,7 @@
 import type { Context } from "hono";
 import { PRODUCT_USER_AGENT, timeoutFetch } from "../github/client";
 import { LOOPOVER_SITE_URL } from "../github/footer";
-import { isOrbBrokerEnabled, issueOrbEnrollment } from "./broker";
+import { countLiveEnrollmentsForInstallation, isOrbBrokerEnabled, issueOrbEnrollment, ORB_SECRET_TYPE_GITHUB_TOKEN, revokeAllLiveEnrollmentsForInstallation } from "./broker";
 
 type GitHubUser = { login: string; id?: number };
 type GitHubOrgMembership = { role?: string; state?: string; organization?: { id?: number } };
@@ -67,7 +67,39 @@ export async function verifyInstallationAdmin(
   return body.state === "active" && body.role === "admin" && body.organization?.id === accountId;
 }
 
-async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise<Response> {
+/** What a maintainer landing on the OAuth callback is asking to do, and which installation it's bound to.
+ *  `installation_id` normally comes from the GitHub-controlled query string (the install/update Setup-URL
+ *  redirect); `rotate`/`revoke` have no such channel (GitHub only ever echoes back `code` + `state` on a bare
+ *  `login/oauth/authorize` bounce, not arbitrary query params we didn't put there ourselves), so those two
+ *  actions are carried in `state` instead, as `"<installationId>:rotate"` / `"<installationId>:revoke"` (#9149)
+ *  — the value {@link orbOAuthAuthorizeUrl} puts in the "Rotate" / "Revoke all" links on the secret page. */
+type OrbOAuthIntent = { installationId: number | null; action: "enroll" | "rotate" | "revoke" };
+
+function parseOAuthState(raw: string | undefined): OrbOAuthIntent {
+  if (!raw) return { installationId: null, action: "enroll" };
+  const [idPart, actionPart] = raw.split(":");
+  const parsedId = Number(idPart);
+  const installationId = Number.isInteger(parsedId) && parsedId > 0 ? parsedId : null;
+  const action = actionPart === "rotate" ? "rotate" : actionPart === "revoke" ? "revoke" : "enroll";
+  return { installationId, action };
+}
+
+/** A `login/oauth/authorize` link that lands the maintainer back on THIS SAME callback with a fresh, single-
+ *  use `code` and `state=<installationId>:<action>` (#9149) — the only way to re-prove admin-of-installation
+ *  for a rotate/revoke action without our own persisted maintainer session. Null when the client id isn't
+ *  configured (broker effectively unusable anyway), so the caller can omit the link entirely rather than
+ *  point at a URL that can never work. */
+function orbOAuthAuthorizeUrl(env: Env, installationId: number, action: "rotate" | "revoke"): string | null {
+  /* v8 ignore next 2 -- defensive: every caller of this function is already past exchangeOrbOAuthCode, which
+     itself returns null (→ the identity-error page, never reaching secretPage) without ORB_GITHUB_CLIENT_ID
+     set — so this branch can't be live-reached through the real callback flow. Kept so a future change that
+     loosens that upstream guard degrades to an omitted link rather than a broken one. */
+  if (!env.ORB_GITHUB_CLIENT_ID) return null;
+  const params = new URLSearchParams({ client_id: env.ORB_GITHUB_CLIENT_ID, state: `${installationId}:${action}` });
+  return `https://github.com/login/oauth/authorize?${params.toString()}`;
+}
+
+async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number, action: "enroll" | "rotate" | "revoke" = "enroll"): Promise<Response> {
   // 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
@@ -102,26 +134,44 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string,
     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);
+  // #9149: a REVOKE request skips the active/disabled checks below on purpose — a maintainer revoking a
+  // leaked secret must be able to do so EVEN ON an installation the operator has since suspended or disabled;
+  // there is no reason unrelated administrative state should block someone locking down their own credential.
+  // It also never touches `registered` (no auto-registration side effect for a request that isn't enrolling).
+  if (action === "revoke") {
+    const revokedCount = await revokeAllLiveEnrollmentsForInstallation(c.env, installationId);
+    return c.html(revokedPage(revokedCount));
+  }
   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);
   // Zero-touch self-service: a verified admin of an ACTIVE, non-disabled install self-registers it (registered=1).
   // installation_id stays bound server-side in the enrollment, so brokered tokens remain scoped to this install.
   if (install.registered !== 1) {
     await c.env.DB.prepare("UPDATE orb_github_installations SET registered = 1, last_event_at = CURRENT_TIMESTAMP WHERE installation_id = ?").bind(installationId).run();
   }
-  const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null });
+  const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null }, ORB_SECRET_TYPE_GITHUB_TOKEN, { rotate: action === "rotate" });
   /* v8 ignore next -- defensive: the existence + admin + active checks above passed and we just set registered=1, so
      issueOrbEnrollment (which re-checks existence + registered) cannot return an error here; kept to degrade safely. */
   if ("error" in result) return c.html(landingPage(c.env, "Couldn't issue an enrollment", "Please retry, or contact the operator."), 409);
-  return c.html(secretPage(result.secret));
+  const liveCount = await countLiveEnrollmentsForInstallation(c.env, installationId);
+  c.header("Cache-Control", "no-store"); // #9149: this response body is a plaintext secret shown exactly once — never cacheable
+  return c.html(secretPage(c.env, result.secret, installationId, liveCount));
 }
 
 export async function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Promise<Response> {
   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);
+  // #9149: installation_id normally arrives in the query string (GitHub's own install/update Setup-URL
+  // redirect); a rotate/revoke request instead carries it inside `state` (see parseOAuthState's doc comment
+  // for why) — the query value, when itself valid, still wins, so a genuine install/update redirect is never
+  // second-guessed by a stray/malformed `state`.
+  const state = parseOAuthState(c.req.query("state"));
+  const rawInstallationId = Number(c.req.query("installation_id"));
+  const queryInstallationId = Number.isInteger(rawInstallationId) && rawInstallationId > 0 ? rawInstallationId : null;
+  const installationId = queryInstallationId ?? state.installationId;
+  // Self-enrollment (or a rotate/revoke of one): a maintainer authorized with an OAuth code + a resolved
+  // installation_id, and the broker is enabled.
+  if (code && installationId !== null && isOrbBrokerEnabled(c.env)) {
+    return handleOrbEnrollment(c, code, installationId, state.action);
   }
   const updated = c.req.query("setup_action") === "update";
   return c.html(
@@ -146,10 +196,30 @@ function landingPage(env: Env, heading: string, message: string): string {
 }
 
 /** 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 {
+ *  to embed; it is never logged. #9149: also surfaces how many enrollments are now live for this installation
+ *  (an operator/maintainer previously had no way to see this had accumulated) and, when the client id is
+ *  configured, links to re-authorize with a `state`-carried rotate/revoke intent (see orbOAuthAuthorizeUrl) —
+ *  a self-hoster otherwise had no reachable path to either action at all. `installationId` is never
+ *  user-supplied at render time (bound server-side, same as the enrollment itself), so it is safe to embed. */
+function secretPage(env: Env, secret: string, installationId: number, liveCount: number): string {
+  const rotateUrl = orbOAuthAuthorizeUrl(env, installationId, "rotate");
+  const revokeUrl = orbOAuthAuthorizeUrl(env, installationId, "revoke");
+  const manageSection =
+    rotateUrl && revokeUrl
+      ? `<p>This installation now has <strong>${liveCount}</strong> live enrollment secret${liveCount === 1 ? "" : "s"}, including this one. <a href="${rotateUrl}">Rotate</a> (revokes every other one and issues this as the only valid secret) or <a href="${revokeUrl}">revoke all</a> if you no longer need self-host access for this installation.</p>`
+      : "";
   return shell(
     "Your enrollment secret",
-    `<p>Set this as <code>ORB_ENROLLMENT_SECRET</code> in your self-host <code>.env</code>, then restart the container. It is shown <strong>once</strong> — store it now.</p><pre>${secret}</pre>`,
+    `<p>Set this as <code>ORB_ENROLLMENT_SECRET</code> in your self-host <code>.env</code>, then restart the container. It is shown <strong>once</strong> — store it now.</p><pre>${secret}</pre>${manageSection}`,
+  );
+}
+
+/** Confirms a revoke-all action (#9149) — the self-hoster-reachable counterpart to secretPage. `count` is
+ *  never user-supplied (it's the return of revokeAllLiveEnrollmentsForInstallation), so it's safe to embed. */
+function revokedPage(count: number): string {
+  const plural = count === 1 ? "secret has" : "secrets have";
+  return shell(
+    "Enrollment secrets revoked",
+    `<p><strong>${count}</strong> live enrollment ${plural} been revoked for this installation. Any self-hosted container still using ${count === 1 ? "it" : "one of them"} loses broker access immediately. Re-run the install flow to issue a new one.</p>`,
   );
 }