Browse Source

Deliver inbound attachments to the agent instead of dropping them

A Chatto message carrying only an image or a PDF has an empty body, and
_dispatch_message_posted returned early on exactly that — so every file a
user sent Hermes disappeared without a trace.

Download each attachment from its (pre-signed) asset URL and push the bytes
through cache_media_bytes(), the same funnel every other platform adapter
uses, then hand the cache paths over as media_urls/media_types. The gateway
takes it from there: vision enrichment for images, text extraction for
documents.

The download is bounded by the gateway's inbound media cap, checked against
Content-Length first and re-checked per chunk so a missing or lying header
cannot smuggle an unbounded body past it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul-Dieter Klumpp 1 week ago
parent
commit
7ada6510de
2 changed files with 289 additions and 4 deletions
  1. 124 2
      adapter.py
  2. 165 2
      test_adapter.py

+ 124 - 2
adapter.py

@@ -44,6 +44,9 @@ from gateway.platforms.base import (
     MessageEvent,
     MessageType,
     ProcessingOutcome,
+    cache_media_bytes,
+    get_inbound_media_max_bytes,
+    validate_inbound_media_size,
 )
 from gateway.config import Platform, PlatformConfig
 
@@ -392,6 +395,108 @@ class ChattoAdapter(BasePlatformAdapter):
         logger.info("Chatto: rejecting message from unauthorized user '%s' (%s)", user.login, user.id)
         return False
 
+    # ------------------------------------------------------------------ #
+    # Inbound attachments
+    # ------------------------------------------------------------------ #
+
+    async def _download_attachment_bytes(self, url: str) -> bytes:
+        """Download an attachment, refusing to buffer more than the gateway cap.
+
+        The Content-Length header is checked first so an oversized asset is
+        rejected before a single chunk is read; the running total is re-checked
+        as chunks arrive, because a missing or lying header must not smuggle an
+        unbounded body past the cap.
+        """
+        import httpx
+
+        max_bytes = get_inbound_media_max_bytes()
+        chunks: List[bytes] = []
+        total = 0
+        async with httpx.AsyncClient(
+            timeout=ChattoConstants.HTTP_TIMEOUT, follow_redirects=True,
+        ) as http:
+            async with http.stream("GET", url) as resp:
+                resp.raise_for_status()
+                declared = resp.headers.get("content-length")
+                if declared:
+                    try:
+                        declared_size = int(declared)
+                    except ValueError:
+                        logger.debug("Chatto: ignoring invalid Content-Length %r", declared)
+                    else:
+                        validate_inbound_media_size(
+                            declared_size, media_type="attachment", max_bytes=max_bytes,
+                        )
+                async for chunk in resp.aiter_bytes():
+                    total += len(chunk)
+                    validate_inbound_media_size(
+                        total, media_type="attachment", max_bytes=max_bytes,
+                    )
+                    chunks.append(chunk)
+        return b"".join(chunks)
+
+    async def _cache_attachments(
+        self, room_id: str, attachments: List[Any],
+    ) -> Tuple[List[str], List[str], List[str]]:
+        """Download message attachments into the gateway media cache.
+
+        Returns ``(media_urls, media_types, media_kinds)`` — the paths are
+        agent-visible cache paths, exactly what ``cache_media_bytes`` yields for
+        every other platform.  A failing attachment is logged and skipped: the
+        message itself still reaches the agent.
+        """
+        media_urls: List[str] = []
+        media_types: List[str] = []
+        media_kinds: List[str] = []
+
+        for att in attachments or []:
+            asset_url = getattr(att, "asset_url", None)
+            url = getattr(asset_url, "url", "") if asset_url else ""
+            filename = getattr(att, "filename", "") or ""
+            content_type = getattr(att, "content_type", "") or ""
+            if not url:
+                # Videos are announced before transcoding finishes, so the
+                # signed URL can legitimately be missing on arrival.
+                logger.info(
+                    "Chatto: attachment '%s' has no asset URL yet, skipping", filename,
+                )
+                continue
+            try:
+                data = await self._download_attachment_bytes(url)
+                cached = cache_media_bytes(
+                    data, filename=filename, mime_type=content_type,
+                )
+            except Exception as e:
+                logger.warning(
+                    "Chatto: failed to cache attachment '%s' (%s): %s",
+                    filename, content_type, e,
+                )
+                continue
+            if cached is None:
+                logger.warning(
+                    "Chatto: attachment '%s' (%s) could not be cached, skipping",
+                    filename, content_type,
+                )
+                continue
+            media_urls.append(cached.path)
+            media_types.append(cached.media_type)
+            media_kinds.append(cached.kind)
+
+        return media_urls, media_types, media_kinds
+
+    @staticmethod
+    def _message_type_for_media_kinds(media_kinds: List[str]) -> MessageType:
+        """Pick the MessageType for a set of cached attachment kinds."""
+        if "document" in media_kinds:
+            return MessageType.DOCUMENT
+        if "image" in media_kinds:
+            return MessageType.PHOTO
+        if "video" in media_kinds:
+            return MessageType.VIDEO
+        if "audio" in media_kinds:
+            return MessageType.AUDIO
+        return MessageType.TEXT
+
     async def _dispatch_message_posted(self, payload: MessagePostedPayload) -> None:
         try:
             client = await self._require_client()
