Skip to content

Commit 19d46f1

Browse files
committed
fix(chat): release-audit follow-ups for Project file tabs and sandbox copy parsing
1 parent 5f49cbf commit 19d46f1

6 files changed

Lines changed: 126 additions & 29 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,22 @@ describe('organization resource stream', () => {
469469
expect(workspace.removeResource).not.toHaveBeenCalled()
470470
expect(mocks.invalidateResourceQueries).not.toHaveBeenCalled()
471471
})
472+
473+
it('opens a Project file tab, which names no workspace, in an organization chat', () => {
474+
const org = makeStreamLoopDeps({ workspaceId: undefined })
475+
const projectFile = {
476+
type: 'file' as const,
477+
id: 'file-1',
478+
title: 'context.md',
479+
owner: { entityType: 'project' as const, entityId: 'project-1' },
480+
}
481+
handleResourceEvent({ deps: org } as StreamLoopContext, {
482+
...removeEvent('file', 'file-1'),
483+
payload: { op: 'upsert', resource: projectFile },
484+
})
485+
expect(org.addResource).toHaveBeenCalledWith(projectFile)
486+
expect(mocks.invalidateResourceQueries).not.toHaveBeenCalled()
487+
})
472488
})
473489

474490
describe('owner-addressed effects on a legacy tab', () => {

‎apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,19 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
194194
if (owner?.entityType === 'project' && payload.resource.type === 'file' && payload.resource.id)
195195
invalidateProjectFileQueries(queryClient, owner.entityId, payload.resource.id)
196196
const workspaceId = payload.resource.workspaceId ?? chatWorkspaceId
197-
if (!workspaceId || (chatWorkspaceId && workspaceId !== chatWorkspaceId)) return
197+
/** A Project file names no workspace, so an organization chat has none to check it against. */
198+
if (
199+
workspaceId
200+
? chatWorkspaceId && workspaceId !== chatWorkspaceId
201+
: owner?.entityType !== 'project'
202+
)
203+
return
198204
// Browser and terminal tabs are projected from the desktop app's live
199205
// lists, never from the stream; older servers announced them as resources.
200206
if (payload.resource.type === 'browser' || payload.resource.type === 'terminal') return
201207
const resourceType = payload.resource.type
202-
invalidateResourceQueries(queryClient, workspaceId, resourceType, payload.resource.id)
208+
if (workspaceId)
209+
invalidateResourceQueries(queryClient, workspaceId, resourceType, payload.resource.id)
203210
if (resourceType === 'workflow' && payload.op !== 'remove' && payload.resource.id) {
204211
notifyWorkflowExternalUpdate(payload.resource.id)
205212
}
@@ -246,7 +253,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
246253
: undefined
247254
const resource: MothershipResource = {
248255
...payload.resource,
249-
...(chatWorkspaceId ? {} : { workspaceId }),
256+
...(!chatWorkspaceId && workspaceId ? { workspaceId } : {}),
250257
type: payload.resource.type as MothershipResourceType,
251258
title:
252259
typeof payload.resource.title === 'string' ? payload.resource.title : payload.resource.id,
@@ -264,7 +271,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
264271
current === getChatResourceSelectionId(resource) ? null : current
265272
)
266273
} else removeResource(resource)
267-
if (resourceType === 'workflow') {
274+
if (resourceType === 'workflow' && workspaceId) {
268275
removeWorkflowFromActiveCache(queryClient, workspaceId, resource.id)
269276
}
270277
return
@@ -352,7 +359,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
352359
} else onResourceEvent?.(getChatResourceSelectionId(resource))
353360
}
354361

