From 61bb57d5ad7391b0f7daef71e16f8b7ecc2c060c Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 6 Aug 2026 17:12:03 +0000 Subject: [PATCH] fix(security): bound concurrent board-key auth work Co-Authored-By: Paperclip --- .../board-key-auth-middleware.test.ts | 44 ++++++++++++++++++- server/src/middleware/auth.ts | 29 ++++++++---- .../board-key-auth-failure-rate-limit.ts | 37 ++++++++++++++++ 3 files changed, 101 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/board-key-auth-middleware.test.ts b/server/src/__tests__/board-key-auth-middleware.test.ts index 46af6019dd..7845c011c7 100644 --- a/server/src/__tests__/board-key-auth-middleware.test.ts +++ b/server/src/__tests__/board-key-auth-middleware.test.ts @@ -34,6 +34,8 @@ function createDbState() { revoked: false, revokeOnTouch: false, lastUsedAt: null as Date | null, + boardKeyLookupBarrier: null as Promise | null, + boardKeyLookupStarts: 0, }; const audits: Array> = []; const key = () => ({ @@ -71,7 +73,13 @@ function createDbState() { ? [] : []; return { - where: () => Promise.resolve(rows), + where: async () => { + if (table === boardApiKeys) { + state.boardKeyLookupStarts += 1; + await state.boardKeyLookupBarrier; + } + return rows; + }, then: (resolve: (value: unknown[]) => unknown) => Promise.resolve(rows).then(resolve), }; }, @@ -201,6 +209,8 @@ describe("board-key authentication middleware", () => { globalLimit: 10_000, credentialMaxEntries: 7, sourceMaxEntries: 5, + sourceInFlightLimit: 2, + globalInFlightLimit: 3, }); for (let index = 0; index < 100; index += 1) { @@ -215,6 +225,8 @@ describe("board-key authentication middleware", () => { credentialEntries: 7, sourceEntries: 5, globalEntries: 1, + sourceInFlightEntries: 0, + globalInFlight: 0, }); }); @@ -226,6 +238,8 @@ describe("board-key authentication middleware", () => { globalLimit: 3, credentialMaxEntries: 10, sourceMaxEntries: 10, + sourceInFlightLimit: 10, + globalInFlightLimit: 10, }); for (let index = 0; index < 3; index += 1) { @@ -235,6 +249,34 @@ describe("board-key authentication middleware", () => { expect(limiter.isLimited({ credentialId: "novel-credential", sourceId: "novel-source", now: 1 })).toBe(true); }); + it("bounds concurrent database authentication starts with pre-lookup admission", async () => { + const { db, state } = createDbState(); + state.keyExists = false; + let releaseLookups!: () => void; + state.boardKeyLookupBarrier = new Promise((resolve) => { + releaseLookups = resolve; + }); + const { app } = createApp(db); + const inFlightLimit = BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS.sourceInFlightLimit; + + const requests = Array.from({ length: inFlightLimit + 6 }, (_, index) => + request(app) + .get("/actor") + .set("Authorization", `Bearer pcp_board_concurrent_${index}`) + .then((response) => response), + ); + + await vi.waitFor(() => expect(state.boardKeyLookupStarts).toBe(inFlightLimit)); + await new Promise((resolve) => setImmediate(resolve)); + expect(state.boardKeyLookupStarts).toBe(inFlightLimit); + releaseLookups(); + + const responses = await Promise.all(requests); + expect(responses.filter((response) => response.status === 401)).toHaveLength(inFlightLimit); + expect(responses.filter((response) => response.status === 429)).toHaveLength(6); + expect(state.boardKeyLookupStarts).toBe(inFlightLimit); + }); + it("does not resurrect last-used state when revocation wins the authentication race", async () => { const { db, state } = createDbState(); state.revokeOnTouch = true; diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 17a1688295..37825327ba 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -352,16 +352,29 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa next(tooManyRequests("Too many authentication failures")); return; } - const authentication = await boardAuth.authenticateBoardApiKey(token); - if (!authentication.ok) { - await auditBoardKeyAuthenticationFailure(db, req, authentication); - next( - boardKeyAuthFailureRateLimiter.recordFailure(failureIdentity) - ? tooManyRequests("Too many authentication failures") - : unauthorized(), - ); + const admission = boardKeyAuthFailureRateLimiter.tryAcquire({ sourceId: failureIdentity.sourceId }); + if (!admission) { + next(tooManyRequests("Too many authentication failures")); return; } + let authentication: Awaited>; + try { + authentication = await boardAuth.authenticateBoardApiKey(token); + } catch (error) { + admission.release(); + throw error; + } + if (!authentication.ok) { + const limited = boardKeyAuthFailureRateLimiter.recordFailure(failureIdentity); + try { + await auditBoardKeyAuthenticationFailure(db, req, authentication); + } finally { + admission.release(); + } + next(limited ? tooManyRequests("Too many authentication failures") : unauthorized()); + return; + } + admission.release(); const { key: boardKey, access, scopeConfig } = authentication; const effectiveCompanyIds = scopeConfig ? scopeConfig.companyIds.filter((companyId) => access.companyIds.includes(companyId)) diff --git a/server/src/security/board-key-auth-failure-rate-limit.ts b/server/src/security/board-key-auth-failure-rate-limit.ts index fa73ae8b4d..1e197ea9aa 100644 --- a/server/src/security/board-key-auth-failure-rate-limit.ts +++ b/server/src/security/board-key-auth-failure-rate-limit.ts @@ -5,6 +5,8 @@ export type BoardKeyAuthFailureRateLimitConfig = { globalLimit: number; credentialMaxEntries: number; sourceMaxEntries: number; + sourceInFlightLimit: number; + globalInFlightLimit: number; }; export const BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS = { @@ -14,6 +16,8 @@ export const BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS = { globalLimit: 1_000, credentialMaxEntries: 4_096, sourceMaxEntries: 1_024, + sourceInFlightLimit: 8, + globalInFlightLimit: 128, } satisfies BoardKeyAuthFailureRateLimitConfig; type FailureEntry = { count: number; resetAt: number }; @@ -86,6 +90,8 @@ export function createBoardKeyAuthFailureRateLimiter( config.sourceMaxEntries, ); const global = new BoundedFixedWindowFailures(config.globalLimit, config.windowMs, 1); + const sourceInFlight = new Map(); + let globalInFlight = 0; return { isLimited(input: { credentialId: string; sourceId: string; now?: number }) { @@ -103,10 +109,39 @@ export function createBoardKeyAuthFailureRateLimiter( return globalLimited || sourceLimited || credentialLimited; }, + tryAcquire(input: { sourceId: string }) { + const currentSourceInFlight = sourceInFlight.get(input.sourceId) ?? 0; + const newSourceWouldExceedStorage = currentSourceInFlight === 0 + && sourceInFlight.size >= config.sourceMaxEntries; + if ( + globalInFlight >= config.globalInFlightLimit + || currentSourceInFlight >= config.sourceInFlightLimit + || newSourceWouldExceedStorage + ) { + return null; + } + + globalInFlight += 1; + sourceInFlight.set(input.sourceId, currentSourceInFlight + 1); + let released = false; + return { + release() { + if (released) return; + released = true; + globalInFlight = Math.max(0, globalInFlight - 1); + const remainingForSource = (sourceInFlight.get(input.sourceId) ?? 1) - 1; + if (remainingForSource <= 0) sourceInFlight.delete(input.sourceId); + else sourceInFlight.set(input.sourceId, remainingForSource); + }, + }; + }, + reset() { credentials.clear(); sources.clear(); global.clear(); + sourceInFlight.clear(); + globalInFlight = 0; }, snapshot() { @@ -114,6 +149,8 @@ export function createBoardKeyAuthFailureRateLimiter( credentialEntries: credentials.size, sourceEntries: sources.size, globalEntries: global.size, + sourceInFlightEntries: sourceInFlight.size, + globalInFlight, }; }, };