fix: enforce inbound media max-bytes during remote fetch

2026-02-21 23:02:17 +01:00
parent dd41fadcaf
commit 73d93dee64
10 changed files with 207 additions and 77 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
 - Security/Archive: block zip symlink escapes during archive extraction.
 - Security/Discord: add `openclaw security audit` warnings for name/tag-based Discord allowlist entries (DM allowlists, guild/channel `users`, and pairing-store entries), highlighting slug-collision risk while keeping name-based matching supported, and canonicalize resolved Discord allowlist names to IDs at runtime without rewriting config files. Thanks @tdjackey for reporting.
 - Security/Gateway: block node-role connections when device identity metadata is missing.
+- Security/Media: enforce inbound media byte limits during download/read across Discord, Telegram, Zalo, Microsoft Teams, and BlueBubbles to prevent oversized payload memory spikes before rejection. This ships in the next npm release. Thanks @tdjackey for reporting.
 - Chat/Usage/TUI: strip synthetic inbound metadata blocks (including `Conversation info` and trailing `Untrusted context` channel metadata wrappers) from displayed conversation history so internal prompt context no longer leaks into user-visible logs.
 - Security/Browser relay: harden extension relay auth token handling for `/extension` and `/cdp` pathways.
 - Cron: persist `delivered` state in cron job records so delivery failures remain visible in status and logs. (#19174) Thanks @simonemacario.
--- a/extensions/bluebubbles/src/attachments.test.ts
+++ b/extensions/bluebubbles/src/attachments.test.ts
@@ -1,18 +1,60 @@
+import type { PluginRuntime } from "openclaw/plugin-sdk";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import "./test-mocks.js";
 import { downloadBlueBubblesAttachment, sendBlueBubblesAttachment } from "./attachments.js";
 import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
+import { setBlueBubblesRuntime } from "./runtime.js";
 import { installBlueBubblesFetchTestHooks } from "./test-harness.js";
 import type { BlueBubblesAttachment } from "./types.js";

 const mockFetch = vi.fn();
+const fetchRemoteMediaMock = vi.fn(
+  async (params: {
+    url: string;
+    maxBytes?: number;
+    fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
+  }) => {
+    const fetchFn = params.fetchImpl ?? fetch;
+    const res = await fetchFn(params.url);
+    if (!res.ok) {
+      const text = await res.text().catch(() => "unknown");
+      throw new Error(
+        `Failed to fetch media from ${params.url}: HTTP ${res.status}; body: ${text}`,
+      );
+    }
+    const buffer = Buffer.from(await res.arrayBuffer());
+    if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) {
+      throw new Error(`payload exceeds maxBytes ${params.maxBytes}`);
+    }
+    return {
+      buffer,
+      contentType: res.headers.get("content-type") ?? undefined,
+      fileName: undefined,
+    };
+  },
+);

 installBlueBubblesFetchTestHooks({
  mockFetch,
  privateApiStatusMock: vi.mocked(getCachedBlueBubblesPrivateApiStatus),
 });

