Telegram file handling errors and unsupported types

Fixes on record

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

• #53912 fix(telegram): surface failed media downloads to user and agent, not a silent empty turn

  merged to main

  files: plugins/platforms/telegram/adapter.py, tests/gateway/test_telegram_documents.py

  Show the diff

  • plugins/platforms/telegram/adapter.py

    @@ -6101,6 +6101,47 @@ def _append_observed_note(existing: Optional[str], note: str) -> str:
                 return note
             return f"{existing}\n\n{note}"
     
    +    async def _surface_media_cache_failure(
    +        self,
    +        msg: Message,
    +        event: MessageEvent,
    +        kind: str,
    +        exc: Exception,
    +        display_name: Optional[str] = None,
    +    ) -> None:
    +        """Surface a failed media download/cache on BOTH ends instead of swallowing it.
    +
    +        When download_as_bytearray()/cache_*_from_bytes() raises (typically a
    +        transient httpx.ConnectError to Telegram's CDN), the attachment never
    +        made it into event.media_urls. Without this, the handler falls through
    +        and dispatches an empty turn: the user thinks the file was delivered,
    +        the agent sees nothing, and the only record is a buried log warning.
    +
    +        This (1) replies to the user in Telegram so they know to retry, and
    +        (2) appends an agent-visible notice to event.text via the existing
    +        observed-note channel so the agent knows an attachment was attempted
    +        and failed — never a silent empty turn. No new event fields (the
    +        structured-event refactor is out of scope per #23045).
    +        """
    +        named = f" ({display_name})" if display_name else ""
    +        try:
    +            await msg.reply_text(
    +                f"\u26a0\ufe0f Couldn't download your {kind}{named} "
    +                f"({exc.__class__.__name__}). Please try sending it again."
    +            )
    +        except Exception as reply_err:
    +            logger.warning(
    +                "[Telegram] Failed to notify user about %s cache failure: %s",
    +                kind,
    +                reply_err,
    +                exc_info=True,
    +            )
    +        agent_note = (
    +            f"[The user attempted to send a {kind}{named} but it could not be "
    +            f"downloaded ({exc.__class__.__name__}); they have been asked to retry.]"
    +        )
    +        event.text = self._append_observed_note(event.text, agent_note)
    +
         def _observe_unmentioned_group_message(
             self,
             message: Message,
    @@ -6562,6 +6603,7 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA
     
                 except Exception as e:
                     logger.warning("[Telegram] Failed to cache photo: %s", e, exc_info=True)
    +                await self._surface_media_cache_failure(msg, event, "photo", e)
     
             # Download voice/audio messages to cache for STT transcription
             if msg.voice:
    @@ -6580,6 +6622,7 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA
                     logger.info("[Telegram] Cached user voice at %s", cached_path)
                 except Exception as e:
                     logger.warning("[Telegram] Failed to cache voice: %s", e, exc_info=True)
    +                await self._surface_media_cache_failure(msg, event, "voice message", e)
             elif msg.audio:
                 try:
                     allowed, note = self._telegram_media_size_allowed(msg.audio, "audio file")
    @@ -6596,6 +6639,7 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA
                     logger.info("[Telegram] Cached user audio at %s", cached_path)
                 except Exception as e:
                     logger.warning("[Telegram] Failed to cache audio: %s", e, exc_info=True)
    +                await self._surface_media_cache_failure(msg, event, "audio file", e)
     
             elif msg.video:
                 try:
    @@ -6619,6 +6663,7 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA
                     logger.info("[Telegram] Cached user video at %s", cached_path)
                 except Exception as e:
                     logger.warning("[Telegram] Failed to cache video: %s", e, exc_info=True)
    +                await self._surface_media_cache_failure(msg, event, "video file", e)
     
             # Download document files to cache for agent processing
             elif msg.document:
    @@ -6751,6 +6796,10 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA
     
                 except Exception as e:
                     logger.warning("[Telegram] Failed to cache document: %s", e, exc_info=True)
    +                await self._surface_media_cache_failure(
    +                    msg, event, "attachment", e,
    +                    display_name=getattr(doc, "file_name", None) or None,
    +                )
     
             media_group_id = getattr(msg, "media_group_id", None)
             if media_group_id:

  • tests/gateway/test_telegram_documents.py

    @@ -106,6 +106,7 @@ def _make_message(document=None, caption=None, media_group_id=None, photo=None):
         msg.from_user.id = 1
         msg.from_user.full_name = "Test User"
         msg.message_thread_id = None
    +    msg.reply_text = AsyncMock()
         return msg
     
     
    @@ -397,7 +398,7 @@ async def test_text_injection_capped(self, adapter):
     
         @pytest.mark.asyncio
         async def test_download_exception_handled(self, adapter):
    -        """If get_file() raises, the handler logs the error without crashing."""
    +        """If get_file() raises, the handler surfaces the failure without crashing."""
             doc = _make_document(file_name="crash.pdf", file_size=100)
             doc.get_file = AsyncMock(side_effect=RuntimeError("Telegram API down"))
             msg = _make_message(document=doc)
    @@ -406,8 +407,73 @@ async def test_download_exception_handled(self, adapter):
             # Should not raise
             await adapter._handle_media_message(update, MagicMock())
             # handle_message should still be called (the handler catches the exception)
    +        # so the agent gets a turn — but now that turn carries a notice instead
    +        # of being an empty/content-less event.
             adapter.handle_message.assert_called_once()
     
    +    @pytest.mark.asyncio
    +    async def test_document_cache_failure_replies_and_signals_agent(self, adapter):
    +        """A failed document download must surface on BOTH ends, not silently.
    +
    +        Regression for #23045 Bug 2: a CDN download/cache failure used to log a
    +        warning and fall through to an empty agent turn — user thinks the file
    +        arrived, agent sees nothing. Now the user gets a Telegram reply AND the
    +        agent's event.text carries an attempted-attachment notice.
    +        """
    +        doc = _make_document(file_name="notes.md", mime_type="text/markdown", file_size=100)
    +        doc.get_file = AsyncMock(side_effect=RuntimeError("Telegram CDN down"))
    +        msg = _make_message(document=doc)
    +        update = _make_update(msg)
    +
    +        await adapter._handle_media_message(update, MagicMock())
    +
    +        # 1. User is told the download failed, with the filename + exception type.
    +        msg.reply_text.assert_awaited_once()
    +        reply = msg.reply_text.await_args.args[0]
    +        assert "Couldn't download" in reply
    +        assert "notes.md" in reply
    +        assert "RuntimeError" in reply
    +
    +        # 2. The agent still gets a turn, but event.text now carries a notice so
    +        #    it knows an attachment was attempted and failed (not a silent empty turn).
    +        adapter.handle_message.assert_called_once()
    +        event = adapter.handle_message.call_args[0][0]
    +        assert event.media_urls == []  # nothing cached
    +        assert "could not be downloaded" in (event.text or "")
    +        assert "notes.md" in (event.text or "")
    +
    +    @pytest.mark.asyncio
    +    async def test_document_cache_failure_reply_error_is_nonfatal(self, adapter):
    +        """If even the user-reply fails, the agent notice is still appended."""
    +        doc = _make_document(file_name="x.bin", mime_type="application/octet-stream", file_size=100)
    +        doc.get_file = AsyncMock(side_effect=RuntimeError("CDN down"))
    +        msg = _make_message(document=doc)
    +        msg.reply_text = AsyncMock(side_effect=RuntimeError("reply failed too"))
    +        update = _make_update(msg)
    +
    +        # Must not raise despite reply_text blowing up
    +        await adapter._handle_media_message(update, MagicMock())
    +        adapter.handle_message.assert_called_once()
    +        event = adapter.handle_message.call_args[0][0]
    +        assert "could not be downloaded" in (event.text or "")
    +
    +    @pytest.mark.asyncio
    +    async def test_voice_cache_failure_replies_and_signals_agent(self, adapter):
    +        """Same fail-closed contract applies to the voice site (#23045 Bug 2 class)."""
    +        msg = _make_message()
    +        msg.voice = MagicMock()
    +        msg.voice.file_size = 100
    +        msg.voice.get_file = AsyncMock(side_effect=RuntimeError("CDN down"))
    +        update = _make_update(msg)
    +
    +        await adapter._handle_media_message(update, MagicMock())
    +
    +        msg.reply_text.assert_awaited_once()
    +        assert "voice message" in msg.reply_text.await_args.args[0]
    +        adapter.handle_message.assert_called_once()
    +        event = adapter.handle_message.call_args[0][0]
    +        assert "could not be downloaded" in (event.text or "")
    +
     
     class TestVideoDownloadBlock:
         @pytest.mark.asyncio

• #65510 fix(gateway): deliver MEDIA: tags for every file type via validated egress

  merged to main

  files: gateway/platforms/base.py, tests/gateway/test_platform_base.py

  Show the diff

  • gateway/platforms/base.py

    @@ -1504,12 +1504,16 @@ def _log_safe_path(path: str) -> str:
         re.IGNORECASE,
     )
     
    -# Extension-less absolute paths (e.g. Caddyfile, Dockerfile, Makefile) are
    -# intentionally excluded from MEDIA_TAG_CLEANUP_RE — they are validated and
    -# delivered via MEDIA_EXTENSIONLESS_TAG_RE so prompt-injection paths that do
    -# not exist on disk are left visible instead of silently dropped. Paths with
    -# an unknown but present extension (e.g. .weirdext) stay on the #34517
    -# bare-path fallback and are not handled here.
    +# Paths NOT covered by MEDIA_TAG_CLEANUP_RE's extension alternation — both
    +# extension-less files (Caddyfile, Dockerfile, Makefile) and files with an
    +# unknown extension (.py, .log, .weirdext, ...) — are validated and delivered
    +# via MEDIA_EXTENSIONLESS_TAG_RE. Every ``MEDIA:`` path is therefore
    +# deliverable regardless of file type (#36060): known extensions extract
    +# unconditionally via the anchored pattern above, everything else extracts
    +# only after ``validate_media_delivery_path`` accepts it (exists on disk, not
    +# under the credential/system denylist, strict-mode rules honored), so
    +# prompt-injection paths that do not validate are left visible instead of
    +# silently dropped.
     MEDIA_EXTENSIONLESS_TAG_RE = re.compile(
         r'''[`"']?MEDIA:\s*'''
         r'''(?P<path>`[^`\n]+`|"[^"\n]+"|'[^'\n]+'|'''
    @@ -1527,8 +1531,16 @@ def _normalize_media_tag_path(raw: str) -> str:
     
     
     def _path_lacks_deliverable_extension(path: str) -> bool:
    -    """True only when the basename has no extension (Caddyfile, Makefile, …)."""
    -    return not Path(path).suffix
    +    """True when MEDIA_TAG_CLEANUP_RE's extension alternation does not cover
    +    ``path`` — either the basename has no extension at all (Caddyfile,
    +    Makefile, …) or the extension is not in MEDIA_DELIVERY_EXTS (.py, .log,
    +    .weirdext, …). Such paths route through the validated delivery pass
    +    (``validate_media_delivery_path``) instead of the unconditional one, so
    +    every file type is deliverable (#36060) while nonexistent / denylisted
    +    paths stay visible in the text.
    +    """
    +    suffix = Path(path).suffix.lower()
    +    return not suffix or suffix not in MEDIA_DELIVERY_EXTS
     
     
     def _strip_media_tag_directives(text: str) -> str:

  • tests/gateway/test_platform_base.py

    @@ -631,12 +631,15 @@ def test_extract_media_and_local_files_share_one_extension_set(self):
     
         def test_unknown_extension_not_black_holed_by_cleanup(self):
             """A MEDIA: tag with an unknown extension is NOT stripped from the
    -        body — it survives so extract_local_files can still see the bare path,
    -        rather than vanishing entirely (the core of issue #34517)."""
    +        body by the extension-anchored cleanup — and when the path does not
    +        validate (nonexistent file here), it is not delivered either, so the
    +        tag survives visibly instead of vanishing (the core of issue #34517).
    +        Validated unknown-extension paths DO deliver — see
    +        TestUniversalMediaEgress (#36060)."""
             from gateway.platforms.base import MEDIA_TAG_CLEANUP_RE
             text = "Saved to MEDIA:/tmp/data.weirdext done"
             media, _ = BasePlatformAdapter.extract_media(text)
    -        assert media == []  # unknown extension is not a deliverable MEDIA tag
    +        assert media == []  # nonexistent path fails validation, not delivered
             stripped = MEDIA_TAG_CLEANUP_RE.sub("", text)
             assert "/tmp/data.weirdext" in stripped  # path preserved, not dropped
     
    @@ -712,6 +715,104 @@ def test_as_document_directive_stripped_without_media_tag(self):
             )
     
     
    +class TestUniversalMediaEgress:
    +    """#36060: every MEDIA: path is deliverable regardless of file type.
    +
    +    Known extensions extract unconditionally (MEDIA_TAG_CLEANUP_RE); unknown
    +    extensions and extension-less files extract via the validated pass —
    +    delivered when validate_media_delivery_path accepts them, left visible
    +    when it does not (nonexistent, denylisted).
    +    """
    +
    +    def _patch_allow_root(self, monkeypatch, root):
    +        monkeypatch.setattr(
    +            "gateway.platforms.base.MEDIA_DELIVERY_SAFE_ROOTS",
    +            (str(root),),
    +        )
    +        monkeypatch.delenv("HERMES_MEDIA_DELIVERY_STRICT", raising=False)
    +
    +    @pytest.mark.parametrize("name", [
    +        "script.py", "server.log", "notes.weirdext", "app.ts", "run.sh",
    +        "config.toml", "styles.css", "contract.sol",
    +    ])
    +    def test_unknown_extension_delivered_when_file_validates(
    +        self, tmp_path, monkeypatch, name,
    +    ):
    +        root = tmp_path / "output"
    +        root.mkdir()
    +        f = root / name
    +        f.write_text("content", encoding="utf-8")
    +        self._patch_allow_root(monkeypatch, root)
    +
    +        content = f"Here you go:\nMEDIA:{f}\nDone."
    +        media, cleaned = BasePlatformAdapter.extract_media(content)
    +        assert len(media) == 1
    +        assert media[0][0] == str(f.resolve())
    +        assert "MEDIA:" not in cleaned
    +        assert "Done." in cleaned
    +
    +    def test_unknown_extension_left_visible_when_not_on_disk(
    +        self, tmp_path, monkeypatch,
    +    ):
    +        root = tmp_path / "output"
    +        root.mkdir()
    +        self._patch_allow_root(monkeypatch, root)
    +
    +        content = "MEDIA:/nonexistent/script.py"
    +        media, cleaned = BasePlatformAdapter.extract_media(content)
    +        assert media == []
    +        assert "MEDIA:/nonexistent/script.py" in cleaned
    +
    +    def test_denylisted_paths_still_rejected_regardless_of_extension(
    +        self, tmp_path, monkeypatch,
    +    ):
    +        # A denylisted path must not deliver even though .py/.log/etc now
    +        # route through the validated pass. _media_delivery_denied_paths()
    +        # reads _MEDIA_DELIVERY_DENIED_PREFIXES at call time, so patching the
    +        # tuple exercises the real denylist logic.
    +        secret_dir = [redacted:secret-assignment] / "secrets"
    +        secret_dir.mkdir()
    +        f = secret_dir / "creds.py"
    +        f.write_text("TOKEN = 'x'", encoding="utf-8")
    +        monkeypatch.setattr(
    +            "gateway.platforms.base._MEDIA_DELIVERY_DENIED_PREFIXES",
    +            (str(secret_dir),),
    +        )
    +        monkeypatch.delenv("HERMES_MEDIA_DELIVERY_STRICT", raising=False)
    +
    +        content = f"MEDIA:{f}"
    +        media, cleaned = BasePlatformAdapter.extract_media(content)
    +        assert media == []
    +        assert "MEDIA:" in cleaned  # rejected tag stays visible
    +
    +    def test_strip_for_display_strips_validated_unknown_extension(
    +        self, tmp_path, monkeypatch,
    +    ):
    +        root = tmp_path / "output"
    +        root.mkdir()
    +        f = root / "server.log"
    +        f.write_text("x", encoding="utf-8")
    +        self._patch_allow_root(monkeypatch, root)
    +
    +        text = f"MEDIA:{f}"
    +        stripped = BasePlatformAdapter.strip_media_directives_for_display(text)
    +        assert "MEDIA:" not in stripped
    +
    +    def test_strip_for_display_keeps_unvalidated_unknown_extension(self):
    +        text = "MEDIA:/nonexistent/server.log"
    +        stripped = BasePlatformAdapter.strip_media_directives_for_display(text)
    +        assert "MEDIA:/nonexistent/server.log" in stripped
    +
    +    def test_known_extension_still_unconditional(self):
    +        # Known extensions keep the pre-#36060 behavior: extracted (and the
    +        # tag stripped) even when the file does not exist — downstream
    +        # delivery surfaces the failure.
    +        content = "MEDIA:/nonexistent/report.pdf"
    +        media, cleaned = BasePlatformAdapter.extract_media(content)
    +        assert media == [("/nonexistent/report.pdf", False)]
    +        assert "MEDIA:" not in cleaned
    +
    +
     class TestMediaDeliveryPathValidation:
         def _patch_roots(self, monkeypatch, *roots):
             monkeypatch.setattr(