Skip to content

Commit 0df7b5c

Browse files
committed
fix(file-doc): stop the permission resend loop and restore the 10 MB proxy body limit
1 parent 7ab2eef commit 0df7b5c

26 files changed

Lines changed: 365 additions & 357 deletions

File tree

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,50 @@ describe('setupWorkspaceFileDocHandlers', () => {
642642
}
643643
})
644644

645+
it('publishes write access to a socket only when it changes, not on every frame', async () => {
646+
// A distinct user/file: role decisions recorded under fake timers outlive this test.
647+
vi.useFakeTimers()
648+
try {
649+
const { io } = createIo()
650+
const { socket, handlers } = setup('socket-capability', io, { userId: 'user-capability' })
651+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-capability', clientId: 7 })
652+
await vi.advanceTimersByTimeAsync(0)
653+
const presence = awarenessFrame(7, 'Capability').frame
654+
const sendFrames = async () => {
655+
for (let sent = 0; sent < 3; sent++) {
656+
void handlers[FILE_DOC_EVENTS.MESSAGE](presence)
657+
await vi.advanceTimersByTimeAsync(0)
658+
}
659+
}
660+
const published = () =>
661+
socket.emit.mock.calls
662+
.filter(([event]) => event === FILE_DOC_EVENTS.PERMISSION)
663+
.map(([, payload]) => (payload as { canWrite: boolean }).canWrite)
664+
const changeRole = async (workspacePermission: 'read' | 'write') => {
665+
mockAuthorizeRoom.mockResolvedValue({
666+
allowed: true,
667+
status: 200,
668+
workspaceId: 'ws-1',
669+
workspacePermission,
670+
})
671+
await vi.advanceTimersByTimeAsync(ROLE_REVALIDATION_TTL_MS + 1_000)
672+
}
673+
674+
await sendFrames()
675+
expect(published()).toEqual([true])
676+
677+
await changeRole('read')
678+
await sendFrames()
679+
expect(published()).toEqual([true, false])
680+
681+
await changeRole('write')
682+
await sendFrames()
683+
expect(published()).toEqual([true, false, true])
684+
} finally {
685+
vi.useRealTimers()
686+
}
687+
})
688+
645689
it('applies + fans out an agent-streamed frame (SYNC_NO_PERSIST) but never persists it', async () => {
646690
mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server'))
647691
const { io, sent } = createIo()

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ interface FileDocRoom {
150150
/** socketId → (clientId → its presence ownership). A socket owns one entry per collaborative provider
151151
* it mounted for this file (see {@link FileDocPresenceOwner}); an empty inner map is never kept. */
152152
owners: Map<string, Map<number, FileDocPresenceOwner>>
153+
/** socketId → the write capability last sent to it, so per-frame checks publish only a change. */
154+
publishedCanWrite: Map<string, boolean>
153155
/**
154156
* The in-flight server seed for this room, or `null`. Concurrent joins await THIS promise rather
155157
* than each starting a fetch — and, unlike a "started" boolean, awaiting it is what lets a second
@@ -1043,6 +1045,7 @@ function getOrCreateRoom(io: Server, target: FileDocTarget): FileDocRoom {
10431045
doc,
10441046
awareness,
10451047
owners: new Map(),
1048+
publishedCanWrite: new Map(),
10461049
seeding: null,
10471050
lastEditorUserId: null,
10481051
edited: false,
@@ -1161,11 +1164,15 @@ function resolveFileDocPermission(
11611164
const room = fileDocRoom(document)
11621165
const accept = (permission: string | null) => {
11631166
if (socketToRoomName.get(socket.id) !== name || fileDocRooms.get(name) !== document) return null
1164-
socket.emit(FILE_DOC_EVENTS.PERMISSION, {
1165-
fileId: document.fileId,
1166-
...fileDocOwnerWireFields(document.owner),
1167-
canWrite: permission === 'write' || permission === 'admin',
1168-
} satisfies FileDocPermission)
1167+
const canWrite = permission === 'write' || permission === 'admin'
1168+
if (document.publishedCanWrite.get(socket.id) !== canWrite) {
1169+
document.publishedCanWrite.set(socket.id, canWrite)
1170+
socket.emit(FILE_DOC_EVENTS.PERMISSION, {
1171+
fileId: document.fileId,
1172+
...fileDocOwnerWireFields(document.owner),
1173+
canWrite,
1174+
} satisfies FileDocPermission)
1175+
}
11691176
if (permission === null)
11701177
evictSocketFromRoom(socket, room, 'Your access to this document has been revoked', io)
11711178
return permission
@@ -1216,7 +1223,16 @@ async function handleMessage(socket: AuthenticatedSocket, io: Server, data: unkn
12161223
}
12171224
const decoder = decoding.createDecoder(bytes)
12181225
const messageType = decoding.readVarUint(decoder)
1219-
const permission = await resolveFileDocPermission(socket, io, name)
1226+
let permission: string | null
1227+
try {
1228+
permission = await resolveFileDocPermission(socket, io, name)
1229+
} catch (error) {
1230+
logger.warn('Dropping file-doc frame while its authorization is unavailable', {
1231+
socketId: socket.id,
1232+
error,
1233+
})
1234+
return
1235+
}
12201236
if (permission === null) return
12211237
const canWrite = permission === 'write' || permission === 'admin'
12221238
if (messageType === FILE_DOC_MESSAGE_TYPE.SYNC) {
@@ -1460,6 +1476,7 @@ export function cleanupFileDocForSocket(socketId: string, io: Server, endOfLife
14601476
// caret via its awareness removal.
14611477
const clientMap = room.owners.get(socketId)
14621478
room.owners.delete(socketId)
1479+
room.publishedCanWrite.delete(socketId)
14631480
clearFileDocPresence(io, socketId, name)
14641481
if (clientMap !== undefined && clientMap.size > 0) {
14651482
// Fires the awareness `update` handler with a non-socket origin → the removals
@@ -1788,6 +1805,7 @@ export function setupWorkspaceFileDocHandlers(
17881805
awarenessProtocol.removeAwarenessStates(entry.awareness, [clientId], null)
17891806
if (clientMap.size === 0) {
17901807
entry.owners.delete(otherSid)
1808+
entry.publishedCanWrite.delete(otherSid)
17911809
socketToRoomName.delete(otherSid)
17921810
clearFileDocPresence(io, otherSid, name)
17931811
io.in(otherSid).socketsLeave(name)
@@ -1832,6 +1850,8 @@ export function setupWorkspaceFileDocHandlers(
18321850
roomName: name,
18331851
user: { socketId: socket.id, userId, userName, avatarUrl },
18341852
}
1853+
/** Co-mounted providers learn from the next frame whether this admission changed access. */
1854+
entry.publishedCanWrite.delete(socket.id)
18351855

18361856
// Name the document this room holds, so a client that still carries a DIFFERENT one (its room
18371857
// outlived by a document rebuilt in its place) can refuse to merge instead of unioning two
Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
import { projectFileDocPersistContract } from '@/lib/api/contracts/project-file-doc'
2+
import { isOffAppHost } from '@/lib/api/mcp/host-routing'
23
import {
34
defineInternalJsonRoute,
45
internalOrchestrationErrorPolicy,
56
internalRateLimits,
67
} from '@/lib/api/server/routes'
78
import { realtimeProjectFileAuth } from '@/lib/auth/realtime-file-delegation'
9+
import { MAX_COLLAB_DOC_STATE_BYTES } from '@/lib/collab-doc/state'
10+
import { OrchestrationError } from '@/lib/core/orchestration/types'
811
import { persistProjectFileDoc } from '@/lib/projects/files/application/documents'
912

13+
/** A base64-encoded maximum snapshot plus 1 MiB for its JSON envelope (17 MiB). */
14+
const PERSIST_MAX_BODY_BYTES = Math.ceil(MAX_COLLAB_DOC_STATE_BYTES / 3) * 4 + 1024 * 1024
15+
1016
export const POST = defineInternalJsonRoute({
1117
contract: projectFileDocPersistContract,
1218
operation: persistProjectFileDoc.operation,
@@ -15,11 +21,15 @@ export const POST = defineInternalJsonRoute({
1521
reason: 'Authenticated realtime relay; socket frame admission bounds collaborative traffic',
1622
}),
1723
errorPolicy: internalOrchestrationErrorPolicy,
24+
/** Excluded from the proxy matcher, so the route refuses the dedicated MCP host itself. */
25+
beforeParse: ({ request }) => {
26+
if (isOffAppHost(request)) throw new OrchestrationError('not_found', 'Not found')
27+
},
1828
mapInput: ({ params, body }) => ({
1929
...params,
2030
docState: new Uint8Array(Buffer.from(body.docState, 'base64')),
2131
expectedVersion: body.expectedVersion,
2232
}),
2333
useCase: persistProjectFileDoc,
24-
parseOptions: { maxBodyBytes: 17 * 1024 * 1024 },
34+
parseOptions: { maxBodyBytes: PERSIST_MAX_BODY_BYTES, bypassesProxy: true },
2535
})

‎apps/sim/app/api/v2/files/[fileId]/route.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,25 @@ describe('v2 single-file routes', () => {
168168
)
169169
})
170170

171+
it('downloads an attachment with its stored type and name, without sniffing', async () => {
172+
mocks.download.mockResolvedValueOnce({
173+
file: fileRecord({ name: 'notes' }),
174+
stream: new Blob(['<p>notes</p>']).stream(),
175+
contentType: 'text/html',
176+
contentLength: 12,
177+
})
178+
179+
const response = await GET(
180+
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`),
181+
context
182+
)
183+
184+
expect(response.status).toBe(200)
185+
expect(response.headers.get('Content-Type')).toBe('text/html')
186+
expect(response.headers.get('Content-Disposition')).toBe('attachment; filename="notes"')
187+
expect(response.headers.get('X-Content-Type-Options')).toBe('nosniff')
188+
})
189+
171190
it('conceals cross-workspace download authorization', async () => {
172191
mocks.download.mockRejectedValue(new NoWorkspaceAccessError())
173192

‎apps/sim/app/api/v2/files/[fileId]/route.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
v2ApiKeyAuth,
1010
v2RateLimits,
1111
} from '@/lib/api/server/routes'
12-
import { FILE_CACHE_CONTROL, presentFileDelivery } from '@/lib/uploads/server/delivery'
12+
import { presentFileAttachment } from '@/lib/uploads/server/delivery'
1313
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
1414
import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
1515
import { downloadWorkspaceFileStream } from '@/lib/workspace-files/application/download-workspace-file'
@@ -45,14 +45,7 @@ export const GET = defineV2BinaryRoute({
4545
}),
4646
useCase: downloadWorkspaceFileStream,
4747
present: ({ file, stream, contentType, contentLength }) =>
48-
presentFileDelivery({
49-
body: stream,
50-
filename: file.name,
51-
contentType,
52-
contentLength,
53-
attachment: true,
54-
cacheControl: FILE_CACHE_CONTROL.noStore,
55-
}),
48+
presentFileAttachment({ body: stream, filename: file.name, contentType, contentLength }),
5649
})
5750

5851
/**

‎apps/sim/app/api/v2/files/[fileId]/versions/[version]/content/route.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { v2DownloadFileVersionContract } from '@/lib/api/contracts/v2/file-versions'
22
import { defineV2BinaryRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
3-
import { FILE_CACHE_CONTROL, presentFileDelivery } from '@/lib/uploads/server/delivery'
3+
import { presentFileAttachment } from '@/lib/uploads/server/delivery'
44
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
55
import { downloadWorkspaceFileVersion } from '@/lib/workspace-files/application/file-versions'
66
import { fileOperations } from '@/lib/workspace-files/application/operations'
@@ -30,12 +30,5 @@ export const GET = defineV2BinaryRoute({
3030
}),
3131
useCase: downloadWorkspaceFileVersion,
3232
present: ({ file, stream, contentType, contentLength }) =>
33-
presentFileDelivery({
34-
body: stream,
35-
filename: file.name,
36-
contentType,
37-
contentLength,
38-
attachment: true,
39-
cacheControl: FILE_CACHE_CONTROL.noStore,
40-
}),
33+
presentFileAttachment({ body: stream, filename: file.name, contentType, contentLength }),
4134
})

‎apps/sim/app/api/v2/files/[fileId]/versions/route.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import { v2ListFileVersionsContract } from '@/lib/api/contracts/v2/file-versions'
22
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
33
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
4-
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
4+
import { toFileVersion, v2FileErrorPolicies } from '@/lib/workspace-files/api'
55
import { listWorkspaceFileVersions } from '@/lib/workspace-files/application/file-versions'
66
import { fileOperations } from '@/lib/workspace-files/application/operations'
7-
import { toV2FileVersions } from '@/app/api/v2/files/utils'
87
import { readSortedCursor, writeSortedCursor } from '@/app/api/v2/lib/response'
98

109
export const dynamic = 'force-dynamic'
@@ -38,8 +37,8 @@ export const GET = defineV2JsonRoute({
3837
),
3938
}),
4039
useCase: listWorkspaceFileVersions,
41-
present: async ({ versions, nextKeys }, { params, query }) => ({
42-
data: await toV2FileVersions(versions),
40+
present: ({ versions, nextKeys }, { params, query }) => ({
41+
data: versions.map(toFileVersion),
4342
nextCursor: writeSortedCursor(
4443
nextKeys,
4544
query.sortBy,

‎apps/sim/app/api/v2/files/utils.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest'
44
vi.mock('@/lib/users/queries', () => usersQueriesMock)
55

66
import type { WorkspaceFileVersionRecord } from '@/lib/uploads/contexts/workspace/file-version-store'
7-
import { toV2FileVersions } from '@/app/api/v2/files/utils'
7+
import { toV2FileVersion } from '@/app/api/v2/files/utils'
88

99
const { mockFindUserEmailsByIds } = usersQueriesMockFns
1010

@@ -23,11 +23,11 @@ const version: WorkspaceFileVersionRecord = {
2323
supersededAt: null,
2424
}
2525

26-
describe('toV2FileVersions', () => {
26+
describe('toV2FileVersion', () => {
2727
it('keeps authors whose accounts no longer exist, with a null email', async () => {
2828
mockFindUserEmailsByIds.mockResolvedValueOnce(new Map([['user-1', 'ada@example.com']]))
2929

30-
const [serialized] = await toV2FileVersions([version])
30+
const serialized = await toV2FileVersion(version)
3131

3232
expect(serialized.authors).toEqual([
3333
{ id: 'user-1', email: 'ada@example.com' },

‎apps/sim/app/api/v2/files/utils.ts‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ import { workspaceResourceWebUrl } from '@/lib/resources'
66
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
77
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager'
88
import type { WorkspaceFileVersionRecord } from '@/lib/uploads/contexts/workspace/file-version-store'
9-
import {
10-
findUserEmailsByIds,
11-
getUserEmailsByIds,
12-
requireResolvedUserEmail,
13-
} from '@/lib/users/queries'
9+
import { getUserEmailsByIds, requireResolvedUserEmail } from '@/lib/users/queries'
10+
import { toFileVersion } from '@/lib/workspace-files/api'
1411
import type { ReadWorkspaceFileTextResult } from '@/lib/workspace-files/application/read-workspace-file-text'
12+
import { projectFileVersionAuthors } from '@/lib/workspace-files/application/version-authors'
1513
import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folder-display-path'
1614

1715
/** Shared serialization for the v2 files surface. */
@@ -93,34 +91,8 @@ export function toV2FileText({
9391
}
9492
}
9593

96-
function serializeV2FileVersion(
97-
record: WorkspaceFileVersionRecord,
98-
emailByUserId: Map<string, string>
99-
): V2FileVersion {
100-
return {
101-
fileId: record.fileId,
102-
version: record.version,
103-
isCurrent: record.isCurrent,
104-
size: record.size,
105-
contentType: record.contentType,
106-
source: record.source,
107-
authors: record.authorUserIds.map((id) => ({ id, email: emailByUserId.get(id) ?? null })),
108-
restoredFromVersion: record.restoredFromVersion,
109-
createdAt: record.createdAt.toISOString(),
110-
updatedAt: record.updatedAt.toISOString(),
111-
supersededAt: record.supersededAt?.toISOString() ?? null,
112-
}
113-
}
114-
115-
/** Serializes file versions, resolving every author's email in one query. */
116-
export async function toV2FileVersions(
117-
records: WorkspaceFileVersionRecord[]
118-
): Promise<V2FileVersion[]> {
119-
const emailByUserId = await findUserEmailsByIds(records.flatMap((record) => record.authorUserIds))
120-
return records.map((record) => serializeV2FileVersion(record, emailByUserId))
121-
}
122-
94+
/** Serializes one version, resolving its authors' emails. */
12395
export async function toV2FileVersion(record: WorkspaceFileVersionRecord): Promise<V2FileVersion> {
124-
const [version] = await toV2FileVersions([record])
125-
return version
96+
const [version] = await projectFileVersionAuthors([record])
97+
return toFileVersion(version)
12698
}

‎apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,13 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => {
103103
expect(mocks.updateContent).not.toHaveBeenCalled()
104104
})
105105

106+
/**
107+
* The route declares a 70 MB inline cap, but Next's proxy truncates a client
108+
* body past 10 MiB, so the parser clamps to that ceiling and answers 413
109+
* rather than letting a truncated prefix surface as malformed JSON.
110+
*/
106111
it('rejects a JSON body above the proxy ceiling after admission', async () => {
107-
const response = await PUT(createRequest({ content: '' }, 17 * 1024 * 1024 + 1), routeContext)
112+
const response = await PUT(createRequest({ content: '' }, 10 * 1024 * 1024 + 1), routeContext)
108113

109114
expect(response.status).toBe(413)
110115
expect(mocks.admit).toHaveBeenCalled()

0 commit comments

Comments
 (0)