security(books): enforce per-file mutation permissions

Require explicit delete or metadata-edit permission before mutating a book file. Keep catalog state intact when physical deletion fails.

Closes #1293

server/src/modules/book/book.controller.test.ts

@@ -992,6 +992,11 @@ describe('BookController', () => {
     }
   });
 
+  it('requires mutation permissions for per-file rename and delete', () => {
+    expect(Reflect.getMetadata(PERMISSION_KEY, BookController.prototype.renameFile)).toBe(Permission.LibraryEditMetadata);
+    expect(Reflect.getMetadata(PERMISSION_KEY, BookController.prototype.deleteFile)).toBe(Permission.LibraryDeleteBooks);
+  });
+
   it('preserves valid surrogate pairs while stripping lone surrogates in download filenames', async () => {
     const { controller, bookService } = makeController();
     const { reply, headers } = makeReply();

server/src/modules/book/book.controller.ts

@@ -484,12 +484,14 @@ export class BookController {
 
   @Patch('files/:fileId')
   @HttpCode(HttpStatus.NO_CONTENT)
+  @RequirePermission(Permission.LibraryEditMetadata)
   async renameFile(@Param('fileId', ParseIntPipe) fileId: number, @Body() dto: UpdateBookFileDto, @CurrentUser() user: RequestUser) {
     await this.bookService.renameFile(fileId, dto, user);
   }
 
   @Delete('files/:fileId')
   @HttpCode(HttpStatus.NO_CONTENT)
+  @RequirePermission(Permission.LibraryDeleteBooks)
   async deleteFile(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser) {
     await this.bookService.deleteFile(fileId, user);
   }

server/src/modules/book/book.service.test.ts

@@ -1,4 +1,4 @@
-import { BadRequestException, ConflictException, ForbiddenException, Logger, NotFoundException } from '@nestjs/common';
+import { BadRequestException, ConflictException, ForbiddenException, InternalServerErrorException, Logger, NotFoundException } from '@nestjs/common';
 import type { MockedFunction } from 'vitest';
 import { access, readdir, rm, stat, rename } from 'fs/promises';
 
@@ -5256,5 +5256,26 @@ describe('BookService', () => {
       expect(bookRepo.deleteBookFile).toHaveBeenCalledWith(fileId);
       expect(bookRepo.updateBookPrimaryFile).toHaveBeenCalledWith(10, 101);
     });
+
+    it('preserves the database row and primary file when disk deletion fails', async () => {
+      const { service, bookRepo } = makeService();
+      const fileId = 100;
+      const file = { absolutePath: '/path/to/old.epub', bookId: 10, libraryId: 1 };
+
+      bookRepo.findFileById = vi.fn().mockResolvedValue(file);
+      bookRepo.deleteBookFile = vi.fn();
+      bookRepo.findFilesForBook = vi.fn();
+      bookRepo.findBookBase = vi.fn();
+      bookRepo.updateBookPrimaryFile = vi.fn();
+      vi.mocked(rm).mockRejectedValue(Object.assign(new Error('permission denied'), { code: 'EACCES' }));
+
+      await expect(service.deleteFile(fileId, makeUser())).rejects.toBeInstanceOf(InternalServerErrorException);
+
+      expect(rm).toHaveBeenCalledWith(file.absolutePath, { force: true });
+      expect(bookRepo.findBookBase).not.toHaveBeenCalled();
+      expect(bookRepo.deleteBookFile).not.toHaveBeenCalled();
+      expect(bookRepo.findFilesForBook).not.toHaveBeenCalled();
+      expect(bookRepo.updateBookPrimaryFile).not.toHaveBeenCalled();
+    });
   });
 });

server/src/modules/book/book.service.ts

@@ -1487,13 +1487,9 @@ export class BookService {
       try {
         await rm(file.absolutePath, { force: true });
       } catch {
-        this.logger.warn(`Failed to physically delete file at ${file.absolutePath}`);
+        throw new InternalServerErrorException('Failed to delete file from disk');
       }
 
-      // the file watcher will eventually catch the unlink and clean up the database.
-      // however, to be responsive, we can manually clean up the database here too,
-      // but if the file is the last file, the scanner logic is better suited to mark the book missing.
-      // So we leave the DB cleanup to the file watcher, which is more robust.
       const book = await this.bookRepo.findBookBase(file.bookId);
       const wasPrimary = book?.primaryFileId === fileId;
 

server/test/authorization-matrix.e2e-spec.ts

@@ -1,5 +1,6 @@
 import { randomUUID } from 'crypto';
 import { readFileSync } from 'fs';
+import { access, chmod } from 'fs/promises';
 import { dirname, join } from 'path';
 import { fileURLToPath } from 'url';
 
@@ -91,6 +92,7 @@ interface Personas {
   koboRevoked: TestUserSession;
   bookDockUser: TestUserSession;
   uploadUser: TestUserSession;
+  downloadOnlyUser: TestUserSession;
   ownerUser: TestUserSession;
   otherUser: TestUserSession;
   targetSuperuser: TestUserSession;
@@ -224,6 +226,7 @@ describe('Authorization matrix (e2e)', () => {
       koboRevoked: await createUserAndLogin(ctx, { permissions: [Permission.KoboSync] }),
       bookDockUser: await createUserAndLogin(ctx, { permissions: [Permission.BookDockAccess, Permission.LibraryUpload] }),
       uploadUser: await createUserAndLogin(ctx, { permissions: [Permission.LibraryUpload] }),
+      downloadOnlyUser: await createUserAndLogin(ctx, { permissions: [Permission.LibraryDownload] }),
       ownerUser: await createUserAndLogin(ctx),
       otherUser: await createUserAndLogin(ctx),
       targetSuperuser: await createUserAndLogin(ctx, { isSuperuser: true }),
@@ -234,6 +237,7 @@ describe('Authorization matrix (e2e)', () => {
       grantLibraryAccess(ctx, personas.allPermsUser.userId, libraryB.libraryId, 'owner'),
       grantLibraryAccess(ctx, personas.metadataEditor.userId, libraryA.libraryId, 'viewer'),
       grantLibraryAccess(ctx, personas.bookDockUser.userId, libraryA.libraryId, 'viewer'),
+      grantLibraryAccess(ctx, personas.downloadOnlyUser.userId, libraryA.libraryId, 'viewer'),
       grantLibraryAccess(ctx, personas.ownerUser.userId, libraryA.libraryId, 'viewer'),
       grantLibraryAccess(ctx, personas.opdsOwner.userId, libraryA.libraryId, 'viewer'),
       grantLibraryAccess(ctx, personas.opdsIntruder.userId, libraryA.libraryId, 'viewer'),
@@ -489,6 +493,56 @@ describe('Authorization matrix (e2e)', () => {
   });
 
   describe('guard matrix - jwt/permission/library/default-password', () => {
+    it('blocks per-file mutations for a download-only viewer without changing the file record', async () => {
+      const renameResponse = await ctx.app.inject({
+        method: 'PATCH',
+        url: `/api/v1/books/files/${bookA.bookFileId}`,
+        headers: authHeader(personas.downloadOnlyUser.accessToken),
+        payload: { filename: 'unauthorized-rename.epub' },
+      });
+      expectError(renameResponse, 403, 'Missing permission: library_edit_metadata');
+
+      const deleteResponse = await ctx.app.inject({
+        method: 'DELETE',
+        url: `/api/v1/books/files/${bookA.bookFileId}`,
+        headers: authHeader(personas.downloadOnlyUser.accessToken),
+      });
+      expectError(deleteResponse, 403, 'Missing permission: library_delete_books');
+
+      await expect(access(bookA.absolutePath)).resolves.toBeUndefined();
+      const [fileRow] = await ctx.db
+        .select({ id: schema.bookFiles.id, absolutePath: schema.bookFiles.absolutePath })
+        .from(schema.bookFiles)
+        .where(eq(schema.bookFiles.id, bookA.bookFileId));
+      expect(fileRow).toEqual({ id: bookA.bookFileId, absolutePath: bookA.absolutePath });
+    });
+
+    it('preserves the file record when physical deletion fails', async () => {
+      const failurePath = await createEpubFixture(libraryA.folderPath, `locked-${randomUUID()}/delete-failure.epub`);
+      await triggerAndWaitForLibraryScan(ctx, libraryA.libraryId);
+      const failureTarget = await locateBookByAbsolutePath(ctx, failurePath);
+      const lockedDirectory = dirname(failurePath);
+
+      await chmod(lockedDirectory, 0o555);
+      try {
+        const response = await ctx.app.inject({
+          method: 'DELETE',
+          url: `/api/v1/books/files/${failureTarget.bookFileId}`,
+          headers: authHeader(personas.allPermsUser.accessToken),
+        });
+        expectError(response, 500, 'Failed to delete file from disk');
+
+        await expect(access(failurePath)).resolves.toBeUndefined();
+        const [fileRow] = await ctx.db
+          .select({ id: schema.bookFiles.id, absolutePath: schema.bookFiles.absolutePath })
+          .from(schema.bookFiles)
+          .where(eq(schema.bookFiles.id, failureTarget.bookFileId));
+        expect(fileRow).toEqual({ id: failureTarget.bookFileId, absolutePath: failurePath });
+      } finally {
+        await chmod(lockedDirectory, 0o755);
+      }
+    });
+
     it('keeps the dashboard scroller inventory in parity with the live application', () => {
       expect(routeInventory.routes).toHaveLength(routeInventory.totalRoutes);
       const inventoryLabels = routeInventory.routes

server/test/e2e/authorization-matrix/route-inventory/manifest.json

@@ -1,10 +1,10 @@
 {
-  "totalRoutes": 453,
+  "totalRoutes": 455,
   "byPermission": {
     "(none)": 168,
     "Permission.ManageMetadataConfig": 34,
     "Permission.EmailSend": 30,
-    "Permission.LibraryEditMetadata": 34,
+    "Permission.LibraryEditMetadata": 35,
     "Permission.BookDockAccess": 17,
     "Permission.ManageAppSettings": 39,
     "Permission.ManageLibraries": 17,
@@ -17,7 +17,7 @@
     "Permission.OpdsAccess": 4,
     "Permission.ViewAuditLog": 1,
     "Permission.ViewUserActivity": 6,
-    "Permission.LibraryDeleteBooks": 10,
+    "Permission.LibraryDeleteBooks": 11,
     "Permission.LibraryDownload": 1,
     "Permission.LibraryUpload": 2,
     "Permission.ManageBookDock": 3,

server/test/e2e/authorization-matrix/route-inventory/routes-02.json

@@ -239,6 +239,30 @@
     "allowDefault": false,
     "useGuards": []
   },
+  {
+    "file": "server/src/modules/book/book.controller.ts",
+    "className": "BookController",
+    "methodName": "renameFile",
+    "httpMethod": "PATCH",
+    "path": "/books/files/:fileId",
+    "permissions": ["Permission.LibraryEditMetadata"],
+    "libraryAccess": [],
+    "isPublic": false,
+    "allowDefault": false,
+    "useGuards": []
+  },
+  {
+    "file": "server/src/modules/book/book.controller.ts",
+    "className": "BookController",
+    "methodName": "deleteFile",
+    "httpMethod": "DELETE",
+    "path": "/books/files/:fileId",
+    "permissions": ["Permission.LibraryDeleteBooks"],
+    "libraryAccess": [],
+    "isPublic": false,
+    "allowDefault": false,
+    "useGuards": []
+  },
   {
     "file": "server/src/modules/book/book.controller.ts",
     "className": "BookController",

server/test/e2e/authorization-matrix/route-inventory/uncovered-routes.json

@@ -9,7 +9,6 @@
   "DELETE /books/:id|:bookId/cover",
   "DELETE /books/:id|:bookId/reading-attempts/:attemptId",
   "DELETE /books/:id|:bookId/sessions/:sessionId",
-  "DELETE /books/files/:fileId",
   "DELETE /books/files/:fileId/progress",
   "DELETE /custom-icons/:slug",
   "DELETE /custom-metadata/fields/:id",
@@ -160,7 +159,6 @@
   "PATCH /books/:id|:bookId/personal-note",
   "PATCH /books/:id|:bookId/reading-attempts/:attemptId",
   "PATCH /books/:id|:bookId/status",
-  "PATCH /books/files/:fileId",
   "PATCH /custom-icons/:slug/svg",
   "PATCH /custom-metadata/fields/:id",
   "PATCH /custom-metadata/fields/reorder",