Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does this actually validate webhook signatures #6

Open
acoulton opened this issue Dec 14, 2018 · 4 comments · May be fixed by #10
Open

Does this actually validate webhook signatures #6

acoulton opened this issue Dec 14, 2018 · 4 comments · May be fixed by #10

Comments

@acoulton
Copy link

This may be me, but it seems like it doesn't validate webhook signatures.

If I configure it with an incorrect webhook secret the requests are still handled as a 200 response. I assume it needs to be passing the X-Hub-Signature, and probably the raw request body, along to probot somewhere?

@ethomson
Copy link

ethomson commented Jan 4, 2019

I agree. I added some code to do the validation in my serverless extension:

const validateSignature = (req) => {
  const given = req.headers['x-hub-signature'] || req.headers['X-Hub-Signature']

  if (! process.env['WEBHOOK_SECRET']) {
    console.error("No shared secret; set the WEBHOOK_SECRET environment variable")
    return false
  }

  var hmac = crypto.createHmac("sha1", process.env['WEBHOOK_SECRET'])
  hmac.update(req.rawBody, 'binary')
  var expected = 'sha1=' + hmac.digest('hex')

  return given.length === expected.length && crypto.timingSafeEqual(Buffer.from(given), Buffer.from(expected))
}

@rthadur
Copy link

rthadur commented Feb 24, 2019

@acoulton @ethomson were you able to deploy to gcf ?

@acoulton
Copy link
Author

@rthadur yes - but ISTR it took a bit of faffing about.

However we subsequently realised that since our app was bridging github to other services the probot approach wasn't that useful for us. When we got a github hook we almost always always need a client to the other services, we only needed a github client when the other services fired their own hooks. And although that was doable by wrapping probot, we weren't really using any of probot's automation and it was just adding an extra layer of complexity over using octokit etc directly. Particularly since we'd have had to add our own webhook validation, etc. So we're no longer using it and I don't think I still have the code I hacked up to experiment with getting the GCF deployment to work.

@ethomson
Copy link

ethomson commented Feb 25, 2019

@rthadur No, and I was never intending to. I was commenting here since I had the same issue in my serverless extension for Azure Functions: https://github.com/ethomson/probot-serverless-azurefunctions

I did deploy my probot to Azure Functions with success (and significant cost savings over my prior environment).

GitHub
Azure Functions adapter for Probot tools. Contribute to ethomson/probot-serverless-azurefunctions development by creating an account on GitHub.

@michaelbeaumont michaelbeaumont linked a pull request Aug 8, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants