mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(telegram): redact bot token from connect/disconnect/send_document/send_video errors
_redact_telegram_error_text() strips bot tokens from api.telegram.org URLs embedded in transport-error text, and is already applied across the send/edit transient-error paths. Four sites still built their message from the raw exception: - connect()'s fatal-error handler is the most severe: the raw text is passed to _set_fatal_error(), which persists it via write_runtime_status() to a dashboard/admin-facing runtime status file, not just a log line. A transient network error during startup commonly embeds the request URL (https://api.telegram.org/bot<TOKEN>/getMe), so this could leak the live bot token into that surface. - disconnect(), send_document(), send_video() build the same unredacted pattern into a warning log line (lower blast radius, but the same leak class). Fix: route all four through the existing _redact_telegram_error_text() helper before building the message/log line, mirroring the send/edit paths exactly. Also drops exc_info=True from the two logger.error/ logger.warning calls that had it — exc_info prints the exception's own traceback (including its unredacted message) separately from the format string, which would otherwise defeat the redaction; the already-redacted sibling call sites in this file follow the same convention.
This commit is contained in:
parent
f10851e3fd
commit
1e2914b40a
2 changed files with 147 additions and 5 deletions
|
|
@ -3306,9 +3306,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
|
||||
except Exception as e:
|
||||
self._release_platform_lock()
|
||||
message = f"Telegram startup failed: {e}"
|
||||
safe_error = _redact_telegram_error_text(e)
|
||||
message = f"Telegram startup failed: {safe_error}"
|
||||
self._set_fatal_error("telegram_connect_error", message, retryable=True)
|
||||
logger.error("[%s] Failed to connect to Telegram: %s", self.name, e, exc_info=True)
|
||||
logger.error("[%s] Failed to connect to Telegram: %s", self.name, safe_error)
|
||||
return False
|
||||
|
||||
async def _set_status_indicator(self, online: bool) -> None:
|
||||
|
|
@ -3446,7 +3447,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
await self._app.stop()
|
||||
await self._app.shutdown()
|
||||
except Exception as e:
|
||||
logger.warning("[%s] Error during Telegram disconnect: %s", self.name, e, exc_info=True)
|
||||
logger.warning(
|
||||
"[%s] Error during Telegram disconnect: %s",
|
||||
self.name, _redact_telegram_error_text(e),
|
||||
)
|
||||
self._release_platform_lock()
|
||||
|
||||
self._app = None
|
||||
|
|
@ -6099,7 +6103,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
)
|
||||
return SendResult(success=True, message_id=str(msg.message_id))
|
||||
except Exception as e:
|
||||
logger.warning("[%s] Failed to send document: %s", self.name, e, exc_info=True)
|
||||
logger.warning(
|
||||
"[%s] Failed to send document: %s",
|
||||
self.name, _redact_telegram_error_text(e),
|
||||
)
|
||||
return await super().send_document(chat_id, file_path, caption, file_name, reply_to, metadata=metadata)
|
||||
|
||||
async def send_video(
|
||||
|
|
@ -6146,7 +6153,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
)
|
||||
return SendResult(success=True, message_id=str(msg.message_id))
|
||||
except Exception as e:
|
||||
logger.warning("[%s] Failed to send video: %s", self.name, e, exc_info=True)
|
||||
logger.warning(
|
||||
"[%s] Failed to send video: %s",
|
||||
self.name, _redact_telegram_error_text(e),
|
||||
)
|
||||
return await super().send_video(chat_id, video_path, caption, reply_to, metadata=metadata)
|
||||
|
||||
async def send_image(
|
||||
|
|
|
|||
132
tests/gateway/test_telegram_error_redaction.py
Normal file
132
tests/gateway/test_telegram_error_redaction.py
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
"""Regression tests for remaining unredacted Telegram transport-error sites.
|
||||
|
||||
``c3ab1424e`` added ``_redact_telegram_error_text()`` (built on
|
||||
``agent.redact``'s bot-token stripping for
|
||||
``api.telegram.org/bot<TOKEN>/...`` URLs) and applied it across the
|
||||
send/edit transient-error paths. Four sites still built their message from
|
||||
the raw exception:
|
||||
|
||||
- ``connect()``'s fatal-error path — the most severe: the raw text is
|
||||
passed to ``_set_fatal_error()``, which *persists* it via
|
||||
``write_runtime_status()`` to a dashboard/admin-facing runtime status
|
||||
file, not just a log line. A transient network error during startup
|
||||
commonly embeds the request URL (``https://api.telegram.org/bot<TOKEN>/
|
||||
getMe``), so this could leak the live bot token into that surface.
|
||||
- ``disconnect()``, ``send_document()``, ``send_video()`` — log-only,
|
||||
lower blast radius, but the same unredacted-exception pattern.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway.config import PlatformConfig
|
||||
from gateway.platforms.base import BasePlatformAdapter
|
||||
from plugins.platforms.telegram.adapter import TelegramAdapter
|
||||
|
||||
_SECRET_TOKEN = "123456789:AAFakeSecretTelegramBotTokenABCDEFGHIJ"
|
||||
_SECRET_URL = f"https://api.telegram.org/bot{_SECRET_TOKEN}/getMe"
|
||||
|
||||
|
||||
def _make_bare_adapter() -> TelegramAdapter:
|
||||
config = PlatformConfig(enabled=True, token=_SECRET_TOKEN, extra={})
|
||||
return TelegramAdapter(config)
|
||||
|
||||
|
||||
def _make_connected_adapter() -> TelegramAdapter:
|
||||
"""Adapter with a mock bot wired, past connect() — for send_* tests."""
|
||||
adapter = _make_bare_adapter()
|
||||
bot = MagicMock()
|
||||
bot.send_chat_action = AsyncMock()
|
||||
adapter._bot = bot
|
||||
return adapter
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_failure_redacts_token_from_fatal_status(monkeypatch):
|
||||
"""A connect()-time exception embedding the bot token URL must not reach
|
||||
the persisted fatal-error status or the log line unredacted."""
|
||||
adapter = _make_bare_adapter()
|
||||
|
||||
def _boom(*_args, **_kwargs):
|
||||
raise RuntimeError(f"Network error connecting to {_SECRET_URL}")
|
||||
|
||||
monkeypatch.setattr(adapter, "_acquire_platform_lock", _boom)
|
||||
monkeypatch.setattr(adapter, "_write_runtime_status_safe", lambda *a, **k: None)
|
||||
|
||||
result = await adapter.connect()
|
||||
|
||||
assert result is False
|
||||
assert adapter._fatal_error_message is not None
|
||||
assert _SECRET_TOKEN not in adapter._fatal_error_message
|
||||
assert "***" in adapter._fatal_error_message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disconnect_failure_redacts_token_in_log(monkeypatch, caplog):
|
||||
"""A disconnect()-time exception embedding the bot token URL must not
|
||||
reach the warning log unredacted."""
|
||||
adapter = _make_connected_adapter()
|
||||
adapter._app = SimpleNamespace(
|
||||
updater=SimpleNamespace(running=False),
|
||||
running=False,
|
||||
shutdown=AsyncMock(side_effect=RuntimeError(f"teardown failed: {_SECRET_URL}")),
|
||||
)
|
||||
adapter._release_platform_lock = lambda: None
|
||||
adapter._cancel_pending_delivery_tasks = AsyncMock()
|
||||
|
||||
with caplog.at_level("WARNING"):
|
||||
await adapter.disconnect()
|
||||
|
||||
logged = "\n".join(r.getMessage() for r in caplog.records)
|
||||
assert _SECRET_TOKEN not in logged
|
||||
assert "Error during Telegram disconnect" in logged
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_document_failure_redacts_token_in_log(monkeypatch, caplog, tmp_path):
|
||||
"""A send_document() transport exception embedding the bot token URL
|
||||
must not reach the warning log unredacted."""
|
||||
adapter = _make_connected_adapter()
|
||||
file_path = tmp_path / "report.pdf"
|
||||
file_path.write_bytes(b"%PDF-1.4 fake")
|
||||
|
||||
monkeypatch.setattr(
|
||||
adapter,
|
||||
"_send_with_dm_topic_reply_anchor_retry",
|
||||
AsyncMock(side_effect=RuntimeError(f"upload failed: {_SECRET_URL}")),
|
||||
)
|
||||
fallback = AsyncMock(return_value=SimpleNamespace(success=False, error="fallback"))
|
||||
monkeypatch.setattr(BasePlatformAdapter, "send_document", fallback)
|
||||
|
||||
with caplog.at_level("WARNING"):
|
||||
await adapter.send_document("123", str(file_path))
|
||||
|
||||
logged = "\n".join(r.getMessage() for r in caplog.records)
|
||||
assert _SECRET_TOKEN not in logged
|
||||
assert "Failed to send document" in logged
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_video_failure_redacts_token_in_log(monkeypatch, caplog, tmp_path):
|
||||
"""A send_video() transport exception embedding the bot token URL must
|
||||
not reach the warning log unredacted."""
|
||||
adapter = _make_connected_adapter()
|
||||
video_path = tmp_path / "clip.mp4"
|
||||
video_path.write_bytes(b"fake mp4 bytes")
|
||||
|
||||
monkeypatch.setattr(
|
||||
adapter,
|
||||
"_send_with_dm_topic_reply_anchor_retry",
|
||||
AsyncMock(side_effect=RuntimeError(f"upload failed: {_SECRET_URL}")),
|
||||
)
|
||||
fallback = AsyncMock(return_value=SimpleNamespace(success=False, error="fallback"))
|
||||
monkeypatch.setattr(BasePlatformAdapter, "send_video", fallback)
|
||||
|
||||
with caplog.at_level("WARNING"):
|
||||
await adapter.send_video("123", str(video_path))
|
||||
|
||||
logged = "\n".join(r.getMessage() for r in caplog.records)
|
||||
assert _SECRET_TOKEN not in logged
|
||||
assert "Failed to send video" in logged
|
||||
Loading…
Add table
Add a link
Reference in a new issue