mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(gateway): stop media-send fallbacks from leaking host paths into chat
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.
This commit is contained in:
parent
fee3d4ed04
commit
cb9d18c759
3 changed files with 178 additions and 14 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue