From b8309fc647f2c8554f135fd6db57428ad7de8429 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Mon, 27 Jul 2026 01:02:42 -0700
Subject: [PATCH] fix(orb): degrade rate-limit installation-identity resolution
 on a DB error instead of escaping uncaught (#9225)

---
 src/auth/rate-limit.ts | 10 +++++++++-
 test/unit/auth.test.ts | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts
index 84b31067c..8e8f7733f 100644
--- a/src/auth/rate-limit.ts
+++ b/src/auth/rate-limit.ts
@@ -237,7 +237,15 @@ async function installationRateLimitIdentity(c: Context<{ Bindings: Env }>): Pro
   if (INSTALLATION_KEYED_ORB_BEARER_PATHS.has(path)) {
     const token = extractBearerToken(c.req.header("authorization"));
     if (!token) return null;
-    const enrollment = await validateOrbRelayEnrollment(c.env, token);
+    // #9225: this middleware runs BEFORE the route handler, which has its own well-tested error response for
+    // an unresolvable enrollment (a clean `503 broker_error` for the relay/token routes' own later call to
+    // this same function). A DB error here must never escape uncaught -- it would surface as a bare framework
+    // 500 upstream of that handler, exactly the class of gap #4995 already fixed for the handler's OWN call.
+    // Degrading to null (→ IP-keyed fallback below) matches peekWebhookInstallationId's sibling pattern and
+    // this function's own doc comment: a DB error is just another "not resolvable" case, alongside a
+    // malformed payload or an unenrolled secret.
+    const enrollment = await validateOrbRelayEnrollment(c.env, token).catch(() => null);
+    if (enrollment === null) return null;
     return "error" in enrollment ? null : `installation:${enrollment.installationId}`;
   }
   return null;
diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts
index 3dad072fd..5dffdfddd 100644
--- a/test/unit/auth.test.ts
+++ b/test/unit/auth.test.ts
@@ -515,6 +515,42 @@ describe("private-beta auth and rate limiting", () => {
     expect(observedKeys[0]).toMatch(/^strict:\/v1\/orb\/token:ip:/);
   });
 
+  it("REGRESSION (#9225): a DB error resolving installation identity falls back to IP-keying instead of escaping uncaught, for all three bearer-keyed paths", async () => {
+    // The bug: installationRateLimitIdentity's ORB_BEARER_PATHS branch called validateOrbRelayEnrollment
+    // unguarded. This middleware runs BEFORE the route handler, so an uncaught rejection here previously
+    // surfaced as a bare framework 500 -- upstream of, and bypassing, the route's own #4995 fix (its `.catch`
+    // around its OWN later call to the same function). Forcing the underlying orb_enrollments lookup to
+    // reject exercises exactly that earlier call site.
+    const observedKeys: string[] = [];
+    const env = rateLimitTestEnv({}, observedKeys);
+    const db = env.DB as unknown as TestD1Database;
+    await db.prepare("INSERT INTO orb_github_installations (installation_id, registered) VALUES (?, 1)").bind(603).run();
+    const { secret } = (await issueOrbEnrollment(env, 603)) as { secret: string };
+
+    const realPrepare = db.prepare.bind(db);
+    db.prepare = ((sql: string) => {
+      const statement = realPrepare(sql);
+      if (!/select .* from ["`]?orb_enrollments["`]?/i.test(sql)) return statement;
+      return {
+        ...statement,
+        bind(...values: unknown[]) {
+          const bound = statement.bind(...(values as never[]));
+          return { ...bound, first: () => Promise.reject(new Error("db unavailable")) };
+        },
+      };
+    }) as typeof realPrepare;
+
+    for (const path of ["/v1/orb/token", "/v1/orb/relay/register", "/v1/orb/relay/pull"] as const) {
+      observedKeys.length = 0;
+      // Falls back to IP-keying (never rejects, never throws) -- the middleware itself must resolve cleanly
+      // regardless of what the route handler downstream will separately decide to respond with.
+      await expect(
+        enforceRateLimit(fakeContext(env, path, { authorization: `Bearer ${secret}`, "cf-connecting-ip": "203.0.113.27" }), "strict"),
+      ).resolves.toBeNull();
+      expect(observedKeys[0]).toMatch(new RegExp(`^strict:${path.replace(/\//g, "\\/")}:ip:`));
+    }
+  });
+
   it("ignores proxy fallback headers when cf-connecting-ip is absent", async () => {
     const observedKeys: string[] = [];
     const env = rateLimitTestEnv({}, observedKeys);