355-
if (resource.type === 'workflow') {
362+
if (resource.type === 'workflow' && workspaceId) {
356363
ensureWorkflowInRegistry(resource.id, resource.title, workspaceId)
357364
}
358365
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
2+
import { schemaMock } from '@sim/testing/mocks/schema.mock'
3+
import { beforeEach, describe, expect, it } from 'vitest'
4+
import { changeStoredChatResources } from '@/lib/mothership/resources/store'
5+
import type { MothershipResource } from '@/lib/mothership/resources/types'
6+
7+
const unscopedFile: MothershipResource = { type: 'file', id: 'file-1', title: 'notes.md' }
8+
const scopedFile: MothershipResource = { ...unscopedFile, workspaceId: 'ws' }
9+
const table: MothershipResource = { type: 'table', id: 'table-1', title: 'Accounts' }
10+
11+
function storeWorkspaceChat(resources: MothershipResource[]) {
12+
queueTableRows(schemaMock.copilotChats, [{ resources, workspaceId: 'ws' }])
13+
}
14+
15+
describe('changeStoredChatResources reorder', () => {
16+
beforeEach(() => {
17+
resetDbChainMock()
18+
})
19+
20+
it('keeps the reorder when a workspace chat holds a tab both unscoped and scoped', async () => {
21+
storeWorkspaceChat([unscopedFile, scopedFile, table])
22+
23+
const reordered = await changeStoredChatResources('chat-1', {
24+
kind: 'reorder',
25+
resources: [table, unscopedFile, scopedFile],
26+
})
27+
28+
expect(reordered.map((resource) => resource.type)).toEqual(['table', 'file'])
29+
})
30+
31+
it('still rejects an order that drops or invents a resource', async () => {
32+
storeWorkspaceChat([scopedFile, table])
33+
await expect(
34+
changeStoredChatResources('chat-1', { kind: 'reorder', resources: [table, table] })
35+
).rejects.toThrow('Reordered resources must match existing resources')
36+
37+
storeWorkspaceChat([scopedFile, table])
38+
await expect(
39+
changeStoredChatResources('chat-1', {
40+
kind: 'reorder',
41+
resources: [table, { ...unscopedFile, id: 'file-2' }],
42+
})
43+
).rejects.toThrow('Reordered resources must match existing resources')
44+
})
45+
})

‎apps/sim/lib/mothership/resources/store.ts‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type MothershipResource,
99
type MothershipResourceUpdate,
1010
mergeChatResource,
11+
reorderStoredChatResources,
1112
sanitizeChatResources,
1213
} from '@/lib/mothership/resources/types'
1314

