security(msteams): isolate group allowlist from pairing-store entries

extensions/msteams/src/monitor-handler/message-handler.authz.test.ts

@@ -0,0 +1,96 @@
+import type { OpenClawConfig, PluginRuntime, RuntimeEnv } from "openclaw/plugin-sdk";
+import { describe, expect, it, vi } from "vitest";
+import type { MSTeamsMessageHandlerDeps } from "../monitor-handler.js";
+import { setMSTeamsRuntime } from "../runtime.js";
+import { createMSTeamsMessageHandler } from "./message-handler.js";
+
+describe("msteams monitor handler authz", () => {
+  it("does not treat DM pairing-store entries as group allowlist entries", async () => {
+    const readAllowFromStore = vi.fn(async () => ["attacker-aad"]);
+    setMSTeamsRuntime({
+      logging: { shouldLogVerbose: () => false },
+      channel: {
+        debounce: {
+          resolveInboundDebounceMs: () => 0,
+          createInboundDebouncer: <T>(params: {
+            onFlush: (entries: T[]) => Promise<void>;
+          }): { enqueue: (entry: T) => Promise<void> } => ({
+            enqueue: async (entry: T) => {
+              await params.onFlush([entry]);
+            },
+          }),
+        },
+        pairing: {
+          readAllowFromStore,
+          upsertPairingRequest: vi.fn(async () => null),
+        },
+        text: {
+          hasControlCommand: () => false,
+        },
+      },
+    } as unknown as PluginRuntime);
+
+    const conversationStore = {
+      upsert: vi.fn(async () => undefined),
+    };
+
+    const deps: MSTeamsMessageHandlerDeps = {
+      cfg: {
+        channels: {
+          msteams: {
+            dmPolicy: "pairing",
+            allowFrom: [],
+            groupPolicy: "allowlist",
+            groupAllowFrom: [],
+          },
+        },
+      } as OpenClawConfig,
+      runtime: { error: vi.fn() } as unknown as RuntimeEnv,
+      appId: "test-app",
+      adapter: {} as MSTeamsMessageHandlerDeps["adapter"],
+      tokenProvider: {
+        getAccessToken: vi.fn(async () => "token"),
+      },
+      textLimit: 4000,
+      mediaMaxBytes: 1024 * 1024,
+      conversationStore:
+        conversationStore as unknown as MSTeamsMessageHandlerDeps["conversationStore"],
+      pollStore: {
+        recordVote: vi.fn(async () => null),
+      } as unknown as MSTeamsMessageHandlerDeps["pollStore"],
+      log: {
+        info: vi.fn(),
+        debug: vi.fn(),
+        error: vi.fn(),
+      } as unknown as MSTeamsMessageHandlerDeps["log"],
+    };
+
+    const handler = createMSTeamsMessageHandler(deps);
+    await handler({
+      activity: {
+        id: "msg-1",
+        type: "message",
+        text: "",
+        from: {
+          id: "attacker-id",
+          aadObjectId: "attacker-aad",
+          name: "Attacker",
+        },
+        recipient: {
+          id: "bot-id",
+          name: "Bot",
+        },
+        conversation: {
+          id: "19:group@thread.tacv2",
+          conversationType: "groupChat",
+        },
+        channelData: {},
+        attachments: [],
+      },
+      sendActivity: vi.fn(async () => undefined),
+    } as unknown as Parameters<typeof handler>[0]);
+
+    expect(readAllowFromStore).toHaveBeenCalledWith("msteams");
+    expect(conversationStore.upsert).not.toHaveBeenCalled();
+  });
+});

extensions/msteams/src/monitor-handler/message-handler.ts

@@ -135,7 +135,8 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
 
     // Check DM policy for direct messages.
     const dmAllowFrom = msteamsCfg?.allowFrom ?? [];
-    const effectiveDmAllowFrom = [...dmAllowFrom.map((v) => String(v)), ...storedAllowFrom];
+    const configuredDmAllowFrom = dmAllowFrom.map((v) => String(v));
+    const effectiveDmAllowFrom = [...configuredDmAllowFrom, ...storedAllowFrom];
     if (isDirectMessage && msteamsCfg) {
       const allowFrom = dmAllowFrom;
 
@@ -189,9 +190,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
           (msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0 ? msteamsCfg.allowFrom : []))
         : [];
     const effectiveGroupAllowFrom =
-      !isDirectMessage && msteamsCfg
+      !isDirectMessage && msteamsCfg ? groupAllowFrom.map((v) => String(v)) : [];
-        ? [...groupAllowFrom.map((v) => String(v)), ...storedAllowFrom]
-        : [];
     const teamId = activity.channelData?.team?.id;
     const teamName = activity.channelData?.team?.name;
     const channelName = activity.channelData?.channel?.name;
@@ -248,9 +247,10 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
       }
     }
 
+    const commandDmAllowFrom = isDirectMessage ? effectiveDmAllowFrom : configuredDmAllowFrom;
     const ownerAllowedForCommands = isMSTeamsGroupAllowed({
       groupPolicy: "allowlist",
-      allowFrom: effectiveDmAllowFrom,
+      allowFrom: commandDmAllowFrom,
       senderId,
       senderName,
       allowNameMatching: isDangerousNameMatchingEnabled(msteamsCfg),
@@ -266,7 +266,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
     const commandGate = resolveControlCommandGate({
       useAccessGroups,
       authorizers: [
-        { configured: effectiveDmAllowFrom.length > 0, allowed: ownerAllowedForCommands },
+        { configured: commandDmAllowFrom.length > 0, allowed: ownerAllowedForCommands },
         { configured: effectiveGroupAllowFrom.length > 0, allowed: groupAllowedForCommands },
       ],
       allowTextCommands: true,