diff --git a/packages/twenty-server/src/app.module.ts b/packages/twenty-server/src/app.module.ts index 4b61f6e9bc..ada929e25b 100644 --- a/packages/twenty-server/src/app.module.ts +++ b/packages/twenty-server/src/app.module.ts @@ -4,6 +4,7 @@ import { Module, RequestMethod, } from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; import { GraphQLModule } from '@nestjs/graphql'; import { ServeStaticModule } from '@nestjs/serve-static'; @@ -31,6 +32,7 @@ import { RestCoreMiddleware } from 'src/engine/middlewares/rest-core.middleware' import { GlobalWorkspaceDataSourceModule } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-datasource.module'; import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; +import { UnhandledExceptionFilter } from 'src/filters/unhandled-exception.filter'; import { ModulesModule } from 'src/modules/modules.module'; import { ClickHouseModule } from './database/clickHouse/clickHouse.module'; @@ -76,6 +78,12 @@ const MIGRATED_REST_METHODS = [ // Conditional modules ...AppModule.getConditionalModules(), ], + providers: [ + { + provide: APP_FILTER, + useClass: UnhandledExceptionFilter, + }, + ], }) export class AppModule { private static getConditionalModules(): DynamicModule[] { diff --git a/packages/twenty-server/src/engine/core-modules/exception-handler/mocks/mock-unhandled-exception.filter.ts b/packages/twenty-server/src/engine/core-modules/exception-handler/mocks/mock-unhandled-exception.filter.ts deleted file mode 100644 index e108157999..0000000000 --- a/packages/twenty-server/src/engine/core-modules/exception-handler/mocks/mock-unhandled-exception.filter.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { - type ArgumentsHost, - Catch, - type ExceptionFilter, -} from '@nestjs/common'; -import { BaseExceptionFilter } from '@nestjs/core'; - -@Catch() -export class MockedUnhandledExceptionFilter - extends BaseExceptionFilter - implements ExceptionFilter -{ - // oxlint-disable-next-line typescript/no-explicit-any - catch(exception: any, _host: ArgumentsHost) { - throw exception; - } -} diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/controllers/field-metadata.controller.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/controllers/field-metadata.controller.ts index 3981db3b13..62b29bfb34 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/controllers/field-metadata.controller.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/controllers/field-metadata.controller.ts @@ -53,6 +53,7 @@ import { toLegacyFieldMetadataListResponse, toLegacyFieldMetadataUpdateResponse, } from 'src/engine/metadata-modules/field-metadata/utils/to-legacy-field-metadata-response.util'; +import { FlatEntityMapsRestApiExceptionFilter } from 'src/engine/metadata-modules/flat-entity/filters/flat-entity-maps-rest-api-exception.filter'; import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service'; import { fromFlatFieldMetadataToFieldMetadataDto } from 'src/engine/metadata-modules/flat-field-metadata/utils/from-flat-field-metadata-to-field-metadata-dto.util'; import { computeUniqueFieldMetadataIdsFromFlatIndexMaps } from 'src/engine/metadata-modules/index-metadata/utils/compute-unique-field-metadata-ids-from-flat-index-maps.util'; @@ -68,6 +69,7 @@ import { PermissionsRestApiExceptionFilter } from 'src/engine/metadata-modules/p PermissionsRestApiExceptionFilter, FieldMetadataRestApiExceptionFilter, ApplicationRestApiExceptionFilter, + FlatEntityMapsRestApiExceptionFilter, ) @UsePipes(new ValidationPipe()) export class FieldMetadataController { diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/controllers/object-metadata.controller.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/controllers/object-metadata.controller.ts index c22ddd0db7..56c846fc5e 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/controllers/object-metadata.controller.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/controllers/object-metadata.controller.ts @@ -37,6 +37,7 @@ import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.g import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { fromFieldMetadataEntityToFieldMetadataDto } from 'src/engine/metadata-modules/field-metadata/utils/from-field-metadata-entity-to-field-metadata-dto.util'; +import { FlatEntityMapsRestApiExceptionFilter } from 'src/engine/metadata-modules/flat-entity/filters/flat-entity-maps-rest-api-exception.filter'; import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service'; import { fromFlatObjectMetadataToObjectMetadataDto } from 'src/engine/metadata-modules/flat-object-metadata/utils/from-flat-object-metadata-to-object-metadata-dto.util'; import { computeUniqueFieldMetadataIdsFromFlatIndexMaps } from 'src/engine/metadata-modules/index-metadata/utils/compute-unique-field-metadata-ids-from-flat-index-maps.util'; @@ -70,6 +71,7 @@ import { PermissionsRestApiExceptionFilter } from 'src/engine/metadata-modules/p PermissionsRestApiExceptionFilter, ObjectMetadataRestApiExceptionFilter, ApplicationRestApiExceptionFilter, + FlatEntityMapsRestApiExceptionFilter, ) @UsePipes(new ValidationPipe()) export class ObjectMetadataController { diff --git a/packages/twenty-server/src/filters/__tests__/unhandled-exception.filter.spec.ts b/packages/twenty-server/src/filters/__tests__/unhandled-exception.filter.spec.ts new file mode 100644 index 0000000000..88e64f76e0 --- /dev/null +++ b/packages/twenty-server/src/filters/__tests__/unhandled-exception.filter.spec.ts @@ -0,0 +1,144 @@ +// oxlint-disable twenty/graphql-resolvers-should-be-guarded +import { + type CanActivate, + Injectable, + Module, + UseGuards, +} from '@nestjs/common'; +import { APP_FILTER } from '@nestjs/core'; +import { + GraphQLModule, + GraphQLSchemaHost, + type GraphQLSchemaHost as GraphQLSchemaHostType, + Mutation, + Query, + Resolver, +} from '@nestjs/graphql'; +import { Test } from '@nestjs/testing'; + +import { YogaDriver, type YogaDriverConfig } from '@graphql-yoga/nestjs'; +import { msg } from '@lingui/core/macro'; +import { type GraphQLSchema, graphql } from 'graphql'; + +import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { + PermissionsException, + PermissionsExceptionCode, + PermissionsExceptionMessage, +} from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter'; +import { UnhandledExceptionFilter } from 'src/filters/unhandled-exception.filter'; + +@Injectable() +class DenyPermissionGuard implements CanActivate { + canActivate(): boolean { + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + { userFriendlyMessage: msg`denied` }, + ); + } +} + +@Resolver() +class TestResolver { + @Query(() => String) + ping(): string { + return 'pong'; + } + + @Mutation(() => String) + @UseGuards(DenyPermissionGuard) + guardedMutation(): string { + return 'ok'; + } +} + +// mirrors CoreEngineModule / MetadataEngineModule: a feature module that +// registers a typed GraphQL exception filter +@Module({ + providers: [ + TestResolver, + { provide: APP_FILTER, useClass: PermissionsGraphqlApiExceptionFilter }, + ], +}) +class FeatureModule {} + +@Module({ + imports: [ + GraphQLModule.forRoot({ + driver: YogaDriver, + autoSchemaFile: true, + }), + FeatureModule, + ], + providers: [{ provide: APP_FILTER, useClass: UnhandledExceptionFilter }], +}) +class RootModuleWithAppFilter {} + +@Module({ + imports: [ + GraphQLModule.forRoot({ + driver: YogaDriver, + autoSchemaFile: true, + }), + FeatureModule, + ], +}) +class RootModuleWithoutAppFilter {} + +const buildSchema = async ( + rootModule: unknown, + registerFilterAtBootstrap: boolean, +) => { + const moduleRef = await Test.createTestingModule({ + // oxlint-disable-next-line typescript/no-explicit-any + imports: [rootModule as any], + }).compile(); + + const app = moduleRef.createNestApplication(); + + if (registerFilterAtBootstrap) { + app.useGlobalFilters(new UnhandledExceptionFilter()); + } + + await app.init(); + + const schema = app.get(GraphQLSchemaHost).schema; + + return { app, schema }; +}; + +const runGuardedMutation = (schema: GraphQLSchema) => + graphql({ schema, source: 'mutation { guardedMutation }' }); + +describe('UnhandledExceptionFilter global registration', () => { + it('lets typed GraphQL filters convert the exception when registered through APP_FILTER on the root module', async () => { + const { app, schema } = await buildSchema(RootModuleWithAppFilter, false); + + const result = await runGuardedMutation(schema); + + expect(result.errors?.[0]?.extensions?.code).toBe(ErrorCode.FORBIDDEN); + expect(result.errors?.[0]?.message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + + await app.close(); + }); + + // guards against reintroducing app.useGlobalFilters(new UnhandledExceptionFilter()): + // Nest checks global filters in reverse registration order and selects a single + // one, so a catch-all registered after bootstrap shadows every typed filter + it('shadows typed GraphQL filters when registered through app.useGlobalFilters', async () => { + const { app, schema } = await buildSchema(RootModuleWithoutAppFilter, true); + + const result = await runGuardedMutation(schema); + + expect(result.errors?.[0]?.extensions?.code).toBeUndefined(); + expect(result.errors?.[0]?.originalError).toBeInstanceOf( + PermissionsException, + ); + + await app.close(); + }); +}); diff --git a/packages/twenty-server/src/main.ts b/packages/twenty-server/src/main.ts index 75eba83ae9..0521f2bda6 100644 --- a/packages/twenty-server/src/main.ts +++ b/packages/twenty-server/src/main.ts @@ -18,7 +18,6 @@ import { getSessionStorageOptions } from 'src/engine/core-modules/session-storag import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { configTransformers } from 'src/engine/core-modules/twenty-config/utils/config-transformers.util'; import { shouldCaptureException } from 'src/engine/utils/global-exception-handler.util'; -import { UnhandledExceptionFilter } from 'src/filters/unhandled-exception.filter'; import { AppModule } from './app.module'; import './instrument'; @@ -76,8 +75,6 @@ const bootstrap = async () => { // Use our logger app.useLogger(logger); - app.useGlobalFilters(new UnhandledExceptionFilter()); - app.useBodyParser('json', { limit: settings.storage.maxFileSize }); app.useBodyParser('urlencoded', { limit: settings.storage.maxFileSize, diff --git a/packages/twenty-server/test/integration/metadata/suites/file/__snapshots__/failing-file-by-id-download.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/file/__snapshots__/failing-file-by-id-download.integration-spec.ts.snap index 59a670e5d6..0c40d7e0a9 100644 --- a/packages/twenty-server/test/integration/metadata/suites/file/__snapshots__/failing-file-by-id-download.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/file/__snapshots__/failing-file-by-id-download.integration-spec.ts.snap @@ -2,14 +2,22 @@ exports[`File-by-id controller download should fail should respond 403 when the URL fileId does not match the token payload 1`] = ` { - "body": {}, + "body": { + "error": "Forbidden", + "message": "Forbidden resource", + "statusCode": 403, + }, "status": 403, } `; exports[`File-by-id controller download should fail should respond 403 when the request has no token query parameter 1`] = ` { - "body": {}, + "body": { + "error": "Forbidden", + "message": "Forbidden resource", + "statusCode": 403, + }, "status": 403, } `; diff --git a/packages/twenty-server/test/integration/utils/create-app.ts b/packages/twenty-server/test/integration/utils/create-app.ts index eabf12acc6..f53204ea25 100644 --- a/packages/twenty-server/test/integration/utils/create-app.ts +++ b/packages/twenty-server/test/integration/utils/create-app.ts @@ -1,4 +1,3 @@ -import { APP_FILTER } from '@nestjs/core'; import { type NestExpressApplication } from '@nestjs/platform-express'; import { Test, @@ -16,7 +15,6 @@ import { StripeSDKService } from 'src/engine/core-modules/billing/stripe/stripe- import { CaptchaDriverFactory } from 'src/engine/core-modules/captcha/captcha-driver.factory'; import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; import { ExceptionHandlerMockService } from 'src/engine/core-modules/exception-handler/mocks/exception-handler-mock.service'; -import { MockedUnhandledExceptionFilter } from 'src/engine/core-modules/exception-handler/mocks/mock-unhandled-exception.filter'; import { JobsModule } from 'src/engine/core-modules/message-queue/jobs.module'; import { MessageQueueModule } from 'src/engine/core-modules/message-queue/message-queue.module'; @@ -44,12 +42,6 @@ export const createApp = async ( const mockExceptionHandlerService = new ExceptionHandlerMockService(); let moduleBuilder: TestingModuleBuilder = Test.createTestingModule({ imports: [AppModule, JobsModule, MessageQueueModule.registerExplorer()], - providers: [ - { - provide: APP_FILTER, - useClass: MockedUnhandledExceptionFilter, - }, - ], }) .overrideProvider(StripeSDKService) .useValue(stripeSDKMockService)