diff --git a/extensions/feishu/runtime-api.ts b/extensions/feishu/runtime-api.ts index ece8df41cca..97211ca75b2 100644 --- a/extensions/feishu/runtime-api.ts +++ b/extensions/feishu/runtime-api.ts @@ -2,3 +2,8 @@ // Keep this barrel thin and aligned with the local extension surface. export * from "openclaw/plugin-sdk/feishu"; +export { + isRequestBodyLimitError, + readRequestBodyWithLimit, + requestBodyErrorToText, +} from "openclaw/plugin-sdk/webhook-ingress"; diff --git a/extensions/feishu/src/monitor.transport.ts b/extensions/feishu/src/monitor.transport.ts index 4ff870f6acb..0ffa1b80cb7 100644 --- a/extensions/feishu/src/monitor.transport.ts +++ b/extensions/feishu/src/monitor.transport.ts @@ -3,9 +3,11 @@ import crypto from "node:crypto"; import * as Lark from "@larksuiteoapi/node-sdk"; import { applyBasicWebhookRequestGuards, - readJsonBodyWithLimit, + isRequestBodyLimitError, type RuntimeEnv, installRequestBodyLimitGuard, + readRequestBodyWithLimit, + requestBodyErrorToText, } from "../runtime-api.js"; import { createFeishuWSClient } from "./client.js"; import { @@ -48,9 +50,18 @@ function buildFeishuWebhookEnvelope( return Object.assign(Object.create({ headers: req.headers }), payload) as Record; } +function parseFeishuWebhookPayload(rawBody: string): Record | null { + try { + const parsed = JSON.parse(rawBody) as unknown; + return isFeishuWebhookPayload(parsed) ? parsed : null; + } catch { + return null; + } +} + function isFeishuWebhookSignatureValid(params: { headers: http.IncomingHttpHeaders; - payload: Record; + rawBody: string; encryptKey?: string; }): boolean { const encryptKey = params.encryptKey?.trim(); @@ -70,7 +81,7 @@ function isFeishuWebhookSignatureValid(params: { const computedSignature = crypto .createHash("sha256") - .update(timestamp + nonce + encryptKey + JSON.stringify(params.payload)) + .update(timestamp + nonce + encryptKey + params.rawBody) .digest("hex"); return timingSafeEqualString(computedSignature, signature); } @@ -185,29 +196,19 @@ export async function monitorWebhook({ void (async () => { try { - const bodyResult = await readJsonBodyWithLimit(req, { + const rawBody = await readRequestBodyWithLimit(req, { maxBytes: FEISHU_WEBHOOK_MAX_BODY_BYTES, timeoutMs: FEISHU_WEBHOOK_BODY_TIMEOUT_MS, }); if (guard.isTripped() || res.writableEnded) { return; } - if (!bodyResult.ok) { - if (bodyResult.code === "INVALID_JSON") { - respondText(res, 400, "Invalid JSON"); - } - return; - } - if (!isFeishuWebhookPayload(bodyResult.value)) { - respondText(res, 400, "Invalid JSON"); - return; - } - // Lark's default adapter drops invalid signatures as an empty 200. Reject here instead. + // Reject invalid signatures before any JSON parsing to keep the auth boundary strict. if ( !isFeishuWebhookSignatureValid({ headers: req.headers, - payload: bodyResult.value, + rawBody, encryptKey: account.encryptKey, }) ) { @@ -215,7 +216,13 @@ export async function monitorWebhook({ return; } - const { isChallenge, challenge } = Lark.generateChallenge(bodyResult.value, { + const payload = parseFeishuWebhookPayload(rawBody); + if (!payload) { + respondText(res, 400, "Invalid JSON"); + return; + } + + const { isChallenge, challenge } = Lark.generateChallenge(payload, { encryptKey: account.encryptKey ?? "", }); if (isChallenge) { @@ -225,16 +232,21 @@ export async function monitorWebhook({ return; } - const value = await eventDispatcher.invoke( - buildFeishuWebhookEnvelope(req, bodyResult.value), - { needCheck: false }, - ); + const value = await eventDispatcher.invoke(buildFeishuWebhookEnvelope(req, payload), { + needCheck: false, + }); if (!res.headersSent) { res.statusCode = 200; res.setHeader("Content-Type", "application/json; charset=utf-8"); res.end(JSON.stringify(value)); } } catch (err) { + if (isRequestBodyLimitError(err)) { + if (!res.headersSent) { + respondText(res, err.statusCode, requestBodyErrorToText(err.code)); + } + return; + } if (!guard.isTripped()) { error(`feishu[${accountId}]: webhook handler error: ${String(err)}`); if (!res.headersSent) { diff --git a/extensions/feishu/src/monitor.webhook-e2e.test.ts b/extensions/feishu/src/monitor.webhook-e2e.test.ts index 33035a735f6..57170d9d25c 100644 --- a/extensions/feishu/src/monitor.webhook-e2e.test.ts +++ b/extensions/feishu/src/monitor.webhook-e2e.test.ts @@ -23,7 +23,7 @@ import { monitorFeishuProvider, stopFeishuMonitor } from "./monitor.js"; function signFeishuPayload(params: { encryptKey: string; - payload: Record; + rawBody: string; timestamp?: string; nonce?: string; }): Record { @@ -31,7 +31,7 @@ function signFeishuPayload(params: { const nonce = params.nonce ?? "nonce-test"; const signature = crypto .createHash("sha256") - .update(timestamp + nonce + params.encryptKey + JSON.stringify(params.payload)) + .update(timestamp + nonce + params.encryptKey + params.rawBody) .digest("hex"); return { "content-type": "application/json", @@ -51,10 +51,11 @@ function encryptFeishuPayload(encryptKey: string, payload: Record) { + const rawBody = JSON.stringify(payload); return await fetch(url, { method: "POST", - headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), - body: JSON.stringify(payload), + headers: signFeishuPayload({ encryptKey: "encrypt_key", rawBody }), + body: rawBody, }); } @@ -76,12 +77,13 @@ describe("Feishu webhook signed-request e2e", () => { monitorFeishuProvider, async (url) => { const payload = { type: "url_verification", challenge: "challenge-token" }; + const rawBody = JSON.stringify(payload); const response = await fetch(url, { method: "POST", headers: { - ...signFeishuPayload({ encryptKey: "wrong_key", payload }), + ...signFeishuPayload({ encryptKey: "wrong_key", rawBody }), }, - body: JSON.stringify(payload), + body: rawBody, }); expect(response.status).toBe(401); @@ -127,7 +129,10 @@ describe("Feishu webhook signed-request e2e", () => { monitorFeishuProvider, async (url) => { const payload = { type: "url_verification", challenge: "challenge-token" }; - const headers = signFeishuPayload({ encryptKey: "encrypt_key", payload }); + const headers = signFeishuPayload({ + encryptKey: "encrypt_key", + rawBody: JSON.stringify(payload), + }); headers["x-lark-signature"] = headers["x-lark-signature"].slice(0, 12); const response = await fetch(url, { @@ -142,7 +147,7 @@ describe("Feishu webhook signed-request e2e", () => { ); }); - it("returns 400 for invalid json before invoking the sdk", async () => { + it("returns 401 for unsigned invalid json before parsing", async () => { probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); await withRunningWebhookMonitor( @@ -160,6 +165,31 @@ describe("Feishu webhook signed-request e2e", () => { body: "{not-json", }); + expect(response.status).toBe(401); + expect(await response.text()).toBe("Invalid signature"); + }, + ); + }); + + it("returns 400 for signed invalid json after signature validation", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "signed-invalid-json", + path: "/hook-e2e-signed-invalid-json", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + monitorFeishuProvider, + async (url) => { + const rawBody = "{not-json"; + const response = await fetch(url, { + method: "POST", + headers: signFeishuPayload({ encryptKey: "encrypt_key", rawBody }), + body: rawBody, + }); + expect(response.status).toBe(400); expect(await response.text()).toBe("Invalid JSON"); }, diff --git a/scripts/check-webhook-auth-body-order.mjs b/scripts/check-webhook-auth-body-order.mjs index aa771cb8e13..8a9cd507cbc 100644 --- a/scripts/check-webhook-auth-body-order.mjs +++ b/scripts/check-webhook-auth-body-order.mjs @@ -8,10 +8,15 @@ import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs" const sourceRoots = ["extensions"]; const enforcedFiles = new Set([ "extensions/bluebubbles/src/monitor.ts", + "extensions/feishu/src/monitor.transport.ts", "extensions/googlechat/src/monitor.ts", "extensions/zalo/src/monitor.webhook.ts", ]); const blockedCallees = new Set(["readJsonBodyWithLimit", "readRequestBodyWithLimit"]); +const allowedCallsites = new Set([ + // Feishu signs the exact wire body, so this handler must read raw bytes before parsing JSON. + "extensions/feishu/src/monitor.transport.ts:199", +]); function getCalleeName(expression) { const callee = unwrapExpression(expression); @@ -46,6 +51,7 @@ export async function main() { sourceRoots, findCallLines: findBlockedWebhookBodyReadLines, skipRelativePath: (relPath) => !enforcedFiles.has(relPath.replaceAll(path.sep, "/")), + allowCallsite: (callsite) => allowedCallsites.has(callsite), header: "Found forbidden low-level body reads in auth-sensitive webhook handlers:", footer: "Use plugin-sdk webhook guards (`readJsonWebhookBodyOrReject` / `readWebhookBodyOrReject`) with explicit pre-auth/post-auth profiles.",