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 <steipete@gmail.com>
2026-08-02 09:51:33 +00:00 · 2026-07-29 19:16:24 +08:00
parent 987522fdcc
commit ebf9fcc02c
3 changed files with 293 additions and 2 deletions
--- a/extensions/feishu/src/message-content-loopback.test.ts
+++ 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<typeof import("openclaw/plugin-sdk/runtime-env")>();
+  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<string, unknown>) : {};
+        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<void>((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<void>((resolve, reject) => {
+        server.close((error) => (error ? reject(error) : resolve()));
+      });
+    }
+  });
+});
--- 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<typeof import("openclaw/plugin-sdk/runtime-env")>();
+  return {
+    ...actual,
+    logVerbose: mockLogVerbose,
+  };
+});
+
 vi.mock("openclaw/plugin-sdk/text-chunking", async (importOriginal) => {
  const actual = await importOriginal<typeof import("openclaw/plugin-sdk/text-chunking")>();
  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", () => {
--- 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,