From 64d0ae540be9208e819da3434af45d9ca69df4c9 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 11 May 2026 11:20:21 +0200 Subject: [PATCH] feat(documents): block rename/move/delete on system folders assertNotSystemManaged centralises the guard so the three mutation paths surface identical ConflictError shapes. System roots and per- entity subfolders are immutable through the rep-facing API; the only way for system_managed to flip back to false is the entity-hard- delete demotion path (next task). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/services/document-folders.service.ts | 35 ++++++---- .../document-folders-system-folders.test.ts | 64 +++++++++++++++++-- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/src/lib/services/document-folders.service.ts b/src/lib/services/document-folders.service.ts index d94fd7f9..e6fcb852 100644 --- a/src/lib/services/document-folders.service.ts +++ b/src/lib/services/document-folders.service.ts @@ -28,6 +28,26 @@ function isSiblingNameConflict(err: unknown): boolean { return constraint === 'uniq_document_folders_sibling_name'; } +/** + * Throws ConflictError if the folder is system-managed. Centralises the + * rejection so rename/move/delete all surface identical error shapes. + */ +async function assertNotSystemManaged( + portId: string, + folderId: string, + action: 'rename' | 'move' | 'delete', +): Promise { + const folder = await db.query.documentFolders.findFirst({ + where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)), + }); + if (!folder) throw new NotFoundError('Folder'); + if (folder.systemManaged) { + const verb = action === 'rename' ? 'renamed' : action === 'move' ? 'moved' : 'deleted'; + throw new ConflictError(`System folders can't be ${verb}`); + } + return folder; +} + export interface FolderNode extends DocumentFolder { children: FolderNode[]; } @@ -130,10 +150,7 @@ export async function renameFolder( if (!trimmed) throw new ValidationError('Folder name cannot be empty'); if (trimmed.length > 200) throw new ValidationError('Folder name cannot exceed 200 chars'); - const existing = await db.query.documentFolders.findFirst({ - where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)), - }); - if (!existing) throw new NotFoundError('Folder'); + const existing = await assertNotSystemManaged(portId, folderId, 'rename'); try { const [updated] = await db @@ -178,10 +195,7 @@ export async function moveFolder( throw new ValidationError('Cannot move a folder under itself (cycle)'); } - const folder = await db.query.documentFolders.findFirst({ - where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)), - }); - if (!folder) throw new NotFoundError('Folder'); + const folder = await assertNotSystemManaged(portId, folderId, 'move'); if (newParentId !== null) { const newParent = await db.query.documentFolders.findFirst({ @@ -247,10 +261,7 @@ export async function deleteFolderSoftRescue( folderId: string, userId: string, ): Promise { - const folder = await db.query.documentFolders.findFirst({ - where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)), - }); - if (!folder) throw new NotFoundError('Folder'); + const folder = await assertNotSystemManaged(portId, folderId, 'delete'); const newParent = folder.parentId; // null = re-parent to root diff --git a/tests/unit/document-folders-system-folders.test.ts b/tests/unit/document-folders-system-folders.test.ts index 6c1bd16f..1caa58a6 100644 --- a/tests/unit/document-folders-system-folders.test.ts +++ b/tests/unit/document-folders-system-folders.test.ts @@ -104,18 +104,13 @@ describe('document-folders service · ensureEntityFolder', () => { const all = await db .select() .from(documentFolders) - .where( - and(eq(documentFolders.entityType, 'client'), eq(documentFolders.entityId, clientId)), - ); + .where(and(eq(documentFolders.entityType, 'client'), eq(documentFolders.entityId, clientId))); expect(all).toHaveLength(1); }); it('appends a numeric suffix on name collision with an existing folder', async () => { // Insert a second client with the exact same fullName as the first. - const [firstClient] = await db - .select() - .from(clients) - .where(eq(clients.id, clientId)); + const [firstClient] = await db.select().from(clients).where(eq(clients.id, clientId)); const sharedName = firstClient!.fullName; const [collidingClient] = await db @@ -138,3 +133,58 @@ describe('document-folders service · ensureEntityFolder', () => { ).rejects.toThrow(/entity type/i); }); }); + +import { + deleteFolderSoftRescue, + moveFolder, + renameFolder, +} from '@/lib/services/document-folders.service'; + +describe('document-folders service · system folder protection', () => { + let portId: string; + let rootId: string; + + beforeEach(async () => { + const port = await makePort(); + portId = port.id; + await db.delete(documentFolders).where(eq(documentFolders.portId, portId)); + const roots = await ensureSystemRoots(portId, TEST_USER_ID); + rootId = roots.find((r) => r.name === 'Clients')!.id; + }); + + it('rejects rename of a system-managed root', async () => { + await expect(renameFolder(portId, rootId, 'Customers', TEST_USER_ID)).rejects.toThrow( + /system folder/i, + ); + }); + + it('rejects move of a system-managed root', async () => { + const other = await ensureSystemRoots(portId, TEST_USER_ID); + const companies = other.find((r) => r.name === 'Companies')!; + await expect(moveFolder(portId, rootId, companies.id, TEST_USER_ID)).rejects.toThrow( + /system folder/i, + ); + }); + + it('rejects delete of a system-managed root', async () => { + await expect(deleteFolderSoftRescue(portId, rootId, TEST_USER_ID)).rejects.toThrow( + /system folder/i, + ); + }); + + it('allows rename/delete of a user folder under a system root', async () => { + const user = await db + .insert(documentFolders) + .values({ + portId, + parentId: rootId, + name: 'Templates', + systemManaged: false, + createdBy: TEST_USER_ID, + }) + .returning(); + await expect( + renameFolder(portId, user[0]!.id, 'My Templates', TEST_USER_ID), + ).resolves.toBeDefined(); + }); +});