From 496ca3a6373a3c1203b7a0b82ed8c93acfbb22e0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 03:10:12 +0000 Subject: [PATCH] fix(feishu): fail closed on webhook signature checks --- extensions/feishu/src/monitor.transport.ts | 110 ++++++- .../feishu/src/monitor.webhook-e2e.test.ts | 306 ++++++++++++++++++ src/plugin-sdk/feishu.ts | 2 +- 3 files changed, 411 insertions(+), 7 deletions(-) create mode 100644 extensions/feishu/src/monitor.webhook-e2e.test.ts diff --git a/extensions/feishu/src/monitor.transport.ts b/extensions/feishu/src/monitor.transport.ts index 49a9130bb..d619f3cdd 100644 --- a/extensions/feishu/src/monitor.transport.ts +++ b/extensions/feishu/src/monitor.transport.ts @@ -1,7 +1,9 @@ import * as http from "http"; +import crypto from "node:crypto"; import * as Lark from "@larksuiteoapi/node-sdk"; import { applyBasicWebhookRequestGuards, + readJsonBodyWithLimit, type RuntimeEnv, installRequestBodyLimitGuard, } from "openclaw/plugin-sdk/feishu"; @@ -26,6 +28,50 @@ export type MonitorTransportParams = { eventDispatcher: Lark.EventDispatcher; }; +function isFeishuWebhookPayload(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +function buildFeishuWebhookEnvelope( + req: http.IncomingMessage, + payload: Record, +): Record { + return Object.assign(Object.create({ headers: req.headers }), payload) as Record; +} + +function isFeishuWebhookSignatureValid(params: { + headers: http.IncomingHttpHeaders; + payload: Record; + encryptKey?: string; +}): boolean { + const encryptKey = params.encryptKey?.trim(); + if (!encryptKey) { + return true; + } + + const timestampHeader = params.headers["x-lark-request-timestamp"]; + const nonceHeader = params.headers["x-lark-request-nonce"]; + const signatureHeader = params.headers["x-lark-signature"]; + const timestamp = Array.isArray(timestampHeader) ? timestampHeader[0] : timestampHeader; + const nonce = Array.isArray(nonceHeader) ? nonceHeader[0] : nonceHeader; + const signature = Array.isArray(signatureHeader) ? signatureHeader[0] : signatureHeader; + if (!timestamp || !nonce || !signature) { + return false; + } + + const computedSignature = crypto + .createHash("sha256") + .update(timestamp + nonce + encryptKey + JSON.stringify(params.payload)) + .digest("hex"); + return computedSignature === signature; +} + +function respondText(res: http.ServerResponse, statusCode: number, body: string): void { + res.statusCode = statusCode; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end(body); +} + export async function monitorWebSocket({ account, accountId, @@ -88,7 +134,6 @@ export async function monitorWebhook({ log(`feishu[${accountId}]: starting Webhook server on ${host}:${port}, path ${path}...`); const server = http.createServer(); - const webhookHandler = Lark.adaptDefault(path, eventDispatcher, { autoChallenge: true }); server.on("request", (req, res) => { res.on("finish", () => { @@ -118,15 +163,68 @@ export async function monitorWebhook({ return; } - void Promise.resolve(webhookHandler(req, res)) - .catch((err) => { + void (async () => { + try { + const bodyResult = await readJsonBodyWithLimit(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. + if ( + !isFeishuWebhookSignatureValid({ + headers: req.headers, + payload: bodyResult.value, + encryptKey: account.encryptKey, + }) + ) { + respondText(res, 401, "Invalid signature"); + return; + } + + const { isChallenge, challenge } = Lark.generateChallenge(bodyResult.value, { + encryptKey: account.encryptKey ?? "", + }); + if (isChallenge) { + res.statusCode = 200; + res.setHeader("Content-Type", "application/json; charset=utf-8"); + res.end(JSON.stringify(challenge)); + return; + } + + const value = await eventDispatcher.invoke( + buildFeishuWebhookEnvelope(req, bodyResult.value), + { 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 (!guard.isTripped()) { error(`feishu[${accountId}]: webhook handler error: ${String(err)}`); + if (!res.headersSent) { + respondText(res, 500, "Internal Server Error"); + } } - }) - .finally(() => { + } finally { guard.dispose(); - }); + } + })(); }); httpServers.set(accountId, server); diff --git a/extensions/feishu/src/monitor.webhook-e2e.test.ts b/extensions/feishu/src/monitor.webhook-e2e.test.ts new file mode 100644 index 000000000..2e73f9734 --- /dev/null +++ b/extensions/feishu/src/monitor.webhook-e2e.test.ts @@ -0,0 +1,306 @@ +import crypto from "node:crypto"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import type { ClawdbotConfig } from "openclaw/plugin-sdk/feishu"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createFeishuRuntimeMockModule } from "./monitor.test-mocks.js"; + +const probeFeishuMock = vi.hoisted(() => vi.fn()); + +vi.mock("./probe.js", () => ({ + probeFeishu: probeFeishuMock, +})); + +vi.mock("./client.js", async () => { + const actual = await vi.importActual("./client.js"); + return { + ...actual, + createFeishuWSClient: vi.fn(() => ({ start: vi.fn() })), + }; +}); + +vi.mock("./runtime.js", () => createFeishuRuntimeMockModule()); + +import { monitorFeishuProvider, stopFeishuMonitor } from "./monitor.js"; + +async function getFreePort(): Promise { + const server = createServer(); + await new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve())); + const address = server.address() as AddressInfo | null; + if (!address) { + throw new Error("missing server address"); + } + await new Promise((resolve) => server.close(() => resolve())); + return address.port; +} + +async function waitUntilServerReady(url: string): Promise { + for (let i = 0; i < 50; i += 1) { + try { + const response = await fetch(url, { method: "GET" }); + if (response.status >= 200 && response.status < 500) { + return; + } + } catch { + // retry + } + await new Promise((resolve) => setTimeout(resolve, 20)); + } + throw new Error(`server did not start: ${url}`); +} + +function buildConfig(params: { + accountId: string; + path: string; + port: number; + verificationToken?: string; + encryptKey?: string; +}): ClawdbotConfig { + return { + channels: { + feishu: { + enabled: true, + accounts: { + [params.accountId]: { + enabled: true, + appId: "cli_test", + appSecret: "secret_test", // pragma: allowlist secret + connectionMode: "webhook", + webhookHost: "127.0.0.1", + webhookPort: params.port, + webhookPath: params.path, + encryptKey: params.encryptKey, + verificationToken: params.verificationToken, + }, + }, + }, + }, + } as ClawdbotConfig; +} + +function signFeishuPayload(params: { + encryptKey: string; + payload: Record; + timestamp?: string; + nonce?: string; +}): Record { + const timestamp = params.timestamp ?? "1711111111"; + const nonce = params.nonce ?? "nonce-test"; + const signature = crypto + .createHash("sha256") + .update(timestamp + nonce + params.encryptKey + JSON.stringify(params.payload)) + .digest("hex"); + return { + "content-type": "application/json", + "x-lark-request-timestamp": timestamp, + "x-lark-request-nonce": nonce, + "x-lark-signature": signature, + }; +} + +function encryptFeishuPayload(encryptKey: string, payload: Record): string { + const iv = crypto.randomBytes(16); + const key = crypto.createHash("sha256").update(encryptKey).digest(); + const cipher = crypto.createCipheriv("aes-256-cbc", key, iv); + const plaintext = Buffer.from(JSON.stringify(payload), "utf8"); + const encrypted = Buffer.concat([cipher.update(plaintext), cipher.final()]); + return Buffer.concat([iv, encrypted]).toString("base64"); +} + +async function withRunningWebhookMonitor( + params: { + accountId: string; + path: string; + verificationToken: string; + encryptKey: string; + }, + run: (url: string) => Promise, +) { + const port = await getFreePort(); + const cfg = buildConfig({ + accountId: params.accountId, + path: params.path, + port, + encryptKey: params.encryptKey, + verificationToken: params.verificationToken, + }); + + const abortController = new AbortController(); + const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; + const monitorPromise = monitorFeishuProvider({ + config: cfg, + runtime, + abortSignal: abortController.signal, + }); + + const url = `http://127.0.0.1:${port}${params.path}`; + await waitUntilServerReady(url); + + try { + await run(url); + } finally { + abortController.abort(); + await monitorPromise; + } +} + +afterEach(() => { + stopFeishuMonitor(); +}); + +describe("Feishu webhook signed-request e2e", () => { + it("rejects invalid signatures with 401 instead of empty 200", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "invalid-signature", + path: "/hook-e2e-invalid-signature", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const payload = { type: "url_verification", challenge: "challenge-token" }; + const response = await fetch(url, { + method: "POST", + headers: { + ...signFeishuPayload({ encryptKey: "wrong_key", payload }), + }, + body: JSON.stringify(payload), + }); + + expect(response.status).toBe(401); + expect(await response.text()).toBe("Invalid signature"); + }, + ); + }); + + it("rejects missing signature headers with 401", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "missing-signature", + path: "/hook-e2e-missing-signature", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const response = await fetch(url, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ type: "url_verification", challenge: "challenge-token" }), + }); + + expect(response.status).toBe(401); + expect(await response.text()).toBe("Invalid signature"); + }, + ); + }); + + it("returns 400 for invalid json before invoking the sdk", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "invalid-json", + path: "/hook-e2e-invalid-json", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const response = await fetch(url, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{not-json", + }); + + expect(response.status).toBe(400); + expect(await response.text()).toBe("Invalid JSON"); + }, + ); + }); + + it("accepts signed plaintext url_verification challenges end-to-end", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "signed-challenge", + path: "/hook-e2e-signed-challenge", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const payload = { type: "url_verification", challenge: "challenge-token" }; + const response = await fetch(url, { + method: "POST", + headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), + body: JSON.stringify(payload), + }); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual({ challenge: "challenge-token" }); + }, + ); + }); + + it("accepts signed non-challenge events and reaches the dispatcher", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "signed-dispatch", + path: "/hook-e2e-signed-dispatch", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const payload = { + schema: "2.0", + header: { event_type: "unknown.event" }, + event: {}, + }; + const response = await fetch(url, { + method: "POST", + headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), + body: JSON.stringify(payload), + }); + + expect(response.status).toBe(200); + expect(await response.text()).toContain("no unknown.event event handle"); + }, + ); + }); + + it("accepts signed encrypted url_verification challenges end-to-end", async () => { + probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" }); + + await withRunningWebhookMonitor( + { + accountId: "encrypted-challenge", + path: "/hook-e2e-encrypted-challenge", + verificationToken: "verify_token", + encryptKey: "encrypt_key", + }, + async (url) => { + const payload = { + encrypt: encryptFeishuPayload("encrypt_key", { + type: "url_verification", + challenge: "encrypted-challenge-token", + }), + }; + const response = await fetch(url, { + method: "POST", + headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), + body: JSON.stringify(payload), + }); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toEqual({ + challenge: "encrypted-challenge-token", + }); + }, + ); + }); +}); diff --git a/src/plugin-sdk/feishu.ts b/src/plugin-sdk/feishu.ts index 88703e6ad..4b8b0b9ab 100644 --- a/src/plugin-sdk/feishu.ts +++ b/src/plugin-sdk/feishu.ts @@ -51,7 +51,7 @@ export { } from "../config/types.secrets.js"; export { buildSecretInputSchema } from "./secret-input-schema.js"; export { createDedupeCache } from "../infra/dedupe.js"; -export { installRequestBodyLimitGuard } from "../infra/http-body.js"; +export { installRequestBodyLimitGuard, readJsonBodyWithLimit } from "../infra/http-body.js"; export { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; export { emptyPluginConfigSchema } from "../plugins/config-schema.js"; export type { PluginRuntime } from "../plugins/runtime/types.js";