diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b05fec4ff7..98b77975d4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai - Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146) - Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts. - Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`. +- Inbound policy hardening: tighten callback and webhook sender checks across Mattermost and Google Chat, match Nextcloud Talk rooms by stable room token, and treat explicit empty Twitch allowlists as deny-all. Thanks @vincentkoc. - macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. Thanks @vincentkoc. - Agents/compaction: extend the enclosing run deadline once while compaction is actively in flight, and abort the underlying SDK compaction on timeout/cancel so large-session compactions stop freezing mid-run. (#46889) Thanks @asyncjason. - Models/openai-completions: default non-native OpenAI-compatible providers to omit tool-definition `strict` fields unless users explicitly opt back in, so tool calling keeps working on providers that reject that option. (#45497) Thanks @sahancava. @@ -36,9 +37,6 @@ Docs: https://docs.openclaw.ai - WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason. - Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026. - Security/device pairing: harden `device.token.rotate` deny handling by keeping public failures generic while logging internal deny reasons and preserving approved-baseline enforcement. (`GHSA-7jrw-x62h-64p8`) - -### Fixes - - Slack/interactive replies: preserve `channelData.slack.blocks` through live DM delivery and preview-finalized edits so Block Kit button and select directives render instead of falling back to raw text. Thanks @vincentkoc. - Zalo/plugin runtime: export `resolveClientIp` from `openclaw/plugin-sdk/zalo` so installed builds no longer crash on startup when the webhook monitor loads from the packaged extension instead of the monorepo source tree. (#46549) Thanks @No898. - CI/channel test routing: move the built-in channel suites into `test:channels` and keep them out of `test:extensions`, so extension CI no longer fails after the channel migration while targeted test routing still sends Slack, Signal, and iMessage suites to the right lane. (#46066) Thanks @scoootscooob. diff --git a/extensions/googlechat/src/auth.test.ts b/extensions/googlechat/src/auth.test.ts new file mode 100644 index 00000000000..9fa39e51c65 --- /dev/null +++ b/extensions/googlechat/src/auth.test.ts @@ -0,0 +1,97 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + verifyIdToken: vi.fn(), +})); + +vi.mock("google-auth-library", () => ({ + GoogleAuth: class {}, + OAuth2Client: class { + verifyIdToken = mocks.verifyIdToken; + }, +})); + +const { verifyGoogleChatRequest } = await import("./auth.js"); + +function mockTicket(payload: Record) { + mocks.verifyIdToken.mockResolvedValue({ + getPayload: () => payload, + }); +} + +describe("verifyGoogleChatRequest", () => { + beforeEach(() => { + mocks.verifyIdToken.mockReset(); + }); + + it("accepts Google Chat app-url tokens from the Chat issuer", async () => { + mockTicket({ + email: "chat@system.gserviceaccount.com", + email_verified: true, + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + }), + ).resolves.toEqual({ ok: true }); + }); + + it("rejects add-on tokens when no principal binding is configured", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-1", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + }), + ).resolves.toEqual({ + ok: false, + reason: "missing add-on principal binding", + }); + }); + + it("accepts add-on tokens only when the bound principal matches", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-1", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + expectedAddOnPrincipal: "principal-1", + }), + ).resolves.toEqual({ ok: true }); + }); + + it("rejects add-on tokens when the bound principal does not match", async () => { + mockTicket({ + email: "service-123@gcp-sa-gsuiteaddons.iam.gserviceaccount.com", + email_verified: true, + sub: "principal-2", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + expectedAddOnPrincipal: "principal-1", + }), + ).resolves.toEqual({ + ok: false, + reason: "unexpected add-on principal: principal-2", + }); + }); +}); diff --git a/extensions/googlechat/src/auth.ts b/extensions/googlechat/src/auth.ts index 6870ea8ec0f..dd20d1267f7 100644 --- a/extensions/googlechat/src/auth.ts +++ b/extensions/googlechat/src/auth.ts @@ -94,6 +94,7 @@ export async function verifyGoogleChatRequest(params: { bearer?: string | null; audienceType?: GoogleChatAudienceType | null; audience?: string | null; + expectedAddOnPrincipal?: string | null; }): Promise<{ ok: boolean; reason?: string }> { const bearer = params.bearer?.trim(); if (!bearer) { @@ -112,10 +113,32 @@ export async function verifyGoogleChatRequest(params: { audience, }); const payload = ticket.getPayload(); - const email = payload?.email ?? ""; - const ok = - payload?.email_verified && (email === CHAT_ISSUER || ADDON_ISSUER_PATTERN.test(email)); - return ok ? { ok: true } : { ok: false, reason: `invalid issuer: ${email}` }; + const email = String(payload?.email ?? "") + .trim() + .toLowerCase(); + if (!payload?.email_verified) { + return { ok: false, reason: "email not verified" }; + } + if (email === CHAT_ISSUER) { + return { ok: true }; + } + if (!ADDON_ISSUER_PATTERN.test(email)) { + return { ok: false, reason: `invalid issuer: ${email}` }; + } + const expectedAddOnPrincipal = params.expectedAddOnPrincipal?.trim().toLowerCase(); + if (!expectedAddOnPrincipal) { + return { ok: false, reason: "missing add-on principal binding" }; + } + const tokenPrincipal = String(payload?.sub ?? "") + .trim() + .toLowerCase(); + if (!tokenPrincipal || tokenPrincipal !== expectedAddOnPrincipal) { + return { + ok: false, + reason: `unexpected add-on principal: ${tokenPrincipal || ""}`, + }; + } + return { ok: true }; } catch (err) { return { ok: false, reason: err instanceof Error ? err.message : "invalid token" }; } diff --git a/extensions/googlechat/src/monitor-webhook.ts b/extensions/googlechat/src/monitor-webhook.ts index cde54214575..56f355cce83 100644 --- a/extensions/googlechat/src/monitor-webhook.ts +++ b/extensions/googlechat/src/monitor-webhook.ts @@ -132,6 +132,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: headerBearer, audienceType: target.audienceType, audience: target.audience, + expectedAddOnPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, @@ -166,6 +167,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: parsed.addOnBearerToken, audienceType: target.audienceType, audience: target.audience, + expectedAddOnPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, diff --git a/extensions/mattermost/src/mattermost/interactions.test.ts b/extensions/mattermost/src/mattermost/interactions.test.ts index 62c7bdb757f..dea16d51e57 100644 --- a/extensions/mattermost/src/mattermost/interactions.test.ts +++ b/extensions/mattermost/src/mattermost/interactions.test.ts @@ -738,6 +738,37 @@ describe("createMattermostInteractionHandler", () => { expectSuccessfulApprovalUpdate(res, requestLog); }); + it("blocks button dispatch when the sender is not allowed for the action", async () => { + const { context, token } = createActionContext(); + const dispatchButtonClick = vi.fn(); + const handleInteraction = vi.fn(); + const handler = createMattermostInteractionHandler({ + client: { + request: async (_path: string, init?: { method?: string }) => + init?.method === "PUT" ? { id: "post-1" } : createActionPost(), + } as unknown as MattermostClient, + botUserId: "bot", + accountId: "acct", + authorizeButtonClick: async () => ({ + ok: false, + response: { + ephemeral_text: "blocked", + }, + }), + handleInteraction, + dispatchButtonClick, + }); + + const res = await runHandler(handler, { + body: createInteractionBody({ context, token }), + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toContain("blocked"); + expect(handleInteraction).not.toHaveBeenCalled(); + expect(dispatchButtonClick).not.toHaveBeenCalled(); + }); + it("forwards fetched post threading metadata to session and button callbacks", async () => { const enqueueSystemEvent = vi.fn(); setMattermostRuntime({ diff --git a/extensions/mattermost/src/mattermost/interactions.ts b/extensions/mattermost/src/mattermost/interactions.ts index f99d0b5d3ac..f4ef06cf1ed 100644 --- a/extensions/mattermost/src/mattermost/interactions.ts +++ b/extensions/mattermost/src/mattermost/interactions.ts @@ -37,6 +37,10 @@ export type MattermostInteractionResponse = { ephemeral_text?: string; }; +export type MattermostInteractionAuthorizationResult = + | { ok: true } + | { ok: false; statusCode?: number; response?: MattermostInteractionResponse }; + export type MattermostInteractiveButtonInput = { id?: string; callback_data?: string; @@ -404,6 +408,10 @@ export function createMattermostInteractionHandler(params: { context: Record; post: MattermostPost; }) => Promise; + authorizeButtonClick?: (opts: { + payload: MattermostInteractionPayload; + post: MattermostPost; + }) => Promise; dispatchButtonClick?: (opts: { channelId: string; userId: string; @@ -566,6 +574,33 @@ export function createMattermostInteractionHandler(params: { `post=${payload.post_id} channel=${payload.channel_id}`, ); + if (params.authorizeButtonClick) { + try { + const authorization = await params.authorizeButtonClick({ + payload, + post: originalPost, + }); + if (!authorization.ok) { + res.statusCode = authorization.statusCode ?? 200; + res.setHeader("Content-Type", "application/json"); + res.end( + JSON.stringify( + authorization.response ?? { + ephemeral_text: "You are not allowed to use this action here.", + }, + ), + ); + return; + } + } catch (err) { + log?.(`mattermost interaction: authorization failed: ${String(err)}`); + res.statusCode = 500; + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ error: "Interaction authorization failed" })); + return; + } + } + if (params.handleInteraction) { try { const response = await params.handleInteraction({ diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 16e3bd6434a..e56e4a9b9af 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -567,6 +567,45 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} trustedProxies: cfg.gateway?.trustedProxies, allowRealIpFallback: cfg.gateway?.allowRealIpFallback === true, handleInteraction: handleModelPickerInteraction, + authorizeButtonClick: async ({ payload, post }) => { + const channelInfo = await resolveChannelInfo(payload.channel_id); + const isDirect = channelInfo?.type?.trim().toUpperCase() === "D"; + const allowTextCommands = core.channel.commands.shouldHandleTextCommands({ + cfg, + surface: "mattermost", + }); + const decision = authorizeMattermostCommandInvocation({ + account, + cfg, + senderId: payload.user_id, + senderName: payload.user_name ?? "", + channelId: payload.channel_id, + channelInfo, + storeAllowFrom: isDirect + ? await readStoreAllowFromForDmPolicy({ + provider: "mattermost", + accountId: account.accountId, + dmPolicy: account.config.dmPolicy ?? "pairing", + readStore: pairing.readStoreForDmPolicy, + }) + : undefined, + allowTextCommands, + hasControlCommand: false, + }); + if (decision.ok) { + return { ok: true }; + } + return { + ok: false, + response: { + update: { + message: post.message ?? "", + props: post.props as Record | undefined, + }, + ephemeral_text: `OpenClaw ignored this action for ${decision.roomLabel}.`, + }, + }; + }, resolveSessionKey: async ({ channelId, userId, post }) => { const channelInfo = await resolveChannelInfo(channelId); const kind = mapMattermostChannelTypeToChatType(channelInfo?.type); diff --git a/extensions/nextcloud-talk/src/inbound.authz.test.ts b/extensions/nextcloud-talk/src/inbound.authz.test.ts index f19fa73e020..bde32abdb3c 100644 --- a/extensions/nextcloud-talk/src/inbound.authz.test.ts +++ b/extensions/nextcloud-talk/src/inbound.authz.test.ts @@ -81,4 +81,77 @@ describe("nextcloud-talk inbound authz", () => { }); expect(buildMentionRegexes).not.toHaveBeenCalled(); }); + + it("matches group rooms by token instead of colliding room names", async () => { + const readAllowFromStore = vi.fn(async () => []); + const buildMentionRegexes = vi.fn(() => [/@openclaw/i]); + + setNextcloudTalkRuntime({ + channel: { + pairing: { + readAllowFromStore, + }, + commands: { + shouldHandleTextCommands: () => false, + }, + text: { + hasControlCommand: () => false, + }, + mentions: { + buildMentionRegexes, + matchesMentionPatterns: () => false, + }, + }, + } as unknown as PluginRuntime); + + const message: NextcloudTalkInboundMessage = { + messageId: "m-2", + roomToken: "room-attacker", + roomName: "Room Trusted", + senderId: "trusted-user", + senderName: "Trusted User", + text: "hello", + mediaType: "text/plain", + timestamp: Date.now(), + isGroupChat: true, + }; + + const account: ResolvedNextcloudTalkAccount = { + accountId: "default", + enabled: true, + baseUrl: "", + secret: "", + secretSource: "none", + config: { + dmPolicy: "pairing", + allowFrom: [], + groupPolicy: "allowlist", + groupAllowFrom: ["trusted-user"], + rooms: { + "room-trusted": { + enabled: true, + }, + }, + }, + }; + + await handleNextcloudTalkInbound({ + message, + account, + config: { + channels: { + "nextcloud-talk": { + groupPolicy: "allowlist", + groupAllowFrom: ["trusted-user"], + }, + }, + }, + runtime: { + log: vi.fn(), + error: vi.fn(), + } as unknown as RuntimeEnv, + }); + + expect(buildMentionRegexes).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/nextcloud-talk/src/inbound.ts b/extensions/nextcloud-talk/src/inbound.ts index 081029782f8..10ecd924fd7 100644 --- a/extensions/nextcloud-talk/src/inbound.ts +++ b/extensions/nextcloud-talk/src/inbound.ts @@ -114,7 +114,6 @@ export async function handleNextcloudTalkInbound(params: { const roomMatch = resolveNextcloudTalkRoomMatch({ rooms: account.config.rooms, roomToken, - roomName, }); const roomConfig = roomMatch.roomConfig; if (isGroup && !roomMatch.allowed) { diff --git a/extensions/nextcloud-talk/src/policy.ts b/extensions/nextcloud-talk/src/policy.ts index 1157384b578..15e19da84de 100644 --- a/extensions/nextcloud-talk/src/policy.ts +++ b/extensions/nextcloud-talk/src/policy.ts @@ -57,16 +57,10 @@ export type NextcloudTalkRoomMatch = { export function resolveNextcloudTalkRoomMatch(params: { rooms?: Record; roomToken: string; - roomName?: string | null; }): NextcloudTalkRoomMatch { const rooms = params.rooms ?? {}; const allowlistConfigured = Object.keys(rooms).length > 0; - const roomName = params.roomName?.trim() || undefined; - const roomCandidates = buildChannelKeyCandidates( - params.roomToken, - roomName, - roomName ? normalizeChannelSlug(roomName) : undefined, - ); + const roomCandidates = buildChannelKeyCandidates(params.roomToken); const match = resolveChannelEntryMatchWithFallback({ entries: rooms, keys: roomCandidates, @@ -101,11 +95,9 @@ export function resolveNextcloudTalkGroupToolPolicy( if (!roomToken) { return undefined; } - const roomName = params.groupChannel?.trim() || undefined; const match = resolveNextcloudTalkRoomMatch({ rooms: cfg.channels?.["nextcloud-talk"]?.rooms, roomToken, - roomName, }); return match.roomConfig?.tools ?? match.wildcardConfig?.tools; } diff --git a/extensions/twitch/src/access-control.test.ts b/extensions/twitch/src/access-control.test.ts index 3d522246700..597ef897f90 100644 --- a/extensions/twitch/src/access-control.test.ts +++ b/extensions/twitch/src/access-control.test.ts @@ -160,6 +160,13 @@ describe("checkTwitchAccessControl", () => { }); }); + it("blocks everyone when allowFrom is explicitly empty", () => { + expectAllowFromBlocked({ + allowFrom: [], + reason: "allowFrom", + }); + }); + it("blocks messages without userId", () => { expectAllowFromBlocked({ allowFrom: ["123456"], diff --git a/extensions/twitch/src/access-control.ts b/extensions/twitch/src/access-control.ts index 5555096d27d..1c4a043d42b 100644 --- a/extensions/twitch/src/access-control.ts +++ b/extensions/twitch/src/access-control.ts @@ -48,8 +48,14 @@ export function checkTwitchAccessControl(params: { } } - if (account.allowFrom && account.allowFrom.length > 0) { + if (account.allowFrom !== undefined) { const allowFrom = account.allowFrom; + if (allowFrom.length === 0) { + return { + allowed: false, + reason: "sender is not in allowFrom allowlist", + }; + } const senderId = message.userId; if (!senderId) { diff --git a/src/config/types.googlechat.ts b/src/config/types.googlechat.ts index fdfc23fd866..1951e51db10 100644 --- a/src/config/types.googlechat.ts +++ b/src/config/types.googlechat.ts @@ -75,6 +75,8 @@ export type GoogleChatAccountConfig = { audienceType?: "app-url" | "project-number"; /** Audience value (app URL or project number). */ audience?: string; + /** Exact add-on principal to accept when app-url delivery uses add-on tokens. */ + appPrincipal?: string; /** Google Chat webhook path (default: /googlechat). */ webhookPath?: string; /** Google Chat webhook URL (used to derive the path). */ diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index e6e4a3aacd2..5f7dd7b8e48 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -767,6 +767,7 @@ export const GoogleChatAccountSchema = z serviceAccountFile: z.string().optional(), audienceType: z.enum(["app-url", "project-number"]).optional(), audience: z.string().optional(), + appPrincipal: z.string().optional(), webhookPath: z.string().optional(), webhookUrl: z.string().optional(), botUser: z.string().optional(),