mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-21 14:11:26 +00:00
* fix(msteams): fix SharePoint media fetch on Node 24+ and stop swallowing errors (#63396) * fix(msteams): extend Node 24 dispatcher fix to Bot Framework attachment view downloads
This commit is contained in:
@@ -190,11 +190,12 @@ const createHostedContentsWithType = (contentType: string, ...ids: string[]) =>
|
||||
ids.map((id) => ({ id, contentType, contentBytes: PNG_BASE64 }));
|
||||
const _createHostedImageContents = (...ids: string[]) =>
|
||||
createHostedContentsWithType(CONTENT_TYPE_IMAGE_PNG, ...ids);
|
||||
const _createPdfResponse = (payload: Buffer | string = PDF_BUFFER) => {
|
||||
type BinaryPayload = Uint8Array | string;
|
||||
const _createPdfResponse = (payload: BinaryPayload = PDF_BUFFER) => {
|
||||
return createBufferResponse(payload, CONTENT_TYPE_APPLICATION_PDF);
|
||||
};
|
||||
const createBufferResponse = (payload: Buffer | string, contentType: string, status = 200) => {
|
||||
const raw = Buffer.isBuffer(payload) ? payload : Buffer.from(payload);
|
||||
const createBufferResponse = (payload: BinaryPayload, contentType: string, status = 200) => {
|
||||
const raw = typeof payload === "string" ? Buffer.from(payload) : payload;
|
||||
return new Response(new Uint8Array(raw), {
|
||||
status,
|
||||
headers: { "content-type": contentType },
|
||||
@@ -654,5 +655,48 @@ describe("msteams attachments", () => {
|
||||
expect(calledUrls.some((url) => url.startsWith(GRAPH_SHARES_URL_PREFIX))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("error logging (issue #63396)", () => {
|
||||
// Before this fix, fetch failures were swallowed by empty `catch {}`
|
||||
// blocks, leaving operators with no signal that SharePoint downloads
|
||||
// were silently failing on Node 24+. These tests pin the logger contract
|
||||
// so the regression cannot return.
|
||||
it("invokes logger.warn when a remote media download fails", async () => {
|
||||
const logger = { warn: vi.fn(), error: vi.fn() };
|
||||
const fetchMock = vi.fn(async () => createTextResponse("server error", 500));
|
||||
|
||||
const media = await downloadMSTeamsAttachments(
|
||||
buildDownloadParams(createImageAttachments(TEST_URL_IMAGE), {
|
||||
fetchFn: asFetchFn(fetchMock),
|
||||
logger,
|
||||
}),
|
||||
);
|
||||
|
||||
expectAttachmentMediaLength(media, 0);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
"msteams attachment download failed",
|
||||
expect.objectContaining({
|
||||
error: expect.stringContaining("HTTP 500"),
|
||||
host: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not log when downloads succeed", async () => {
|
||||
const logger = { warn: vi.fn(), error: vi.fn() };
|
||||
const fetchMock = createOkFetchMock(CONTENT_TYPE_IMAGE_PNG);
|
||||
|
||||
const media = await downloadMSTeamsAttachments(
|
||||
buildDownloadParams(createImageAttachments(TEST_URL_IMAGE), {
|
||||
fetchFn: asFetchFn(fetchMock),
|
||||
logger,
|
||||
}),
|
||||
);
|
||||
|
||||
expectAttachmentMediaLength(media, 1);
|
||||
expect(logger.warn).not.toHaveBeenCalled();
|
||||
expect(logger.error).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,23 +7,6 @@ import {
|
||||
} from "./bot-framework.js";
|
||||
import type { MSTeamsAccessTokenProvider } from "./types.js";
|
||||
|
||||
vi.mock("../../runtime-api.js", async () => {
|
||||
const actual =
|
||||
await vi.importActual<typeof import("../../runtime-api.js")>("../../runtime-api.js");
|
||||
return {
|
||||
...actual,
|
||||
fetchWithSsrFGuard: async (params: {
|
||||
url: string;
|
||||
init?: RequestInit;
|
||||
fetchImpl?: typeof fetch;
|
||||
}) => ({
|
||||
response: await (params.fetchImpl ?? fetch)(params.url, params.init),
|
||||
finalUrl: params.url,
|
||||
release: async () => {},
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
type SavedCall = {
|
||||
buffer: Buffer;
|
||||
contentType?: string;
|
||||
@@ -242,6 +225,150 @@ describe("downloadMSTeamsBotFrameworkAttachment", () => {
|
||||
expect(media).toBeUndefined();
|
||||
expect(fetchFn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("Node 24+ dispatcher bypass (issue #63396)", () => {
|
||||
it("drives the caller's fetchFn directly without the pinned undici dispatcher", async () => {
|
||||
// Regression: before the fix, fetchBotFrameworkAttachment* routed
|
||||
// through `fetchWithSsrFGuard`, which installs a `createPinnedDispatcher`
|
||||
// incompatible with Node 24+'s built-in undici v7. Downloads failed with
|
||||
// "invalid onRequestStart method". The fix switches to
|
||||
// `safeFetchWithPolicy`, which calls the supplied `fetchFn` directly
|
||||
// and never attaches a pinned dispatcher. Verify the caller's `fetchFn`
|
||||
// is invoked (no dispatcher in init).
|
||||
const fileBytes = Buffer.from("BFBYTES", "utf-8");
|
||||
const fetchCalls: Array<{ url: string; init?: RequestInit }> = [];
|
||||
const fetchFn: typeof fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
|
||||
fetchCalls.push({ url, init });
|
||||
if (url.endsWith("/v3/attachments/att-1")) {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
name: "doc.pdf",
|
||||
type: "application/pdf",
|
||||
views: [{ viewId: "original", size: fileBytes.byteLength }],
|
||||
}),
|
||||
{ status: 200, headers: { "content-type": "application/json" } },
|
||||
);
|
||||
}
|
||||
if (url.endsWith("/v3/attachments/att-1/views/original")) {
|
||||
return new Response(fileBytes, {
|
||||
status: 200,
|
||||
headers: { "content-length": String(fileBytes.byteLength) },
|
||||
});
|
||||
}
|
||||
return new Response("not found", { status: 404 });
|
||||
}) as typeof fetch;
|
||||
|
||||
const media = await downloadMSTeamsBotFrameworkAttachment({
|
||||
serviceUrl: "https://smba.trafficmanager.net/amer",
|
||||
attachmentId: "att-1",
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(media).toBeDefined();
|
||||
// Both the attachment info call and the view call should be observed,
|
||||
// confirming the direct fetch path was taken (no dispatcher interception).
|
||||
expect(fetchCalls).toHaveLength(2);
|
||||
expect(fetchCalls[0].url.endsWith("/v3/attachments/att-1")).toBe(true);
|
||||
expect(fetchCalls[1].url.endsWith("/v3/attachments/att-1/views/original")).toBe(true);
|
||||
// Verify no pinned undici dispatcher is attached on either request.
|
||||
for (const call of fetchCalls) {
|
||||
const init = call.init as RequestInit & { dispatcher?: unknown };
|
||||
expect(init?.dispatcher).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it("logs a warning when the attachmentInfo fetch throws (no longer silently swallowed)", async () => {
|
||||
const warn = vi.fn();
|
||||
const logger = { warn };
|
||||
const error = new TypeError("fetch failed | invalid onRequestStart method");
|
||||
const fetchFn: typeof fetch = (async () => {
|
||||
throw error;
|
||||
}) as typeof fetch;
|
||||
|
||||
const media = await downloadMSTeamsBotFrameworkAttachment({
|
||||
serviceUrl: "https://smba.trafficmanager.net/amer",
|
||||
attachmentId: "att-1",
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
logger,
|
||||
});
|
||||
|
||||
expect(media).toBeUndefined();
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"msteams botFramework attachmentInfo fetch failed",
|
||||
expect.objectContaining({
|
||||
error: expect.stringContaining("invalid onRequestStart method"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("logs a warning when the attachmentView fetch throws", async () => {
|
||||
const warn = vi.fn();
|
||||
const logger = { warn };
|
||||
const fetchFn: typeof fetch = (async (input: RequestInfo | URL) => {
|
||||
const url =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
|
||||
if (url.endsWith("/v3/attachments/att-1")) {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
name: "doc.pdf",
|
||||
type: "application/pdf",
|
||||
views: [{ viewId: "original", size: 10 }],
|
||||
}),
|
||||
{ status: 200 },
|
||||
);
|
||||
}
|
||||
throw new TypeError("fetch failed");
|
||||
}) as typeof fetch;
|
||||
|
||||
const media = await downloadMSTeamsBotFrameworkAttachment({
|
||||
serviceUrl: "https://smba.trafficmanager.net/amer",
|
||||
attachmentId: "att-1",
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
logger,
|
||||
});
|
||||
|
||||
expect(media).toBeUndefined();
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"msteams botFramework attachmentView fetch failed",
|
||||
expect.objectContaining({
|
||||
error: expect.stringContaining("fetch failed"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("logs a warning on non-ok attachmentInfo response", async () => {
|
||||
const warn = vi.fn();
|
||||
const fetchFn = createMockFetch([
|
||||
{
|
||||
match: /\/v3\/attachments\/att-1$/,
|
||||
response: new Response("server error", { status: 500 }),
|
||||
},
|
||||
]);
|
||||
|
||||
const media = await downloadMSTeamsBotFrameworkAttachment({
|
||||
serviceUrl: "https://smba.trafficmanager.net/amer",
|
||||
attachmentId: "att-1",
|
||||
tokenProvider: buildTokenProvider(),
|
||||
maxBytes: 10_000_000,
|
||||
fetchFn,
|
||||
logger: { warn },
|
||||
});
|
||||
|
||||
expect(media).toBeUndefined();
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"msteams botFramework attachmentInfo non-ok",
|
||||
expect.objectContaining({ status: 500 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("downloadMSTeamsBotFrameworkAttachments", () => {
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
import { Buffer } from "node:buffer";
|
||||
import { fetchWithSsrFGuard, type SsrFPolicy } from "../../runtime-api.js";
|
||||
import { getMSTeamsRuntime } from "../runtime.js";
|
||||
import { ensureUserAgentHeader } from "../user-agent.js";
|
||||
import {
|
||||
inferPlaceholder,
|
||||
isUrlAllowed,
|
||||
type MSTeamsAttachmentDownloadLogger,
|
||||
type MSTeamsAttachmentFetchPolicy,
|
||||
resolveAttachmentFetchPolicy,
|
||||
resolveMediaSsrfPolicy,
|
||||
safeFetchWithPolicy,
|
||||
} from "./shared.js";
|
||||
import type {
|
||||
MSTeamsAccessTokenProvider,
|
||||
@@ -57,30 +57,47 @@ async function fetchBotFrameworkAttachmentInfo(params: {
|
||||
serviceUrl: string;
|
||||
attachmentId: string;
|
||||
accessToken: string;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
fetchFn?: typeof fetch;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<BotFrameworkAttachmentInfo | undefined> {
|
||||
const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}`;
|
||||
const { response, release } = await fetchWithSsrFGuard({
|
||||
url,
|
||||
fetchImpl: params.fetchFn ?? fetch,
|
||||
init: {
|
||||
headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
|
||||
},
|
||||
policy: params.ssrfPolicy,
|
||||
auditContext: "msteams.botframework.attachmentInfo",
|
||||
});
|
||||
// Use `safeFetchWithPolicy` instead of `fetchWithSsrFGuard`. The strict
|
||||
// pinned undici dispatcher used by `fetchWithSsrFGuard` is incompatible
|
||||
// with Node 24+'s built-in undici v7 and silently breaks Bot Framework
|
||||
// attachment downloads (same root cause as the SharePoint fix in #63396).
|
||||
// `safeFetchWithPolicy` already enforces hostname allowlist validation
|
||||
// across every redirect hop, which is sufficient for these attachment
|
||||
// service URLs.
|
||||
let response: Response;
|
||||
try {
|
||||
if (!response.ok) {
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
return (await response.json()) as BotFrameworkAttachmentInfo;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
} finally {
|
||||
await release();
|
||||
response = await safeFetchWithPolicy({
|
||||
url,
|
||||
policy: params.policy,
|
||||
fetchFn: params.fetchFn,
|
||||
requestInit: {
|
||||
headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentInfo fetch failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
if (!response.ok) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentInfo non-ok", {
|
||||
status: response.status,
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
return (await response.json()) as BotFrameworkAttachmentInfo;
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentInfo parse failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,35 +107,51 @@ async function fetchBotFrameworkAttachmentView(params: {
|
||||
viewId: string;
|
||||
accessToken: string;
|
||||
maxBytes: number;
|
||||
policy: MSTeamsAttachmentFetchPolicy;
|
||||
fetchFn?: typeof fetch;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<Buffer | undefined> {
|
||||
const url = `${normalizeServiceUrl(params.serviceUrl)}/v3/attachments/${encodeURIComponent(params.attachmentId)}/views/${encodeURIComponent(params.viewId)}`;
|
||||
const { response, release } = await fetchWithSsrFGuard({
|
||||
url,
|
||||
fetchImpl: params.fetchFn ?? fetch,
|
||||
init: {
|
||||
headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
|
||||
},
|
||||
policy: params.ssrfPolicy,
|
||||
auditContext: "msteams.botframework.attachmentView",
|
||||
});
|
||||
// See `fetchBotFrameworkAttachmentInfo` for why this uses
|
||||
// `safeFetchWithPolicy` instead of `fetchWithSsrFGuard` on Node 24+ (#63396).
|
||||
let response: Response;
|
||||
try {
|
||||
response = await safeFetchWithPolicy({
|
||||
url,
|
||||
policy: params.policy,
|
||||
fetchFn: params.fetchFn,
|
||||
requestInit: {
|
||||
headers: ensureUserAgentHeader({ Authorization: `Bearer ${params.accessToken}` }),
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentView fetch failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
if (!response.ok) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentView non-ok", {
|
||||
status: response.status,
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (contentLength && Number(contentLength) > params.maxBytes) {
|
||||
return undefined;
|
||||
}
|
||||
try {
|
||||
if (!response.ok) {
|
||||
return undefined;
|
||||
}
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (contentLength && Number(contentLength) > params.maxBytes) {
|
||||
return undefined;
|
||||
}
|
||||
const arrayBuffer = await response.arrayBuffer();
|
||||
const buffer = Buffer.from(arrayBuffer);
|
||||
if (buffer.byteLength > params.maxBytes) {
|
||||
return undefined;
|
||||
}
|
||||
return buffer;
|
||||
} finally {
|
||||
await release();
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework attachmentView body read failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,6 +172,7 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
fileNameHint?: string | null;
|
||||
contentTypeHint?: string | null;
|
||||
preserveFilenames?: boolean;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<MSTeamsInboundMedia | undefined> {
|
||||
if (!params.serviceUrl || !params.attachmentId || !params.tokenProvider) {
|
||||
return undefined;
|
||||
@@ -151,12 +185,14 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
if (!isUrlAllowed(baseUrl, policy.allowHosts)) {
|
||||
return undefined;
|
||||
}
|
||||
const ssrfPolicy = resolveMediaSsrfPolicy(policy.allowHosts);
|
||||
|
||||
let accessToken: string;
|
||||
try {
|
||||
accessToken = await params.tokenProvider.getAccessToken(BOT_FRAMEWORK_SCOPE);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework token acquisition failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
if (!accessToken) {
|
||||
@@ -167,8 +203,9 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
serviceUrl: params.serviceUrl,
|
||||
attachmentId: params.attachmentId,
|
||||
accessToken,
|
||||
policy,
|
||||
fetchFn: params.fetchFn,
|
||||
ssrfPolicy,
|
||||
logger: params.logger,
|
||||
});
|
||||
if (!info) {
|
||||
return undefined;
|
||||
@@ -200,8 +237,9 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
viewId,
|
||||
accessToken,
|
||||
maxBytes: params.maxBytes,
|
||||
policy,
|
||||
fetchFn: params.fetchFn,
|
||||
ssrfPolicy,
|
||||
logger: params.logger,
|
||||
});
|
||||
if (!buffer) {
|
||||
return undefined;
|
||||
@@ -236,7 +274,10 @@ export async function downloadMSTeamsBotFrameworkAttachment(params: {
|
||||
contentType: saved.contentType,
|
||||
placeholder: inferPlaceholder({ contentType: saved.contentType, fileName: fileNameHint }),
|
||||
};
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework save failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
@@ -258,6 +299,7 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
|
||||
fileNameHint?: string | null;
|
||||
contentTypeHint?: string | null;
|
||||
preserveFilenames?: boolean;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<MSTeamsGraphMediaResult> {
|
||||
const seen = new Set<string>();
|
||||
const unique: string[] = [];
|
||||
@@ -290,12 +332,16 @@ export async function downloadMSTeamsBotFrameworkAttachments(params: {
|
||||
fileNameHint: params.fileNameHint,
|
||||
contentTypeHint: params.contentTypeHint,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
logger: params.logger,
|
||||
});
|
||||
if (item) {
|
||||
media.push(item);
|
||||
}
|
||||
} catch {
|
||||
// Ignore per-attachment failures and continue.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams botFramework attachment download failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
attachmentId,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
isDownloadableAttachment,
|
||||
isRecord,
|
||||
isUrlAllowed,
|
||||
type MSTeamsAttachmentDownloadLogger,
|
||||
type MSTeamsAttachmentFetchPolicy,
|
||||
normalizeContentType,
|
||||
resolveMediaSsrfPolicy,
|
||||
@@ -179,6 +180,12 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
fetchFn?: typeof fetch;
|
||||
/** When true, embeds original filename in stored path for later extraction. */
|
||||
preserveFilenames?: boolean;
|
||||
/**
|
||||
* Optional logger used to surface inline data decode failures and remote
|
||||
* media download errors. Errors that are not logged here are invisible at
|
||||
* INFO level and block diagnosis of issues like #63396.
|
||||
*/
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<MSTeamsInboundMedia[]> {
|
||||
const list = Array.isArray(params.attachments) ? params.attachments : [];
|
||||
if (list.length === 0) {
|
||||
@@ -245,8 +252,10 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
contentType: saved.contentType,
|
||||
placeholder: inline.placeholder,
|
||||
});
|
||||
} catch {
|
||||
// Ignore decode failures and continue.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams inline attachment decode failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
}
|
||||
for (const candidate of candidates) {
|
||||
@@ -262,6 +271,11 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
placeholder: candidate.placeholder,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
ssrfPolicy,
|
||||
// `fetchImpl` below already validates each hop against the hostname
|
||||
// allowlist via `safeFetchWithPolicy`, so skip `fetchRemoteMedia`'s
|
||||
// strict SSRF dispatcher (incompatible with Node 24+ / undici v7;
|
||||
// see issue #63396).
|
||||
useDirectFetch: true,
|
||||
fetchImpl: (input, init) =>
|
||||
fetchWithAuthFallback({
|
||||
url: resolveRequestUrl(input),
|
||||
@@ -272,13 +286,24 @@ export async function downloadMSTeamsAttachments(params: {
|
||||
}),
|
||||
});
|
||||
out.push(media);
|
||||
} catch {
|
||||
// Ignore download failures and continue with next candidate.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams attachment download failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
host: safeHostForLog(candidate.url),
|
||||
});
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function safeHostForLog(url: string): string {
|
||||
try {
|
||||
return new URL(url).host;
|
||||
} catch {
|
||||
return "invalid-url";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated Use `downloadMSTeamsAttachments` instead (supports all file types).
|
||||
*/
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
inferPlaceholder,
|
||||
readNestedString,
|
||||
isUrlAllowed,
|
||||
type MSTeamsAttachmentDownloadLogger,
|
||||
type MSTeamsAttachmentFetchPolicy,
|
||||
normalizeContentType,
|
||||
resolveMediaSsrfPolicy,
|
||||
@@ -177,6 +178,7 @@ async function downloadGraphHostedContent(params: {
|
||||
fetchFn?: typeof fetch;
|
||||
preserveFilenames?: boolean;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<{ media: MSTeamsInboundMedia[]; status: number; count: number }> {
|
||||
const hosted = await fetchGraphCollection<GraphHostedContent>({
|
||||
url: `${params.messageUrl}/hostedContents`,
|
||||
@@ -198,7 +200,10 @@ async function downloadGraphHostedContent(params: {
|
||||
}
|
||||
try {
|
||||
buffer = Buffer.from(contentBytes, "base64");
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams graph hostedContent base64 decode failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
} else if (item.id) {
|
||||
@@ -228,7 +233,10 @@ async function downloadGraphHostedContent(params: {
|
||||
} finally {
|
||||
await release();
|
||||
}
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams graph hostedContent value fetch failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
@@ -254,8 +262,10 @@ async function downloadGraphHostedContent(params: {
|
||||
contentType: saved.contentType,
|
||||
placeholder: inferPlaceholder({ contentType: saved.contentType }),
|
||||
});
|
||||
} catch {
|
||||
// Ignore save failures.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams graph hostedContent save failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -271,6 +281,12 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
fetchFn?: typeof fetch;
|
||||
/** When true, embeds original filename in stored path for later extraction. */
|
||||
preserveFilenames?: boolean;
|
||||
/**
|
||||
* Optional logger used to surface Graph/SharePoint fetch errors. Without
|
||||
* it, empty `catch` blocks hide failures like the Node 24+ undici
|
||||
* incompatibility that silently broke SharePoint downloads (#63396).
|
||||
*/
|
||||
logger?: MSTeamsAttachmentDownloadLogger;
|
||||
}): Promise<MSTeamsGraphMediaResult> {
|
||||
if (!params.messageUrl || !params.tokenProvider) {
|
||||
return { media: [] };
|
||||
@@ -284,7 +300,10 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
let accessToken: string;
|
||||
try {
|
||||
accessToken = await params.tokenProvider.getAccessToken("https://graph.microsoft.com");
|
||||
} catch {
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams graph token acquisition failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return { media: [], messageUrl, tokenError: true };
|
||||
}
|
||||
|
||||
@@ -303,6 +322,11 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
auditContext: "msteams.graph.message",
|
||||
});
|
||||
try {
|
||||
if (!msgRes.ok) {
|
||||
params.logger?.warn?.("msteams graph message fetch non-ok", {
|
||||
status: msgRes.status,
|
||||
});
|
||||
}
|
||||
if (msgRes.ok) {
|
||||
const msgData = (await msgRes.json()) as {
|
||||
body?: { content?: string; contentType?: string };
|
||||
@@ -340,6 +364,12 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
contentTypeHint: "application/octet-stream",
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
ssrfPolicy,
|
||||
// The `fetchImpl` below already validates each redirect hop via
|
||||
// `safeFetchWithPolicy`, so bypass `fetchRemoteMedia`'s strict
|
||||
// SSRF dispatcher. That dispatcher is incompatible with Node
|
||||
// 24+'s built-in undici v7 and silently breaks SharePoint
|
||||
// downloads (#63396).
|
||||
useDirectFetch: true,
|
||||
fetchImpl: async (input, init) => {
|
||||
const requestUrl = resolveRequestUrl(input);
|
||||
const headers = ensureUserAgentHeader(init?.headers);
|
||||
@@ -362,16 +392,21 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
});
|
||||
sharePointMedia.push(media);
|
||||
downloadedReferenceUrls.add(shareUrl);
|
||||
} catch {
|
||||
// Ignore SharePoint download failures.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams SharePoint reference download failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
name,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
await release();
|
||||
}
|
||||
} catch {
|
||||
// Ignore message fetch failures.
|
||||
} catch (err) {
|
||||
params.logger?.warn?.("msteams graph message fetch failed", {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
|
||||
const hosted = await downloadGraphHostedContent({
|
||||
@@ -381,6 +416,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
fetchFn: params.fetchFn,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
ssrfPolicy,
|
||||
logger: params.logger,
|
||||
});
|
||||
|
||||
const attachments = await fetchGraphCollection<GraphAttachment>({
|
||||
@@ -413,6 +449,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
||||
authAllowHosts: policy.authAllowHosts,
|
||||
fetchFn: params.fetchFn,
|
||||
preserveFilenames: params.preserveFilenames,
|
||||
logger: params.logger,
|
||||
});
|
||||
|
||||
return {
|
||||
|
||||
137
extensions/msteams/src/attachments/remote-media.test.ts
Normal file
137
extensions/msteams/src/attachments/remote-media.test.ts
Normal file
@@ -0,0 +1,137 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Mock the runtime so we can assert whether the strict-dispatcher path
|
||||
// (`fetchRemoteMedia`) was invoked versus the new direct-fetch path added
|
||||
// for issue #63396 (Node 24+ / undici v7 compat).
|
||||
const runtimeFetchRemoteMediaMock = vi.fn();
|
||||
const runtimeDetectMimeMock = vi.fn(async () => "image/png");
|
||||
const runtimeSaveMediaBufferMock = vi.fn(async (_buf: Buffer, contentType?: string) => ({
|
||||
id: "saved",
|
||||
path: "/tmp/saved.png",
|
||||
size: 42,
|
||||
contentType: contentType ?? "image/png",
|
||||
}));
|
||||
|
||||
vi.mock("../runtime.js", () => ({
|
||||
getMSTeamsRuntime: () => ({
|
||||
media: { detectMime: runtimeDetectMimeMock },
|
||||
channel: {
|
||||
media: {
|
||||
fetchRemoteMedia: runtimeFetchRemoteMediaMock,
|
||||
saveMediaBuffer: runtimeSaveMediaBufferMock,
|
||||
},
|
||||
},
|
||||
}),
|
||||
}));
|
||||
|
||||
import { downloadAndStoreMSTeamsRemoteMedia } from "./remote-media.js";
|
||||
|
||||
const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
|
||||
|
||||
function jsonResponse(body: BodyInit, init?: ResponseInit): Response {
|
||||
return new Response(body, init);
|
||||
}
|
||||
|
||||
describe("downloadAndStoreMSTeamsRemoteMedia", () => {
|
||||
beforeEach(() => {
|
||||
runtimeFetchRemoteMediaMock.mockReset();
|
||||
runtimeDetectMimeMock.mockClear();
|
||||
runtimeSaveMediaBufferMock.mockClear();
|
||||
});
|
||||
|
||||
describe("useDirectFetch: true (Node 24+ / undici v7 path for issue #63396)", () => {
|
||||
it("bypasses fetchRemoteMedia and calls the supplied fetchImpl directly", async () => {
|
||||
// `fetchImpl` here simulates the "pre-validated hostname" contract from
|
||||
// `safeFetchWithPolicy`: the caller has already enforced the allowlist,
|
||||
// so the strict SSRF dispatcher is not needed.
|
||||
const fetchImpl = vi.fn(async (_input: RequestInfo | URL, _init?: RequestInit) =>
|
||||
jsonResponse(PNG_BYTES, { status: 200, headers: { "content-type": "image/png" } }),
|
||||
);
|
||||
|
||||
const result = await downloadAndStoreMSTeamsRemoteMedia({
|
||||
url: "https://graph.microsoft.com/v1.0/shares/abc/driveItem/content",
|
||||
filePathHint: "file.png",
|
||||
maxBytes: 1024,
|
||||
useDirectFetch: true,
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(1);
|
||||
const [calledUrl] = fetchImpl.mock.calls[0] ?? [];
|
||||
expect(calledUrl).toBe("https://graph.microsoft.com/v1.0/shares/abc/driveItem/content");
|
||||
expect(runtimeFetchRemoteMediaMock).not.toHaveBeenCalled();
|
||||
expect(result.path).toBe("/tmp/saved.png");
|
||||
});
|
||||
|
||||
it("surfaces HTTP errors as exceptions (no silent drop)", async () => {
|
||||
const fetchImpl = vi.fn(async () => jsonResponse("nope", { status: 403 }));
|
||||
|
||||
await expect(
|
||||
downloadAndStoreMSTeamsRemoteMedia({
|
||||
url: "https://graph.microsoft.com/v1.0/shares/abc/driveItem/content",
|
||||
filePathHint: "file.png",
|
||||
maxBytes: 1024,
|
||||
useDirectFetch: true,
|
||||
fetchImpl,
|
||||
}),
|
||||
).rejects.toThrow(/HTTP 403/);
|
||||
expect(runtimeFetchRemoteMediaMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a response whose Content-Length exceeds maxBytes", async () => {
|
||||
const fetchImpl = vi.fn(async () =>
|
||||
jsonResponse(PNG_BYTES, {
|
||||
status: 200,
|
||||
headers: { "content-length": "999999" },
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(
|
||||
downloadAndStoreMSTeamsRemoteMedia({
|
||||
url: "https://graph.microsoft.com/v1.0/shares/abc/driveItem/content",
|
||||
filePathHint: "file.png",
|
||||
maxBytes: 1024,
|
||||
useDirectFetch: true,
|
||||
fetchImpl,
|
||||
}),
|
||||
).rejects.toThrow(/exceeds maxBytes/);
|
||||
expect(runtimeFetchRemoteMediaMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to the runtime fetchRemoteMedia path when useDirectFetch is omitted", async () => {
|
||||
// Non-SharePoint caller, no pre-validated fetchImpl: make sure the strict
|
||||
// SSRF dispatcher path is still used.
|
||||
runtimeFetchRemoteMediaMock.mockResolvedValueOnce({
|
||||
buffer: PNG_BYTES,
|
||||
contentType: "image/png",
|
||||
fileName: "file.png",
|
||||
});
|
||||
|
||||
await downloadAndStoreMSTeamsRemoteMedia({
|
||||
url: "https://tenant.sharepoint.com/file.png",
|
||||
filePathHint: "file.png",
|
||||
maxBytes: 1024,
|
||||
});
|
||||
|
||||
expect(runtimeFetchRemoteMediaMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not use the direct path when useDirectFetch is true but fetchImpl is missing", async () => {
|
||||
runtimeFetchRemoteMediaMock.mockResolvedValueOnce({
|
||||
buffer: PNG_BYTES,
|
||||
contentType: "image/png",
|
||||
});
|
||||
|
||||
await downloadAndStoreMSTeamsRemoteMedia({
|
||||
url: "https://graph.microsoft.com/v1.0/shares/abc/driveItem/content",
|
||||
filePathHint: "file.png",
|
||||
maxBytes: 1024,
|
||||
useDirectFetch: true,
|
||||
});
|
||||
|
||||
// Without a fetchImpl to delegate to, we must fall back to the runtime
|
||||
// path rather than crashing.
|
||||
expect(runtimeFetchRemoteMediaMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,4 @@
|
||||
import { readResponseWithLimit } from "openclaw/plugin-sdk/media-runtime";
|
||||
import type { SsrFPolicy } from "../../runtime-api.js";
|
||||
import { getMSTeamsRuntime } from "../runtime.js";
|
||||
import { inferPlaceholder } from "./shared.js";
|
||||
@@ -5,6 +6,56 @@ import type { MSTeamsInboundMedia } from "./types.js";
|
||||
|
||||
type FetchLike = (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>;
|
||||
|
||||
type FetchedRemoteMedia = {
|
||||
buffer: Buffer;
|
||||
contentType?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Direct fetch path used when the caller's `fetchImpl` has already validated
|
||||
* the URL against a hostname allowlist (for example `safeFetchWithPolicy`).
|
||||
*
|
||||
* Bypasses the strict SSRF dispatcher on `fetchRemoteMedia` because:
|
||||
* 1. The pinned undici dispatcher used by `fetchRemoteMedia` is incompatible
|
||||
* with Node 24+'s built-in undici v7 (fails with "invalid onRequestStart
|
||||
* method"), which silently breaks SharePoint/OneDrive downloads. See
|
||||
* issue #63396.
|
||||
* 2. SSRF protection is already enforced by the caller's `fetchImpl`
|
||||
* (`safeFetch` validates every redirect hop against the hostname
|
||||
* allowlist before following).
|
||||
*/
|
||||
async function fetchRemoteMediaDirect(params: {
|
||||
url: string;
|
||||
fetchImpl: FetchLike;
|
||||
maxBytes: number;
|
||||
}): Promise<FetchedRemoteMedia> {
|
||||
const response = await params.fetchImpl(params.url, { redirect: "follow" });
|
||||
if (!response.ok) {
|
||||
const statusText = response.statusText ? ` ${response.statusText}` : "";
|
||||
throw new Error(`HTTP ${response.status}${statusText}`);
|
||||
}
|
||||
|
||||
// Enforce the max-bytes cap before buffering the full body so a rogue
|
||||
// response cannot drive RSS usage past the configured limit.
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (contentLength) {
|
||||
const length = Number(contentLength);
|
||||
if (Number.isFinite(length) && length > params.maxBytes) {
|
||||
throw new Error(`content length ${length} exceeds maxBytes ${params.maxBytes}`);
|
||||
}
|
||||
}
|
||||
|
||||
const buffer = await readResponseWithLimit(response, params.maxBytes, {
|
||||
onOverflow: ({ size, maxBytes }) =>
|
||||
new Error(`payload size ${size} exceeds maxBytes ${maxBytes}`),
|
||||
});
|
||||
|
||||
return {
|
||||
buffer,
|
||||
contentType: response.headers.get("content-type") ?? undefined,
|
||||
};
|
||||
}
|
||||
|
||||
export async function downloadAndStoreMSTeamsRemoteMedia(params: {
|
||||
url: string;
|
||||
filePathHint: string;
|
||||
@@ -14,14 +65,30 @@ export async function downloadAndStoreMSTeamsRemoteMedia(params: {
|
||||
contentTypeHint?: string;
|
||||
placeholder?: string;
|
||||
preserveFilenames?: boolean;
|
||||
/**
|
||||
* Opt into a direct fetch path that bypasses `fetchRemoteMedia`'s strict
|
||||
* SSRF dispatcher. Required for SharePoint/OneDrive downloads on Node 24+
|
||||
* (see issue #63396). Only safe when the supplied `fetchImpl` has already
|
||||
* validated the URL against a hostname allowlist.
|
||||
*/
|
||||
useDirectFetch?: boolean;
|
||||
}): Promise<MSTeamsInboundMedia> {
|
||||
const fetched = await getMSTeamsRuntime().channel.media.fetchRemoteMedia({
|
||||
url: params.url,
|
||||
fetchImpl: params.fetchImpl,
|
||||
filePathHint: params.filePathHint,
|
||||
maxBytes: params.maxBytes,
|
||||
ssrfPolicy: params.ssrfPolicy,
|
||||
});
|
||||
let fetched: FetchedRemoteMedia;
|
||||
if (params.useDirectFetch && params.fetchImpl) {
|
||||
fetched = await fetchRemoteMediaDirect({
|
||||
url: params.url,
|
||||
fetchImpl: params.fetchImpl,
|
||||
maxBytes: params.maxBytes,
|
||||
});
|
||||
} else {
|
||||
fetched = await getMSTeamsRuntime().channel.media.fetchRemoteMedia({
|
||||
url: params.url,
|
||||
fetchImpl: params.fetchImpl,
|
||||
filePathHint: params.filePathHint,
|
||||
maxBytes: params.maxBytes,
|
||||
ssrfPolicy: params.ssrfPolicy,
|
||||
});
|
||||
}
|
||||
const mime = await getMSTeamsRuntime().media.detectMime({
|
||||
buffer: fetched.buffer,
|
||||
headerMime: fetched.contentType ?? params.contentTypeHint,
|
||||
|
||||
@@ -391,6 +391,17 @@ export type MSTeamsAttachmentFetchPolicy = {
|
||||
authAllowHosts: string[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Logger surface for attachment download errors. Structured so callers can
|
||||
* pass `MSTeamsMonitorLogger` directly without adapters. Optional `warn`/
|
||||
* `error` methods prevent silent swallowing of fetch failures — see issue
|
||||
* #63396 where empty `catch {}` blocks hid a Node 24+ undici incompatibility.
|
||||
*/
|
||||
export type MSTeamsAttachmentDownloadLogger = {
|
||||
warn?: (message: string, meta?: Record<string, unknown>) => void;
|
||||
error?: (message: string, meta?: Record<string, unknown>) => void;
|
||||
};
|
||||
|
||||
export function resolveAttachmentFetchPolicy(params?: {
|
||||
allowHosts?: string[];
|
||||
authAllowHosts?: string[];
|
||||
|
||||
@@ -14,6 +14,8 @@ import type { MSTeamsTurnContext } from "../sdk-types.js";
|
||||
|
||||
type MSTeamsLogger = {
|
||||
debug?: (message: string, meta?: Record<string, unknown>) => void;
|
||||
warn?: (message: string, meta?: Record<string, unknown>) => void;
|
||||
error?: (message: string, meta?: Record<string, unknown>) => void;
|
||||
};
|
||||
|
||||
export async function resolveMSTeamsInboundMedia(params: {
|
||||
@@ -54,6 +56,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
||||
allowHosts,
|
||||
authAllowHosts: params.authAllowHosts,
|
||||
preserveFilenames,
|
||||
logger: log,
|
||||
});
|
||||
|
||||
if (mediaList.length === 0) {
|
||||
@@ -87,6 +90,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
||||
allowHosts,
|
||||
authAllowHosts: params.authAllowHosts,
|
||||
preserveFilenames,
|
||||
logger: log,
|
||||
});
|
||||
if (bfMedia.media.length > 0) {
|
||||
mediaList = bfMedia.media;
|
||||
@@ -137,6 +141,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
||||
allowHosts,
|
||||
authAllowHosts: params.authAllowHosts,
|
||||
preserveFilenames,
|
||||
logger: log,
|
||||
});
|
||||
attempts.push({
|
||||
url: messageUrl,
|
||||
|
||||
Reference in New Issue
Block a user