From cb9d18c759417f0bf0ff94fb932a423c74d706d5 Mon Sep 17 00:00:00 2001 From: UgwujaGeorge Date: Tue, 30 Jun 2026 03:09:56 -0700 Subject: [PATCH] fix(gateway): stop media-send fallbacks from leaking host paths into chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base BasePlatformAdapter implementations of send_voice, send_video, send_document, and send_image_file forwarded their *_path argument verbatim into the chat text (e.g. "🎬 Video: /home/.../hermes/cache/..."). Telegram, Discord, and Slack adapters all fall back to those base methods when their native send raises — so a rejected video on Telegram surfaced the host filesystem layout to the user instead of a useful message. Replace the path-echo with a friendly notice, log the path for operator diagnostics, and keep the user-supplied caption intact. The Slack adapter had three identical sites that fell through to the same path-echo on its own native upload failures; fix those too. send_document still surfaces the caller-provided file_name (or the basename derived from it) since that is the user-facing filename, not a host path. Add regression tests asserting the *_path argument never appears in the fallback content while caption text and explicit file_name still do. --- gateway/platforms/base.py | 57 ++++++++++--- plugins/platforms/slack/adapter.py | 12 ++- tests/gateway/test_platform_base.py | 123 ++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 14 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 01c015fb2e5..d34ad36a403 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -3148,12 +3148,20 @@ class BasePlatformAdapter(ABC): ) -> SendResult: """ Send an audio file as a native voice message via the platform API. - + Override in subclasses to send audio as voice bubbles (Telegram) - or file attachments (Discord). Default falls back to sending the - file path as text. + or file attachments (Discord). Default falls back to a friendly + notice — never echo the local audio_path into chat, since it is a + host filesystem path that would leak the Hermes home layout. """ - text = f"🔊 Audio: {audio_path}" + # audio_path is intentionally NOT included in the chat text — it is a + # host-local path that leaks filesystem layout. The path is logged for + # operator diagnostics instead. + logger.warning( + "[%s] send_voice fallback: native audio send unavailable for %s", + self.name, audio_path, + ) + text = "⚠️ Couldn't deliver the audio attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id=chat_id, content=text, reply_to=reply_to, metadata=metadata) @@ -3192,9 +3200,16 @@ class BasePlatformAdapter(ABC): Send a video natively via the platform API. Override in subclasses to send videos as inline playable media. - Default falls back to sending the file path as text. + Default falls back to a friendly notice — never echo the local + video_path into chat, since it is a host filesystem path that + would leak the Hermes home layout. """ - text = f"🎬 Video: {video_path}" + # See send_voice for the rationale: do not echo host paths into chat. + logger.warning( + "[%s] send_video fallback: native video send unavailable for %s", + self.name, video_path, + ) + text = "⚠️ Couldn't deliver the video attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id=chat_id, content=text, reply_to=reply_to, metadata=metadata) @@ -3213,9 +3228,22 @@ class BasePlatformAdapter(ABC): Send a document/file natively via the platform API. Override in subclasses to send files as downloadable attachments. - Default falls back to sending the file path as text. + Default falls back to a friendly notice — never echo the local + file_path into chat, since it is a host filesystem path that + would leak the Hermes home layout. """ - text = f"📎 File: {file_path}" + # See send_voice for the rationale: do not echo host paths into chat. + logger.warning( + "[%s] send_document fallback: native file send unavailable for %s", + self.name, file_path, + ) + # file_name is supplied by callers and represents the user-facing + # filename (already non-sensitive — it is what the agent named the + # output). Only show it when the caller passed one explicitly. + if file_name: + text = f"⚠️ Couldn't deliver the file attachment ({file_name})." + else: + text = "⚠️ Couldn't deliver the file attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id=chat_id, content=text, reply_to=reply_to, metadata=metadata) @@ -3233,10 +3261,17 @@ class BasePlatformAdapter(ABC): Send a local image file natively via the platform API. Unlike send_image() which takes a URL, this takes a local file path. - Override in subclasses for native photo attachments. - Default falls back to sending the file path as text. + Override in subclasses for native photo attachments. Default falls + back to a friendly notice — never echo the local image_path into + chat, since it is a host filesystem path that would leak the + Hermes home layout. """ - text = f"🖼️ Image: {image_path}" + # See send_voice for the rationale: do not echo host paths into chat. + logger.warning( + "[%s] send_image_file fallback: native image send unavailable for %s", + self.name, image_path, + ) + text = "⚠️ Couldn't deliver the image attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id=chat_id, content=text, reply_to=reply_to, metadata=metadata) diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 77bfe78fb58..95d1a7ad0d9 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -2012,7 +2012,8 @@ class SlackAdapter(BasePlatformAdapter): e, exc_info=True, ) - text = f"🖼️ Image: {image_path}" + # image_path is a host-local path; never echo it into chat. + text = "⚠️ Couldn't deliver the image attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata) @@ -2164,7 +2165,8 @@ class SlackAdapter(BasePlatformAdapter): e, exc_info=True, ) - text = f"🎬 Video: {video_path}" + # video_path is a host-local path; never echo it into chat. + text = "⚠️ Couldn't deliver the video attachment." if caption: text = f"{caption}\n{text}" return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata) @@ -2223,7 +2225,11 @@ class SlackAdapter(BasePlatformAdapter): e, exc_info=True, ) - text = f"📎 File: {file_path}" + # file_path is a host-local path; never echo it into chat. + # display_name comes from caller-supplied file_name (or basename + # of the host path) and is the user-facing filename only — safe + # to surface so the user knows which file failed. + text = f"⚠️ Couldn't deliver the file attachment ({display_name})." if caption: text = f"{caption}\n{text}" return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata) diff --git a/tests/gateway/test_platform_base.py b/tests/gateway/test_platform_base.py index 92348d565b1..f69a211fa37 100644 --- a/tests/gateway/test_platform_base.py +++ b/tests/gateway/test_platform_base.py @@ -1702,3 +1702,126 @@ class TestMediaDeliveryDiagnosability: assert any(r.endswith("cache/documents") for r in roots) # Legacy layout still present. assert any(r.endswith("image_cache") for r in roots) + + +# --------------------------------------------------------------------------- +# Media-send fallback must not leak host filesystem paths into chat +# --------------------------------------------------------------------------- + + +class _CapturingAdapter(BasePlatformAdapter): + """Minimal concrete BasePlatformAdapter that records what send() sees. + + The four media-send fallbacks (send_voice, send_video, send_document, + send_image_file) historically forwarded their *_path argument into the + chat text. That argument is a host filesystem path inside the Hermes + cache, so any subclass that fell back to super() — like the Telegram + adapter on a rejected video — would leak the host's directory layout + into the user's chat. + """ + + def __init__(self): + from gateway.config import Platform, PlatformConfig + super().__init__(PlatformConfig(enabled=True), Platform.TELEGRAM) + self.sent: list[dict] = [] + + async def connect(self) -> bool: # pragma: no cover - not exercised + return True + + async def disconnect(self) -> None: # pragma: no cover - not exercised + return None + + async def get_chat_info(self, chat_id): # pragma: no cover - not exercised + return {"name": chat_id, "type": "dm"} + + async def send(self, chat_id, content, reply_to=None, metadata=None): + from gateway.platforms.base import SendResult + self.sent.append({ + "chat_id": chat_id, + "content": content, + "reply_to": reply_to, + "metadata": metadata, + }) + return SendResult(success=True, message_id="m1") + + +class TestMediaFallbackDoesNotLeakHostPath: + """Regression: the four base-class media fallbacks must not echo *_path. + + Telegram, Discord, and Slack adapters all fall back to these base + implementations on native-send failure. When they did, the user saw + a chat message like ``🎬 Video: /home/.../hermes/cache/video/abc.mp4`` + — a host filesystem path with no actionable information. + """ + + SENSITIVE_PATH = "/home/jayne/.hermes/cache/media/sensitive_host_path_abc123.bin" + + @pytest.mark.asyncio + async def test_send_voice_fallback_omits_audio_path(self): + adapter = _CapturingAdapter() + result = await adapter.send_voice(chat_id="123", audio_path=self.SENSITIVE_PATH) + assert result.success + assert len(adapter.sent) == 1 + sent_text = adapter.sent[0]["content"] + assert self.SENSITIVE_PATH not in sent_text + assert "/home/" not in sent_text + assert "audio" in sent_text.lower() + + @pytest.mark.asyncio + async def test_send_video_fallback_omits_video_path(self): + adapter = _CapturingAdapter() + result = await adapter.send_video(chat_id="123", video_path=self.SENSITIVE_PATH) + assert result.success + sent_text = adapter.sent[0]["content"] + assert self.SENSITIVE_PATH not in sent_text + assert "/home/" not in sent_text + assert "video" in sent_text.lower() + + @pytest.mark.asyncio + async def test_send_document_fallback_omits_file_path(self): + adapter = _CapturingAdapter() + result = await adapter.send_document(chat_id="123", file_path=self.SENSITIVE_PATH) + assert result.success + sent_text = adapter.sent[0]["content"] + assert self.SENSITIVE_PATH not in sent_text + assert "/home/" not in sent_text + assert "file" in sent_text.lower() + + @pytest.mark.asyncio + async def test_send_document_fallback_includes_explicit_filename_only(self): + """A caller-supplied file_name is user-facing and may be shown — but + the host file_path argument must still be suppressed.""" + adapter = _CapturingAdapter() + result = await adapter.send_document( + chat_id="123", + file_path=self.SENSITIVE_PATH, + file_name="report.pdf", + ) + assert result.success + sent_text = adapter.sent[0]["content"] + assert self.SENSITIVE_PATH not in sent_text + assert "/home/" not in sent_text + assert "report.pdf" in sent_text + + @pytest.mark.asyncio + async def test_send_image_file_fallback_omits_image_path(self): + adapter = _CapturingAdapter() + result = await adapter.send_image_file(chat_id="123", image_path=self.SENSITIVE_PATH) + assert result.success + sent_text = adapter.sent[0]["content"] + assert self.SENSITIVE_PATH not in sent_text + assert "/home/" not in sent_text + assert "image" in sent_text.lower() + + @pytest.mark.asyncio + async def test_caption_is_preserved_in_fallback(self): + """The user-supplied caption is still shown — only the path is suppressed.""" + adapter = _CapturingAdapter() + await adapter.send_video( + chat_id="123", + video_path=self.SENSITIVE_PATH, + caption="Here's the daily summary.", + ) + sent_text = adapter.sent[0]["content"] + assert "Here's the daily summary." in sent_text + assert self.SENSITIVE_PATH not in sent_text