fix(server): restrict /webhooks/server dispatch to server-route-exposed functions (#22469)
## What `ServerRouteTriggerService.findResolver` resolved a logic function purely by `universalIdentifier` and app-registration ownership, then executed it before its resolver result shape was validated. As a result the public `/webhooks/server/:universalIdentifier` route could dispatch any owner-workspace app function — including ones exposed only as authenticated HTTP routes, tools, or workflow actions — instead of only functions declared as server-route resolvers. ## Change `findResolver` now requires `serverRouteTriggerSettings`: - DB predicate `serverRouteTriggerSettings: Not(IsNull())`, so non-exposed functions are never fetched - in-memory `isDefined(...)` guard alongside the existing owner-workspace check A function that did not opt into server-route exposure is now rejected at `findResolver`, before any execution. A legitimately exposed resolver is unaffected. ## Tests - Unit (`server-route-trigger.service.spec.ts`): asserts the resolver query carries the exposure predicate, and that an owner-workspace function without `serverRouteTriggerSettings` is rejected and never handed to the executor. - Integration (`server-route-trigger-authorization.integration-spec.ts`): exercises the public endpoint end to end — a non-exposed owner-workspace function is rejected before execution, while a server-route-exposed resolver still passes the boundary. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22469?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:
+87
-111
@@ -46,35 +46,59 @@ const buildRequest = (body: object | null = {}): Request =>
|
||||
body,
|
||||
}) as unknown as Request;
|
||||
|
||||
type QueryBuilderMock = {
|
||||
innerJoinAndSelect: jest.Mock;
|
||||
where: jest.Mock;
|
||||
andWhere: jest.Mock;
|
||||
getOne: jest.Mock;
|
||||
};
|
||||
|
||||
const buildResolverRow = () => ({
|
||||
id: 'resolver-id',
|
||||
universalIdentifier: RESOLVER_UID,
|
||||
workspaceId: 'owner-ws',
|
||||
serverRouteTriggerSettings: { forwardedRequestHeaders: ['x-test'] },
|
||||
application: {
|
||||
applicationRegistration: { id: 'reg-1', ownerWorkspaceId: 'owner-ws' },
|
||||
},
|
||||
});
|
||||
|
||||
describe('ServerRouteTriggerService', () => {
|
||||
let service: ServerRouteTriggerService;
|
||||
let logicFunctionRepository: jest.Mocked<
|
||||
Pick<Repository<LogicFunctionEntity>, 'find' | 'findOne'>
|
||||
>;
|
||||
let logicFunctionRepository: {
|
||||
createQueryBuilder: jest.Mock;
|
||||
findOne: jest.Mock;
|
||||
};
|
||||
let logicFunctionExecutorService: jest.Mocked<
|
||||
Pick<LogicFunctionExecutorService, 'execute'>
|
||||
>;
|
||||
|
||||
let resolverRow: unknown;
|
||||
let queryBuilder: QueryBuilderMock;
|
||||
|
||||
const buildQueryBuilderMock = (): QueryBuilderMock => {
|
||||
const qb: QueryBuilderMock = {
|
||||
innerJoinAndSelect: jest.fn(() => qb),
|
||||
where: jest.fn(() => qb),
|
||||
andWhere: jest.fn(() => qb),
|
||||
getOne: jest.fn(() => Promise.resolve(resolverRow)),
|
||||
};
|
||||
|
||||
return qb;
|
||||
};
|
||||
|
||||
const handle = () =>
|
||||
service.handle({
|
||||
request: buildRequest(),
|
||||
resolverLogicFunctionUniversalIdentifier: RESOLVER_UID,
|
||||
});
|
||||
|
||||
const buildResolverRow = (overrides: Record<string, unknown> = {}) => ({
|
||||
id: 'resolver-id',
|
||||
universalIdentifier: RESOLVER_UID,
|
||||
workspaceId: 'owner-ws',
|
||||
serverRouteTriggerSettings: { forwardedRequestHeaders: ['x-test'] },
|
||||
application: {
|
||||
applicationRegistration: { id: 'reg-1', ownerWorkspaceId: 'owner-ws' },
|
||||
},
|
||||
...overrides,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
resolverRow = buildResolverRow();
|
||||
queryBuilder = buildQueryBuilderMock();
|
||||
|
||||
logicFunctionRepository = {
|
||||
find: jest.fn().mockResolvedValue([buildResolverRow()]),
|
||||
createQueryBuilder: jest.fn(() => queryBuilder),
|
||||
findOne: jest
|
||||
.fn()
|
||||
// resolver lookup inside runFunction
|
||||
@@ -129,68 +153,56 @@ describe('ServerRouteTriggerService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('throws LOGIC_FUNCTION_NOT_FOUND when no row matches the universalIdentifier', async () => {
|
||||
logicFunctionRepository.find.mockResolvedValue([]);
|
||||
|
||||
await expect(handle()).rejects.toMatchObject({
|
||||
code: ServerRouteTriggerExceptionCode.LOGIC_FUNCTION_NOT_FOUND,
|
||||
});
|
||||
});
|
||||
|
||||
it('throws LOGIC_FUNCTION_NOT_FOUND when only non-owner-workspace copies exist', async () => {
|
||||
logicFunctionRepository.find.mockResolvedValue([
|
||||
buildResolverRow({
|
||||
workspaceId: 'other-ws',
|
||||
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('picks the owner-workspace copy when multiple workspaces installed the app', async () => {
|
||||
logicFunctionRepository.find.mockResolvedValue([
|
||||
buildResolverRow({
|
||||
id: 'tenant-copy',
|
||||
workspaceId: 'tenant-ws',
|
||||
application: {
|
||||
applicationRegistration: {
|
||||
id: 'reg-1',
|
||||
ownerWorkspaceId: 'owner-ws',
|
||||
},
|
||||
},
|
||||
}),
|
||||
buildResolverRow({
|
||||
id: 'owner-copy',
|
||||
workspaceId: 'owner-ws',
|
||||
application: {
|
||||
applicationRegistration: {
|
||||
id: 'reg-1',
|
||||
ownerWorkspaceId: 'owner-ws',
|
||||
},
|
||||
},
|
||||
}),
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
] as any);
|
||||
|
||||
it('queries the resolver by universalIdentifier and joins the application registration chain', async () => {
|
||||
await handle();
|
||||
|
||||
// runFunction's internal findOne is called with the
|
||||
// (universalIdentifier, workspaceId) of the owner-workspace copy.
|
||||
expect(logicFunctionRepository.findOne).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({
|
||||
universalIdentifier: RESOLVER_UID,
|
||||
workspaceId: 'owner-ws',
|
||||
}),
|
||||
}),
|
||||
expect(logicFunctionRepository.createQueryBuilder).toHaveBeenCalledWith(
|
||||
'logicFunction',
|
||||
);
|
||||
expect(queryBuilder.innerJoinAndSelect).toHaveBeenCalledWith(
|
||||
'logicFunction.application',
|
||||
'application',
|
||||
);
|
||||
expect(queryBuilder.innerJoinAndSelect).toHaveBeenCalledWith(
|
||||
'application.applicationRegistration',
|
||||
'applicationRegistration',
|
||||
);
|
||||
expect(queryBuilder.where).toHaveBeenCalledWith(
|
||||
'logicFunction.universalIdentifier = :universalIdentifier',
|
||||
{ universalIdentifier: RESOLVER_UID },
|
||||
);
|
||||
});
|
||||
|
||||
it('restricts the resolver query to server-route-exposed, owner-workspace functions', async () => {
|
||||
await handle();
|
||||
|
||||
expect(queryBuilder.andWhere).toHaveBeenCalledWith(
|
||||
'logicFunction.serverRouteTriggerSettings IS NOT NULL',
|
||||
);
|
||||
expect(queryBuilder.andWhere).toHaveBeenCalledWith(
|
||||
'logicFunction.workspaceId = applicationRegistration.ownerWorkspaceId',
|
||||
);
|
||||
});
|
||||
|
||||
it('throws LOGIC_FUNCTION_NOT_FOUND and executes nothing when the query returns no server-route resolver', async () => {
|
||||
resolverRow = null;
|
||||
|
||||
await expect(handle()).rejects.toMatchObject({
|
||||
code: ServerRouteTriggerExceptionCode.LOGIC_FUNCTION_NOT_FOUND,
|
||||
});
|
||||
expect(logicFunctionExecutorService.execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects and executes nothing when the resolver requires authentication', async () => {
|
||||
resolverRow = {
|
||||
...buildResolverRow(),
|
||||
httpRouteTriggerSettings: { isAuthRequired: true },
|
||||
};
|
||||
|
||||
await expect(handle()).rejects.toMatchObject({
|
||||
code: ServerRouteTriggerExceptionCode.RESOLVER_REQUIRES_AUTHENTICATION,
|
||||
});
|
||||
expect(logicFunctionExecutorService.execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws RESOLVER_INVALID_RESULT when the resolver does not return a workspaceId', async () => {
|
||||
@@ -232,11 +244,7 @@ describe('ServerRouteTriggerService', () => {
|
||||
logicFunctionRepository.findOne.mockReset();
|
||||
logicFunctionRepository.findOne
|
||||
// resolver lookup succeeds
|
||||
.mockResolvedValueOnce({
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
id: 'resolver-id',
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
} as any)
|
||||
.mockResolvedValueOnce({ id: 'resolver-id' })
|
||||
// target lookup returns null
|
||||
.mockResolvedValueOnce(null);
|
||||
|
||||
@@ -286,23 +294,6 @@ describe('ServerRouteTriggerService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('looks up the resolver by universalIdentifier and loads the application registration chain', async () => {
|
||||
await handle();
|
||||
|
||||
const findArgs = logicFunctionRepository.find.mock.calls[0][0];
|
||||
|
||||
expect(findArgs?.where).toEqual(
|
||||
expect.objectContaining({ universalIdentifier: RESOLVER_UID }),
|
||||
);
|
||||
expect(findArgs?.relations).toEqual(
|
||||
expect.objectContaining({
|
||||
application: expect.objectContaining({
|
||||
applicationRegistration: true,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('maps a LogicFunctionExecutionException(RATE_LIMIT_EXCEEDED) to the server-route rate-limit code', async () => {
|
||||
logicFunctionExecutorService.execute.mockReset();
|
||||
logicFunctionExecutorService.execute.mockRejectedValue(
|
||||
@@ -332,21 +323,6 @@ describe('ServerRouteTriggerService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
+9
@@ -63,6 +63,15 @@ export class ServerRouteTriggerRestApiExceptionFilter implements ExceptionFilter
|
||||
undefined,
|
||||
{ shouldBeCapturedBySentry: false },
|
||||
);
|
||||
case ServerRouteTriggerExceptionCode.RESOLVER_REQUIRES_AUTHENTICATION:
|
||||
return this.httpExceptionHandlerService.handleError(
|
||||
exception as CustomException,
|
||||
response,
|
||||
403,
|
||||
undefined,
|
||||
undefined,
|
||||
{ shouldBeCapturedBySentry: false },
|
||||
);
|
||||
default: {
|
||||
return this.httpExceptionHandlerService.handleError(
|
||||
exception as CustomException,
|
||||
|
||||
+3
@@ -10,6 +10,7 @@ export enum ServerRouteTriggerExceptionCode {
|
||||
SERVER_ROUTE_USER_UNCAUGHT_ERROR = 'SERVER_ROUTE_USER_UNCAUGHT_ERROR',
|
||||
SERVER_ROUTE_PLATFORM_ERROR = 'SERVER_ROUTE_PLATFORM_ERROR',
|
||||
RESOLVER_INVALID_RESULT = 'RESOLVER_INVALID_RESULT',
|
||||
RESOLVER_REQUIRES_AUTHENTICATION = 'RESOLVER_REQUIRES_AUTHENTICATION',
|
||||
}
|
||||
|
||||
const getServerRouteTriggerExceptionUserFriendlyMessage = (
|
||||
@@ -26,6 +27,8 @@ const getServerRouteTriggerExceptionUserFriendlyMessage = (
|
||||
return msg`An unexpected error occurred while handling the server route.`;
|
||||
case ServerRouteTriggerExceptionCode.RESOLVER_INVALID_RESULT:
|
||||
return msg`Resolver logic function returned an invalid result.`;
|
||||
case ServerRouteTriggerExceptionCode.RESOLVER_REQUIRES_AUTHENTICATION:
|
||||
return msg`Server logic function requires authentication.`;
|
||||
default:
|
||||
assertUnreachable(code);
|
||||
}
|
||||
|
||||
+22
-11
@@ -57,6 +57,13 @@ export class ServerRouteTriggerService {
|
||||
);
|
||||
}
|
||||
|
||||
if (resolver.httpRouteTriggerSettings?.isAuthRequired === true) {
|
||||
throw new ServerRouteTriggerException(
|
||||
`Server resolver function ${resolverLogicFunctionUniversalIdentifier} requires authentication and cannot be dispatched through the public server route`,
|
||||
ServerRouteTriggerExceptionCode.RESOLVER_REQUIRES_AUTHENTICATION,
|
||||
);
|
||||
}
|
||||
|
||||
const applicationRegistrationId =
|
||||
resolver.application?.applicationRegistration?.id;
|
||||
|
||||
@@ -105,18 +112,22 @@ export class ServerRouteTriggerService {
|
||||
}: {
|
||||
logicFunctionUniversalIdentifier: string;
|
||||
}): Promise<LogicFunctionEntity | null> {
|
||||
const candidates = await this.logicFunctionRepository.find({
|
||||
where: { universalIdentifier: logicFunctionUniversalIdentifier },
|
||||
relations: { application: { applicationRegistration: true } },
|
||||
});
|
||||
|
||||
return (
|
||||
candidates.find(
|
||||
(candidate) =>
|
||||
isDefined(candidate.application?.applicationRegistration) &&
|
||||
candidate.workspaceId ===
|
||||
candidate.application.applicationRegistration.ownerWorkspaceId,
|
||||
) ?? null
|
||||
(await this.logicFunctionRepository
|
||||
.createQueryBuilder('logicFunction')
|
||||
.innerJoinAndSelect('logicFunction.application', 'application')
|
||||
.innerJoinAndSelect(
|
||||
'application.applicationRegistration',
|
||||
'applicationRegistration',
|
||||
)
|
||||
.where('logicFunction.universalIdentifier = :universalIdentifier', {
|
||||
universalIdentifier: logicFunctionUniversalIdentifier,
|
||||
})
|
||||
.andWhere('logicFunction.serverRouteTriggerSettings IS NOT NULL')
|
||||
.andWhere(
|
||||
'logicFunction.workspaceId = applicationRegistration.ownerWorkspaceId',
|
||||
)
|
||||
.getOne()) ?? null
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user