romtuck/openclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: allow genuine HTML file downloads

Browse Source 
Address Codex review feedback: only reject HTML responses when the file
metadata indicates a non-HTML type. Genuine HTML files (with mimetype
text/html or .html extension) are now allowed through.

- Check file.mimetype and file.name extension before rejecting HTML
- Add tests for legitimate HTML file downloads
This commit is contained in:
Yoshihiro Takahara 2026-01-30 13:52:46 +00:00
parent da6f3e21a5
commit b305640e69
2 changed files with 75 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/slack/monitor/media.test.ts
View File

@ -289,6 +289,76 @@ describe("resolveSlackMedia", () => {
expect(result).toBeNull(); 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 () => { it("falls through to next file when first returns HTML", async () => {
// Mock the store module // Mock the store module
vi.doMock("../../media/store.js", () => ({ vi.doMock("../../media/store.js", () => ({

6
src/slack/monitor/media.ts
View File

@ -82,8 +82,12 @@ export async function resolveSlackMedia(params: {
// Guard: reject if we received HTML instead of expected media. // Guard: reject if we received HTML instead of expected media.
// This happens when Slack auth fails and returns a login page. // 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 detectedMime = fetched.contentType?.split(";")[0]?.trim().toLowerCase();
if (detectedMime === "text/html" || looksLikeHtml(fetched.buffer)) { 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"; const fileId = file.name ?? file.id ?? "unknown";
logWarn( logWarn(
`slack: received HTML instead of media for file ${fileId}; possible auth failure or expired URL`, `slack: received HTML instead of media for file ${fileId}; possible auth failure or expired URL`,