Skip to content

Commit f199b81

Browse files
eleqtriziteleqtrizit
committed
fix(msteams): block untrusted Teams service URLs
1 parent 5574f75 commit f199b81

8 files changed

Lines changed: 404 additions & 34 deletions

File tree

‎extensions/msteams/src/bot-framework-service-url.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import {
2+
buildHostnameAllowlistPolicyFromSuffixAllowlist,
3+
isHttpsUrlAllowedByHostnameSuffixAllowlist,
4+
normalizeHostnameSuffixAllowlist,
5+
type SsrFPolicy,
6+
} from "openclaw/plugin-sdk/ssrf-policy";
7+
8+
const DEFAULT_BOT_FRAMEWORK_SERVICE_URL_HOST_ALLOWLIST = [
9+
// Microsoft Teams Bot Framework serviceUrl endpoints documented for
10+
// commercial, GCC, GCC High, and DOD clouds. These are the only hosts that may
11+
// receive Bot Framework service tokens from this plugin.
12+
"smba.trafficmanager.net",
13+
"smba.infra.gcc.teams.microsoft.com",
14+
"smba.infra.gov.teams.microsoft.us",
15+
"smba.infra.dod.teams.microsoft.us",
16+
] as const;
17+
18+
export const BOT_FRAMEWORK_SERVICE_URL_HOST_ALLOWLIST = normalizeHostnameSuffixAllowlist(
19+
DEFAULT_BOT_FRAMEWORK_SERVICE_URL_HOST_ALLOWLIST,
20+
);
21+
22+
const serviceUrlSsrfPolicy = buildHostnameAllowlistPolicyFromSuffixAllowlist(
23+
BOT_FRAMEWORK_SERVICE_URL_HOST_ALLOWLIST,
24+
);
25+
26+
if (!serviceUrlSsrfPolicy) {
27+
throw new Error("Microsoft Teams Bot Framework serviceUrl allowlist is empty");
28+
}
29+
30+
export const BOT_FRAMEWORK_SERVICE_URL_SSRF_POLICY: SsrFPolicy = serviceUrlSsrfPolicy;
31+
32+
export function describeBotFrameworkServiceUrlHost(serviceUrl: string): string {
33+
try {
34+
const parsed = new URL(serviceUrl.trim());
35+
return parsed.hostname || "invalid-url";
36+
} catch {
37+
return "invalid-url";
38+
}
39+
}
40+
41+
export function isAllowedBotFrameworkServiceUrl(serviceUrl: unknown): serviceUrl is string {
42+
if (typeof serviceUrl !== "string") {
43+
return false;
44+
}
45+
const trimmed = serviceUrl.trim();
46+
return Boolean(
47+
trimmed &&
48+
isHttpsUrlAllowedByHostnameSuffixAllowlist(trimmed, BOT_FRAMEWORK_SERVICE_URL_HOST_ALLOWLIST),
49+
);
50+
}
51+
52+
export function tryNormalizeBotFrameworkServiceUrl(serviceUrl: unknown): string | undefined {
53+
if (!isAllowedBotFrameworkServiceUrl(serviceUrl)) {
54+
return undefined;
55+
}
56+
return serviceUrl.trim().replace(/\/+$/, "");
57+
}
58+
59+
export function normalizeBotFrameworkServiceUrl(serviceUrl: string): string {
60+
const normalized = tryNormalizeBotFrameworkServiceUrl(serviceUrl);
61+
if (normalized) {
62+
return normalized;
63+
}
64+
throw new Error(
65+
`Blocked Microsoft Teams serviceUrl host: ${describeBotFrameworkServiceUrlHost(serviceUrl)}`,
66+
);
67+
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import path from "node:path";
22
import { resolveThreadSessionKeys } from "openclaw/plugin-sdk/routing";
33
import { appendRegularFile } from "openclaw/plugin-sdk/security-runtime";
44
import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/string-coerce-runtime";
5+
import { tryNormalizeBotFrameworkServiceUrl } from "./bot-framework-service-url.js";
56
import { formatUnknownError } from "./errors.js";
67
import { buildFeedbackEvent, runFeedbackReflection } from "./feedback-reflection.js";
78
import { respondToMSTeamsFileConsentInvoke } from "./file-consent-invoke.js";
@@ -268,6 +269,7 @@ async function handleFeedbackInvoke(
268269
}
269270

270271
// Build conversation reference for proactive messages (ack + reflection follow-up)
272+
const serviceUrl = tryNormalizeBotFrameworkServiceUrl(activity.serviceUrl);
271273
const conversationRef = {
272274
activityId: activity.id,
273275
user: {
@@ -287,7 +289,7 @@ async function handleFeedbackInvoke(
287289
tenantId: activity.conversation?.tenantId,
288290
},
289291
channelId: activity.channelId ?? "msteams",
290-
serviceUrl: activity.serviceUrl,
292+
...(serviceUrl ? { serviceUrl } : {}),
291293
locale: activity.locale,
292294
};
293295

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ describe("msteams monitor handler authz", () => {
441441
tenantId: "tenant-1",
442442
aadObjectId: "new-user-aad",
443443
channelId: "msteams",
444-
serviceUrl: "https://smba.trafficmanager.net/amer/",
444+
serviceUrl: "https://smba.trafficmanager.net/amer",
445445
locale: "en-US",
446446
timezone: "America/New_York",
447447
});
@@ -507,6 +507,48 @@ describe("msteams monitor handler authz", () => {
507507
expect(storedConversation.tenantId).toBe("tenant-from-channel-data");
508508
});
509509

510+
it("does not persist blocked serviceUrl hosts in conversation references", async () => {
511+
const { conversationStore, deps } = createDeps({
512+
channels: {
513+
msteams: {
514+
dmPolicy: "allowlist",
515+
allowFrom: ["sender-aad"],
516+
},
517+
},
518+
} as OpenClawConfig);
519+
520+
const handler = createMSTeamsMessageHandler(deps);
521+
await handler({
522+
activity: {
523+
id: "msg-blocked-service-url",
524+
type: "message",
525+
text: "hello",
526+
from: {
527+
id: "sender-id",
528+
aadObjectId: "sender-aad",
529+
name: "Sender",
530+
},
531+
recipient: {
532+
id: "bot-id",
533+
name: "Bot",
534+
},
535+
conversation: {
536+
id: "a:personal-chat",
537+
conversationType: "personal",
538+
},
539+
channelId: "msteams",
540+
serviceUrl: "https://attacker.example.com/teams/",
541+
channelData: {},
542+
attachments: [],
543+
},
544+
sendActivity: vi.fn(async () => undefined),
545+
} as unknown as Parameters<typeof handler>[0]);
546+
547+
expect(conversationStore.upsert).toHaveBeenCalledTimes(1);
548+
const storedRef = recordFromMockCall(mockCallArg(conversationStore.upsert, 0, 1));
549+
expect("serviceUrl" in storedRef).toBe(false);
550+
});
551+
510552
it("stores no tenantId when channelData.tenant is missing", async () => {
511553
const { conversationStore, deps } = createDeps({
512554
channels: {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
summarizeMSTeamsHtmlAttachments,
2727
} from "../attachments.js";
2828
import { isRecord } from "../attachments/shared.js";
29+
import { tryNormalizeBotFrameworkServiceUrl } from "../bot-framework-service-url.js";
2930
import type { StoredConversationReference } from "../conversation-store.js";
3031
import { formatUnknownError } from "../errors.js";
3132
import {
@@ -154,6 +155,7 @@ function buildStoredConversationReference(params: {
154155
const channelDataTenantId = activity.channelData?.tenant?.id;
155156
const tenantId = channelDataTenantId ?? conversation?.tenantId;
156157
const aadObjectId = from?.aadObjectId;
158+
const serviceUrl = tryNormalizeBotFrameworkServiceUrl(activity.serviceUrl);
157159
return {
158160
activityId: activity.id,
159161
user: from ? { id: from.id, name: from.name, aadObjectId: from.aadObjectId } : undefined,
@@ -168,7 +170,7 @@ function buildStoredConversationReference(params: {
168170
...(aadObjectId ? { aadObjectId } : {}),
169171
teamId,
170172
channelId: activity.channelId,
171-
serviceUrl: activity.serviceUrl,
173+
...(serviceUrl ? { serviceUrl } : {}),
172174
locale: activity.locale,
173175
...(clientInfo?.timezone ? { timezone: clientInfo.timezone } : {}),
174176
...(threadId ? { threadId } : {}),

0 commit comments

Comments
 (0)