diff --git a/package-lock.json b/package-lock.json index 106c5a3f2..22089d79e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10012,7 +10012,6 @@ "version": "4.3.1", "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-4.3.1.tgz", "integrity": "sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A==", - "dev": true, "license": "MIT", "engines": { "node": ">=0.10.0" @@ -15370,6 +15369,26 @@ "node": ">= 0.6" } }, + "node_modules/nestjs-zod": { + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/nestjs-zod/-/nestjs-zod-5.5.0.tgz", + "integrity": "sha512-V0WMtmICygYE2RoMKk8ExEeoetkC7PoUmUlZNP4JNk1YgrnUnG1vgkRn2DBNqsDi+Obv3c9AOssmXe/k/AvBOg==", + "license": "MIT", + "dependencies": { + "deepmerge": "^4.3.1" + }, + "peerDependencies": { + "@nestjs/common": "^10.0.0 || ^11.0.0", + "@nestjs/swagger": "^7.4.2 || ^8.0.0 || ^11.0.0", + "rxjs": "^7.0.0", + "zod": "^3.25.0 || ^4.0.0" + }, + "peerDependenciesMeta": { + "@nestjs/swagger": { + "optional": true + } + } + }, "node_modules/node-abi": { "version": "3.92.0", "resolved": "https://registry.npmjs.org/node-abi/-/node-abi-3.92.0.tgz", @@ -21397,6 +21416,7 @@ "jimp": "^1.6.1", "json5": "^2.2.3", "jsonwebtoken": "^9.0.2", + "nestjs-zod": "^5.5.0", "node-cron": "^4.2.1", "nodemailer": "^9.0.1", "otplib": "^12.0.1", diff --git a/server/package.json b/server/package.json index 2c32442bc..435ad19eb 100644 --- a/server/package.json +++ b/server/package.json @@ -44,6 +44,7 @@ "jimp": "^1.6.1", "json5": "^2.2.3", "jsonwebtoken": "^9.0.2", + "nestjs-zod": "^5.5.0", "node-cron": "^4.2.1", "nodemailer": "^9.0.1", "otplib": "^12.0.1", diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index 4274b25f8..13a087733 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -10,6 +10,7 @@ import { apiDocsEnabled } from './nest/common/api-docs.kill-switch'; import { setupApiDocs } from './nest/platform/api-docs'; import { McpRegistryService } from '@trek/nest-mcp'; import { setMcpRegistry } from './mcp/registry-handoff'; +import { validateBodyContracts } from './nest/common/validate-body-contracts'; /** * Builds the unified TREK NestJS application that serves the ENTIRE surface — the @@ -61,5 +62,9 @@ export async function buildApp(): Promise { // The /mcp handler is mounted pre-init (step 3) and has no DI access — hand // it the boot-discovered registry now that the container is fully built. setMcpRegistry(app.get(McpRegistryService)); + // Fail closed on unvalidated mutation bodies: every POST/PUT/PATCH @Body() + // must carry a createZodDto class (validated by the global ZodValidationPipe) + // or sit on the ratchet-only legacy allow-list — otherwise refuse to boot. + validateBodyContracts(app); return app; } diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 0fd0cb8b3..6e2286bf5 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; -import { APP_FILTER, APP_INTERCEPTOR } from '@nestjs/core'; +import { APP_FILTER, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core'; +import { ZodValidationPipe } from './common/zod-validation.pipe'; import { AppConfigModule } from './app-config/app-config.module'; import { DatabaseModule } from './database/database.module'; import { HealthController } from './health/health.controller'; @@ -69,6 +70,12 @@ import { IdempotencyInterceptor } from './common/idempotency.interceptor'; // Replays the X-Idempotency-Key the client sends on every write, matching // the legacy applyIdempotency middleware so retried mutations don't double-apply. { provide: APP_INTERCEPTOR, useClass: IdempotencyInterceptor }, + // Global Zod validation: any parameter typed with a createZodDto class + // (the .dto.ts wrappers over @trek/shared schemas) is validated; + // everything else passes through untouched. Paired with the boot gate in + // common/validate-body-contracts.ts so unvalidated mutation bodies refuse + // to boot instead of shipping silently. + { provide: APP_PIPE, useClass: ZodValidationPipe }, ], }) export class AppModule {} diff --git a/server/src/nest/collections/collections.controller.ts b/server/src/nest/collections/collections.controller.ts index 593bb213d..d9505468f 100644 --- a/server/src/nest/collections/collections.controller.ts +++ b/server/src/nest/collections/collections.controller.ts @@ -27,46 +27,28 @@ import { CollectionsService } from './collections.service'; import { CollectionsAddonGuard } from './collections-addon.guard'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; import { PLACE_IMAGE_UPLOAD } from '../common/place-image-upload'; import { placeImageUrl } from '../../services/placeImage'; import { isDemoEmail } from '../../services/demo'; import { - collectionCreateRequestSchema, - collectionUpdateRequestSchema, - collectionSavePlaceRequestSchema, - collectionSaveFromTripRequestSchema, - collectionSaveFromTripManyRequestSchema, - collectionPlaceUpdateRequestSchema, - collectionSetStatusRequestSchema, - placeRatingRequestSchema, - collectionCopyToTripRequestSchema, - collectionInviteRequestSchema, - collectionInviteActionRequestSchema, - collectionInviteCancelRequestSchema, - collectionRemoveMemberRequestSchema, - collectionSetMemberRoleRequestSchema, - collectionLabelCreateRequestSchema, - collectionLabelUpdateRequestSchema, - collectionLabelAssignRequestSchema, - type CollectionLabelCreateRequest, - type CollectionLabelUpdateRequest, - type CollectionLabelAssignRequest, - type CollectionCreateRequest, - type CollectionUpdateRequest, - type CollectionSavePlaceRequest, - type CollectionSaveFromTripRequest, - type CollectionSaveFromTripManyRequest, - type CollectionPlaceUpdateRequest, - type CollectionSetStatusRequest, - type PlaceRatingRequest, - type CollectionCopyToTripRequest, - type CollectionInviteRequest, - type CollectionInviteActionRequest, - type CollectionInviteCancelRequest, - type CollectionRemoveMemberRequest, - type CollectionSetMemberRoleRequest, -} from '@trek/shared'; + CollectionCreateDto, + CollectionUpdateDto, + CollectionSavePlaceDto, + CollectionSaveFromTripDto, + CollectionSaveFromTripManyDto, + CollectionPlaceUpdateDto, + CollectionSetStatusDto, + CollectionCopyToTripDto, + CollectionInviteDto, + CollectionInviteActionDto, + CollectionInviteCancelDto, + CollectionRemoveMemberDto, + CollectionSetMemberRoleDto, + CollectionLabelCreateDto, + CollectionLabelUpdateDto, + CollectionLabelAssignDto, +} from './collections.dto'; +import { PlaceRatingDto } from '../places/places.dto'; const MAX_COVER_SIZE = 20 * 1024 * 1024; const coversDir = path.join(__dirname, '../../../uploads/covers'); @@ -109,7 +91,7 @@ export class CollectionsController { } @Post() - create(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionCreateRequestSchema)) body: CollectionCreateRequest) { + create(@CurrentUser() user: User, @Body() body: CollectionCreateDto) { return this.collections.createCollection(user.id, body); } @@ -126,19 +108,19 @@ export class CollectionsController { // ── Places (static prefixes before /:id) ──────────────────────────────────── @Post('places') @HttpCode(200) - savePlace(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionSavePlaceRequestSchema)) body: CollectionSavePlaceRequest, @Headers('x-socket-id') socketId?: string) { + savePlace(@CurrentUser() user: User, @Body() body: CollectionSavePlaceDto, @Headers('x-socket-id') socketId?: string) { return this.collections.savePlace(user.id, body, socketId); } @Post('places/from-trip') @HttpCode(200) - saveFromTrip(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionSaveFromTripRequestSchema)) body: CollectionSaveFromTripRequest) { + saveFromTrip(@CurrentUser() user: User, @Body() body: CollectionSaveFromTripDto) { return this.collections.saveFromTripPlace(user.id, body.collection_id, body.source_trip_id, body.source_place_id, body.force); } @Post('places/from-trip-many') @HttpCode(200) - saveFromTripMany(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionSaveFromTripManyRequestSchema)) body: CollectionSaveFromTripManyRequest) { + saveFromTripMany(@CurrentUser() user: User, @Body() body: CollectionSaveFromTripManyDto) { return this.collections.saveFromTripPlaces(user.id, body.collection_id, body.source_trip_id, body.source_place_ids, body.force); } @@ -155,7 +137,7 @@ export class CollectionsController { updatePlace( @CurrentUser() user: User, @Param('pid') pid: string, - @Body(new ZodValidationPipe(collectionPlaceUpdateRequestSchema)) body: CollectionPlaceUpdateRequest, + @Body() body: CollectionPlaceUpdateDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.updatePlace(user.id, Number(pid), body, socketId); @@ -166,7 +148,7 @@ export class CollectionsController { setStatus( @CurrentUser() user: User, @Param('pid') pid: string, - @Body(new ZodValidationPipe(collectionSetStatusRequestSchema)) body: CollectionSetStatusRequest, + @Body() body: CollectionSetStatusDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.setStatus(user.id, Number(pid), body.status, socketId); @@ -176,7 +158,7 @@ export class CollectionsController { setRating( @CurrentUser() user: User, @Param('pid') pid: string, - @Body(new ZodValidationPipe(placeRatingRequestSchema)) body: PlaceRatingRequest, + @Body() body: PlaceRatingDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.setRating(user.id, Number(pid), body.rating, socketId); @@ -212,7 +194,7 @@ export class CollectionsController { // ── Copy to trip ──────────────────────────────────────────────────────────── @Post('copy-to-trip') @HttpCode(200) - copyToTrip(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionCopyToTripRequestSchema)) body: CollectionCopyToTripRequest) { + copyToTrip(@CurrentUser() user: User, @Body() body: CollectionCopyToTripDto) { return this.collections.copyToTrip(user.id, body); } @@ -221,7 +203,7 @@ export class CollectionsController { @HttpCode(200) createLabel( @CurrentUser() user: User, - @Body(new ZodValidationPipe(collectionLabelCreateRequestSchema)) body: CollectionLabelCreateRequest, + @Body() body: CollectionLabelCreateDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.createLabel(user.id, body.collection_id, body.name, body.color, socketId); @@ -231,7 +213,7 @@ export class CollectionsController { updateLabel( @CurrentUser() user: User, @Param('lid') lid: string, - @Body(new ZodValidationPipe(collectionLabelUpdateRequestSchema)) body: CollectionLabelUpdateRequest, + @Body() body: CollectionLabelUpdateDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.updateLabel(user.id, Number(lid), body, socketId); @@ -247,7 +229,7 @@ export class CollectionsController { @HttpCode(200) assignLabels( @CurrentUser() user: User, - @Body(new ZodValidationPipe(collectionLabelAssignRequestSchema)) body: CollectionLabelAssignRequest, + @Body() body: CollectionLabelAssignDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.assignLabels(user.id, body.label_ids, body.place_ids, false, socketId); @@ -257,7 +239,7 @@ export class CollectionsController { @HttpCode(200) unassignLabels( @CurrentUser() user: User, - @Body(new ZodValidationPipe(collectionLabelAssignRequestSchema)) body: CollectionLabelAssignRequest, + @Body() body: CollectionLabelAssignDto, @Headers('x-socket-id') socketId?: string, ) { return this.collections.assignLabels(user.id, body.label_ids, body.place_ids, true, socketId); @@ -285,7 +267,7 @@ export class CollectionsController { // ── Fusion invitations ────────────────────────────────────────────────────── @Post('invite') @HttpCode(200) - invite(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionInviteRequestSchema)) body: CollectionInviteRequest) { + invite(@CurrentUser() user: User, @Body() body: CollectionInviteDto) { this.collections.assertAccess(user.id, body.collection_id); // 404 if not visible (no enumeration) if (!this.collections.isOwner(user.id, body.collection_id)) { throw new HttpException({ error: 'Only the owner can invite' }, 403); @@ -299,7 +281,7 @@ export class CollectionsController { @Post('invite/accept') @HttpCode(200) - acceptInvite(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionInviteActionRequestSchema)) body: CollectionInviteActionRequest, @Headers('x-socket-id') socketId?: string) { + acceptInvite(@CurrentUser() user: User, @Body() body: CollectionInviteActionDto, @Headers('x-socket-id') socketId?: string) { const result = this.collections.acceptInvite(user.id, body.collection_id, socketId); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -309,14 +291,14 @@ export class CollectionsController { @Post('invite/decline') @HttpCode(200) - declineInvite(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionInviteActionRequestSchema)) body: CollectionInviteActionRequest, @Headers('x-socket-id') socketId?: string) { + declineInvite(@CurrentUser() user: User, @Body() body: CollectionInviteActionDto, @Headers('x-socket-id') socketId?: string) { this.collections.declineInvite(user.id, body.collection_id, socketId); return { success: true }; } @Post('invite/cancel') @HttpCode(200) - cancelInvite(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionInviteCancelRequestSchema)) body: CollectionInviteCancelRequest) { + cancelInvite(@CurrentUser() user: User, @Body() body: CollectionInviteCancelDto) { this.collections.assertAccess(user.id, body.collection_id); // 404 if not visible if (!this.collections.isOwner(user.id, body.collection_id)) { throw new HttpException({ error: 'Only the owner can cancel invites' }, 403); @@ -327,14 +309,14 @@ export class CollectionsController { @Post('leave') @HttpCode(200) - leave(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionInviteActionRequestSchema)) body: CollectionInviteActionRequest, @Headers('x-socket-id') socketId?: string) { + leave(@CurrentUser() user: User, @Body() body: CollectionInviteActionDto, @Headers('x-socket-id') socketId?: string) { this.collections.leaveCollection(user.id, body.collection_id, socketId); return { success: true }; } @Post('members/remove') @HttpCode(200) - removeMember(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionRemoveMemberRequestSchema)) body: CollectionRemoveMemberRequest) { + removeMember(@CurrentUser() user: User, @Body() body: CollectionRemoveMemberDto) { this.collections.assertAccess(user.id, body.collection_id); // 404 if not visible if (!this.collections.isOwner(user.id, body.collection_id)) { throw new HttpException({ error: 'Only the owner can remove members' }, 403); @@ -345,7 +327,7 @@ export class CollectionsController { @Post('members/role') @HttpCode(200) - setMemberRole(@CurrentUser() user: User, @Body(new ZodValidationPipe(collectionSetMemberRoleRequestSchema)) body: CollectionSetMemberRoleRequest) { + setMemberRole(@CurrentUser() user: User, @Body() body: CollectionSetMemberRoleDto) { this.collections.assertAccess(user.id, body.collection_id); // 404 if not visible if (!this.collections.isOwner(user.id, body.collection_id)) { throw new HttpException({ error: 'Only the owner can change member roles' }, 403); @@ -381,7 +363,7 @@ export class CollectionsController { } @Patch(':id') - update(@CurrentUser() user: User, @Param('id') id: string, @Body(new ZodValidationPipe(collectionUpdateRequestSchema)) body: CollectionUpdateRequest, @Headers('x-socket-id') socketId?: string) { + update(@CurrentUser() user: User, @Param('id') id: string, @Body() body: CollectionUpdateDto, @Headers('x-socket-id') socketId?: string) { return this.collections.updateCollection(user.id, Number(id), body, socketId); } diff --git a/server/src/nest/collections/collections.dto.ts b/server/src/nest/collections/collections.dto.ts new file mode 100644 index 000000000..4b4bf2d44 --- /dev/null +++ b/server/src/nest/collections/collections.dto.ts @@ -0,0 +1,43 @@ +import { createZodDto } from 'nestjs-zod'; +import { + collectionCreateRequestSchema, + collectionUpdateRequestSchema, + collectionSavePlaceRequestSchema, + collectionSaveFromTripRequestSchema, + collectionSaveFromTripManyRequestSchema, + collectionPlaceUpdateRequestSchema, + collectionSetStatusRequestSchema, + collectionCopyToTripRequestSchema, + collectionInviteRequestSchema, + collectionInviteActionRequestSchema, + collectionInviteCancelRequestSchema, + collectionRemoveMemberRequestSchema, + collectionSetMemberRoleRequestSchema, + collectionLabelCreateRequestSchema, + collectionLabelUpdateRequestSchema, + collectionLabelAssignRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared collections + * contracts. Typing a controller parameter with one of these classes is what + * lets the global ZodValidationPipe (APP_PIPE) validate it by metatype — the + * Zod schemas in shared/ remain the single source of truth for the contract. + */ + +export class CollectionCreateDto extends createZodDto(collectionCreateRequestSchema) {} +export class CollectionUpdateDto extends createZodDto(collectionUpdateRequestSchema) {} +export class CollectionSavePlaceDto extends createZodDto(collectionSavePlaceRequestSchema) {} +export class CollectionSaveFromTripDto extends createZodDto(collectionSaveFromTripRequestSchema) {} +export class CollectionSaveFromTripManyDto extends createZodDto(collectionSaveFromTripManyRequestSchema) {} +export class CollectionPlaceUpdateDto extends createZodDto(collectionPlaceUpdateRequestSchema) {} +export class CollectionSetStatusDto extends createZodDto(collectionSetStatusRequestSchema) {} +export class CollectionCopyToTripDto extends createZodDto(collectionCopyToTripRequestSchema) {} +export class CollectionInviteDto extends createZodDto(collectionInviteRequestSchema) {} +export class CollectionInviteActionDto extends createZodDto(collectionInviteActionRequestSchema) {} +export class CollectionInviteCancelDto extends createZodDto(collectionInviteCancelRequestSchema) {} +export class CollectionRemoveMemberDto extends createZodDto(collectionRemoveMemberRequestSchema) {} +export class CollectionSetMemberRoleDto extends createZodDto(collectionSetMemberRoleRequestSchema) {} +export class CollectionLabelCreateDto extends createZodDto(collectionLabelCreateRequestSchema) {} +export class CollectionLabelUpdateDto extends createZodDto(collectionLabelUpdateRequestSchema) {} +export class CollectionLabelAssignDto extends createZodDto(collectionLabelAssignRequestSchema) {} diff --git a/server/src/nest/common/api-zod.ts b/server/src/nest/common/api-zod.ts deleted file mode 100644 index f1c85248d..000000000 --- a/server/src/nest/common/api-zod.ts +++ /dev/null @@ -1,87 +0,0 @@ -import type { INestApplication } from '@nestjs/common'; -import { RequestMethod } from '@nestjs/common'; -import { ModulesContainer } from '@nestjs/core'; -import { PATH_METADATA, METHOD_METADATA, ROUTE_ARGS_METADATA } from '@nestjs/common/constants'; -import { z, type ZodType } from 'zod'; -import type { OpenAPIObject } from '@nestjs/swagger'; -import { ZodValidationPipe } from './zod-validation.pipe'; - -/** - * Phase 2 of #1412: real request-body schemas without annotating anything - * twice. The API validates with Zod via `@Body(new ZodValidationPipe(schema))` - * — that pipe IS the schema source of truth, so this enricher walks every - * controller route, finds full-body Zod pipes, and writes the converted - * JSON schema into the generated OpenAPI document. Any route that gains a - * ZodValidationPipe is documented automatically from then on. - */ - -/** Zod → OpenAPI 3.0 schema; degrades to a bare object for unrepresentable schemas. */ -export function zodToOpenApi(schema: ZodType): Record { - try { - return z.toJSONSchema(schema, { target: 'openapi-3.0', io: 'input', unrepresentable: 'any' }) as Record; - } catch { - return { type: 'object' }; - } -} - -const HTTP_METHOD: Partial> = { - [RequestMethod.GET]: 'get', - [RequestMethod.POST]: 'post', - [RequestMethod.PUT]: 'put', - [RequestMethod.PATCH]: 'patch', - [RequestMethod.DELETE]: 'delete', -}; - -// RouteParamtypes.BODY — the numeric key prefix @Body() writes into -// ROUTE_ARGS_METADATA (":"). The enum lives in a private -// @nestjs/common path, so the value is pinned here with a boot-time test. -const BODY_PARAMTYPE = '3'; - -function joinPath(base: string | undefined, sub: string | undefined): string { - const parts = `${base ?? ''}/${sub ?? ''}`.split('/').filter(Boolean); - // Express ":param" → OpenAPI "{param}" - return '/' + parts.map((p) => (p.startsWith(':') ? `{${p.slice(1)}}` : p)).join('/'); -} - -/** Find full-body Zod schemas per route and merge them into the document. */ -export function attachZodBodySchemas(app: INestApplication, document: OpenAPIObject): void { - const modules = app.get(ModulesContainer, { strict: false }); - for (const module of modules.values()) { - for (const wrapper of module.controllers.values()) { - const ctor = wrapper.metatype as (abstract new (...args: never[]) => unknown) | undefined; - if (!ctor || typeof ctor !== 'function') continue; - const basePath = Reflect.getMetadata(PATH_METADATA, ctor) as string | undefined; - if (basePath === undefined) continue; - - for (const name of Object.getOwnPropertyNames(ctor.prototype)) { - if (name === 'constructor') continue; - const handler = (ctor.prototype as Record)[name]; - if (typeof handler !== 'function') continue; - const methodEnum = Reflect.getMetadata(METHOD_METADATA, handler) as RequestMethod | undefined; - const verb = methodEnum !== undefined ? HTTP_METHOD[methodEnum] : undefined; - if (!verb) continue; - - const args = (Reflect.getMetadata(ROUTE_ARGS_METADATA, ctor, name) ?? {}) as Record< - string, - { index: number; data?: unknown; pipes?: unknown[] } - >; - const bodyArg = Object.entries(args).find(([key, meta]) => - key.startsWith(`${BODY_PARAMTYPE}:`) - // @Body('field') picks a sub-field — only whole-body pipes describe the request body - && meta.data === undefined - && (meta.pipes ?? []).some((p) => p instanceof ZodValidationPipe), - ); - if (!bodyArg) continue; - const zodPipe = (bodyArg[1].pipes ?? []).find((p): p is ZodValidationPipe => p instanceof ZodValidationPipe)!; - - const route = document.paths[joinPath(basePath, Reflect.getMetadata(PATH_METADATA, handler) as string | undefined)]; - const operation = route?.[verb as 'get' | 'post' | 'put' | 'patch' | 'delete']; - if (!operation) continue; - operation.requestBody = { - required: true, - content: { 'application/json': { schema: zodToOpenApi(zodPipe.schema) } }, - }; - } - } - } -} diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts new file mode 100644 index 000000000..ad2679ae3 --- /dev/null +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -0,0 +1,186 @@ +/** + * Legacy mutation handlers that still read @Body() without a createZodDto + * class — grandfathered when the fail-closed boot gate landed + * (validate-body-contracts.ts). Entries are `ControllerClass.methodName`. + * + * RATCHET ONLY: never add an entry. When a domain migrates, wrap its + * @trek/shared schema in a colocated .dto.ts and DELETE the routes + * here — the gate throws on stale entries, so removal is enforced too. + */ +export const BODY_CONTRACT_ALLOW_LIST: string[] = [ + 'AccommodationsController.create', + 'AccommodationsController.update', + 'AdminController.createInvite', + 'AdminController.createPackingTemplate', + 'AdminController.createTemplateCategory', + 'AdminController.createTemplateItem', + 'AdminController.createUser', + 'AdminController.devTestNotification', + 'AdminController.savePermissions', + 'AdminController.setDefaultUserSettings', + 'AdminController.setNotificationPrefs', + 'AdminController.updateAddon', + 'AdminController.updateBagTracking', + 'AdminController.updateCollabFeatures', + 'AdminController.updateOidc', + 'AdminController.updatePackingTemplate', + 'AdminController.updatePlacesAutocomplete', + 'AdminController.updatePlacesDetails', + 'AdminController.updatePlacesPhotos', + 'AdminController.updateTemplateCategory', + 'AdminController.updateTemplateItem', + 'AdminController.updateUser', + 'AssignmentOpsController.move', + 'AssignmentOpsController.setParticipants', + 'AssignmentOpsController.time', + 'AssignmentOpsController.transport', + 'AtlasController.createBucketItem', + 'AtlasController.markRegion', + 'AtlasController.updateBucketItem', + 'AuthController.apiKeys', + 'AuthController.changePassword', + 'AuthController.createMcpToken', + 'AuthController.mapsKey', + 'AuthController.mfaDisable', + 'AuthController.mfaEnable', + 'AuthController.resourceToken', + 'AuthController.updateAppSettings', + 'AuthController.updateSettings', + 'AuthPublicController.forgotPassword', + 'AuthPublicController.login', + 'AuthPublicController.register', + 'AuthPublicController.resetPassword', + 'AuthPublicController.verifyMfaLogin', + 'BackupController.updateAutoSettings', + 'BookingImportController.confirm', + 'BookingImportController.preview', + 'BookingImportController.previewAsync', + 'BudgetController.create', + 'BudgetController.createSettlement', + 'BudgetController.reorderCategories', + 'BudgetController.reorderItems', + 'BudgetController.setPayers', + 'BudgetController.toggleMemberPaid', + 'BudgetController.update', + 'BudgetController.updateMembers', + 'BudgetController.updateSettlement', + 'CategoriesController.create', + 'CategoriesController.update', + 'CollabController.createMessage', + 'CollabController.createNote', + 'CollabController.createPoll', + 'CollabController.react', + 'CollabController.updateNote', + 'CollabController.votePoll', + 'CollectionsController.deleteMany', + 'CollectionsController.reorder', + 'DayAssignmentsController.create', + 'DayAssignmentsController.reorder', + 'DayNotesController.create', + 'DayNotesController.update', + 'DaysController.create', + 'DaysController.reorder', + 'DaysController.transport', + 'DaysController.update', + 'FilesController.link', + 'FilesController.update', + 'FilesController.upload', + 'ImmichMemoriesController.putSettings', + 'ImmichMemoriesController.search', + 'ImmichMemoriesController.test', + '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', + 'MapsController.autocomplete', + 'MapsController.resolveUrl', + 'MapsController.search', + 'NotificationsController.respond', + 'NotificationsController.setPreferences', + 'NotificationsController.testNtfy', + 'NotificationsController.testSmtp', + 'NotificationsController.testWebhook', + 'OauthApiController.authorize', + 'OauthApiController.createClient', + 'PackingController.applyTemplate', + 'PackingController.create', + 'PackingController.createBag', + 'PackingController.importItems', + 'PackingController.reorder', + 'PackingController.saveAsTemplate', + 'PackingController.setBagMembers', + 'PackingController.setSharing', + 'PackingController.update', + 'PackingController.updateBag', + 'PackingController.updateCategoryAssignees', + 'PasskeyController.loginVerify', + 'PasskeyController.registerOptions', + 'PasskeyController.registerVerify', + 'PasskeyController.rename', + 'PlacesController.bulkDelete', + 'PlacesController.bulkUpdate', + 'PlacesController.create', + 'PlacesController.importGoogle', + 'PlacesController.importGpx', + 'PlacesController.importMap', + 'PlacesController.importNaver', + 'PlacesController.update', + 'PluginRoutesController.route', + 'PluginUserSettingsController.update', + 'PluginsController.activate', + 'PluginsController.install', + 'PluginsController.link', + 'PluginsController.retrust', + 'PluginsController.setEgressHosts', + 'PluginsController.uninstall', + 'PluginsController.updateConfig', + 'ReservationsController.create', + 'ReservationsController.update', + 'ReservationsController.updatePositions', + 'ReservationsController.updateTravelers', + 'SettingsController.bulk', + 'SettingsController.upsert', + 'SynologyMemoriesController.putSettings', + 'SynologyMemoriesController.search', + 'SynologyMemoriesController.test', + 'TagsController.create', + 'TagsController.update', + 'TripInviteLinkController.create', + 'TripShareController.create', + 'TripsController.addMember', + 'TripsController.copy', + 'TripsController.create', + 'TripsController.createGuest', + 'TripsController.renameGuest', + 'TripsController.transferOwnership', + 'TripsController.update', + 'UnifiedMemoriesController.addPhotos', + 'UnifiedMemoriesController.createAlbumLink', + 'UnifiedMemoriesController.setSharing', + 'VacayController.acceptInvite', + 'VacayController.addHolidayCalendar', + 'VacayController.addYear', + 'VacayController.cancelInvite', + 'VacayController.companyHoliday', + 'VacayController.declineInvite', + 'VacayController.invite', + 'VacayController.setColor', + 'VacayController.share', + 'VacayController.toggleEntry', + 'VacayController.updateHolidayCalendar', + 'VacayController.updatePlan', + 'VacayController.updateStats', +]; diff --git a/server/src/nest/common/validate-body-contracts.ts b/server/src/nest/common/validate-body-contracts.ts new file mode 100644 index 000000000..dccd22f60 --- /dev/null +++ b/server/src/nest/common/validate-body-contracts.ts @@ -0,0 +1,98 @@ +import type { INestApplication } from '@nestjs/common'; +import { RequestMethod } from '@nestjs/common'; +import { METHOD_METADATA, ROUTE_ARGS_METADATA } from '@nestjs/common/constants'; +import { RouteParamtypes } from '@nestjs/common/enums/route-paramtypes.enum'; +import { ModulesContainer } from '@nestjs/core'; +import { isZodDto } from 'nestjs-zod/dto'; +import { BODY_CONTRACT_ALLOW_LIST } from './body-contract-allow-list'; + +const MUTATION_METHODS = new Set([ + RequestMethod.POST, + RequestMethod.PUT, + RequestMethod.PATCH, +]); + +/** + * Boot-time fail-closed gate: every POST/PUT/PATCH handler that reads the + * request body (@Body() / @Body('field')) must type it with a + * `createZodDto(...)` class so the global ZodValidationPipe actually validates + * it — or be explicitly listed in body-contract-allow-list.ts (legacy routes + * whose domains have not migrated yet; the list only ratchets down). + * + * Called from buildApp() right after app.init() (same seam as the MCP registry + * handoff), so a maintainer who adds an unvalidated mutation route gets a + * refused boot in dev and CI instead of silently shipping an unvalidated + * endpoint. Same philosophy as McpRegistryService.validate(): misconfiguration + * breaks app boot, not runtime requests. + * + * A stale allow-list entry (route removed or migrated) also throws, so the + * list cannot quietly over-allow. + */ +export function validateBodyContracts( + app: INestApplication, + allowList: readonly string[] = BODY_CONTRACT_ALLOW_LIST, +): void { + const modules = app.get(ModulesContainer, { strict: false }); + const offenders: string[] = []; + const seen = new Set(); + + for (const module of modules.values()) { + for (const wrapper of module.controllers.values()) { + const ctor = wrapper.metatype as (abstract new (...args: never[]) => unknown) | undefined; + if (!ctor || typeof ctor !== 'function') continue; + + for (const name of Object.getOwnPropertyNames(ctor.prototype)) { + if (name === 'constructor') continue; + const handler = (ctor.prototype as Record)[name]; + if (typeof handler !== 'function') continue; + const methodEnum = Reflect.getMetadata(METHOD_METADATA, handler) as RequestMethod | undefined; + if (methodEnum === undefined || !MUTATION_METHODS.has(methodEnum)) continue; + + const args = (Reflect.getMetadata(ROUTE_ARGS_METADATA, ctor, name) ?? {}) as Record< + string, + { index: number } + >; + const paramtypes = (Reflect.getMetadata('design:paramtypes', ctor.prototype, name) ?? []) as unknown[]; + + for (const key of Object.keys(args)) { + // ROUTE_ARGS_METADATA keys are `${paramtype}:${index}` for built-in + // decorators; custom decorators (@CurrentUser) use a different shape + // and never parse to the BODY paramtype. + const paramtype = Number(key.split(':')[0]); + if (paramtype !== (RouteParamtypes.BODY as number)) continue; + + const route = `${ctor.name}.${name}`; + if (seen.has(route)) continue; + seen.add(route); + + const metatype = paramtypes[args[key].index]; + if (typeof metatype === 'function' && isZodDto(metatype)) continue; + offenders.push(route); + } + } + } + } + + const allowed = new Set(allowList); + const violations = offenders.filter((route) => !allowed.has(route)); + const stale = allowList.filter((route) => !offenders.includes(route)); + + const problems: string[] = []; + if (violations.length) { + problems.push( + `${violations.length} mutation handler(s) read @Body() without a createZodDto class ` + + `(add a .dto.ts wrapper over the @trek/shared schema — do NOT extend the allow-list): ` + + violations.sort().join(', '), + ); + } + if (stale.length) { + problems.push( + `stale body-contract allow-list entr${stale.length === 1 ? 'y' : 'ies'} ` + + `(route removed or now validated — delete from body-contract-allow-list.ts): ` + + stale.sort().join(', '), + ); + } + if (problems.length) { + throw new Error(`Unvalidated body contracts: ${problems.join('; ')}`); + } +} diff --git a/server/src/nest/common/zod-validation.pipe.ts b/server/src/nest/common/zod-validation.pipe.ts index bfb0207c5..c60abc298 100644 --- a/server/src/nest/common/zod-validation.pipe.ts +++ b/server/src/nest/common/zod-validation.pipe.ts @@ -1,28 +1,43 @@ -import { ArgumentMetadata, HttpException, Injectable, PipeTransform } from '@nestjs/common'; -import type { ZodType } from 'zod'; +import { HttpException } from '@nestjs/common'; +import { createZodValidationPipe } from 'nestjs-zod'; /** - * Validates an incoming @Body()/@Query() against a Zod schema (from @trek/shared) - * and returns the parsed, typed value. On failure it throws TREK's error envelope - * `{ error: string }` with status 400 — the same shape the legacy routes produce, - * so the client's error handling is unaffected. + * TREK's Zod validation pipe, built on nestjs-zod so it works in BOTH modes: * - * Usage: `@Body(new ZodValidationPipe(someSchema)) dto: Dto`. + * - **Global** (`APP_PIPE` in AppModule): validates every parameter whose + * metatype is a `createZodDto(...)` class (the `.dto.ts` wrappers + * over the @trek/shared schemas) and passes everything else through + * untouched — legacy untyped bodies keep their exact behavior until their + * domain migrates. The boot gate in `validate-body-contracts.ts` is what + * makes forgetting a DTO fail closed. + * - **Per-parameter** (`@Body(new ZodValidationPipe(schema))`): still accepts + * an explicit schema/DTO for one-off cases. + * + * On failure it throws TREK's error envelope `{ error: string }` with status + * 400 — the same shape the legacy routes produce (`field: message; ...`, + * root-level issues labeled `body`), so the client's error handling and the + * pinned test assertions are unaffected by the nestjs-zod migration. */ -@Injectable() -export class ZodValidationPipe implements PipeTransform { - // Public so the API-docs enricher can lift the schema into the OpenAPI - // document (#1412) — the pipe stays the single source of truth. - constructor(readonly schema: ZodType) {} - - transform(value: unknown, _metadata: ArgumentMetadata): unknown { - const result = this.schema.safeParse(value); - if (!result.success) { - const message = result.error.issues - .map((i) => `${i.path.join('.') || 'body'}: ${i.message}`) - .join('; '); - throw new HttpException({ error: message }, 400); - } - return result.data; - } +interface ZodIssueLike { + path: (string | number | symbol)[]; + message: string; } + +function isZodErrorLike(error: unknown): error is { issues: ZodIssueLike[] } { + return ( + typeof error === 'object' && error !== null && + Array.isArray((error as { issues?: unknown }).issues) + ); +} + +export const ZodValidationPipe = createZodValidationPipe({ + createValidationException: (error: unknown): Error => { + if (isZodErrorLike(error)) { + const message = error.issues + .map((i) => `${i.path.map(String).join('.') || 'body'}: ${i.message}`) + .join('; '); + return new HttpException({ error: message }, 400); + } + return new HttpException({ error: 'Validation failed' }, 400); + }, +}); diff --git a/server/src/nest/health/health.controller.ts b/server/src/nest/health/health.controller.ts index e4c907d39..63db5cd23 100644 --- a/server/src/nest/health/health.controller.ts +++ b/server/src/nest/health/health.controller.ts @@ -1,13 +1,16 @@ import { Body, Controller, Get, Post, UseGuards } from '@nestjs/common'; import { z } from 'zod'; +import { createZodDto } from 'nestjs-zod'; import type { User } from '../../types'; import { HealthService } from './health.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; -// Local demo schema (real domains import their schema from @trek/shared). +// Local demo schema (real domains import their schema from @trek/shared and +// wrap it in a colocated .dto.ts). The DTO class is what the global +// ZodValidationPipe (APP_PIPE) discovers by metatype. const echoSchema = z.object({ name: z.string().min(1) }); +class EchoDto extends createZodDto(echoSchema) {} /** * Foundation smoke endpoints for the co-hosted NestJS app. @@ -32,10 +35,10 @@ export class HealthController { return user; } - /** Validated: proves the Zod pipe (400 + { error } on failure) and body parsing. */ + /** Validated: proves the global Zod pipe (400 + { error } on failure) and body parsing. */ @Post('echo') @UseGuards(JwtAuthGuard) - echo(@Body(new ZodValidationPipe(echoSchema)) body: z.infer) { + echo(@Body() body: EchoDto) { return { youSent: body }; } } diff --git a/server/src/nest/integrations/airtrail-import.controller.ts b/server/src/nest/integrations/airtrail-import.controller.ts index 82f5dbda6..a0ded35e2 100644 --- a/server/src/nest/integrations/airtrail-import.controller.ts +++ b/server/src/nest/integrations/airtrail-import.controller.ts @@ -2,9 +2,9 @@ import { Body, Controller, Headers, HttpException, Param, Post, UseGuards } from import type { User } from '../../types'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; import { AirtrailAddonGuard } from './airtrail-addon.guard'; -import { airtrailImportSchema, type AirtrailImport, type AirtrailImportResult } from '@trek/shared'; +import { AirtrailImportDto } from './airtrail.dto'; +import type { AirtrailImportResult } from '@trek/shared'; import { verifyTripAccess } from '../../services/tripAccess'; import { checkPermission } from '../../services/permissions'; import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; @@ -29,7 +29,7 @@ export class AirtrailImportController { async importAirtrail( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body(new ZodValidationPipe(airtrailImportSchema)) body: AirtrailImport, + @Body() body: AirtrailImportDto, @Headers('x-socket-id') socketId?: string, ): Promise { this.requireEdit(tripId, user); diff --git a/server/src/nest/integrations/airtrail.controller.ts b/server/src/nest/integrations/airtrail.controller.ts index 1261abb64..b2a360ef9 100644 --- a/server/src/nest/integrations/airtrail.controller.ts +++ b/server/src/nest/integrations/airtrail.controller.ts @@ -3,10 +3,9 @@ import type { Request } from 'express'; import type { User } from '../../types'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; import { AirtrailAddonGuard } from './airtrail-addon.guard'; +import { AirtrailSettingsDto } from './airtrail.dto'; import { getClientIp } from '../../services/auditLog'; -import { airtrailSettingsSchema, type AirtrailSettings } from '@trek/shared'; import { getConnectionSettings, getConnectionStatus, @@ -35,7 +34,7 @@ export class AirtrailController { @Put('settings') async putSettings( @CurrentUser() user: User, - @Body(new ZodValidationPipe(airtrailSettingsSchema)) body: AirtrailSettings, + @Body() body: AirtrailSettingsDto, @Req() req: Request, ) { const result = await saveSettings( @@ -77,7 +76,7 @@ export class AirtrailController { @HttpCode(200) test( @CurrentUser() user: User, - @Body(new ZodValidationPipe(airtrailSettingsSchema)) body: AirtrailSettings, + @Body() body: AirtrailSettingsDto, ) { return testConnection(user.id, body.url, body.apiKey, !!body.allowInsecureTls); } diff --git a/server/src/nest/integrations/airtrail.dto.ts b/server/src/nest/integrations/airtrail.dto.ts new file mode 100644 index 000000000..1906d4177 --- /dev/null +++ b/server/src/nest/integrations/airtrail.dto.ts @@ -0,0 +1,10 @@ +/** + * Server-side createZodDto wrappers over the @trek/shared AirTrail contracts, + * so the global ZodValidationPipe (APP_PIPE) validates bodies by metatype — + * the shared Zod schemas stay the single source of truth. + */ +import { createZodDto } from 'nestjs-zod'; +import { airtrailImportSchema, airtrailSettingsSchema } from '@trek/shared'; + +export class AirtrailSettingsDto extends createZodDto(airtrailSettingsSchema) {} +export class AirtrailImportDto extends createZodDto(airtrailImportSchema) {} diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index 51b5d0017..054bc9732 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -25,8 +25,7 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { PLACE_IMAGE_UPLOAD } from '../common/place-image-upload'; import { placeImageUrl } from '../../services/placeImage'; import { isDemoEmail } from '../../services/demo'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; -import { placeRatingRequestSchema, type PlaceRatingRequest } from '@trek/shared'; +import { PlaceRatingDto } from './places.dto'; const STRING_LIMITS: Record = { name: 200, description: 2000, address: 500, notes: 2000 }; const UPLOAD = { storage: memoryStorage(), limits: { fileSize: 10 * 1024 * 1024 } }; @@ -307,7 +306,7 @@ export class PlacesController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body(new ZodValidationPipe(placeRatingRequestSchema)) body: PlaceRatingRequest, + @Body() body: PlaceRatingDto, @Headers('x-socket-id') socketId?: string, ) { // Deliberately no place_edit check: every trip member may cast their own diff --git a/server/src/nest/places/places.dto.ts b/server/src/nest/places/places.dto.ts new file mode 100644 index 000000000..eb18a47c9 --- /dev/null +++ b/server/src/nest/places/places.dto.ts @@ -0,0 +1,11 @@ +import { createZodDto } from 'nestjs-zod'; +import { placeRatingRequestSchema } from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared places contracts. + * Typing a controller parameter with one of these classes is what lets the + * global ZodValidationPipe (APP_PIPE) validate it by metatype — the Zod + * schemas in shared/ remain the single source of truth for the contract. + */ + +export class PlaceRatingDto extends createZodDto(placeRatingRequestSchema) {} diff --git a/server/src/nest/platform/api-docs.ts b/server/src/nest/platform/api-docs.ts index efa6031db..2a33dae41 100644 --- a/server/src/nest/platform/api-docs.ts +++ b/server/src/nest/platform/api-docs.ts @@ -1,7 +1,7 @@ import type { INestApplication } from '@nestjs/common'; import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; +import { cleanupOpenApiDoc } from 'nestjs-zod'; import { readEnv } from '../../app-config'; -import { attachZodBodySchemas } from '../common/api-zod'; /** * Swagger UI + OpenAPI spec for the REST API (#1412), gated behind @@ -29,10 +29,12 @@ export function setupApiDocs(app: INestApplication): void { .addCookieAuth('trek_session') .addSecurityRequirements('session') .build(); - const document = SwaggerModule.createDocument(app, config); - // Lift the Zod schemas the routes already validate with into the document — - // no double annotation, every ZodValidationPipe body is documented. - attachZodBodySchemas(app, document); + const rawDocument = SwaggerModule.createDocument(app, config); + // The createZodDto classes on @Body() params carry their @trek/shared Zod + // schemas into the document; cleanupOpenApiDoc (nestjs-zod) normalizes the + // zod-generated JSON Schema output — no double annotation, every validated + // body is documented. + const document = cleanupOpenApiDoc(rawDocument); SwaggerModule.setup('api/docs', app, document, { jsonDocumentUrl: 'api/docs-json', yamlDocumentUrl: 'api/docs-yaml', diff --git a/server/src/nest/todo/todo.controller.ts b/server/src/nest/todo/todo.controller.ts index c737abdb9..aab26af73 100644 --- a/server/src/nest/todo/todo.controller.ts +++ b/server/src/nest/todo/todo.controller.ts @@ -12,17 +12,20 @@ import { } from '@nestjs/common'; import type { User } from '../../types'; import { TodoService } from './todo.service'; +import { TodoCreateItemDto, TodoUpdateItemDto, TodoReorderDto, TodoCategoryAssigneesDto } from './todo.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; /** * /api/trips/:tripId/todo — trip-scoped task list. * - * Byte-identical to the legacy Express route (server/src/routes/todo.ts): every - * handler verifies trip access (404); mutations check the 'packing_edit' - * permission (403); create is 201, the rest 200; the bespoke 400/404 bodies are - * reproduced; mutations broadcast over WebSocket with the forwarded X-Socket-Id. - * /reorder is declared before /:id so it wins over the param. + * Every handler verifies trip access (404); mutations check the 'packing_edit' + * permission (403); create is 201, the rest 200; mutations broadcast over + * WebSocket with the forwarded X-Socket-Id. /reorder is declared before /:id so + * it wins over the param. Bodies validate against the @trek/shared todo schemas + * via the DTO classes in todo.dto.ts + the global ZodValidationPipe (400 with + * the standard `{ error }` envelope on mismatch — this replaced the legacy + * bespoke 'Item name is required' check). */ @Controller('api/trips/:tripId/todo') @UseGuards(JwtAuthGuard) @@ -53,14 +56,11 @@ export class TodoController { create( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: { name?: string; category?: string; due_date?: string; description?: string; assigned_user_id?: number; priority?: number }, + @Body() body: TodoCreateItemDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.name) { - throw new HttpException({ error: 'Item name is required' }, 400); - } const { name, category, due_date, description, assigned_user_id, priority } = body; const item = this.todo.createItem(tripId, { name, category, due_date, description, assigned_user_id, priority }); this.todo.broadcast(tripId, 'todo:created', { item }, socketId); @@ -71,11 +71,11 @@ export class TodoController { reorder( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('orderedIds') orderedIds: number[], + @Body() body: TodoReorderDto, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - this.todo.reorderItems(tripId, orderedIds); + this.todo.reorderItems(tripId, body.orderedIds); return { success: true }; } @@ -84,13 +84,21 @@ export class TodoController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body() body: Record, + @Body() body: TodoUpdateItemDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const { name, checked, category, due_date, description, assigned_user_id, priority } = body as Record; - const updated = this.todo.updateItem(tripId, id, { name, checked, category, due_date, description, assigned_user_id, priority }, Object.keys(body)); + const { name, checked, category, due_date, description, assigned_user_id, priority } = body; + // bodyKeys carries which keys the request actually provided (the null-clear + // protocol); the parsed body only ever holds known schema keys. + const updated = this.todo.updateItem( + tripId, + id, + // checked arrives as boolean or legacy 0/1 — normalize to the 0/1 the SQL binds. + { name, checked: checked === undefined ? undefined : checked ? 1 : 0, category, due_date, description, assigned_user_id, priority }, + Object.keys(body), + ); if (!updated) { throw new HttpException({ error: 'Item not found' }, 404); } @@ -125,13 +133,13 @@ export class TodoController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('categoryName') categoryName: string, - @Body('user_ids') userIds: number[], + @Body() body: TodoCategoryAssigneesDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); const category = decodeURIComponent(categoryName); - const rows = this.todo.updateCategoryAssignees(tripId, category, userIds); + const rows = this.todo.updateCategoryAssignees(tripId, category, body.user_ids); this.todo.broadcast(tripId, 'todo:assignees', { category, assignees: rows }, socketId); return { assignees: rows }; } diff --git a/server/src/nest/todo/todo.dto.ts b/server/src/nest/todo/todo.dto.ts new file mode 100644 index 000000000..48556da99 --- /dev/null +++ b/server/src/nest/todo/todo.dto.ts @@ -0,0 +1,18 @@ +import { createZodDto } from 'nestjs-zod'; +import { + todoCreateItemRequestSchema, + todoUpdateItemRequestSchema, + todoReorderRequestSchema, + todoCategoryAssigneesRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared todo 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. + */ +export class TodoCreateItemDto extends createZodDto(todoCreateItemRequestSchema) {} +export class TodoUpdateItemDto extends createZodDto(todoUpdateItemRequestSchema) {} +export class TodoReorderDto extends createZodDto(todoReorderRequestSchema) {} +export class TodoCategoryAssigneesDto extends createZodDto(todoCategoryAssigneesRequestSchema) {} diff --git a/server/src/nest/vacay/vacay.controller.ts b/server/src/nest/vacay/vacay.controller.ts index e8430d918..70d901d61 100644 --- a/server/src/nest/vacay/vacay.controller.ts +++ b/server/src/nest/vacay/vacay.controller.ts @@ -12,17 +12,11 @@ import { Query, UseGuards, } from '@nestjs/common'; -import { - vacayShareUpdateRequestSchema, - vacayYearSettingsRequestSchema, - type VacayShareUpdateRequest, - type VacayYearSettingsRequest, -} from '@trek/shared'; import type { User } from '../../types'; import { VacayService } from './vacay.service'; +import { VacayShareUpdateDto, VacayYearSettingsDto } from './vacay.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { ZodValidationPipe } from '../common/zod-validation.pipe'; /** * /api/addons/vacay — shared vacation-day planner. @@ -188,7 +182,7 @@ export class VacayController { @Put('year-settings') updateYearSettings( @CurrentUser() user: User, - @Body(new ZodValidationPipe(vacayYearSettingsRequestSchema)) body: VacayYearSettingsRequest, + @Body() body: VacayYearSettingsDto, ) { return { settings: this.vacay.updateYearSettings(user.id, body) }; } @@ -294,7 +288,7 @@ export class VacayController { updateShare( @CurrentUser() user: User, @Param('id') idParam: string, - @Body(new ZodValidationPipe(vacayShareUpdateRequestSchema)) body: VacayShareUpdateRequest, + @Body() body: VacayShareUpdateDto, @Headers('x-socket-id') socketId?: string, ) { if (!this.vacay.setShareHidden(parseInt(idParam), user.id, body.hidden, socketId)) { diff --git a/server/src/nest/vacay/vacay.dto.ts b/server/src/nest/vacay/vacay.dto.ts new file mode 100644 index 000000000..3d0a6cc13 --- /dev/null +++ b/server/src/nest/vacay/vacay.dto.ts @@ -0,0 +1,10 @@ +/** + * Server-side createZodDto wrappers over the @trek/shared vacay contracts, so + * the global ZodValidationPipe (APP_PIPE) validates bodies by metatype — the + * shared Zod schemas stay the single source of truth. + */ +import { createZodDto } from 'nestjs-zod'; +import { vacayShareUpdateRequestSchema, vacayYearSettingsRequestSchema } from '@trek/shared'; + +export class VacayYearSettingsDto extends createZodDto(vacayYearSettingsRequestSchema) {} +export class VacayShareUpdateDto extends createZodDto(vacayShareUpdateRequestSchema) {} diff --git a/server/tests/e2e/collections.e2e.test.ts b/server/tests/e2e/collections.e2e.test.ts index e336a040b..124ec7055 100644 --- a/server/tests/e2e/collections.e2e.test.ts +++ b/server/tests/e2e/collections.e2e.test.ts @@ -41,6 +41,7 @@ import { runMigrations } from '../../src/db/migrations'; import { createUser, createTrip, createCategory } from '../helpers/factories'; import { CollectionsModule } from '../../src/nest/collections/collections.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; describe('Collections e2e (real auth guard + real service + temp SQLite)', () => { let server: Server; @@ -54,6 +55,9 @@ describe('Collections e2e (real auth guard + real service + temp SQLite)', () => const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): DTO-typed bodies validate + // by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } diff --git a/server/tests/e2e/places.e2e.test.ts b/server/tests/e2e/places.e2e.test.ts index 4e66894b6..2ce10f28a 100644 --- a/server/tests/e2e/places.e2e.test.ts +++ b/server/tests/e2e/places.e2e.test.ts @@ -43,6 +43,7 @@ vi.mock('../../src/services/placeService', () => pl); import { PlacesModule } from '../../src/nest/places/places.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; describe('Places e2e (real auth guard + temp SQLite)', () => { let server: Server; @@ -53,6 +54,9 @@ describe('Places e2e (real auth guard + temp SQLite)', () => { const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): DTO-typed bodies validate + // by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } diff --git a/server/tests/e2e/todo.e2e.test.ts b/server/tests/e2e/todo.e2e.test.ts index 025927611..cac521725 100644 --- a/server/tests/e2e/todo.e2e.test.ts +++ b/server/tests/e2e/todo.e2e.test.ts @@ -69,6 +69,7 @@ vi.mock('../../src/services/permissions', () => ({ checkPermission })); import { TodoModule } from '../../src/nest/todo/todo.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; function insertItem(tripId: number, name: string, extra: Partial<{ sort_order: number; due_date: string }> = {}): number { const res = db @@ -87,6 +88,9 @@ describe('To-do e2e (real auth guard + real SQL over temp SQLite)', () => { const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): the DTO-typed bodies + // validate by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } @@ -138,10 +142,23 @@ describe('To-do e2e (real auth guard + real SQL over temp SQLite)', () => { expect(db.prepare('SELECT name FROM todo_items WHERE id = ?').get(res.body.item.id)).toEqual({ name: 'Book hotel' }); }); - it('400 on create without a name', async () => { + it('400 from the Zod pipe on create without a name', async () => { const res = await request(server).post(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(1)).send({}); expect(res.status).toBe(400); - expect(res.body).toEqual({ error: 'Item name is required' }); + expect(res.body.error).toContain('name'); + }); + + it('400 from the Zod pipe on reorder without orderedIds', async () => { + const res = await request(server).put(`/api/trips/${tripId}/todo/reorder`).set('Cookie', sessionCookie(1)).send({}); + expect(res.status).toBe(400); + expect(res.body.error).toContain('orderedIds'); + }); + + it('accepts the legacy numeric checked form through the pipe', async () => { + const id = insertItem(tripId, 'Toggle me'); + const res = await request(server).put(`/api/trips/${tripId}/todo/${id}`).set('Cookie', sessionCookie(1)).send({ checked: 1 }); + expect(res.status).toBe(200); + expect(res.body.item.checked).toBe(1); }); it('403 on create without permission, writing nothing', async () => { diff --git a/server/tests/e2e/vacay.e2e.test.ts b/server/tests/e2e/vacay.e2e.test.ts index 5cc81dcef..0414a1d40 100644 --- a/server/tests/e2e/vacay.e2e.test.ts +++ b/server/tests/e2e/vacay.e2e.test.ts @@ -38,6 +38,7 @@ vi.mock('../../src/services/vacayService', () => svc); import { VacayModule } from '../../src/nest/vacay/vacay.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; describe('Vacay e2e (real auth guard + temp SQLite)', () => { let server: Server; @@ -48,6 +49,9 @@ describe('Vacay e2e (real auth guard + temp SQLite)', () => { const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): DTO-typed bodies validate + // by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } diff --git a/server/tests/integration/api-docs.test.ts b/server/tests/integration/api-docs.test.ts index bb03dbfdb..795f66294 100644 --- a/server/tests/integration/api-docs.test.ts +++ b/server/tests/integration/api-docs.test.ts @@ -105,11 +105,14 @@ describe('API-DOCS (#1412) — flag-gated OpenAPI surface', () => { it('DOCS-004 — Zod request bodies are lifted into the spec (no double annotation)', async () => { const res = await request(instance).get('/api/docs-json'); - // collections create validates with collectionCreateRequestSchema via - // ZodValidationPipe — the enricher must surface its object schema. + // collections create validates with CollectionCreateDto (createZodDto over + // collectionCreateRequestSchema) via the global ZodValidationPipe — the DTO + // metatype must surface as a $ref to a component object schema. const create = res.body.paths['/api/addons/collections']?.post; expect(create).toBeDefined(); - const schema = create.requestBody?.content?.['application/json']?.schema; + const ref = create.requestBody?.content?.['application/json']?.schema?.$ref as string | undefined; + expect(ref).toMatch(/^#\/components\/schemas\//); + const schema = res.body.components.schemas[ref!.split('/').pop()!]; expect(schema?.type).toBe('object'); expect(schema?.properties?.name).toBeDefined(); }); diff --git a/server/tests/unit/nest/api-zod.test.ts b/server/tests/unit/nest/api-zod.test.ts deleted file mode 100644 index 554c8e53b..000000000 --- a/server/tests/unit/nest/api-zod.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -/** - * zodToOpenApi (#1412): Zod → OpenAPI 3.0 conversion + the degrade path. - * The end-to-end enricher behaviour is covered by tests/integration/api-docs. - */ -import { describe, it, expect } from 'vitest'; -import { z } from 'zod'; -import { zodToOpenApi } from '../../../src/nest/common/api-zod'; - -describe('zodToOpenApi', () => { - it('converts an object schema with required/optional split', () => { - const out = zodToOpenApi(z.object({ name: z.string(), count: z.number().optional() })) as { - type: string; properties: Record; required: string[]; - }; - expect(out.type).toBe('object'); - expect(out.properties.name).toEqual({ type: 'string' }); - expect(out.properties.count).toEqual({ type: 'number' }); - expect(out.required).toEqual(['name']); - }); - - it('survives transform-heavy schemas via unrepresentable: any', () => { - const out = zodToOpenApi(z.object({ when: z.string().transform((s) => new Date(s)) })); - expect((out as { type: string }).type).toBe('object'); - }); - - it('degrades to a bare object instead of throwing on a broken schema', () => { - expect(zodToOpenApi({} as never)).toEqual({ type: 'object' }); - }); -}); diff --git a/server/tests/unit/nest/todo.controller.test.ts b/server/tests/unit/nest/todo.controller.test.ts index 838d5be30..a79ef5bc2 100644 --- a/server/tests/unit/nest/todo.controller.test.ts +++ b/server/tests/unit/nest/todo.controller.test.ts @@ -48,11 +48,10 @@ describe('TodoController (parity with the legacy /api/trips/:tripId/todo route)' }); }); - it('400 when name missing', () => { - expect(thrown(() => new TodoController(makeService()).create(user, '5', {}))).toEqual({ - status: 400, body: { error: 'Item name is required' }, - }); - }); + // The missing-name 400 moved from a bespoke controller check into the + // ZodValidationPipe (todoCreateItemRequestSchema) — direct method calls + // bypass parameter pipes, so that path is covered by the e2e suite and + // the schema spec in @trek/shared. it('creates and broadcasts', () => { const createItem = vi.fn().mockReturnValue({ id: 9, name: 'Pack' }); @@ -71,14 +70,21 @@ describe('TodoController (parity with the legacy /api/trips/:tripId/todo route)' }); }); - it('updates, forwards changed keys, broadcasts', () => { + it('updates, forwards changed keys, normalizes checked to 0/1, broadcasts', () => { const updateItem = vi.fn().mockReturnValue({ id: 9 }); const broadcast = vi.fn(); const svc = makeService({ updateItem, broadcast } as Partial); new TodoController(svc).update(user, '5', '9', { checked: true }, 'sock'); - expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ checked: true }), ['checked']); + expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ checked: 1 }), ['checked']); expect(broadcast).toHaveBeenCalledWith('5', 'todo:updated', { item: { id: 9 } }, 'sock'); }); + + it('normalizes checked false to 0', () => { + const updateItem = vi.fn().mockReturnValue({ id: 9 }); + const svc = makeService({ updateItem, broadcast: vi.fn() } as Partial); + new TodoController(svc).update(user, '5', '9', { checked: false }); + expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ checked: 0 }), ['checked']); + }); }); describe('DELETE /:id', () => { @@ -101,7 +107,7 @@ describe('TodoController (parity with the legacy /api/trips/:tripId/todo route)' it('PUT /reorder succeeds with permission', () => { const reorderItems = vi.fn(); const svc = makeService({ reorderItems } as Partial); - expect(new TodoController(svc).reorder(user, '5', [3, 1, 2])).toEqual({ success: true }); + expect(new TodoController(svc).reorder(user, '5', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); expect(reorderItems).toHaveBeenCalledWith('5', [3, 1, 2]); }); @@ -115,7 +121,7 @@ describe('TodoController (parity with the legacy /api/trips/:tripId/todo route)' const updateCategoryAssignees = vi.fn().mockReturnValue([{ user_id: 2 }]); const broadcast = vi.fn(); const svc = makeService({ updateCategoryAssignees, broadcast } as Partial); - new TodoController(svc).updateCategoryAssignees(user, '5', 'To%20Buy', [2], 'sock'); + new TodoController(svc).updateCategoryAssignees(user, '5', 'To%20Buy', { user_ids: [2] }, 'sock'); expect(updateCategoryAssignees).toHaveBeenCalledWith('5', 'To Buy', [2]); expect(broadcast).toHaveBeenCalledWith('5', 'todo:assignees', { category: 'To Buy', assignees: [{ user_id: 2 }] }, 'sock'); }); diff --git a/server/tests/unit/nest/validate-body-contracts.test.ts b/server/tests/unit/nest/validate-body-contracts.test.ts new file mode 100644 index 000000000..96e4a7829 --- /dev/null +++ b/server/tests/unit/nest/validate-body-contracts.test.ts @@ -0,0 +1,100 @@ +/** + * Unit tests for the fail-closed body-contract boot gate + * (src/nest/common/validate-body-contracts.ts): a POST/PUT/PATCH @Body() + * without a createZodDto metatype must refuse boot unless allow-listed, and + * 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 { Test } from '@nestjs/testing'; +import type { INestApplication } from '@nestjs/common'; +import { createZodDto } from 'nestjs-zod'; +import { z } from 'zod'; +import { validateBodyContracts } from '../../../src/nest/common/validate-body-contracts'; + +class GoodDto extends createZodDto(z.object({ name: z.string() })) {} + +@Controller('gate-test/good') +class GoodController { + @Post() + create(@Body() body: GoodDto) { + return body; + } + + // Non-mutation and body-less handlers are out of scope for the gate. + @Get() + list() { + return []; + } + + @Put('no-body/:id') + touch() { + return { ok: true }; + } +} + +@Controller('gate-test/bad') +class BadController { + @Put(':id') + update(@Body() body: Record) { + return body; + } +} + +@Controller('gate-test/subfield') +class SubfieldController { + @Post() + reorder(@Body('orderedIds') orderedIds: number[]) { + return orderedIds; + } +} + +async function buildApp(controllers: unknown[]): Promise { + const moduleRef = await Test.createTestingModule({ + controllers: controllers as never[], + }).compile(); + return moduleRef.createNestApplication(); +} + +describe('validateBodyContracts (fail-closed boot gate)', () => { + let goodApp: INestApplication; + let badApp: INestApplication; + let subfieldApp: INestApplication; + + beforeAll(async () => { + goodApp = await buildApp([GoodController]); + badApp = await buildApp([GoodController, BadController]); + subfieldApp = await buildApp([SubfieldController]); + }); + + afterAll(async () => { + await Promise.all([goodApp.close(), badApp.close(), subfieldApp.close()]); + }); + + it('GATE-001 — passes when every mutation body is a createZodDto class', () => { + expect(() => validateBodyContracts(goodApp, [])).not.toThrow(); + }); + + it('GATE-002 — throws naming the handler whose @Body() lacks a DTO', () => { + expect(() => validateBodyContracts(badApp, [])).toThrow(/BadController\.update/); + }); + + it('GATE-003 — @Body(\'field\') sub-reads are unvalidated and flagged too', () => { + expect(() => validateBodyContracts(subfieldApp, [])).toThrow(/SubfieldController\.reorder/); + }); + + it('GATE-004 — an allow-listed legacy route passes', () => { + expect(() => validateBodyContracts(badApp, ['BadController.update'])).not.toThrow(); + }); + + it('GATE-005 — a stale allow-list entry throws (ratchet enforcement)', () => { + expect(() => validateBodyContracts(goodApp, ['GhostController.gone'])).toThrow( + /stale body-contract allow-list entry.*GhostController\.gone/, + ); + }); + + it('GATE-006 — the real allow-list is sorted and duplicate-free (reviewable ratchet)', async () => { + const { BODY_CONTRACT_ALLOW_LIST } = await import('../../../src/nest/common/body-contract-allow-list'); + expect(BODY_CONTRACT_ALLOW_LIST).toEqual([...new Set(BODY_CONTRACT_ALLOW_LIST)].sort()); + }); +});