fix: gate Teams media auth retries

steipete · steipete · commit 41cc5bcd4f1d · 2026-02-02T02:08:13.000-08:00
diff --git a/docs/channels/msteams.md b/docs/channels/msteams.md
@@ -456,6 +456,7 @@ Key settings (see `/gateway/configuration` for shared channel patterns):
 - `channels.msteams.textChunkLimit`: outbound text chunk size.
 - `channels.msteams.chunkMode`: `length` (default) or `newline` to split on blank lines (paragraph boundaries) before length chunking.
 - `channels.msteams.mediaAllowHosts`: allowlist for inbound attachment hosts (defaults to Microsoft/Teams domains).
+- `channels.msteams.mediaAuthAllowHosts`: allowlist for attaching Authorization headers on media retries (defaults to Graph + Bot Framework hosts).
 - `channels.msteams.requireMention`: require @mention in channels/groups (default true).
 - `channels.msteams.replyStyle`: `thread | top-level` (see [Reply Style](#reply-style-threads-vs-posts)).
 - `channels.msteams.teams.<teamId>.replyStyle`: per-team override.
@@ -518,6 +519,7 @@ Teams recently introduced two channel UI styles over the same underlying data mo
 
 Without Graph permissions, channel messages with images will be received as text-only (the image content is not accessible to the bot).
 By default, OpenClaw only downloads media from Microsoft/Teams hostnames. Override with `channels.msteams.mediaAllowHosts` (use `["*"]` to allow any host).
+Authorization headers are only attached for hosts in `channels.msteams.mediaAuthAllowHosts` (defaults to Graph + Bot Framework hosts). Keep this list strict (avoid multi-tenant suffixes).
 
 ## Sending files in group chats
 
diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts
@@ -241,6 +241,7 @@ describe("msteams attachments", () => {
         maxBytes: 1024 * 1024,
         tokenProvider: { getAccessToken: vi.fn(async () => "token") },
         allowHosts: ["x"],
+        authAllowHosts: ["x"],
         fetchFn: fetchMock as unknown as typeof fetch,
       });
 
@@ -249,6 +250,41 @@ describe("msteams attachments", () => {
       expect(fetchMock).toHaveBeenCalledTimes(2);
     });
 
+    it("skips auth retries when the host is not in auth allowlist", async () => {
+      const { downloadMSTeamsAttachments } = await load();
+      const tokenProvider = { getAccessToken: vi.fn(async () => "token") };
+      const fetchMock = vi.fn(async (_url: string, opts?: RequestInit) => {
+        const hasAuth = Boolean(
+          opts &&
+            typeof opts === "object" &&
+            "headers" in opts &&
+            (opts.headers as Record<string, string>)?.Authorization,
+        );
+        if (!hasAuth) {
+          return new Response("forbidden", { status: 403 });
+        }
+        return new Response(Buffer.from("png"), {
+          status: 200,
+          headers: { "content-type": "image/png" },
+        });
+      });
+
+      const media = await downloadMSTeamsAttachments({
+        attachments: [
+          { contentType: "image/png", contentUrl: "https://attacker.azureedge.net/img" },
+        ],
+        maxBytes: 1024 * 1024,
+        tokenProvider,
+        allowHosts: ["azureedge.net"],
+        authAllowHosts: ["graph.microsoft.com"],
+        fetchFn: fetchMock as unknown as typeof fetch,
+      });
+
+      expect(media).toHaveLength(0);
+      expect(fetchMock).toHaveBeenCalledTimes(1);
+      expect(tokenProvider.getAccessToken).not.toHaveBeenCalled();
+    });
+
     it("skips urls outside the allowlist", async () => {
       const { downloadMSTeamsAttachments } = await load();
       const fetchMock = vi.fn();
diff --git a/extensions/msteams/src/attachments/download.ts b/extensions/msteams/src/attachments/download.ts
@@ -11,6 +11,7 @@ import {
   isRecord,
   isUrlAllowed,
   normalizeContentType,
+  resolveAuthAllowedHosts,
   resolveAllowedHosts,
 } from "./shared.js";
 
@@ -85,6 +86,8 @@ async function fetchWithAuthFallback(params: {
   url: string;
   tokenProvider?: MSTeamsAccessTokenProvider;
   fetchFn?: typeof fetch;
+  allowHosts: string[];
+  authAllowHosts: string[];
 }): Promise<Response> {
   const fetchFn = params.fetchFn ?? fetch;
   const firstAttempt = await fetchFn(params.url);
@@ -97,17 +100,40 @@ async function fetchWithAuthFallback(params: {
   if (firstAttempt.status !== 401 && firstAttempt.status !== 403) {
     return firstAttempt;
   }
+  if (!isUrlAllowed(params.url, params.authAllowHosts)) {
+    return firstAttempt;
+  }
 
   const scopes = scopeCandidatesForUrl(params.url);
   for (const scope of scopes) {
     try {
       const token = await params.tokenProvider.getAccessToken(scope);
       const res = await fetchFn(params.url, {
         headers: { Authorization: `Bearer ${token}` },
+        redirect: "manual",
       });
       if (res.ok) {
         return res;
       }
+      const redirectUrl = readRedirectUrl(params.url, res);
+      if (redirectUrl && isUrlAllowed(redirectUrl, params.allowHosts)) {
+        const redirectRes = await fetchFn(redirectUrl);
+        if (redirectRes.ok) {
+          return redirectRes;
+        }
+        if (
+          (redirectRes.status === 401 || redirectRes.status === 403) &&
+          isUrlAllowed(redirectUrl, params.authAllowHosts)
+        ) {
+          const redirectAuthRes = await fetchFn(redirectUrl, {
+            headers: { Authorization: `Bearer ${token}` },
+            redirect: "manual",
+          });
+          if (redirectAuthRes.ok) {
+            return redirectAuthRes;
+          }
+        }
+      }
     } catch {
       // Try the next scope.
     }
@@ -116,6 +142,21 @@ async function fetchWithAuthFallback(params: {
   return firstAttempt;
 }
 
+function readRedirectUrl(baseUrl: string, res: Response): string | null {
+  if (![301, 302, 303, 307, 308].includes(res.status)) {
+    return null;
+  }
+  const location = res.headers.get("location");
+  if (!location) {
+    return null;
+  }
+  try {
+    return new URL(location, baseUrl).toString();
+  } catch {
+    return null;
+  }
+}
+
 /**
  * Download all file attachments from a Teams message (images, documents, etc.).
  * Renamed from downloadMSTeamsImageAttachments to support all file types.
@@ -125,6 +166,7 @@ export async function downloadMSTeamsAttachments(params: {
   maxBytes: number;
   tokenProvider?: MSTeamsAccessTokenProvider;
   allowHosts?: string[];
+  authAllowHosts?: string[];
   fetchFn?: typeof fetch;
   /** When true, embeds original filename in stored path for later extraction. */
   preserveFilenames?: boolean;
@@ -134,6 +176,7 @@ export async function downloadMSTeamsAttachments(params: {
     return [];
   }
   const allowHosts = resolveAllowedHosts(params.allowHosts);
+  const authAllowHosts = resolveAuthAllowedHosts(params.authAllowHosts);
 
   // Download ANY downloadable attachment (not just images)
   const downloadable = list.filter(isDownloadableAttachment);
@@ -199,6 +242,8 @@ export async function downloadMSTeamsAttachments(params: {
         url: candidate.url,
         tokenProvider: params.tokenProvider,
         fetchFn: params.fetchFn,
+        allowHosts,
+        authAllowHosts,
       });
       if (!res.ok) {
         continue;
diff --git a/extensions/msteams/src/attachments/graph.ts b/extensions/msteams/src/attachments/graph.ts
@@ -215,6 +215,7 @@ export async function downloadMSTeamsGraphMedia(params: {
   tokenProvider?: MSTeamsAccessTokenProvider;
   maxBytes: number;
   allowHosts?: string[];
+  authAllowHosts?: string[];
   fetchFn?: typeof fetch;
   /** When true, embeds original filename in stored path for later extraction. */
   preserveFilenames?: boolean;
@@ -336,6 +337,7 @@ export async function downloadMSTeamsGraphMedia(params: {
     maxBytes: params.maxBytes,
     tokenProvider: params.tokenProvider,
     allowHosts,
+    authAllowHosts: params.authAllowHosts,
     fetchFn: params.fetchFn,
     preserveFilenames: params.preserveFilenames,
   });
diff --git a/extensions/msteams/src/attachments/shared.ts b/extensions/msteams/src/attachments/shared.ts
@@ -48,6 +48,15 @@ export const DEFAULT_MEDIA_HOST_ALLOWLIST = [
   "microsoft.com",
 ] as const;
 
+export const DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST = [
+  "api.botframework.com",
+  "botframework.com",
+  "graph.microsoft.com",
+  "graph.microsoft.us",
+  "graph.microsoft.de",
+  "graph.microsoft.cn",
+] as const;
+
 export const GRAPH_ROOT = "https://graph.microsoft.com/v1.0";
 
 export function isRecord(value: unknown): value is Record<string, unknown> {
@@ -250,6 +259,17 @@ export function resolveAllowedHosts(input?: string[]): string[] {
   return normalized;
 }
 
+export function resolveAuthAllowedHosts(input?: string[]): string[] {
+  if (!Array.isArray(input) || input.length === 0) {
+    return DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST.slice();
+  }
+  const normalized = input.map(normalizeAllowHost).filter(Boolean);
+  if (normalized.includes("*")) {
+    return ["*"];
+  }
+  return normalized;
+}
+
 function isHostAllowed(host: string, allowlist: string[]): boolean {
   if (allowlist.includes("*")) {
     return true;
diff --git a/extensions/msteams/src/monitor-handler/inbound-media.ts b/extensions/msteams/src/monitor-handler/inbound-media.ts
@@ -18,6 +18,7 @@ export async function resolveMSTeamsInboundMedia(params: {
   htmlSummary?: MSTeamsHtmlAttachmentSummary;
   maxBytes: number;
   allowHosts?: string[];
+  authAllowHosts?: string[];
   tokenProvider: MSTeamsAccessTokenProvider;
   conversationType: string;
   conversationId: string;
@@ -46,6 +47,7 @@ export async function resolveMSTeamsInboundMedia(params: {
     maxBytes,
     tokenProvider,
     allowHosts,
+    authAllowHosts: params.authAllowHosts,
     preserveFilenames,
   });
 
@@ -85,6 +87,7 @@ export async function resolveMSTeamsInboundMedia(params: {
             tokenProvider,
             maxBytes,
             allowHosts,
+            authAllowHosts: params.authAllowHosts,
             preserveFilenames,
           });
           attempts.push({
diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts
@@ -403,6 +403,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
       maxBytes: mediaMaxBytes,
       tokenProvider,
       allowHosts: msteamsCfg?.mediaAllowHosts,
+      authAllowHosts: msteamsCfg?.mediaAuthAllowHosts,
       conversationType,
       conversationId,
       conversationMessageId: conversationMessageId ?? undefined,
diff --git a/src/config/types.msteams.ts b/src/config/types.msteams.ts
@@ -83,6 +83,11 @@ export type MSTeamsConfig = {
    * Use ["*"] to allow any host (not recommended).
    */
   mediaAllowHosts?: Array<string>;
+  /**
+   * Allowed host suffixes for attaching Authorization headers to inbound media retries.
+   * Use specific hosts only; avoid multi-tenant suffixes.
+   */
+  mediaAuthAllowHosts?: Array<string>;
   /** Default: require @mention to respond in channels/groups. */
   requireMention?: boolean;
   /** Max group/channel messages to keep as history context (0 disables). */
diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts
@@ -800,6 +800,7 @@ export const MSTeamsConfigSchema = z
     chunkMode: z.enum(["length", "newline"]).optional(),
     blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
     mediaAllowHosts: z.array(z.string()).optional(),
+    mediaAuthAllowHosts: z.array(z.string()).optional(),
     requireMention: z.boolean().optional(),
     historyLimit: z.number().int().min(0).optional(),
     dmHistoryLimit: z.number().int().min(0).optional(),
