Merge b305640e69 into da71eaebd2
This commit is contained in:
tumf
GitHub
commit
87f50ab68c
@ -242,6 +242,161 @@ describe("resolveSlackMedia", () => {
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects HTML response (auth failure) and returns null", async () => {
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// Simulate Slack returning an HTML login page instead of the image
|
||||
const htmlContent = `<!DOCTYPE html>
|
||||
<html data-cdn="https://a.slack-edge.com/">
|
||||
<head><title>Slack</title></head>
|
||||
<body>
|
||||
<script>redirectURL: "/files-pri/T123-F456/download/image.png"</script>
|
||||
</body>
|
||||
</html>`;
|
||||
const htmlResponse = new Response(htmlContent, {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/html; charset=utf-8" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(htmlResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [{ url_private_download: "https://files.slack.com/test.png", name: "test.png" }],
|
||||
token: "invalid-or-expired-token",
|
||||
maxBytes: 10 * 1024 * 1024,
|
||||
});
|
||||
|
||||
// Should reject HTML and return null
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects HTML response detected by buffer content even if content-type header is missing", async () => {
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// HTML content but no content-type header (edge case)
|
||||
const htmlContent = `<!doctype html><html><body>Slack login page</body></html>`;
|
||||
const htmlResponse = new Response(htmlContent, {
|
||||
status: 200,
|
||||
// No content-type header
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(htmlResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 10 * 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("allows genuine HTML file when mimetype indicates text/html", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
saveMediaBuffer: vi.fn().mockResolvedValue({
|
||||
path: "/tmp/test.html",
|
||||
contentType: "text/html",
|
||||
}),
|
||||
}));
|
||||
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// A genuine HTML file shared by a user
|
||||
const htmlContent = `<!DOCTYPE html><html><body>User's HTML page</body></html>`;
|
||||
const htmlResponse = new Response(htmlContent, {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/html; charset=utf-8" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(htmlResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [
|
||||
{
|
||||
url_private: "https://files.slack.com/page.html",
|
||||
name: "page.html",
|
||||
mimetype: "text/html",
|
||||
},
|
||||
],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 10 * 1024 * 1024,
|
||||
});
|
||||
|
||||
// Should allow genuine HTML files
|
||||
expect(result).not.toBeNull();
|
||||
expect(result?.contentType).toBe("text/html");
|
||||
});
|
||||
|
||||
it("allows HTML file based on .html extension even without mimetype", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
saveMediaBuffer: vi.fn().mockResolvedValue({
|
||||
path: "/tmp/test.html",
|
||||
contentType: "text/html",
|
||||
}),
|
||||
}));
|
||||
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
const htmlContent = `<!DOCTYPE html><html><body>User's HTML snippet</body></html>`;
|
||||
const htmlResponse = new Response(htmlContent, {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/html" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(htmlResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [
|
||||
{
|
||||
url_private: "https://files.slack.com/snippet.html",
|
||||
name: "snippet.html",
|
||||
// No mimetype set
|
||||
},
|
||||
],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 10 * 1024 * 1024,
|
||||
});
|
||||
|
||||
// Should allow based on .html extension
|
||||
expect(result).not.toBeNull();
|
||||
});
|
||||
|
||||
it("falls through to next file when first returns HTML", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
saveMediaBuffer: vi.fn().mockResolvedValue({
|
||||
path: "/tmp/test.jpg",
|
||||
contentType: "image/jpeg",
|
||||
}),
|
||||
}));
|
||||
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// First file: HTML (auth failure)
|
||||
const htmlResponse = new Response("<!DOCTYPE html><html>login</html>", {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/html" },
|
||||
});
|
||||
// Second file: actual image
|
||||
const imageResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(htmlResponse).mockResolvedValueOnce(imageResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [
|
||||
{ url_private: "https://files.slack.com/first.png", name: "first.png" },
|
||||
{ url_private: "https://files.slack.com/second.jpg", name: "second.jpg" },
|
||||
],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 10 * 1024 * 1024,
|
||||
});
|
||||
|
||||
// Should skip HTML and succeed with the second file
|
||||
expect(result).not.toBeNull();
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("falls through to next file when first file returns error", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
|
||||
@ -3,8 +3,23 @@ import type { WebClient as SlackWebClient } from "@slack/web-api";
|
||||
import type { FetchLike } from "../../media/fetch.js";
|
||||
import { fetchRemoteMedia } from "../../media/fetch.js";
|
||||
import { saveMediaBuffer } from "../../media/store.js";
|
||||
import { logWarn } from "../../logger.js";
|
||||
import type { SlackFile } from "../types.js";
|
||||
|
||||
/**
|
||||
* Detects if buffer content looks like HTML (login page / error page).
|
||||
* Slack sometimes returns HTML login pages when auth fails instead of binary media.
|
||||
*/
|
||||
function looksLikeHtml(buffer: Buffer): boolean {
|
||||
const head = buffer.subarray(0, 512).toString("utf-8").trim().toLowerCase();
|
||||
return (
|
||||
head.startsWith("<!doctype html") ||
|
||||
head.startsWith("<html") ||
|
||||
head.includes("slack-edge.com") ||
|
||||
head.includes("redirecturl:")
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches a URL with Authorization header, handling cross-origin redirects.
|
||||
* Node.js fetch strips Authorization headers on cross-origin redirects for security.
|
||||
@ -64,6 +79,21 @@ export async function resolveSlackMedia(params: {
|
||||
filePathHint: file.name,
|
||||
});
|
||||
if (fetched.buffer.byteLength > params.maxBytes) continue;
|
||||
|
||||
// Guard: reject if we received HTML instead of expected media.
|
||||
// This happens when Slack auth fails and returns a login page.
|
||||
// Skip this check if the file metadata indicates it's genuinely an HTML file.
|
||||
const detectedMime = fetched.contentType?.split(";")[0]?.trim().toLowerCase();
|
||||
const expectedMime = file.mimetype?.split(";")[0]?.trim().toLowerCase();
|
||||
const isExpectedHtml =
|
||||
expectedMime === "text/html" || file.name?.toLowerCase().endsWith(".html");
|
||||
if (!isExpectedHtml && (detectedMime === "text/html" || looksLikeHtml(fetched.buffer))) {
|
||||
const fileId = file.name ?? file.id ?? "unknown";
|
||||
logWarn(
|
||||
`slack: received HTML instead of media for file ${fileId}; possible auth failure or expired URL`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const saved = await saveMediaBuffer(
|
||||
fetched.buffer,
|
||||
fetched.contentType ?? file.mimetype,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user