From cc21160d838018f707272c7cf844dfc393901e0b Mon Sep 17 00:00:00 2001 From: martmull Date: Wed, 24 Jun 2026 18:22:01 +0200 Subject: [PATCH] fix(server): scope server-route target dispatch to the resolver's application (#22101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Security follow-up to #22002 (server-exposed logic functions). That PR's `ServerRouteTriggerService` resolved the **target** logic function by `(universalIdentifier, workspaceId)` alone, with no application scoping: ```ts // before const logicFunction = await this.logicFunctionRepository.findOne({ where: { universalIdentifier, workspaceId }, }); ``` Both values come straight from the resolver's return value. Because the only gate was "a function with that UID exists in that workspace", a resolver (owner-workspace code) could dispatch to a logic function belonging to a **different application**, or to a workspace where its own application is **not installed**, and read the target's return value back in the HTTP response (`buildRouteTriggerResponse(targetResult.data)`) — a cross-tenant / cross-application isolation break. The implementation this replaced (the deleted `server-webhook-trigger.service.ts`) enforced both checks: the app had to be installed in the target workspace, and the target function was scoped by `applicationId`. This PR restores that guarantee. ## Changes - **Scope the target dispatch to the resolver's `applicationRegistration`.** `handle()` captures `resolver.application.applicationRegistration.id` and threads it into the target `findOne` as `application: { applicationRegistrationId }` (joining the `application` relation). The target must belong to the same registration — which also guarantees the application is installed in the resolved workspace (no installed copy → no matching row). The resolver lookup itself is unchanged. - **Stop leaking raw internal error messages.** The `runFunction` catch block logged the raw executor/`Error.message` *and* returned it to the (unauthenticated) caller. It now logs the detail server-side and returns a generic, per-code message. - **Tests**: fixtures carry an `applicationRegistration.id`; new cases assert the target lookup is scoped to the resolver's registration, that a resolver not linked to a registration is rejected, and that a platform error returns the generic message instead of the raw internal text. Feature remains gated behind `IS_SERVER_LOGIC_FUNCTION_ENABLED` (default off). ## Test plan - [ ] `npx jest server-route-trigger` (verifying locally; environment dependency install was flaky) - [ ] `npx nx typecheck twenty-server` - [ ] `npx nx lint:diff-with-main twenty-server` https://claude.ai/code/session_014TNdRvQjjR8wN6MLTJ7rTE --- _Generated by [Claude Code](https://claude.ai/code/session_014TNdRvQjjR8wN6MLTJ7rTE)_ Review in cubic --- .../server-route-trigger.service.spec.ts | 61 ++++++++++++++++++- .../server-route-trigger.service.ts | 38 +++++++++++- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/server-route-trigger/__tests__/server-route-trigger.service.spec.ts b/packages/twenty-server/src/engine/core-modules/server-route-trigger/__tests__/server-route-trigger.service.spec.ts index b61ef4479e..a791cea707 100644 --- a/packages/twenty-server/src/engine/core-modules/server-route-trigger/__tests__/server-route-trigger.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/server-route-trigger/__tests__/server-route-trigger.service.spec.ts @@ -67,7 +67,7 @@ describe('ServerRouteTriggerService', () => { workspaceId: 'owner-ws', serverRouteTriggerSettings: { forwardedRequestHeaders: ['x-test'] }, application: { - applicationRegistration: { ownerWorkspaceId: 'owner-ws' }, + applicationRegistration: { id: 'reg-1', ownerWorkspaceId: 'owner-ws' }, }, ...overrides, }); @@ -159,14 +159,20 @@ describe('ServerRouteTriggerService', () => { id: 'tenant-copy', workspaceId: 'tenant-ws', application: { - applicationRegistration: { ownerWorkspaceId: 'owner-ws' }, + applicationRegistration: { + id: 'reg-1', + ownerWorkspaceId: 'owner-ws', + }, }, }), buildResolverRow({ id: 'owner-copy', workspaceId: 'owner-ws', application: { - applicationRegistration: { ownerWorkspaceId: 'owner-ws' }, + applicationRegistration: { + id: 'reg-1', + ownerWorkspaceId: 'owner-ws', + }, }, }), // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -310,4 +316,53 @@ describe('ServerRouteTriggerService', () => { code: ServerRouteTriggerExceptionCode.RATE_LIMIT_EXCEEDED, }); }); + + it('scopes the target lookup to the resolver application registration', async () => { + await handle(); + + expect(logicFunctionRepository.findOne).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + where: expect.objectContaining({ + universalIdentifier: TARGET_UID, + workspaceId: 'target-ws', + application: { applicationRegistrationId: 'reg-1' }, + }), + }), + ); + }); + + it('throws LOGIC_FUNCTION_NOT_FOUND when the resolver is not linked to an application registration', async () => { + logicFunctionRepository.find.mockResolvedValue([ + buildResolverRow({ + application: { + applicationRegistration: { ownerWorkspaceId: 'owner-ws' }, + }, + }), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ] as any); + + await expect(handle()).rejects.toMatchObject({ + code: ServerRouteTriggerExceptionCode.LOGIC_FUNCTION_NOT_FOUND, + }); + }); + + it('does not leak the raw executor error message to the caller', async () => { + logicFunctionExecutorService.execute.mockReset(); + logicFunctionExecutorService.execute + .mockResolvedValueOnce( + buildExecuteResult({ + workspaceId: 'target-ws', + targetLogicFunctionUniversalIdentifier: TARGET_UID, + }), + ) + .mockRejectedValueOnce( + new Error('internal: connection to lambda-internal:5000 refused'), + ); + + await expect(handle()).rejects.toMatchObject({ + code: ServerRouteTriggerExceptionCode.SERVER_ROUTE_PLATFORM_ERROR, + message: 'An unexpected error occurred while handling the server route', + }); + }); }); diff --git a/packages/twenty-server/src/engine/core-modules/server-route-trigger/server-route-trigger.service.ts b/packages/twenty-server/src/engine/core-modules/server-route-trigger/server-route-trigger.service.ts index 82c8b8ecf8..99bb8c0859 100644 --- a/packages/twenty-server/src/engine/core-modules/server-route-trigger/server-route-trigger.service.ts +++ b/packages/twenty-server/src/engine/core-modules/server-route-trigger/server-route-trigger.service.ts @@ -57,6 +57,16 @@ export class ServerRouteTriggerService { ); } + const applicationRegistrationId = + resolver.application?.applicationRegistration?.id; + + if (!isDefined(applicationRegistrationId)) { + throw new ServerRouteTriggerException( + `Server resolver function ${resolverLogicFunctionUniversalIdentifier} is not linked to an application registration`, + ServerRouteTriggerExceptionCode.LOGIC_FUNCTION_NOT_FOUND, + ); + } + const event = buildLogicFunctionEvent({ request, pathParameters: {}, @@ -77,6 +87,7 @@ export class ServerRouteTriggerService { resolved.targetLogicFunctionUniversalIdentifier, workspaceId: resolved.workspaceId, payload: resolved.payload ?? event, + applicationRegistrationId, }); if (isDefined(targetResult.error)) { @@ -151,16 +162,24 @@ export class ServerRouteTriggerService { logicFunctionUniversalIdentifier, workspaceId, payload, + applicationRegistrationId, }: { logicFunctionUniversalIdentifier: string; workspaceId: string; payload: object; + applicationRegistrationId?: string; }): Promise<{ data: object | null; error?: { errorMessage: string } }> { const logicFunction = await this.logicFunctionRepository.findOne({ where: { universalIdentifier: logicFunctionUniversalIdentifier, workspaceId, + ...(isDefined(applicationRegistrationId) + ? { application: { applicationRegistrationId } } + : {}), }, + ...(isDefined(applicationRegistrationId) + ? { relations: { application: true } } + : {}), }); if (!isDefined(logicFunction)) { @@ -181,13 +200,28 @@ export class ServerRouteTriggerService { `Server logic function ${logicFunction.id} failed in workspace ${workspaceId}: ${error instanceof Error ? error.message : String(error)}`, error instanceof Error ? error.stack : undefined, ); + const code = this.mapExecutorErrorToServerRouteCode(error); + throw new ServerRouteTriggerException( - error instanceof Error ? error.message : String(error), - this.mapExecutorErrorToServerRouteCode(error), + this.getPublicErrorMessageForCode(code), + code, ); } } + private getPublicErrorMessageForCode( + code: ServerRouteTriggerExceptionCode, + ): string { + switch (code) { + case ServerRouteTriggerExceptionCode.RATE_LIMIT_EXCEEDED: + return 'Rate limit exceeded'; + case ServerRouteTriggerExceptionCode.LOGIC_FUNCTION_NOT_FOUND: + return 'Logic function not found'; + default: + return 'An unexpected error occurred while handling the server route'; + } + } + private mapExecutorErrorToServerRouteCode( error: unknown, ): ServerRouteTriggerExceptionCode {