diff --git a/server/src/nest/backup/backup.controller.ts b/server/src/nest/backup/backup.controller.ts index 567c3df7a..0cac39833 100644 --- a/server/src/nest/backup/backup.controller.ts +++ b/server/src/nest/backup/backup.controller.ts @@ -23,6 +23,7 @@ import { BackupService } from './backup.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { AdminGuard } from '../auth/admin.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { AutoBackupSettingsDto } from './backup.dto'; import { getClientIp } from '../audit/client-ip'; import { AuditService } from '../audit/audit.service'; import { getUploadTmpDir, MAX_BACKUP_UPLOAD_SIZE } from './backup.impl'; @@ -142,7 +143,7 @@ export class BackupController { } @Put('auto-settings') - updateAutoSettings(@CurrentUser() user: User, @Body() body: Record, @Req() req: Request) { + updateAutoSettings(@CurrentUser() user: User, @Body() body: AutoBackupSettingsDto, @Req() req: Request) { try { const settings = this.backup.updateAutoSettings(body || {}); this.audit.writeAudit({ userId: user.id, action: 'backup.auto_settings', ip: getClientIp(req), details: { enabled: settings.enabled, interval: settings.interval, keep_days: settings.keep_days } }); diff --git a/server/src/nest/backup/backup.dto.ts b/server/src/nest/backup/backup.dto.ts new file mode 100644 index 000000000..663e8d616 --- /dev/null +++ b/server/src/nest/backup/backup.dto.ts @@ -0,0 +1,12 @@ +import { createZodDto } from 'nestjs-zod'; +import { autoBackupSettingsRequestSchema } from '@trek/shared'; + +/** + * Server-side createZodDto wrapper over the @trek/shared backup contract. + * + * The schema is a passthrough that pins only the well-known toggles: the body is + * normalised server-side by parseAutoBackupBody, which coerces strings and + * clamps ranges, and the handler maps its throw to the 400. Tightening here + * would take that answer away from the service. + */ +export class AutoBackupSettingsDto extends createZodDto(autoBackupSettingsRequestSchema) {} diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index e2df38e66..e4384485e 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -8,26 +8,9 @@ * here — the gate throws on stale entries, so removal is enforced too. */ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ - 'BackupController.updateAutoSettings', 'BookingImportController.confirm', 'BookingImportController.preview', 'BookingImportController.previewAsync', - 'JourneyController.addContributor', - 'JourneyController.addTrip', - 'JourneyController.create', - 'JourneyController.createEntry', - 'JourneyController.galleryProviderPhotos', - 'JourneyController.linkPhoto', - 'JourneyController.preferences', - 'JourneyController.providerPhotos', - 'JourneyController.reorderEntries', - 'JourneyController.setShareLink', - 'JourneyController.update', - 'JourneyController.updateContributor', - 'JourneyController.updateEntry', - 'JourneyController.updatePhoto', - 'JourneyController.uploadEntryPhotos', - 'JourneyController.uploadGalleryVideo', 'LlmLocalController.pull', 'OauthApiController.authorize', 'OauthApiController.createClient', diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index 062b2fee6..34b56357c 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -27,6 +27,12 @@ import { RequireAddon } from '../addons/require-addon.decorator'; import { ADDON_IDS } from '../../addons'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { + JourneyAddTripDto, JourneyContributorAddDto, JourneyContributorUpdateDto, JourneyCreateDto, + JourneyEntryCreateDto, JourneyEntryPhotoUploadDto, JourneyEntryUpdateDto, JourneyGalleryVideoDto, + JourneyLinkPhotoDto, JourneyPhotoUpdateDto, JourneyPreferencesDto, JourneyProviderPhotosDto, + JourneyReorderEntriesDto, JourneyShareLinkDto, JourneyUpdateDto, +} from './journey.dto'; import { isVideoMime, isVideoExtension, MAX_VIDEO_SIZE } from '../files/files.constants'; import { getAllowedExtensions } from '../files/files.bridge'; @@ -113,13 +119,15 @@ export class JourneyController { } @Post() - create(@CurrentUser() user: User, @Body() body: { title?: string; subtitle?: string; trip_ids?: unknown[] }) { + create(@CurrentUser() user: User, @Body() body: JourneyCreateDto) { if (!body.title || typeof body.title !== 'string' || !body.title.trim()) { throw new HttpException({ error: 'Title is required' }, 400); } return this.journey.createJourney(user.id, { - title: body.title.trim(), - subtitle: body.subtitle, + // The guard three lines up proved both of these; the schema stays + // permissive so that guard keeps answering, not the pipe. + title: (body.title as string).trim(), + subtitle: body.subtitle as string | undefined, trip_ids: Array.isArray(body.trip_ids) ? body.trip_ids.map(Number) : [], }); } @@ -136,7 +144,7 @@ export class JourneyController { // ── Entries (prefix /entries — before /:id) ───────────────────────────── @Patch('entries/:entryId') - updateEntry(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: Record, @Headers('x-socket-id') socketId?: string) { + updateEntry(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: JourneyEntryUpdateDto, @Headers('x-socket-id') socketId?: string) { const result = this.journey.updateEntry(Number(entryId), user.id, body, socketId); if (!result) { throw new HttpException({ error: 'Entry not found' }, 404); @@ -154,14 +162,14 @@ export class JourneyController { @Post('entries/:entryId/photos') @UseInterceptors(FilesInterceptor('photos', undefined, IMAGE_UPLOAD)) - async uploadEntryPhotos(@CurrentUser() user: User, @Param('entryId') entryId: string, @UploadedFiles() files: Express.Multer.File[] | undefined, @Body() body: { caption?: string }) { + async uploadEntryPhotos(@CurrentUser() user: User, @Param('entryId') entryId: string, @UploadedFiles() files: Express.Multer.File[] | undefined, @Body() body: JourneyEntryPhotoUploadDto) { if (!files?.length) { throw new HttpException({ error: 'No files uploaded' }, 400); } const results: unknown[] = []; for (const file of files) { const relativePath = `journey/${file.filename}`; - const photo = this.journey.addPhoto(Number(entryId), user.id, relativePath, undefined, body?.caption); + const photo = this.journey.addPhoto(Number(entryId), user.id, relativePath, undefined, body?.caption as string | undefined); if (!photo) continue; // Mirror to Immich only when the user explicitly opted in (#730). if (this.journey.immichAutoUploadEnabled(user.id)) { @@ -184,13 +192,13 @@ export class JourneyController { } @Post('entries/:entryId/provider-photos') - providerPhotos(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: { provider?: string; asset_id?: string; asset_ids?: unknown[]; caption?: string; passphrase?: string; media_type?: string; media_types?: unknown[] }) { + providerPhotos(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: JourneyProviderPhotosDto) { const pp = body.passphrase && typeof body.passphrase === 'string' ? body.passphrase : undefined; if (Array.isArray(body.asset_ids) && body.provider) { const added: unknown[] = []; body.asset_ids.forEach((id, i) => { const mt = Array.isArray(body.media_types) && body.media_types[i] === 'video' ? 'video' : 'image'; - const photo = this.journey.addProviderPhoto(Number(entryId), user.id, body.provider!, String(id), body.caption, pp, mt); + const photo = this.journey.addProviderPhoto(Number(entryId), user.id, String(body.provider), String(id), body.caption as string | undefined, pp, mt); if (photo) added.push(photo); }); return { photos: added, added: added.length }; @@ -198,7 +206,7 @@ export class JourneyController { if (!body.provider || !body.asset_id) { throw new HttpException({ error: 'provider and asset_id required' }, 400); } - const photo = this.journey.addProviderPhoto(Number(entryId), user.id, body.provider, body.asset_id, body.caption, pp, body.media_type === 'video' ? 'video' : 'image'); + const photo = this.journey.addProviderPhoto(Number(entryId), user.id, String(body.provider), String(body.asset_id), body.caption as string | undefined, pp, body.media_type === 'video' ? 'video' : 'image'); if (!photo) { throw new HttpException({ error: 'Not allowed or duplicate' }, 403); } @@ -206,7 +214,7 @@ export class JourneyController { } @Post('entries/:entryId/link-photo') - linkPhoto(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: { journey_photo_id?: unknown; photo_id?: unknown }) { + linkPhoto(@CurrentUser() user: User, @Param('entryId') entryId: string, @Body() body: JourneyLinkPhotoDto) { const journeyPhotoId = body.journey_photo_id ?? body.photo_id; if (!journeyPhotoId) { throw new HttpException({ error: 'journey_photo_id required' }, 400); @@ -227,7 +235,7 @@ export class JourneyController { } @Patch('photos/:photoId') - updatePhoto(@CurrentUser() user: User, @Param('photoId') photoId: string, @Body() body: Record) { + updatePhoto(@CurrentUser() user: User, @Param('photoId') photoId: string, @Body() body: JourneyPhotoUpdateDto) { const result = this.journey.updatePhoto(Number(photoId), user.id, body); if (!result) { throw new HttpException({ error: 'Photo not found' }, 404); @@ -268,7 +276,7 @@ export class JourneyController { @CurrentUser() user: User, @Param('id') id: string, @UploadedFiles() files: { video?: Express.Multer.File[]; poster?: Express.Multer.File[] } | undefined, - @Body() body: { duration_ms?: string }, + @Body() body: JourneyGalleryVideoDto, ) { const video = files?.video?.[0]; const poster = files?.poster?.[0]; @@ -298,13 +306,13 @@ export class JourneyController { } @Post(':id/gallery/provider-photos') - galleryProviderPhotos(@CurrentUser() user: User, @Param('id') id: string, @Body() body: { provider?: string; asset_id?: string; asset_ids?: unknown[]; passphrase?: string; media_type?: string; media_types?: unknown[] }) { + galleryProviderPhotos(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyProviderPhotosDto) { const pp = body.passphrase && typeof body.passphrase === 'string' ? body.passphrase : undefined; if (Array.isArray(body.asset_ids) && body.provider) { const added: unknown[] = []; body.asset_ids.forEach((aid, i) => { const mt = Array.isArray(body.media_types) && body.media_types[i] === 'video' ? 'video' : 'image'; - const photo = this.journey.addProviderPhotoToGallery(Number(id), user.id, body.provider!, String(aid), undefined, pp, mt); + const photo = this.journey.addProviderPhotoToGallery(Number(id), user.id, String(body.provider), String(aid), undefined, pp, mt); if (photo) added.push(photo); }); return { photos: added, added: added.length }; @@ -312,7 +320,7 @@ export class JourneyController { if (!body.provider || !body.asset_id) { throw new HttpException({ error: 'provider and asset_id required' }, 400); } - const photo = this.journey.addProviderPhotoToGallery(Number(id), user.id, body.provider, body.asset_id, undefined, pp, body.media_type === 'video' ? 'video' : 'image'); + const photo = this.journey.addProviderPhotoToGallery(Number(id), user.id, String(body.provider), String(body.asset_id), undefined, pp, body.media_type === 'video' ? 'video' : 'image'); if (!photo) { throw new HttpException({ error: 'Not allowed or duplicate' }, 403); } @@ -342,7 +350,7 @@ export class JourneyController { } @Patch(':id') - update(@CurrentUser() user: User, @Param('id') id: string, @Body() body: Record) { + update(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyUpdateDto) { const result = this.journey.updateJourney(Number(id), user.id, body); if (!result) { throw new HttpException({ error: 'Journey not found' }, 404); @@ -375,7 +383,7 @@ export class JourneyController { // ── Journey trips ─────────────────────────────────────────────────────── @Post(':id/trips') @HttpCode(200) // Express answers with res.json (200). - addTrip(@CurrentUser() user: User, @Param('id') id: string, @Body() body: { trip_id?: unknown }) { + addTrip(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyAddTripDto) { if (!body.trip_id) { throw new HttpException({ error: 'trip_id required' }, 400); } @@ -404,7 +412,7 @@ export class JourneyController { } @Post(':id/entries') - createEntry(@CurrentUser() user: User, @Param('id') id: string, @Body() body: Record & { entry_date?: unknown }, @Headers('x-socket-id') socketId?: string) { + createEntry(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyEntryCreateDto, @Headers('x-socket-id') socketId?: string) { if (!body.entry_date) { throw new HttpException({ error: 'entry_date is required' }, 400); } @@ -416,7 +424,7 @@ export class JourneyController { } @Put(':id/entries/reorder') - reorderEntries(@CurrentUser() user: User, @Param('id') id: string, @Body() body: { orderedIds?: unknown }, @Headers('x-socket-id') socketId?: string) { + reorderEntries(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyReorderEntriesDto, @Headers('x-socket-id') socketId?: string) { const orderedIds = body.orderedIds; if (!Array.isArray(orderedIds) || !orderedIds.every((v) => Number.isFinite(Number(v)))) { throw new HttpException({ error: 'orderedIds must be an array of numbers' }, 400); @@ -429,18 +437,18 @@ export class JourneyController { // ── Contributors ──────────────────────────────────────────────────────── @Post(':id/contributors') - addContributor(@CurrentUser() user: User, @Param('id') id: string, @Body() body: { user_id?: unknown; role?: 'editor' | 'viewer' }) { + addContributor(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyContributorAddDto) { if (!body.user_id) { throw new HttpException({ error: 'user_id required' }, 400); } - if (!this.journey.addContributor(Number(id), user.id, Number(body.user_id), body.role || 'viewer')) { + if (!this.journey.addContributor(Number(id), user.id, Number(body.user_id), (body.role as 'editor' | 'viewer') || 'viewer')) { throw new HttpException({ error: 'Not allowed' }, 403); } return { success: true }; } @Patch(':id/contributors/:userId') - updateContributor(@CurrentUser() user: User, @Param('id') id: string, @Param('userId') userId: string, @Body() body: { role?: 'editor' | 'viewer' }) { + updateContributor(@CurrentUser() user: User, @Param('id') id: string, @Param('userId') userId: string, @Body() body: JourneyContributorUpdateDto) { if (!this.journey.updateContributorRole(Number(id), user.id, Number(userId), body.role as 'editor' | 'viewer')) { throw new HttpException({ error: 'Not allowed' }, 403); } @@ -457,7 +465,7 @@ export class JourneyController { // ── User Preferences ──────────────────────────────────────────────────── @Patch(':id/preferences') - preferences(@CurrentUser() user: User, @Param('id') id: string, @Body() body: Record) { + preferences(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyPreferencesDto) { const result = this.journey.updateJourneyPreferences(Number(id), user.id, body); if (!result) { throw new HttpException({ error: 'Not allowed' }, 403); @@ -473,11 +481,11 @@ export class JourneyController { @Post(':id/share-link') @HttpCode(200) // Express answers with res.json (200). - setShareLink(@CurrentUser() user: User, @Param('id') id: string, @Body() body: { share_timeline?: boolean; share_gallery?: boolean; share_map?: boolean }) { + setShareLink(@CurrentUser() user: User, @Param('id') id: string, @Body() body: JourneyShareLinkDto) { const result = this.journey.createOrUpdateJourneyShareLink(Number(id), user.id, { - share_timeline: body.share_timeline, - share_gallery: body.share_gallery, - share_map: body.share_map, + share_timeline: body.share_timeline as boolean | undefined, + share_gallery: body.share_gallery as boolean | undefined, + share_map: body.share_map as boolean | undefined, }); if (!result) { throw new HttpException({ error: 'Not allowed' }, 403); diff --git a/server/src/nest/journey/journey.dto.ts b/server/src/nest/journey/journey.dto.ts new file mode 100644 index 000000000..d1df7e371 --- /dev/null +++ b/server/src/nest/journey/journey.dto.ts @@ -0,0 +1,51 @@ +import { createZodDto } from 'nestjs-zod'; +import { + journeyAddTripRequestSchema, + journeyContributorRequestSchema, + journeyContributorUpdateRequestSchema, + journeyCreateRequestSchema, + journeyEntryCreateRequestSchema, + journeyEntryPhotoUploadRequestSchema, + journeyEntryUpdateRequestSchema, + journeyGalleryVideoRequestSchema, + journeyLinkPhotoRequestSchema, + journeyPhotoUpdateRequestSchema, + journeyPreferencesRequestSchema, + journeyProviderPhotosRequestSchema, + journeyReorderEntriesRequestSchema, + journeyShareLinkRequestSchema, + journeyUpdateRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared journey 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. + * + * The schemas behind them are deliberately permissive, and the bespoke 400s stay + * in the controller: eight of these handlers answer with their own message + * ('Title is required', 'trip_id required', 'provider and asset_id required', + * 'journey_photo_id required', 'entry_date is required', 'user_id required'), + * and those bodies are pinned. A guard-tight schema would have the pipe answer + * first, with a different body — a client-visible change, not a refactor. + * + * These sixteen routes were the largest block in + * common/body-contract-allow-list.ts; the boot gate throws on stale entries, so + * they were removed in the same change. + */ +export class JourneyCreateDto extends createZodDto(journeyCreateRequestSchema) {} +export class JourneyUpdateDto extends createZodDto(journeyUpdateRequestSchema) {} +export class JourneyAddTripDto extends createZodDto(journeyAddTripRequestSchema) {} +export class JourneyEntryCreateDto extends createZodDto(journeyEntryCreateRequestSchema) {} +export class JourneyEntryUpdateDto extends createZodDto(journeyEntryUpdateRequestSchema) {} +export class JourneyReorderEntriesDto extends createZodDto(journeyReorderEntriesRequestSchema) {} +export class JourneyContributorAddDto extends createZodDto(journeyContributorRequestSchema) {} +export class JourneyContributorUpdateDto extends createZodDto(journeyContributorUpdateRequestSchema) {} +export class JourneyPreferencesDto extends createZodDto(journeyPreferencesRequestSchema) {} +export class JourneyShareLinkDto extends createZodDto(journeyShareLinkRequestSchema) {} +export class JourneyProviderPhotosDto extends createZodDto(journeyProviderPhotosRequestSchema) {} +export class JourneyLinkPhotoDto extends createZodDto(journeyLinkPhotoRequestSchema) {} +export class JourneyPhotoUpdateDto extends createZodDto(journeyPhotoUpdateRequestSchema) {} +export class JourneyEntryPhotoUploadDto extends createZodDto(journeyEntryPhotoUploadRequestSchema) {} +export class JourneyGalleryVideoDto extends createZodDto(journeyGalleryVideoRequestSchema) {} diff --git a/shared/src/journey/journey.schema.spec.ts b/shared/src/journey/journey.schema.spec.ts index bb7230d8b..659cb2a4b 100644 --- a/shared/src/journey/journey.schema.spec.ts +++ b/shared/src/journey/journey.schema.spec.ts @@ -1,3 +1,4 @@ +import { describe, it, expect } from 'vitest'; import { journeyCreateRequestSchema, journeyAddTripRequestSchema, @@ -5,67 +6,91 @@ import { journeyContributorRequestSchema, journeyProviderPhotosRequestSchema, journeyShareLinkRequestSchema, + journeyUpdateRequestSchema, + journeyPreferencesRequestSchema, + journeyLinkPhotoRequestSchema, } from './journey.schema'; -import { describe, it, expect } from 'vitest'; - +/** + * These schemas describe the journey bodies rather than tighten them, and the + * cases below are what makes that checkable: every shape the handler still + * rejects itself has to PARSE here, or the global pipe would answer first with + * a different error body than the one the e2e cases pin. + * + * This inverts the assertions the file carried while the schemas were strict. + * That was the point of the change. + */ describe('journeyCreateRequestSchema', () => { - it('requires a title; subtitle + trip_ids optional', () => { + it('accepts the well-formed body', () => { expect(journeyCreateRequestSchema.safeParse({ title: 'Trip of a lifetime' }).success).toBe(true); expect(journeyCreateRequestSchema.safeParse({ title: 'X', trip_ids: [1, '2'] }).success).toBe(true); - expect(journeyCreateRequestSchema.safeParse({ subtitle: 'no title' }).success).toBe(false); + }); + + it('leaves the missing/blank/non-string title to the handler', () => { + // The handler answers 'Title is required' for all three. + expect(journeyCreateRequestSchema.safeParse({ subtitle: 'no title' }).success).toBe(true); + expect(journeyCreateRequestSchema.safeParse({ title: ' ' }).success).toBe(true); + expect(journeyCreateRequestSchema.safeParse({ title: 42 }).success).toBe(true); + }); + + it('leaves a non-array trip_ids to the handler, which reads it as none', () => { + expect(journeyCreateRequestSchema.safeParse({ title: 'X', trip_ids: 'nope' }).success).toBe(true); }); }); describe('journeyAddTripRequestSchema', () => { - it('requires a trip_id (string or number)', () => { + it('accepts an id and leaves the missing one to the handler', () => { expect(journeyAddTripRequestSchema.safeParse({ trip_id: 5 }).success).toBe(true); expect(journeyAddTripRequestSchema.safeParse({ trip_id: '5' }).success).toBe(true); - expect(journeyAddTripRequestSchema.safeParse({}).success).toBe(false); + // The handler answers 'trip_id required'. + expect(journeyAddTripRequestSchema.safeParse({}).success).toBe(true); }); }); describe('journeyReorderEntriesRequestSchema', () => { - it('requires a non-empty orderedIds array', () => { - expect(journeyReorderEntriesRequestSchema.safeParse({ orderedIds: [3, 1, 2] }).success).toBe(true); - expect(journeyReorderEntriesRequestSchema.safeParse({ orderedIds: [] }).success).toBe(false); + it('accepts a list, and an empty one, which the handler short-circuits', () => { + expect(journeyReorderEntriesRequestSchema.safeParse({ orderedIds: [3, '1'] }).success).toBe(true); + expect(journeyReorderEntriesRequestSchema.safeParse({ orderedIds: [] }).success).toBe(true); + expect(journeyReorderEntriesRequestSchema.safeParse({}).success).toBe(true); }); }); describe('journeyContributorRequestSchema', () => { - it('requires user_id; role limited to editor/viewer', () => { - expect(journeyContributorRequestSchema.safeParse({ user_id: 2 }).success).toBe(true); - expect(journeyContributorRequestSchema.safeParse({ user_id: 2, role: 'editor' }).success).toBe(true); - expect(journeyContributorRequestSchema.safeParse({ user_id: 2, role: 'admin' }).success).toBe(false); + it('accepts the roles and leaves an unknown one to the handler', () => { + expect(journeyContributorRequestSchema.safeParse({ user_id: 3, role: 'editor' }).success).toBe(true); + expect(journeyContributorRequestSchema.safeParse({ user_id: 3 }).success).toBe(true); + // Never validated: the handler casts it. + expect(journeyContributorRequestSchema.safeParse({ user_id: 3, role: 'admin' }).success).toBe(true); + // The handler answers 'user_id required'. + expect(journeyContributorRequestSchema.safeParse({}).success).toBe(true); }); }); describe('journeyProviderPhotosRequestSchema', () => { - it('requires a provider; accepts single asset_id or a batch', () => { - expect( - journeyProviderPhotosRequestSchema.safeParse({ - provider: 'immich', - asset_id: 'a1', - }).success, - ).toBe(true); - expect( - journeyProviderPhotosRequestSchema.safeParse({ - provider: 'immich', - asset_ids: ['a1', 'a2'], - }).success, - ).toBe(true); - expect(journeyProviderPhotosRequestSchema.safeParse({ asset_id: 'a1' }).success).toBe(false); + it('accepts single and batch, and leaves the incomplete pair to the handler', () => { + expect(journeyProviderPhotosRequestSchema.safeParse({ provider: 'immich', asset_id: 'a1' }).success).toBe(true); + expect(journeyProviderPhotosRequestSchema.safeParse({ provider: 'immich', asset_ids: ['a1', 2], media_types: ['video'] }).success).toBe(true); + // The handler answers 'provider and asset_id required'. + expect(journeyProviderPhotosRequestSchema.safeParse({ asset_id: 'a1' }).success).toBe(true); + expect(journeyProviderPhotosRequestSchema.safeParse({}).success).toBe(true); }); }); describe('journeyShareLinkRequestSchema', () => { - it('accepts optional share toggles', () => { - expect( - journeyShareLinkRequestSchema.safeParse({ - share_timeline: true, - share_gallery: false, - }).success, - ).toBe(true); + it('accepts the flags in whatever shape the client sends them', () => { + expect(journeyShareLinkRequestSchema.safeParse({ share_timeline: true, share_gallery: false }).success).toBe(true); expect(journeyShareLinkRequestSchema.safeParse({}).success).toBe(true); + // The controller coerces with !!; a string flag has always worked. + expect(journeyShareLinkRequestSchema.safeParse({ share_map: 'true' }).success).toBe(true); + }); +}); + +describe('the free-form bodies keep their unknown keys', () => { + it('does not strip what the services are handed whole', () => { + // The whole reason these are loose objects: a strict object drops unknown + // keys, and update/preferences/link are forwarded to the service as sent. + expect(journeyUpdateRequestSchema.parse({ title: 'X', cover_photo_id: 9 })).toEqual({ title: 'X', cover_photo_id: 9 }); + expect(journeyPreferencesRequestSchema.parse({ layout: 'grid', density: 2 })).toEqual({ layout: 'grid', density: 2 }); + expect(journeyLinkPhotoRequestSchema.parse({ journey_photo_id: 4, extra: true })).toEqual({ journey_photo_id: 4, extra: true }); }); }); diff --git a/shared/src/journey/journey.schema.ts b/shared/src/journey/journey.schema.ts index b88b97d68..a8e2fd1b1 100644 --- a/shared/src/journey/journey.schema.ts +++ b/shared/src/journey/journey.schema.ts @@ -7,50 +7,126 @@ import { z } from 'zod'; * * Authenticated routes live under /api/journeys (gated by the Journey addon); * the public read/photo-proxy routes live under /api/public/journey and are - * share-token validated. Access control lives inside journeyService (it returns - * null/false → the controller maps to 403/404), so these schemas pin the - * well-defined request bodies; entry create/update stay open-ended (forwarded - * to the service) and the bespoke 400/403/404 messages pin the rest. + * share-token validated. Access control lives inside the journey services (they + * return null/false → the controller maps to 403/404). + * + * Deliberately permissive, the same doctrine as memories (#1842): eight of the + * sixteen handlers validate their own body and answer with their own text — + * 'Title is required', 'trip_id required', 'provider and asset_id required', + * and so on — and those messages are pinned by e2e and integration cases. A + * strict schema would have the global pipe answer first, with a different body, + * which is a client-visible change wearing the costume of a refactor. So every + * field the handler type-checks itself is `unknown` here: the schema describes + * the shape that already works, and the boot-time contract gate gets something + * to point at. + * + * They are LOOSE objects for the second memories reason: a strict object strips + * unknown keys, and the update/entry/preferences bodies are forwarded to the + * services whole. Stripping there would silently drop a field added later. */ -export const journeyCreateRequestSchema = z.object({ - title: z.string().min(1), - subtitle: z.string().optional(), - trip_ids: z.array(z.union([z.string(), z.number()])).optional(), +export const journeyCreateRequestSchema = z.looseObject({ + // The handler does `!body.title || typeof body.title !== 'string' || !trim()` + // and answers 'Title is required'. Typing it here would pre-empt that. + title: z.unknown().optional(), + subtitle: z.unknown().optional(), + // `Array.isArray(body.trip_ids) ? body.trip_ids.map(Number) : []` — anything + // that is not an array is silently treated as none. + trip_ids: z.unknown().optional(), }); export type JourneyCreateRequest = z.infer; -export const journeyAddTripRequestSchema = z.object({ - trip_id: z.union([z.string(), z.number()]), +/** Free-form: forwarded to the service as-is. */ +export const journeyUpdateRequestSchema = z.looseObject({}); +export type JourneyUpdateRequest = z.infer; + +export const journeyAddTripRequestSchema = z.looseObject({ + // Handler answers 'trip_id required'. + trip_id: z.unknown().optional(), }); export type JourneyAddTripRequest = z.infer; -export const journeyReorderEntriesRequestSchema = z.object({ - orderedIds: z.array(z.union([z.string(), z.number()])).min(1), +export const journeyReorderEntriesRequestSchema = z.looseObject({ + // No .min(1): an empty list is accepted today and short-circuits in the + // handler, and the handler's own 'orderedIds must be...' check is the one + // that answers for a malformed list. + orderedIds: z.unknown().optional(), }); export type JourneyReorderEntriesRequest = z.infer; -export const journeyContributorRequestSchema = z.object({ - user_id: z.union([z.string(), z.number()]), - role: z.enum(['editor', 'viewer']).optional(), +export const journeyContributorRequestSchema = z.looseObject({ + // Handler answers 'user_id required'; role is cast, never validated. + user_id: z.unknown().optional(), + role: z.unknown().optional(), }); export type JourneyContributorRequest = z.infer; -export const journeyProviderPhotosRequestSchema = z.object({ - provider: z.string().min(1), - asset_id: z.string().optional(), - asset_ids: z.array(z.union([z.string(), z.number()])).optional(), - caption: z.string().optional(), - passphrase: z.string().optional(), +export const journeyContributorUpdateRequestSchema = z.looseObject({ + role: z.unknown().optional(), +}); +export type JourneyContributorUpdateRequest = z.infer; + +export const journeyProviderPhotosRequestSchema = z.looseObject({ + // Handler answers 'provider and asset_id required' when either is missing, + // and branches on Array.isArray(asset_ids) before that. + provider: z.unknown().optional(), + asset_id: z.unknown().optional(), + asset_ids: z.unknown().optional(), + caption: z.unknown().optional(), + passphrase: z.unknown().optional(), // Per-asset 'image' | 'video' discriminator, parallel to asset_ids (#823). - media_type: z.string().optional(), - media_types: z.array(z.string()).optional(), + media_type: z.unknown().optional(), + media_types: z.unknown().optional(), }); export type JourneyProviderPhotosRequest = z.infer; -export const journeyShareLinkRequestSchema = z.object({ - share_timeline: z.boolean().optional(), - share_gallery: z.boolean().optional(), - share_map: z.boolean().optional(), +export const journeyLinkPhotoRequestSchema = z.looseObject({ + // Handler answers 'journey_photo_id required'. + journey_photo_id: z.unknown().optional(), + photo_id: z.unknown().optional(), +}); +export type JourneyLinkPhotoRequest = z.infer; + +/** Free-form: forwarded to the service as-is. */ +export const journeyPhotoUpdateRequestSchema = z.looseObject({}); +export type JourneyPhotoUpdateRequest = z.infer; + +/** Free-form plus the entry_date the handler demands itself. */ +export const journeyEntryCreateRequestSchema = z.looseObject({ + entry_date: z.unknown().optional(), +}); +export type JourneyEntryCreateRequest = z.infer; + +/** Free-form: forwarded to the service as-is. */ +export const journeyEntryUpdateRequestSchema = z.looseObject({}); +export type JourneyEntryUpdateRequest = z.infer; + +/** + * Multipart field alongside the uploaded files. + * + * `.optional()` is not decoration: a multipart request with files and no text + * fields arrives with NO body at all, and the handler answers 'No files + * uploaded' for the empty case. Without this the pipe would answer + * 'body: Invalid input: expected object' first and the message would change. + */ +export const journeyEntryPhotoUploadRequestSchema = z.looseObject({ + caption: z.unknown().optional(), +}).optional(); +export type JourneyEntryPhotoUploadRequest = z.infer; + +/** Multipart field alongside the uploaded video — absent body for the same reason. */ +export const journeyGalleryVideoRequestSchema = z.looseObject({ + duration_ms: z.unknown().optional(), +}).optional(); +export type JourneyGalleryVideoRequest = z.infer; + +/** Free-form: per-user display preferences, forwarded whole. */ +export const journeyPreferencesRequestSchema = z.looseObject({}); +export type JourneyPreferencesRequest = z.infer; + +export const journeyShareLinkRequestSchema = z.looseObject({ + share_timeline: z.unknown().optional(), + share_gallery: z.unknown().optional(), + share_map: z.unknown().optional(), }); export type JourneyShareLinkRequest = z.infer;