mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-13 11:00:50 +00:00
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user