feat: only accept string payloads for webhooks.verify() and webhooks.verifyAndReceive() and the middleware (#793)

BREAKING CHANGE: Only accept string payloads for `webhooks.verify()` and `webhooks.verifyAndReceive()`
BREAKING CHANGE: The middleware now expects only string payloads for `request.body`

Author: wolfy1339

src/index.ts

@@ -1,7 +1,7 @@
 import { createLogger } from "./createLogger";
 import { createEventHandler } from "./event-handler/index";
 import { sign } from "./sign";
-import { verify } from "./verify";
+import { verify } from "@octokit/webhooks-methods";
 import { verifyAndReceive } from "./verify-and-receive";
 import {
   EmitterWebhookEvent,
@@ -13,27 +13,15 @@ import {
   WebhookError,
   WebhookEventHandlerError,
   EmitterWebhookEventWithStringPayloadAndSignature,
-  EmitterWebhookEventWithSignature,
 } from "./types";
 
 export { createNodeMiddleware } from "./middleware/node/index";
 export { emitterEventNames } from "./generated/webhook-names";
 
-export type Iverify = {
-  /** @deprecated Passing a JSON payload object to `verify()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`. */
-  (eventPayload: object, signature: string): Promise<boolean>;
-  (eventPayload: string, signature: string): Promise<boolean>;
-};
-export type IverifyAndReceive = {
-  /** @deprecated Passing a JSON payload object to `verifyAndReceive()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`. */
-  (options: EmitterWebhookEventWithSignature): Promise<void>;
-  (options: EmitterWebhookEventWithStringPayloadAndSignature): Promise<void>;
-};
-
 // U holds the return value of `transform` function in Options
 class Webhooks<TTransformed = unknown> {
   public sign: (payload: string | object) => Promise<string>;
-  public verify: Iverify;
+  public verify: (eventPayload: string, signature: string) => Promise<boolean>;
   public on: <E extends EmitterWebhookEventName>(
     event: E | E[],
     callback: HandlerFunction<E, TTransformed>
@@ -45,7 +33,9 @@ class Webhooks<TTransformed = unknown> {
     callback: RemoveHandlerFunction<E, TTransformed>
   ) => void;
   public receive: (event: EmitterWebhookEvent) => Promise<void>;
-  public verifyAndReceive: IverifyAndReceive;
+  public verifyAndReceive: (
+    options: EmitterWebhookEventWithStringPayloadAndSignature
+  ) => Promise<void>;
 
   constructor(options: Options<TTransformed> & { secret: string }) {
     if (!options || !options.secret) {
@@ -60,31 +50,13 @@ class Webhooks<TTransformed = unknown> {
     };
 
     this.sign = sign.bind(null, options.secret);
-    this.verify = (eventPayload: object | string, signature: string) => {
-      if (typeof eventPayload === "object") {
-        console.error(
-          "[@octokit/webhooks] Passing a JSON payload object to `verify()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`"
-        );
-      }
-      return verify(options.secret, eventPayload, signature);
-    };
+    this.verify = verify.bind(null, options.secret);
     this.on = state.eventHandler.on;
     this.onAny = state.eventHandler.onAny;
     this.onError = state.eventHandler.onError;
     this.removeListener = state.eventHandler.removeListener;
     this.receive = state.eventHandler.receive;
-    this.verifyAndReceive = (
-      options:
-        | EmitterWebhookEventWithStringPayloadAndSignature
-        | EmitterWebhookEventWithSignature
-    ) => {
-      if (typeof options.payload === "object") {
-        console.error(
-          "[@octokit/webhooks] Passing a JSON payload object to `verifyAndReceive()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`"
-        );
-      }
-      return verifyAndReceive(state, options);
-    };
+    this.verifyAndReceive = verifyAndReceive.bind(null, state);
   }
 }
 

src/middleware/node/get-payload.ts

@@ -1,4 +1,3 @@
-import { WebhookEvent } from "@octokit/webhooks-types";
 // @ts-ignore to address #245
 import AggregateError from "aggregate-error";
 
@@ -12,11 +11,11 @@ import AggregateError from "aggregate-error";
 // }
 type IncomingMessage = any;
 
-export function getPayload(request: IncomingMessage): Promise<WebhookEvent> {
+export function getPayload(request: IncomingMessage): Promise<string> {
   // If request.body already exists we can stop here
   // See https://github.com/octokit/webhooks.js/pull/23
 
-  if (request.body) return Promise.resolve(request.body as WebhookEvent);
+  if (request.body) return Promise.resolve(request.body);
 
   return new Promise((resolve, reject) => {
     let data = "";
@@ -28,7 +27,8 @@ export function getPayload(request: IncomingMessage): Promise<WebhookEvent> {
     request.on("data", (chunk: string) => (data += chunk));
     request.on("end", () => {
       try {
-        resolve(JSON.parse(data));
+        JSON.parse(data); // check if data is valid JSON
+        resolve(data);
       } catch (error: any) {
         error.message = "Invalid JSON";
         error.status = 400;

src/middleware/node/middleware.ts

@@ -79,7 +79,7 @@ export async function middleware(
     await webhooks.verifyAndReceive({
       id: id,
       name: eventName as any,
-      payload: payload as any,
+      payload,
       signature: signatureSHA256,
     });
     clearTimeout(timeout);

src/types.ts

@@ -21,10 +21,6 @@ export type EmitterWebhookEventWithStringPayloadAndSignature = {
   payload: string;
   signature: string;
 };
-/** @deprecated */
-export type EmitterWebhookEventWithSignature = EmitterWebhookEvent & {
-  signature: string;
-};
 
 interface BaseWebhookEvent<TName extends WebhookEventName> {
   id: string;

src/verify-and-receive.ts

@@ -1,24 +1,18 @@
 import { verify } from "@octokit/webhooks-methods";
 
-import { toNormalizedJsonString } from "./to-normalized-json-string";
 import {
   EmitterWebhookEventWithStringPayloadAndSignature,
-  EmitterWebhookEventWithSignature,
   State,
 } from "./types";
 
 export async function verifyAndReceive(
   state: State & { secret: string },
-  event:
-    | EmitterWebhookEventWithStringPayloadAndSignature
-    | EmitterWebhookEventWithSignature
+  event: EmitterWebhookEventWithStringPayloadAndSignature
 ): Promise<any> {
   // verify will validate that the secret is not undefined
   const matchesSignature = await verify(
     state.secret,
-    typeof event.payload === "object"
-      ? toNormalizedJsonString(event.payload)
-      : event.payload,
+    event.payload,
     event.signature
   );
 
@@ -35,9 +29,6 @@ export async function verifyAndReceive(
   return state.eventHandler.receive({
     id: event.id,
     name: event.name,
-    payload:
-      typeof event.payload === "string"
-        ? JSON.parse(event.payload)
-        : event.payload,
+    payload: JSON.parse(event.payload),
   });
 }

src/verify.ts

@@ -75,7 +75,7 @@ describe("createNodeMiddleware(webhooks)", () => {
       req.once("data", (chunk) => dataChunks.push(chunk));
       req.once("end", () => {
         // @ts-expect-error - TS2339: Property 'body' does not exist on type 'IncomingMessage'.
-        req.body = JSON.parse(Buffer.concat(dataChunks).toString());
+        req.body = Buffer.concat(dataChunks).toString();
         middleware(req, res);
       });
     }).listen();

test/integration/node-middleware.test.ts

@@ -32,18 +32,15 @@ describe("Webhooks", () => {
     await webhooks.sign(pushEventPayloadString);
   });
 
-  test("webhooks.verify(payload, signature) with object payload containing special characters", async () => {
+  test("webhooks.verify(payload, signature) with string payload containing special characters", async () => {
     const secret = "mysecret";
     const webhooks = new Webhooks({ secret });
 
-    const payload = {
+    const payload = toNormalizedJsonString({
       foo: "Foo\n\u001b[34mbar: ♥♥♥♥♥♥♥♥\nthis-is-lost\u001b[0m\u001b[2K",
-    };
+    });
 
-    await webhooks.verify(
-      payload,
-      await sign(secret, toNormalizedJsonString(payload))
-    );
+    await webhooks.verify(payload, await sign(secret, payload));
   });
 
   test("webhooks.verify(payload, signature) with string payload", async () => {

test/integration/webhooks.test.ts

@@ -83,7 +83,7 @@ export default async function () {
   webhooks.onAny(async ({ id, name, payload }) => {
     console.log(name, "event received", id);
     const sig = await webhooks.sign(payload);
-    webhooks.verify(payload, sig);
+    webhooks.verify(JSON.stringify(payload), sig);
   });
 
   webhooks.on("check_run.completed", () => {});