4ff9cba76d
## Context Sentry [TWENTY-SERVER-60Y](https://twenty-v7.sentry.io/issues/6633503406) ("Permission Denied: Entity performing the request does not have permission") is still firing at full rate on `v2.25.0`: ~10.8k events in the last 7 days, 24k total. #23104 tried to fix it by registering `PermissionsGraphqlApiExceptionFilter` globally via `APP_FILTER`. That registration is correct but **inert in production**, and the integration test added alongside it passes for a reason unrelated to prod behaviour. ## Root cause `main.ts` registered a catch-all filter after bootstrap: ```ts app.useGlobalFilters(new UnhandledExceptionFilter()); ``` Nest builds each resolver's filter list as `[...global, ...class, ...method]`, reverses it, and selects **exactly one** matching filter — there is no chaining. `APP_FILTER` providers are collected during module scan; `useGlobalFilters` appends after that, so the catch-all ended up at the head of the list: ``` 1. UnhandledExceptionFilter @Catch() <- matches everything, wins 2. PermissionsGraphqlApiExceptionFilter <- never reached 3. BillingGraphqlApiExceptionFilter <- never reached ``` On a GraphQL host `UnhandledExceptionFilter` then no-ops: `host.switchToHttp().getResponse()` returns the GraphQL args object, `response.header` is undefined, so it hits `return;`. Nest treats a falsy return as unhandled and rethrows the original `PermissionsException`, which reaches the Yoga error hook as a non-`BaseGraphQLError`, is serialized `INTERNAL_SERVER_ERROR`, and is reported by `shouldCaptureException`. The 28 resolvers carrying `@UseFilters(PermissionsGraphqlApiExceptionFilter)` were unaffected — method-level filters are evaluated before globals. Only the resolvers relying on the global registration leaked, which is exactly the set showing up in Sentry (`findOneApplication`, `uploadFilesFieldFileByUniversalIdentifier`, `UpdatePageLayoutWithTabsAndWidgets`, ...). Two other global filters were shadowed the same way and have never run: `BillingGraphqlApiExceptionFilter` and `FlatEntityMapsGraphqlApiExceptionFilter`. ## Why the existing test did not catch it `test/integration/utils/create-app.ts` builds the app from `AppModule` directly and never executes `main.ts`, so `useGlobalFilters` does not exist in the test process. It registered `MockedUnhandledExceptionFilter` as an `APP_FILTER` on the root testing module, which is collected *first* and therefore evaluated *last* — the exact inverse of production precedence. The `findOneApplication` denial test passed while the same query kept reporting to Sentry. ## Fix Register `UnhandledExceptionFilter` through `APP_FILTER` on `AppModule`. Root-module providers are scanned first, so it is collected first and evaluated last. The filter stays global, stays catch-all, and keeps its CORS-header role for HTTP; it simply no longer cuts in front of the typed filters. Un-shadowing the other two global filters means they now actually run, so `FileStorageExceptionFilter` and `FlatEntityMapsGraphqlApiExceptionFilter` get the `host.getType() !== 'graphql'` rethrow that `Billing` and `Permissions` already had. Without it they would start throwing GraphQL error objects into the REST pipeline. `MockedUnhandledExceptionFilter` is removed: `AppModule` now supplies the real filter in the same position, so the mock was dead weight. ## Test Verified against a real server (not the integration harness), calling the exact document from Sentry event `8d19eb7c` as a member with no permission flags: ``` query ($v1:UUID){findOneApplication(id:$v1){applicationVariables{key,value}}} ``` | | response code | exceptions captured | |---|---|---| | before | `INTERNAL_SERVER_ERROR` | 1 | | after | `FORBIDDEN` | 0 | Capture count measured through the console exception-handler driver, i.e. the same `captureExceptions` call site that is the Sentry driver in production. New unit spec `src/filters/__tests__/unhandled-exception.filter.spec.ts` boots a Nest + Yoga app both ways: it asserts `FORBIDDEN` with the `APP_FILTER` registration, and pins the shadowing behaviour of `app.useGlobalFilters` so the pattern cannot come back unnoticed. `granular-settings-permissions.integration-spec.ts` passes (10/10). Note it also passes *without* this fix — the harness cannot observe bootstrap-only configuration, which is the underlying reason #23104 shipped green. Closing that gap properly means sharing the post-`create` bootstrap between `main.ts` and `create-app.ts`; left as a follow-up. `file-storage-exception-filter.spec.ts` extended with a non-GraphQL host case. ## CI follow-up `failing-file-by-id-download.integration-spec.ts` snapshots were updated. That REST endpoint's 403 body changed in tests from `{}` to `{"statusCode":403,"error":"Forbidden","message":"Forbidden resource"}`. The old `{}` was an artifact of the mock: `MockedUnhandledExceptionFilter` rethrew, the exception escaped Nest's handler into Express's default error handler, and supertest saw an empty body. Production has always run the real `UnhandledExceptionFilter`, which writes `response.status(status).json(exception.response)` — the new snapshot. Production HTTP behaviour is unchanged by this PR: no other global filter matches an `HttpException` (the typed ones rethrow outside GraphQL), so the same filter handles it whether it is evaluated first or last.
155 lines
5.6 KiB
TypeScript
155 lines
5.6 KiB
TypeScript
import {
|
|
type DynamicModule,
|
|
type MiddlewareConsumer,
|
|
Module,
|
|
RequestMethod,
|
|
} from '@nestjs/common';
|
|
import { APP_FILTER } from '@nestjs/core';
|
|
import { GraphQLModule } from '@nestjs/graphql';
|
|
import { ServeStaticModule } from '@nestjs/serve-static';
|
|
|
|
import { existsSync } from 'fs';
|
|
import { join } from 'path';
|
|
|
|
import { YogaDriver, type YogaDriverConfig } from '@graphql-yoga/nestjs';
|
|
import { SentryModule } from '@sentry/nestjs/setup';
|
|
|
|
import { AdminPanelGraphQLApiModule } from 'src/engine/api/graphql/admin-panel-graphql-api.module';
|
|
import { CoreGraphQLApiModule } from 'src/engine/api/graphql/core-graphql-api.module';
|
|
import { GraphQLConfigModule } from 'src/engine/api/graphql/graphql-config/graphql-config.module';
|
|
import { GraphQLConfigService } from 'src/engine/api/graphql/graphql-config/graphql-config.service';
|
|
import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module';
|
|
import { McpMethodGuardMiddleware } from 'src/engine/api/mcp/middlewares/mcp-method-guard.middleware';
|
|
import { McpModule } from 'src/engine/api/mcp/mcp.module';
|
|
import { RestApiModule } from 'src/engine/api/rest/rest-api.module';
|
|
import { WorkspaceAuthContextMiddleware } from 'src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware';
|
|
import { MetricsModule } from 'src/engine/core-modules/metrics/metrics.module';
|
|
import { DataloaderModule } from 'src/engine/dataloaders/dataloader.module';
|
|
import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/workspace-metadata-version/workspace-metadata-version.module';
|
|
import { GraphQLHydrateRequestFromTokenMiddleware } from 'src/engine/middlewares/graphql-hydrate-request-from-token.middleware';
|
|
import { MiddlewareModule } from 'src/engine/middlewares/middleware.module';
|
|
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';
|
|
import { CoreEngineModule } from './engine/core-modules/core-engine.module';
|
|
import { I18nModule } from './engine/core-modules/i18n/i18n.module';
|
|
|
|
// TODO: Remove this middleware when all the rest endpoints are migrated to TwentyORM
|
|
const MIGRATED_REST_METHODS = [
|
|
RequestMethod.DELETE,
|
|
RequestMethod.POST,
|
|
RequestMethod.PATCH,
|
|
RequestMethod.PUT,
|
|
RequestMethod.GET,
|
|
];
|
|
|
|
@Module({
|
|
imports: [
|
|
SentryModule.forRoot(),
|
|
GraphQLModule.forRootAsync<YogaDriverConfig>({
|
|
driver: YogaDriver,
|
|
imports: [GraphQLConfigModule, MetricsModule, DataloaderModule],
|
|
useClass: GraphQLConfigService,
|
|
}),
|
|
TwentyORMModule,
|
|
GlobalWorkspaceDataSourceModule,
|
|
ClickHouseModule,
|
|
// Core engine module, contains all the core modules
|
|
CoreEngineModule,
|
|
// Modules module, contains all business logic modules
|
|
ModulesModule,
|
|
// Needed for the user workspace middleware
|
|
WorkspaceCacheStorageModule,
|
|
// Api modules
|
|
CoreGraphQLApiModule,
|
|
MetadataGraphQLApiModule,
|
|
AdminPanelGraphQLApiModule,
|
|
RestApiModule,
|
|
McpModule,
|
|
MiddlewareModule,
|
|
WorkspaceMetadataVersionModule,
|
|
// I18n module for translations
|
|
I18nModule,
|
|
// Conditional modules
|
|
...AppModule.getConditionalModules(),
|
|
],
|
|
providers: [
|
|
{
|
|
provide: APP_FILTER,
|
|
useClass: UnhandledExceptionFilter,
|
|
},
|
|
],
|
|
})
|
|
export class AppModule {
|
|
private static getConditionalModules(): DynamicModule[] {
|
|
const modules: DynamicModule[] = [];
|
|
const frontPath = join(__dirname, 'front');
|
|
|
|
// NestJS DevTools - can be useful for debugging and profiling
|
|
/* if (process.env.NODE_ENV === NodeEnvironment.DEVELOPMENT) {
|
|
modules.push(
|
|
DevtoolsModule.register({
|
|
http: true,
|
|
}),
|
|
);
|
|
} */
|
|
|
|
if (existsSync(frontPath)) {
|
|
modules.push(
|
|
ServeStaticModule.forRoot({
|
|
rootPath: frontPath,
|
|
}),
|
|
);
|
|
}
|
|
|
|
// Messaque Queue explorer only for sync driver
|
|
// Maybe we don't need to conditionaly register the explorer, because we're creating a jobs module
|
|
// that will expose classes that are only used in the queue worker
|
|
/*
|
|
if (process.env.MESSAGE_QUEUE_TYPE === MessageQueueDriverType.Sync) {
|
|
modules.push(MessageQueueModule.registerExplorer());
|
|
}
|
|
*/
|
|
|
|
return modules;
|
|
}
|
|
|
|
configure(consumer: MiddlewareConsumer) {
|
|
consumer
|
|
.apply(
|
|
GraphQLHydrateRequestFromTokenMiddleware,
|
|
WorkspaceAuthContextMiddleware,
|
|
)
|
|
.forRoutes({ path: 'graphql', method: RequestMethod.ALL });
|
|
|
|
consumer
|
|
.apply(
|
|
GraphQLHydrateRequestFromTokenMiddleware,
|
|
WorkspaceAuthContextMiddleware,
|
|
)
|
|
.forRoutes({ path: 'metadata', method: RequestMethod.ALL });
|
|
|
|
consumer
|
|
.apply(
|
|
GraphQLHydrateRequestFromTokenMiddleware,
|
|
WorkspaceAuthContextMiddleware,
|
|
)
|
|
.forRoutes({ path: 'admin-panel', method: RequestMethod.ALL });
|
|
|
|
consumer
|
|
.apply(McpMethodGuardMiddleware)
|
|
.forRoutes({ path: 'mcp', method: RequestMethod.ALL });
|
|
|
|
for (const method of MIGRATED_REST_METHODS) {
|
|
consumer
|
|
.apply(RestCoreMiddleware, WorkspaceAuthContextMiddleware)
|
|
.forRoutes({ path: 'rest/*path', method });
|
|
}
|
|
}
|
|
}
|