From 97f524cdfd20843ec376589d9badc3742cbd759a Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 8 Aug 2026 11:58:18 +0200 Subject: [PATCH] feat(server): give the memories endpoints their Zod contracts The last nine memories entries in the body-contract allow list are gone: three each on the immich, synology and unified controllers. They read @Body() as Record and coerced by hand. The schemas are deliberately permissive and loose, and that is the point rather than a shortcut. Paging arrives as `2` or `'2'` because the controller does `Number(page) || 1`; synology_skip_ssl arrives as a boolean or the string 'true'; auto_upload is accepted in any shape because only a real boolean is acted on. A schema that described the ideal body instead of the real one would turn working clients into 400s, and parity outranks tightening. Narrowing them is a separate, client-visible change. Loose objects for the same reason: a strict object strips unknown keys, and `selections[]` goes straight to the services as `Selection[]` - stripping there would be a silent data loss the day a provider adds a field. This unblocks be-memories-fold, which could not move a controller whose body had no contract to move with it. --- .../nest/common/body-contract-allow-list.ts | 9 -- server/src/nest/memories/immich.controller.ts | 7 +- server/src/nest/memories/memories.dto.ts | 34 ++++++ .../src/nest/memories/synology.controller.ts | 7 +- .../src/nest/memories/unified.controller.ts | 7 +- shared/src/index.ts | 1 + shared/src/memories/memories.schema.spec.ts | 92 +++++++++++++++ shared/src/memories/memories.schema.ts | 111 ++++++++++++++++++ 8 files changed, 250 insertions(+), 18 deletions(-) create mode 100644 server/src/nest/memories/memories.dto.ts create mode 100644 shared/src/memories/memories.schema.spec.ts create mode 100644 shared/src/memories/memories.schema.ts diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index a1d70afd1..e2df38e66 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -12,9 +12,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'BookingImportController.confirm', 'BookingImportController.preview', 'BookingImportController.previewAsync', - 'ImmichMemoriesController.putSettings', - 'ImmichMemoriesController.search', - 'ImmichMemoriesController.test', 'JourneyController.addContributor', 'JourneyController.addTrip', 'JourneyController.create', @@ -43,12 +40,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'PluginsController.setEgressHosts', 'PluginsController.uninstall', 'PluginsController.updateConfig', - 'SynologyMemoriesController.putSettings', - 'SynologyMemoriesController.search', - 'SynologyMemoriesController.test', 'TagsController.create', 'TagsController.update', - 'UnifiedMemoriesController.addPhotos', - 'UnifiedMemoriesController.createAlbumLink', - 'UnifiedMemoriesController.setSharing', ]; diff --git a/server/src/nest/memories/immich.controller.ts b/server/src/nest/memories/immich.controller.ts index 769a9a5b0..4c8c694af 100644 --- a/server/src/nest/memories/immich.controller.ts +++ b/server/src/nest/memories/immich.controller.ts @@ -5,6 +5,7 @@ import { MemoriesService } from './memories.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { getClientIp } from '../audit/client-ip'; +import { ImmichSearchDto, ImmichSettingsDto, ImmichTestDto } from './memories.dto'; /** * /api/integrations/memories/immich — Immich connection, browse/search, asset @@ -30,7 +31,7 @@ export class ImmichMemoriesController { @Put('settings') async putSettings( @CurrentUser() user: User, - @Body() body: { immich_url?: string; immich_api_key?: string; auto_upload?: unknown }, + @Body() body: ImmichSettingsDto, @Req() req: Request, @Res() res: Response, ): Promise { @@ -57,7 +58,7 @@ export class ImmichMemoriesController { @Post('test') @HttpCode(200) - async test(@Body() body: { immich_url?: string; immich_api_key?: string }) { + async test(@Body() body: ImmichTestDto) { const { immich_url, immich_api_key } = body; if (!immich_url || !immich_api_key) { return { connected: false, error: 'URL and API key required' }; @@ -77,7 +78,7 @@ export class ImmichMemoriesController { @Post('search') @HttpCode(200) - async search(@CurrentUser() user: User, @Body() body: Record, @Res() res: Response): Promise { + async search(@CurrentUser() user: User, @Body() body: ImmichSearchDto, @Res() res: Response): Promise { const { from, to, size, page } = body as { from?: string; to?: string; size?: unknown; page?: unknown }; const pageNum = Math.max(1, Number(page) || 1); const pageSize = Math.min(Number(size) || 50, 200); diff --git a/server/src/nest/memories/memories.dto.ts b/server/src/nest/memories/memories.dto.ts new file mode 100644 index 000000000..1f978ead5 --- /dev/null +++ b/server/src/nest/memories/memories.dto.ts @@ -0,0 +1,34 @@ +import { createZodDto } from 'nestjs-zod'; +import { + addTripPhotosSchema, + createAlbumLinkSchema, + immichSearchSchema, + immichSettingsSchema, + immichTestSchema, + setTripPhotoSharingSchema, + synologySearchSchema, + synologySettingsSchema, + synologyTestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared memories contracts. + * The global ZodValidationPipe (APP_PIPE in app.module.ts) validates any @Body() + * parameter typed with one of these classes by metatype — the Zod schemas in + * shared/ remain the single source of truth for the wire contract. + * + * These nine routes were the last memories entries in + * common/body-contract-allow-list.ts; the boot gate throws on stale entries, so + * they were removed in the same change. + */ +export class ImmichSettingsDto extends createZodDto(immichSettingsSchema) {} +export class ImmichTestDto extends createZodDto(immichTestSchema) {} +export class ImmichSearchDto extends createZodDto(immichSearchSchema) {} + +export class SynologySettingsDto extends createZodDto(synologySettingsSchema) {} +export class SynologyTestDto extends createZodDto(synologyTestSchema) {} +export class SynologySearchDto extends createZodDto(synologySearchSchema) {} + +export class AddTripPhotosDto extends createZodDto(addTripPhotosSchema) {} +export class SetTripPhotoSharingDto extends createZodDto(setTripPhotoSharingSchema) {} +export class CreateAlbumLinkDto extends createZodDto(createAlbumLinkSchema) {} diff --git a/server/src/nest/memories/synology.controller.ts b/server/src/nest/memories/synology.controller.ts index 88db6133e..4e9cb676d 100644 --- a/server/src/nest/memories/synology.controller.ts +++ b/server/src/nest/memories/synology.controller.ts @@ -6,6 +6,7 @@ import { fail, success } from '../../services/memories/helpersService'; import { MemoriesService } from './memories.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { SynologySearchDto, SynologySettingsDto, SynologyTestDto } from './memories.dto'; function _parseStringBodyField(value: unknown): string { return String(value ?? '').trim(); @@ -47,7 +48,7 @@ export class SynologyMemoriesController { } @Put('settings') - async putSettings(@CurrentUser() user: User, @Body() body: Record, @Res() res: Response): Promise { + async putSettings(@CurrentUser() user: User, @Body() body: SynologySettingsDto, @Res() res: Response): Promise { const synology_url = _parseStringBodyField(body.synology_url); const synology_username = _parseStringBodyField(body.synology_username); const synology_password = _parseStringBodyField(body.synology_password); @@ -67,7 +68,7 @@ export class SynologyMemoriesController { @Post('test') @HttpCode(200) - async test(@CurrentUser() user: User, @Body() body: Record, @Res() res: Response): Promise { + async test(@CurrentUser() user: User, @Body() body: SynologyTestDto, @Res() res: Response): Promise { const synology_url = _parseStringBodyField(body.synology_url); const synology_username = _parseStringBodyField(body.synology_username); const synology_password = _parseStringBodyField(body.synology_password); @@ -110,7 +111,7 @@ export class SynologyMemoriesController { @Post('search') @HttpCode(200) - async search(@CurrentUser() user: User, @Body() body: Record, @Res() res: Response): Promise { + async search(@CurrentUser() user: User, @Body() body: SynologySearchDto, @Res() res: Response): Promise { const from = _parseStringBodyField(body.from); const to = _parseStringBodyField(body.to); let offset = _parseNumberBodyField(body.offset, 0); diff --git a/server/src/nest/memories/unified.controller.ts b/server/src/nest/memories/unified.controller.ts index 0c2770861..06e19ec41 100644 --- a/server/src/nest/memories/unified.controller.ts +++ b/server/src/nest/memories/unified.controller.ts @@ -5,6 +5,7 @@ import type { Selection } from '../../services/memories/helpersService'; 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'; /** * /api/integrations/memories/unified — provider-agnostic trip photo + album-link @@ -35,7 +36,7 @@ export class UnifiedMemoriesController { async addPhotos( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: Record, + @Body() body: AddTripPhotosDto, @Headers('x-socket-id') sid: string, @Res() res: Response, ): Promise { @@ -53,7 +54,7 @@ export class UnifiedMemoriesController { async setSharing( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: Record, + @Body() body: SetTripPhotoSharingDto, @Res() res: Response, ): Promise { const result = await this.memories.setTripPhotoSharing(tripId, user.id, Number(body?.photo_id), body?.shared as boolean); @@ -94,7 +95,7 @@ export class UnifiedMemoriesController { createAlbumLink( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: Record, + @Body() body: CreateAlbumLinkDto, @Res() res: Response, ): void { const passphrase = body?.passphrase ? String(body.passphrase) : undefined; diff --git a/shared/src/index.ts b/shared/src/index.ts index 3c32aa6bf..686da6a1a 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -20,6 +20,7 @@ export * from './maps/maps.schema'; export * from './category/category.schema'; export * from './tag/tag.schema'; export * from './notification/notification.schema'; +export * from './memories/memories.schema'; export * from './atlas/atlas.schema'; export * from './vacay/vacay.schema'; export * from './packing/packing.schema'; diff --git a/shared/src/memories/memories.schema.spec.ts b/shared/src/memories/memories.schema.spec.ts new file mode 100644 index 000000000..75509dcc5 --- /dev/null +++ b/shared/src/memories/memories.schema.spec.ts @@ -0,0 +1,92 @@ +import { describe, it, expect } from 'vitest'; +import { + addTripPhotosSchema, + createAlbumLinkSchema, + immichSearchSchema, + immichSettingsSchema, + setTripPhotoSharingSchema, + synologySearchSchema, + synologySettingsSchema, + synologyTestSchema, +} from './memories.schema'; + +/** + * These contracts exist to describe what the endpoints already accept, not to + * tighten them — the nine routes were read as `Record` and + * coerced by hand before. The cases below pin that tolerance, because a + * well-meaning narrowing here turns working clients into 400s. + */ + +describe('immich contracts', () => { + it('accepts the settings body with an empty payload — every field is optional', () => { + expect(immichSettingsSchema.safeParse({}).success).toBe(true); + }); + + it('accepts auto_upload in any shape, since only a real boolean is acted on', () => { + for (const auto_upload of [true, false, 'yes', 1, null]) { + expect(immichSettingsSchema.safeParse({ auto_upload }).success).toBe(true); + } + }); + + it('accepts search paging as numbers OR strings (the controller does Number(x) || fallback)', () => { + expect(immichSearchSchema.safeParse({ page: 2, size: 50 }).success).toBe(true); + expect(immichSearchSchema.safeParse({ page: '2', size: '50' }).success).toBe(true); + }); + + it('rejects a page that is neither', () => { + expect(immichSearchSchema.safeParse({ page: { nope: true } }).success).toBe(false); + }); +}); + +describe('synology contracts', () => { + it("accepts synology_skip_ssl as a boolean or the string 'true'/'false'", () => { + for (const synology_skip_ssl of [true, false, 'true', 'false']) { + expect(synologySettingsSchema.safeParse({ synology_skip_ssl }).success).toBe(true); + } + }); + + it('lets the test body carry an OTP on top of the settings fields', () => { + const parsed = synologyTestSchema.safeParse({ + synology_url: 'https://nas.example.org', + synology_username: 'ada', + synology_password: 'secret', + synology_otp: '123456', + }); + expect(parsed.success).toBe(true); + }); + + it('accepts every paging field of the search body, in either type', () => { + expect(synologySearchSchema.safeParse({ offset: '10', page: 2, limit: '100', size: 0 }).success).toBe(true); + }); +}); + +describe('unified contracts', () => { + it('accepts a selection list and keeps unknown keys on the items', () => { + const parsed = addTripPhotosSchema.safeParse({ + selections: [{ provider: 'immich', asset_ids: ['a', 'b'], passphrase: 'p', future_field: 1 }], + shared: false, + }); + expect(parsed.success).toBe(true); + // Loose on purpose: `selections` is handed to the services as `Selection[]`, + // so a stripped key would be a silent data loss the day a provider adds one. + expect(parsed.success && (parsed.data.selections![0] as Record).future_field).toBe(1); + }); + + it('accepts a body with no selections at all (the controller falls back to [])', () => { + expect(addTripPhotosSchema.safeParse({}).success).toBe(true); + }); + + it('rejects a selection missing its asset ids', () => { + expect(addTripPhotosSchema.safeParse({ selections: [{ provider: 'immich' }] }).success).toBe(false); + }); + + it('accepts photo_id as a number or a string', () => { + expect(setTripPhotoSharingSchema.safeParse({ photo_id: 7, shared: true }).success).toBe(true); + expect(setTripPhotoSharingSchema.safeParse({ photo_id: '7', shared: true }).success).toBe(true); + }); + + it('leaves the album-link fields untyped — the service validates them, and it answers 400 with its own wording', () => { + expect(createAlbumLinkSchema.safeParse({}).success).toBe(true); + expect(createAlbumLinkSchema.safeParse({ provider: 'synology', album_id: 12, album_name: null }).success).toBe(true); + }); +}); diff --git a/shared/src/memories/memories.schema.ts b/shared/src/memories/memories.schema.ts new file mode 100644 index 000000000..0e4ca299e --- /dev/null +++ b/shared/src/memories/memories.schema.ts @@ -0,0 +1,111 @@ +import { z } from 'zod'; + +/** + * Contracts for the memories (photo provider) mutation endpoints. + * + * Deliberately permissive. Every one of these bodies was read as + * `Record` and coerced by hand in the controller — + * `Number(page) || 1`, `body.synology_skip_ssl === 'true'`, + * `body?.shared === undefined ? true : !!body?.shared`. A strict schema would + * turn requests the client sends today into 400s, and the parity rule outranks + * tightening: these schemas describe the shape that already works, so the + * boot-time contract gate has something to point at and the client has a typed + * surface. Narrowing them is a separate, client-visible change. + * + * They are LOOSE objects for the same reason: a strict object strips unknown + * keys, and `selections[]` is handed straight to the services as `Selection[]`. + * Stripping there would silently drop a field a provider adds later. + */ + +/** Accepts what `_parseStringBodyField` accepted: a string, or nothing. */ +const optionalText = z.string().optional().nullable(); + +/** `x === true || x === 'true'` — the checkbox arrives either way. */ +const looseBoolean = z.union([z.boolean(), z.literal('true'), z.literal('false')]).optional(); + +/** `Number(x) || fallback` — the client sends these as strings from query-ish forms. */ +const looseNumber = z.union([z.number(), z.string()]).optional(); + +// ── Immich ──────────────────────────────────────────────────────────────── + +export const immichSettingsSchema = z.looseObject({ + immich_url: optionalText, + immich_api_key: optionalText, + // Applied only when it is a real boolean (`typeof auto_upload === 'boolean'`), + // so anything else is accepted and ignored, exactly as before. + auto_upload: z.unknown().optional(), +}); + +export const immichTestSchema = z.looseObject({ + immich_url: optionalText, + immich_api_key: optionalText, +}); + +export const immichSearchSchema = z.looseObject({ + from: optionalText, + to: optionalText, + size: looseNumber, + page: looseNumber, +}); + +// ── Synology ────────────────────────────────────────────────────────────── + +export const synologySettingsSchema = z.looseObject({ + synology_url: optionalText, + synology_username: optionalText, + synology_password: optionalText, + synology_skip_ssl: looseBoolean, +}); + +export const synologyTestSchema = synologySettingsSchema.extend({ + synology_otp: optionalText, +}); + +export const synologySearchSchema = z.looseObject({ + from: optionalText, + to: optionalText, + offset: looseNumber, + page: looseNumber, + limit: looseNumber, + size: looseNumber, +}); + +// ── Unified (provider-agnostic trip photo surface) ──────────────────────── + +/** One provider's picked assets. Mirrors the `Selection` type the services take. */ +export const photoSelectionSchema = z.looseObject({ + provider: z.string(), + asset_ids: z.array(z.string()), + owner_id: z.number().optional(), + passphrase: z.string().optional(), +}); + +export const addTripPhotosSchema = z.looseObject({ + // Non-arrays fall back to [] rather than 400 (`Array.isArray(body?.selections) ? … : []`). + selections: z.array(photoSelectionSchema).optional(), + // Absent means shared; anything else is read for truthiness. + shared: z.unknown().optional(), +}); + +export const setTripPhotoSharingSchema = z.looseObject({ + photo_id: looseNumber, + shared: z.unknown().optional(), +}); + +export const createAlbumLinkSchema = z.looseObject({ + provider: z.unknown().optional(), + album_id: z.unknown().optional(), + album_name: z.unknown().optional(), + passphrase: z.unknown().optional(), +}); + +export type ImmichSettingsInput = z.infer; +export type ImmichTestInput = z.infer; +export type ImmichSearchInput = z.infer; +export type SynologySettingsInput = z.infer; +export type SynologyTestInput = z.infer; +export type SynologySearchInput = z.infer; +export type PhotoSelectionInput = z.infer; +export type AddTripPhotosInput = z.infer; +export type SetTripPhotoSharingInput = z.infer; +export type CreateAlbumLinkInput = z.infer;