fix(msteams): bound Graph attachment JSON responses (#101082)

* fix(extensions/msteams): bound Graph collection JSON response read to prevent OOM * fix(msteams): bound Graph message JSON reads * test(msteams): exercise Graph overflow response * refactor(msteams): stream Graph hosted content --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-07-10 20:36:09 +00:00 · 2026-07-07 08:18:09 +08:00
parent bce414d8b8
commit 090c3a26d0
3 changed files with 150 additions and 116 deletions
--- a/extensions/msteams/src/attachments.graph.test.ts
+++ b/extensions/msteams/src/attachments.graph.test.ts
@@ -73,6 +73,9 @@ const saveResponseMediaMock = vi.fn(
    },
  ) => {
    const buffer = Buffer.from(await res.arrayBuffer());
+    if (options.maxBytes !== undefined && buffer.byteLength > options.maxBytes) {
+      throw new Error(`payload exceeds maxBytes ${options.maxBytes}`);
+    }
    return await saveMediaBufferMock(
      buffer,
      options.fallbackContentType,
@@ -159,7 +162,7 @@ const expectMediaBufferSaved = () => {
 };

 const createHostedContentsWithType = (contentType: string, ...ids: string[]) =>
-  ids.map((id) => ({ id, contentType, contentBytes: PNG_BUFFER.toString("base64") }));
+  ids.map((id) => ({ id, contentType }));
 const createHostedImageContents = (...ids: string[]) =>
  createHostedContentsWithType(CONTENT_TYPE_IMAGE_PNG, ...ids);
 const createReferenceAttachment = (shareUrl = DEFAULT_SHARE_REFERENCE_URL) => ({
@@ -263,7 +266,13 @@ const runGraphMediaSuccessCase = async ({

 const GRAPH_MEDIA_SUCCESS_CASES: GraphMediaSuccessCase[] = [
  withLabel("downloads hostedContents images", {
-    buildOptions: () => ({ hostedContents: createHostedImageContents("1") }),
+    buildOptions: () => ({
+      hostedContents: createHostedImageContents("1"),
+      onUnhandled: (url) =>
+        url.endsWith("/hostedContents/1/$value")
+          ? createBufferResponse(PNG_BUFFER, CONTENT_TYPE_IMAGE_PNG)
+          : undefined,
+    }),
    expectedLength: 1,
    assert: ({ fetchMock }) => {
      expect(fetchMock).toHaveBeenCalled();
@@ -288,6 +297,10 @@ const GRAPH_MEDIA_SUCCESS_CASES: GraphMediaSuccessCase[] = [
        hostedContents: createHostedImageContents("hosted-1"),
        ...buildDefaultShareReferenceGraphFetchOptions({
          onShareRequest: () => createPdfResponse(),
+          onUnhandled: (url) =>
+            url.endsWith("/hostedContents/hosted-1/$value")
+              ? createBufferResponse(PNG_BUFFER, CONTENT_TYPE_IMAGE_PNG)
+              : undefined,
        }),
      };
    },
@@ -388,28 +401,19 @@ describe("msteams graph attachments", () => {
    expect(calledUrls).not.toContain(escapedUrl);
  });

-  it("skips inline hosted content when estimated decoded bytes exceed maxBytes", async () => {
-    const oversizedBase64 = "A".repeat(16);
-    const bufferFromSpy = vi.spyOn(Buffer, "from");
+  it("enforces maxBytes while streaming hosted content", async () => {
+    const { media } = await downloadGraphMediaWithMockOptions(
+      {
+        hostedContents: createHostedImageContents("hosted-oversized"),
+        onUnhandled: (url) =>
+          url.endsWith("/hostedContents/hosted-oversized/$value")
+            ? createBufferResponse("too large", CONTENT_TYPE_IMAGE_PNG)
+            : undefined,
+      },
+      { maxBytes: 4 },
+    );

-    try {
-      const { media } = await downloadGraphMediaWithMockOptions(
-        {
-          hostedContents: [
-            {
-              id: "hosted-oversized",
-              contentType: CONTENT_TYPE_IMAGE_PNG,
-              contentBytes: oversizedBase64,
-            },
-          ],
-        },
-        { maxBytes: 4 },
-      );
-
-      expect(media.media).toStrictEqual([]);
-      expect(bufferFromSpy).not.toHaveBeenCalledWith(oversizedBase64, "base64");
-    } finally {
-      bufferFromSpy.mockRestore();
-    }
+    expect(media.media).toStrictEqual([]);
+    expect(saveResponseMediaMock).toHaveBeenCalledTimes(1);
  });
 });
--- a/extensions/msteams/src/attachments/graph.test.ts
+++ b/extensions/msteams/src/attachments/graph.test.ts
@@ -82,6 +82,10 @@ function mockBinaryResponse(data: Uint8Array, status = 200) {
  return new Response(Buffer.from(data) as BodyInit, { status });
 }

+function oversizedGraphJson(payload: Record<string, unknown>): string {
+  return JSON.stringify({ ...payload, padding: "x".repeat(16 * 1024 * 1024) });
+}
+
 type GuardedFetchParams = { url: string; init?: RequestInit };

 function guardedFetchResult(params: GuardedFetchParams, response: Response) {
@@ -113,6 +117,11 @@ function mockGraphMediaFetch(options: {
  vi.mocked(fetchWithSsrFGuard).mockImplementation(async (params: GuardedFetchParams) => {
    options.fetchCalls?.push(params.url);
    const url = params.url;
+    for (const [fragment, response] of Object.entries(options.valueResponses ?? {})) {
+      if (url.includes(fragment)) {
+        return guardedFetchResult(params, response);
+      }
+    }
    if (url.endsWith(`/messages/${options.messageId}`) && !url.includes("hostedContents")) {
      return guardedFetchResult(
        params,
@@ -122,11 +131,6 @@ function mockGraphMediaFetch(options: {
    if (url.endsWith("/hostedContents")) {
      return guardedFetchResult(params, mockFetchResponse({ value: options.hostedContents ?? [] }));
    }
-    for (const [fragment, response] of Object.entries(options.valueResponses ?? {})) {
-      if (url.includes(fragment)) {
-        return guardedFetchResult(params, response);
-      }
-    }
    return guardedFetchResult(params, mockFetchResponse({}, 404));
  });
 }
@@ -136,14 +140,14 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {
    vi.clearAllMocks();
  });

-  it("fetches $value endpoint when contentBytes is null but item.id exists", async () => {
+  it("fetches hosted bytes from the documented $value endpoint", async () => {
    const imageBytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); // PNG magic bytes

    const fetchCalls: string[] = [];

    mockGraphMediaFetch({
      messageId: "msg-1",
-      hostedContents: [{ id: "hosted-123", contentType: "image/png", contentBytes: null }],
+      hostedContents: [{ id: "hosted-123", contentType: "image/png" }],
      valueResponses: {
        "/hostedContents/hosted-123/$value": mockBinaryResponse(imageBytes),
      },
@@ -164,10 +168,10 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {
    expect(result.hostedCount).toBe(1);
  });

-  it("skips hosted content when contentBytes is null and id is missing", async () => {
+  it("skips hosted content when the list item has no id", async () => {
    mockGraphMediaFetch({
      messageId: "msg-2",
-      hostedContents: [{ contentType: "image/png", contentBytes: null }],
+      hostedContents: [{ contentType: "image/png" }],
    });

    const result = await downloadMSTeamsGraphMedia({
@@ -176,7 +180,7 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {
      maxBytes: 10 * 1024 * 1024,
    });

-    // No media because there's no id to fetch $value from and no contentBytes
+    // No media because there is no id for the required $value fetch.
    expect(result.media).toHaveLength(0);
  });

@@ -185,7 +189,7 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {

    mockGraphMediaFetch({
      messageId: "msg-cl",
-      hostedContents: [{ id: "hosted-big", contentType: "image/png", contentBytes: null }],
+      hostedContents: [{ id: "hosted-big", contentType: "image/png" }],
      valueResponses: {
        "/hostedContents/hosted-big/$value": new Response(
          Buffer.from(new Uint8Array([0x89, 0x50, 0x4e, 0x47])) as BodyInit,
@@ -211,13 +215,46 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {
    expect(result.media).toHaveLength(0);
  });

-  it("uses inline contentBytes when available instead of $value", async () => {
+  it("skips hosted content when the Graph collection response exceeds the byte cap", async () => {
+    const fetchCalls: string[] = [];
+    const hugeBody = oversizedGraphJson({
+      value: [{ id: "hosted-huge", contentType: "image/png" }],
+    });
+    const hugeResponse = new Response(hugeBody, {
+      status: 200,
+      headers: { "content-type": "application/json" },
+    });
+
+    mockGraphMediaFetch({
+      messageId: "msg-huge",
+      valueResponses: {
+        "/hostedContents": hugeResponse,
+      },
+      fetchCalls,
+    });
+
+    const result = await downloadMSTeamsGraphMedia({
+      messageUrl: "https://graph.microsoft.com/v1.0/chats/c/messages/msg-huge",
+      tokenProvider: { getAccessToken: vi.fn(async () => "test-token") },
+      maxBytes: 10 * 1024 * 1024,
+    });
+
+    expect(result.media).toHaveLength(0);
+    expect(result.hostedCount).toBe(0);
+    expect(hugeResponse.bodyUsed).toBe(true);
+  });
+
+  it("ignores unexpected inline bytes and still fetches bounded $value", async () => {
    const fetchCalls: string[] = [];
    const base64Png = Buffer.from([0x89, 0x50, 0x4e, 0x47]).toString("base64");
+    const imageBytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47]);

    mockGraphMediaFetch({
      messageId: "msg-3",
      hostedContents: [{ id: "hosted-456", contentType: "image/png", contentBytes: base64Png }],
+      valueResponses: {
+        "/hostedContents/hosted-456/$value": mockBinaryResponse(imageBytes),
+      },
      fetchCalls,
    });

@@ -227,9 +264,9 @@ describe("downloadMSTeamsGraphMedia hosted content $value fallback", () => {
      maxBytes: 10 * 1024 * 1024,
    });

-    // Should NOT have fetched $value since contentBytes was available
-    const valueCall = fetchCalls.find((u) => u.includes("/$value"));
-    expect(valueCall).toBeUndefined();
+    expect(fetchCalls).toContain(
+      "https://graph.microsoft.com/v1.0/chats/c/messages/msg-3/hostedContents/hosted-456/$value",
+    );
    expect(result.media.length).toBeGreaterThan(0);
  });

@@ -366,6 +403,29 @@ describe("downloadMSTeamsGraphMedia attachment sourcing and error logging", () =
    expect(result.attachmentCount).toBe(1);
  });

+  it("skips message metadata when the Graph response exceeds the byte cap", async () => {
+    mockGraphMediaFetch({
+      messageId: "msg-huge",
+      messageResponse: oversizedGraphJson({ attachments: [] }),
+    });
+    const logger = { warn: vi.fn() };
+
+    const result = await downloadMSTeamsGraphMedia({
+      messageUrl: "https://graph.microsoft.com/v1.0/chats/c/messages/msg-huge",
+      tokenProvider: { getAccessToken: vi.fn(async () => "test-token") },
+      maxBytes: 10 * 1024 * 1024,
+      logger,
+    });
+
+    expect(result.media).toHaveLength(0);
+    expect(result.attachmentCount).toBe(0);
+    const [message, context] = requireFirstMockCall(logger.warn, "message parse warning");
+    expect(message).toBe("msteams graph message parse failed");
+    expect((context as { error?: unknown }).error).toBe(
+      "MS Teams Graph message: JSON response exceeds 16777216 bytes",
+    );
+  });
+
  it("logs a debug event when the message fetch throws instead of swallowing it", async () => {
    // Regression test for #51749: empty `catch {}` blocks used to hide the
    // real error, producing misleading `graph media fetch empty` diagnostics
--- a/extensions/msteams/src/attachments/graph.ts
+++ b/extensions/msteams/src/attachments/graph.ts
@@ -1,4 +1,8 @@
 // Msteams plugin module implements graph behavior.
+import {
+  readProviderJsonArrayFieldResponse,
+  readProviderJsonResponse,
+} from "openclaw/plugin-sdk/provider-http";
 import { fetchWithSsrFGuard, type SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime";
 import {
  normalizeLowercaseStringOrEmpty,
@@ -14,7 +18,6 @@ import {
  applyAuthorizationHeaderForUrl,
  encodeGraphShareId,
  GRAPH_ROOT,
-  estimateBase64DecodedBytes,
  inferPlaceholder,
  readNestedString,
  isUrlAllowed,
@@ -38,7 +41,6 @@ import type {
 type GraphHostedContent = {
  id?: string | null;
  contentType?: string | null;
-  contentBytes?: string | null;
 };

 type GraphAttachment = {
@@ -143,8 +145,12 @@ async function fetchGraphCollection(params: {
      return { status, items: [] };
    }
    try {
-      const data = (await response.json()) as { value?: unknown[] };
-      return { status, items: Array.isArray(data.value) ? data.value : [] };
+      const items = await readProviderJsonArrayFieldResponse(
+        response,
+        "MS Teams Graph collection",
+        "value",
+      );
+      return { status, items };
    } catch {
      return { status, items: [] };
    }
@@ -196,85 +202,46 @@ async function downloadGraphHostedContent(params: {

  const out: MSTeamsInboundMedia[] = [];
  for (const item of hosted.items) {
-    const contentBytes = typeof item.contentBytes === "string" ? item.contentBytes : "";
-    let buffer: Buffer;
-    if (contentBytes) {
-      if (estimateBase64DecodedBytes(contentBytes) > params.maxBytes) {
-        continue;
-      }
-      try {
-        buffer = Buffer.from(contentBytes, "base64");
-      } catch (err) {
-        params.logger?.warn?.("msteams graph hostedContent base64 decode failed", {
-          error: err instanceof Error ? err.message : String(err),
-        });
-        continue;
-      }
-    } else if (item.id) {
-      // contentBytes not inline — fetch from the individual $value endpoint.
-      try {
-        const valueUrl = `${params.messageUrl}/hostedContents/${encodeURIComponent(item.id)}/$value`;
-        const { response: valRes, release } = await fetchWithSsrFGuard({
-          url: valueUrl,
-          fetchImpl: params.fetchFn ?? fetch,
-          init: {
-            headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
-          },
-          policy: params.ssrfPolicy,
-          auditContext: "msteams.graph.hostedContent.value",
-        });
-        try {
-          if (!valRes.ok) {
-            continue;
-          }
-          const saved = await getMSTeamsRuntime().channel.media.saveResponseMedia(valRes, {
-            sourceUrl: valueUrl,
-            maxBytes: params.maxBytes,
-            fallbackContentType: item.contentType ?? undefined,
-            subdir: "inbound",
-          });
-          out.push({
-            path: saved.path,
-            contentType: saved.contentType,
-            placeholder: inferPlaceholder({ contentType: saved.contentType }),
-          });
-        } finally {
-          await release();
-        }
-      } catch (err) {
-        params.logger?.warn?.("msteams graph hostedContent value fetch failed", {
-          error: err instanceof Error ? err.message : String(err),
-        });
-        continue;
-      }
-      continue;
-    } else {
+    if (!item.id) {
      continue;
    }
-    if (buffer.byteLength > params.maxBytes) {
-      continue;
-    }
-    const mime = await getMSTeamsRuntime().media.detectMime({
-      buffer,
-      headerMime: item.contentType ?? undefined,
-    });
-    // Download any file type, not just images
+
+    // Graph's list API returns metadata only; hosted bytes live at `$value`.
+    // Keep the JSON cap independent from the configured binary media limit.
    try {
-      const saved = await getMSTeamsRuntime().channel.media.saveMediaBuffer(
-        buffer,
-        mime ?? item.contentType ?? undefined,
-        "inbound",
-        params.maxBytes,
-      );
-      out.push({
-        path: saved.path,
-        contentType: saved.contentType,
-        placeholder: inferPlaceholder({ contentType: saved.contentType }),
+      const valueUrl = `${params.messageUrl}/hostedContents/${encodeURIComponent(item.id)}/$value`;
+      const { response: valRes, release } = await fetchWithSsrFGuard({
+        url: valueUrl,
+        fetchImpl: params.fetchFn ?? fetch,
+        init: {
+          headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
+        },
+        policy: params.ssrfPolicy,
+        auditContext: "msteams.graph.hostedContent.value",
      });
+      try {
+        if (!valRes.ok) {
+          continue;
+        }
+        const saved = await getMSTeamsRuntime().channel.media.saveResponseMedia(valRes, {
+          sourceUrl: valueUrl,
+          maxBytes: params.maxBytes,
+          fallbackContentType: item.contentType ?? undefined,
+          subdir: "inbound",
+        });
+        out.push({
+          path: saved.path,
+          contentType: saved.contentType,
+          placeholder: inferPlaceholder({ contentType: saved.contentType }),
+        });
+      } finally {
+        await release();
+      }
    } catch (err) {
-      params.logger?.warn?.("msteams graph hostedContent save failed", {
+      params.logger?.warn?.("msteams graph hostedContent value fetch failed", {
        error: err instanceof Error ? err.message : String(err),
      });
+      continue;
    }
  }

@@ -345,7 +312,10 @@ export async function downloadMSTeamsGraphMedia(params: {
          attachments?: GraphAttachment[];
        };
        try {
-          msgData = (await msgRes.json()) as typeof msgData;
+          msgData = await readProviderJsonResponse<typeof msgData>(
+            msgRes,
+            "MS Teams Graph message",
+          );
        } catch (err) {
          debugLog?.debug?.("graph media message parse failed", {
            messageUrl,