Skip to content

Commit 73d93de

Browse files
steipetesteipete
committed
fix: enforce inbound media max-bytes during remote fetch
1 parent dd41fad commit 73d93de

File tree

10 files changed

+207
-77
lines changed

10 files changed

+207
-77
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
2424
- Security/Archive: block zip symlink escapes during archive extraction.
2525
- 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.
2626
- Security/Gateway: block node-role connections when device identity metadata is missing.
27+
- 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.
2728
- 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.
2829
- Security/Browser relay: harden extension relay auth token handling for `/extension` and `/cdp` pathways.
2930
- Cron: persist `delivered` state in cron job records so delivery failures remain visible in status and logs. (#19174) Thanks @simonemacario.

‎extensions/bluebubbles/src/attachments.test.ts‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,60 @@
1+
import type { PluginRuntime } from "openclaw/plugin-sdk";
12
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
23
import "./test-mocks.js";
34
import { downloadBlueBubblesAttachment, sendBlueBubblesAttachment } from "./attachments.js";
45
import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
6+
import { setBlueBubblesRuntime } from "./runtime.js";
57
import { installBlueBubblesFetchTestHooks } from "./test-harness.js";
68
import type { BlueBubblesAttachment } from "./types.js";
79

810
const mockFetch = vi.fn();
11+
const fetchRemoteMediaMock = vi.fn(
12+
async (params: {
13+
url: string;
14+
maxBytes?: number;
15+
fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
16+
}) => {
17+
const fetchFn = params.fetchImpl ?? fetch;
18+
const res = await fetchFn(params.url);
19+
if (!res.ok) {
20+
const text = await res.text().catch(() => "unknown");
21+
throw new Error(
22+
`Failed to fetch media from ${params.url}: HTTP ${res.status}; body: ${text}`,
23+
);
24+
}
25+
const buffer = Buffer.from(await res.arrayBuffer());
26+
if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) {
27+
throw new Error(`payload exceeds maxBytes ${params.maxBytes}`);
28+
}
29+
return {
30+
buffer,
31+
contentType: res.headers.get("content-type") ?? undefined,
32+
fileName: undefined,
33+
};
34+
},
35+
);
936

1037
installBlueBubblesFetchTestHooks({
1138
mockFetch,
1239
privateApiStatusMock: vi.mocked(getCachedBlueBubblesPrivateApiStatus),
1340
});
1441

