From 48cbfdfac0883c07d5cd00bdf9a13f028bb4e18e Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 12 Mar 2026 10:50:36 -0400 Subject: [PATCH] Hardening: require LINE webhook signatures (#44090) * LINE: require webhook signatures in express handler * LINE: require webhook signatures in node handler * LINE: update express signature tests * LINE: update node signature tests * Changelog: note LINE webhook hardening * LINE: validate signatures before parsing webhook bodies * LINE: reject missing signatures before body reads --- CHANGELOG.md | 1 + src/line/webhook-node.test.ts | 24 ++++++++++++++++------- src/line/webhook-node.ts | 37 ++++++++++++----------------------- src/line/webhook-utils.ts | 6 ------ src/line/webhook.test.ts | 19 +++++++++++++++++- src/line/webhook.ts | 22 ++++++++++----------- 6 files changed, 60 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04511504943..bde1850b7ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai - Security/browser.request: block persistent browser profile create/delete routes from write-scoped `browser.request` so callers can no longer persist admin-only browser profile changes through the browser control surface. (`GHSA-vmhq-cqm9-6p7q`)(#43800) Thanks @tdjackey and @vincentkoc. - Security/agent: reject public spawned-run lineage fields and keep workspace inheritance on the internal spawned-session path so external `agent` callers can no longer override the gateway workspace boundary. (`GHSA-2rqg-gjgv-84jm`)(#43801) Thanks @tdjackey and @vincentkoc. - Security/exec allowlist: preserve POSIX case sensitivity and keep `?` within a single path segment so exact-looking allowlist patterns no longer overmatch executables across case or directory boundaries. (`GHSA-f8r2-vg7x-gh8m`)(#43798) Thanks @zpbrent and @vincentkoc. +- Security/LINE webhook: require signatures for empty-event POST probes too so unsigned requests no longer confirm webhook reachability with a `200` response. (`GHSA-mhxh-9pjm-w7q5`)(#44090) Thanks @TerminalsandCoffee and @vincentkoc. ### Changes diff --git a/src/line/webhook-node.test.ts b/src/line/webhook-node.test.ts index 82cc8d1f1f0..e4d8d7870f5 100644 --- a/src/line/webhook-node.test.ts +++ b/src/line/webhook-node.test.ts @@ -86,13 +86,26 @@ describe("createLineNodeWebhookHandler", () => { expect(res.body).toBeUndefined(); }); - it("returns 200 for verification request (empty events, no signature)", async () => { + it("rejects verification-shaped requests without a signature", async () => { const rawBody = JSON.stringify({ events: [] }); const { bot, handler } = createPostWebhookTestHarness(rawBody); const { res, headers } = createRes(); await handler({ method: "POST", headers: {} } as unknown as IncomingMessage, res); + expect(res.statusCode).toBe(400); + expect(headers["content-type"]).toBe("application/json"); + expect(res.body).toBe(JSON.stringify({ error: "Missing X-Line-Signature header" })); + expect(bot.handleWebhook).not.toHaveBeenCalled(); + }); + + it("accepts signed verification-shaped requests without dispatching events", async () => { + const rawBody = JSON.stringify({ events: [] }); + const { bot, handler, secret } = createPostWebhookTestHarness(rawBody); + + const { res, headers } = createRes(); + await runSignedPost({ handler, rawBody, secret, res }); + expect(res.statusCode).toBe(200); expect(headers["content-type"]).toBe("application/json"); expect(res.body).toBe(JSON.stringify({ status: "ok" })); @@ -121,13 +134,10 @@ describe("createLineNodeWebhookHandler", () => { expect(bot.handleWebhook).not.toHaveBeenCalled(); }); - it("uses a tight body-read limit for unsigned POST requests", async () => { + it("rejects unsigned POST requests before reading the body", async () => { const bot = { handleWebhook: vi.fn(async () => {}) }; const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; - const readBody = vi.fn(async (_req: IncomingMessage, maxBytes: number) => { - expect(maxBytes).toBe(4096); - return JSON.stringify({ events: [{ type: "message" }] }); - }); + const readBody = vi.fn(async () => JSON.stringify({ events: [{ type: "message" }] })); const handler = createLineNodeWebhookHandler({ channelSecret: "secret", bot, @@ -139,7 +149,7 @@ describe("createLineNodeWebhookHandler", () => { await handler({ method: "POST", headers: {} } as unknown as IncomingMessage, res); expect(res.statusCode).toBe(400); - expect(readBody).toHaveBeenCalledTimes(1); + expect(readBody).not.toHaveBeenCalled(); expect(bot.handleWebhook).not.toHaveBeenCalled(); }); diff --git a/src/line/webhook-node.ts b/src/line/webhook-node.ts index 7d531cbed55..9bbc45b258a 100644 --- a/src/line/webhook-node.ts +++ b/src/line/webhook-node.ts @@ -8,11 +8,10 @@ import { } from "../infra/http-body.js"; import type { RuntimeEnv } from "../runtime.js"; import { validateLineSignature } from "./signature.js"; -import { isLineWebhookVerificationRequest, parseLineWebhookBody } from "./webhook-utils.js"; +import { parseLineWebhookBody } from "./webhook-utils.js"; const LINE_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024; const LINE_WEBHOOK_PREAUTH_MAX_BODY_BYTES = 64 * 1024; -const LINE_WEBHOOK_UNSIGNED_MAX_BODY_BYTES = 4 * 1024; const LINE_WEBHOOK_PREAUTH_BODY_TIMEOUT_MS = 5_000; export async function readLineWebhookRequestBody( @@ -65,30 +64,12 @@ export function createLineNodeWebhookHandler(params: { const signatureHeader = req.headers["x-line-signature"]; const signature = typeof signatureHeader === "string" - ? signatureHeader + ? signatureHeader.trim() : Array.isArray(signatureHeader) - ? signatureHeader[0] - : undefined; - const hasSignature = typeof signature === "string" && signature.trim().length > 0; - const bodyLimit = hasSignature - ? Math.min(maxBodyBytes, LINE_WEBHOOK_PREAUTH_MAX_BODY_BYTES) - : Math.min(maxBodyBytes, LINE_WEBHOOK_UNSIGNED_MAX_BODY_BYTES); - const rawBody = await readBody(req, bodyLimit, LINE_WEBHOOK_PREAUTH_BODY_TIMEOUT_MS); + ? (signatureHeader[0] ?? "").trim() + : ""; - // Parse once; we may need it for verification requests and for event processing. - const body = parseLineWebhookBody(rawBody); - - // LINE webhook verification sends POST {"events":[]} without a - // signature header. Return 200 so the LINE Developers Console - // "Verify" button succeeds. - if (!hasSignature) { - if (isLineWebhookVerificationRequest(body)) { - logVerbose("line: webhook verification request (empty events, no signature) - 200 OK"); - res.statusCode = 200; - res.setHeader("Content-Type", "application/json"); - res.end(JSON.stringify({ status: "ok" })); - return; - } + if (!signature) { logVerbose("line: webhook missing X-Line-Signature header"); res.statusCode = 400; res.setHeader("Content-Type", "application/json"); @@ -96,6 +77,12 @@ export function createLineNodeWebhookHandler(params: { return; } + const rawBody = await readBody( + req, + Math.min(maxBodyBytes, LINE_WEBHOOK_PREAUTH_MAX_BODY_BYTES), + LINE_WEBHOOK_PREAUTH_BODY_TIMEOUT_MS, + ); + if (!validateLineSignature(rawBody, signature, params.channelSecret)) { logVerbose("line: webhook signature validation failed"); res.statusCode = 401; @@ -104,6 +91,8 @@ export function createLineNodeWebhookHandler(params: { return; } + const body = parseLineWebhookBody(rawBody); + if (!body) { res.statusCode = 400; res.setHeader("Content-Type", "application/json"); diff --git a/src/line/webhook-utils.ts b/src/line/webhook-utils.ts index a0ea410fefe..1f0a8dee69b 100644 --- a/src/line/webhook-utils.ts +++ b/src/line/webhook-utils.ts @@ -7,9 +7,3 @@ export function parseLineWebhookBody(rawBody: string): WebhookRequestBody | null return null; } } - -export function isLineWebhookVerificationRequest( - body: WebhookRequestBody | null | undefined, -): boolean { - return !!body && Array.isArray(body.events) && body.events.length === 0; -} diff --git a/src/line/webhook.test.ts b/src/line/webhook.test.ts index 19640fd3114..9b3b9c0539a 100644 --- a/src/line/webhook.test.ts +++ b/src/line/webhook.test.ts @@ -87,17 +87,34 @@ describe("createLineWebhookMiddleware", () => { expect(onEvents).not.toHaveBeenCalled(); }); - it("returns 200 for verification request (empty events, no signature)", async () => { + it("rejects verification-shaped requests without a signature", async () => { const { res, onEvents } = await invokeWebhook({ body: JSON.stringify({ events: [] }), headers: {}, autoSign: false, }); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ error: "Missing X-Line-Signature header" }); + expect(onEvents).not.toHaveBeenCalled(); + }); + + it("accepts signed verification-shaped requests without dispatching events", async () => { + const { res, onEvents } = await invokeWebhook({ + body: JSON.stringify({ events: [] }), + }); expect(res.status).toHaveBeenCalledWith(200); expect(res.json).toHaveBeenCalledWith({ status: "ok" }); expect(onEvents).not.toHaveBeenCalled(); }); + it("rejects oversized signed payloads before JSON parsing", async () => { + const largeBody = JSON.stringify({ events: [], payload: "x".repeat(70 * 1024) }); + const { res, onEvents } = await invokeWebhook({ body: largeBody }); + expect(res.status).toHaveBeenCalledWith(413); + expect(res.json).toHaveBeenCalledWith({ error: "Payload too large" }); + expect(onEvents).not.toHaveBeenCalled(); + }); + it("rejects missing signature when events are non-empty", async () => { const { res, onEvents } = await invokeWebhook({ body: JSON.stringify({ events: [{ type: "message" }] }), diff --git a/src/line/webhook.ts b/src/line/webhook.ts index d16ee4aa7c9..99c338db2f9 100644 --- a/src/line/webhook.ts +++ b/src/line/webhook.ts @@ -3,7 +3,9 @@ import type { Request, Response, NextFunction } from "express"; import { logVerbose, danger } from "../globals.js"; import type { RuntimeEnv } from "../runtime.js"; import { validateLineSignature } from "./signature.js"; -import { isLineWebhookVerificationRequest, parseLineWebhookBody } from "./webhook-utils.js"; +import { parseLineWebhookBody } from "./webhook-utils.js"; + +const LINE_WEBHOOK_MAX_RAW_BODY_BYTES = 64 * 1024; export interface LineWebhookOptions { channelSecret: string; @@ -39,26 +41,22 @@ export function createLineWebhookMiddleware( return async (req: Request, res: Response, _next: NextFunction): Promise => { try { const signature = req.headers["x-line-signature"]; - const rawBody = readRawBody(req); - const body = parseWebhookBody(req, rawBody); - // LINE webhook verification sends POST {"events":[]} without a - // signature header. Return 200 immediately so the LINE Developers - // Console "Verify" button succeeds. if (!signature || typeof signature !== "string") { - if (isLineWebhookVerificationRequest(body)) { - logVerbose("line: webhook verification request (empty events, no signature) - 200 OK"); - res.status(200).json({ status: "ok" }); - return; - } res.status(400).json({ error: "Missing X-Line-Signature header" }); return; } + const rawBody = readRawBody(req); + if (!rawBody) { res.status(400).json({ error: "Missing raw request body for signature verification" }); return; } + if (Buffer.byteLength(rawBody, "utf-8") > LINE_WEBHOOK_MAX_RAW_BODY_BYTES) { + res.status(413).json({ error: "Payload too large" }); + return; + } if (!validateLineSignature(rawBody, signature, channelSecret)) { logVerbose("line: webhook signature validation failed"); @@ -66,6 +64,8 @@ export function createLineWebhookMiddleware( return; } + const body = parseWebhookBody(req, rawBody); + if (!body) { res.status(400).json({ error: "Invalid webhook payload" }); return;