mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(slack): remove unused unsafe auth fetch helper (#99944)
* fix(slack): remove unused unsafe auth fetch helper Co-authored-by: liyuanbin <li.yuanbin1@xydigit.com> * fix(slack): remove unused unsafe auth fetch helper --------- Co-authored-by: liyuanbin <li.yuanbin1@xydigit.com>
This commit is contained in:
committed by
GitHub
parent
020f44aba8
commit
706443c79b
@@ -2,7 +2,6 @@
|
||||
import type { WebClient } from "@slack/web-api";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
fetchWithSlackAuth,
|
||||
resolveSlackAttachmentContent,
|
||||
resolveSlackMedia,
|
||||
resolveSlackThreadHistory,
|
||||
@@ -246,172 +245,6 @@ async function expectPrivateDownloadRedirect(params: {
|
||||
expect(getRequestHeader(1, "Authorization")).toBe(params.secondAuthorization);
|
||||
}
|
||||
|
||||
describe("fetchWithSlackAuth", () => {
|
||||
beforeEach(() => {
|
||||
// Create a new mock for each test
|
||||
mockFetch = vi.fn<FetchMock>(
|
||||
async (_input: RequestInfo | URL, _init?: RequestInit) => new Response(),
|
||||
);
|
||||
globalThis.fetch = withFetchPreconnect(mockFetch);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore original fetch
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
it("sends Authorization header on initial request with manual redirect", async () => {
|
||||
// Simulate direct 200 response (no redirect)
|
||||
const mockResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(mockResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(mockResponse);
|
||||
|
||||
// Verify fetch was called with correct params
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
expect(mockFetch).toHaveBeenCalledWith("https://files.slack.com/test.jpg", {
|
||||
headers: { Authorization: "Bearer xoxb-test-token" },
|
||||
redirect: "manual",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects non-Slack hosts to avoid leaking tokens", async () => {
|
||||
await expect(
|
||||
fetchWithSlackAuth("https://example.com/test.jpg", "xoxb-test-token"),
|
||||
).rejects.toThrow(/non-Slack host|non-Slack/i);
|
||||
|
||||
// Should fail fast without attempting a fetch.
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("strips Authorization header on cross-origin redirects", async () => {
|
||||
// First call: redirect response from Slack
|
||||
const redirectResponse = new Response("redirect body", {
|
||||
status: 302,
|
||||
headers: { location: "https://cdn.slack-edge.com/presigned-url?sig=abc123" },
|
||||
});
|
||||
const cancel = vi.spyOn(redirectResponse.body!, "cancel").mockResolvedValue(undefined);
|
||||
|
||||
// Second call: actual file content from CDN
|
||||
const fileResponse = new Response(Buffer.from("actual image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(fileResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
|
||||
// First call should have Authorization header and manual redirect
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(1, "https://files.slack.com/test.jpg", {
|
||||
headers: { Authorization: "Bearer xoxb-test-token" },
|
||||
redirect: "manual",
|
||||
});
|
||||
|
||||
// Second call should follow the redirect without Authorization
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"https://cdn.slack-edge.com/presigned-url?sig=abc123",
|
||||
{ redirect: "follow" },
|
||||
);
|
||||
expect(cancel).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("preserves Authorization header on same-origin redirects", async () => {
|
||||
const redirectResponse = new Response("redirect body", {
|
||||
status: 302,
|
||||
headers: { location: "/files/redirect-target" },
|
||||
});
|
||||
const cancel = vi.spyOn(redirectResponse.body!, "cancel").mockResolvedValue(undefined);
|
||||
|
||||
const fileResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
await fetchWithSlackAuth("https://files.slack.com/original.jpg", "xoxb-test-token");
|
||||
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://files.slack.com/files/redirect-target", {
|
||||
headers: { Authorization: "Bearer xoxb-test-token" },
|
||||
redirect: "follow",
|
||||
});
|
||||
expect(cancel).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("returns redirect response when no location header is provided", async () => {
|
||||
// Redirect without location header
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 302,
|
||||
// No location header
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
// Should return the redirect response directly
|
||||
expect(result).toBe(redirectResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns redirect response when location header is malformed", async () => {
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "http://[::1" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(redirectResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns 4xx/5xx responses directly without following", async () => {
|
||||
const errorResponse = new Response("Not Found", {
|
||||
status: 404,
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(errorResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(errorResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("handles 301 permanent redirects", async () => {
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 301,
|
||||
headers: { location: "https://cdn.slack.com/new-url" },
|
||||
});
|
||||
|
||||
const fileResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://cdn.slack.com/new-url", {
|
||||
redirect: "follow",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveSlackMedia", () => {
|
||||
beforeEach(() => {
|
||||
mockFetch = vi.fn();
|
||||
|
||||
@@ -100,58 +100,6 @@ function createSlackMediaFetch(): FetchLike {
|
||||
};
|
||||
}
|
||||
|
||||
function resolveSlackFetchForRuntime(): typeof fetch {
|
||||
return isMockedFetch(globalThis.fetch) ? globalThis.fetch : fetchWithRuntimeDispatcher;
|
||||
}
|
||||
|
||||
async function cancelUnreadResponseBody(response: Response): Promise<void> {
|
||||
if (!response.bodyUsed) {
|
||||
await response.body?.cancel().catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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);
|
||||
const fetchImpl = resolveSlackFetchForRuntime();
|
||||
|
||||
const initialRes = await fetchImpl(parsed.href, {
|
||||
headers: authHeaders,
|
||||
redirect: "manual",
|
||||
});
|
||||
|
||||
if (initialRes.status < 300 || initialRes.status >= 400) {
|
||||
return initialRes;
|
||||
}
|
||||
|
||||
const redirectUrl = initialRes.headers.get("location");
|
||||
if (!redirectUrl) {
|
||||
return initialRes;
|
||||
}
|
||||
|
||||
let resolvedUrl: URL;
|
||||
try {
|
||||
resolvedUrl = new URL(redirectUrl, parsed.href);
|
||||
} catch {
|
||||
return initialRes;
|
||||
}
|
||||
if (resolvedUrl.protocol !== "https:") {
|
||||
return initialRes;
|
||||
}
|
||||
await cancelUnreadResponseBody(initialRes);
|
||||
if (resolvedUrl.origin === parsed.origin) {
|
||||
return fetchImpl(resolvedUrl.toString(), {
|
||||
headers: authHeaders,
|
||||
redirect: "follow",
|
||||
});
|
||||
}
|
||||
return fetchImpl(resolvedUrl.toString(), { redirect: "follow" });
|
||||
}
|
||||
|
||||
const SLACK_MEDIA_SSRF_POLICY = {
|
||||
allowedHostnames: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"],
|
||||
hostnameAllowlist: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"],
|
||||
|
||||
Reference in New Issue
Block a user