feat(GitHub Trigger Node): Add automatic webhook signature verification (#24203)

packages/nodes-base/nodes/Github/GithubTrigger.node.ts

@@ -1,3 +1,4 @@
+import { randomBytes } from 'crypto';
 import type {
 	IHookFunctions,
 	IWebhookFunctions,
@@ -10,6 +11,7 @@ import type {
 import { NodeConnectionTypes, NodeApiError, NodeOperationError } from 'n8n-workflow';
 
 import { githubApiRequest } from './GenericFunctions';
+import { verifySignature } from './GithubTriggerHelpers';
 import { getRepositories, getUsers } from './SearchFunctions';
 
 export class GithubTrigger implements INodeType {
@@ -507,12 +509,16 @@ export class GithubTrigger implements INodeType {
 				const endpoint = `/repos/${owner}/${repository}/hooks`;
 				const options = this.getNodeParameter('options') as { insecureSSL: boolean };
 
+				// Generate a secure random secret for webhook signature verification
+				const webhookSecret = randomBytes(32).toString('hex');
+
 				const body = {
 					name: 'web',
 					config: {
 						url: webhookUrl,
 						content_type: 'json',
 						insecure_ssl: options.insecureSSL ? '1' : '0',
+						secret: webhookSecret,
 					},
 					events,
 					active: true,
@@ -538,6 +544,8 @@ export class GithubTrigger implements INodeType {
 									// create it again simply save the webhook-id
 									webhookData.webhookId = webhook.id as string;
 									webhookData.webhookEvents = webhook.events as string[];
+									// Legacy webhook without secret on GitHub's side - not setting webhookData.webhookSecret
+									// so signature verification is skipped. To enable it, deactivate and reactivate the workflow.
 									return true;
 								}
 							}
@@ -570,6 +578,7 @@ export class GithubTrigger implements INodeType {
 
 				webhookData.webhookId = responseData.id as string;
 				webhookData.webhookEvents = responseData.events as string[];
+				webhookData.webhookSecret = webhookSecret;
 
 				return true;
 			},
@@ -594,6 +603,7 @@ export class GithubTrigger implements INodeType {
 					// that no webhooks are registered anymore
 					delete webhookData.webhookId;
 					delete webhookData.webhookEvents;
+					delete webhookData.webhookSecret;
 				}
 
 				return true;
@@ -609,9 +619,19 @@ export class GithubTrigger implements INodeType {
 	};
 
 	async webhook(this: IWebhookFunctions): Promise<IWebhookResponseData> {
+		// Verify the webhook signature before processing
+		if (!verifySignature.call(this)) {
+			const res = this.getResponseObject();
+			res.status(401).send('Unauthorized').end();
+
+			return {
+				noWebhookResponse: true,
+			};
+		}
+
 		const bodyData = this.getBodyData();
 
-		// Check if the webhook is only the ping from Github to confirm if it workshook_id
+		// Check if the webhook is only the ping from Github to confirm if it works
 		if (bodyData.hook_id !== undefined && bodyData.action === undefined) {
 			// Is only the ping and not an actual webhook call. So return 'OK'
 			// but do not start the workflow.

packages/nodes-base/nodes/Github/GithubTriggerHelpers.ts

@@ -0,0 +1,72 @@
+import { createHmac, timingSafeEqual } from 'crypto';
+import type { IWebhookFunctions } from 'n8n-workflow';
+
+/**
+ * Verifies the GitHub webhook signature using HMAC-SHA256.
+ *
+ * GitHub sends a signature in the `X-Hub-Signature-256` header in the format:
+ * `sha256=<HMAC hex digest>`
+ *
+ * This function computes the expected signature using the stored webhook secret
+ * and compares it with the provided signature using a constant-time comparison.
+ *
+ * @returns true if signature is valid or no secret is configured, false otherwise
+ */
+export function verifySignature(this: IWebhookFunctions): boolean {
+	// Get the secret from workflow static data (set during webhook creation)
+	const webhookData = this.getWorkflowStaticData('node');
+	const webhookSecret = webhookData.webhookSecret as string | undefined;
+
+	// If no secret is configured, skip verification (backwards compatibility)
+	if (!webhookSecret) {
+		return true;
+	}
+
+	const req = this.getRequestObject();
+
+	// Get the signature from GitHub's header
+	const signature = req.header('x-hub-signature-256');
+	if (!signature) {
+		return false;
+	}
+
+	// Validate signature format (must start with "sha256=")
+	if (!signature.startsWith('sha256=')) {
+		return false;
+	}
+
+	// Extract just the hex digest part
+	const providedSignature = signature.substring(7);
+
+	try {
+		// Get the raw request body
+		if (!req.rawBody) {
+			return false;
+		}
+
+		// Compute HMAC-SHA256 of the raw body using our secret
+		const hmac = createHmac('sha256', webhookSecret);
+
+		if (Buffer.isBuffer(req.rawBody)) {
+			hmac.update(req.rawBody);
+		} else {
+			const rawBodyString =
+				typeof req.rawBody === 'string' ? req.rawBody : JSON.stringify(req.rawBody);
+			hmac.update(rawBodyString);
+		}
+
+		const computedSignature = hmac.digest('hex');
+
+		const computedBuffer = Buffer.from(computedSignature, 'utf8');
+		const providedBuffer = Buffer.from(providedSignature, 'utf8');
+
+		// Buffers must be same length for timingSafeEqual
+		if (computedBuffer.length !== providedBuffer.length) {
+			return false;
+		}
+
+		return timingSafeEqual(computedBuffer, providedBuffer);
+	} catch {
+		return false;
+	}
+}

packages/nodes-base/nodes/Github/__tests__/GithubTriggerHelpers.test.ts

@@ -0,0 +1,159 @@
+import { createHmac, timingSafeEqual } from 'crypto';
+import { verifySignature } from '../GithubTriggerHelpers';
+
+jest.mock('crypto', () => ({
+	...jest.requireActual('crypto'),
+	createHmac: jest.fn().mockReturnValue({
+		update: jest.fn().mockReturnThis(),
+		digest: jest
+			.fn()
+			.mockReturnValue('757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17'),
+	}),
+	timingSafeEqual: jest.fn(),
+}));
+
+describe('GithubTriggerHelpers', () => {
+	let mockWebhookFunctions: {
+		getWorkflowStaticData: jest.Mock;
+		getRequestObject: jest.Mock;
+	};
+	const testWebhookSecret = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2';
+	const testBody =
+		'{"action":"opened","pull_request":{"id":123},"repository":{"full_name":"owner/repo"}}';
+	const testSignature = 'sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17';
+
+	beforeEach(() => {
+		jest.clearAllMocks();
+
+		mockWebhookFunctions = {
+			getWorkflowStaticData: jest.fn(),
+			getRequestObject: jest.fn(),
+		};
+
+		// Default mock return values
+		mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({
+			webhookSecret: testWebhookSecret,
+		});
+
+		mockWebhookFunctions.getRequestObject.mockReturnValue({
+			header: jest.fn().mockImplementation((header) => {
+				if (header === 'x-hub-signature-256') return testSignature;
+				return null;
+			}),
+			rawBody: testBody,
+		});
+	});
+
+	describe('verifySignature', () => {
+		it('should return true when no webhook secret is stored (backwards compatibility)', () => {
+			mockWebhookFunctions.getWorkflowStaticData.mockReturnValue({});
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(true);
+			expect(mockWebhookFunctions.getWorkflowStaticData).toHaveBeenCalledWith('node');
+		});
+
+		it('should return false when signature header is missing', () => {
+			mockWebhookFunctions.getRequestObject.mockReturnValue({
+				header: jest.fn().mockReturnValue(null),
+				rawBody: testBody,
+			});
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+		});
+
+		it('should return false when signature does not start with sha256=', () => {
+			mockWebhookFunctions.getRequestObject.mockReturnValue({
+				header: jest.fn().mockImplementation((header) => {
+					if (header === 'x-hub-signature-256') return 'invalid-format-signature';
+					return null;
+				}),
+				rawBody: testBody,
+			});
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+		});
+
+		it('should return false when rawBody is missing', () => {
+			mockWebhookFunctions.getRequestObject.mockReturnValue({
+				header: jest.fn().mockImplementation((header) => {
+					if (header === 'x-hub-signature-256') return testSignature;
+					return null;
+				}),
+				rawBody: undefined,
+			});
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+		});
+
+		it('should return true when signature is valid', () => {
+			(timingSafeEqual as jest.Mock).mockReturnValue(true);
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(true);
+			expect(createHmac).toHaveBeenCalledWith('sha256', testWebhookSecret);
+			expect(timingSafeEqual).toHaveBeenCalled();
+		});
+
+		it('should return false when signature is invalid', () => {
+			(timingSafeEqual as jest.Mock).mockReturnValue(false);
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+			expect(createHmac).toHaveBeenCalledWith('sha256', testWebhookSecret);
+			expect(timingSafeEqual).toHaveBeenCalled();
+		});
+
+		it('should handle Buffer rawBody correctly', () => {
+			const bufferBody = Buffer.from(testBody);
+			mockWebhookFunctions.getRequestObject.mockReturnValue({
+				header: jest.fn().mockImplementation((header) => {
+					if (header === 'x-hub-signature-256') return testSignature;
+					return null;
+				}),
+				rawBody: bufferBody,
+			});
+			(timingSafeEqual as jest.Mock).mockReturnValue(true);
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(true);
+			const mockHmac = createHmac('sha256', testWebhookSecret);
+			expect(mockHmac.update).toHaveBeenCalledWith(bufferBody);
+		});
+
+		it('should return false when computed and provided signatures have different lengths', () => {
+			// Mock a different length signature
+			const mockHmacInstance = {
+				update: jest.fn().mockReturnThis(),
+				digest: jest.fn().mockReturnValue('short'),
+			};
+			(createHmac as jest.Mock).mockReturnValue(mockHmacInstance);
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+			// timingSafeEqual should not be called if lengths don't match
+			expect(timingSafeEqual).not.toHaveBeenCalled();
+		});
+
+		it('should return false when an error occurs during verification', () => {
+			(createHmac as jest.Mock).mockImplementation(() => {
+				throw new Error('Crypto error');
+			});
+
+			const result = verifySignature.call(mockWebhookFunctions as never);
+
+			expect(result).toBe(false);
+		});
+	});
+});

packages/nodes-base/nodes/Github/__tests__/node/GithubTrigger.node.test.ts

@@ -1,5 +1,6 @@
 import { GithubTrigger } from '../../GithubTrigger.node';
 import * as GenericFunctions from '../../GenericFunctions';
+import * as GithubTriggerHelpers from '../../GithubTriggerHelpers';
 import { NodeOperationError } from 'n8n-workflow';
 
 describe('GithubTrigger Node', () => {
@@ -53,19 +54,43 @@ describe('GithubTrigger Node', () => {
 			};
 		});
 
-		it('should return true and set webhookId when creation succeeds', async () => {
+		it('should return true and set webhookId and webhookSecret when creation succeeds', async () => {
 			const createdWebhook = { id: '789', active: true };
 
-			jest.spyOn(GenericFunctions, 'githubApiRequest').mockResolvedValueOnce(createdWebhook); // Simulate successful POST
+			jest.spyOn(GenericFunctions, 'githubApiRequest').mockResolvedValueOnce(createdWebhook);
 
 			const trigger = new GithubTrigger();
 			const result = await trigger.webhookMethods.default.create.call(mockThis);
 
 			expect(result).toBe(true);
 			expect(webhookData.webhookId).toBe('789');
+			expect(webhookData.webhookSecret).toBeDefined();
+			expect(typeof webhookData.webhookSecret).toBe('string');
+			expect(webhookData.webhookSecret.length).toBe(64); // 32 bytes in hex
 		});
 
-		it('should handle 422 by checking for existing matching webhook', async () => {
+		it('should send the secret to GitHub API when creating webhook', async () => {
+			const createdWebhook = { id: '789', active: true };
+
+			const apiRequestSpy = jest
+				.spyOn(GenericFunctions, 'githubApiRequest')
+				.mockResolvedValueOnce(createdWebhook);
+
+			const trigger = new GithubTrigger();
+			await trigger.webhookMethods.default.create.call(mockThis);
+
+			expect(apiRequestSpy).toHaveBeenCalledWith(
+				'POST',
+				'/repos/some-owner/some-repo/hooks',
+				expect.objectContaining({
+					config: expect.objectContaining({
+						secret: expect.any(String),
+					}),
+				}),
+			);
+		});
+
+		it('should handle 422 by checking for existing matching webhook (no secret stored)', async () => {
 			const existingWebhook = {
 				id: '123',
 				events: ['push'],
@@ -82,6 +107,8 @@ describe('GithubTrigger Node', () => {
 
 			expect(result).toBe(true);
 			expect(webhookData.webhookId).toBe('123');
+			// Existing webhook won't have secret stored (backwards compatibility)
+			expect(webhookData.webhookSecret).toBeUndefined();
 		});
 
 		it('should throw NodeOperationError if repo is not found (404)', async () => {
@@ -98,4 +125,96 @@ describe('GithubTrigger Node', () => {
 			);
 		});
 	});
+
+	describe('delete webhook method', () => {
+		let webhookData: Record<string, any>;
+		let mockThis: any;
+
+		beforeEach(() => {
+			webhookData = {
+				webhookId: '123456',
+				webhookEvents: ['push'],
+				webhookSecret: 'test-secret',
+			};
+
+			mockThis = {
+				getWorkflowStaticData: () => webhookData,
+				getNodeParameter: jest.fn().mockImplementation((name: string) => {
+					if (name === 'owner') return 'some-owner';
+					if (name === 'repository') return 'some-repo';
+				}),
+			};
+		});
+
+		it('should delete webhook data including secret when deletion succeeds', async () => {
+			jest.spyOn(GenericFunctions, 'githubApiRequest').mockResolvedValueOnce({});
+
+			const trigger = new GithubTrigger();
+			const result = await trigger.webhookMethods.default.delete.call(mockThis);
+
+			expect(result).toBe(true);
+			expect(webhookData.webhookId).toBeUndefined();
+			expect(webhookData.webhookEvents).toBeUndefined();
+			expect(webhookData.webhookSecret).toBeUndefined();
+		});
+	});
+
+	describe('webhook method', () => {
+		let mockThis: any;
+		let webhookData: Record<string, any>;
+
+		beforeEach(() => {
+			webhookData = {
+				webhookSecret: 'test-secret',
+			};
+
+			mockThis = {
+				getWorkflowStaticData: () => webhookData,
+				getBodyData: jest.fn().mockReturnValue({ action: 'opened' }),
+				getHeaderData: jest.fn().mockReturnValue({}),
+				getQueryData: jest.fn().mockReturnValue({}),
+				getResponseObject: jest.fn().mockReturnValue({
+					status: jest.fn().mockReturnThis(),
+					send: jest.fn().mockReturnThis(),
+					end: jest.fn(),
+				}),
+				getRequestObject: jest.fn().mockReturnValue({
+					header: jest.fn(),
+					rawBody: '{}',
+				}),
+				helpers: {
+					returnJsonArray: jest.fn().mockImplementation((data) => data),
+				},
+			};
+		});
+
+		it('should reject with 401 when signature verification fails', async () => {
+			jest.spyOn(GithubTriggerHelpers, 'verifySignature').mockReturnValueOnce(false);
+
+			const trigger = new GithubTrigger();
+			const result = await trigger.webhook.call(mockThis);
+
+			expect(result).toEqual({ noWebhookResponse: true });
+			expect(mockThis.getResponseObject).toHaveBeenCalled();
+		});
+
+		it('should process webhook when signature verification succeeds', async () => {
+			jest.spyOn(GithubTriggerHelpers, 'verifySignature').mockReturnValueOnce(true);
+
+			const trigger = new GithubTrigger();
+			const result = await trigger.webhook.call(mockThis);
+
+			expect(result).toHaveProperty('workflowData');
+		});
+
+		it('should return OK for ping events when signature verification succeeds', async () => {
+			jest.spyOn(GithubTriggerHelpers, 'verifySignature').mockReturnValueOnce(true);
+			mockThis.getBodyData.mockReturnValue({ hook_id: '123' });
+
+			const trigger = new GithubTrigger();
+			const result = await trigger.webhook.call(mockThis);
+
+			expect(result).toEqual({ webhookResponse: 'OK' });
+		});
+	});
 });