diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 0b74c4e15f..7e8e3c72d3 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -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.") diff --git a/tests/gateway/test_telegram_approval_buttons.py b/tests/gateway/test_telegram_approval_buttons.py index 93b5f82eef..6145526f39 100644 --- a/tests/gateway/test_telegram_approval_buttons.py +++ b/tests/gateway/test_telegram_approval_buttons.py @@ -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"