refactor(security): unify webhook guardrails across channels

2026-04-27 00:17:29 +00:00 · 2026-03-02 00:31:31 +00:00
parent 58659b931b
commit 3a68c56264
7 changed files with 450 additions and 108 deletions
--- a/extensions/feishu/src/monitor.ts
+++ b/extensions/feishu/src/monitor.ts
@@ -2,12 +2,15 @@ import * as crypto from "crypto";
 import * as http from "http";
 import * as Lark from "@larksuiteoapi/node-sdk";
 import {
+  applyBasicWebhookRequestGuards,
  type ClawdbotConfig,
-  createBoundedCounter,
  createFixedWindowRateLimiter,
+  createWebhookAnomalyTracker,
  type RuntimeEnv,
  type HistoryEntry,
  installRequestBodyLimitGuard,
+  WEBHOOK_ANOMALY_COUNTER_DEFAULTS,
+  WEBHOOK_RATE_LIMIT_DEFAULTS,
 } from "openclaw/plugin-sdk";
 import { resolveFeishuAccount, listEnabledFeishuAccounts } from "./accounts.js";
 import { handleFeishuMessage, type FeishuMessageEvent, type FeishuBotAddedEvent } from "./bot.js";
@@ -30,12 +33,6 @@ const httpServers = new Map<string, http.Server>();
 const botOpenIds = new Map<string, string>();
 const FEISHU_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024;
 const FEISHU_WEBHOOK_BODY_TIMEOUT_MS = 30_000;
