Skip to content

Commit 3a68c56

Browse files
steipetesteipete
committed
refactor(security): unify webhook guardrails across channels
1 parent 58659b9 commit 3a68c56

7 files changed

Lines changed: 450 additions & 108 deletions

File tree

‎extensions/feishu/src/monitor.ts‎

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@ import * as crypto from "crypto";
22
import * as http from "http";
33
import * as Lark from "@larksuiteoapi/node-sdk";
44
import {
5+
applyBasicWebhookRequestGuards,
56
type ClawdbotConfig,
6-
createBoundedCounter,
77
createFixedWindowRateLimiter,
8+
createWebhookAnomalyTracker,
89
type RuntimeEnv,
910
type HistoryEntry,
1011
installRequestBodyLimitGuard,
12+
WEBHOOK_ANOMALY_COUNTER_DEFAULTS,
13+
WEBHOOK_RATE_LIMIT_DEFAULTS,
1114
} from "openclaw/plugin-sdk";
1215
import { resolveFeishuAccount, listEnabledFeishuAccounts } from "./accounts.js";
1316
import { handleFeishuMessage, type FeishuMessageEvent, type FeishuBotAddedEvent } from "./bot.js";
@@ -30,12 +33,6 @@ const httpServers = new Map<string, http.Server>();
3033
const botOpenIds = new Map<string, string>();
3134
const FEISHU_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024;
3235
const FEISHU_WEBHOOK_BODY_TIMEOUT_MS = 30_000;
33-
const FEISHU_WEBHOOK_RATE_LIMIT_WINDOW_MS = 60_000;
34-
const FEISHU_WEBHOOK_RATE_LIMIT_MAX_REQUESTS = 120;
35-
const FEISHU_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS = 4_096;
36-
const FEISHU_WEBHOOK_COUNTER_LOG_EVERY = 25;
37-
const FEISHU_WEBHOOK_COUNTER_MAX_TRACKED_KEYS = 4_096;
38-
const FEISHU_WEBHOOK_COUNTER_TTL_MS = 6 * 60 * 60_000;
3936
const FEISHU_REACTION_VERIFY_TIMEOUT_MS = 1_500;
4037

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