@@ -404,8 +509,11 @@ class ChattoAdapter(BasePlatformAdapter):
         message = await payload.fetch_message(client=client)
         if message is None or message.deleted_at:
             return
-        message_body = message.body
-        if not message_body:
+        message_body = message.body or ""
+        attachments = list(message.attachments or [])
+        # A message carrying only an image/PDF has an empty body — dropping it
+        # here is what made attachments sent to Hermes disappear silently.
+        if not message_body and not attachments:
             return
 
         if message.actor_id in self._user_cache:
@@ -476,11 +584,24 @@ class ChattoAdapter(BasePlatformAdapter):
 
         my_body = message_body.lstrip() if msg_type == MessageType.COMMAND else message_body
 
+        # Attachments — download and hand the local cache paths to the gateway,
+        # which runs vision enrichment / document extraction off media_urls.
+        media_urls, media_types, media_kinds = await self._cache_attachments(
+            payload.room_id, attachments,
+        )
+        if media_kinds and msg_type != MessageType.COMMAND:
+            # Same precedence as the Teams/Signal adapters: document-context
+            # injection gates strictly on DOCUMENT, image handling keys off the
+            # per-path image/* MIME regardless of message_type.
+            msg_type = self._message_type_for_media_kinds(media_kinds)
+
         message_event = MessageEvent(
             text=my_body,
             source=source,
             message_id=message.id,
             message_type=msg_type,
+            media_urls=media_urls,
+            media_types=media_types,
             timestamp=message.created_at or datetime.now(timezone.utc),
             raw_message=message,
             reply_to_message_id=message.id,
@@ -1200,6 +1321,7 @@ class ChattoAdapter(BasePlatformAdapter):
 
 
 
+
 # ---------------------------------------------------------------------------
 # Cron / out-of-process delivery
 # ---------------------------------------------------------------------------

+ 165 - 2
test_adapter.py

@@ -38,10 +38,24 @@ from adapter import (
     hermes_validate_config as validate_config,
     register,
 )
-from chattolib.types import Room, RoomKind
+from chattolib.realtime_types import ReactionPayload
+from chattolib.types import (
+    AssetUrl,
+    Message,
+    MessageAttachment,
+    Room,
+    RoomKind,
+    User,
+)
 from platform_config import ChattoConstants
 from gateway.config import PlatformConfig
-from gateway.platforms.base import SendResult, MessageEvent, MessageType
+from gateway.platforms.base import (
+    CachedMedia,
+    MessageEvent,
+    MessageType,
+    SendResult,
+    get_inbound_media_max_bytes,
+)
 
 _EMOJI_TO_SHORTCODE = ChattoConstants.EMOJI_TO_SHORTCODE
 _MAX_MESSAGE_LENGTH = ChattoConstants.MAX_MESSAGE_LENGTH
@@ -114,6 +128,47 @@ def _make_room(room_id, name, kind):
                 archived=False, group_id="", universal=kind != RoomKind.DM)
 
 
+def _make_user(user_id, login):
+    """Build a real chattolib User, as the member directory would return."""
+    return User(id=user_id, login=login, display_name=login.replace("_", " ").title())
+
+
+def _make_attachment(filename, content_type, url="https://cdn.example.com/a"):
+    """Build a real MessageAttachment carrying a (pre-signed) asset URL."""
+    return MessageAttachment(
+        id="asset-" + filename,
+        filename=filename,
+        content_type=content_type,
+        asset_url=AssetUrl(url=url),
+    )
+
+
+def _make_message(body="hi", attachments=None, message_id="msg-1", room_id="room-1"):
+    """Build a real chattolib Message, as fetch_message() would return."""
+    return Message(
+        id=message_id,
+        room_id=room_id,
+        created_at=None,
+        actor_id="user-1",
+        body=body,
+        attachments=list(attachments or []),
+    )
+
+
+def _make_posted_payload(room_id="room-1", message_event_id="msg-1"):
+    """A message_posted payload whose fetch_message() the caller stubs."""
+    payload = MagicMock()
+    payload.room_id = room_id
+    payload.message_event_id = message_event_id
+    payload.thread_root_event_id = None
+    return payload
+
+
+def _cached(path, media_type, kind):
+    """The CachedMedia that cache_media_bytes() would return for an attachment."""
+    return CachedMedia(path=path, media_type=media_type, kind=kind, display_name="f")
+
+
 def _make_adapter(**extra_overrides):
     """Create a ChattoAdapter with mocked config."""
     _clear_chatto_env()
@@ -339,6 +394,114 @@ class TestMessageEditing:
         adapter._chatto_client.delete_message.assert_called_once()
 
 
