From e2aaeed12d48ce07baacbef7de5d66517116a4e3 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Thu, 5 Mar 2026 11:24:46 +0700 Subject: [PATCH 1/2] fix: multi-group moderation gaps in topic guard and spam handlers - topic_guard: raise ApplicationHandlerStop after handling warning-topic messages to prevent downstream handlers from processing them - topic_guard: handle edited messages (update.message or update.edited_message) - topic_guard: fail-closed on get_chat_member error (delete message in warning topic on API failure), scoped to confirmed warning-topic only - main: register edited_message handler for topic_guard using combined MESSAGE | EDITED_MESSAGE filter at group=-1 - main: add 'edited_message' to allowed_updates in run_polling - main: add periodic admin cache refresh via JobQueue (every 10 minutes) with graceful fallback to cached data on failure - tests: improve coverage from 99% to 99.9% (519 tests, 1 unreachable line) Amp-Thread-ID: https://ampcode.com/threads/T-019cbc2b-3b09-743f-9ecc-eaa7dfd0234c Co-authored-by: Amp --- src/bot/handlers/topic_guard.py | 69 ++++++++++------- src/bot/main.py | 41 +++++++++- tests/test_anti_spam.py | 9 +++ tests/test_check.py | 32 ++++++++ tests/test_database.py | 54 +++++++++++++ tests/test_dm_handler.py | 19 +++++ tests/test_group_config.py | 10 +++ tests/test_topic_guard.py | 130 +++++++++++++++++++++++++++++--- tests/test_verify_handler.py | 31 ++++++++ 9 files changed, 355 insertions(+), 40 deletions(-) diff --git a/src/bot/handlers/topic_guard.py b/src/bot/handlers/topic_guard.py index 64bf015..3975eef 100644 --- a/src/bot/handlers/topic_guard.py +++ b/src/bot/handlers/topic_guard.py @@ -9,7 +9,7 @@ import logging from telegram import Update -from telegram.ext import ContextTypes +from telegram.ext import ApplicationHandlerStop, ContextTypes from bot.group_config import get_group_config_for_update @@ -31,41 +31,44 @@ async def guard_warning_topic(update: Update, context: ContextTypes.DEFAULT_TYPE update: Telegram update containing the message. context: Bot context with helper methods. """ - try: - # Skip if no message or sender - if not update.message or not update.message.from_user: - logger.info("No message or no sender, skipping") - return + message = update.message or update.edited_message + + # Skip if no message or sender + if not message or not message.from_user: + logger.info("No message or no sender, skipping") + return + + group_config = get_group_config_for_update(update) + user = message.from_user + chat_id = update.effective_chat.id if update.effective_chat else None + thread_id = message.message_thread_id - group_config = get_group_config_for_update(update) - user = update.message.from_user - chat_id = update.effective_chat.id if update.effective_chat else None - thread_id = update.message.message_thread_id + logger.info( + f"Topic guard called: user_id={user.id}, chat_id={chat_id}, thread_id={thread_id}" + ) + # Only process messages from monitored groups + if group_config is None: logger.info( - f"Topic guard called: user_id={user.id}, chat_id={chat_id}, thread_id={thread_id}" + f"Chat not monitored (chat_id={chat_id}), skipping" ) + return - # Only process messages from monitored groups - if group_config is None: - logger.info( - f"Chat not monitored (chat_id={chat_id}), skipping" - ) - return - - # Only guard the warning topic, not other topics - if thread_id != group_config.warning_topic_id: - logger.info( - f"Wrong topic (thread_id={thread_id}, expected {group_config.warning_topic_id}), skipping" - ) - return + # Only guard the warning topic, not other topics + if thread_id != group_config.warning_topic_id: + logger.info( + f"Wrong topic (thread_id={thread_id}, expected {group_config.warning_topic_id}), skipping" + ) + return + # From here on, we're in the warning topic - use try/except with fail-closed + try: bot_id = context.bot.id # Allow bot's own messages if user.id == bot_id: logger.info(f"Allowing bot's own message (bot_id={bot_id})") - return + raise ApplicationHandlerStop # Check if user is an admin or creator logger.info(f"Checking admin status for user {user.id} ({user.full_name})") @@ -79,17 +82,29 @@ async def guard_warning_topic(update: Update, context: ContextTypes.DEFAULT_TYPE logger.info( f"Allowing message from {chat_member.status} {user.id} ({user.full_name})" ) - return + raise ApplicationHandlerStop # Delete message from non-admin user logger.info( f"Deleting message from non-admin user {user.id} ({user.full_name}) " f"in warning topic (group_id={group_config.group_id}, thread_id={thread_id})" ) - await update.message.delete() + await message.delete() + raise ApplicationHandlerStop + except ApplicationHandlerStop: + raise except Exception as e: logger.error( f"Error in topic guard handler: {e}", exc_info=True, ) + # Fail-closed: delete message in warning topic on error + try: + await message.delete() + except Exception as delete_error: + logger.error( + f"Failed to delete message during error recovery: {delete_error}", + exc_info=True, + ) + raise ApplicationHandlerStop diff --git a/src/bot/main.py b/src/bot/main.py index 8e0cd64..4e15540 100644 --- a/src/bot/main.py +++ b/src/bot/main.py @@ -110,6 +110,33 @@ def configure_logging() -> None: logger = logging.getLogger(__name__) +async def refresh_admin_ids(context: ContextTypes.DEFAULT_TYPE) -> None: + """ + Periodically refresh cached admin IDs for all monitored groups. + + Called by JobQueue every 10 minutes to keep admin rosters up to date + when promotions/demotions happen after startup. + """ + registry = get_group_registry() + group_admin_ids: dict[int, list[int]] = {} + all_admin_ids: set[int] = set() + + for gc in registry.all_groups(): + try: + ids = await fetch_group_admin_ids(context.bot, gc.group_id) + group_admin_ids[gc.group_id] = ids + all_admin_ids.update(ids) + except Exception as e: + logger.error(f"Failed to refresh admin IDs for group {gc.group_id}: {e}") + existing = context.bot_data.get("group_admin_ids", {}).get(gc.group_id, []) + group_admin_ids[gc.group_id] = existing + all_admin_ids.update(existing) + + context.bot_data["group_admin_ids"] = group_admin_ids + context.bot_data["admin_ids"] = list(all_admin_ids) + logger.info(f"Refreshed admin IDs: {len(all_admin_ids)} unique admin(s) across {len(group_admin_ids)} group(s)") + + async def error_handler(update: object, context: ContextTypes.DEFAULT_TYPE) -> None: """ Handle errors in the bot. @@ -212,12 +239,12 @@ def main() -> None: # messages in the warning topic before other handlers process them application.add_handler( MessageHandler( - filters.ALL, + filters.UpdateType.MESSAGE | filters.UpdateType.EDITED_MESSAGE, guard_warning_topic, ), group=-1, ) - logger.info("Registered handler: topic_guard (group=-1)") + logger.info("Registered handler: topic_guard (group=-1, message + edited_message)") # Handler 2: /verify command - allows admins to whitelist users in DM application.add_handler( @@ -331,10 +358,18 @@ def main() -> None: ) logger.info("JobQueue registered: auto_restrict_job (every 5 minutes, first run in 5 minutes)") + application.job_queue.run_repeating( + refresh_admin_ids, + interval=600, + first=600, + name="refresh_admin_ids_job" + ) + logger.info("JobQueue registered: refresh_admin_ids_job (every 10 minutes)") + logger.info(f"Starting bot polling for {group_count} group(s)") logger.info("All handlers registered successfully") - application.run_polling(allowed_updates=["message", "callback_query", "chat_member"]) + application.run_polling(allowed_updates=["message", "edited_message", "callback_query", "chat_member"]) if __name__ == "__main__": diff --git a/tests/test_anti_spam.py b/tests/test_anti_spam.py index 836aaa8..7a520fa 100644 --- a/tests/test_anti_spam.py +++ b/tests/test_anti_spam.py @@ -848,6 +848,15 @@ def test_mixed_urls_returns_true(self): assert has_non_whitelisted_inline_keyboard_urls(msg) is True + def test_none_button_skipped(self): + """Test that None buttons in a row don't crash.""" + reply_markup = MagicMock() + reply_markup.inline_keyboard = [[None]] + msg = MagicMock(spec=Message) + msg.reply_markup = reply_markup + + assert has_non_whitelisted_inline_keyboard_urls(msg) is False + def test_callback_data_buttons_returns_false(self): """Test that buttons without URLs (callback_data buttons) return False.""" button = MagicMock() diff --git a/tests/test_check.py b/tests/test_check.py index 1e3bf5c..7ae071a 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -604,3 +604,35 @@ async def test_warn_callback_get_chat_timeout( query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args assert "timeout" in call_args.args[0].lower() + + async def test_warn_callback_per_group_send_failure_all_groups( + self, mock_context, mock_settings, mock_registry + ): + """When send_message fails for all groups, shows 'all groups failed' message.""" + update = MagicMock() + query = MagicMock() + query.from_user = MagicMock() + query.from_user.id = 12345 + query.from_user.full_name = "Admin User" + query.data = "warn:555666:pu" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + + mock_chat = MagicMock() + mock_chat.full_name = "Test User" + mock_chat.username = "testuser" + mock_context.bot.get_chat.return_value = mock_chat + mock_context.bot.send_message.side_effect = RuntimeError("Connection lost") + + with ( + patch( + "bot.handlers.check.get_group_registry", + return_value=mock_registry, + ), + ): + await handle_warn_callback(update, mock_context) + + query.edit_message_text.assert_called_once() + call_args = query.edit_message_text.call_args + assert "Gagal mengirim peringatan ke semua grup" in call_args.args[0] diff --git a/tests/test_database.py b/tests/test_database.py index b106f05..60f778a 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -333,3 +333,57 @@ def test_clear_new_user_probation(self, db_service: DatabaseService): record = db_service.get_new_user_probation(user_id=1001, group_id=-100) assert record is None + + +class TestGetWarningsPastTimeThresholdForGroup: + def test_returns_empty_when_no_warnings(self, db_service): + result = db_service.get_warnings_past_time_threshold_for_group( + group_id=-100999, threshold=timedelta(minutes=1440) + ) + assert result == [] + + def test_returns_warnings_past_threshold_for_group(self, db_service): + old_time = datetime.now(UTC) - timedelta(minutes=1500) + record = db_service.get_or_create_user_warning(user_id=123, group_id=-100999) + + from sqlmodel import Session, select + + with Session(db_service._engine) as session: + stmt = select(UserWarning).where(UserWarning.id == record.id) + db_record = session.exec(stmt).first() + db_record.first_warned_at = old_time + session.add(db_record) + session.commit() + + result = db_service.get_warnings_past_time_threshold_for_group( + group_id=-100999, threshold=timedelta(minutes=1440) + ) + assert len(result) == 1 + assert result[0].user_id == 123 + + def test_does_not_return_warnings_from_different_group(self, db_service): + old_time = datetime.now(UTC) - timedelta(minutes=1500) + record = db_service.get_or_create_user_warning(user_id=123, group_id=-100999) + + from sqlmodel import Session, select + + with Session(db_service._engine) as session: + stmt = select(UserWarning).where(UserWarning.id == record.id) + db_record = session.exec(stmt).first() + db_record.first_warned_at = old_time + session.add(db_record) + session.commit() + + result = db_service.get_warnings_past_time_threshold_for_group( + group_id=-200888, threshold=timedelta(minutes=1440) + ) + assert result == [] + + def test_does_not_return_restricted_warnings(self, db_service): + db_service.get_or_create_user_warning(user_id=123, group_id=-100999) + db_service.mark_user_restricted(user_id=123, group_id=-100999) + + result = db_service.get_warnings_past_time_threshold_for_group( + group_id=-100999, threshold=timedelta(minutes=0) + ) + assert result == [] diff --git a/tests/test_dm_handler.py b/tests/test_dm_handler.py index 80eeda7..fa29d05 100644 --- a/tests/test_dm_handler.py +++ b/tests/test_dm_handler.py @@ -108,6 +108,25 @@ async def test_user_not_in_group( assert "belum bergabung di grup" in call_args.args[0] mock_context.bot.restrict_chat_member.assert_not_called() + async def test_get_user_status_exception_continues( + self, mock_update, mock_context, mock_settings, mock_registry, temp_db + ): + """When get_user_status raises an Exception, handler catches it and treats user as not in group.""" + with ( + patch("bot.handlers.dm.get_settings", return_value=mock_settings), + patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), + patch( + "bot.handlers.dm.get_user_status", + new_callable=AsyncMock, + side_effect=RuntimeError("API error"), + ), + ): + await handle_dm(mock_update, mock_context) + + mock_update.message.reply_text.assert_called_once() + call_args = mock_update.message.reply_text.call_args + assert "belum bergabung di grup" in call_args.args[0] + async def test_user_left_group( self, mock_update, mock_context, mock_settings, mock_registry, temp_db ): diff --git a/tests/test_group_config.py b/tests/test_group_config.py index 07e5757..f7ae56d 100644 --- a/tests/test_group_config.py +++ b/tests/test_group_config.py @@ -292,6 +292,16 @@ def test_falls_back_to_env(self): class TestGetGroupConfigForUpdate: + def test_returns_none_when_registry_not_initialized(self): + reset_group_registry() + + update = MagicMock() + update.effective_chat = MagicMock() + update.effective_chat.id = -100 + + result = get_group_config_for_update(update) + assert result is None + def test_returns_config_for_monitored_group(self): gc = GroupConfig(group_id=-100, warning_topic_id=1) registry = GroupRegistry() diff --git a/tests/test_topic_guard.py b/tests/test_topic_guard.py index 2ec8b29..412fc24 100644 --- a/tests/test_topic_guard.py +++ b/tests/test_topic_guard.py @@ -1,6 +1,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from telegram.ext import ApplicationHandlerStop from bot.group_config import GroupConfig from bot.handlers.topic_guard import guard_warning_topic @@ -23,6 +24,7 @@ def mock_update(): update.message.from_user.full_name = "Test User" update.message.message_thread_id = 42 update.message.delete = AsyncMock() + update.edited_message = None update.effective_chat = MagicMock() update.effective_chat.id = -1001234567890 return update @@ -40,6 +42,7 @@ class TestGuardWarningTopic: async def test_no_message(self, mock_context): update = MagicMock() update.message = None + update.edited_message = None await guard_warning_topic(update, mock_context) @@ -49,6 +52,7 @@ async def test_no_user(self, mock_context): update = MagicMock() update.message = MagicMock() update.message.from_user = None + update.edited_message = None await guard_warning_topic(update, mock_context) @@ -78,7 +82,8 @@ async def test_bot_message_allowed(self, mock_update, mock_context, group_config mock_update.message.from_user.id = 99999 # Same as bot id with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_context.bot.get_chat_member.assert_not_called() mock_update.message.delete.assert_not_called() @@ -91,7 +96,8 @@ async def test_admin_message_allowed( mock_context.bot.get_chat_member.return_value = chat_member with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_context.bot.get_chat_member.assert_called_once_with( chat_id=-1001234567890, @@ -107,7 +113,8 @@ async def test_creator_message_allowed( mock_context.bot.get_chat_member.return_value = chat_member with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_update.message.delete.assert_not_called() @@ -119,7 +126,8 @@ async def test_regular_user_message_deleted( mock_context.bot.get_chat_member.return_value = chat_member with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_update.message.delete.assert_called_once() @@ -131,23 +139,125 @@ async def test_restricted_user_message_deleted( mock_context.bot.get_chat_member.return_value = chat_member with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_update.message.delete.assert_called_once() + async def test_get_group_config_error_does_not_delete( + self, mock_update, mock_context + ): + """Test that if get_group_config_for_update raises, message is NOT deleted.""" + with patch( + "bot.handlers.topic_guard.get_group_config_for_update", + side_effect=Exception("config lookup failed"), + ): + with pytest.raises(Exception, match="config lookup failed"): + await guard_warning_topic(mock_update, mock_context) + + mock_update.message.delete.assert_not_called() + + +class TestGuardWarningTopicEditedMessage: + async def test_edited_message_from_bot_allowed(self, mock_context, group_config): + update = MagicMock() + update.message = None + update.edited_message = MagicMock() + update.edited_message.from_user = MagicMock() + update.edited_message.from_user.id = 99999 # bot id + update.edited_message.from_user.full_name = "Bot" + update.edited_message.message_thread_id = 42 + update.edited_message.delete = AsyncMock() + update.effective_chat = MagicMock() + update.effective_chat.id = -1001234567890 + + with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(update, mock_context) + + mock_context.bot.get_chat_member.assert_not_called() + update.edited_message.delete.assert_not_called() + + async def test_edited_message_from_admin_allowed(self, mock_context, group_config): + update = MagicMock() + update.message = None + update.edited_message = MagicMock() + update.edited_message.from_user = MagicMock() + update.edited_message.from_user.id = 12345 + update.edited_message.from_user.full_name = "Admin User" + update.edited_message.message_thread_id = 42 + update.edited_message.delete = AsyncMock() + update.effective_chat = MagicMock() + update.effective_chat.id = -1001234567890 + + chat_member = MagicMock() + chat_member.status = "administrator" + mock_context.bot.get_chat_member.return_value = chat_member + + with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(update, mock_context) + + update.edited_message.delete.assert_not_called() + + async def test_edited_message_from_regular_user_deleted(self, mock_context, group_config): + update = MagicMock() + update.message = None + update.edited_message = MagicMock() + update.edited_message.from_user = MagicMock() + update.edited_message.from_user.id = 12345 + update.edited_message.from_user.full_name = "Regular User" + update.edited_message.message_thread_id = 42 + update.edited_message.delete = AsyncMock() + update.effective_chat = MagicMock() + update.effective_chat.id = -1001234567890 + + chat_member = MagicMock() + chat_member.status = "member" + mock_context.bot.get_chat_member.return_value = chat_member + + with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(update, mock_context) + + update.edited_message.delete.assert_called_once() + class TestGuardWarningTopicErrorHandling: - async def test_delete_message_exception_logged( + async def test_get_chat_member_error_deletes_message( self, mock_update, mock_context, group_config ): - """Test when update.message.delete() raises an exception (lines 91-92).""" + """Test fail-closed: on get_chat_member error, message is still deleted.""" + mock_context.bot.get_chat_member.side_effect = Exception("API error") + + with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) + + mock_update.message.delete.assert_called_once() + + async def test_delete_message_exception_still_raises_stop( + self, mock_update, mock_context, group_config + ): + """Test when delete in normal flow raises, error handler still deletes and raises stop.""" chat_member = MagicMock() chat_member.status = "member" mock_context.bot.get_chat_member.return_value = chat_member - mock_update.message.delete.side_effect = Exception("test error") + mock_update.message.delete.side_effect = Exception("delete error") with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): - # Should not raise, error is caught and logged - await guard_warning_topic(mock_update, mock_context) + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) + + async def test_error_recovery_delete_also_fails( + self, mock_update, mock_context, group_config + ): + """Test when both get_chat_member and recovery delete fail, still raises stop.""" + mock_context.bot.get_chat_member.side_effect = Exception("API error") + mock_update.message.delete.side_effect = Exception("delete also failed") + + with patch("bot.handlers.topic_guard.get_group_config_for_update", return_value=group_config): + with pytest.raises(ApplicationHandlerStop): + await guard_warning_topic(mock_update, mock_context) mock_update.message.delete.assert_called_once() diff --git a/tests/test_verify_handler.py b/tests/test_verify_handler.py index 480b0e5..403d2b1 100644 --- a/tests/test_verify_handler.py +++ b/tests/test_verify_handler.py @@ -388,6 +388,37 @@ async def test_verify_without_warnings_no_notification( # Should NOT send notification to warning topic mock_context.bot.send_message.assert_not_called() + async def test_verify_per_group_exception_caught_and_continues( + self, mock_update, mock_context, temp_db, monkeypatch + ): + """Test that a generic Exception in per-group verification is caught and continues.""" + gc = GroupConfig(group_id=-1001234567890, warning_topic_id=12345) + registry = GroupRegistry() + registry.register(gc) + monkeypatch.setattr("bot.handlers.verify.get_group_registry", lambda: registry) + + target_user_id = 99999999 + db = get_database() + + # Seed a warning so delete_warnings returns > 0, triggering get_chat/send_message + db.get_or_create_user_warning(target_user_id, gc.group_id) + db.increment_message_count(target_user_id, gc.group_id) + + mock_context.args = [str(target_user_id)] + + # Make restrict_chat_member raise a generic Exception (not BadRequest) + mock_context.bot.restrict_chat_member.side_effect = RuntimeError("Network error") + + await handle_verify_command(mock_update, mock_context) + + # Should still return success message despite per-group failure + mock_update.message.reply_text.assert_called_once() + call_args = mock_update.message.reply_text.call_args + assert "diverifikasi" in call_args.args[0] + + # User should still be whitelisted + assert db.is_user_photo_whitelisted(target_user_id) + class TestHandleUnverifyCommand: async def test_no_message(self, mock_context): From 70bc8b8caa3da7683ad784c1d30ce43a423e15ec Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Thu, 5 Mar 2026 11:29:34 +0700 Subject: [PATCH 2/2] docs: update README and AGENTS.md with topic guard, admin refresh, and coverage changes - Handler priority groups: document all 6 groups (-1 through 4) - Topic guard: document edited message handling, ApplicationHandlerStop, fail-closed behavior - Admin cache: document periodic refresh (every 10 min) with graceful fallback - allowed_updates: reflect addition of edited_message - Test coverage: update stats to 519 tests, 99.9% coverage - Test file list: add missing test_check.py, test_duplicate_spam.py - Architecture: reflect all handler modules and their priority groups Amp-Thread-ID: https://ampcode.com/threads/T-019cbc2b-3b09-743f-9ecc-eaa7dfd0234c Co-authored-by: Amp --- AGENTS.md | 30 ++++++++++++++++++++++++------ README.md | 42 +++++++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 29f8e92..5480a52 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,7 +58,7 @@ PythonID/ │ └── database/ │ ├── models.py # SQLModel schemas (4 tables) │ └── service.py # DatabaseService singleton (645 lines) -├── tests/ # pytest-asyncio (18 files, 99% coverage) +├── tests/ # pytest-asyncio (19 files, 99.9% coverage) └── data/bot.db # SQLite (auto-created, WAL mode) ``` @@ -91,16 +91,32 @@ PythonID/ ### Handler Priority Groups ```python # main.py - Order matters! -group=-1 # topic_guard: Runs FIRST, deletes unauthorized warning topic msgs -group=0 # Commands, DM, anti_spam: Default priority -group=1 # message_handler: Runs LAST, profile compliance check +group=-1 # topic_guard: Runs FIRST, deletes unauthorized warning topic msgs, raises ApplicationHandlerStop +group=0 # Commands, DM, captcha: Default priority +group=1 # inline_keyboard_spam: Catches inline keyboard URL spam +group=2 # new_user_spam: Probation enforcement (links/forwards) +group=3 # duplicate_spam: Repeated message detection +group=4 # message_handler: Runs LAST, profile compliance check ``` +### Topic Guard Design +- Handles both `message` and `edited_message` updates (combined filter) +- Raises `ApplicationHandlerStop` after handling ANY warning-topic message (allows or deletes) +- This prevents downstream spam/profile handlers from processing warning-topic traffic +- **Fail-closed**: On `get_chat_member` API error, deletes the message (scoped to confirmed warning-topic only) +- Early returns (no message, wrong group, wrong topic) happen OUTSIDE the try/except block + ### Singletons - `get_settings()` — Pydantic settings, `@lru_cache` - `get_database()` — DatabaseService, lazy init - `BotInfoCache` — Class-level cache for bot username/ID +### Admin Cache +- Fetched at startup in `post_init()` and stored in `bot_data["group_admin_ids"]` (per-group) and `bot_data["admin_ids"]` (union) +- Refreshed every 10 minutes via `refresh_admin_ids` JobQueue job +- On refresh failure for a group, falls back to existing cached data (not empty list) +- Spam handlers use cached admin IDs; topic_guard uses live `get_chat_member` API call + ### Multi-Group Support - `GroupConfig` — Pydantic model for per-group settings (warning thresholds, captcha, probation) - `GroupRegistry` — O(1) lookup by group_id, manages all monitored groups @@ -140,7 +156,7 @@ Time threshold → Auto-restrict via scheduler (parallel path) - **Fixtures**: `mock_update`, `mock_context`, `mock_settings` — copy from existing tests - **Database tests**: Use `temp_db` fixture with `tempfile.TemporaryDirectory` - **Mocking**: `AsyncMock` for Telegram API; no real network calls -- **Coverage**: 100% maintained — check before committing +- **Coverage**: 99.9% maintained (519 tests) — check before committing ## Anti-Patterns (THIS PROJECT) @@ -186,8 +202,10 @@ if user.id not in admin_ids: ## Notes - Topic guard runs at `group=-1` to intercept unauthorized messages BEFORE other handlers +- Topic guard handles both messages and edited messages, raises `ApplicationHandlerStop` to block downstream handlers - JobQueue auto-restriction job runs every 5 minutes (first run after 5 min delay) -- Bot uses `allowed_updates=["message", "callback_query", "chat_member"]` +- JobQueue admin refresh job runs every 10 minutes (first run after 10 min delay) +- Bot uses `allowed_updates=["message", "edited_message", "callback_query", "chat_member"]` - Captcha uses both `ChatMemberHandler` (for "Hide Join" groups) and `MessageHandler` fallback - Multi-group: handlers use `get_group_config_for_update()` instead of `settings.group_id` - Captcha callback data encodes group_id: `captcha_verify_{group_id}_{user_id}` to avoid ambiguity diff --git a/README.md b/README.md index 8e0ed41..7e52319 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ A comprehensive Telegram bot for managing group members with profile verificatio - Checks if users have a public profile picture - Checks if users have a username set - Sends warnings to a dedicated topic (thread) for non-compliant users -- **Warning topic protection**: Only admins and the bot can post in the warning topic +- **Warning topic protection**: Only admins and the bot can post in the warning topic (messages + edited messages) ### Restriction & Unrestriction - **Progressive restriction**: Optional mode to restrict users after multiple warnings (message-based) @@ -194,15 +194,14 @@ uv run pytest -v ### Test Coverage The project maintains comprehensive test coverage: -- **Coverage**: 99% (1,396 statements) -- **Tests**: 442 total -- **Pass Rate**: 100% (442/442 passed) -- **All modules**: 100% coverage including JobQueue scheduler integration, captcha verification, and anti-spam enforcement - - Services: `bot_info.py` (100%), `group_config.py` (97%), `scheduler.py` (100%), `user_checker.py` (100%), `telegram_utils.py` (100%), `captcha_recovery.py` (100%) - - Handlers: `anti_spam.py` (100%), `captcha.py` (100%), `check.py` (98%), `dm.py` (100%), `message.py` (100%), `topic_guard.py` (100%), `verify.py` (100%) - - Database: `service.py` (100%), `models.py` (100%) - - Config: `config.py` (100%) - - Constants: `constants.py` (100%) +- **Coverage**: 99.9% (1,570 statements, 1 unreachable line) +- **Tests**: 519 total +- **Pass Rate**: 100% (519/519 passed) +- **All modules at 100%** except one unreachable line in `anti_spam.py` + - Services: `bot_info.py`, `scheduler.py`, `user_checker.py`, `telegram_utils.py`, `captcha_recovery.py` — all 100% + - Handlers: `anti_spam.py` (99%), `captcha.py`, `check.py`, `dm.py`, `message.py`, `topic_guard.py`, `verify.py`, `duplicate_spam.py` — all 100% + - Database: `service.py`, `models.py` — all 100% + - Config: `config.py`, `group_config.py`, `constants.py` — all 100% All modules are fully unit tested with: - Mocked async dependencies (telegram bot API calls) @@ -227,16 +226,18 @@ PythonID/ │ ├── test_bot_info.py │ ├── test_captcha.py │ ├── test_captcha_recovery.py +│ ├── test_check.py │ ├── test_config.py │ ├── test_constants.py │ ├── test_database.py │ ├── test_dm_handler.py +│ ├── test_duplicate_spam.py +│ ├── test_group_config.py │ ├── test_message_handler.py │ ├── test_photo_verification.py │ ├── test_scheduler.py # JobQueue scheduler tests │ ├── test_telegram_utils.py │ ├── test_topic_guard.py -│ ├── test_group_config.py │ ├── test_user_checker.py │ ├── test_verify_handler.py │ └── test_whitelist.py @@ -441,14 +442,16 @@ flowchart TD The bot is organized into clear modules for maintainability: -- **main.py**: Entry point with python-telegram-bot's JobQueue integration and graceful shutdown -- **handlers/**: Message processing logic - - `message.py`: Monitors group messages and sends warnings/restrictions +- **main.py**: Entry point with python-telegram-bot's JobQueue integration, admin cache refresh, and graceful shutdown +- **handlers/**: Message processing logic (priority groups -1 through 4) + - `topic_guard.py`: Protects warning topic (group=-1, messages + edited messages, fail-closed) + - `message.py`: Monitors group messages and sends warnings/restrictions (group=4) - `dm.py`: Handles DM unrestriction flow - - `topic_guard.py`: Protects warning topic from unauthorized messages - `captcha.py`: Captcha verification for new members - - `anti_spam.py`: Anti-spam enforcement for users on probation + - `anti_spam.py`: Inline keyboard spam (group=1) + new user probation enforcement (group=2) + - `duplicate_spam.py`: Repeated message detection (group=3) - `verify.py`: /verify and /unverify command handlers + - `check.py`: /check command + forwarded message handling - **services/**: Business logic and utilities - `scheduler.py`: JobQueue background job that runs every 5 minutes for time-based auto-restrictions - `user_checker.py`: Profile validation (photo + username check) @@ -492,6 +495,9 @@ The bot runs a JobQueue background job every 5 minutes that: This ensures users cannot evade restrictions by simply not sending messages. +### Admin Cache Refresh +Admin IDs are fetched at startup and refreshed every 10 minutes via a JobQueue job. If the refresh fails for a group, the bot falls back to the previously cached data (never an empty list). Spam handlers use the cached admin IDs for fast lookups, while the topic guard uses live `get_chat_member` API calls for maximum accuracy. + ### Message Templates and Constants All warning and restriction messages are centralized in `constants.py` for consistency: - `WARNING_MESSAGE_NO_RESTRICTION`: Used in warning-only mode @@ -504,7 +510,9 @@ All messages are formatted with proper Indonesian language patterns and include ### Warning Topic Protection - Only group administrators and the bot itself can post in the warning topic -- Messages from regular users are automatically deleted +- Messages and edited messages from regular users are automatically deleted +- Uses `ApplicationHandlerStop` to prevent downstream handlers from processing warning-topic traffic +- **Fail-closed**: On API errors, messages in the warning topic are deleted (erring on the side of protection) ### DM Unrestriction Flow When a restricted user DMs the bot (or sends `/start`):