fix(slack): bound inbound media downloads

This commit is contained in:
Peter Steinberger
2026-04-28 11:35:20 +01:00
parent a722da3ed0
commit 5a1ff1347d
5 changed files with 190 additions and 13 deletions

View File

@@ -6,6 +6,7 @@ import {
resolveSlackThreadHistory,
resolveSlackThreadStarter,
resetSlackThreadStarterCacheForTest,
SLACK_MEDIA_READ_IDLE_TIMEOUT_MS,
} from "./media.js";
import type { FetchLike, SavedMedia } from "./media.runtime.js";
import * as mediaRuntime from "./media.runtime.js";
@@ -19,7 +20,10 @@ const fetchRemoteMediaMock = vi.hoisted(() =>
url: string;
fetchImpl: FetchLike;
filePathHint?: string;
maxBytes?: number;
readIdleTimeoutMs?: number;
requestInit?: RequestInit;
ssrfPolicy?: unknown;
}) => {
let response = await params.fetchImpl(params.url, {
...params.requestInit,
@@ -355,6 +359,65 @@ describe("resolveSlackMedia", () => {
expect(result).toBeNull();
});
it("passes bounded media download timeouts while preserving Slack auth", async () => {
vi.spyOn(mediaRuntime, "saveMediaBuffer").mockResolvedValue(
createSavedMedia("/tmp/test.jpg", "image/jpeg"),
);
mockFetch.mockResolvedValueOnce(
new Response(Buffer.from("image data"), {
status: 200,
headers: { "content-type": "image/jpeg" },
}),
);
const result = await resolveSlackMedia({
files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).not.toBeNull();
const fetchOptions = fetchRemoteMediaMock.mock.calls[0]?.[0];
expect(fetchOptions?.readIdleTimeoutMs).toBe(SLACK_MEDIA_READ_IDLE_TIMEOUT_MS);
expect(fetchOptions?.requestInit?.signal).toBeInstanceOf(AbortSignal);
expect(new Headers(fetchOptions?.requestInit?.headers).get("Authorization")).toBe(
"Bearer xoxb-test-token",
);
});
it("returns null when a media download exceeds the total timeout", async () => {
vi.useFakeTimers();
try {
let abortSignal: AbortSignal | undefined;
fetchRemoteMediaMock.mockImplementationOnce(
(params) =>
new Promise<never>((_resolve, reject) => {
abortSignal = params.requestInit?.signal ?? undefined;
abortSignal?.addEventListener(
"abort",
() => {
reject(Object.assign(new Error("aborted"), { name: "AbortError" }));
},
{ once: true },
);
}),
);
const resultPromise = resolveSlackMedia({
files: [{ url_private: "https://files.slack.com/slow.jpg", name: "slow.jpg" }],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
totalTimeoutMs: 25,
});
await vi.advanceTimersByTimeAsync(25);
await expect(resultPromise).resolves.toBeNull();
expect(abortSignal?.aborted).toBe(true);
} finally {
vi.useRealTimers();
}
});
it("returns null when no files are provided", async () => {
const result = await resolveSlackMedia({
files: [],

View File

@@ -146,6 +146,92 @@ const SLACK_MEDIA_SSRF_POLICY = {
hostnameAllowlist: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"],
allowRfc2544BenchmarkRange: true,
};
export const SLACK_MEDIA_READ_IDLE_TIMEOUT_MS = 60_000;
export const SLACK_MEDIA_TOTAL_TIMEOUT_MS = 120_000;
type SlackFetchRemoteMediaOptions = Parameters<typeof fetchRemoteMedia>[0];
function mergeAbortSignals(signals: Array<AbortSignal | undefined>): AbortSignal | undefined {
const activeSignals = signals.filter((signal): signal is AbortSignal => Boolean(signal));
if (activeSignals.length === 0) {
return undefined;
}
if (activeSignals.length === 1) {
return activeSignals[0];
}
if (typeof AbortSignal.any === "function") {
return AbortSignal.any(activeSignals);
}
const controller = new AbortController();
for (const signal of activeSignals) {
if (signal.aborted) {
controller.abort();
return controller.signal;
}
}
const abort = () => {
controller.abort();
for (const signal of activeSignals) {
signal.removeEventListener("abort", abort);
}
};
for (const signal of activeSignals) {
signal.addEventListener("abort", abort, { once: true });
}
return controller.signal;
}
async function fetchSlackMedia(params: {
options: SlackFetchRemoteMediaOptions;
readIdleTimeoutMs?: number;
totalTimeoutMs?: number;
abortSignal?: AbortSignal;
}): ReturnType<typeof fetchRemoteMedia> {
const timeoutAbortController = params.totalTimeoutMs ? new AbortController() : undefined;
const signal = mergeAbortSignals([
params.abortSignal,
params.options.requestInit?.signal ?? undefined,
timeoutAbortController?.signal,
]);
let timedOut = false;
let timeoutHandle: ReturnType<typeof setTimeout> | null = null;
const fetchPromise = fetchRemoteMedia({
...params.options,
readIdleTimeoutMs: params.readIdleTimeoutMs ?? SLACK_MEDIA_READ_IDLE_TIMEOUT_MS,
...(signal
? {
requestInit: {
...params.options.requestInit,
signal,
},
}
: {}),
}).catch((error) => {
if (timedOut) {
return new Promise<never>(() => {});
}
throw error;
});
try {
if (!params.totalTimeoutMs) {
return await fetchPromise;
}
const timeoutPromise = new Promise<never>((_, reject) => {
timeoutHandle = setTimeout(() => {
timedOut = true;
timeoutAbortController?.abort();
reject(new Error(`slack media download timed out after ${params.totalTimeoutMs}ms`));
}, params.totalTimeoutMs);
timeoutHandle.unref?.();
});
return await Promise.race([fetchPromise, timeoutPromise]);
} finally {
if (timeoutHandle) {
clearTimeout(timeoutHandle);
}
}
}
/**
* Slack voice messages (audio clips, huddle recordings) carry a `subtype` of
@@ -229,6 +315,9 @@ export async function resolveSlackMedia(params: {
files?: SlackFile[];
token: string;
maxBytes: number;
readIdleTimeoutMs?: number;
totalTimeoutMs?: number;
abortSignal?: AbortSignal;
}): Promise<SlackMediaResult[] | null> {
const files = params.files ?? [];
const limitedFiles =
@@ -245,13 +334,18 @@ export async function resolveSlackMedia(params: {
try {
const { url: slackUrl, requestInit } = createSlackMediaRequest(url, params.token);
const fetchImpl = createSlackMediaFetch();
const fetched = await fetchRemoteMedia({
url: slackUrl,
fetchImpl,
requestInit,
filePathHint: file.name,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
const fetched = await fetchSlackMedia({
options: {
url: slackUrl,
fetchImpl,
requestInit,
filePathHint: file.name,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
},
readIdleTimeoutMs: params.readIdleTimeoutMs,
totalTimeoutMs: params.totalTimeoutMs ?? SLACK_MEDIA_TOTAL_TIMEOUT_MS,
abortSignal: params.abortSignal,
});
if (fetched.buffer.byteLength > params.maxBytes) {
return null;
@@ -299,6 +393,9 @@ export async function resolveSlackAttachmentContent(params: {
attachments?: SlackAttachment[];
token: string;
maxBytes: number;
readIdleTimeoutMs?: number;
totalTimeoutMs?: number;
abortSignal?: AbortSignal;
}): Promise<{ text: string; media: SlackMediaResult[] } | null> {
const attachments = params.attachments;
if (!attachments || attachments.length === 0) {
@@ -328,12 +425,17 @@ export async function resolveSlackAttachmentContent(params: {
try {
const { url: slackUrl, requestInit } = createSlackMediaRequest(imageUrl, params.token);
const fetchImpl = createSlackMediaFetch();
const fetched = await fetchRemoteMedia({
url: slackUrl,
fetchImpl,
requestInit,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
const fetched = await fetchSlackMedia({
options: {
url: slackUrl,
fetchImpl,
requestInit,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
},
readIdleTimeoutMs: params.readIdleTimeoutMs,
totalTimeoutMs: params.totalTimeoutMs ?? SLACK_MEDIA_TOTAL_TIMEOUT_MS,
abortSignal: params.abortSignal,
});
if (fetched.buffer.byteLength <= params.maxBytes) {
const saved = await saveMediaBuffer(
@@ -359,6 +461,9 @@ export async function resolveSlackAttachmentContent(params: {
files: att.files,
token: params.token,
maxBytes: params.maxBytes,
readIdleTimeoutMs: params.readIdleTimeoutMs,
totalTimeoutMs: params.totalTimeoutMs,
abortSignal: params.abortSignal,
});
if (fileMedia) {
allMedia.push(...fileMedia);