-const FEISHU_WEBHOOK_RATE_LIMIT_WINDOW_MS = 60_000;
-const FEISHU_WEBHOOK_RATE_LIMIT_MAX_REQUESTS = 120;
-const FEISHU_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS = 4_096;
-const FEISHU_WEBHOOK_COUNTER_LOG_EVERY = 25;
-const FEISHU_WEBHOOK_COUNTER_MAX_TRACKED_KEYS = 4_096;
-const FEISHU_WEBHOOK_COUNTER_TTL_MS = 6 * 60 * 60_000;
 const FEISHU_REACTION_VERIFY_TIMEOUT_MS = 1_500;

 export type FeishuReactionCreatedEvent = {
@@ -60,27 +57,19 @@ type ResolveReactionSyntheticEventParams = {
 };

 const feishuWebhookRateLimiter = createFixedWindowRateLimiter({
-  windowMs: FEISHU_WEBHOOK_RATE_LIMIT_WINDOW_MS,
-  maxRequests: FEISHU_WEBHOOK_RATE_LIMIT_MAX_REQUESTS,
-  maxTrackedKeys: FEISHU_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS,
+  windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
+  maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
+  maxTrackedKeys: WEBHOOK_RATE_LIMIT_DEFAULTS.maxTrackedKeys,
 });
-const feishuWebhookStatusCounters = createBoundedCounter({
-  maxTrackedKeys: FEISHU_WEBHOOK_COUNTER_MAX_TRACKED_KEYS,
-  ttlMs: FEISHU_WEBHOOK_COUNTER_TTL_MS,
+const feishuWebhookAnomalyTracker = createWebhookAnomalyTracker({
+  maxTrackedKeys: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.maxTrackedKeys,
+  ttlMs: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.ttlMs,
+  logEvery: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.logEvery,
 });

-function isJsonContentType(value: string | string[] | undefined): boolean {
-  const first = Array.isArray(value) ? value[0] : value;
-  if (!first) {
-    return false;
-  }
-  const mediaType = first.split(";", 1)[0]?.trim().toLowerCase();
-  return mediaType === "application/json" || Boolean(mediaType?.endsWith("+json"));
-}
-
 export function clearFeishuWebhookRateLimitStateForTest(): void {
  feishuWebhookRateLimiter.clear();
-  feishuWebhookStatusCounters.clear();
+  feishuWebhookAnomalyTracker.clear();
 }

 export function getFeishuWebhookRateLimitStateSizeForTest(): number {
@@ -91,25 +80,19 @@ export function isWebhookRateLimitedForTest(key: string, nowMs: number): boolean
  return feishuWebhookRateLimiter.isRateLimited(key, nowMs);
 }

-function isWebhookRateLimited(key: string, nowMs: number): boolean {
-  return isWebhookRateLimitedForTest(key, nowMs);
-}
-
 function recordWebhookStatus(
  runtime: RuntimeEnv | undefined,
  accountId: string,
  path: string,
  statusCode: number,
 ): void {
-  if (![400, 401, 408, 413, 415, 429].includes(statusCode)) {
-    return;
-  }
-  const key = `${accountId}:${path}:${statusCode}`;
-  const next = feishuWebhookStatusCounters.increment(key);
-  if (next === 1 || next % FEISHU_WEBHOOK_COUNTER_LOG_EVERY === 0) {
-    const log = runtime?.log ?? console.log;
-    log(`feishu[${accountId}]: webhook anomaly path=${path} status=${statusCode} count=${next}`);
-  }
+  feishuWebhookAnomalyTracker.record({
+    key: `${accountId}:${path}:${statusCode}`,
+    statusCode,
+    log: runtime?.log ?? console.log,
+    message: (count) =>
+      `feishu[${accountId}]: webhook anomaly path=${path} status=${statusCode} count=${count}`,
+  });
 }

 async function withTimeout<T>(promise: Promise<T>, timeoutMs: number): Promise<T | null> {
@@ -462,15 +445,16 @@ async function monitorWebhook({
    });

    const rateLimitKey = `${accountId}:${path}:${req.socket.remoteAddress ?? "unknown"}`;
-    if (isWebhookRateLimited(rateLimitKey, Date.now())) {
-      res.statusCode = 429;
-      res.end("Too Many Requests");
-      return;
-    }
-
-    if (req.method === "POST" && !isJsonContentType(req.headers["content-type"])) {
-      res.statusCode = 415;
-      res.end("Unsupported Media Type");
+    if (
+      !applyBasicWebhookRequestGuards({
+        req,
+        res,
+        rateLimiter: feishuWebhookRateLimiter,
+        rateLimitKey,
+        nowMs: Date.now(),
+        requireJsonContentType: true,
+      })
+    ) {
      return;
    }

--- a/extensions/zalo/src/monitor.webhook.ts
+++ b/extensions/zalo/src/monitor.webhook.ts
@@ -2,27 +2,22 @@ import { timingSafeEqual } from "node:crypto";
 import type { IncomingMessage, ServerResponse } from "node:http";
 import type { OpenClawConfig } from "openclaw/plugin-sdk";
 import {
-  createBoundedCounter,
  createDedupeCache,
  createFixedWindowRateLimiter,
-  readJsonBodyWithLimit,
+  createWebhookAnomalyTracker,
+  readJsonWebhookBodyOrReject,
+  applyBasicWebhookRequestGuards,
  registerWebhookTarget,
-  rejectNonPostWebhookRequest,
-  requestBodyErrorToText,
  resolveSingleWebhookTarget,
  resolveWebhookTargets,
+  WEBHOOK_ANOMALY_COUNTER_DEFAULTS,
+  WEBHOOK_RATE_LIMIT_DEFAULTS,
 } from "openclaw/plugin-sdk";
 import type { ResolvedZaloAccount } from "./accounts.js";
 import type { ZaloFetch, ZaloUpdate } from "./api.js";
 import type { ZaloRuntimeEnv } from "./monitor.js";

-const ZALO_WEBHOOK_RATE_LIMIT_WINDOW_MS = 60_000;
-const ZALO_WEBHOOK_RATE_LIMIT_MAX_REQUESTS = 120;
-const ZALO_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS = 4_096;
 const ZALO_WEBHOOK_REPLAY_WINDOW_MS = 5 * 60_000;
-const ZALO_WEBHOOK_COUNTER_LOG_EVERY = 25;
-const ZALO_WEBHOOK_COUNTER_MAX_TRACKED_KEYS = 4_096;
-const ZALO_WEBHOOK_COUNTER_TTL_MS = 6 * 60 * 60_000;

 export type ZaloWebhookTarget = {
  token: string;
@@ -44,22 +39,23 @@ export type ZaloWebhookProcessUpdate = (params: {

 const webhookTargets = new Map<string, ZaloWebhookTarget[]>();
 const webhookRateLimiter = createFixedWindowRateLimiter({
-  windowMs: ZALO_WEBHOOK_RATE_LIMIT_WINDOW_MS,
-  maxRequests: ZALO_WEBHOOK_RATE_LIMIT_MAX_REQUESTS,
-  maxTrackedKeys: ZALO_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS,
+  windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
+  maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
+  maxTrackedKeys: WEBHOOK_RATE_LIMIT_DEFAULTS.maxTrackedKeys,
 });
 const recentWebhookEvents = createDedupeCache({
  ttlMs: ZALO_WEBHOOK_REPLAY_WINDOW_MS,
  maxSize: 5000,
 });
-const webhookStatusCounters = createBoundedCounter({
-  maxTrackedKeys: ZALO_WEBHOOK_COUNTER_MAX_TRACKED_KEYS,
-  ttlMs: ZALO_WEBHOOK_COUNTER_TTL_MS,
+const webhookAnomalyTracker = createWebhookAnomalyTracker({
+  maxTrackedKeys: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.maxTrackedKeys,
+  ttlMs: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.ttlMs,
+  logEvery: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.logEvery,
 });

 export function clearZaloWebhookSecurityStateForTest(): void {
  webhookRateLimiter.clear();
-  webhookStatusCounters.clear();
+  webhookAnomalyTracker.clear();
 }

 export function getZaloWebhookRateLimitStateSizeForTest(): number {
@@ -67,16 +63,7 @@ export function getZaloWebhookRateLimitStateSizeForTest(): number {
 }

 export function getZaloWebhookStatusCounterSizeForTest(): number {
-  return webhookStatusCounters.size();
-}
-
-function isJsonContentType(value: string | string[] | undefined): boolean {
-  const first = Array.isArray(value) ? value[0] : value;
-  if (!first) {
-    return false;
-  }
-  const mediaType = first.split(";", 1)[0]?.trim().toLowerCase();
-  return mediaType === "application/json" || Boolean(mediaType?.endsWith("+json"));
+  return webhookAnomalyTracker.size();
 }

 function timingSafeEquals(left: string, right: string): boolean {
@@ -110,16 +97,13 @@ function recordWebhookStatus(
  path: string,
  statusCode: number,
 ): void {
-  if (![400, 401, 408, 413, 415, 429].includes(statusCode)) {
-    return;
-  }
-  const key = `${path}:${statusCode}`;
-  const next = webhookStatusCounters.increment(key);
-  if (next === 1 || next % ZALO_WEBHOOK_COUNTER_LOG_EVERY === 0) {
-    runtime?.log?.(
-      `[zalo] webhook anomaly path=${path} status=${statusCode} count=${String(next)}`,
-    );
-  }
+  webhookAnomalyTracker.record({
+    key: `${path}:${statusCode}`,
+    statusCode,
+    log: runtime?.log,
+    message: (count) =>
+      `[zalo] webhook anomaly path=${path} status=${statusCode} count=${String(count)}`,
+  });
 }

 export function registerZaloWebhookTarget(target: ZaloWebhookTarget): () => void {
@@ -137,7 +121,13 @@ export async function handleZaloWebhookRequest(
  }
  const { targets, path } = resolved;

-  if (rejectNonPostWebhookRequest(req, res)) {
+  if (
+    !applyBasicWebhookRequestGuards({
+      req,
+      res,
+      allowMethods: ["POST"],
+    })
+  ) {
    return true;
  }

@@ -161,41 +151,34 @@ export async function handleZaloWebhookRequest(
  const rateLimitKey = `${path}:${req.socket.remoteAddress ?? "unknown"}`;
  const nowMs = Date.now();

-  if (webhookRateLimiter.isRateLimited(rateLimitKey, nowMs)) {
-    res.statusCode = 429;
-    res.end("Too Many Requests");
+  if (
+    !applyBasicWebhookRequestGuards({
+      req,
+      res,
+      rateLimiter: webhookRateLimiter,
+      rateLimitKey,
+      nowMs,
+      requireJsonContentType: true,
+    })
+  ) {
    recordWebhookStatus(target.runtime, path, res.statusCode);
    return true;
  }
-
-  if (!isJsonContentType(req.headers["content-type"])) {
-    res.statusCode = 415;
-    res.end("Unsupported Media Type");
-    recordWebhookStatus(target.runtime, path, res.statusCode);
-    return true;
-  }
-
-  const body = await readJsonBodyWithLimit(req, {
+  const body = await readJsonWebhookBodyOrReject({
+    req,
+    res,
    maxBytes: 1024 * 1024,
    timeoutMs: 30_000,
    emptyObjectOnEmpty: false,
+    invalidJsonMessage: "Bad Request",
  });
  if (!body.ok) {
-    res.statusCode =
-      body.code === "PAYLOAD_TOO_LARGE" ? 413 : body.code === "REQUEST_BODY_TIMEOUT" ? 408 : 400;
-    const message =
-      body.code === "PAYLOAD_TOO_LARGE"
-        ? requestBodyErrorToText("PAYLOAD_TOO_LARGE")
-        : body.code === "REQUEST_BODY_TIMEOUT"
-          ? requestBodyErrorToText("REQUEST_BODY_TIMEOUT")
-          : "Bad Request";
-    res.end(message);
    recordWebhookStatus(target.runtime, path, res.statusCode);
    return true;
  }
+  const raw = body.value;

  // Zalo sends updates directly as { event_name, message, ... }, not wrapped in { ok, result }.
-  const raw = body.value;
  const record = raw && typeof raw === "object" ? (raw as Record<string, unknown>) : null;
  const update: ZaloUpdate | undefined =
    record && record.ok === true && record.result