fix: enforce explicit group auth boundaries across channels

src/web/inbound/access-control.ts

@@ -10,7 +10,10 @@ import {
   readChannelAllowFromStore,
   upsertChannelPairingRequest,
 } from "../../pairing/pairing-store.js";
-import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js";
+import {
+  readStoreAllowFromForDmPolicy,
+  resolveDmGroupAccessWithLists,
+} from "../../security/dm-policy-shared.js";
 import { isSelfChatMode, normalizeE164 } from "../../utils.js";
 import { resolveWhatsAppAccount } from "../accounts.js";
 
@@ -60,22 +63,18 @@ export async function checkInboundAccessControl(params: {
     accountId: params.accountId,
   });
   const dmPolicy = account.dmPolicy ?? "pairing";
-  const configuredAllowFrom = account.allowFrom;
+  const configuredAllowFrom = account.allowFrom ?? [];
   const storeAllowFrom = await readStoreAllowFromForDmPolicy({
     provider: "whatsapp",
     dmPolicy,
     readStore: (provider) => readChannelAllowFromStore(provider, process.env, account.accountId),
   });
   // Without user config, default to self-only DM access so the owner can talk to themselves.
-  const combinedAllowFrom = Array.from(
-    new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom]),
-  );
   const defaultAllowFrom =
-    combinedAllowFrom.length === 0 && params.selfE164 ? [params.selfE164] : undefined;
-  const allowFrom = combinedAllowFrom.length > 0 ? combinedAllowFrom : defaultAllowFrom;
+    configuredAllowFrom.length === 0 && params.selfE164 ? [params.selfE164] : [];
+  const dmAllowFrom = configuredAllowFrom.length > 0 ? configuredAllowFrom : defaultAllowFrom;
   const groupAllowFrom =
-    account.groupAllowFrom ??
-    (configuredAllowFrom && configuredAllowFrom.length > 0 ? configuredAllowFrom : undefined);
+    account.groupAllowFrom ?? (configuredAllowFrom.length > 0 ? configuredAllowFrom : undefined);
   const isSamePhone = params.from === params.selfE164;
   const isSelfChat = account.selfChatMode ?? isSelfChatMode(params.selfE164, configuredAllowFrom);
   const pairingGraceMs =
