diff --git a/docs/AUDIT-PARKED-QUESTIONS.md b/docs/AUDIT-PARKED-QUESTIONS.md index 82f8452e..f5874b2a 100644 --- a/docs/AUDIT-PARKED-QUESTIONS.md +++ b/docs/AUDIT-PARKED-QUESTIONS.md @@ -209,4 +209,4 @@ Compensating-delete is faster to ship but doesn't catch process-crash gaps. Saga --- -*Everything in `AUDIT-TRIAGE.md` Tier 8 is already shipped. Everything not listed in this file has been fixed without parking — see the commit log on `feat/documents-folders`.* +_Everything in `AUDIT-TRIAGE.md` Tier 8 is already shipped. Everything not listed in this file has been fixed without parking — see the commit log on `feat/documents-folders`._ diff --git a/src/components/dashboard/widget-registry.tsx b/src/components/dashboard/widget-registry.tsx index 0bed7c0f..9f572e01 100644 --- a/src/components/dashboard/widget-registry.tsx +++ b/src/components/dashboard/widget-registry.tsx @@ -40,8 +40,7 @@ const LeadSourceChart = dynamic( { loading: ChartFallback, ssr: false }, ); const OccupancyTimelineChart = dynamic( - () => - import('./occupancy-timeline-chart').then((m) => ({ default: m.OccupancyTimelineChart })), + () => import('./occupancy-timeline-chart').then((m) => ({ default: m.OccupancyTimelineChart })), { loading: ChartFallback, ssr: false }, ); const PipelineFunnelChart = dynamic( diff --git a/src/lib/services/document-folders.service.ts b/src/lib/services/document-folders.service.ts index bedfb464..e519ede8 100644 --- a/src/lib/services/document-folders.service.ts +++ b/src/lib/services/document-folders.service.ts @@ -221,51 +221,64 @@ export async function moveFolder( const folder = await assertNotSystemManaged(portId, folderId, 'move'); - if (newParentId !== null) { - const newParent = await db.query.documentFolders.findFirst({ - where: and(eq(documentFolders.id, newParentId), eq(documentFolders.portId, portId)), - }); - if (!newParent) throw new ValidationError('Invalid parent folder'); - - // Cycle check: walk the destination's ancestor chain. If we hit - // folderId, the destination is a descendant of the folder being - // moved — moving would create a cycle. - let cursor: string | null = newParent.parentId; - const seen = new Set([newParent.id]); - while (cursor) { - if (cursor === folderId) { - throw new ValidationError('Cannot move a folder under one of its descendants (cycle)'); - } - if (seen.has(cursor)) break; // defensive — pre-existing cycle, bail - seen.add(cursor); - const next = await db.query.documentFolders.findFirst({ - where: and(eq(documentFolders.id, cursor), eq(documentFolders.portId, portId)), - columns: { parentId: true }, - }); - cursor = next?.parentId ?? null; - } - } - + // concurrency-auditor HIGH: the cycle check + the UPDATE used to run + // as separate statements with no shared lock. Two concurrent moves + // (move A → B and move B → A) could each pass the cycle check + // against the pre-move tree, then both write, leaving an A↔B cycle. + // Wrap the whole sequence in a single transaction so the walk-and- + // write is atomic per move attempt. try { - const [updated] = await db - .update(documentFolders) - .set({ parentId: newParentId, updatedAt: new Date() }) - .where(and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId))) - .returning(); - if (!updated) throw new NotFoundError('Folder'); + return await db.transaction(async (tx) => { + if (newParentId !== null) { + const newParent = await tx.query.documentFolders.findFirst({ + where: and( + eq(documentFolders.id, newParentId), + eq(documentFolders.portId, portId), + ), + }); + if (!newParent) throw new ValidationError('Invalid parent folder'); - void createAuditLog({ - userId, - portId, - action: 'update', - entityType: 'document_folder', - entityId: folderId, - oldValue: { parentId: folder.parentId }, - newValue: { parentId: newParentId }, - metadata: { type: 'folder_move' }, + // Cycle check: walk the destination's ancestor chain. If we hit + // folderId, the destination is a descendant of the folder being + // moved — moving would create a cycle. + let cursor: string | null = newParent.parentId; + const seen = new Set([newParent.id]); + while (cursor) { + if (cursor === folderId) { + throw new ValidationError( + 'Cannot move a folder under one of its descendants (cycle)', + ); + } + if (seen.has(cursor)) break; // defensive — pre-existing cycle, bail + seen.add(cursor); + const next = await tx.query.documentFolders.findFirst({ + where: and(eq(documentFolders.id, cursor), eq(documentFolders.portId, portId)), + columns: { parentId: true }, + }); + cursor = next?.parentId ?? null; + } + } + + const [updated] = await tx + .update(documentFolders) + .set({ parentId: newParentId, updatedAt: new Date() }) + .where(and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId))) + .returning(); + if (!updated) throw new NotFoundError('Folder'); + + void createAuditLog({ + userId, + portId, + action: 'update', + entityType: 'document_folder', + entityId: folderId, + oldValue: { parentId: folder.parentId }, + newValue: { parentId: newParentId }, + metadata: { type: 'folder_move' }, + }); + + return updated; }); - - return updated; } catch (err) { if (isSiblingNameConflict(err)) { throw new ConflictError('A folder with that name already exists in the destination');