From 29407814ccb0c4ee9d9b008aec397dfd98e86469 Mon Sep 17 00:00:00 2001 From: nobswebdev Date: Tue, 27 Oct 2026 21:07:01 +0100 Subject: [PATCH] Add UploadCleanupService for cron purge and delete uploads. Move token delete and storage purge off UploadService; enforce expiry and max views on GET without purging until the scheduled cleanup job runs. --- backend/src/modules/upload/UploadModule.ts | 3 +- .../upload/controllers/UploadController.ts | 8 +- .../services/UploadCleanupService.spec.ts | 198 ++++++++++++++++++ .../upload/services/UploadCleanupService.ts | 64 ++++++ .../upload/services/UploadService.spec.ts | 79 +------ .../modules/upload/services/UploadService.ts | 51 +---- backend/src/utils/hashDeleteToken.ts | 4 + 7 files changed, 286 insertions(+), 121 deletions(-) create mode 100644 backend/src/modules/upload/services/UploadCleanupService.spec.ts create mode 100644 backend/src/modules/upload/services/UploadCleanupService.ts create mode 100644 backend/src/utils/hashDeleteToken.ts diff --git a/backend/src/modules/upload/UploadModule.ts b/backend/src/modules/upload/UploadModule.ts index 0ba3a09..fea22ef 100644 --- a/backend/src/modules/upload/UploadModule.ts +++ b/backend/src/modules/upload/UploadModule.ts @@ -6,11 +6,12 @@ import { UploadController } from './controllers/UploadController'; import { Upload } from './entities/Upload'; import { UploadHotlink } from './entities/UploadHotlink'; import { UploadZk } from './entities/UploadZk'; +import { UploadCleanupService } from './services/UploadCleanupService'; import { UploadService } from './services/UploadService'; @Module({ imports: [TypeOrmModule.forFeature([Upload, UploadZk, UploadHotlink]), StorageModule], controllers: [UploadController, HotlinkImageController], - providers: [UploadService] + providers: [UploadService, UploadCleanupService] }) export class UploadModule {} diff --git a/backend/src/modules/upload/controllers/UploadController.ts b/backend/src/modules/upload/controllers/UploadController.ts index b4124bf..3c85f62 100644 --- a/backend/src/modules/upload/controllers/UploadController.ts +++ b/backend/src/modules/upload/controllers/UploadController.ts @@ -4,11 +4,15 @@ import { throttleProfiles } from '../../../config/throttleProfiles'; import { CreateHotlinkUploadDto } from '../dto/CreateHotlinkUploadDto'; import { CreateZkUploadDto } from '../dto/CreateZkUploadDto'; import { DeleteUploadQueryDto } from '../dto/DeleteUploadQueryDto'; +import { UploadCleanupService } from '../services/UploadCleanupService'; import { UploadService } from '../services/UploadService'; @Controller('uploads') export class UploadController { - constructor(private readonly uploadService: UploadService) {} + constructor( + private readonly uploadService: UploadService, + private readonly uploadCleanupService: UploadCleanupService + ) {} @Post('zk') @Throttle(throttleProfiles.upload) @@ -30,6 +34,6 @@ export class UploadController { @Delete(':id') @HttpCode(HttpStatus.NO_CONTENT) deleteUpload(@Param('id', ParseUUIDPipe) id: string, @Query() { delete_token }: DeleteUploadQueryDto) { - return this.uploadService.deleteUpload(id, delete_token); + return this.uploadCleanupService.deleteUpload(id, delete_token); } } diff --git a/backend/src/modules/upload/services/UploadCleanupService.spec.ts b/backend/src/modules/upload/services/UploadCleanupService.spec.ts new file mode 100644 index 0000000..8f79ff3 --- /dev/null +++ b/backend/src/modules/upload/services/UploadCleanupService.spec.ts @@ -0,0 +1,198 @@ +import { NotFoundException } from '@nestjs/common'; +import type { DataSource, EntityManager, Repository } from 'typeorm'; +import { hashDeleteToken } from '../../../utils/hashDeleteToken'; +import type { Upload } from '../entities/Upload'; +import { StorageService } from '../../storage/services/StorageService'; +import { UploadCleanupService } from './UploadCleanupService'; + +jest.mock('@nestjs/typeorm', () => ({ + InjectDataSource: () => (): void => undefined, + InjectRepository: () => (): void => undefined +})); + +const fixedId = '11111111-1111-4111-8111-111111111111'; +const fixedDeleteToken = 'fixed-delete-token'; + +describe('UploadCleanupService', () => { + let service: UploadCleanupService; + let storageService: jest.Mocked>; + let dataSource: jest.Mocked>; + let uploadRepository: { + createQueryBuilder: jest.Mock; + findOne: jest.Mock; + remove: jest.Mock; + }; + let getMany: jest.Mock; + let take: jest.Mock; + let transaction: jest.Mock; + + beforeEach(() => { + getMany = jest.fn().mockResolvedValue([]); + take = jest.fn().mockReturnValue({ getMany }); + const orWhere = jest.fn().mockReturnValue({ take }); + const where = jest.fn().mockReturnValue({ orWhere }); + + uploadRepository = { + createQueryBuilder: jest.fn().mockReturnValue({ where }), + findOne: jest.fn(), + remove: jest.fn().mockResolvedValue(undefined) + }; + + transaction = jest.fn(async (runInTransaction: (manager: EntityManager) => Promise) => { + const manager = { + getRepository: jest.fn(() => uploadRepository) + } as unknown as EntityManager; + + return runInTransaction(manager); + }); + + dataSource = { transaction }; + + storageService = { + unlink: jest.fn().mockResolvedValue(undefined) + }; + + service = new UploadCleanupService( + dataSource as unknown as DataSource, + storageService as unknown as StorageService, + uploadRepository as unknown as Repository + ); + }); + + describe('purgeUploads', () => { + it('purges each eligible upload up to the batch size', async () => { + const expired = { id: fixedId, storagePath: fixedId }; + const viewCapped = { + id: '22222222-2222-4222-8222-222222222222', + storagePath: '22222222-2222-4222-8222-222222222222' + }; + + getMany.mockResolvedValue([expired, viewCapped]); + + await expect(service.purgeUploads()).resolves.toBeUndefined(); + + expect(uploadRepository.createQueryBuilder).toHaveBeenCalledWith('upload'); + expect(take).toHaveBeenCalledWith(100); + expect(storageService.unlink).toHaveBeenCalledWith(fixedId); + expect(storageService.unlink).toHaveBeenCalledWith(viewCapped.storagePath); + expect(uploadRepository.remove).toHaveBeenCalledWith(expired); + expect(uploadRepository.remove).toHaveBeenCalledWith(viewCapped); + }); + + it('does nothing when no uploads are eligible', async () => { + getMany.mockResolvedValue([]); + + await expect(service.purgeUploads()).resolves.toBeUndefined(); + + expect(storageService.unlink).not.toHaveBeenCalled(); + expect(uploadRepository.remove).not.toHaveBeenCalled(); + }); + + it('still removes row when ciphertext file is already missing', async () => { + const upload = { id: fixedId, storagePath: fixedId }; + + getMany.mockResolvedValue([upload]); + storageService.unlink.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + + await expect(service.purgeUploads()).resolves.toBeUndefined(); + + expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + }); + }); + + describe('deleteUpload', () => { + it('unlinks ciphertext and removes upload when delete token matches', async () => { + const upload = { + storagePath: fixedId, + deleteTokenHash: hashDeleteToken(fixedDeleteToken) + }; + + uploadRepository.findOne.mockResolvedValue(upload); + + await expect(service.deleteUpload(fixedId, fixedDeleteToken)).resolves.toBeUndefined(); + + expect(uploadRepository.findOne).toHaveBeenCalledWith({ + where: { id: fixedId, deleteTokenHash: hashDeleteToken(fixedDeleteToken) } + }); + expect(storageService.unlink).toHaveBeenCalledWith(fixedId); + expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + }); + + it('throws NotFoundException when upload is missing', async () => { + uploadRepository.findOne.mockResolvedValue(null); + + await expect(service.deleteUpload(fixedId, fixedDeleteToken)).rejects.toThrow(NotFoundException); + expect(storageService.unlink).not.toHaveBeenCalled(); + expect(uploadRepository.remove).not.toHaveBeenCalled(); + }); + + it('throws NotFoundException when delete token is wrong', async () => { + uploadRepository.findOne.mockResolvedValue(null); + + await expect(service.deleteUpload(fixedId, 'wrong-token')).rejects.toThrow(NotFoundException); + expect(storageService.unlink).not.toHaveBeenCalled(); + expect(uploadRepository.remove).not.toHaveBeenCalled(); + }); + + it('still removes upload when ciphertext file is already missing', async () => { + const upload = { + storagePath: fixedId, + deleteTokenHash: hashDeleteToken(fixedDeleteToken) + }; + + uploadRepository.findOne.mockResolvedValue(upload); + storageService.unlink.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + + await expect(service.deleteUpload(fixedId, fixedDeleteToken)).resolves.toBeUndefined(); + + expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + }); + + it('propagates non-ENOENT storage unlink errors', async () => { + uploadRepository.findOne.mockResolvedValue({ + storagePath: fixedId, + deleteTokenHash: hashDeleteToken(fixedDeleteToken) + }); + storageService.unlink.mockRejectedValue(new Error('permission denied')); + + await expect(service.deleteUpload(fixedId, fixedDeleteToken)).rejects.toThrow('permission denied'); + expect(uploadRepository.remove).not.toHaveBeenCalled(); + }); + }); + + describe('purgeUpload', () => { + it('unlinks ciphertext and removes upload', async () => { + const upload = { id: fixedId, storagePath: fixedId }; + + await expect( + service.purgeUpload(upload as never, uploadRepository as never) + ).resolves.toBeUndefined(); + + expect(storageService.unlink).toHaveBeenCalledWith(fixedId); + expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + }); + + it('still removes row when ciphertext file is already missing', async () => { + const upload = { id: fixedId, storagePath: fixedId }; + + storageService.unlink.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + + await expect( + service.purgeUpload(upload as never, uploadRepository as never) + ).resolves.toBeUndefined(); + + expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + }); + + it('propagates non-ENOENT storage unlink errors', async () => { + const upload = { id: fixedId, storagePath: fixedId }; + + storageService.unlink.mockRejectedValue(new Error('permission denied')); + + await expect(service.purgeUpload(upload as never, uploadRepository as never)).rejects.toThrow( + 'permission denied' + ); + expect(uploadRepository.remove).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/backend/src/modules/upload/services/UploadCleanupService.ts b/backend/src/modules/upload/services/UploadCleanupService.ts new file mode 100644 index 0000000..167185e --- /dev/null +++ b/backend/src/modules/upload/services/UploadCleanupService.ts @@ -0,0 +1,64 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; +import { Cron, CronExpression } from '@nestjs/schedule'; +import { DataSource, Repository } from 'typeorm'; +import { hashDeleteToken } from '../../../utils/hashDeleteToken'; +import { isErrorEnoent } from '../../../utils/isErrorEnoent'; +import { StorageService } from '../../storage/services/StorageService'; +import { Upload } from '../entities/Upload'; + +@Injectable() +export class UploadCleanupService { + constructor( + @InjectDataSource() + private readonly dataSource: DataSource, + private readonly storageService: StorageService, + @InjectRepository(Upload) + private readonly uploadRepository: Repository + ) {} + + async deleteUpload(id: string, deleteToken: string): Promise { + await this.dataSource.transaction(async manager => { + const uploadRepository = manager.getRepository(Upload); + + const upload = await uploadRepository.findOne({ + where: { id, deleteTokenHash: hashDeleteToken(deleteToken) } + }); + + if (!upload) { + throw new NotFoundException(); + } + + await this.purgeUpload(upload, uploadRepository); + }); + } + + @Cron(CronExpression.EVERY_MINUTE) + async purgeUploads(): Promise { + const purgeBatchSize = 100; + const now = new Date(); + + const uploads = await this.uploadRepository + .createQueryBuilder('upload') + .where('upload.expiresAt IS NOT NULL AND upload.expiresAt < :now', { now }) + .orWhere('upload.maxViews IS NOT NULL AND upload.viewCount >= upload.maxViews') + .take(purgeBatchSize) + .getMany(); + + for (const upload of uploads) { + await this.purgeUpload(upload, this.uploadRepository); + } + } + + async purgeUpload(upload: Upload, uploadRepository: Repository): Promise { + try { + await this.storageService.unlink(upload.storagePath); + } catch (error: unknown) { + if (!isErrorEnoent(error)) { + throw error; + } + } + + await uploadRepository.remove(upload); + } +} diff --git a/backend/src/modules/upload/services/UploadService.spec.ts b/backend/src/modules/upload/services/UploadService.spec.ts index f796dea..e638663 100644 --- a/backend/src/modules/upload/services/UploadService.spec.ts +++ b/backend/src/modules/upload/services/UploadService.spec.ts @@ -1,5 +1,6 @@ import { BadRequestException, NotFoundException } from '@nestjs/common'; -import { createCipheriv, createHash, randomBytes } from 'node:crypto'; +import { createCipheriv, randomBytes } from 'node:crypto'; +import { hashDeleteToken } from '../../../utils/hashDeleteToken'; import { AES_GCM_IV_BYTE_LENGTH } from '../../../consts/crypto/aesGcmIvByteLength'; import type { DataSource, EntityManager, Repository } from 'typeorm'; import type { Upload } from '../entities/Upload'; @@ -39,8 +40,6 @@ const ciphertextBase64 = (fill = 0xcd): string => Buffer.alloc(64, fill).toStrin const wrapFieldBase64 = (length: number): string => Buffer.alloc(length, 0x22).toString('base64'); -const hashDeleteToken = (token: string): string => createHash('sha256').update(token, 'utf8').digest('hex'); - const encryptHotlinkCiphertext = (plaintext: Buffer, key: Buffer, iv: Buffer): Buffer => { const cipher = createCipheriv('aes-256-gcm', key, iv); @@ -340,7 +339,7 @@ describe('UploadService', () => { expect(uploadRepository.increment).not.toHaveBeenCalled(); }); - it('purges upload after the last allowed view', async () => { + it('does not purge on GET after the last allowed view', async () => { const iv = Buffer.alloc(12, 0x11); const ciphertext = Buffer.alloc(64, 0xcd); const upload = { @@ -367,8 +366,8 @@ describe('UploadService', () => { }); expect(uploadRepository.increment).toHaveBeenCalledWith({ id: fixedId }, 'viewCount', 1); - expect(storageService.unlink).toHaveBeenCalledWith(fixedId); - expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + expect(storageService.unlink).not.toHaveBeenCalled(); + expect(uploadRepository.remove).not.toHaveBeenCalled(); }); }); @@ -460,7 +459,7 @@ describe('UploadService', () => { expect(uploadRepository.increment).not.toHaveBeenCalled(); }); - it('purges upload after the last allowed view', async () => { + it('does not purge on GET after the last allowed view', async () => { const upload = { id: fixedId, storagePath: fixedId, @@ -479,8 +478,8 @@ describe('UploadService', () => { }); expect(uploadRepository.increment).toHaveBeenCalledWith({ id: fixedId }, 'viewCount', 1); - expect(storageService.unlink).toHaveBeenCalledWith(fixedId); - expect(uploadRepository.remove).toHaveBeenCalledWith(upload); + expect(storageService.unlink).not.toHaveBeenCalled(); + expect(uploadRepository.remove).not.toHaveBeenCalled(); }); it('does not purge when views remain below maxViews', async () => { @@ -504,68 +503,6 @@ describe('UploadService', () => { }); }); - describe('deleteUpload', () => { - const deleteToken = fixedDeleteTokenBytes.toString('base64url'); - - it('unlinks ciphertext and removes upload when delete token matches', async () => { - const upload = { - storagePath: fixedId, - deleteTokenHash: hashDeleteToken(deleteToken) - }; - - uploadRepository.findOne.mockResolvedValue(upload); - - await expect(service.deleteUpload(fixedId, deleteToken)).resolves.toBeUndefined(); - - expect(uploadRepository.findOne).toHaveBeenCalledWith({ - where: { id: fixedId, deleteTokenHash: hashDeleteToken(deleteToken) } - }); - expect(storageService.unlink).toHaveBeenCalledWith(fixedId); - expect(uploadRepository.remove).toHaveBeenCalledWith(upload); - }); - - it('throws NotFoundException when upload is missing', async () => { - uploadRepository.findOne.mockResolvedValue(null); - - await expect(service.deleteUpload(fixedId, deleteToken)).rejects.toThrow(NotFoundException); - expect(storageService.unlink).not.toHaveBeenCalled(); - expect(uploadRepository.remove).not.toHaveBeenCalled(); - }); - - it('throws NotFoundException when delete token is wrong', async () => { - uploadRepository.findOne.mockResolvedValue(null); - - await expect(service.deleteUpload(fixedId, 'wrong-token')).rejects.toThrow(NotFoundException); - expect(storageService.unlink).not.toHaveBeenCalled(); - expect(uploadRepository.remove).not.toHaveBeenCalled(); - }); - - it('still removes upload when ciphertext file is already missing', async () => { - const upload = { - storagePath: fixedId, - deleteTokenHash: hashDeleteToken(deleteToken) - }; - - uploadRepository.findOne.mockResolvedValue(upload); - storageService.unlink.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); - - await expect(service.deleteUpload(fixedId, deleteToken)).resolves.toBeUndefined(); - - expect(uploadRepository.remove).toHaveBeenCalledWith(upload); - }); - - it('propagates non-ENOENT storage unlink errors', async () => { - uploadRepository.findOne.mockResolvedValue({ - storagePath: fixedId, - deleteTokenHash: hashDeleteToken(deleteToken) - }); - storageService.unlink.mockRejectedValue(new Error('permission denied')); - - await expect(service.deleteUpload(fixedId, deleteToken)).rejects.toThrow('permission denied'); - expect(uploadRepository.remove).not.toHaveBeenCalled(); - }); - }); - describe('transaction rollback on storage failure', () => { it('propagates storage write errors from the transaction', async () => { storageService.write.mockRejectedValue(new Error('disk full')); diff --git a/backend/src/modules/upload/services/UploadService.ts b/backend/src/modules/upload/services/UploadService.ts index 2eae57d..e9f44ca 100644 --- a/backend/src/modules/upload/services/UploadService.ts +++ b/backend/src/modules/upload/services/UploadService.ts @@ -2,11 +2,12 @@ import { BadRequestException, Injectable, NotFoundException } from '@nestjs/comm import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; import dayjs from 'dayjs'; import { fileTypeFromBuffer } from 'file-type'; -import { createHash, randomBytes, randomUUID } from 'node:crypto'; +import { randomBytes, randomUUID } from 'node:crypto'; import { DataSource, Repository } from 'typeorm'; import { decryptAesGcm } from '../../../utils/decryptAesGcm'; import { fromBase64 } from '../../../utils/fromBase64'; import { fromBase64Url } from '../../../utils/fromBase64Url'; +import { hashDeleteToken } from '../../../utils/hashDeleteToken'; import { isErrorEnoent } from '../../../utils/isErrorEnoent'; import { toBase64 } from '../../../utils/toBase64'; import { StorageService } from '../../storage/services/StorageService'; @@ -55,7 +56,7 @@ export class UploadService { storagePath, iv: fromBase64(iv), maxViews, - deleteTokenHash: this.hashDeleteToken(deleteToken), + deleteTokenHash: hashDeleteToken(deleteToken), expiresAt, zk: { passwordProtected, @@ -101,7 +102,7 @@ export class UploadService { storagePath, iv: fromBase64(iv), maxViews, - deleteTokenHash: this.hashDeleteToken(deleteToken), + deleteTokenHash: hashDeleteToken(deleteToken), expiresAt, hotlink: { mimeType } }); @@ -166,12 +167,6 @@ export class UploadService { await this.uploadRepository.increment({ id }, 'viewCount', 1); - const nextViewCount = upload.viewCount + 1; - - if (typeof upload.maxViews === 'number' && nextViewCount >= upload.maxViews) { - await this.purgeUpload(upload, this.uploadRepository); - } - return result; } @@ -217,50 +212,12 @@ export class UploadService { await this.uploadRepository.increment({ id }, 'viewCount', 1); - const nextViewCount = upload.viewCount + 1; - - if (typeof upload.maxViews === 'number' && nextViewCount >= upload.maxViews) { - await this.purgeUpload(upload, this.uploadRepository); - } - return { body, mimeType: upload.hotlink.mimeType }; } - async deleteUpload(id: string, deleteToken: string): Promise { - await this.dataSource.transaction(async manager => { - const uploadRepository = manager.getRepository(Upload); - - const upload = await uploadRepository.findOne({ - where: { id, deleteTokenHash: this.hashDeleteToken(deleteToken) } - }); - - if (!upload) { - throw new NotFoundException(); - } - - await this.purgeUpload(upload, uploadRepository); - }); - } - - private async purgeUpload(upload: Upload, uploadRepository: Repository): Promise { - try { - await this.storageService.unlink(upload.storagePath); - } catch (error: unknown) { - if (!isErrorEnoent(error)) { - throw error; - } - } - - await uploadRepository.remove(upload); - } - - private hashDeleteToken(token: string): string { - return createHash('sha256').update(token, 'utf8').digest('hex'); - } - private async throwIfUnencryptedImage(ciphertext: Buffer): Promise { const detected = await fileTypeFromBuffer(ciphertext); diff --git a/backend/src/utils/hashDeleteToken.ts b/backend/src/utils/hashDeleteToken.ts new file mode 100644 index 0000000..74e6256 --- /dev/null +++ b/backend/src/utils/hashDeleteToken.ts @@ -0,0 +1,4 @@ +import { createHash } from 'node:crypto'; + +export const hashDeleteToken = (token: string): string => + createHash('sha256').update(token, 'utf8').digest('hex');