fix(media): add UUID validation and update tests accordingly

Signed-off-by: Erik Michelson <github@erik.michelson.eu>

backend/src/media/backends/filesystem-backend.spec.ts

@@ -0,0 +1,183 @@
+/*
+ * SPDX-FileCopyrightText: 2026 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+import { describe, it, expect, beforeAll, beforeEach, jest } from '@jest/globals';
+import { MediaBackendType } from '@hedgedoc/commons';
+import { promises as fs } from 'fs';
+import { Mock } from 'ts-mockery';
+
+import { MediaConfig } from '../../config/media.config';
+import { MediaBackendError } from '../../errors/errors';
+import { ConsoleLoggerService } from '../../logger/console-logger.service';
+import { FilesystemBackend } from './filesystem-backend';
+
+jest.mock('fs', () => ({
+  promises: {
+    access: jest.fn(),
+    mkdir: jest.fn(),
+    readFile: jest.fn(),
+    unlink: jest.fn(),
+    writeFile: jest.fn(),
+  },
+}));
+
+describe('filesystem backend', () => {
+  const mockedUploadPath = '/tmp/test_uploads';
+  const mockedUuid = 'cbe87987-8e70-4092-a879-878e70b09245';
+  const mockedBuffer = Buffer.from('test');
+
+  const mockedLoggerService = Mock.of<ConsoleLoggerService>({
+    setContext: jest.fn(),
+    error: jest.fn(),
+    debug: jest.fn(),
+  });
+
+  function mockMediaConfig(): MediaConfig {
+    return Mock.of<MediaConfig>({
+      backend: {
+        type: MediaBackendType.FILESYSTEM,
+        filesystem: {
+          uploadPath: mockedUploadPath,
+        },
+      },
+    });
+  }
+
+  let sut: FilesystemBackend;
+
+  beforeAll(() => {
+    sut = new FilesystemBackend(mockedLoggerService, mockMediaConfig());
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    jest.spyOn(fs, 'access').mockResolvedValue(undefined);
+  });
+
+  describe('saveFile', () => {
+    it('writes the buffer to the expected path and returns the backend data', async () => {
+      const writeFileSpy = jest.spyOn(fs, 'writeFile').mockResolvedValue(undefined);
+
+      const result = await sut.saveFile(mockedUuid, mockedBuffer, {
+        mime: 'image/png',
+        ext: 'png',
+      });
+
+      expect(writeFileSpy).toHaveBeenCalledWith(
+        `${mockedUploadPath}/${mockedUuid}.png`,
+        mockedBuffer,
+        null,
+      );
+      expect(result).toBe(JSON.stringify({ ext: 'png', mime: 'image/png' }));
+    });
+
+    it('throws a MediaBackendError if the extension is not alphanumeric', async () => {
+      const writeFileSpy = jest.spyOn(fs, 'writeFile');
+
+      await expect(
+        sut.saveFile(mockedUuid, mockedBuffer, { mime: 'image/png', ext: '../png' as never }),
+      ).rejects.toThrow(new MediaBackendError('Invalid file extension: ../png'));
+      expect(writeFileSpy).not.toHaveBeenCalled();
+    });
+
+    it('throws a MediaBackendError if the file could not be written', async () => {
+      jest.spyOn(fs, 'writeFile').mockRejectedValue(new Error('mocked error'));
+
+      await expect(
+        sut.saveFile(mockedUuid, mockedBuffer, { mime: 'image/png', ext: 'png' }),
+      ).rejects.toThrow(`Could not save file '${mockedUploadPath}/${mockedUuid}.png'`);
+    });
+  });
+
+  describe('deleteFile', () => {
+    it('unlinks the file at the expected path', async () => {
+      const unlinkSpy = jest.spyOn(fs, 'unlink').mockResolvedValue(undefined);
+
+      await sut.deleteFile(mockedUuid, JSON.stringify({ ext: 'png' }));
+
+      expect(unlinkSpy).toHaveBeenCalledWith(`${mockedUploadPath}/${mockedUuid}.png`);
+    });
+
+    it('throws a MediaBackendError if no backend data is provided', async () => {
+      await expect(sut.deleteFile(mockedUuid, '')).rejects.toThrow('No backend data provided');
+    });
+
+    it('throws a MediaBackendError if the backend data has no extension', async () => {
+      await expect(sut.deleteFile(mockedUuid, JSON.stringify({ ext: '' }))).rejects.toThrow(
+        'No file extension in backend data',
+      );
+    });
+
+    it('throws a MediaBackendError if the extension is not alphanumeric', async () => {
+      const unlinkSpy = jest.spyOn(fs, 'unlink');
+
+      await expect(sut.deleteFile(mockedUuid, JSON.stringify({ ext: '../png' }))).rejects.toThrow(
+        new MediaBackendError('Invalid file extension: ../png'),
+      );
+      expect(unlinkSpy).not.toHaveBeenCalled();
+    });
+
+    it('throws a MediaBackendError if the file could not be deleted', async () => {
+      jest.spyOn(fs, 'unlink').mockRejectedValue(new Error('mocked error'));
+
+      await expect(sut.deleteFile(mockedUuid, JSON.stringify({ ext: 'png' }))).rejects.toThrow(
+        `Could not delete file '${mockedUploadPath}/${mockedUuid}.png'`,
+      );
+    });
+  });
+
+  describe('getFileUrl', () => {
+    it('returns the public media url for the file', async () => {
+      await expect(sut.getFileUrl(mockedUuid, '')).resolves.toBe(`/media/${mockedUuid}`);
+    });
+  });
+
+  describe('getFileResponse', () => {
+    it('reads the file from disk and returns its content together with the mime type', async () => {
+      const readFileSpy = jest.spyOn(fs, 'readFile').mockResolvedValue(mockedBuffer);
+
+      const result = await sut.getFileResponse(
+        mockedUuid,
+        JSON.stringify({ ext: 'png', mime: 'image/png' }),
+      );
+
+      expect(readFileSpy).toHaveBeenCalledWith(`${mockedUploadPath}/${mockedUuid}.png`);
+      expect(result).toEqual({
+        buffer: mockedBuffer,
+        contentType: 'image/png',
+        fileName: `${mockedUuid}.png`,
+      });
+    });
+
+    it('falls back to application/octet-stream if no mime type is stored', async () => {
+      jest.spyOn(fs, 'readFile').mockResolvedValue(mockedBuffer);
+
+      const result = await sut.getFileResponse(mockedUuid, JSON.stringify({ ext: 'png' }));
+
+      expect(result.contentType).toBe('application/octet-stream');
+    });
+
+    it('throws a MediaBackendError if no backend data is provided', async () => {
+      await expect(sut.getFileResponse(mockedUuid, null)).rejects.toThrow(
+        'No backend data provided',
+      );
+    });
+
+    it('throws a MediaBackendError if the backend data has no extension', async () => {
+      await expect(
+        sut.getFileResponse(mockedUuid, JSON.stringify({ ext: '', mime: 'image/png' })),
+      ).rejects.toThrow('No file extension in backend data');
+    });
+
+    it('throws a MediaBackendError if the extension is not alphanumeric', async () => {
+      const readFileSpy = jest.spyOn(fs, 'readFile');
+
+      await expect(
+        sut.getFileResponse(mockedUuid, JSON.stringify({ ext: '../png', mime: 'image/png' })),
+      ).rejects.toThrow(new MediaBackendError('Invalid file extension: ../png'));
+      expect(readFileSpy).not.toHaveBeenCalled();
+    });
+  });
+});

backend/src/media/backends/filesystem-backend.ts

@@ -13,7 +13,7 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config';
 import { MediaBackendError } from '../../errors/errors';
 import { ConsoleLoggerService } from '../../logger/console-logger.service';
 import { MediaBackend } from '../media-backend.interface';
-import { MediaFileResponse } from '../media-response.interface'
+import { MediaFileResponse } from '../media-response.interface';
 
 @Injectable()
 export class FilesystemBackend implements MediaBackend {
@@ -83,7 +83,7 @@ export class FilesystemBackend implements MediaBackend {
     if (!backendData) {
       throw new MediaBackendError('No backend data provided');
     }
-    const { ext, mime } = JSON.parse(backendData) as { ext: string, mime: string };
+    const { ext, mime } = JSON.parse(backendData) as { ext: string; mime: string };
     if (!ext) {
       throw new MediaBackendError('No file extension in backend data');
     }
@@ -94,6 +94,9 @@ export class FilesystemBackend implements MediaBackend {
   }
 
   private getFilePath(fileName: string, extension: string): string {
+    if (!/^[a-zA-Z0-9]+$/.test(extension)) {
+      throw new MediaBackendError(`Invalid file extension: ${extension}`);
+    }
     return join(this.uploadDirectory, `${fileName}.${extension}`);
   }
 

backend/src/media/media.service.spec.ts

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
-import { describe, it, expect, beforeAll, afterEach, jest } from '@jest/globals';
+import { describe, it, expect, beforeAll, beforeEach, afterEach, jest } from '@jest/globals';
 import {
   FieldNameMediaUpload,
   FieldNameMediaUploadNote,
@@ -38,6 +38,7 @@ describe('MediaService', () => {
   const userId = 1;
   const noteId = 2;
   const uuid = '0198c9b6-117f-7215-93e2-5ca4b718225f';
+  const invalidUuid = 'not-a-valid-uuid';
   const fileName = 'test.png';
   const backendType = MediaBackendType.FILESYSTEM;
   const backendData = JSON.stringify({ ext: 'png' });
@@ -73,6 +74,10 @@ describe('MediaService', () => {
     fileSystemBackend = module.get<FilesystemBackend>(FilesystemBackend);
   });
 
+  beforeEach(() => {
+    jest.spyOn(uuidModule, 'validate').mockReturnValue(true);
+  });
+
   afterEach(() => {
     tracker.reset();
     jest.clearAllMocks();
@@ -223,10 +228,10 @@ describe('MediaService', () => {
         );
       const result = await service.getFileResponse(uuid);
       expect(result).toEqual({
-        kind: 'file',
+        type: 'file',
         buffer: fileBuffer,
         contentType: 'image/png',
-        fileName,
+        fileName: `${uuid}.png`,
       });
       expect(tracker.history.select).toHaveLength(1);
       expect(tracker.history.select[0].bindings).toEqual([uuid, 1]);
@@ -270,6 +275,14 @@ describe('MediaService', () => {
       await expect(service.findUploadByUuid(uuid)).rejects.toThrow(NotInDBError);
       expectBindings(tracker, 'select', [[uuid]], true);
     });
+
+    it('throws NotInDBError when given an invalid uuid', async () => {
+      jest.spyOn(uuidModule, 'validate').mockReturnValueOnce(false);
+      await expect(service.findUploadByUuid(invalidUuid)).rejects.toThrow(
+        new NotInDBError('Invalid media upload id provided', 'MediaService', 'findUploadByUuid'),
+      );
+      expect(tracker.history.select).toHaveLength(0);
+    });
   });
 
   describe('getMediaUploadUuidsByUserId', () => {
@@ -346,4 +359,18 @@ describe('MediaService', () => {
       ]);
     });
   });
+
+  describe('getMediaUploadDtoByUuid', () => {
+    it('throws NotInDBError when given an invalid uuid', async () => {
+      jest.spyOn(uuidModule, 'validate').mockReturnValueOnce(false);
+      await expect(service.getMediaUploadDtoByUuid(invalidUuid)).rejects.toThrow(
+        new NotInDBError(
+          'Invalid media upload id provided',
+          'MediaService',
+          'getMediaUploadDtoByUuid',
+        ),
+      );
+      expect(tracker.history.select).toHaveLength(0);
+    });
+  });
 });

backend/src/media/media.service.ts

@@ -21,7 +21,7 @@ import { ModuleRef } from '@nestjs/core';
 import * as FileType from 'file-type';
 import { Knex } from 'knex';
 import { InjectConnection } from 'nest-knexjs';
-import { v7 as uuidV7 } from 'uuid';
+import { v7 as uuidV7, validate as validateUuid } from 'uuid';
 
 import mediaConfiguration, { MediaConfig } from '../config/media.config';
 import { MediaUploadDto } from '../dtos/media-upload.dto';
@@ -40,7 +40,7 @@ import { ImgurBackend } from './backends/imgur-backend';
 import { S3Backend } from './backends/s3-backend';
 import { WebdavBackend } from './backends/webdav-backend';
 import { MediaBackend } from './media-backend.interface';
-import { MediaResponse } from './media-response.interface'
+import { MediaResponse } from './media-response.interface';
 
 @Injectable()
 export class MediaService {
@@ -213,6 +213,13 @@ export class MediaService {
    * @throws NotInDBError if the MediaUpload entity with the provided UUID is not found in the database
    */
   async findUploadByUuid(uuid: string): Promise<MediaUpload> {
+    if (!validateUuid(uuid)) {
+      throw new NotInDBError(
+        'Invalid media upload id provided',
+        this.logger.getContext(),
+        'findUploadByUuid',
+      );
+    }
     const mediaUpload = await this.knex(TableMediaUpload)
       .select()
       .where(FieldNameMediaUpload.uuid, uuid)
@@ -328,6 +335,13 @@ export class MediaService {
    * @returns The {@link MediaUploadDto}
    */
   async getMediaUploadDtoByUuid(uuid: string): Promise<MediaUploadDto> {
+    if (!validateUuid(uuid)) {
+      throw new NotInDBError(
+        'Invalid media upload id provided',
+        this.logger.getContext(),
+        'getMediaUploadDtoByUuid',
+      );
+    }
     const mediaUpload = await this.knex(TableMediaUpload)
       .join(
         TableUser,