42+
const runtimeStub = {
43+
channel: {
44+
media: {
45+
fetchRemoteMedia:
46+
fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"],
47+
},
48+
},
49+
} as unknown as PluginRuntime;
50+
1551
describe("downloadBlueBubblesAttachment", () => {
52+
beforeEach(() => {
53+
fetchRemoteMediaMock.mockClear();
54+
mockFetch.mockReset();
55+
setBlueBubblesRuntime(runtimeStub);
56+
});
57+
1658
it("throws when guid is missing", async () => {
1759
const attachment: BlueBubblesAttachment = {};
1860
await expect(
@@ -120,7 +162,7 @@ describe("downloadBlueBubblesAttachment", () => {
120162
serverUrl: "http://localhost:1234",
121163
password: "test",
122164
}),
123-
).rejects.toThrow("download failed (404): Attachment not found");
165+
).rejects.toThrow("Attachment not found");
124166
});
125167

126168
it("throws when attachment exceeds max bytes", async () => {
@@ -229,6 +271,8 @@ describe("sendBlueBubblesAttachment", () => {
229271
beforeEach(() => {
230272
vi.stubGlobal("fetch", mockFetch);
231273
mockFetch.mockReset();
274+
fetchRemoteMediaMock.mockClear();
275+
setBlueBubblesRuntime(runtimeStub);
232276
vi.mocked(getCachedBlueBubblesPrivateApiStatus).mockReset();
233277
vi.mocked(getCachedBlueBubblesPrivateApiStatus).mockReturnValue(null);
234278
});

‎extensions/bluebubbles/src/attachments.ts‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk";
44
import { resolveBlueBubblesServerAccount } from "./account-resolve.js";
55
import { postMultipartFormData } from "./multipart.js";
66
import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
7+
import { getBlueBubblesRuntime } from "./runtime.js";
78
import { extractBlueBubblesMessageId, resolveBlueBubblesSendTarget } from "./send-helpers.js";
89
import { resolveChatGuidForTarget } from "./send.js";
910
import {
@@ -57,6 +58,19 @@ function resolveAccount(params: BlueBubblesAttachmentOpts) {
5758
return resolveBlueBubblesServerAccount(params);
5859
}
5960

61+
function resolveRequestUrl(input: RequestInfo | URL): string {
62+
if (typeof input === "string") {
63+
return input;
64+
}
65+
if (input instanceof URL) {
66+
return input.toString();
67+
}
68+
if (typeof input === "object" && input && "url" in input && typeof input.url === "string") {
69+
return input.url;
70+
}
71+
return String(input);
72+
}
73+
6074
export async function downloadBlueBubblesAttachment(
6175
attachment: BlueBubblesAttachment,
6276
opts: BlueBubblesAttachmentOpts & { maxBytes?: number } = {},
@@ -71,20 +85,30 @@ export async function downloadBlueBubblesAttachment(
7185
path: `/api/v1/attachment/${encodeURIComponent(guid)}/download`,
7286
password,
7387
});
74-
const res = await blueBubblesFetchWithTimeout(url, { method: "GET" }, opts.timeoutMs);
75-
if (!res.ok) {
76-
const errorText = await res.text().catch(() => "");
77-
throw new Error(
78-
`BlueBubbles attachment download failed (${res.status}): ${errorText || "unknown"}`,
79-
);
80-
}
81-
const contentType = res.headers.get("content-type") ?? undefined;
82-
const buf = new Uint8Array(await res.arrayBuffer());
8388
const maxBytes = typeof opts.maxBytes === "number" ? opts.maxBytes : DEFAULT_ATTACHMENT_MAX_BYTES;
84-
if (buf.byteLength > maxBytes) {
85-
throw new Error(`BlueBubbles attachment too large (${buf.byteLength} bytes)`);
89+
try {
90+
const fetched = await getBlueBubblesRuntime().channel.media.fetchRemoteMedia({
91+
url,
92+
filePathHint: attachment.transferName ?? attachment.guid ?? "attachment",
93+
maxBytes,
94+
fetchImpl: async (input, init) =>
95+
await blueBubblesFetchWithTimeout(
96+
resolveRequestUrl(input),
97+
{ ...init, method: init?.method ?? "GET" },
98+
opts.timeoutMs,
99+
),
100+
});
101+
return {
102+
buffer: new Uint8Array(fetched.buffer),
103+
contentType: fetched.contentType ?? attachment.mimeType ?? undefined,
104+
};
105+
} catch (error) {
106+
const text = error instanceof Error ? error.message : String(error);
107+
if (/(?:maxBytes|content length|payload exceeds)/i.test(text)) {
108+
throw new Error(`BlueBubbles attachment too large (limit ${maxBytes} bytes)`);
109+
}
110+
throw new Error(`BlueBubbles attachment download failed: ${text}`);
86111
}
87-
return { buffer: buf, contentType: contentType ?? attachment.mimeType ?? undefined };
88112
}
89113

90114
export type SendBlueBubblesAttachmentResult = {

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

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,38 @@ const saveMediaBufferMock = vi.fn(async () => ({
77
path: "/tmp/saved.png",
88
contentType: "image/png",
99
}));
10+
const fetchRemoteMediaMock = vi.fn(
11+
async (params: {
12+
url: string;
13+
maxBytes?: number;
14+
filePathHint?: string;
15+
fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
16+
}) => {
17+
const fetchFn = params.fetchImpl ?? fetch;
18+
const res = await fetchFn(params.url);
19+
if (!res.ok) {
20+
throw new Error(`HTTP ${res.status}`);
21+
}
22+
const buffer = Buffer.from(await res.arrayBuffer());
23+
if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) {
24+
throw new Error(`payload exceeds maxBytes ${params.maxBytes}`);
25+
}
26+
return {
27+
buffer,
28+
contentType: res.headers.get("content-type") ?? undefined,
29+
fileName: params.filePathHint,
30+
};
31+
},
32+
);
1033

1134
const runtimeStub = {
1235
media: {
1336
detectMime: detectMimeMock as unknown as PluginRuntime["media"]["detectMime"],
1437
},
1538
channel: {
1639
media: {
40+
fetchRemoteMedia:
41+
fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"],
1742
saveMediaBuffer:
1843
saveMediaBufferMock as unknown as PluginRuntime["channel"]["media"]["saveMediaBuffer"],
1944
},
@@ -28,6 +53,7 @@ describe("msteams attachments", () => {
2853
beforeEach(() => {
2954
detectMimeMock.mockClear();
3055
saveMediaBufferMock.mockClear();
56+
fetchRemoteMediaMock.mockClear();
3157
setMSTeamsRuntime(runtimeStub);
3258
});
3359

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

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

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

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

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

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

204230
it("stores inline data:image base64 payloads", async () => {
@@ -222,12 +248,8 @@ describe("msteams attachments", () => {
222248
it("retries with auth when the first request is unauthorized", async () => {
223249
const { downloadMSTeamsAttachments } = await load();
224250
const fetchMock = vi.fn(async (_url: string, opts?: RequestInit) => {
225-
const hasAuth = Boolean(
226-
opts &&
227-
typeof opts === "object" &&
228-
"headers" in opts &&
229-
(opts.headers as Record<string, string>)?.Authorization,
230-
);
251+
const headers = new Headers(opts?.headers);
252+
const hasAuth = Boolean(headers.get("Authorization"));
231253
if (!hasAuth) {
232254
return new Response("unauthorized", { status: 401 });
233255
}
@@ -255,12 +277,8 @@ describe("msteams attachments", () => {
255277
const { downloadMSTeamsAttachments } = await load();
256278
const tokenProvider = { getAccessToken: vi.fn(async () => "token") };
257279
const fetchMock = vi.fn(async (_url: string, opts?: RequestInit) => {
258-
const hasAuth = Boolean(
259-
opts &&
260-
typeof opts === "object" &&
261-
"headers" in opts &&
262-
(opts.headers as Record<string, string>)?.Authorization,
263-
);
280+
const headers = new Headers(opts?.headers);
281+
const hasAuth = Boolean(headers.get("Authorization"));
264282
if (!hasAuth) {
265283
return new Response("forbidden", { status: 403 });
266284
}

‎extensions/msteams/src/attachments/download.ts‎

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,12 @@ async function fetchWithAuthFallback(params: {
8686
url: string;
8787
tokenProvider?: MSTeamsAccessTokenProvider;
8888
fetchFn?: typeof fetch;
89+
requestInit?: RequestInit;
8990
allowHosts: string[];
9091
authAllowHosts: string[];
9192
}): Promise<Response> {
9293
const fetchFn = params.fetchFn ?? fetch;
93-
const firstAttempt = await fetchFn(params.url);
94+
const firstAttempt = await fetchFn(params.url, params.requestInit);
9495
if (firstAttempt.ok) {
9596
return firstAttempt;
9697
}
@@ -108,25 +109,31 @@ async function fetchWithAuthFallback(params: {
108109
for (const scope of scopes) {
109110
try {
110111
const token = await params.tokenProvider.getAccessToken(scope);
112+
const authHeaders = new Headers(params.requestInit?.headers);
113+
authHeaders.set("Authorization", `Bearer ${token}`);
111114
const res = await fetchFn(params.url, {
112-
headers: { Authorization: `Bearer ${token}` },
115+
...params.requestInit,
116+
headers: authHeaders,
113117
redirect: "manual",
114118
});
115119
if (res.ok) {
116120
return res;
117121
}
118122
const redirectUrl = readRedirectUrl(params.url, res);
119123
if (redirectUrl && isUrlAllowed(redirectUrl, params.allowHosts)) {
120-
const redirectRes = await fetchFn(redirectUrl);
124+
const redirectRes = await fetchFn(redirectUrl, params.requestInit);
121125
if (redirectRes.ok) {
122126
return redirectRes;
123127
}
124128
if (
125129
(redirectRes.status === 401 || redirectRes.status === 403) &&
126130
isUrlAllowed(redirectUrl, params.authAllowHosts)
127131
) {
132+
const redirectAuthHeaders = new Headers(params.requestInit?.headers);
133+
redirectAuthHeaders.set("Authorization", `Bearer ${token}`);
128134
const redirectAuthRes = await fetchFn(redirectUrl, {
129-
headers: { Authorization: `Bearer ${token}` },
135+
...params.requestInit,
136+
headers: redirectAuthHeaders,
130137
redirect: "manual",
131138
});
132139
if (redirectAuthRes.ok) {
@@ -142,6 +149,19 @@ async function fetchWithAuthFallback(params: {
142149
return firstAttempt;
143150
}
144151

152+
function resolveRequestUrl(input: RequestInfo | URL): string {
153+
if (typeof input === "string") {
154+
return input;
155+
}
156+
if (input instanceof URL) {
157+
return input.toString();
158+
}
159+
if (typeof input === "object" && input && "url" in input && typeof input.url === "string") {
160+
return input.url;
161+
}
162+
return String(input);
163+
}
164+
145165
function readRedirectUrl(baseUrl: string, res: Response): string | null {
146166
if (![301, 302, 303, 307, 308].includes(res.status)) {
147167
return null;
@@ -238,28 +258,28 @@ export async function downloadMSTeamsAttachments(params: {
238258
continue;
239259
}
240260
try {
241-
const res = await fetchWithAuthFallback({
261+
const fetched = await getMSTeamsRuntime().channel.media.fetchRemoteMedia({
242262
url: candidate.url,
243-
tokenProvider: params.tokenProvider,
244-
fetchFn: params.fetchFn,
245-
allowHosts,
246-
authAllowHosts,
263+
fetchImpl: (input, init) =>
264+
fetchWithAuthFallback({
265+
url: resolveRequestUrl(input),
266+
tokenProvider: params.tokenProvider,
267+
fetchFn: params.fetchFn,
268+
requestInit: init,
269+
allowHosts,
270+
authAllowHosts,
271+
}),
272+
filePathHint: candidate.fileHint ?? candidate.url,
273+
maxBytes: params.maxBytes,
247274
});
248-
if (!res.ok) {
249-
continue;
250-
}
251-
const buffer = Buffer.from(await res.arrayBuffer());
252-
if (buffer.byteLength > params.maxBytes) {
253-
continue;
254-
}
255275
const mime = await getMSTeamsRuntime().media.detectMime({
256-
buffer,
257-
headerMime: res.headers.get("content-type"),
276+
buffer: fetched.buffer,
277+
headerMime: fetched.contentType,
258278
filePath: candidate.fileHint ?? candidate.url,
259279
});
260280
const originalFilename = params.preserveFilenames ? candidate.fileHint : undefined;
261281
const saved = await getMSTeamsRuntime().channel.media.saveMediaBuffer(
262-
buffer,
282+
fetched.buffer,
263283
mime ?? candidate.contentTypeHint,
264284
"inbound",
265285
params.maxBytes,

0 commit comments

Comments
 (0)