Skip to content

Commit 0711c61

Browse files
committed
fix(folders): large-scale audit fixes across locking engine and folder orchestration
- resource-lock.ts: fix ancestor-walk short-circuit that let a locked ancestor go unnoticed when bypassing a direct lock on the target itself (affects assertFolderMutableUnlessUnlocking/assertResourceMutableUnlessUnlocking) - workflow.ts: thread the inherited flag through WorkflowLockedError (was always false) - folders/orchestration.ts: recheck cycle-freedom inside the transaction for kb/table folder reparents (was only checked against a stale pre-tx read); add missing recordAudit + 409-on-duplicate-name for kb/table folder create/update - workflows/persistence/duplicate.ts: assertTargetFolderMutable now scopes by resourceType='workflow' and row-locks the ancestor chain (was letting a workflow duplicate reparent into a file/kb/table folder) - table/service.ts: wrap renameTable's lock checks and write in a transaction (was running with no transaction and no FOR UPDATE recheck) - workflow-lifecycle.ts: wrap performUpdateWorkflow in a transaction with an in-tx lock recheck (was relying solely on a non-transactional route pre-check) - knowledge/service.ts: recheck updateKnowledgeBase's lock inside the transaction after acquiring the row lock - workflows/orchestration/folder-lifecycle.ts: check the target folder's own lock before cascading into children (was checking only after) - workspace-files/orchestration/file-folder-lifecycle.ts: propagate the real errorCode from performRestoreFolder instead of hardcoding not_found - uploads/contexts/workspace/workspace-file-folder-manager.ts: fix missing import of assertFolderMutableUnlessUnlocking (broke typecheck) - hooks/queries/folders.ts: invalidate the correct resource-type-scoped list on folder delete/restore instead of always invalidating workflow lists - knowledge.tsx: folder rows now honor the active sort column/direction - background/cleanup-soft-deletes.ts: hard-delete retention now covers file/table/knowledge_base folders, not just workflow folders - workspace-vfs.ts: surface archived table/knowledge_base folders under recently-deleted/ so they can be discovered and restored - table API routes: include folderId/locked in create and detail responses - schema.mock.ts: add missing folderId/locked to workspaceFiles/knowledgeBase/ userTableDefinitions mocks - add regression test coverage for the resource-lock ancestor-walk fix and update table service tests for the new transactional renameTable call shape
1 parent 93c6995 commit 0711c61

19 files changed

Lines changed: 534 additions & 96 deletions

File tree

apps/sim/app/api/table/[tableId]/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export const GET = withRouteHandler(async (request: NextRequest, { params }: Tab
6767
metadata: table.metadata ?? null,
6868
rowCount: table.rowCount,
6969
maxRows: maxRowsPerTable,
70+
folderId: table.folderId,
7071
locked: table.locked,
7172
createdAt:
7273
table.createdAt instanceof Date

apps/sim/app/api/table/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
129129
},
130130
rowCount: table.rowCount,
131131
maxRows: table.maxRows,
132+
folderId: table.folderId,
133+
locked: table.locked,
132134
createdAt:
133135
table.createdAt instanceof Date
134136
? table.createdAt.toISOString()

apps/sim/app/workspace/[workspaceId]/knowledge/knowledge.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,20 @@ export function Knowledge() {
324324
folder.name.toLowerCase().includes(debouncedSearchQuery.toLowerCase())
325325
)
326326
: siblings
327-
return [...searched].sort((a, b) => a.name.localeCompare(b.name))
328-
}, [folders, currentFolderId, debouncedSearchQuery])
327+
const col = activeSort?.column ?? 'name'
328+
const dir = activeSort?.direction ?? 'asc'
329+
return [...searched].sort((a, b) => {
330+
let cmp = 0
331+
if (col === 'updated') {
332+
cmp = new Date(a.updatedAt).getTime() - new Date(b.updatedAt).getTime()
333+
} else if (col === 'created') {
334+
cmp = new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()
335+
} else {
336+
cmp = a.name.localeCompare(b.name)
337+
}
338+
return dir === 'asc' ? cmp : -cmp
339+
})
340+
}, [folders, currentFolderId, debouncedSearchQuery, activeSort])
329341

