From 7bb7de61092f1d06e571bed1d1939ad0eae680da Mon Sep 17 00:00:00 2001 From: uzlopak Date: Sat, 25 Nov 2023 11:13:22 +0100 Subject: [PATCH] fix: export createNodeHandler --- src/index.ts | 1 + src/middleware/node/get-payload.ts | 5 - src/middleware/node/handler.ts | 118 +++++++ src/middleware/node/middleware.ts | 92 +----- test/integration/node-handler.test.ts | 454 ++++++++++++++++++++++++++ 5 files changed, 575 insertions(+), 95 deletions(-) create mode 100644 src/middleware/node/handler.ts create mode 100644 test/integration/node-handler.test.ts diff --git a/src/index.ts b/src/index.ts index 5dc8d91b..ba38a027 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,7 @@ import type { } from "./types.js"; export { createNodeMiddleware } from "./middleware/node/index.js"; +export { createNodeHandler } from "./middleware/node/handler.js"; export { emitterEventNames } from "./generated/webhook-names.js"; // U holds the return value of `transform` function in Options diff --git a/src/middleware/node/get-payload.ts b/src/middleware/node/get-payload.ts index 9d1d4909..4eb4d897 100644 --- a/src/middleware/node/get-payload.ts +++ b/src/middleware/node/get-payload.ts @@ -12,11 +12,6 @@ import AggregateError from "aggregate-error"; type IncomingMessage = any; export function getPayload(request: IncomingMessage): Promise { - // 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); - return new Promise((resolve, reject) => { let data = ""; diff --git a/src/middleware/node/handler.ts b/src/middleware/node/handler.ts new file mode 100644 index 00000000..18f3b736 --- /dev/null +++ b/src/middleware/node/handler.ts @@ -0,0 +1,118 @@ +// remove type imports from http for Deno compatibility +// see https://github.com/octokit/octokit.js/issues/2075#issuecomment-817361886 +// import { IncomingMessage, ServerResponse } from "http"; +type IncomingMessage = any; +type ServerResponse = any; + +import type { WebhookEventName } from "@octokit/webhooks-types"; + +import type { Webhooks } from "../../index.js"; +import type { WebhookEventHandlerError } from "../../types.js"; +import type { MiddlewareOptions } from "./types.js"; +import { getMissingHeaders } from "./get-missing-headers.js"; +import { getPayload } from "./get-payload.js"; + +type Handler = ( + request: IncomingMessage, + response: ServerResponse, +) => Promise; +export function createNodeHandler( + webhooks: Webhooks, + options?: Pick, +): Handler { + const logger = options?.log || console; + return async function handler( + request: IncomingMessage, + response: ServerResponse, + ): Promise { + // Check if the Content-Type header is `application/json` and allow for charset to be specified in it + // Otherwise, return a 415 Unsupported Media Type error + // See https://github.com/octokit/webhooks.js/issues/158 + if ( + !request.headers["content-type"] || + !request.headers["content-type"].startsWith("application/json") + ) { + response.writeHead(415, { + "content-type": "application/json", + accept: "application/json", + }); + response.end( + JSON.stringify({ + error: `Unsupported "Content-Type" header value. Must be "application/json"`, + }), + ); + return true; + } + + const missingHeaders = getMissingHeaders(request).join(", "); + + if (missingHeaders) { + response.writeHead(400, { + "content-type": "application/json", + }); + response.end( + JSON.stringify({ + error: `Required headers missing: ${missingHeaders}`, + }), + ); + + return true; + } + + const eventName = request.headers["x-github-event"] as WebhookEventName; + const signatureSHA256 = request.headers["x-hub-signature-256"] as string; + const id = request.headers["x-github-delivery"] as string; + + logger.debug(`${eventName} event received (id: ${id})`); + + // GitHub will abort the request if it does not receive a response within 10s + // See https://github.com/octokit/webhooks.js/issues/185 + let didTimeout = false; + const timeout = setTimeout(() => { + didTimeout = true; + response.statusCode = 202; + response.end("still processing\n"); + }, 9000).unref(); + + try { + // If request.body already exists we dont need to wait for getPayload + // See https://github.com/octokit/webhooks.js/pull/23 + + const payload = request.body || (await getPayload(request)); + + await webhooks.verifyAndReceive({ + id: id, + name: eventName as any, + payload, + signature: signatureSHA256, + }); + clearTimeout(timeout); + + if (didTimeout) return true; + + response.end("ok\n"); + return true; + } catch (error) { + clearTimeout(timeout); + + if (didTimeout) return true; + + const err = Array.from(error as WebhookEventHandlerError)[0]; + const errorMessage = err.message + ? `${err.name}: ${err.message}` + : "Error: An Unspecified error occurred"; + response.statusCode = + typeof err.status !== "undefined" ? err.status : 500; + + logger.error(error); + + response.end( + JSON.stringify({ + error: errorMessage, + }), + ); + + return true; + } + }; +} diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts index 4947172d..a27cfee8 100644 --- a/src/middleware/node/middleware.ts +++ b/src/middleware/node/middleware.ts @@ -4,14 +4,10 @@ type IncomingMessage = any; type ServerResponse = any; -import type { WebhookEventName } from "@octokit/webhooks-types"; - import type { Webhooks } from "../../index.js"; -import type { WebhookEventHandlerError } from "../../types.js"; import type { MiddlewareOptions } from "./types.js"; -import { getMissingHeaders } from "./get-missing-headers.js"; -import { getPayload } from "./get-payload.js"; import { onUnhandledRequestDefault } from "./on-unhandled-request-default.js"; +import { createNodeHandler } from "./handler.js"; export async function middleware( webhooks: Webhooks, @@ -43,89 +39,5 @@ export async function middleware( return true; } - // Check if the Content-Type header is `application/json` and allow for charset to be specified in it - // Otherwise, return a 415 Unsupported Media Type error - // See https://github.com/octokit/webhooks.js/issues/158 - if ( - !request.headers["content-type"] || - !request.headers["content-type"].startsWith("application/json") - ) { - response.writeHead(415, { - "content-type": "application/json", - accept: "application/json", - }); - response.end( - JSON.stringify({ - error: `Unsupported "Content-Type" header value. Must be "application/json"`, - }), - ); - return true; - } - - const missingHeaders = getMissingHeaders(request).join(", "); - - if (missingHeaders) { - response.writeHead(400, { - "content-type": "application/json", - }); - response.end( - JSON.stringify({ - error: `Required headers missing: ${missingHeaders}`, - }), - ); - - return true; - } - - const eventName = request.headers["x-github-event"] as WebhookEventName; - const signatureSHA256 = request.headers["x-hub-signature-256"] as string; - const id = request.headers["x-github-delivery"] as string; - - options.log.debug(`${eventName} event received (id: ${id})`); - - // GitHub will abort the request if it does not receive a response within 10s - // See https://github.com/octokit/webhooks.js/issues/185 - let didTimeout = false; - const timeout = setTimeout(() => { - didTimeout = true; - response.statusCode = 202; - response.end("still processing\n"); - }, 9000).unref(); - - try { - const payload = await getPayload(request); - - await webhooks.verifyAndReceive({ - id: id, - name: eventName as any, - payload, - signature: signatureSHA256, - }); - clearTimeout(timeout); - - if (didTimeout) return true; - - response.end("ok\n"); - return true; - } catch (error) { - clearTimeout(timeout); - - if (didTimeout) return true; - - const err = Array.from(error as WebhookEventHandlerError)[0]; - const errorMessage = err.message - ? `${err.name}: ${err.message}` - : "Error: An Unspecified error occurred"; - response.statusCode = typeof err.status !== "undefined" ? err.status : 500; - - options.log.error(error); - - response.end( - JSON.stringify({ - error: errorMessage, - }), - ); - - return true; - } + return createNodeHandler(webhooks, options)(request, response); } diff --git a/test/integration/node-handler.test.ts b/test/integration/node-handler.test.ts new file mode 100644 index 00000000..f02aa16c --- /dev/null +++ b/test/integration/node-handler.test.ts @@ -0,0 +1,454 @@ +import { createServer } from "node:http"; +import { readFileSync } from "node:fs"; + +import { sign } from "@octokit/webhooks-methods"; + +import { createNodeHandler, Webhooks } from "../../src/index.ts"; + +const pushEventPayload = readFileSync( + "test/fixtures/push-payload.json", + "utf-8", +); +let signatureSha256: string; + +describe("createNodeHandler(webhooks)", () => { + beforeAll(async () => { + signatureSha256 = await sign( + { secret: "mySecret", algorithm: "sha256" }, + pushEventPayload, + ); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test("README example", async () => { + expect.assertions(3); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.on("push", (event) => { + expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000"); + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + expect(response.status).toEqual(200); + await expect(response.text()).resolves.toBe("ok\n"); + + server.close(); + }); + + test("request.body already parsed (e.g. Lambda)", async () => { + expect.assertions(3); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + const dataChunks: any[] = []; + const middleware = createNodeHandler(webhooks); + + const server = createServer((req, res) => { + req.once("data", (chunk) => dataChunks.push(chunk)); + req.once("end", () => { + // @ts-expect-error - TS2339: Property 'body' does not exist on type 'IncomingMessage'. + req.body = Buffer.concat(dataChunks).toString(); + middleware(req, res); + }); + }).listen(); + + webhooks.on("push", (event) => { + expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000"); + }); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + expect(response.status).toEqual(200); + expect(await response.text()).toEqual("ok\n"); + + server.close(); + }); + + test("Handles invalid Content-Type", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "text/plain", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toBe( + '{"error":"Unsupported \\"Content-Type\\" header value. Must be \\"application/json\\""}', + ); + expect(response.status).toEqual(415); + + server.close(); + }); + + test("Handles Missing Content-Type", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toBe( + '{"error":"Unsupported \\"Content-Type\\" header value. Must be \\"application/json\\""}', + ); + expect(response.status).toEqual(415); + + server.close(); + }); + + test("Handles invalid JSON", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const payload = '{"name":"invalid"'; + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": await sign("mySecret", payload), + }, + body: payload, + }, + ); + + expect(response.status).toEqual(400); + + await expect(response.text()).resolves.toMatch(/SyntaxError: Invalid JSON/); + + server.close(); + }); + + test("Handles non POST request", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "PUT", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: "invalid", + }, + ); + + expect(response.status).toEqual(400); + + await expect(response.text()).resolves.toMatch( + /{"error":"Error: \[@octokit\/webhooks\] signature does not match event payload and secret"}/, + ); + + server.close(); + }); + + test("Handles missing headers", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + // "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: "invalid", + }, + ); + + expect(response.status).toEqual(400); + + await expect(response.text()).resolves.toMatch( + /Required headers missing: x-github-event/, + ); + + server.close(); + }); + + test("Handles non-request errors", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.on("push", () => { + throw new Error("boom"); + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toMatch(/Error: boom/); + expect(response.status).toEqual(500); + + server.close(); + }); + + test("Handles empty errors", async () => { + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.on("push", () => { + throw new Error(); + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toMatch( + /Error: An Unspecified error occurred/, + ); + expect(response.status).toEqual(500); + + server.close(); + }); + + test("Handles timeout", async () => { + jest.useFakeTimers(); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.on("push", async () => { + jest.advanceTimersByTime(10000); + server.close(); + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toMatch(/still processing/); + expect(response.status).toEqual(202); + }); + + test("Handles timeout with error", async () => { + jest.useFakeTimers(); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.on("push", async () => { + jest.advanceTimersByTime(10000); + server.close(); + throw new Error("oops"); + }); + + const server = createServer(createNodeHandler(webhooks)).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }, + ); + + await expect(response.text()).resolves.toMatch(/still processing/); + expect(response.status).toEqual(202); + }); + + test("Handles invalid signature", async () => { + expect.assertions(3); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + webhooks.onError((error) => { + expect(error.message).toContain( + "signature does not match event payload and secret", + ); + }); + + const log = { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }; + const middleware = createNodeHandler(webhooks, { log }); + const server = createServer(middleware).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "X-GitHub-Delivery": "1", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": "", + }, + body: pushEventPayload, + }, + ); + + expect(response.status).toEqual(400); + await expect(response.text()).resolves.toBe( + '{"error":"Error: [@octokit/webhooks] signature does not match event payload and secret"}', + ); + + server.close(); + }); +});