fix: harden msteams group access

This commit is contained in:
Peter Steinberger
2026-01-12 08:31:59 +00:00
parent 4d075a703e
commit 006e1352d8
12 changed files with 206 additions and 7 deletions

View File

@@ -1481,6 +1481,16 @@ describe("legacy config detection", () => {
}
});
it("defaults msteams.groupPolicy to allowlist when msteams section exists", async () => {
vi.resetModules();
const { validateConfigObject } = await import("./config.js");
const res = validateConfigObject({ msteams: {} });
expect(res.ok).toBe(true);
if (res.ok) {
expect(res.config.msteams?.groupPolicy).toBe("allowlist");
}
});
it("rejects unsafe executable config values", async () => {
vi.resetModules();
const { validateConfigObject } = await import("./config.js");

View File

@@ -763,6 +763,15 @@ export type MSTeamsConfig = {
dmPolicy?: DmPolicy;
/** Allowlist for DM senders (AAD object IDs or UPNs). */
allowFrom?: Array<string>;
/** Optional allowlist for group/channel senders (AAD object IDs or UPNs). */
groupAllowFrom?: Array<string>;
/**
* Controls how group/channel messages are handled:
* - "open": groups bypass allowFrom; mention-gating applies
* - "disabled": block all group messages
* - "allowlist": only allow group messages from senders in groupAllowFrom/allowFrom
*/
groupPolicy?: GroupPolicy;
/** Outbound text chunk size (chars). Default: 4000. */
textChunkLimit?: number;
/** Merge streamed block replies before sending. */

View File

@@ -619,6 +619,8 @@ const MSTeamsConfigSchema = z
.optional(),
dmPolicy: DmPolicySchema.optional().default("pairing"),
allowFrom: z.array(z.string()).optional(),
groupAllowFrom: z.array(z.string()).optional(),
groupPolicy: GroupPolicySchema.optional().default("allowlist"),
textChunkLimit: z.number().int().positive().optional(),
blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
mediaAllowHosts: z.array(z.string()).optional(),

View File

@@ -39,6 +39,7 @@ import {
import type { MSTeamsAdapter } from "./messenger.js";
import type { MSTeamsMonitorLogger } from "./monitor-types.js";
import {
isMSTeamsGroupAllowed,
resolveMSTeamsReplyPolicy,
resolveMSTeamsRouteConfig,
} from "./policy.js";
@@ -176,6 +177,9 @@ function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
const senderName = from.name ?? from.id;
const senderId = from.aadObjectId ?? from.id;
const storedAllowFrom = await readProviderAllowFromStore("msteams").catch(
() => [],
);
// Check DM policy for direct messages
if (isDirectMessage && msteamsCfg) {
@@ -189,7 +193,6 @@ function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
if (dmPolicy !== "open") {
// Check allowlist - look up from config and pairing store
const storedAllowFrom = await readProviderAllowFromStore("msteams");
const effectiveAllowFrom = [
...allowFrom.map((v) => String(v).toLowerCase()),
...storedAllowFrom,
@@ -225,6 +228,49 @@ function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
}
}
if (!isDirectMessage && msteamsCfg) {
const groupPolicy = msteamsCfg.groupPolicy ?? "allowlist";
const groupAllowFrom =
msteamsCfg.groupAllowFrom ??
(msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0
? msteamsCfg.allowFrom
: []);
const effectiveGroupAllowFrom = [
...groupAllowFrom.map((v) => String(v)),
...storedAllowFrom,
];
if (groupPolicy === "disabled") {
log.debug("dropping group message (groupPolicy: disabled)", {
conversationId,
});
return;
}
if (groupPolicy === "allowlist") {
if (effectiveGroupAllowFrom.length === 0) {
log.debug(
"dropping group message (groupPolicy: allowlist, no groupAllowFrom)",
{ conversationId },
);
return;
}
const allowed = isMSTeamsGroupAllowed({
groupPolicy,
allowFrom: effectiveGroupAllowFrom,
senderId,
senderName,
});
if (!allowed) {
log.debug("dropping group message (not in groupAllowFrom)", {
sender: senderId,
label: senderName,
});
return;
}
}
}
// Build conversation reference for proactive replies
const agent = activity.recipient;
const teamId = activity.channelData?.team?.id;

View File

@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import type { MSTeamsConfig } from "../config/types.js";
import {
isMSTeamsGroupAllowed,
resolveMSTeamsReplyPolicy,
resolveMSTeamsRouteConfig,
} from "./policy.js";
@@ -96,4 +97,72 @@ describe("msteams policy", () => {
expect(policy).toEqual({ requireMention: false, replyStyle: "thread" });
});
});
describe("isMSTeamsGroupAllowed", () => {
it("allows when policy is open", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "open",
allowFrom: [],
senderId: "user-id",
senderName: "User",
}),
).toBe(true);
});
it("blocks when policy is disabled", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "disabled",
allowFrom: ["user-id"],
senderId: "user-id",
senderName: "User",
}),
).toBe(false);
});
it("blocks allowlist when empty", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "allowlist",
allowFrom: [],
senderId: "user-id",
senderName: "User",
}),
).toBe(false);
});
it("allows allowlist when sender matches", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "allowlist",
allowFrom: ["User-Id"],
senderId: "user-id",
senderName: "User",
}),
).toBe(true);
});
it("allows allowlist when sender name matches", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "allowlist",
allowFrom: ["user"],
senderId: "other",
senderName: "User",
}),
).toBe(true);
});
it("allows allowlist wildcard", () => {
expect(
isMSTeamsGroupAllowed({
groupPolicy: "allowlist",
allowFrom: ["*"],
senderId: "other",
senderName: "User",
}),
).toBe(true);
});
});
});

View File

@@ -1,4 +1,5 @@
import type {
GroupPolicy,
MSTeamsChannelConfig,
MSTeamsConfig,
MSTeamsReplyStyle,
@@ -56,3 +57,25 @@ export function resolveMSTeamsReplyPolicy(params: {
return { requireMention, replyStyle };
}
export function isMSTeamsGroupAllowed(params: {
groupPolicy: GroupPolicy;
allowFrom: Array<string | number>;
senderId: string;
senderName?: string | null;
}): boolean {
const { groupPolicy } = params;
if (groupPolicy === "disabled") return false;
if (groupPolicy === "open") return true;
const allowFrom = params.allowFrom
.map((entry) => String(entry).trim().toLowerCase())
.filter(Boolean);
if (allowFrom.length === 0) return false;
if (allowFrom.includes("*")) return true;
const senderId = params.senderId.toLowerCase();
const senderName = params.senderName?.toLowerCase();
return (
allowFrom.includes(senderId) ||
(senderName ? allowFrom.includes(senderName) : false)
);
}

View File

@@ -80,6 +80,15 @@ export const msteamsPlugin: ProviderPlugin<ResolvedMSTeamsAccount> = {
.filter(Boolean)
.map((entry) => entry.toLowerCase()),
},
security: {
collectWarnings: ({ cfg }) => {
const groupPolicy = cfg.msteams?.groupPolicy ?? "allowlist";
if (groupPolicy !== "open") return [];
return [
`- MS Teams groups: groupPolicy="open" allows any member to trigger (mention-gated). Set msteams.groupPolicy="allowlist" + msteams.groupAllowFrom to restrict senders.`,
];
},
},
setup: {
resolveAccountId: () => DEFAULT_ACCOUNT_ID,
applyAccountConfig: ({ cfg }) => ({