diff --git a/README.md b/README.md index a78abb6..e33b636 100644 --- a/README.md +++ b/README.md @@ -31,10 +31,9 @@ platforms such as GitHub discussions/issues might be added in the future. ### Discord * Setup an application at [discord developer portal](https://discord.com/developers/applications). -* On `Bot` page enabled these: - * Presence Intent - * Server Members Intent - * Copy the `Token` +* On the `Bot` page, copy the `Token`. +* Do not enable privileged gateway intents. The bot uses non-privileged message events and fetches individual server + members on demand. ### Reddit diff --git a/src/discord_bot/bot.py b/src/discord_bot/bot.py index 2e459e4..b893dce 100644 --- a/src/discord_bot/bot.py +++ b/src/discord_bot/bot.py @@ -22,15 +22,18 @@ class Bot(discord.Bot): Discord bot class. This class extends the discord.Bot class to include additional functionality. The class will automatically - enable all intents and sync commands on startup. The class will also update the bot presence, username, and avatar - when the bot is ready. + enable the default non-privileged intents and sync commands on startup. The class will also update the bot presence, + username, and avatar when the bot is ready. """ def __init__(self, *args, **kwargs): # tasks need to be imported here to avoid circular imports from src.discord_bot import tasks if 'intents' not in kwargs: - intents = discord.Intents.all() + intents = discord.Intents.default() + intents.members = False + intents.presences = False + intents.message_content = False kwargs['intents'] = intents if 'auto_sync_commands' not in kwargs: kwargs['auto_sync_commands'] = True diff --git a/src/discord_bot/cogs/autoban.py b/src/discord_bot/cogs/autoban.py index 9e0d539..a6a365f 100644 --- a/src/discord_bot/cogs/autoban.py +++ b/src/discord_bot/cogs/autoban.py @@ -52,20 +52,16 @@ async def on_message(self, message: discord.Message): reason="Automatic ban: posted in restricted channel.", delete_message_seconds=604800, # Delete messages from the past 7 days ) - # repr() sanitizes the content, escaping newlines and other special characters - # to prevent log injection attacks. - safe_content = repr(message.content) - attachment_urls = [a.url for a in message.attachments] logger.warning( "Auto-banned user %s (%s) for posting in channel %s (%s). " - "Message content: %s. " - "Attachments: %s.", + "Message ID: %s. " + "Message created at: %s.", message.author, message.author.id, message.channel.name, message.channel.id, - safe_content, - attachment_urls, + message.id, + message.created_at.isoformat(), ) except discord.Forbidden: logger.error( diff --git a/src/discord_bot/tasks.py b/src/discord_bot/tasks.py index e20efb4..ac66d40 100644 --- a/src/discord_bot/tasks.py +++ b/src/discord_bot/tasks.py @@ -2,6 +2,7 @@ import asyncio import copy from datetime import datetime, UTC +import logging # lib imports import discord @@ -12,6 +13,9 @@ from src.discord_bot.bot import Bot +logger = logging.getLogger(__name__) + + @tasks.loop(seconds=30) async def clean_ephemeral_cache(bot: Bot) -> bool: """ @@ -84,10 +88,13 @@ async def _process_discord_user_roles( github_sponsors: dict, test_mode: bool, ): - user_id = user_data.get('discord_id') + user_id = user_data.get('user_id') or user_data.get('discord_id') if not user_id: return + # Normalize records created by the legacy schema. + user_data['user_id'] = int(user_id) + # Revocable roles were added by this bot and can be removed if no longer applicable. revocable_roles = user_data.get('roles', []).copy() _update_sponsor_role_data(user_data=user_data, github_sponsors=github_sponsors) @@ -151,7 +158,12 @@ async def _sync_guild_roles( revocable_roles: list[str], test_mode: bool, ): - member = guild.get_member(user_id) + try: + member = await guild.fetch_member(user_id) + except discord.HTTPException as error: + logger.warning("Unable to fetch Discord user %s from guild %s: %s", user_id, guild.id, error) + return + if not member: return @@ -208,4 +220,5 @@ async def _run_role_action(bot: Bot, test_mode: bool, action, role: discord.Role def _update_discord_user(bot: Bot, user_data: dict): with bot.db as db: users_table = db.table('discord_users') - users_table.update(user_data, doc_ids=[user_data.get('doc_id')]) + doc_id = getattr(user_data, 'doc_id', None) or user_data.get('doc_id') + users_table.update(user_data, doc_ids=[doc_id]) diff --git a/tests/unit/discord/test_autoban.py b/tests/unit/discord/test_autoban.py new file mode 100644 index 0000000..c4a15ba --- /dev/null +++ b/tests/unit/discord/test_autoban.py @@ -0,0 +1,38 @@ +# standard imports +from datetime import datetime, UTC +import logging +from types import SimpleNamespace + +# lib imports +import pytest + +# local imports +from src.discord_bot.cogs.autoban import AutoBanCog + + +@pytest.mark.asyncio +async def test_autoban_does_not_read_message_content(caplog, mocker, monkeypatch): + monkeypatch.setenv('DISCORD_AUTOBAN_CHANNEL_ID', '123') + guild = SimpleNamespace( + ban=mocker.AsyncMock(), + id=456, + name='Test guild', + ) + message = SimpleNamespace( + author=SimpleNamespace(bot=False, id=789), + channel=SimpleNamespace(id=123, name='restricted'), + created_at=datetime(2026, 9, 13, tzinfo=UTC), + guild=guild, + id=101112, + ) + + with caplog.at_level(logging.WARNING): + await AutoBanCog(bot=mocker.Mock()).on_message(message) + + guild.ban.assert_awaited_once_with( + user=message.author, + reason="Automatic ban: posted in restricted channel.", + delete_message_seconds=604800, + ) + assert "Message ID: 101112" in caplog.text + assert "Message created at: 2026-09-13T00:00:00+00:00" in caplog.text diff --git a/tests/unit/discord/test_discord_bot.py b/tests/unit/discord/test_discord_bot.py index cb53388..502f42a 100644 --- a/tests/unit/discord/test_discord_bot.py +++ b/tests/unit/discord/test_discord_bot.py @@ -12,6 +12,9 @@ def test_bot_on_ready(discord_bot): assert discord_bot is not None + assert discord_bot.intents.members is False + assert discord_bot.intents.presences is False + assert discord_bot.intents.message_content is False assert discord_bot.guilds assert discord_bot.guilds[0].name == "ReenigneArcher's test server" assert discord_bot.user.id == 939171917578002502 diff --git a/tests/unit/discord/test_tasks.py b/tests/unit/discord/test_tasks.py index 49ee51b..32112fd 100644 --- a/tests/unit/discord/test_tasks.py +++ b/tests/unit/discord/test_tasks.py @@ -4,6 +4,7 @@ from types import SimpleNamespace # lib imports +import discord import pytest # local imports @@ -86,6 +87,11 @@ async def test_role_update_task(discord_bot, discord_db_users, mocker, skip): # Patch datetime.datetime at the location where it's imported in `tasks` mock_datetime = mocker.patch('src.discord_bot.tasks.datetime', autospec=True) mock_datetime.now.return_value = datetime(2023, 1, 1, 0, 1 if skip else 0, 0, tzinfo=timezone.utc) + mocker.patch( + 'src.discord_bot.tasks.sponsors.get_github_sponsors', + return_value=github_sponsors_payload(), + ) + sync_guild_roles = mocker.patch('src.discord_bot.tasks._sync_guild_roles', new_callable=mocker.AsyncMock) # Run the task result = await tasks.role_update_task(bot=discord_bot, test_mode=True) @@ -94,6 +100,7 @@ async def test_role_update_task(discord_bot, discord_db_users, mocker, skip): # Verify that datetime.now() was called mock_datetime.now.assert_called_once() + assert sync_guild_roles.await_count == (0 if skip else len(discord_bot.guilds)) def github_sponsors_payload(monthly_amount=25, login='test_user'): @@ -170,7 +177,7 @@ def test_role_map(mocker): async def test_sync_guild_roles_skips_missing_member_or_role(mocker): bot = SimpleNamespace(loop=mocker.Mock()) guild = SimpleNamespace( - get_member=mocker.Mock(return_value=None), + fetch_member=mocker.AsyncMock(return_value=None), roles=[], ) @@ -183,11 +190,11 @@ async def test_sync_guild_roles_skips_missing_member_or_role(mocker): test_mode=False, ) - guild.get_member.assert_called_once_with(123) + guild.fetch_member.assert_awaited_once_with(123) member = SimpleNamespace(add_roles=mocker.AsyncMock(), remove_roles=mocker.AsyncMock()) role = SimpleNamespace(name='github-user') - guild.get_member.return_value = member + guild.fetch_member.return_value = member guild.roles = [role] await tasks._sync_guild_roles( @@ -203,6 +210,29 @@ async def test_sync_guild_roles_skips_missing_member_or_role(mocker): member.remove_roles.assert_not_called() +@pytest.mark.asyncio +async def test_sync_guild_roles_handles_member_fetch_failure(caplog, mocker): + response = SimpleNamespace(status=404, reason='Not Found') + guild = SimpleNamespace( + fetch_member=mocker.AsyncMock( + side_effect=discord.NotFound(response=response, message='Unknown Member'), + ), + id=456, + roles=[], + ) + + await tasks._sync_guild_roles( + bot=SimpleNamespace(loop=mocker.Mock()), + guild=guild, + user_id=123, + user_roles=['github-user'], + revocable_roles=[], + test_mode=False, + ) + + assert "Unable to fetch Discord user 123 from guild 456" in caplog.text + + @pytest.mark.asyncio async def test_sync_member_role_adds_and_removes(mocker): bot = SimpleNamespace(loop=mocker.Mock()) @@ -249,19 +279,24 @@ async def test_run_role_action_test_mode(mocker): @pytest.mark.asyncio -async def test_process_discord_user_roles(mocker): +@pytest.mark.parametrize(('id_field', 'id_value'), [ + ('user_id', 123), + ('discord_id', '123'), +]) +async def test_process_discord_user_roles(mocker, id_field, id_value): role = SimpleNamespace(name='github-user') member = SimpleNamespace(add_roles=mocker.AsyncMock(), remove_roles=mocker.AsyncMock()) guild = SimpleNamespace( - get_member=mocker.Mock(return_value=member), + fetch_member=mocker.AsyncMock(return_value=member), roles=[role], + id=456, ) users_table = SimpleNamespace(update=mocker.Mock()) db_context = mocker.MagicMock() db_context.__enter__.return_value.table.return_value = users_table bot = SimpleNamespace(db=db_context, guilds=[guild], loop=mocker.Mock()) user_data = { - 'discord_id': '123', + id_field: id_value, 'github_username': 'test_user', 'roles': [], 'doc_id': 5, @@ -275,6 +310,7 @@ async def test_process_discord_user_roles(mocker): ) member.add_roles.assert_awaited_once_with(role) + assert user_data['user_id'] == 123 users_table.update.assert_called_once_with(user_data, doc_ids=[5])