feat(server): validate collections reorder and delete-many bodies

server/src/nest/collections/collections.controller.ts

@@ -33,6 +33,8 @@ import { isDemoEmail } from '../../services/demo';
 import {
   CollectionCreateDto,
   CollectionUpdateDto,
+  CollectionReorderDto,
+  CollectionDeleteManyDto,
   CollectionSavePlaceDto,
   CollectionSaveFromTripDto,
   CollectionSaveFromTripManyDto,
@@ -97,11 +99,8 @@ export class CollectionsController {
 
   @Post('reorder')
   @HttpCode(200)
-  reorder(@CurrentUser() user: User, @Body('orderedIds') orderedIds: unknown) {
-    if (!Array.isArray(orderedIds) || !orderedIds.every((v) => Number.isFinite(Number(v)))) {
-      throw new HttpException({ error: 'orderedIds must be an array of numbers' }, 400);
-    }
-    this.collections.reorderCollections(user.id, orderedIds.map(Number));
+  reorder(@CurrentUser() user: User, @Body() body: CollectionReorderDto) {
+    this.collections.reorderCollections(user.id, body.orderedIds);
     return { success: true };
   }
 
@@ -126,11 +125,8 @@ export class CollectionsController {
 
   @Post('places/delete-many')
   @HttpCode(200)
-  deleteMany(@CurrentUser() user: User, @Body('ids') ids: unknown, @Headers('x-socket-id') socketId?: string) {
-    if (!Array.isArray(ids) || !ids.every((v) => Number.isFinite(Number(v)))) {
-      throw new HttpException({ error: 'ids must be an array of numbers' }, 400);
-    }
-    return { deleted: this.collections.deletePlacesMany(user.id, ids.map(Number), socketId) };
+  deleteMany(@CurrentUser() user: User, @Body() body: CollectionDeleteManyDto, @Headers('x-socket-id') socketId?: string) {
+    return { deleted: this.collections.deletePlacesMany(user.id, body.ids, socketId) };
   }
 
   @Patch('places/:pid')

server/src/nest/collections/collections.dto.ts

@@ -2,6 +2,8 @@ import { createZodDto } from 'nestjs-zod';
 import {
   collectionCreateRequestSchema,
   collectionUpdateRequestSchema,
+  collectionReorderRequestSchema,
+  collectionDeleteManyRequestSchema,
   collectionSavePlaceRequestSchema,
   collectionSaveFromTripRequestSchema,
   collectionSaveFromTripManyRequestSchema,
@@ -27,6 +29,8 @@ import {
 
 export class CollectionCreateDto extends createZodDto(collectionCreateRequestSchema) {}
 export class CollectionUpdateDto extends createZodDto(collectionUpdateRequestSchema) {}
+export class CollectionReorderDto extends createZodDto(collectionReorderRequestSchema) {}
+export class CollectionDeleteManyDto extends createZodDto(collectionDeleteManyRequestSchema) {}
 export class CollectionSavePlaceDto extends createZodDto(collectionSavePlaceRequestSchema) {}
 export class CollectionSaveFromTripDto extends createZodDto(collectionSaveFromTripRequestSchema) {}
 export class CollectionSaveFromTripManyDto extends createZodDto(collectionSaveFromTripManyRequestSchema) {}

server/src/nest/common/body-contract-allow-list.ts

@@ -51,8 +51,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [
   'BookingImportController.previewAsync',
   'CategoriesController.create',
   'CategoriesController.update',
-  'CollectionsController.deleteMany',
-  'CollectionsController.reorder',
   'ImmichMemoriesController.putSettings',
   'ImmichMemoriesController.search',
   'ImmichMemoriesController.test',

server/tests/unit/nest/collections.controller.test.ts

@@ -68,15 +68,12 @@ describe('CollectionsController', () => {
   });
 
   describe('reorder', () => {
-    it('400 when orderedIds is not an array of numbers', () => {
-      expect(thrown(() => new CollectionsController(makeService()).reorder(user, 'nope' as never)))
-        .toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } });
-      expect(thrown(() => new CollectionsController(makeService()).reorder(user, [1, 'x'] as never)))
-        .toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } });
-    });
+    // The legacy 'orderedIds must be an array of numbers' 400 is gone:
+    // collectionReorderRequestSchema types the array, so the pipe rejects a
+    // bad payload before the handler runs.
     it('reorders a valid array', () => {
       const svc = makeService();
-      expect(new CollectionsController(svc).reorder(user, [3, 1, 2] as never)).toEqual({ success: true });
+      expect(new CollectionsController(svc).reorder(user, { orderedIds: [3, 1, 2] } as never)).toEqual({ success: true });
       expect(svc.reorderCollections).toHaveBeenCalledWith(1, [3, 1, 2]);
     });
   });
@@ -98,11 +95,12 @@ describe('CollectionsController', () => {
       expect(c.copyToTrip(user, { trip_id: 5, place_ids: [9] } as never)).toEqual({ copied: 1, skipped: [] });
     });
 
-    it('deleteMany 400 on a bad payload, deletes a valid one', () => {
-      expect(thrown(() => new CollectionsController(makeService()).deleteMany(user, { nope: 1 } as never)))
-        .toEqual({ status: 400, body: { error: 'ids must be an array of numbers' } });
+    // The legacy 'ids must be an array of numbers' 400 is gone:
+    // collectionDeleteManyRequestSchema types the array, so the pipe rejects a
+    // bad payload before the handler runs.
+    it('deleteMany deletes a valid list', () => {
       const svc = makeService();
-      expect(new CollectionsController(svc).deleteMany(user, [1, 2] as never, 'sid')).toEqual({ deleted: [1, 2] });
+      expect(new CollectionsController(svc).deleteMany(user, { ids: [1, 2] } as never, 'sid')).toEqual({ deleted: [1, 2] });
       expect(svc.deletePlacesMany).toHaveBeenCalledWith(1, [1, 2], 'sid');
     });
   });

shared/src/collection/collection.schema.spec.ts

@@ -1,4 +1,8 @@
-import { collectionPlaceUpdateRequestSchema } from './collection.schema';
+import {
+  collectionDeleteManyRequestSchema,
+  collectionPlaceUpdateRequestSchema,
+  collectionReorderRequestSchema,
+} from './collection.schema';
 
 import { describe, expect, it } from 'vitest';
 
@@ -20,3 +24,20 @@ describe('collectionPlaceUpdateRequestSchema', () => {
     expect(collectionPlaceUpdateRequestSchema.parse({ status: 'bogus' as never }).status).toBe('idea');
   });
 });
