fix(realtime): enforce room access continuously, not only at join (#6170)

* fix(realtime): enforce room access continuously, not only at join

File-doc, table, and workspace-list rooms authorized once at JOIN and never
again, so a member whose workspace access was revoked or downgraded kept live
collaborative write access — including durable Yjs document writes — for the
whole lifetime of an already-open socket. The access-revalidation sweep
explicitly skipped every non-workflow room.

- sweep every room type, authorizing each against its own resource
- share one membership policy (ROOM_MEMBERSHIP_ACTIONS) between the join check
  and the sweep, so a file-doc room keeps requiring write in both
- gate file-doc document frames and table cell selections on the cached
  permission, evicting on a confirmed loss of access
- re-check the cached decision before a join commits, so a join that authorized
  just before a revocation cannot re-enter the room
- never let a join's own cache write clobber a revocation recorded mid-flight
- surface room-access-revoked to clients; the file-doc editor falls back to
  read-only instead of accepting keystrokes that go nowhere

* refactor(realtime): one shared eviction path for revoked room access

The sweep and both per-frame gates each open-coded emit + leave + local-state
cleanup. Route them all through evictSocketFromRoom so they cannot diverge on
what eviction means; workflow keeps its historical access-revoked payload.

* fix(realtime): re-check access before workspace-list room joins too

The workspace-files / workspace-tables joins committed straight from their
authorize result, so a join that authorized just before a revocation could put
the socket back in a room the sweep had already evicted it from. Mirrors the
guard the file-doc and table joins already had.

* fix(realtime): order role-cache writes by read start, not write time

Two authorizations can start in one order and finish in the other, so the
decision written last can come from the older read. A join that authorized
before a revocation but returned after the sweep's denial would bury it,
handing the socket another full cache TTL of access. Every writer now takes a
monotonic ticket before it queries and yields only to a later-started read.

* chore(realtime): drop the test-only unguarded role-cache writer

Tests can express the same setup with commitRoomPermission + a read ticket, so
the cache has exactly one write path and no export without a production caller.

* fix(realtime): keep handler-initiated table eviction retryable

Evicting leaves the Socket.IO room synchronously, which is also how the sweep
discovers work — so a presence removal failing in the per-frame path could never
be retried and left a ghost collaborator until disconnect. Failed (or
unconfirmed) removals now hand off to the sweep's existing cleanup lane instead
of a second retry loop.

* 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.

* fix(realtime): keep the join generation guard after the access re-check

The access re-resolve added in the previous commit sat AFTER the generation /
superseded guard in the table and workspace-list joins, so a leave or a newer
join landing during that await no longer cancelled the stale join — it would go
on to leave the room the client had switched to and commit the abandoned one.
The guard is now the last thing before the commit in all three handlers, as it
already was for file-doc and workflow.

* test(realtime): use the shared sleep helper in the new join tests

check:utils bans the inline new Promise(setTimeout) form; the two stalled-join
tests were the only new offenders.

* fix(realtime): leave the prior table room only once the join is certain

A table switch left the previous room before the access re-check ran, so a
denial there aborted the join and left the client in no table room at all —
silently dropped from one it may still be allowed to occupy. The leave now
happens after the re-check, matching the file-doc and workspace-list joins.

* 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.

Author: waleedlatif1

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

@@ -1,25 +1,29 @@
 /**
  * @vitest-environment node
  *
- * Tests for the periodic read-access re-validation sweep. The security contract:
- * a socket is evicted only when its role resolves to `null` (a confirmed
- * revocation), and a transient failure never evicts a still-authorized socket.
+ * Tests for the periodic access re-validation sweep, which covers EVERY room type
+ * a socket occupies. The security contract: a socket is evicted only when its
+ * permission definitively fails the level that room requires (a confirmed
+ * revocation or downgrade), and a transient failure never evicts a still-authorized
+ * socket.
  */
+import { ROOM_TYPES } from '@sim/realtime-protocol/rooms'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const { mockResolveRole } = vi.hoisted(() => ({
   mockResolveRole: vi.fn(),
 }))
 
 vi.mock('@/middleware/permissions', () => ({
-  resolveCurrentWorkflowRole: mockResolveRole,
+  resolveCurrentRoomPermission: mockResolveRole,
   ROLE_REVALIDATION_TTL_MS: 30_000,
 }))
 
 import {
   ACCESS_REVALIDATION_SWEEP_INTERVAL_MS,
   startAccessRevalidationSweep,
 } from '@/access-revalidation'
+import { registerRoomEvictionHandler } from '@/handlers/room-eviction'
 import type { IRoomManager, UserPresence } from '@/rooms'
 
 interface FakeSocket {
@@ -30,9 +34,9 @@ interface FakeSocket {
   leave: ReturnType<typeof vi.fn>
 }
 
-function makeSocket(id: string, userId: string | undefined, workflowId?: string): FakeSocket {
+function makeSocket(id: string, userId: string | undefined, room?: string): FakeSocket {
   const rooms = new Set<string>([id])
-  if (workflowId) rooms.add(workflowId)
+  if (room) rooms.add(room)
   return {
     id,
     userId,
@@ -131,7 +135,7 @@ describe('access-revalidation sweep', () => {
     expect(manager.removeUserFromRoom).not.toHaveBeenCalled()
   })
 
-  it('resolves with the static safe fallback and no presence reads in the scan', async () => {
+  it('resolves with the room safe fallback and no presence reads in the scan', async () => {
     const socket = makeSocket('sock-1', 'user-1', 'wf-1')
     const manager = makeManager([socket], [{ socketId: 'sock-1', role: 'admin' }])
     mockResolveRole.mockResolvedValue('admin')
@@ -140,32 +144,123 @@ describe('access-revalidation sweep', () => {
     await sweep.runOnce()
     sweep.stop()
 
-    expect(mockResolveRole).toHaveBeenCalledWith('user-1', 'wf-1', 'read')
+    expect(mockResolveRole).toHaveBeenCalledWith('user-1', { type: 'workflow', id: 'wf-1' }, 'read')
     // The security scan must stay Redis-free — presence is never consulted.
     expect(manager.getRoomUsers).not.toHaveBeenCalled()
   })
 
-  it('never evicts a socket joined only to a non-workflow room (files/tables/file-doc)', async () => {
-    // The sweep shares one io with the files/tables/file-doc handlers. Those rooms are
-    // namespaced (`workspace-files:ws-1`, `table:t-1`), so treating every socket.rooms
-    // entry as a workflow id would resolve a bogus permission → null → evict the socket
-    // from its files/table room every pass. Non-workflow rooms must be filtered out.
+  it('sweeps non-workflow rooms against their own resource, not a bogus workflow id', async () => {
+    // The sweep shares one io with the files/tables/file-doc handlers. Their rooms are
+    // namespaced (`workspace-files:ws-1`, `table:t-1`), so each name is decoded and
+    // authorized as its own room type — the whole point of covering them at all.
     const filesSocket = makeSocket('sock-1', 'user-1', 'workspace-files:ws-1')
     const tableSocket = makeSocket('sock-2', 'user-2', 'table:t-1')
     const manager = makeManager([filesSocket, tableSocket])
-    // Even if the role resolver would say "no access", these must never be swept.
-    mockResolveRole.mockResolvedValue(null)
+    mockResolveRole.mockResolvedValue('write')
 
     const sweep = startAccessRevalidationSweep(manager)
     await sweep.runOnce()
     sweep.stop()
 
-    expect(mockResolveRole).not.toHaveBeenCalled()
+    expect(mockResolveRole).toHaveBeenCalledWith(
+      'user-1',
+      { type: 'workspace-files', id: 'ws-1' },
+      'read'
+    )
+    expect(mockResolveRole).toHaveBeenCalledWith('user-2', { type: 'table', id: 't-1' }, 'read')
+    // Still authorized: nobody is evicted.
     expect(filesSocket.leave).not.toHaveBeenCalled()
-    expect(filesSocket.emit).not.toHaveBeenCalled()
     expect(tableSocket.leave).not.toHaveBeenCalled()
-    expect(tableSocket.emit).not.toHaveBeenCalled()
+  })
+
+  it('evicts a revoked socket from a presence-free workspace-files room without touching presence', async () => {
+    const socket = makeSocket('sock-1', 'user-1', 'workspace-files:ws-1')
+    const manager = makeManager([socket])
+    mockResolveRole.mockResolvedValue(null)
+
+    const sweep = startAccessRevalidationSweep(manager)
+    await sweep.runOnce()
+    sweep.stop()
+
+    expect(socket.emit).toHaveBeenCalledWith(
+      'room-access-revoked',
+      expect.objectContaining({ room: { type: 'workspace-files', id: 'ws-1' } })
+    )
+    expect(socket.leave).toHaveBeenCalledWith('workspace-files:ws-1')
+    // These rooms hold no room-manager presence, so nothing is owed to the cleanup lane.
     expect(manager.removeUserFromRoom).not.toHaveBeenCalled()
+    expect(manager.broadcastPresenceUpdate).not.toHaveBeenCalled()
+  })
+
+  it('evicts a revoked socket from a table room and clears its presence', async () => {
+    const socket = makeSocket('sock-1', 'user-1', 'table:t-1')
+    const manager = makeManager([socket], [{ socketId: 'sock-1', role: 'read' }])
+    mockResolveRole.mockResolvedValue(null)
+
+    const sweep = startAccessRevalidationSweep(manager)
+    await sweep.runOnce()
+    sweep.stop()
+
+    expect(socket.leave).toHaveBeenCalledWith('table:t-1')
+    expect(manager.removeUserFromRoom).toHaveBeenCalledWith({ type: 'table', id: 't-1' }, 'sock-1')
+    expect(manager.broadcastPresenceUpdate).toHaveBeenCalledWith({ type: 'table', id: 't-1' })
+  })
+
+  it('evicts a file-doc socket downgraded to read, and keeps its table room', async () => {
+    // A file-doc room IS the editor and requires `write`; a table room requires only
+    // `read`. One downgraded user in both rooms must lose exactly the document.
+    const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
+    socket.rooms.add('table:t-1')
+    const manager = makeManager([socket])
+    mockResolveRole.mockResolvedValue('read')
+
+    const sweep = startAccessRevalidationSweep(manager)
+    await sweep.runOnce()
+    sweep.stop()
+
+    expect(socket.leave).toHaveBeenCalledWith('workspace-file-doc:file-1')
+    expect(socket.leave).not.toHaveBeenCalledWith('table:t-1')
+    expect(socket.emit).toHaveBeenCalledWith(
+      'room-access-revoked',
+      expect.objectContaining({ room: { type: 'workspace-file-doc', id: 'file-1' } })
+    )
+  })
+
+  it('falls back to the room type own membership level on a cold-cache failure', async () => {
+    // A static 'read' fallback would have evicted every file-doc socket (which needs
+    // `write`) the first time the DB blipped with a cold cache.
+    const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
+    const manager = makeManager([socket])
+    mockResolveRole.mockResolvedValue('write')
+
+    const sweep = startAccessRevalidationSweep(manager)
+    await sweep.runOnce()
+    sweep.stop()
+
+    expect(mockResolveRole).toHaveBeenCalledWith(
+      'user-1',
+      { type: 'workspace-file-doc', id: 'file-1' },
+      'write'
+    )
+    expect(socket.leave).not.toHaveBeenCalled()
+  })
+
+  it('runs the room type registered eviction handler so handler-local state is dropped', async () => {
+    const evicted = vi.fn()
+    registerRoomEvictionHandler(ROOM_TYPES.WORKSPACE_FILE_DOC, evicted)
+    const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1')
+    const manager = makeManager([socket])
+    mockResolveRole.mockResolvedValue(null)
+
+    const sweep = startAccessRevalidationSweep(manager)
+    await sweep.runOnce()
+    sweep.stop()
+
+    expect(evicted).toHaveBeenCalledWith(
+      'sock-1',
+      { type: 'workspace-file-doc', id: 'file-1' },
+      manager.io
+    )
   })
 
   it('evicts only the revoked socket, not co-members of the room', async () => {