From 107bda27c9d857832d79ce57259104969f549329 Mon Sep 17 00:00:00 2001 From: Brian Mendonca Date: Tue, 24 Feb 2026 21:29:20 -0700 Subject: [PATCH] security(msteams): isolate group allowlist from pairing-store entries --- .../message-handler.authz.test.ts | 96 +++++++++++++++++++ .../src/monitor-handler/message-handler.ts | 12 +-- 2 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 extensions/msteams/src/monitor-handler/message-handler.authz.test.ts diff --git a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts new file mode 100644 index 00000000000..124599147a8 --- /dev/null +++ b/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: (params: { + onFlush: (entries: T[]) => Promise; + }): { enqueue: (entry: T) => Promise } => ({ + 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[0]); + + expect(readAllowFromStore).toHaveBeenCalledWith("msteams"); + expect(conversationStore.upsert).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index 085efeeb0a8..a87f704a340 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/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 - ? [...groupAllowFrom.map((v) => String(v)), ...storedAllowFrom] - : []; + !isDirectMessage && msteamsCfg ? groupAllowFrom.map((v) => String(v)) : []; 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,