diff --git a/CHANGELOG.md b/CHANGELOG.md index c44881f918d..e803b1a260a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ Docs: https://docs.openclaw.ai - Security/Archive: enforce archive extraction entry/size limits to prevent resource exhaustion from high-expansion ZIP/TAR archives. Thanks @vincentkoc. - Security/Media: reject oversized base64-backed input media before decoding to avoid large allocations. Thanks @vincentkoc. - Security/Gateway: reject oversized base64 chat attachments before decoding to avoid large allocations. Thanks @vincentkoc. +- Security/Zalo: reject ambiguous shared-path webhook routing when multiple webhook targets match the same secret. +- Security/BlueBubbles: reject ambiguous shared-path webhook routing when multiple webhook targets match the same guid/password. - Cron/Slack: preserve agent identity (name and icon) when cron jobs deliver outbound messages. (#16242) Thanks @robbyczgw-cla. ## 2026.2.14 diff --git a/extensions/bluebubbles/src/monitor.test.ts b/extensions/bluebubbles/src/monitor.test.ts index 6aae7e7c54a..0fe34082b9a 100644 --- a/extensions/bluebubbles/src/monitor.test.ts +++ b/extensions/bluebubbles/src/monitor.test.ts @@ -557,6 +557,114 @@ describe("BlueBubbles webhook monitor", () => { expect(res.statusCode).toBe(401); }); + it("rejects ambiguous routing when multiple targets match the same password", async () => { + const accountA = createMockAccount({ password: "secret-token" }); + const accountB = createMockAccount({ password: "secret-token" }); + const config: OpenClawConfig = {}; + const core = createMockRuntime(); + setBlueBubblesRuntime(core); + + const sinkA = vi.fn(); + const sinkB = vi.fn(); + + const req = createMockRequest("POST", "/bluebubbles-webhook?password=secret-token", { + type: "new-message", + data: { + text: "hello", + handle: { address: "+15551234567" }, + isGroup: false, + isFromMe: false, + guid: "msg-1", + }, + }); + (req as unknown as { socket: { remoteAddress: string } }).socket = { + remoteAddress: "192.168.1.100", + }; + + const unregisterA = registerBlueBubblesWebhookTarget({ + account: accountA, + config, + runtime: { log: vi.fn(), error: vi.fn() }, + core, + path: "/bluebubbles-webhook", + statusSink: sinkA, + }); + const unregisterB = registerBlueBubblesWebhookTarget({ + account: accountB, + config, + runtime: { log: vi.fn(), error: vi.fn() }, + core, + path: "/bluebubbles-webhook", + statusSink: sinkB, + }); + unregister = () => { + unregisterA(); + unregisterB(); + }; + + const res = createMockResponse(); + const handled = await handleBlueBubblesWebhookRequest(req, res); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(401); + expect(sinkA).not.toHaveBeenCalled(); + expect(sinkB).not.toHaveBeenCalled(); + }); + + it("does not route to passwordless targets when a password-authenticated target matches", async () => { + const accountStrict = createMockAccount({ password: "secret-token" }); + const accountFallback = createMockAccount({ password: undefined }); + const config: OpenClawConfig = {}; + const core = createMockRuntime(); + setBlueBubblesRuntime(core); + + const sinkStrict = vi.fn(); + const sinkFallback = vi.fn(); + + const req = createMockRequest("POST", "/bluebubbles-webhook?password=secret-token", { + type: "new-message", + data: { + text: "hello", + handle: { address: "+15551234567" }, + isGroup: false, + isFromMe: false, + guid: "msg-1", + }, + }); + (req as unknown as { socket: { remoteAddress: string } }).socket = { + remoteAddress: "192.168.1.100", + }; + + const unregisterStrict = registerBlueBubblesWebhookTarget({ + account: accountStrict, + config, + runtime: { log: vi.fn(), error: vi.fn() }, + core, + path: "/bluebubbles-webhook", + statusSink: sinkStrict, + }); + const unregisterFallback = registerBlueBubblesWebhookTarget({ + account: accountFallback, + config, + runtime: { log: vi.fn(), error: vi.fn() }, + core, + path: "/bluebubbles-webhook", + statusSink: sinkFallback, + }); + unregister = () => { + unregisterStrict(); + unregisterFallback(); + }; + + const res = createMockResponse(); + const handled = await handleBlueBubblesWebhookRequest(req, res); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(200); + expect(sinkStrict).toHaveBeenCalledTimes(1); + expect(sinkFallback).not.toHaveBeenCalled(); + }); + it("requires authentication for loopback requests when password is configured", async () => { const account = createMockAccount({ password: "secret-token" }); const config: OpenClawConfig = {}; diff --git a/extensions/bluebubbles/src/monitor.ts b/extensions/bluebubbles/src/monitor.ts index 40bbad1419f..0c565d44f8f 100644 --- a/extensions/bluebubbles/src/monitor.ts +++ b/extensions/bluebubbles/src/monitor.ts @@ -398,23 +398,31 @@ export async function handleBlueBubblesWebhookRequest( return true; } - const matching = targets.filter((target) => { - const token = target.account.config.password?.trim(); + const guidParam = url.searchParams.get("guid") ?? url.searchParams.get("password"); + const headerToken = + req.headers["x-guid"] ?? + req.headers["x-password"] ?? + req.headers["x-bluebubbles-guid"] ?? + req.headers["authorization"]; + const guid = (Array.isArray(headerToken) ? headerToken[0] : headerToken) ?? guidParam ?? ""; + + const strictMatches: WebhookTarget[] = []; + const fallbackTargets: WebhookTarget[] = []; + for (const target of targets) { + const token = target.account.config.password?.trim() ?? ""; if (!token) { - return true; + fallbackTargets.push(target); + continue; } - const guidParam = url.searchParams.get("guid") ?? url.searchParams.get("password"); - const headerToken = - req.headers["x-guid"] ?? - req.headers["x-password"] ?? - req.headers["x-bluebubbles-guid"] ?? - req.headers["authorization"]; - const guid = (Array.isArray(headerToken) ? headerToken[0] : headerToken) ?? guidParam ?? ""; if (guid && guid.trim() === token) { - return true; + strictMatches.push(target); + if (strictMatches.length > 1) { + break; + } } - return false; - }); + } + + const matching = strictMatches.length > 0 ? strictMatches : fallbackTargets; if (matching.length === 0) { res.statusCode = 401; @@ -425,24 +433,30 @@ export async function handleBlueBubblesWebhookRequest( return true; } - for (const target of matching) { - target.statusSink?.({ lastInboundAt: Date.now() }); - if (reaction) { - processReaction(reaction, target).catch((err) => { - target.runtime.error?.( - `[${target.account.accountId}] BlueBubbles reaction failed: ${String(err)}`, - ); - }); - } else if (message) { - // Route messages through debouncer to coalesce rapid-fire events - // (e.g., text message + URL balloon arriving as separate webhooks) - const debouncer = getOrCreateDebouncer(target); - debouncer.enqueue({ message, target }).catch((err) => { - target.runtime.error?.( - `[${target.account.accountId}] BlueBubbles webhook failed: ${String(err)}`, - ); - }); - } + if (matching.length > 1) { + res.statusCode = 401; + res.end("ambiguous webhook target"); + console.warn(`[bluebubbles] webhook rejected: ambiguous target match path=${path}`); + return true; + } + + const target = matching[0]; + target.statusSink?.({ lastInboundAt: Date.now() }); + if (reaction) { + processReaction(reaction, target).catch((err) => { + target.runtime.error?.( + `[${target.account.accountId}] BlueBubbles reaction failed: ${String(err)}`, + ); + }); + } else if (message) { + // Route messages through debouncer to coalesce rapid-fire events + // (e.g., text message + URL balloon arriving as separate webhooks) + const debouncer = getOrCreateDebouncer(target); + debouncer.enqueue({ message, target }).catch((err) => { + target.runtime.error?.( + `[${target.account.accountId}] BlueBubbles webhook failed: ${String(err)}`, + ); + }); } res.statusCode = 200; diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index 171033b75e3..2c41d8262ca 100644 --- a/extensions/zalo/src/monitor.ts +++ b/extensions/zalo/src/monitor.ts @@ -143,12 +143,18 @@ export async function handleZaloWebhookRequest( } const headerToken = String(req.headers["x-bot-api-secret-token"] ?? ""); - const target = targets.find((entry) => entry.secret === headerToken); - if (!target) { + const matching = targets.filter((entry) => entry.secret === headerToken); + if (matching.length === 0) { res.statusCode = 401; res.end("unauthorized"); return true; } + if (matching.length > 1) { + res.statusCode = 401; + res.end("ambiguous webhook target"); + return true; + } + const target = matching[0]; const body = await readJsonBodyWithLimit(req, { maxBytes: 1024 * 1024, diff --git a/extensions/zalo/src/monitor.webhook.test.ts b/extensions/zalo/src/monitor.webhook.test.ts index 60d042e2e84..8f864b5b5af 100644 --- a/extensions/zalo/src/monitor.webhook.test.ts +++ b/extensions/zalo/src/monitor.webhook.test.ts @@ -1,7 +1,7 @@ import type { AddressInfo } from "node:net"; import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk"; import { createServer } from "node:http"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { ResolvedZaloAccount } from "./types.js"; import { handleZaloWebhookRequest, registerZaloWebhookTarget } from "./monitor.js"; @@ -70,4 +70,68 @@ describe("handleZaloWebhookRequest", () => { unregister(); } }); + + it("rejects ambiguous routing when multiple targets match the same secret", async () => { + const core = {} as PluginRuntime; + const account: ResolvedZaloAccount = { + accountId: "default", + enabled: true, + token: "tok", + tokenSource: "config", + config: {}, + }; + const sinkA = vi.fn(); + const sinkB = vi.fn(); + const unregisterA = registerZaloWebhookTarget({ + token: "tok", + account, + config: {} as OpenClawConfig, + runtime: {}, + core, + secret: "secret", + path: "/hook", + mediaMaxMb: 5, + statusSink: sinkA, + }); + const unregisterB = registerZaloWebhookTarget({ + token: "tok", + account, + config: {} as OpenClawConfig, + runtime: {}, + core, + secret: "secret", + path: "/hook", + mediaMaxMb: 5, + statusSink: sinkB, + }); + + try { + await withServer( + async (req, res) => { + const handled = await handleZaloWebhookRequest(req, res); + if (!handled) { + res.statusCode = 404; + res.end("not found"); + } + }, + async (baseUrl) => { + const response = await fetch(`${baseUrl}/hook`, { + method: "POST", + headers: { + "x-bot-api-secret-token": "secret", + "content-type": "application/json", + }, + body: "{}", + }); + + expect(response.status).toBe(401); + expect(sinkA).not.toHaveBeenCalled(); + expect(sinkB).not.toHaveBeenCalled(); + }, + ); + } finally { + unregisterA(); + unregisterB(); + } + }); });