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

signature does not match event payload and secret when receiving JSON body #593

Closed
rr-codes opened this issue Jun 19, 2021 · 13 comments · Fixed by #595
Closed

signature does not match event payload and secret when receiving JSON body #593

rr-codes opened this issue Jun 19, 2021 · 13 comments · Fixed by #595
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects

Comments

@rr-codes
Copy link

What happened?

After updating @octokit/webhooks from v9.5.1 to v9.8.2, I now receive this error on all webhook events

AggregateError: 
2021-06-19T02:53:43.735393200Z     Error: [@octokit/webhooks] signature does not match event payload and secret
2021-06-19T02:53:43.735470900Z         at verifyAndReceive (/usr/src/app/node_modules/@octokit/webhooks/dist-src/verify-and-receive.js:9:23)
2021-06-19T02:53:43.735505400Z         at middleware (/usr/src/app/node_modules/@octokit/webhooks/dist-src/middleware/node/middleware.js:39:9)
2021-06-19T02:53:43.735535200Z     at receive (/usr/src/app/node_modules/@octokit/webhooks/dist-src/event-handler/receive.js:15:37)
2021-06-19T02:53:43.735560400Z     at verifyAndReceive (/usr/src/app/node_modules/@octokit/webhooks/dist-src/verify-and-receive.js:10:35)
2021-06-19T02:53:43.735587700Z     at processTicksAndRejections (internal/process/task_queues.js:95:5)
2021-06-19T02:53:43.735621500Z     at middleware (/usr/src/app/node_modules/@octokit/webhooks/dist-src/middleware/node/middleware.js:39:9) {
2021-06-19T02:53:43.735656300Z   event: Error: [@octokit/webhooks] signature does not match event payload and secret
2021-06-19T02:53:43.735774200Z       at verifyAndReceive (/usr/src/app/node_modules/@octokit/webhooks/dist-src/verify-and-receive.js:9:23)
2021-06-19T02:53:43.736161500Z       at processTicksAndRejections (internal/process/task_queues.js:95:5)
2021-06-19T02:53:43.736196500Z       at middleware (/usr/src/app/node_modules/@octokit/webhooks/dist-src/middleware/node/middleware.js:39:9) {
2021-06-19T02:53:43.736222800Z     event: {
2021-06-19T02:53:43.736245700Z       id: '8f2b4480-d0a9-11eb-901c-fd8bfc9148cf',
2021-06-19T02:53:43.736268800Z       name: 'issues',
2021-06-19T02:53:43.736298100Z       payload: [Object],
2021-06-19T02:53:43.736336100Z       signature: 'sha256=7fd7cab774ec3308791747ec253ec258e0add40ece88bc565035de0b48690448'
2021-06-19T02:53:43.736362700Z     },
2021-06-19T02:53:43.736449500Z     status: 400
2021-06-19T02:53:43.736496500Z   },
2021-06-19T02:53:43.736775800Z   errors: [
2021-06-19T02:53:43.736883300Z     Error: [@octokit/webhooks] signature does not match event payload and secret
2021-06-19T02:53:43.736914200Z         at verifyAndReceive (/usr/src/app/node_modules/@octokit/webhooks/dist-src/verify-and-receive.js:9:23)
2021-06-19T02:53:43.736945900Z         at processTicksAndRejections (internal/process/task_queues.js:95:5)
2021-06-19T02:53:43.736972800Z         at middleware (/usr/src/app/node_modules/@octokit/webhooks/dist-src/middleware/node/middleware.js:39:9) {
2021-06-19T02:53:43.737002700Z       event: [Object],
2021-06-19T02:53:43.737031400Z       status: 400
2021-06-19T02:53:43.737110800Z     }
2021-06-19T02:53:43.737136200Z   ]
2021-06-19T02:53:43.737160300Z }
2021-06-19T02:53:43.772597000Z POST /api/webhooks/github 400 56.694 ms - -

I am using the Express framework with the following middleware

app.use((req, res, next) => {
  if (req.originalUrl.includes('/stripe/webhook')) {
    bodyParser.raw({ type: 'application/json' })(req, res, next);
  } else {
    bodyParser.json()(req, res, next);
  }
});

