Only accept string payloads for `webhooks.verify()` and `webhooks.verifyAndReceive()`

[gr2m]

What’s missing?

We have recurring problems with some event payloads where the code returns a "signature does not match" error, mostly reported by Probot users who are probably the biggest share of users of @octokit/webhooks.

I think we should stop making assumptions about how a JSON payload is stringified by GitHub and only accept the raw request string, as all other webhooks SDKs do that I know of, e.g. stripe

I'd consider passing the raw request body string as a best practice today, and we enforce best practices in the @octokit modules.

The challenge that this will bring is that the raw request body is not always easily accessible by server frameworks or serverless environments, so we should document how to do it with e.g. express, AWS Lambda, Vercel, Begin, Azure Functions, Google Cloud Functions, Cloudflare Workers, and invite users to add examples for other platforms.

Alternatives you tried

n/a

[wolfy1339]

An express usage example

import express from "express";
import { verify } from "@octokit/webhooks-methods";

const app = express();
app.use(express.raw({ type: "*/*" })));
const GH_WEBHOOK_SECRET = "secret";

app.post("/api/github/webhooks", (req, res) => {
    const signature = req.headers['x-hub-signature-256'];
    verify(GH_WEBHOOK_SECRET, req.rawBody, signature);
});

[gr2m]

Vercel has a guide to handle Stripe's webhooks: https://vercel.com/guides/getting-started-with-nextjs-typescript-stripe#step-3:-handling-webhooks-&-checking-their-signatures

The important for Vercel's serverless functions is

// Stripe requires the raw body to construct the event.
export const config = {
  api: {
    bodyParser: false,
  },
}

[rr-codes]

I just updated this package from v9.5.1 to v9.8.2 and now I am experiencing this issue with every payload I receive

[github-actions[bot]]

:tada: This issue has been resolved in version 10.5.0 :tada:

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot :package::rocket:

[wolfy1339]

Reopening, as it's not completely finished. The middleware needs to be adapted

[github-actions[bot]]

:tada: This issue has been resolved in version 11.0.0-beta.3 :tada:

The release is available on:

• GitHub release
• npm package (@beta dist-tag)

Your semantic-release bot :package::rocket:

[wolfy1339]

@octokit/js Should we also do the same or the sign() function? That way we don't need to have a custom wrapper function, and can get rid of toNormalizedJsonString() completely

[github-actions[bot]]

:tada: This issue has been resolved in version 11.0.0-beta.4 :tada:

The release is available on:

• GitHub release
• npm package (@beta dist-tag)

Your semantic-release bot :package::rocket:

[gr2m]

Sounds good to me

[philipandersson]

As you pointed out @wolfy1339, the middleware still needs to be adapted. Right now, the results of getPayload() is passed to verifyAndReceive(). Wouldn't this change be required before releasing v11?

https://github.com/octokit/webhooks.js/blob/61241f7f0486dc14aa02881d3c4c180e2ef237c6/src/middleware/node/middleware.ts#L78

[github-actions[bot]]

:tada: This issue has been resolved in version 11.0.0 :tada:

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot :package::rocket:

[austenstone]

To get rid of the deprecation warning using octokit@^2.0.15 I had to explicitly pass onUnhandledRequest: undefined.

import { App, createNodeMiddleware } from "octokit";
...
    this.server = createServer(createNodeMiddleware(app, {
      onUnhandledRequest: undefined
    })).listen(port);