fix(server): scope server-route target dispatch to the resolver's application (#22101)
## 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)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22101?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+58
-3
@@ -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',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+36
-2
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user