330342
const processedKBs = useMemo(() => {
331343
let result = filterKnowledgeBases(knowledgeBases, debouncedSearchQuery).filter(

apps/sim/background/cleanup-soft-deletes.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,34 @@ const CLEANUP_TARGETS = [
139139
table: workflowFolder,
140140
softDeleteCol: workflowFolder.deletedAt,
141141
wsCol: workflowFolder.workspaceId,
142-
// `folder` is now a polymorphic table shared with file/knowledge_base/table
143-
// folders — scope hard-delete to workflow folders only.
142+
// `folder` is now a polymorphic table shared across all four resourceTypes --
143+
// each resourceType gets its own CLEANUP_TARGETS entry (below) scoped by
144+
// `extraPredicate` so every resourceType's soft-deleted folders eventually get
145+
// purged, not just workflow's.
144146
extraPredicate: eq(workflowFolder.resourceType, 'workflow'),
145147
name: 'workflowFolder',
146148
},
149+
{
150+
table: workflowFolder,
151+
softDeleteCol: workflowFolder.deletedAt,
152+
wsCol: workflowFolder.workspaceId,
153+
extraPredicate: eq(workflowFolder.resourceType, 'file'),
154+
name: 'fileFolder',
155+
},
156+
{
157+
table: workflowFolder,
158+
softDeleteCol: workflowFolder.deletedAt,
159+
wsCol: workflowFolder.workspaceId,
160+
extraPredicate: eq(workflowFolder.resourceType, 'knowledge_base'),
161+
name: 'knowledgeBaseFolder',
162+
},
163+
{
164+
table: workflowFolder,
165+
softDeleteCol: workflowFolder.deletedAt,
166+
wsCol: workflowFolder.workspaceId,
167+
extraPredicate: eq(workflowFolder.resourceType, 'table'),
168+
name: 'tableFolder',
169+
},
147170
{
148171
table: knowledgeBase,
149172
softDeleteCol: knowledgeBase.deletedAt,

apps/sim/hooks/queries/folders.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import { generateId } from '@sim/utils/id'
3+
import type { QueryClient } from '@tanstack/react-query'
34
import { keepPreviousData, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
45
import { requestJson } from '@/lib/api/client/request'
56
import {
@@ -13,17 +14,44 @@ import {
1314
restoreFolderContract,
1415
updateFolderContract,
1516
} from '@/lib/api/contracts'
17+
import { knowledgeKeys } from '@/hooks/queries/kb/knowledge'
1618
import { getFolderMap } from '@/hooks/queries/utils/folder-cache'
1719
import { type FolderQueryScope, folderKeys } from '@/hooks/queries/utils/folder-keys'
1820
import { invalidateWorkflowLists } from '@/hooks/queries/utils/invalidate-workflow-lists'
1921
import {
2022
createOptimisticMutationHandlers,
2123
generateTempId,
2224
} from '@/hooks/queries/utils/optimistic-mutation'
25+
import { tableKeys } from '@/hooks/queries/utils/table-keys'
2326
import { getTopInsertionSortOrder } from '@/hooks/queries/utils/top-insertion-sort-order'
2427
import { getWorkflows } from '@/hooks/queries/utils/workflow-cache'
28+
import { workspaceFilesKeys } from '@/hooks/queries/workspace-files'
2529
import type { Folder } from '@/stores/folders/types'
2630

31+
/**
32+
* A folder delete/restore cascades to the resources it contains -- invalidate the
33+
* resource-type-scoped list (not just the folder list itself) so those cascaded
34+
* changes show up without a manual refetch.
35+
*/
36+
function invalidateResourceListsForFolder(
37+
queryClient: QueryClient,
38+
workspaceId: string,
39+
resourceType: FolderResourceType = DEFAULT_FOLDER_RESOURCE_TYPE
40+
) {
41+
switch (resourceType) {
42+
case 'workflow':
43+
return invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived'])
44+
case 'table':
45+
return queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
46+
case 'knowledge_base':
47+
return queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() })
48+
case 'file':
49+
return queryClient.invalidateQueries({ queryKey: workspaceFilesKeys.lists() })
50+
default:
51+
return Promise.resolve()
52+
}
53+
}
54+
2755
const logger = createLogger('FolderQueries')
2856

2957
export const FOLDER_LIST_STALE_TIME = 60 * 1000
@@ -268,7 +296,11 @@ export function useDeleteFolderMutation() {
268296
},
269297
onSettled: (_data, _error, variables) => {
270298
queryClient.invalidateQueries({ queryKey: folderKeys.lists() })
271-
return invalidateWorkflowLists(queryClient, variables.workspaceId, ['active', 'archived'])
299+
return invalidateResourceListsForFolder(
300+
queryClient,
301+
variables.workspaceId,
302+
variables.resourceType
303+
)
272304
},
273305
})
274306
}
@@ -291,7 +323,11 @@ export function useRestoreFolder() {
291323
},
292324
onSettled: (_data, _error, variables) => {
293325
queryClient.invalidateQueries({ queryKey: folderKeys.lists() })
294-
return invalidateWorkflowLists(queryClient, variables.workspaceId, ['active', 'archived'])
326+
return invalidateResourceListsForFolder(
327+
queryClient,
328+
variables.workspaceId,
329+
variables.resourceType
330+
)
295331
},
296332
})
297333
}

