Skip to content

Commit ed4bdde

Browse files
committed
fix(files): harden bulk admission and staged publication
1 parent f7a6a5a commit ed4bdde

14 files changed

Lines changed: 456 additions & 77 deletions

File tree

‎apps/sim/app/api/projects/[id]/files/download/route.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export const GET = defineInternalBinaryRoute({
1414
operation: downloadProjectFileItems.operation,
1515
rateLimit: internalRateLimits.none({ reason: 'Authenticated bounded Project archive download' }),
1616
errorPolicy: internalOrchestrationErrorPolicy,
17+
headSafe: false,
1718
mapInput: ({ params, query }) => ({ projectId: params.id, ...query }),
1819
useCase: downloadProjectFileItems,
1920
present: presentProjectFileDownload,

‎apps/sim/lib/api/contracts/project-file-downloads.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { PASTE_LIMITS } from '@sim/utils/paste'
21
import type { z } from 'zod'
32
import {
43
projectFileParamsSchema,
@@ -18,8 +17,11 @@ export const downloadProjectFileItemsContract = defineRouteContract({
1817
response: { mode: 'binary' },
1918
})
2019

21-
/** JSON can encode each source byte as a six-byte Unicode escape, plus the envelope. */
22-
export const MAX_PROJECT_FILE_SNAPSHOT_BODY_BYTES = 6 * PASTE_LIMITS.RICH_MARKDOWN_BYTES + 1024
20+
/**
21+
* The proxy accepts at most 10 MiB of encoded JSON, independently of the decoded Markdown cap.
22+
* Heavily escaped snapshots can hit this wire limit before their decoded-content limit.
23+
*/
24+
export const MAX_PROJECT_FILE_SNAPSHOT_BODY_BYTES = 10 * 1024 * 1024
2325

2426
export const exportProjectFileSnapshotBodySchema = exportWorkspaceFileSnapshotBodySchema.strict()
2527
export type ExportProjectFileSnapshotBody = z.input<typeof exportProjectFileSnapshotBodySchema>

‎apps/sim/lib/api/contracts/v2/openapi/files-audit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export const unpublishedFileCopyOpenApiRoutes = [
163163
operationId: 'copyFileItems',
164164
summary: 'Copy File Items',
165165
description: `Copy selected files and folder trees between workspace or Project owners. Source read and destination write access are checked independently. Copies receive new identities and retain source secret provenance. Registration is atomic, with destination names resolved by the existing copy rules. ${WORKSPACE_API_KEY_DENIED}`,
166-
errors: [...RESOURCE_CONFLICT_ERRORS, 'PayloadTooLarge'],
166+
errors: [...RESOURCE_CONFLICT_ERRORS, 'PayloadTooLarge', 'UnsupportedMediaType'],
167167
success: { description: 'New file and folder identities with their destination owner.' },
168168
}),
169169
{

‎apps/sim/lib/projects/files/__integration__/content-bulk.integration.ts‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { mkdtempSync } from 'node:fs'
22
import { mkdir, readdir, readFile, rm, truncate, writeFile } from 'node:fs/promises'
33
import { tmpdir } from 'node:os'
44
import { dirname, join } from 'node:path'
5+
import type { Principal } from '@sim/auth/principal'
56
import { db } from '@sim/db'
67
import {
78
folder,
@@ -34,6 +35,7 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
3435
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
3536
vi.mock('@/lib/messaging/email/mailer', () => emailMailerMock)
3637

38+
import { observeContentDelivery } from '@/lib/core/application/content-delivery'
3739
import {
3840
createProjectFile,
3941
readProjectFileContent,
@@ -159,6 +161,73 @@ async function rows(projectId: string) {
159161
}
160162

161163
describe('Project file content against PostgreSQL and the local object store', () => {
164+
check(
165+
'copy selection ignores unrelated folders when the owner exceeds the selection limit',
166+
async () => {
167+
const { snapshotFileCopyInTx } = await import('@/lib/workspace-files/copy')
168+
const f = await fixture()
169+
const selected = generateId()
170+
await db.insert(folder).values(
171+
Array.from({ length: 5_001 }, (_, index) => ({
172+
id: index === 0 ? selected : generateId(),
173+
projectId: f.projectId,
174+
resourceType: 'file' as const,
175+
name: `folder-${index}`,
176+
userId: f.ownerId,
177+
}))
178+
)
179+
const result = await db.transaction((tx) =>
180+
snapshotFileCopyInTx(tx, {
181+
owner: { entityType: 'project', entityId: f.projectId },
182+
fileIds: [],
183+
folderIds: [selected],
184+
})
185+
)
186+
expect(result.folders.map((item) => item.id)).toEqual([selected])
187+
expect(result.files).toHaveLength(0)
188+
}
189+
)
190+
191+
check('copy retains committed bytes when COMMIT acknowledgement is lost', async () => {
192+
const { copyFileItems } = await import('@/lib/workspace-files/application/copy-file-items')
193+
const f = await fixture()
194+
const source = await createProjectFile.execute({
195+
principal: f.principal,
196+
input: createInput(f.projectId, 'retained after lost acknowledgement'),
197+
})
198+
const original = db.transaction.bind(db)
199+
vi.spyOn(db, 'transaction').mockImplementation(async (callback, config) => {
200+
const result = await original(callback, config)
201+
if (result && typeof result === 'object' && 'usage' in result && 'result' in result)
202+
throw new Error('Lost COMMIT acknowledgement')
203+
return result
204+
})
205+
await expect(
206+
copyFileItems.execute({
207+
principal: f.principal,
208+
input: {
209+
source: {
210+
owner: { entityType: 'project', entityId: f.projectId },
211+
fileIds: [source.file.id],
212+
folderIds: [],
213+
},
214+
destination: { owner: { entityType: 'project', entityId: f.projectId }, folderId: null },
215+
},
216+
})
217+
).rejects.toThrow('Lost COMMIT acknowledgement')
218+
vi.restoreAllMocks()
219+
const copied = (await rows(f.projectId)).find((file) => file.id !== source.file.id)
220+
if (!copied) throw new Error('Copy did not commit')
221+
expect(await readFile(join(localStorageRoot, copied.key), 'utf8')).toBe(
222+
'retained after lost acknowledgement'
223+
)
224+
const cleanup = await db
225+
.select()
226+
.from(outboxEvent)
227+
.where(sql`${outboxEvent.payload}::text LIKE ${`%${copied.key}%`}`)
228+
expect(cleanup).toHaveLength(0)
229+
})
230+
162231
check(
163232
'compound copy remaps selected page assets before staging and bills the actual rendered source bytes',
164233
async () => {
@@ -449,6 +518,99 @@ describe('Project file content against PostgreSQL and the local object store', (
449518
})
450519

451520
describe('Project archive download and Markdown snapshot export', () => {
521+
check('snapshot export refuses an opaque embedded asset before fetching it', async () => {
522+
const { exportProjectFileSnapshot } = await import('@/lib/projects/files/application/downloads')
523+
const f = await fixture()
524+
const source = await createProjectFile.execute({
525+
principal: f.principal,
526+
input: createInput(f.projectId, '# source'),
527+
})
528+
const image = await createProjectFile.execute({
529+
principal: f.principal,
530+
input: {
531+
...createInput(f.projectId, 'image bytes'),
532+
name: 'image.png',
533+
contentType: 'image/png',
534+
},
535+
})
536+
const principal: Principal = {
537+
kind: 'resource_delegated',
538+
serviceId: 'copilot',
539+
subjectUserId: f.editorId,
540+
audience: 'sim:project-files',
541+
delegationId: generateId(),
542+
issuedAt: new Date(),
543+
expiresAt: new Date(Date.now() + 30_000),
544+
scope: { kind: 'entity', entityType: 'project', entityId: f.projectId },
545+
invocation: { kind: 'workspace', workspaceId: f.workspaceId },
546+
}
547+
const read = vi.spyOn(storage, 'downloadFile')
548+
await expect(
549+
observeContentDelivery(
550+
{
551+
async admit(sources) {
552+
expect(sources.every((item) => item.view === 'opaque')).toBe(true)
553+
return sources.map((item) => item.id !== image.file.id)
554+
},
555+
},
556+
() =>
557+
exportProjectFileSnapshot.execute({
558+
principal,
559+
input: {
560+
projectId: f.projectId,
561+
fileId: source.file.id,
562+
content: `![image](sim:file/${image.file.id}?project=${f.projectId})`,
563+
secretProvenance: { status: 'exact', entries: [] },
564+
},
565+
})
566+
)
567+
).rejects.toMatchObject({ code: 'forbidden' })
568+
expect(read).not.toHaveBeenCalled()
569+
})
570+
571+
for (const phase of ['before-read', 'before-delivery'] as const) {
572+
check(`Project archive opaque admission refuses ${phase}`, async () => {
573+
const { downloadProjectFileItems } = await import(
574+
'@/lib/projects/files/application/downloads'
575+
)
576+
const f = await fixture()
577+
const { file } = await createProjectFile.execute({
578+
principal: f.principal,
579+
input: createInput(f.projectId, 'private bytes'),
580+
})
581+
const principal: Principal = {
582+
kind: 'resource_delegated',
583+
serviceId: 'copilot',
584+
subjectUserId: f.editorId,
585+
audience: 'sim:project-files',
586+
delegationId: generateId(),
587+
issuedAt: new Date(),
588+
expiresAt: new Date(Date.now() + 30_000),
589+
scope: { kind: 'entity', entityType: 'project', entityId: f.projectId },
590+
invocation: { kind: 'workspace', workspaceId: f.workspaceId },
591+
}
592+
const read = vi.spyOn(storage, 'downloadFile')
593+
let admissions = 0
594+
await expect(
595+
observeContentDelivery(
596+
{
597+
async admit(sources) {
598+
admissions++
599+
expect(sources.every((source) => source.view === 'opaque')).toBe(true)
600+
return sources.map(() => phase === 'before-delivery' && admissions === 1)
601+
},
602+
},
603+
() =>
604+
downloadProjectFileItems.execute({
605+
principal,
606+
input: { projectId: f.projectId, fileIds: [file.id], folderIds: [] },
607+
})
608+
)
609+
).rejects.toMatchObject({ code: 'forbidden' })
610+
expect(read).toHaveBeenCalledTimes(phase === 'before-read' ? 0 : 1)
611+
})
612+
}
613+
452614
check(
453615
'Project recursive download retains nested paths, deduplicates selections, and excludes archived or foreign items',
454616
async () => {
@@ -908,6 +1070,52 @@ describe('Project ZIP extraction against PostgreSQL and local storage', () => {
9081070
})
9091071
}
9101072

1073+
check('extraction retains committed bytes when COMMIT acknowledgement is lost', async () => {
1074+
const { extractProjectFile } = await import('@/lib/projects/files/application/extract')
1075+
const f = await fixture()
1076+
const source = await archive(f, { 'retained.txt': 'retained extracted bytes' })
1077+
const original = db.transaction.bind(db)
1078+
vi.spyOn(db, 'transaction').mockImplementation(async (callback, config) => {
1079+
const result = await original(callback, config)
1080+
if (result && typeof result === 'object' && 'usage' in result && 'result' in result)
1081+
throw new Error('Lost COMMIT acknowledgement')
1082+
return result
1083+
})
1084+
await expect(
1085+
extractProjectFile.execute({
1086+
principal: f.principal,
1087+
input: { projectId: f.projectId, fileId: source.file.id },
1088+
})
1089+
).rejects.toThrow('Lost COMMIT acknowledgement')
1090+
vi.restoreAllMocks()
1091+
const child = (await rows(f.projectId)).find((file) => file.id !== source.file.id)
1092+
if (!child) throw new Error('Extraction did not commit')
1093+
expect(await readFile(join(localStorageRoot, child.key), 'utf8')).toBe(
1094+
'retained extracted bytes'
1095+
)
1096+
const cleanup = await db
1097+
.select()
1098+
.from(outboxEvent)
1099+
.where(sql`${outboxEvent.payload}::text LIKE ${`%${child.key}%`}`)
1100+
expect(cleanup).toHaveLength(0)
1101+
})
1102+
1103+
check('extraction rejects an overlong directory before staging a member', async () => {
1104+
const { extractProjectFile } = await import('@/lib/projects/files/application/extract')
1105+
const f = await fixture()
1106+
const source = await archive(f, { [`${'x'.repeat(256)}/entry.txt`]: 'content' })
1107+
const upload = vi.spyOn(storage, 'uploadFile')
1108+
await expect(
1109+
extractProjectFile.execute({
1110+
principal: f.principal,
1111+
input: { projectId: f.projectId, fileId: source.file.id },
1112+
})
1113+
).rejects.toMatchObject({ code: 'validation' })
1114+
expect(upload).not.toHaveBeenCalled()
1115+
expect(await folders(f.projectId)).toHaveLength(0)
1116+
expect(await rows(f.projectId)).toHaveLength(1)
1117+
})
1118+
9111119
async function folders(projectId: string) {
9121120
return db.select().from(folder).where(eq(folder.projectId, projectId))
9131121
}

0 commit comments

Comments
 (0)