@@ -112,25 +113,20 @@ export async function changeStoredChatResources(
112113
const incoming = sanitizeChatResources(
113114
change.resources.filter((resource) => resource.id !== 'streaming-file')
114115
)
115-
const byKey = new Map(existing.map((resource) => [keyOf(resource), resource]))
116116
if (change.kind === 'reorder') {
117-
const keys = incoming.map(keyOf)
118-
if (
119-
keys.length !== byKey.size ||
120-
new Set(keys).size !== keys.length ||
121-
keys.some((key) => !byKey.has(key))
122-
) {
117+
const reordered = reorderStoredChatResources(
118+
existing,
119+
incoming,
120+
chat.workspaceId ?? undefined
121+
)
122+
if (!reordered)
123123
throw new OrchestrationError(
124124
'validation',
125125
'Reordered resources must match existing resources'
126126
)
127-
}
128-
resources = keys.map((key) => {
129-
const resource = byKey.get(key)
130-
if (!resource) throw new OrchestrationError('validation', 'Resource order changed')
131-
return resource
132-
})
127+
resources = reordered
133128
} else {
129+
const byKey = new Map(existing.map((resource) => [keyOf(resource), resource]))
134130
for (const resource of incoming) {
135131
const key = keyOf(resource)
136132
const previous = byKey.get(key)

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import { urlsMockFns } from '@sim/testing/mocks/urls.mock'
77
import { beforeEach, describe, expect, it, vi } from 'vitest'
88
import { copilotRequestPrincipal, isCopilotRequest } from '@/lib/api/server/routes/copilot-request'
9+
import { DEFAULT_MAX_JSON_BODY_BYTES } from '@/lib/api/server/validation'
910
import {
1011
type ContentSource,
1112
noteContentDeliveryOperation,
@@ -465,3 +466,31 @@ it('validates a copy body itself once the lease negotiated the file-owner protoc
465466
expect(response.status).toBe(400)
466467
expect(fetcher).not.toHaveBeenCalled()
467468
})
469+
470+
it('leaves an oversized copy body to the workspace route without reading it past the cap', async () => {
471+
readScope.mockResolvedValue({ ...scope, fileOwnerProtocolVersion: undefined })
472+
routeMatcher.mockReturnValue({ params: {}, load: async () => ({ POST: fetcher }) })
473+
fetcher.mockResolvedValue(Response.json({ error: 'Request body is too large' }, { status: 413 }))
474+
const projectCopy = new TextEncoder().encode(
475+
JSON.stringify({
476+
source: { owner: workspaceOwner, fileIds: ['file'] },
477+
destination: { owner: { entityType: 'project', entityId: 'project' } },
478+
})
479+
)
480+
const padding = new Uint8Array(1024 * 1024).fill(0x20)
481+
const paddingChunks = Math.ceil((2 * DEFAULT_MAX_JSON_BODY_BYTES) / padding.byteLength)
482+
let pulledChunks = 0
483+
const body = new ReadableStream<Uint8Array>({
484+
pull(controller) {
485+
if (pulledChunks === 0) controller.enqueue(projectCopy)
486+
else if (pulledChunks <= paddingChunks) controller.enqueue(padding)
487+
else controller.close()
488+
pulledChunks++
489+
},
490+
})
491+
const init: RequestInit & { duplex: 'half' } = { method: 'POST', body, duplex: 'half' }
492+
const response = await proxySandboxResourceRequest(request('/api/v2/files/copy', init), token)
493+
expect(response.status).toBe(413)
494+
expect(fetcher).toHaveBeenCalledTimes(1)
495+
expect(pulledChunks).toBeLessThan(paddingChunks)
496+
})

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.ts‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { createLogger } from '@sim/logger'
22
import { generateId } from '@sim/utils/id'
33
import { NextRequest } from 'next/server'
4-
import { fileCopyInputSchema } from '@/lib/api/contracts/file-copy-input'
54
import { v2CopyFileItemsContract } from '@/lib/api/contracts/v2/file-copy'
65
import { markCopilotRequest } from '@/lib/api/server/routes/copilot-request'
76
import { matchV2Route } from '@/lib/api/server/routes/in-process-transport'
7+
import { V2_PARSE_DEFAULTS } from '@/lib/api/server/routes/v2-json-route'
8+
import { parseRequest } from '@/lib/api/server/validation'
89
import { withWorkspaceInvocationScope } from '@/lib/core/application/workspace-invocation-scope'
910
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
1011
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
@@ -203,18 +204,21 @@ async function proxyAuthorizedSandboxRequest(
203204
})
204205
}
205206

206-
/** A valid copy naming a non-workspace source or destination needs the file-owner protocol. */
207+
/**
208+
* A valid copy naming a non-workspace source or destination needs the file-owner protocol. Parsed
209+
* as the copy route parses it, under the same body cap: an oversized or invalid body stays on the
210+
* workspace route, which returns its own 413 or 400.
211+
*/
207212
async function copiesBeyondWorkspaces(request: Request): Promise<boolean> {
208-
let body: unknown
209-
try {
210-
body = await request.clone().json()
211-
} catch {
212-
return false
213-
}
214-
const parsed = fileCopyInputSchema.safeParse(body)
213+
const parsed = await parseRequest(
214+
v2CopyFileItemsContract,
215+
new NextRequest(request.clone()),
216+
{},
217+
V2_PARSE_DEFAULTS
218+
)
215219
return (
216220
parsed.success &&
217-
(parsed.data.source.owner.entityType !== 'workspace' ||
218-
parsed.data.destination.owner.entityType !== 'workspace')
221+
(parsed.data.body.source.owner.entityType !== 'workspace' ||
222+
parsed.data.body.destination.owner.entityType !== 'workspace')
219223
)
220224
}

0 commit comments

Comments
 (0)