From 6e7c2817a3d8dac9847d1dade38165e0f9e8f52e Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 4 Sep 2025 14:22:01 -0400 Subject: [PATCH] fix: asset upload metadata validation (#21594) --- .../asset-media.controller.spec.ts | 89 +++++++++++++++---- server/src/dtos/asset-media.dto.ts | 15 +++- server/test/utils.ts | 26 +++++- 3 files changed, 106 insertions(+), 24 deletions(-) diff --git a/server/src/controllers/asset-media.controller.spec.ts b/server/src/controllers/asset-media.controller.spec.ts index 67bdeff222..eb594fbe47 100644 --- a/server/src/controllers/asset-media.controller.spec.ts +++ b/server/src/controllers/asset-media.controller.spec.ts @@ -1,4 +1,6 @@ import { AssetMediaController } from 'src/controllers/asset-media.controller'; +import { AssetMediaStatus } from 'src/dtos/asset-media-response.dto'; +import { AssetMetadataKey } from 'src/enum'; import { LoggingRepository } from 'src/repositories/logging.repository'; import { AssetMediaService } from 'src/services/asset-media.service'; import request from 'supertest'; @@ -11,7 +13,7 @@ const makeUploadDto = (options?: { omit: string }): Record => { deviceId: 'TEST', fileCreatedAt: new Date().toISOString(), fileModifiedAt: new Date().toISOString(), - isFavorite: 'testing', + isFavorite: 'false', duration: '0:00:00.000000', }; @@ -27,16 +29,20 @@ describe(AssetMediaController.name, () => { let ctx: ControllerContext; const assetData = Buffer.from('123'); const filename = 'example.png'; + const service = mockBaseService(AssetMediaService); beforeAll(async () => { ctx = await controllerSetup(AssetMediaController, [ { provide: LoggingRepository, useValue: automock(LoggingRepository, { strict: false }) }, - { provide: AssetMediaService, useValue: mockBaseService(AssetMediaService) }, + { provide: AssetMediaService, useValue: service }, ]); return () => ctx.close(); }); beforeEach(() => { + service.resetAllMocks(); + service.uploadAsset.mockResolvedValue({ status: AssetMediaStatus.DUPLICATE, id: factory.uuid() }); + ctx.reset(); }); @@ -46,13 +52,61 @@ describe(AssetMediaController.name, () => { expect(ctx.authenticate).toHaveBeenCalled(); }); + it('should accept metadata', async () => { + const mobileMetadata = { key: AssetMetadataKey.MobileApp, value: { iCloudId: '123' } }; + const { status } = await request(ctx.getHttpServer()) + .post('/assets') + .attach('assetData', assetData, filename) + .field({ + ...makeUploadDto(), + metadata: JSON.stringify([mobileMetadata]), + }); + + expect(service.uploadAsset).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ metadata: [mobileMetadata] }), + expect.objectContaining({ originalName: 'example.png' }), + undefined, + ); + + expect(status).toBe(200); + }); + + it('should handle invalid metadata json', async () => { + const { status, body } = await request(ctx.getHttpServer()) + .post('/assets') + .attach('assetData', assetData, filename) + .field({ + ...makeUploadDto(), + metadata: 'not-a-string-string', + }); + + expect(status).toBe(400); + expect(body).toEqual(factory.responses.badRequest(['metadata must be valid JSON'])); + }); + + it('should validate iCloudId is a string', async () => { + const { status, body } = await request(ctx.getHttpServer()) + .post('/assets') + .attach('assetData', assetData, filename) + .field({ + ...makeUploadDto(), + metadata: JSON.stringify([{ key: AssetMetadataKey.MobileApp, value: { iCloudId: 123 } }]), + }); + + expect(status).toBe(400); + expect(body).toEqual(factory.responses.badRequest(['metadata.0.value.iCloudId must be a string'])); + }); + it('should require `deviceAssetId`', async () => { const { status, body } = await request(ctx.getHttpServer()) .post('/assets') .attach('assetData', assetData, filename) .field({ ...makeUploadDto({ omit: 'deviceAssetId' }) }); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual( + factory.responses.badRequest(['deviceAssetId must be a string', 'deviceAssetId should not be empty']), + ); }); it('should require `deviceId`', async () => { @@ -61,7 +115,7 @@ describe(AssetMediaController.name, () => { .attach('assetData', assetData, filename) .field({ ...makeUploadDto({ omit: 'deviceId' }) }); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual(factory.responses.badRequest(['deviceId must be a string', 'deviceId should not be empty'])); }); it('should require `fileCreatedAt`', async () => { @@ -70,25 +124,20 @@ describe(AssetMediaController.name, () => { .attach('assetData', assetData, filename) .field({ ...makeUploadDto({ omit: 'fileCreatedAt' }) }); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual( + factory.responses.badRequest(['fileCreatedAt must be a Date instance', 'fileCreatedAt should not be empty']), + ); }); it('should require `fileModifiedAt`', async () => { const { status, body } = await request(ctx.getHttpServer()) .post('/assets') .attach('assetData', assetData, filename) - .field({ ...makeUploadDto({ omit: 'fileModifiedAt' }) }); + .field(makeUploadDto({ omit: 'fileModifiedAt' })); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); - }); - - it('should require `duration`', async () => { - const { status, body } = await request(ctx.getHttpServer()) - .post('/assets') - .attach('assetData', assetData, filename) - .field({ ...makeUploadDto({ omit: 'duration' }) }); - expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual( + factory.responses.badRequest(['fileModifiedAt must be a Date instance', 'fileModifiedAt should not be empty']), + ); }); it('should throw if `isFavorite` is not a boolean', async () => { @@ -97,16 +146,18 @@ describe(AssetMediaController.name, () => { .attach('assetData', assetData, filename) .field({ ...makeUploadDto(), isFavorite: 'not-a-boolean' }); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual(factory.responses.badRequest(['isFavorite must be a boolean value'])); }); it('should throw if `visibility` is not an enum', async () => { const { status, body } = await request(ctx.getHttpServer()) .post('/assets') .attach('assetData', assetData, filename) - .field({ ...makeUploadDto(), visibility: 'not-a-boolean' }); + .field({ ...makeUploadDto(), visibility: 'not-an-option' }); expect(status).toBe(400); - expect(body).toEqual(factory.responses.badRequest()); + expect(body).toEqual( + factory.responses.badRequest([expect.stringContaining('visibility must be one of the following values:')]), + ); }); // TODO figure out how to deal with `sendFile` diff --git a/server/src/dtos/asset-media.dto.ts b/server/src/dtos/asset-media.dto.ts index 25395000cd..755069d827 100644 --- a/server/src/dtos/asset-media.dto.ts +++ b/server/src/dtos/asset-media.dto.ts @@ -1,5 +1,6 @@ +import { BadRequestException } from '@nestjs/common'; import { ApiProperty } from '@nestjs/swagger'; -import { Type } from 'class-transformer'; +import { plainToInstance, Transform, Type } from 'class-transformer'; import { ArrayNotEmpty, IsArray, IsNotEmpty, IsString, ValidateNested } from 'class-validator'; import { AssetMetadataUpsertItemDto } from 'src/dtos/asset.dto'; import { AssetVisibility } from 'src/enum'; @@ -65,10 +66,18 @@ export class AssetMediaCreateDto extends AssetMediaBase { @ValidateUUID({ optional: true }) livePhotoVideoId?: string; + @Transform(({ value }) => { + try { + const json = JSON.parse(value); + const items = Array.isArray(json) ? json : [json]; + return items.map((item) => plainToInstance(AssetMetadataUpsertItemDto, item)); + } catch { + throw new BadRequestException(['metadata must be valid JSON']); + } + }) @Optional() - @IsArray() @ValidateNested({ each: true }) - @Type(() => AssetMetadataUpsertItemDto) + @IsArray() metadata!: AssetMetadataUpsertItemDto[]; @ApiProperty({ type: 'string', format: 'binary', required: false }) diff --git a/server/test/utils.ts b/server/test/utils.ts index 95f9741d7c..c23341d64c 100644 --- a/server/test/utils.ts +++ b/server/test/utils.ts @@ -1,12 +1,16 @@ -import { CallHandler, Provider, ValidationPipe } from '@nestjs/common'; +import { CallHandler, ExecutionContext, Provider, ValidationPipe } from '@nestjs/common'; import { APP_GUARD, APP_PIPE } from '@nestjs/core'; +import { transformException } from '@nestjs/platform-express/multer/multer/multer.utils'; import { Test } from '@nestjs/testing'; import { ClassConstructor } from 'class-transformer'; +import { NextFunction } from 'express'; import { Kysely } from 'kysely'; +import multer from 'multer'; import { ChildProcessWithoutNullStreams } from 'node:child_process'; import { Readable, Writable } from 'node:stream'; import { PNG } from 'pngjs'; import postgres from 'postgres'; +import { UploadFieldName } from 'src/dtos/asset-media.dto'; import { AssetUploadInterceptor } from 'src/middleware/asset-upload.interceptor'; import { AuthGuard } from 'src/middleware/auth.guard'; import { FileUploadInterceptor } from 'src/middleware/file-upload.interceptor'; @@ -82,6 +86,24 @@ export type ControllerContext = { export const controllerSetup = async (controller: ClassConstructor, providers: Provider[]) => { const noopInterceptor = { intercept: (ctx: never, next: CallHandler) => next.handle() }; + const upload = multer({ storage: multer.memoryStorage() }); + const memoryFileInterceptor = { + intercept: async (ctx: ExecutionContext, next: CallHandler) => { + const context = ctx.switchToHttp(); + const handler = upload.fields([ + { name: UploadFieldName.ASSET_DATA, maxCount: 1 }, + { name: UploadFieldName.SIDECAR_DATA, maxCount: 1 }, + ]); + + await new Promise((resolve, reject) => { + const next: NextFunction = (error) => (error ? reject(transformException(error)) : resolve()); + const maybePromise = handler(context.getRequest(), context.getResponse(), next); + Promise.resolve(maybePromise).catch((error) => reject(error)); + }); + + return next.handle(); + }, + }; const moduleRef = await Test.createTestingModule({ controllers: [controller], providers: [ @@ -93,7 +115,7 @@ export const controllerSetup = async (controller: ClassConstructor, pro ], }) .overrideInterceptor(FileUploadInterceptor) - .useValue(noopInterceptor) + .useValue(memoryFileInterceptor) .overrideInterceptor(AssetUploadInterceptor) .useValue(noopInterceptor) .compile();