admin/Moltbot
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(slack): reject HTML responses when downloading media (#4665)

Browse Source 
* fix(slack): reject HTML responses when downloading media

Slack sometimes returns HTML login pages instead of binary media when
authentication fails or URLs expire. This change detects HTML responses
by checking content-type header and buffer content, then skips to the
next available file URL.

* fix: format import order and add braces to continue statement

* chore: format Slack media tests

* chore: apply formatter to Slack media tests

* fix(slack): merge auth-header forwarding and html media guard

---------

Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
tumf
2026-03-02 02:20:25 +09:00
committed by GitHub
parent 6dbbc58a8d
commit e0571399ac
3 changed files with 73 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -84,6 +84,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Slack/Bot attachment-only messages: when `allowBots: true`, bot messages with empty `text` now include non-forwarded attachment `text`/`fallback` content so webhook alerts are not silently dropped. (#27616)
- Slack/Inbound media auth + HTML guard: keep Slack auth headers on forwarded shared attachment image downloads, and reject login/error HTML payloads (while allowing expected `.html` uploads) when resolving Slack media so auth failures do not silently pass as files. (#18642)
- Slack/Security ingress mismatch guard: drop slash-command and interaction payloads when app/team identifiers do not match the active Slack account context (including nested `team.id` interaction payloads), preventing cross-app or cross-workspace payload injection into system-event handling. (#29091) Thanks @Solvely-Colin.
- Cron/Failure alerts: add configurable repeated-failure alerting with per-job overrides and Web UI cron editor support (`inherit|disabled|custom` with threshold/cooldown/channel/target fields). (#24789) Thanks xbrak.
- Cron/Isolated model defaults: resolve isolated cron `subagents.model` (including object-form `primary`) through allowlist-aware model selection so isolated cron runs honor subagent model defaults unless explicitly overridden by job payload model. (#11474) Thanks @AnonO6.

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

@@ -245,6 +245,52 @@ describe("resolveSlackMedia", () => {
expect(mockFetch).not.toHaveBeenCalled();
});
it("rejects HTML auth pages for non-HTML files", async () => {
const saveMediaBufferMock = vi.spyOn(mediaStore, "saveMediaBuffer");
mockFetch.mockResolvedValueOnce(
new Response("<!DOCTYPE html><html><body>login</body></html>", {
status: 200,
headers: { "content-type": "text/html; charset=utf-8" },
}),
);
const result = await resolveSlackMedia({
files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).toBeNull();
expect(saveMediaBufferMock).not.toHaveBeenCalled();
});
it("allows expected HTML uploads", async () => {
vi.spyOn(mediaStore, "saveMediaBuffer").mockResolvedValue(
createSavedMedia("/tmp/page.html", "text/html"),
);
mockFetch.mockResolvedValueOnce(
new Response("<!doctype html><html><body>ok</body></html>", {
status: 200,
headers: { "content-type": "text/html" },
}),
);
const result = await resolveSlackMedia({
files: [
{
url_private: "https://files.slack.com/page.html",
name: "page.html",
mimetype: "text/html",
},
],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).not.toBeNull();
expect(result?.[0]?.path).toBe("/tmp/page.html");
});
it("overrides video/* MIME to audio/* for slack_audio voice messages", async () => {
// saveMediaBuffer re-detects MIME from buffer bytes, so it may return
// video/mp4 for MP4 containers. Verify resolveSlackMedia preserves
@@ -525,6 +571,11 @@ describe("resolveSlackAttachmentContent", () => {
},
],
});
const firstCall = mockFetch.mock.calls[0];
expect(firstCall?.[0]).toBe("https://files.slack.com/forwarded.jpg");
const firstInit = firstCall?.[1];
expect(firstInit?.redirect).toBe("manual");
expect(new Headers(firstInit?.headers).get("Authorization")).toBe("Bearer xoxb-test-token");
});
});

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

@@ -125,6 +125,11 @@ function resolveSlackMediaMimetype(
return mime;
}
function looksLikeHtmlBuffer(buffer: Buffer): boolean {
const head = buffer.subarray(0, 512).toString("utf-8").replace(/^\s+/, "").toLowerCase();
return head.startsWith("<!doctype html") || head.startsWith("<html");
}
export type SlackMediaResult = {
path: string;
contentType?: string;
@@ -217,6 +222,20 @@ export async function resolveSlackMedia(params: {
if (fetched.buffer.byteLength > params.maxBytes) {
return null;
}
// Guard against auth/login HTML pages returned instead of binary media.
// Allow user-provided HTML files through.
const fileMime = file.mimetype?.toLowerCase();
const fileName = file.name?.toLowerCase() ?? "";
const isExpectedHtml =
fileMime === "text/html" || fileName.endsWith(".html") || fileName.endsWith(".htm");
if (!isExpectedHtml) {
const detectedMime = fetched.contentType?.split(";")[0]?.trim().toLowerCase();
if (detectedMime === "text/html" || looksLikeHtmlBuffer(fetched.buffer)) {
return null;
}
}
const effectiveMime = resolveSlackMediaMimetype(file, fetched.contentType);
const saved = await saveMediaBuffer(
fetched.buffer,
@@ -273,8 +292,10 @@ export async function resolveSlackAttachmentContent(params: {
const imageUrl = resolveForwardedAttachmentImageUrl(att);
if (imageUrl) {
try {
const fetchImpl = createSlackMediaFetch(params.token);
const fetched = await fetchRemoteMedia({
url: imageUrl,
fetchImpl,
maxBytes: params.maxBytes,
});
if (fetched.buffer.byteLength <= params.maxBytes) {