Slack image cache receiving HTML instead of images

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #7125 fix(gateway): validate image downloads before caching (cross-platform)

  merged to main

  files: gateway/platforms/base.py, gateway/platforms/email.py, gateway/platforms/slack.py, gateway/platforms/wecom.py, tests/gateway/test_media_download_retry.py

  Show the diff

  • gateway/platforms/base.py

    @@ -216,6 +216,23 @@ def get_image_cache_dir() -> Path:
         return IMAGE_CACHE_DIR
     
     
    +def _looks_like_image(data: bytes) -> bool:
    +    """Return True if *data* starts with a known image magic-byte sequence."""
    +    if len(data) < 4:
    +        return False
    +    if data[:8] == b"\x89PNG\r\n\x1a\n":
    +        return True
    +    if data[:3] == b"\xff\xd8\xff":
    +        return True
    +    if data[:6] in (b"GIF87a", b"GIF89a"):
    +        return True
    +    if data[:2] == b"BM":
    +        return True
    +    if data[:4] == b"RIFF" and len(data) >= 12 and data[8:12] == b"WEBP":
    +        return True
    +    return False
    +
    +
     def cache_image_from_bytes(data: bytes, ext: str = ".jpg") -> str:
         """
         Save raw image bytes to the cache and return the absolute file path.
    @@ -226,7 +243,17 @@ def cache_image_from_bytes(data: bytes, ext: str = ".jpg") -> str:
     
         Returns:
             Absolute path to the cached image file as a string.
    +
    +    Raises:
    +        ValueError: If *data* does not look like a valid image (e.g. an HTML
    +            error page returned by the upstream server).
         """
    +    if not _looks_like_image(data):
    +        snippet = data[:80].decode("utf-8", errors="replace")
    +        raise ValueError(
    +            f"Refusing to cache non-image data as {ext} "
    +            f"(starts with: {snippet!r})"
    +        )
         cache_dir = get_image_cache_dir()
         filename = f"img_{uuid.uuid4().hex[:12]}{ext}"
         filepath = cache_dir / filename

  • gateway/platforms/email.py

    @@ -195,7 +195,11 @@ def _extract_attachments(
     
             ext = Path(filename).suffix.lower()
             if ext in _IMAGE_EXTS:
    -            cached_path = cache_image_from_bytes(payload, ext)
    +            try:
    +                cached_path = cache_image_from_bytes(payload, ext)
    +            except ValueError:
    +                logger.debug("Skipping non-image attachment %s (invalid magic bytes)", filename)
    +                continue
                 attachments.append({
                     "path": cached_path,
                     "filename": filename,

  • gateway/platforms/slack.py

    @@ -1596,6 +1596,18 @@ async def _download_slack_file(self, url: str, ext: str, audio: bool = False, te
                         )
                         response.raise_for_status()
     
    +                    # Slack may return an HTML sign-in/redirect page
    +                    # instead of actual media bytes (e.g. expired token,
    +                    # restricted file access).  Detect this early so we
    +                    # don't cache bogus data and confuse downstream tools.
    +                    ct = response.headers.get("content-type", "")
    +                    if "text/html" in ct:
    +                        raise ValueError(
    +                            "Slack returned HTML instead of media "
    +                            f"(content-type: {ct}); "
    +                            "check bot token scopes and file permissions"
    +                        )
    +
                         if audio:
                             from gateway.platforms.base import cache_audio_from_bytes
                             return cache_audio_from_bytes(response.content, ext)

  • gateway/platforms/wecom.py

    @@ -696,7 +696,11 @@ async def _cache_media(self, kind: str, media: Dict[str, Any]) -> Optional[Tuple
     
                 if kind == "image":
                     ext = self._detect_image_ext(raw)
    -                return cache_image_from_bytes(raw, ext), self._mime_for_ext(ext, fallback="image/jpeg")
    +                try:
    +                    return cache_image_from_bytes(raw, ext), self._mime_for_ext(ext, fallback="image/jpeg")
    +                except ValueError as exc:
    +                    logger.warning("[%s] Rejected non-image bytes: %s", self.name, exc)
    +                    return None
     
                 filename = str(media.get("filename") or media.get("name") or "wecom_file")
                 return cache_document_from_bytes(raw, filename), mimetypes.guess_type(filename)[0] or "application/octet-stream"
    @@ -722,7 +726,11 @@ async def _cache_media(self, kind: str, media: Dict[str, Any]) -> Optional[Tuple
             content_type = str(headers.get("content-type") or "").split(";", 1)[0].strip() or "application/octet-stream"
             if kind == "image":
                 ext = self._guess_extension(url, content_type, fallback=self._detect_image_ext(raw))
    -            return cache_image_from_bytes(raw, ext), content_type or self._mime_for_ext(ext, fallback="image/jpeg")
    +            try:
    +                return cache_image_from_bytes(raw, ext), content_type or self._mime_for_ext(ext, fallback="image/jpeg")
    +            except ValueError as exc:
    +                logger.warning("[%s] Rejected non-image bytes from %s: %s", self.name, url, exc)
    +                return None
     
             filename = self._guess_filename(url, headers.get("content-disposition"), content_type)
             return cache_document_from_bytes(raw, filename), content_type

  • tests/gateway/test_media_download_retry.py

    @@ -34,6 +34,45 @@ def _make_timeout_error() -> httpx.TimeoutException:
         return httpx.TimeoutException("timed out")
     
     
    +# ---------------------------------------------------------------------------
    +# cache_image_from_bytes (base.py)
    +# ---------------------------------------------------------------------------
    +
    +
    +class TestCacheImageFromBytes:
    +    """Tests for gateway.platforms.base.cache_image_from_bytes"""
    +
    +    def test_caches_valid_jpeg(self, tmp_path, monkeypatch):
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        from gateway.platforms.base import cache_image_from_bytes
    +        path = cache_image_from_bytes(b"\xff\xd8\xff fake jpeg data", ".jpg")
    +        assert path.endswith(".jpg")
    +
    +    def test_caches_valid_png(self, tmp_path, monkeypatch):
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        from gateway.platforms.base import cache_image_from_bytes
    +        path = cache_image_from_bytes(b"\x89PNG\r\n\x1a\n fake png data", ".png")
    +        assert path.endswith(".png")
    +
    +    def test_rejects_html_content(self, tmp_path, monkeypatch):
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        from gateway.platforms.base import cache_image_from_bytes
    +        with pytest.raises(ValueError, match="non-image data"):
    +            cache_image_from_bytes(b"<!DOCTYPE html><html><title>Slack</title></html>", ".png")
    +
    +    def test_rejects_empty_data(self, tmp_path, monkeypatch):
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        from gateway.platforms.base import cache_image_from_bytes
    +        with pytest.raises(ValueError, match="non-image data"):
    +            cache_image_from_bytes(b"", ".jpg")
    +
    +    def test_rejects_plain_text(self, tmp_path, monkeypatch):
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        from gateway.platforms.base import cache_image_from_bytes
    +        with pytest.raises(ValueError, match="non-image data"):
    +            cache_image_from_bytes(b"just some text, not an image", ".jpg")
    +
    +
     # ---------------------------------------------------------------------------
     # cache_image_from_url (base.py)
     # ---------------------------------------------------------------------------
    @@ -71,7 +110,7 @@ def test_retries_on_timeout_then_succeeds(self, _mock_safe, tmp_path, monkeypatc
             monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
     
             fake_response = MagicMock()
    -        fake_response.content = b"image data"
    +        fake_response.content = b"\xff\xd8\xff image data"
             fake_response.raise_for_status = MagicMock()
     
             mock_client = AsyncMock()
    @@ -101,7 +140,7 @@ def test_retries_on_429_then_succeeds(self, _mock_safe, tmp_path, monkeypatch):
             monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
     
             ok_response = MagicMock()
    -        ok_response.content = b"image data"
    +        ok_response.content = b"\xff\xd8\xff image data"
             ok_response.raise_for_status = MagicMock()
     
             mock_client = AsyncMock()
    @@ -395,8 +434,9 @@ def test_success_on_first_attempt(self, tmp_path, monkeypatch):
             adapter = _make_slack_adapter()
     
             fake_response = MagicMock()
    -        fake_response.content = b"fake image bytes"
    +        fake_response.content = b"\x89PNG\r\n\x1a\n fake png"
             fake_response.raise_for_status = MagicMock()
    +        fake_response.headers = {"content-type": "image/png"}
     
             mock_client = AsyncMock()
             mock_client.get = AsyncMock(return_value=fake_response)
    @@ -413,14 +453,44 @@ async def run():
             assert path.endswith(".jpg")
             mock_client.get.assert_called_once()
     
    +    def test_rejects_html_response(self, tmp_path, monkeypatch):
    +        """An HTML sign-in page from Slack is rejected, not cached as image."""
    +        monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
    +        adapter = _make_slack_adapter()
    +
    +        fake_response = MagicMock()
    +        fake_response.content = b"<!DOCTYPE html><html><title>Slack</title></html>"
    +        fake_response.raise_for_status = MagicMock()
    +        fake_response.headers = {"content-type": "text/html; charset=utf-8"}
    +
    +        mock_client = AsyncMock()
    +        mock_client.get = AsyncMock(return_value=fake_response)
    +        mock_client.__aenter__ = AsyncMock(return_value=mock_client)
    +        mock_client.__aexit__ = AsyncMock(return_value=False)
    +
    +        async def run():
    +            with patch("httpx.AsyncClient", return_value=mock_client):
    +                await adapter._download_slack_file(
    +                    "https://files.slack.com/img.jpg", ext=".jpg"
    +                )
    +
    +        with pytest.raises(ValueError, match="HTML instead of media"):
    +            asyncio.run(run())
    +
    +        # Verify nothing was cached
    +        img_dir = tmp_path / "img"
    +        if img_dir.exists():
    +            assert list(img_dir.iterdir()) == []
    +
         def test_retries_on_timeout_then_succeeds(self, tmp_path, monkeypatch):
             """Timeout on first attempt triggers retry; success on second."""
             monkeypatch.setattr("gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "img")
             adapter = _make_slack_adapter()
     
             fake_response = MagicMock()
    -        fake_response.content = b"image bytes"
    +        fake_response.content = b"\x89PNG\r\n\x1a\n image bytes"
             fake_response.raise_for_status = MagicMock()
    +        fake_response.headers = {"content-type": "image/png"}
     
             mock_client = AsyncMock()
             mock_client.get = AsyncMock(

• #16206 fix(slack): surface attachment access diagnostics

  merged to main

  files: gateway/platforms/slack.py, tests/gateway/test_media_download_retry.py, tests/gateway/test_slack.py, website/docs/user-guide/messaging/slack.md

  Show the diff

  • gateway/platforms/slack.py

    @@ -15,7 +15,7 @@
     import re
     import time
     from dataclasses import dataclass, field
    -from typing import Dict, Optional, Any, Tuple
    +from typing import Dict, Optional, Any, Tuple, List
     
     try:
         from slack_bolt.async_app import AsyncApp
    @@ -121,6 +121,63 @@ def __init__(self, config: PlatformConfig):
             # clear them (chat_id → thread_ts).
             self._active_status_threads: Dict[str, str] = {}
     
    +    def _describe_slack_api_error(self, response: Any, *, file_obj: Optional[Dict[str, Any]] = None) -> Optional[str]:
    +        """Convert Slack API auth/permission failures into actionable user-facing text."""
    +        if response is None or not hasattr(response, "get"):
    +            return None
    +
    +        error = str(response.get("error", "") or "").strip()
    +        if not error:
    +            return None
    +
    +        file_label = str((file_obj or {}).get("name") or (file_obj or {}).get("id") or "this attachment")
    +        needed = str(response.get("needed", "") or "").strip()
    +        provided = str(response.get("provided", "") or "").strip()
    +        reinstall_hint = " Update the Slack app scopes/settings and reinstall the app to the workspace."
    +        provided_hint = f" Current bot scopes: {provided}." if provided else ""
    +
    +        if error == "missing_scope":
    +            needed_hint = f"Missing scope: {needed}." if needed else "Missing required Slack scope."
    +            return f"Slack attachment access failed for {file_label}. {needed_hint}{provided_hint}{reinstall_hint}"
    +        if error in {"not_authed", "invalid_auth", "account_inactive", "token_revoked"}:
    +            return f"Slack attachment access failed for {file_label} because the bot token is not authorized ({error}). Refresh the token/reinstall the app."
    +        if error in {"file_not_found", "file_deleted"}:
    +            return f"Slack attachment {file_label} is no longer available ({error})."
    +        if error in {"access_denied", "file_access_denied", "no_permission", "not_allowed_token_type", "restricted_action"}:
    +            return f"Slack attachment access failed for {file_label} because the bot does not have permission ({error}). Check workspace permissions/scopes and reinstall if needed."
    +        return None
    +
    +    def _describe_slack_download_failure(self, exc: Exception, *, file_obj: Optional[Dict[str, Any]] = None) -> Optional[str]:
    +        """Translate Slack download exceptions into user-facing attachment diagnostics."""
    +        file_label = str((file_obj or {}).get("name") or (file_obj or {}).get("id") or "this attachment")
    +
    +        response = getattr(exc, "response", None)
    +        api_detail = self._describe_slack_api_error(response, file_obj=file_obj)
    +        if api_detail:
    +            return api_detail
    +
    +        try:
    +            import httpx
    +        except Exception:  # pragma: no cover
    +            httpx = None
    +
    +        if httpx is not None and isinstance(exc, httpx.HTTPStatusError):
    +            status = exc.response.status_code
    +            if status == 401:
    +                return f"Slack attachment access failed for {file_label} with HTTP 401. The bot token is not authorized for this file."
    +            if status == 403:
    +                return f"Slack attachment access failed for {file_label} with HTTP 403. The bot likely lacks permission or scope to read this file."
    +            if status == 404:
    +                return f"Slack attachment {file_label} returned HTTP 404 and is no longer reachable."
    +
    +        message = str(exc)
    +        if "Slack returned HTML instead of media" in message or "non-image data" in message:
    +            return (
    +                f"Slack attachment access failed for {file_label}: Slack returned an HTML/login or non-media response. "
    +                "This usually means a scope, auth, or file-permission problem."
    +            )
    +        return None
    +
         async def connect(self) -> bool:
             """Connect to Slack via Socket Mode."""
             if not SLACK_AVAILABLE:
    @@ -1193,6 +1250,7 @@ async def _handle_slack_message(self, event: dict) -> None:
             # Handle file attachments
             media_urls = []
             media_types = []
    +        attachment_notices: List[str] = []
             files = event.get("files", [])
             for f in files:
                 # Slack Connect channels return stub file objects with
    @@ -1209,13 +1267,24 @@ async def _handle_slack_message(self, event: dict) -> None:
                         if info_resp.get("ok"):
                             f = info_resp["file"]
                         else:
    -                        logger.warning(
    -                            "[Slack] files.info failed for %s: %s",
    -                            file_id, info_resp.get("error"),
    -                        )
    +                        detail = self._describe_slack_api_error(info_resp, file_obj=f)
    +                        if detail:
    +                            attachment_notices.append(detail)
    +                            logger.warning("[Slack] %s", detail)
    +                        else:
    +                            logger.warning(
    +                                "[Slack] files.info failed for %s: %s",
    +                                file_id, info_resp.get("error"),
    +                            )
                             continue
                     except Exception as e:
    -                    logger.warning("[Slack] files.info error for %s: %s", file_id, e, exc_info=True)
    +                    response = getattr(e, "response", None)
    +                    detail = self._describe_slack_api_error(response, file_obj=f)
    +                    if detail:
    +                        attachment_notices.append(detail)
    +                        logger.warning("[Slack] %s", detail)
    +                    else:
    +                        logger.warning("[Slack] files.info error for %s: %s", file_id, e, exc_info=True)
                         continue
     
                 mimetype = f.get("mimetype", "unknown")
    @@ -1231,7 +1300,12 @@ async def _handle_slack_message(self, event: dict) -> None:
                         media_types.append(mimetype)
                         msg_type = MessageType.PHOTO
                     except Exception as e:  # pragma: no cover - defensive logging
    -                    logger.warning("[Slack] Failed to cache image from %s: %s", url, e, exc_info=True)
    +                    detail = self._describe_slack_download_failure(e, file_obj=f)
    +                    if detail:
    +                        attachment_notices.append(detail)
    +                        logger.warning("[Slack] %s", detail)
    +                    else:
    +                        logger.warning("[Slack] Failed to cache image from %s: %s", url, e, exc_info=True)
                 elif mimetype.startswith("audio/") and url:
                     try:
                         ext = "." + mimetype.split("/")[-1].split(";")[0]
    @@ -1242,7 +1316,12 @@ async def _handle_slack_message(self, event: dict) -> None:
                         media_types.append(mimetype)
                         msg_type = MessageType.VOICE
                     except Exception as e:  # pragma: no cover - defensive logging
    -                    logger.warning("[Slack] Failed to cache audio from %s: %s", url, e, exc_info=True)
    +                    detail = self._describe_slack_download_failure(e, file_obj=f)
    +                    if detail:
    +                        attachment_notices.append(detail)
    +                        logger.warning("[Slack] %s", detail)
    +                    else:
    +                        logger.warning("[Slack] Failed to cache audio from %s: %s", url, e, exc_info=True)
                 elif url:
                     # Try to handle as a document attachment
                     try:
    @@ -1294,7 +1373,16 @@ async def _handle_slack_message(self, event: dict) -> None:
                                 pass  # Binary content, skip injection
     
                     except Exception as e:  # pragma: no cover - defensive logging
    -                    logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True)
    +                    detail = self._describe_slack_download_failure(e, file_obj=f)
    +                    if detail:
    +                        attachment_notices.append(detail)
    +                        logger.warning("[Slack] %s", detail)
    +                    else:
    +                        logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True)
    +
    +        if attachment_notices:
    +            notice_block = "[Slack attachment notice]\n" + "\n".join(f"- {n}" for n in attachment_notices)
    +            text = f"{notice_block}\n\n{text}" if text else notice_block
     
             # Resolve user display name (cached after first lookup)
             user_name = await self._resolve_user_name(user_id, chat_id=channel_id)

  • tests/gateway/test_media_download_retry.py

    @@ -540,7 +540,7 @@ def _ensure_slack_mock():
     
     
     def _make_slack_adapter():
    -    config = PlatformConfig(enabled=True, token="[redacted:secret-assignment]")
    +    config = PlatformConfig(enabled=True, token="***")
         adapter = SlackAdapter(config)
         adapter._app = MagicMock()
         adapter._app.client = AsyncMock()
    @@ -549,6 +549,39 @@ def _make_slack_adapter():
         return adapter
     
     
    +# ---------------------------------------------------------------------------
    +# SlackAdapter diagnostics helpers
    +# ---------------------------------------------------------------------------
    +
    +class TestSlackAttachmentDiagnostics:
    +    def test_missing_scope_error_returns_actionable_notice(self):
    +        """_describe_slack_api_error translates a missing_scope response into
    +        a user-facing notice mentioning the needed scope and the reinstall
    +        step. This is the helper used by every files.info call site (Slack
    +        Connect stubs + post-download failures) to surface scope problems
    +        without making an extra probe call per attachment.
    +        """
    +        adapter = _make_slack_adapter()
    +
    +        response = {
    +            "error": "missing_scope",
    +            "needed": "files:read",
    +            "provided": "chat:write,files:write",
    +        }
    +        detail = adapter._describe_slack_api_error(response, file_obj={"id": "F123", "name": "photo.jpg"})
    +        assert detail is not None
    +        assert "files:read" in detail
    +        assert "reinstall" in detail.lower()
    +        assert "chat:write,files:write" in detail
    +
    +    def test_download_failure_403_returns_permission_notice(self):
    +        adapter = _make_slack_adapter()
    +        exc = _make_http_status_error(403)
    +        detail = adapter._describe_slack_download_failure(exc, file_obj={"name": "report.pdf"})
    +        assert "403" in detail
    +        assert "permission or scope" in detail
    +
    +
     # ---------------------------------------------------------------------------
     # SlackAdapter._download_slack_file
     # ---------------------------------------------------------------------------

  • tests/gateway/test_slack.py

    @@ -511,6 +511,35 @@ async def test_image_still_handled(self, adapter):
             msg_event = adapter.handle_message.call_args[0][0]
             assert msg_event.message_type == MessageType.PHOTO
     
    +    @pytest.mark.asyncio
    +    async def test_download_failure_is_surfaced_in_message_text(self, adapter):
    +        """Attachment download failures (401/403/HTML-body/etc.) should be
    +        translated into a user-facing `[Slack attachment notice]` block so
    +        the agent can tell the user what to fix (e.g. missing files:read
    +        scope). No proactive files.info probe is made — the diagnostic
    +        runs only when the download actually fails.
    +        """
    +        import httpx
    +        req = httpx.Request("GET", "https://files.slack.com/photo.jpg")
    +        resp = httpx.Response(403, request=req)
    +
    +        with patch.object(adapter, "_download_slack_file", new_callable=AsyncMock) as dl:
    +            dl.side_effect = httpx.HTTPStatusError("403", request=req, response=resp)
    +            event = self._make_event(text="what's in this?", files=[{
    +                "id": "F123",
    +                "mimetype": "image/jpeg",
    +                "name": "photo.jpg",
    +                "url_private_download": "https://files.slack.com/photo.jpg",
    +                "size": 1024,
    +            }])
    +            await adapter._handle_slack_message(event)
    +
    +        msg_event = adapter.handle_message.call_args[0][0]
    +        assert msg_event.message_type == MessageType.TEXT
    +        assert "[Slack attachment notice]" in msg_event.text
    +        assert "403" in msg_event.text
    +        assert "what's in this?" in msg_event.text
    +
     
     # ---------------------------------------------------------------------------
     # TestMessageRouting

  • website/docs/user-guide/messaging/slack.md

    @@ -82,7 +82,8 @@ Navigate to **Features → OAuth & Permissions** in the sidebar. Scroll to **Sco
     
     :::caution Missing scopes = missing features
     Without `channels:history` and `groups:history`, the bot **will not receive messages in channels** —
    -it will only work in DMs. These are the most commonly missed scopes.
    +it will only work in DMs. Without `files:read`, Hermes can chat but **cannot reliably read user-uploaded attachments**.
    +These are the most commonly missed scopes.
     :::
     
     **Optional scopes:**
    @@ -520,7 +521,8 @@ Keys are Slack channel IDs (find them via channel details → "About" → scroll
     | "Sending messages to this app has been turned off" in DMs | Enable the **Messages Tab** in App Home settings (see Step 5) |
     | "not_authed" or "invalid_auth" errors | Regenerate your Bot Token and App Token, update `.env` |
     | Bot responds but can't post in a channel | Invite the bot to the channel with `/invite @Hermes Agent` |
    -| "missing_scope" error | Add the required scope in OAuth & Permissions, then **reinstall** the app |
    +| Bot can chat but can't read uploaded images/files | Add `files:read`, then **reinstall** the app. Hermes now surfaces attachment access diagnostics in-chat when Slack returns scope/auth/permission failures. |
    +| `missing_scope` error | Add the required scope in OAuth & Permissions, then **reinstall** the app |
     | Socket disconnects frequently | Check your network; Bolt auto-reconnects but unstable connections cause lag |
     | Changed scopes/events but nothing changed | You **must reinstall** the app to your workspace after any scope or event subscription change |