Skip to content

Commit 90979d7

Browse files
drobison00drobison00
authored
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.
1 parent 8eb577b commit 90979d7

3 files changed

Lines changed: 266 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai
3131
- 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.
3232
- 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.
3333
- Twitch/setup: load Twitch through the bundled setup-entry discovery path and keep setup/status account detection aligned with runtime config. (#68008) Thanks @gumadeiras.
34+
- 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)
3435

3536
## 2026.4.15
3637

‎extensions/feishu/src/bot.card-action.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@ vi.mock("./bot.js", () => ({
2525
handleFeishuMessage: vi.fn(),
2626
}));
2727

28+
const createFeishuClientMock = vi.hoisted(() => vi.fn());
2829
const sendCardFeishuMock = vi.hoisted(() => vi.fn());
2930
const sendMessageFeishuMock = vi.hoisted(() => vi.fn());
3031

32+
vi.mock("./client.js", () => ({
33+
createFeishuClient: createFeishuClientMock,
34+
}));
35+
3136
vi.mock("./send.js", () => ({
3237
sendCardFeishu: sendCardFeishuMock,
3338
sendMessageFeishu: sendMessageFeishuMock,
@@ -89,6 +94,13 @@ describe("Feishu Card Action Handler", () => {
8994

9095
beforeEach(() => {
9196
vi.clearAllMocks();
97+
createFeishuClientMock.mockReset().mockReturnValue({
98+
im: {
99+
chat: {
100+
get: vi.fn().mockResolvedValue({ code: 0, data: { chat_type: "group" } }),
101+
},
102+
},
103+
});
92104
vi.mocked(handleFeishuMessage)
93105
.mockReset()
94106
.mockResolvedValue(undefined as never);
@@ -354,6 +366,142 @@ describe("Feishu Card Action Handler", () => {
354366
);
355367
});
356368

369+
it("resolves DM chat type from the Feishu chat API when card context omits it", async () => {
370+
createFeishuClientMock.mockReturnValueOnce({
371+
im: {
372+
chat: {
373+
get: vi.fn().mockResolvedValue({ code: 0, data: { chat_type: "p2p" } }),
374+
},
375+
},
376+
});
377+
const event = createCardActionEvent({
378+
token: "tok9b",
379+
chatId: "oc_dm_chat_123",
380+
actionValue: { text: "/help" },
381+
});
382+
383+
await handleFeishuCardAction({ cfg, event, runtime });
384+
385+
expect(handleFeishuMessage).toHaveBeenCalledWith(
386+
expect.objectContaining({
387+
event: expect.objectContaining({
388+
message: expect.objectContaining({
389+
chat_id: "oc_dm_chat_123",
390+
chat_type: "p2p",
391+
}),
392+
}),
393+
}),
394+
);
395+
expect(createFeishuClientMock).toHaveBeenCalledTimes(1);
396+
});
397+
398+
it("uses resolved DM chat type when building approval cards without stored context", async () => {
399+
createFeishuClientMock.mockReturnValueOnce({
400+
im: {
401+
chat: {
402+
get: vi.fn().mockResolvedValue({ code: 0, data: { chat_mode: "p2p" } }),
403+
},
404+
},
405+
});
406+
const event = createCardActionEvent({
407+
token: "tok9c",
408+
chatId: "oc_dm_chat_234",
409+
actionValue: createFeishuCardInteractionEnvelope({
410+
k: "meta",
411+
a: FEISHU_APPROVAL_REQUEST_ACTION,
412+
m: {
413+
command: "/new",
414+
prompt: "Start a fresh session?",
415+
},
416+
c: {
417+
u: "u123",
418+
h: "oc_dm_chat_234",
419+
e: Date.now() + 60_000,
420+
},
421+
}),
422+
});
423+
424+
await handleFeishuCardAction({ cfg, event, runtime, accountId: "main" });
425+
426+
expect(sendCardFeishuMock).toHaveBeenCalledWith(
427+
expect.objectContaining({
428+
card: expect.objectContaining({
429+
body: expect.objectContaining({
430+
elements: expect.arrayContaining([
431+
expect.objectContaining({
432+
tag: "action",
433+
actions: expect.arrayContaining([
434+
expect.objectContaining({
435+
value: expect.objectContaining({
436+
c: expect.objectContaining({
437+
t: "p2p",
438+
}),
439+
}),
440+
}),
441+
]),
442+
}),
443+
]),
444+
}),
445+
}),
446+
}),
447+
);
448+
expect(createFeishuClientMock).toHaveBeenCalledTimes(1);
449+
});
450+
451+
it("falls back to p2p when Feishu chat API returns an error", async () => {
452+
createFeishuClientMock.mockReturnValueOnce({
453+
im: {
454+
chat: {
455+
get: vi.fn().mockResolvedValue({ code: 99, msg: "not found" }),
456+
},
457+
},
458+
});
459+
const event = createCardActionEvent({
460+
token: "tok9d",
461+
chatId: "oc_unknown_chat_456",
462+
actionValue: { text: "/help" },
463+
});
464+
465+
await handleFeishuCardAction({ cfg, event, runtime });
466+
467+
expect(handleFeishuMessage).toHaveBeenCalledWith(
468+
expect.objectContaining({
469+
event: expect.objectContaining({
470+
message: expect.objectContaining({
471+
chat_type: "p2p",
472+
}),
473+
}),
474+
}),
475+
);
476+
});
477+
478+
it("falls back to p2p when Feishu chat API throws", async () => {
479+
createFeishuClientMock.mockReturnValueOnce({
480+
im: {
481+
chat: {
482+
get: vi.fn().mockRejectedValue(new Error("network failure")),
483+
},
484+
},
485+
});
486+
const event = createCardActionEvent({
487+
token: "tok9e",
488+
chatId: "oc_broken_chat_789",
489+
actionValue: { text: "/help" },
490+
});
491+
492+
await handleFeishuCardAction({ cfg, event, runtime });
493+
494+
expect(handleFeishuMessage).toHaveBeenCalledWith(
495+
expect.objectContaining({
496+
event: expect.objectContaining({
497+
message: expect.objectContaining({
498+
chat_type: "p2p",
499+
}),
500+
}),
501+
}),
502+
);
503+
});
504+
357505
it("drops duplicate structured callback tokens", async () => {
358506
const event = createStructuredQuickActionEvent({
359507
token: "tok10",

‎extensions/feishu/src/card-action.ts‎

Lines changed: 117 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ClawdbotConfig, RuntimeEnv } from "../runtime-api.js";
22
import { resolveFeishuRuntimeAccount } from "./accounts.js";
33
import { handleFeishuMessage, type FeishuMessageEvent } from "./bot.js";
44
import { decodeFeishuCardAction, buildFeishuCardActionTextFallback } from "./card-interaction.js";
5+
import { createFeishuClient } from "./client.js";
56
import {
67
createApprovalCard,
78
FEISHU_APPROVAL_CANCEL_ACTION,
@@ -104,7 +105,7 @@ function releaseFeishuCardActionToken(params: { token: string; accountId: string
104105
function buildSyntheticMessageEvent(
105106
event: FeishuCardActionEvent,
106107
content: string,
107-
chatType?: "p2p" | "group",
108+
chatType: "p2p" | "group",
108109
): FeishuMessageEvent {
109110
return {
110111
sender: {
@@ -117,7 +118,7 @@ function buildSyntheticMessageEvent(
117118
message: {
118119
message_id: `card-action-${event.token}`,
119120
chat_id: event.context.chat_id || event.operator.open_id,
120-
chat_type: chatType ?? (event.context.chat_id ? "group" : "p2p"),
121+
chat_type: chatType,
121122
message_type: "text",
122123
content: JSON.stringify({ text: content }),
123124
},
@@ -136,20 +137,124 @@ async function dispatchSyntheticCommand(params: {
136137
cfg: ClawdbotConfig;
137138
event: FeishuCardActionEvent;
138139
command: string;
140+
account: ReturnType<typeof resolveFeishuRuntimeAccount>;
139141
botOpenId?: string;
140142
runtime?: RuntimeEnv;
141143
accountId?: string;
142144
chatType?: "p2p" | "group";
143145
}): Promise<void> {
146+
const resolvedChatType = await resolveCardActionChatType({
147+
event: params.event,
148+
account: params.account,
149+
chatType: params.chatType,
150+
log: params.runtime?.log ?? console.log,
151+
});
144152
await handleFeishuMessage({
145153
cfg: params.cfg,
146-
event: buildSyntheticMessageEvent(params.event, params.command, params.chatType),
154+
event: buildSyntheticMessageEvent(params.event, params.command, resolvedChatType),
147155
botOpenId: params.botOpenId,
148156
runtime: params.runtime,
149157
accountId: params.accountId,
150158
});
151159
}
152160

161+
// Feishu's im.chat.get returns two fields:
162+
// chat_mode: conversation type — "p2p" | "group" | "topic"
163+
// chat_type: privacy classification — "private" | "public"
164+
// We check chat_mode first because it directly indicates conversation type.
165+
// "private" maps to "p2p" as the safe-failure direction (restrictive DM
166+
// policy) — a private group chat misclassified as p2p is safer than the
167+
// reverse. "topic" and "public" are treated as group semantics.
168+
function normalizeResolvedCardActionChatType(value: unknown): "p2p" | "group" | undefined {
169+
if (value === "group" || value === "topic" || value === "public") {
170+
return "group";
171+
}
172+
if (value === "p2p" || value === "private") {
173+
return "p2p";
174+
}
175+
return undefined;
176+
}
177+
178+
const resolvedChatTypeCache = new Map<string, { value: "p2p" | "group"; expiresAt: number }>();
179+
const CHAT_TYPE_CACHE_TTL_MS = 30 * 60_000;
180+
const CHAT_TYPE_CACHE_MAX_SIZE = 5_000;
181+
182+
function pruneChatTypeCache(now: number): void {
183+
for (const [key, entry] of resolvedChatTypeCache.entries()) {
184+
if (entry.expiresAt <= now) {
185+
resolvedChatTypeCache.delete(key);
186+
}
187+
}
188+
if (resolvedChatTypeCache.size > CHAT_TYPE_CACHE_MAX_SIZE) {
189+
const excess = resolvedChatTypeCache.size - CHAT_TYPE_CACHE_MAX_SIZE;
190+
const iter = resolvedChatTypeCache.keys();
191+
for (let i = 0; i < excess; i++) {
192+
const key = iter.next().value;
193+
if (key !== undefined) {
194+
resolvedChatTypeCache.delete(key);
195+
}
196+
}
197+
}
198+
}
199+
200+
function sanitizeLogValue(v: string): string {
201+
return v.replace(/[\r\n]/g, " ").slice(0, 500);
202+
}
203+
204+
async function resolveCardActionChatType(params: {
205+
event: FeishuCardActionEvent;
206+
account: ReturnType<typeof resolveFeishuRuntimeAccount>;
207+
chatType?: "p2p" | "group";
208+
log: (message: string) => void;
209+
}): Promise<"p2p" | "group"> {
210+
const explicitChatType = normalizeResolvedCardActionChatType(params.chatType);
211+
if (explicitChatType) {
212+
return explicitChatType;
213+
}
214+
215+
const chatId = params.event.context.chat_id?.trim();
216+
if (!chatId) {
217+
return "p2p";
218+
}
219+
220+
const cacheKey = `${params.account.accountId}:${chatId}`;
221+
const now = Date.now();
222+
pruneChatTypeCache(now);
223+
const cached = resolvedChatTypeCache.get(cacheKey);
224+
if (cached) {
225+
return cached.value;
226+
}
227+
228+
try {
229+
const response = (await createFeishuClient(params.account).im.chat.get({
230+
path: { chat_id: chatId },
231+
})) as { code?: number; msg?: string; data?: { chat_type?: unknown; chat_mode?: unknown } };
232+
if (response.code === 0) {
233+
const resolvedChatType =
234+
normalizeResolvedCardActionChatType(response.data?.chat_mode) ??
235+
normalizeResolvedCardActionChatType(response.data?.chat_type);
236+
if (resolvedChatType) {
237+
resolvedChatTypeCache.set(cacheKey, { value: resolvedChatType, expiresAt: now + CHAT_TYPE_CACHE_TTL_MS });
238+
return resolvedChatType;
239+
}
240+
params.log(
241+
`feishu[${params.account.accountId}]: card action missing chat type for chat; defaulting to p2p`,
242+
);
243+
} else {
244+
params.log(
245+
`feishu[${params.account.accountId}]: failed to resolve chat type: ${sanitizeLogValue(response.msg ?? "unknown error")}; defaulting to p2p`,
246+
);
247+
}
248+
} catch (err) {
249+
const message = err instanceof Error ? err.message : "unknown";
250+
params.log(
251+
`feishu[${params.account.accountId}]: failed to resolve chat type: ${sanitizeLogValue(message)}; defaulting to p2p`,
252+
);
253+
}
254+
255+
return "p2p";
256+
}
257+
153258
async function sendInvalidInteractionNotice(params: {
154259
cfg: ClawdbotConfig;
155260
event: FeishuCardActionEvent;
@@ -246,7 +351,12 @@ export async function handleFeishuCardAction(params: {
246351
prompt,
247352
sessionKey: envelope.c?.s,
248353
expiresAt: Date.now() + FEISHU_APPROVAL_CARD_TTL_MS,
249-
chatType: envelope.c?.t ?? (event.context.chat_id ? "group" : "p2p"),
354+
chatType: await resolveCardActionChatType({
355+
event,
356+
account,
357+
chatType: envelope.c?.t,
358+
log,
359+
}),
250360
confirmLabel: command === "/reset" ? "Reset" : "Confirm",
251361
}),
252362
accountId,
@@ -282,10 +392,11 @@ export async function handleFeishuCardAction(params: {
282392
cfg,
283393
event,
284394
command,
395+
account,
285396
botOpenId: params.botOpenId,
286397
runtime,
287398
accountId,
288-
chatType: envelope.c?.t ?? (event.context.chat_id ? "group" : "p2p"),
399+
chatType: envelope.c?.t,
289400
});
290401
completeFeishuCardActionToken({ token: event.token, accountId: account.accountId });
291402
return;
@@ -311,6 +422,7 @@ export async function handleFeishuCardAction(params: {
311422
cfg,
312423
event,
313424
command: content,
425+
account,
314426
botOpenId: params.botOpenId,
315427
runtime,
316428
accountId,

0 commit comments

Comments
 (0)