Revert "fix(realtime): re-validate socket role and evict revoked collaborators"#5051
Conversation
…aborator…" This reverts commit 4ab8760.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Realtime mutating handlers ( Removed infrastructure: Associated tests for authorization, access reconciliation, and the permissions-updated route are deleted; integration mocks drop Reviewed by Cursor Bugbot for commit edd4e5f. Configure here. |
Greptile SummaryThis PR reverts #5050, which had introduced real-time socket eviction and role re-validation for revoked or downgraded collaborators. The revert removes all three enforcement layers that were added: the push-driven
Confidence Score: 3/5Merging removes all active-session enforcement for revoked and downgraded collaborators; a removed member can continue editing a workflow over their open socket until they close the tab. Both the push-eviction path (called from the member-removal and permission-update API routes) and the TTL-based per-operation re-validation are gone. The only remaining guard is the role check at socket join time, which is never refreshed mid-session. This leaves a window where access changes made via the REST API are silently not enforced on existing realtime connections. The PR description gives no rationale for the revert, so it is unclear whether a safer replacement is coming or whether the regression is intentional. apps/sim/app/api/workspaces/members/[id]/route.ts and apps/sim/app/api/workspaces/[id]/permissions/route.ts are the two places where realtime enforcement was removed; apps/realtime/src/handlers/operations.ts, subblocks.ts, and variables.ts are where per-operation re-validation was dropped.
|
| Filename | Overview |
|---|---|
| apps/realtime/src/handlers/eviction.ts | Deleted: socket eviction handler that forced a socket to leave a room and broadcast updated presence when access was revoked. |
| apps/realtime/src/handlers/operations.ts | Reverts per-operation live role re-validation back to checking only the stale in-memory cached role; removes ACCESS_REVOKED eviction path. |
| apps/realtime/src/handlers/subblocks.ts | Reverts back to cached-role-only permission check; removes ACCESS_REVOKED handling and eviction on subblock operations. |
| apps/realtime/src/handlers/variables.ts | Reverts back to cached-role-only permission check; removes ACCESS_REVOKED handling and eviction on variable operations. |
| apps/realtime/src/middleware/permissions.ts | Removes authorizeSocketOperation (TTL-based DB re-validation) and the ROLE_REVALIDATION_TTL_MS constant; only checkRolePermission (cached-only) remains. |
| apps/realtime/src/rooms/access.ts | Deleted: push-driven room reconciliation that evicted revoked users and refreshed downgraded roles across all workflow rooms in a workspace. |
| apps/realtime/src/rooms/types.ts | Removes roleCheckedAt from UserPresence and updateUserRole/handleWorkspaceAccessChange from IRoomManager interface. |
| apps/realtime/src/rooms/redis-manager.ts | Removes Lua script and implementation for updateUserRole and handleWorkspaceAccessChange; script cache slot freed. |
| apps/realtime/src/rooms/memory-manager.ts | Removes updateUserRole and handleWorkspaceAccessChange implementations from in-memory manager. |
| apps/realtime/src/routes/http.ts | Removes the POST /api/permissions-updated HTTP endpoint that the main app used to trigger realtime room reconciliation. |
| apps/sim/app/api/workspaces/[id]/permissions/route.ts | Removes push notification to realtime server after permission updates; downgraded or revoked collaborators are no longer evicted from active sessions. |
| apps/sim/app/api/workspaces/members/[id]/route.ts | Removes push notification to realtime server after member removal; deleted members retain active socket access until they disconnect. |
| apps/sim/lib/workspaces/permissions/realtime.ts | Deleted: notifyWorkspaceAccessChanged helper that POSTed to the realtime server's /api/permissions-updated endpoint. |
Sequence Diagram
sequenceDiagram
participant Admin
participant SimAPI as sim API
participant DB
participant RealtimeServer as Realtime Server
participant RevokedSocket as Revoked User Socket
Note over Admin,RevokedSocket: BEFORE revert (PR #5050 behaviour)
Admin->>SimAPI: DELETE /api/workspaces/members/:id
SimAPI->>DB: Remove member
SimAPI->>RealtimeServer: POST /api/permissions-updated
RealtimeServer->>RealtimeServer: reconcileWorkspaceAccessChange()
RealtimeServer->>RevokedSocket: emit workflow-permissions-revoked
RealtimeServer->>RealtimeServer: socket.leave(workflowId)
Note over RevokedSocket: Connection closed from room
Note over Admin,RevokedSocket: AFTER revert (current state)
Admin->>SimAPI: DELETE /api/workspaces/members/:id
SimAPI->>DB: Remove member
Note over RealtimeServer,RevokedSocket: No notification sent
RevokedSocket->>RealtimeServer: socket event (edit, subblock, variable...)
RealtimeServer->>RealtimeServer: checkRolePermission(cachedRole) ✓
RealtimeServer->>DB: Persist workflow edit
Note over RevokedSocket: User continues editing indefinitely
Reviews (1): Last reviewed commit: "Revert "fix(realtime): re-validate socke..." | Re-trigger Greptile
| } | ||
| ) | ||
|
|
||
| // Evict the removed user from any active realtime workflow rooms so their | ||
| // live read/write access ends immediately, not just on the next REST call. | ||
| await notifyWorkspaceAccessChanged(workspaceId, userId) | ||
|
|
||
| /** |
There was a problem hiding this comment.
When a member is deleted, the database row is removed but their existing WebSocket connection is never notified or closed. The removed notifyWorkspaceAccessChanged call was the only mechanism that caused the realtime server to evict those sockets. Without it, a deleted member can continue reading and writing the workflow in real-time until they reload the page or lose their connection — potentially indefinitely on a stable connection.
| }) | ||
| } | ||
|
|
||
| // Reconcile active realtime rooms so downgraded collaborators lose write | ||
| // access promptly instead of riding their cached role until disconnect. | ||
| await Promise.all( | ||
| body.updates | ||
| .filter((update) => permLookup.get(update.userId)?.permission !== update.permissions) | ||
| .map((update) => notifyWorkspaceAccessChanged(workspaceId, update.userId)) | ||
| ) | ||
|
|
||
| const updatedUsers = await getUsersWithPermissions(workspaceId) | ||
|
|
||
| for (const update of body.updates) { |
There was a problem hiding this comment.
The notifyWorkspaceAccessChanged call that triggered realtime role reconciliation has been removed. A collaborator who is downgraded from write to read (or fully removed) while connected will continue to use their old role on the socket for the life of the session. The checkRolePermission call remaining in the handlers only evaluates the in-memory cached role, which is now never refreshed after join.
Reverts #5050