From 5570e1a946145df3a057cc644cf2c69b30e4cf92 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 24 Jan 2026 23:23:24 +0000 Subject: [PATCH] fix: polish Google Chat plugin (#1635) (thanks @iHildy) Co-authored-by: Ian Hildebrand --- CHANGELOG.md | 1 + docs/channels/googlechat.md | 4 +- docs/gateway/configuration.md | 3 +- extensions/googlechat/package.json | 7 ++- extensions/googlechat/src/api.test.ts | 62 +++++++++++++++++++++++ extensions/googlechat/src/api.ts | 44 ++++++++++++++-- extensions/googlechat/src/channel.ts | 3 +- extensions/googlechat/src/monitor.test.ts | 27 ++++++++++ extensions/googlechat/src/monitor.ts | 42 +++++++++++++-- extensions/googlechat/src/onboarding.ts | 4 +- extensions/googlechat/src/targets.test.ts | 35 +++++++++++++ extensions/googlechat/src/targets.ts | 8 ++- pnpm-lock.yaml | 7 +-- src/commands/channels/resolve.ts | 1 + 14 files changed, 232 insertions(+), 16 deletions(-) create mode 100644 extensions/googlechat/src/api.test.ts create mode 100644 extensions/googlechat/src/monitor.test.ts create mode 100644 extensions/googlechat/src/targets.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cd285db1e..9a919f983 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.clawd.bot - Gateway: reduce log noise for late invokes + remote node probes; debounce skills refresh. (#1607) Thanks @petter-b. - macOS: default direct-transport `ws://` URLs to port 18789; document `gateway.remote.transport`. (#1603) Thanks @ngutman. - Voice Call: return stream TwiML for outbound conversation calls on initial Twilio webhook. (#1634) +- Google Chat: tighten email allowlist matching, typing cleanup, media caps, and onboarding/docs/tests. (#1635) Thanks @iHildy. ## 2026.1.23-1 diff --git a/docs/channels/googlechat.md b/docs/channels/googlechat.md index 19c3916e3..bd745caa2 100644 --- a/docs/channels/googlechat.md +++ b/docs/channels/googlechat.md @@ -134,7 +134,7 @@ Configure your tunnel's ingress rules to only route the webhook path: ## Targets Use these identifiers for delivery and allowlists: -- Direct messages: `users/` (Clawdbot resolves to a DM space automatically). +- Direct messages: `users/` or `users/` (email addresses are accepted). - Spaces: `spaces/`. ## Config highlights @@ -162,6 +162,7 @@ Use these identifiers for delivery and allowlists: } }, actions: { reactions: true }, + typingIndicator: "message", mediaMaxMb: 20 } } @@ -172,6 +173,7 @@ Notes: - Service account credentials can also be passed inline with `serviceAccount` (JSON string). - Default webhook path is `/googlechat` if `webhookPath` isn’t set. - Reactions are available via the `reactions` tool and `channels action` when `actions.reactions` is enabled. +- `typingIndicator` supports `none`, `message` (default), and `reaction` (reaction requires user OAuth). - Attachments are downloaded through the Chat API and stored in the media pipeline (size capped by `mediaMaxMb`). ## Troubleshooting diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 7d23f934e..f4655aeae 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -1145,6 +1145,7 @@ Multi-account support lives under `channels.googlechat.accounts` (see the multi- "spaces/AAAA": { allow: true, requireMention: true } }, actions: { reactions: true }, + typingIndicator: "message", mediaMaxMb: 20 } } @@ -1155,7 +1156,7 @@ Notes: - Service account JSON can be inline (`serviceAccount`) or file-based (`serviceAccountFile`). - Env fallbacks for the default account: `GOOGLE_CHAT_SERVICE_ACCOUNT` or `GOOGLE_CHAT_SERVICE_ACCOUNT_FILE`. - `audienceType` + `audience` must match the Chat app’s webhook auth config. -- Use `spaces/` or `users/` when setting delivery targets. +- Use `spaces/` or `users/` when setting delivery targets. ### `channels.slack` (socket mode) diff --git a/extensions/googlechat/package.json b/extensions/googlechat/package.json index d335c092a..cf73b6795 100644 --- a/extensions/googlechat/package.json +++ b/extensions/googlechat/package.json @@ -28,7 +28,12 @@ } }, "dependencies": { - "clawdbot": "workspace:*", "google-auth-library": "^10.5.0" + }, + "devDependencies": { + "clawdbot": "workspace:*" + }, + "peerDependencies": { + "clawdbot": ">=2026.1.24-0" } } diff --git a/extensions/googlechat/src/api.test.ts b/extensions/googlechat/src/api.test.ts new file mode 100644 index 000000000..959b396df --- /dev/null +++ b/extensions/googlechat/src/api.test.ts @@ -0,0 +1,62 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { ResolvedGoogleChatAccount } from "./accounts.js"; +import { downloadGoogleChatMedia } from "./api.js"; + +vi.mock("./auth.js", () => ({ + getGoogleChatAccessToken: vi.fn().mockResolvedValue("token"), +})); + +const account = { + accountId: "default", + enabled: true, + credentialSource: "inline", + config: {}, +} as ResolvedGoogleChatAccount; + +describe("downloadGoogleChatMedia", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("rejects when content-length exceeds max bytes", async () => { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1, 2, 3])); + controller.close(); + }, + }); + const response = new Response(body, { + status: 200, + headers: { "content-length": "50", "content-type": "application/octet-stream" }, + }); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); + + await expect( + downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), + ).rejects.toThrow(/max bytes/i); + }); + + it("rejects when streamed payload exceeds max bytes", async () => { + const chunks = [new Uint8Array(6), new Uint8Array(6)]; + let index = 0; + const body = new ReadableStream({ + pull(controller) { + if (index < chunks.length) { + controller.enqueue(chunks[index++]); + } else { + controller.close(); + } + }, + }); + const response = new Response(body, { + status: 200, + headers: { "content-type": "application/octet-stream" }, + }); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); + + await expect( + downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), + ).rejects.toThrow(/max bytes/i); + }); +}); diff --git a/extensions/googlechat/src/api.ts b/extensions/googlechat/src/api.ts index 832e27f59..6ecef0a80 100644 --- a/extensions/googlechat/src/api.ts +++ b/extensions/googlechat/src/api.ts @@ -51,6 +51,7 @@ async function fetchBuffer( account: ResolvedGoogleChatAccount, url: string, init?: RequestInit, + options?: { maxBytes?: number }, ): Promise<{ buffer: Buffer; contentType?: string }> { const token = await getGoogleChatAccessToken(account); const res = await fetch(url, { @@ -64,7 +65,34 @@ async function fetchBuffer( const text = await res.text().catch(() => ""); throw new Error(`Google Chat API ${res.status}: ${text || res.statusText}`); } - const buffer = Buffer.from(await res.arrayBuffer()); + const maxBytes = options?.maxBytes; + const lengthHeader = res.headers.get("content-length"); + if (maxBytes && lengthHeader) { + const length = Number(lengthHeader); + if (Number.isFinite(length) && length > maxBytes) { + throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); + } + } + if (!maxBytes || !res.body) { + const buffer = Buffer.from(await res.arrayBuffer()); + const contentType = res.headers.get("content-type") ?? undefined; + return { buffer, contentType }; + } + const reader = res.body.getReader(); + const chunks: Buffer[] = []; + let total = 0; + while (true) { + const { done, value } = await reader.read(); + if (done) break; + if (!value) continue; + total += value.length; + if (total > maxBytes) { + await reader.cancel(); + throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); + } + chunks.push(Buffer.from(value)); + } + const buffer = Buffer.concat(chunks, total); const contentType = res.headers.get("content-type") ?? undefined; return { buffer, contentType }; } @@ -108,6 +136,15 @@ export async function updateGoogleChatMessage(params: { return { messageName: result.name }; } +export async function deleteGoogleChatMessage(params: { + account: ResolvedGoogleChatAccount; + messageName: string; +}): Promise { + const { account, messageName } = params; + const url = `${CHAT_API_BASE}/${messageName}`; + await fetchOk(account, url, { method: "DELETE" }); +} + export async function uploadGoogleChatAttachment(params: { account: ResolvedGoogleChatAccount; space: string; @@ -151,10 +188,11 @@ export async function uploadGoogleChatAttachment(params: { export async function downloadGoogleChatMedia(params: { account: ResolvedGoogleChatAccount; resourceName: string; + maxBytes?: number; }): Promise<{ buffer: Buffer; contentType?: string }> { - const { account, resourceName } = params; + const { account, resourceName, maxBytes } = params; const url = `${CHAT_API_BASE}/media/${resourceName}?alt=media`; - return await fetchBuffer(account, url); + return await fetchBuffer(account, url, undefined, { maxBytes }); } export async function createGoogleChatReaction(params: { diff --git a/extensions/googlechat/src/channel.ts b/extensions/googlechat/src/channel.ts index af04116c4..dc8a27414 100644 --- a/extensions/googlechat/src/channel.ts +++ b/extensions/googlechat/src/channel.ts @@ -42,7 +42,8 @@ const meta = getChatChannelMeta("googlechat"); const formatAllowFromEntry = (entry: string) => entry .trim() - .replace(/^(googlechat|gchat):/i, "") + .replace(/^(googlechat|google-chat|gchat):/i, "") + .replace(/^user:/i, "") .replace(/^users\//i, "") .toLowerCase(); diff --git a/extensions/googlechat/src/monitor.test.ts b/extensions/googlechat/src/monitor.test.ts new file mode 100644 index 000000000..5604671ad --- /dev/null +++ b/extensions/googlechat/src/monitor.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; + +import { isSenderAllowed } from "./monitor.js"; + +describe("isSenderAllowed", () => { + it("matches allowlist entries with users/", () => { + expect( + isSenderAllowed("users/123", "Jane@Example.com", ["users/jane@example.com"]), + ).toBe(true); + }); + + it("matches allowlist entries with raw email", () => { + expect(isSenderAllowed("users/123", "Jane@Example.com", ["jane@example.com"])).toBe( + true, + ); + }); + + it("still matches user id entries", () => { + expect(isSenderAllowed("users/abc", "jane@example.com", ["users/abc"])).toBe(true); + }); + + it("rejects non-matching emails", () => { + expect(isSenderAllowed("users/123", "jane@example.com", ["users/other@example.com"])).toBe( + false, + ); + }); +}); diff --git a/extensions/googlechat/src/monitor.ts b/extensions/googlechat/src/monitor.ts index 5ca29cf50..b94aa2e89 100644 --- a/extensions/googlechat/src/monitor.ts +++ b/extensions/googlechat/src/monitor.ts @@ -8,6 +8,7 @@ import { } from "./accounts.js"; import { downloadGoogleChatMedia, + deleteGoogleChatMessage, sendGoogleChatMessage, updateGoogleChatMessage, } from "./api.js"; @@ -296,7 +297,11 @@ function normalizeUserId(raw?: string | null): string { return trimmed.replace(/^users\//i, "").toLowerCase(); } -function isSenderAllowed(senderId: string, senderEmail: string | undefined, allowFrom: string[]) { +export function isSenderAllowed( + senderId: string, + senderEmail: string | undefined, + allowFrom: string[], +) { if (allowFrom.includes("*")) return true; const normalizedSenderId = normalizeUserId(senderId); const normalizedEmail = senderEmail?.trim().toLowerCase() ?? ""; @@ -305,8 +310,11 @@ function isSenderAllowed(senderId: string, senderEmail: string | undefined, allo if (!normalized) return false; if (normalized === normalizedSenderId) return true; if (normalizedEmail && normalized === normalizedEmail) return true; + if (normalizedEmail && normalized.replace(/^users\//i, "") === normalizedEmail) return true; if (normalized.replace(/^users\//i, "") === normalizedSenderId) return true; - if (normalized.replace(/^(googlechat|gchat):/i, "") === normalizedSenderId) return true; + if (normalized.replace(/^(googlechat|google-chat|gchat):/i, "") === normalizedSenderId) { + return true; + } return false; }); } @@ -700,7 +708,7 @@ async function downloadAttachment( const resourceName = attachment.attachmentDataRef?.resourceName; if (!resourceName) return null; const maxBytes = Math.max(1, mediaMaxMb) * 1024 * 1024; - const downloaded = await downloadGoogleChatMedia({ account, resourceName }); + const downloaded = await downloadGoogleChatMedia({ account, resourceName, maxBytes }); const saved = await core.channel.media.saveMediaBuffer( downloaded.buffer, downloaded.contentType ?? attachment.contentType, @@ -728,9 +736,35 @@ async function deliverGoogleChatReply(params: { : []; if (mediaList.length > 0) { + let suppressCaption = false; + if (typingMessageName) { + try { + await deleteGoogleChatMessage({ + account, + messageName: typingMessageName, + }); + } catch (err) { + runtime.error?.(`Google Chat typing cleanup failed: ${String(err)}`); + const fallbackText = payload.text?.trim() + ? payload.text + : mediaList.length > 1 + ? "Sent attachments." + : "Sent attachment."; + try { + await updateGoogleChatMessage({ + account, + messageName: typingMessageName, + text: fallbackText, + }); + suppressCaption = Boolean(payload.text?.trim()); + } catch (updateErr) { + runtime.error?.(`Google Chat typing update failed: ${String(updateErr)}`); + } + } + } let first = true; for (const mediaUrl of mediaList) { - const caption = first ? payload.text : undefined; + const caption = first && !suppressCaption ? payload.text : undefined; first = false; try { const loaded = await core.channel.media.fetchRemoteMedia(mediaUrl, { diff --git a/extensions/googlechat/src/onboarding.ts b/extensions/googlechat/src/onboarding.ts index 56ac8c62d..1b1a02371 100644 --- a/extensions/googlechat/src/onboarding.ts +++ b/extensions/googlechat/src/onboarding.ts @@ -136,7 +136,9 @@ async function promptCredentials(params: { }): Promise { const { cfg, prompter, accountId } = params; const envReady = - Boolean(process.env[ENV_SERVICE_ACCOUNT]) || Boolean(process.env[ENV_SERVICE_ACCOUNT_FILE]); + accountId === DEFAULT_ACCOUNT_ID && + (Boolean(process.env[ENV_SERVICE_ACCOUNT]) || + Boolean(process.env[ENV_SERVICE_ACCOUNT_FILE])); if (envReady) { const useEnv = await prompter.confirm({ message: "Use GOOGLE_CHAT_SERVICE_ACCOUNT env vars?", diff --git a/extensions/googlechat/src/targets.test.ts b/extensions/googlechat/src/targets.test.ts new file mode 100644 index 000000000..de9d96a0e --- /dev/null +++ b/extensions/googlechat/src/targets.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; + +import { + isGoogleChatSpaceTarget, + isGoogleChatUserTarget, + normalizeGoogleChatTarget, +} from "./targets.js"; + +describe("normalizeGoogleChatTarget", () => { + it("normalizes provider prefixes", () => { + expect(normalizeGoogleChatTarget("googlechat:users/123")).toBe("users/123"); + expect(normalizeGoogleChatTarget("google-chat:spaces/AAA")).toBe("spaces/AAA"); + expect(normalizeGoogleChatTarget("gchat:user:User@Example.com")).toBe( + "users/user@example.com", + ); + }); + + it("normalizes email targets to users/", () => { + expect(normalizeGoogleChatTarget("User@Example.com")).toBe("users/user@example.com"); + expect(normalizeGoogleChatTarget("users/User@Example.com")).toBe("users/user@example.com"); + }); + + it("preserves space targets", () => { + expect(normalizeGoogleChatTarget("space:spaces/BBB")).toBe("spaces/BBB"); + expect(normalizeGoogleChatTarget("spaces/CCC")).toBe("spaces/CCC"); + }); +}); + +describe("target helpers", () => { + it("detects user and space targets", () => { + expect(isGoogleChatUserTarget("users/abc")).toBe(true); + expect(isGoogleChatSpaceTarget("spaces/abc")).toBe(true); + expect(isGoogleChatUserTarget("spaces/abc")).toBe(false); + }); +}); diff --git a/extensions/googlechat/src/targets.ts b/extensions/googlechat/src/targets.ts index bff812962..58df49484 100644 --- a/extensions/googlechat/src/targets.ts +++ b/extensions/googlechat/src/targets.ts @@ -4,10 +4,16 @@ import { findGoogleChatDirectMessage } from "./api.js"; export function normalizeGoogleChatTarget(raw?: string | null): string | undefined { const trimmed = raw?.trim(); if (!trimmed) return undefined; - const withoutPrefix = trimmed.replace(/^(googlechat|gchat):/i, ""); + const withoutPrefix = trimmed.replace(/^(googlechat|google-chat|gchat):/i, ""); const normalized = withoutPrefix .replace(/^user:/i, "users/") .replace(/^space:/i, "spaces/"); + if (isGoogleChatUserTarget(normalized)) { + const suffix = normalized.slice("users/".length); + return suffix.includes("@") ? `users/${suffix.toLowerCase()}` : normalized; + } + if (isGoogleChatSpaceTarget(normalized)) return normalized; + if (normalized.includes("@")) return `users/${normalized.toLowerCase()}`; return normalized; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0df307cd0..4ee87da5d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -304,12 +304,13 @@ importers: extensions/googlechat: dependencies: - clawdbot: - specifier: workspace:* - version: link:../.. google-auth-library: specifier: ^10.5.0 version: 10.5.0 + devDependencies: + clawdbot: + specifier: workspace:* + version: link:../.. extensions/imessage: {} diff --git a/src/commands/channels/resolve.ts b/src/commands/channels/resolve.ts index c3022d932..e2944a972 100644 --- a/src/commands/channels/resolve.ts +++ b/src/commands/channels/resolve.ts @@ -35,6 +35,7 @@ function detectAutoKind(input: string): ChannelResolveKind { if (!trimmed) return "group"; if (trimmed.startsWith("@")) return "user"; if (/^<@!?/.test(trimmed)) return "user"; + if (/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmed)) return "user"; if ( /^(user|discord|slack|matrix|msteams|teams|zalo|zalouser|googlechat|google-chat|gchat):/i.test( trimmed,