fix(orb): installation/permission/config integrity — suspension backfill, broker identity, numeric env validation, migration content hash

apps/loopover-ui/src/lib/selfhost-env-reference.ts

@@ -183,7 +183,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
   },
   {
     name: "CRON_INTERVAL_MS",
-    firstReference: "src/server.ts",
+    firstReference: "src/selfhost/preflight.ts",
   },
   {
     name: "DATABASE_PATH",
@@ -231,7 +231,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
   },
   {
     name: "GITHUB_CACHE_TTL_SECONDS",
-    firstReference: "src/server.ts",
+    firstReference: "src/selfhost/preflight.ts",
   },
   {
     name: "GITHUB_INSTALLATION_CONCURRENCY_DEFER_MS",
@@ -483,7 +483,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
   },
   {
     name: "PORT",
-    firstReference: "src/server.ts",
+    firstReference: "src/selfhost/preflight.ts",
   },
   {
     name: "POSTHOG_API_KEY",
@@ -690,7 +690,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
   "| `CONFIG_DIR_EMPTY_ACKNOWLEDGED` | `src/server.ts` |",
   "| `CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT` | `src/queue/processors.ts` |",
   "| `CONTRIBUTOR_EVIDENCE_BATCH_SIZE` | `src/queue/processors.ts` |",
-  "| `CRON_INTERVAL_MS` | `src/server.ts` |",
+  "| `CRON_INTERVAL_MS` | `src/selfhost/preflight.ts` |",
   "| `DATABASE_PATH` | `src/server.ts` |",
   "| `DATABASE_URL` | `src/selfhost/preflight.ts` |",
   "| `DISCORD_REPO_WEBHOOKS` | `src/services/notify-discord.ts` |",
@@ -702,7 +702,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
   "| `GITHUB_APP_ID` | `src/selfhost/orb-collector.ts` |",
   "| `GITHUB_APP_PRIVATE_KEY` | `src/selfhost/orb-collector.ts` |",
   "| `GITHUB_APP_SLUG` | `src/selfhost/pg-queue.ts` |",
-  "| `GITHUB_CACHE_TTL_SECONDS` | `src/server.ts` |",
+  "| `GITHUB_CACHE_TTL_SECONDS` | `src/selfhost/preflight.ts` |",
   "| `GITHUB_INSTALLATION_CONCURRENCY_DEFER_MS` | `src/selfhost/installation-concurrency-admission.ts` |",
   "| `GITHUB_INSTALLATION_CONCURRENCY_ENABLED` | `src/selfhost/installation-concurrency-admission.ts` |",
   "| `GITHUB_INSTALLATION_CONCURRENCY_LIMIT` | `src/selfhost/installation-concurrency-admission.ts` |",
@@ -765,7 +765,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
   "| `PAGERDUTY_ROUTING_KEY` | `src/services/notify-pagerduty.ts` |",
   "| `PGPOOL_MAX` | `src/selfhost/queue-common.ts` |",
   "| `PGVECTOR_ENABLED` | `src/server.ts` |",
-  "| `PORT` | `src/server.ts` |",
+  "| `PORT` | `src/selfhost/preflight.ts` |",
   "| `POSTHOG_API_KEY` | `src/selfhost/otel.ts` |",
   "| `POSTHOG_DISABLED` | `src/selfhost/posthog.ts` |",
   "| `POSTHOG_ENVIRONMENT` | `src/selfhost/otel.ts` |",

src/github/app.ts

@@ -242,12 +242,33 @@ async function mintInstallationToken(
   if (isOrbBrokerMode(env)) {
     try {
       const brokered = await fetchBrokeredInstallationToken(env, fetch, { forceRefresh });
-      await writeCachedToken(installationId, {
+      // #9152: the broker's installationId is either 0 (fetchBrokeredInstallationToken's default for an older/
+      // minimal broker reply that never echoed the field back) or must equal the install THIS call requested.
+      // Previously the cache write and the returned token were UNGUARDED — only the permissions write below
+      // checked this — so a stale caller-side installationId (e.g. `repositories.installation_id` left behind
+      // after the maintainer uninstalled/reinstalled the App, which GitHub assigns a NEW id) could cache and
+      // return a token the broker minted for a DIFFERENT install under THIS install's cache key. In a hosted/
+      // multi-tenant broker that is real cross-tenant token exposure, not just a stale-cache correctness bug.
+      // Throw instead of silently serving another install's token; the catch below still applies its existing
+      // stale-cache grace / rethrow behavior for this like any other broker-call failure.
+      if (brokered.installationId !== 0 && brokered.installationId !== installationId) {
+        throw new Error(
+          `Orb broker minted a token for installation ${brokered.installationId}, not the requested ${installationId}.`,
+        );
+      }
+      // Cache under the broker's own claimed id when it provided one (guaranteed equal to installationId by the
+      // guard above) so the cache key always reflects what was ACTUALLY minted, not just what was asked for.
+      const mintedInstallationId = brokered.installationId || installationId;
+      await writeCachedToken(mintedInstallationId, {
         token: brokered.token,
         expiresAtMs: brokered.expiresAtMs,
       });
-      if (brokered.installationId === installationId && Object.keys(brokered.permissions).length > 0) {
-        await updateInstallationPermissions(env, installationId, brokered.permissions).catch((error) => {
+      // No longer gated on `brokered.installationId === installationId`: that condition was ALWAYS false for a
+      // legacy broker reply that omits installationId (defaults to 0), so permissions from such a reply were
+      // silently never persisted. The mismatch case above already throws, so every reply reaching here is
+      // either explicitly confirmed for this install or carries no installationId claim to check.
+      if (Object.keys(brokered.permissions).length > 0) {
+        await updateInstallationPermissions(env, mintedInstallationId, brokered.permissions).catch((error) => {
           console.warn(
             JSON.stringify({
               level: "warn",
@@ -480,8 +501,15 @@ export async function getRepositoryCollaboratorPermission(
   }
   const payload = (await response.json()) as {
     permission?: GitHubRepositoryCollaboratorPermission;
+    role_name?: string;
   };
-  return payload.permission ?? null;
+  // #9152: `permission` is GitHub's coarse legacy field — only ever admin/write/read/none, so Maintain collapses
+  // into "write" and Triage into "read" — making `permission === "maintain"` dead at both call sites
+  // (isPerTenantAdmin, resolveRealRepoPermissionAssociation). `role_name` carries the actual granular tier
+  // (including "maintain"/"triage", or a custom org role) and is always present on this endpoint's real response;
+  // prefer it, falling back to `permission` only for a response that omits it (defensive — keeps the pre-#9152
+  // behavior for any caller/fixture that only ever set `permission`).
+  return payload.role_name ?? payload.permission ?? null;
 }
 
 /** Account-age throttle (#2561, anti-abuse): `GET /users/{login}` for `created_at`, so a repo can flag a

src/orb/app-auth.ts

@@ -31,6 +31,11 @@ export interface OrbAppInstallation {
   accountType: string | null;
   accountId: number | null;
   repositorySelection: string | null;
+  // #9151: GitHub's own suspension signal — the App can be suspended by the account owner without an
+  // `uninstall`, and `GET /app/installations` keeps listing a suspended install (unlike a real uninstall,
+  // which drops it from this list entirely) with this populated. Parsed here so the backfill can write it
+  // through instead of assuming every install GitHub returns is unsuspended.
+  suspendedAt: string | null;
 }
 
 /** Lists every installation of the Orb App (paginated). The backfill reads this to recover installs whose
@@ -44,9 +49,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; id?: number } | null; repository_selection?: string }>;
+    const rows = (await response.json()) as Array<{ id?: number; account?: { login?: string; type?: string; id?: number } | null; repository_selection?: string; suspended_at?: string | null }>;
     for (const row of rows) {
-      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 (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, suspendedAt: row.suspended_at ?? 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

@@ -46,19 +46,28 @@ export async function upsertOrbInstallation(env: Env, eventName: string, payload
  * `installation` webhook fired before the receiver's secret was configured (so they were never recorded). Upserts
  * each install WITHOUT touching `registered`, so a re-run never re-trusts an opted-out install; new rows land at
  * the default registered=0 (the manual-onboarding gate).
+ *
+ * #9151: `suspended_at` is written through from GitHub's own `listOrbAppInstallations` response instead of being
+ * hardcoded to NULL — a suspension recorded by the `installation.suspend` webhook (above) is the ONLY registry-side
+ * signal that an account owner revoked consent (see brokerOrbToken's eligibility check, broker.ts, and the
+ * "Installation not active" check, oauth.ts), and no `unsuspend` webhook is guaranteed to ever arrive to restore it
+ * if a backfill silently erased it. `removed_at` stays cleared to NULL: unlike suspension, GitHub's
+ * `GET /app/installations` (what `listOrbAppInstallations` walks) never lists an actually-uninstalled App at
+ * all — every install this loop sees is, by definition, currently installed, so clearing `removed_at` here
+ * reflects reality rather than resurrecting a removed install the backfill never actually saw.
  */
 export async function backfillOrbInstallations(env: Env): Promise<{ backfilled: number }> {
   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, account_id, repository_selection, last_event_at)
-       VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP)
+      `INSERT INTO orb_github_installations (installation_id, account_login, account_type, account_id, repository_selection, suspended_at, last_event_at)
+       VALUES (?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)
        ON CONFLICT(installation_id) DO UPDATE SET
          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,
+         repository_selection = excluded.repository_selection, suspended_at = excluded.suspended_at, removed_at = NULL,
          last_event_at = CURRENT_TIMESTAMP`,
     )
-      .bind(inst.id, inst.accountLogin, inst.accountType, inst.accountId, inst.repositorySelection)
+      .bind(inst.id, inst.accountLogin, inst.accountType, inst.accountId, inst.repositorySelection, inst.suspendedAt)
       .run();
   }
   return { backfilled: installs.length };

src/selfhost/cron-alignment.ts

@@ -1,3 +1,13 @@
+// #9157: the floor below which CRON_INTERVAL_MS is rejected rather than honored. A malformed value (a unit
+// suffix like "2m"/"120s", a numeric separator like "120_000", or outright garbage) parses to NaN, and Node
+// coerces BOTH `setTimeout(fn, NaN)` and `setInterval(fn, NaN)` to a 1ms delay — turning a single operator typo
+// into ~1000 scheduled ticks/second instead of one every two minutes, each running the full worker.scheduled()
+// fan-out. 0 takes the identical path (`x % 0` is NaN) and is NOT treated as "disable the cron" — there is no
+// supported way to run this entrypoint without its maintenance cron, so 0 is just another invalid value, not a
+// meaningful opt-out. Shared by preflight.ts (boot-time hard failure) and server.ts (runtime defense-in-depth
+// clamp via parsePositiveIntEnv) so the two enforce the exact same floor.
+export const CRON_INTERVAL_MIN_MS = 10_000; // 10s — comfortably above any accidental near-zero value, well under the 120s default
+
 /** Milliseconds from `nowMs` until the next wall-clock boundary of `intervalMs`, so a self-host `setTimeout`
  *  can phase-align its first tick to the same instants Cloudflare's own cron trigger would fire on (e.g. the
  *  every-2-minutes trigger fires exactly at :00, :02, :04, … UTC). Computed against epoch -- itself minute-aligned --

src/selfhost/migrate.ts

@@ -2,10 +2,16 @@
 // files Cloudflare applies via `wrangler d1 migrations apply` — they're plain SQLite DDL, so they run as-is
 // through the D1 adapter's exec(). Tracked in a `_selfhost_migrations` table so a restart re-applies only the
 // new ones (idempotent), mirroring wrangler's migration ledger.
+import { createHash } from "node:crypto";
 import { readdirSync, readFileSync } from "node:fs";
 import { join } from "node:path";
 import { errorMessage } from "../utils/json";
 
+/** SHA-256 of a migration file's raw content, hex-encoded — the ledger's content-identity check (#9164). */
+function contentSha256(sql: string): string {
+  return createHash("sha256").update(sql, "utf8").digest("hex");
+}
+
 function splitSqlStatements(sql: string): string[] {
   const statements: string[] = [];
   let start = 0;
@@ -96,21 +102,84 @@ function sqlQuote(value: string): string {
  * only works per-statement. So a file that fails atomically is retried through the original tolerant path.
  * The two orders matter: atomic first means a genuine mid-file crash is the case that gets protected, and the
  * tolerant fallback only runs for a database that was already inconsistent before this boot began.
+ *
+ * #9164 — the ledger also records each applied file's `content_sha256`, checked on every boot against the
+ * file's CURRENT on-disk hash. Before this, the ledger recorded only a filename: a migration edited in place
+ * after it was applied (rather than shipping a new numbered file) was invisible forever — `applied.has(file)`
+ * still matched, so the edit was silently skipped, and the running DB quietly diverged from the repo's
+ * declared schema with no signal anywhere (not at boot, not in `/health`, not in `preflight.ts`). A mismatch
+ * now fails boot loudly (thrown, after a structured console.error) rather than being silently skipped — same
+ * "fail loudly, don't limp along on a maybe-wrong schema" posture as `assertSelfHostPreflight`.
+ *
+ * The four grandfathered duplicate-NUMBER pairs (0015/0017/0074/0156, see migration-collisions.ts) need no
+ * special handling here: the ledger — and this drift check — key on the FILENAME, not the leading number, so
+ * each grandfathered file already has its own independent ledger row and hash exactly like any other
+ * migration. Their collision is a distinct, already-solved concern (duplicate NUMBER assignment, caught by
+ * scripts/check-migrations.ts pre-merge); content identity per file is unaffected by it.
+ *
+ * No down-migration path is added here. Forward-only migrations remain this repo's model — as with every
+ * existing migrations/*.sql file, reverting a change means writing a new migration that undoes it, not
+ * running this one "backwards".
  */
 export async function runSelfHostMigrations(db: D1Database, dir: string): Promise<number> {
-  await db.exec("CREATE TABLE IF NOT EXISTS _selfhost_migrations (name TEXT PRIMARY KEY, applied_at TEXT NOT NULL)");
-  const existing = await db.prepare("SELECT name FROM _selfhost_migrations").all<{ name: string }>();
-  const applied = new Set(existing.results.map((r) => r.name));
+  await db.exec("CREATE TABLE IF NOT EXISTS _selfhost_migrations (name TEXT PRIMARY KEY, applied_at TEXT NOT NULL, content_sha256 TEXT)");
+  // A pre-#9164 ledger predates the content_sha256 column; ADD COLUMN is idempotent the same way every other
+  // "might already be there" DDL in this file is tolerated (duplicate column / already exists), so a fresh
+  // table (which already has the column from CREATE TABLE above) and an upgrading one both end up identical.
+  try {
+    await db.exec("ALTER TABLE _selfhost_migrations ADD COLUMN content_sha256 TEXT");
+  } catch (error) {
+    if (!/duplicate column|already exists/i.test(errorMessage(error))) throw error;
+  }
+
+  const existing = await db.prepare("SELECT name, content_sha256 FROM _selfhost_migrations").all<{ name: string; content_sha256: string | null }>();
+  const appliedHashes = new Map(existing.results.map((r) => [r.name, r.content_sha256]));
   const files = readdirSync(dir).filter((f) => f.endsWith(".sql")).sort();
+
+  // Content-drift check (#9164): for every file already recorded as applied, recompute its CURRENT on-disk hash
+  // and compare it to the hash recorded at apply time. A row with no stored hash predates this column —
+  // backfilled below to the file's CURRENT content as its baseline (there is no way to recover the hash of
+  // whatever actually ran before this column existed; the current content is the best available starting
+  // point). A mismatch means the file's body changed AFTER it was applied — the running DB was never updated
+  // to match, so the schema has silently drifted with no other signal. This is the opposite case from the
+  // per-statement drift TOLERANCE below (that tolerance is for a database catching itself up to a migration it
+  // never fully applied); here the migration WAS fully applied, and it is the file's declared intent that
+  // changed out from under it, so it must fail loudly rather than be silently skipped.
+  const hashBackfills: Array<{ file: string; hash: string }> = [];
+  const drifted: string[] = [];
+  for (const [name, storedHash] of appliedHashes) {
+    if (!files.includes(name)) continue; // file removed from the repo since applying — a different concern (renumbering/collisions), not content drift
+    const currentHash = contentSha256(readFileSync(join(dir, name), "utf8"));
+    if (storedHash === null) hashBackfills.push({ file: name, hash: currentHash });
+    else if (storedHash !== currentHash) drifted.push(name);
+  }
+  if (drifted.length > 0) {
+    console.error(
+      JSON.stringify({
+        level: "error",
+        event: "selfhost_migration_content_drift",
+        files: drifted,
+      }),
+    );
+    throw new Error(
+      `Applied migration(s) edited after being applied — on-disk content no longer matches the hash recorded at apply time: ${drifted.join(", ")}. Add a NEW migration to make the change instead of editing an already-applied file, or restore its original content.`,
+    );
+  }
+  for (const { file, hash } of hashBackfills) {
+    await db.prepare("UPDATE _selfhost_migrations SET content_sha256 = ? WHERE name = ?").bind(hash, file).run();
+  }
+
+  const applied = new Set(appliedHashes.keys());
   // Real Cloudflare D1 has no transactional multi-statement surface; only the two self-host adapters implement
   // this. Absent it, behavior is exactly the pre-#9027 path.
   const execTransaction = (db as unknown as { execTransaction?: (sql: string) => Promise<void> }).execTransaction;
   let count = 0;
   for (const file of files) {
     if (applied.has(file)) continue;
     const sql = readFileSync(join(dir, file), "utf8");
+    const hash = contentSha256(sql);
     const statements = splitSqlStatements(sql);
-    const ledger = `INSERT INTO _selfhost_migrations (name, applied_at) VALUES (${sqlQuote(file)}, ${sqlQuote(new Date().toISOString())});`;
+    const ledger = `INSERT INTO _selfhost_migrations (name, applied_at, content_sha256) VALUES (${sqlQuote(file)}, ${sqlQuote(new Date().toISOString())}, ${sqlQuote(hash)});`;
     if (execTransaction) {
       try {
         // splitSqlStatements keeps each statement's own trailing `;` but returns a final unterminated tail
@@ -146,7 +215,7 @@ export async function runSelfHostMigrations(db: D1Database, dir: string): Promis
           throw error;
       }
     }
-    await db.prepare("INSERT INTO _selfhost_migrations (name, applied_at) VALUES (?, ?)").bind(file, new Date().toISOString()).run();
+    await db.prepare("INSERT INTO _selfhost_migrations (name, applied_at, content_sha256) VALUES (?, ?, ?)").bind(file, new Date().toISOString(), hash).run();
     count += 1;
   }
   return count;