fix(slack): preserve auth on same-origin media redirects (#62996) (thanks @vincentkoc)

- Verified: pnpm build\n- Verified: pnpm test extensions/slack/src/monitor/media.test.ts\n- Verified: pnpm exec oxlint extensions/slack/src/monitor/media.ts extensions/slack/src/monitor/media.test.ts\n- Verified: pnpm exec oxfmt --check extensions/slack/src/monitor/media.ts extensions/slack/src/monitor/media.test.ts CHANGELOG.md\n\nRepo-wide pnpm lint and pnpm test were not clean on current main outside this fix, and the first full-suite test attempt from the default core sparse profile was additionally contaminated by missing ui/packages/OpenClawKit paths until they were materialized.
This commit is contained in:
Vincent Koc
2026-04-08 08:11:27 +01:00
committed by GitHub
parent 8cbd60d203
commit 55cbcd829d
3 changed files with 129 additions and 42 deletions

View File

@@ -4,6 +4,10 @@ Docs: https://docs.openclaw.ai
## Unreleased
### Fixes
- Slack/media: preserve bearer auth across same-origin `files.slack.com` redirects while still stripping it on cross-origin Slack CDN hops, so `url_private_download` image attachments load again. (#62960) Thanks @vincentkoc.
## 2026.4.8
### Fixes

View File

@@ -22,6 +22,11 @@ const createSavedMedia = (filePath: string, contentType: string): SavedMedia =>
contentType,
});
function getRequestHeader(callIndex: number, headerName: string): string | null {
const init = mockFetch.mock.calls[callIndex]?.[1];
return new Headers(init?.headers).get(headerName);
}
describe("fetchWithSlackAuth", () => {
beforeEach(() => {
// Create a new mock for each test
@@ -65,7 +70,7 @@ describe("fetchWithSlackAuth", () => {
expect(mockFetch).not.toHaveBeenCalled();
});
it("follows redirects without Authorization header", async () => {
it("strips Authorization header on cross-origin redirects", async () => {
// First call: redirect response from Slack
const redirectResponse = new Response(null, {
status: 302,
@@ -99,8 +104,7 @@ describe("fetchWithSlackAuth", () => {
);
});
it("handles relative redirect URLs", async () => {
// Redirect with relative URL
it("preserves Authorization header on same-origin redirects", async () => {
const redirectResponse = new Response(null, {
status: 302,
headers: { location: "/files/redirect-target" },
@@ -115,8 +119,8 @@ describe("fetchWithSlackAuth", () => {
await fetchWithSlackAuth("https://files.slack.com/original.jpg", "xoxb-test-token");
// Second call should resolve the relative URL against the original
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://files.slack.com/files/redirect-target", {
headers: { Authorization: "Bearer xoxb-test-token" },
redirect: "follow",
});
});
@@ -212,6 +216,74 @@ describe("resolveSlackMedia", () => {
);
});
it("preserves Authorization on same-origin redirects for private downloads", async () => {
vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue(
createSavedMedia("/tmp/test.jpg", "image/jpeg"),
);
mockFetch
.mockResolvedValueOnce(
new Response(null, {
status: 302,
headers: { location: "/files/redirect-target" },
}),
)
.mockResolvedValueOnce(
new Response(Buffer.from("image data"), {
status: 200,
headers: { "content-type": "image/jpeg" },
}),
);
const result = await resolveSlackMedia({
files: [{ url_private_download: "https://files.slack.com/download.jpg", name: "test.jpg" }],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).not.toBeNull();
expect(mockFetch).toHaveBeenCalledTimes(2);
expect(mockFetch.mock.calls[0]?.[0]).toBe("https://files.slack.com/download.jpg");
expect(mockFetch.mock.calls[1]?.[0]).toBe("https://files.slack.com/files/redirect-target");
expect(getRequestHeader(0, "Authorization")).toBe("Bearer xoxb-test-token");
expect(getRequestHeader(1, "Authorization")).toBe("Bearer xoxb-test-token");
});
it("strips Authorization on cross-origin redirects for private downloads", async () => {
vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue(
createSavedMedia("/tmp/test.jpg", "image/jpeg"),
);
mockFetch
.mockResolvedValueOnce(
new Response(null, {
status: 302,
headers: { location: "https://downloads.slack-edge.com/presigned-url?sig=abc123" },
}),
)
.mockResolvedValueOnce(
new Response(Buffer.from("image data"), {
status: 200,
headers: { "content-type": "image/jpeg" },
}),
);
const result = await resolveSlackMedia({
files: [{ url_private_download: "https://files.slack.com/download.jpg", name: "test.jpg" }],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).not.toBeNull();
expect(mockFetch).toHaveBeenCalledTimes(2);
expect(mockFetch.mock.calls[0]?.[0]).toBe("https://files.slack.com/download.jpg");
expect(mockFetch.mock.calls[1]?.[0]).toBe(
"https://downloads.slack-edge.com/presigned-url?sig=abc123",
);
expect(getRequestHeader(0, "Authorization")).toBe("Bearer xoxb-test-token");
expect(getRequestHeader(1, "Authorization")).toBeNull();
});
it("returns null when download fails", async () => {
// Simulate a network error
mockFetch.mockRejectedValueOnce(new Error("Network error"));
@@ -481,9 +553,7 @@ describe("resolveSlackMedia", () => {
}) as typeof fetch;
const runtimeFetchSpy = vi
.spyOn(ssrf, "fetchWithRuntimeDispatcher")
.mockImplementation(async (_input: RequestInfo | URL, init?: RequestInit) => {
expect(init).toMatchObject({ redirect: "manual" });
expect(init && "dispatcher" in init).toBe(true);
.mockImplementation(async () => {
return new Response(Buffer.from("image data"), {
status: 200,
headers: { "content-type": "image/jpeg" },
@@ -498,6 +568,13 @@ describe("resolveSlackMedia", () => {
expect(result).not.toBeNull();
expect(runtimeFetchSpy).toHaveBeenCalled();
expect(runtimeFetchSpy.mock.calls[0]?.[1]).toMatchObject({ redirect: "manual" });
expect(
runtimeFetchSpy.mock.calls[0]?.[1] && "dispatcher" in runtimeFetchSpy.mock.calls[0][1],
).toBe(true);
expect(new Headers(runtimeFetchSpy.mock.calls[0]?.[1]?.headers).get("Authorization")).toBe(
"Bearer xoxb-test-token",
);
});
});

View File

@@ -43,6 +43,26 @@ function assertSlackFileUrl(rawUrl: string): URL {
return parsed;
}
function createSlackAuthHeaders(token: string): HeadersInit {
return { Authorization: `Bearer ${token}` };
}
function createSlackMediaRequest(
url: string,
token: string,
): {
url: string;
requestInit: RequestInit;
} {
const parsed = assertSlackFileUrl(url);
return {
url: parsed.href,
// Let the shared guarded-fetch redirect logic preserve auth on same-origin
// Slack hops and strip it once the redirect crosses origins.
requestInit: { headers: createSlackAuthHeaders(token) },
};
}
function isMockedFetch(fetchImpl: typeof fetch | undefined): boolean {
if (typeof fetchImpl !== "function") {
return false;
@@ -50,68 +70,53 @@ function isMockedFetch(fetchImpl: typeof fetch | undefined): boolean {
return typeof (fetchImpl as typeof fetch & { mock?: unknown }).mock === "object";
}
function createSlackMediaFetch(token: string): FetchLike {
let includeAuth = true;
function createSlackMediaFetch(): FetchLike {
return async (input, init) => {
const url = resolveRequestUrl(input);
if (!url) {
throw new Error("Unsupported fetch input: expected string, URL, or Request");
}
const { headers: initHeaders, redirect: _redirect, ...rest } = init ?? {};
const headers = new Headers(initHeaders);
const parsed = assertSlackFileUrl(url);
const fetchImpl =
"dispatcher" in (init ?? {}) && !isMockedFetch(globalThis.fetch)
? fetchWithRuntimeDispatcher
: globalThis.fetch;
if (includeAuth) {
includeAuth = false;
const parsed = assertSlackFileUrl(url);
headers.set("Authorization", `Bearer ${token}`);
return fetchImpl(parsed.href, { ...rest, headers, redirect: "manual" });
}
headers.delete("Authorization");
return fetchImpl(url, { ...rest, headers, redirect: "manual" });
return fetchImpl(parsed.href, { ...init, redirect: "manual" });
};
}
/**
* Fetches a URL with Authorization header, handling cross-origin redirects.
* Node.js fetch strips Authorization headers on cross-origin redirects for security.
* Slack's file URLs redirect to CDN domains with pre-signed URLs that don't need the
* Authorization header, so we handle the initial auth request manually.
* Fetches a URL with Authorization header while keeping same-origin redirects
* authenticated and dropping auth once the redirect crosses origins.
*/
export async function fetchWithSlackAuth(url: string, token: string): Promise<Response> {
const parsed = assertSlackFileUrl(url);
const authHeaders = createSlackAuthHeaders(token);
// Initial request with auth and manual redirect handling
const initialRes = await fetch(parsed.href, {
headers: { Authorization: `Bearer ${token}` },
headers: authHeaders,
redirect: "manual",
});
// If not a redirect, return the response directly
if (initialRes.status < 300 || initialRes.status >= 400) {
return initialRes;
}
// Handle redirect - the redirected URL should be pre-signed and not need auth
const redirectUrl = initialRes.headers.get("location");
if (!redirectUrl) {
return initialRes;
}
// Resolve relative URLs against the original
const resolvedUrl = new URL(redirectUrl, parsed.href);
// Only follow safe protocols (we do NOT include Authorization on redirects).
if (resolvedUrl.protocol !== "https:") {
return initialRes;
}
// Follow the redirect without the Authorization header
// (Slack's CDN URLs are pre-signed and don't need it)
if (resolvedUrl.origin === parsed.origin) {
return fetch(resolvedUrl.toString(), {
headers: authHeaders,
redirect: "follow",
});
}
return fetch(resolvedUrl.toString(), { redirect: "follow" });
}
@@ -223,13 +228,12 @@ export async function resolveSlackMedia(params: {
return null;
}
try {
// Note: fetchRemoteMedia calls fetchImpl(url) with the URL string today and
// 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 { url: slackUrl, requestInit } = createSlackMediaRequest(url, params.token);
const fetchImpl = createSlackMediaFetch();
const fetched = await fetchRemoteMedia({
url,
url: slackUrl,
fetchImpl,
requestInit,
filePathHint: file.name,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
@@ -307,10 +311,12 @@ export async function resolveSlackAttachmentContent(params: {
const imageUrl = resolveForwardedAttachmentImageUrl(att);
if (imageUrl) {
try {
const fetchImpl = createSlackMediaFetch(params.token);
const { url: slackUrl, requestInit } = createSlackMediaRequest(imageUrl, params.token);
const fetchImpl = createSlackMediaFetch();
const fetched = await fetchRemoteMedia({
url: imageUrl,
url: slackUrl,
fetchImpl,
requestInit,
maxBytes: params.maxBytes,
ssrfPolicy: SLACK_MEDIA_SSRF_POLICY,
});