From 669bc5f95fb743676b9e01dc60df734ef477da22 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:36:26 +0200 Subject: [PATCH] Bugfix/harden import endpoint by hiding internal error messages (#8281) * Hide internal error messages * Update changelog --- CHANGELOG.md | 1 + .../import/import.controller.spec.ts | 78 +++++++++++++++++++ .../app/endpoints/import/import.controller.ts | 19 ++++- 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/app/endpoints/import/import.controller.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a6959f512..4a58b2a19a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Improved the performance of the historical market data table of the admin control panel by counting the market data per asset profile +- Hardened the `POST api/v1/import` endpoint by hiding internal error messages ## 3.91.0 - 2026-10-23 diff --git a/apps/api/src/app/endpoints/import/import.controller.spec.ts b/apps/api/src/app/endpoints/import/import.controller.spec.ts new file mode 100644 index 0000000000..6e7aa7c803 --- /dev/null +++ b/apps/api/src/app/endpoints/import/import.controller.spec.ts @@ -0,0 +1,78 @@ +import { ImportValidationError } from '@ghostfolio/api/services/import/errors/import-validation.error'; +import { ImportDataDto } from '@ghostfolio/api/services/import/import-data.dto'; +import { ImportService } from '@ghostfolio/api/services/import/import.service'; +import { permissions } from '@ghostfolio/common/permissions'; +import type { RequestWithUser } from '@ghostfolio/common/types'; + +import { HttpException, Logger } from '@nestjs/common'; +import { getReasonPhrase, StatusCodes } from 'http-status-codes'; + +import { ImportController } from './import.controller'; + +describe('ImportController', () => { + const importData = { activities: [] } as ImportDataDto; + + let importController: ImportController; + let importService: ImportService; + let logError: jest.SpyInstance; + + beforeEach(() => { + logError = jest.spyOn(Logger.prototype, 'error').mockImplementation(); + + importService = { + import: jest.fn() + } as unknown as ImportService; + + importController = new ImportController(importService, { + user: { permissions: [permissions.createAccount] } + } as unknown as RequestWithUser); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('import', () => { + it('Gives the status 400 with the message of an error for the caller', async () => { + jest + .mocked(importService.import) + .mockRejectedValueOnce( + new ImportValidationError('activities.0.accountId ("1") is not valid') + ); + + await expect(importController.import(importData)).rejects.toMatchObject({ + response: { message: ['activities.0.accountId ("1") is not valid'] }, + status: StatusCodes.BAD_REQUEST + }); + }); + + it('Gives the status 400 with the reason phrase of the status of an HTTP exception', async () => { + jest + .mocked(importService.import) + .mockRejectedValueOnce( + new HttpException( + 'Unique constraint failed on the fields: (`name`,`userId`)', + StatusCodes.CONFLICT + ) + ); + + await expect(importController.import(importData)).rejects.toMatchObject({ + response: { message: [getReasonPhrase(StatusCodes.CONFLICT)] }, + status: StatusCodes.BAD_REQUEST + }); + }); + + it('Gives the status 400 without the message of every other error', async () => { + const error = new Error('Invalid `prisma.order.create()` invocation'); + + jest.mocked(importService.import).mockRejectedValueOnce(error); + + await expect(importController.import(importData)).rejects.toMatchObject({ + response: { message: [getReasonPhrase(StatusCodes.BAD_REQUEST)] }, + status: StatusCodes.BAD_REQUEST + }); + + expect(logError).toHaveBeenCalledWith(error); + }); + }); +}); diff --git a/apps/api/src/app/endpoints/import/import.controller.ts b/apps/api/src/app/endpoints/import/import.controller.ts index 9c5c162e4d..0cc542976e 100644 --- a/apps/api/src/app/endpoints/import/import.controller.ts +++ b/apps/api/src/app/endpoints/import/import.controller.ts @@ -1,4 +1,5 @@ import { HasPermission } from '@ghostfolio/api/decorators/has-permission.decorator'; +import { CallerFacingError } from '@ghostfolio/api/errors/caller-facing.error'; import { HasPermissionGuard } from '@ghostfolio/api/guards/has-permission.guard'; import { TransformDataSourceInRequestInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-request/transform-data-source-in-request.interceptor'; import { TransformDataSourceInResponseInterceptor } from '@ghostfolio/api/interceptors/transform-data-source-in-response/transform-data-source-in-response.interceptor'; @@ -73,7 +74,7 @@ export class ImportController { throw new HttpException( { error: getReasonPhrase(StatusCodes.BAD_REQUEST), - message: [error.message] + message: [this.getMessageOfError(error)] }, StatusCodes.BAD_REQUEST ); @@ -100,6 +101,20 @@ export class ImportController { userId: this.request.user.id }); - return { activities: activities.slice(0, maxActivitiesToImport) }; + return { + activities: activities.slice(0, maxActivitiesToImport) + }; + } + + private getMessageOfError(error: unknown) { + if (error instanceof CallerFacingError) { + return error.message; + } + + if (error instanceof HttpException) { + return getReasonPhrase(error.getStatus()); + } + + return getReasonPhrase(StatusCodes.BAD_REQUEST); } }