From daf13dbb0616115bf0aa946f46c3ba2afb93d283 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 05:43:30 +0100 Subject: [PATCH] fix: enforce feishu dm policy + pairing flow (#14876) (thanks @coygeek) --- CHANGELOG.md | 1 + docs/channels/pairing.md | 2 +- extensions/feishu/src/bot.test.ts | 176 ++++++++++++++++++++++++++++-- extensions/feishu/src/bot.ts | 59 +++++++--- 4 files changed, 214 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3a0cf1de9..6192fa29f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -140,6 +140,7 @@ Docs: https://docs.openclaw.ai - Sessions: prune stale entries, cap session store size, rotate large stores, accept duration/size thresholds, default to warn-only maintenance, and prune cron run sessions after retention windows. (#13083) Thanks @skyfallsin, @Glucksberg, @gumadeiras. - CI: Implement pipeline and workflow order. Thanks @quotentiroler. - WhatsApp: preserve original filenames for inbound documents. (#12691) Thanks @akramcodez. +- Feishu: enforce DM `dmPolicy`/pairing gating and sender allow checks for inbound DMs. (#14876) Thanks @coygeek. - Telegram: harden quote parsing; preserve quote context; avoid QUOTE_TEXT_INVALID; avoid nested reply quote misclassification. (#12156) Thanks @rybnikov. - Telegram: recover proactive sends when stale topic thread IDs are used by retrying without `message_thread_id`. (#11620) - Discord: auto-create forum/media thread posts on send, with chunked follow-up replies and media handling for forum sends. (#12380) Thanks @magendary, @thewilloftheshadow. diff --git a/docs/channels/pairing.md b/docs/channels/pairing.md index bdd5297507..4b575eb87c 100644 --- a/docs/channels/pairing.md +++ b/docs/channels/pairing.md @@ -36,7 +36,7 @@ openclaw pairing list telegram openclaw pairing approve telegram ``` -Supported channels: `telegram`, `whatsapp`, `signal`, `imessage`, `discord`, `slack`. +Supported channels: `telegram`, `whatsapp`, `signal`, `imessage`, `discord`, `slack`, `feishu`. ### Where the state lives diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 26c52e83e8..63a2af835c 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -4,18 +4,27 @@ import type { FeishuMessageEvent } from "./bot.js"; import { handleFeishuMessage } from "./bot.js"; import { setFeishuRuntime } from "./runtime.js"; -const { mockCreateFeishuReplyDispatcher } = vi.hoisted(() => ({ - mockCreateFeishuReplyDispatcher: vi.fn(() => ({ - dispatcher: vi.fn(), - replyOptions: {}, - markDispatchIdle: vi.fn(), - })), -})); +const { mockCreateFeishuReplyDispatcher, mockSendMessageFeishu, mockGetMessageFeishu } = vi.hoisted( + () => ({ + mockCreateFeishuReplyDispatcher: vi.fn(() => ({ + dispatcher: vi.fn(), + replyOptions: {}, + markDispatchIdle: vi.fn(), + })), + mockSendMessageFeishu: vi.fn().mockResolvedValue({ messageId: "pairing-msg", chatId: "oc-dm" }), + mockGetMessageFeishu: vi.fn().mockResolvedValue(null), + }), +); vi.mock("./reply-dispatcher.js", () => ({ createFeishuReplyDispatcher: mockCreateFeishuReplyDispatcher, })); +vi.mock("./send.js", () => ({ + sendMessageFeishu: mockSendMessageFeishu, + getMessageFeishu: mockGetMessageFeishu, +})); + describe("handleFeishuMessage command authorization", () => { const mockFinalizeInboundContext = vi.fn((ctx: unknown) => ctx); const mockDispatchReplyFromConfig = vi @@ -24,6 +33,8 @@ describe("handleFeishuMessage command authorization", () => { const mockResolveCommandAuthorizedFromAuthorizers = vi.fn(() => false); const mockShouldComputeCommandAuthorized = vi.fn(() => true); const mockReadAllowFromStore = vi.fn().mockResolvedValue([]); + const mockUpsertPairingRequest = vi.fn().mockResolvedValue({ code: "ABCDEFGH", created: false }); + const mockBuildPairingReply = vi.fn(() => "Pairing response"); beforeEach(() => { vi.clearAllMocks(); @@ -52,6 +63,8 @@ describe("handleFeishuMessage command authorization", () => { }, pairing: { readAllowFromStore: mockReadAllowFromStore, + upsertPairingRequest: mockUpsertPairingRequest, + buildPairingReply: mockBuildPairingReply, }, }, } as unknown as PluginRuntime); @@ -62,7 +75,7 @@ describe("handleFeishuMessage command authorization", () => { commands: { useAccessGroups: true }, channels: { feishu: { - dmPolicy: "pairing", + dmPolicy: "open", allowFrom: ["ou-admin"], }, }, @@ -102,4 +115,151 @@ describe("handleFeishuMessage command authorization", () => { }), ); }); + + it("reads pairing allow store for non-command DMs when dmPolicy is pairing", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockReadAllowFromStore.mockResolvedValue(["ou-attacker"]); + + const cfg: ClawdbotConfig = { + commands: { useAccessGroups: true }, + channels: { + feishu: { + dmPolicy: "pairing", + allowFrom: [], + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { + sender_id: { + open_id: "ou-attacker", + }, + }, + message: { + message_id: "msg-read-store-non-command", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello there" }), + }, + }; + + await handleFeishuMessage({ + cfg, + event, + runtime: { log: vi.fn(), error: vi.fn() } as RuntimeEnv, + }); + + expect(mockReadAllowFromStore).toHaveBeenCalledWith("feishu"); + expect(mockResolveCommandAuthorizedFromAuthorizers).not.toHaveBeenCalled(); + expect(mockFinalizeInboundContext).toHaveBeenCalledTimes(1); + expect(mockDispatchReplyFromConfig).toHaveBeenCalledTimes(1); + }); + + it("creates pairing request and drops unauthorized DMs in pairing mode", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockReadAllowFromStore.mockResolvedValue([]); + mockUpsertPairingRequest.mockResolvedValue({ code: "ABCDEFGH", created: true }); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + dmPolicy: "pairing", + allowFrom: [], + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { + sender_id: { + open_id: "ou-unapproved", + }, + }, + message: { + message_id: "msg-pairing-flow", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }; + + await handleFeishuMessage({ + cfg, + event, + runtime: { log: vi.fn(), error: vi.fn() } as RuntimeEnv, + }); + + expect(mockUpsertPairingRequest).toHaveBeenCalledWith({ + channel: "feishu", + id: "ou-unapproved", + meta: { name: undefined }, + }); + expect(mockBuildPairingReply).toHaveBeenCalledWith({ + channel: "feishu", + idLine: "Your Feishu user id: ou-unapproved", + code: "ABCDEFGH", + }); + expect(mockSendMessageFeishu).toHaveBeenCalledWith( + expect.objectContaining({ + to: "user:ou-unapproved", + accountId: "default", + }), + ); + expect(mockFinalizeInboundContext).not.toHaveBeenCalled(); + expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + + it("computes group command authorization from group allowFrom", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(true); + mockResolveCommandAuthorizedFromAuthorizers.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + commands: { useAccessGroups: true }, + channels: { + feishu: { + groups: { + "oc-group": { + requireMention: false, + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { + sender_id: { + open_id: "ou-attacker", + }, + }, + message: { + message_id: "msg-group-command-auth", + chat_id: "oc-group", + chat_type: "group", + message_type: "text", + content: JSON.stringify({ text: "/status" }), + }, + }; + + await handleFeishuMessage({ + cfg, + event, + runtime: { log: vi.fn(), error: vi.fn() } as RuntimeEnv, + }); + + expect(mockResolveCommandAuthorizedFromAuthorizers).toHaveBeenCalledWith({ + useAccessGroups: true, + authorizers: [{ configured: false, allowed: false }], + }); + expect(mockFinalizeInboundContext).toHaveBeenCalledWith( + expect.objectContaining({ + ChatType: "group", + CommandAuthorized: false, + SenderId: "ou-attacker", + }), + ); + }); }); diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index eed6f5bc52..ba10c803ad 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -21,7 +21,7 @@ import { } from "./policy.js"; import { createFeishuReplyDispatcher } from "./reply-dispatcher.js"; import { getFeishuRuntime } from "./runtime.js"; -import { getMessageFeishu } from "./send.js"; +import { getMessageFeishu, sendMessageFeishu } from "./send.js"; // --- Message deduplication --- // Prevent duplicate processing when WebSocket reconnects or Feishu redelivers messages. @@ -647,16 +647,6 @@ export async function handleFeishuMessage(params: { return; } } else { - if (dmPolicy === "allowlist") { - const match = resolveFeishuAllowlistMatch({ - allowFrom: configAllowFrom, - senderId: ctx.senderOpenId, - }); - if (!match.allowed) { - log(`feishu[${account.accountId}]: sender ${ctx.senderOpenId} not in DM allowlist`); - return; - } - } } try { @@ -666,12 +656,51 @@ export async function handleFeishuMessage(params: { cfg, ); const storeAllowFrom = - !isGroup && shouldComputeCommandAuthorized + !isGroup && (dmPolicy !== "open" || shouldComputeCommandAuthorized) ? await core.channel.pairing.readAllowFromStore("feishu").catch(() => []) : []; - const commandAllowFrom = isGroup - ? (groupConfig?.allowFrom ?? []) - : [...configAllowFrom, ...storeAllowFrom]; + const effectiveDmAllowFrom = [...configAllowFrom, ...storeAllowFrom]; + const dmAllowed = resolveFeishuAllowlistMatch({ + allowFrom: effectiveDmAllowFrom, + senderId: ctx.senderOpenId, + senderName: ctx.senderName, + }).allowed; + + if (!isGroup && dmPolicy !== "open" && !dmAllowed) { + if (dmPolicy === "pairing") { + const { code, created } = await core.channel.pairing.upsertPairingRequest({ + channel: "feishu", + id: ctx.senderOpenId, + meta: { name: ctx.senderName }, + }); + if (created) { + log(`feishu[${account.accountId}]: pairing request sender=${ctx.senderOpenId}`); + try { + await sendMessageFeishu({ + cfg, + to: `user:${ctx.senderOpenId}`, + text: core.channel.pairing.buildPairingReply({ + channel: "feishu", + idLine: `Your Feishu user id: ${ctx.senderOpenId}`, + code, + }), + accountId: account.accountId, + }); + } catch (err) { + log( + `feishu[${account.accountId}]: pairing reply failed for ${ctx.senderOpenId}: ${String(err)}`, + ); + } + } + } else { + log( + `feishu[${account.accountId}]: blocked unauthorized sender ${ctx.senderOpenId} (dmPolicy=${dmPolicy})`, + ); + } + return; + } + + const commandAllowFrom = isGroup ? (groupConfig?.allowFrom ?? []) : effectiveDmAllowFrom; const senderAllowedForCommands = resolveFeishuAllowlistMatch({ allowFrom: commandAllowFrom, senderId: ctx.senderOpenId,