From 4540790cb62412676f7b61cfc6e47443f84a251e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 20:08:17 +0100 Subject: [PATCH] refactor(bluebubbles): share dm/group access policy checks --- .../bluebubbles/src/monitor-processing.ts | 219 ++++++++---------- src/plugin-sdk/index.ts | 5 + src/security/dm-policy-shared.test.ts | 96 +++++++- src/security/dm-policy-shared.ts | 71 ++++++ 4 files changed, 265 insertions(+), 126 deletions(-) diff --git a/extensions/bluebubbles/src/monitor-processing.ts b/extensions/bluebubbles/src/monitor-processing.ts index 0719c548556..9b61fc9ec58 100644 --- a/extensions/bluebubbles/src/monitor-processing.ts +++ b/extensions/bluebubbles/src/monitor-processing.ts @@ -5,6 +5,8 @@ import { logInboundDrop, logTypingFailure, resolveAckReaction, + resolveDmGroupAccessDecision, + resolveEffectiveAllowFromLists, resolveControlCommandGate, stripMarkdown, } from "openclaw/plugin-sdk"; @@ -323,41 +325,50 @@ export async function processMessage( const dmPolicy = account.config.dmPolicy ?? "pairing"; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - const configAllowFrom = (account.config.allowFrom ?? []).map((entry) => String(entry)); - const configGroupAllowFrom = (account.config.groupAllowFrom ?? []).map((entry) => String(entry)); const storeAllowFrom = await core.channel.pairing .readAllowFromStore("bluebubbles") .catch(() => []); - const effectiveAllowFrom = [...configAllowFrom, ...storeAllowFrom] - .map((entry) => String(entry).trim()) - .filter(Boolean); - const effectiveGroupAllowFrom = [ - ...(configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom), - ...storeAllowFrom, - ] - .map((entry) => String(entry).trim()) - .filter(Boolean); + const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveEffectiveAllowFromLists({ + allowFrom: account.config.allowFrom, + groupAllowFrom: account.config.groupAllowFrom, + storeAllowFrom, + }); const groupAllowEntry = formatGroupAllowlistEntry({ chatGuid: message.chatGuid, chatId: message.chatId ?? undefined, chatIdentifier: message.chatIdentifier ?? undefined, }); const groupName = message.chatName?.trim() || undefined; + const accessDecision = resolveDmGroupAccessDecision({ + isGroup, + dmPolicy, + groupPolicy, + effectiveAllowFrom, + effectiveGroupAllowFrom, + isSenderAllowed: (allowFrom) => + isAllowedBlueBubblesSender({ + allowFrom, + sender: message.senderId, + chatId: message.chatId ?? undefined, + chatGuid: message.chatGuid ?? undefined, + chatIdentifier: message.chatIdentifier ?? undefined, + }), + }); - if (isGroup) { - if (groupPolicy === "disabled") { - logVerbose(core, runtime, "Blocked BlueBubbles group message (groupPolicy=disabled)"); - logGroupAllowlistHint({ - runtime, - reason: "groupPolicy=disabled", - entry: groupAllowEntry, - chatName: groupName, - accountId: account.accountId, - }); - return; - } - if (groupPolicy === "allowlist") { - if (effectiveGroupAllowFrom.length === 0) { + if (accessDecision.decision !== "allow") { + if (isGroup) { + if (accessDecision.reason === "groupPolicy=disabled") { + logVerbose(core, runtime, "Blocked BlueBubbles group message (groupPolicy=disabled)"); + logGroupAllowlistHint({ + runtime, + reason: "groupPolicy=disabled", + entry: groupAllowEntry, + chatName: groupName, + accountId: account.accountId, + }); + return; + } + if (accessDecision.reason === "groupPolicy=allowlist (empty allowlist)") { logVerbose(core, runtime, "Blocked BlueBubbles group message (no allowlist)"); logGroupAllowlistHint({ runtime, @@ -368,14 +379,7 @@ export async function processMessage( }); return; } - const allowed = isAllowedBlueBubblesSender({ - allowFrom: effectiveGroupAllowFrom, - sender: message.senderId, - chatId: message.chatId ?? undefined, - chatGuid: message.chatGuid ?? undefined, - chatIdentifier: message.chatIdentifier ?? undefined, - }); - if (!allowed) { + if (accessDecision.reason === "groupPolicy=allowlist (not allowlisted)") { logVerbose( core, runtime, @@ -395,70 +399,60 @@ export async function processMessage( }); return; } + return; } - } else { - if (dmPolicy === "disabled") { + + if (accessDecision.reason === "dmPolicy=disabled") { logVerbose(core, runtime, `Blocked BlueBubbles DM from ${message.senderId}`); logVerbose(core, runtime, `drop: dmPolicy disabled sender=${message.senderId}`); return; } - if (dmPolicy !== "open") { - const allowed = isAllowedBlueBubblesSender({ - allowFrom: effectiveAllowFrom, - sender: message.senderId, - chatId: message.chatId ?? undefined, - chatGuid: message.chatGuid ?? undefined, - chatIdentifier: message.chatIdentifier ?? undefined, + + if (accessDecision.decision === "pairing") { + const { code, created } = await core.channel.pairing.upsertPairingRequest({ + channel: "bluebubbles", + id: message.senderId, + meta: { name: message.senderName }, }); - if (!allowed) { - if (dmPolicy === "pairing") { - const { code, created } = await core.channel.pairing.upsertPairingRequest({ - channel: "bluebubbles", - id: message.senderId, - meta: { name: message.senderName }, - }); - runtime.log?.( - `[bluebubbles] pairing request sender=${message.senderId} created=${created}`, + runtime.log?.(`[bluebubbles] pairing request sender=${message.senderId} created=${created}`); + if (created) { + logVerbose(core, runtime, `bluebubbles pairing request sender=${message.senderId}`); + try { + await sendMessageBlueBubbles( + message.senderId, + core.channel.pairing.buildPairingReply({ + channel: "bluebubbles", + idLine: `Your BlueBubbles sender id: ${message.senderId}`, + code, + }), + { cfg: config, accountId: account.accountId }, ); - if (created) { - logVerbose(core, runtime, `bluebubbles pairing request sender=${message.senderId}`); - try { - await sendMessageBlueBubbles( - message.senderId, - core.channel.pairing.buildPairingReply({ - channel: "bluebubbles", - idLine: `Your BlueBubbles sender id: ${message.senderId}`, - code, - }), - { cfg: config, accountId: account.accountId }, - ); - statusSink?.({ lastOutboundAt: Date.now() }); - } catch (err) { - logVerbose( - core, - runtime, - `bluebubbles pairing reply failed for ${message.senderId}: ${String(err)}`, - ); - runtime.error?.( - `[bluebubbles] pairing reply failed sender=${message.senderId}: ${String(err)}`, - ); - } - } - } else { + statusSink?.({ lastOutboundAt: Date.now() }); + } catch (err) { logVerbose( core, runtime, - `Blocked unauthorized BlueBubbles sender ${message.senderId} (dmPolicy=${dmPolicy})`, + `bluebubbles pairing reply failed for ${message.senderId}: ${String(err)}`, ); - logVerbose( - core, - runtime, - `drop: dm sender not allowed sender=${message.senderId} allowFrom=${effectiveAllowFrom.join(",")}`, + runtime.error?.( + `[bluebubbles] pairing reply failed sender=${message.senderId}: ${String(err)}`, ); } - return; } + return; } + + logVerbose( + core, + runtime, + `Blocked unauthorized BlueBubbles sender ${message.senderId} (dmPolicy=${dmPolicy})`, + ); + logVerbose( + core, + runtime, + `drop: dm sender not allowed sender=${message.senderId} allowFrom=${effectiveAllowFrom.join(",")}`, + ); + return; } const chatId = message.chatId ?? undefined; @@ -1106,56 +1100,31 @@ export async function processReaction( const dmPolicy = account.config.dmPolicy ?? "pairing"; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - const configAllowFrom = (account.config.allowFrom ?? []).map((entry) => String(entry)); - const configGroupAllowFrom = (account.config.groupAllowFrom ?? []).map((entry) => String(entry)); const storeAllowFrom = await core.channel.pairing .readAllowFromStore("bluebubbles") .catch(() => []); - const effectiveAllowFrom = [...configAllowFrom, ...storeAllowFrom] - .map((entry) => String(entry).trim()) - .filter(Boolean); - const effectiveGroupAllowFrom = [ - ...(configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom), - ...storeAllowFrom, - ] - .map((entry) => String(entry).trim()) - .filter(Boolean); - - if (reaction.isGroup) { - if (groupPolicy === "disabled") { - return; - } - if (groupPolicy === "allowlist") { - if (effectiveGroupAllowFrom.length === 0) { - return; - } - const allowed = isAllowedBlueBubblesSender({ - allowFrom: effectiveGroupAllowFrom, + const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveEffectiveAllowFromLists({ + allowFrom: account.config.allowFrom, + groupAllowFrom: account.config.groupAllowFrom, + storeAllowFrom, + }); + const accessDecision = resolveDmGroupAccessDecision({ + isGroup: reaction.isGroup, + dmPolicy, + groupPolicy, + effectiveAllowFrom, + effectiveGroupAllowFrom, + isSenderAllowed: (allowFrom) => + isAllowedBlueBubblesSender({ + allowFrom, sender: reaction.senderId, chatId: reaction.chatId ?? undefined, chatGuid: reaction.chatGuid ?? undefined, chatIdentifier: reaction.chatIdentifier ?? undefined, - }); - if (!allowed) { - return; - } - } - } else { - if (dmPolicy === "disabled") { - return; - } - if (dmPolicy !== "open") { - const allowed = isAllowedBlueBubblesSender({ - allowFrom: effectiveAllowFrom, - sender: reaction.senderId, - chatId: reaction.chatId ?? undefined, - chatGuid: reaction.chatGuid ?? undefined, - chatIdentifier: reaction.chatIdentifier ?? undefined, - }); - if (!allowed) { - return; - } - } + }), + }); + if (accessDecision.decision !== "allow") { + return; } const chatId = reaction.chatId ?? undefined; diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index d76a8807a34..53f3b5a6c71 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -310,6 +310,11 @@ export { readStringParam, } from "../agents/tools/common.js"; export { formatDocsLink } from "../terminal/links.js"; +export { + resolveDmAllowState, + resolveDmGroupAccessDecision, + resolveEffectiveAllowFromLists, +} from "../security/dm-policy-shared.js"; export type { HookEntry } from "../hooks/types.js"; export { clamp, escapeRegExp, normalizeE164, safeParseJson, sleep } from "../utils.js"; export { stripAnsi } from "../terminal/ansi.js"; diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts index 13acf939aba..bedc1ac67b0 100644 --- a/src/security/dm-policy-shared.test.ts +++ b/src/security/dm-policy-shared.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { resolveDmAllowState } from "./dm-policy-shared.js"; +import { + resolveDmAllowState, + resolveDmGroupAccessDecision, + resolveEffectiveAllowFromLists, +} from "./dm-policy-shared.js"; describe("security/dm-policy-shared", () => { it("normalizes config + store allow entries and counts distinct senders", async () => { @@ -28,4 +32,94 @@ describe("security/dm-policy-shared", () => { expect(state.allowCount).toBe(0); expect(state.isMultiUserDm).toBe(false); }); + + it("builds effective DM/group allowlists from config + pairing store", () => { + const lists = resolveEffectiveAllowFromLists({ + allowFrom: [" owner ", "", "owner2"], + groupAllowFrom: ["group:abc"], + storeAllowFrom: [" owner3 ", ""], + }); + expect(lists.effectiveAllowFrom).toEqual(["owner", "owner2", "owner3"]); + expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc", "owner3"]); + }); + + it("falls back to DM allowlist for groups when groupAllowFrom is empty", () => { + const lists = resolveEffectiveAllowFromLists({ + allowFrom: [" owner "], + groupAllowFrom: [], + storeAllowFrom: [" owner2 "], + }); + expect(lists.effectiveAllowFrom).toEqual(["owner", "owner2"]); + expect(lists.effectiveGroupAllowFrom).toEqual(["owner", "owner2"]); + }); + + const channels = [ + "bluebubbles", + "imessage", + "signal", + "telegram", + "whatsapp", + "msteams", + "matrix", + "zalo", + ] as const; + + for (const channel of channels) { + it(`[${channel}] blocks DM allowlist mode when allowlist is empty`, () => { + const decision = resolveDmGroupAccessDecision({ + isGroup: false, + dmPolicy: "allowlist", + groupPolicy: "allowlist", + effectiveAllowFrom: [], + effectiveGroupAllowFrom: [], + isSenderAllowed: () => false, + }); + expect(decision).toEqual({ + decision: "block", + reason: "dmPolicy=allowlist (not allowlisted)", + }); + }); + + it(`[${channel}] uses pairing flow when DM sender is not allowlisted`, () => { + const decision = resolveDmGroupAccessDecision({ + isGroup: false, + dmPolicy: "pairing", + groupPolicy: "allowlist", + effectiveAllowFrom: [], + effectiveGroupAllowFrom: [], + isSenderAllowed: () => false, + }); + expect(decision).toEqual({ + decision: "pairing", + reason: "dmPolicy=pairing (not allowlisted)", + }); + }); + + it(`[${channel}] allows DM sender when allowlisted`, () => { + const decision = resolveDmGroupAccessDecision({ + isGroup: false, + dmPolicy: "allowlist", + groupPolicy: "allowlist", + effectiveAllowFrom: ["owner"], + effectiveGroupAllowFrom: [], + isSenderAllowed: () => true, + }); + expect(decision.decision).toBe("allow"); + }); + + it(`[${channel}] blocks group allowlist mode when sender/group is not allowlisted`, () => { + const decision = resolveDmGroupAccessDecision({ + isGroup: true, + dmPolicy: "pairing", + groupPolicy: "allowlist", + effectiveAllowFrom: ["owner"], + effectiveGroupAllowFrom: ["group:abc"], + isSenderAllowed: () => false, + }); + expect(decision).toEqual({ + decision: "block", + reason: "groupPolicy=allowlist (not allowlisted)", + }); + }); + } }); diff --git a/src/security/dm-policy-shared.ts b/src/security/dm-policy-shared.ts index b9338fdac75..8e0d80306a1 100644 --- a/src/security/dm-policy-shared.ts +++ b/src/security/dm-policy-shared.ts @@ -2,6 +2,77 @@ import type { ChannelId } from "../channels/plugins/types.js"; import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; +export function resolveEffectiveAllowFromLists(params: { + allowFrom?: Array | null; + groupAllowFrom?: Array | null; + storeAllowFrom?: Array | null; +}): { + effectiveAllowFrom: string[]; + effectiveGroupAllowFrom: string[]; +} { + const configAllowFrom = normalizeStringEntries( + Array.isArray(params.allowFrom) ? params.allowFrom : undefined, + ); + const configGroupAllowFrom = normalizeStringEntries( + Array.isArray(params.groupAllowFrom) ? params.groupAllowFrom : undefined, + ); + const storeAllowFrom = normalizeStringEntries( + Array.isArray(params.storeAllowFrom) ? params.storeAllowFrom : undefined, + ); + const effectiveAllowFrom = normalizeStringEntries([...configAllowFrom, ...storeAllowFrom]); + const groupBase = configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom; + const effectiveGroupAllowFrom = normalizeStringEntries([...groupBase, ...storeAllowFrom]); + return { effectiveAllowFrom, effectiveGroupAllowFrom }; +} + +export type DmGroupAccessDecision = "allow" | "block" | "pairing"; + +export function resolveDmGroupAccessDecision(params: { + isGroup: boolean; + dmPolicy?: string | null; + groupPolicy?: string | null; + effectiveAllowFrom: Array; + effectiveGroupAllowFrom: Array; + isSenderAllowed: (allowFrom: string[]) => boolean; +}): { + decision: DmGroupAccessDecision; + reason: string; +} { + const dmPolicy = params.dmPolicy ?? "pairing"; + const groupPolicy = params.groupPolicy ?? "allowlist"; + const effectiveAllowFrom = normalizeStringEntries(params.effectiveAllowFrom); + const effectiveGroupAllowFrom = normalizeStringEntries(params.effectiveGroupAllowFrom); + + if (params.isGroup) { + if (groupPolicy === "disabled") { + return { decision: "block", reason: "groupPolicy=disabled" }; + } + if (groupPolicy === "allowlist") { + if (effectiveGroupAllowFrom.length === 0) { + return { decision: "block", reason: "groupPolicy=allowlist (empty allowlist)" }; + } + if (!params.isSenderAllowed(effectiveGroupAllowFrom)) { + return { decision: "block", reason: "groupPolicy=allowlist (not allowlisted)" }; + } + } + return { decision: "allow", reason: `groupPolicy=${groupPolicy}` }; + } + + if (dmPolicy === "disabled") { + return { decision: "block", reason: "dmPolicy=disabled" }; + } + if (dmPolicy === "open") { + return { decision: "allow", reason: "dmPolicy=open" }; + } + if (params.isSenderAllowed(effectiveAllowFrom)) { + return { decision: "allow", reason: `dmPolicy=${dmPolicy} (allowlisted)` }; + } + if (dmPolicy === "pairing") { + return { decision: "pairing", reason: "dmPolicy=pairing (not allowlisted)" }; + } + return { decision: "block", reason: `dmPolicy=${dmPolicy} (not allowlisted)` }; +} + export async function resolveDmAllowState(params: { provider: ChannelId; allowFrom?: Array | null;