From 90979d7c3ef7ec30b9f8aa6963a5e38d2f17d166 Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Fri, 17 Apr 2026 12:29:04 -0600 Subject: [PATCH] fix(feishu): resolve card-action chat type before dispatch (#68201) * fix(feishu): resolve card-action chat type before dispatch * changelog: resolve card-action chat type before dispatch (#68201) * address review: prefer chat_mode over chat_type, add error-path tests - Swap resolution order to check chat_mode (conversation type) before chat_type (privacy classification), since Feishu's chat_type can return "private" for private group chats which would be wrongly classified as p2p. - Treat "topic" as group semantics in the normalizer. - Add comment explaining the field semantics and why "private" maps to "p2p" (safe-failure direction). - Add two error-path tests: API returns non-zero code, and API throws. * map chat_type=public to group in normalizer Feishu's chat_type can return "public" for public group chats. Without this mapping the fallback resolver would miss it and default to p2p, routing a group card action through DM handling. * address Aisle: cache chat-type lookups and scrub log output - Add a 30-minute TTL cache for chatId -> chatType so repeated card actions on the same chat skip the Feishu API call. - Strip chatId, event.token, and raw error strings from log messages; use err.message instead of String(err) to avoid leaking stack traces or HTTP internals from the Feishu SDK. * prune expired chat-type cache entries Add pruneChatTypeCache() called on each lookup so expired entries are evicted and the cache stays bounded in long-running processes. * address Aisle: scope cache by account, cap size, sanitize logs - Key cache by accountId:chatId to prevent cross-account contamination. - Cap cache at 5000 entries and evict oldest when exceeded. - Sanitize response.msg and err.message with CR/LF stripping and length cap before logging to prevent log injection. --- CHANGELOG.md | 1 + extensions/feishu/src/bot.card-action.test.ts | 148 ++++++++++++++++++ extensions/feishu/src/card-action.ts | 122 ++++++++++++++- 3 files changed, 266 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 684d19f11c3..be6957c430f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai - OpenAI Codex/OAuth: treat the OpenAI TLS prerequisites probe as advisory instead of a hard blocker, so Codex sign-in can still proceed when the speculative Node/OpenSSL precheck fails but the real OAuth flow still works. Thanks @vincentkoc. - Models status/OAuth health: align OAuth health reporting with the same effective credential view runtime uses, so expired refreshable sessions stop showing healthy by default and fresher imported Codex CLI credentials surface correctly in `models status`, doctor, and gateway auth status. Thanks @vincentkoc. - Twitch/setup: load Twitch through the bundled setup-entry discovery path and keep setup/status account detection aligned with runtime config. (#68008) Thanks @gumadeiras. +- Feishu/card actions: resolve card-action chat type from the Feishu chat API when stored context is missing, preferring `chat_mode` over `chat_type`, so DM-originated card actions no longer bypass `dmPolicy` by falling through to the group handling path. (#68201) ## 2026.4.15 diff --git a/extensions/feishu/src/bot.card-action.test.ts b/extensions/feishu/src/bot.card-action.test.ts index 783342ff979..83bd583fb8f 100644 --- a/extensions/feishu/src/bot.card-action.test.ts +++ b/extensions/feishu/src/bot.card-action.test.ts @@ -25,9 +25,14 @@ vi.mock("./bot.js", () => ({ handleFeishuMessage: vi.fn(), })); +const createFeishuClientMock = vi.hoisted(() => vi.fn()); const sendCardFeishuMock = vi.hoisted(() => vi.fn()); const sendMessageFeishuMock = vi.hoisted(() => vi.fn()); +vi.mock("./client.js", () => ({ + createFeishuClient: createFeishuClientMock, +})); + vi.mock("./send.js", () => ({ sendCardFeishu: sendCardFeishuMock, sendMessageFeishu: sendMessageFeishuMock, @@ -89,6 +94,13 @@ describe("Feishu Card Action Handler", () => { beforeEach(() => { vi.clearAllMocks(); + createFeishuClientMock.mockReset().mockReturnValue({ + im: { + chat: { + get: vi.fn().mockResolvedValue({ code: 0, data: { chat_type: "group" } }), + }, + }, + }); vi.mocked(handleFeishuMessage) .mockReset() .mockResolvedValue(undefined as never); @@ -354,6 +366,142 @@ describe("Feishu Card Action Handler", () => { ); }); + it("resolves DM chat type from the Feishu chat API when card context omits it", async () => { + createFeishuClientMock.mockReturnValueOnce({ + im: { + chat: { + get: vi.fn().mockResolvedValue({ code: 0, data: { chat_type: "p2p" } }), + }, + }, + }); + const event = createCardActionEvent({ + token: "tok9b", + chatId: "oc_dm_chat_123", + actionValue: { text: "/help" }, + }); + + await handleFeishuCardAction({ cfg, event, runtime }); + + expect(handleFeishuMessage).toHaveBeenCalledWith( + expect.objectContaining({ + event: expect.objectContaining({ + message: expect.objectContaining({ + chat_id: "oc_dm_chat_123", + chat_type: "p2p", + }), + }), + }), + ); + expect(createFeishuClientMock).toHaveBeenCalledTimes(1); + }); + + it("uses resolved DM chat type when building approval cards without stored context", async () => { + createFeishuClientMock.mockReturnValueOnce({ + im: { + chat: { + get: vi.fn().mockResolvedValue({ code: 0, data: { chat_mode: "p2p" } }), + }, + }, + }); + const event = createCardActionEvent({ + token: "tok9c", + chatId: "oc_dm_chat_234", + actionValue: createFeishuCardInteractionEnvelope({ + k: "meta", + a: FEISHU_APPROVAL_REQUEST_ACTION, + m: { + command: "/new", + prompt: "Start a fresh session?", + }, + c: { + u: "u123", + h: "oc_dm_chat_234", + e: Date.now() + 60_000, + }, + }), + }); + + await handleFeishuCardAction({ cfg, event, runtime, accountId: "main" }); + + expect(sendCardFeishuMock).toHaveBeenCalledWith( + expect.objectContaining({ + card: expect.objectContaining({ + body: expect.objectContaining({ + elements: expect.arrayContaining([ + expect.objectContaining({ + tag: "action", + actions: expect.arrayContaining([ + expect.objectContaining({ + value: expect.objectContaining({ + c: expect.objectContaining({ + t: "p2p", + }), + }), + }), + ]), + }), + ]), + }), + }), + }), + ); + expect(createFeishuClientMock).toHaveBeenCalledTimes(1); + }); + + it("falls back to p2p when Feishu chat API returns an error", async () => { + createFeishuClientMock.mockReturnValueOnce({ + im: { + chat: { + get: vi.fn().mockResolvedValue({ code: 99, msg: "not found" }), + }, + }, + }); + const event = createCardActionEvent({ + token: "tok9d", + chatId: "oc_unknown_chat_456", + actionValue: { text: "/help" }, + }); + + await handleFeishuCardAction({ cfg, event, runtime }); + + expect(handleFeishuMessage).toHaveBeenCalledWith( + expect.objectContaining({ + event: expect.objectContaining({ + message: expect.objectContaining({ + chat_type: "p2p", + }), + }), + }), + ); + }); + + it("falls back to p2p when Feishu chat API throws", async () => { + createFeishuClientMock.mockReturnValueOnce({ + im: { + chat: { + get: vi.fn().mockRejectedValue(new Error("network failure")), + }, + }, + }); + const event = createCardActionEvent({ + token: "tok9e", + chatId: "oc_broken_chat_789", + actionValue: { text: "/help" }, + }); + + await handleFeishuCardAction({ cfg, event, runtime }); + + expect(handleFeishuMessage).toHaveBeenCalledWith( + expect.objectContaining({ + event: expect.objectContaining({ + message: expect.objectContaining({ + chat_type: "p2p", + }), + }), + }), + ); + }); + it("drops duplicate structured callback tokens", async () => { const event = createStructuredQuickActionEvent({ token: "tok10", diff --git a/extensions/feishu/src/card-action.ts b/extensions/feishu/src/card-action.ts index dc88d51cadb..0c53e105f40 100644 --- a/extensions/feishu/src/card-action.ts +++ b/extensions/feishu/src/card-action.ts @@ -2,6 +2,7 @@ import type { ClawdbotConfig, RuntimeEnv } from "../runtime-api.js"; import { resolveFeishuRuntimeAccount } from "./accounts.js"; import { handleFeishuMessage, type FeishuMessageEvent } from "./bot.js"; import { decodeFeishuCardAction, buildFeishuCardActionTextFallback } from "./card-interaction.js"; +import { createFeishuClient } from "./client.js"; import { createApprovalCard, FEISHU_APPROVAL_CANCEL_ACTION, @@ -104,7 +105,7 @@ function releaseFeishuCardActionToken(params: { token: string; accountId: string function buildSyntheticMessageEvent( event: FeishuCardActionEvent, content: string, - chatType?: "p2p" | "group", + chatType: "p2p" | "group", ): FeishuMessageEvent { return { sender: { @@ -117,7 +118,7 @@ function buildSyntheticMessageEvent( message: { message_id: `card-action-${event.token}`, chat_id: event.context.chat_id || event.operator.open_id, - chat_type: chatType ?? (event.context.chat_id ? "group" : "p2p"), + chat_type: chatType, message_type: "text", content: JSON.stringify({ text: content }), }, @@ -136,20 +137,124 @@ async function dispatchSyntheticCommand(params: { cfg: ClawdbotConfig; event: FeishuCardActionEvent; command: string; + account: ReturnType; botOpenId?: string; runtime?: RuntimeEnv; accountId?: string; chatType?: "p2p" | "group"; }): Promise { + const resolvedChatType = await resolveCardActionChatType({ + event: params.event, + account: params.account, + chatType: params.chatType, + log: params.runtime?.log ?? console.log, + }); await handleFeishuMessage({ cfg: params.cfg, - event: buildSyntheticMessageEvent(params.event, params.command, params.chatType), + event: buildSyntheticMessageEvent(params.event, params.command, resolvedChatType), botOpenId: params.botOpenId, runtime: params.runtime, accountId: params.accountId, }); } +// Feishu's im.chat.get returns two fields: +// chat_mode: conversation type — "p2p" | "group" | "topic" +// chat_type: privacy classification — "private" | "public" +// We check chat_mode first because it directly indicates conversation type. +// "private" maps to "p2p" as the safe-failure direction (restrictive DM +// policy) — a private group chat misclassified as p2p is safer than the +// reverse. "topic" and "public" are treated as group semantics. +function normalizeResolvedCardActionChatType(value: unknown): "p2p" | "group" | undefined { + if (value === "group" || value === "topic" || value === "public") { + return "group"; + } + if (value === "p2p" || value === "private") { + return "p2p"; + } + return undefined; +} + +const resolvedChatTypeCache = new Map(); +const CHAT_TYPE_CACHE_TTL_MS = 30 * 60_000; +const CHAT_TYPE_CACHE_MAX_SIZE = 5_000; + +function pruneChatTypeCache(now: number): void { + for (const [key, entry] of resolvedChatTypeCache.entries()) { + if (entry.expiresAt <= now) { + resolvedChatTypeCache.delete(key); + } + } + if (resolvedChatTypeCache.size > CHAT_TYPE_CACHE_MAX_SIZE) { + const excess = resolvedChatTypeCache.size - CHAT_TYPE_CACHE_MAX_SIZE; + const iter = resolvedChatTypeCache.keys(); + for (let i = 0; i < excess; i++) { + const key = iter.next().value; + if (key !== undefined) { + resolvedChatTypeCache.delete(key); + } + } + } +} + +function sanitizeLogValue(v: string): string { + return v.replace(/[\r\n]/g, " ").slice(0, 500); +} + +async function resolveCardActionChatType(params: { + event: FeishuCardActionEvent; + account: ReturnType; + chatType?: "p2p" | "group"; + log: (message: string) => void; +}): Promise<"p2p" | "group"> { + const explicitChatType = normalizeResolvedCardActionChatType(params.chatType); + if (explicitChatType) { + return explicitChatType; + } + + const chatId = params.event.context.chat_id?.trim(); + if (!chatId) { + return "p2p"; + } + + const cacheKey = `${params.account.accountId}:${chatId}`; + const now = Date.now(); + pruneChatTypeCache(now); + const cached = resolvedChatTypeCache.get(cacheKey); + if (cached) { + return cached.value; + } + + try { + const response = (await createFeishuClient(params.account).im.chat.get({ + path: { chat_id: chatId }, + })) as { code?: number; msg?: string; data?: { chat_type?: unknown; chat_mode?: unknown } }; + if (response.code === 0) { + const resolvedChatType = + normalizeResolvedCardActionChatType(response.data?.chat_mode) ?? + normalizeResolvedCardActionChatType(response.data?.chat_type); + if (resolvedChatType) { + resolvedChatTypeCache.set(cacheKey, { value: resolvedChatType, expiresAt: now + CHAT_TYPE_CACHE_TTL_MS }); + return resolvedChatType; + } + params.log( + `feishu[${params.account.accountId}]: card action missing chat type for chat; defaulting to p2p`, + ); + } else { + params.log( + `feishu[${params.account.accountId}]: failed to resolve chat type: ${sanitizeLogValue(response.msg ?? "unknown error")}; defaulting to p2p`, + ); + } + } catch (err) { + const message = err instanceof Error ? err.message : "unknown"; + params.log( + `feishu[${params.account.accountId}]: failed to resolve chat type: ${sanitizeLogValue(message)}; defaulting to p2p`, + ); + } + + return "p2p"; +} + async function sendInvalidInteractionNotice(params: { cfg: ClawdbotConfig; event: FeishuCardActionEvent; @@ -246,7 +351,12 @@ export async function handleFeishuCardAction(params: { prompt, sessionKey: envelope.c?.s, expiresAt: Date.now() + FEISHU_APPROVAL_CARD_TTL_MS, - chatType: envelope.c?.t ?? (event.context.chat_id ? "group" : "p2p"), + chatType: await resolveCardActionChatType({ + event, + account, + chatType: envelope.c?.t, + log, + }), confirmLabel: command === "/reset" ? "Reset" : "Confirm", }), accountId, @@ -282,10 +392,11 @@ export async function handleFeishuCardAction(params: { cfg, event, command, + account, botOpenId: params.botOpenId, runtime, accountId, - chatType: envelope.c?.t ?? (event.context.chat_id ? "group" : "p2p"), + chatType: envelope.c?.t, }); completeFeishuCardActionToken({ token: event.token, accountId: account.accountId }); return; @@ -311,6 +422,7 @@ export async function handleFeishuCardAction(params: { cfg, event, command: content, + account, botOpenId: params.botOpenId, runtime, accountId,