From f96ba87f033a14183fa0ede912df3a592eef55ff Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 12 Mar 2026 12:30:50 -0400 Subject: [PATCH] Zalo: rate limit invalid webhook secret guesses before auth (#44173) * Zalo: rate limit webhook guesses before auth * Tests: cover pre-auth Zalo webhook rate limiting * Changelog: note Zalo pre-auth rate limiting * Zalo: preserve auth-before-content-type response ordering * Tests: cover auth-before-content-type webhook ordering * Zalo: split auth and unauth webhook rate-limit buckets * Tests: cover auth bucket split for Zalo webhook rate limiting * Zalo: use trusted proxy client IP for webhook rate limiting * Tests: cover trusted proxy client IP rate limiting for Zalo --- CHANGELOG.md | 1 + extensions/zalo/src/monitor.webhook.test.ts | 95 ++++++++++++++++++++- extensions/zalo/src/monitor.webhook.ts | 40 +++++++-- 3 files changed, 128 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7afb8cc61be..1a08ef00e0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai - Security/WebSocket preauth: shorten unauthenticated handshake retention and reject oversized pre-auth frames before application-layer parsing to reduce pre-pairing exposure on unsupported public deployments. (`GHSA-jv4g-m82p-2j93`)(#44089) (`GHSA-xwx2-ppv2-wx98`)(#44089) Thanks @ez-lbz and @vincentkoc. - Security/Feishu reactions: preserve looked-up group chat typing and fail closed on ambiguous reaction context so group authorization and mention gating cannot be bypassed through synthetic `p2p` reactions. (`GHSA-m69h-jm2f-2pv8`)(#44088) Thanks @zpbrent and @vincentkoc. - Security/LINE webhook: require signatures for empty-event POST probes too so unsigned requests no longer confirm webhook reachability with a `200` response. (`GHSA-mhxh-9pjm-w7q5`)(#44090) Thanks @TerminalsandCoffee and @vincentkoc. +- Security/Zalo webhook: rate limit invalid secret guesses before auth so weak webhook secrets cannot be brute-forced through unauthenticated churned requests without pre-auth `429` responses. (`GHSA-5m9r-p9g7-679c`)(#44173) Thanks @zpbrent and @vincentkoc. - Security/plugins: disable implicit workspace plugin auto-load so cloned repositories cannot execute workspace plugin code without an explicit trust decision. (`GHSA-99qw-6mr3-36qr`)(#44174) Thanks @lintsinghua and @vincentkoc. ### Changes diff --git a/extensions/zalo/src/monitor.webhook.test.ts b/extensions/zalo/src/monitor.webhook.test.ts index 297d8249d3a..57b5f43202e 100644 --- a/extensions/zalo/src/monitor.webhook.test.ts +++ b/extensions/zalo/src/monitor.webhook.test.ts @@ -283,6 +283,7 @@ describe("handleZaloWebhookRequest", () => { try { await withServer(webhookRequestHandler, async (baseUrl) => { + let saw429 = false; for (let i = 0; i < 200; i += 1) { const response = await fetch(`${baseUrl}/hook-query-status?nonce=${i}`, { method: "POST", @@ -292,10 +293,15 @@ describe("handleZaloWebhookRequest", () => { }, body: "{}", }); - expect(response.status).toBe(401); + expect([401, 429]).toContain(response.status); + if (response.status === 429) { + saw429 = true; + break; + } } - expect(getZaloWebhookStatusCounterSizeForTest()).toBe(1); + expect(saw429).toBe(true); + expect(getZaloWebhookStatusCounterSizeForTest()).toBe(2); }); } finally { unregister(); @@ -322,6 +328,91 @@ describe("handleZaloWebhookRequest", () => { } }); + it("rate limits unauthorized secret guesses before authentication succeeds", async () => { + const unregister = registerTarget({ path: "/hook-preauth-rate" }); + + try { + await withServer(webhookRequestHandler, async (baseUrl) => { + const saw429 = await postUntilRateLimited({ + baseUrl, + path: "/hook-preauth-rate", + secret: "invalid-token", // pragma: allowlist secret + withNonceQuery: true, + }); + + expect(saw429).toBe(true); + expect(getZaloWebhookRateLimitStateSizeForTest()).toBe(1); + }); + } finally { + unregister(); + } + }); + + it("does not let unauthorized floods rate-limit authenticated traffic from a different trusted forwarded client IP", async () => { + const unregister = registerTarget({ + path: "/hook-preauth-split", + config: { + gateway: { + trustedProxies: ["127.0.0.1"], + }, + } as OpenClawConfig, + }); + + try { + await withServer(webhookRequestHandler, async (baseUrl) => { + for (let i = 0; i < 130; i += 1) { + const response = await fetch(`${baseUrl}/hook-preauth-split?nonce=${i}`, { + method: "POST", + headers: { + "x-bot-api-secret-token": "invalid-token", // pragma: allowlist secret + "content-type": "application/json", + "x-forwarded-for": "203.0.113.10", + }, + body: "{}", + }); + if (response.status === 429) { + break; + } + } + + const validResponse = await fetch(`${baseUrl}/hook-preauth-split`, { + method: "POST", + headers: { + "x-bot-api-secret-token": "secret", + "content-type": "application/json", + "x-forwarded-for": "198.51.100.20", + }, + body: JSON.stringify({ event_name: "message.unsupported.received" }), + }); + + expect(validResponse.status).toBe(200); + }); + } finally { + unregister(); + } + }); + + it("still returns 401 before 415 when both secret and content-type are invalid", async () => { + const unregister = registerTarget({ path: "/hook-auth-before-type" }); + + try { + await withServer(webhookRequestHandler, async (baseUrl) => { + const response = await fetch(`${baseUrl}/hook-auth-before-type`, { + method: "POST", + headers: { + "x-bot-api-secret-token": "invalid-token", // pragma: allowlist secret + "content-type": "text/plain", + }, + body: "not-json", + }); + + expect(response.status).toBe(401); + }); + } finally { + unregister(); + } + }); + it("scopes DM pairing store reads and writes to accountId", async () => { const { core, readAllowFromStore, upsertPairingRequest } = createPairingAuthCore({ pairingCreated: false, diff --git a/extensions/zalo/src/monitor.webhook.ts b/extensions/zalo/src/monitor.webhook.ts index 8fad827fddc..ef10d3a9a0e 100644 --- a/extensions/zalo/src/monitor.webhook.ts +++ b/extensions/zalo/src/monitor.webhook.ts @@ -16,6 +16,7 @@ import { WEBHOOK_ANOMALY_COUNTER_DEFAULTS, WEBHOOK_RATE_LIMIT_DEFAULTS, } from "openclaw/plugin-sdk/zalo"; +import { resolveClientIp } from "../../../src/gateway/net.js"; import type { ResolvedZaloAccount } from "./accounts.js"; import type { ZaloFetch, ZaloUpdate } from "./api.js"; import type { ZaloRuntimeEnv } from "./monitor.js"; @@ -109,6 +110,10 @@ function recordWebhookStatus( }); } +function headerValue(value: string | string[] | undefined): string | undefined { + return Array.isArray(value) ? value[0] : value; +} + export function registerZaloWebhookTarget( target: ZaloWebhookTarget, opts?: { @@ -140,6 +145,33 @@ export async function handleZaloWebhookRequest( targetsByPath: webhookTargets, allowMethods: ["POST"], handle: async ({ targets, path }) => { + const trustedProxies = targets[0]?.config.gateway?.trustedProxies; + const allowRealIpFallback = targets[0]?.config.gateway?.allowRealIpFallback === true; + const clientIp = + resolveClientIp({ + remoteAddr: req.socket.remoteAddress, + forwardedFor: headerValue(req.headers["x-forwarded-for"]), + realIp: headerValue(req.headers["x-real-ip"]), + trustedProxies, + allowRealIpFallback, + }) ?? + req.socket.remoteAddress ?? + "unknown"; + const rateLimitKey = `${path}:${clientIp}`; + const nowMs = Date.now(); + if ( + !applyBasicWebhookRequestGuards({ + req, + res, + rateLimiter: webhookRateLimiter, + rateLimitKey, + nowMs, + }) + ) { + recordWebhookStatus(targets[0]?.runtime, path, res.statusCode); + return true; + } + const headerToken = String(req.headers["x-bot-api-secret-token"] ?? ""); const target = resolveWebhookTargetWithAuthOrRejectSync({ targets, @@ -150,16 +182,12 @@ export async function handleZaloWebhookRequest( recordWebhookStatus(targets[0]?.runtime, path, res.statusCode); return true; } - const rateLimitKey = `${path}:${req.socket.remoteAddress ?? "unknown"}`; - const nowMs = Date.now(); - + // Preserve the historical 401-before-415 ordering for invalid secrets while still + // consuming rate-limit budget on unauthenticated guesses. if ( !applyBasicWebhookRequestGuards({ req, res, - rateLimiter: webhookRateLimiter, - rateLimitKey, - nowMs, requireJsonContentType: true, }) ) {