fix(msteams): harden security-sensitive flows (#65841)

* fix(msteams): validate participant graph params

* fix(msteams): restore media fetch ip guard

* fix(msteams): open delegated auth urls without shell

extensions/msteams/src/attachments.test.ts

@@ -208,6 +208,7 @@ const _createGraphCollectionResponse = (value: unknown[]) => createJsonResponse(
 const createNotFoundResponse = () => new Response("not found", { status: 404 });
 const createRedirectResponse = (location: string, status = 302) =>
   new Response(null, { status, headers: { location } });
+const publicResolve = async () => ({ address: "13.107.136.10" });
 
 const createOkFetchMock = (contentType: string, payload = "png") =>
   vi.fn(async (_input: RequestInfo | URL, _init?: RequestInit) =>
@@ -223,6 +224,7 @@ const buildDownloadParams = (
     attachments,
     maxBytes: DEFAULT_MAX_BYTES,
     allowHosts: DEFAULT_ALLOW_HOSTS,
+    resolveFn: publicResolve,
     ...overrides,
   };
 };

extensions/msteams/src/attachments/bot-framework.ts

@@ -6,6 +6,7 @@ import {
   isUrlAllowed,
   type MSTeamsAttachmentDownloadLogger,
   type MSTeamsAttachmentFetchPolicy,
+  type MSTeamsAttachmentResolveFn,
   resolveAttachmentFetchPolicy,
   safeFetchWithPolicy,
 } from "./shared.js";
@@ -59,6 +60,7 @@ async function fetchBotFrameworkAttachmentInfo(params: {
   accessToken: string;
   policy: MSTeamsAttachmentFetchPolicy;
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   logger?: MSTeamsAttachmentDownloadLogger;
 }): Promise<BotFrameworkAttachmentInfo | undefined> {
   const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}`;
@@ -75,6 +77,7 @@ async function fetchBotFrameworkAttachmentInfo(params: {
       url,
       policy: params.policy,
       fetchFn: params.fetchFn,
+      resolveFn: params.resolveFn,
       requestInit: {
         headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
       },
@@ -109,6 +112,7 @@ async function fetchBotFrameworkAttachmentView(params: {
   maxBytes: number;
   policy: MSTeamsAttachmentFetchPolicy;
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   logger?: MSTeamsAttachmentDownloadLogger;
 }): Promise<Buffer | undefined> {
   const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}/views/${encodeURIComponent(params.viewId)}`;
@@ -120,6 +124,7 @@ async function fetchBotFrameworkAttachmentView(params: {
       url,
       policy: params.policy,
       fetchFn: params.fetchFn,
+      resolveFn: params.resolveFn,
       requestInit: {
         headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
       },
@@ -169,6 +174,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
   allowHosts?: string[];
   authAllowHosts?: string[];
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   fileNameHint?: string | null;
   contentTypeHint?: string | null;
   preserveFilenames?: boolean;
@@ -205,6 +211,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
     accessToken,
     policy,
     fetchFn: params.fetchFn,
+    resolveFn: params.resolveFn,
     logger: params.logger,
   });
   if (!info) {
@@ -239,6 +246,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
     maxBytes: params.maxBytes,
     policy,
     fetchFn: params.fetchFn,
+    resolveFn: params.resolveFn,
     logger: params.logger,
   });
   if (!buffer) {
@@ -296,6 +304,7 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
   allowHosts?: string[];
   authAllowHosts?: string[];
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   fileNameHint?: string | null;
   contentTypeHint?: string | null;
   preserveFilenames?: boolean;
@@ -329,6 +338,7 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
         allowHosts: params.allowHosts,
         authAllowHosts: params.authAllowHosts,
         fetchFn: params.fetchFn,
+        resolveFn: params.resolveFn,
         fileNameHint: params.fileNameHint,
         contentTypeHint: params.contentTypeHint,
         preserveFilenames: params.preserveFilenames,

extensions/msteams/src/attachments/download.ts

@@ -12,6 +12,7 @@ import {
   isUrlAllowed,
   type MSTeamsAttachmentDownloadLogger,
   type MSTeamsAttachmentFetchPolicy,
+  type MSTeamsAttachmentResolveFn,
   normalizeContentType,
   resolveMediaSsrfPolicy,
   resolveAttachmentFetchPolicy,
@@ -111,6 +112,7 @@ async function fetchWithAuthFallback(params: {
   tokenProvider?: MSTeamsAccessTokenProvider;
   fetchFn?: typeof fetch;
   requestInit?: RequestInit;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   policy: MSTeamsAttachmentFetchPolicy;
 }): Promise<Response> {
   const firstAttempt = await safeFetchWithPolicy({
@@ -118,6 +120,7 @@ async function fetchWithAuthFallback(params: {
     policy: params.policy,
     fetchFn: params.fetchFn,
     requestInit: params.requestInit,
+    resolveFn: params.resolveFn,
   });
   if (firstAttempt.ok) {
     return firstAttempt;
@@ -147,6 +150,7 @@ async function fetchWithAuthFallback(params: {
           ...params.requestInit,
           headers: authHeaders,
         },
+        resolveFn: params.resolveFn,
       });
       if (authAttempt.ok) {
         return authAttempt;
@@ -178,6 +182,7 @@ export async function downloadMSTeamsAttachments(params: {
   allowHosts?: string[];
   authAllowHosts?: string[];
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   /** When true, embeds original filename in stored path for later extraction. */
   preserveFilenames?: boolean;
   /**
@@ -282,6 +287,7 @@ export async function downloadMSTeamsAttachments(params: {
             tokenProvider: params.tokenProvider,
             fetchFn: params.fetchFn,
             requestInit: init,
+            resolveFn: params.resolveFn,
             policy,
           }),
       });

extensions/msteams/src/attachments/graph.ts

@@ -18,6 +18,7 @@ import {
   isUrlAllowed,
   type MSTeamsAttachmentDownloadLogger,
   type MSTeamsAttachmentFetchPolicy,
+  type MSTeamsAttachmentResolveFn,
   normalizeContentType,
   resolveMediaSsrfPolicy,
   resolveAttachmentFetchPolicy,
@@ -280,6 +281,7 @@ export async function downloadMSTeamsGraphMedia(params: {
   allowHosts?: string[];
   authAllowHosts?: string[];
   fetchFn?: typeof fetch;
+  resolveFn?: MSTeamsAttachmentResolveFn;
   /** When true, embeds original filename in stored path for later extraction. */
   preserveFilenames?: boolean;
   /** Optional logger used to surface Graph/SharePoint fetch errors. */
@@ -394,6 +396,7 @@ export async function downloadMSTeamsGraphMedia(params: {
                     ...init,
                     headers,
                   },
+                  resolveFn: params.resolveFn,
                 });
               },
             });
@@ -459,6 +462,7 @@ export async function downloadMSTeamsGraphMedia(params: {
       allowHosts: policy.allowHosts,
       authAllowHosts: policy.authAllowHosts,
       fetchFn: params.fetchFn,
+      resolveFn: params.resolveFn,
       preserveFilenames: params.preserveFilenames,
       logger: params.logger,
     });

extensions/msteams/src/attachments/shared.test.ts

@@ -254,6 +254,18 @@ describe("safeFetch", () => {
     expect(fetchMock).not.toHaveBeenCalled();
   });
 
+  it("blocks private hosts with the default resolver", async () => {
+    const fetchMock = vi.fn();
+    await expect(
+      safeFetch({
+        url: "https://localhost/file.pdf",
+        allowHosts: ["localhost"],
+        fetchFn: fetchMock as unknown as typeof fetch,
+      }),
+    ).rejects.toThrow("Initial download URL blocked");
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
   it("blocks when initial URL DNS resolution fails", async () => {
     const fetchMock = vi.fn();
     await expect(

extensions/msteams/src/attachments/shared.ts

@@ -402,6 +402,8 @@ export type MSTeamsAttachmentDownloadLogger = {
   error?: (message: string, meta?: Record<string, unknown>) => void;
 };
 
+export type MSTeamsAttachmentResolveFn = (hostname: string) => Promise<{ address: string }>;
+
 export function resolveAttachmentFetchPolicy(params?: {
   allowHosts?: string[];
   authAllowHosts?: string[];
@@ -453,7 +455,7 @@ export const isPrivateOrReservedIP: (ip: string) => boolean = isPrivateIpAddress
  */
 export async function resolveAndValidateIP(
   hostname: string,
-  resolveFn?: (hostname: string) => Promise<{ address: string }>,
+  resolveFn?: MSTeamsAttachmentResolveFn,
 ): Promise<string> {
   const resolve = resolveFn ?? lookup;
   let resolved: { address: string };
@@ -490,10 +492,10 @@ export async function safeFetch(params: {
   authorizationAllowHosts?: string[];
   fetchFn?: typeof fetch;
   requestInit?: RequestInit;
-  resolveFn?: (hostname: string) => Promise<{ address: string }>;
+  resolveFn?: MSTeamsAttachmentResolveFn;
 }): Promise<Response> {
   const fetchFn = params.fetchFn ?? fetch;
-  const resolveFn = params.resolveFn;
+  const resolveFn = params.resolveFn ?? lookup;
   const hasDispatcher = Boolean(
     params.requestInit &&
     typeof params.requestInit === "object" &&
@@ -577,7 +579,7 @@ export async function safeFetchWithPolicy(params: {
   policy: MSTeamsAttachmentFetchPolicy;
   fetchFn?: typeof fetch;
   requestInit?: RequestInit;
-  resolveFn?: (hostname: string) => Promise<{ address: string }>;
+  resolveFn?: MSTeamsAttachmentResolveFn;
 }): Promise<Response> {
   return await safeFetch({
     url: params.url,

extensions/msteams/src/graph-group-management.test.ts

@@ -86,6 +86,38 @@ describe("addParticipantMSTeams", () => {
     });
   });
 
+  it("normalizes role casing and whitespace", async () => {
+    mockState.postGraphJson.mockResolvedValue({});
+
+    await addParticipantMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: CHAT_ID,
+      userId: "user-aad-id-2",
+      role: " OWNER ",
+    });
+
+    expect(mockState.postGraphJson).toHaveBeenCalledWith(
+      expect.objectContaining({
+        body: expect.objectContaining({
+          roles: ["owner"],
+        }),
+      }),
+    );
+  });
+
+  it("rejects unknown roles", async () => {
+    await expect(
+      addParticipantMSTeams({
+        cfg: {} as OpenClawConfig,
+        to: CHAT_ID,
+        userId: "user-aad-id-2",
+        role: "admin",
+      }),
+    ).rejects.toThrow('role must be "member" or "owner"');
+
+    expect(mockState.postGraphJson).not.toHaveBeenCalled();
+  });
+
   it("constructs correct user@odata.bind URL", async () => {
     mockState.postGraphJson.mockResolvedValue({});
 
@@ -101,6 +133,21 @@ describe("addParticipantMSTeams", () => {
     );
   });
 
+  it("escapes user ids before building the OData bind URL", async () => {
+    mockState.postGraphJson.mockResolvedValue({});
+
+    await addParticipantMSTeams({
+      cfg: {} as OpenClawConfig,
+      to: CHAT_ID,
+      userId: "o'hara@example.com",
+    });
+
+    const calledBody = mockState.postGraphJson.mock.calls[0][0].body;
+    expect(calledBody["user@odata.bind"]).toBe(
+      "https://graph.microsoft.com/v1.0/users('o''hara@example.com')",
+    );
+  });
+
   it("adds member to a channel", async () => {
     mockState.postGraphJson.mockResolvedValue({});
 

extensions/msteams/src/graph-group-management.ts

@@ -2,6 +2,7 @@ import type { OpenClawConfig } from "../runtime-api.js";
 import { resolveConversationPath, resolveGraphConversationId } from "./graph-messages.js";
 import {
   deleteGraphRequest,
+  escapeOData,
   fetchGraphJson,
   patchGraphJson,
   postGraphJson,
@@ -23,6 +24,19 @@ export type AddParticipantMSTeamsResult = {
   added: { userId: string; chatId: string };
 };
 
+type ConversationMemberRole = "member" | "owner";
+
+function normalizeConversationMemberRole(role: string | undefined): ConversationMemberRole {
+  const normalized = role?.trim().toLowerCase() ?? "";
+  if (!normalized) {
+    return "member";
+  }
+  if (normalized === "member" || normalized === "owner") {
+    return normalized;
+  }
+  throw new Error('MS Teams participant role must be "member" or "owner".');
+}
+
 /**
  * Add a user to a chat or channel via Graph API.
  */
@@ -35,8 +49,8 @@ export async function addParticipantMSTeams(
 
   const body = {
     "@odata.type": "#microsoft.graph.aadUserConversationMember",
-    roles: [params.role || "member"],
-    "user@odata.bind": `https://graph.microsoft.com/v1.0/users('${params.userId}')`,
+    roles: [normalizeConversationMemberRole(params.role)],
+    "user@odata.bind": `https://graph.microsoft.com/v1.0/users('${escapeOData(params.userId)}')`,
   };
 
   await postGraphJson<unknown>({

extensions/msteams/src/setup-surface.test.ts

@@ -1,7 +1,10 @@
+import { EventEmitter } from "node:events";
 import { DEFAULT_ACCOUNT_ID } from "openclaw/plugin-sdk/setup";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { createMSTeamsSetupWizardBase, msteamsSetupAdapter } from "./setup-core.js";
+import { openDelegatedOAuthUrl } from "./setup-surface.js";
 
+const spawn = vi.hoisted(() => vi.fn());
 const resolveMSTeamsUserAllowlist = vi.hoisted(() => vi.fn());
 const resolveMSTeamsChannelAllowlist = vi.hoisted(() => vi.fn());
 const normalizeSecretInputString = vi.hoisted(() =>
@@ -25,10 +28,19 @@ vi.mock("./token.js", () => ({
   resolveMSTeamsCredentials,
 }));
 
+vi.mock("node:child_process", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:child_process")>();
+  return {
+    ...actual,
+    spawn,
+  };
+});
+
 describe("msteams setup surface", () => {
   const msteamsSetupWizard = createMSTeamsSetupWizardBase();
 
   beforeEach(() => {
+    spawn.mockReset();
     resolveMSTeamsUserAllowlist.mockReset();
     resolveMSTeamsChannelAllowlist.mockReset();
     normalizeSecretInputString.mockClear();
@@ -46,6 +58,21 @@ describe("msteams setup surface", () => {
     );
   });
 
+  it("opens delegated OAuth URLs without invoking a shell", async () => {
+    const url = "https://login.microsoftonline.com/auth?state=$(touch pwned)";
+    const child = new EventEmitter();
+    spawn.mockReturnValue(child);
+
+    const result = openDelegatedOAuthUrl(url);
+    child.emit("exit", 0, null);
+
+    await expect(result).resolves.toBeUndefined();
+    expect(spawn).toHaveBeenCalledWith(process.platform === "darwin" ? "open" : "xdg-open", [url], {
+      stdio: "ignore",
+      shell: false,
+    });
+  });
+
   it("enables the msteams channel without dropping existing config", () => {
     expect(
       msteamsSetupAdapter.applyAccountConfig?.({

extensions/msteams/src/setup-surface.ts

@@ -1,4 +1,4 @@
-import { exec } from "node:child_process";
+import { spawn } from "node:child_process";
 import {
   createTopLevelChannelAllowFromSetter,
   createTopLevelChannelDmPolicy,
@@ -29,6 +29,22 @@ const setMSTeamsGroupPolicy = createTopLevelChannelGroupPolicySetter({
   enabled: true,
 });
 
+export function openDelegatedOAuthUrl(url: string): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    const cmd = process.platform === "darwin" ? "open" : "xdg-open";
+    const child = spawn(cmd, [url], { stdio: "ignore", shell: false });
+    child.once("error", reject);
+    child.once("exit", (code, signal) => {
+      if (code === 0) {
+        resolve();
+        return;
+      }
+      const reason = signal ? `signal ${signal}` : `code ${code ?? "unknown"}`;
+      reject(new Error(`${cmd} failed with ${reason}`));
+    });
+  });
+}
+
 function looksLikeGuid(value: string): boolean {
   return /^[0-9a-fA-F-]{16,}$/.test(value);
 }
@@ -270,11 +286,7 @@ export const msteamsSetupWizard: ChannelSetupWizard = {
           const tokens = await loginMSTeamsDelegated(
             {
               isRemote: shouldUseManualOAuthFlow(isRemote),
-              openUrl: (url) =>
-                new Promise<void>((resolve, reject) => {
-                  const cmd = process.platform === "darwin" ? "open" : "xdg-open";
-                  exec(`${cmd} ${JSON.stringify(url)}`, (err) => (err ? reject(err) : resolve()));
-                }),
+              openUrl: openDelegatedOAuthUrl,
               log: (msg) => params.prompter.note(msg),
               note: (msg, title) => params.prompter.note(msg, title),
               prompt: (msg) => params.prompter.text({ message: msg }),