fix(security): keep DM pairing allowlists out of group auth

steipete · steipete · commit 8bdda7a651c2 · 2026-02-26T12:58:18.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -95,6 +95,7 @@ Docs: https://docs.openclaw.ai
 - Security/Slack member + message subtype events: gate `member_*` plus `message_changed`/`message_deleted`/`thread_broadcast` system-event enqueue through shared sender authorization so DM `dmPolicy`/`allowFrom` and channel `users` allowlists are enforced consistently for non-message ingress; message subtype system events now fail closed when sender identity is missing, with regression coverage. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
 - Security/Telegram reactions: enforce `dmPolicy`/`allowFrom` and group allowlist authorization on `message_reaction` events before enqueueing reaction system events, preventing unauthorized reaction-triggered input in DMs and groups; ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
 - Security/Telegram group allowlist: fail closed for group sender authorization by removing DM pairing-store fallback from group allowlist evaluation; group sender access now requires explicit `groupAllowFrom` or per-group/per-topic `allowFrom`. (#25988) Thanks @bmendonca3.
+- Security/DM-group allowlist boundaries: keep DM pairing-store approvals DM-only by removing pairing-store inheritance from group sender authorization in LINE and Mattermost message preflight, and by centralizing shared DM/group allowlist composition so group checks never include pairing-store entries. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
 - Security/Slack interactions: enforce channel/DM authorization and modal actor binding (`private_metadata.userId`) before enqueueing `block_action`/`view_submission`/`view_closed` system events, with regression coverage for unauthorized senders and missing/mismatched actor metadata. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
 - Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
 - Security/Nextcloud Talk: reject unsigned webhook traffic before full body reads, reducing unauthenticated request-body exposure, with auth-order regression coverage. (#26118) Thanks @bmendonca3.
diff --git a/docs/channels/groups.md b/docs/channels/groups.md
@@ -184,6 +184,7 @@ Notes:
 
 - `groupPolicy` is separate from mention-gating (which requires @mentions).
 - WhatsApp/Telegram/Signal/iMessage/Microsoft Teams/Zalo: use `groupAllowFrom` (fallback: explicit `allowFrom`).
+- DM pairing approvals (`*-allowFrom` store entries) apply to DM access only; group sender authorization stays explicit to group allowlists.
 - Discord: allowlist uses `channels.discord.guilds.<id>.channels`.
 - Slack: allowlist uses `channels.slack.channels`.
 - Matrix: allowlist uses `channels.matrix.groups` (room IDs, aliases, or names). Use `channels.matrix.groupAllowFrom` to restrict senders; per-room `users` allowlists are also supported.
diff --git a/extensions/mattermost/src/mattermost/monitor.authz.test.ts b/extensions/mattermost/src/mattermost/monitor.authz.test.ts
@@ -0,0 +1,37 @@
+import { describe, expect, it } from "vitest";
+import { resolveMattermostEffectiveAllowFromLists } from "./monitor.js";
+
+describe("mattermost monitor authz", () => {
+  it("keeps DM allowlist merged with pairing-store entries", () => {
+    const resolved = resolveMattermostEffectiveAllowFromLists({
+      dmPolicy: "pairing",
+      allowFrom: ["@trusted-user"],
+      groupAllowFrom: ["@group-owner"],
+      storeAllowFrom: ["user:attacker"],
+    });
+
+    expect(resolved.effectiveAllowFrom).toEqual(["trusted-user", "attacker"]);
+  });
+
+  it("uses explicit groupAllowFrom without pairing-store inheritance", () => {
+    const resolved = resolveMattermostEffectiveAllowFromLists({
+      dmPolicy: "pairing",
+      allowFrom: ["@trusted-user"],
+      groupAllowFrom: ["@group-owner"],
+      storeAllowFrom: ["user:attacker"],
+    });
+
+    expect(resolved.effectiveGroupAllowFrom).toEqual(["group-owner"]);
+  });
+
+  it("does not inherit pairing-store entries into group allowlist", () => {
+    const resolved = resolveMattermostEffectiveAllowFromLists({
+      dmPolicy: "pairing",
+      allowFrom: ["@trusted-user"],
+      storeAllowFrom: ["user:attacker"],
+    });
+
+    expect(resolved.effectiveAllowFrom).toEqual(["trusted-user", "attacker"]);
+    expect(resolved.effectiveGroupAllowFrom).toEqual(["trusted-user"]);
+  });
+});
diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts
@@ -18,6 +18,7 @@ import {
   isDangerousNameMatchingEnabled,
   resolveControlCommandGate,
   resolveDmGroupAccessWithLists,
+  resolveEffectiveAllowFromLists,
   resolveAllowlistProviderRuntimeGroupPolicy,
   resolveDefaultGroupPolicy,
   resolveChannelMediaMaxBytes,
@@ -150,6 +151,23 @@ function normalizeAllowList(entries: Array<string | number>): string[] {
   return Array.from(new Set(normalized));
 }
 
+export function resolveMattermostEffectiveAllowFromLists(params: {
+  allowFrom?: Array<string | number> | null;
+  groupAllowFrom?: Array<string | number> | null;
+  storeAllowFrom?: Array<string | number> | null;
+  dmPolicy?: string | null;
+}): {
+  effectiveAllowFrom: string[];
+  effectiveGroupAllowFrom: string[];
+} {
+  return resolveEffectiveAllowFromLists({
+    allowFrom: normalizeAllowList(params.allowFrom ?? []),
+    groupAllowFrom: normalizeAllowList(params.groupAllowFrom ?? []),
+    storeAllowFrom: normalizeAllowList(params.storeAllowFrom ?? []),
+    dmPolicy: params.dmPolicy,
+  });
+}
+
 function isSenderAllowed(params: {
   senderId: string;
   senderName?: string;
@@ -400,20 +418,18 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
       senderId;
     const rawText = post.message?.trim() || "";
     const dmPolicy = account.config.dmPolicy ?? "pairing";
-    const configAllowFrom = normalizeAllowList(account.config.allowFrom ?? []);
-    const configGroupAllowFrom = normalizeAllowList(account.config.groupAllowFrom ?? []);
     const storeAllowFrom = normalizeAllowList(
       dmPolicy === "allowlist"
         ? []
         : await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
     );
-    const effectiveAllowFrom = Array.from(new Set([...configAllowFrom, ...storeAllowFrom]));
-    const effectiveGroupAllowFrom = Array.from(
-      new Set([
-        ...(configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom),
-        ...storeAllowFrom,
-      ]),
-    );
+    const { effectiveAllowFrom, effectiveGroupAllowFrom } =
+      resolveMattermostEffectiveAllowFromLists({
+        dmPolicy,
+        allowFrom: account.config.allowFrom,
+        groupAllowFrom: account.config.groupAllowFrom,
+        storeAllowFrom,
+      });
     const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
       cfg,
       surface: "mattermost",
diff --git a/src/channels/allow-from.test.ts b/src/channels/allow-from.test.ts
@@ -1,10 +1,15 @@
 import { describe, expect, it } from "vitest";
-import { firstDefined, isSenderIdAllowed, mergeAllowFromSources } from "./allow-from.js";
+import {
+  firstDefined,
+  isSenderIdAllowed,
+  mergeDmAllowFromSources,
+  resolveGroupAllowFromSources,
+} from "./allow-from.js";
 
-describe("mergeAllowFromSources", () => {
+describe("mergeDmAllowFromSources", () => {
   it("merges, trims, and filters empty values", () => {
     expect(
-      mergeAllowFromSources({
+      mergeDmAllowFromSources({
         allowFrom: ["  line:user:abc  ", "", 123],
         storeAllowFrom: ["   ", "telegram:456"],
       }),
@@ -13,7 +18,7 @@ describe("mergeAllowFromSources", () => {
 
   it("excludes pairing-store entries when dmPolicy is allowlist", () => {
     expect(
-      mergeAllowFromSources({
+      mergeDmAllowFromSources({
         allowFrom: ["+1111"],
         storeAllowFrom: ["+2222", "+3333"],
         dmPolicy: "allowlist",
@@ -23,7 +28,7 @@ describe("mergeAllowFromSources", () => {
 
   it("keeps pairing-store entries for non-allowlist policies", () => {
     expect(
-      mergeAllowFromSources({
+      mergeDmAllowFromSources({
         allowFrom: ["+1111"],
         storeAllowFrom: ["+2222"],
         dmPolicy: "pairing",
@@ -32,6 +37,26 @@ describe("mergeAllowFromSources", () => {
   });
 });
 
+describe("resolveGroupAllowFromSources", () => {
+  it("prefers explicit group allowlist", () => {
+    expect(
+      resolveGroupAllowFromSources({
+        allowFrom: ["owner"],
+        groupAllowFrom: ["group-owner", " group-admin "],
+      }),
+    ).toEqual(["group-owner", "group-admin"]);
+  });
+
+  it("falls back to DM allowlist when group allowlist is unset/empty", () => {
+    expect(
+      resolveGroupAllowFromSources({
+        allowFrom: [" owner ", "", "owner2"],
+        groupAllowFrom: [],
+      }),
+    ).toEqual(["owner", "owner2"]);
+  });
+});
+
 describe("firstDefined", () => {
   it("returns the first non-undefined value", () => {
     expect(firstDefined(undefined, undefined, "x", "y")).toBe("x");
diff --git a/src/channels/allow-from.ts b/src/channels/allow-from.ts
@@ -1,6 +1,6 @@
-export function mergeAllowFromSources(params: {
+export function mergeDmAllowFromSources(params: {
   allowFrom?: Array<string | number>;
-  storeAllowFrom?: string[];
+  storeAllowFrom?: Array<string | number>;
   dmPolicy?: string;
 }): string[] {
   const storeEntries = params.dmPolicy === "allowlist" ? [] : (params.storeAllowFrom ?? []);
@@ -9,6 +9,17 @@ export function mergeAllowFromSources(params: {
     .filter(Boolean);
 }
 
+export function resolveGroupAllowFromSources(params: {
+  allowFrom?: Array<string | number>;
+  groupAllowFrom?: Array<string | number>;
+}): string[] {
+  const scoped =
+    params.groupAllowFrom && params.groupAllowFrom.length > 0
+      ? params.groupAllowFrom
+      : (params.allowFrom ?? []);
+  return scoped.map((value) => String(value).trim()).filter(Boolean);
+}
+
 export function firstDefined<T>(...values: Array<T | undefined>) {
   for (const value of values) {
     if (typeof value !== "undefined") {
diff --git a/src/line/bot-access.ts b/src/line/bot-access.ts
@@ -1,4 +1,8 @@
-import { firstDefined, isSenderIdAllowed, mergeAllowFromSources } from "../channels/allow-from.js";
+import {
+  firstDefined,
+  isSenderIdAllowed,
+  mergeDmAllowFromSources,
+} from "../channels/allow-from.js";
 
 export type NormalizedAllowFrom = {
   entries: string[];
@@ -27,11 +31,11 @@ export const normalizeAllowFrom = (list?: Array<string | number>): NormalizedAll
   };
 };
 
-export const normalizeAllowFromWithStore = (params: {
+export const normalizeDmAllowFromWithStore = (params: {
   allowFrom?: Array<string | number>;
   storeAllowFrom?: string[];
   dmPolicy?: string;
-}): NormalizedAllowFrom => normalizeAllowFrom(mergeAllowFromSources(params));
+}): NormalizedAllowFrom => normalizeAllowFrom(mergeDmAllowFromSources(params));
 
 export const isSenderAllowed = (params: {
   allow: NormalizedAllowFrom;
diff --git a/src/line/bot-handlers.test.ts b/src/line/bot-handlers.test.ts
@@ -182,6 +182,41 @@ describe("handleLineWebhookEvents", () => {
     expect(processMessage).toHaveBeenCalledTimes(1);
   });
 
+  it("blocks group sender that is only present in pairing-store allowlist", async () => {
+    const processMessage = vi.fn();
+    readAllowFromStoreMock.mockResolvedValueOnce(["user-paired"]);
+    const event = {
+      type: "message",
+      message: { id: "m3b", type: "text", text: "hi" },
+      replyToken: "reply-token",
+      timestamp: Date.now(),
+      source: { type: "group", groupId: "group-1", userId: "user-paired" },
+      mode: "active",
+      webhookEventId: "evt-3b",
+      deliveryContext: { isRedelivery: false },
+    } as MessageEvent;
+
+    await handleLineWebhookEvents([event], {
+      cfg: {
+        channels: { line: { groupPolicy: "allowlist", groupAllowFrom: ["user-owner"] } },
+      },
+      account: {
+        accountId: "default",
+        enabled: true,
+        channelAccessToken: "token",
+        channelSecret: "secret",
+        tokenSource: "config",
+        config: { groupPolicy: "allowlist", groupAllowFrom: ["user-owner"] },
+      },
+      runtime: createRuntime(),
+      mediaMaxBytes: 1,
+      processMessage,
+    });
+
+    expect(buildLineMessageContextMock).not.toHaveBeenCalled();
+    expect(processMessage).not.toHaveBeenCalled();
+  });
+
   it("blocks group messages when wildcard group config disables groups", async () => {
     const processMessage = vi.fn();
     const event = {
diff --git a/src/line/bot-handlers.ts b/src/line/bot-handlers.ts
@@ -21,7 +21,12 @@ import {
   upsertChannelPairingRequest,
 } from "../pairing/pairing-store.js";
 import type { RuntimeEnv } from "../runtime.js";
-import { firstDefined, isSenderAllowed, normalizeAllowFromWithStore } from "./bot-access.js";
+import {
+  firstDefined,
+  isSenderAllowed,
+  normalizeAllowFrom,
+  normalizeDmAllowFromWithStore,
+} from "./bot-access.js";
 import {
   getLineSourceInfo,
   buildLineMessageContext,
@@ -117,7 +122,7 @@ async function shouldProcessLineEvent(
   const dmPolicy = account.config.dmPolicy ?? "pairing";
 
   const storeAllowFrom = await readChannelAllowFromStore("line").catch(() => []);
-  const effectiveDmAllow = normalizeAllowFromWithStore({
+  const effectiveDmAllow = normalizeDmAllowFromWithStore({
     allowFrom: account.config.allowFrom,
     storeAllowFrom,
     dmPolicy,
@@ -132,11 +137,9 @@ async function shouldProcessLineEvent(
     account.config.groupAllowFrom,
     fallbackGroupAllowFrom,
   );
-  const effectiveGroupAllow = normalizeAllowFromWithStore({
-    allowFrom: groupAllowFrom,
-    storeAllowFrom,
-    dmPolicy,
-  });
+  // Group authorization stays explicit to group allowlists and must not
+  // inherit DM pairing-store identities.
+  const effectiveGroupAllow = normalizeAllowFrom(groupAllowFrom);
   const defaultGroupPolicy = resolveDefaultGroupPolicy(cfg);
   const { groupPolicy, providerMissingFallbackApplied } =
     resolveAllowlistProviderRuntimeGroupPolicy({
diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts
@@ -41,7 +41,7 @@ describe("security/dm-policy-shared", () => {
       storeAllowFrom: [" owner3 ", ""],
     });
     expect(lists.effectiveAllowFrom).toEqual(["owner", "owner2", "owner3"]);
-    expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc", "owner3"]);
+    expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc"]);
   });
 
   it("falls back to DM allowlist for groups when groupAllowFrom is empty", () => {
@@ -51,7 +51,7 @@ describe("security/dm-policy-shared", () => {
       storeAllowFrom: [" owner2 "],
     });
     expect(lists.effectiveAllowFrom).toEqual(["owner", "owner2"]);
-    expect(lists.effectiveGroupAllowFrom).toEqual(["owner", "owner2"]);
+    expect(lists.effectiveGroupAllowFrom).toEqual(["owner"]);
   });
 
   it("excludes storeAllowFrom when dmPolicy is allowlist", () => {
@@ -65,15 +65,15 @@ describe("security/dm-policy-shared", () => {
     expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc"]);
   });
 
-  it("includes storeAllowFrom when dmPolicy is pairing", () => {
+  it("keeps group allowlist explicit when dmPolicy is pairing", () => {
     const lists = resolveEffectiveAllowFromLists({
       allowFrom: ["+1111"],
       groupAllowFrom: [],
       storeAllowFrom: ["+2222"],
       dmPolicy: "pairing",
     });
     expect(lists.effectiveAllowFrom).toEqual(["+1111", "+2222"]);
-    expect(lists.effectiveGroupAllowFrom).toEqual(["+1111", "+2222"]);
+    expect(lists.effectiveGroupAllowFrom).toEqual(["+1111"]);
   });
 
   it("resolves access + effective allowlists in one shared call", () => {
@@ -89,7 +89,7 @@ describe("security/dm-policy-shared", () => {
     expect(resolved.decision).toBe("allow");
     expect(resolved.reason).toBe("dmPolicy=pairing (allowlisted)");
     expect(resolved.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
-    expect(resolved.effectiveGroupAllowFrom).toEqual(["group:room", "paired-user"]);
+    expect(resolved.effectiveGroupAllowFrom).toEqual(["group:room"]);
   });
 
   it("keeps allowlist mode strict in shared resolver (no pairing-store fallback)", () => {
diff --git a/src/security/dm-policy-shared.ts b/src/security/dm-policy-shared.ts
@@ -1,3 +1,4 @@
+import { mergeDmAllowFromSources, resolveGroupAllowFromSources } from "../channels/allow-from.js";
 import type { ChannelId } from "../channels/plugins/types.js";
 import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
 import { normalizeStringEntries } from "../shared/string-normalization.js";
@@ -11,21 +12,23 @@ export function resolveEffectiveAllowFromLists(params: {
   effectiveAllowFrom: string[];
   effectiveGroupAllowFrom: string[];
 } {
-  const configAllowFrom = normalizeStringEntries(
-    Array.isArray(params.allowFrom) ? params.allowFrom : undefined,
+  const allowFrom = Array.isArray(params.allowFrom) ? params.allowFrom : undefined;
+  const groupAllowFrom = Array.isArray(params.groupAllowFrom) ? params.groupAllowFrom : undefined;
+  const storeAllowFrom = Array.isArray(params.storeAllowFrom) ? params.storeAllowFrom : undefined;
+  const effectiveAllowFrom = normalizeStringEntries(
+    mergeDmAllowFromSources({
+      allowFrom,
+      storeAllowFrom,
+      dmPolicy: params.dmPolicy ?? undefined,
+    }),
   );
-  const configGroupAllowFrom = normalizeStringEntries(
-    Array.isArray(params.groupAllowFrom) ? params.groupAllowFrom : undefined,
+  // Group auth is explicit (groupAllowFrom fallback allowFrom). Pairing store is DM-only.
+  const effectiveGroupAllowFrom = normalizeStringEntries(
+    resolveGroupAllowFromSources({
+      allowFrom,
+      groupAllowFrom,
+    }),
   );
-  const storeAllowFrom =
-    params.dmPolicy === "allowlist"
-      ? []
-      : 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 };
 }
 
diff --git a/src/telegram/bot-access.ts b/src/telegram/bot-access.ts
diff --git a/src/telegram/bot-handlers.ts b/src/telegram/bot-handlers.ts
diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts
diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts
