From d2ddc26e89bf57b71accd9bf79afee627555e424 Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Tue, 9 Jun 2026 12:52:24 -0700 Subject: [PATCH] fix(msteams): require admin for group actions (#91746) --- .../msteams/src/channel.actions.test.ts | 167 ++++++++++++++++++ extensions/msteams/src/channel.ts | 27 +++ 2 files changed, 194 insertions(+) diff --git a/extensions/msteams/src/channel.actions.test.ts b/extensions/msteams/src/channel.actions.test.ts index 9c7e5fcde29..b488a0dec48 100644 --- a/extensions/msteams/src/channel.actions.test.ts +++ b/extensions/msteams/src/channel.actions.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { msteamsPlugin } from "./channel.js"; const { + addParticipantMSTeamsMock, editMessageMSTeamsMock, deleteMessageMSTeamsMock, getChannelInfoMSTeamsMock, @@ -13,11 +14,14 @@ const { listReactionsMSTeamsMock, pinMessageMSTeamsMock, reactMessageMSTeamsMock, + removeParticipantMSTeamsMock, + renameGroupMSTeamsMock, searchMessagesMSTeamsMock, sendAdaptiveCardMSTeamsMock, sendMessageMSTeamsMock, unpinMessageMSTeamsMock, } = vi.hoisted(() => ({ + addParticipantMSTeamsMock: vi.fn(), editMessageMSTeamsMock: vi.fn(), deleteMessageMSTeamsMock: vi.fn(), getChannelInfoMSTeamsMock: vi.fn(), @@ -27,6 +31,8 @@ const { listReactionsMSTeamsMock: vi.fn(), pinMessageMSTeamsMock: vi.fn(), reactMessageMSTeamsMock: vi.fn(), + removeParticipantMSTeamsMock: vi.fn(), + renameGroupMSTeamsMock: vi.fn(), searchMessagesMSTeamsMock: vi.fn(), sendAdaptiveCardMSTeamsMock: vi.fn(), sendMessageMSTeamsMock: vi.fn(), @@ -35,6 +41,7 @@ const { vi.mock("./channel.runtime.js", () => ({ msTeamsChannelRuntime: { + addParticipantMSTeams: addParticipantMSTeamsMock, editMessageMSTeams: editMessageMSTeamsMock, deleteMessageMSTeams: deleteMessageMSTeamsMock, getChannelInfoMSTeams: getChannelInfoMSTeamsMock, @@ -44,6 +51,8 @@ vi.mock("./channel.runtime.js", () => ({ listReactionsMSTeams: listReactionsMSTeamsMock, pinMessageMSTeams: pinMessageMSTeamsMock, reactMessageMSTeams: reactMessageMSTeamsMock, + removeParticipantMSTeams: removeParticipantMSTeamsMock, + renameGroupMSTeams: renameGroupMSTeamsMock, searchMessagesMSTeams: searchMessagesMSTeamsMock, sendAdaptiveCardMSTeams: sendAdaptiveCardMSTeamsMock, sendMessageMSTeams: sendMessageMSTeamsMock, @@ -52,6 +61,7 @@ vi.mock("./channel.runtime.js", () => ({ })); const actionMocks = [ + addParticipantMSTeamsMock, editMessageMSTeamsMock, deleteMessageMSTeamsMock, getChannelInfoMSTeamsMock, @@ -61,6 +71,8 @@ const actionMocks = [ listReactionsMSTeamsMock, pinMessageMSTeamsMock, reactMessageMSTeamsMock, + removeParticipantMSTeamsMock, + renameGroupMSTeamsMock, searchMessagesMSTeamsMock, sendAdaptiveCardMSTeamsMock, sendMessageMSTeamsMock, @@ -82,6 +94,8 @@ const reactMissingEmojiError = "React requires an emoji (reaction type). Valid types: like, heart, laugh, surprised, sad, angry."; const reactMissingEmojiDetail = "React requires an emoji (reaction type)."; const searchMissingQueryError = "Search requires a target (to) and query."; +const groupManagementAuthError = + "Microsoft Teams group management requires an owner or operator.admin requester."; function padded(value: string) { return ` ${value} `; @@ -114,6 +128,9 @@ async function runAction(params: { toolContext?: Record; mediaLocalRoots?: readonly string[]; mediaReadFile?: (filePath: string) => Promise; + requesterSenderId?: string | null; + senderIsOwner?: boolean; + gatewayClientScopes?: readonly string[]; }) { const handleAction = requireMSTeamsHandleAction(); return await handleAction({ @@ -124,6 +141,9 @@ async function runAction(params: { mediaLocalRoots: params.mediaLocalRoots, mediaReadFile: params.mediaReadFile, toolContext: params.toolContext, + requesterSenderId: params.requesterSenderId, + senderIsOwner: params.senderIsOwner, + gatewayClientScopes: params.gatewayClientScopes, } as Parameters>[0]); } @@ -182,6 +202,9 @@ async function expectSuccessfulAction(params: { toolContext?: Parameters[0]["toolContext"]; mediaLocalRoots?: Parameters[0]["mediaLocalRoots"]; mediaReadFile?: Parameters[0]["mediaReadFile"]; + requesterSenderId?: Parameters[0]["requesterSenderId"]; + senderIsOwner?: Parameters[0]["senderIsOwner"]; + gatewayClientScopes?: Parameters[0]["gatewayClientScopes"]; runtimeParams: Record; details: Record; contentDetails?: Record; @@ -193,6 +216,9 @@ async function expectSuccessfulAction(params: { mediaLocalRoots: params.mediaLocalRoots, mediaReadFile: params.mediaReadFile, toolContext: params.toolContext, + requesterSenderId: params.requesterSenderId, + senderIsOwner: params.senderIsOwner, + gatewayClientScopes: params.gatewayClientScopes, }); expectActionRuntimeCall(params.mockFn, params.runtimeParams); expectActionSuccess(result, params.details, params.contentDetails); @@ -351,6 +377,147 @@ describe("msteamsPlugin message actions", () => { }); }); + it("requires trusted requester sender for Teams group-management actions from Teams turns", () => { + const requiresTrustedRequesterSender = msteamsPlugin.actions?.requiresTrustedRequesterSender; + if (!requiresTrustedRequesterSender) { + throw new Error("msteams actions.requiresTrustedRequesterSender unavailable"); + } + + for (const action of ["addParticipant", "removeParticipant", "renameGroup"] as const) { + expect( + requiresTrustedRequesterSender({ + action, + toolContext: { currentChannelProvider: "msteams" }, + }), + ).toBe(true); + } + expect( + requiresTrustedRequesterSender({ + action: "addParticipant", + toolContext: { currentChannelProvider: "discord" }, + }), + ).toBe(false); + expect( + requiresTrustedRequesterSender({ + action: "read", + toolContext: { currentChannelProvider: "msteams" }, + }), + ).toBe(false); + }); + + it("rejects group-management actions from non-owner non-admin callers", async () => { + const cases = [ + { + action: "addParticipant", + mockFn: addParticipantMSTeamsMock, + params: { target: targetChannelId, userId: "user-1" }, + }, + { + action: "removeParticipant", + mockFn: removeParticipantMSTeamsMock, + params: { target: targetChannelId, userId: "user-1" }, + }, + { + action: "renameGroup", + mockFn: renameGroupMSTeamsMock, + params: { target: targetChannelId, name: "Renamed group" }, + }, + ] as const; + + for (const testCase of cases) { + await expectActionError( + { + action: testCase.action, + params: testCase.params, + senderIsOwner: false, + gatewayClientScopes: ["operator.write"], + }, + groupManagementAuthError, + ); + expect(testCase.mockFn).not.toHaveBeenCalled(); + } + }); + + it("allows owner-authorized group-management actions", async () => { + await expectSuccessfulAction({ + mockFn: addParticipantMSTeamsMock, + mockResult: { added: { userId: "user-1", chatId: targetChannelId } }, + action: "addParticipant", + actionParams: { + target: targetChannelId, + userId: " user-1 ", + role: " owner ", + }, + senderIsOwner: true, + runtimeParams: { + to: targetChannelId, + userId: "user-1", + role: "owner", + }, + details: okMSTeamsActionDetails("addParticipant", { + added: { userId: "user-1", chatId: targetChannelId }, + }), + contentDetails: { + ok: true, + channel: "msteams", + action: "addParticipant", + added: { userId: "user-1", chatId: targetChannelId }, + }, + }); + }); + + it("allows operator.admin group-management actions without owner sender status", async () => { + await expectSuccessfulAction({ + mockFn: removeParticipantMSTeamsMock, + mockResult: { removed: { userId: "user-1", chatId: targetChannelId } }, + action: "removeParticipant", + actionParams: { + target: targetChannelId, + userId: " user-1 ", + }, + senderIsOwner: false, + gatewayClientScopes: ["operator.admin"], + runtimeParams: { + to: targetChannelId, + userId: "user-1", + }, + details: okMSTeamsActionDetails("removeParticipant", { + removed: { userId: "user-1", chatId: targetChannelId }, + }), + contentDetails: { + ok: true, + channel: "msteams", + action: "removeParticipant", + removed: { userId: "user-1", chatId: targetChannelId }, + }, + }); + + await expectSuccessfulAction({ + mockFn: renameGroupMSTeamsMock, + mockResult: { renamed: { chatId: targetChannelId, newName: "Renamed group" } }, + action: "renameGroup", + actionParams: { + target: targetChannelId, + name: " Renamed group ", + }, + senderIsOwner: false, + gatewayClientScopes: ["operator.admin"], + runtimeParams: { + to: targetChannelId, + name: "Renamed group", + }, + details: okMSTeamsActionDetails("renameGroup", { + renamed: { chatId: targetChannelId, newName: "Renamed group" }, + }), + contentDetails: { + ok: true, + channel: "msteams", + action: "renameGroup", + renamed: { chatId: targetChannelId, newName: "Renamed group" }, + }, + }); + }); + it("accepts target as an alias for pin actions", async () => { await expectSuccessfulAction({ mockFn: pinMessageMSTeamsMock, diff --git a/extensions/msteams/src/channel.ts b/extensions/msteams/src/channel.ts index b7d7fbf4a34..6a29738b49e 100644 --- a/extensions/msteams/src/channel.ts +++ b/extensions/msteams/src/channel.ts @@ -89,6 +89,12 @@ const TEAMS_GRAPH_PERMISSION_HINTS: Record = { "Files.Read.All": "files (OneDrive)", }; +const MSTEAMS_GROUP_MANAGEMENT_ACTIONS = new Set([ + "addParticipant", + "removeParticipant", + "renameGroup", +]); + const collectMSTeamsSecurityWarnings = createAllowlistProviderGroupPolicyWarningCollector<{ cfg: OpenClawConfig; }>({ @@ -178,6 +184,18 @@ function actionError(message: string) { }; } +function requireMSTeamsGroupManagementAuthorization(ctx: { + senderIsOwner?: boolean; + gatewayClientScopes?: readonly string[]; +}): ReturnType | null { + if (ctx.senderIsOwner === true || ctx.gatewayClientScopes?.includes("operator.admin")) { + return null; + } + return actionError( + "Microsoft Teams group management requires an owner or operator.admin requester.", + ); +} + function resolveActionTarget( params: Record, currentChannelId?: string | null, @@ -690,7 +708,16 @@ export const msteamsPlugin: ChannelPlugin + normalizeOptionalString(toolContext?.currentChannelProvider)?.toLowerCase() === + "msteams" && MSTEAMS_GROUP_MANAGEMENT_ACTIONS.has(action), handleAction: async (ctx) => { + if (MSTEAMS_GROUP_MANAGEMENT_ACTIONS.has(ctx.action)) { + const authError = requireMSTeamsGroupManagementAuthorization(ctx); + if (authError) { + return authError; + } + } const presentation = ctx.action === "send" ? normalizeMessagePresentation(ctx.params.presentation)