diff --git a/server/src/nest/common/validate-body-contracts.ts b/server/src/nest/common/validate-body-contracts.ts index dccd22f60..5e093948f 100644 --- a/server/src/nest/common/validate-body-contracts.ts +++ b/server/src/nest/common/validate-body-contracts.ts @@ -10,6 +10,10 @@ const MUTATION_METHODS = new Set([ RequestMethod.POST, RequestMethod.PUT, RequestMethod.PATCH, + // DELETE belongs here too. It is rare for one to read a body, but the two that + // do are writes like any other, and leaving the method out meant their bodies + // reached the handler with nothing having looked at them. + RequestMethod.DELETE, ]); /** diff --git a/server/src/nest/memories/memories.dto.ts b/server/src/nest/memories/memories.dto.ts index 1f978ead5..d0303a19a 100644 --- a/server/src/nest/memories/memories.dto.ts +++ b/server/src/nest/memories/memories.dto.ts @@ -5,6 +5,7 @@ import { immichSearchSchema, immichSettingsSchema, immichTestSchema, + removeTripPhotoSchema, setTripPhotoSharingSchema, synologySearchSchema, synologySettingsSchema, @@ -31,4 +32,5 @@ export class SynologySearchDto extends createZodDto(synologySearchSchema) {} export class AddTripPhotosDto extends createZodDto(addTripPhotosSchema) {} export class SetTripPhotoSharingDto extends createZodDto(setTripPhotoSharingSchema) {} +export class RemoveTripPhotoDto extends createZodDto(removeTripPhotoSchema) {} export class CreateAlbumLinkDto extends createZodDto(createAlbumLinkSchema) {} diff --git a/server/src/nest/memories/unified.controller.ts b/server/src/nest/memories/unified.controller.ts index 3810af95a..ce5524fcd 100644 --- a/server/src/nest/memories/unified.controller.ts +++ b/server/src/nest/memories/unified.controller.ts @@ -5,7 +5,7 @@ import type { Selection } from './memories.helpers'; import { MemoriesService } from './memories.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { AddTripPhotosDto, CreateAlbumLinkDto, SetTripPhotoSharingDto } from './memories.dto'; +import { AddTripPhotosDto, CreateAlbumLinkDto, RemoveTripPhotoDto, SetTripPhotoSharingDto } from './memories.dto'; /** * /api/integrations/memories/unified — provider-agnostic trip photo + album-link @@ -69,7 +69,7 @@ export class UnifiedMemoriesController { async removePhoto( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: Record, + @Body() body: RemoveTripPhotoDto, @Res() res: Response, ): Promise { const result = this.memories.removeTripPhoto(tripId, user.id, Number(body?.photo_id)); diff --git a/server/tests/unit/nest/validate-body-contracts.test.ts b/server/tests/unit/nest/validate-body-contracts.test.ts index 96e4a7829..f17c6474e 100644 --- a/server/tests/unit/nest/validate-body-contracts.test.ts +++ b/server/tests/unit/nest/validate-body-contracts.test.ts @@ -5,7 +5,7 @@ * stale allow-list entries must refuse boot too (the list only ratchets down). */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { Body, Controller, Get, Post, Put } from '@nestjs/common'; +import { Body, Controller, Delete, Get, Post, Put } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import type { INestApplication } from '@nestjs/common'; import { createZodDto } from 'nestjs-zod'; @@ -41,6 +41,14 @@ class BadController { } } +@Controller('gate-test/delete-body') +class DeleteBodyController { + @Delete() + remove(@Body() body: Record) { + return body; + } +} + @Controller('gate-test/subfield') class SubfieldController { @Post() @@ -60,15 +68,17 @@ describe('validateBodyContracts (fail-closed boot gate)', () => { let goodApp: INestApplication; let badApp: INestApplication; let subfieldApp: INestApplication; + let deleteBodyApp: INestApplication; beforeAll(async () => { goodApp = await buildApp([GoodController]); badApp = await buildApp([GoodController, BadController]); subfieldApp = await buildApp([SubfieldController]); + deleteBodyApp = await buildApp([DeleteBodyController]); }); afterAll(async () => { - await Promise.all([goodApp.close(), badApp.close(), subfieldApp.close()]); + await Promise.all([goodApp.close(), badApp.close(), subfieldApp.close(), deleteBodyApp.close()]); }); it('GATE-001 — passes when every mutation body is a createZodDto class', () => { @@ -83,6 +93,10 @@ describe('validateBodyContracts (fail-closed boot gate)', () => { expect(() => validateBodyContracts(subfieldApp, [])).toThrow(/SubfieldController\.reorder/); }); + it('GATE-007 — a DELETE that reads a body is held to the same contract', () => { + expect(() => validateBodyContracts(deleteBodyApp, [])).toThrow(/DeleteBodyController\.remove/); + }); + it('GATE-004 — an allow-listed legacy route passes', () => { expect(() => validateBodyContracts(badApp, ['BadController.update'])).not.toThrow(); }); diff --git a/shared/src/memories/memories.schema.ts b/shared/src/memories/memories.schema.ts index 0e4ca299e..b1ae1bfe7 100644 --- a/shared/src/memories/memories.schema.ts +++ b/shared/src/memories/memories.schema.ts @@ -92,6 +92,11 @@ export const setTripPhotoSharingSchema = z.looseObject({ shared: z.unknown().optional(), }); +/** The photo to unlink from the trip. Same shape the sharing toggle takes. */ +export const removeTripPhotoSchema = z.looseObject({ + photo_id: looseNumber, +}); + export const createAlbumLinkSchema = z.looseObject({ provider: z.unknown().optional(), album_id: z.unknown().optional(), @@ -108,4 +113,5 @@ export type SynologySearchInput = z.infer; export type PhotoSelectionInput = z.infer; export type AddTripPhotosInput = z.infer; export type SetTripPhotoSharingInput = z.infer; +export type RemoveTripPhotoInput = z.infer; export type CreateAlbumLinkInput = z.infer;