+const runtimeStub = {
+  channel: {
+    media: {
+      fetchRemoteMedia:
+        fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"],
+    },
+  },
+} as unknown as PluginRuntime;
+
 describe("downloadBlueBubblesAttachment", () => {
+  beforeEach(() => {
+    fetchRemoteMediaMock.mockClear();
+    mockFetch.mockReset();
+    setBlueBubblesRuntime(runtimeStub);
+  });
+
  it("throws when guid is missing", async () => {
    const attachment: BlueBubblesAttachment = {};
    await expect(
@@ -120,7 +162,7 @@ describe("downloadBlueBubblesAttachment", () => {
        serverUrl: "http://localhost:1234",
        password: "test",
      }),
-    ).rejects.toThrow("download failed (404): Attachment not found");
+    ).rejects.toThrow("Attachment not found");
  });

  it("throws when attachment exceeds max bytes", async () => {
@@ -229,6 +271,8 @@ describe("sendBlueBubblesAttachment", () => {
  beforeEach(() => {
    vi.stubGlobal("fetch", mockFetch);
    mockFetch.mockReset();
+    fetchRemoteMediaMock.mockClear();
+    setBlueBubblesRuntime(runtimeStub);
    vi.mocked(getCachedBlueBubblesPrivateApiStatus).mockReset();
    vi.mocked(getCachedBlueBubblesPrivateApiStatus).mockReturnValue(null);
  });
--- a/extensions/bluebubbles/src/attachments.ts
+++ b/extensions/bluebubbles/src/attachments.ts
@@ -4,6 +4,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk";
 import { resolveBlueBubblesServerAccount } from "./account-resolve.js";
 import { postMultipartFormData } from "./multipart.js";
 import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
+import { getBlueBubblesRuntime } from "./runtime.js";
 import { extractBlueBubblesMessageId, resolveBlueBubblesSendTarget } from "./send-helpers.js";
 import { resolveChatGuidForTarget } from "./send.js";
 import {
@@ -57,6 +58,19 @@ function resolveAccount(params: BlueBubblesAttachmentOpts) {
  return resolveBlueBubblesServerAccount(params);
 }

+function resolveRequestUrl(input: RequestInfo | URL): string {
+  if (typeof input === "string") {
+    return input;
+  }
+  if (input instanceof URL) {
+    return input.toString();
+  }
+  if (typeof input === "object" && input && "url" in input && typeof input.url === "string") {
+    return input.url;
+  }
+  return String(input);
+}
+
 export async function downloadBlueBubblesAttachment(
  attachment: BlueBubblesAttachment,
  opts: BlueBubblesAttachmentOpts & { maxBytes?: number } = {},
@@ -71,20 +85,30 @@ export async function downloadBlueBubblesAttachment(
    path: `/api/v1/attachment/${encodeURIComponent(guid)}/download`,
    password,
  });
-  const res = await blueBubblesFetchWithTimeout(url, { method: "GET" }, opts.timeoutMs);
-  if (!res.ok) {
-    const errorText = await res.text().catch(() => "");
-    throw new Error(
-      `BlueBubbles attachment download failed (${res.status}): ${errorText || "unknown"}`,
-    );
-  }
-  const contentType = res.headers.get("content-type") ?? undefined;
-  const buf = new Uint8Array(await res.arrayBuffer());
  const maxBytes = typeof opts.maxBytes === "number" ? opts.maxBytes : DEFAULT_ATTACHMENT_MAX_BYTES;
-  if (buf.byteLength > maxBytes) {
-    throw new Error(`BlueBubbles attachment too large (${buf.byteLength} bytes)`);
+  try {
+    const fetched = await getBlueBubblesRuntime().channel.media.fetchRemoteMedia({
+      url,
+      filePathHint: attachment.transferName ?? attachment.guid ?? "attachment",
+      maxBytes,
+      fetchImpl: async (input, init) =>
+        await blueBubblesFetchWithTimeout(
+          resolveRequestUrl(input),
+          { ...init, method: init?.method ?? "GET" },
+          opts.timeoutMs,
+        ),
+    });
+    return {
+      buffer: new Uint8Array(fetched.buffer),
+      contentType: fetched.contentType ?? attachment.mimeType ?? undefined,
+    };
+  } catch (error) {
+    const text = error instanceof Error ? error.message : String(error);
+    if (/(?:maxBytes|content length|payload exceeds)/i.test(text)) {
+      throw new Error(`BlueBubbles attachment too large (limit ${maxBytes} bytes)`);
+    }
+    throw new Error(`BlueBubbles attachment download failed: ${text}`);
  }
-  return { buffer: buf, contentType: contentType ?? attachment.mimeType ?? undefined };
 }

 export type SendBlueBubblesAttachmentResult = {
--- a/extensions/msteams/src/attachments.test.ts
+++ b/extensions/msteams/src/attachments.test.ts
@@ -7,6 +7,29 @@ const saveMediaBufferMock = vi.fn(async () => ({
  path: "/tmp/saved.png",
  contentType: "image/png",
 }));
+const fetchRemoteMediaMock = vi.fn(
+  async (params: {
+    url: string;
+    maxBytes?: number;
+    filePathHint?: string;
+    fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
+  }) => {
+    const fetchFn = params.fetchImpl ?? fetch;
+    const res = await fetchFn(params.url);
+    if (!res.ok) {
+      throw new Error(`HTTP ${res.status}`);
+    }
+    const buffer = Buffer.from(await res.arrayBuffer());
+    if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) {
+      throw new Error(`payload exceeds maxBytes ${params.maxBytes}`);
+    }
+    return {
+      buffer,
+      contentType: res.headers.get("content-type") ?? undefined,
+      fileName: params.filePathHint,
+    };
+  },
+);

 const runtimeStub = {
  media: {
@@ -14,6 +37,8 @@ const runtimeStub = {
  },
  channel: {
    media: {
+      fetchRemoteMedia:
+        fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"],
      saveMediaBuffer:
        saveMediaBufferMock as unknown as PluginRuntime["channel"]["media"]["saveMediaBuffer"],
    },
@@ -28,6 +53,7 @@ describe("msteams attachments", () => {
  beforeEach(() => {
    detectMimeMock.mockClear();
    saveMediaBufferMock.mockClear();
+    fetchRemoteMediaMock.mockClear();
    setMSTeamsRuntime(runtimeStub);
  });

@@ -118,7 +144,7 @@ describe("msteams attachments", () => {
        fetchFn: fetchMock as unknown as typeof fetch,
      });

-      expect(fetchMock).toHaveBeenCalledWith("https://x/img");
+      expect(fetchMock).toHaveBeenCalledWith("https://x/img", undefined);
      expect(saveMediaBufferMock).toHaveBeenCalled();
      expect(media).toHaveLength(1);
      expect(media[0]?.path).toBe("/tmp/saved.png");
@@ -145,7 +171,7 @@ describe("msteams attachments", () => {
        fetchFn: fetchMock as unknown as typeof fetch,
      });

-      expect(fetchMock).toHaveBeenCalledWith("https://x/dl");
+      expect(fetchMock).toHaveBeenCalledWith("https://x/dl", undefined);
      expect(media).toHaveLength(1);
    });

@@ -170,7 +196,7 @@ describe("msteams attachments", () => {
        fetchFn: fetchMock as unknown as typeof fetch,
      });

-      expect(fetchMock).toHaveBeenCalledWith("https://x/doc.pdf");
+      expect(fetchMock).toHaveBeenCalledWith("https://x/doc.pdf", undefined);
      expect(media).toHaveLength(1);
      expect(media[0]?.path).toBe("/tmp/saved.pdf");
      expect(media[0]?.placeholder).toBe("<media:document>");
@@ -198,7 +224,7 @@ describe("msteams attachments", () => {
      });

      expect(media).toHaveLength(1);
-      expect(fetchMock).toHaveBeenCalledWith("https://x/inline.png");
+      expect(fetchMock).toHaveBeenCalledWith("https://x/inline.png", undefined);
    });

    it("stores inline data:image base64 payloads", async () => {
@@ -222,12 +248,8 @@ describe("msteams attachments", () => {
    it("retries with auth when the first request is unauthorized", async () => {
      const { downloadMSTeamsAttachments } = await load();
      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,
-        );
+        const headers = new Headers(opts?.headers);
+        const hasAuth = Boolean(headers.get("Authorization"));
        if (!hasAuth) {
          return new Response("unauthorized", { status: 401 });
        }
@@ -255,12 +277,8 @@ describe("msteams attachments", () => {
      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,
-        );
+        const headers = new Headers(opts?.headers);
+        const hasAuth = Boolean(headers.get("Authorization"));
        if (!hasAuth) {
          return new Response("forbidden", { status: 403 });
        }
--- a/extensions/msteams/src/attachments/download.ts
+++ b/extensions/msteams/src/attachments/download.ts
@@ -86,11 +86,12 @@ async function fetchWithAuthFallback(params: {
  url: string;
  tokenProvider?: MSTeamsAccessTokenProvider;
  fetchFn?: typeof fetch;
+  requestInit?: RequestInit;
  allowHosts: string[];
  authAllowHosts: string[];
 }): Promise<Response> {
  const fetchFn = params.fetchFn ?? fetch;
-  const firstAttempt = await fetchFn(params.url);
+  const firstAttempt = await fetchFn(params.url, params.requestInit);
  if (firstAttempt.ok) {
    return firstAttempt;
  }
@@ -108,8 +109,11 @@ async function fetchWithAuthFallback(params: {
  for (const scope of scopes) {
    try {
      const token = await params.tokenProvider.getAccessToken(scope);
+      const authHeaders = new Headers(params.requestInit?.headers);
+      authHeaders.set("Authorization", `Bearer ${token}`);
      const res = await fetchFn(params.url, {
-        headers: { Authorization: `Bearer ${token}` },
+        ...params.requestInit,
+        headers: authHeaders,
        redirect: "manual",
      });
      if (res.ok) {
@@ -117,7 +121,7 @@ async function fetchWithAuthFallback(params: {
      }
      const redirectUrl = readRedirectUrl(params.url, res);
      if (redirectUrl && isUrlAllowed(redirectUrl, params.allowHosts)) {
-        const redirectRes = await fetchFn(redirectUrl);
+        const redirectRes = await fetchFn(redirectUrl, params.requestInit);
        if (redirectRes.ok) {
          return redirectRes;
        }
@@ -125,8 +129,11 @@ async function fetchWithAuthFallback(params: {
          (redirectRes.status === 401 || redirectRes.status === 403) &&
          isUrlAllowed(redirectUrl, params.authAllowHosts)
        ) {
+          const redirectAuthHeaders = new Headers(params.requestInit?.headers);
+          redirectAuthHeaders.set("Authorization", `Bearer ${token}`);
          const redirectAuthRes = await fetchFn(redirectUrl, {
-            headers: { Authorization: `Bearer ${token}` },
+            ...params.requestInit,
+            headers: redirectAuthHeaders,
            redirect: "manual",
          });
          if (redirectAuthRes.ok) {
@@ -142,6 +149,19 @@ async function fetchWithAuthFallback(params: {
  return firstAttempt;
 }

+function resolveRequestUrl(input: RequestInfo | URL): string {
+  if (typeof input === "string") {
+    return input;
+  }
+  if (input instanceof URL) {
+    return input.toString();
+  }
+  if (typeof input === "object" && input && "url" in input && typeof input.url === "string") {
+    return input.url;
+  }
+  return String(input);
+}
+
 function readRedirectUrl(baseUrl: string, res: Response): string | null {
  if (![301, 302, 303, 307, 308].includes(res.status)) {
    return null;
@@ -238,28 +258,28 @@ export async function downloadMSTeamsAttachments(params: {
      continue;
    }
    try {
-      const res = await fetchWithAuthFallback({
+      const fetched = await getMSTeamsRuntime().channel.media.fetchRemoteMedia({
        url: candidate.url,
-        tokenProvider: params.tokenProvider,
-        fetchFn: params.fetchFn,
-        allowHosts,
-        authAllowHosts,
+        fetchImpl: (input, init) =>
+          fetchWithAuthFallback({
+            url: resolveRequestUrl(input),
+            tokenProvider: params.tokenProvider,
+            fetchFn: params.fetchFn,
+            requestInit: init,
+            allowHosts,
+            authAllowHosts,
+          }),
+        filePathHint: candidate.fileHint ?? candidate.url,
+        maxBytes: params.maxBytes,
      });
-      if (!res.ok) {
-        continue;
-      }
-      const buffer = Buffer.from(await res.arrayBuffer());
-      if (buffer.byteLength > params.maxBytes) {
-        continue;
-      }
      const mime = await getMSTeamsRuntime().media.detectMime({
-        buffer,
-        headerMime: res.headers.get("content-type"),
+        buffer: fetched.buffer,
+        headerMime: fetched.contentType,
        filePath: candidate.fileHint ?? candidate.url,
      });
      const originalFilename = params.preserveFilenames ? candidate.fileHint : undefined;
      const saved = await getMSTeamsRuntime().channel.media.saveMediaBuffer(
-        buffer,
+        fetched.buffer,
        mime ?? candidate.contentTypeHint,
        "inbound",
        params.maxBytes,
--- a/extensions/msteams/src/attachments/graph.ts
+++ b/extensions/msteams/src/attachments/graph.ts
@@ -14,6 +14,19 @@ import type {
  MSTeamsInboundMedia,
 } from "./types.js";

+function resolveRequestUrl(input: RequestInfo | URL): string {
+  if (typeof input === "string") {
+    return input;
+  }
+  if (input instanceof URL) {
+    return input.toString();
+  }
+  if (typeof input === "object" && input && "url" in input && typeof input.url === "string") {
+    return input.url;
+  }
+  return String(input);
+}
+
 type GraphHostedContent = {
  id?: string | null;
  contentType?: string | null;
@@ -265,35 +278,39 @@ export async function downloadMSTeamsGraphMedia(params: {
          const encodedUrl = Buffer.from(shareUrl).toString("base64url");
          const sharesUrl = `${GRAPH_ROOT}/shares/u!${encodedUrl}/driveItem/content`;

-          const spRes = await fetchFn(sharesUrl, {
-            headers: { Authorization: `Bearer ${accessToken}` },
-            redirect: "follow",
+          const fetched = await getMSTeamsRuntime().channel.media.fetchRemoteMedia({
+            url: sharesUrl,
+            filePathHint: name,
+            maxBytes: params.maxBytes,
+            fetchImpl: async (input, init) => {
+              const headers = new Headers(init?.headers);
+              headers.set("Authorization", `Bearer ${accessToken}`);
+              return await fetchFn(resolveRequestUrl(input), {
+                ...init,
+                headers,
+                redirect: "follow",
+              });
+            },
          });
-
-          if (spRes.ok) {
-            const buffer = Buffer.from(await spRes.arrayBuffer());
-            if (buffer.byteLength <= params.maxBytes) {
-              const mime = await getMSTeamsRuntime().media.detectMime({
-                buffer,
-                headerMime: spRes.headers.get("content-type") ?? undefined,
-                filePath: name,
-              });
-              const originalFilename = params.preserveFilenames ? name : undefined;
-              const saved = await getMSTeamsRuntime().channel.media.saveMediaBuffer(
-                buffer,
-                mime ?? "application/octet-stream",
-                "inbound",
-                params.maxBytes,
-                originalFilename,
-              );
-              sharePointMedia.push({
-                path: saved.path,
-                contentType: saved.contentType,
-                placeholder: inferPlaceholder({ contentType: saved.contentType, fileName: name }),
-              });
-              downloadedReferenceUrls.add(shareUrl);
-            }
-          }
+          const mime = await getMSTeamsRuntime().media.detectMime({
+            buffer: fetched.buffer,
+            headerMime: fetched.contentType,
+            filePath: name,
+          });
+          const originalFilename = params.preserveFilenames ? name : undefined;
+          const saved = await getMSTeamsRuntime().channel.media.saveMediaBuffer(
+            fetched.buffer,
+            mime ?? "application/octet-stream",
+            "inbound",
+            params.maxBytes,
+            originalFilename,
+          );
+          sharePointMedia.push({
+            path: saved.path,
+            contentType: saved.contentType,
+            placeholder: inferPlaceholder({ contentType: saved.contentType, fileName: name }),
+          });
+          downloadedReferenceUrls.add(shareUrl);
        } catch {
          // Ignore SharePoint download failures.
        }
--- a/extensions/zalo/src/monitor.ts
+++ b/extensions/zalo/src/monitor.ts
@@ -447,7 +447,7 @@ async function handleImageMessage(
  if (photo) {
    try {
      const maxBytes = mediaMaxMb * 1024 * 1024;
-      const fetched = await core.channel.media.fetchRemoteMedia({ url: photo });
+      const fetched = await core.channel.media.fetchRemoteMedia({ url: photo, maxBytes });
      const saved = await core.channel.media.saveMediaBuffer(
        fetched.buffer,
        fetched.contentType,
--- a/src/discord/monitor/message-utils.test.ts
+++ b/src/discord/monitor/message-utils.test.ts
@@ -92,6 +92,7 @@ describe("resolveForwardedMediaList", () => {
    expect(fetchRemoteMedia).toHaveBeenCalledWith({
      url: attachment.url,
      filePathHint: attachment.filename,
+      maxBytes: 512,
    });
    expect(saveMediaBuffer).toHaveBeenCalledTimes(1);
    expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512);
@@ -132,6 +133,7 @@ describe("resolveForwardedMediaList", () => {
    expect(fetchRemoteMedia).toHaveBeenCalledWith({
      url: "https://media.discordapp.net/stickers/sticker-1.png",
      filePathHint: "wave.png",
+      maxBytes: 512,
    });
    expect(saveMediaBuffer).toHaveBeenCalledTimes(1);
    expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512);
@@ -198,6 +200,7 @@ describe("resolveMediaList", () => {
    expect(fetchRemoteMedia).toHaveBeenCalledWith({
      url: "https://media.discordapp.net/stickers/sticker-2.png",
      filePathHint: "hello.png",
+      maxBytes: 512,
    });
    expect(saveMediaBuffer).toHaveBeenCalledTimes(1);
    expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512);
--- a/src/discord/monitor/message-utils.ts
+++ b/src/discord/monitor/message-utils.ts
@@ -218,6 +218,7 @@ async function appendResolvedMediaFromAttachments(params: {
      const fetched = await fetchRemoteMedia({
        url: attachment.url,
        filePathHint: attachment.filename ?? attachment.url,
+        maxBytes: params.maxBytes,
      });
      const saved = await saveMediaBuffer(
        fetched.buffer,
@@ -307,6 +308,7 @@ async function appendResolvedMediaFromStickers(params: {
        const fetched = await fetchRemoteMedia({
          url: candidate.url,
          filePathHint: candidate.fileName,
+          maxBytes: params.maxBytes,
        });
        const saved = await saveMediaBuffer(
          fetched.buffer,
--- a/src/telegram/bot/delivery.ts
+++ b/src/telegram/bot/delivery.ts
@@ -319,6 +319,7 @@ export async function resolveMedia(
      url,
      fetchImpl,
      filePathHint: filePath,
+      maxBytes,
    });
    const originalName = fetched.fileName ?? filePath;
    return saveMediaBuffer(fetched.buffer, fetched.contentType, "inbound", maxBytes, originalName);