From 5a1ff1347dfaed1bd26dc86d2309b03cc1cb1ecc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 11:35:20 +0100 Subject: [PATCH] fix(slack): bound inbound media downloads --- CHANGELOG.md | 1 + docs/channels/slack.md | 2 + docs/gateway/config-channels.md | 6 + extensions/slack/src/monitor/media.test.ts | 63 ++++++++++ extensions/slack/src/monitor/media.ts | 131 +++++++++++++++++++-- 5 files changed, 190 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de70f4686d..305d44054ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai - Agents/media: keep long-running `video_generate` and `music_generate` tasks fresh while provider jobs are still pending, so task maintenance does not mark active Discord media renders lost before completion. Thanks @vincentkoc. - CLI/status: treat scope-limited gateway probes as reachable-but-degraded in shared status scans, so `openclaw status --all` no longer reports a live gateway as unreachable after `missing scope: operator.read`. Fixes #49180; supersedes #47981. Thanks @openjay. - Slack/Socket Mode: use a 15s Slack SDK pong timeout by default and add `channels.slack.socketMode.clientPingTimeout`, `serverPingTimeout`, and `pingPongLoggingEnabled` overrides so stale-websocket handling no longer depends on app-event health heuristics. Fixes #14248; refs #58519, #64009, and #63488. Thanks @shivasymbl and @freerk. +- Slack/media: bound private file and forwarded attachment downloads with idle and total timeouts while preserving placeholder fallback, so stalled Slack `file_share` media no longer wedges inbound message handling. Fixes #61850. Thanks @bassboy2k. - Plugins/inspector: keep bundled plugin runtime capture quiet and config-tolerant for Codex, memory-lancedb, Feishu, Mattermost, QQBot, and Tlon so plugin-inspector JSON checks can validate the full bundled set. Thanks @vincentkoc. - Slack/auto-reply: keep fully consumed text reset triggers such as `new session` out of `BodyForAgent` after directive cleanup, so configured Slack reset phrases do not leak into the fresh model turn. Fixes #73137. Thanks @neeravmakwana. - Plugins/runtime deps: prune stale retained bundled runtime deps and keep doctor/secret channel contract scans on lightweight artifacts, so disabled bundled channels stop preserving old dependency trees or importing heavy plugin surfaces. Thanks @SymbolStar and @vincentkoc. diff --git a/docs/channels/slack.md b/docs/channels/slack.md index 0eaef6997fa..9a4abd41686 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -632,6 +632,8 @@ Notes: Slack file attachments are downloaded from Slack-hosted private URLs (token-authenticated request flow) and written to the media store when fetch succeeds and size limits permit. File placeholders include the Slack `fileId` so agents can fetch the original file with `download-file`. + Downloads use bounded idle and total timeouts. If Slack file retrieval stalls or fails, OpenClaw keeps processing the message and falls back to the file placeholder. + Runtime inbound size cap defaults to `20MB` unless overridden by `channels.slack.mediaMaxMb`. diff --git a/docs/gateway/config-channels.md b/docs/gateway/config-channels.md index 04a1463b0b0..fa2672d3d18 100644 --- a/docs/gateway/config-channels.md +++ b/docs/gateway/config-channels.md @@ -390,6 +390,11 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat enabled: true, botToken: "xoxb-...", appToken: "xapp-...", + socketMode: { + clientPingTimeout: 15000, + serverPingTimeout: 30000, + pingPongLoggingEnabled: false, + }, dmPolicy: "pairing", allowFrom: ["U123", "U456", "*"], dm: { enabled: true, groupEnabled: false, groupChannels: ["G123"] }, @@ -448,6 +453,7 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat - **Socket mode** requires both `botToken` and `appToken` (`SLACK_BOT_TOKEN` + `SLACK_APP_TOKEN` for default account env fallback). - **HTTP mode** requires `botToken` plus `signingSecret` (at root or per-account). +- `socketMode` passes Slack SDK Socket Mode transport tuning through to the public Bolt receiver API. Use it only when investigating ping/pong timeout or stale websocket behavior. - `botToken`, `appToken`, `signingSecret`, and `userToken` accept plaintext strings or SecretRef objects. - Slack account snapshots expose per-credential source/status fields such as diff --git a/extensions/slack/src/monitor/media.test.ts b/extensions/slack/src/monitor/media.test.ts index 55bb6ce8887..5487b679d02 100644 --- a/extensions/slack/src/monitor/media.test.ts +++ b/extensions/slack/src/monitor/media.test.ts @@ -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((_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: [], diff --git a/extensions/slack/src/monitor/media.ts b/extensions/slack/src/monitor/media.ts index e4177087b39..b1f20910cd7 100644 --- a/extensions/slack/src/monitor/media.ts +++ b/extensions/slack/src/monitor/media.ts @@ -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[0]; + +function mergeAbortSignals(signals: Array): 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 { + 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 | 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(() => {}); + } + throw error; + }); + + try { + if (!params.totalTimeoutMs) { + return await fetchPromise; + } + const timeoutPromise = new Promise((_, 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 { 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);