What did you expect to happen?

For it to work as expected, and not be a breaking change.

What the problem might be

The problem is #587, which updated to the breaking change in @octokit/webhooks-methods, and seemingly added a compatibility layer, however that seems to have had no effect for my set up, resulting in the breaking change persisting.

Changing the middleware to be this works:

app.use((req, res, next) => {
  if (req.originalUrl.includes('/stripe/webhook')) {
    bodyParser.raw({ type: 'application/json' })(req, res, next);
  } else if (req.originalUrl.includes('/webhooks/github')) {
    bodyParser.text({ type: 'application/json' })(req, res, next);
  } else {
    bodyParser.json()(req, res, next);
  }
})
@rr-codes rr-codes added the Type: Bug Something isn't working as documented, or is being fixed label Jun 19, 2021
@ghost ghost added this to Bugs in JS Jun 19, 2021
@gr2m
Copy link
Contributor

gr2m commented Jun 19, 2021

Can you pin the version to 9.8.0 and see if it works?

@rr-codes
Copy link
Author

@gr2m Confirmed that v9.8.0 works, but not v9.8.1 or v9.8.2

@wolfy1339
Copy link
Member

I believe the bug lies somewhere within the toNormalizedJsonString() function.

At first glance, the code is exactly the same as what was removed in @octokit/webhooks-methods

This is a strange issue.

I can have a better look later today.

@rr-codes
Copy link
Author

@wolfy1339 I just investigated it a bit, and it seems like just JSON.stringify(payload) is sufficient, and it works. This is what I used to test:

  app.post('/api/webhooks/github', async (req, res) => {
    const matchesSignature = await webhooks.verify(
      JSON.stringify(req.body),
      req.headers['x-hub-signature-256']
    );

    if (!matchesSignature) {
      res.sendStatus(400);
      return;
    }

    await webhooks.receive({
      id: req.headers['x-github-delivery'],
      name: req.headers['x-github-event'],
      payload: req.body,
    });

    res.sendStatus(200);
  });

@wolfy1339
Copy link
Member

Thanks for that 👍

@mbazalik
Copy link

the issue is that the stringification does not work the same:

original method:

JSON.stringify(payload).replace(/[^\\]\\u[\da-f]{4}/g, s => {
    return s.substr(0, 3) + s.substr(3).toUpperCase();
  })

new method

(JSON.stringify(payload, null, 2) + "\n").replace(/[^\\]\\u[\da-f]{4}/g, (s) => {    return s.substr(0, 3) + s.substr(3).toUpperCase();  });

The new method adds new lines and indentation into the string, resulting in different hash

@gr2m
Copy link
Contributor

gr2m commented Jun 21, 2021

The new method adds new lines and indentation into the string, resulting in different hash

But that's what GitHub does as far as I know. Does this fail in a testing environment for you, or in production?

@gr2m
Copy link
Contributor

gr2m commented Jun 21, 2021

I was wrong, sorry. I should have verified my assumption before making this change. I now did by setting the webhook URL to https://httpbin.org/post and creating a test event. The response clearly shows that there are no extra white spaces anywhere in request body of webhook requests.

@gr2m gr2m closed this as completed in #595 Jun 21, 2021
JS automation moved this from Bugs to Done Jun 21, 2021
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 9.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gr2m
Copy link
Contributor

gr2m commented Jun 21, 2021

@mbazalik can you please test with 9.8.3?

@rr-codes
Copy link
Author

thanks @wolfy1339 for fixing so quickly! works perfectly now!!

@mrousavy
Copy link

Hey all!

I'm still receiving this error - I confirmed that my WEBHOOKS_SECRET env variable is exactly the same on GitHub as well as on Vercel (deployed to Vercel), but it still fails with the error message

Error: [@octokit/webhooks] signature does not match event payload and secret

I'm on "@octokit/webhooks" "^12.0.10", is there anything I can try?

@wolfy1339
Copy link
Member

@mrousavy Please see probot/probot#1973 probot/probot#1955

Please don't comment on old issues, open up a new one. And please only post your issue in one place

@octokit octokit locked as resolved and limited conversation to collaborators Jun 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects
No open projects
JS
  
Done
5 participants