Skip to content

Commit 54fdd4f

Browse files
committed
fix(realtime): close the table join window between re-check and commit
Moving the prior-room leave after the access re-check left Redis awaits between that check and socket.join, and superseded() only watches the join generation — so a sweep revocation landing in that window could still put a revoked socket back in the room. A synchronous cache peek immediately before the commit closes it without reintroducing the await; the authoritative resolve moments earlier wrote a fresh entry, so a differing read IS the revocation being guarded.
1 parent 9b52f63 commit 54fdd4f

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

apps/realtime/src/handlers/tables.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,33 @@ describe('setupTablesHandlers', () => {
346346
}
347347
})
348348

349+
it('aborts the join when a revocation lands during the prior-room leave', async () => {
350+
// The prior-room leave is the only await left between the authoritative access
351+
// re-check and the commit, so a sweep revocation recorded in that window must still
352+
// stop the join — `superseded()` alone only watches the join generation.
353+
const prior = { type: ROOM_TYPES.TABLE, id: 'table-prior-2' }
354+
const target = { type: ROOM_TYPES.TABLE, id: 'table-target-2' }
355+
const { socket, handlers } = createSocket({ id: 'socket-window', userId: 'user-window' })
356+
const roomManager = createRoomManager({
357+
getRoomForSocket: vi.fn().mockResolvedValue(prior),
358+
removeUserFromRoom: vi.fn().mockImplementation(async () => {
359+
// The sweep records the revocation while the prior-room leave is in flight.
360+
commitRoomPermission('user-window', target, null, beginRoomPermissionRead())
361+
return true
362+
}),
363+
})
364+
setupTablesHandlers(socket as unknown as SetupArg, roomManager)
365+
366+
await handlers[TABLE_PRESENCE_EVENTS.JOIN]({ tableId: 'table-target-2' })
367+
368+
expect(socket.emit).toHaveBeenCalledWith(
369+
TABLE_PRESENCE_EVENTS.JOIN_ERROR,
370+
expect.objectContaining({ code: 'ACCESS_DENIED', retryable: false })
371+
)
372+
expect(socket.join).not.toHaveBeenCalled()
373+
expect(roomManager.addUserToRoom).not.toHaveBeenCalled()
374+
})
375+
349376
it('drops a malformed cell selection without storing or relaying it', async () => {
350377
const { socket, handlers, toEmit } = createSocket()
351378
const roomManager = createRoomManager({

apps/realtime/src/handlers/tables.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,32 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
278278

279279
// Final re-check before the membership commit: a LEAVE or a newer JOIN enqueued during the
280280
// awaits above — including the access re-resolve — bumped the generation, or the socket
281-
// disconnected. This is the LAST await before registering, so nothing can interleave
282-
// between it and the commit.
281+
// disconnected.
283282
if (superseded()) return
284283

284+
// The prior-room leave above is the one place this handler still awaits AFTER the
285+
// authoritative access re-check (file-doc and the workspace-list rooms leave
286+
// synchronously, so they have no such window). A sweep revocation landing in that
287+
// window would otherwise let this join put a revoked socket back in the room, since
288+
// `superseded()` only watches the join generation. A cache PEEK is the right
289+
// instrument here and needs no await: the authoritative resolve moments ago wrote a
290+
// fresh entry, so the only way this reads differently is a newer decision recorded
291+
// since — exactly the revocation being guarded against. Synchronous, so nothing can
292+
// interleave between it and the join below.
293+
// `undefined` stays "unknown, not denied" here as everywhere else in this handler —
294+
// only a definitively cached insufficient permission aborts a join the authoritative
295+
// check just passed.
296+
const finalCheck = peekRoomPermission(userId, room)
297+
if (finalCheck !== undefined && !satisfiesRoomMembership(finalCheck, ROOM_TYPES.TABLE)) {
298+
socket.emit(TABLE_PRESENCE_EVENTS.JOIN_ERROR, {
299+
tableId,
300+
error: 'Access denied to table',
301+
code: 'ACCESS_DENIED',
302+
retryable: false,
303+
})
304+
return
305+
}
306+
285307
socket.join(roomName(room))
286308

287309
const presence: UserPresence = {

0 commit comments

Comments
 (0)