From 25f28ee299dc52919d3d77f1c92c7b99c78019fe Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Wed, 22 Jul 2026 12:10:49 +0200 Subject: [PATCH] Fix TWENTY-SERVER-60Y: register permissions exception filter globally (#23104) ## Context Sentry [TWENTY-SERVER-60Y](https://twenty-v7.sentry.io/issues/6633503406) ("Permission Denied: Entity performing the request does not have permission") has ~12.9k occurrences / 151 users. It groups by the shared throw site `settings-permission.guard.ts:57`, so it's a **catch-all bucket** for settings-permission denials across many resolvers, not a single operation. Sampled events include `findOneApplication` (app runtimes reading their own `applicationVariables`), `uploadFilesFieldFileByUniversalIdentifier`, `UpdatePageLayoutWithTabsAndWidgets`, `CreateFileUpload`, etc. ## Root cause Settings-permission denials are only converted to a client-appropriate `FORBIDDEN` when a resolver manually attaches `PermissionsGraphqlApiExceptionFilter`. **28** GraphQL resolvers do; **~35** guarded resolvers do not. On those, the guard-thrown `PermissionsException` (a `CustomException`, not a `BaseGraphQLError`) falls through to the Yoga error hook, is serialized as `INTERNAL_SERVER_ERROR`, and `shouldCaptureException` reports it to Sentry as a 500-class error. REST is not affected: every `SettingsPermissionGuard` controller already carries `PermissionsRestApiExceptionFilter` (15/15). ## Fix Register `PermissionsGraphqlApiExceptionFilter` globally via `APP_FILTER` in the core and metadata engine modules, mirroring the existing global GraphQL filters `BillingGraphqlApiExceptionFilter` and `FlatEntityMapsGraphqlApiExceptionFilter`. The filter is guarded on the GraphQL context (`host.getType()`), so REST keeps its existing per-controller filter untouched and no request-scoped dependency is pulled into a global provider. Every settings-guarded resolver now returns `FORBIDDEN` for denials, which is both the correct client error code and excluded from Sentry. Existing per-resolver `@UseFilters(PermissionsGraphqlApiExceptionFilter)` entries remain valid (handler-scoped takes precedence, identical result); collapsing them into the global registration is a possible follow-up. ## Test Added a case to `granular-settings-permissions.integration-spec.ts`: a member without the `APPLICATIONS` flag calling `findOneApplication{applicationVariables{key value}}` (the exact Sentry query, and a resolver with **no** resolver-scoped filter) must receive `FORBIDDEN`, exercising the global filter. Verified against the local test DB: - With the global filter: passes (`FORBIDDEN`). - Without it: fails with `INTERNAL_SERVER_ERROR`, reproducing the leak. - The existing roles / workspace-members / api-keys denial tests (per-resolver filters) still pass, confirming no precedence conflict and no boot issue from dual `APP_FILTER` registration. Fixes TWENTY-SERVER-60Y --- .../engine/core-modules/core-engine.module.ts | 5 ++++ .../metadata-engine.module.ts | 5 ++++ ...ermissions-graphql-api-exception.filter.ts | 13 +++++++-- ...r-settings-permissions.integration-spec.ts | 28 +++++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/core-engine.module.ts b/packages/twenty-server/src/engine/core-modules/core-engine.module.ts index b3df8b1823..a48380094c 100644 --- a/packages/twenty-server/src/engine/core-modules/core-engine.module.ts +++ b/packages/twenty-server/src/engine/core-modules/core-engine.module.ts @@ -20,6 +20,7 @@ import { BillingWebhookModule } from 'src/engine/core-modules/billing-webhook/bi import { AppBillingModule } from 'src/engine/core-modules/billing/app-billing/app-billing.module'; import { BillingModule } from 'src/engine/core-modules/billing/billing.module'; import { BillingGraphqlApiExceptionFilter } from 'src/engine/core-modules/billing/filters/billing-graphql-api-exception.filter'; +import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter'; import { CacheStorageModule } from 'src/engine/core-modules/cache-storage/cache-storage.module'; import { TimelineCalendarEventModule } from 'src/engine/core-modules/calendar/timeline-calendar-event.module'; import { CaptchaModule } from 'src/engine/core-modules/captcha/captcha.module'; @@ -174,6 +175,10 @@ import { FileModule } from './file/file.module'; provide: APP_FILTER, useClass: BillingGraphqlApiExceptionFilter, }, + { + provide: APP_FILTER, + useClass: PermissionsGraphqlApiExceptionFilter, + }, ], exports: [ EventLogsViewerModule, diff --git a/packages/twenty-server/src/engine/metadata-modules/metadata-engine.module.ts b/packages/twenty-server/src/engine/metadata-modules/metadata-engine.module.ts index 2f4818241d..a80a88fa44 100644 --- a/packages/twenty-server/src/engine/metadata-modules/metadata-engine.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/metadata-engine.module.ts @@ -13,6 +13,7 @@ import { ConnectedAccountMetadataModule } from 'src/engine/metadata-modules/conn import { CommandMenuItemModule } from 'src/engine/metadata-modules/command-menu-item/command-menu-item.module'; import { FieldMetadataModule } from 'src/engine/metadata-modules/field-metadata/field-metadata.module'; import { FlatEntityMapsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/flat-entity/filters/flat-entity-maps-graphql-api-exception.filter'; +import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter'; import { FrontComponentModule } from 'src/engine/metadata-modules/front-component/front-component.module'; import { LogicFunctionLayerModule } from 'src/engine/metadata-modules/logic-function-layer/logic-function-layer.module'; import { LogicFunctionModule } from 'src/engine/metadata-modules/logic-function/logic-function.module'; @@ -69,6 +70,10 @@ import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/work provide: APP_FILTER, useClass: FlatEntityMapsGraphqlApiExceptionFilter, }, + { + provide: APP_FILTER, + useClass: PermissionsGraphqlApiExceptionFilter, + }, ], exports: [ FieldMetadataModule, diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter.ts index 8a3365158c..d8dfbe91c3 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter.ts @@ -1,11 +1,20 @@ -import { Catch, type ExceptionFilter } from '@nestjs/common'; +import { + type ArgumentsHost, + Catch, + type ExceptionFilter, +} from '@nestjs/common'; +import { type GqlContextType } from '@nestjs/graphql'; import { PermissionsException } from 'src/engine/metadata-modules/permissions/permissions.exception'; import { permissionGraphqlApiExceptionHandler } from 'src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util'; @Catch(PermissionsException) export class PermissionsGraphqlApiExceptionFilter implements ExceptionFilter { - catch(exception: PermissionsException) { + catch(exception: PermissionsException, host: ArgumentsHost) { + if (host.getType() !== 'graphql') { + throw exception; + } + return permissionGraphqlApiExceptionHandler(exception); } } diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/granular-settings-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/granular-settings-permissions.integration-spec.ts index e64f6e766b..f4ab63405c 100644 --- a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/granular-settings-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/granular-settings-permissions.integration-spec.ts @@ -300,6 +300,34 @@ describe('Granular settings permissions', () => { expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); }); + it('should deny access to applications operations when user does not have APPLICATIONS setting permission', async () => { + const findOneApplicationQuery = { + query: ` + query FindOneApplication { + findOneApplication(id: "20202020-1c25-4d02-bf25-6aeccf7ea419") { + applicationVariables { + key + value + } + } + } + `, + }; + + const response = await client + .post('/metadata') + .set('Authorization', `Bearer ${APPLE_JONY_MEMBER_ACCESS_TOKEN}`) + .send(findOneApplicationQuery); + + expect(response.status).toBe(200); + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); + it('should deny access to API keys operations when user does not have API_KEYS_AND_WEBHOOKS setting permission', async () => { // Test creating an API key (requires API_KEYS_AND_WEBHOOKS permission) const createApiKeyQuery = {