fix(security): centralize dm/group allowlist auth composition

src/imessage/monitor/inbound-processing.ts

@@ -20,7 +20,7 @@ import {
   resolveChannelGroupRequireMention,
 } from "../../config/group-policy.js";
 import { resolveAgentRoute } from "../../routing/resolve-route.js";
-import { resolveEffectiveAllowFromLists } from "../../security/dm-policy-shared.js";
+import { resolveDmGroupAccessWithLists } from "../../security/dm-policy-shared.js";
 import { truncateUtf16Safe } from "../../utils.js";
 import {
   formatIMessageChatTarget,
@@ -139,72 +139,61 @@ export function resolveIMessageInboundDecision(params: {
   }
 
   const groupId = isGroup ? groupIdCandidate : undefined;
-  const { effectiveAllowFrom: effectiveDmAllowFrom, effectiveGroupAllowFrom } =
-    resolveEffectiveAllowFromLists({
-      allowFrom: params.allowFrom,
-      groupAllowFrom: params.groupAllowFrom,
-      storeAllowFrom: params.storeAllowFrom,
-      dmPolicy: params.dmPolicy,
-      groupAllowFromFallbackToAllowFrom: false,
-    });
+  const accessDecision = resolveDmGroupAccessWithLists({
+    isGroup,
+    dmPolicy: params.dmPolicy,
+    groupPolicy: params.groupPolicy,
+    allowFrom: params.allowFrom,
+    groupAllowFrom: params.groupAllowFrom,
+    storeAllowFrom: params.storeAllowFrom,
+    groupAllowFromFallbackToAllowFrom: false,
+    isSenderAllowed: (allowFrom) =>
+      isAllowedIMessageSender({
+        allowFrom,
+        sender,
+        chatId,
+        chatGuid,
+        chatIdentifier,
+      }),
+  });
+  const effectiveDmAllowFrom = accessDecision.effectiveAllowFrom;
+  const effectiveGroupAllowFrom = accessDecision.effectiveGroupAllowFrom;
+  const dmAuthorized = !isGroup && accessDecision.decision === "allow";
 
-  if (isGroup) {
-    if (params.groupPolicy === "disabled") {
-      params.logVerbose?.("Blocked iMessage group message (groupPolicy: disabled)");
-      return { kind: "drop", reason: "groupPolicy disabled" };
-    }
-    if (params.groupPolicy === "allowlist") {
-      if (effectiveGroupAllowFrom.length === 0) {
+  if (accessDecision.decision !== "allow") {
+    if (isGroup) {
+      if (accessDecision.reason === "groupPolicy=disabled") {
+        params.logVerbose?.("Blocked iMessage group message (groupPolicy: disabled)");
+        return { kind: "drop", reason: "groupPolicy disabled" };
+      }
+      if (accessDecision.reason === "groupPolicy=allowlist (empty allowlist)") {
         params.logVerbose?.(
           "Blocked iMessage group message (groupPolicy: allowlist, no groupAllowFrom)",
         );
         return { kind: "drop", reason: "groupPolicy allowlist (empty groupAllowFrom)" };
       }
-      const allowed = isAllowedIMessageSender({
-        allowFrom: effectiveGroupAllowFrom,
-        sender,
-        chatId,
-        chatGuid,
-        chatIdentifier,
-      });
-      if (!allowed) {
+      if (accessDecision.reason === "groupPolicy=allowlist (not allowlisted)") {
         params.logVerbose?.(`Blocked iMessage sender ${sender} (not in groupAllowFrom)`);
         return { kind: "drop", reason: "not in groupAllowFrom" };
       }
+      params.logVerbose?.(`Blocked iMessage group message (${accessDecision.reason})`);
+      return { kind: "drop", reason: accessDecision.reason };
     }
-    if (groupListPolicy.allowlistEnabled && !groupListPolicy.allowed) {
-      params.logVerbose?.(
-        `imessage: skipping group message (${groupId ?? "unknown"}) not in allowlist`,
-      );
-      return { kind: "drop", reason: "group id not in allowlist" };
-    }
-  }
-
-  const dmHasWildcard = effectiveDmAllowFrom.includes("*");
-  const dmAuthorized =
-    params.dmPolicy === "open"
-      ? true
-      : dmHasWildcard ||
-        (effectiveDmAllowFrom.length > 0 &&
-          isAllowedIMessageSender({
-            allowFrom: effectiveDmAllowFrom,
-            sender,
-            chatId,
-            chatGuid,
-            chatIdentifier,
-          }));
-
-  if (!isGroup) {
-    if (params.dmPolicy === "disabled") {
+    if (accessDecision.reason === "dmPolicy=disabled") {
       return { kind: "drop", reason: "dmPolicy disabled" };
     }
-    if (!dmAuthorized) {
-      if (params.dmPolicy === "pairing") {
-        return { kind: "pairing", senderId: senderNormalized };
-      }
-      params.logVerbose?.(`Blocked iMessage sender ${sender} (dmPolicy=${params.dmPolicy})`);
-      return { kind: "drop", reason: "dmPolicy blocked" };
+    if (accessDecision.decision === "pairing") {
+      return { kind: "pairing", senderId: senderNormalized };
     }
+    params.logVerbose?.(`Blocked iMessage sender ${sender} (dmPolicy=${params.dmPolicy})`);
+    return { kind: "drop", reason: "dmPolicy blocked" };
+  }
+
+  if (isGroup && groupListPolicy.allowlistEnabled && !groupListPolicy.allowed) {
+    params.logVerbose?.(
+      `imessage: skipping group message (${groupId ?? "unknown"}) not in allowlist`,
+    );
+    return { kind: "drop", reason: "group id not in allowlist" };
   }
 
   const route = resolveAgentRoute({

src/security/dm-policy-channel-smoke.test.ts

@@ -0,0 +1,65 @@
+import { describe, expect, it } from "vitest";
+import { isAllowedBlueBubblesSender } from "../../extensions/bluebubbles/src/targets.js";
+import { isMattermostSenderAllowed } from "../../extensions/mattermost/src/mattermost/monitor-auth.js";
+import { isSignalSenderAllowed, type SignalSender } from "../signal/identity.js";
+import { resolveDmGroupAccessWithLists } from "./dm-policy-shared.js";
+
+type ChannelSmokeCase = {
+  name: string;
+  storeAllowFrom: string[];
+  isSenderAllowed: (allowFrom: string[]) => boolean;
+};
+
+const signalSender: SignalSender = {
+  kind: "phone",
+  raw: "+15550001111",
+  e164: "+15550001111",
+};
+
+const cases: ChannelSmokeCase[] = [
+  {
+    name: "bluebubbles",
+    storeAllowFrom: ["attacker-user"],
+    isSenderAllowed: (allowFrom) =>
+      isAllowedBlueBubblesSender({
+        allowFrom,
+        sender: "attacker-user",
+        chatId: 101,
+      }),
+  },
+  {
+    name: "signal",
+    storeAllowFrom: [signalSender.e164],
+    isSenderAllowed: (allowFrom) => isSignalSenderAllowed(signalSender, allowFrom),
+  },
+  {
+    name: "mattermost",
+    storeAllowFrom: ["user:attacker-user"],
+    isSenderAllowed: (allowFrom) =>
+      isMattermostSenderAllowed({
+        senderId: "attacker-user",
+        senderName: "Attacker",
+        allowFrom,
+      }),
+  },
+];
+
+describe("security/dm-policy-shared channel smoke", () => {
+  for (const testCase of cases) {
+    for (const ingress of ["message", "reaction"] as const) {
+      it(`[${testCase.name}] blocks group ${ingress} when sender is only in pairing store`, () => {
+        const access = resolveDmGroupAccessWithLists({
+          isGroup: true,
+          dmPolicy: "pairing",
+          groupPolicy: "allowlist",
+          allowFrom: ["owner-user"],
+          groupAllowFrom: ["group-owner"],
+          storeAllowFrom: testCase.storeAllowFrom,
+          isSenderAllowed: testCase.isSenderAllowed,
+        });
+        expect(access.decision).toBe("block");
+        expect(access.reason).toBe("groupPolicy=allowlist (not allowlisted)");
+      });
+    }
+  }
+});

src/security/dm-policy-shared.test.ts

@@ -133,56 +133,88 @@ describe("security/dm-policy-shared", () => {
     const cases = [
       {
         name: "dmPolicy=open",
+        isGroup: false,
         dmPolicy: "open" as const,
+        groupPolicy: "allowlist" as const,
         allowFrom: [] as string[],
-        senderAllowed: false,
+        groupAllowFrom: [] as string[],
+        storeAllowFrom: [] as string[],
+        isSenderAllowed: () => false,
         expectedDecision: "allow" as const,
         expectedReactionAllowed: true,
       },
       {
         name: "dmPolicy=disabled",
+        isGroup: false,
         dmPolicy: "disabled" as const,
+        groupPolicy: "allowlist" as const,
         allowFrom: [] as string[],
-        senderAllowed: false,
+        groupAllowFrom: [] as string[],
+        storeAllowFrom: [] as string[],
+        isSenderAllowed: () => false,
         expectedDecision: "block" as const,
         expectedReactionAllowed: false,
       },
       {
         name: "dmPolicy=allowlist unauthorized",
+        isGroup: false,
         dmPolicy: "allowlist" as const,
+        groupPolicy: "allowlist" as const,
         allowFrom: ["owner"],
-        senderAllowed: false,
+        groupAllowFrom: [] as string[],
+        storeAllowFrom: [] as string[],
+        isSenderAllowed: () => false,
         expectedDecision: "block" as const,
         expectedReactionAllowed: false,
       },
       {
         name: "dmPolicy=allowlist authorized",
+        isGroup: false,
         dmPolicy: "allowlist" as const,
+        groupPolicy: "allowlist" as const,
         allowFrom: ["owner"],
-        senderAllowed: true,
+        groupAllowFrom: [] as string[],
+        storeAllowFrom: [] as string[],
+        isSenderAllowed: () => true,
         expectedDecision: "allow" as const,
         expectedReactionAllowed: true,
       },
       {
         name: "dmPolicy=pairing unauthorized",
+        isGroup: false,
         dmPolicy: "pairing" as const,
+        groupPolicy: "allowlist" as const,
         allowFrom: [] as string[],
-        senderAllowed: false,
+        groupAllowFrom: [] as string[],
+        storeAllowFrom: [] as string[],
+        isSenderAllowed: () => false,
         expectedDecision: "pairing" as const,
         expectedReactionAllowed: false,
       },
+      {
+        name: "groupPolicy=allowlist rejects DM-paired sender not in explicit group list",
+        isGroup: true,
+        dmPolicy: "pairing" as const,
+        groupPolicy: "allowlist" as const,
+        allowFrom: ["owner"] as string[],
+        groupAllowFrom: ["group-owner"] as string[],
+        storeAllowFrom: ["paired-user"] as string[],
+        isSenderAllowed: (allowFrom: string[]) => allowFrom.includes("paired-user"),
+        expectedDecision: "block" as const,
+        expectedReactionAllowed: false,
+      },
     ];
 
     for (const channel of channels) {
       for (const testCase of cases) {
         const access = resolveDmGroupAccessWithLists({
-          isGroup: false,
+          isGroup: testCase.isGroup,
           dmPolicy: testCase.dmPolicy,
-          groupPolicy: "allowlist",
+          groupPolicy: testCase.groupPolicy,
           allowFrom: testCase.allowFrom,
-          groupAllowFrom: [],
-          storeAllowFrom: [],
-          isSenderAllowed: () => testCase.senderAllowed,
+          groupAllowFrom: testCase.groupAllowFrom,
+          storeAllowFrom: testCase.storeAllowFrom,
+          isSenderAllowed: testCase.isSenderAllowed,
         });
         const reactionAllowed = access.decision === "allow";
         expect(access.decision, `[${channel}] ${testCase.name}`).toBe(testCase.expectedDecision);