mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:20:43 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<typeof resolveFeishuRuntimeAccount>;
|
||||
botOpenId?: string;
|
||||
runtime?: RuntimeEnv;
|
||||
accountId?: string;
|
||||
chatType?: "p2p" | "group";
|
||||
}): Promise<void> {
|
||||
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<string, { value: "p2p" | "group"; expiresAt: number }>();
|
||||
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<typeof resolveFeishuRuntimeAccount>;
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user