From 0df7b5cfa489ca5e41038fa407cc570bc468bdf5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 16:54:13 -0700 Subject: [PATCH 1/3] fix(file-doc): stop the permission resend loop and restore the 10 MB proxy body limit --- apps/realtime/src/handlers/file-doc.test.ts | 44 ++++ apps/realtime/src/handlers/file-doc.ts | 32 ++- .../[projectId]/[fileId]/persist/route.ts | 12 +- .../app/api/v2/files/[fileId]/route.test.ts | 19 ++ apps/sim/app/api/v2/files/[fileId]/route.ts | 11 +- .../versions/[version]/content/route.ts | 11 +- .../api/v2/files/[fileId]/versions/route.ts | 7 +- apps/sim/app/api/v2/files/utils.test.ts | 6 +- apps/sim/app/api/v2/files/utils.ts | 40 +-- .../[id]/files/[fileId]/content/route.test.ts | 7 +- .../collaboration/file-doc-provider.test.ts | 21 ++ .../collaboration/file-doc-provider.ts | 4 +- .../editor-lifecycle.test.tsx | 58 +++++ .../rich-markdown-editor.tsx | 25 +- .../lib/api/mcp/generated/v2-operations.ts | 246 ------------------ .../api/server/routes/internal-json-route.ts | 2 +- apps/sim/lib/api/server/validation.test.ts | 11 +- apps/sim/lib/api/server/validation.ts | 72 ++++- apps/sim/lib/core/config/request-limits.ts | 2 - apps/sim/lib/uploads/server/delivery.ts | 39 ++- apps/sim/next.config.ts | 2 - apps/sim/proxy.test.ts | 6 + apps/sim/proxy.ts | 7 +- packages/sim-cli/src/program.ts | 5 +- scripts/generate-cli-docs.ts | 3 +- scripts/generate-v2-mcp-operations.ts | 30 ++- 26 files changed, 365 insertions(+), 357 deletions(-) delete mode 100644 apps/sim/lib/core/config/request-limits.ts diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 5f30b7ed880..e850dda2eb3 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -642,6 +642,50 @@ describe('setupWorkspaceFileDocHandlers', () => { } }) + it('publishes write access to a socket only when it changes, not on every frame', async () => { + // A distinct user/file: role decisions recorded under fake timers outlive this test. + vi.useFakeTimers() + try { + const { io } = createIo() + const { socket, handlers } = setup('socket-capability', io, { userId: 'user-capability' }) + await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-capability', clientId: 7 }) + await vi.advanceTimersByTimeAsync(0) + const presence = awarenessFrame(7, 'Capability').frame + const sendFrames = async () => { + for (let sent = 0; sent < 3; sent++) { + void handlers[FILE_DOC_EVENTS.MESSAGE](presence) + await vi.advanceTimersByTimeAsync(0) + } + } + const published = () => + socket.emit.mock.calls + .filter(([event]) => event === FILE_DOC_EVENTS.PERMISSION) + .map(([, payload]) => (payload as { canWrite: boolean }).canWrite) + const changeRole = async (workspacePermission: 'read' | 'write') => { + mockAuthorizeRoom.mockResolvedValue({ + allowed: true, + status: 200, + workspaceId: 'ws-1', + workspacePermission, + }) + await vi.advanceTimersByTimeAsync(ROLE_REVALIDATION_TTL_MS + 1_000) + } + + await sendFrames() + expect(published()).toEqual([true]) + + await changeRole('read') + await sendFrames() + expect(published()).toEqual([true, false]) + + await changeRole('write') + await sendFrames() + expect(published()).toEqual([true, false, true]) + } finally { + vi.useRealTimers() + } + }) + it('applies + fans out an agent-streamed frame (SYNC_NO_PERSIST) but never persists it', async () => { mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) const { io, sent } = createIo() diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index 20797b6e6ad..d26c6089d4f 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -150,6 +150,8 @@ interface FileDocRoom { /** socketId → (clientId → its presence ownership). A socket owns one entry per collaborative provider * it mounted for this file (see {@link FileDocPresenceOwner}); an empty inner map is never kept. */ owners: Map> + /** socketId → the write capability last sent to it, so per-frame checks publish only a change. */ + publishedCanWrite: Map /** * The in-flight server seed for this room, or `null`. Concurrent joins await THIS promise rather * 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 { doc, awareness, owners: new Map(), + publishedCanWrite: new Map(), seeding: null, lastEditorUserId: null, edited: false, @@ -1161,11 +1164,15 @@ function resolveFileDocPermission( const room = fileDocRoom(document) const accept = (permission: string | null) => { if (socketToRoomName.get(socket.id) !== name || fileDocRooms.get(name) !== document) return null - socket.emit(FILE_DOC_EVENTS.PERMISSION, { - fileId: document.fileId, - ...fileDocOwnerWireFields(document.owner), - canWrite: permission === 'write' || permission === 'admin', - } satisfies FileDocPermission) + const canWrite = permission === 'write' || permission === 'admin' + if (document.publishedCanWrite.get(socket.id) !== canWrite) { + document.publishedCanWrite.set(socket.id, canWrite) + socket.emit(FILE_DOC_EVENTS.PERMISSION, { + fileId: document.fileId, + ...fileDocOwnerWireFields(document.owner), + canWrite, + } satisfies FileDocPermission) + } if (permission === null) evictSocketFromRoom(socket, room, 'Your access to this document has been revoked', io) return permission @@ -1216,7 +1223,16 @@ async function handleMessage(socket: AuthenticatedSocket, io: Server, data: unkn } const decoder = decoding.createDecoder(bytes) const messageType = decoding.readVarUint(decoder) - const permission = await resolveFileDocPermission(socket, io, name) + let permission: string | null + try { + permission = await resolveFileDocPermission(socket, io, name) + } catch (error) { + logger.warn('Dropping file-doc frame while its authorization is unavailable', { + socketId: socket.id, + error, + }) + return + } if (permission === null) return const canWrite = permission === 'write' || permission === 'admin' if (messageType === FILE_DOC_MESSAGE_TYPE.SYNC) { @@ -1460,6 +1476,7 @@ export function cleanupFileDocForSocket(socketId: string, io: Server, endOfLife // caret via its awareness removal. const clientMap = room.owners.get(socketId) room.owners.delete(socketId) + room.publishedCanWrite.delete(socketId) clearFileDocPresence(io, socketId, name) if (clientMap !== undefined && clientMap.size > 0) { // Fires the awareness `update` handler with a non-socket origin → the removals @@ -1788,6 +1805,7 @@ export function setupWorkspaceFileDocHandlers( awarenessProtocol.removeAwarenessStates(entry.awareness, [clientId], null) if (clientMap.size === 0) { entry.owners.delete(otherSid) + entry.publishedCanWrite.delete(otherSid) socketToRoomName.delete(otherSid) clearFileDocPresence(io, otherSid, name) io.in(otherSid).socketsLeave(name) @@ -1832,6 +1850,8 @@ export function setupWorkspaceFileDocHandlers( roomName: name, user: { socketId: socket.id, userId, userName, avatarUrl }, } + /** Co-mounted providers learn from the next frame whether this admission changed access. */ + entry.publishedCanWrite.delete(socket.id) // Name the document this room holds, so a client that still carries a DIFFERENT one (its room // outlived by a document rebuilt in its place) can refuse to merge instead of unioning two diff --git a/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts b/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts index 05223b4958f..ac26030165d 100644 --- a/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts +++ b/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts @@ -1,12 +1,18 @@ import { projectFileDocPersistContract } from '@/lib/api/contracts/project-file-doc' +import { isOffAppHost } from '@/lib/api/mcp/host-routing' import { defineInternalJsonRoute, internalOrchestrationErrorPolicy, internalRateLimits, } from '@/lib/api/server/routes' import { realtimeProjectFileAuth } from '@/lib/auth/realtime-file-delegation' +import { MAX_COLLAB_DOC_STATE_BYTES } from '@/lib/collab-doc/state' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { persistProjectFileDoc } from '@/lib/projects/files/application/documents' +/** A base64-encoded maximum snapshot plus 1 MiB for its JSON envelope (17 MiB). */ +const PERSIST_MAX_BODY_BYTES = Math.ceil(MAX_COLLAB_DOC_STATE_BYTES / 3) * 4 + 1024 * 1024 + export const POST = defineInternalJsonRoute({ contract: projectFileDocPersistContract, operation: persistProjectFileDoc.operation, @@ -15,11 +21,15 @@ export const POST = defineInternalJsonRoute({ reason: 'Authenticated realtime relay; socket frame admission bounds collaborative traffic', }), errorPolicy: internalOrchestrationErrorPolicy, + /** Excluded from the proxy matcher, so the route refuses the dedicated MCP host itself. */ + beforeParse: ({ request }) => { + if (isOffAppHost(request)) throw new OrchestrationError('not_found', 'Not found') + }, mapInput: ({ params, body }) => ({ ...params, docState: new Uint8Array(Buffer.from(body.docState, 'base64')), expectedVersion: body.expectedVersion, }), useCase: persistProjectFileDoc, - parseOptions: { maxBodyBytes: 17 * 1024 * 1024 }, + parseOptions: { maxBodyBytes: PERSIST_MAX_BODY_BYTES, bypassesProxy: true }, }) diff --git a/apps/sim/app/api/v2/files/[fileId]/route.test.ts b/apps/sim/app/api/v2/files/[fileId]/route.test.ts index 99835d09c11..16a98c257ea 100644 --- a/apps/sim/app/api/v2/files/[fileId]/route.test.ts +++ b/apps/sim/app/api/v2/files/[fileId]/route.test.ts @@ -168,6 +168,25 @@ describe('v2 single-file routes', () => { ) }) + it('downloads an attachment with its stored type and name, without sniffing', async () => { + mocks.download.mockResolvedValueOnce({ + file: fileRecord({ name: 'notes' }), + stream: new Blob(['

notes

']).stream(), + contentType: 'text/html', + contentLength: 12, + }) + + const response = await GET( + new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`), + context + ) + + expect(response.status).toBe(200) + expect(response.headers.get('Content-Type')).toBe('text/html') + expect(response.headers.get('Content-Disposition')).toBe('attachment; filename="notes"') + expect(response.headers.get('X-Content-Type-Options')).toBe('nosniff') + }) + it('conceals cross-workspace download authorization', async () => { mocks.download.mockRejectedValue(new NoWorkspaceAccessError()) diff --git a/apps/sim/app/api/v2/files/[fileId]/route.ts b/apps/sim/app/api/v2/files/[fileId]/route.ts index a81e5b9ea15..983f1c7b082 100644 --- a/apps/sim/app/api/v2/files/[fileId]/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/route.ts @@ -9,7 +9,7 @@ import { v2ApiKeyAuth, v2RateLimits, } from '@/lib/api/server/routes' -import { FILE_CACHE_CONTROL, presentFileDelivery } from '@/lib/uploads/server/delivery' +import { presentFileAttachment } from '@/lib/uploads/server/delivery' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file' import { downloadWorkspaceFileStream } from '@/lib/workspace-files/application/download-workspace-file' @@ -45,14 +45,7 @@ export const GET = defineV2BinaryRoute({ }), useCase: downloadWorkspaceFileStream, present: ({ file, stream, contentType, contentLength }) => - presentFileDelivery({ - body: stream, - filename: file.name, - contentType, - contentLength, - attachment: true, - cacheControl: FILE_CACHE_CONTROL.noStore, - }), + presentFileAttachment({ body: stream, filename: file.name, contentType, contentLength }), }) /** diff --git a/apps/sim/app/api/v2/files/[fileId]/versions/[version]/content/route.ts b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/content/route.ts index 5c5c16fe85f..13646658ffd 100644 --- a/apps/sim/app/api/v2/files/[fileId]/versions/[version]/content/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/content/route.ts @@ -1,6 +1,6 @@ import { v2DownloadFileVersionContract } from '@/lib/api/contracts/v2/file-versions' import { defineV2BinaryRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' -import { FILE_CACHE_CONTROL, presentFileDelivery } from '@/lib/uploads/server/delivery' +import { presentFileAttachment } from '@/lib/uploads/server/delivery' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { downloadWorkspaceFileVersion } from '@/lib/workspace-files/application/file-versions' import { fileOperations } from '@/lib/workspace-files/application/operations' @@ -30,12 +30,5 @@ export const GET = defineV2BinaryRoute({ }), useCase: downloadWorkspaceFileVersion, present: ({ file, stream, contentType, contentLength }) => - presentFileDelivery({ - body: stream, - filename: file.name, - contentType, - contentLength, - attachment: true, - cacheControl: FILE_CACHE_CONTROL.noStore, - }), + presentFileAttachment({ body: stream, filename: file.name, contentType, contentLength }), }) diff --git a/apps/sim/app/api/v2/files/[fileId]/versions/route.ts b/apps/sim/app/api/v2/files/[fileId]/versions/route.ts index 999323890d2..755f053807e 100644 --- a/apps/sim/app/api/v2/files/[fileId]/versions/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/versions/route.ts @@ -1,10 +1,9 @@ import { v2ListFileVersionsContract } from '@/lib/api/contracts/v2/file-versions' import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' -import { v2FileErrorPolicies } from '@/lib/workspace-files/api' +import { toFileVersion, v2FileErrorPolicies } from '@/lib/workspace-files/api' import { listWorkspaceFileVersions } from '@/lib/workspace-files/application/file-versions' import { fileOperations } from '@/lib/workspace-files/application/operations' -import { toV2FileVersions } from '@/app/api/v2/files/utils' import { readSortedCursor, writeSortedCursor } from '@/app/api/v2/lib/response' export const dynamic = 'force-dynamic' @@ -38,8 +37,8 @@ export const GET = defineV2JsonRoute({ ), }), useCase: listWorkspaceFileVersions, - present: async ({ versions, nextKeys }, { params, query }) => ({ - data: await toV2FileVersions(versions), + present: ({ versions, nextKeys }, { params, query }) => ({ + data: versions.map(toFileVersion), nextCursor: writeSortedCursor( nextKeys, query.sortBy, diff --git a/apps/sim/app/api/v2/files/utils.test.ts b/apps/sim/app/api/v2/files/utils.test.ts index 7a033620280..d91e337ade9 100644 --- a/apps/sim/app/api/v2/files/utils.test.ts +++ b/apps/sim/app/api/v2/files/utils.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest' vi.mock('@/lib/users/queries', () => usersQueriesMock) import type { WorkspaceFileVersionRecord } from '@/lib/uploads/contexts/workspace/file-version-store' -import { toV2FileVersions } from '@/app/api/v2/files/utils' +import { toV2FileVersion } from '@/app/api/v2/files/utils' const { mockFindUserEmailsByIds } = usersQueriesMockFns @@ -23,11 +23,11 @@ const version: WorkspaceFileVersionRecord = { supersededAt: null, } -describe('toV2FileVersions', () => { +describe('toV2FileVersion', () => { it('keeps authors whose accounts no longer exist, with a null email', async () => { mockFindUserEmailsByIds.mockResolvedValueOnce(new Map([['user-1', 'ada@example.com']])) - const [serialized] = await toV2FileVersions([version]) + const serialized = await toV2FileVersion(version) expect(serialized.authors).toEqual([ { id: 'user-1', email: 'ada@example.com' }, diff --git a/apps/sim/app/api/v2/files/utils.ts b/apps/sim/app/api/v2/files/utils.ts index bdeb3007f9f..488eef4444d 100644 --- a/apps/sim/app/api/v2/files/utils.ts +++ b/apps/sim/app/api/v2/files/utils.ts @@ -6,12 +6,10 @@ import { workspaceResourceWebUrl } from '@/lib/resources' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager' import type { WorkspaceFileVersionRecord } from '@/lib/uploads/contexts/workspace/file-version-store' -import { - findUserEmailsByIds, - getUserEmailsByIds, - requireResolvedUserEmail, -} from '@/lib/users/queries' +import { getUserEmailsByIds, requireResolvedUserEmail } from '@/lib/users/queries' +import { toFileVersion } from '@/lib/workspace-files/api' import type { ReadWorkspaceFileTextResult } from '@/lib/workspace-files/application/read-workspace-file-text' +import { projectFileVersionAuthors } from '@/lib/workspace-files/application/version-authors' import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folder-display-path' /** Shared serialization for the v2 files surface. */ @@ -93,34 +91,8 @@ export function toV2FileText({ } } -function serializeV2FileVersion( - record: WorkspaceFileVersionRecord, - emailByUserId: Map -): V2FileVersion { - return { - fileId: record.fileId, - version: record.version, - isCurrent: record.isCurrent, - size: record.size, - contentType: record.contentType, - source: record.source, - authors: record.authorUserIds.map((id) => ({ id, email: emailByUserId.get(id) ?? null })), - restoredFromVersion: record.restoredFromVersion, - createdAt: record.createdAt.toISOString(), - updatedAt: record.updatedAt.toISOString(), - supersededAt: record.supersededAt?.toISOString() ?? null, - } -} - -/** Serializes file versions, resolving every author's email in one query. */ -export async function toV2FileVersions( - records: WorkspaceFileVersionRecord[] -): Promise { - const emailByUserId = await findUserEmailsByIds(records.flatMap((record) => record.authorUserIds)) - return records.map((record) => serializeV2FileVersion(record, emailByUserId)) -} - +/** Serializes one version, resolving its authors' emails. */ export async function toV2FileVersion(record: WorkspaceFileVersionRecord): Promise { - const [version] = await toV2FileVersions([record]) - return version + const [version] = await projectFileVersionAuthors([record]) + return toFileVersion(version) } diff --git a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts index a2c3f14599f..7cbe7d5d6da 100644 --- a/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/[fileId]/content/route.test.ts @@ -103,8 +103,13 @@ describe('PUT /api/workspaces/[id]/files/[fileId]/content', () => { expect(mocks.updateContent).not.toHaveBeenCalled() }) + /** + * The route declares a 70 MB inline cap, but Next's proxy truncates a client + * body past 10 MiB, so the parser clamps to that ceiling and answers 413 + * rather than letting a truncated prefix surface as malformed JSON. + */ it('rejects a JSON body above the proxy ceiling after admission', async () => { - const response = await PUT(createRequest({ content: '' }, 17 * 1024 * 1024 + 1), routeContext) + const response = await PUT(createRequest({ content: '' }, 10 * 1024 * 1024 + 1), routeContext) expect(response.status).toBe(413) expect(mocks.admit).toHaveBeenCalled() diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts index a34fd55f286..6eeb7dcae65 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts @@ -649,6 +649,27 @@ describe('FileDocProvider', () => { } }) + it('sends an in-flight update once while the relay restates write access around each frame', async () => { + vi.useFakeTimers() + try { + const { provider, doc, emit, fire } = createProvider(true) + doc.getMap(FILE_DOC_SEED.configMap).set(FILE_DOC_SEED.docIdKey, 'doc-1') + acceptJoin(fire, doc.clientID, 'doc-1') + emit.mockClear() + + doc.getText('default').insert(0, 'awaiting acknowledgement') + await vi.advanceTimersByTimeAsync(UPDATE_BATCH_TEST_WINDOW_MS) + for (let frame = 0; frame < 5; frame++) { + fire(FILE_DOC_EVENTS.PERMISSION, { fileId: 'file-1', canWrite: true }) + } + + expect(emit.mock.calls.filter(([event]) => event === FILE_DOC_EVENTS.UPDATE)).toHaveLength(1) + provider.destroy() + } finally { + vi.useRealTimers() + } + }) + it('rejoins before retrying an update rejected because the room membership went stale', async () => { vi.useFakeTimers() try { diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts index 5d80c1812c5..2f6ee2d337b 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts @@ -1098,8 +1098,10 @@ export class FileDocProvider extends ObservableV2 { private handlePermission = (data: FileDocPermission) => { if (!this.matchesTarget(data) || data.fileId !== this.fileId) return + // The relay restates access around every frame; only a regrant resends, never a confirmation. + const regranted = data.canWrite && !this.canWrite this.setCanWrite(data.canWrite) - if (data.canWrite) this.sendInFlightUpdate() + if (regranted) this.sendInFlightUpdate() } private setCanWrite(canWrite: boolean) { diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx index 1a12ef886dc..4c362000aff 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx @@ -349,6 +349,64 @@ describe('loaded rich editor lifecycle', () => { } ) + it('reads locally when a writer-only relay rejects a read-only member', async () => { + authClientMockFns.mockUseSession.mockReturnValue({ + data: { user: { id: 'user-1', name: 'User' } }, + isPending: false, + }) + editableFileContent.mockReturnValue({ + content: 'Original', + acceptedBaselineContent: 'Original', + isStreamInteractionLocked: false, + isContentLoading: false, + hasContentError: false, + setDraftContent: onChange, + saveImmediately: onSaveShortcut, + }) + const provider = new FakeFileDocProvider() + const doc = new Y.Doc() + const awareness = new Awareness(doc) + collaborationRef.current = { + canWrite: false, + doc, + awareness, + provider, + user: { name: 'User', color: '#000000', clientId: doc.clientID }, + } + try { + await act(async () => + root.render( + + ) + ) + expect(useFileDocCollaboration).toHaveBeenLastCalledWith( + expect.objectContaining({ enabled: true }) + ) + await act(async () => + provider.fail({ + fileId: FILE.id, + error: 'Access denied to file', + code: 'ACCESS_DENIED', + retryable: false, + }) + ) + expect(useFileDocCollaboration).toHaveBeenLastCalledWith( + expect.objectContaining({ enabled: false }) + ) + expect(getEditor().isEditable).toBe(false) + expect(getEditor().view.dom.closest('.hidden')).toBeNull() + } finally { + await act(async () => root.render(null)) + awareness.destroy() + doc.destroy() + } + }) + it.each(['file', 'preview context'] as const)( 'does not recover another %s after a write-loss remount', async (changedScope) => { diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index 8d73bc078cd..8af75ef3b90 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -336,6 +336,19 @@ function RichMarkdownSurface({ : 1, })) }, [file.id, previewContextKey]) + /** A relay older than read membership rejects a read-only member, who then reads the file locally. */ + const [localReadOnly, setLocalReadOnly] = useState<{ + fileId: string + previewContextKey?: string + } | null>(null) + const collaborationDeclined = + !canEdit && + localReadOnly?.fileId === file.id && + localReadOnly.previewContextKey === previewContextKey + const declineCollaboration = useCallback( + () => setLocalReadOnly({ fileId: file.id, previewContextKey }), + [file.id, previewContextKey] + ) const { content, @@ -390,9 +403,10 @@ function RichMarkdownSurface({ /> )} 0} onWriteAccessLost={resetCollaboration} + onReadOnlyJoinRejected={declineCollaboration} file={file} workspaceId={workspaceId} content={content} @@ -406,7 +420,7 @@ function RichMarkdownSurface({ streamOperation={streamOperation} disableStreamingAutoScroll={disableStreamingAutoScroll} disableTagging={disableTagging} - collaborative={collaborative} + collaborative={collaborative && !collaborationDeclined} onChange={setDraftContent} onSaveShortcut={saveImmediately} downloadSourceRef={downloadSourceRef} @@ -422,6 +436,7 @@ function RichMarkdownSurface({ interface LoadedRichMarkdownEditorProps { recoveringCollaboration?: boolean onWriteAccessLost?: () => void + onReadOnlyJoinRejected?: () => void file: WorkspaceFileRecord workspaceId: string /** The live content from the engine — grows as the agent streams, then settles to the saved doc. */ @@ -479,6 +494,7 @@ function lockSettled(content: string): SettledContent { export function LoadedRichMarkdownEditor({ recoveringCollaboration = false, onWriteAccessLost, + onReadOnlyJoinRejected, file, workspaceId, content, @@ -983,6 +999,9 @@ export function LoadedRichMarkdownEditor({ const synced = provider.synced const seeded = config.get(FILE_DOC_SEED.flag) === true const fatal = provider.joinError?.retryable === false + if (fatal && !canEdit && provider.joinError?.code === 'ACCESS_DENIED') { + onReadOnlyJoinRejected?.() + } setReady( isCollabReady({ synced, seeded, fatal }), fatal, @@ -1006,7 +1025,7 @@ export function LoadedRichMarkdownEditor({ // ungate it while the doc is unready. onClientAutosaveChange(false) } - }, [collaboration, editor, onClientAutosaveChange]) + }, [collaboration, editor, onClientAutosaveChange, canEdit, onReadOnlyJoinRejected]) /** * Owns editability for the collaborative lifecycle: `useEditor`'s `editable` is only the initial diff --git a/apps/sim/lib/api/mcp/generated/v2-operations.ts b/apps/sim/lib/api/mcp/generated/v2-operations.ts index a52f2a6b33d..a585e7f3629 100644 --- a/apps/sim/lib/api/mcp/generated/v2-operations.ts +++ b/apps/sim/lib/api/mcp/generated/v2-operations.ts @@ -185,37 +185,6 @@ import { v2RemovePermissionGroupMemberContract, v2UpdatePermissionGroupContract, } from '@/lib/api/contracts/v2/permission-groups' -import { - v2CreateProjectFileFolderContract, - v2ListProjectFileFoldersContract, - v2RestoreProjectFileFolderContract, - v2UpdateProjectFileFolderContract, -} from '@/lib/api/contracts/v2/project-file-folders' -import { - v2ArchiveProjectFileItemsContract, - v2MoveProjectFileItemsContract, - v2RenameProjectFileContract, - v2RestoreProjectFileContract, -} from '@/lib/api/contracts/v2/project-file-lifecycle' -import { - v2AbortProjectFileUploadContract, - v2CompleteProjectFileUploadContract, - v2CreateProjectFileUploadContract, - v2GetProjectFileUploadContract, - v2GetProjectFileUploadPartUrlsContract, -} from '@/lib/api/contracts/v2/project-file-uploads' -import { - v2DeleteProjectFileVersionContract, - v2GetProjectFileVersionContract, - v2ListProjectFileVersionsContract, - v2RevertProjectFileVersionContract, -} from '@/lib/api/contracts/v2/project-file-versions' -import { - v2CreateProjectFileContract, - v2GetProjectFileMetadataContract, - v2ListProjectFilesContract, - v2UpdateProjectFileContentContract, -} from '@/lib/api/contracts/v2/project-files' import { v2CreateSandboxContract, v2DeleteSandboxContract, @@ -403,17 +372,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.DELETE ), }, - abortProjectFileUpload: { - contract: v2AbortProjectFileUploadContract, - summary: 'Abort Project file upload', - description: - 'Abort a pending Project upload and schedule its unregistered bytes for cleanup. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/uploads/[uploadId]/route').then( - (route) => route.DELETE - ), - }, activateWorkflowVersion: { contract: v2ActivateWorkflowVersionContract, summary: 'Activate workflow version', @@ -490,15 +448,6 @@ export const V2_MCP_OPERATIONS = { handler: () => import('@/app/api/v2/workflows/[workflowId]/variables/route').then((route) => route.PATCH), }, - archiveProjectFileItems: { - contract: v2ArchiveProjectFileItemsContract, - summary: 'Archive Project File Items', - description: - 'Archive selected files and folders. Folder contents are archived recursively and remain recoverable until retention removes them. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/archive/route').then((route) => route.POST), - }, bulkAddPermissionGroupMembers: { contract: v2BulkAddPermissionGroupMembersContract, summary: 'Bulk add permission group members', @@ -662,17 +611,6 @@ export const V2_MCP_OPERATIONS = { '@/app/api/v2/knowledge/[knowledgeBaseId]/documents/uploads/[uploadId]/complete/route' ).then((route) => route.POST), }, - completeProjectFileUpload: { - contract: v2CompleteProjectFileUploadContract, - summary: 'Complete Project file upload', - description: - 'Finalize verified bytes and atomically register one Project file. Retrying completion returns the same file without billing twice. Current edit access is checked again. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/uploads/[uploadId]/complete/route').then( - (route) => route.POST - ), - }, completeTableImport: { contract: v2CompleteTableImportContract, summary: 'Complete table import upload', @@ -829,33 +767,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.POST ), }, - createProjectFile: { - contract: v2CreateProjectFileContract, - summary: 'Create Project File', - description: - 'Create a shared Project file from inline text or base64 bytes. Names are exact; an existing sibling name returns a conflict. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/route').then((route) => route.POST), - }, - createProjectFileFolder: { - contract: v2CreateProjectFileFolderContract, - summary: 'Create Project file folder', - description: - 'Create a folder under an existing parent, or at the Project root when parentId is omitted. Sibling names must be unique. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/folders/route').then((route) => route.POST), - }, - createProjectFileUpload: { - contract: v2CreateProjectFileUploadContract, - summary: 'Create Project file upload', - description: - 'Create a resumable Project file upload. The file is registered only after the signed transfer and completion succeed. Creation returns an upload-token. Send that token with the original API credential on subsequent control requests. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/uploads/route').then((route) => route.POST), - }, createSandbox: { contract: v2CreateSandboxContract, summary: 'Create sandbox', @@ -1106,17 +1017,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.DELETE ), }, - deleteProjectFileVersion: { - contract: v2DeleteProjectFileVersionContract, - summary: 'Delete Project File Version', - description: - 'Permanently remove one superseded Project file version from history. Deleting the current version returns `409`; other versions and the current file remain available. Stored-object cleanup is retried asynchronously when needed. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/versions/[version]/route').then( - (route) => route.DELETE - ), - }, deleteSandbox: { contract: v2DeleteSandboxContract, summary: 'Delete sandbox', @@ -1543,50 +1443,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.GET ), }, - getProjectFileMetadata: { - contract: v2GetProjectFileMetadataContract, - summary: 'Get Project File Metadata', - description: - "Get an active file's metadata and Project ownership. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.", - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/metadata/route').then( - (route) => route.GET - ), - }, - getProjectFileUpload: { - contract: v2GetProjectFileUploadContract, - summary: 'Get Project file upload', - description: - 'Read the current state of a Project upload, including its file after completion. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/uploads/[uploadId]/route').then( - (route) => route.GET - ), - }, - getProjectFileUploadPartUrls: { - contract: v2GetProjectFileUploadPartUrlsContract, - summary: 'Get Project file upload part URLs', - description: - 'Request signed multipart transfer URLs for an active Project upload. Send exactly the returned transfer headers when uploading each part. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/uploads/[uploadId]/parts/route').then( - (route) => route.POST - ), - }, - getProjectFileVersion: { - contract: v2GetProjectFileVersionContract, - summary: 'Get Project File Version', - description: - 'Get metadata and author attribution for a recorded Project file version. Missing or permanently removed versions return `404`. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/versions/[version]/route').then( - (route) => route.GET - ), - }, getRowEnrichment: { contract: v2GetRowEnrichmentContract, summary: 'Get row group run', @@ -2144,35 +2000,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.GET ), }, - listProjectFileFolders: { - contract: v2ListProjectFileFoldersContract, - summary: 'List Project file folders', - description: - 'List the Project folder tree with stable identifiers. Use scope=archived to find folders eligible for restore. Returns one page; `nextCursor` is always null. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/folders/route').then((route) => route.GET), - }, - listProjectFiles: { - contract: v2ListProjectFilesContract, - summary: 'List Project Files', - description: - 'List shared Project files with cursor pagination. Use scope=archived to find archived files. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/route').then((route) => route.GET), - }, - listProjectFileVersions: { - contract: v2ListProjectFileVersionsContract, - summary: 'List Project File Versions', - description: - 'List recorded versions of a shared Project file, newest first by default. Retention follows the Project payer and preserves the newest ten versions; removed versions leave gaps in numbering. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:read`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/versions/route').then( - (route) => route.GET - ), - }, listSandboxes: { contract: v2ListSandboxesContract, summary: 'List sandboxes', @@ -2370,15 +2197,6 @@ export const V2_MCP_OPERATIONS = { 'Move up to 1,000 files to a folder path or the workspace root.\n\nOAuth scope: `api:write`.', handler: () => import('@/app/api/v2/files/move/route').then((route) => route.POST), }, - moveProjectFileItems: { - contract: v2MoveProjectFileItemsContract, - summary: 'Move Project File Items', - description: - 'Move selected files and folders into an existing folder within the same Project. Folder contents move with their parent. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/move/route').then((route) => route.POST), - }, moveTables: { contract: v2MoveTablesContract, summary: 'Move tables and folders', @@ -2574,15 +2392,6 @@ export const V2_MCP_OPERATIONS = { 'Rename a workspace file without changing its containing folder.\n\nOAuth scope: `api:write`.', handler: () => import('@/app/api/v2/files/[fileId]/route').then((route) => route.PATCH), }, - renameProjectFile: { - contract: v2RenameProjectFileContract, - summary: 'Rename Project File', - description: - 'Rename a shared Project file while retaining its identity and history. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/route').then((route) => route.PATCH), - }, replaceWorkflowChatDeployment: { contract: v2ReplaceWorkflowChatDeploymentContract, summary: 'Create or replace workflow chat deployment', @@ -2647,28 +2456,6 @@ export const V2_MCP_OPERATIONS = { handler: () => import('@/app/api/v2/knowledge/[knowledgeBaseId]/restore/route').then((route) => route.POST), }, - restoreProjectFile: { - contract: v2RestoreProjectFileContract, - summary: 'Restore Project File', - description: - 'Restore an archived Project file. If its former folder is unavailable, restore it to the Project root with an available name. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/restore/route').then( - (route) => route.POST - ), - }, - restoreProjectFileFolder: { - contract: v2RestoreProjectFileFolderContract, - summary: 'Restore Project file folder', - description: - 'Restore an archived folder and the files and subfolders archived with it. Find identifiers with List Project File Folders using scope=archived. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/folders/[folderId]/restore/route').then( - (route) => route.POST - ), - }, restoreTable: { contract: v2RestoreTableContract, summary: 'Restore table', @@ -2712,17 +2499,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.POST ), }, - revertProjectFileVersion: { - contract: v2RevertProjectFileVersionContract, - summary: 'Revert Project File Version', - description: - 'Make an earlier version current by recording its source bytes as a new revert version. Reverting to the current version is a no-op. Use expectedRevision to reject changes made since the last read; a stale revision or concurrent edit returns `409`. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import( - '@/app/api/v2/projects/[projectId]/files/[fileId]/versions/[version]/revert/route' - ).then((route) => route.POST), - }, revertWorkflowVersion: { contract: v2RevertWorkflowVersionContract, summary: 'Revert workflow to version', @@ -3032,28 +2808,6 @@ export const V2_MCP_OPERATIONS = { (route) => route.PATCH ), }, - updateProjectFileContent: { - contract: v2UpdateProjectFileContentContract, - summary: 'Replace Project File Content', - description: - 'Replace the complete content of a Project file. Supply expectedRevision to reject a stale edit with 409. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/[fileId]/content/route').then( - (route) => route.PUT - ), - }, - updateProjectFileFolder: { - contract: v2UpdateProjectFileFolderContract, - summary: 'Update Project file folder', - description: - 'Rename, move, or reorder a folder while retaining its identity and descendants. Omitted fields stay unchanged; parentId=null moves the folder to the root. Cross-owner parents and cycles are rejected. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.', - workspaceKeyUnsupported: true, - handler: () => - import('@/app/api/v2/projects/[projectId]/files/folders/[folderId]/route').then( - (route) => route.PATCH - ), - }, updateRowsByFilter: { contract: v2UpdateRowsByFilterContract, summary: 'Update rows by filter', diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index d5703f3ad30..eb60596d658 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -238,7 +238,7 @@ export interface InternalJsonResponseFinalization { type InternalJsonParseOptions = Pick< ParseRequestOptions, - 'maxBodyBytes' | 'validationErrorResponse' | 'optionalJsonBody' + 'maxBodyBytes' | 'bypassesProxy' | 'validationErrorResponse' | 'optionalJsonBody' > /** diff --git a/apps/sim/lib/api/server/validation.test.ts b/apps/sim/lib/api/server/validation.test.ts index 86a619fad17..4e1bffc5335 100644 --- a/apps/sim/lib/api/server/validation.test.ts +++ b/apps/sim/lib/api/server/validation.test.ts @@ -10,7 +10,7 @@ import { * Next.js truncates a proxied client body past `experimental.proxyClientMaxBodySize` * without signalling it, so this is the largest body a handler can actually receive. */ -const PROXY_CLIENT_MAX_BODY_BYTES = 17 * 1024 * 1024 +const PROXY_CLIENT_MAX_BODY_BYTES = 10 * 1024 * 1024 /** Mirrors `MAX_WORKSPACE_FILE_INLINE_BODY_BYTES` — an explicit override above the ceiling. */ const INLINE_FILE_BODY_BYTES = 70 * 1024 * 1024 @@ -40,9 +40,9 @@ describe('DEFAULT_MAX_JSON_BODY_BYTES', () => { }) describe('parseJsonBody default size boundary', () => { - it('accepts a body at the default cap', async () => { + it('accepts a body at the proxy cap', async () => { const result = await parseJsonBody( - requestDeclaring(DEFAULT_MAX_JSON_BODY_BYTES, JSON.stringify({ value: 'ok' })) + requestDeclaring(PROXY_CLIENT_MAX_BODY_BYTES, JSON.stringify({ value: 'ok' })) ) expect(result.success).toBe(true) @@ -75,12 +75,13 @@ describe('parseJsonBody default size boundary', () => { }) }) - it('accepts a document snapshot above the default cap with its explicit limit', async () => { + it('keeps an over-ceiling override only for a route the proxy never buffers', async () => { const body = JSON.stringify({ update: 'a'.repeat(12 * 1024 * 1024) }) const result = await parseJsonBody( requestDeclaring(Buffer.byteLength(body), body), 'response', - 17 * 1024 * 1024 + 17 * 1024 * 1024, + true ) expect(result.success).toBe(true) }) diff --git a/apps/sim/lib/api/server/validation.ts b/apps/sim/lib/api/server/validation.ts index 0221dbeeb05..966e3b93d46 100644 --- a/apps/sim/lib/api/server/validation.ts +++ b/apps/sim/lib/api/server/validation.ts @@ -14,22 +14,58 @@ import { } from '@/lib/api/server/blank-query-values' import { nulByteValidationError } from '@/lib/api/server/nul-bytes' import { env } from '@/lib/core/config/env' -import { PROXY_CLIENT_MAX_BODY_BYTES } from '@/lib/core/config/request-limits' import { assertContentLengthWithinLimit, isPayloadSizeLimitError, readStreamToBufferWithLimit, } from '@/lib/core/utils/stream-limits' -/** Ordinary JSON keeps its existing cap; larger authorized routes opt in explicitly. */ +/** + * Next.js buffers the client body for the proxy and *silently truncates* anything + * past `experimental.proxyClientMaxBodySize` (default 10 MB), and `apps/sim/proxy.ts` + * matches `/api/:path*`. A larger body therefore reaches the handler as a truncated + * prefix, which fails JSON parsing — so an oversized request has to be rejected on + * its declared size before it is read, or the truncation gets misreported as a + * malformed body. + */ +const PROXY_CLIENT_MAX_BODY_BYTES = 10 * 1024 * 1024 + +/** + * Default upper bound on the JSON request body that contract routes will read + * and parse into memory. Without a cap an unauthenticated caller could buffer a + * large body before schema validation runs. Override per-route via + * `ParseRequestOptions.maxBodyBytes`. + * + * Falls back to 50 MB if the env value is missing or non-numeric so a misconfig + * can never silently disable the cap (a NaN limit would never reject), then + * clamps to {@link PROXY_CLIENT_MAX_BODY_BYTES} because the app can never + * actually receive more than the proxy forwards. + */ export const DEFAULT_MAX_JSON_BODY_BYTES = Math.min( Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) || 50 * 1024 * 1024, - 10 * 1024 * 1024 + PROXY_CLIENT_MAX_BODY_BYTES ) -/** Reject before Next's proxy truncates the body and turns an oversize request into malformed JSON. */ -function clampToProxyLimit(maxBytes: number): number { - return Math.min(maxBytes, PROXY_CLIENT_MAX_BODY_BYTES) +/** + * Clamps a per-route body cap to {@link PROXY_CLIENT_MAX_BODY_BYTES}. + * + * A route that raises `maxBodyBytes` above the proxy ceiling cannot actually + * receive a body that large: the proxy truncates the stream, the handler parses + * a prefix, and the caller gets `400 "Request body must be valid JSON"` for a + * request whose only fault was its size. Clamping at the point of use turns that + * into an accurate `413`; nothing that succeeds today changes, because a body + * over the ceiling already fails — just less honestly. + * + * Consequence worth keeping in view: `MAX_WORKSPACE_FILE_INLINE_BODY_BYTES` + * (70 MB) exists so a 50 MiB file can be sent inline as base64, and that ceiling + * stays unreachable until `experimental.proxyClientMaxBodySize` is raised in + * `apps/sim/next.config.ts`. Raising it changes the memory profile of every + * `/api` route, so it is a separate decision — this clamp only makes the limit + * that is actually in force report itself correctly. A route the proxy matcher + * excludes is never buffered, so it declares `bypassesProxy` and keeps its own cap. + */ +function clampToProxyLimit(maxBytes: number, bypassesProxy = false): number { + return bypassesProxy ? maxBytes : Math.min(maxBytes, PROXY_CLIENT_MAX_BODY_BYTES) } export interface ValidationErrorBody { @@ -66,6 +102,11 @@ export interface ParseRequestOptions { * a value above what the proxy forwards is clamped — see {@link clampToProxyLimit}. */ maxBodyBytes?: number + /** + * The route is excluded from the `apps/sim/proxy.ts` matcher, so Next never buffers or + * truncates its body and {@link maxBodyBytes} applies unclamped. + */ + bypassesProxy?: boolean /** Treat an absent or whitespace-only body as `undefined` before contract validation. */ optionalJsonBody?: boolean /** @@ -141,7 +182,8 @@ async function readJsonBodyWithLimit(request: Request, maxBytes: number): Promis export async function parseJsonBody( request: Request, invalidJson: ParseRequestOptions['invalidJson'] = 'response', - maxBytes: number = DEFAULT_MAX_JSON_BODY_BYTES + maxBytes: number = DEFAULT_MAX_JSON_BODY_BYTES, + bypassesProxy = false ): Promise< | { success: true; data: unknown } | { @@ -150,7 +192,7 @@ export async function parseJsonBody( response: NextResponse<{ error: string }> } > { - const limit = clampToProxyLimit(maxBytes) + const limit = clampToProxyLimit(maxBytes, bypassesProxy) try { return { success: true, data: await readJsonBodyWithLimit(request, limit) } } catch (error) { @@ -182,7 +224,8 @@ export async function parseJsonBody( */ export async function parseOptionalJsonBody( request: Request, - maxBytes: number = DEFAULT_MAX_JSON_BODY_BYTES + maxBytes: number = DEFAULT_MAX_JSON_BODY_BYTES, + bypassesProxy = false ): Promise< | { success: true; data: unknown } | { @@ -191,7 +234,7 @@ export async function parseOptionalJsonBody( response: NextResponse<{ error: string }> } > { - const limit = clampToProxyLimit(maxBytes) + const limit = clampToProxyLimit(maxBytes, bypassesProxy) try { assertContentLengthWithinLimit(request.headers, limit, REQUEST_BODY_LABEL) @@ -273,8 +316,13 @@ export async function parseRequest( let body: unknown if (shouldReadJsonBody(contract)) { const parsedBody = options?.optionalJsonBody - ? await parseOptionalJsonBody(request, options.maxBodyBytes) - : await parseJsonBody(request, options?.invalidJson, options?.maxBodyBytes) + ? await parseOptionalJsonBody(request, options.maxBodyBytes, options.bypassesProxy) + : await parseJsonBody( + request, + options?.invalidJson, + options?.maxBodyBytes, + options?.bypassesProxy + ) if (!parsedBody.success) { if ( options?.invalidJsonResponse && diff --git a/apps/sim/lib/core/config/request-limits.ts b/apps/sim/lib/core/config/request-limits.ts deleted file mode 100644 index ea877ba1521..00000000000 --- a/apps/sim/lib/core/config/request-limits.ts +++ /dev/null @@ -1,2 +0,0 @@ -/** Next proxy buffer accommodates the base64 envelope for a 12 MiB collaborative snapshot. */ -export const PROXY_CLIENT_MAX_BODY_BYTES = 17 * 1024 * 1024 diff --git a/apps/sim/lib/uploads/server/delivery.ts b/apps/sim/lib/uploads/server/delivery.ts index 7f38bad0231..4358a128e14 100644 --- a/apps/sim/lib/uploads/server/delivery.ts +++ b/apps/sim/lib/uploads/server/delivery.ts @@ -1,4 +1,5 @@ import { ensureFileNameExtension } from '@/lib/uploads/utils/file' +import { normalizeMimeType } from '@/lib/uploads/utils/mime' const SAFE_INLINE_TYPES = new Set([ 'image/png', @@ -40,6 +41,37 @@ function getSecureFileHeaders(filename: string, originalContentType: string) { } } +const SVG_SANDBOX_CSP = "default-src 'none'; style-src 'unsafe-inline'; sandbox;" + +/** Stops a browser from sniffing a delivered file, and from running an SVG's scripts. */ +function contentSafetyHeaders(contentType: string): Record { + return { + 'X-Content-Type-Options': 'nosniff', + ...(normalizeMimeType(contentType) === 'image/svg+xml' + ? { 'Content-Security-Policy': SVG_SANDBOX_CSP } + : {}), + } +} + +/** + * An attachment is saved, never rendered, so it keeps the stored type and name; the inline-safety + * rewrites in {@link presentFileDelivery} would only change what the caller downloads. + */ +export function presentFileAttachment(input: { + body: ReadableStream + filename: string + contentType: string + contentLength: number +}) { + return { + body: input.body, + contentType: input.contentType, + contentDisposition: `attachment; ${encodeFilenameForHeader(input.filename)}`, + contentLength: input.contentLength, + headers: contentSafetyHeaders(input.contentType), + } +} + /** * Percent-encode a filename as an RFC 8187 `ext-value`. * @@ -106,14 +138,9 @@ function fileDeliveryHeaders(input: { 'Content-Type': secure.contentType, 'Content-Disposition': `${input.attachment ? 'attachment' : secure.disposition}; ${encodeFilenameForHeader(filename)}`, 'Cache-Control': input.cacheControl, - 'X-Content-Type-Options': 'nosniff', + ...contentSafetyHeaders(secure.contentType), }) if (input.contentLength !== undefined) headers.set('Content-Length', String(input.contentLength)) - if (secure.contentType.split(';', 1)[0].trim().toLowerCase() === 'image/svg+xml') - headers.set( - 'Content-Security-Policy', - "default-src 'none'; style-src 'unsafe-inline'; sandbox;" - ) return headers } diff --git a/apps/sim/next.config.ts b/apps/sim/next.config.ts index 4e9c354a3b1..2f07796494b 100644 --- a/apps/sim/next.config.ts +++ b/apps/sim/next.config.ts @@ -1,7 +1,6 @@ import path from 'node:path' import { SIM_SITE_URL } from '@sim/utils/site' import type { NextConfig } from 'next' -import { PROXY_CLIENT_MAX_BODY_BYTES } from '@/lib/core/config/request-limits' import { env, isTruthy } from './lib/core/config/env' import { isDev } from './lib/core/config/env-flags' import { @@ -156,7 +155,6 @@ const nextConfig: NextConfig = { ], }, experimental: { - proxyClientMaxBodySize: PROXY_CLIENT_MAX_BODY_BYTES, /** * Turbopack's dev filesystem cache stays ON (this is also the Next default * since v16.1). It is what makes a dev-server restart cheap: without it every diff --git a/apps/sim/proxy.test.ts b/apps/sim/proxy.test.ts index 2d83f47c0b4..00c1df9ad8f 100644 --- a/apps/sim/proxy.test.ts +++ b/apps/sim/proxy.test.ts @@ -169,10 +169,16 @@ describe('proxy matcher', () => { expect(matches('/api/desktop/tool/file')).toBe(false) }) + it('leaves the document snapshot persist unbuffered, so a whole snapshot reaches the route', () => { + expect(matches('/api/internal/project-file-doc/project-1/file-1/persist')).toBe(false) + }) + it('still runs for every other API route', () => { expect(matches('/api/desktop/tool/claim')).toBe(true) expect(matches('/api/desktop/tool/import/other')).toBe(true) expect(matches('/api/workflows/wf-1/execute')).toBe(true) expect(matches('/api/files/upload')).toBe(true) + expect(matches('/api/internal/project-file-doc/project-1/file-1/seed')).toBe(true) + expect(matches('/api/internal/project-file-doc/project-1/file-1/persist/other')).toBe(true) }) }) diff --git a/apps/sim/proxy.ts b/apps/sim/proxy.ts index 3cf2424b72e..c5ff9e998b9 100644 --- a/apps/sim/proxy.ts +++ b/apps/sim/proxy.ts @@ -469,9 +469,10 @@ export const config = { '/login', '/signup', '/invite/:path*', // Match invitation routes - // Runtime CORS. The desktop's raw upload routes are left out: running the proxy makes Next - // buffer the body, cutting it off at its 10 MB proxy limit, and those bodies are whole files. - '/api/((?!desktop/tool/(?:import|file)$).*)', + // Runtime CORS. The desktop's raw upload routes and the realtime relay's document persist are + // left out: running the proxy makes Next buffer the body, cutting it off at its 10 MB proxy + // limit, and those bodies are whole files or document snapshots. + '/api/((?!desktop/tool/(?:import|file)$|internal/project-file-doc/[^/]+/[^/]+/persist$).*)', // Next's image optimizer fetches public editorial images without a User-Agent. '/((?!api/|api$|_next/static|_next/image|(?:blog|changelog)/.*\\.(?:avif|gif|jpe?g|png|svg|webp)$|ingest|favicon.ico|logo/|landing/|static/|footer/|social/|enterprise/|favicon/|twitter/|robots.txt|sitemap.xml).*)', ], diff --git a/packages/sim-cli/src/program.ts b/packages/sim-cli/src/program.ts index 53f9d744d67..92d2025ab50 100644 --- a/packages/sim-cli/src/program.ts +++ b/packages/sim-cli/src/program.ts @@ -62,6 +62,9 @@ best match rather than repeating similar searches, then run === END AGENT COMMAND DISCOVERY === ` +/** Executable but unpublished until launch, so kept out of `--help` and `sim cli` search. */ +const UNPUBLISHED_GROUPS: ReadonlySet = new Set(['projects']) + /** The root's own value-taking options, which consume the token after them. */ const ROOT_VALUE_FLAGS: ReadonlySet = new Set([ '-P', @@ -190,7 +193,7 @@ export function buildProgram( program.addCommand(cliCommand()) for (const command of buildGeneratedCommands()) { - program.addCommand(command) + program.addCommand(command, { hidden: UNPUBLISHED_GROUPS.has(command.name()) }) } attachCredentialCommands(program) diff --git a/scripts/generate-cli-docs.ts b/scripts/generate-cli-docs.ts index 5f8114dc086..627175aa1a9 100644 --- a/scripts/generate-cli-docs.ts +++ b/scripts/generate-cli-docs.ts @@ -1293,8 +1293,7 @@ function build(): Map { assertGuidesExist() const program = buildProgram({ version: false }) - /** Projects is executable but unpublished until launch; do not change CLI help or admission. */ - const top = subcommands(program).filter((command) => command.name() !== 'projects') + const top = subcommands(program) const groups = top.filter((command) => subcommands(command).length > 0) const globals = top .filter((command) => subcommands(command).length === 0) diff --git a/scripts/generate-v2-mcp-operations.ts b/scripts/generate-v2-mcp-operations.ts index e6a7ffca87b..118739f4a21 100644 --- a/scripts/generate-v2-mcp-operations.ts +++ b/scripts/generate-v2-mcp-operations.ts @@ -12,10 +12,11 @@ * * Operations come from {@link collectOperations} — the same contract discovery * the CLI generator uses — so the terminal and MCP expose one operation set - * under one set of names. An operation is left out only when its transport - * cannot be expressed as a JSON tool call: a binary response, or a body that - * must be streamed as multipart. A route built by anything this script does not - * recognize fails generation rather than being guessed at. + * under one set of names. An operation is left out only while its reference is + * unpublished, or when its transport cannot be expressed as a JSON tool call: a + * binary response, or a body that must be streamed as multipart. A route built by + * anything this script does not recognize fails generation rather than being + * guessed at. * * Usage: * bun run scripts/generate-v2-mcp-operations.ts # write the generated file @@ -159,9 +160,23 @@ function format(source: string): string { return result.stdout } +/** `METHOD path` keys of operations whose public reference is not yet published. */ +async function loadUnpublishedOperationKeys(): Promise> { + const { unpublishedProjectFileDocumentation } = await import( + path.join(APP_ROOT, 'lib/api/contracts/v2/openapi/unpublished-project-files.ts') + ) + return new Set( + unpublishedProjectFileDocumentation.routes.map( + ({ contract }: { contract: Operation['contract'] }) => + docPathKey(contract.method, contract.path) + ) + ) +} + async function main() { const check = process.argv.includes('--check') const docs = await loadSummaries(await loadWorkspaceKeyDenialMarkers()) + const unpublished = await loadUnpublishedOperationKeys() const readRoute = (relativePath: string) => { const file = path.join(APP_ROOT, relativePath) return existsSync(file) ? readFileSync(file, 'utf8') : null @@ -170,7 +185,10 @@ async function main() { const operations: McpOperation[] = [] let excluded = 0 for (const operation of await collectOperations()) { - if (classifyOperation(operation, readRoute) === 'excluded') { + if ( + unpublished.has(docPathKey(operation.contract.method, operation.contract.path)) || + classifyOperation(operation, readRoute) === 'excluded' + ) { excluded++ continue } @@ -201,7 +219,7 @@ async function main() { writeFileSync(OUTPUT, generated) console.log( - `Wrote ${relative} — ${operations.length} operations (${excluded} excluded: binary or multipart transport).` + `Wrote ${relative} — ${operations.length} operations (${excluded} excluded: unpublished, or binary or multipart transport).` ) } From 72dee6cf24d369a34aa0ec07ad4acbe6a7cb793e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 17:04:11 -0700 Subject: [PATCH 2/3] fix(file-doc): admit joins on their validated permission and route sweep capability through the per-socket de-dup --- apps/realtime/src/access-revalidation.test.ts | 16 +++-- apps/realtime/src/access-revalidation.ts | 18 +----- apps/realtime/src/handlers/file-doc.test.ts | 22 +++++++ apps/realtime/src/handlers/file-doc.ts | 59 ++++++++++++------- apps/realtime/src/handlers/room-permission.ts | 41 +++++++++++++ .../[projectId]/[fileId]/persist/route.ts | 14 +++-- apps/sim/lib/api/server/validation.ts | 9 +-- 7 files changed, 128 insertions(+), 51 deletions(-) create mode 100644 apps/realtime/src/handlers/room-permission.ts diff --git a/apps/realtime/src/access-revalidation.test.ts b/apps/realtime/src/access-revalidation.test.ts index 577b634cdb8..ef3616e46e8 100644 --- a/apps/realtime/src/access-revalidation.test.ts +++ b/apps/realtime/src/access-revalidation.test.ts @@ -13,6 +13,7 @@ import { ACCESS_REVALIDATION_SWEEP_INTERVAL_MS, startAccessRevalidationSweep, } from '@/access-revalidation' +import { registerRoomPermissionPublisher } from '@/handlers/room-permission' import type { IRoomManager, UserPresence } from '@/rooms' interface FakeSocket { @@ -131,6 +132,8 @@ describe('access-revalidation sweep', () => { }) it('keeps a downgraded reader subscribed and reports the loss of write access', async () => { + const publish = vi.fn() + registerRoomPermissionPublisher('workspace-file-doc', publish) const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1') socket.rooms.add('table:t-1') const manager = makeManager([socket]) @@ -139,20 +142,23 @@ describe('access-revalidation sweep', () => { await sweep.runOnce() sweep.stop() expect(socket.leave).not.toHaveBeenCalled() - expect(socket.emit).toHaveBeenCalledWith('file-doc-permission', { - fileId: 'file-1', - canWrite: false, - }) + expect(publish).toHaveBeenCalledWith( + socket, + { type: 'workspace-file-doc', id: 'file-1' }, + 'read' + ) }) it('retains a document subscription for retry when authorization is unavailable', async () => { + const publish = vi.fn() + registerRoomPermissionPublisher('workspace-file-doc', publish) const socket = makeSocket('sock-1', 'user-1', 'workspace-file-doc:file-1') mockResolveRole.mockRejectedValue(new Error('database unavailable')) const sweep = startAccessRevalidationSweep(makeManager([socket])) await sweep.runOnce() sweep.stop() expect(socket.leave).not.toHaveBeenCalled() - expect(socket.emit).not.toHaveBeenCalledWith('file-doc-permission', expect.anything()) + expect(publish).not.toHaveBeenCalled() }) it('evicts only the revoked socket, not co-members of the room', async () => { diff --git a/apps/realtime/src/access-revalidation.ts b/apps/realtime/src/access-revalidation.ts index c31b1ed914b..897b10f890e 100644 --- a/apps/realtime/src/access-revalidation.ts +++ b/apps/realtime/src/access-revalidation.ts @@ -1,11 +1,9 @@ import { createLogger } from '@sim/logger' import { ROOM_MEMBERSHIP_ACTIONS, satisfiesRoomMembership } from '@sim/platform-authz/room-policy' import type { AccessRevokedBroadcast } from '@sim/realtime-protocol/events' -import { FILE_DOC_EVENTS, type FileDocPermission } from '@sim/realtime-protocol/file-doc' import { isProjectRoom, parseRoomName, - projectFileDocTarget, ROOM_TYPES, type RoomRef, type RoomType, @@ -17,6 +15,7 @@ import { runRoomEvictionHandler, setEvictionCleanupSink, } from '@/handlers/room-eviction' +import { publishRoomPermission } from '@/handlers/room-permission' import type { AuthenticatedSocket } from '@/middleware/auth' import { ROLE_REVALIDATION_TTL_MS, resolveCurrentRoomPermission } from '@/middleware/permissions' import type { IRoomManager } from '@/rooms' @@ -331,20 +330,7 @@ export function startAccessRevalidationSweep(roomManager: IRoomManager): AccessR ]) // {@link SCAN_TIMED_OUT} is the only symbol this race can yield; matching on // the type narrows it out of the permission comparison below. - if ( - typeof role !== 'symbol' && - (room.type === ROOM_TYPES.PROJECT_FILE_DOC || room.type === ROOM_TYPES.WORKSPACE_FILE_DOC) - ) { - const target = - room.type === ROOM_TYPES.PROJECT_FILE_DOC - ? projectFileDocTarget(room) - : { fileId: room.id } - if (target) - socket.emit(FILE_DOC_EVENTS.PERMISSION, { - ...target, - canWrite: role === 'write' || role === 'admin', - } satisfies FileDocPermission) - } + if (typeof role !== 'symbol') publishRoomPermission(socket, room, role) if (typeof role === 'symbol') { logger.warn( `Authorization check timed out for user ${userId} on ${name}; skipping this pass` diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index e850dda2eb3..0cc8e50fa0b 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -42,6 +42,7 @@ import { setupWorkspaceFileDocHandlers, } from '@/handlers/file-doc' import { FileDocInvalidatedError, getFileDocStore } from '@/handlers/file-doc-store' +import { publishRoomPermission } from '@/handlers/room-permission' import * as permissions from '@/middleware/permissions' import { beginRoomPermissionRead, @@ -686,6 +687,27 @@ describe('setupWorkspaceFileDocHandlers', () => { } }) + it('publishes a re-validated permission only when it changes what the socket last heard', async () => { + const { io } = createIo() + const { socket, handlers } = setup('socket-sweep', io) + await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 8 }) + await handlers[FILE_DOC_EVENTS.MESSAGE](awarenessFrame(8, 'Sweep').frame) + const room = { type: ROOM_TYPES.WORKSPACE_FILE_DOC, id: 'file-1' } as const + const subscriber = socket as unknown as Parameters[0] + const published = () => + socket.emit.mock.calls + .filter(([event]) => event === FILE_DOC_EVENTS.PERMISSION) + .map(([, payload]) => (payload as { canWrite: boolean }).canWrite) + + publishRoomPermission(subscriber, room, 'write') + publishRoomPermission(subscriber, room, 'write') + expect(published()).toEqual([true]) + + publishRoomPermission(subscriber, room, 'read') + publishRoomPermission(subscriber, room, 'read') + expect(published()).toEqual([true, false]) + }) + it('applies + fans out an agent-streamed frame (SYNC_NO_PERSIST) but never persists it', async () => { mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server')) const { io, sent } = createIo() diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index d26c6089d4f..518b87bbf17 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -75,6 +75,7 @@ import { } from '@/handlers/file-doc-store' import { evictSocketFromRoom, registerRoomEvictionHandler } from '@/handlers/room-eviction' import { resolveRoomJoinAuth } from '@/handlers/room-join-auth' +import { registerRoomPermissionPublisher } from '@/handlers/room-permission' import type { AuthenticatedSocket } from '@/middleware/auth' import { peekRoomPermission, resolveCurrentRoomPermission } from '@/middleware/permissions' import type { IRoomManager } from '@/rooms' @@ -404,7 +405,10 @@ async function persistRoom(name: string, room: FileDocRoom, final: boolean): Pro } try { const snapshot = await store.getStreamSnapshot(name, generation) - snapshotEditor = snapshot?.editor ?? null + /** Entries without an author (older relays, durable merges) keep workspace attribution local. */ + snapshotEditor = + snapshot?.editor ?? + (fileDocOwnerAdapter(room.owner).allowLocalSnapshotFallback ? localEditor : null) return snapshot?.docState ?? null } catch (streamError) { if ( @@ -1152,6 +1156,22 @@ function emitJoinError( }) } +/** Tells a socket its write capability only when it differs from what the socket last heard. */ +function publishWriteAccess( + socket: AuthenticatedSocket, + document: FileDocRoom, + permission: string | null +): void { + const canWrite = permission === 'write' || permission === 'admin' + if (document.publishedCanWrite.get(socket.id) === canWrite) return + document.publishedCanWrite.set(socket.id, canWrite) + socket.emit(FILE_DOC_EVENTS.PERMISSION, { + fileId: document.fileId, + ...fileDocOwnerWireFields(document.owner), + canWrite, + } satisfies FileDocPermission) +} + /** Resolve read membership and publish the separate editing capability after authorization. */ function resolveFileDocPermission( socket: AuthenticatedSocket, @@ -1164,15 +1184,7 @@ function resolveFileDocPermission( const room = fileDocRoom(document) const accept = (permission: string | null) => { if (socketToRoomName.get(socket.id) !== name || fileDocRooms.get(name) !== document) return null - const canWrite = permission === 'write' || permission === 'admin' - if (document.publishedCanWrite.get(socket.id) !== canWrite) { - document.publishedCanWrite.set(socket.id, canWrite) - socket.emit(FILE_DOC_EVENTS.PERMISSION, { - fileId: document.fileId, - ...fileDocOwnerWireFields(document.owner), - canWrite, - } satisfies FileDocPermission) - } + publishWriteAccess(socket, document, permission) if (permission === null) evictSocketFromRoom(socket, room, 'Your access to this document has been revoked', io) return permission @@ -1189,11 +1201,18 @@ function resolveFileDocPermission( * revocation and left the Socket.IO room). Scoped to the evicted room so a socket * that has since switched documents keeps the one it legitimately holds. */ -for (const type of [ROOM_TYPES.WORKSPACE_FILE_DOC, ROOM_TYPES.PROJECT_FILE_DOC]) +for (const type of [ROOM_TYPES.WORKSPACE_FILE_DOC, ROOM_TYPES.PROJECT_FILE_DOC]) { registerRoomEvictionHandler(type, (socketId, room, io) => { if (socketToRoomName.get(socketId) !== roomName(room)) return cleanupFileDocForSocket(socketId, io) }) + registerRoomPermissionPublisher(type, (socket, room, permission) => { + const name = roomName(room) + const document = fileDocRooms.get(name) + if (document && socketToRoomName.get(socket.id) === name) + publishWriteAccess(socket, document, permission) + }) +} async function handleMessage(socket: AuthenticatedSocket, io: Server, data: unknown) { const name = socketToRoomName.get(socket.id) @@ -1229,7 +1248,7 @@ async function handleMessage(socket: AuthenticatedSocket, io: Server, data: unkn } catch (error) { logger.warn('Dropping file-doc frame while its authorization is unavailable', { socketId: socket.id, - error, + error: getErrorMessage(error), }) return } @@ -1673,10 +1692,11 @@ export function setupWorkspaceFileDocHandlers( !socket.disconnected && joinGeneration.get(socket.id) === generation && fileDocRooms.get(name) === entry - const canRegisterJoin = () => { - if (!isCurrentJoin()) return false + /** The permission this admission is still current under, or `null` once it is not. */ + const canRegisterJoin = (): string | null => { + if (!isCurrentJoin()) return null const permission = peekRoomPermission(userId, room) - if (satisfiesRoomMembership(permission ?? null, room.type)) return true + if (permission && satisfiesRoomMembership(permission, room.type)) return permission emitJoinError( socket, fileId, @@ -1685,7 +1705,7 @@ export function setupWorkspaceFileDocHandlers( permission === undefined ? 'JOIN_FAILED' : 'ACCESS_DENIED', permission === undefined ) - return false + return null } try { // A client is attached to a WHOLE document or to nothing. A room assembles itself from the @@ -1838,7 +1858,8 @@ export function setupWorkspaceFileDocHandlers( socketToRoomName.set(socket.id, name) registered = true await changeMembership(() => socket.leave(`${name}:pending`)) - if (!canRegisterJoin()) { + const admittedPermission = canRegisterJoin() + if (!admittedPermission) { if (isCurrentJoin() && socketToRoomName.get(socket.id) === name) { cleanupFileDocForSocket(socket.id, io) await leaveUnclaimedRoom(name) @@ -1864,9 +1885,7 @@ export function setupWorkspaceFileDocHandlers( schemaVersion: FILE_DOC_SCHEMA_VERSION, ...fileDocOwnerWireFields(entry.owner), acknowledgedUpdates: true as const, - canWrite: - peekRoomPermission(userId, room) === 'write' || - peekRoomPermission(userId, room) === 'admin', + canWrite: admittedPermission === 'write' || admittedPermission === 'admin', }) // Server-authenticated roster → everyone in the room, including this joiner. broadcastFileDocPresence(io, name, entry) diff --git a/apps/realtime/src/handlers/room-permission.ts b/apps/realtime/src/handlers/room-permission.ts new file mode 100644 index 00000000000..4ca6e744197 --- /dev/null +++ b/apps/realtime/src/handlers/room-permission.ts @@ -0,0 +1,41 @@ +import { createLogger } from '@sim/logger' +import type { RoomRef, RoomType } from '@sim/realtime-protocol/rooms' +import type { AuthenticatedSocket } from '@/middleware/auth' + +const logger = createLogger('RoomPermission') + +/** Tells a socket, in its room type's own wire form, what a re-validated permission now allows. */ +export type RoomPermissionPublisher = ( + socket: AuthenticatedSocket, + room: RoomRef, + permission: string | null +) => void + +const publishers = new Map() + +/** + * Registers how a room type announces a re-validated permission. A registry for the same reason + * as the eviction handlers: the access re-validation sweep stays domain-neutral, and the room's + * handler keeps its own record of what each socket was last told. + */ +export function registerRoomPermissionPublisher( + type: RoomType, + publisher: RoomPermissionPublisher +): void { + publishers.set(type, publisher) +} + +/** Publishes a re-validated permission through the room type's publisher, if it has one. */ +export function publishRoomPermission( + socket: AuthenticatedSocket, + room: RoomRef, + permission: string | null +): void { + const publisher = publishers.get(room.type) + if (!publisher) return + try { + publisher(socket, room, permission) + } catch (error) { + logger.warn(`Room permission publish failed for socket ${socket.id} on ${room.type}`, error) + } +} diff --git a/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts b/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts index ac26030165d..4699695c7ad 100644 --- a/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts +++ b/apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts @@ -1,3 +1,4 @@ +import { NextResponse } from 'next/server' import { projectFileDocPersistContract } from '@/lib/api/contracts/project-file-doc' import { isOffAppHost } from '@/lib/api/mcp/host-routing' import { @@ -7,13 +8,12 @@ import { } from '@/lib/api/server/routes' import { realtimeProjectFileAuth } from '@/lib/auth/realtime-file-delegation' import { MAX_COLLAB_DOC_STATE_BYTES } from '@/lib/collab-doc/state' -import { OrchestrationError } from '@/lib/core/orchestration/types' import { persistProjectFileDoc } from '@/lib/projects/files/application/documents' /** A base64-encoded maximum snapshot plus 1 MiB for its JSON envelope (17 MiB). */ const PERSIST_MAX_BODY_BYTES = Math.ceil(MAX_COLLAB_DOC_STATE_BYTES / 3) * 4 + 1024 * 1024 -export const POST = defineInternalJsonRoute({ +const persist = defineInternalJsonRoute({ contract: projectFileDocPersistContract, operation: persistProjectFileDoc.operation, auth: realtimeProjectFileAuth, @@ -21,10 +21,6 @@ export const POST = defineInternalJsonRoute({ reason: 'Authenticated realtime relay; socket frame admission bounds collaborative traffic', }), errorPolicy: internalOrchestrationErrorPolicy, - /** Excluded from the proxy matcher, so the route refuses the dedicated MCP host itself. */ - beforeParse: ({ request }) => { - if (isOffAppHost(request)) throw new OrchestrationError('not_found', 'Not found') - }, mapInput: ({ params, body }) => ({ ...params, docState: new Uint8Array(Buffer.from(body.docState, 'base64')), @@ -33,3 +29,9 @@ export const POST = defineInternalJsonRoute({ useCase: persistProjectFileDoc, parseOptions: { maxBodyBytes: PERSIST_MAX_BODY_BYTES, bypassesProxy: true }, }) + +/** Excluded from the proxy matcher, so the route refuses the dedicated MCP host before auth itself. */ +export const POST: typeof persist = (request, context) => + isOffAppHost(request) + ? Promise.resolve(new NextResponse(null, { status: 404 })) + : persist(request, context) diff --git a/apps/sim/lib/api/server/validation.ts b/apps/sim/lib/api/server/validation.ts index 966e3b93d46..d5b7a2eb401 100644 --- a/apps/sim/lib/api/server/validation.ts +++ b/apps/sim/lib/api/server/validation.ts @@ -23,10 +23,11 @@ import { /** * Next.js buffers the client body for the proxy and *silently truncates* anything * past `experimental.proxyClientMaxBodySize` (default 10 MB), and `apps/sim/proxy.ts` - * matches `/api/:path*`. A larger body therefore reaches the handler as a truncated - * prefix, which fails JSON parsing — so an oversized request has to be rejected on - * its declared size before it is read, or the truncation gets misreported as a - * malformed body. + * matches every `/api` route except the few its matcher excludes (see + * {@link ParseRequestOptions.bypassesProxy}). A larger body therefore reaches the + * handler as a truncated prefix, which fails JSON parsing — so an oversized request + * has to be rejected on its declared size before it is read, or the truncation gets + * misreported as a malformed body. */ const PROXY_CLIENT_MAX_BODY_BYTES = 10 * 1024 * 1024 From 1eb5758ecccc82f60692e3311496676f1dfe49f1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 14 Oct 2026 17:09:46 -0700 Subject: [PATCH 3/3] fix(file-doc): reschedule an unattributed persist and pin admission and persist attribution with tests --- .../src/handlers/file-doc.redis-store.test.ts | 51 ++++++++++++++++++- apps/realtime/src/handlers/file-doc.test.ts | 22 ++++++++ apps/realtime/src/handlers/file-doc.ts | 6 ++- apps/realtime/src/handlers/room-permission.ts | 5 +- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/apps/realtime/src/handlers/file-doc.redis-store.test.ts b/apps/realtime/src/handlers/file-doc.redis-store.test.ts index 8ebd3e66eb6..6f5d451f3c8 100644 --- a/apps/realtime/src/handlers/file-doc.redis-store.test.ts +++ b/apps/realtime/src/handlers/file-doc.redis-store.test.ts @@ -20,9 +20,10 @@ import * as syncProtocol from 'y-protocols/sync' import * as Y from 'yjs' import type { IRoomManager } from '@/rooms' -const { mockAuthorizeRoom, mockFetchFileDocSeed } = vi.hoisted(() => ({ +const { mockAuthorizeRoom, mockFetchFileDocSeed, mockFetchFileDocPersist } = vi.hoisted(() => ({ mockAuthorizeRoom: vi.fn(), mockFetchFileDocSeed: vi.fn(), + mockFetchFileDocPersist: vi.fn(), })) vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: mockAuthorizeRoom })) @@ -30,7 +31,7 @@ vi.mock('@sim/platform-authz/rooms', () => ({ authorizeRoom: mockAuthorizeRoom } vi.mock('@/handlers/file-doc-app', () => ({ fetchFileDocSeed: mockFetchFileDocSeed, fetchFileDocMerge: vi.fn(), - fetchFileDocPersist: vi.fn().mockResolvedValue({ status: 'persisted', version: 1 }), + fetchFileDocPersist: mockFetchFileDocPersist, })) /** One in-memory Redis backing per test — only the stream/lock ops the store actually uses. */ @@ -269,6 +270,7 @@ describe('file-doc join readiness (shared store enabled)', () => { workspacePermission: 'write', }) mockFetchFileDocSeed.mockResolvedValue(null) + mockFetchFileDocPersist.mockResolvedValue({ status: 'persisted', version: 2 }) }) afterEach(() => { @@ -326,4 +328,49 @@ describe('file-doc join readiness (shared store enabled)', () => { expect(mockFetchFileDocSeed).not.toHaveBeenCalled() expect(statesDeliveredTo(socket)).toEqual(['seeded by the writer that held the lock']) }) + + it('persists unattributed stream state under the local editor when the last collaborator leaves', async () => { + backing.kv.set(`filedoc:generation:${ROOM_NAME}`, 'doc-1') + backing.kv.set(`filedoc:syncver:${ROOM_NAME}`, '1') + const doc = new Y.Doc() + doc.getText(FIELD).insert(0, 'AAA') + doc.getMap(FILE_DOC_SEED.configMap).set(FILE_DOC_SEED.flag, true) + doc.getMap(FILE_DOC_SEED.configMap).set(FILE_DOC_SEED.docIdKey, 'doc-1') + backing.streams.set(STREAM_KEY, [ + { + id: `${++backing.seq}-0`, + message: { u: Buffer.from(Y.encodeStateAsUpdate(doc)).toString('base64'), g: 'doc-1' }, + }, + ]) + const afterSeed = Y.encodeStateVector(doc) + doc.getText(FIELD).insert(0, 'BBB') + const edit = Y.encodeStateAsUpdate(doc, afterSeed) + doc.destroy() + + const { handlers } = setup('socket-1', sockets) + await handlers[FILE_DOC_EVENTS.JOIN]({ + fileId: FILE_ID, + clientId: 1, + schemaVersion: FILE_DOC_SCHEMA_VERSION, + }) + const acknowledge = vi.fn() + await (handlers[FILE_DOC_EVENTS.UPDATE] as (...args: unknown[]) => Promise)( + { fileId: FILE_ID, docId: 'doc-1', updateId: 'update-1', update: edit }, + acknowledge + ) + await vi.waitFor(() => + expect(acknowledge).toHaveBeenCalledWith({ status: 'accepted', updateId: 'update-1' }) + ) + + cleanupFileDocForSocket('socket-1', createIo(sockets), true) + await vi.waitFor(() => + expect(mockFetchFileDocPersist).toHaveBeenCalledWith( + 'ws-1', + FILE_ID, + 'user-1', + expect.any(Uint8Array), + 1 + ) + ) + }) }) diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 0cc8e50fa0b..621d3361088 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -687,6 +687,28 @@ describe('setupWorkspaceFileDocHandlers', () => { } }) + it('tells an admitted writer it can write even if its cached role lapses before the reply', async () => { + // A distinct user/file: role decisions recorded under fake timers outlive this test. + vi.useFakeTimers({ toFake: ['Date'] }) + try { + const { io } = createIo() + const data: Record = {} + // Registration records presence just before the reply; expire the role cache right there. + Object.defineProperty(data, 'fileDocPresence', { + set: () => vi.setSystemTime(Date.now() + ROLE_REVALIDATION_TTL_MS + 1), + }) + const { socket, handlers } = setup('socket-lapse', io, { userId: 'user-lapse', data }) + await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-lapse', clientId: 9 }) + + expect(socket.emit).toHaveBeenCalledWith( + FILE_DOC_EVENTS.JOIN_SUCCESS, + expect.objectContaining({ fileId: 'file-lapse', canWrite: true }) + ) + } finally { + vi.useRealTimers() + } + }) + it('publishes a re-validated permission only when it changes what the socket last heard', async () => { const { io } = createIo() const { socket, handlers } = setup('socket-sweep', io) diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index 518b87bbf17..b22b720b9bf 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -467,7 +467,11 @@ async function persistRoom(name: string, room: FileDocRoom, final: boolean): Pro const docState = await captureState() if (!docState) return // nothing seeded/authoritative to persist yet if (!(await store.isDocumentGenerationCurrent(name, generation))) return - if (!snapshotEditor) return + if (!snapshotEditor) { + // The claimed window must not strand the edits; a later flush may find an author. + if (!final && fileDocRooms.get(name) === room) schedulePersist(name, room) + return + } const result = await fileDocOwnerAdapter(room.owner).persist( room, snapshotEditor, diff --git a/apps/realtime/src/handlers/room-permission.ts b/apps/realtime/src/handlers/room-permission.ts index 4ca6e744197..e26aad41211 100644 --- a/apps/realtime/src/handlers/room-permission.ts +++ b/apps/realtime/src/handlers/room-permission.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { RoomRef, RoomType } from '@sim/realtime-protocol/rooms' +import { getErrorMessage } from '@sim/utils/errors' import type { AuthenticatedSocket } from '@/middleware/auth' const logger = createLogger('RoomPermission') @@ -36,6 +37,8 @@ export function publishRoomPermission( try { publisher(socket, room, permission) } catch (error) { - logger.warn(`Room permission publish failed for socket ${socket.id} on ${room.type}`, error) + logger.warn(`Room permission publish failed for socket ${socket.id} on ${room.type}`, { + error: getErrorMessage(error), + }) } }