Skip to content

Commit ead74af

Browse files
icecrasher321claude
andcommitted
fix(realtime): bound authorization waits in the revocation scan
Race each socket's authorization check against a per-socket timeout and cap the whole pass with a budget below the sweep interval, so a hanging DB query skips that socket for the pass (never evicting on uncertainty) instead of wedging the scan lane and starving subsequent ticks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4db1756 commit ead74af

2 files changed

Lines changed: 82 additions & 2 deletions

File tree

apps/realtime/src/access-revalidation.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,41 @@ describe('access-revalidation sweep', () => {
284284
expect(manager.broadcastPresenceUpdate).not.toHaveBeenCalled()
285285
})
286286

287+
it('skips a socket whose authorization query hangs and still evicts the rest', async () => {
288+
vi.useFakeTimers()
289+
try {
290+
const hung = makeSocket('sock-1', 'user-1', 'wf-1')
291+
const revoked = makeSocket('sock-2', 'user-2', 'wf-1')
292+
const manager = makeManager([hung, revoked])
293+
// user-1's authorization query hangs (wedged DB connection); user-2's
294+
// resolves to a confirmed revocation.
295+
mockResolveRole.mockImplementation(async (userId: string) => {
296+
if (userId === 'user-1') return new Promise(() => {})
297+
return null
298+
})
299+
300+
const sweep = startAccessRevalidationSweep(manager)
301+
302+
// First tick starts the scan; the per-socket timeout fires at +5s and the
303+
// scan moves on to evict the revoked socket in the same pass.
304+
await vi.advanceTimersByTimeAsync(ACCESS_REVALIDATION_SWEEP_INTERVAL_MS)
305+
await vi.advanceTimersByTimeAsync(10_000)
306+
307+
expect(hung.leave).not.toHaveBeenCalled()
308+
expect(revoked.leave).toHaveBeenCalledWith('wf-1')
309+
310+
// The next tick's scan still runs — the hung query did not wedge the lane.
311+
const callsAfterFirstPass = mockResolveRole.mock.calls.length
312+
await vi.advanceTimersByTimeAsync(ACCESS_REVALIDATION_SWEEP_INTERVAL_MS)
313+
await vi.advanceTimersByTimeAsync(10_000)
314+
sweep.stop()
315+
316+
expect(mockResolveRole.mock.calls.length).toBeGreaterThan(callsAfterFirstPass)
317+
} finally {
318+
vi.useRealTimers()
319+
}
320+
})
321+
287322
it('keeps scanning on later ticks while a deferred cleanup hangs', async () => {
288323
vi.useFakeTimers()
289324
try {

apps/realtime/src/access-revalidation.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import type { AccessRevokedBroadcast } from '@sim/realtime-protocol/events'
3+
import { sleep } from '@sim/utils/helpers'
34
import type { AuthenticatedSocket } from '@/middleware/auth'
45
import { ROLE_REVALIDATION_TTL_MS, resolveCurrentWorkflowRole } from '@/middleware/permissions'
56
import type { IRoomManager } from '@/rooms'
@@ -28,6 +29,25 @@ export const ACCESS_REVALIDATION_SWEEP_INTERVAL_MS = ROLE_REVALIDATION_TTL_MS
2829
*/
2930
const FALLBACK_ROLE = 'read'
3031

32+
/**
33+
* Upper bound on a single socket's authorization check inside the scan. A DB
34+
* query that hangs (wedged connection, exhausted pool, network partition) must
35+
* not wedge the scan lane — on timeout the socket is skipped for this pass
36+
* (never evicted) and re-checked next pass, where the single-flighted
37+
* resolution is re-raced and acted on once it finally settles.
38+
*/
39+
const SCAN_SOCKET_TIMEOUT_MS = 5_000
40+
41+
/**
42+
* Hard budget for one whole scan pass, deliberately below
43+
* {@link ACCESS_REVALIDATION_SWEEP_INTERVAL_MS} so `scanRunning` can never
44+
* starve subsequent ticks: a pass that runs out of budget ends early and the
45+
* remaining sockets are evaluated on the next pass.
46+
*/
47+
const SCAN_PASS_BUDGET_MS = 20_000
48+
49+
const SCAN_TIMED_OUT = Symbol('scan-timed-out')
50+
3151
export interface AccessRevalidationSweep {
3252
/** Stop the periodic sweep (clears the interval). */
3353
stop: () => void
@@ -75,7 +95,10 @@ function collectLocalMemberships(io: IRoomManager['io']): Map<string, Authentica
7595
* best-effort room-state cleanup (Redis presence) runs in its own lane. A Redis
7696
* outage — including commands that hang in the client's offline queue rather
7797
* than failing — can therefore stall only presence cleanup, never revocation
78-
* enforcement on subsequent ticks.
98+
* enforcement on subsequent ticks. Within the scan, every authorization wait is
99+
* bounded ({@link SCAN_SOCKET_TIMEOUT_MS}) and the whole pass has a hard budget
100+
* below the interval ({@link SCAN_PASS_BUDGET_MS}), so a hanging DB query can
101+
* delay a socket's re-check but can never wedge the scan lane itself.
79102
*/
80103
export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessRevalidationSweep {
81104
const io = roomManager.io
@@ -177,14 +200,36 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR
177200

178201
async function scanMemberships(): Promise<void> {
179202
const memberships = collectLocalMemberships(io)
203+
const deadline = Date.now() + SCAN_PASS_BUDGET_MS
180204

181205
for (const [workflowId, sockets] of memberships) {
182206
for (const socket of sockets) {
183207
const userId = socket.userId
184208
if (!userId) continue
185209

210+
const remainingBudget = deadline - Date.now()
211+
if (remainingBudget <= 0) {
212+
logger.warn(
213+
'Access re-validation scan budget exhausted; remaining sockets defer to the next pass'
214+
)
215+
return
216+
}
217+
186218
try {
187-
const role = await resolveCurrentWorkflowRole(userId, workflowId, FALLBACK_ROLE)
219+
// Bounded wait: a hanging authorization query skips this socket for
220+
// the pass instead of wedging the scan lane. The single-flighted
221+
// resolution keeps running in the background and is re-raced next
222+
// pass, so it is acted on once it settles.
223+
const role = await Promise.race([
224+
resolveCurrentWorkflowRole(userId, workflowId, FALLBACK_ROLE),
225+
sleep(Math.min(SCAN_SOCKET_TIMEOUT_MS, remainingBudget)).then(() => SCAN_TIMED_OUT),
226+
])
227+
if (role === SCAN_TIMED_OUT) {
228+
logger.warn(
229+
`Authorization check timed out for user ${userId} on workflow ${workflowId}; skipping this pass`
230+
)
231+
continue
232+
}
188233
if (role === null) {
189234
revokeSocket(socket, workflowId)
190235
}

0 commit comments

Comments
 (0)