fix(googlechat): reject ambiguous webhook routing

This commit is contained in:
Peter Steinberger
2026-02-14 17:11:39 +01:00
parent 9dce3d8bf8
commit 61d59a8028
3 changed files with 176 additions and 4 deletions

View File

@@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai
- macOS: hard-limit unkeyed `openclaw://agent` deep links and ignore `deliver` / `to` / `channel` unless a valid unattended key is provided. Thanks @Cillian-Collins.
- Plugins: suppress false duplicate plugin id warnings when the same extension is discovered via multiple paths (config/workspace/global vs bundled), while still warning on genuine duplicates. (#16222) Thanks @shadril238.
- Security/Google Chat: deprecate `users/<email>` allowlists (treat `users/...` as immutable user id only); keep raw email allowlists for usability. Thanks @vincentkoc.
- Security/Google Chat: reject ambiguous shared-path webhook routing when multiple webhook targets verify successfully (prevents cross-account policy-context misrouting). Thanks @vincentkoc.
- Security/Browser: block cross-origin mutating requests to loopback browser control routes (CSRF hardening). Thanks @vincentkoc.
- Security/Nostr: require loopback source and block cross-origin profile mutation/import attempts. Thanks @vincentkoc.
- Security/Archive: enforce archive extraction entry/size limits to prevent resource exhaustion from high-expansion ZIP/TAR archives. Thanks @vincentkoc.

View File

@@ -248,7 +248,7 @@ export async function handleGoogleChatWebhookRequest(
? authHeaderNow.slice("bearer ".length)
: bearer;
let selected: WebhookTarget | undefined;
const matchedTargets: WebhookTarget[] = [];
for (const target of targets) {
const audienceType = target.audienceType;
const audience = target.audience;
@@ -258,17 +258,26 @@ export async function handleGoogleChatWebhookRequest(
audience,
});
if (verification.ok) {
selected = target;
break;
matchedTargets.push(target);
if (matchedTargets.length > 1) {
break;
}
}
}
if (!selected) {
if (matchedTargets.length === 0) {
res.statusCode = 401;
res.end("unauthorized");
return true;
}
if (matchedTargets.length > 1) {
res.statusCode = 401;
res.end("ambiguous webhook target");
return true;
}
const selected = matchedTargets[0];
selected.statusSink?.({ lastInboundAt: Date.now() });
processGoogleChatEvent(event, selected).catch((err) => {
selected?.runtime.error?.(

View File

@@ -0,0 +1,162 @@
import type { IncomingMessage, ServerResponse } from "node:http";
import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk";
import { EventEmitter } from "node:events";
import { describe, expect, it, vi } from "vitest";
import type { ResolvedGoogleChatAccount } from "./accounts.js";
import { verifyGoogleChatRequest } from "./auth.js";
import { handleGoogleChatWebhookRequest, registerGoogleChatWebhookTarget } from "./monitor.js";
vi.mock("./auth.js", () => ({
verifyGoogleChatRequest: vi.fn(),
}));
function createWebhookRequest(params: {
authorization?: string;
payload: unknown;
path?: string;
}): IncomingMessage {
const req = new EventEmitter() as IncomingMessage & { destroyed?: boolean; destroy: () => void };
req.method = "POST";
req.url = params.path ?? "/googlechat";
req.headers = {
authorization: params.authorization ?? "",
"content-type": "application/json",
};
req.destroyed = false;
req.destroy = () => {
req.destroyed = true;
};
void Promise.resolve().then(() => {
req.emit("data", Buffer.from(JSON.stringify(params.payload), "utf-8"));
if (!req.destroyed) {
req.emit("end");
}
});
return req;
}
function createWebhookResponse(): ServerResponse & { body?: string } {
const headers: Record<string, string> = {};
const res = {
headersSent: false,
statusCode: 200,
setHeader: (key: string, value: string) => {
headers[key.toLowerCase()] = value;
return res;
},
end: (body?: string) => {
res.headersSent = true;
res.body = body;
return res;
},
} as unknown as ServerResponse & { body?: string };
return res;
}
const baseAccount = (accountId: string) =>
({
accountId,
enabled: true,
credentialSource: "none",
config: {},
}) as ResolvedGoogleChatAccount;
describe("Google Chat webhook routing", () => {
it("rejects ambiguous routing when multiple targets on the same path verify successfully", async () => {
vi.mocked(verifyGoogleChatRequest).mockResolvedValue({ ok: true });
const sinkA = vi.fn();
const sinkB = vi.fn();
const core = {} as PluginRuntime;
const config = {} as OpenClawConfig;
const unregisterA = registerGoogleChatWebhookTarget({
account: baseAccount("A"),
config,
runtime: {},
core,
path: "/googlechat",
statusSink: sinkA,
mediaMaxMb: 5,
});
const unregisterB = registerGoogleChatWebhookTarget({
account: baseAccount("B"),
config,
runtime: {},
core,
path: "/googlechat",
statusSink: sinkB,
mediaMaxMb: 5,
});
try {
const res = createWebhookResponse();
const handled = await handleGoogleChatWebhookRequest(
createWebhookRequest({
authorization: "Bearer test-token",
payload: { type: "ADDED_TO_SPACE", space: { name: "spaces/AAA" } },
}),
res,
);
expect(handled).toBe(true);
expect(res.statusCode).toBe(401);
expect(sinkA).not.toHaveBeenCalled();
expect(sinkB).not.toHaveBeenCalled();
} finally {
unregisterA();
unregisterB();
}
});
it("routes to the single verified target when earlier targets fail verification", async () => {
vi.mocked(verifyGoogleChatRequest)
.mockResolvedValueOnce({ ok: false, reason: "invalid" })
.mockResolvedValueOnce({ ok: true });
const sinkA = vi.fn();
const sinkB = vi.fn();
const core = {} as PluginRuntime;
const config = {} as OpenClawConfig;
const unregisterA = registerGoogleChatWebhookTarget({
account: baseAccount("A"),
config,
runtime: {},
core,
path: "/googlechat",
statusSink: sinkA,
mediaMaxMb: 5,
});
const unregisterB = registerGoogleChatWebhookTarget({
account: baseAccount("B"),
config,
runtime: {},
core,
path: "/googlechat",
statusSink: sinkB,
mediaMaxMb: 5,
});
try {
const res = createWebhookResponse();
const handled = await handleGoogleChatWebhookRequest(
createWebhookRequest({
authorization: "Bearer test-token",
payload: { type: "ADDED_TO_SPACE", space: { name: "spaces/BBB" } },
}),
res,
);
expect(handled).toBe(true);
expect(res.statusCode).toBe(200);
expect(sinkA).not.toHaveBeenCalled();
expect(sinkB).toHaveBeenCalledTimes(1);
} finally {
unregisterA();
unregisterB();
}
});
});