refactor(bluebubbles): share dm/group access policy checks

2026-03-07 22:44:16 +00:00 · 2026-02-21 20:08:17 +01:00
parent c3af00bddb
commit 4540790cb6
4 changed files with 265 additions and 126 deletions
--- 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;
--- 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";
--- 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)",
+      });
+    });
+  }
 });
--- 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<string | number> | null;
+  groupAllowFrom?: Array<string | number> | null;
+  storeAllowFrom?: Array<string | number> | 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<string | number>;
+  effectiveGroupAllowFrom: Array<string | number>;
+  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<string | number> | null;