fix(auth): fail open when the rate-limit Durable Object errors (#5000) (#5041)

enforceRateLimit runs as global middleware ahead of every route's own
try/catch, and no app.onError is registered anywhere -- an uncaught
Durable Object hiccup (eviction, migration, a rolling-deploy blip)
escaped as Hono's bare, unstructured 500 for whatever route the
caller happened to be hitting. Traced the 95 orb_broker_unavailable
(500) events back to this middleware, not the /v1/orb/token handler
itself: that handler and its DB-touching helpers were already
hardened by the earlier #orb-broker-500 fix, but this shared
rate-limit check ran ahead of it, unguarded, for every route. Fail
open on both the DO check and its 429-denial audit write -- the rate
limiter protects the app, it must not crash the request it's gating.

Author: JSONbored

src/auth/rate-limit.ts

@@ -58,24 +58,39 @@ export async function enforceRateLimit(c: Context<{ Bindings: Env }>, routeClass
   if (!c.env.RATE_LIMITER) return null;
   const config = CONFIG[routeClass];
   const key = await rateLimitKey(c, routeClass);
-  const id = c.env.RATE_LIMITER.idFromName(key);
-  const decisionResponse = await c.env.RATE_LIMITER.get(id).fetch("https://rate-limit/check", {
-    method: "POST",
-    body: JSON.stringify({ key, ...config }),
-  });
+  let decisionResponse: Response;
+  try {
+    const id = c.env.RATE_LIMITER.idFromName(key);
+    decisionResponse = await c.env.RATE_LIMITER.get(id).fetch("https://rate-limit/check", {
+      method: "POST",
+      body: JSON.stringify({ key, ...config }),
+    });
+  } catch (error) {
+    // Fail OPEN (#5000): this middleware runs on every route ahead of the handler's own try/catch, and no
+    // app.onError is registered anywhere -- an uncaught Durable Object hiccup (eviction, migration, a
+    // rolling-deploy blip) previously escaped as Hono's bare, unstructured 500 for whatever route the caller
+    // happened to be hitting, indistinguishable from a real application bug in that route. The rate limiter
+    // exists to protect the app, not crash the request it's supposed to be gating.
+    console.error(JSON.stringify({ level: "error", event: "rate_limit_check_failed", routeClass, message: error instanceof Error ? error.message : String(error) }));
+    return null;
+  }
   const decision = (await decisionResponse.json().catch(() => ({}))) as Partial<RateLimitDecision>;
   if (decisionResponse.status !== 429) {
     c.res.headers.set("x-ratelimit-limit", String(decision.limit ?? config.limit));
     c.res.headers.set("x-ratelimit-remaining", String(decision.remaining ?? config.limit));
     if (decision.resetAt) c.res.headers.set("x-ratelimit-reset", decision.resetAt);
     return null;
   }
+  // Best-effort: the 429 itself must still reach the caller even if this audit write fails (#5000, same
+  // fail-open reasoning as the DO call above).
   await recordAuditEvent(c.env, {
     eventType: "rate_limit.denied",
     actor: await actorHint(c),
     route: c.req.path,
     outcome: "denied",
     metadata: { routeClass, retryAfterSeconds: decision.retryAfterSeconds ?? null },
+  }).catch((error) => {
+    console.warn(JSON.stringify({ level: "warn", event: "rate_limit_denied_audit_failed", routeClass, message: error instanceof Error ? error.message : String(error) }));
   });
   return c.json(
     {

test/unit/auth.test.ts

@@ -479,6 +479,85 @@ describe("private-beta auth and rate limiting", () => {
     expect(JSON.parse(tokenAudit?.metadata_json ?? "{}")).toMatchObject({ retryAfterSeconds: 17 });
   });
 
+  it("REGRESSION (#5000): fails OPEN when the rate-limit Durable Object itself throws, instead of crashing the request with a bare framework 500", async () => {
+    const throwingLimiter = {
+      idFromName(name: string) {
+        return name;
+      },
+      get() {
+        return {
+          async fetch() {
+            throw new Error("Durable Object reset");
+          },
+        };
+      },
+    };
+    const env = createTestEnv({ RATE_LIMITER: throwingLimiter as unknown as DurableObjectNamespace });
+    const errors = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+    await expect(enforceRateLimit(fakeContext(env, "/v1/orb/token", { "cf-connecting-ip": "203.0.113.9" }), "strict")).resolves.toBeNull();
+
+    expect(errors.mock.calls.some(([line]) => typeof line === "string" && line.includes("rate_limit_check_failed") && line.includes("\"routeClass\":\"strict\""))).toBe(true);
+    errors.mockRestore();
+  });
+
+  it("REGRESSION (#5000): still fails open when the Durable Object rejects with a non-Error value", async () => {
+    const throwingLimiter = {
+      idFromName(name: string) {
+        return name;
+      },
+      get() {
+        return {
+          async fetch() {
+            // Deliberately a non-Error rejection -- exercises the `error instanceof Error` false branch in the fail-open catch below.
+            throw "not an Error instance";
+          },
+        };
+      },
+    };
+    const env = createTestEnv({ RATE_LIMITER: throwingLimiter as unknown as DurableObjectNamespace });
+    const errors = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+    await expect(enforceRateLimit(fakeContext(env, "/v1/orb/token", { "cf-connecting-ip": "203.0.113.10" }), "strict")).resolves.toBeNull();
+
+    expect(errors.mock.calls.some(([line]) => typeof line === "string" && line.includes("rate_limit_check_failed") && line.includes("not an Error instance"))).toBe(true);
+    errors.mockRestore();
+  });
+
+  it("REGRESSION (#5000): a failed rate_limit.denied audit write does not stop the 429 itself from reaching the caller", async () => {
+    const deniedEnv = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 429, body: { resetAt: "2026-05-25T00:04:00.000Z" } }) as unknown as DurableObjectNamespace });
+    const realPrepare = deniedEnv.DB.prepare.bind(deniedEnv.DB);
+    deniedEnv.DB.prepare = ((sql: string) => {
+      if (/^insert into "?audit_events"?/i.test(sql)) throw new Error("poisoned query");
+      return realPrepare(sql);
+    }) as typeof deniedEnv.DB.prepare;
+    const warnings = vi.spyOn(console, "warn").mockImplementation(() => undefined);
+
+    const response = await enforceRateLimit(fakeContext(deniedEnv, "/v1/local/branch-analysis", { "cf-connecting-ip": "203.0.113.44" }), "expensive");
+
+    expect(response?.status).toBe(429);
+    await expect(response?.json()).resolves.toMatchObject({ error: "rate_limited", routeClass: "expensive" });
+    expect(warnings.mock.calls.some(([line]) => typeof line === "string" && line.includes("rate_limit_denied_audit_failed") && line.includes("poisoned query"))).toBe(true);
+    warnings.mockRestore();
+  });
+
+  it("REGRESSION (#5000): still fails open on a non-Error audit-write rejection", async () => {
+    const deniedEnv = createTestEnv({ RATE_LIMITER: rateLimiterNamespace({ status: 429, body: { resetAt: "2026-05-25T00:05:00.000Z" } }) as unknown as DurableObjectNamespace });
+    const realPrepare = deniedEnv.DB.prepare.bind(deniedEnv.DB);
+    deniedEnv.DB.prepare = ((sql: string) => {
+      // Deliberately a non-Error throw -- exercises the `error instanceof Error` false branch in the fail-open catch below.
+      if (/^insert into "?audit_events"?/i.test(sql)) throw "not an Error instance";
+      return realPrepare(sql);
+    }) as typeof deniedEnv.DB.prepare;
+    const warnings = vi.spyOn(console, "warn").mockImplementation(() => undefined);
+
+    const response = await enforceRateLimit(fakeContext(deniedEnv, "/v1/local/branch-analysis", { "cf-connecting-ip": "203.0.113.45" }), "expensive");
+
+    expect(response?.status).toBe(429);
+    expect(warnings.mock.calls.some(([line]) => typeof line === "string" && line.includes("rate_limit_denied_audit_failed") && line.includes("not an Error instance"))).toBe(true);
+    warnings.mockRestore();
+  });
+
   it("starts GitHub device flow and rejects malformed provider responses", async () => {
     const env = createTestEnv({ GITHUB_OAUTH_CLIENT_ID: "client-id" });
     vi.stubGlobal("fetch", async () =>