fix(msteams): require admin for group actions (#91746)

This commit is contained in:
Agustin Rivera
2026-06-09 12:52:24 -07:00
committed by GitHub
parent 96a49caffa
commit d2ddc26e89
2 changed files with 194 additions and 0 deletions

View File

@@ -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<string, unknown>;
mediaLocalRoots?: readonly string[];
mediaReadFile?: (filePath: string) => Promise<Buffer>;
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<ReturnType<typeof requireMSTeamsHandleAction>>[0]);
}
@@ -182,6 +202,9 @@ async function expectSuccessfulAction(params: {
toolContext?: Parameters<typeof runAction>[0]["toolContext"];
mediaLocalRoots?: Parameters<typeof runAction>[0]["mediaLocalRoots"];
mediaReadFile?: Parameters<typeof runAction>[0]["mediaReadFile"];
requesterSenderId?: Parameters<typeof runAction>[0]["requesterSenderId"];
senderIsOwner?: Parameters<typeof runAction>[0]["senderIsOwner"];
gatewayClientScopes?: Parameters<typeof runAction>[0]["gatewayClientScopes"];
runtimeParams: Record<string, unknown>;
details: Record<string, unknown>;
contentDetails?: Record<string, unknown>;
@@ -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,

View File

@@ -89,6 +89,12 @@ const TEAMS_GRAPH_PERMISSION_HINTS: Record<string, string> = {
"Files.Read.All": "files (OneDrive)",
};
const MSTEAMS_GROUP_MANAGEMENT_ACTIONS = new Set<ChannelMessageActionName>([
"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<typeof actionError> | 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<string, unknown>,
currentChannelId?: string | null,
@@ -690,7 +708,16 @@ export const msteamsPlugin: ChannelPlugin<ResolvedMSTeamsAccount, ProbeMSTeamsRe
},
actions: {
describeMessageTool: describeMSTeamsMessageTool,
requiresTrustedRequesterSender: ({ action, toolContext }) =>
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)