From 9a5bfb1fe56d9000a6e089bf0c3fca8081710bb0 Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:47:56 -0600 Subject: [PATCH] fix(line): synthesize media/auth/routing webhook regressions (openclaw#32546) thanks @Takhoffman Verified: - pnpm build - pnpm check - pnpm test:macmini Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- CHANGELOG.md | 5 ++ src/line/bot-handlers.test.ts | 102 ++++++++++++++++++++++- src/line/bot-handlers.ts | 99 ++++++++++++++++++---- src/line/bot-message-context.test.ts | 120 +++++++++++++++++++++++++++ src/line/bot-message-context.ts | 14 +++- src/line/download.test.ts | 40 ++------- src/line/download.ts | 25 +++--- src/line/webhook-node.test.ts | 47 ++++++++++- src/line/webhook-node.ts | 15 ++-- src/line/webhook.test.ts | 14 ++++ src/line/webhook.ts | 4 +- 11 files changed, 409 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b85503c092..e0cb53962a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ Docs: https://docs.openclaw.ai ### Fixes +- LINE/media download synthesis: fix file-media download handling and M4A audio classification across overlapping LINE regressions. (from #26386, #27761, #27787, #29509, #29755, #29776, #29785, #32240) Thanks @kevinWangSheng, @loiie45e, @carrotRakko, @Sid-Qin, @codeafridi, and @bmendonca3. +- LINE/context and routing synthesis: fix group/room peer routing and command-authorization context propagation, and keep processing later events in mixed-success webhook batches. (from #21955, #24475, #27035, #28286) Thanks @lailoo, @mcaxtr, @jervyclaw, @Glucksberg, and @Takhoffman. +- LINE/status/config/webhook synthesis: fix status false positives from snapshot/config state and accept LINE webhook HEAD probes for compatibility. (from #10487, #25726, #27537, #27908, #31387) Thanks @BlueBirdBack, @stakeswky, @loiie45e, @puritysb, and @mcaxtr. +- LINE cleanup/test follow-ups: fold cleanup/test learnings into the synthesis review path while keeping runtime changes focused on regression fixes. (from #17630, #17289) Thanks @Clawborn and @davidahmann. + ## 2026.3.2 ### Changes diff --git a/src/line/bot-handlers.test.ts b/src/line/bot-handlers.test.ts index 54125e5c65c..85c8b273161 100644 --- a/src/line/bot-handlers.test.ts +++ b/src/line/bot-handlers.test.ts @@ -16,10 +16,15 @@ vi.mock("../pairing/pairing-messages.js", () => ({ buildPairingReply: () => "pairing-reply", })); +const { downloadLineMediaMock } = vi.hoisted(() => ({ + downloadLineMediaMock: vi.fn(async () => ({ + path: "/tmp/line-media-file.pdf", + contentType: "application/pdf", + })), +})); + vi.mock("./download.js", () => ({ - downloadLineMedia: async () => { - throw new Error("downloadLineMedia should not be called from bot-handlers tests"); - }, + downloadLineMedia: downloadLineMediaMock, })); vi.mock("./send.js", () => ({ @@ -80,6 +85,7 @@ describe("handleLineWebhookEvents", () => { beforeEach(() => { buildLineMessageContextMock.mockClear(); buildLinePostbackContextMock.mockClear(); + downloadLineMediaMock.mockClear(); readAllowFromStoreMock.mockClear(); upsertPairingRequestMock.mockClear(); }); @@ -248,4 +254,94 @@ describe("handleLineWebhookEvents", () => { expect(processMessage).not.toHaveBeenCalled(); expect(buildLineMessageContextMock).not.toHaveBeenCalled(); }); + + it("downloads file attachments and forwards media refs to message context", async () => { + const processMessage = vi.fn(); + const event = { + type: "message", + message: { id: "mf-1", type: "file", fileName: "doc.pdf", fileSize: "42" }, + replyToken: "reply-token", + timestamp: Date.now(), + source: { type: "user", userId: "user-file" }, + mode: "active", + webhookEventId: "evt-file-1", + deliveryContext: { isRedelivery: false }, + } as MessageEvent; + + await handleLineWebhookEvents([event], { + cfg: { channels: { line: {} } }, + account: { + accountId: "default", + enabled: true, + channelAccessToken: "token", + channelSecret: "secret", + tokenSource: "config", + config: { dmPolicy: "open" }, + }, + runtime: createRuntime(), + mediaMaxBytes: 1234, + processMessage, + }); + + expect(downloadLineMediaMock).toHaveBeenCalledTimes(1); + expect(downloadLineMediaMock).toHaveBeenCalledWith("mf-1", "token", 1234); + expect(buildLineMessageContextMock).toHaveBeenCalledTimes(1); + expect(buildLineMessageContextMock).toHaveBeenCalledWith( + expect.objectContaining({ + commandAuthorized: false, + allMedia: [ + { + path: "/tmp/line-media-file.pdf", + contentType: "application/pdf", + }, + ], + }), + ); + expect(processMessage).toHaveBeenCalledTimes(1); + }); + + it("continues processing later events when one event handler fails", async () => { + const failingEvent = { + type: "message", + message: { id: "m-err", type: "text", text: "hi" }, + replyToken: "reply-token", + timestamp: Date.now(), + source: { type: "user", userId: "user-err" }, + mode: "active", + webhookEventId: "evt-err", + deliveryContext: { isRedelivery: false }, + } as MessageEvent; + const laterEvent = { + ...failingEvent, + message: { id: "m-later", type: "text", text: "hello" }, + webhookEventId: "evt-later", + } as MessageEvent; + const runtime = createRuntime(); + let invocation = 0; + const processMessage = vi.fn(async () => { + if (invocation === 0) { + invocation += 1; + throw new Error("boom"); + } + invocation += 1; + }); + + await handleLineWebhookEvents([failingEvent, laterEvent], { + cfg: { channels: { line: {} } }, + account: { + accountId: "default", + enabled: true, + channelAccessToken: "token", + channelSecret: "secret", + tokenSource: "config", + config: { dmPolicy: "open" }, + }, + runtime, + mediaMaxBytes: 1234, + processMessage, + }); + + expect(processMessage).toHaveBeenCalledTimes(2); + expect(runtime.error).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/line/bot-handlers.ts b/src/line/bot-handlers.ts index ae432bcc599..52b0cd76095 100644 --- a/src/line/bot-handlers.ts +++ b/src/line/bot-handlers.ts @@ -7,6 +7,8 @@ import type { LeaveEvent, PostbackEvent, } from "@line/bot-sdk"; +import { hasControlCommand } from "../auto-reply/command-detection.js"; +import { resolveControlCommandGate } from "../channels/command-gating.js"; import type { OpenClawConfig } from "../config/config.js"; import { resolveAllowlistProviderRuntimeGroupPolicy, @@ -42,6 +44,19 @@ interface MediaRef { contentType?: string; } +const LINE_DOWNLOADABLE_MESSAGE_TYPES: ReadonlySet = new Set([ + "image", + "video", + "audio", + "file", +]); + +function isDownloadableLineMessageType( + messageType: MessageEvent["message"]["type"], +): messageType is "image" | "video" | "audio" | "file" { + return LINE_DOWNLOADABLE_MESSAGE_TYPES.has(messageType); +} + export interface LineHandlerContext { cfg: OpenClawConfig; account: ResolvedLineAccount; @@ -113,10 +128,15 @@ async function sendLinePairingReply(params: { } } +type LineAccessDecision = { + allowed: boolean; + commandAuthorized: boolean; +}; + async function shouldProcessLineEvent( event: MessageEvent | PostbackEvent, context: LineHandlerContext, -): Promise { +): Promise { const { cfg, account } = context; const { userId, groupId, roomId, isGroup } = getLineSourceInfo(event.source); const senderId = userId ?? ""; @@ -159,45 +179,59 @@ async function shouldProcessLineEvent( log: (message) => logVerbose(message), }); + const denied = { allowed: false, commandAuthorized: false }; + if (isGroup) { if (groupConfig?.enabled === false) { logVerbose(`Blocked line group ${groupId ?? roomId ?? "unknown"} (group disabled)`); - return false; + return denied; } if (typeof groupAllowOverride !== "undefined") { if (!senderId) { logVerbose("Blocked line group message (group allowFrom override, no sender ID)"); - return false; + return denied; } if (!isSenderAllowed({ allow: effectiveGroupAllow, senderId })) { logVerbose(`Blocked line group sender ${senderId} (group allowFrom override)`); - return false; + return denied; } } if (groupPolicy === "disabled") { logVerbose("Blocked line group message (groupPolicy: disabled)"); - return false; + return denied; } if (groupPolicy === "allowlist") { if (!senderId) { logVerbose("Blocked line group message (no sender ID, groupPolicy: allowlist)"); - return false; + return denied; } if (!effectiveGroupAllow.hasEntries) { logVerbose("Blocked line group message (groupPolicy: allowlist, no groupAllowFrom)"); - return false; + return denied; } if (!isSenderAllowed({ allow: effectiveGroupAllow, senderId })) { logVerbose(`Blocked line group message from ${senderId} (groupPolicy: allowlist)`); - return false; + return denied; } } - return true; + + // Resolve command authorization using the same pattern as Telegram/Discord/Slack. + const allowForCommands = effectiveGroupAllow; + const senderAllowedForCommands = isSenderAllowed({ allow: allowForCommands, senderId }); + const useAccessGroups = cfg.commands?.useAccessGroups !== false; + const rawText = resolveEventRawText(event); + const commandGate = resolveControlCommandGate({ + useAccessGroups, + authorizers: [{ configured: allowForCommands.hasEntries, allowed: senderAllowedForCommands }], + allowTextCommands: true, + hasControlCommand: hasControlCommand(rawText, cfg), + }); + return { allowed: true, commandAuthorized: commandGate.commandAuthorized }; } if (dmPolicy === "disabled") { logVerbose("Blocked line sender (dmPolicy: disabled)"); - return false; + return denied; } const dmAllowed = dmPolicy === "open" || isSenderAllowed({ allow: effectiveDmAllow, senderId }); @@ -205,7 +239,7 @@ async function shouldProcessLineEvent( if (dmPolicy === "pairing") { if (!senderId) { logVerbose("Blocked line sender (dmPolicy: pairing, no sender ID)"); - return false; + return denied; } await sendLinePairingReply({ senderId, @@ -215,24 +249,51 @@ async function shouldProcessLineEvent( } else { logVerbose(`Blocked line sender ${senderId || "unknown"} (dmPolicy: ${dmPolicy})`); } - return false; + return denied; } - return true; + // Resolve command authorization for DMs. + const allowForCommands = effectiveDmAllow; + const senderAllowedForCommands = isSenderAllowed({ allow: allowForCommands, senderId }); + const useAccessGroups = cfg.commands?.useAccessGroups !== false; + const rawText = resolveEventRawText(event); + const commandGate = resolveControlCommandGate({ + useAccessGroups, + authorizers: [{ configured: allowForCommands.hasEntries, allowed: senderAllowedForCommands }], + allowTextCommands: true, + hasControlCommand: hasControlCommand(rawText, cfg), + }); + return { allowed: true, commandAuthorized: commandGate.commandAuthorized }; +} + +/** Extract raw text from a LINE message or postback event for command detection. */ +function resolveEventRawText(event: MessageEvent | PostbackEvent): string { + if (event.type === "message") { + const msg = event.message; + if (msg.type === "text") { + return msg.text; + } + return ""; + } + if (event.type === "postback") { + return event.postback?.data?.trim() ?? ""; + } + return ""; } async function handleMessageEvent(event: MessageEvent, context: LineHandlerContext): Promise { const { cfg, account, runtime, mediaMaxBytes, processMessage } = context; const message = event.message; - if (!(await shouldProcessLineEvent(event, context))) { + const decision = await shouldProcessLineEvent(event, context); + if (!decision.allowed) { return; } // Download media if applicable const allMedia: MediaRef[] = []; - if (message.type === "image" || message.type === "video" || message.type === "audio") { + if (isDownloadableLineMessageType(message.type)) { try { const media = await downloadLineMedia(message.id, account.channelAccessToken, mediaMaxBytes); allMedia.push({ @@ -255,6 +316,7 @@ async function handleMessageEvent(event: MessageEvent, context: LineHandlerConte allMedia, cfg, account, + commandAuthorized: decision.commandAuthorized, }); if (!messageContext) { @@ -298,7 +360,8 @@ async function handlePostbackEvent( const data = event.postback.data; logVerbose(`line: received postback: ${data}`); - if (!(await shouldProcessLineEvent(event, context))) { + const decision = await shouldProcessLineEvent(event, context); + if (!decision.allowed) { return; } @@ -306,6 +369,7 @@ async function handlePostbackEvent( event, cfg: context.cfg, account: context.account, + commandAuthorized: decision.commandAuthorized, }); if (!postbackContext) { return; @@ -344,6 +408,9 @@ export async function handleLineWebhookEvents( } } catch (err) { context.runtime.error?.(danger(`line: event handler failed: ${String(err)}`)); + // Continue processing remaining events in this batch. Webhook ACK is sent + // before processing, so dropping later events here would make them unrecoverable. + continue; } } } diff --git a/src/line/bot-message-context.test.ts b/src/line/bot-message-context.test.ts index 5d61b85d386..98d1dfdebac 100644 --- a/src/line/bot-message-context.test.ts +++ b/src/line/bot-message-context.test.ts @@ -75,6 +75,7 @@ describe("buildLineMessageContext", () => { allMedia: [], cfg, account, + commandAuthorized: true, }); expect(context).not.toBeNull(); if (!context) { @@ -92,6 +93,7 @@ describe("buildLineMessageContext", () => { event, cfg, account, + commandAuthorized: true, }); expect(context?.ctxPayload.OriginatingTo).toBe("line:group:group-2"); @@ -105,9 +107,127 @@ describe("buildLineMessageContext", () => { event, cfg, account, + commandAuthorized: true, }); expect(context?.ctxPayload.OriginatingTo).toBe("line:room:room-1"); expect(context?.ctxPayload.To).toBe("line:room:room-1"); }); + + it("sets CommandAuthorized=true when authorized", async () => { + const event = createMessageEvent({ type: "user", userId: "user-auth" }); + + const context = await buildLineMessageContext({ + event, + allMedia: [], + cfg, + account, + commandAuthorized: true, + }); + + expect(context?.ctxPayload.CommandAuthorized).toBe(true); + }); + + it("sets CommandAuthorized=false when not authorized", async () => { + const event = createMessageEvent({ type: "user", userId: "user-noauth" }); + + const context = await buildLineMessageContext({ + event, + allMedia: [], + cfg, + account, + commandAuthorized: false, + }); + + expect(context?.ctxPayload.CommandAuthorized).toBe(false); + }); + + it("sets CommandAuthorized on postback context", async () => { + const event = createPostbackEvent({ type: "user", userId: "user-pb" }); + + const context = await buildLinePostbackContext({ + event, + cfg, + account, + commandAuthorized: true, + }); + + expect(context?.ctxPayload.CommandAuthorized).toBe(true); + }); + + it("group peer binding matches raw groupId without prefix (#21907)", async () => { + const groupId = "Cc7e3bece1234567890abcdef"; + const bindingCfg: OpenClawConfig = { + session: { store: storePath }, + agents: { + list: [{ id: "main" }, { id: "line-group-agent" }], + }, + bindings: [ + { + agentId: "line-group-agent", + match: { channel: "line", peer: { kind: "group", id: groupId } }, + }, + ], + }; + + const event = { + type: "message", + message: { id: "msg-1", type: "text", text: "hello" }, + replyToken: "reply-token", + timestamp: Date.now(), + source: { type: "group", groupId, userId: "user-1" }, + mode: "active", + webhookEventId: "evt-1", + deliveryContext: { isRedelivery: false }, + } as MessageEvent; + + const context = await buildLineMessageContext({ + event, + allMedia: [], + cfg: bindingCfg, + account, + commandAuthorized: true, + }); + expect(context).not.toBeNull(); + expect(context!.route.agentId).toBe("line-group-agent"); + expect(context!.route.matchedBy).toBe("binding.peer"); + }); + + it("room peer binding matches raw roomId without prefix (#21907)", async () => { + const roomId = "Rr1234567890abcdef"; + const bindingCfg: OpenClawConfig = { + session: { store: storePath }, + agents: { + list: [{ id: "main" }, { id: "line-room-agent" }], + }, + bindings: [ + { + agentId: "line-room-agent", + match: { channel: "line", peer: { kind: "group", id: roomId } }, + }, + ], + }; + + const event = { + type: "message", + message: { id: "msg-2", type: "text", text: "hello" }, + replyToken: "reply-token", + timestamp: Date.now(), + source: { type: "room", roomId, userId: "user-2" }, + mode: "active", + webhookEventId: "evt-2", + deliveryContext: { isRedelivery: false }, + } as MessageEvent; + + const context = await buildLineMessageContext({ + event, + allMedia: [], + cfg: bindingCfg, + account, + commandAuthorized: true, + }); + expect(context).not.toBeNull(); + expect(context!.route.agentId).toBe("line-room-agent"); + expect(context!.route.matchedBy).toBe("binding.peer"); + }); }); diff --git a/src/line/bot-message-context.ts b/src/line/bot-message-context.ts index 46d2e48b833..5df06b6b79c 100644 --- a/src/line/bot-message-context.ts +++ b/src/line/bot-message-context.ts @@ -22,6 +22,7 @@ interface BuildLineMessageContextParams { allMedia: MediaRef[]; cfg: OpenClawConfig; account: ResolvedLineAccount; + commandAuthorized: boolean; } export type LineSourceInfo = { @@ -49,10 +50,10 @@ export function getLineSourceInfo(source: EventSource): LineSourceInfo { function buildPeerId(source: EventSource): string { if (source.type === "group" && source.groupId) { - return `group:${source.groupId}`; + return source.groupId; } if (source.type === "room" && source.roomId) { - return `room:${source.roomId}`; + return source.roomId; } if (source.type === "user" && source.userId) { return source.userId; @@ -229,6 +230,7 @@ async function finalizeLineInboundContext(params: { rawBody: string; timestamp: number; messageSid: string; + commandAuthorized: boolean; media: { firstPath: string | undefined; firstContentType?: string; @@ -300,6 +302,7 @@ async function finalizeLineInboundContext(params: { MediaUrls: params.media.paths, MediaTypes: params.media.types, ...params.locationContext, + CommandAuthorized: params.commandAuthorized, OriginatingChannel: "line" as const, OriginatingTo: originatingTo, GroupSystemPrompt: params.source.isGroup @@ -359,7 +362,7 @@ async function finalizeLineInboundContext(params: { } export async function buildLineMessageContext(params: BuildLineMessageContextParams) { - const { event, allMedia, cfg, account } = params; + const { event, allMedia, cfg, account, commandAuthorized } = params; const source = event.source; const { userId, groupId, roomId, isGroup, peerId, route } = resolveLineInboundRoute({ @@ -405,6 +408,7 @@ export async function buildLineMessageContext(params: BuildLineMessageContextPar rawBody, timestamp, messageSid: messageId, + commandAuthorized, media: { firstPath: allMedia[0]?.path, firstContentType: allMedia[0]?.contentType, @@ -435,8 +439,9 @@ export async function buildLinePostbackContext(params: { event: PostbackEvent; cfg: OpenClawConfig; account: ResolvedLineAccount; + commandAuthorized: boolean; }) { - const { event, cfg, account } = params; + const { event, cfg, account, commandAuthorized } = params; const source = event.source; const { userId, groupId, roomId, isGroup, peerId, route } = resolveLineInboundRoute({ @@ -468,6 +473,7 @@ export async function buildLinePostbackContext(params: { rawBody, timestamp, messageSid, + commandAuthorized, media: { firstPath: "", firstContentType: undefined, diff --git a/src/line/download.test.ts b/src/line/download.test.ts index ea9a236867f..1a0d12b2a3b 100644 --- a/src/line/download.test.ts +++ b/src/line/download.test.ts @@ -67,46 +67,24 @@ describe("downloadLineMedia", () => { expect(writeSpy).not.toHaveBeenCalled(); }); - it("detects M4A audio from ftyp major brand (#29751)", async () => { - // Real M4A magic bytes: size(4) + "ftyp" + "M4A " - const m4a = Buffer.from([ - 0x00, - 0x00, - 0x00, - 0x1c, // box size - 0x66, - 0x74, - 0x79, - 0x70, // "ftyp" - 0x4d, - 0x34, - 0x41, - 0x20, // "M4A " major brand + it("classifies M4A ftyp major brand as audio/mp4", async () => { + const m4aHeader = Buffer.from([ + 0x00, 0x00, 0x00, 0x1c, 0x66, 0x74, 0x79, 0x70, 0x4d, 0x34, 0x41, 0x20, ]); - getMessageContentMock.mockResolvedValueOnce(chunks([m4a])); - vi.spyOn(fs.promises, "writeFile").mockResolvedValueOnce(undefined); + getMessageContentMock.mockResolvedValueOnce(chunks([m4aHeader])); + const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValueOnce(undefined); - const result = await downloadLineMedia("mid-m4a", "token"); + const result = await downloadLineMedia("mid-audio", "token"); + const writtenPath = writeSpy.mock.calls[0]?.[0]; expect(result.contentType).toBe("audio/mp4"); expect(result.path).toMatch(/\.m4a$/); + expect(writtenPath).toBe(result.path); }); it("detects MP4 video from ftyp major brand (isom)", async () => { - // MP4 video magic bytes: size(4) + "ftyp" + "isom" const mp4 = Buffer.from([ - 0x00, - 0x00, - 0x00, - 0x1c, - 0x66, - 0x74, - 0x79, - 0x70, - 0x69, - 0x73, - 0x6f, - 0x6d, // "isom" major brand + 0x00, 0x00, 0x00, 0x1c, 0x66, 0x74, 0x79, 0x70, 0x69, 0x73, 0x6f, 0x6d, ]); getMessageContentMock.mockResolvedValueOnce(chunks([mp4])); vi.spyOn(fs.promises, "writeFile").mockResolvedValueOnce(undefined); diff --git a/src/line/download.ts b/src/line/download.ts index 29f01258794..8ec7ad45c32 100644 --- a/src/line/download.ts +++ b/src/line/download.ts @@ -9,6 +9,8 @@ interface DownloadResult { size: number; } +const AUDIO_BRANDS = new Set(["m4a ", "m4b ", "m4p ", "m4r ", "f4a ", "f4b "]); + export async function downloadLineMedia( messageId: string, channelAccessToken: string, @@ -53,6 +55,13 @@ export async function downloadLineMedia( } function detectContentType(buffer: Buffer): string { + const hasFtypBox = + buffer.length >= 12 && + buffer[4] === 0x66 && + buffer[5] === 0x74 && + buffer[6] === 0x79 && + buffer[7] === 0x70; + // Check magic bytes if (buffer.length >= 2) { // JPEG @@ -80,17 +89,11 @@ function detectContentType(buffer: Buffer): string { ) { return "image/webp"; } - // MPEG-4 container (ftyp box) — distinguish audio (M4A) from video (MP4) - // by checking the major brand at bytes 8-11. - if ( - buffer.length >= 12 && - buffer[4] === 0x66 && - buffer[5] === 0x74 && - buffer[6] === 0x79 && - buffer[7] === 0x70 - ) { - const brand = String.fromCharCode(buffer[8], buffer[9], buffer[10], buffer[11]); - if (brand === "M4A " || brand === "M4B ") { + if (hasFtypBox) { + // ISO BMFF containers share `ftyp`; use major brand to separate common + // M4A audio payloads from video mp4 containers. + const majorBrand = buffer.toString("ascii", 8, 12).toLowerCase(); + if (AUDIO_BRANDS.has(majorBrand)) { return "audio/mp4"; } return "video/mp4"; diff --git a/src/line/webhook-node.test.ts b/src/line/webhook-node.test.ts index 07035c64521..105a20060d8 100644 --- a/src/line/webhook-node.test.ts +++ b/src/line/webhook-node.test.ts @@ -69,6 +69,23 @@ describe("createLineNodeWebhookHandler", () => { expect(res.body).toBe("OK"); }); + it("returns 204 for HEAD", async () => { + const bot = { handleWebhook: vi.fn(async () => {}) }; + const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; + const handler = createLineNodeWebhookHandler({ + channelSecret: "secret", + bot, + runtime, + readBody: async () => "", + }); + + const { res } = createRes(); + await handler({ method: "HEAD", headers: {} } as unknown as IncomingMessage, res); + + expect(res.statusCode).toBe(204); + expect(res.body).toBeUndefined(); + }); + it("returns 200 for verification request (empty events, no signature)", async () => { const rawBody = JSON.stringify({ events: [] }); const { bot, handler } = createPostWebhookTestHarness(rawBody); @@ -82,14 +99,14 @@ describe("createLineNodeWebhookHandler", () => { expect(bot.handleWebhook).not.toHaveBeenCalled(); }); - it("returns 405 for non-GET/non-POST methods", async () => { + it("returns 405 for non-GET/HEAD/POST methods", async () => { const { bot, handler } = createPostWebhookTestHarness(JSON.stringify({ events: [] })); const { res, headers } = createRes(); await handler({ method: "PUT", headers: {} } as unknown as IncomingMessage, res); expect(res.statusCode).toBe(405); - expect(headers.allow).toBe("GET, POST"); + expect(headers.allow).toBe("GET, HEAD, POST"); expect(bot.handleWebhook).not.toHaveBeenCalled(); }); @@ -178,6 +195,32 @@ describe("createLineNodeWebhookHandler", () => { ); }); + it("returns 200 immediately and logs when event processing fails", async () => { + const rawBody = JSON.stringify({ events: [{ type: "message" }] }); + const { secret } = createPostWebhookTestHarness(rawBody); + const failingBot = { + handleWebhook: vi.fn(async () => { + throw new Error("transient failure"); + }), + }; + const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; + const failingHandler = createLineNodeWebhookHandler({ + channelSecret: secret, + bot: failingBot, + runtime, + readBody: async () => rawBody, + }); + + const { res } = createRes(); + await runSignedPost({ handler: failingHandler, rawBody, secret, res }); + await Promise.resolve(); + + expect(res.statusCode).toBe(200); + expect(res.body).toBe(JSON.stringify({ status: "ok" })); + expect(failingBot.handleWebhook).toHaveBeenCalledTimes(1); + expect(runtime.error).toHaveBeenCalledTimes(1); + }); + it("returns 400 for invalid JSON payload even when signature is valid", async () => { const rawBody = "not json"; const { bot, handler, secret } = createPostWebhookTestHarness(rawBody); diff --git a/src/line/webhook-node.ts b/src/line/webhook-node.ts index 81e2a082210..de6c08a1cd6 100644 --- a/src/line/webhook-node.ts +++ b/src/line/webhook-node.ts @@ -39,8 +39,13 @@ export function createLineNodeWebhookHandler(params: { const readBody = params.readBody ?? readLineWebhookRequestBody; return async (req: IncomingMessage, res: ServerResponse) => { - // Handle GET requests for webhook verification - if (req.method === "GET") { + // Some webhook validators and health probes use GET/HEAD. + if (req.method === "GET" || req.method === "HEAD") { + if (req.method === "HEAD") { + res.statusCode = 204; + res.end(); + return; + } res.statusCode = 200; res.setHeader("Content-Type", "text/plain"); res.end("OK"); @@ -50,7 +55,7 @@ export function createLineNodeWebhookHandler(params: { // Only accept POST requests if (req.method !== "POST") { res.statusCode = 405; - res.setHeader("Allow", "GET, POST"); + res.setHeader("Allow", "GET, HEAD, POST"); res.setHeader("Content-Type", "application/json"); res.end(JSON.stringify({ error: "Method Not Allowed" })); return; @@ -106,15 +111,13 @@ export function createLineNodeWebhookHandler(params: { return; } - // Respond immediately with 200 to avoid LINE timeout res.statusCode = 200; res.setHeader("Content-Type", "application/json"); res.end(JSON.stringify({ status: "ok" })); - // Process events asynchronously if (body.events && body.events.length > 0) { logVerbose(`line: received ${body.events.length} webhook events`); - await params.bot.handleWebhook(body).catch((err) => { + void params.bot.handleWebhook(body).catch((err) => { params.runtime.error?.(danger(`line webhook handler failed: ${String(err)}`)); }); } diff --git a/src/line/webhook.test.ts b/src/line/webhook.test.ts index 513a0874e4c..6943d2aa995 100644 --- a/src/line/webhook.test.ts +++ b/src/line/webhook.test.ts @@ -111,4 +111,18 @@ describe("createLineWebhookMiddleware", () => { }); expect(onEvents).not.toHaveBeenCalled(); }); + + it("returns 200 immediately when onEvents fails", async () => { + const { res, onEvents } = await invokeWebhook({ + body: JSON.stringify({ events: [{ type: "message" }] }), + onEvents: vi.fn(async () => { + throw new Error("transient failure"); + }), + }); + await Promise.resolve(); + + expect(onEvents).toHaveBeenCalledTimes(1); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ status: "ok" }); + }); }); diff --git a/src/line/webhook.ts b/src/line/webhook.ts index 173d247243a..f2452decc47 100644 --- a/src/line/webhook.ts +++ b/src/line/webhook.ts @@ -71,13 +71,11 @@ export function createLineWebhookMiddleware( return; } - // Respond immediately to avoid timeout res.status(200).json({ status: "ok" }); - // Process events asynchronously if (body.events && body.events.length > 0) { logVerbose(`line: received ${body.events.length} webhook events`); - await onEvents(body).catch((err) => { + void onEvents(body).catch((err) => { runtime?.error?.(danger(`line webhook handler failed: ${String(err)}`)); }); }