Harden Feishu webhook replay guards (#66707)

eleqtrizit · drobison00 · web-flow · commit c8003f1b33ed · 2026-04-14T13:50:41.000-06:00
* fix(feishu): harden webhook replay guards * changelog: note Feishu webhook + card-action fail-closed hardening (#66707) * fix(feishu): move blank-token check above decodeFeishuCardAction Run the early-return guard against a missing/blank card-action token before decoding the card-action payload. Decoding is side-effect-free so this is a readability + tiny-perf nit, not a correctness change. Matches Greptile's P2 suggestion. --------- Co-authored-by: Devin Robison <drobison@nvidia.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
 - Agents/failover: classify OpenAI-compatible `finish_reason: network_error` stream failures as timeout so model fallback retries continue instead of stopping with an unknown failover reason. (#61784) thanks @lawrence3699.
 - Onboarding/channels: normalize channel setup metadata before discovery and validation so malformed or mixed-shape channel plugin metadata no longer breaks setup and onboarding channel lists. (#66706) Thanks @darkamenosa.
 - Slack/native commands: fix option menus for slash commands such as `/verbose` when Slack renders native buttons by giving each button a unique action ID while still routing them through the shared `openclaw_cmdarg*` listener. Thanks @Wangmerlyn.
+- Feishu/webhook: harden the webhook transport and card-action replay guards to fail closed on missing `encryptKey` and blank callback tokens — refuse to start the webhook transport without an `encryptKey`, reject unsigned requests when no key is present instead of accepting them, and drop blank card-action tokens before the dedupe claim and dispatcher. Defense-in-depth over the already-closed monitor-account layer. (#66707) Thanks @eleqtrizit.
 
 ## 2026.4.14
 
diff --git a/extensions/feishu/src/bot.card-action.test.ts b/extensions/feishu/src/bot.card-action.test.ts
@@ -367,6 +367,29 @@ describe("Feishu Card Action Handler", () => {
     expect(handleFeishuMessage).toHaveBeenCalledTimes(1);
   });
 
+  it("rejects empty callback tokens before dispatch", async () => {
+    const log = vi.fn();
+    const event = createStructuredQuickActionEvent({
+      token: "   ",
+      action: "feishu.quick_actions.help",
+      command: "/help",
+    });
+
+    await handleFeishuCardAction({
+      cfg,
+      event,
+      runtime: {
+        ...runtime,
+        log,
+      },
+    });
+
+    expect(handleFeishuMessage).not.toHaveBeenCalled();
+    expect(log).toHaveBeenCalledWith(
+      "feishu[mock-account]: rejected card action from u123: missing token",
+    );
+  });
+
   it("keeps a claimed token completed after a non-retryable dispatch failure", async () => {
     const event = createStructuredQuickActionEvent({
       token: "tok11",
diff --git a/extensions/feishu/src/card-action.ts b/extensions/feishu/src/card-action.ts
@@ -63,7 +63,7 @@ function beginFeishuCardActionToken(params: {
   pruneProcessedCardActionTokens(now);
   const normalizedToken = params.token.trim();
   if (!normalizedToken) {
-    return true;
+    return false;
   }
   const key = `${params.accountId}:${normalizedToken}`;
   const existing = processedCardActionTokens.get(key);
@@ -183,6 +183,12 @@ export async function handleFeishuCardAction(params: {
   const { cfg, event, runtime, accountId } = params;
   const account = resolveFeishuRuntimeAccount({ cfg, accountId });
   const log = runtime?.log ?? console.log;
+  if (!event.token.trim()) {
+    log(
+      `feishu[${account.accountId}]: rejected card action from ${event.operator.open_id}: missing token`,
+    );
+    return;
+  }
   const decoded = decodeFeishuCardAction({ event });
   const claimedToken = beginFeishuCardActionToken({
     token: event.token,
diff --git a/extensions/feishu/src/monitor.card-action.lifecycle.test.ts b/extensions/feishu/src/monitor.card-action.lifecycle.test.ts
@@ -32,7 +32,7 @@ const {
 } = getFeishuLifecycleTestMocks();
 
 let _handlers: Record<string, (data: unknown) => Promise<void>> = {};
-let lastRuntime: ReturnType<typeof createRuntimeEnv> | null = null;
+let lastRuntime: { error: ReturnType<typeof vi.fn> } | null = null;
 const originalStateDir = process.env.OPENCLAW_STATE_DIR;
 const lifecycleConfig = createFeishuLifecycleConfig({
   accountId: "acct-card",
@@ -219,4 +219,41 @@ describe("Feishu card-action lifecycle", () => {
     expect(dispatchReplyFromConfigMock).toHaveBeenCalledTimes(1);
     expectFeishuReplyDispatcherSentFinalReplyOnce({ createFeishuReplyDispatcherMock });
   });
+
+  it("drops malformed card-action events with empty tokens before handler dispatch", async () => {
+    const onCardAction = await setupLifecycleMonitor();
+
+    await onCardAction({
+      operator: {
+        open_id: "ou_user1",
+        user_id: "user_1",
+        union_id: "union_1",
+      },
+      token: "",
+      action: {
+        tag: "button",
+        value: createFeishuCardInteractionEnvelope({
+          k: "quick",
+          a: "feishu.quick_actions.help",
+          q: "/help",
+          c: {
+            u: "ou_user1",
+            h: "p2p:ou_user1",
+            t: "p2p",
+            e: Date.now() + 60_000,
+          },
+        }),
+      },
+      context: {
+        open_id: "ou_user1",
+        user_id: "user_1",
+        chat_id: "p2p:ou_user1",
+      },
+    });
+
+    expect(lastRuntime?.error).toHaveBeenCalledWith(
+      "feishu[acct-card]: ignoring malformed card action payload",
+    );
+    expect(dispatchReplyFromConfigMock).not.toHaveBeenCalled();
+  });
 });
diff --git a/extensions/feishu/src/monitor.transport.ts b/extensions/feishu/src/monitor.transport.ts
@@ -56,7 +56,7 @@ function isFeishuWebhookSignatureValid(params: {
 }): boolean {
   const encryptKey = params.encryptKey?.trim();
   if (!encryptKey) {
-    return true;
+    return false;
   }
 
   const timestampHeader = params.headers["x-lark-request-timestamp"];
@@ -149,6 +149,10 @@ export async function monitorWebhook({
 }: MonitorTransportParams): Promise<void> {
   const log = runtime?.log ?? console.log;
   const error = runtime?.error ?? console.error;
+  const encryptKey = account.encryptKey?.trim();
+  if (!encryptKey) {
+    throw new Error(`Feishu account "${accountId}" webhook mode requires encryptKey`);
+  }
 
   const port = account.config.webhookPort ?? 3000;
   const path = account.config.webhookPath ?? "/feishu/events";
@@ -208,7 +212,7 @@ export async function monitorWebhook({
           !isFeishuWebhookSignatureValid({
             headers: req.headers,
             rawBody,
-            encryptKey: account.encryptKey,
+            encryptKey,
           })
         ) {
           respondText(res, 401, "Invalid signature");
@@ -222,7 +226,7 @@ export async function monitorWebhook({
         }
 
         const { isChallenge, challenge } = Lark.generateChallenge(payload, {
-          encryptKey: account.encryptKey ?? "",
+          encryptKey,
         });
         if (isChallenge) {
           res.statusCode = 200;
diff --git a/extensions/feishu/src/monitor.webhook-security.test.ts b/extensions/feishu/src/monitor.webhook-security.test.ts
@@ -28,13 +28,16 @@ vi.mock("@larksuiteoapi/node-sdk", () => ({
   ),
 }));
 
+import type { RuntimeEnv } from "../runtime-api.js";
 import {
   clearFeishuWebhookRateLimitStateForTest,
   getFeishuWebhookRateLimitStateSizeForTest,
   isWebhookRateLimitedForTest,
   monitorFeishuProvider,
   stopFeishuMonitor,
 } from "./monitor.js";
+import { monitorWebhook } from "./monitor.transport.js";
+import type { ResolvedFeishuAccount } from "./types.js";
 
 async function waitForSlowBodyTimeoutResponse(
   url: string,
@@ -110,6 +113,33 @@ describe("Feishu webhook security hardening", () => {
     await expect(monitorFeishuProvider({ config: cfg })).rejects.toThrow(/requires encryptKey/i);
   });
 
+  it("refuses to start the webhook transport without encryptKey", async () => {
+    const account = {
+      accountId: "transport-missing-encrypt-key",
+      config: {
+        enabled: true,
+        connectionMode: "webhook",
+        webhookHost: "127.0.0.1",
+        webhookPort: await getFreePort(),
+        webhookPath: "/hook-transport-missing-encrypt",
+      },
+    } as ResolvedFeishuAccount;
+
+    await expect(
+      monitorWebhook({
+        account,
+        accountId: account.accountId,
+        runtime: {
+          log: vi.fn(),
+          error: vi.fn(),
+          exit: vi.fn(),
+        } as RuntimeEnv,
+        abortSignal: new AbortController().signal,
+        eventDispatcher: {} as never,
+      }),
+    ).rejects.toThrow(/requires encryptKey/i);
+  });
+
   it("returns 415 for POST requests without json content type", async () => {
     probeFeishuMock.mockResolvedValue({ ok: true, botOpenId: "bot_open_id" });
     await withRunningWebhookMonitor(