6259
const feishuWebhookRateLimiter = createFixedWindowRateLimiter({
63-
windowMs: FEISHU_WEBHOOK_RATE_LIMIT_WINDOW_MS,
64-
maxRequests: FEISHU_WEBHOOK_RATE_LIMIT_MAX_REQUESTS,
65-
maxTrackedKeys: FEISHU_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS,
60+
windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
61+
maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
62+
maxTrackedKeys: WEBHOOK_RATE_LIMIT_DEFAULTS.maxTrackedKeys,
6663
});
67-
const feishuWebhookStatusCounters = createBoundedCounter({
68-
maxTrackedKeys: FEISHU_WEBHOOK_COUNTER_MAX_TRACKED_KEYS,
69-
ttlMs: FEISHU_WEBHOOK_COUNTER_TTL_MS,
64+
const feishuWebhookAnomalyTracker = createWebhookAnomalyTracker({
65+
maxTrackedKeys: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.maxTrackedKeys,
66+
ttlMs: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.ttlMs,
67+
logEvery: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.logEvery,
7068
});
7169

72-
function isJsonContentType(value: string | string[] | undefined): boolean {
73-
const first = Array.isArray(value) ? value[0] : value;
74-
if (!first) {
75-
return false;
76-
}
77-
const mediaType = first.split(";", 1)[0]?.trim().toLowerCase();
78-
return mediaType === "application/json" || Boolean(mediaType?.endsWith("+json"));
79-
}
80-
8170
export function clearFeishuWebhookRateLimitStateForTest(): void {
8271
feishuWebhookRateLimiter.clear();
83-
feishuWebhookStatusCounters.clear();
72+
feishuWebhookAnomalyTracker.clear();
8473
}
8574

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

94-
function isWebhookRateLimited(key: string, nowMs: number): boolean {
95-
return isWebhookRateLimitedForTest(key, nowMs);
96-
}
97-
9883
function recordWebhookStatus(
9984
runtime: RuntimeEnv | undefined,
10085
accountId: string,
10186
path: string,
10287
statusCode: number,
10388
): void {
104-
if (![400, 401, 408, 413, 415, 429].includes(statusCode)) {
105-
return;
106-
}
107-
const key = `${accountId}:${path}:${statusCode}`;
108-
const next = feishuWebhookStatusCounters.increment(key);
109-
if (next === 1 || next % FEISHU_WEBHOOK_COUNTER_LOG_EVERY === 0) {
110-
const log = runtime?.log ?? console.log;
111-
log(`feishu[${accountId}]: webhook anomaly path=${path} status=${statusCode} count=${next}`);
112-
}
89+
feishuWebhookAnomalyTracker.record({
90+
key: `${accountId}:${path}:${statusCode}`,
91+
statusCode,
92+
log: runtime?.log ?? console.log,
93+
message: (count) =>
94+
`feishu[${accountId}]: webhook anomaly path=${path} status=${statusCode} count=${count}`,
95+
});
11396
}
11497

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

464447
const rateLimitKey = `${accountId}:${path}:${req.socket.remoteAddress ?? "unknown"}`;
465-
if (isWebhookRateLimited(rateLimitKey, Date.now())) {
466-
res.statusCode = 429;
467-
res.end("Too Many Requests");
468-
return;
469-
}
470-
471-
if (req.method === "POST" && !isJsonContentType(req.headers["content-type"])) {
472-
res.statusCode = 415;
473-
res.end("Unsupported Media Type");
448+
if (
449+
!applyBasicWebhookRequestGuards({
450+
req,
451+
res,
452+
rateLimiter: feishuWebhookRateLimiter,
453+
rateLimitKey,
454+
nowMs: Date.now(),
455+
requireJsonContentType: true,
456+
})
457+
) {
474458
return;
475459
}
476460

‎extensions/zalo/src/monitor.webhook.ts‎

Lines changed: 43 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,22 @@ import { timingSafeEqual } from "node:crypto";
22
import type { IncomingMessage, ServerResponse } from "node:http";
33
import type { OpenClawConfig } from "openclaw/plugin-sdk";
44
import {
5-
createBoundedCounter,
65
createDedupeCache,
76
createFixedWindowRateLimiter,
8-
readJsonBodyWithLimit,
7+
createWebhookAnomalyTracker,
8+
readJsonWebhookBodyOrReject,
9+
applyBasicWebhookRequestGuards,
910
registerWebhookTarget,
10-
rejectNonPostWebhookRequest,
11-
requestBodyErrorToText,
1211
resolveSingleWebhookTarget,
1312
resolveWebhookTargets,
13+
WEBHOOK_ANOMALY_COUNTER_DEFAULTS,
14+
WEBHOOK_RATE_LIMIT_DEFAULTS,
1415
} from "openclaw/plugin-sdk";
1516
import type { ResolvedZaloAccount } from "./accounts.js";
1617
import type { ZaloFetch, ZaloUpdate } from "./api.js";
1718
import type { ZaloRuntimeEnv } from "./monitor.js";
1819

19-
const ZALO_WEBHOOK_RATE_LIMIT_WINDOW_MS = 60_000;
20-
const ZALO_WEBHOOK_RATE_LIMIT_MAX_REQUESTS = 120;
21-
const ZALO_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS = 4_096;
2220
const ZALO_WEBHOOK_REPLAY_WINDOW_MS = 5 * 60_000;
23-
const ZALO_WEBHOOK_COUNTER_LOG_EVERY = 25;
24-
const ZALO_WEBHOOK_COUNTER_MAX_TRACKED_KEYS = 4_096;
25-
const ZALO_WEBHOOK_COUNTER_TTL_MS = 6 * 60 * 60_000;
2621

2722
export type ZaloWebhookTarget = {
2823
token: string;
@@ -44,39 +39,31 @@ export type ZaloWebhookProcessUpdate = (params: {
4439

4540
const webhookTargets = new Map<string, ZaloWebhookTarget[]>();
4641
const webhookRateLimiter = createFixedWindowRateLimiter({
47-
windowMs: ZALO_WEBHOOK_RATE_LIMIT_WINDOW_MS,
48-
maxRequests: ZALO_WEBHOOK_RATE_LIMIT_MAX_REQUESTS,
49-
maxTrackedKeys: ZALO_WEBHOOK_RATE_LIMIT_MAX_TRACKED_KEYS,
42+
windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
43+
maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
44+
maxTrackedKeys: WEBHOOK_RATE_LIMIT_DEFAULTS.maxTrackedKeys,
5045
});
5146
const recentWebhookEvents = createDedupeCache({
5247
ttlMs: ZALO_WEBHOOK_REPLAY_WINDOW_MS,
5348
maxSize: 5000,
5449
});
55-
const webhookStatusCounters = createBoundedCounter({
56-
maxTrackedKeys: ZALO_WEBHOOK_COUNTER_MAX_TRACKED_KEYS,
57-
ttlMs: ZALO_WEBHOOK_COUNTER_TTL_MS,
50+
const webhookAnomalyTracker = createWebhookAnomalyTracker({
51+
maxTrackedKeys: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.maxTrackedKeys,
52+
ttlMs: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.ttlMs,
53+
logEvery: WEBHOOK_ANOMALY_COUNTER_DEFAULTS.logEvery,
5854
});
5955

6056
export function clearZaloWebhookSecurityStateForTest(): void {
6157
webhookRateLimiter.clear();
62-
webhookStatusCounters.clear();
58+
webhookAnomalyTracker.clear();
6359
}
6460

6561
export function getZaloWebhookRateLimitStateSizeForTest(): number {
6662
return webhookRateLimiter.size();
6763
}
6864

6965
export function getZaloWebhookStatusCounterSizeForTest(): number {
70-
return webhookStatusCounters.size();
71-
}
72-
73-
function isJsonContentType(value: string | string[] | undefined): boolean {
74-
const first = Array.isArray(value) ? value[0] : value;
75-
if (!first) {
76-
return false;
77-
}
78-
const mediaType = first.split(";", 1)[0]?.trim().toLowerCase();
79-
return mediaType === "application/json" || Boolean(mediaType?.endsWith("+json"));
66+
return webhookAnomalyTracker.size();
8067
}
8168

8269
function timingSafeEquals(left: string, right: string): boolean {
@@ -110,16 +97,13 @@ function recordWebhookStatus(
11097
path: string,
11198
statusCode: number,
11299
): void {
113-
if (![400, 401, 408, 413, 415, 429].includes(statusCode)) {
114-
return;
115-
}
116-
const key = `${path}:${statusCode}`;
117-
const next = webhookStatusCounters.increment(key);
118-
if (next === 1 || next % ZALO_WEBHOOK_COUNTER_LOG_EVERY === 0) {
119-
runtime?.log?.(
120-
`[zalo] webhook anomaly path=${path} status=${statusCode} count=${String(next)}`,
121-
);
122-
}
100+
webhookAnomalyTracker.record({
101+
key: `${path}:${statusCode}`,
102+
statusCode,
103+
log: runtime?.log,
104+
message: (count) =>
105+
`[zalo] webhook anomaly path=${path} status=${statusCode} count=${String(count)}`,
106+
});
123107
}
124108

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

140-
if (rejectNonPostWebhookRequest(req, res)) {
124+
if (
125+
!applyBasicWebhookRequestGuards({
126+
req,
127+
res,
128+
allowMethods: ["POST"],
129+
})
130+
) {
141131
return true;
142132
}
143133

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

164-
if (webhookRateLimiter.isRateLimited(rateLimitKey, nowMs)) {
165-
res.statusCode = 429;
166-
res.end("Too Many Requests");
154+
if (
155+
!applyBasicWebhookRequestGuards({
156+
req,
157+
res,
158+
rateLimiter: webhookRateLimiter,
159+
rateLimitKey,
160+
nowMs,
161+
requireJsonContentType: true,
162+
})
163+
) {
167164
recordWebhookStatus(target.runtime, path, res.statusCode);
168165
return true;
169166
}
170-
171-
if (!isJsonContentType(req.headers["content-type"])) {
172-
res.statusCode = 415;
173-
res.end("Unsupported Media Type");
174-
recordWebhookStatus(target.runtime, path, res.statusCode);
175-
return true;
176-
}
177-
178-
const body = await readJsonBodyWithLimit(req, {
167+
const body = await readJsonWebhookBodyOrReject({
168+
req,
169+
res,
179170
maxBytes: 1024 * 1024,
180171
timeoutMs: 30_000,
181172
emptyObjectOnEmpty: false,
173+
invalidJsonMessage: "Bad Request",
182174
});
183175
if (!body.ok) {
184-
res.statusCode =
185-
body.code === "PAYLOAD_TOO_LARGE" ? 413 : body.code === "REQUEST_BODY_TIMEOUT" ? 408 : 400;
186-
const message =
187-
body.code === "PAYLOAD_TOO_LARGE"
188-
? requestBodyErrorToText("PAYLOAD_TOO_LARGE")
189-
: body.code === "REQUEST_BODY_TIMEOUT"
190-
? requestBodyErrorToText("REQUEST_BODY_TIMEOUT")
191-
: "Bad Request";
192-
res.end(message);
193176
recordWebhookStatus(target.runtime, path, res.statusCode);
194177
return true;
195178
}
179+
const raw = body.value;
196180

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

‎src/plugin-sdk/index.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ export {
129129
resolveWebhookTargets,
130130
} from "./webhook-targets.js";
131131
export type { WebhookTargetMatchResult } from "./webhook-targets.js";
132+
export {
133+
applyBasicWebhookRequestGuards,
134+
isJsonContentType,
135+
readJsonWebhookBodyOrReject,
136+
} from "./webhook-request-guards.js";
132137
export type { AgentMediaPayload } from "./agent-media-payload.js";
133138
export { buildAgentMediaPayload } from "./agent-media-payload.js";
134139
export {
@@ -297,8 +302,19 @@ export {
297302
readRequestBodyWithLimit,
298303
requestBodyErrorToText,
299304
} from "../infra/http-body.js";
300-
export { createBoundedCounter, createFixedWindowRateLimiter } from "./webhook-memory-guards.js";
301-
export type { BoundedCounter, FixedWindowRateLimiter } from "./webhook-memory-guards.js";
305+
export {
306+
WEBHOOK_ANOMALY_COUNTER_DEFAULTS,
307+
WEBHOOK_ANOMALY_STATUS_CODES,
308+
WEBHOOK_RATE_LIMIT_DEFAULTS,
309+
createBoundedCounter,
310+
createFixedWindowRateLimiter,
311+
createWebhookAnomalyTracker,
312+
} from "./webhook-memory-guards.js";
313+
export type {
314+
BoundedCounter,
315+
FixedWindowRateLimiter,
316+
WebhookAnomalyTracker,
317+
} from "./webhook-memory-guards.js";
302318

303319
export { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js";
304320
export {

0 commit comments

Comments
 (0)