+
+// The two ratchet schemas replace hand-rolled "array of numbers" checks — they
+// must stay exactly that permissive (empty arrays included, no int/positive).
+describe('collectionReorderRequestSchema / collectionDeleteManyRequestSchema', () => {
+  it('accepts any array of numbers, empty included', () => {
+    expect(collectionReorderRequestSchema.parse({ orderedIds: [] }).orderedIds).toEqual([]);
+    expect(collectionReorderRequestSchema.parse({ orderedIds: [3, 1, 2] }).orderedIds).toEqual([3, 1, 2]);
+    expect(collectionDeleteManyRequestSchema.parse({ ids: [] }).ids).toEqual([]);
+    expect(collectionDeleteManyRequestSchema.parse({ ids: [7] }).ids).toEqual([7]);
+  });
+
+  it('rejects non-arrays and non-numeric members', () => {
+    expect(() => collectionReorderRequestSchema.parse({ orderedIds: 'nope' })).toThrow();
+    expect(() => collectionReorderRequestSchema.parse({})).toThrow();
+    expect(() => collectionDeleteManyRequestSchema.parse({ ids: ['a'] })).toThrow();
+  });
+});

shared/src/collection/collection.schema.ts

@@ -128,6 +128,14 @@ export const collectionUpdateRequestSchema = collectionCreateRequestSchema.parti
 });
 export type CollectionUpdateRequest = z.infer<typeof collectionUpdateRequestSchema>;
 
+/** Reorder the caller's lists — every visible collection id in the desired order.
+ *  Plain z.number() (no .min/.int) mirrors the legacy hand-rolled check the DTO
+ *  ratchet replaced: any array of numbers, empty included. */
+export const collectionReorderRequestSchema = z.object({
+  orderedIds: z.array(z.number()),
+});
+export type CollectionReorderRequest = z.infer<typeof collectionReorderRequestSchema>;
+
 /** Save a place into a list from a raw maps/manual payload (or carrying provenance). */
 export const collectionSavePlaceRequestSchema = z.object({
   collection_id: z.number(),
@@ -198,6 +206,13 @@ export type CollectionPlaceUpdateRequest = z.infer<typeof collectionPlaceUpdateR
 export const collectionSetStatusRequestSchema = z.object({ status: collectionStatusSchema });
 export type CollectionSetStatusRequest = z.infer<typeof collectionSetStatusRequestSchema>;
 
+/** Bulk-delete saved places. Plain z.number() (no .min/.int) mirrors the legacy
+ *  hand-rolled check the DTO ratchet replaced (same shape as placeBulkDeleteRequestSchema). */
+export const collectionDeleteManyRequestSchema = z.object({
+  ids: z.array(z.number()),
+});
+export type CollectionDeleteManyRequest = z.infer<typeof collectionDeleteManyRequestSchema>;
+
 /** Copy one or many saved places INTO a trip (dedup precheck on server). */
 export const collectionCopyToTripRequestSchema = z.object({
   trip_id: z.number(),