fix(server): hold DELETE bodies to the same contract as every other write

The body-contract boot gate only looked at POST/PUT/PATCH, so a DELETE that
reads a body slipped past it. Two do; one was already typed, the other read
photo_id straight off an untyped Record. Give it a contract from shared like
everything else, then add DELETE to the gate so the next one cannot repeat it.
This commit is contained in:
Maurice
2026-08-23 09:34:53 +02:00
committed by Maurice
parent 3f8a7a1ea9
commit b163c1e05d
5 changed files with 30 additions and 4 deletions
@@ -10,6 +10,10 @@ const MUTATION_METHODS = new Set<RequestMethod>([
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,
]);
/**
+2
View File
@@ -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) {}
@@ -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<string, unknown>,
@Body() body: RemoveTripPhotoDto,
@Res() res: Response,
): Promise<void> {
const result = this.memories.removeTripPhoto(tripId, user.id, Number(body?.photo_id));
@@ -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<string, unknown>) {
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();
});
+6
View File
@@ -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<typeof synologySearchSchema>;
export type PhotoSelectionInput = z.infer<typeof photoSelectionSchema>;
export type AddTripPhotosInput = z.infer<typeof addTripPhotosSchema>;
export type SetTripPhotoSharingInput = z.infer<typeof setTripPhotoSharingSchema>;
export type RemoveTripPhotoInput = z.infer<typeof removeTripPhotoSchema>;
export type CreateAlbumLinkInput = z.infer<typeof createAlbumLinkSchema>;