From f45e5a6569aab1d58cc6de25b19f1dc4c8779b85 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 31 Mar 2026 19:43:54 +0900 Subject: [PATCH] fix(feishu): filter fetched group thread context (#58237) * fix(feishu): filter fetched group thread context * fix(feishu): preserve filtered thread bootstrap --- CHANGELOG.md | 1 + extensions/feishu/src/bot.test.ts | 127 ++++++++++++++++++++++++++++++ extensions/feishu/src/bot.ts | 98 ++++++++++++++++++++--- 3 files changed, 215 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20f64819977..404fb1e9b36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,7 @@ Docs: https://docs.openclaw.ai - Gateway/attachments: offload large inbound images without leaking `media://` markers into text-only runs, preserve mixed attachment order for model input/transcripts, and fail closed when model image capability cannot be resolved. (#55513) Thanks @Syysean. - Agents/subagents: fix interim subagent runtime display so `/subagents list` and `/subagents info` stop inflating short runtimes and show second-level durations correctly. (#57739) Thanks @samzong. - Diffs/config: preserve schema-shaped plugin config parsing from `diffsPluginConfigSchema.safeParse()`, so direct callers keep `defaults` and `security` sections instead of receiving flattened tool defaults. (#57904) Thanks @gumadeiras. +- Feishu/groups: keep quoted replies and topic bootstrap context aligned with group sender allowlists so only allowlisted thread messages seed agent context. Thanks @AntAISecurityLab and @vincentkoc. - Diffs: fall back to plain text when `lang` hints are invalid during diff render and viewer hydration, so bad or stale language values no longer break the diff viewer. (#57902) Thanks @gumadeiras. - Doctor/plugins: skip false Matrix legacy-helper warnings when no migration plans exist, and keep bundled `enabledByDefault` plugins in the gateway startup set. (#57931) Thanks @dinakars777. - Zalo/webhooks: scope replay dedupe to the authenticated target so one configured account can no longer cause same-id inbound events for another target to be dropped. Thanks @smaeljaish771 and @vincentkoc. diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 6f91d84d759..4e3b4e7f4d5 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -1148,6 +1148,57 @@ describe("handleFeishuMessage command authorization", () => { expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); }); + it("drops quoted group context from senders outside the group sender allowlist", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockGetMessageFeishu.mockResolvedValueOnce({ + messageId: "om_parent_blocked", + chatId: "oc-group", + senderId: "ou-blocked", + senderType: "user", + content: "blocked quoted content", + contentType: "text", + }); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groupPolicy: "open", + groupSenderAllowFrom: ["ou-allowed"], + groups: { + "oc-group": { + requireMention: false, + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { + sender_id: { + open_id: "ou-allowed", + }, + }, + message: { + message_id: "msg-group-quoted-filter", + parent_id: "om_parent_blocked", + chat_id: "oc-group", + chat_type: "group", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockFinalizeInboundContext).toHaveBeenCalledWith( + expect.objectContaining({ + ReplyToId: "om_parent_blocked", + ReplyToBody: undefined, + }), + ); + }); + it("dispatches group image message when groupPolicy is open (requireMention defaults to false)", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); @@ -2458,6 +2509,82 @@ describe("handleFeishuMessage command authorization", () => { ); }); + it("filters topic bootstrap context to allowlisted group senders", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockGetMessageFeishu.mockResolvedValue({ + messageId: "om_topic_root", + chatId: "oc-group", + senderId: "ou-blocked", + senderType: "user", + content: "blocked root starter", + contentType: "text", + threadId: "omt_topic_1", + }); + mockListFeishuThreadMessages.mockResolvedValue([ + { + messageId: "om_blocked_reply", + senderId: "ou-blocked", + senderType: "user", + content: "blocked follow-up", + contentType: "text", + createTime: 1710000000000, + }, + { + messageId: "om_bot_reply", + senderId: "app_1", + senderType: "app", + content: "assistant reply", + contentType: "text", + createTime: 1710000001000, + }, + { + messageId: "om_allowed_reply", + senderId: "ou-allowed", + senderType: "user", + content: "allowed follow-up", + contentType: "text", + createTime: 1710000002000, + }, + ]); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groupPolicy: "open", + groupSenderAllowFrom: ["ou-allowed"], + groups: { + "oc-group": { + requireMention: false, + groupSessionScope: "group_topic", + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-allowed" } }, + message: { + message_id: "om_topic_followup_allowlisted", + root_id: "om_topic_root", + thread_id: "omt_topic_1", + chat_id: "oc-group", + chat_type: "group", + message_type: "text", + content: JSON.stringify({ text: "current turn" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockFinalizeInboundContext).toHaveBeenCalledWith( + expect.objectContaining({ + ThreadStarterBody: "assistant reply", + ThreadHistoryBody: "assistant reply\n\nallowed follow-up", + }), + ); + }); + it("does not dispatch twice for the same image message_id (concurrent dedupe)", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 878cdf076ea..45cef586e18 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -45,7 +45,7 @@ import { import { createFeishuReplyDispatcher } from "./reply-dispatcher.js"; import { getFeishuRuntime } from "./runtime.js"; import { getMessageFeishu, listFeishuThreadMessages, sendMessageFeishu } from "./send.js"; -import type { FeishuMessageContext } from "./types.js"; +import type { FeishuMessageContext, FeishuMessageInfo } from "./types.js"; import type { DynamicAgentCreationConfig } from "./types.js"; export { toMessageResourceType } from "./bot-content.js"; @@ -218,6 +218,49 @@ export function buildFeishuAgentBody(params: { return messageBody; } +function shouldIncludeFetchedGroupContextMessage(params: { + isGroup: boolean; + allowFrom: Array; + senderId?: string; + senderType?: string; +}): boolean { + if (!params.isGroup || params.allowFrom.length === 0) { + return true; + } + if (params.senderType === "app") { + return true; + } + const senderId = params.senderId?.trim(); + if (!senderId) { + return false; + } + return isFeishuGroupAllowed({ + groupPolicy: "allowlist", + allowFrom: params.allowFrom, + senderId, + senderName: undefined, + }); +} + +function filterFetchedGroupContextMessages< + T extends Pick, +>( + messages: readonly T[], + params: { + isGroup: boolean; + allowFrom: Array; + }, +): T[] { + return messages.filter((message) => + shouldIncludeFetchedGroupContextMessage({ + isGroup: params.isGroup, + allowFrom: params.allowFrom, + senderId: message.senderId, + senderType: message.senderType, + }), + ); +} + export async function handleFeishuMessage(params: { cfg: ClawdbotConfig; event: FeishuMessageEvent; @@ -337,6 +380,11 @@ export async function handleFeishuMessage(params: { const groupConfig = isGroup ? resolveFeishuGroupConfig({ cfg: feishuCfg, groupId: ctx.chatId }) : undefined; + const effectiveGroupSenderAllowFrom = isGroup + ? (groupConfig?.allowFrom?.length ?? 0) > 0 + ? (groupConfig?.allowFrom ?? []) + : (feishuCfg?.groupSenderAllowFrom ?? []) + : []; const groupSession = isGroup ? resolveFeishuGroupSession({ chatId: ctx.chatId, @@ -402,14 +450,10 @@ export async function handleFeishuMessage(params: { } // Sender-level allowlist: per-group allowFrom takes precedence, then global groupSenderAllowFrom - const perGroupSenderAllowFrom = groupConfig?.allowFrom ?? []; - const globalSenderAllowFrom = feishuCfg?.groupSenderAllowFrom ?? []; - const effectiveSenderAllowFrom = - perGroupSenderAllowFrom.length > 0 ? perGroupSenderAllowFrom : globalSenderAllowFrom; - if (effectiveSenderAllowFrom.length > 0) { + if (effectiveGroupSenderAllowFrom.length > 0) { const senderAllowed = isFeishuGroupAllowed({ groupPolicy: "allowlist", - allowFrom: effectiveSenderAllowFrom, + allowFrom: effectiveGroupSenderAllowFrom, senderId: ctx.senderOpenId, senderIds: [senderUserId], senderName: ctx.senderName, @@ -691,11 +735,23 @@ export async function handleFeishuMessage(params: { messageId: ctx.parentId, accountId: account.accountId, }); - if (quotedMessageInfo) { + if ( + quotedMessageInfo && + shouldIncludeFetchedGroupContextMessage({ + isGroup, + allowFrom: effectiveGroupSenderAllowFrom, + senderId: quotedMessageInfo.senderId, + senderType: quotedMessageInfo.senderType, + }) + ) { quotedContent = quotedMessageInfo.content; log( `feishu[${account.accountId}]: fetched quoted message: ${quotedContent?.slice(0, 100)}`, ); + } else if (quotedMessageInfo) { + log( + `feishu[${account.accountId}]: skipped quoted message from sender ${quotedMessageInfo.senderId ?? "unknown"} due to group sender allowlist`, + ); } } catch (err) { log(`feishu[${account.accountId}]: failed to fetch quoted message: ${String(err)}`); @@ -767,6 +823,7 @@ export async function handleFeishuMessage(params: { } >(); let rootMessageInfo: Awaited> | undefined; + let rootMessageThreadId: string | undefined; let rootMessageFetched = false; const getRootMessageInfo = async () => { if (!ctx.rootId) { @@ -788,6 +845,21 @@ export async function handleFeishuMessage(params: { rootMessageInfo = null; } } + rootMessageThreadId = rootMessageInfo?.threadId; + if ( + rootMessageInfo && + !shouldIncludeFetchedGroupContextMessage({ + isGroup, + allowFrom: effectiveGroupSenderAllowFrom, + senderId: rootMessageInfo.senderId, + senderType: rootMessageInfo.senderType, + }) + ) { + log( + `feishu[${account.accountId}]: skipped thread starter from sender ${rootMessageInfo.senderId ?? "unknown"} due to group sender allowlist`, + ); + rootMessageInfo = null; + } } return rootMessageInfo ?? null; }; @@ -827,7 +899,7 @@ export async function handleFeishuMessage(params: { } const rootMsg = await getRootMessageInfo(); - let feishuThreadId = ctx.threadId ?? rootMsg?.threadId; + let feishuThreadId = ctx.threadId ?? rootMessageThreadId ?? rootMsg?.threadId; if (feishuThreadId) { log(`feishu[${account.accountId}]: resolved thread ID: ${feishuThreadId}`); } @@ -854,14 +926,18 @@ export async function handleFeishuMessage(params: { .map((id) => id?.trim()) .filter((id): id is string => id !== undefined && id.length > 0), ); + const allowlistedMessages = filterFetchedGroupContextMessages(threadMessages, { + isGroup, + allowFrom: effectiveGroupSenderAllowFrom, + }); const relevantMessages = (senderScoped - ? threadMessages.filter( + ? allowlistedMessages.filter( (msg) => msg.senderType === "app" || (msg.senderId !== undefined && senderIds.has(msg.senderId.trim())), ) - : threadMessages) ?? []; + : allowlistedMessages) ?? []; const threadStarterBody = rootMsg?.content ?? relevantMessages[0]?.content; const includeStarterInHistory = Boolean(rootMsg?.content || ctx.rootId);