fix(realtime): re-resolve access at join commit instead of peeking the cache

The pre-commit recheck peeked the role cache, which reports an EXPIRED entry as
unknown and fails open — so a join stalled longer than the cache TTL could
re-enter a room the sweep had already evicted it from, including a file-doc room
where the next cold-cache frame is accepted as a durable write. All three joins
now re-resolve the way the workflow join always has; it is normally a cache hit,
since the join's own authorize just warmed it.

Author: waleedlatif1

apps/realtime/src/handlers/file-doc.test.ts

@@ -275,6 +275,43 @@ describe('setupWorkspaceFileDocHandlers', () => {
     expect(joinSuccessFileId(socket)).toBeUndefined()
   })
 
+  it('re-reads access when the cached decision expired mid-join, instead of failing open', async () => {
+    // A join stalled longer than the cache TTL: the sweep's denial is recorded with a
+    // later read ticket (so this join's own allow is correctly dropped) but has since
+    // expired. Peeking the cache would read that as "unknown" and let the socket back
+    // into the document, so the join must re-resolve against the database.
+    vi.useFakeTimers()
+    try {
+      const room = { type: ROOM_TYPES.WORKSPACE_FILE_DOC, id: 'file-stale' }
+      const { io } = createIo()
+      const { socket, handlers } = setup('socket-stale', io, { userId: 'user-stale' })
+
+      mockAuthorizeRoom.mockImplementation(async ({ action }: { action: string }) => {
+        // The authoritative current answer: access is gone.
+        if (action !== 'write')
+          return { allowed: false, status: 403, workspaceId: 'ws-1', workspacePermission: null }
+        // This join's own authorize saw the pre-revocation state, and the sweep records
+        // the revocation (later read ticket) while it is still in flight.
+        commitRoomPermission('user-stale', room, null, beginRoomPermissionRead())
+        await new Promise((resolve) => setTimeout(resolve, 31_000))
+        return { allowed: true, status: 200, workspaceId: 'ws-1', workspacePermission: 'write' }
+      })
+
+      const joining = handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-stale', clientId: 1 })
+      await vi.advanceTimersByTimeAsync(31_000)
+      await joining
+
+      expect(socket.emit).toHaveBeenCalledWith(
+        FILE_DOC_EVENTS.JOIN_ERROR,
+        expect.objectContaining({ code: 'ACCESS_DENIED', retryable: false })
+      )
+      expect(socket.join).not.toHaveBeenCalled()
+      expect(joinSuccessFileId(socket)).toBeUndefined()
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('requires write permission and reports 404 as NOT_FOUND', async () => {
     mockAuthorizeRoom.mockResolvedValue({ allowed: false, status: 404, workspacePermission: null })
     const { io } = createIo()

apps/realtime/src/handlers/file-doc.ts

@@ -1091,25 +1091,27 @@ export function setupWorkspaceFileDocHandlers(
       // awareness). Resolved here so the generation guard below also covers this await.
       const avatarUrl = await resolveAvatarUrl(socket, userId)
 
+      // Re-check access immediately before registering, mirroring the workflow join: the
+      // access re-validation sweep records a revocation BEFORE it evicts, so a join that
+      // authorized just before the revocation must not complete afterwards and re-bind
+      // the socket to the document. This RE-RESOLVES rather than peeking the cache — a
+      // peek treats an expired entry as unknown and fails open, which a join stalled
+      // longer than the cache TTL would slip straight through. Normally a cache hit (this
+      // join's own authorize just warmed it), so it costs no extra query.
+      const currentPermission = await resolveCurrentRoomPermission(userId, room, FILE_DOC_ACTION)
+      if (!satisfiesRoomMembership(currentPermission, ROOM_TYPES.WORKSPACE_FILE_DOC)) {
+        logger.warn(`User ${userId} lost write access to file ${fileId} before the join completed`)
+        emitJoinError(socket, fileId, 'Access denied to file', 'ACCESS_DENIED', false)
+        return
+      }
+
       // Abort a JOIN superseded during authorization/identity resolution: the socket
       // disconnected, or a newer JOIN (a document switch) bumped the generation. Registering
       // here would leak a dead socket's room or bind the socket to the wrong document.
+      // Last await before the commit, so nothing can interleave between the access
+      // re-check above and the registration below.
       if (socket.disconnected || joinGeneration.get(socket.id) !== generation) return
 
-      // Re-check the cached decision immediately before registering: the access
-      // re-validation sweep records a revocation BEFORE it evicts, so a join that
-      // authorized just before the revocation cannot complete afterwards and re-bind
-      // the socket to the document. `undefined` (nothing cached) is "unknown", never a
-      // denial — the authorize above is then the freshest word we have.
-      const recheck = peekRoomPermission(userId, room)
-      if (
-        recheck !== undefined &&
-        !satisfiesRoomMembership(recheck, ROOM_TYPES.WORKSPACE_FILE_DOC)
-      ) {
-        emitJoinError(socket, fileId, 'Access denied to file', 'ACCESS_DENIED', false)
-        return
-      }
-
       const entry = getOrCreateRoom(io, room)
 
       // A client id must be owned by at most one user, or a peer could bind an active

apps/realtime/src/handlers/tables.ts

@@ -260,12 +260,14 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
       // awaits above bumped the generation, or the socket disconnected. Abort before registering.
       if (superseded()) return
 
-      // Re-check the cached decision too: the access re-validation sweep records a
-      // revocation BEFORE it evicts, so a join that authorized just before the
-      // revocation cannot complete afterwards and put the socket back in the room.
-      // `undefined` (nothing cached) is "unknown", never a denial.
-      const recheck = peekRoomPermission(userId, room)
-      if (recheck !== undefined && !satisfiesRoomMembership(recheck, ROOM_TYPES.TABLE)) {
+      // Re-check access too: the access re-validation sweep records a revocation BEFORE
+      // it evicts, so a join that authorized just before the revocation must not
+      // complete afterwards and put the socket back in the room. RE-RESOLVES rather
+      // than peeking — a peek treats an expired entry as unknown and fails open, which
+      // a join stalled longer than the cache TTL would slip through. Normally a cache
+      // hit (this join's own authorize just warmed it).
+      const currentPermission = await resolveCurrentRoomPermission(userId, room, TABLE_ACTION)
+      if (!satisfiesRoomMembership(currentPermission, ROOM_TYPES.TABLE)) {
         socket.emit(TABLE_PRESENCE_EVENTS.JOIN_ERROR, {
           tableId,
           error: 'Access denied to table',

apps/realtime/src/handlers/workspace-invalidation-room.ts

@@ -3,7 +3,7 @@ import { ROOM_MEMBERSHIP_ACTIONS, satisfiesRoomMembership } from '@sim/platform-
 import { type RoomRef, type RoomType, roomName } from '@sim/realtime-protocol/rooms'
 import { resolveRoomJoinAuth } from '@/handlers/room-join-auth'
 import type { AuthenticatedSocket } from '@/middleware/auth'
-import { peekRoomPermission } from '@/middleware/permissions'
+import { resolveCurrentRoomPermission } from '@/middleware/permissions'
 import type { IRoomManager } from '@/rooms'
 
 const logger = createLogger('WorkspaceInvalidationRoom')
@@ -108,13 +108,19 @@ export function setupWorkspaceInvalidationRoom(
       // stale join can't leave the room the client has since switched to.
       if (joinGeneration !== joinAttempt || socket.disconnected) return
 
-      // Re-check the cached decision before committing: the access re-validation sweep
-      // records a revocation BEFORE it evicts, so a join that authorized just before the
+      // Re-check access before committing: the access re-validation sweep records a
+      // revocation BEFORE it evicts, so a join that authorized just before the
       // revocation must not complete afterwards and put the socket back in the room.
-      // `undefined` (nothing cached) is "unknown", never a denial — the authorize above
-      // is then the freshest word we have. Mirrors the file-doc and table joins.
-      const recheck = peekRoomPermission(socket.userId, ref)
-      if (recheck !== undefined && !satisfiesRoomMembership(recheck, roomType)) {
+      // RE-RESOLVES rather than peeking — a peek treats an expired entry as unknown and
+      // fails open, which a join stalled longer than the cache TTL would slip through.
+      // Normally a cache hit (this join's own authorize just warmed it). Mirrors the
+      // file-doc and table joins.
+      const currentPermission = await resolveCurrentRoomPermission(
+        socket.userId,
+        ref,
+        ROOM_MEMBERSHIP_ACTIONS[roomType]
+      )
+      if (!satisfiesRoomMembership(currentPermission, roomType)) {
         socket.emit(errorEvent, {
           workspaceId,
           error: 'Access denied to workspace',