From 19efd8168d90c482e25967954cc42a1b7e62b029 Mon Sep 17 00:00:00 2001 From: xmtp-coder-agent <> Date: Wed, 25 Mar 2026 20:28:28 +0000 Subject: [PATCH 1/3] refactor: extract webhook signature validation into Hono middleware Move GitHub webhook signature verification (missing signature check, HMAC validation) from inline route handler logic into a dedicated webhookSignatureMiddleware. The middleware stores the verified raw body in Hono context variables for downstream use. Resolves https://github.com/xmtplabs/coder-action/issues/77 Co-Authored-By: Claude Opus 4.6 --- src/server.ts | 164 +++++++++++++++++++++++++++++--------------------- 1 file changed, 96 insertions(+), 68 deletions(-) diff --git a/src/server.ts b/src/server.ts index 0b455d2..31a9b9d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,4 +1,5 @@ import { Hono } from "hono"; +import { createMiddleware } from "hono/factory"; import type { ContentfulStatusCode } from "hono/utils/http-status"; import { verify } from "@octokit/webhooks-methods"; import type { Logger } from "./logger"; @@ -34,16 +35,21 @@ function safeStringField(payload: unknown, ...path: string[]): string | null { return typeof current === "string" ? current : null; } -export function createApp(options: CreateAppOptions): Hono { - const { webhookSecret, handleWebhook, logger } = options; - const app = new Hono(); - - app.get("/healthz", (c) => { - return c.text("ok", 200); - }); +type WebhookEnv = { + Variables: { + rawBody: string; + }; +}; - app.post("/api/webhooks", async (c) => { - const startTime = Date.now(); +/** + * Hono middleware that verifies the GitHub webhook signature. + * On success, stores the raw request body in `c.var.rawBody` for downstream handlers. + */ +export function webhookSignatureMiddleware( + webhookSecret: string, + logger: Logger, +) { + return createMiddleware(async (c, next) => { const rawBody = await c.req.text(); const signature = c.req.header("X-Hub-Signature-256"); @@ -72,73 +78,95 @@ export function createApp(options: CreateAppOptions): Hono { return c.text("Unauthorized: invalid signature", 401); } - const eventName = c.req.header("X-GitHub-Event"); - if (!eventName) { - logger.info("Webhook rejected: missing X-GitHub-Event", { - event: null, - delivery_id: null, - status: 400, - }); - return c.text("Bad Request: missing X-GitHub-Event header", 400); - } - - const deliveryId = c.req.header("X-GitHub-Delivery") ?? "unknown"; - - let payload: unknown; - try { - payload = JSON.parse(rawBody); - } catch { - logger.error("Webhook rejected: invalid JSON body", { - event: eventName, - delivery_id: deliveryId, - status: 400, - }); - return c.text("Bad Request: invalid JSON body", 400); - } - - // Create per-request child logger with request context - const reqLogger = logger.child({ - deliveryId, - eventName, - }); + c.set("rawBody", rawBody); + await next(); + }); +} - reqLogger.info("Raw webhook received", { payload }); +export function createApp(options: CreateAppOptions): Hono { + const { webhookSecret, handleWebhook, logger } = options; + const app = new Hono(); - // Extract action and repository for structured logging - const payloadAction = safeStringField(payload, "action"); - const payloadRepo = safeStringField(payload, "repository", "full_name"); + app.get("/healthz", (c) => { + return c.text("ok", 200); + }); - let handleResult: WebhookHandleResult; - try { - handleResult = await handleWebhook( - eventName, + app.post( + "/api/webhooks", + webhookSignatureMiddleware(webhookSecret, logger), + async (c) => { + const startTime = Date.now(); + const rawBody = c.get("rawBody"); + + const eventName = c.req.header("X-GitHub-Event"); + if (!eventName) { + logger.info("Webhook rejected: missing X-GitHub-Event", { + event: null, + delivery_id: null, + status: 400, + }); + return c.text("Bad Request: missing X-GitHub-Event header", 400); + } + + const deliveryId = c.req.header("X-GitHub-Delivery") ?? "unknown"; + + let payload: unknown; + try { + payload = JSON.parse(rawBody); + } catch { + logger.error("Webhook rejected: invalid JSON body", { + event: eventName, + delivery_id: deliveryId, + status: 400, + }); + return c.text("Bad Request: invalid JSON body", 400); + } + + // Create per-request child logger with request context + const reqLogger = logger.child({ deliveryId, - payload, - reqLogger, - ); - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - reqLogger.error(`Webhook handler error: ${message}`, { + eventName, + }); + + reqLogger.info("Raw webhook received", { payload }); + + // Extract action and repository for structured logging + const payloadAction = safeStringField(payload, "action"); + const payloadRepo = safeStringField(payload, "repository", "full_name"); + + let handleResult: WebhookHandleResult; + try { + handleResult = await handleWebhook( + eventName, + deliveryId, + payload, + reqLogger, + ); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + reqLogger.error(`Webhook handler error: ${message}`, { + action: payloadAction, + repository: payloadRepo, + status: 500, + error: message, + duration_ms: Date.now() - startTime, + }); + return c.text("Internal Server Error", 500); + } + + const responseStatus = (handleResult.status ?? + 200) as ContentfulStatusCode; + reqLogger.info("Webhook processed", { action: payloadAction, repository: payloadRepo, - status: 500, - error: message, + handler: handleResult.handler ?? null, + dispatched: handleResult.dispatched, + status: responseStatus, duration_ms: Date.now() - startTime, }); - return c.text("Internal Server Error", 500); - } - - const responseStatus = (handleResult.status ?? 200) as ContentfulStatusCode; - reqLogger.info("Webhook processed", { - action: payloadAction, - repository: payloadRepo, - handler: handleResult.handler ?? null, - dispatched: handleResult.dispatched, - status: responseStatus, - duration_ms: Date.now() - startTime, - }); - return c.text("ok", responseStatus); - }); + return c.text("ok", responseStatus); + }, + ); return app; } From f6018a1655c570b910154fa07c1f02b118a87dff Mon Sep 17 00:00:00 2001 From: xmtp-coder-agent <> Date: Wed, 25 Mar 2026 20:41:35 +0000 Subject: [PATCH 2/3] refactor: move webhook signature middleware to its own file Extract webhookSignatureMiddleware and WebhookEnv type into src/middleware.ts per review feedback. Co-Authored-By: Claude Opus 4.6 --- src/middleware.ts | 51 +++++++++++++++++++++++++++++++++++++++++++++++ src/server.ts | 51 +---------------------------------------------- 2 files changed, 52 insertions(+), 50 deletions(-) create mode 100644 src/middleware.ts diff --git a/src/middleware.ts b/src/middleware.ts new file mode 100644 index 0000000..7716734 --- /dev/null +++ b/src/middleware.ts @@ -0,0 +1,51 @@ +import { createMiddleware } from "hono/factory"; +import { verify } from "@octokit/webhooks-methods"; +import type { Logger } from "./logger"; + +export type WebhookEnv = { + Variables: { + rawBody: string; + }; +}; + +/** + * Hono middleware that verifies the GitHub webhook signature. + * On success, stores the raw request body in `c.var.rawBody` for downstream handlers. + */ +export function webhookSignatureMiddleware( + webhookSecret: string, + logger: Logger, +) { + return createMiddleware(async (c, next) => { + const rawBody = await c.req.text(); + + const signature = c.req.header("X-Hub-Signature-256"); + if (!signature) { + logger.info("Webhook rejected: missing signature", { + event: null, + delivery_id: null, + status: 401, + }); + return c.text("Unauthorized: missing signature", 401); + } + + let signatureValid: boolean; + try { + signatureValid = await verify(webhookSecret, rawBody, signature); + } catch { + signatureValid = false; + } + + if (!signatureValid) { + logger.info("Webhook rejected: invalid signature", { + event: null, + delivery_id: null, + status: 401, + }); + return c.text("Unauthorized: invalid signature", 401); + } + + c.set("rawBody", rawBody); + await next(); + }); +} diff --git a/src/server.ts b/src/server.ts index 31a9b9d..4cc176c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,8 +1,7 @@ import { Hono } from "hono"; -import { createMiddleware } from "hono/factory"; import type { ContentfulStatusCode } from "hono/utils/http-status"; -import { verify } from "@octokit/webhooks-methods"; import type { Logger } from "./logger"; +import { type WebhookEnv, webhookSignatureMiddleware } from "./middleware"; export interface WebhookHandleResult { dispatched: boolean; @@ -35,54 +34,6 @@ function safeStringField(payload: unknown, ...path: string[]): string | null { return typeof current === "string" ? current : null; } -type WebhookEnv = { - Variables: { - rawBody: string; - }; -}; - -/** - * Hono middleware that verifies the GitHub webhook signature. - * On success, stores the raw request body in `c.var.rawBody` for downstream handlers. - */ -export function webhookSignatureMiddleware( - webhookSecret: string, - logger: Logger, -) { - return createMiddleware(async (c, next) => { - const rawBody = await c.req.text(); - - const signature = c.req.header("X-Hub-Signature-256"); - if (!signature) { - logger.info("Webhook rejected: missing signature", { - event: null, - delivery_id: null, - status: 401, - }); - return c.text("Unauthorized: missing signature", 401); - } - - let signatureValid: boolean; - try { - signatureValid = await verify(webhookSecret, rawBody, signature); - } catch { - signatureValid = false; - } - - if (!signatureValid) { - logger.info("Webhook rejected: invalid signature", { - event: null, - delivery_id: null, - status: 401, - }); - return c.text("Unauthorized: invalid signature", 401); - } - - c.set("rawBody", rawBody); - await next(); - }); -} - export function createApp(options: CreateAppOptions): Hono { const { webhookSecret, handleWebhook, logger } = options; const app = new Hono(); From 15c9507186ea5e3e08661685c3784684d2d92410 Mon Sep 17 00:00:00 2001 From: xmtp-coder-agent <> Date: Wed, 25 Mar 2026 20:44:29 +0000 Subject: [PATCH 3/3] refactor: extract signature error texts into constants Co-Authored-By: Claude Opus 4.6 --- src/middleware.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/middleware.ts b/src/middleware.ts index 7716734..87b1b31 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -2,6 +2,9 @@ import { createMiddleware } from "hono/factory"; import { verify } from "@octokit/webhooks-methods"; import type { Logger } from "./logger"; +const ERR_MISSING_SIGNATURE = "Unauthorized: missing signature"; +const ERR_INVALID_SIGNATURE = "Unauthorized: invalid signature"; + export type WebhookEnv = { Variables: { rawBody: string; @@ -26,7 +29,7 @@ export function webhookSignatureMiddleware( delivery_id: null, status: 401, }); - return c.text("Unauthorized: missing signature", 401); + return c.text(ERR_MISSING_SIGNATURE, 401); } let signatureValid: boolean; @@ -42,7 +45,7 @@ export function webhookSignatureMiddleware( delivery_id: null, status: 401, }); - return c.text("Unauthorized: invalid signature", 401); + return c.text(ERR_INVALID_SIGNATURE, 401); } c.set("rawBody", rawBody);