Skip to content

Commit 706443c

Browse files
steipetemiorbnli
steipete
and
miorbnli
authored
fix(slack): remove unused unsafe auth fetch helper (#99944)
* fix(slack): remove unused unsafe auth fetch helper Co-authored-by: liyuanbin <[email protected]> * fix(slack): remove unused unsafe auth fetch helper --------- Co-authored-by: liyuanbin <[email protected]>
1 parent 020f44a commit 706443c

2 files changed

Lines changed: 0 additions & 219 deletions

File tree

‎extensions/slack/src/monitor/media.test.ts‎

Lines changed: 0 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import type { WebClient } from "@slack/web-api";
33
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44
import {
5-
fetchWithSlackAuth,
65
resolveSlackAttachmentContent,
76
resolveSlackMedia,
87
resolveSlackThreadHistory,
@@ -246,172 +245,6 @@ async function expectPrivateDownloadRedirect(params: {
246245
expect(getRequestHeader(1, "Authorization")).toBe(params.secondAuthorization);
247246
}
248247

249-
describe("fetchWithSlackAuth", () => {
250-
beforeEach(() => {
251-
// Create a new mock for each test
252-
mockFetch = vi.fn<FetchMock>(
253-
async (_input: RequestInfo | URL, _init?: RequestInit) => new Response(),
254-
);
255-
globalThis.fetch = withFetchPreconnect(mockFetch);
256-
});
257-
258-
afterEach(() => {
259-
// Restore original fetch
260-
globalThis.fetch = originalFetch;
261-
});
262-
263-
it("sends Authorization header on initial request with manual redirect", async () => {
264-
// Simulate direct 200 response (no redirect)
265-
const mockResponse = new Response(Buffer.from("image data"), {
266-
status: 200,
267-
headers: { "content-type": "image/jpeg" },
268-
});
269-
mockFetch.mockResolvedValueOnce(mockResponse);
270-
271-
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
272-
273-
expect(result).toBe(mockResponse);
274-
275-
// Verify fetch was called with correct params
276-
expect(mockFetch).toHaveBeenCalledTimes(1);
277-
expect(mockFetch).toHaveBeenCalledWith("https://files.slack.com/test.jpg", {
278-
headers: { Authorization: "Bearer xoxb-test-token" },
279-
redirect: "manual",
280-
});
281-
});
282-
283-
it("rejects non-Slack hosts to avoid leaking tokens", async () => {
284-
await expect(
285-
fetchWithSlackAuth("https://example.com/test.jpg", "xoxb-test-token"),
286-
).rejects.toThrow(/non-Slack host|non-Slack/i);
287-
288-
// Should fail fast without attempting a fetch.
289-
expect(mockFetch).not.toHaveBeenCalled();
290-
});
291-
292-
it("strips Authorization header on cross-origin redirects", async () => {
293-
// First call: redirect response from Slack
294-
const redirectResponse = new Response("redirect body", {
295-
status: 302,
296-
headers: { location: "https://cdn.slack-edge.com/presigned-url?sig=abc123" },
297-
});
298-
const cancel = vi.spyOn(redirectResponse.body!, "cancel").mockResolvedValue(undefined);
299-
300-
// Second call: actual file content from CDN
301-
const fileResponse = new Response(Buffer.from("actual image data"), {
302-
status: 200,
303-
headers: { "content-type": "image/jpeg" },
304-
});
305-
306-
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
307-
308-
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
309-
310-
expect(result).toBe(fileResponse);
311-
expect(mockFetch).toHaveBeenCalledTimes(2);
312-
313-
// First call should have Authorization header and manual redirect
314-
expect(mockFetch).toHaveBeenNthCalledWith(1, "https://files.slack.com/test.jpg", {
315-
headers: { Authorization: "Bearer xoxb-test-token" },
316-
redirect: "manual",
317-
});
318-
319-
// Second call should follow the redirect without Authorization
320-
expect(mockFetch).toHaveBeenNthCalledWith(
321-
2,
322-
"https://cdn.slack-edge.com/presigned-url?sig=abc123",
323-
{ redirect: "follow" },
324-
);
325-
expect(cancel).toHaveBeenCalledOnce();
326-
});
327-
328-
it("preserves Authorization header on same-origin redirects", async () => {
329-
const redirectResponse = new Response("redirect body", {
330-
status: 302,
331-
headers: { location: "/files/redirect-target" },
332-
});
333-
const cancel = vi.spyOn(redirectResponse.body!, "cancel").mockResolvedValue(undefined);
334-
335-
const fileResponse = new Response(Buffer.from("image data"), {
336-
status: 200,
337-
headers: { "content-type": "image/jpeg" },
338-
});
339-
340-
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
341-
342-
await fetchWithSlackAuth("https://files.slack.com/original.jpg", "xoxb-test-token");
343-
344-
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://files.slack.com/files/redirect-target", {
345-
headers: { Authorization: "Bearer xoxb-test-token" },
346-
redirect: "follow",
347-
});
348-
expect(cancel).toHaveBeenCalledOnce();
349-
});
350-
351-
it("returns redirect response when no location header is provided", async () => {
352-
// Redirect without location header
353-
const redirectResponse = new Response(null, {
354-
status: 302,
355-
// No location header
356-
});
357-
358-
mockFetch.mockResolvedValueOnce(redirectResponse);
359-
360-
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
361-
362-
// Should return the redirect response directly
363-
expect(result).toBe(redirectResponse);
364-
expect(mockFetch).toHaveBeenCalledTimes(1);
365-
});
366-
367-
it("returns redirect response when location header is malformed", async () => {
368-
const redirectResponse = new Response(null, {
369-
status: 302,
370-
headers: { location: "http://[::1" },
371-
});
372-
373-
mockFetch.mockResolvedValueOnce(redirectResponse);
374-
375-
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
376-
377-
expect(result).toBe(redirectResponse);
378-
expect(mockFetch).toHaveBeenCalledTimes(1);
379-
});
380-
381-
it("returns 4xx/5xx responses directly without following", async () => {
382-
const errorResponse = new Response("Not Found", {
383-
status: 404,
384-
});
385-
386-
mockFetch.mockResolvedValueOnce(errorResponse);
387-
388-
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
389-
390-
expect(result).toBe(errorResponse);
391-
expect(mockFetch).toHaveBeenCalledTimes(1);
392-
});
393-
394-
it("handles 301 permanent redirects", async () => {
395-
const redirectResponse = new Response(null, {
396-
status: 301,
397-
headers: { location: "https://cdn.slack.com/new-url" },
398-
});
399-
400-
const fileResponse = new Response(Buffer.from("image data"), {
401-
status: 200,
402-
});
403-
404-
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
405-
406-
await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
407-
408-
expect(mockFetch).toHaveBeenCalledTimes(2);
409-
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://cdn.slack.com/new-url", {
410-
redirect: "follow",
411-
});
412-
});
413-
});
414-
415248
describe("resolveSlackMedia", () => {
416249
beforeEach(() => {
417250
mockFetch = vi.fn();

‎extensions/slack/src/monitor/media.ts‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -100,58 +100,6 @@ function createSlackMediaFetch(): FetchLike {
100100
};
101101
}
102102

103-
function resolveSlackFetchForRuntime(): typeof fetch {
104-
return isMockedFetch(globalThis.fetch) ? globalThis.fetch : fetchWithRuntimeDispatcher;
105-
}
106-
107-
async function cancelUnreadResponseBody(response: Response): Promise<void> {
108-
if (!response.bodyUsed) {
109-
await response.body?.cancel().catch(() => undefined);
110-
}
111-
}
112-
113-
/**
114-
* Fetches a URL with Authorization header while keeping same-origin redirects
115-
* authenticated and dropping auth once the redirect crosses origins.
116-
*/
117-
export async function fetchWithSlackAuth(url: string, token: string): Promise<Response> {
118-
const parsed = assertSlackFileUrl(url);
119-
const authHeaders = createSlackAuthHeaders(token);
120-
const fetchImpl = resolveSlackFetchForRuntime();
121-
122-
const initialRes = await fetchImpl(parsed.href, {
123-
headers: authHeaders,
124-
redirect: "manual",
125-
});
126-
127-
if (initialRes.status < 300 || initialRes.status >= 400) {
128-
return initialRes;
129-
}
130-
131-
const redirectUrl = initialRes.headers.get("location");
132-
if (!redirectUrl) {
133-
return initialRes;
134-
}
135-
136-
let resolvedUrl: URL;
137-
try {
138-
resolvedUrl = new URL(redirectUrl, parsed.href);
139-
} catch {
140-
return initialRes;
141-
}
142-
if (resolvedUrl.protocol !== "https:") {
143-
return initialRes;
144-
}
145-
await cancelUnreadResponseBody(initialRes);
146-
if (resolvedUrl.origin === parsed.origin) {
147-
return fetchImpl(resolvedUrl.toString(), {
148-
headers: authHeaders,
149-
redirect: "follow",
150-
});
151-
}
152-
return fetchImpl(resolvedUrl.toString(), { redirect: "follow" });
153-
}
154-
155103
const SLACK_MEDIA_SSRF_POLICY = {
156104
allowedHostnames: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"],
157105
hostnameAllowlist: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"],

0 commit comments

Comments
 (0)