+# -- Inbound attachments --
+
+class TestInboundAttachments:
+    """Messages carrying files must reach the agent, body or not."""
+
+    @pytest_asyncio.fixture
+    def adapter(self):
+        adapter = _make_adapter()
+        adapter.chatto_config.allow_all_users.value = True
+        adapter.me = _make_user("bot-user-id", "hermes_bot")
+        adapter._room_kinds["room-1"] = RoomKind.DM
+        adapter._user_cache["user-1"] = _make_user("user-1", "alice")
+        adapter.handle_message = AsyncMock()
+        adapter._download_attachment_bytes = AsyncMock(return_value=b"\x89PNG\r\n\x1a\nrest")
+        return adapter
+
+    async def test_image_attachment_becomes_media_url(self, adapter):
+        payload = _make_posted_payload()
+        adapter._chatto_client.get_room = AsyncMock()
+        message = _make_message(
+            body="look at this",
+            attachments=[_make_attachment("shot.png", "image/png")],
+        )
+        payload.fetch_message = AsyncMock(return_value=message)
+
+        with patch("adapter.cache_media_bytes", return_value=_cached("/cache/shot.png", "image/png", "image")):
+            await adapter._dispatch_message_posted(payload)
+
+        event = adapter.handle_message.call_args.args[0]
+        assert event.media_urls == ["/cache/shot.png"]
+        assert event.media_types == ["image/png"]
+        assert event.message_type == MessageType.PHOTO
+
+    async def test_attachment_only_message_is_not_dropped(self, adapter):
+        """The empty-body early return is what silently ate file uploads."""
+        payload = _make_posted_payload()
+        message = _make_message(
+            body="", attachments=[_make_attachment("report.pdf", "application/pdf")],
+        )
+        payload.fetch_message = AsyncMock(return_value=message)
+
+        with patch("adapter.cache_media_bytes", return_value=_cached("/cache/report.pdf", "application/pdf", "document")):
+            await adapter._dispatch_message_posted(payload)
+
+        adapter.handle_message.assert_called_once()
+        event = adapter.handle_message.call_args.args[0]
+        assert event.message_type == MessageType.DOCUMENT
+        assert event.media_urls == ["/cache/report.pdf"]
+
+    async def test_empty_message_without_attachments_is_dropped(self, adapter):
+        payload = _make_posted_payload()
+        payload.fetch_message = AsyncMock(return_value=_make_message(body=""))
+        await adapter._dispatch_message_posted(payload)
+        adapter.handle_message.assert_not_called()
+
+    async def test_download_failure_still_delivers_the_text(self, adapter):
+        adapter._download_attachment_bytes = AsyncMock(side_effect=RuntimeError("404"))
+        payload = _make_posted_payload()
+        payload.fetch_message = AsyncMock(return_value=_make_message(
+            body="see attached", attachments=[_make_attachment("a.png", "image/png")],
+        ))
+
+        await adapter._dispatch_message_posted(payload)
+
+        event = adapter.handle_message.call_args.args[0]
+        assert event.text == "see attached"
+        assert event.media_urls == []
+        assert event.message_type == MessageType.TEXT
+
+    async def test_attachment_without_asset_url_is_skipped(self, adapter):
+        """Videos are announced before transcoding finishes."""
+        payload = _make_posted_payload()
+        att = _make_attachment("clip.mp4", "video/mp4")
+        att.asset_url = None
+        payload.fetch_message = AsyncMock(return_value=_make_message(
+            body="clip", attachments=[att],
+        ))
+
+        await adapter._dispatch_message_posted(payload)
+
+        event = adapter.handle_message.call_args.args[0]
+        assert event.media_urls == []
+        adapter._download_attachment_bytes.assert_not_called()
+
+    async def test_document_wins_over_image(self, adapter):
+        """Mixed batches classify as DOCUMENT — that gates context injection."""
+        assert adapter._message_type_for_media_kinds(["image", "document"]) is MessageType.DOCUMENT
+        assert adapter._message_type_for_media_kinds(["image"]) is MessageType.PHOTO
+        assert adapter._message_type_for_media_kinds(["video"]) is MessageType.VIDEO
+        assert adapter._message_type_for_media_kinds(["audio"]) is MessageType.AUDIO
+        assert adapter._message_type_for_media_kinds([]) is MessageType.TEXT
+
+    async def test_oversized_attachment_is_rejected(self, adapter):
+        """The gateway media cap must bound what a hostile upload can buffer."""
+        import httpx
+
+        big = get_inbound_media_max_bytes() + 1
+        transport = httpx.MockTransport(lambda request: httpx.Response(
+            200, headers={"content-length": str(big)}, content=b"x",
+        ))
+        real_adapter = _make_adapter()
+        real_client_cls = httpx.AsyncClient
+
+        with patch("httpx.AsyncClient", lambda **kw: real_client_cls(transport=transport)):
+            with pytest.raises(ValueError):
+                await real_adapter._download_attachment_bytes("https://chat.example.com/a.png")
+
+
 # NOTE: there are deliberately no tests for get_user(), set_presence() or
 # set_custom_status() on the adapter. Those are not adapter responsibilities —
 # callers use the chattolib client directly, which exposes them (client.get_user,