fix(security): reject ambiguous webhook target matches

This commit is contained in:
Peter Steinberger
2026-02-14 17:28:18 +01:00
parent b908388245
commit 188c4cd076
5 changed files with 228 additions and 34 deletions

View File

@@ -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

View File

@@ -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 = {};

View File

@@ -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;

View File

@@ -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,

View File

@@ -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();
}
});
});