apps/sim/lib/copilot/vfs/workspace-vfs.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,6 +2235,8 @@ export class WorkspaceVFS {
22352235
archivedFiles,
22362236
archivedFileFolders,
22372237
archivedKBs,
2238+
archivedTableFolders,
2239+
archivedKnowledgeBaseFolders,
22382240
] = await Promise.all([
22392241
listWorkflows(workspaceId, { scope: 'archived' }),
22402242
db
@@ -2255,6 +2257,38 @@ export class WorkspaceVFS {
22552257
listWorkspaceFiles(workspaceId, { scope: 'archived' }),
22562258
listWorkspaceFileFolders(workspaceId, { scope: 'archived' }),
22572259
getKnowledgeBases(userId, workspaceId, 'archived'),
2260+
// `table`/`knowledge_base` folders live in the same polymorphic `folder`
2261+
// table as workflow folders -- without these, an archived table/KB folder
2262+
// is invisible under `recently-deleted/` and the agent has no way to
2263+
// discover it in order to restore it.
2264+
db
2265+
.select({
2266+
id: workflowFolder.id,
2267+
name: workflowFolder.name,
2268+
archivedAt: workflowFolder.deletedAt,
2269+
})
2270+
.from(workflowFolder)
2271+
.where(
2272+
and(
2273+
eq(workflowFolder.workspaceId, workspaceId),
2274+
eq(workflowFolder.resourceType, 'table'),
2275+
isNotNull(workflowFolder.deletedAt)
2276+
)
2277+
),
2278+
db
2279+
.select({
2280+
id: workflowFolder.id,
2281+
name: workflowFolder.name,
2282+
archivedAt: workflowFolder.deletedAt,
2283+
})
2284+
.from(workflowFolder)
2285+
.where(
2286+
and(
2287+
eq(workflowFolder.workspaceId, workspaceId),
2288+
eq(workflowFolder.resourceType, 'knowledge_base'),
2289+
isNotNull(workflowFolder.deletedAt)
2290+
)
2291+
),
22582292
])
22592293

22602294
for (const wf of archivedWorkflows) {
@@ -2294,6 +2328,40 @@ export class WorkspaceVFS {
22942328
)
22952329
}
22962330

2331+
for (const folder of archivedTableFolders) {
2332+
const safeName = sanitizeName(folder.name)
2333+
this.files.set(
2334+
`recently-deleted/table-folders/${safeName}/meta.json`,
2335+
JSON.stringify(
2336+
{
2337+
id: folder.id,
2338+
name: folder.name,
2339+
archivedAt: folder.archivedAt,
2340+
type: 'table_folder',
2341+
},
2342+
null,
2343+
2
2344+
)
2345+
)
2346+
}
2347+
2348+
for (const folder of archivedKnowledgeBaseFolders) {
2349+
const safeName = sanitizeName(folder.name)
2350+
this.files.set(
2351+
`recently-deleted/knowledgebase-folders/${safeName}/meta.json`,
2352+
JSON.stringify(
2353+
{
2354+
id: folder.id,
2355+
name: folder.name,
2356+
archivedAt: folder.archivedAt,
2357+
type: 'knowledge_base_folder',
2358+
},
2359+
null,
2360+
2
2361+
)
2362+
)
2363+
}
2364+
22972365
for (const folder of archivedFileFolders) {
22982366
const safePath = folder.path
22992367
.split('/')

apps/sim/lib/folders/orchestration.ts

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,16 @@ export async function performCreateFolder(
765765
return row
766766
})
767767

768+
recordAudit({
769+
workspaceId: params.workspaceId,
770+
actorId: params.userId,
771+
action: AuditAction.FOLDER_CREATED,
772+
resourceType: AuditResourceType.FOLDER,
773+
resourceId: created.id,
774+
resourceName: created.name,
775+
description: `Created ${cascadeResourceLabel(params.resourceType)} folder "${created.name}"`,
776+
})
777+
768778
return { success: true, folder: created }
769779
} catch (error) {
770780
// Propagate uncaught so the route's ResourceLockedError -> 423 handling applies.
@@ -774,6 +784,13 @@ export async function performCreateFolder(
774784
if (error instanceof FolderParentNotFoundError) {
775785
return { success: false, error: error.message, errorCode: 'validation' }
776786
}
787+
if (getPostgresErrorCode(error) === '23505') {
788+
return {
789+
success: false,
790+
error: 'A folder with this name already exists in this location',
791+
errorCode: 'conflict',
792+
}
793+
}
777794
throw error
778795
}
779796
}
@@ -862,6 +879,21 @@ export async function performUpdateFolder(
862879
if (!targetParent) {
863880
throw new FolderParentNotFoundError('Parent folder not found')
864881
}
882+
883+
// The cycle check above (`checkFolderCircularReference`) ran against an
884+
// unlocked pre-transaction snapshot -- a concurrent reparent could form a
885+
// cycle in the window between that read and this transaction (the batch
886+
// reorder path was hardened against exactly this race). Re-run it here
887+
// against `tx` so it observes the same FOR-UPDATE-locked parent chain this
888+
// transaction is about to write.
889+
const wouldCreateCycleInTx = await checkFolderCircularReference(
890+
params.folderId,
891+
params.parentId,
892+
tx
893+
)
894+
if (wouldCreateCycleInTx) {
895+
throw new FolderCycleError('Cannot create circular folder reference')
896+
}
865897
}
866898

