Properly verify webhook signature

index.js

@@ -2,15 +2,18 @@ const { createProbot } = require('probot')
 const { resolve } = require('probot/lib/resolver')
 const { findPrivateKey } = require('probot/lib/private-key')
 const { template } = require('./views/probot')
+const verify = require('@octokit/webhooks/verify')
 
 let probot
 
-const loadProbot = appFn => {
-  probot = probot || createProbot({
-    id: process.env.APP_ID,
-    secret: process.env.WEBHOOK_SECRET,
-    cert: findPrivateKey()
-  })
+const getOpts = () => ({
+  id: process.env.APP_ID,
+  secret: process.env.WEBHOOK_SECRET,
+  cert: findPrivateKey()
+})
+
+const loadProbot = (opts, appFn) => {
+  probot = probot || createProbot(opts)
 
   if (typeof appFn === 'string') {
     appFn = resolve(appFn)
@@ -32,21 +35,46 @@ module.exports.serverless = appFn => {
       })
     }
 
+    const opts = getOpts()
     // Otherwise let's listen handle the payload
-    probot = probot || loadProbot(appFn)
+    probot = probot || loadProbot(opts, appFn)
 
     // Determine incoming webhook event type
     const name = request.get('x-github-event') || request.get('X-GitHub-Event')
     const id = request.get('x-github-delivery') || request.get('X-GitHub-Delivery')
 
+    // Verify signed payload
+    const signature = request.get('x-hub-signature') || request.get('X-Hub-Signature')
+    const body = request.body
+    if (!opts.secret) {
+      console.error('secret not set')
+      response.sendStatus(500)
+      return
+    }
+    let matchesSignature = false
+    try {
+      matchesSignature = verify(opts.secret, body, signature)
+    } catch (err) {
+      if (err instanceof TypeError) {
+        response.sendStatus(400)
+        return
+      }
+      throw err
+    }
+    if (!matchesSignature) {
+      console.error('signature does not match event payload and secret')
+      response.sendStatus(400)
+      return
+    }
+
     // Do the thing
     console.log(`Received event ${name}${request.body.action ? ('.' + request.body.action) : ''}`)
     if (name) {
       try {
         await probot.receive({
           name,
           id,
-          payload: request.body
+          payload: body
         })
         response.send({
           statusCode: 200,

package.json

@@ -11,6 +11,7 @@
     "url": "git+https://github.com/probot/serverless-gcf.git"
   },
   "devDependencies": {
+    "@octokit/webhooks": "^5.0.2",
     "jest": "^23.6.0",
     "probot": "^7.1.2",
     "standard": "^10.0.3"

tests/index.test.js

@@ -1,4 +1,5 @@
 const { serverless } = require('../')
+const sign = require('@octokit/webhooks/sign')
 
 describe('serverless-gcf', () => {
   let spy, handler, response
@@ -11,6 +12,9 @@ describe('serverless-gcf', () => {
       app.on('issues', spy)
     })
   })
+  afterEach(() => {
+    process.env.WEBHOOK_SECRET = ''
+  })
 
   it('responds with the homepage', async () => {
     const request = { method: 'GET', path: '/probot' }
@@ -20,15 +24,19 @@ describe('serverless-gcf', () => {
   })
 
   it('calls the event handler', async () => {
+    process.env.WEBHOOK_SECRET = 'secret'
+    const body = {
+      installation: { id: 1 }
+    }
+    const signature = sign('secret', body)
     const request = {
-      body: {
-        installation: { id: 1 }
-      },
+      body,
       get (string) {
         return this[string]
       },
       'x-github-event': 'issues',
-      'x-github-delivery': 123
+      'x-github-delivery': 123,
+      'x-hub-signature': signature
     }
 
     await handler(request, response)
@@ -37,6 +45,7 @@ describe('serverless-gcf', () => {
   })
 
   it('does nothing if there are missing headers', async () => {
+    process.env.WEBHOOK_SECRET = 'secret'
     const request = {
       body: {
         installation: { id: 1 }
@@ -51,4 +60,65 @@ describe('serverless-gcf', () => {
     expect(spy).not.toHaveBeenCalled()
     expect(response.sendStatus).toHaveBeenCalledWith(400)
   })
+
+  it('does not allow invalid signatures', async () => {
+    process.env.WEBHOOK_SECRET = 'secret'
+    const body = {
+      installation: { id: 1 }
+    }
+    const signature = sign('wrong_secret', body)
+    const request = {
+      body,
+      get (string) {
+        return this[string]
+      },
+      'x-github-event': 'issues',
+      'x-github-delivery': 123,
+      'x-hub-signature': signature
+    }
+
+    await handler(request, response)
+    expect(response.send).not.toHaveBeenCalled()
+    expect(spy).not.toHaveBeenCalled()
+    expect(response.sendStatus).toHaveBeenCalledWith(400)
+  })
+
+  it('requires the secret to be set', async () => {
+    const body = {
+      installation: { id: 1 }
+    }
+    const request = {
+      body,
+      get (string) {
+        return this[string]
+      },
+      'x-github-event': 'issues',
+      'x-github-delivery': 123
+    }
+
+    await handler(request, response)
+    expect(response.send).not.toHaveBeenCalled()
+    expect(spy).not.toHaveBeenCalled()
+    expect(response.sendStatus).toHaveBeenCalledWith(500)
+  })
+
+  it('requires a signature', async () => {
+    process.env.WEBHOOK_SECRET = 'secret'
+    const body = {
+      installation: { id: 1 }
+    }
+    const request = {
+      body,
+      get (string) {
+        return this[string]
+      },
+      'x-github-event': 'issues',
+      'x-github-delivery': 123
+    }
+
+    await handler(request, response)
+    expect(response.send).not.toHaveBeenCalled()
+    expect(spy).not.toHaveBeenCalled()
+    expect(response.sendStatus).toHaveBeenCalledWith(400)
+  })
 })