From 39a45121d9b672efe0ddee9255e5ae4e869a01f7 Mon Sep 17 00:00:00 2001 From: Sid Date: Mon, 2 Mar 2026 01:30:10 +0800 Subject: [PATCH] fix(discord,slack): add SSRF policy for media downloads in proxy environments (#25475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(discord,slack): add SSRF policy for media downloads in proxy environments Discord and Slack media downloads (attachments, stickers, forwarded images) call fetchRemoteMedia without any ssrfPolicy. When running behind a local transparent proxy (Clash, mihomo, Shadowrocket) in fake-ip mode, DNS returns virtual IPs in the 198.18.0.0/15 range, which the SSRF guard blocks. Add per-channel SSRF policy constants—matching the pattern already applied to Telegram on main—that allowlist known CDN hostnames and set allowRfc2544BenchmarkRange: true. Refs #25355, #25322 Co-authored-by: Cursor * chore(slack): keep raw-fetch allowlist line anchors stable --------- Co-authored-by: Cursor Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- src/discord/monitor/message-utils.test.ts | 34 ++++++++++ src/discord/monitor/message-utils.ts | 8 +++ src/slack/monitor/media.test.ts | 75 +++++++++++++++++++++++ src/slack/monitor/media.ts | 7 +++ 4 files changed, 124 insertions(+) diff --git a/src/discord/monitor/message-utils.test.ts b/src/discord/monitor/message-utils.test.ts index 28dd142a1e4..300780b0d54 100644 --- a/src/discord/monitor/message-utils.test.ts +++ b/src/discord/monitor/message-utils.test.ts @@ -94,6 +94,7 @@ describe("resolveForwardedMediaList", () => { filePathHint: attachment.filename, maxBytes: 512, fetchImpl: undefined, + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), }); expect(saveMediaBuffer).toHaveBeenCalledTimes(1); expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); @@ -168,6 +169,7 @@ describe("resolveForwardedMediaList", () => { filePathHint: "wave.png", maxBytes: 512, fetchImpl: undefined, + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), }); expect(saveMediaBuffer).toHaveBeenCalledTimes(1); expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); @@ -236,6 +238,7 @@ describe("resolveMediaList", () => { filePathHint: "hello.png", maxBytes: 512, fetchImpl: undefined, + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), }); expect(saveMediaBuffer).toHaveBeenCalledTimes(1); expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); @@ -278,6 +281,37 @@ describe("resolveMediaList", () => { }); }); +describe("Discord media SSRF policy", () => { + beforeEach(() => { + fetchRemoteMedia.mockClear(); + saveMediaBuffer.mockClear(); + }); + + it("passes ssrfPolicy with Discord CDN allowedHostnames and allowRfc2544BenchmarkRange", async () => { + fetchRemoteMedia.mockResolvedValueOnce({ + buffer: Buffer.from("img"), + contentType: "image/png", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/a.png", + contentType: "image/png", + }); + + await resolveMediaList( + asMessage({ + attachments: [{ id: "a1", url: "https://cdn.discordapp.com/a.png", filename: "a.png" }], + }), + 1024, + ); + + const policy = fetchRemoteMedia.mock.calls[0][0].ssrfPolicy; + expect(policy).toEqual({ + allowedHostnames: ["cdn.discordapp.com", "media.discordapp.net"], + allowRfc2544BenchmarkRange: true, + }); + }); +}); + describe("resolveDiscordMessageText", () => { it("includes forwarded message snapshots in body text", () => { const text = resolveDiscordMessageText( diff --git a/src/discord/monitor/message-utils.ts b/src/discord/monitor/message-utils.ts index b18e877b1ce..bf446686e59 100644 --- a/src/discord/monitor/message-utils.ts +++ b/src/discord/monitor/message-utils.ts @@ -2,9 +2,15 @@ import type { ChannelType, Client, Message } from "@buape/carbon"; import { StickerFormatType, type APIAttachment, type APIStickerItem } from "discord-api-types/v10"; import { buildMediaPayload } from "../../channels/plugins/media-payload.js"; import { logVerbose } from "../../globals.js"; +import type { SsrFPolicy } from "../../infra/net/ssrf.js"; import { fetchRemoteMedia, type FetchLike } from "../../media/fetch.js"; import { saveMediaBuffer } from "../../media/store.js"; +const DISCORD_MEDIA_SSRF_POLICY: SsrFPolicy = { + allowedHostnames: ["cdn.discordapp.com", "media.discordapp.net"], + allowRfc2544BenchmarkRange: true, +}; + export type DiscordMediaInfo = { path: string; contentType?: string; @@ -228,6 +234,7 @@ async function appendResolvedMediaFromAttachments(params: { filePathHint: attachment.filename ?? attachment.url, maxBytes: params.maxBytes, fetchImpl: params.fetchImpl, + ssrfPolicy: DISCORD_MEDIA_SSRF_POLICY, }); const saved = await saveMediaBuffer( fetched.buffer, @@ -320,6 +327,7 @@ async function appendResolvedMediaFromStickers(params: { filePathHint: candidate.fileName, maxBytes: params.maxBytes, fetchImpl: params.fetchImpl, + ssrfPolicy: DISCORD_MEDIA_SSRF_POLICY, }); const saved = await saveMediaBuffer( fetched.buffer, diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts index b6ad169feb2..c521360fde7 100644 --- a/src/slack/monitor/media.test.ts +++ b/src/slack/monitor/media.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as ssrf from "../../infra/net/ssrf.js"; +import * as mediaFetch from "../../media/fetch.js"; import type { SavedMedia } from "../../media/store.js"; import * as mediaStore from "../../media/store.js"; import { mockPinnedHostnameResolution } from "../../test-helpers/ssrf.js"; @@ -472,6 +473,80 @@ describe("resolveSlackMedia", () => { }); }); +describe("Slack media SSRF policy", () => { + const originalFetchLocal = globalThis.fetch; + + beforeEach(() => { + mockFetch = vi.fn(); + globalThis.fetch = withFetchPreconnect(mockFetch); + mockPinnedHostnameResolution(); + }); + + afterEach(() => { + globalThis.fetch = originalFetchLocal; + vi.restoreAllMocks(); + }); + + it("passes ssrfPolicy with Slack CDN allowedHostnames and allowRfc2544BenchmarkRange to file downloads", async () => { + vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue( + createSavedMedia("/tmp/test.jpg", "image/jpeg"), + ); + mockFetch.mockResolvedValueOnce( + new Response(Buffer.from("img"), { status: 200, headers: { "content-type": "image/jpeg" } }), + ); + + const spy = vi.spyOn(mediaFetch, "fetchRemoteMedia"); + + await resolveSlackMedia({ + files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }], + token: "xoxb-test-token", + maxBytes: 1024, + }); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), + }), + ); + + const policy = spy.mock.calls[0][0].ssrfPolicy; + expect(policy?.allowedHostnames).toEqual( + expect.arrayContaining(["*.slack.com", "*.slack-edge.com", "*.slack-files.com"]), + ); + }); + + it("passes ssrfPolicy to forwarded attachment image downloads", async () => { + vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue( + createSavedMedia("/tmp/fwd.jpg", "image/jpeg"), + ); + vi.spyOn(ssrf, "resolvePinnedHostnameWithPolicy").mockImplementation(async (hostname) => { + const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); + return { + hostname: normalized, + addresses: ["93.184.216.34"], + lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses: ["93.184.216.34"] }), + }; + }); + mockFetch.mockResolvedValueOnce( + new Response(Buffer.from("fwd"), { status: 200, headers: { "content-type": "image/jpeg" } }), + ); + + const spy = vi.spyOn(mediaFetch, "fetchRemoteMedia"); + + await resolveSlackAttachmentContent({ + attachments: [{ is_share: true, image_url: "https://files.slack.com/forwarded.jpg" }], + token: "xoxb-test-token", + maxBytes: 1024, + }); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), + }), + ); + }); +}); + describe("resolveSlackAttachmentContent", () => { beforeEach(() => { mockFetch = vi.fn(); diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index a71523b43ad..fca883966a8 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -108,6 +108,11 @@ export async function fetchWithSlackAuth(url: string, token: string): Promise params.maxBytes) { return null; @@ -297,6 +303,7 @@ export async function resolveSlackAttachmentContent(params: { url: imageUrl, fetchImpl, maxBytes: params.maxBytes, + ssrfPolicy: SLACK_MEDIA_SSRF_POLICY, }); if (fetched.buffer.byteLength <= params.maxBytes) { const saved = await saveMediaBuffer(