Skip to content

Commit 8ae486a

Browse files
committed
fix(realtime): harden seeder recovery, join-generation, and misc robustness
Final line-by-line audit follow-ups (all LOW/MED, no P0/P1): - file-doc: a sole client whose seed FETCH fails was added to triedSeeders, re-election found nobody, and the document stayed permanently empty until reload. Re-offer seeding a bounded number of rounds (MAX_SEED_ROUNDS) before giving up. Also bound clientId to a non-negative integer (it is an ownership key). - tables + workspace-files: validate the room id BEFORE advancing joinGeneration, so a malformed/rejected join can't cancel a legitimate in-flight join. - workflow + tables + workspace-files: suppress the client-facing join error when the op was already superseded (a retryable error naming the abandoned room could make a client re-join and cancel its newer join). The rollback still runs. - redis-manager: set isConnected=true only after scriptLoad succeeds (and reset it on failure) so isReady() can't report ready while the Lua SHAs are null. - connection: apply the presence-bearing filter to the manager-removed set too (symmetry with the fallback path). - http.ts: validate workflowId on the four workflow endpoints (matching the files one). - client: clear a pending join-retry timer before rescheduling (reconnect churn no longer orphans a stray extra join); clear caret fade timers on plugin destroy; seed-effect cleanup reports NOT-ready (safe direction). - Tests: bounded seeder recovery, cell-selection strip-junk, TABLE round-trip, presenceEventName. Gates: tsc (sim/realtime/packages) 0, 208 realtime + 12 protocol tests, biome, api-validation, boundaries, prune.
1 parent a694f75 commit 8ae486a

14 files changed

Lines changed: 197 additions & 47 deletions

File tree