@@ -87,18 +86,6 @@ export async function checkInboundAccessControl(params: {
     typeof params.messageTimestampMs === "number" &&
     params.messageTimestampMs < params.connectedAtMs - pairingGraceMs;
 
-  // Pre-compute normalized allowlists for filtering.
-  const dmHasWildcard = allowFrom?.includes("*") ?? false;
-  const normalizedAllowFrom =
-    allowFrom && allowFrom.length > 0
-      ? allowFrom.filter((entry) => entry !== "*").map(normalizeE164)
-      : [];
-  const groupHasWildcard = groupAllowFrom?.includes("*") ?? false;
-  const normalizedGroupAllowFrom =
-    groupAllowFrom && groupAllowFrom.length > 0
-      ? groupAllowFrom.filter((entry) => entry !== "*").map(normalizeE164)
-      : [];
-
   // Group policy filtering:
   // - "open": groups bypass allowFrom, only mention-gating applies
   // - "disabled": block all group messages entirely
@@ -115,8 +102,45 @@ export async function checkInboundAccessControl(params: {
     accountId: account.accountId,
     log: (message) => logVerbose(message),
   });
-  if (params.group && groupPolicy === "disabled") {
-    logVerbose("Blocked group message (groupPolicy: disabled)");
+  const normalizedDmSender = normalizeE164(params.from);
+  const normalizedGroupSender =
+    typeof params.senderE164 === "string" ? normalizeE164(params.senderE164) : null;
+  const access = resolveDmGroupAccessWithLists({
+    isGroup: params.group,
+    dmPolicy,
+    groupPolicy,
+    // Groups intentionally fall back to configured allowFrom only (not DM self-chat fallback).
+    allowFrom: params.group ? configuredAllowFrom : dmAllowFrom,
+    groupAllowFrom,
+    storeAllowFrom,
+    isSenderAllowed: (allowEntries) => {
+      const hasWildcard = allowEntries.includes("*");
+      if (hasWildcard) {
+        return true;
+      }
+      const normalizedEntrySet = new Set(
+        allowEntries
+          .map((entry) => normalizeE164(String(entry)))
+          .filter((entry): entry is string => Boolean(entry)),
+      );
+      if (!params.group && isSamePhone) {
+        return true;
+      }
+      return params.group
+        ? Boolean(normalizedGroupSender && normalizedEntrySet.has(normalizedGroupSender))
+        : normalizedEntrySet.has(normalizedDmSender);
+    },
+  });
+  if (params.group && access.decision !== "allow") {
+    if (access.reason === "groupPolicy=disabled") {
+      logVerbose("Blocked group message (groupPolicy: disabled)");
+    } else if (access.reason === "groupPolicy=allowlist (empty allowlist)") {
+      logVerbose("Blocked group message (groupPolicy: allowlist, no groupAllowFrom)");
+    } else {
+      logVerbose(
+        `Blocked group message from ${params.senderE164 ?? "unknown sender"} (groupPolicy: allowlist)`,
+      );
+    }
     return {
       allowed: false,
       shouldMarkRead: false,
@@ -124,31 +148,6 @@ export async function checkInboundAccessControl(params: {
       resolvedAccountId: account.accountId,
     };
   }
-  if (params.group && groupPolicy === "allowlist") {
-    if (!groupAllowFrom || groupAllowFrom.length === 0) {
-      logVerbose("Blocked group message (groupPolicy: allowlist, no groupAllowFrom)");
-      return {
-        allowed: false,
-        shouldMarkRead: false,
-        isSelfChat,
-        resolvedAccountId: account.accountId,
-      };
-    }
-    const senderAllowed =
-      groupHasWildcard ||
-      (params.senderE164 != null && normalizedGroupAllowFrom.includes(params.senderE164));
-    if (!senderAllowed) {
-      logVerbose(
-        `Blocked group message from ${params.senderE164 ?? "unknown sender"} (groupPolicy: allowlist)`,
-      );
-      return {
-        allowed: false,
-        shouldMarkRead: false,
-        isSelfChat,
-        resolvedAccountId: account.accountId,
-      };
-    }
-  }
 
   // DM access control (secure defaults): "pairing" (default) / "allowlist" / "open" / "disabled".
   if (!params.group) {
@@ -161,7 +160,7 @@ export async function checkInboundAccessControl(params: {
         resolvedAccountId: account.accountId,
       };
     }
-    if (dmPolicy === "disabled") {
+    if (access.decision === "block" && access.reason === "dmPolicy=disabled") {
       logVerbose("Blocked dm (dmPolicy: disabled)");
       return {
         allowed: false,
@@ -170,49 +169,49 @@ export async function checkInboundAccessControl(params: {
         resolvedAccountId: account.accountId,
       };
     }
-    if (dmPolicy !== "open" && !isSamePhone) {
+    if (access.decision === "pairing" && !isSamePhone) {
       const candidate = params.from;
-      const allowed =
-        dmHasWildcard ||
-        (normalizedAllowFrom.length > 0 && normalizedAllowFrom.includes(candidate));
-      if (!allowed) {
-        if (dmPolicy === "pairing") {
-          if (suppressPairingReply) {
-            logVerbose(`Skipping pairing reply for historical DM from ${candidate}.`);
-          } else {
-            const { code, created } = await upsertChannelPairingRequest({
-              channel: "whatsapp",
-              id: candidate,
-              accountId: account.accountId,
-              meta: { name: (params.pushName ?? "").trim() || undefined },
+      if (suppressPairingReply) {
+        logVerbose(`Skipping pairing reply for historical DM from ${candidate}.`);
+      } else {
+        const { code, created } = await upsertChannelPairingRequest({
+          channel: "whatsapp",
+          id: candidate,
+          accountId: account.accountId,
+          meta: { name: (params.pushName ?? "").trim() || undefined },
+        });
+        if (created) {
+          logVerbose(
+            `whatsapp pairing request sender=${candidate} name=${params.pushName ?? "unknown"}`,
+          );
+          try {
+            await params.sock.sendMessage(params.remoteJid, {
+              text: buildPairingReply({
+                channel: "whatsapp",
+                idLine: `Your WhatsApp phone number: ${candidate}`,
+                code,
+              }),
             });
-            if (created) {
-              logVerbose(
-                `whatsapp pairing request sender=${candidate} name=${params.pushName ?? "unknown"}`,
-              );
-              try {
-                await params.sock.sendMessage(params.remoteJid, {
-                  text: buildPairingReply({
-                    channel: "whatsapp",
-                    idLine: `Your WhatsApp phone number: ${candidate}`,
-                    code,
-                  }),
-                });
-              } catch (err) {
-                logVerbose(`whatsapp pairing reply failed for ${candidate}: ${String(err)}`);
-              }
-            }
+          } catch (err) {
+            logVerbose(`whatsapp pairing reply failed for ${candidate}: ${String(err)}`);
           }
-        } else {
-          logVerbose(`Blocked unauthorized sender ${candidate} (dmPolicy=${dmPolicy})`);
         }
-        return {
-          allowed: false,
-          shouldMarkRead: false,
-          isSelfChat,
-          resolvedAccountId: account.accountId,
-        };
       }
+      return {
+        allowed: false,
+        shouldMarkRead: false,
+        isSelfChat,
+        resolvedAccountId: account.accountId,
+      };
+    }
+    if (access.decision !== "allow") {
+      logVerbose(`Blocked unauthorized sender ${params.from} (dmPolicy=${dmPolicy})`);
+      return {
+        allowed: false,
+        shouldMarkRead: false,
+        isSelfChat,
+        resolvedAccountId: account.accountId,
+      };
     }
   }