mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-27 09:21:35 +07:00
fix(bluebubbles): throttle webhook auth guesses (#55133)
* fix(bluebubbles): throttle webhook auth guesses * test(bluebubbles): isolate attachment ssrf config * test(bluebubbles): hoist attachment mocks * docs: refresh bluebubbles config baseline * fix(bluebubbles): trust proxied webhook client IPs * fix(bluebubbles): honor trusted proxy webhook IPs * fix(bluebubbles): honor real-ip fallback for webhooks
This commit is contained in:
@@ -5088,6 +5088,15 @@
|
||||
"path": "src/infra/http-body.ts"
|
||||
}
|
||||
},
|
||||
{
|
||||
"declaration": "export function resolveRequestClientIp(req?: IncomingMessage | undefined, trustedProxies?: string[] | undefined, allowRealIpFallback?: boolean): string | undefined;",
|
||||
"exportName": "resolveRequestClientIp",
|
||||
"kind": "function",
|
||||
"source": {
|
||||
"line": 186,
|
||||
"path": "src/gateway/net.ts"
|
||||
}
|
||||
},
|
||||
{
|
||||
"declaration": "export function resolveSingleWebhookTarget<T>(targets: readonly T[], isMatch: (target: T) => boolean): WebhookTargetMatchResult<T>;",
|
||||
"exportName": "resolveSingleWebhookTarget",
|
||||
|
||||
@@ -560,6 +560,7 @@
|
||||
{"declaration":"export function registerWebhookTarget<T extends { path: string; }>(targetsByPath: Map<string, T[]>, target: T, opts?: RegisterWebhookTargetOptions<T> | undefined): RegisteredWebhookTarget<T>;","entrypoint":"webhook-ingress","exportName":"registerWebhookTarget","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":61,"sourcePath":"src/plugin-sdk/webhook-targets.ts"}
|
||||
{"declaration":"export function registerWebhookTargetWithPluginRoute<T extends { path: string; }>(params: { targetsByPath: Map<string, T[]>; target: T; route: RegisterWebhookPluginRouteOptions; onLastPathTargetRemoved?: ((params: { ...; }) => void) | undefined; }): RegisteredWebhookTarget<...>;","entrypoint":"webhook-ingress","exportName":"registerWebhookTargetWithPluginRoute","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":30,"sourcePath":"src/plugin-sdk/webhook-targets.ts"}
|
||||
{"declaration":"export function requestBodyErrorToText(code: RequestBodyLimitErrorCode): string;","entrypoint":"webhook-ingress","exportName":"requestBodyErrorToText","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":60,"sourcePath":"src/infra/http-body.ts"}
|
||||
{"declaration":"export function resolveRequestClientIp(req?: IncomingMessage | undefined, trustedProxies?: string[] | undefined, allowRealIpFallback?: boolean): string | undefined;","entrypoint":"webhook-ingress","exportName":"resolveRequestClientIp","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":186,"sourcePath":"src/gateway/net.ts"}
|
||||
{"declaration":"export function resolveSingleWebhookTarget<T>(targets: readonly T[], isMatch: (target: T) => boolean): WebhookTargetMatchResult<T>;","entrypoint":"webhook-ingress","exportName":"resolveSingleWebhookTarget","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":193,"sourcePath":"src/plugin-sdk/webhook-targets.ts"}
|
||||
{"declaration":"export function resolveSingleWebhookTargetAsync<T>(targets: readonly T[], isMatch: (target: T) => Promise<boolean>): Promise<WebhookTargetMatchResult<T>>;","entrypoint":"webhook-ingress","exportName":"resolveSingleWebhookTargetAsync","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":212,"sourcePath":"src/plugin-sdk/webhook-targets.ts"}
|
||||
{"declaration":"export function resolveWebhookPath(params: { webhookPath?: string | undefined; webhookUrl?: string | undefined; defaultPath?: string | null | undefined; }): string | null;","entrypoint":"webhook-ingress","exportName":"resolveWebhookPath","importSpecifier":"openclaw/plugin-sdk/webhook-ingress","kind":"function","recordType":"export","sourceLine":15,"sourcePath":"src/plugin-sdk/webhook-path.ts"}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { downloadBlueBubblesAttachment, sendBlueBubblesAttachment } from "./attachments.js";
|
||||
import "./test-mocks.js";
|
||||
import { downloadBlueBubblesAttachment, sendBlueBubblesAttachment } from "./attachments.js";
|
||||
import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
|
||||
import type { PluginRuntime } from "./runtime-api.js";
|
||||
import { setBlueBubblesRuntime } from "./runtime.js";
|
||||
@@ -295,6 +295,7 @@ describe("downloadBlueBubblesAttachment", () => {
|
||||
await downloadBlueBubblesAttachment(attachment, {
|
||||
serverUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
cfg: { channels: { bluebubbles: {} } },
|
||||
});
|
||||
|
||||
const fetchMediaArgs = fetchRemoteMediaMock.mock.calls[0][0] as Record<string, unknown>;
|
||||
@@ -308,6 +309,7 @@ describe("downloadBlueBubblesAttachment", () => {
|
||||
await downloadBlueBubblesAttachment(attachment, {
|
||||
serverUrl: "http://192.168.1.5:1234",
|
||||
password: "test",
|
||||
cfg: { channels: { bluebubbles: {} } },
|
||||
});
|
||||
|
||||
const fetchMediaArgs = fetchRemoteMediaMock.mock.calls[0][0] as Record<string, unknown>;
|
||||
|
||||
@@ -16,18 +16,31 @@ import {
|
||||
} from "./monitor-shared.js";
|
||||
import { fetchBlueBubblesServerInfo } from "./probe.js";
|
||||
import {
|
||||
WEBHOOK_RATE_LIMIT_DEFAULTS,
|
||||
createFixedWindowRateLimiter,
|
||||
createWebhookInFlightLimiter,
|
||||
registerWebhookTargetWithPluginRoute,
|
||||
readWebhookBodyOrReject,
|
||||
resolveRequestClientIp,
|
||||
resolveWebhookTargetWithAuthOrRejectSync,
|
||||
withResolvedWebhookRequestPipeline,
|
||||
} from "./runtime-api.js";
|
||||
import { getBlueBubblesRuntime } from "./runtime.js";
|
||||
|
||||
const webhookTargets = new Map<string, WebhookTarget[]>();
|
||||
const webhookRateLimiter = createFixedWindowRateLimiter({
|
||||
windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
|
||||
maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
|
||||
maxTrackedKeys: WEBHOOK_RATE_LIMIT_DEFAULTS.maxTrackedKeys,
|
||||
});
|
||||
const webhookInFlightLimiter = createWebhookInFlightLimiter();
|
||||
const debounceRegistry = createBlueBubblesDebounceRegistry({ processMessage });
|
||||
|
||||
export function clearBlueBubblesWebhookSecurityStateForTest(): void {
|
||||
webhookRateLimiter.clear();
|
||||
webhookInFlightLimiter.clear();
|
||||
}
|
||||
|
||||
export function registerBlueBubblesWebhookTarget(target: WebhookTarget): () => void {
|
||||
const registered = registerWebhookTargetWithPluginRoute({
|
||||
targetsByPath: webhookTargets,
|
||||
@@ -117,18 +130,62 @@ function safeEqualSecret(aRaw: string, bRaw: string): boolean {
|
||||
return timingSafeEqual(bufA, bufB);
|
||||
}
|
||||
|
||||
function collectTrustedProxies(targets: readonly WebhookTarget[]): string[] {
|
||||
const proxies = new Set<string>();
|
||||
for (const target of targets) {
|
||||
for (const proxy of target.config.gateway?.trustedProxies ?? []) {
|
||||
const normalized = proxy.trim();
|
||||
if (normalized) {
|
||||
proxies.add(normalized);
|
||||
}
|
||||
}
|
||||
}
|
||||
return [...proxies];
|
||||
}
|
||||
|
||||
function resolveWebhookAllowRealIpFallback(targets: readonly WebhookTarget[]): boolean {
|
||||
return targets.some((target) => target.config.gateway?.allowRealIpFallback === true);
|
||||
}
|
||||
|
||||
function resolveWebhookClientIp(
|
||||
req: IncomingMessage,
|
||||
trustedProxies: readonly string[],
|
||||
allowRealIpFallback: boolean,
|
||||
): string {
|
||||
if (!req.headers["x-forwarded-for"] && !(allowRealIpFallback && req.headers["x-real-ip"])) {
|
||||
return req.socket.remoteAddress ?? "unknown";
|
||||
}
|
||||
|
||||
// Mirror gateway client-IP trust rules so limiter buckets follow configured proxy hops.
|
||||
return (
|
||||
resolveRequestClientIp(req, [...trustedProxies], allowRealIpFallback) ??
|
||||
req.socket.remoteAddress ??
|
||||
"unknown"
|
||||
);
|
||||
}
|
||||
|
||||
export async function handleBlueBubblesWebhookRequest(
|
||||
req: IncomingMessage,
|
||||
res: ServerResponse,
|
||||
): Promise<boolean> {
|
||||
const requestUrl = new URL(req.url ?? "/", "http://localhost");
|
||||
const normalizedPath = normalizeWebhookPath(requestUrl.pathname);
|
||||
const pathTargets = webhookTargets.get(normalizedPath) ?? [];
|
||||
const trustedProxies = collectTrustedProxies(pathTargets);
|
||||
const allowRealIpFallback = resolveWebhookAllowRealIpFallback(pathTargets);
|
||||
const clientIp = resolveWebhookClientIp(req, trustedProxies, allowRealIpFallback);
|
||||
const rateLimitKey = `${normalizedPath}:${clientIp}`;
|
||||
return await withResolvedWebhookRequestPipeline({
|
||||
req,
|
||||
res,
|
||||
targetsByPath: webhookTargets,
|
||||
allowMethods: ["POST"],
|
||||
rateLimiter: webhookRateLimiter,
|
||||
rateLimitKey,
|
||||
inFlightLimiter: webhookInFlightLimiter,
|
||||
inFlightKey: `${normalizedPath}:${clientIp}`,
|
||||
handle: async ({ path, targets }) => {
|
||||
const url = new URL(req.url ?? "/", "http://localhost");
|
||||
const url = requestUrl;
|
||||
const guidParam = url.searchParams.get("guid") ?? url.searchParams.get("password");
|
||||
const headerToken =
|
||||
req.headers["x-guid"] ??
|
||||
|
||||
@@ -406,6 +406,163 @@ describe("BlueBubbles webhook monitor", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rate limits repeated invalid password guesses from the same client", async () => {
|
||||
setupWebhookTarget({
|
||||
account: createMockAccount({
|
||||
password: "99999999",
|
||||
}),
|
||||
});
|
||||
|
||||
let saw429 = false;
|
||||
// Default webhook fixed-window budget is 120 requests/minute, so loop past it.
|
||||
for (let i = 0; i < 130; i += 1) {
|
||||
const candidate = String(i).padStart(8, "0");
|
||||
const { res } = await dispatchWebhookPayloadForTest(
|
||||
createPasswordQueryRequestParamsForTest({
|
||||
password: candidate,
|
||||
body: createTimestampedNewMessagePayloadForTest({
|
||||
guid: `msg-${i}`,
|
||||
text: `hello ${i}`,
|
||||
}),
|
||||
remoteAddress: "192.168.1.100",
|
||||
}),
|
||||
);
|
||||
|
||||
if (res.statusCode === 429) {
|
||||
saw429 = true;
|
||||
break;
|
||||
}
|
||||
|
||||
expect(res.statusCode).toBe(401);
|
||||
}
|
||||
|
||||
expect(saw429).toBe(true);
|
||||
expect(mockDispatchReplyWithBufferedBlockDispatcher).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps forwarded clients behind configured trusted proxies in separate auth buckets", async () => {
|
||||
setupWebhookTarget({
|
||||
account: createMockAccount({
|
||||
password: "99999999",
|
||||
}),
|
||||
config: {
|
||||
gateway: {
|
||||
trustedProxies: ["10.0.0.0/8"],
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
});
|
||||
|
||||
let saw429 = false;
|
||||
for (let i = 0; i < 130; i += 1) {
|
||||
const candidate = String(i).padStart(8, "0");
|
||||
const { res } = await dispatchWebhookPayloadForTest(
|
||||
createPasswordQueryRequestParamsForTest({
|
||||
password: candidate,
|
||||
body: createTimestampedNewMessagePayloadForTest({
|
||||
guid: `proxy-msg-${i}`,
|
||||
text: `hello proxy ${i}`,
|
||||
}),
|
||||
remoteAddress: "10.0.0.5",
|
||||
overrides: {
|
||||
headers: {
|
||||
host: "localhost",
|
||||
"x-forwarded-for": "203.0.113.10",
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
if (res.statusCode === 429) {
|
||||
saw429 = true;
|
||||
break;
|
||||
}
|
||||
|
||||
expect(res.statusCode).toBe(401);
|
||||
}
|
||||
|
||||
expect(saw429).toBe(true);
|
||||
|
||||
await expectWebhookRequestStatusForTest(
|
||||
createPasswordQueryRequestParamsForTest({
|
||||
password: "wrong-pass",
|
||||
body: createTimestampedNewMessagePayloadForTest({
|
||||
guid: "proxy-msg-other-client",
|
||||
text: "hello other proxy client",
|
||||
}),
|
||||
remoteAddress: "10.0.0.5",
|
||||
overrides: {
|
||||
headers: {
|
||||
host: "localhost",
|
||||
"x-forwarded-for": "203.0.113.11",
|
||||
},
|
||||
},
|
||||
}),
|
||||
401,
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps real-ip fallback clients behind trusted proxies in separate auth buckets", async () => {
|
||||
setupWebhookTarget({
|
||||
account: createMockAccount({
|
||||
password: "99999999",
|
||||
}),
|
||||
config: {
|
||||
gateway: {
|
||||
trustedProxies: ["10.0.0.0/8"],
|
||||
allowRealIpFallback: true,
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
});
|
||||
|
||||
let saw429 = false;
|
||||
for (let i = 0; i < 130; i += 1) {
|
||||
const candidate = String(i).padStart(8, "0");
|
||||
const { res } = await dispatchWebhookPayloadForTest(
|
||||
createPasswordQueryRequestParamsForTest({
|
||||
password: candidate,
|
||||
body: createTimestampedNewMessagePayloadForTest({
|
||||
guid: `real-ip-msg-${i}`,
|
||||
text: `hello real ip ${i}`,
|
||||
}),
|
||||
remoteAddress: "10.0.0.5",
|
||||
overrides: {
|
||||
headers: {
|
||||
host: "localhost",
|
||||
"x-real-ip": "203.0.113.10",
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
if (res.statusCode === 429) {
|
||||
saw429 = true;
|
||||
break;
|
||||
}
|
||||
|
||||
expect(res.statusCode).toBe(401);
|
||||
}
|
||||
|
||||
expect(saw429).toBe(true);
|
||||
|
||||
await expectWebhookRequestStatusForTest(
|
||||
createPasswordQueryRequestParamsForTest({
|
||||
password: "wrong-pass",
|
||||
body: createTimestampedNewMessagePayloadForTest({
|
||||
guid: "real-ip-msg-other-client",
|
||||
text: "hello other real ip client",
|
||||
}),
|
||||
remoteAddress: "10.0.0.5",
|
||||
overrides: {
|
||||
headers: {
|
||||
host: "localhost",
|
||||
"x-real-ip": "203.0.113.11",
|
||||
},
|
||||
},
|
||||
}),
|
||||
401,
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects ambiguous routing when multiple targets match the same password", async () => {
|
||||
const targetA = createProtectedWebhookTarget();
|
||||
const targetB = createProtectedWebhookTarget();
|
||||
|
||||
@@ -87,10 +87,13 @@ export {
|
||||
} from "./status-helpers.js";
|
||||
export { extractToolSend } from "./tool-send.js";
|
||||
export {
|
||||
WEBHOOK_RATE_LIMIT_DEFAULTS,
|
||||
createFixedWindowRateLimiter,
|
||||
createWebhookInFlightLimiter,
|
||||
normalizeWebhookPath,
|
||||
readWebhookBodyOrReject,
|
||||
registerWebhookTargetWithPluginRoute,
|
||||
resolveRequestClientIp,
|
||||
resolveWebhookTargets,
|
||||
resolveWebhookTargetWithAuthOrRejectSync,
|
||||
withResolvedWebhookRequestPipeline,
|
||||
|
||||
@@ -40,4 +40,5 @@ export {
|
||||
type WebhookTargetMatchResult,
|
||||
} from "./webhook-targets.js";
|
||||
export { normalizeWebhookPath, resolveWebhookPath } from "./webhook-path.js";
|
||||
export { resolveRequestClientIp } from "../gateway/net.js";
|
||||
export { normalizePluginHttpPath } from "../plugins/http-path.js";
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { vi } from "vitest";
|
||||
import type { BlueBubblesHistoryFetchResult } from "../../../extensions/bluebubbles/src/history.js";
|
||||
import { _resetBlueBubblesShortIdState } from "../../../extensions/bluebubbles/src/monitor.js";
|
||||
import {
|
||||
_resetBlueBubblesShortIdState,
|
||||
clearBlueBubblesWebhookSecurityStateForTest,
|
||||
} from "../../../extensions/bluebubbles/src/monitor.js";
|
||||
import type { PluginRuntime } from "../../../extensions/bluebubbles/src/runtime-api.js";
|
||||
import { setBlueBubblesRuntime } from "../../../extensions/bluebubbles/src/runtime.js";
|
||||
import { createPluginRuntimeMock } from "./plugin-runtime-mock.js";
|
||||
@@ -131,6 +134,7 @@ export function resetBlueBubblesMonitorTestState(params: {
|
||||
}) {
|
||||
vi.clearAllMocks();
|
||||
_resetBlueBubblesShortIdState();
|
||||
clearBlueBubblesWebhookSecurityStateForTest();
|
||||
params.extraReset?.();
|
||||
params.fetchHistoryMock.mockResolvedValue({ entries: [], resolved: true });
|
||||
params.readAllowFromStoreMock.mockResolvedValue([]);
|
||||
|
||||
Reference in New Issue
Block a user