apps/realtime/src/handlers/connection.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,13 @@ export function setupConnectionHandlers(socket: AuthenticatedSocket, roomManager
5454
// rooms empty). Attempt removal for any room the manager didn't already
5555
// remove — best-effort, since a transient Redis error can't be recovered here.
5656
const wasInRooms = new Map<string, RoomRef>()
57-
for (const room of removedRooms) wasInRooms.set(roomName(room), room)
57+
// Only presence-bearing rooms get a corrective broadcast. Manager-removed rooms are
58+
// presence-bearing by construction today (only workflow/table write the socket→room hash),
59+
// but filter symmetrically with the fallback path below so a future room type that ever
60+
// tracks presence here can't emit a bogus presence-update no client listens to.
61+
for (const room of removedRooms) {
62+
if (PRESENCE_BEARING_TYPES.has(room.type)) wasInRooms.set(roomName(room), room)
63+
}
5864
for (const name of liveRoomNames) {
5965
// `wasInRooms.has(name)` already excludes every room the manager removed (same
6066
// room-name key via the roomName/parseRoomName bijection). Skip room types with no

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,22 @@ describe('setupWorkspaceFileDocHandlers', () => {
665665
expect(sent.find((m) => m.event === FILE_DOC_EVENTS.SEED_REQUEST)?.target).toBe('socket-b')
666666
})
667667

668+
it('re-offers seeding to a sole client that missed the deadline, then gives up after a bound', async () => {
669+
const { io, sent } = createIo()
670+
const a = setup('socket-a', io)
671+
await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
672+
expect(sent.filter((m) => m.event === FILE_DOC_EVENTS.SEED_REQUEST).length).toBe(1)
673+
674+
// The sole client keeps missing the deadline. Without recovery it would be permanently excluded
675+
// (empty document forever); instead it is re-offered a bounded number of rounds.
676+
for (let i = 0; i < 6; i++) vi.advanceTimersByTime(10_000)
677+
678+
// Initial offer + MAX_SEED_ROUNDS (3) re-offers = 4 total, all to socket-a; then it stops.
679+
const requests = sent.filter((m) => m.event === FILE_DOC_EVENTS.SEED_REQUEST)
680+
expect(requests.length).toBe(4)
681+
expect(requests.every((m) => m.target === 'socket-a')).toBe(true)
682+
})
683+
668684
it('cancels the seed deadline once the document is seeded', async () => {
669685
const { io, sent } = createIo()
670686
const a = setup('socket-a', io)

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,16 @@ const logger = createLogger('FileDocHandlers')
5050
*/
5151
const SEED_DEADLINE_MS = 10_000
5252

53+
/**
54+
* Max times the room re-offers seeding to its owners after every one of them has
55+
* missed a deadline. Without this, a sole client whose seed *fetch fails* (not just
56+
* slow) is added to `triedSeeders`, re-election finds no un-tried owner, and the
57+
* document is left permanently empty until that client reloads. Each exhausted round
58+
* clears `triedSeeders` and re-offers, giving a transient failure a bounded number of
59+
* retries (~MAX × deadline) before the room genuinely gives up.
60+
*/
61+
const MAX_SEED_ROUNDS = 3
62+
5363
/** A socket's presence ownership within a room. */
5464
interface FileDocOwner {
5565
/**
@@ -81,6 +91,10 @@ interface FileDocRoom {
8191
/** Sockets that were elected but failed to seed within the deadline; skipped
8292
* on re-election so a single stuck/withholding client can't block the room. */
8393
triedSeeders: Set<string>
94+
/** Count of full re-offer rounds spent after every owner missed a deadline;
95+
* bounds the retry loop (see {@link MAX_SEED_ROUNDS}) so a permanently-broken
96+
* seed doesn't reset forever. */
97+
seedRounds: number
8498
}
8599

86100
/** Live documents keyed by Socket.IO room name. Module-global: one Y.Doc per file. */
@@ -222,6 +236,19 @@ function electSeederIfNeeded(io: Server, room: FileDocRoom) {
222236
room.triedSeeders.add(elected)
223237
room.seederSocketId = null
224238
electSeederIfNeeded(io, room)
239+
// If that left no seeder (every current owner has now been tried) while the doc is still
240+
// unseeded and owners remain, re-offer the whole room a bounded number of times — otherwise a
241+
// sole client whose seed fetch failed leaves the document permanently empty.
242+
if (
243+
room.seederSocketId === null &&
244+
!isDocSeeded(room.doc) &&
245+
room.owners.size > 0 &&
246+
room.seedRounds < MAX_SEED_ROUNDS
247+
) {
248+
room.seedRounds += 1
249+
room.triedSeeders.clear()
250+
electSeederIfNeeded(io, room)
251+
}
225252
}, SEED_DEADLINE_MS)
226253
}
227254

@@ -248,6 +275,7 @@ function getOrCreateRoom(io: Server, ref: RoomRef): FileDocRoom {
248275
seederSocketId: null,
249276
seedTimer: null,
250277
triedSeeders: new Set(),
278+
seedRounds: 0,
251279
}
252280
fileDocRooms.set(name, room)
253281

@@ -418,7 +446,14 @@ export function setupWorkspaceFileDocHandlers(
418446
emitJoinError(socket, fileId, 'Realtime unavailable', 'ROOM_MANAGER_UNAVAILABLE', true)
419447
return
420448
}
421-
if (typeof fileId !== 'string' || fileId.length === 0 || typeof clientId !== 'number') {
449+
if (
450+
typeof fileId !== 'string' ||
451+
fileId.length === 0 ||
452+
// A Yjs clientID is a uint32; reject NaN/Infinity/negative/non-integer so a malformed id
453+
// can't become a bogus ownership key.
454+
!Number.isInteger(clientId) ||
455+
clientId < 0
456+
) {
422457
emitJoinError(socket, fileId, 'Invalid join payload', 'INVALID_PAYLOAD', false)
423458
return
424459
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,37 @@ describe('setupTablesHandlers', () => {
187187
expect(toEmit).not.toHaveBeenCalled()
188188
})
189189

190+
it('strips unknown/oversized fields from an otherwise-valid selection before storing or relaying', async () => {
191+
const { socket, handlers, toEmit } = createSocket()
192+
const roomManager = createRoomManager({
193+
getRoomForSocket: vi.fn().mockResolvedValue(TABLE_ROOM),
194+
})
195+
setupTablesHandlers(socket as unknown as SetupArg, roomManager)
196+
197+
await handlers[TABLE_PRESENCE_EVENTS.CELL_SELECTION]({
198+
cell: {
199+
anchor: { rowId: 'row-1', columnId: 'col-a', junk: 'x'.repeat(10_000) },
200+
focus: { rowId: 'row-1', columnId: 'col-a' },
201+
editing: true,
202+
bloat: 'x'.repeat(100_000),
203+
},
204+
})
205+
206+
// Only the whitelisted fields survive — a hostile peer can't amplify an oversized object.
207+
const expected = {
208+
anchor: { rowId: 'row-1', columnId: 'col-a' },
209+
focus: { rowId: 'row-1', columnId: 'col-a' },
210+
editing: true,
211+
}
212+
expect(roomManager.updateUserActivity).toHaveBeenCalledWith(TABLE_ROOM, 'socket-1', {
213+
cell: expected,
214+
})
215+
expect(toEmit).toHaveBeenCalledWith(TABLE_PRESENCE_EVENTS.CELL_SELECTION, {
216+
socketId: 'socket-1',
217+
cell: expected,
218+
})
219+
})
220+
190221
it('skips a superseded queued join on a fast table switch', async () => {
191222
const { socket, handlers } = createSocket()
192223
const roomManager = createRoomManager()

apps/realtime/src/handlers/tables.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,17 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
7878
let opChain: Promise<void> = Promise.resolve()
7979

8080
socket.on(TABLE_PRESENCE_EVENTS.JOIN, ({ tableId, tabSessionId }: JoinTablePayload) => {
81+
// Validate the id BEFORE claiming a generation, so a malformed join can't advance
82+
// joinGeneration and cancel a legitimate in-flight join for another table.
83+
if (typeof tableId !== 'string' || tableId.length === 0) {
84+
socket.emit(TABLE_PRESENCE_EVENTS.JOIN_ERROR, {
85+
tableId: typeof tableId === 'string' ? tableId : '',
86+
error: 'Invalid table id',
87+
code: 'INVALID_PAYLOAD',
88+
retryable: false,
89+
})
90+
return
91+
}
8192
const joinAttempt = (joinGeneration += 1)
8293
currentTableId = tableId
8394
opChain = opChain
@@ -119,17 +130,6 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
119130
return
120131
}
121132

122-
// Validate the client-supplied id before it reaches the DB query.
123-
if (typeof tableId !== 'string' || tableId.length === 0) {
124-
socket.emit(TABLE_PRESENCE_EVENTS.JOIN_ERROR, {
125-
tableId: typeof tableId === 'string' ? tableId : '',
126-
error: 'Invalid table id',
127-
code: 'INVALID_PAYLOAD',
128-
retryable: false,
129-
})
130-
return
131-
}
132-
133133
const room = tableRoom(tableId)
134134

135135
const authorized = await resolveRoomJoinAuth({
@@ -244,6 +244,10 @@ export function setupTablesHandlers(socket: AuthenticatedSocket, roomManager: IR
244244
// Best-effort rollback — the original join failure is the one surfaced below, so a
245245
// secondary cleanup error must not mask it or throw out of the error handler.
246246
}
247+
// Suppress the client-facing error when this join was already superseded: the client has moved
248+
// to a newer table, and a retryable error naming the abandoned one could make it re-join and
249+
// supersede the newer join. The rollback above still runs.
250+
if (superseded()) return
247251
socket.emit(TABLE_PRESENCE_EVENTS.JOIN_ERROR, {
248252
tableId,
249253
error: 'Failed to join table',

apps/realtime/src/handlers/workflow.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,10 @@ export function setupWorkflowHandlers(socket: AuthenticatedSocket, roomManager:
266266
// room state, never the newer op's.
267267
socket.leave(workflowId)
268268
await roomManager.removeUserFromRoom(wf(workflowId), socket.id)
269+
// Suppress the client-facing error when this join was already superseded: the client has moved
270+
// to a newer workflow, and a retryable error naming the abandoned one could make it re-join and
271+
// supersede the newer join (an A/B flicker). The rollback above still runs.
272+
if (superseded()) return
269273
const isReady = roomManager.isReady()
270274
socket.emit('join-workflow-error', {
271275
workflowId,

apps/realtime/src/handlers/workspace-files.ts

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -44,41 +44,43 @@ export function setupWorkspaceFilesHandlers(
4444
let currentWorkspace: string | null = null
4545

4646
socket.on('join-workspace-files', async ({ workspaceId }: JoinPayload) => {
47-
const joinAttempt = (joinGeneration += 1)
48-
currentWorkspace = workspaceId
49-
try {
50-
if (!socket.userId || !socket.userName) {
51-
socket.emit('join-workspace-files-error', {
52-
workspaceId,
53-
error: 'Authentication required',
54-
code: 'AUTHENTICATION_REQUIRED',
55-
retryable: false,
56-
})
57-
return
58-
}
47+
// Validate synchronously BEFORE claiming a generation, so a rejected/malformed join can't
48+
// advance joinGeneration and cancel a legitimate in-flight join for another workspace.
49+
if (!socket.userId || !socket.userName) {
50+
socket.emit('join-workspace-files-error', {
51+
workspaceId,
52+
error: 'Authentication required',
53+
code: 'AUTHENTICATION_REQUIRED',
54+
retryable: false,
55+
})
56+
return
57+
}
5958

60-
if (!roomManager.isReady()) {
61-
socket.emit('join-workspace-files-error', {
62-
workspaceId,
63-
error: 'Realtime unavailable',
64-
code: 'ROOM_MANAGER_UNAVAILABLE',
65-
retryable: true,
66-
})
67-
return
68-
}
59+
if (!roomManager.isReady()) {
60+
socket.emit('join-workspace-files-error', {
61+
workspaceId,
62+
error: 'Realtime unavailable',
63+
code: 'ROOM_MANAGER_UNAVAILABLE',
64+
retryable: true,
65+
})
66+
return
67+
}
6968

70-
// Validate the client-supplied id before it reaches the DB query (join payloads are
71-
// otherwise raw client input).
72-
if (typeof workspaceId !== 'string' || workspaceId.length === 0) {
73-
socket.emit('join-workspace-files-error', {
74-
workspaceId: typeof workspaceId === 'string' ? workspaceId : '',
75-
error: 'Invalid workspace id',
76-
code: 'INVALID_PAYLOAD',
77-
retryable: false,
78-
})
79-
return
80-
}
69+
// Validate the client-supplied id before it reaches the DB query (join payloads are
70+
// otherwise raw client input) and before advancing the generation.
71+
if (typeof workspaceId !== 'string' || workspaceId.length === 0) {
72+
socket.emit('join-workspace-files-error', {
73+
workspaceId: typeof workspaceId === 'string' ? workspaceId : '',
74+
error: 'Invalid workspace id',
75+
code: 'INVALID_PAYLOAD',
76+
retryable: false,
77+
})
78+
return
79+
}
8180

81+
const joinAttempt = (joinGeneration += 1)
82+
currentWorkspace = workspaceId
83+
try {
8284
const room = filesRoom(workspaceId)
8385

8486
const authorized = await resolveRoomJoinAuth({
@@ -115,6 +117,10 @@ export function setupWorkspaceFilesHandlers(
115117
try {
116118
socket.leave(roomName(filesRoom(workspaceId)))
117119
} catch {}
120+
// Suppress the client-facing error when this join was already superseded: the client has
121+
// switched to a newer workspace, and a retryable error naming the abandoned one could make it
122+
// re-join and cancel the newer join. The leave above still runs.
123+
if (joinGeneration !== joinAttempt || socket.disconnected) return
118124
socket.emit('join-workspace-files-error', {
119125
workspaceId,
120126
error: 'Failed to join workspace files',

apps/realtime/src/rooms/redis-manager.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,18 @@ export class RedisRoomManager implements IRoomManager {
168168

169169
try {
170170
await this.redis.connect()
171-
this.isConnected = true
172171

173172
this.removeRoomScriptSha = await this.redis.scriptLoad(REMOVE_ROOM_SCRIPT)
174173
this.updateActivityScriptSha = await this.redis.scriptLoad(UPDATE_ACTIVITY_SCRIPT)
175174

175+
// Mark ready only after the scripts load — isReady() gates removeUserFromRoom/updateUserActivity,
176+
// which silently no-op without a script SHA. Setting the flag before scriptLoad would make
177+
// isReady() lie if scriptLoad threw.
178+
this.isConnected = true
179+
176180
logger.info('RedisRoomManager connected to Redis and scripts loaded')
177181
} catch (error) {
182+
this.isConnected = false
178183
logger.error('Failed to connect to Redis:', error)
179184
throw error
180185
}

apps/realtime/src/routes/http.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ function readRequestBody(req: IncomingMessage): Promise<string> {
4242
})
4343
}
4444

45+
function isNonEmptyString(value: unknown): value is string {
46+
return typeof value === 'string' && value.length > 0
47+
}
48+
4549
function sendSuccess(res: ServerResponse): void {
4650
res.writeHead(200, { 'Content-Type': 'application/json' })
4751
res.end(JSON.stringify({ success: true }))
@@ -102,6 +106,7 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
102106
try {
103107
const body = await readRequestBody(req)
104108
const { workflowId } = JSON.parse(body)
109+
if (!isNonEmptyString(workflowId)) return sendError(res, 'Invalid workflowId', 400)
105110
await workflowRoomService.handleWorkflowDeletion(workflowId)
106111
sendSuccess(res)
107112
} catch (error) {
@@ -116,6 +121,7 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
116121
try {
117122
const body = await readRequestBody(req)
118123
const { workflowId } = JSON.parse(body)
124+
if (!isNonEmptyString(workflowId)) return sendError(res, 'Invalid workflowId', 400)
119125
await workflowRoomService.handleWorkflowUpdate(workflowId)
120126
sendSuccess(res)
121127
} catch (error) {
@@ -130,6 +136,7 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
130136
try {
131137
const body = await readRequestBody(req)
132138
const { workflowId } = JSON.parse(body)
139+
if (!isNonEmptyString(workflowId)) return sendError(res, 'Invalid workflowId', 400)
133140
await workflowRoomService.handleWorkflowDeployed(workflowId)
134141
sendSuccess(res)
135142
} catch (error) {
@@ -144,6 +151,7 @@ export function createHttpHandler(roomManager: IRoomManager, logger: Logger) {
144151
try {
145152
const body = await readRequestBody(req)
146153
const { workflowId, timestamp } = JSON.parse(body)
154+
if (!isNonEmptyString(workflowId)) return sendError(res, 'Invalid workflowId', 400)
147155
await workflowRoomService.handleWorkflowRevert(workflowId, timestamp)
148156
sendSuccess(res)
149157
} catch (error) {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,17 @@ export function createCaretActivityExtension(awareness: Awareness): Extension {
135135
destroy: () => {
136136
awareness.off('change', onChange)
137137
if (raf) cancelAnimationFrame(raf)
138+
// Clear any pending fade timers for this editor's carets so they don't fire on
139+
// detached nodes after unmount (harmless no-op, but a genuine leaked timer).
140+
for (const caret of editorView.dom.querySelectorAll<HTMLElement>(
141+
'.collaboration-carets__caret'
142+
)) {
143+
const timer = caretFadeTimers.get(caret)
144+
if (timer) {
145+
clearTimeout(timer)
146+
caretFadeTimers.delete(caret)
147+
}
148+
}
138149
},
139150
}
140151
},

0 commit comments

Comments
 (0)