From ebf9fcc02cc214d16980bc03e4cd47cffa92e4b6 Mon Sep 17 00:00:00 2001 From: zw-xysk Date: Wed, 29 Jul 2026 19:16:24 +0800 Subject: [PATCH] fix(feishu): log message content JSON parse failures instead of silently swallowing (#107947) * fix(feishu): log message content JSON parse failures instead of silently swallowing Replace formatErrorMessage(err) with safe metadata-only logging in parseFeishuMessageContent to prevent potential message content leaks through V8 JSON.parse error messages. Changes: - Remove formatErrorMessage import (security: V8 JSON.parse errors can include input content in the message) - Log only msgType and optional messageId (safe metadata) when parse fails, never the exception message or raw content - Add assertion that raw content is NOT present in the log output - Pass messageId through to enable richer diagnostics The raw content is still preserved as the function return value (existing fallback behavior). * fix(feishu): move parse-failure test into getMessageFeishu suite The test 'logs a safe diagnostic (not raw content) when message content is not valid JSON' was declared after the closing brace of describe('getMessageFeishu'), so it did not inherit that suite's fixture setup and reset hooks (beforeEach/afterAll). Move it inside the suite so it benefits from the shared mock reset and cleanup. Fixes ClawSweeper P2: 'Keep the parse-failure test inside the fetch suite' --------- Co-authored-by: Peter Steinberger --- .../src/message-content-loopback.test.ts | 239 ++++++++++++++++++ extensions/feishu/src/send.test.ts | 45 ++++ extensions/feishu/src/send.ts | 11 +- 3 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 extensions/feishu/src/message-content-loopback.test.ts diff --git a/extensions/feishu/src/message-content-loopback.test.ts b/extensions/feishu/src/message-content-loopback.test.ts new file mode 100644 index 000000000000..c7e49bd77171 --- /dev/null +++ b/extensions/feishu/src/message-content-loopback.test.ts @@ -0,0 +1,239 @@ +// Prove Feishu message parsing against the real SDK, auth exchange, and HTTP transport. +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import * as Lark from "@larksuiteoapi/node-sdk"; +import { describe, expect, it, vi } from "vitest"; +import type { ClawdbotConfig } from "../runtime-api.js"; +import { getMessageFeishu } from "./send.js"; + +const { mockLogVerbose } = vi.hoisted(() => ({ mockLogVerbose: vi.fn() })); + +vi.mock("openclaw/plugin-sdk/runtime-env", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, logVerbose: mockLogVerbose }; +}); + +type RecordedMessageRequest = { + method: string; + path: string; + cardMessageContentType?: string; + appId?: string; + authorization?: string; +}; + +describe("Feishu message content over the real Lark SDK", () => { + it("authenticates message reads and diagnoses malformed content without exposing its bytes", async () => { + const requests: RecordedMessageRequest[] = []; + const sensitiveContentByMessageId = new Map([ + ["om_loopback_bad_text", { type: "text", content: "LKTEXT107" }], + ["om_loopback_bad_post", { type: "post", content: "LKPOST107" }], + ["om_loopback_bad_card", { type: "interactive", content: "LKCARD107" }], + ]); + + const server = createServer((request, response) => { + void (async () => { + const chunks: Buffer[] = []; + for await (const chunk of request) { + chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); + } + + const url = new URL(request.url ?? "/", "http://127.0.0.1"); + const rawBody = Buffer.concat(chunks).toString("utf8"); + const body = rawBody ? (JSON.parse(rawBody) as Record) : {}; + const record: RecordedMessageRequest = { + method: request.method ?? "", + path: url.pathname, + ...(url.searchParams.get("card_msg_content_type") + ? { cardMessageContentType: url.searchParams.get("card_msg_content_type") ?? undefined } + : {}), + ...(typeof body.app_id === "string" ? { appId: body.app_id } : {}), + ...(typeof request.headers.authorization === "string" + ? { authorization: request.headers.authorization } + : {}), + }; + requests.push(record); + + const sendJson = (status: number, payload: unknown) => { + response.writeHead(status, { "content-type": "application/json" }); + response.end(JSON.stringify(payload)); + }; + + if (url.pathname === "/open-apis/auth/v3/tenant_access_token/internal") { + if ( + request.method !== "POST" || + body.app_id !== "cli_feishu_content_107947" || + body.app_secret !== "loopback-placeholder" + ) { + sendJson(401, { code: 99991663, msg: "invalid loopback application" }); + return; + } + sendJson(200, { + code: 0, + msg: "ok", + tenant_access_token: "tat-feishu-content-107947", + expire: 7200, + }); + return; + } + + if ( + request.method !== "GET" || + record.authorization !== "Bearer tat-feishu-content-107947" || + record.cardMessageContentType !== "user_card_content" + ) { + sendJson(401, { code: 99991663, msg: "missing authenticated message request" }); + return; + } + + const messageId = url.pathname.match(/^\/open-apis\/im\/v1\/messages\/([^/]+)$/)?.[1]; + if (!messageId) { + sendJson(404, { code: 404, msg: "unexpected loopback message route" }); + return; + } + + if (messageId === "om_loopback_vendor_error") { + sendJson(200, { code: 230001, msg: "message not found" }); + return; + } + + const malformed = sensitiveContentByMessageId.get(messageId); + const message = malformed + ? { msg_type: malformed.type, body: { content: malformed.content } } + : messageId === "om_loopback_valid_text" + ? { msg_type: "text", body: { content: JSON.stringify({ text: "safe message" }) } } + : messageId === "om_loopback_empty_text" + ? { msg_type: "text", body: { content: "" } } + : undefined; + + if (!message) { + sendJson(404, { code: 404, msg: "unknown loopback message" }); + return; + } + + sendJson(200, { + code: 0, + msg: "ok", + data: { + items: [ + { + message_id: messageId, + chat_id: "oc_feishu_content_107947", + chat_type: "group", + ...message, + }, + ], + }, + }); + })().catch((error: unknown) => { + response.writeHead(500, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: String(error) })); + }); + }); + + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + + const address = server.address() as AddressInfo; + const loopbackOrigin = `http://127.0.0.1:${address.port}`; + // Preserve the real SDK domain; its path filler interprets custom :port + // values as route parameters. Redirect the original Axios transport only. + const loopbackInterceptor = Lark.defaultHttpInstance.interceptors.request.use( + (options) => { + const upstream = new URL(options.url ?? ""); + if (upstream.hostname === "open.feishu.cn") { + options.url = new URL( + `${upstream.pathname}${upstream.search}`, + loopbackOrigin, + ).toString(); + } + return options; + }, + undefined, + { synchronous: true }, + ); + + try { + mockLogVerbose.mockClear(); + const cfg = { + channels: { + feishu: { + enabled: true, + appId: "cli_feishu_content_107947", + appSecret: "loopback-placeholder", // pragma: allowlist secret + domain: "feishu", + }, + }, + } as ClawdbotConfig; + + for (const [messageId, malformed] of sensitiveContentByMessageId) { + let exposedParserError: unknown; + try { + JSON.parse(malformed.content); + } catch (error) { + exposedParserError = error; + } + expect(exposedParserError).toBeInstanceOf(SyntaxError); + expect((exposedParserError as Error).message).toContain(malformed.content); + + await expect(getMessageFeishu({ cfg, messageId })).resolves.toMatchObject({ + messageId, + chatId: "oc_feishu_content_107947", + contentType: malformed.type, + content: malformed.content, + }); + expect(mockLogVerbose).toHaveBeenCalledWith( + `feishu message content parse failed for ${malformed.type} message (id: ${messageId})`, + ); + } + + await expect( + getMessageFeishu({ cfg, messageId: "om_loopback_valid_text" }), + ).resolves.toMatchObject({ + messageId: "om_loopback_valid_text", + contentType: "text", + content: "safe message", + }); + + await expect( + getMessageFeishu({ cfg, messageId: "om_loopback_empty_text" }), + ).resolves.toMatchObject({ + messageId: "om_loopback_empty_text", + contentType: "text", + content: "", + }); + + await expect( + getMessageFeishu({ cfg, messageId: "om_loopback_vendor_error" }), + ).resolves.toBeNull(); + + const loggedValues = mockLogVerbose.mock.calls.flat().map(String).join("\n"); + for (const { content } of sensitiveContentByMessageId.values()) { + expect(loggedValues).not.toContain(content); + } + expect(loggedValues).not.toContain("loopback-placeholder"); + expect(mockLogVerbose).toHaveBeenCalledTimes(sensitiveContentByMessageId.size); + + expect(requests.filter((request) => request.path.includes("tenant_access_token"))).toEqual([ + { + method: "POST", + path: "/open-apis/auth/v3/tenant_access_token/internal", + appId: "cli_feishu_content_107947", + }, + ]); + expect(requests.filter((request) => request.method === "GET")).toHaveLength(6); + for (const request of requests.filter((entry) => entry.method === "GET")) { + expect(request).toMatchObject({ + authorization: "Bearer tat-feishu-content-107947", + cardMessageContentType: "user_card_content", + }); + } + } finally { + Lark.defaultHttpInstance.interceptors.request.eject(loopbackInterceptor); + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + }); +}); diff --git a/extensions/feishu/src/send.test.ts b/extensions/feishu/src/send.test.ts index cafba1c7a974..141fe9db1460 100644 --- a/extensions/feishu/src/send.test.ts +++ b/extensions/feishu/src/send.test.ts @@ -8,6 +8,7 @@ const { mockClientList, mockClientPatch, mockCreateFeishuClient, + mockLogVerbose, mockResolveMarkdownTableMode, mockResolveFeishuAccount, mockRuntimeConvertMarkdownTables, @@ -18,6 +19,7 @@ const { mockClientList: vi.fn(), mockClientPatch: vi.fn(), mockCreateFeishuClient: vi.fn(), + mockLogVerbose: vi.fn(), mockResolveMarkdownTableMode: vi.fn(() => "preserve"), mockResolveFeishuAccount: vi.fn(), mockRuntimeConvertMarkdownTables: vi.fn((text: string) => text), @@ -28,6 +30,14 @@ vi.mock("openclaw/plugin-sdk/markdown-table-runtime", () => ({ resolveMarkdownTableMode: mockResolveMarkdownTableMode, })); +vi.mock("openclaw/plugin-sdk/runtime-env", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + logVerbose: mockLogVerbose, + }; +}); + vi.mock("openclaw/plugin-sdk/text-chunking", async (importOriginal) => { const actual = await importOriginal(); return { @@ -79,6 +89,7 @@ describe("getMessageFeishu", () => { afterAll(() => { vi.doUnmock("openclaw/plugin-sdk/markdown-table-runtime"); + vi.doUnmock("openclaw/plugin-sdk/runtime-env"); vi.doUnmock("openclaw/plugin-sdk/text-chunking"); vi.doUnmock("./client.js"); vi.doUnmock("./accounts.js"); @@ -693,6 +704,40 @@ describe("getMessageFeishu", () => { }, ]); }); + + it("logs a safe diagnostic (not raw content) when message content is not valid JSON", async () => { + mockClientGet.mockResolvedValueOnce({ + code: 0, + data: { + message_id: "om_bad_json", + chat_id: "oc_test", + chat_type: "group", + msg_type: "text", + body: { + content: "{bad json}", + }, + sender: { + id: "ou_1", + sender_type: "user", + }, + }, + }); + + const result = await getMessageFeishu({ + cfg: {} as ClawdbotConfig, + messageId: "om_bad_json", + }); + + expect(mockLogVerbose).toHaveBeenCalledWith( + expect.stringContaining("feishu message content parse failed for text message"), + ); + expect(mockLogVerbose.mock.calls.flat().map(String).join("\n")).not.toContain("{bad json}"); + expect(result).toMatchObject({ + messageId: "om_bad_json", + contentType: "text", + content: "{bad json}", + }); + }); }); describe("editMessageFeishu", () => { diff --git a/extensions/feishu/src/send.ts b/extensions/feishu/src/send.ts index 25275665953c..c86bb3cd38aa 100644 --- a/extensions/feishu/src/send.ts +++ b/extensions/feishu/src/send.ts @@ -1,6 +1,7 @@ // Feishu plugin module implements send behavior. import { resolveMarkdownTableMode } from "openclaw/plugin-sdk/markdown-table-runtime"; import { parseStrictNonNegativeInteger } from "openclaw/plugin-sdk/number-runtime"; +import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { isRecord, normalizeLowercaseStringOrEmpty, @@ -318,7 +319,11 @@ function parseInteractiveCardContent(parsed: unknown): string { return parseInteractivePostFallback(parsed) ?? INTERACTIVE_CARD_FALLBACK_TEXT; } -function parseFeishuMessageContent(rawContent: string, msgType: string): string { +function parseFeishuMessageContent( + rawContent: string, + msgType: string, + messageId?: string, +): string { if (!rawContent) { return ""; } @@ -327,6 +332,8 @@ function parseFeishuMessageContent(rawContent: string, msgType: string): string try { parsed = JSON.parse(rawContent); } catch { + const safeId = messageId ? ` (id: ${messageId})` : ""; + logVerbose(`feishu message content parse failed for ${msgType} message${safeId}`); return rawContent; } @@ -379,7 +386,7 @@ function parseFeishuMessageItem( senderId: item.sender?.id, senderOpenId: item.sender?.id_type === "open_id" ? item.sender?.id : undefined, senderType: item.sender?.sender_type, - content: parseFeishuMessageContent(rawContent, msgType), + content: parseFeishuMessageContent(rawContent, msgType, item.message_id), contentType: msgType, createTime: parseStrictNonNegativeInteger(item.create_time), threadId: item.thread_id || undefined,