fix: Validate github signature

Author: kbkk

.env.dev-sample

@@ -6,4 +6,5 @@ YOUTUBE_API_KEY=
 SPOTIFY_CLIENT_ID=
 SPOTIFY_SECRET=
 
-DISCORD_RECEIVE_GITHUB_WEBHOOK_URL=
+GITHUB_WEBHOOK_SECRET=
+GITHUB_WEBHOOK_DISCORD_URL=

src/handle-github-webhook.spec.ts

@@ -14,46 +14,52 @@ import handleGithubWebhook from './handle-github-webhook';
 import * as handleGithubWebhookModule from './handle-github-webhook';
 import * as Config from './config';
 import createHttpServer from './http-server';
+import * as crypto from 'crypto';
 
-const MOCK_DISCORD_WEBHOOK_URL = 'https://discord.com/api/webhooks/12345/s3creTk3Y';
+const GITHUB_WEBHOOK_SECRET = 's3creTk3Y';
+const GITHUB_WEBHOOK_DISCORD_URL = 'https://discord.com/api/webhooks/12345/key';
 
 describe('handleGithubWebhook - unit tests', () => {
   beforeEach(() => {
-    Sinon.stub(Config, 'getConfig').returns(MOCK_DISCORD_WEBHOOK_URL);
+    const getConfigStub = Sinon.stub(Config, 'getConfig');
+    getConfigStub.withArgs('GITHUB_WEBHOOK_SECRET').returns(GITHUB_WEBHOOK_SECRET);
+    getConfigStub.withArgs('GITHUB_WEBHOOK_DISCORD_URL').returns(GITHUB_WEBHOOK_DISCORD_URL);
   });
 
   afterEach(() => {
     Sinon.restore();
   });
 
-  it('should return 401 if webhook secret is not provided', async () => {
-    const result = await handleGithubWebhook('/githubWebhook/wrong-secret', {});
+  it('should return 401 if webhook signature is invalid', async () => {
+    const result = await handleGithubWebhook({}, Buffer.from(''), {});
 
     expect(result).to.eql({ statusCode: 401 });
   });
 
   it('should return 200 if webhook was filtered out', async () => {
-    const result = await handleGithubWebhook('/githubWebhook/s3creTk3Y', {
-      sender: {
-        login: 'dependabot',
-      },
-    });
+    const result = await handleGithubWebhook(
+      ...makeHandleGithubWebhookParams({
+        sender: {
+          login: 'dependabot',
+        },
+      })
+    );
 
     expect(result).to.eql({ statusCode: 200 });
   });
 
   it('should return status code from discord if the webhook was forwarded', async () => {
     const payload = { sender: { login: 'kbkk' } };
 
-    nock('https://discord.com').post('/api/webhooks/12345/s3creTk3Y', payload).reply(599);
+    nock('https://discord.com').post('/api/webhooks/12345/key', payload).reply(599);
 
-    const result = await handleGithubWebhook('/githubWebhook/s3creTk3Y', payload);
+    const result = await handleGithubWebhook(...makeHandleGithubWebhookParams(payload));
 
     expect(result).to.eql({ statusCode: 599 });
   });
 });
 
-describe('handleGithubWebhook - integration tests', () => {
+describe.only('handleGithubWebhook - integration tests', () => {
   let httpServer: Http.Server;
   let baseUrl: string;
 
@@ -79,7 +85,7 @@ describe('handleGithubWebhook - integration tests', () => {
   it('should respond with 400 on invalid request', async () => {
     const jsonParseSpy = Sinon.spy(JSON, 'parse');
 
-    const { status } = await fetch(`${baseUrl}/githubWebhook/test`, {
+    const { status } = await fetch(`${baseUrl}/githubWebhook`, {
       method: 'POST',
       body: 'invalid json',
     });
@@ -88,18 +94,23 @@ describe('handleGithubWebhook - integration tests', () => {
     expect(status).to.eql(400);
   });
 
-  it('should call handleGithubWebhook with url and body', async () => {
+  it('should call handleGithubWebhook with headers, rawBody and body', async () => {
     const handleGithubWebhookStub = Sinon.stub(handleGithubWebhookModule, 'default');
     handleGithubWebhookStub.resolves({ statusCode: 200 });
 
-    await fetch(`${baseUrl}/githubWebhook/test`, {
+    await fetch(`${baseUrl}/githubWebhook`, {
       method: 'POST',
       body: '{"a":1}',
+      headers: {
+        'x-hub-signature': 'test-signature',
+      },
     });
 
-    expect(handleGithubWebhookStub).to.have.been.calledOnceWithExactly('/githubWebhook/test', {
-      a: 1,
-    });
+    expect(handleGithubWebhookStub).to.have.been.calledOnceWithExactly(
+      Sinon.match({ 'x-hub-signature': 'test-signature' }),
+      Buffer.from('{"a":1}'),
+      { a: 1 }
+    );
   });
 
   it('should respond with status code from handleGithubWebhook', async () => {
@@ -114,3 +125,23 @@ describe('handleGithubWebhook - integration tests', () => {
     expect(status).eql(599);
   });
 });
+
+function forgeSignature(rawBody: Buffer): string {
+  const hmacString = crypto.createHmac('sha1', GITHUB_WEBHOOK_SECRET).update(rawBody).digest('hex');
+  const signature = `sha1=${hmacString}`;
+
+  return signature;
+}
+
+function makeHandleGithubWebhookParams(body: object): Parameters<typeof handleGithubWebhook> {
+  const rawBody = Buffer.from(JSON.stringify(body));
+  const signature = forgeSignature(rawBody);
+
+  return [
+    {
+      'x-hub-signature': signature,
+    },
+    rawBody,
+    body,
+  ];
+}

src/handle-github-webhook.ts

@@ -1,3 +1,5 @@
+import * as crypto from 'crypto';
+import { IncomingHttpHeaders } from 'http';
 import fetch from 'node-fetch';
 import { getConfig } from './config';
 
@@ -14,18 +16,19 @@ interface GithubWebhookPullRequest {
 }
 
 async function handleGithubWebhook(
-  requestPath: string,
+  headers: IncomingHttpHeaders,
+  rawBody: Buffer,
   body: object
 ): Promise<{ statusCode: number }> {
-  if (!checkWebhookSecret(requestPath)) {
+  if (!validateGithubSignature((headers['x-hub-signature'] ?? '') as string, rawBody)) {
     return { statusCode: 401 };
   }
 
   if (!shouldSendWebhook(body)) {
     return { statusCode: 200 };
   }
 
-  const discordWebhookUrl = getConfig('DISCORD_RECEIVE_GITHUB_WEBHOOK_URL');
+  const discordWebhookUrl = getConfig('GITHUB_WEBHOOK_DISCORD_URL');
 
   const { status } = await fetch(discordWebhookUrl, {
     method: 'POST',
@@ -43,16 +46,15 @@ function shouldSendWebhook(body: object | GithubWebhookPullRequest) {
   return true;
 }
 
-/**
- * Given the configured webhook url is https://discord.com/api/webhooks/12345/s3creTk3Y
- * then the bot should be called with https://tow-bot/githubWebhook/s3creTk3Y
- */
-function checkWebhookSecret(requestPath: string): boolean {
-  const discordWebhookUrl = getConfig('DISCORD_RECEIVE_GITHUB_WEBHOOK_URL');
+function validateGithubSignature(receivedSignature: string, rawBody: Buffer) {
+  const githubSecret = getConfig('GITHUB_WEBHOOK_SECRET');
+  const hmacString = crypto.createHmac('sha1', githubSecret).update(rawBody).digest('hex');
+  const expectedSignature = `sha1=${hmacString}`;
 
-  const secret = discordWebhookUrl.split('/').pop() ?? '';
-
-  return requestPath.includes(secret);
+  return (
+    receivedSignature.length === expectedSignature.length &&
+    crypto.timingSafeEqual(Buffer.from(receivedSignature), Buffer.from(expectedSignature))
+  );
 }
 
 export default handleGithubWebhook;

src/http-server.ts

@@ -19,9 +19,11 @@ function createHttpServer(
       }
 
       try {
-        const body = JSON.parse(Buffer.concat(chunks).toString());
+        const rawBody = Buffer.concat(chunks);
+        const body = JSON.parse(rawBody.toString());
+        const headers = req.headers;
 
-        const { statusCode } = await handleGithubWebhook(req.url, body);
+        const { statusCode } = await handleGithubWebhook(headers, rawBody, body);
 
         res.statusCode = statusCode;
         res.end();