867899
const [row] = await tx
@@ -880,6 +912,17 @@ export async function performUpdateFolder(
880912
})
881913

882914
if (!updated) return { success: false, error: 'Folder not found', errorCode: 'not_found' }
915+
916+
recordAudit({
917+
workspaceId: params.workspaceId,
918+
actorId: params.userId,
919+
action: AuditAction.FOLDER_UPDATED,
920+
resourceType: AuditResourceType.FOLDER,
921+
resourceId: updated.id,
922+
resourceName: updated.name,
923+
description: `Updated ${cascadeResourceLabel(params.resourceType)} folder "${updated.name}"`,
924+
})
925+
883926
return { success: true, folder: updated }
884927
} catch (error) {
885928
// Propagate uncaught so the route's ResourceLockedError -> 423 handling applies.
@@ -889,6 +932,16 @@ export async function performUpdateFolder(
889932
if (error instanceof FolderParentNotFoundError) {
890933
return { success: false, error: error.message, errorCode: 'validation' }
891934
}
935+
if (error instanceof FolderCycleError) {
936+
return { success: false, error: error.message, errorCode: 'validation' }
937+
}
938+
if (getPostgresErrorCode(error) === '23505') {
939+
return {
940+
success: false,
941+
error: 'A folder with this name already exists in this location',
942+
errorCode: 'conflict',
943+
}
944+
}
892945
throw error
893946
}
894947
}
@@ -935,7 +988,7 @@ class FolderParentNotFoundError extends Error {}
935988
/** Marks a concurrent-deletion race caught inside the reorder transaction as a 404, not a 500. */
936989
class FolderReorderNotFoundError extends Error {}
937990
/** Marks a cycle caught inside the reorder transaction as a 400, not a 500. */
938-
class FolderReorderCycleError extends Error {}
991+
class FolderCycleError extends Error {}
939992
/** Marks a closure that went stale between discovery and locking, caught inside the reorder transaction, as a 409, not a 500. */
940993
class FolderReorderStaleClosureError extends Error {}
941994

@@ -1111,7 +1164,7 @@ export async function performReorderFolders(params: PerformReorderFoldersParams)
11111164
let cursor: string | null = update.id
11121165
while (cursor) {
11131166
if (visited.has(cursor)) {
1114-
throw new FolderReorderCycleError('Cannot create circular folder reference')
1167+
throw new FolderCycleError('Cannot create circular folder reference')
11151168
}
11161169
visited.add(cursor)
11171170
cursor = lockedParentById.get(cursor) ?? null
@@ -1148,7 +1201,7 @@ export async function performReorderFolders(params: PerformReorderFoldersParams)
11481201
if (error instanceof FolderReorderNotFoundError) {
11491202
return { success: false, updated: 0, error: error.message, errorCode: 'not_found' }
11501203
}
1151-
if (error instanceof FolderReorderCycleError) {
1204+
if (error instanceof FolderCycleError) {
11521205
return { success: false, updated: 0, error: error.message, errorCode: 'validation' }
11531206
}
11541207
if (error instanceof FolderReorderStaleClosureError) {

0 commit comments

Comments
 (0)