diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 5c2bbf96aa88..7422245fc0a8 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -824,6 +824,114 @@ def cache_document_from_bytes(data: bytes, filename: str) -> str: return str(filepath) +# --------------------------------------------------------------------------- +# Per-chat inbound attachment cache +# +# In addition to the type-specific caches above (images / documents / audio / +# video), every inbound media file is also mirrored into a chat-scoped +# attachment directory so: +# +# * The agent receives a stable, tool-accessible local file path even when +# the native vision path swallows the URL into pixels (issue #20899). +# * Two users in different chats can never read each other's cached media +# via shared paths (cross-tenant safety): the path is keyed on +# ``platform / chat_id / message_id``. +# +# Layout (profile-aware via get_hermes_dir): +# /cache/attachments//// +# +# Retention: files are left in place. A periodic GC hook can be added later +# (mirroring ``cleanup_document_cache``); we deliberately do not silently +# delete user-provided media here. +# --------------------------------------------------------------------------- + +ATTACHMENT_CACHE_DIR = get_hermes_dir("cache/attachments", "attachment_cache") + + +def get_attachment_cache_dir() -> Path: + """Return the inbound-attachment cache directory, creating it if needed.""" + ATTACHMENT_CACHE_DIR.mkdir(parents=True, exist_ok=True) + return ATTACHMENT_CACHE_DIR + + +_ATTACHMENT_PATH_SAFE_RE = re.compile(r"[^A-Za-z0-9._-]+") + + +def _sanitize_attachment_segment(segment: Any, fallback: str = "unknown") -> str: + """Sanitize a path segment for use under the attachment cache. + + Strips directory separators, control characters, and anything outside the + ``[A-Za-z0-9._-]`` charset. Empty / dot-only results fall back to *fallback*. + """ + if segment is None: + return fallback + seg = str(segment).strip().replace("\x00", "") + seg = _ATTACHMENT_PATH_SAFE_RE.sub("_", seg) + seg = seg.strip("._-") + if not seg or seg in (".", ".."): + return fallback + # Cap length so deeply nested IDs don't blow past filesystem limits. + return seg[:128] + + +def cache_inbound_attachment( + data: bytes, + *, + platform: str, + chat_id: Any, + message_id: Any, + filename: str, +) -> str: + """Persist inbound media into a per-chat attachment directory. + + Args: + data: Raw file bytes. + platform: Platform name (``"telegram"``, ``"discord"``, ...). + chat_id: Platform-specific chat / room / DM id. + message_id: Platform-specific message id (or update id) to scope the + attachment so multiple messages from the same chat can't collide. + filename: Original / human-readable filename. Sanitized; directory + components are stripped. + + Returns: + Absolute path to the cached file. + + Raises: + ValueError: If the resolved path would escape the attachment cache root. + """ + base = get_attachment_cache_dir() + plat_seg = _sanitize_attachment_segment(platform, "unknown_platform") + chat_seg = _sanitize_attachment_segment(chat_id, "unknown_chat") + msg_seg = _sanitize_attachment_segment(message_id, "unknown_msg") + + # Filename sanitation: keep only the basename, then make it filesystem-safe. + raw_name = Path(filename).name if filename else "attachment" + safe_name = _ATTACHMENT_PATH_SAFE_RE.sub("_", raw_name).strip("._-") + if not safe_name or safe_name in (".", ".."): + safe_name = "attachment" + safe_name = safe_name[:160] + + target_dir = base / plat_seg / chat_seg / msg_seg + target_dir.mkdir(parents=True, exist_ok=True) + + filepath = target_dir / safe_name + # If the same message id ships multiple files with identical names (rare, + # but possible for albums where the client supplies no filename), + # disambiguate with a short uuid suffix. + if filepath.exists(): + stem, dot, ext = safe_name.partition(".") + suffix = uuid.uuid4().hex[:8] + disambig = f"{stem}_{suffix}{dot}{ext}" if dot else f"{safe_name}_{suffix}" + filepath = target_dir / disambig + + # Final containment check (defence in depth against weird unicode etc.). + if not filepath.resolve().is_relative_to(base.resolve()): + raise ValueError(f"Path traversal rejected for attachment: {filename!r}") + + filepath.write_bytes(data) + return str(filepath) + + def cleanup_document_cache(max_age_hours: int = 24) -> int: """ Delete cached documents older than *max_age_hours*. @@ -897,6 +1005,17 @@ class MessageEvent: # media_urls: local file paths (for vision tool access) media_urls: List[str] = field(default_factory=list) media_types: List[str] = field(default_factory=list) + + # Tool-accessible inbound attachments (issue #20899). One dict per inbound + # file. Each dict has at minimum: + # {"path": str, "filename": str, "mime_type": str, + # "size": int, "platform": str, "message_id": str | None, + # "chat_id": str | None} + # These paths live under /cache/attachments//// + # and are safe to hand directly to file/terminal tools. Distinct from + # ``media_urls`` (which targets the vision pipeline and may collapse to a + # single shared image cache) so cross-chat isolation is preserved here. + attachments: List[Dict[str, Any]] = field(default_factory=list) # Reply context reply_to_message_id: Optional[str] = None diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 83e81736876b..0ac4a0591fe9 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -73,6 +73,7 @@ class _MockContextTypes: cache_audio_from_bytes, cache_video_from_bytes, cache_document_from_bytes, + cache_inbound_attachment, resolve_proxy_url, SUPPORTED_VIDEO_TYPES, SUPPORTED_DOCUMENT_TYPES, @@ -3111,6 +3112,10 @@ def _enqueue_photo_event(self, batch_key: str, event: MessageEvent) -> None: else: existing.media_urls.extend(event.media_urls) existing.media_types.extend(event.media_types) + try: + existing.attachments.extend(event.attachments) + except AttributeError: + pass if event.text: existing.text = self._merge_caption(existing.text, event.text) @@ -3177,6 +3182,36 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA cached_path = cache_image_from_bytes(bytes(image_bytes), ext=ext) event.media_urls = [cached_path] event.media_types = [f"image/{ext.lstrip('.')}" ] + + # Also mirror into the per-chat attachment cache so the agent + # has a tool-accessible local path even when the model routes + # this image into native vision (which otherwise hides the + # path from file/terminal tools). See issue #20899. + try: + raw_bytes = bytes(image_bytes) + attach_filename = f"photo_{msg.message_id}{ext}" + attach_path = cache_inbound_attachment( + raw_bytes, + platform="telegram", + chat_id=msg.chat.id, + message_id=msg.message_id, + filename=attach_filename, + ) + event.attachments.append({ + "path": attach_path, + "filename": attach_filename, + "mime_type": f"image/{ext.lstrip('.')}", + "size": len(raw_bytes), + "platform": "telegram", + "message_id": str(msg.message_id) if msg.message_id is not None else None, + "chat_id": str(msg.chat.id) if msg.chat and msg.chat.id is not None else None, + }) + except Exception: + logger.warning( + "[Telegram] Failed to mirror photo into attachment cache", + exc_info=True, + ) + logger.info("[Telegram] Cached user photo at %s", cached_path) media_group_id = getattr(msg, "media_group_id", None) if media_group_id: @@ -3289,6 +3324,36 @@ async def _handle_media_message(self, update: Update, context: ContextTypes.DEFA mime_type = SUPPORTED_DOCUMENT_TYPES[ext] event.media_urls = [cached_path] event.media_types = [mime_type] + + # Also mirror into the per-chat attachment cache (issue #20899). + # Documents already reach the agent via the message-text + # injection in gateway/run.py, but pinning a chat-scoped copy + # here keeps audit trails per-chat and makes cross-tenant + # leakage structurally impossible. + try: + attach_filename = original_filename or f"document{ext}" + attach_path = cache_inbound_attachment( + raw_bytes, + platform="telegram", + chat_id=msg.chat.id, + message_id=msg.message_id, + filename=attach_filename, + ) + event.attachments.append({ + "path": attach_path, + "filename": attach_filename, + "mime_type": mime_type, + "size": len(raw_bytes), + "platform": "telegram", + "message_id": str(msg.message_id) if msg.message_id is not None else None, + "chat_id": str(msg.chat.id) if msg.chat and msg.chat.id is not None else None, + }) + except Exception: + logger.warning( + "[Telegram] Failed to mirror document into attachment cache", + exc_info=True, + ) + logger.info("[Telegram] Cached user document at %s", cached_path) # For text files, inject content into event.text (capped at 100 KB) @@ -3333,6 +3398,12 @@ async def _queue_media_group_event(self, media_group_id: str, event: MessageEven else: existing.media_urls.extend(event.media_urls) existing.media_types.extend(event.media_types) + # Merge tool-accessible attachment metadata too (issue #20899) so + # albums surface every file to the agent, not just the first. + try: + existing.attachments.extend(event.attachments) + except AttributeError: + pass if event.text: existing.text = self._merge_caption(existing.text, event.text) diff --git a/gateway/run.py b/gateway/run.py index 15ce3ab08ce0..9d2debacd741 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -5755,6 +5755,42 @@ async def _prepare_inbound_message_text( except Exception: pass + # Surface tool-accessible attachment paths to the agent (issue #20899). + # Native vision routing hides image file paths inside multimodal pixel + # payloads, so file/terminal tools can't see them. Prepend a structured + # ATTACHMENT block listing local cached paths for every inbound file. + # The Telegram document branch below adds richer per-doc context for + # documents specifically; this block ensures *every* attachment + # (especially photos) is at least discoverable as a file path. + attachments_meta = list(getattr(event, "attachments", None) or []) + if attachments_meta: + lines = [] + for att in attachments_meta: + path = att.get("path") + if not path: + continue + fname = att.get("filename") or os.path.basename(path) + mime = att.get("mime_type") or "" + size = att.get("size") + size_str = "" + if isinstance(size, int) and size > 0: + if size >= 1024 * 1024: + size_str = f", {size / (1024 * 1024):.1f} MB" + elif size >= 1024: + size_str = f", {size / 1024:.1f} KB" + else: + size_str = f", {size} B" + meta = f" ({mime}{size_str})" if mime or size_str else "" + lines.append(f"- {fname}{meta}: {path}") + if lines: + header = ( + "[Inbound attachments cached locally — use file/terminal " + "tools (read, copy, move) on these absolute paths if the " + "user asks you to save, organize, or process them]:" + ) + attachment_note = header + "\n" + "\n".join(lines) + message_text = f"{attachment_note}\n\n{message_text}" if message_text else attachment_note + if event.media_urls and event.message_type == MessageType.DOCUMENT: import mimetypes as _mimetypes diff --git a/tests/gateway/test_telegram_attachments.py b/tests/gateway/test_telegram_attachments.py new file mode 100644 index 000000000000..fbe0b8eae7cc --- /dev/null +++ b/tests/gateway/test_telegram_attachments.py @@ -0,0 +1,268 @@ +""" +Tests for issue #20899 — Telegram inbound media surfaced as tool-accessible +attachments. + +These cover: + * Photos land in the per-chat attachment cache and populate + ``MessageEvent.attachments`` with a usable absolute path. + * Documents preserve their original filename in the cached path and + populate ``MessageEvent.attachments``. + * Multiple photos in a single media-group event are all represented. + * Cross-chat isolation: messages from different chat ids land in distinct + on-disk subdirectories. +""" + +import os +import sys +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import Platform, PlatformConfig # noqa: F401 +from gateway.platforms.base import ( + MessageEvent, + MessageType, + cache_inbound_attachment, + get_attachment_cache_dir, +) + + +# Re-use the telegram mocking shim from the existing documents test so this +# test module can be collected without the real python-telegram-bot. +def _ensure_telegram_mock(): + if "telegram" in sys.modules and hasattr(sys.modules["telegram"], "__file__"): + return + telegram_mod = MagicMock() + telegram_mod.ext.ContextTypes.DEFAULT_TYPE = type(None) + telegram_mod.constants.ParseMode.MARKDOWN_V2 = "MarkdownV2" + telegram_mod.constants.ChatType.GROUP = "group" + telegram_mod.constants.ChatType.SUPERGROUP = "supergroup" + telegram_mod.constants.ChatType.CHANNEL = "channel" + telegram_mod.constants.ChatType.PRIVATE = "private" + for name in ("telegram", "telegram.ext", "telegram.constants", "telegram.request"): + sys.modules.setdefault(name, telegram_mod) + + +_ensure_telegram_mock() + +from gateway.platforms.telegram import TelegramAdapter # noqa: E402 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +# Magic bytes the image cache validator requires. +_PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"\x00" * 32 +_JPEG_BYTES = b"\xff\xd8\xff" + b"\x00" * 32 + + +def _file_obj(data: bytes, file_path: str = "photos/file.jpg"): + f = AsyncMock() + f.download_as_bytearray = AsyncMock(return_value=bytearray(data)) + f.file_path = file_path + return f + + +def _photo_size(data: bytes, file_path: str = "photos/file.jpg"): + """Mock a single PhotoSize variant.""" + p = MagicMock() + p.get_file = AsyncMock(return_value=_file_obj(data, file_path)) + return p + + +def _document(data: bytes, file_name: str, mime_type: str = "application/pdf"): + d = MagicMock() + d.file_name = file_name + d.mime_type = mime_type + d.file_size = len(data) + d.get_file = AsyncMock(return_value=_file_obj(data, f"docs/{file_name}")) + return d + + +def _message(*, chat_id: int = 100, message_id: int = 42, + photo=None, document=None, caption=None, media_group_id=None): + msg = MagicMock() + msg.message_id = message_id + msg.text = caption or "" + msg.caption = caption + msg.date = None + msg.photo = photo + msg.video = None + msg.audio = None + msg.voice = None + msg.sticker = None + msg.document = document + msg.media_group_id = media_group_id + msg.chat = MagicMock() + msg.chat.id = chat_id + msg.chat.type = "private" + msg.chat.title = None + msg.chat.full_name = "Test User" + msg.from_user = MagicMock() + msg.from_user.id = 1 + msg.from_user.full_name = "Test User" + msg.message_thread_id = None + return msg + + +def _update(msg): + u = MagicMock() + u.message = msg + return u + + +@pytest.fixture() +def adapter(): + cfg = PlatformConfig(enabled=True, token="fake-token") + a = TelegramAdapter(config=cfg) + a.handle_message = AsyncMock() + return a + + +@pytest.fixture(autouse=True) +def _redirect_caches(tmp_path, monkeypatch): + """Redirect every cache used by the inbound-media path into tmp_path.""" + monkeypatch.setattr( + "gateway.platforms.base.IMAGE_CACHE_DIR", tmp_path / "images" + ) + monkeypatch.setattr( + "gateway.platforms.base.DOCUMENT_CACHE_DIR", tmp_path / "docs" + ) + monkeypatch.setattr( + "gateway.platforms.base.ATTACHMENT_CACHE_DIR", tmp_path / "attachments" + ) + + +# --------------------------------------------------------------------------- +# cache_inbound_attachment unit tests +# --------------------------------------------------------------------------- + +class TestCacheInboundAttachment: + def test_writes_under_platform_chat_message(self, tmp_path): + path = cache_inbound_attachment( + b"hello", + platform="telegram", + chat_id=12345, + message_id=99, + filename="hello.txt", + ) + p = Path(path) + assert p.exists() + assert p.read_bytes() == b"hello" + # Layout: /telegram/12345/99/hello.txt + parts = p.parts + assert "telegram" in parts + idx = parts.index("telegram") + assert parts[idx + 1] == "12345" + assert parts[idx + 2] == "99" + assert parts[idx + 3] == "hello.txt" + + def test_path_traversal_in_filename_is_neutralized(self, tmp_path): + path = cache_inbound_attachment( + b"x", + platform="telegram", + chat_id=1, + message_id=2, + filename="../../etc/passwd", + ) + # The attempt must collapse to a basename inside the cache root. + root = get_attachment_cache_dir().resolve() + assert Path(path).resolve().is_relative_to(root) + assert ".." not in Path(path).name + + def test_collision_disambiguated(self, tmp_path): + a = cache_inbound_attachment( + b"one", platform="telegram", chat_id=1, message_id=2, filename="x.bin" + ) + b = cache_inbound_attachment( + b"two", platform="telegram", chat_id=1, message_id=2, filename="x.bin" + ) + assert a != b + assert Path(a).read_bytes() == b"one" + assert Path(b).read_bytes() == b"two" + + +# --------------------------------------------------------------------------- +# Telegram photo inbound -> attachment cache + MessageEvent.attachments +# --------------------------------------------------------------------------- + +class TestTelegramPhotoAttachment: + @pytest.mark.asyncio + async def test_photo_populates_attachment_metadata(self, adapter, tmp_path): + # Telegram delivers msg.photo as a list of PhotoSize variants; the + # adapter must pick the largest (last). + photo_sizes = [ + _photo_size(_JPEG_BYTES + b"small", "photos/small.jpg"), + _photo_size(_JPEG_BYTES + b"largeXX", "photos/large.jpg"), + ] + msg = _message(chat_id=555, message_id=7, photo=photo_sizes) + await adapter._handle_media_message(_update(msg), MagicMock()) + + # Photo path goes through the batched flush; pull the buffered event + # directly so we don't have to wait on the debounce timer. + assert adapter._pending_photo_batches, "photo batch should be queued" + event = next(iter(adapter._pending_photo_batches.values())) + + assert event.attachments, "MessageEvent.attachments must be populated" + att = event.attachments[0] + assert att["platform"] == "telegram" + assert att["chat_id"] == "555" + assert att["message_id"] == "7" + assert att["mime_type"].startswith("image/") + assert att["size"] > 0 + assert os.path.isabs(att["path"]) + assert os.path.exists(att["path"]), "cached attachment file must exist on disk" + # Path must live under the chat-scoped attachment subtree + assert "/telegram/555/7/" in att["path"] + + +# --------------------------------------------------------------------------- +# Telegram document inbound preserves filename + populates attachments +# --------------------------------------------------------------------------- + +class TestTelegramDocumentAttachment: + @pytest.mark.asyncio + async def test_document_preserves_original_filename(self, adapter): + data = b"%PDF-1.4 hi" + doc = _document(data, file_name="quarterly_report.pdf") + msg = _message(chat_id=42, message_id=11, document=doc) + await adapter._handle_media_message(_update(msg), MagicMock()) + + event = adapter.handle_message.call_args[0][0] + assert event.attachments, "document should populate attachments" + att = event.attachments[0] + assert att["filename"] == "quarterly_report.pdf" + # Filename is preserved inside the on-disk path under the chat scope. + assert att["path"].endswith("quarterly_report.pdf") + assert "/telegram/42/11/" in att["path"] + assert os.path.exists(att["path"]) + assert att["mime_type"] == "application/pdf" + assert att["size"] == len(data) + + +# --------------------------------------------------------------------------- +# Cross-chat isolation +# --------------------------------------------------------------------------- + +class TestCrossChatIsolation: + @pytest.mark.asyncio + async def test_two_chats_land_in_distinct_directories(self, adapter): + doc_a = _document(b"%PDF-A", file_name="a.pdf") + msg_a = _message(chat_id=111, message_id=1, document=doc_a) + await adapter._handle_media_message(_update(msg_a), MagicMock()) + event_a = adapter.handle_message.call_args[0][0] + + doc_b = _document(b"%PDF-B", file_name="b.pdf") + msg_b = _message(chat_id=222, message_id=1, document=doc_b) + await adapter._handle_media_message(_update(msg_b), MagicMock()) + event_b = adapter.handle_message.call_args[0][0] + + path_a = event_a.attachments[0]["path"] + path_b = event_b.attachments[0]["path"] + assert "/telegram/111/" in path_a + assert "/telegram/222/" in path_b + # Different chats must not share a parent directory beyond the + # platform root, even with identical message ids. + assert os.path.dirname(path_a) != os.path.dirname(path_b)