From bade5289b67d8c28e7f5d7f2e5ea3fb6d13884ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Mon, 9 Feb 2026 09:23:57 +0100 Subject: [PATCH] Fix Cloudflare webhook guard validation logic (#17780) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Restructures the `CloudflareSecretMatchGuard` to properly validate the `cf-webhook-auth` header before performing the comparison — checks for missing, empty, or mismatched-length values first - Removes `@ts-expect-error` workarounds with explicit typed header access - Expands test coverage with additional edge cases (missing header, wrong value, empty string, length mismatch) ## Test plan - [x] Unit tests pass (6 tests covering matching, missing config, missing header, wrong value, empty string, length mismatch) Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --- .../guards/cloudflare-secret.guard.ts | 40 +++++---- .../guards/cloudflare-secret.spec.ts | 83 ++++++++++++------- 2 files changed, 76 insertions(+), 47 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.guard.ts b/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.guard.ts index 912158f2118..32dc968be2e 100644 --- a/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.guard.ts +++ b/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.guard.ts @@ -4,6 +4,7 @@ import { type CanActivate, type ExecutionContext, Injectable, + InternalServerErrorException, } from '@nestjs/common'; import { timingSafeEqual } from 'crypto'; @@ -15,24 +16,33 @@ export class CloudflareSecretMatchGuard implements CanActivate { constructor(private readonly twentyConfigService: TwentyConfigService) {} canActivate(context: ExecutionContext): boolean { - try { - const request = context.switchToHttp().getRequest(); + const cloudflareWebhookSecret = this.twentyConfigService.get( + 'CLOUDFLARE_WEBHOOK_SECRET', + ); - const cloudflareWebhookSecret = this.twentyConfigService.get( - 'CLOUDFLARE_WEBHOOK_SECRET', + if (!cloudflareWebhookSecret) { + throw new InternalServerErrorException( + 'CLOUDFLARE_WEBHOOK_SECRET is not configured', ); + } - if ( - !cloudflareWebhookSecret || - (cloudflareWebhookSecret && - // @ts-expect-error legacy noImplicitAny - (typeof request.headers['cf-webhook-auth'] === 'string' || - timingSafeEqual( - // @ts-expect-error legacy noImplicitAny - Buffer.from(request.headers['cf-webhook-auth']), - Buffer.from(cloudflareWebhookSecret), - ))) - ) { + try { + const request = context.switchToHttp().getRequest(); + + const headerValue = request.headers['cf-webhook-auth']; + + if (typeof headerValue !== 'string' || headerValue.length === 0) { + return false; + } + + const headerBuffer = Buffer.from(headerValue); + const secretBuffer = Buffer.from(cloudflareWebhookSecret); + + if (headerBuffer.length !== secretBuffer.length) { + return false; + } + + if (timingSafeEqual(headerBuffer, secretBuffer)) { return true; } diff --git a/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.spec.ts b/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.spec.ts index d5d4a027da5..b51ff48bec5 100644 --- a/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/cloudflare/guards/cloudflare-secret.spec.ts @@ -1,10 +1,15 @@ import { type ExecutionContext } from '@nestjs/common'; -import * as crypto from 'crypto'; - import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { CloudflareSecretMatchGuard } from 'src/engine/core-modules/cloudflare/guards/cloudflare-secret.guard'; +const buildMockContext = (headers: Record) => + ({ + switchToHttp: () => ({ + getRequest: () => ({ headers }), + }), + }) as unknown as ExecutionContext; + describe('CloudflareSecretMatchGuard.canActivate', () => { let guard: CloudflareSecretMatchGuard; let twentyConfigService: TwentyConfigService; @@ -17,48 +22,62 @@ describe('CloudflareSecretMatchGuard.canActivate', () => { }); it('should return true when the webhook secret matches', () => { - const mockRequest = { headers: { 'cf-webhook-auth': 'valid-secret' } }; - jest.spyOn(twentyConfigService, 'get').mockReturnValue('valid-secret'); - const mockContext = { - switchToHttp: () => ({ - getRequest: () => mockRequest, - }), - } as unknown as ExecutionContext; + const context = buildMockContext({ + 'cf-webhook-auth': 'valid-secret', + }); - jest.spyOn(crypto, 'timingSafeEqual').mockReturnValue(true); - - expect(guard.canActivate(mockContext)).toBe(true); + expect(guard.canActivate(context)).toBe(true); }); - it('should return true when env is not set', () => { - const mockRequest = { headers: { 'cf-webhook-auth': 'valid-secret' } }; - + it('should throw InternalServerErrorException when env is not set', () => { jest.spyOn(twentyConfigService, 'get').mockReturnValue(undefined); - const mockContext = { - switchToHttp: () => ({ - getRequest: () => mockRequest, - }), - } as unknown as ExecutionContext; + const context = buildMockContext({ + 'cf-webhook-auth': 'any-value', + }); - jest.spyOn(crypto, 'timingSafeEqual').mockReturnValue(true); - - expect(guard.canActivate(mockContext)).toBe(true); + expect(() => guard.canActivate(context)).toThrow( + 'CLOUDFLARE_WEBHOOK_SECRET is not configured', + ); }); - it('should return false if an error occurs', () => { - const mockRequest = { headers: {} }; - + it('should return false when the header is missing', () => { jest.spyOn(twentyConfigService, 'get').mockReturnValue('valid-secret'); - const mockContext = { - switchToHttp: () => ({ - getRequest: () => mockRequest, - }), - } as unknown as ExecutionContext; + const context = buildMockContext({}); - expect(guard.canActivate(mockContext)).toBe(false); + expect(guard.canActivate(context)).toBe(false); + }); + + it('should return false when the header value is wrong', () => { + jest.spyOn(twentyConfigService, 'get').mockReturnValue('valid-secret'); + + const context = buildMockContext({ + 'cf-webhook-auth': 'wrong-secret', + }); + + expect(guard.canActivate(context)).toBe(false); + }); + + it('should return false when the header is an empty string', () => { + jest.spyOn(twentyConfigService, 'get').mockReturnValue('valid-secret'); + + const context = buildMockContext({ + 'cf-webhook-auth': '', + }); + + expect(guard.canActivate(context)).toBe(false); + }); + + it('should return false when header length differs from secret', () => { + jest.spyOn(twentyConfigService, 'get').mockReturnValue('short'); + + const context = buildMockContext({ + 'cf-webhook-auth': 'much-longer-value', + }); + + expect(guard.canActivate(context)).toBe(false); }); });