Skip to content

Commit 63fee57

Browse files
committed
fix(files): recheck archive admission and hydrate copy paths once
1 parent f919e38 commit 63fee57

5 files changed

Lines changed: 105 additions & 13 deletions

File tree

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,62 @@ async function rows(projectId: string) {
161161
}
162162

163163
describe('Project file content against PostgreSQL and the local object store', () => {
164+
check(
165+
'folder tree copy hydrates destination paths once while preserving nested paths',
166+
async () => {
167+
const { copyFileItems } = await import('@/lib/workspace-files/application/copy-file-items')
168+
const f = await fixture()
169+
const root = await createProjectFileFolder.execute({
170+
principal: f.principal,
171+
input: { projectId: f.projectId, name: 'Root' },
172+
})
173+
await db.insert(folder).values(
174+
Array.from({ length: 20 }, (_, index) => ({
175+
id: generateId(),
176+
projectId: f.projectId,
177+
parentId: root.folder.id,
178+
resourceType: 'file' as const,
179+
name: `Child-${index}`,
180+
userId: f.ownerId,
181+
}))
182+
)
183+
const originalDebug = db.$client.options.debug
184+
const queries: string[] = []
185+
db.$client.options.debug = (_connection, query) => {
186+
queries.push(query)
187+
}
188+
try {
189+
const result = await copyFileItems.execute({
190+
principal: f.principal,
191+
input: {
192+
source: {
193+
owner: { entityType: 'project', entityId: f.projectId },
194+
fileIds: [],
195+
folderIds: [root.folder.id],
196+
},
197+
destination: {
198+
owner: { entityType: 'workspace', entityId: f.workspaceId },
199+
folderId: null,
200+
},
201+
},
202+
})
203+
expect(result.folders).toHaveLength(21)
204+
const copiedRoot = result.folders.find((item) => item.parentId === null)
205+
if (!copiedRoot) throw new Error('Copied root missing')
206+
expect(result.folders.filter((item) => item.parentId === copiedRoot.id)).toHaveLength(20)
207+
for (const item of result.folders.filter((item) => item.id !== copiedRoot.id))
208+
expect(item.path).toBe(`${copiedRoot.path}/${item.name}`)
209+
expect(
210+
queries.filter((query) =>
211+
query.startsWith('select "id", "name", "parent_id" from "folder"')
212+
)
213+
).toHaveLength(0)
214+
} finally {
215+
db.$client.options.debug = originalDebug
216+
}
217+
}
218+
)
219+
164220
check(
165221
'copy selection ignores unrelated folders when the owner exceeds the selection limit',
166222
async () => {

‎apps/sim/lib/uploads/contexts/workspace/file-folder-manager.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -679,11 +679,21 @@ export async function createWorkspaceFileFolder(
679679
return workspaceFolderRecord(folder)
680680
}
681681

682-
/** Creates one folder under the caller's owner transaction and the shared hierarchy lock. */
682+
/** Creates one folder and hydrates its display path for single-item callers. */
683683
export async function createFileFolder<O extends EditableFileOwner>(
684684
params: CreateFileFolderParams<O>,
685685
tx: DbTransaction
686686
): Promise<FileFolderRecord<O>> {
687+
const inserted = await createFileFolderRow(params, tx)
688+
const paths = buildWorkspaceFileFolderPathMap(await selectFileFolderPathRows(params.owner, tx))
689+
return mapFileFolder(inserted, params.owner, paths)
690+
}
691+
692+
/** Uses canonical folder validation and locking without rebuilding paths during a bulk mutation. */
693+
export async function createFileFolderRow<O extends EditableFileOwner>(
694+
params: CreateFileFolderParams<O>,
695+
tx: DbTransaction
696+
): Promise<typeof folderTable.$inferSelect> {
687697
const requestedName = normalizeWorkspaceFileItemName(params.name, 'Folder')
688698
await acquireFileFolderMutationLock(tx, params.owner)
689699
const parentId = await assertFileFolderTarget(params.owner, params.parentId, tx)
@@ -745,8 +755,7 @@ export async function createFileFolder<O extends EditableFileOwner>(
745755
if (getPostgresErrorCode(error) === '23505') throw new WorkspaceFileFolderConflictError(name)
746756
throw error
747757
}
748-
const paths = buildWorkspaceFileFolderPathMap(await selectFileFolderPathRows(params.owner, tx))
749-
return mapFileFolder(inserted, params.owner, paths)
758+
return inserted
750759
}
751760

752761
/**

‎apps/sim/lib/workspace-files/application/download-workspace-file-items.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ const mockProvenance =
6363
import { DEFAULT_PERMISSION_GROUP_CONFIG } from '@/lib/permission-groups/fields'
6464
import { downloadWorkspaceFileItems } from '@/lib/workspace-files/application/download-workspace-file-items'
6565
import { MAX_ZIP_DOWNLOAD_BYTES } from '@/lib/workspace-files/limits'
66+
import { recheckFileReadReceipt } from '@/lib/workspace-files/read-receipt'
6667

6768
const { mockResolveVerifiedUserAccessControlContext: mockAccessContext } =
6869
permissionGroupsResolveMockFns
@@ -111,6 +112,7 @@ function file(id: string, name: string, folderId: string | null = null, size = 1
111112
describe('downloadWorkspaceFileItems', () => {
112113
beforeEach(() => {
113114
events.length = 0
115+
vi.mocked(recheckFileReadReceipt).mockResolvedValue({ status: 'exact', entries: [] })
114116
mockLoadContext.mockImplementation(async () => {
115117
events.push('resolve')
116118
return workspace
@@ -318,6 +320,19 @@ describe('downloadWorkspaceFileItems', () => {
318320
).rejects.toMatchObject({ code: 'validation' })
319321
})
320322

323+
it('refuses refreshed unsafe provenance even after initial admission succeeds', async () => {
324+
vi.mocked(recheckFileReadReceipt).mockResolvedValue({ status: 'unknown' })
325+
await expect(
326+
observeContentDelivery(secretFreeGate, () =>
327+
downloadWorkspaceFileItems.execute({
328+
principal: delegatedPrincipal,
329+
input: { workspaceId: 'ws-1', fileIds: ['f1'], folderIds: [] },
330+
})
331+
)
332+
).rejects.toMatchObject({ code: 'forbidden' })
333+
expect(recheckFileReadReceipt).toHaveBeenCalled()
334+
})
335+
321336
it('rejects an unknown folder in a mixed selection', async () => {
322337
await expect(
323338
downloadWorkspaceFileItems.execute({

‎apps/sim/lib/workspace-files/application/download-workspace-file-items.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
ContentRefusedError,
77
capabilityGovernedPrincipalUserId,
88
} from '@/lib/core/application'
9+
import { requireAdmittedContent } from '@/lib/core/application/content-delivery'
910
import { OrchestrationError } from '@/lib/core/orchestration/types'
1011
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1112
import { assertWorkspaceCapability } from '@/lib/permission-groups/capability-assertions'
@@ -16,6 +17,7 @@ import {
1617
loadWorkspaceFileOperationContext,
1718
type WorkspaceFileRecord,
1819
} from '@/lib/uploads/contexts/workspace'
20+
import { mergeWorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/file-secret-provenance'
1921
import { docNotReadyMessage, isDocNotReadyError } from '@/lib/uploads/utils/doc-not-ready'
2022
import {
2123
formatFileSize,
@@ -245,12 +247,20 @@ async function executeDownloadWorkspaceFileItems({
245247
}))
246248
)
247249
)
248-
await db.transaction(
250+
const provenance = await db.transaction(
249251
async (tx) => {
250-
for (const receipt of receipts) await recheckFileReadReceipt(tx, receipt)
252+
const evidence = []
253+
for (const receipt of receipts) evidence.push(await recheckFileReadReceipt(tx, receipt))
254+
return mergeWorkspaceFileSecretProvenance(...evidence)
251255
},
252256
{ isolationLevel: 'repeatable read', accessMode: 'read only' }
253257
)
258+
await requireAdmittedContent(principal, {
259+
kind: 'workspace-file',
260+
id: context.workspaceId,
261+
provenance,
262+
view: 'opaque',
263+
})
254264
const included = new Set(filesToZip.map((file) => file.id))
255265
return {
256266
excludedFileIds: selectedFiles.filter((file) => !included.has(file.id)).map((file) => file.id),

‎apps/sim/lib/workspace-files/copy.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { DbTransaction } from '@/lib/db/types'
55
import {
66
assertFileFolderTarget,
77
buildWorkspaceFileFolderPathMap,
8-
createFileFolder,
8+
createFileFolderRow,
99
listFileFolders,
1010
} from '@/lib/uploads/contexts/workspace/file-folder-manager'
1111
import {
@@ -171,12 +171,11 @@ export async function commitFileCopyInTx(
171171
const targetFolderId = await assertFileFolderTarget(destination.owner, destination.folderId, tx)
172172
const pending = new Map(snapshot.folders.map((row) => [row.id, row]))
173173
const mapped = new Map<string, string>()
174-
const folders: CopiedFileItems['folders'] = []
175174
while (pending.size) {
176175
let progressed = false
177176
for (const [id, source] of pending) {
178177
if (source.parentId && pending.has(source.parentId)) continue
179-
const created = await createFileFolder(
178+
const created = await createFileFolderRow(
180179
{
181180
owner: destination.owner,
182181
userId,
@@ -188,7 +187,6 @@ export async function commitFileCopyInTx(
188187
},
189188
tx
190189
)
191-
folders.push(created)
192190
mapped.set(id, created.id)
193191
pending.delete(id)
194192
progressed = true
@@ -216,11 +214,15 @@ export async function commitFileCopyInTx(
216214
})
217215
)
218216
}
219-
const paths = buildWorkspaceFileFolderPathMap(
220-
await listFileFolders(destination.owner, undefined, tx)
221-
)
217+
const destinationFolders = await listFileFolders(destination.owner, undefined, tx)
218+
const paths = buildWorkspaceFileFolderPathMap(destinationFolders)
219+
const folderById = new Map(destinationFolders.map((folder) => [folder.id, folder]))
222220
return {
223221
files: createdFiles.map((file) => mapFileRecord(file, destination.owner, paths)),
224-
folders,
222+
folders: [...mapped.values()].map((id) => {
223+
const folder = folderById.get(id)
224+
if (!folder) throw new Error('Copied folder is missing from the destination')
225+
return folder
226+
}),
225227
}
226228
}

0 commit comments

Comments
 (0)