Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/auth/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions test/unit/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading