fix: guard remote media fetches with SSRF checks

This commit is contained in:
Peter Steinberger
2026-02-02 04:04:27 -08:00
parent d842b28a15
commit 81c68f582d
11 changed files with 422 additions and 241 deletions

View File

@@ -1,4 +1,5 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import * as ssrf from "../../infra/net/ssrf.js";
// Store original fetch
const originalFetch = globalThis.fetch;
@@ -171,11 +172,21 @@ describe("resolveSlackMedia", () => {
beforeEach(() => {
mockFetch = vi.fn();
globalThis.fetch = mockFetch as typeof fetch;
vi.spyOn(ssrf, "resolvePinnedHostname").mockImplementation(async (hostname) => {
const normalized = hostname.trim().toLowerCase().replace(/\.$/, "");
const addresses = ["93.184.216.34"];
return {
hostname: normalized,
addresses,
lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses }),
};
});
});
afterEach(() => {
globalThis.fetch = originalFetch;
vi.resetModules();
vi.restoreAllMocks();
});
it("prefers url_private_download over url_private", async () => {

View File

@@ -44,6 +44,38 @@ function assertSlackFileUrl(rawUrl: string): URL {
return parsed;
}
function resolveRequestUrl(input: RequestInfo | URL): string {
if (typeof input === "string") {
return input;
}
if (input instanceof URL) {
return input.toString();
}
if ("url" in input && typeof input.url === "string") {
return input.url;
}
return String(input);
}
function createSlackMediaFetch(token: string): FetchLike {
let includeAuth = true;
return async (input, init) => {
const url = resolveRequestUrl(input);
const { headers: initHeaders, redirect: _redirect, ...rest } = init ?? {};
const headers = new Headers(initHeaders);
if (includeAuth) {
includeAuth = false;
const parsed = assertSlackFileUrl(url);
headers.set("Authorization", `Bearer ${token}`);
return fetch(parsed.href, { ...rest, headers, redirect: "manual" });
}
headers.delete("Authorization");
return fetch(url, { ...rest, headers, redirect: "manual" });
};
}
/**
* Fetches a URL with Authorization header, handling cross-origin redirects.
* Node.js fetch strips Authorization headers on cross-origin redirects for security.
@@ -100,13 +132,9 @@ export async function resolveSlackMedia(params: {
}
try {
// Note: fetchRemoteMedia calls fetchImpl(url) with the URL string today and
// handles size limits internally. We ignore init options because
// fetchWithSlackAuth handles redirect/auth behavior specially.
const fetchImpl: FetchLike = (input) => {
const inputUrl =
typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
return fetchWithSlackAuth(inputUrl, params.token);
};
// handles size limits internally. Provide a fetcher that uses auth once, then lets
// the redirect chain continue without credentials.
const fetchImpl = createSlackMediaFetch(params.token);
const fetched = await fetchRemoteMedia({
url,
fetchImpl,