From 61d59a802869177d9cef52204767cd83357ab79e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 17:11:39 +0100 Subject: [PATCH] fix(googlechat): reject ambiguous webhook routing --- CHANGELOG.md | 1 + extensions/googlechat/src/monitor.ts | 17 +- .../src/monitor.webhook-routing.test.ts | 162 ++++++++++++++++++ 3 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 extensions/googlechat/src/monitor.webhook-routing.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d81831229..b2e58ecc940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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/` 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. diff --git a/extensions/googlechat/src/monitor.ts b/extensions/googlechat/src/monitor.ts index 9482cd0bce0..34f62930ae7 100644 --- a/extensions/googlechat/src/monitor.ts +++ b/extensions/googlechat/src/monitor.ts @@ -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?.( diff --git a/extensions/googlechat/src/monitor.webhook-routing.test.ts b/extensions/googlechat/src/monitor.webhook-routing.test.ts new file mode 100644 index 00000000000..eec7293d74e --- /dev/null +++ b/extensions/googlechat/src/monitor.webhook-routing.test.ts @@ -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 = {}; + 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(); + } + }); +});