mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: allow Telegram username allowlists for approval buttons
This commit is contained in:
parent
6af04474a3
commit
15feb6ab67
2 changed files with 68 additions and 12 deletions
|
|
@ -250,13 +250,30 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
self._approval_state: Dict[int, str] = {}
|
||||
|
||||
@staticmethod
|
||||
def _is_callback_user_authorized(user_id: str) -> bool:
|
||||
"""Return whether a Telegram inline-button caller may perform gated actions."""
|
||||
def _is_callback_user_authorized(user_id: str, username: Optional[str] = None) -> bool:
|
||||
"""Return whether a Telegram inline-button caller may perform gated actions.
|
||||
|
||||
TELEGRAM_ALLOWED_USERS historically accepts either numeric Telegram user IDs
|
||||
or @usernames. Approval/update buttons only exposed the numeric callback user
|
||||
ID, which broke existing username-based allowlists after the button flow
|
||||
shipped. Accept both forms here so button auth matches normal Telegram auth.
|
||||
"""
|
||||
allowed_csv = os.getenv("TELEGRAM_ALLOWED_USERS", "").strip()
|
||||
if not allowed_csv:
|
||||
return True
|
||||
allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()}
|
||||
return "*" in allowed_ids or user_id in allowed_ids
|
||||
|
||||
allowed_entries = {entry.strip().lower() for entry in allowed_csv.split(",") if entry.strip()}
|
||||
if "*" in allowed_entries:
|
||||
return True
|
||||
|
||||
normalized_id = str(user_id or "").strip().lower()
|
||||
normalized_username = str(username or "").strip().lower().lstrip("@")
|
||||
candidates = {normalized_id}
|
||||
if normalized_username:
|
||||
candidates.add(normalized_username)
|
||||
candidates.add(f"@{normalized_username}")
|
||||
|
||||
return any(candidate and candidate in allowed_entries for candidate in candidates)
|
||||
|
||||
@classmethod
|
||||
def _metadata_thread_id(cls, metadata: Optional[Dict[str, Any]]) -> Optional[str]:
|
||||
|
|
@ -1582,7 +1599,8 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
|
||||
# Only authorized users may click approval buttons.
|
||||
caller_id = str(getattr(query.from_user, "id", ""))
|
||||
if not self._is_callback_user_authorized(caller_id):
|
||||
caller_username = str(getattr(query.from_user, "username", "") or "")
|
||||
if not self._is_callback_user_authorized(caller_id, caller_username):
|
||||
await query.answer(text="⛔ You are not authorized to approve commands.")
|
||||
return
|
||||
|
||||
|
|
@ -1630,7 +1648,8 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
return
|
||||
answer = data.split(":", 1)[1] # "y" or "n"
|
||||
caller_id = str(getattr(query.from_user, "id", ""))
|
||||
if not self._is_callback_user_authorized(caller_id):
|
||||
caller_username = str(getattr(query.from_user, "username", "") or "")
|
||||
if not self._is_callback_user_authorized(caller_id, caller_username):
|
||||
await query.answer(text="⛔ You are not authorized to answer update prompts.")
|
||||
return
|
||||
await query.answer(text=f"Sent '{answer}' to the update process.")
|
||||
|
|
|
|||
|
|
@ -187,6 +187,8 @@ class TestTelegramApprovalCallback:
|
|||
query.message = MagicMock()
|
||||
query.message.chat_id = 12345
|
||||
query.from_user = MagicMock()
|
||||
query.from_user.id = 123
|
||||
query.from_user.username = "norbert"
|
||||
query.from_user.first_name = "Norbert"
|
||||
query.answer = AsyncMock()
|
||||
query.edit_message_text = AsyncMock()
|
||||
|
|
@ -215,6 +217,8 @@ class TestTelegramApprovalCallback:
|
|||
query.message = MagicMock()
|
||||
query.message.chat_id = 12345
|
||||
query.from_user = MagicMock()
|
||||
query.from_user.id = 123
|
||||
query.from_user.username = "alice"
|
||||
query.from_user.first_name = "Alice"
|
||||
query.answer = AsyncMock()
|
||||
query.edit_message_text = AsyncMock()
|
||||
|
|
@ -240,6 +244,8 @@ class TestTelegramApprovalCallback:
|
|||
query.message = MagicMock()
|
||||
query.message.chat_id = 12345
|
||||
query.from_user = MagicMock()
|
||||
query.from_user.id = 123
|
||||
query.from_user.username = "bob"
|
||||
query.from_user.first_name = "Bob"
|
||||
query.answer = AsyncMock()
|
||||
|
||||
|
|
@ -334,16 +340,47 @@ class TestTelegramApprovalCallback:
|
|||
assert not (tmp_path / ".update_response").exists()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_prompt_callback_allows_authorized_user(self, tmp_path):
|
||||
"""Allowed Telegram users can still answer update prompt buttons."""
|
||||
async def test_approval_callback_allows_username_allowlist(self):
|
||||
"""Username-based TELEGRAM_ALLOWED_USERS entries should work for buttons too."""
|
||||
adapter = _make_adapter()
|
||||
adapter._approval_state[7] = "agent:main:telegram:dm:12345"
|
||||
|
||||
query = AsyncMock()
|
||||
query.data = "update_prompt:n"
|
||||
query.data = "ea:session:7"
|
||||
query.message = MagicMock()
|
||||
query.message.chat_id = 12345
|
||||
query.from_user = MagicMock()
|
||||
query.from_user.id = 111
|
||||
query.from_user.id = 222
|
||||
query.from_user.username = "waynemattadeen"
|
||||
query.from_user.first_name = "Wayne"
|
||||
query.answer = AsyncMock()
|
||||
query.edit_message_text = AsyncMock()
|
||||
|
||||
update = MagicMock()
|
||||
update.callback_query = query
|
||||
context = MagicMock()
|
||||
|
||||
with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "@waynemattadeen"}):
|
||||
with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve:
|
||||
await adapter._handle_callback_query(update, context)
|
||||
|
||||
mock_resolve.assert_called_once_with("agent:main:telegram:dm:12345", "session")
|
||||
query.answer.assert_called_once()
|
||||
query.edit_message_text.assert_called_once()
|
||||
assert 7 not in adapter._approval_state
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_prompt_callback_allows_username_allowlist(self, tmp_path):
|
||||
"""Username-based TELEGRAM_ALLOWED_USERS entries should work for update buttons."""
|
||||
adapter = _make_adapter()
|
||||
|
||||
query = AsyncMock()
|
||||
query.data = "update_prompt:y"
|
||||
query.message = MagicMock()
|
||||
query.message.chat_id = 12345
|
||||
query.from_user = MagicMock()
|
||||
query.from_user.id = 222
|
||||
query.from_user.username = "waynemattadeen"
|
||||
query.answer = AsyncMock()
|
||||
query.edit_message_text = AsyncMock()
|
||||
|
||||
|
|
@ -352,9 +389,9 @@ class TestTelegramApprovalCallback:
|
|||
context = MagicMock()
|
||||
|
||||
with patch("hermes_constants.get_hermes_home", return_value=tmp_path):
|
||||
with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "111"}):
|
||||
with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "@waynemattadeen"}):
|
||||
await adapter._handle_callback_query(update, context)
|
||||
|
||||
query.answer.assert_called_once()
|
||||
query.edit_message_text.assert_called_once()
|
||||
assert (tmp_path / ".update_response").read_text() == "n"
|
||||
assert (tmp_path / ".update_response").read_text() == "y"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue