From 8a7a85174a13e5e39416b79988bc0466fe1db93a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 11:13:55 +0200 Subject: [PATCH 01/12] Add a pytest test suite and GitHub Actions CI Covers every comandos/*.py cog plus utils.py and configuration.py (103 tests), using lightweight fakes for discord.py objects instead of a full test harness like dpytest: - conftest.py (root): sandboxes a throwaway config.toml + logs/ dir and chdir()s into it before any comandos module is imported, since every such module instantiates the Config() singleton at import time and config.toml is git-ignored (real secrets). - tests/factories.py: fakes for members/roles/channels/messages/ attachments/bots. Uses MagicMock(spec=...) specifically where production code runs isinstance() checks (discord.Member, discord.TextChannel, discord.Interaction), plain objects elsewhere. - tests/conftest.py: fixtures wiring FloodSpam/Moderacion cogs the way on_ready() would, without needing a real discord.Client/Guild. - .github/workflows/tests.yml: runs the suite on push to main and on every pull request. Along the way this surfaced a few pre-existing bugs, documented as tests/comments rather than fixed here (out of scope for "add tests"): - archivar.archivar_canal() returns (False, None) on a write failure; since a non-empty tuple is always truthy, archivar()'s `if status:` treats that as success (test_failure_tuple_is_still_truthy). - FloodSpam.on_message re-fetches the channel via bot.get_channel(message.channel.id) instead of using message.channel directly; a cache miss makes self._msg_channel None and crashes the first .send() call downstream. - Moderacion._aceptar_mensaje's jump_url is built from self._msg_id, a single field shared across all pending submissions, rather than the specific post being accepted (vp.post_id) - a second submission arriving before the first is moderated could produce a jump_url pointing at the wrong message. --- .github/workflows/tests.yml | 28 +++ .gitignore | 1 + conftest.py | 120 +++++++++ pytest.ini | 4 + requirements-dev.txt | 4 + tests/__init__.py | 0 tests/conftest.py | 59 +++++ tests/factories.py | 204 ++++++++++++++++ tests/test_archivar.py | 116 +++++++++ tests/test_ayuda.py | 61 +++++ tests/test_configuration.py | 61 +++++ tests/test_enviar.py | 32 +++ tests/test_flood.py | 469 ++++++++++++++++++++++++++++++++++++ tests/test_limpia.py | 50 ++++ tests/test_moderacion.py | 320 ++++++++++++++++++++++++ tests/test_ping.py | 63 +++++ tests/test_utils.py | 51 ++++ utils.py | 4 +- 18 files changed, 1645 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/tests.yml create mode 100644 conftest.py create mode 100644 pytest.ini create mode 100644 requirements-dev.txt create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/factories.py create mode 100644 tests/test_archivar.py create mode 100644 tests/test_ayuda.py create mode 100644 tests/test_configuration.py create mode 100644 tests/test_enviar.py create mode 100644 tests/test_flood.py create mode 100644 tests/test_limpia.py create mode 100644 tests/test_moderacion.py create mode 100644 tests/test_ping.py create mode 100644 tests/test_utils.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..5027080 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,28 @@ +name: Tests + +on: + push: + branches: [main] + pull_request: + +jobs: + pytest: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.11"] + + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: pip + + - name: Install dependencies + run: pip install -r requirements-dev.txt + + - name: Run tests + run: pytest -v diff --git a/.gitignore b/.gitignore index c857a09..7daa5fe 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ config.toml __pycache__ bot.log +.pytest_cache diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..2fc48a8 --- /dev/null +++ b/conftest.py @@ -0,0 +1,120 @@ +"""Root conftest.py. + +Every module under ``comandos/`` (and ``configuration.py`` itself) calls +``Config()`` at *import time*, and ``Config.__init__`` reads ``config.toml`` +from the current working directory, exiting the process if it's missing or +incomplete. ``config.toml`` is git-ignored (it holds real bot secrets), so it +won't exist in a fresh checkout or in CI. + +To make the test modules importable at all, we build a throwaway +``config.toml`` plus the ``logs/`` directory it expects, and ``chdir`` into +that sandbox *before* pytest imports any test module. This has to happen as +plain module-level code (not inside a fixture) because pytest imports +conftest.py files before it collects/imports the test modules that in turn +``import comandos.flood`` etc. +""" +import os +import shutil +import tempfile +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent + +_SANDBOX = Path(tempfile.mkdtemp(prefix="pyes-bot-tests-")) +(_SANDBOX / "logs").mkdir() +(_SANDBOX / "config.toml").write_text( + """ +[bot] +token = "test-token" +id = 111111111111111111 +log_file = "logs/bot_log.csv" + +[moderation] +log_file = "logs/mod_log.csv" +channel_id = 222222222222222222 +role = "Coordinacion" +muted_role = "Muted" + +[server] +guild = 333333333333333333 + +[channels] + [channels.eventos] + main = 444444444444444444 + moderation = 555555555555555555 + submission = 666666666666666666 +""" +) + +# A couple of commands read files via relative paths at call time (not +# import time) - e.g. ping.py's "resources/llama.gif". Make sure those are +# still reachable from the sandboxed working directory. +if (_REPO_ROOT / "resources").is_dir(): + shutil.copytree(_REPO_ROOT / "resources", _SANDBOX / "resources") + +os.chdir(_SANDBOX) + + +@pytest.fixture(scope="session") +def sandbox_dir(): + """The temporary directory tests are running from.""" + return _SANDBOX + + +@pytest.fixture(scope="session") +def repo_root(): + """The actual repository root, for tests that need real project files.""" + return _REPO_ROOT + + +@pytest.fixture +def config(): + """The (singleton) Config instance, backed by the sandboxed config.toml.""" + from configuration import Config + + return Config() + + +@pytest.fixture +def isolated_logs(config, tmp_path, monkeypatch): + """Point every log file Config knows about at a fresh, empty file. + + Config is a singleton shared across the whole test session, and several + of its log paths are appended to by the code under test (e.g. + ``add_spam_message``). Without this, one test's writes would leak into + the next test that also touches those files. + """ + logs_dir = tmp_path / "logs" + logs_dir.mkdir() + + for attr, filename in [ + ("log_spam_file", "spam_log.csv"), + ("log_image_spam_file", "image_spam_log.csv"), + ("log_mod_file", "mod_log.csv"), + ("log_accepted_file", "mod_log_accepted.csv"), + ("log_rejected_file", "mod_log_rejected.csv"), + ("log_main_file", "main_log.csv"), + ("log_file", "bot_log.csv"), + ]: + path = logs_dir / filename + path.write_text("\n") + monkeypatch.setattr(config, attr, path) + + return config + + +@pytest.fixture(autouse=True) +def patched_message_delete(monkeypatch): + """The production code deletes messages via ``discord.Message.delete(message)`` + (an unbound call on the real class) rather than ``message.delete()``, so + fakes that aren't real ``discord.Message`` instances need this patched to + avoid hitting real discord.py internals/HTTP calls. + """ + import discord + from unittest.mock import AsyncMock + + mock_delete = AsyncMock() + monkeypatch.setattr(discord.Message, "delete", mock_delete) + return mock_delete diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..01d0c51 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,4 @@ +[pytest] +asyncio_mode = auto +filterwarnings = + ignore:'asyncio.iscoroutinefunction' is deprecated:DeprecationWarning:discord.ext.commands.core diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..83b2c4b --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,4 @@ +-r requirements.txt + +pytest +pytest-asyncio diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..8b07dba --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,59 @@ +import pandas as pd +import pytest + +from comandos.flood import FloodSpam +from comandos.moderacion import Moderacion +from tests.factories import make_bot, make_role, make_text_channel + + +@pytest.fixture +def coord_role(): + return make_role(name="Coordinacion", id=1) + + +@pytest.fixture +def muted_role(): + return make_role(name="Muted", id=2) + + +@pytest.fixture +def mod_channel(): + return make_text_channel(id=999, name="mod-general") + + +@pytest.fixture +def flood_cog(isolated_logs, coord_role, muted_role, mod_channel): + """A FloodSpam cog wired up the way on_ready() would, without needing a + real discord.Client/Guild.""" + bot = make_bot(channels={mod_channel.id: mod_channel}) + cog = FloodSpam(bot) + cog._coord_role = coord_role + cog._muted_role = muted_role + cog._main_mod_channel = mod_channel + return cog + + +@pytest.fixture +def moderacion_channels(config): + ids = config.CHANNELS["eventos"] + return { + "main": make_text_channel(id=ids["main"], name="eventos"), + "mod": make_text_channel(id=ids["moderation"], name="eventos-mod"), + "sub": make_text_channel(id=ids["submission"], name="envio-eventos"), + } + + +@pytest.fixture +async def moderacion_cog(isolated_logs, moderacion_channels): + """A Moderacion cog wired up the way on_ready() would (channel mapping + populated from config.CHANNELS), with an empty ``data_mod`` table.""" + from types import SimpleNamespace + + channels = {c.id: c for c in moderacion_channels.values()} + bot = make_bot(channels=channels, guild=SimpleNamespace(id=333333333333333333)) + bot.data_mod = pd.DataFrame( + columns=["date", "message_id", "channel", "author_id", "author", "message"] + ) + cog = Moderacion(bot) + await cog.on_ready() + return cog diff --git a/tests/factories.py b/tests/factories.py new file mode 100644 index 0000000..4b08e0f --- /dev/null +++ b/tests/factories.py @@ -0,0 +1,204 @@ +"""Lightweight fakes for discord.py objects used across the test suite. + +We deliberately avoid a full discord.py test harness (e.g. dpytest) and +instead build just-enough fakes: + +- Plain objects (``SimpleNamespace``/small classes) for anything the + production code only reads attributes off of or calls async methods on. +- ``unittest.mock.MagicMock(spec=...)`` for anything the code runs + ``isinstance()`` checks against - Mock's ``spec=`` makes + ``isinstance(mock, SpecClass)`` return True, which a plain fake can't do. +""" +from io import BytesIO +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import discord +from PIL import Image + +from utils import strip_message + + +def make_role(name="Coordinacion", id=1000): + return SimpleNamespace(name=name, id=id, mention=f"@{name}") + + +def make_member(name="usuario", id=123, roles=None, bot=False): + """A fake author. Uses ``spec=discord.Member`` so that the + ``isinstance(author, discord.Member)`` check in ``spam_check`` passes.""" + member = MagicMock(spec=discord.Member) + member.name = name + member.id = id + member.bot = bot + member.discriminator = "0" + member.mention = f"<@{id}>" + member.roles = roles if roles is not None else [] + member.add_roles = AsyncMock() + member.remove_roles = AsyncMock() + member.__str__.return_value = name + return member + + +def make_attachment(content_type="image/png", data=b"fake-image-bytes", filename="image.png"): + return SimpleNamespace( + content_type=content_type, + filename=filename, + read=AsyncMock(return_value=data), + ) + + +def make_thread(): + return SimpleNamespace(send=AsyncMock()) + + +async def _async_history(messages): + for m in messages: + yield m + + +def make_text_channel(id=555, name="general", thread=None, history_messages=None): + """Uses ``spec=discord.TextChannel`` for the isinstance checks in + ``archivar.py``/``limpia.py``.""" + channel = MagicMock(spec=discord.TextChannel) + channel.id = id + channel.name = name + channel.mention = f"#{name}" + channel.category_id = None + channel.send = AsyncMock() + channel.create_thread = AsyncMock(return_value=thread or make_thread()) + channel.purge = AsyncMock() + channel.delete_messages = AsyncMock() + channel.history = MagicMock( + side_effect=lambda *a, **kw: _async_history(history_messages or []) + ) + return channel + + +def make_dm_channel(): + """A channel type NOT accepted by archivar.py/limpia.py's isinstance checks.""" + return MagicMock(spec=discord.DMChannel) + + +def make_category(name="categoria", channels=None): + category = MagicMock(spec=discord.CategoryChannel) + category.name = name + category.channels = channels or [] + return category + + +def make_message( + content="", + author=None, + channel=None, + attachments=None, + mentions=None, + role_mentions=None, + id=999, +): + return SimpleNamespace( + id=id, + content=content, + author=author if author is not None else make_member(), + channel=channel if channel is not None else make_text_channel(), + attachments=attachments if attachments is not None else [], + mentions=mentions if mentions is not None else [], + role_mentions=role_mentions if role_mentions is not None else [], + ) + + +def make_bot(channels=None, guild=None, user=None, users=None, guilds=None): + """``channels``/``users`` are kept as live dicts on the returned bot (as + ``bot.channels``/``bot.users_by_id``) so tests can register more entries + later, e.g. when a message arrives on a channel ``on_message`` looks up + via ``bot.get_channel(message.channel.id)``.""" + channels = dict(channels or {}) + users = dict(users or {}) + bot = SimpleNamespace( + channels=channels, + users_by_id=users, + get_guild=lambda gid: guild, + user=user, + guilds=guilds if guilds is not None else ([guild] if guild else []), + process_commands=AsyncMock(), + ) + bot.get_channel = lambda cid: bot.channels.get(cid) + bot.get_user = lambda uid: bot.users_by_id.get(uid) + return bot + + +def make_ctx(author=None, channel=None, content="", reference=None): + """A fake ``commands.Context`` - text-command style invocation (as + opposed to a slash-command ``discord.Interaction``).""" + channel = channel if channel is not None else make_text_channel() + message = SimpleNamespace( + content=content, + channel=channel, + reference=reference, + delete=AsyncMock(), + ) + return SimpleNamespace( + author=author if author is not None else make_member(), + channel=channel, + message=message, + send=AsyncMock(), + defer=AsyncMock(), + ) + + +def make_interaction(user=None, channel=None): + """Uses ``spec=discord.Interaction`` for the ``isinstance()`` checks + Moderacion uses to tell slash-command interactions apart from regular + text-command invocations.""" + interaction = MagicMock(spec=discord.Interaction) + interaction.user = user if user is not None else make_member() + interaction.channel = channel if channel is not None else make_text_channel() + # `_is_valid_channel` compares `channel_mod.id == ctx.message.channel.id` + # even for interactions, so this needs to line up with `.channel` too. + interaction.message = SimpleNamespace(channel=interaction.channel) + interaction.response = MagicMock() + interaction.response.send_message = AsyncMock() + interaction.response.send_modal = AsyncMock() + return interaction + + +def encode_for_mod_row(text: str) -> str: + """Matches ``Moderacion.on_message``'s encoding of a message's content + into the ``data_mod``/log-file ``message`` column: a base64-encoded + ``bytes`` object rendered through an f-string (so later ``eval()``'d + back into a real ``bytes`` object by the accept/reject/list commands).""" + import base64 + + return f"{base64.b64encode(text.encode('utf-8'))}" + + +def make_png_bytes(color=(255, 0, 0), size=(8, 8)): + """A tiny, valid PNG - for tests that hash/sanitize real image bytes.""" + buf = BytesIO() + Image.new("RGB", size, color=color).save(buf, format="PNG") + return buf.getvalue() + + +def bind_commands(cog): + """Set ``command.cog`` for every ``@commands.command``/``hybrid_command`` + defined on this cog, the way ``bot.add_cog()`` would. + + Our tests instantiate cogs directly without registering them on a real + ``discord.ext.commands.Bot``, but discord.py's ``Command.__call__`` only + binds ``self`` (the cog instance) to the callback when ``command.cog`` + is set - otherwise calling e.g. ``cog.some_command(ctx)`` directly (or a + command calling a sibling command the same way) raises a confusing + "missing 1 required positional argument: 'ctx'". + """ + for command in cog.get_commands(): + command.cog = cog + return cog + + +def prime_cog(cog, message): + """Mirror the attribute setup ``FloodSpam.on_message`` does before + delegating to its individual ``*_check`` methods, so those methods can + be unit-tested directly without going through the full listener.""" + cog._msg_channel = message.channel + cog._msg_content = strip_message(message.content) + cog._msg_author = message.author + cog._msg_author_mention = message.author.mention diff --git a/tests/test_archivar.py b/tests/test_archivar.py new file mode 100644 index 0000000..bfaab1b --- /dev/null +++ b/tests/test_archivar.py @@ -0,0 +1,116 @@ +from unittest.mock import MagicMock + +import discord + +from comandos.archivar import Archivar +from tests.factories import ( + bind_commands, + make_bot, + make_category, + make_ctx, + make_dm_channel, + make_message, + make_text_channel, +) + + +class TestArchivarCanal: + def test_writes_header_and_rows(self, tmp_path): + cog = Archivar(make_bot()) + channel = make_text_channel(id=1, name="general") + messages = [ + make_message(id=10, content="hola\ncon salto de linea", channel=channel), + make_message(id=11, content="segundo mensaje", channel=channel), + ] + target = tmp_path / "archivo.csv" + + status = cog.archivar_canal(str(target), messages) + + assert status == (True, str(target)) + lines = target.read_text().splitlines() + assert lines[0].startswith("id;content;channel_id") + assert len(lines) == 3 + # Newlines inside content are escaped, not left as real line breaks. + assert "hola\\ncon salto de linea" in lines[1] + + def test_stops_and_returns_none_for_unsupported_channel_type(self, tmp_path): + cog = Archivar(make_bot()) + messages = [make_message(id=1, channel=make_dm_channel())] + target = tmp_path / "archivo.csv" + + assert cog.archivar_canal(str(target), messages) is None + + def test_write_failure_returns_false_none(self, tmp_path): + cog = Archivar(make_bot()) + messages = [make_message(id=1)] + # A directory can't be opened for writing as a file. + bad_target = tmp_path + + assert cog.archivar_canal(str(bad_target), messages) == (False, None) + + def test_failure_tuple_is_still_truthy(self, tmp_path): + """Documents a real footgun in archivar(): ``status = archivar_canal(...)`` + followed by ``if status:`` treats a write failure as success, because + ``(False, None)`` is a non-empty tuple and therefore truthy in Python. + """ + cog = Archivar(make_bot()) + status = cog.archivar_canal(str(tmp_path), [make_message(id=1)]) + + assert status == (False, None) + assert bool(status) is True # the actual footgun + + +class TestArchivarCommand: + async def test_sends_success_embed_with_the_file(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + mod_channel = make_text_channel(id=1, name="mod") + cog = bind_commands(Archivar(make_bot())) + cog.mod_channel = mod_channel + + channel = make_text_channel( + id=2, + name="general", + history_messages=[make_message(id=1), make_message(id=2)], + ) + ctx = make_ctx(channel=mod_channel) + + await cog.archivar(ctx, channel=channel) + + mod_channel.send.assert_awaited_once() + _, kwargs = mod_channel.send.call_args + assert "2 mensajes" in kwargs["embed"].description + + async def test_sends_error_embed_when_archiving_fails(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + mod_channel = make_text_channel(id=1, name="mod") + cog = bind_commands(Archivar(make_bot())) + cog.mod_channel = mod_channel + + channel = make_text_channel( + id=2, name="general", history_messages=[make_message(id=1, channel=make_dm_channel())] + ) + ctx = make_ctx(channel=mod_channel) + + await cog.archivar(ctx, channel=channel) + + mod_channel.send.assert_awaited_once() + (msg,), _ = mod_channel.send.call_args + assert "Error" in msg + + +class TestArchivarCategoria: + async def test_only_archives_text_channels(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + mod_channel = make_text_channel(id=1, name="mod") + cog = bind_commands(Archivar(make_bot())) + cog.mod_channel = mod_channel + + text_channel = make_text_channel(id=2, name="canal-texto", history_messages=[]) + voice_channel = MagicMock(spec=discord.VoiceChannel) + category = make_category(channels=[text_channel, voice_channel]) + ctx = make_ctx(channel=mod_channel) + + await cog.archivar_categoria(ctx, category=category) + + # Only one embed sent - for the TextChannel. The VoiceChannel was skipped. + mod_channel.send.assert_awaited_once() diff --git a/tests/test_ayuda.py b/tests/test_ayuda.py new file mode 100644 index 0000000..a81fedd --- /dev/null +++ b/tests/test_ayuda.py @@ -0,0 +1,61 @@ +from comandos.ayuda import Ayuda +from tests.factories import bind_commands, make_bot, make_ctx, make_member, make_text_channel + + +class TestGetModHelp: + def test_lists_moderation_commands(self): + cog = Ayuda(make_bot()) + + embed = cog.get_mod_help() + + names = [f.name for f in embed.fields] + assert "`%mod`" in names + assert "`%aceptar ID`" in names + assert "`%rechazar ID RAZON`" in names + + +class TestGetMainHelp: + def test_lists_encuesta_usage(self): + cog = Ayuda(make_bot()) + + embed = cog.get_main_help() + + assert any("encuesta" in f.name for f in embed.fields) + + +class TestMensajeAyuda: + async def test_ignores_the_bot_itself(self, config): + cog = bind_commands(Ayuda(make_bot())) + ctx = make_ctx(author=make_member(id=config.BOT_ID)) + + await cog.mensaje_ayuda(ctx) + + ctx.channel.send.assert_not_awaited() + + async def test_sends_mod_help_inside_a_moderation_channel(self): + mod_channel = make_text_channel(id=1, name="mod") + bot = make_bot(channels={mod_channel.id: mod_channel}) + cog = bind_commands(Ayuda(bot)) + ctx = make_ctx(channel=mod_channel) + + await cog.mensaje_ayuda(ctx) + + mod_channel.send.assert_awaited_once() + _, kwargs = mod_channel.send.call_args + assert kwargs["embed"].title == "Comandos Disponibles" + names = [f.name for f in kwargs["embed"].fields] + assert "`%mod`" in names + + async def test_sends_main_help_outside_a_moderation_channel(self): + # bot.get_channel(ctx.channel.id) returns None - not a known channel. + bot = make_bot(channels={}) + cog = bind_commands(Ayuda(bot)) + channel = make_text_channel(id=99, name="general") + ctx = make_ctx(channel=channel) + + await cog.mensaje_ayuda(ctx) + + channel.send.assert_awaited_once() + _, kwargs = channel.send.call_args + names = [f.name for f in kwargs["embed"].fields] + assert any("encuesta" in name for name in names) diff --git a/tests/test_configuration.py b/tests/test_configuration.py new file mode 100644 index 0000000..9390be8 --- /dev/null +++ b/tests/test_configuration.py @@ -0,0 +1,61 @@ +from configuration import Config + + +class TestSingleton: + def test_config_is_a_singleton(self, config): + assert Config() is config + + def test_reflects_sandboxed_toml(self, config): + assert config.MOD_ROLE == "Coordinacion" + assert config.MUTED_ROLE == "Muted" + assert config.BOT_ID == 111111111111111111 + assert "eventos" in config.CHANNELS + + def test_hardcoded_limits(self, config): + # Not read from config.toml - always set in __init__. + assert config.FLOOD_LIMIT == 3 + assert config.MENTIONS_LIMIT == 3 + assert config.IMAGE_ATTACHMENT_LIMIT == 2 + assert config.IMAGE_BURST_WINDOW == 60 * 5 + + +class TestCheckCreateFile: + def test_creates_missing_file_with_header(self, config, tmp_path): + target = tmp_path / "new_log.csv" + assert not target.exists() + + config.check_create_file(target, "a;b;c\n") + + assert target.read_text() == "a;b;c\n" + + def test_does_not_overwrite_existing_file(self, config, tmp_path): + target = tmp_path / "existing_log.csv" + target.write_text("already;here\nfoo;bar\n") + + config.check_create_file(target, "a;b;c\n") + + assert target.read_text() == "already;here\nfoo;bar\n" + + +class TestGetSpamMessages: + def test_reads_lines_as_a_set(self, config, isolated_logs): + isolated_logs.log_spam_file.write_text("mensaje uno\nmensaje dos\n") + + assert config.get_spam_messages() == {"mensaje uno", "mensaje dos"} + + def test_empty_file_returns_empty_set(self, config, isolated_logs): + isolated_logs.log_spam_file.write_text("") + + assert config.get_spam_messages() == set() + + +class TestGetSpamImageHashes: + def test_reads_hashes_as_a_set(self, config, isolated_logs): + isolated_logs.log_image_spam_file.write_text("abc123\ndef456\n") + + assert config.get_spam_image_hashes() == {"abc123", "def456"} + + def test_skips_blank_lines(self, config, isolated_logs): + isolated_logs.log_image_spam_file.write_text("abc123\n\n\ndef456\n") + + assert config.get_spam_image_hashes() == {"abc123", "def456"} diff --git a/tests/test_enviar.py b/tests/test_enviar.py new file mode 100644 index 0000000..4cceca8 --- /dev/null +++ b/tests/test_enviar.py @@ -0,0 +1,32 @@ +from unittest.mock import AsyncMock + +from comandos.enviar import Enviar +from tests.factories import bind_commands, make_bot, make_ctx, make_text_channel + + +class TestEnviar: + async def test_replies_and_sends_to_target_channel(self): + cog = bind_commands(Enviar(make_bot())) + target = make_text_channel(id=1, name="anuncios") + ctx = make_ctx() + ctx.reply = AsyncMock() + + await cog.enviar(ctx, channel=target, message="hola a todos") + + ctx.reply.assert_awaited_once() + target.send.assert_awaited_once() + _, kwargs = target.send.call_args + assert kwargs["embed"].description == "hola a todos" + + async def test_falls_back_to_channel_send_when_reply_unavailable(self): + cog = bind_commands(Enviar(make_bot())) + target = make_text_channel(id=1, name="anuncios") + # A plain SimpleNamespace ctx has no ``.reply`` attribute at all, + # which is exactly the AttributeError the production code falls + # back on (e.g. a hybrid command invoked in a context without it). + ctx = make_ctx() + + await cog.enviar(ctx, channel=target, message="hola a todos") + + ctx.channel.send.assert_awaited_once() + target.send.assert_awaited_once() diff --git a/tests/test_flood.py b/tests/test_flood.py new file mode 100644 index 0000000..e8ff73c --- /dev/null +++ b/tests/test_flood.py @@ -0,0 +1,469 @@ +from unittest.mock import AsyncMock + +import discord +import pytest + +from tests.factories import ( + make_attachment, + make_member, + make_message, + make_png_bytes, + make_text_channel, + prime_cog, +) + + +# --------------------------------------------------------------------------- +# spam_check +# --------------------------------------------------------------------------- +class TestSpamCheck: + async def test_ignores_non_member_author(self, flood_cog): + message = make_message(content="discord nitro free http://evil") + message.author = object() # not a discord.Member + + assert await flood_cog.spam_check(message) is None + + async def test_no_match_returns_false(self, flood_cog): + message = make_message(content="hola a todos, buen dia") + + assert await flood_cog.spam_check(message) is False + + @pytest.mark.parametrize( + "content", + [ + "gana discord nitro free aqui http://scam.example", + "everyone free steam gift http://scam.example", + ], + ) + async def test_match_mutes_and_notifies(self, flood_cog, content): + member = make_member(name="victima") + message = make_message(content=content, author=member) + prime_cog(flood_cog, message) + + result = await flood_cog.spam_check(message) + + assert result is True + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + message.channel.send.assert_awaited_once() + _, kwargs = message.channel.send.call_args + assert kwargs["embed"].title.endswith("Alerta de posible SCAM") + + +# --------------------------------------------------------------------------- +# flood_check +# --------------------------------------------------------------------------- +class TestFloodCheck: + async def test_empty_content_is_a_noop(self, flood_cog): + message = make_message(content="") + prime_cog(flood_cog, message) + + assert await flood_cog.flood_check(message) is False + assert flood_cog.messages.normal == {} + + async def test_below_flood_limit_does_not_mute(self, flood_cog, config): + member = make_member(name="repetidor") + message = make_message(content="hola hola hola", author=member) + prime_cog(flood_cog, message) + + for _ in range(config.FLOOD_LIMIT - 1): + await flood_cog.flood_check(message) + + member.add_roles.assert_not_awaited() + + async def test_reaching_flood_limit_mutes_and_caches(self, flood_cog, config): + member = make_member(name="repetidor") + message = make_message(content="hola hola hola", author=member) + prime_cog(flood_cog, message) + + for _ in range(config.FLOOD_LIMIT): + await flood_cog.flood_check(message) + + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + assert "hola hola hola" in flood_cog.messages.spam + # Counter resets after muting + assert flood_cog.messages.normal[member] == {} + + async def test_different_authors_counted_separately(self, flood_cog, config): + alice = make_member(name="alice", id=1) + bob = make_member(name="bob", id=2) + + for _ in range(config.FLOOD_LIMIT - 1): + msg = make_message(content="mismo mensaje", author=alice) + prime_cog(flood_cog, msg) + await flood_cog.flood_check(msg) + + msg = make_message(content="mismo mensaje", author=bob) + prime_cog(flood_cog, msg) + await flood_cog.flood_check(msg) + + alice.add_roles.assert_not_awaited() + bob.add_roles.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# mention_check +# --------------------------------------------------------------------------- +class TestMentionCheck: + async def test_below_limit_returns_false(self, flood_cog, config): + mentions = [make_member(id=i) for i in range(config.MENTIONS_LIMIT - 1)] + message = make_message(mentions=mentions) + prime_cog(flood_cog, message) + + assert await flood_cog.mention_check(message) is False + + async def test_at_limit_mutes_and_alerts(self, flood_cog, config): + member = make_member(name="mencionador") + mentions = [make_member(id=i) for i in range(config.MENTIONS_LIMIT)] + message = make_message(author=member, mentions=mentions) + prime_cog(flood_cog, message) + + assert await flood_cog.mention_check(message) is True + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + + async def test_mentions_and_role_mentions_add_up(self, flood_cog, config): + member = make_member(name="mencionador") + mentions = [make_member(id=1)] + role_mentions = [object() for _ in range(config.MENTIONS_LIMIT - 1)] + message = make_message(author=member, mentions=mentions, role_mentions=role_mentions) + prime_cog(flood_cog, message) + + assert await flood_cog.mention_check(message) is True + + +# --------------------------------------------------------------------------- +# attachment_check +# --------------------------------------------------------------------------- +class TestAttachmentCheckFastPath: + async def test_no_images_returns_false(self, flood_cog): + message = make_message(attachments=[make_attachment(content_type="text/plain")]) + prime_cog(flood_cog, message) + + assert await flood_cog.attachment_check(message) is False + + async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( + self, flood_cog, patched_message_delete + ): + data = make_png_bytes() + digest = __import__("hashlib").sha256(data).hexdigest() + flood_cog.messages.image_spam.add(digest) + + member = make_member(name="reincidente") + message = make_message( + author=member, + attachments=[make_attachment(data=data)], + ) + prime_cog(flood_cog, message) + + assert await flood_cog.attachment_check(message) is True + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + patched_message_delete.assert_awaited_once_with(message) + + +class TestAttachmentCheckBurstPath: + async def test_single_channel_two_images_does_not_trigger(self, flood_cog): + member = make_member(name="autor") + message = make_message( + author=member, + attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], + ) + prime_cog(flood_cog, message) + + assert await flood_cog.attachment_check(message) is False + member.add_roles.assert_not_awaited() + + async def test_single_image_across_channels_does_not_trigger(self, flood_cog): + member = make_member(name="autor") + channel_a = make_text_channel(id=1) + channel_b = make_text_channel(id=2) + + for channel in (channel_a, channel_b): + message = make_message( + author=member, channel=channel, attachments=[make_attachment()] + ) + prime_cog(flood_cog, message) + assert await flood_cog.attachment_check(message) is False + + member.add_roles.assert_not_awaited() + + async def test_two_images_two_channels_triggers_on_the_second_message( + self, flood_cog, patched_message_delete + ): + member = make_member(name="comprometido") + channel_a = make_text_channel(id=1) + channel_b = make_text_channel(id=2) + + first = make_message( + author=member, + channel=channel_a, + attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], + ) + prime_cog(flood_cog, first) + first_result = await flood_cog.attachment_check(first) + + second = make_message( + author=member, + channel=channel_b, + attachments=[make_attachment(filename="b1.png"), make_attachment(filename="b2.png")], + ) + prime_cog(flood_cog, second) + second_result = await flood_cog.attachment_check(second) + + # The first channel's message is never retroactively touched - only + # the message that crosses the 2-channel threshold gets acted on. + assert first_result is False + assert second_result is True + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + patched_message_delete.assert_awaited_once_with(second) + + async def test_images_get_cached_for_the_fast_path(self, flood_cog): + member = make_member(name="comprometido") + data_a, data_b = make_png_bytes((255, 0, 0)), make_png_bytes((0, 255, 0)) + + first = make_message( + author=member, + channel=make_text_channel(id=1), + attachments=[make_attachment(data=data_a), make_attachment(data=data_b)], + ) + prime_cog(flood_cog, first) + await flood_cog.attachment_check(first) + + second = make_message( + author=member, + channel=make_text_channel(id=2), + attachments=[make_attachment(data=data_a), make_attachment(data=data_b)], + ) + prime_cog(flood_cog, second) + await flood_cog.attachment_check(second) + + import hashlib + + assert hashlib.sha256(data_a).hexdigest() in flood_cog.messages.image_spam + assert hashlib.sha256(data_b).hexdigest() in flood_cog.messages.image_spam + + async def test_outside_burst_window_does_not_trigger(self, flood_cog, config, monkeypatch): + import comandos.flood as flood_module + + member = make_member(name="lento") + clock = iter([1000.0, 1000.0 + config.IMAGE_BURST_WINDOW + 1]) + monkeypatch.setattr(flood_module.time, "time", lambda: next(clock)) + + first = make_message( + author=member, + channel=make_text_channel(id=1), + attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], + ) + prime_cog(flood_cog, first) + await flood_cog.attachment_check(first) + + second = make_message( + author=member, + channel=make_text_channel(id=2), + attachments=[make_attachment(filename="b1.png"), make_attachment(filename="b2.png")], + ) + prime_cog(flood_cog, second) + result = await flood_cog.attachment_check(second) + + assert result is False + member.add_roles.assert_not_awaited() + + async def test_same_channel_twice_is_not_two_distinct_channels(self, flood_cog): + member = make_member(name="autor") + channel = make_text_channel(id=1) + + for _ in range(3): + message = make_message( + author=member, + channel=channel, + attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], + ) + prime_cog(flood_cog, message) + result = await flood_cog.attachment_check(message) + + assert result is False + member.add_roles.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# _sanitize_attachment / _hash_attachment +# --------------------------------------------------------------------------- +class TestSanitizeAttachment: + async def test_valid_image_round_trips_as_spoiler_file(self, flood_cog): + attachment = make_attachment(data=make_png_bytes()) + + result = await flood_cog._sanitize_attachment(attachment) + + assert result is not None + assert isinstance(result, discord.File) + assert result.spoiler is True + assert result.filename.endswith("evidencia.png") or "SPOILER" in result.filename + + async def test_garbage_bytes_returns_none(self, flood_cog): + attachment = make_attachment(data=b"not an image, just garbage" * 10) + + assert await flood_cog._sanitize_attachment(attachment) is None + + async def test_truncated_image_returns_none(self, flood_cog): + attachment = make_attachment(data=make_png_bytes()[:15]) + + assert await flood_cog._sanitize_attachment(attachment) is None + + +class TestHashAttachment: + async def test_matches_sha256_of_bytes(self, flood_cog): + import hashlib + + data = b"some bytes" + attachment = make_attachment(data=data) + + assert await flood_cog._hash_attachment(attachment) == hashlib.sha256(data).hexdigest() + + +# --------------------------------------------------------------------------- +# add_spam_message / add_spam_image_hash +# --------------------------------------------------------------------------- +class TestAddSpamHelpers: + def test_add_spam_message_persists_and_caches(self, flood_cog, isolated_logs): + flood_cog.add_spam_message("mensaje malo") + + assert "mensaje malo" in flood_cog.messages.spam + assert "mensaje malo" in isolated_logs.log_spam_file.read_text() + + def test_add_spam_image_hash_persists_and_caches(self, flood_cog, isolated_logs): + flood_cog.add_spam_image_hash("deadbeef") + + assert "deadbeef" in flood_cog.messages.image_spam + assert "deadbeef" in isolated_logs.log_image_spam_file.read_text() + + +# --------------------------------------------------------------------------- +# alert_moderation +# --------------------------------------------------------------------------- +class TestAlertModeration: + async def test_creates_thread_and_sends_embed(self, flood_cog): + member = make_member(name="alguien") + message = make_message(author=member) + prime_cog(flood_cog, message) + + await flood_cog.alert_moderation("Alerta de prueba", "scam") + + flood_cog.main_mod_channel.create_thread.assert_awaited_once() + _, kwargs = flood_cog.main_mod_channel.create_thread.call_args + assert member.mention in kwargs["name"] + + thread = flood_cog.main_mod_channel.create_thread.return_value + thread.send.assert_awaited_once() + + async def test_unknown_reason_raises(self, flood_cog): + message = make_message() + prime_cog(flood_cog, message) + + with pytest.raises(KeyError): + await flood_cog.alert_moderation("Título", "no-existe") + + async def test_attachments_are_forwarded_sanitized_and_spoilered(self, flood_cog): + message = make_message() + prime_cog(flood_cog, message) + images = [make_attachment(data=make_png_bytes())] + + await flood_cog.alert_moderation("Alerta", "known_image", attachments=images) + + thread = flood_cog.main_mod_channel.create_thread.return_value + _, kwargs = thread.send.call_args + assert len(kwargs["files"]) == 1 + assert kwargs["files"][0].spoiler is True + + async def test_undecodable_attachment_is_skipped_not_forwarded(self, flood_cog): + message = make_message() + prime_cog(flood_cog, message) + images = [make_attachment(data=b"garbage" * 10)] + + await flood_cog.alert_moderation("Alerta", "known_image", attachments=images) + + thread = flood_cog.main_mod_channel.create_thread.return_value + _, kwargs = thread.send.call_args + assert kwargs["files"] == [] + + async def test_no_attachments_means_no_warning_field(self, flood_cog): + message = make_message() + prime_cog(flood_cog, message) + + await flood_cog.alert_moderation("Alerta", "scam") + + thread = flood_cog.main_mod_channel.create_thread.return_value + _, kwargs = thread.send.call_args + field_names = [f.name for f in kwargs["embed"].fields] + assert not any("Imágenes adjuntas" in name for name in field_names) + + +# --------------------------------------------------------------------------- +# on_message pipeline +# --------------------------------------------------------------------------- +class TestOnMessagePipeline: + async def test_ignores_messages_from_bots(self, flood_cog): + member = make_member(name="unbot", bot=True) + message = make_message(content="discord nitro free http://x", author=member) + + await flood_cog.on_message(message) + + member.add_roles.assert_not_awaited() + + async def test_ignores_the_configured_bot_id(self, flood_cog, config): + member = make_member(name="elbot", id=config.BOT_ID, bot=False) + message = make_message(content="discord nitro free http://x", author=member) + + await flood_cog.on_message(message) + + member.add_roles.assert_not_awaited() + + async def test_ignores_short_textless_messages_without_attachments(self, flood_cog): + member = make_member(name="alguien") + message = make_message(content="ok", author=member) + + await flood_cog.on_message(message) + + # Never even gets far enough to set up per-message state. + assert flood_cog._msg_author is None + + async def test_short_caption_with_attachments_is_still_processed(self, flood_cog): + member = make_member(name="alguien") + message = make_message( + content="ok", + author=member, + attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], + ) + + await flood_cog.on_message(message) + + # It went through the pipeline (attachment_check saw it), even though + # the caption alone would have been skipped. + assert flood_cog._msg_author is member + + async def test_skips_coordination_role_members(self, flood_cog): + message = make_message( + content="discord nitro free http://x", + author=make_member(name="mod", roles=[flood_cog.coord_role]), + ) + + result = None + try: + result = await flood_cog.on_message(message) + finally: + pass + + message.author.add_roles.assert_not_awaited() + + async def test_known_spam_text_is_deleted_and_author_muted( + self, flood_cog, patched_message_delete + ): + flood_cog.messages.spam.add("mensaje ya conocido como spam") + member = make_member(name="repetidor") + channel = make_text_channel(id=42) + flood_cog.bot.channels[channel.id] = channel + message = make_message( + content="Mensaje YA conocido como SPAM", author=member, channel=channel + ) + + await flood_cog.on_message(message) + + member.add_roles.assert_awaited_once_with(flood_cog.muted_role) + patched_message_delete.assert_awaited_once_with(message) diff --git a/tests/test_limpia.py b/tests/test_limpia.py new file mode 100644 index 0000000..e43ce7e --- /dev/null +++ b/tests/test_limpia.py @@ -0,0 +1,50 @@ +from types import SimpleNamespace + +from comandos.limpia import Limpia +from tests.factories import ( + bind_commands, + make_bot, + make_ctx, + make_dm_channel, + make_message, + make_text_channel, +) + + +class TestPurge: + async def test_ignores_unsupported_channel_types(self): + cog = bind_commands(Limpia(make_bot())) + ctx = make_ctx(channel=make_dm_channel()) + + await cog.purge(ctx, limit=5) + + # Nothing to assert on the channel itself (it's not a spec that + # exposes purge/delete_messages), just that we returned early + # without raising. + + async def test_no_reply_purges_the_channel_and_the_command_message(self): + channel = make_text_channel(id=1, name="general") + cog = bind_commands(Limpia(make_bot())) + ctx = make_ctx(channel=channel) + + await cog.purge(ctx, limit=3) + + ctx.defer.assert_awaited_once_with(ephemeral=True) + channel.purge.assert_any_await(limit=4) + ctx.message.delete.assert_awaited_once() + ctx.send.assert_awaited_once() + channel.purge.assert_any_await(limit=1) + + async def test_reply_deletes_messages_up_to_the_referenced_one(self): + target = make_message(id=42) + history = [make_message(id=1), make_message(id=2), target, make_message(id=3)] + channel = make_text_channel(id=1, name="general", history_messages=history) + cog = bind_commands(Limpia(make_bot())) + ctx = make_ctx(channel=channel, reference=SimpleNamespace(message_id=42)) + ctx.message.channel = channel + + await cog.purge(ctx) + + channel.delete_messages.assert_awaited_once() + (deleted,), _ = channel.delete_messages.call_args + assert [m.id for m in deleted] == [1, 2, 42] diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py new file mode 100644 index 0000000..6fd066c --- /dev/null +++ b/tests/test_moderacion.py @@ -0,0 +1,320 @@ +from unittest.mock import AsyncMock + +import pandas as pd +import pytest + +from tests.factories import ( + encode_for_mod_row, + make_ctx, + make_interaction, + make_member, +) + + +def add_pending_row(cog, post_id, *, channel="envio-eventos", author_id=42, author_name="autor", + content="contenido de prueba"): + new_row = { + "date": "2026-01-01 00:00:00", + "message_id": str(post_id), + "channel": channel, + "author_id": str(author_id), + "author": author_name, + "message": encode_for_mod_row(content), + } + cog.bot.data_mod = pd.concat([cog.bot.data_mod, pd.DataFrame([new_row])], ignore_index=True) + return new_row + + +# --------------------------------------------------------------------------- +# small helpers +# --------------------------------------------------------------------------- +class TestResolveAuthor: + def test_regular_context_uses_author(self, moderacion_cog): + member = make_member(name="alguien") + ctx = make_ctx(author=member) + + assert moderacion_cog._resolve_author(ctx) is member + + def test_interaction_uses_user(self, moderacion_cog): + member = make_member(name="alguien") + interaction = make_interaction(user=member) + + assert moderacion_cog._resolve_author(interaction) is member + + +class TestIsBot: + def test_true_for_configured_bot_id(self, moderacion_cog, config): + ctx = make_ctx(author=make_member(id=config.BOT_ID)) + + assert moderacion_cog._is_bot(ctx) is True + + def test_false_for_regular_user(self, moderacion_cog): + ctx = make_ctx(author=make_member(id=12345)) + + assert moderacion_cog._is_bot(ctx) is False + + +class TestIsValidChannel: + def test_true_when_channel_is_registered_on_the_bot(self, moderacion_cog, moderacion_channels): + ctx = make_ctx(channel=moderacion_channels["mod"]) + + assert moderacion_cog._is_valid_channel(ctx) is True + + +class TestGetChannelsMainModSub: + def test_resolves_the_three_channels(self, moderacion_cog, moderacion_channels): + main, mod, sub = moderacion_cog.get_channels_main_mod_sub(moderacion_channels["sub"].id) + + assert main is moderacion_channels["main"] + assert mod is moderacion_channels["mod"] + assert sub is moderacion_channels["sub"] + + +# --------------------------------------------------------------------------- +# _parse_post_id +# --------------------------------------------------------------------------- +class TestParsePostId: + async def test_interaction_with_message_id_returns_it_directly( + self, moderacion_cog, moderacion_channels + ): + interaction = make_interaction(channel=moderacion_channels["mod"]) + + result = await moderacion_cog._parse_post_id(interaction, 555, "%aceptar") + + assert result == "555" + + async def test_valid_numeric_id_from_message_content(self, moderacion_cog, moderacion_channels): + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 123") + + assert await moderacion_cog._parse_post_id(ctx, None, "%aceptar") == "123" + + async def test_non_numeric_id_reports_error_and_returns_none( + self, moderacion_cog, moderacion_channels + ): + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar abc") + + result = await moderacion_cog._parse_post_id(ctx, None, "%aceptar") + + assert result is None + moderacion_channels["mod"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["mod"].send.call_args + assert "abc" in msg + + +# --------------------------------------------------------------------------- +# _get_validated_post +# --------------------------------------------------------------------------- +class TestGetValidatedPost: + async def test_happy_path_resolves_everything(self, moderacion_cog, moderacion_channels): + add_pending_row(moderacion_cog, post_id=1, author_id=99) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 1") + + vp = await moderacion_cog._get_validated_post(ctx, None, "%aceptar") + + assert vp is not None + assert vp.post_id == "1" + assert vp.message_dec == "contenido de prueba" + assert vp.ch_main is moderacion_channels["main"] + assert vp.ch_mod is moderacion_channels["mod"] + assert vp.ch_sub is moderacion_channels["sub"] + assert vp.author.id == 99 + + async def test_unknown_post_id_reports_error_and_returns_none( + self, moderacion_cog, moderacion_channels + ): + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 999") + + vp = await moderacion_cog._get_validated_post(ctx, None, "%aceptar") + + assert vp is None + moderacion_channels["mod"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["mod"].send.call_args + assert "999" in msg + + async def test_bot_author_returns_none(self, moderacion_cog, moderacion_channels, config): + add_pending_row(moderacion_cog, post_id=1) + ctx = make_ctx( + author=make_member(id=config.BOT_ID), + channel=moderacion_channels["mod"], + content="%aceptar 1", + ) + + assert await moderacion_cog._get_validated_post(ctx, None, "%aceptar") is None + + +# --------------------------------------------------------------------------- +# _log_action / log_on_message +# --------------------------------------------------------------------------- +class TestLogAction: + def test_aceptar_writes_expected_line(self, moderacion_cog, isolated_logs): + row = pd.DataFrame([add_pending_row(moderacion_cog, post_id=1)]) + + moderacion_cog._log_action("aceptar", row, "1", "moderador#0") + + content = isolated_logs.log_accepted_file.read_text() + assert '"1"' in content + assert '"moderador#0"' in content + + def test_rechazar_includes_reason(self, moderacion_cog, isolated_logs): + row = pd.DataFrame([add_pending_row(moderacion_cog, post_id=2)]) + + moderacion_cog._log_action("rechazar", row, "2", "moderador#0", "le falta info") + + content = isolated_logs.log_rejected_file.read_text() + assert '"le falta info"' in content + + +class TestLogOnMessage: + def test_appends_row_and_writes_log_line(self, moderacion_cog, isolated_logs): + moderacion_cog._msg_id = 777 + moderacion_cog._msg_enc = encode_for_mod_row("hola mundo") + author = make_member(id=55, name="autor") + + before = len(moderacion_cog.bot.data_mod) + moderacion_cog.log_on_message("envio-eventos", author) + + assert len(moderacion_cog.bot.data_mod) == before + 1 + assert "777" in isolated_logs.log_mod_file.read_text() + + +# --------------------------------------------------------------------------- +# get_mod_pending +# --------------------------------------------------------------------------- +class TestGetModPending: + def test_empty_data_sets_footer(self, moderacion_cog): + embed = moderacion_cog.get_mod_pending(moderacion_cog.bot.data_mod) + + assert embed.footer.text == "No hay mensajes pendientes de moderación" + assert len(embed.fields) == 0 + + def test_lists_pending_posts_with_known_authors(self, moderacion_cog): + add_pending_row(moderacion_cog, post_id=1, author_id=99, content="hola" * 20) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + + embed = moderacion_cog.get_mod_pending(moderacion_cog.bot.data_mod) + + assert len(embed.fields) == 1 + assert "1" in embed.fields[0].name + + def test_skips_posts_from_users_no_longer_in_the_server(self, moderacion_cog): + add_pending_row(moderacion_cog, post_id=1, author_id=404) + # bot.get_user(404) resolves to None - author has left the server. + + embed = moderacion_cog.get_mod_pending(moderacion_cog.bot.data_mod) + + assert len(embed.fields) == 0 + assert embed.footer.text == "No hay mensajes pendientes de moderación" + + +# --------------------------------------------------------------------------- +# _aceptar_mensaje / _rechazar_mensaje +# --------------------------------------------------------------------------- +class TestAceptarMensaje: + async def test_removes_pending_row_and_notifies_channels( + self, moderacion_cog, moderacion_channels, config + ): + add_pending_row(moderacion_cog, post_id=1, author_id=99, content="contenido aprobado") + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + moderacion_cog._msg_id = 12345 + + ctx = make_ctx( + author=make_member(name="moderador"), + channel=moderacion_channels["mod"], + content="%aceptar 1", + ) + + await moderacion_cog._aceptar_mensaje(ctx) + + assert moderacion_cog.bot.data_mod.empty + moderacion_channels["mod"].send.assert_awaited_once() + moderacion_channels["main"].send.assert_awaited_once() + (msg,), _ = moderacion_channels["main"].send.call_args + assert "contenido aprobado" in msg + + async def test_unknown_post_id_does_not_touch_channels(self, moderacion_cog, moderacion_channels): + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 999") + + await moderacion_cog._aceptar_mensaje(ctx) + + moderacion_channels["main"].send.assert_not_awaited() + + +class TestRechazarMensaje: + async def test_removes_pending_row_and_notifies_with_reason( + self, moderacion_cog, moderacion_channels + ): + add_pending_row(moderacion_cog, post_id=2, author_id=99, content="contenido rechazado") + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + + ctx = make_ctx( + author=make_member(name="moderador"), + channel=moderacion_channels["mod"], + content="%rechazar 2 le falta info aqui", + ) + + await moderacion_cog._rechazar_mensaje(ctx) + + assert moderacion_cog.bot.data_mod.empty + moderacion_channels["sub"].send.assert_awaited_once() + _, kwargs = moderacion_channels["sub"].send.call_args + assert "le falta info aqui" in kwargs["embed"].fields[0].value + + async def test_interaction_path_uses_the_provided_reason( + self, moderacion_cog, moderacion_channels + ): + add_pending_row(moderacion_cog, post_id=3, author_id=99, content="contenido") + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + interaction = make_interaction(channel=moderacion_channels["mod"]) + + await moderacion_cog._rechazar_mensaje(interaction, message_id=3, reason="motivo modal") + + _, kwargs = moderacion_channels["sub"].send.call_args + assert "motivo modal" in kwargs["embed"].fields[0].value + + +# --------------------------------------------------------------------------- +# on_message listener +# --------------------------------------------------------------------------- +class TestOnMessage: + async def test_ignores_limpia_command(self, moderacion_cog, moderacion_channels): + message = make_ctx(channel=moderacion_channels["sub"], content="%limpia").message + message.author = make_member() + message.id = 1 + + before = len(moderacion_cog.bot.data_mod) + await moderacion_cog.on_message(message) + + assert len(moderacion_cog.bot.data_mod) == before + + async def test_ignores_channels_outside_the_submission_mapping( + self, moderacion_cog, moderacion_channels + ): + other_channel = moderacion_channels["mod"] # not a "submission" channel + message = make_ctx(channel=other_channel, content="hola").message + message.author = make_member() + message.id = 2 + + before = len(moderacion_cog.bot.data_mod) + await moderacion_cog.on_message(message) + + assert len(moderacion_cog.bot.data_mod) == before + + async def test_submission_gets_logged_and_forwarded_to_mod_channel( + self, moderacion_cog, moderacion_channels, monkeypatch + ): + import comandos.moderacion as moderacion_module + + monkeypatch.setattr(moderacion_module.asyncio, "sleep", AsyncMock()) + + member = make_member(name="remitente") + message = make_ctx(channel=moderacion_channels["sub"], content="mi propuesta de evento").message + message.author = member + message.id = 321 + + before = len(moderacion_cog.bot.data_mod) + await moderacion_cog.on_message(message) + + assert len(moderacion_cog.bot.data_mod) == before + 1 + moderacion_channels["sub"].send.assert_awaited_once() + moderacion_channels["mod"].send.assert_awaited_once() diff --git a/tests/test_ping.py b/tests/test_ping.py new file mode 100644 index 0000000..857ccba --- /dev/null +++ b/tests/test_ping.py @@ -0,0 +1,63 @@ +from types import SimpleNamespace + +from comandos.ping import Ping +from tests.factories import bind_commands, make_bot, make_ctx, make_message + + +def make_bot_user(mentioned=True): + return SimpleNamespace(mentioned_in=lambda message: mentioned) + + +class TestHasGlobalMention: + def test_true_for_everyone(self): + cog = Ping(make_bot()) + assert cog.has_global_mention(SimpleNamespace(content="hola @everyone")) is True + + def test_true_for_here(self): + cog = Ping(make_bot()) + assert cog.has_global_mention(SimpleNamespace(content="hola @here")) is True + + def test_false_for_a_regular_mention(self): + cog = Ping(make_bot()) + assert cog.has_global_mention(SimpleNamespace(content="hola <@123>")) is False + + +class TestPingPong: + async def test_replies_pong_ephemerally(self): + cog = bind_commands(Ping(make_bot())) + ctx = make_ctx() + + await cog.pingpong(ctx) + + ctx.send.assert_awaited_once_with("pong", ephemeral=True) + + +class TestOnMessage: + async def test_sends_the_llama_gif_when_mentioned(self): + bot = make_bot(user=make_bot_user(mentioned=True)) + cog = Ping(bot) + message = make_message(content="hola bot") + + await cog.on_message(message) + + message.channel.send.assert_awaited_once() + _, kwargs = message.channel.send.call_args + assert kwargs["file"].filename == "resources/llama.gif" + + async def test_ignores_global_mentions(self): + bot = make_bot(user=make_bot_user(mentioned=True)) + cog = Ping(bot) + message = make_message(content="@everyone hola bot") + + await cog.on_message(message) + + message.channel.send.assert_not_awaited() + + async def test_ignores_messages_that_do_not_mention_the_bot(self): + bot = make_bot(user=make_bot_user(mentioned=False)) + cog = Ping(bot) + message = make_message(content="hola a todos") + + await cog.on_message(message) + + message.channel.send.assert_not_awaited() diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..2ac0a48 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,51 @@ +from tests.factories import make_message +from utils import get_message_to_moderate, get_moderation_channel, strip_message + + +class TestStripMessage: + def test_lowercases(self): + assert strip_message("HOLA Mundo") == "hola mundo" + + def test_removes_newlines_and_tabs(self): + assert strip_message("hola\nmundo\tcon\rtabs") == "hola mundo con tabs" + + def test_collapses_multiple_whitespace(self): + assert strip_message("hola mundo") == "hola mundo" + + def test_removes_mentions(self): + assert strip_message("hola <@123456789> mundo") == "hola mundo" + + def test_removes_multiple_mentions(self): + assert strip_message("<@111> hola <@!222> mundo") == "hola mundo" + + def test_strips_surrounding_whitespace(self): + assert strip_message(" hola mundo ") == "hola mundo" + + def test_empty_string(self): + assert strip_message("") == "" + + +class TestGetModerationChannel: + def test_returns_bot_get_channel_result(self): + sentinel = object() + + class FakeBot: + def get_channel(self, channel_id): + assert channel_id == 42 + return sentinel + + assert get_moderation_channel(FakeBot(), 42) is sentinel + + +class TestGetMessageToModerate: + def test_embed_contains_message_and_commands(self): + message = make_message(content="hola, este es mi post") + message.id = 4242 + + embed = get_message_to_moderate(message) + + assert "hola, este es mi post" in embed.description + assert "%aceptar 4242" in embed.description + assert "%rechazar 4242" in embed.description + assert message.channel.mention in embed.description + assert message.author.mention in embed.description diff --git a/utils.py b/utils.py index e8b5e60..41cb7c1 100644 --- a/utils.py +++ b/utils.py @@ -1,6 +1,6 @@ import re import discord -from datetime import datetime +from datetime import datetime, timezone from configuration import Config @@ -17,7 +17,7 @@ def get_moderation_channel(bot, channel_id): def get_message_to_moderate(message): msg = ( - f"{datetime.utcnow()} UTC\n" + f"{datetime.now(timezone.utc).replace(tzinfo=None)} UTC\n" f"Mensaje enviado desde {message.channel.mention} por {message.author.mention}\n\n" f"```\n{message.content}\n```\n\n**¿Cumple con todos los requisitos?**\n\n" f"{aceptar_emoji} Para aceptarlo, envía el siguiente mensaje:\n\n" From 323266f930d361b086e690a2977d539230ef9cd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 13:55:14 +0200 Subject: [PATCH 02/12] Data-driven cog registration, __main__ guard, replace print() with logging - bot.py: register cogs from a COGS tuple instead of 7 hand-written add_cog() calls - adding a cog is now "add a class to the tuple" instead of touching both an import block and a registration block. - bot.py: guard the asyncio.run(main()) entry point with `if __name__ == "__main__":` - importing the module (e.g. from a test, or a REPL) no longer connects to Discord as a side effect. - Replace every print()/print(f"LOG: ...") call across bot.py, configuration.py, and comandos/*.py with logging.getLogger(__name__) calls at an appropriate level (debug for trace-only messages, info for state changes, warning/error/exception for actual problems). discord.utils.setup_logging() in bot.py configures the root logger by default, so these now actually reach bot.log instead of only ever appearing on stdout. - limpia.py: drop a leftover print(dir(...)) debug dump that had no operational value even as a log line. --- bot.py | 28 ++++++++++++++-------------- comandos/archivar.py | 8 +++++--- comandos/flood.py | 20 ++++++++++---------- comandos/limpia.py | 6 ++++-- comandos/moderacion.py | 4 +++- configuration.py | 19 ++++++++++++------- 6 files changed, 48 insertions(+), 37 deletions(-) diff --git a/bot.py b/bot.py index ea9d4df..b3e316b 100644 --- a/bot.py +++ b/bot.py @@ -16,6 +16,11 @@ from comandos.archivar import Archivar from comandos.enviar import Enviar +# Every cog to register on startup. Add a class here (and to the imports +# above) to wire up a new command/listener group - nothing else needs to +# change. +COGS = (Ping, Ayuda, Limpia, Archivar, Moderacion, FloodSpam, Enviar) + # Global instance of the server guild = None @@ -28,6 +33,7 @@ handler = logging.FileHandler(filename="bot.log", encoding="utf-8", mode="w") discord.utils.setup_logging(level=logging.INFO, handler=handler) +logger = logging.getLogger(__name__) @bot.event @@ -48,16 +54,16 @@ async def on_message(message: discord.Message): @bot.event async def on_ready(): - print("Syncing tree...") + logger.info("Syncing tree...") # await bot.tree.sync() @bot.event async def on_command_error(msg, error): if isinstance(error, (commands.MissingRole, commands.MissingAnyRole)): - print(f"MissingRole ERROR: {error}") + logger.warning("MissingRole ERROR: %s", error) else: - print(error) + logger.error("Unhandled command error", exc_info=error) async def main(): @@ -73,21 +79,15 @@ async def main(): # keeping the data in the bot instance bot.data_mod = data_mod[~data_mod["message_id"].isin(ready_ids)] # type: ignore[attr-defined] - await bot.add_cog(Ping(bot)) - await bot.add_cog(Ayuda(bot)) - - await bot.add_cog(Limpia(bot)) - await bot.add_cog(Archivar(bot)) - - await bot.add_cog(Moderacion(bot)) - await bot.add_cog(FloodSpam(bot)) - await bot.add_cog(Enviar(bot)) + for cog_cls in COGS: + await bot.add_cog(cog_cls(bot)) # Removing the help command # bot.remove_command("help") - print("Running...") + logger.info("Running...") await bot.start(config.TOKEN) -asyncio.run(main()) +if __name__ == "__main__": + asyncio.run(main()) diff --git a/comandos/archivar.py b/comandos/archivar.py index e36449a..12090a3 100644 --- a/comandos/archivar.py +++ b/comandos/archivar.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime from typing import List, Optional @@ -7,6 +8,7 @@ from configuration import Config config = Config() +logger = logging.getLogger(__name__) class Archivar(commands.Cog): @@ -78,9 +80,9 @@ def archivar_canal(self, filename: str, messages: List[discord.Message]): f"{m_id};{m_content};{m_channel_id};{m_channel_name};{m_channel_category};" f"{m_author_id};{m_author_name}#{m_author_discriminator};{m_author_bot}\n" ) - print(f"File written: {filename}") - except Exception as e: - print(f"{type(e).__name__}: {e}") + logger.info("File written: %s", filename) + except Exception: + logger.exception("Failed to archive channel into %s", filename) return False, None return True, filename diff --git a/comandos/flood.py b/comandos/flood.py index 282a5c1..2aeb0ab 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -1,4 +1,5 @@ import hashlib +import logging import time from io import BytesIO @@ -14,6 +15,7 @@ from typing import Optional config = Config() +logger = logging.getLogger(__name__) SPAM_WORDS = [ ("discord", "nitro", "free", "http"), @@ -112,7 +114,7 @@ async def clear_messages(self): @commands.Cog.listener() async def on_message(self, message): - print("FloodSpam.on_message") + logger.debug("on_message: %s", message.id) await self.bot.process_commands(message) if message.author.bot or message.author.id == config.BOT_ID: @@ -131,7 +133,6 @@ async def on_message(self, message): if self.coord_role in self._msg_author.roles: return - print("FloodSpam.on_message: attachment_check") if await self.attachment_check(message): return @@ -175,7 +176,6 @@ async def on_message(self, message): ) await self._msg_channel.send(embed=embed, delete_after=300) - print("FloodSpam.on_message: spam_check") if await self.spam_check(message): self.add_spam_message(self._msg_content) await discord.Message.delete(message) @@ -218,7 +218,7 @@ async def spam_check(self, message: discord.Message): return True async def flood_check(self, message): - print(f"LOG: flood_check: {message}") + logger.debug("flood_check: %s", message.id) # Textless (image-only) messages are handled by attachment_check if not self._msg_content: @@ -283,7 +283,7 @@ async def _sanitize_attachment(attachment: discord.Attachment) -> Optional[disco buf.seek(0) return discord.File(buf, filename="evidencia.png", spoiler=True) except (UnidentifiedImageError, OSError, ValueError): - print(f"LOG: _sanitize_attachment: could not decode {attachment.filename!r}, skipping") + logger.warning("_sanitize_attachment: could not decode %r, skipping", attachment.filename) return None async def attachment_check(self, message: discord.Message) -> bool: @@ -298,7 +298,7 @@ async def attachment_check(self, message: discord.Message) -> bool: same images across the server. When this fires, the offending images are hashed and cached for the fast path above. """ - print("LOG: attachment_check") + logger.debug("attachment_check: %s", message.id) images = [ a for a in message.attachments @@ -383,7 +383,7 @@ async def attachment_check(self, message: discord.Message) -> bool: return True async def mention_check(self, message): - print("LOG: mention_check") + logger.debug("mention_check: %s", message.id) # Skip if 2 mentions or less if (len(message.mentions) + len(message.role_mentions)) < config.MENTIONS_LIMIT: @@ -412,19 +412,19 @@ async def mention_check(self, message): return True def add_spam_message(self, message): - print("LOG: add_spam_message") + logger.info("add_spam_message: %r", message) with open(config.log_spam_file, "a") as f: f.write(f"{message}\n") self.messages.spam.add(message) def add_spam_image_hash(self, digest): - print("LOG: add_spam_image_hash") + logger.info("add_spam_image_hash: %s", digest) with open(config.log_image_spam_file, "a") as f: f.write(f"{digest}\n") self.messages.image_spam.add(digest) async def alert_moderation(self, title, reason, attachments=None): - print("LOG: alert_moderation") + logger.debug("alert_moderation: %s (%s)", title, reason) d_msg = { "menciones": ( diff --git a/comandos/limpia.py b/comandos/limpia.py index 3073f32..293a3a5 100644 --- a/comandos/limpia.py +++ b/comandos/limpia.py @@ -1,3 +1,5 @@ +import logging + import discord from discord.ext import commands from discord import app_commands @@ -5,6 +7,7 @@ from configuration import Config config = Config() +logger = logging.getLogger(__name__) class Limpia(commands.Cog): @@ -27,7 +30,6 @@ async def purge(self, ctx: commands.Context, limit: int = 1) -> None: if not isinstance(channel, (discord.TextChannel, discord.Thread, discord.VoiceChannel)): return - print(dir(ctx.message.reference)) if hasattr(ctx.message.reference, "message_id"): reply_id = ctx.message.reference.message_id msg = [] @@ -42,7 +44,7 @@ async def purge(self, ctx: commands.Context, limit: int = 1) -> None: try: await ctx.message.delete() except discord.NotFound: - print("Slash command, no need to remove command message") + logger.debug("Slash command, no need to remove command message") # await ctx.channel.typing() embed = discord.Embed( diff --git a/comandos/moderacion.py b/comandos/moderacion.py index dc26597..d5b5ee7 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -1,5 +1,6 @@ import asyncio import base64 +import logging from datetime import datetime from dataclasses import dataclass from typing import Optional @@ -12,6 +13,7 @@ from utils import get_moderation_channel, get_message_to_moderate, aceptar_emoji, rechazar_emoji config = Config() +logger = logging.getLogger(__name__) EMBED_COLOR = 0x2B597B @@ -319,7 +321,7 @@ def get_mod_pending(self, data): for idx, mod_row in data.iterrows(): author = self.bot.get_user(int(mod_row["author_id"])) if not author: - print(f"El author '{mod_row['author_id']}' ya no existe en el server.") + logger.warning("El author '%s' ya no existe en el server.", mod_row["author_id"]) continue m_message = base64.b64decode(eval(mod_row["message"])).decode("utf-8") embed.add_field( diff --git a/configuration.py b/configuration.py index 1d3e999..b1f56e1 100644 --- a/configuration.py +++ b/configuration.py @@ -1,8 +1,11 @@ +import logging import sys import toml from pathlib import Path +logger = logging.getLogger(__name__) + class Singleton(type): _instances = {} @@ -16,13 +19,13 @@ def __call__(cls, *args, **kwargs): class Config(metaclass=Singleton): def __init__(self): # Configuration file - print("Config __init__") + logger.debug("Config __init__") config = None with open("config.toml") as f: config = toml.loads(f.read()) if not config: - print("Error: Failed to load the config") + logger.error("Failed to load the config") sys.exit(-1) try: @@ -46,8 +49,10 @@ def __init__(self): self.IMAGE_ATTACHMENT_LIMIT = 2 self.IMAGE_BURST_WINDOW = 60 * 5 except KeyError: - print("Error while reading the configuration file. " - "Make sure it contains all the required field") + logger.error( + "Error while reading the configuration file. " + "Make sure it contains all the required field" + ) sys.exit(-1) self.setup_log_files() @@ -94,9 +99,9 @@ def get_spam_messages(self): d = set() with open(self.log_spam_file) as f: for line in f.readlines(): - print(">>>", line.strip()) + logger.debug("Loaded known spam message: %r", line.strip()) d.add(line.strip()) - print("LOG: get_spam_messages", d) + logger.debug("get_spam_messages: %s", d) return d def get_spam_image_hashes(self): @@ -107,7 +112,7 @@ def get_spam_image_hashes(self): line = line.strip() if line: d.add(line) - print("LOG: get_spam_image_hashes", d) + logger.debug("get_spam_image_hashes: %s", d) return d def check_create_file(self, fname: Path, msg: str) -> None: From 1221caad9bcf38b09e242cde8650ba2e486d2a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 21:41:52 +0200 Subject: [PATCH 03/12] Fix three previously-flagged bugs: archivar, flood channel lookup, moderacion jump_url - archivar.archivar_canal() now always returns a (success, filename) tuple, including on the unsupported-channel-type path (previously a bare None there, inconsistent with the (False, None) exception path). archivar() unpacks status/filename explicitly instead of treating the whole tuple as truthy, so a write failure - which returns the non-empty-but-failed (False, None) tuple - is no longer mistaken for success and no longer crashes trying to attach a file that was never written. - FloodSpam.on_message now sets self._msg_channel = message.channel directly instead of re-fetching it via self.bot.get_channel(message.channel.id). The refetch offered no benefit and turned a cache miss into a None channel that crashed the first .send() call downstream. - Moderacion._aceptar_mensaje now sends to the destination channel first and links to that message's own .jump_url, instead of building a URL from self._msg_id - a field holding the *original submission's* id in a different channel, mutated by every incoming submission, so a second one arriving before the first was moderated could point the link at the wrong message entirely. Updated/added tests in tests/test_archivar.py, tests/test_flood.py, and tests/test_moderacion.py to match the corrected behavior and guard against regressions. --- comandos/archivar.py | 14 +++++++++++--- comandos/flood.py | 2 +- comandos/moderacion.py | 10 +++++++--- tests/test_archivar.py | 41 +++++++++++++++++++++++++++------------- tests/test_flood.py | 24 ++++++++++++++++++----- tests/test_moderacion.py | 14 ++++++++++++-- 6 files changed, 78 insertions(+), 27 deletions(-) diff --git a/comandos/archivar.py b/comandos/archivar.py index 12090a3..29702f9 100644 --- a/comandos/archivar.py +++ b/comandos/archivar.py @@ -30,7 +30,7 @@ async def archivar(self, ctx, *, channel: discord.TextChannel) -> Optional[disco filename = f"{timestamp}_canal_{channel.name}.csv" messages = [m async for m in channel.history(limit=None)] - status = self.archivar_canal(filename, messages) + status, archived_filename = self.archivar_canal(filename, messages) if status: e = discord.Embed( @@ -38,7 +38,7 @@ async def archivar(self, ctx, *, channel: discord.TextChannel) -> Optional[disco description=f"El canal {channel.mention} tiene {len(messages)} mensajes", colour=0xFF0000, ) - await self.mod_channel.send(embed=e, file=discord.File(filename)) + await self.mod_channel.send(embed=e, file=discord.File(archived_filename)) else: await self.mod_channel.send(f"Error: Canal '{channel.name}' no fue archivado.") @@ -55,6 +55,14 @@ async def archivar_categoria(self, ctx, *, category: discord.CategoryChannel): await self.archivar(ctx, channel=channel) def archivar_canal(self, filename: str, messages: List[discord.Message]): + """Write ``messages`` to ``filename`` as CSV. + + Always returns a ``(success, filename)`` tuple - never just a bare + falsy value - so callers can safely do + ``status, archived_filename = self.archivar_canal(...)`` and check + ``status`` directly, instead of risking a non-empty-but-failed + tuple being treated as truthy. + """ try: with open(filename, "w") as f: f.write( @@ -65,7 +73,7 @@ def archivar_canal(self, filename: str, messages: List[discord.Message]): for msg in messages: if not isinstance(msg.channel, (discord.TextChannel, discord.Thread, discord.VoiceChannel)): - return + return False, None m_id = msg.id m_content = msg.content.strip().replace("\n", "\\n") diff --git a/comandos/flood.py b/comandos/flood.py index 2aeb0ab..8bb168f 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -124,7 +124,7 @@ async def on_message(self, message): # (e.g. an image-only spam message has no text at all). if len(message.content) < 5 and not message.attachments: return - self._msg_channel = self.bot.get_channel(message.channel.id) + self._msg_channel = message.channel self._msg_content = strip_message(message.content) self._msg_author = message.author self._msg_author_mention = self._msg_author.mention diff --git a/comandos/moderacion.py b/comandos/moderacion.py index d5b5ee7..d89d154 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -261,12 +261,16 @@ async def _aceptar_mensaje(self, ctx, message_id: Optional[int] = None): self._log_action("aceptar", vp.mod_row, vp.post_id, moderator) self.bot.data_mod = self.bot.data_mod[~vp.condition] - jump_url = f"https://discord.com/channels/{self.bot.guilds[0].id}/{vp.ch_main.id}/{self._msg_id}" + # Send to the destination channel first so the confirmation below can + # link to the message that was actually posted there, instead of + # guessing at a URL (the old code built the link from self._msg_id - + # the *original submission's* id in a different channel entirely - + # before the message below even existed). + sent_message = await vp.ch_main.send(f"> [Enviado por {vp.author.mention}]\n{vp.message_dec}") await vp.ch_mod.send( f"{aceptar_emoji} Mensaje `{vp.post_id}` aceptado, " - f"enviado al canal {vp.ch_main.mention}\nVer en {jump_url}" + f"enviado al canal {vp.ch_main.mention}\nVer en {sent_message.jump_url}" ) - await vp.ch_main.send(f"> [Enviado por {vp.author.mention}]\n{vp.message_dec}") @commands.command(name="aceptar", help="Comando para aceptar mensajes en moderación") @commands.has_role(config.MOD_ROLE) diff --git a/tests/test_archivar.py b/tests/test_archivar.py index bfaab1b..8c7f31e 100644 --- a/tests/test_archivar.py +++ b/tests/test_archivar.py @@ -33,12 +33,12 @@ def test_writes_header_and_rows(self, tmp_path): # Newlines inside content are escaped, not left as real line breaks. assert "hola\\ncon salto de linea" in lines[1] - def test_stops_and_returns_none_for_unsupported_channel_type(self, tmp_path): + def test_stops_and_returns_false_for_unsupported_channel_type(self, tmp_path): cog = Archivar(make_bot()) messages = [make_message(id=1, channel=make_dm_channel())] target = tmp_path / "archivo.csv" - assert cog.archivar_canal(str(target), messages) is None + assert cog.archivar_canal(str(target), messages) == (False, None) def test_write_failure_returns_false_none(self, tmp_path): cog = Archivar(make_bot()) @@ -48,17 +48,6 @@ def test_write_failure_returns_false_none(self, tmp_path): assert cog.archivar_canal(str(bad_target), messages) == (False, None) - def test_failure_tuple_is_still_truthy(self, tmp_path): - """Documents a real footgun in archivar(): ``status = archivar_canal(...)`` - followed by ``if status:`` treats a write failure as success, because - ``(False, None)`` is a non-empty tuple and therefore truthy in Python. - """ - cog = Archivar(make_bot()) - status = cog.archivar_canal(str(tmp_path), [make_message(id=1)]) - - assert status == (False, None) - assert bool(status) is True # the actual footgun - class TestArchivarCommand: async def test_sends_success_embed_with_the_file(self, tmp_path, monkeypatch): @@ -97,6 +86,32 @@ async def test_sends_error_embed_when_archiving_fails(self, tmp_path, monkeypatc (msg,), _ = mod_channel.send.call_args assert "Error" in msg + async def test_write_exception_reports_error_instead_of_crashing(self, tmp_path, monkeypatch): + """Regression test: archivar_canal() returning (False, None) on a + write failure used to be treated as success by ``if status:`` + (a non-empty tuple is always truthy), which would then try to + attach a file that was never written - crashing instead of just + reporting the error. + """ + monkeypatch.chdir(tmp_path) + mod_channel = make_text_channel(id=1, name="mod") + cog = bind_commands(Archivar(make_bot())) + cog.mod_channel = mod_channel + + # A channel name containing "/" makes the auto-generated filename + # point at a non-existent subdirectory, so open(filename, "w") fails. + channel = make_text_channel( + id=2, name="no-existe/canal", history_messages=[make_message(id=1)] + ) + ctx = make_ctx(channel=mod_channel) + + await cog.archivar(ctx, channel=channel) + + mod_channel.send.assert_awaited_once() + (msg,), kwargs = mod_channel.send.call_args + assert "Error" in msg + assert "file" not in kwargs + class TestArchivarCategoria: async def test_only_archives_text_channels(self, tmp_path, monkeypatch): diff --git a/tests/test_flood.py b/tests/test_flood.py index e8ff73c..7d61a14 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -399,6 +399,24 @@ async def test_no_attachments_means_no_warning_field(self, flood_cog): # on_message pipeline # --------------------------------------------------------------------------- class TestOnMessagePipeline: + async def test_uses_message_channel_directly_not_a_bot_cache_lookup(self, flood_cog): + """Regression test: on_message used to do + ``self._msg_channel = self.bot.get_channel(message.channel.id)`` + instead of just using ``message.channel``. A cache miss there made + ``_msg_channel`` None and crashed the first ``.send()`` downstream - + here the channel is never registered on the bot at all, so this + would fail the old way if the bug came back. + """ + member = make_member(name="repetidor") + message = make_message(content="discord nitro free http://x", author=member) + assert flood_cog.bot.get_channel(message.channel.id) is None + + await flood_cog.on_message(message) + + # The point here isn't *how many* times it's sent, just that it + # didn't crash trying to call .send() on a None channel. + message.channel.send.assert_awaited() + async def test_ignores_messages_from_bots(self, flood_cog): member = make_member(name="unbot", bot=True) message = make_message(content="discord nitro free http://x", author=member) @@ -457,11 +475,7 @@ async def test_known_spam_text_is_deleted_and_author_muted( ): flood_cog.messages.spam.add("mensaje ya conocido como spam") member = make_member(name="repetidor") - channel = make_text_channel(id=42) - flood_cog.bot.channels[channel.id] = channel - message = make_message( - content="Mensaje YA conocido como SPAM", author=member, channel=channel - ) + message = make_message(content="Mensaje YA conocido como SPAM", author=member) await flood_cog.on_message(message) diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py index 6fd066c..af91491 100644 --- a/tests/test_moderacion.py +++ b/tests/test_moderacion.py @@ -1,3 +1,4 @@ +from types import SimpleNamespace from unittest.mock import AsyncMock import pandas as pd @@ -216,7 +217,9 @@ async def test_removes_pending_row_and_notifies_channels( ): add_pending_row(moderacion_cog, post_id=1, author_id=99, content="contenido aprobado") moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") - moderacion_cog._msg_id = 12345 + moderacion_channels["main"].send = AsyncMock( + return_value=SimpleNamespace(jump_url="https://discord.com/channels/1/2/3") + ) ctx = make_ctx( author=make_member(name="moderador"), @@ -227,11 +230,18 @@ async def test_removes_pending_row_and_notifies_channels( await moderacion_cog._aceptar_mensaje(ctx) assert moderacion_cog.bot.data_mod.empty - moderacion_channels["mod"].send.assert_awaited_once() moderacion_channels["main"].send.assert_awaited_once() (msg,), _ = moderacion_channels["main"].send.call_args assert "contenido aprobado" in msg + # Regression test: the confirmation sent to the mod channel used to + # link to a jump_url built from self._msg_id (the *original + # submission's* id, in a different channel) instead of the message + # that was actually just posted to ch_main. + moderacion_channels["mod"].send.assert_awaited_once() + (mod_msg,), _ = moderacion_channels["mod"].send.call_args + assert "https://discord.com/channels/1/2/3" in mod_msg + async def test_unknown_post_id_does_not_touch_channels(self, moderacion_cog, moderacion_channels): ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 999") From 8dcfc116fd983d37e0e8ee3f447bffbaa51bbd91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 23:14:15 +0200 Subject: [PATCH 04/12] Remove eval() from moderacion.py's message encode/decode Messages were stored as base64, but via f"{base64.b64encode(...)}" - the repr of a bytes object (e.g. "b'aG9sYQ=='") - and reversed with eval() to get the bytes back before b64decode. Replace with two small helpers: - _encode_message(): base64-encodes and stores a plain ASCII string instead of a bytes repr. - _decode_message(): b64decode()s that string directly. Falls back to parsing the legacy bytes-repr format with ast.literal_eval() (not eval()) so pending rows already logged before this change still decode correctly instead of breaking on deploy. --- comandos/moderacion.py | 30 ++++++++++++++++++++++++++---- tests/factories.py | 15 +++++++++++---- tests/test_moderacion.py | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/comandos/moderacion.py b/comandos/moderacion.py index d89d154..f784f26 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -1,3 +1,4 @@ +import ast import asyncio import base64 import logging @@ -18,6 +19,27 @@ EMBED_COLOR = 0x2B597B +def _encode_message(content: str) -> str: + """Base64-encode a message's content for storage in data_mod/the log files.""" + return base64.b64encode(content.encode("utf-8")).decode("ascii") + + +def _decode_message(stored: str) -> str: + """Reverse ``_encode_message``. + + Also tolerates the legacy on-disk format: older code stored the + *repr* of the base64 `bytes` object (e.g. ``"b'aG9sYQ=='"``, via + ``f"{base64.b64encode(...)}"``) and reversed it with ``eval()``. Rows + written before this change still look like that, so a plain + ``b64decode`` fails validation and we fall back to safely parsing that + literal with ``ast.literal_eval`` instead - no ``eval()`` involved. + """ + try: + return base64.b64decode(stored, validate=True).decode("utf-8") + except ValueError: + return base64.b64decode(ast.literal_eval(stored)).decode("utf-8") + + @dataclass class ValidatedPost: post_id: str @@ -148,7 +170,7 @@ async def _get_validated_post( ]["submission"] ch_main, ch_mod, ch_sub = self.get_channels_main_mod_sub(channel_id) - message_dec = base64.b64decode(eval(mod_row["message"].values[0])).decode("utf-8") + message_dec = _decode_message(mod_row["message"].values[0]) author = self.bot.get_user(int(mod_row["author_id"].values[0])) return ValidatedPost( @@ -231,7 +253,7 @@ async def on_message(self, message): return self._msg_id = message.id - self._msg_enc = base64.b64encode(message.content.encode("utf-8")) + self._msg_enc = _encode_message(message.content) ch_main, ch_mod, ch_sub = self.get_channels_main_mod_sub(ch_id) self.log_on_message(ch_sub, message.author) @@ -327,7 +349,7 @@ def get_mod_pending(self, data): if not author: logger.warning("El author '%s' ya no existe en el server.", mod_row["author_id"]) continue - m_message = base64.b64decode(eval(mod_row["message"])).decode("utf-8") + m_message = _decode_message(mod_row["message"]) embed.add_field( name=f"ID: `{mod_row['message_id']}`", value=f"{m_message[:30]}...\nFecha: `{mod_row['date']}`\nAutor: {author.mention}", @@ -363,7 +385,7 @@ async def mostrar_mensajes(self, ctx): condition = self.bot.data_mod["message_id"] == post_id mod_row = self.bot.data_mod[condition] author = self.bot.get_user(int(mod_row["author_id"].values[0])) - m_message = base64.b64decode(eval(mod_row["message"].values[0])).decode("utf-8") + m_message = _decode_message(mod_row["message"].values[0]) embed = discord.Embed( title="Mensaje pendiente de moderación", diff --git a/tests/factories.py b/tests/factories.py index 4b08e0f..5cdb9f0 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -162,10 +162,17 @@ def make_interaction(user=None, channel=None): def encode_for_mod_row(text: str) -> str: - """Matches ``Moderacion.on_message``'s encoding of a message's content - into the ``data_mod``/log-file ``message`` column: a base64-encoded - ``bytes`` object rendered through an f-string (so later ``eval()``'d - back into a real ``bytes`` object by the accept/reject/list commands).""" + """Matches ``Moderacion``'s current encoding of a message's content into + the ``data_mod``/log-file ``message`` column: a plain base64 string.""" + import base64 + + return base64.b64encode(text.encode("utf-8")).decode("ascii") + + +def encode_for_mod_row_legacy(text: str) -> str: + """Matches the *old* (pre-fix) encoding: the repr of a base64 ``bytes`` + object, e.g. ``"b'aG9sYQ=='"``. Used to test that rows logged before + the eval()-removal fix still decode correctly.""" import base64 return f"{base64.b64encode(text.encode('utf-8'))}" diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py index af91491..b6be80b 100644 --- a/tests/test_moderacion.py +++ b/tests/test_moderacion.py @@ -4,8 +4,10 @@ import pandas as pd import pytest +from comandos.moderacion import _decode_message, _encode_message from tests.factories import ( encode_for_mod_row, + encode_for_mod_row_legacy, make_ctx, make_interaction, make_member, @@ -13,19 +15,38 @@ def add_pending_row(cog, post_id, *, channel="envio-eventos", author_id=42, author_name="autor", - content="contenido de prueba"): + content="contenido de prueba", legacy_encoding=False): + encode = encode_for_mod_row_legacy if legacy_encoding else encode_for_mod_row new_row = { "date": "2026-01-01 00:00:00", "message_id": str(post_id), "channel": channel, "author_id": str(author_id), "author": author_name, - "message": encode_for_mod_row(content), + "message": encode(content), } cog.bot.data_mod = pd.concat([cog.bot.data_mod, pd.DataFrame([new_row])], ignore_index=True) return new_row +class TestMessageEncoding: + def test_round_trips(self): + assert _decode_message(_encode_message("hola mundo")) == "hola mundo" + + def test_round_trips_accented_and_emoji(self): + text = "¿Cómo estás? 🎉" + assert _decode_message(_encode_message(text)) == text + + def test_decodes_the_legacy_bytes_repr_format(self): + """Rows written before the eval()-removal fix stored the repr of a + base64 bytes object (e.g. "b'aG9sYQ=='") instead of a plain base64 + string. _decode_message must still handle those without eval().""" + legacy = encode_for_mod_row_legacy("mensaje antiguo") + + assert legacy.startswith("b'") + assert _decode_message(legacy) == "mensaje antiguo" + + # --------------------------------------------------------------------------- # small helpers # --------------------------------------------------------------------------- @@ -121,6 +142,18 @@ async def test_happy_path_resolves_everything(self, moderacion_cog, moderacion_c assert vp.ch_sub is moderacion_channels["sub"] assert vp.author.id == 99 + async def test_resolves_a_row_logged_before_the_eval_removal_fix( + self, moderacion_cog, moderacion_channels + ): + add_pending_row(moderacion_cog, post_id=2, author_id=99, legacy_encoding=True) + moderacion_cog.bot.users_by_id[99] = make_member(id=99, name="remitente") + ctx = make_ctx(channel=moderacion_channels["mod"], content="%aceptar 2") + + vp = await moderacion_cog._get_validated_post(ctx, None, "%aceptar") + + assert vp is not None + assert vp.message_dec == "contenido de prueba" + async def test_unknown_post_id_reports_error_and_returns_none( self, moderacion_cog, moderacion_channels ): From f0351473453e5e92bce65cdf6185c806305f2da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 23:30:54 +0200 Subject: [PATCH 05/12] flood.py: read each attachment's bytes once, not up to 3 times attachment_check() previously called attachment.read() separately in the fast-path hash-check loop, again to cache the hash on a burst trigger, and a third time inside alert_moderation's sanitize step - up to 3 CDN round-trips per image for a single message. _hash_attachment/_sanitize_attachment (which took a discord.Attachment and read it themselves) are now _hash_bytes/_sanitize_bytes, operating on already-read bytes. attachment_check() reads each attachment once up front and reuses those bytes for hashing, caching, and (via alert_moderation's new image_bytes= parameter, replacing attachments=) sanitizing. --- comandos/flood.py | 38 ++++++++++++++++--------------- tests/test_flood.py | 55 +++++++++++++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/comandos/flood.py b/comandos/flood.py index 8bb168f..b8b0999 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -257,13 +257,12 @@ async def flood_check(self, message): await self._msg_channel.send(embed=embed, delete_after = 120) @staticmethod - async def _hash_attachment(attachment: discord.Attachment) -> str: - data = await attachment.read() + def _hash_bytes(data: bytes) -> str: return hashlib.sha256(data).hexdigest() @staticmethod - async def _sanitize_attachment(attachment: discord.Attachment) -> Optional[discord.File]: - """Decode and re-encode an attachment before it's shown to moderators. + async def _sanitize_bytes(data: bytes) -> Optional[discord.File]: + """Decode and re-encode image bytes before they're shown to moderators. Images from a compromised/malicious account are untrusted input: a crafted file could try to exploit a bug in whatever renders its @@ -271,10 +270,9 @@ async def _sanitize_attachment(attachment: discord.Attachment) -> Optional[disco via Pillow into a fresh PNG strips anything relying on a malformed file structure, and the result is still sent as a spoiler so viewing it requires an explicit click rather than an automatic preview. - Returns ``None`` if the attachment can't be safely decoded. + Returns ``None`` if the image can't be safely decoded. """ try: - data = await attachment.read() with Image.open(BytesIO(data)) as img: img.load() clean = img.convert("RGB") @@ -283,7 +281,7 @@ async def _sanitize_attachment(attachment: discord.Attachment) -> Optional[disco buf.seek(0) return discord.File(buf, filename="evidencia.png", spoiler=True) except (UnidentifiedImageError, OSError, ValueError): - logger.warning("_sanitize_attachment: could not decode %r, skipping", attachment.filename) + logger.warning("_sanitize_bytes: could not decode image, skipping") return None async def attachment_check(self, message: discord.Message) -> bool: @@ -307,14 +305,19 @@ async def attachment_check(self, message: discord.Message) -> bool: if not images: return False + # Read each attachment's bytes once and reuse them below for hashing + # and (if needed) sanitizing, instead of re-downloading from + # Discord's CDN for each separate step. + image_bytes = [await a.read() for a in images] + # Fast path: any image already known to be spam/scam - for attachment in images: - digest = await self._hash_attachment(attachment) + for data in image_bytes: + digest = self._hash_bytes(data) if digest in self.messages.image_spam: await self.alert_moderation( "Alerta de SPAM (Imagen conocida)", "known_image", - attachments=images, + image_bytes=image_bytes, ) # Set muted role @@ -353,16 +356,15 @@ async def attachment_check(self, message: discord.Message) -> bool: await self.alert_moderation( "Alerta de SPAM (Imágenes en varios canales)", "image_burst", - attachments=images, + image_bytes=image_bytes, ) # Set muted role await self._msg_author.add_roles(self.muted_role) # Cache the images involved so future occurrences hit the fast path - for attachment in images: - digest = await self._hash_attachment(attachment) - self.add_spam_image_hash(digest) + for data in image_bytes: + self.add_spam_image_hash(self._hash_bytes(data)) # Reset author's channel tracking now that we've acted on it self.messages.image_authors[self._msg_author] = {} @@ -423,7 +425,7 @@ def add_spam_image_hash(self, digest): f.write(f"{digest}\n") self.messages.image_spam.add(digest) - async def alert_moderation(self, title, reason, attachments=None): + async def alert_moderation(self, title, reason, image_bytes=None): logger.debug("alert_moderation: %s (%s)", title, reason) d_msg = { @@ -483,9 +485,9 @@ async def alert_moderation(self, title, reason, attachments=None): # a malformed file to exploit an image parser) and sent as a spoiler # so viewing them requires an explicit click. files = [] - if attachments: - for attachment in attachments: - sanitized = await self._sanitize_attachment(attachment) + if image_bytes: + for data in image_bytes: + sanitized = await self._sanitize_bytes(data) if sanitized is not None: files.append(sanitized) embed.add_field( diff --git a/tests/test_flood.py b/tests/test_flood.py index 7d61a14..98271bd 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -160,6 +160,32 @@ async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( class TestAttachmentCheckBurstPath: + async def test_each_attachment_is_only_downloaded_once(self, flood_cog): + """Regression test: within a single attachment_check() call, + attachment.read() used to be called once in the fast-path + hash-check loop, again to cache the hash on a burst trigger, and a + third time inside alert_moderation's sanitize step - up to 3 CDN + downloads per image for the one message that triggers the burst. + """ + member = make_member(name="comprometido") + first = make_message( + author=member, + channel=make_text_channel(id=1), + attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], + ) + prime_cog(flood_cog, first) + await flood_cog.attachment_check(first) + + b1 = make_attachment(filename="b1.png", data=make_png_bytes((255, 0, 0))) + b2 = make_attachment(filename="b2.png", data=make_png_bytes((0, 255, 0))) + second = make_message(author=member, channel=make_text_channel(id=2), attachments=[b1, b2]) + prime_cog(flood_cog, second) + result = await flood_cog.attachment_check(second) + + assert result is True # sanity check that the burst path actually ran + b1.read.assert_awaited_once() + b2.read.assert_awaited_once() + async def test_single_channel_two_images_does_not_trigger(self, flood_cog): member = make_member(name="autor") message = make_message( @@ -284,13 +310,11 @@ async def test_same_channel_twice_is_not_two_distinct_channels(self, flood_cog): # --------------------------------------------------------------------------- -# _sanitize_attachment / _hash_attachment +# _sanitize_bytes / _hash_bytes # --------------------------------------------------------------------------- -class TestSanitizeAttachment: +class TestSanitizeBytes: async def test_valid_image_round_trips_as_spoiler_file(self, flood_cog): - attachment = make_attachment(data=make_png_bytes()) - - result = await flood_cog._sanitize_attachment(attachment) + result = await flood_cog._sanitize_bytes(make_png_bytes()) assert result is not None assert isinstance(result, discord.File) @@ -298,24 +322,19 @@ async def test_valid_image_round_trips_as_spoiler_file(self, flood_cog): assert result.filename.endswith("evidencia.png") or "SPOILER" in result.filename async def test_garbage_bytes_returns_none(self, flood_cog): - attachment = make_attachment(data=b"not an image, just garbage" * 10) - - assert await flood_cog._sanitize_attachment(attachment) is None + assert await flood_cog._sanitize_bytes(b"not an image, just garbage" * 10) is None async def test_truncated_image_returns_none(self, flood_cog): - attachment = make_attachment(data=make_png_bytes()[:15]) - - assert await flood_cog._sanitize_attachment(attachment) is None + assert await flood_cog._sanitize_bytes(make_png_bytes()[:15]) is None -class TestHashAttachment: - async def test_matches_sha256_of_bytes(self, flood_cog): +class TestHashBytes: + def test_matches_sha256_of_bytes(self, flood_cog): import hashlib data = b"some bytes" - attachment = make_attachment(data=data) - assert await flood_cog._hash_attachment(attachment) == hashlib.sha256(data).hexdigest() + assert flood_cog._hash_bytes(data) == hashlib.sha256(data).hexdigest() # --------------------------------------------------------------------------- @@ -363,9 +382,8 @@ async def test_unknown_reason_raises(self, flood_cog): async def test_attachments_are_forwarded_sanitized_and_spoilered(self, flood_cog): message = make_message() prime_cog(flood_cog, message) - images = [make_attachment(data=make_png_bytes())] - await flood_cog.alert_moderation("Alerta", "known_image", attachments=images) + await flood_cog.alert_moderation("Alerta", "known_image", image_bytes=[make_png_bytes()]) thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args @@ -375,9 +393,8 @@ async def test_attachments_are_forwarded_sanitized_and_spoilered(self, flood_cog async def test_undecodable_attachment_is_skipped_not_forwarded(self, flood_cog): message = make_message() prime_cog(flood_cog, message) - images = [make_attachment(data=b"garbage" * 10)] - await flood_cog.alert_moderation("Alerta", "known_image", attachments=images) + await flood_cog.alert_moderation("Alerta", "known_image", image_bytes=[b"garbage" * 10]) thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args From e676f508128b230e7ee508a5bf5cae1ce7947c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 23:46:45 +0200 Subject: [PATCH 06/12] Add a shared colors.py, remove duplicated/scattered embed color literals WARNING_COLOR (flood.py) and EMBED_COLOR (moderacion.py) were both 0x2B597B, redefined under two different names; ayuda.py and utils.py had the same value hardcoded inline with no name at all. archivar.py, limpia.py, and enviar.py each had their own one-off hex literal too. Add colors.py with named constants (BRAND, ARCHIVE, SUCCESS, BROADCAST) and use them everywhere instead, so there's one place to look up or change any embed color in the project. --- colors.py | 21 +++++++++++++++++++++ comandos/archivar.py | 3 ++- comandos/ayuda.py | 5 +++-- comandos/enviar.py | 6 +++--- comandos/flood.py | 21 ++++++++++----------- comandos/limpia.py | 3 ++- comandos/moderacion.py | 11 +++++------ utils.py | 3 ++- 8 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 colors.py diff --git a/colors.py b/colors.py new file mode 100644 index 0000000..617c5c1 --- /dev/null +++ b/colors.py @@ -0,0 +1,21 @@ +"""Shared Discord embed colors, used across all cogs. + +Centralizing these avoids the same hex value being redefined under +different names in different modules (``WARNING_COLOR`` in flood.py and +``EMBED_COLOR`` in moderacion.py were both ``0x2B597B``), and gives every +other embed color in the project a name instead of leaving it as an +unexplained magic number. +""" + +# The project's primary/brand color - used for most informational and +# warning embeds: moderation alerts, help text, moderation confirmations. +BRAND = 0x2B597B + +# archivar.py's "channel archived" confirmation embed. +ARCHIVE = 0xFF0000 + +# limpia.py's "messages purged" confirmation embed. +SUCCESS = 0x178D38 + +# enviar.py's broadcast-message embeds. +BROADCAST = 0xFDC130 diff --git a/comandos/archivar.py b/comandos/archivar.py index 29702f9..028f609 100644 --- a/comandos/archivar.py +++ b/comandos/archivar.py @@ -5,6 +5,7 @@ import discord from discord.ext import commands +import colors from configuration import Config config = Config() @@ -36,7 +37,7 @@ async def archivar(self, ctx, *, channel: discord.TextChannel) -> Optional[disco e = discord.Embed( title="\N{PAGE FACING UP} Canal Archivado", description=f"El canal {channel.mention} tiene {len(messages)} mensajes", - colour=0xFF0000, + colour=colors.ARCHIVE, ) await self.mod_channel.send(embed=e, file=discord.File(archived_filename)) else: diff --git a/comandos/ayuda.py b/comandos/ayuda.py index b319f2c..677cf36 100644 --- a/comandos/ayuda.py +++ b/comandos/ayuda.py @@ -1,6 +1,7 @@ import discord from discord.ext import commands +import colors from configuration import Config from utils import get_moderation_channel @@ -31,7 +32,7 @@ async def mensaje_ayuda(self, ctx): def get_mod_help(self): embed = discord.Embed( title="Comandos Disponibles", - colour=0x2B597B, + colour=colors.BRAND, ) embed.add_field( name="`%mod`", @@ -66,7 +67,7 @@ def get_mod_help(self): def get_main_help(self): embed = discord.Embed( title="Comandos Disponibles", - colour=0x2B597B, + colour=colors.BRAND, ) embed.add_field( name='`%encuesta "pregunta"`', diff --git a/comandos/enviar.py b/comandos/enviar.py index ce69019..2aeeb67 100644 --- a/comandos/enviar.py +++ b/comandos/enviar.py @@ -1,10 +1,10 @@ from discord.ext import commands from discord import TextChannel, Embed, app_commands +import colors from configuration import Config config = Config() -COLOR_MSG = 0xfdc130 class Enviar(commands.Cog): @@ -22,7 +22,7 @@ async def enviar(self, ctx: commands.Context, channel: TextChannel, *, message: reply_embed = Embed( title=f"Mensaje enviado a {channel}", description=f"{channel.mention}:\n{message}", - colour=COLOR_MSG, + colour=colors.BROADCAST, ) try: @@ -33,7 +33,7 @@ async def enviar(self, ctx: commands.Context, channel: TextChannel, *, message: embed = Embed( title="Mensaje de Coordinación", description=message, - colour=COLOR_MSG, + colour=colors.BROADCAST, ) # Send the command to the channel passed to the command diff --git a/comandos/flood.py b/comandos/flood.py index b8b0999..62bfcf2 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -8,6 +8,7 @@ from discord.ext import commands, tasks from PIL import Image, UnidentifiedImageError +import colors from configuration import Config from messages import Messages from utils import strip_message @@ -33,8 +34,6 @@ ("gratis", "full", "youtube.com", "telegra.ph"), ] -WARNING_COLOR = 0x2B597B - # Modal view to 'ban' or 'remove role' from users that get reported # as spam. class ModActionView(discord.ui.View): @@ -156,7 +155,7 @@ async def on_message(self, message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SPAM", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) await self._msg_channel.send(embed=embed, delete_after = 60) @@ -172,7 +171,7 @@ async def on_message(self, message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SPAM", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) await self._msg_channel.send(embed=embed, delete_after=300) @@ -186,7 +185,7 @@ async def on_message(self, message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SCAM", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) await self._msg_channel.send(embed=embed, delete_after = 300) @@ -211,7 +210,7 @@ async def spam_check(self, message: discord.Message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SCAM", description=_msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) # Send message notifying the user is muted await message.channel.send(embed=embed, delete_after = 300) @@ -251,7 +250,7 @@ async def flood_check(self, message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SCAM", description=_msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) # Send message notifying the user is muted await self._msg_channel.send(embed=embed, delete_after = 120) @@ -331,7 +330,7 @@ async def attachment_check(self, message: discord.Message) -> bool: embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SPAM", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) await self._msg_channel.send(embed=embed, delete_after=60) return True @@ -379,7 +378,7 @@ async def attachment_check(self, message: discord.Message) -> bool: embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SCAM", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) await self._msg_channel.send(embed=embed, delete_after=300) return True @@ -406,7 +405,7 @@ async def mention_check(self, message): embed = discord.Embed( title="\N{NO ENTRY} Alerta de SPAM de menciones", description=_msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) # Send message notifying the user is muted await self._msg_channel.send(embed=embed, delete_after = 300) @@ -459,7 +458,7 @@ async def alert_moderation(self, title, reason, image_bytes=None): embed = discord.Embed( title=f"\N{NO ENTRY} {title}", description=msg, - colour=WARNING_COLOR, + colour=colors.BRAND, ) embed.add_field(name="Mensaje", value=f"`{repr(self._msg_content)[1:-1]}`", inline=False) embed.add_field( diff --git a/comandos/limpia.py b/comandos/limpia.py index 293a3a5..fd1d8bb 100644 --- a/comandos/limpia.py +++ b/comandos/limpia.py @@ -4,6 +4,7 @@ from discord.ext import commands from discord import app_commands +import colors from configuration import Config config = Config() @@ -50,7 +51,7 @@ async def purge(self, ctx: commands.Context, limit: int = 1) -> None: embed = discord.Embed( title=f"Borrados '{limit}' mensajes\n\n", description=f"Comando ejecuta por {ctx.author.mention}", - colour=0x178D38, + colour=colors.SUCCESS, ) await ctx.send(embed=embed, ephemeral=True) await channel.purge(limit=1) diff --git a/comandos/moderacion.py b/comandos/moderacion.py index f784f26..aede552 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -10,14 +10,13 @@ import discord from discord.ext import commands +import colors from configuration import Config from utils import get_moderation_channel, get_message_to_moderate, aceptar_emoji, rechazar_emoji config = Config() logger = logging.getLogger(__name__) -EMBED_COLOR = 0x2B597B - def _encode_message(content: str) -> str: """Base64-encode a message's content for storage in data_mod/the log files.""" @@ -261,7 +260,7 @@ async def on_message(self, message): embed = discord.Embed( title="Mensaje Enviado", description=f"Gracias {message.author.mention}, tu mensaje espera moderación.", - colour=EMBED_COLOR, + colour=colors.BRAND, ) reply_msg = await ch_sub.send(embed=embed) @@ -318,7 +317,7 @@ async def _rechazar_mensaje( embed = discord.Embed( title="Mensaje rechazado", description=f"{vp.author.mention} tu mensaje necesita atención.", - colour=EMBED_COLOR, + colour=colors.BRAND, ) embed.add_field( name="Razón rechazado", @@ -342,7 +341,7 @@ def get_mod_pending(self, data): messages = False embed = discord.Embed( title="Mensajes pendientes de moderación", - colour=EMBED_COLOR, + colour=colors.BRAND, ) for idx, mod_row in data.iterrows(): author = self.bot.get_user(int(mod_row["author_id"])) @@ -394,6 +393,6 @@ async def mostrar_mensajes(self, ctx): f"**ID:** {mod_row['message_id'].values[0]}\n" f"**Mensaje:**\n```\n{m_message}\n```\n" ), - colour=EMBED_COLOR, + colour=colors.BRAND, ) await channel_mod.send(embed=embed) diff --git a/utils.py b/utils.py index 41cb7c1..45defb9 100644 --- a/utils.py +++ b/utils.py @@ -2,6 +2,7 @@ import discord from datetime import datetime, timezone +import colors from configuration import Config config = Config() @@ -28,7 +29,7 @@ def get_message_to_moderate(message): embed = discord.Embed( title="Moderación de mensaje", description=msg, - colour=0x2B597B, + colour=colors.BRAND, ) return embed From a8a0e11d08048546b773c2a7cf741bf6bd987fc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Wed, 12 Aug 2026 23:55:37 +0200 Subject: [PATCH 07/12] flood.py: consistent SPAM/SCAM wording, notify line, safer message quoting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - flood_check (repeated messages) and attachment_check's image_burst path titled their public embeds "posible SCAM", despite being behavioral detections (repetition, image bursts) rather than the scam-link detection spam_check actually does. Retitled both to "posible SPAM", consistent with the other behavioral checks (mentions, known text/images). - known_image and image_burst's public notices were missing the "El equipo de coordinación ha sido notificado" line that every other mute-and-notify path already includes. - Replaced the repr(self._msg_content)[1:-1] quote-stripping hack in alert_moderation's "Mensaje" field with an explicit backtick escape (repr never actually escaped backticks, just stripped its own quote characters) and a "(sin texto)" placeholder for image-only messages instead of a blank code span. --- comandos/flood.py | 16 +++++++++++----- tests/test_flood.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/comandos/flood.py b/comandos/flood.py index 62bfcf2..d66e7c9 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -248,7 +248,7 @@ async def flood_check(self, message): "repetitivos. El equipo de coordinación ha sido notificado." ) embed = discord.Embed( - title="\N{NO ENTRY} Alerta de posible SCAM", + title="\N{NO ENTRY} Alerta de posible SPAM", description=_msg, colour=colors.BRAND, ) @@ -325,7 +325,8 @@ async def attachment_check(self, message: discord.Message) -> bool: await discord.Message.delete(message) msg = ( f"El mensaje del usuario {self._msg_author_mention} fue borrado por " - "contener una imagen detectada previamente como spam." + "contener una imagen detectada previamente como spam.\nEl equipo de " + "coordinación ha sido notificado." ) embed = discord.Embed( title="\N{NO ENTRY} Alerta de posible SPAM", @@ -373,10 +374,11 @@ async def attachment_check(self, message: discord.Message) -> bool: f"El mensaje del usuario {self._msg_author_mention} fue borrado por compartir " "imágenes en varios canales en poco tiempo, lo cual podría indicar una cuenta " "comprometida.\nEvita **hacer click** en enlaces o seguir instrucciones de " - "imágenes de **usuarios que no conozcas**." + "imágenes de **usuarios que no conozcas**.\nEl equipo de coordinación ha sido " + "notificado." ) embed = discord.Embed( - title="\N{NO ENTRY} Alerta de posible SCAM", + title="\N{NO ENTRY} Alerta de posible SPAM", description=msg, colour=colors.BRAND, ) @@ -460,7 +462,11 @@ async def alert_moderation(self, title, reason, image_bytes=None): description=msg, colour=colors.BRAND, ) - embed.add_field(name="Mensaje", value=f"`{repr(self._msg_content)[1:-1]}`", inline=False) + # Escape backticks so message content can't break out of the inline + # code span (repr(...)[1:-1] used to do this by stripping repr's + # quote characters - fragile, and didn't actually escape backticks). + safe_content = self._msg_content.replace("`", "'") if self._msg_content else "(sin texto)" + embed.add_field(name="Mensaje", value=f"`{safe_content}`", inline=False) embed.add_field( name="En caso de ser spam", value=( diff --git a/tests/test_flood.py b/tests/test_flood.py index 98271bd..2844c83 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -82,6 +82,12 @@ async def test_reaching_flood_limit_mutes_and_caches(self, flood_cog, config): assert "hola hola hola" in flood_cog.messages.spam # Counter resets after muting assert flood_cog.messages.normal[member] == {} + # Repeated messages are behavioral spam, not a scam-link detection - + # the public notice should say so consistently with the other + # behavioral checks (mentions, known text/images). + message.channel.send.assert_awaited_once() + _, kwargs = message.channel.send.call_args + assert kwargs["embed"].title.endswith("Alerta de posible SPAM") async def test_different_authors_counted_separately(self, flood_cog, config): alice = make_member(name="alice", id=1) @@ -158,6 +164,10 @@ async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( member.add_roles.assert_awaited_once_with(flood_cog.muted_role) patched_message_delete.assert_awaited_once_with(message) + message.channel.send.assert_awaited_once() + _, kwargs = message.channel.send.call_args + assert "equipo de coordinación ha sido notificado" in kwargs["embed"].description + class TestAttachmentCheckBurstPath: async def test_each_attachment_is_only_downloaded_once(self, flood_cog): @@ -241,6 +251,14 @@ async def test_two_images_two_channels_triggers_on_the_second_message( member.add_roles.assert_awaited_once_with(flood_cog.muted_role) patched_message_delete.assert_awaited_once_with(second) + # Consistent wording with the other behavioral (non scam-link) + # detections: "posible SPAM", and reassurance that the mod team + # was notified (alert_moderation posts to the mod thread). + second.channel.send.assert_awaited_once() + _, kwargs = second.channel.send.call_args + assert kwargs["embed"].title.endswith("Alerta de posible SPAM") + assert "equipo de coordinación ha sido notificado" in kwargs["embed"].description + async def test_images_get_cached_for_the_fast_path(self, flood_cog): member = make_member(name="comprometido") data_a, data_b = make_png_bytes((255, 0, 0)), make_png_bytes((0, 255, 0)) @@ -358,6 +376,33 @@ def test_add_spam_image_hash_persists_and_caches(self, flood_cog, isolated_logs) # alert_moderation # --------------------------------------------------------------------------- class TestAlertModeration: + async def test_backticks_in_content_do_not_break_the_code_span(self, flood_cog): + """Regression test: the old repr(self._msg_content)[1:-1] trick + stripped repr()'s own quote characters but never escaped backticks, + so a message containing one could break out of the inline code + span in the "Mensaje" field. + """ + message = make_message(content="mira este `codigo` raro") + prime_cog(flood_cog, message) + + await flood_cog.alert_moderation("Alerta", "scam") + + thread = flood_cog.main_mod_channel.create_thread.return_value + _, kwargs = thread.send.call_args + mensaje_field = next(f for f in kwargs["embed"].fields if f.name == "Mensaje") + assert mensaje_field.value.count("`") == 2 # only the wrapping backticks + + async def test_empty_content_shows_a_placeholder(self, flood_cog): + message = make_message(content="") + prime_cog(flood_cog, message) + + await flood_cog.alert_moderation("Alerta", "known_image") + + thread = flood_cog.main_mod_channel.create_thread.return_value + _, kwargs = thread.send.call_args + mensaje_field = next(f for f in kwargs["embed"].fields if f.name == "Mensaje") + assert "(sin texto)" in mensaje_field.value + async def test_creates_thread_and_sends_embed(self, flood_cog): member = make_member(name="alguien") message = make_message(author=member) From c8ae192233ff6b89d6d4ea949d52f2a6f68f4091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Thu, 13 Aug 2026 00:04:26 +0200 Subject: [PATCH 08/12] flood.py: replace shared per-message instance state with MessageContext FloodSpam previously threaded per-message data (channel/content/author/ author_mention) through self._msg_channel/_msg_content/_msg_author/ _msg_author_mention, set once at the top of on_message and read back by whichever check ran next. This made those methods hard to call/test independently (tests needed a prime_cog() helper to fake the setup), and - since real handling has plenty of await points in between - a second on_message() call for a *different* message running concurrently could overwrite that shared state while the first call was still relying on it. Introduce a small frozen MessageContext dataclass (message + stripped content, with channel/author/author_mention as properties) built once in on_message and passed explicitly to spam_check/flood_check/ mention_check/attachment_check/alert_moderation instead. No behavior change - same checks, same order, same messages. tests/factories.py's prime_cog() (which poked the old private attributes) is replaced with make_context(), building a real MessageContext the same way on_message does. --- comandos/flood.py | 187 +++++++++++++++++++++++++------------------- tests/factories.py | 16 ++-- tests/test_flood.py | 148 +++++++++++++++-------------------- 3 files changed, 178 insertions(+), 173 deletions(-) diff --git a/comandos/flood.py b/comandos/flood.py index d66e7c9..3c917f3 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -2,6 +2,7 @@ import logging import time +from dataclasses import dataclass from io import BytesIO import discord @@ -34,6 +35,38 @@ ("gratis", "full", "youtube.com", "telegra.ph"), ] + +@dataclass(frozen=True) +class MessageContext: + """Per-message state, threaded explicitly through the ``*_check`` + methods and ``alert_moderation`` as a parameter. + + This used to live on shared ``FloodSpam`` instance attributes + (``self._msg_channel``/``_msg_content``/``_msg_author``/ + ``_msg_author_mention``), set once at the top of ``on_message`` and + read back by whichever check ran next. That made those methods hard + to call/test independently, and - since real handling has plenty of + ``await`` points in between - a second ``on_message`` call for a + *different* message running concurrently could overwrite that shared + state while the first call was still relying on it. + """ + + message: discord.Message + content: str # message.content, stripped via strip_message() + + @property + def channel(self): + return self.message.channel + + @property + def author(self): + return self.message.author + + @property + def author_mention(self): + return self.message.author.mention + + # Modal view to 'ban' or 'remove role' from users that get reported # as spam. class ModActionView(discord.ui.View): @@ -68,11 +101,6 @@ def __init__(self, bot): self._coord_role: Optional[discord.Role] = None self._muted_role: Optional[discord.Role] = None - self._msg_channel: Optional[discord.TextChannel | discord.ForumChannel | discord.VoiceChannel] = None - self._msg_content: Optional[str] = None - self._msg_author: Optional[discord.Member] = None - self._msg_author_mention: Optional[str] = None - @property def muted_role(self) -> discord.Role: assert self._muted_role is not None, "Muted role not found - make sure it exists first" @@ -123,33 +151,32 @@ async def on_message(self, message): # (e.g. an image-only spam message has no text at all). if len(message.content) < 5 and not message.attachments: return - self._msg_channel = message.channel - self._msg_content = strip_message(message.content) - self._msg_author = message.author - self._msg_author_mention = self._msg_author.mention + + ctx = MessageContext(message=message, content=strip_message(message.content)) # skip coord role - if self.coord_role in self._msg_author.roles: + if self.coord_role in ctx.author.roles: return - if await self.attachment_check(message): + if await self.attachment_check(ctx): return - if await self.flood_check(message): + if await self.flood_check(ctx): return - if self._msg_content in self.messages.spam: + if ctx.content in self.messages.spam: await self.alert_moderation( + ctx, "Alerta de SPAM (Mensaje conocido)", "known", ) # Set muted role - await self._msg_author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) await discord.Message.delete(message) msg = ( - f"El mensaje del usuario {self._msg_author_mention} fue borrado por ser un " + f"El mensaje del usuario {ctx.author_mention} fue borrado por ser un " "mensaje detectado previamente como spam.\n" ) embed = discord.Embed( @@ -157,14 +184,14 @@ async def on_message(self, message): description=msg, colour=colors.BRAND, ) - await self._msg_channel.send(embed=embed, delete_after = 60) + await ctx.channel.send(embed=embed, delete_after = 60) # Check first more than 3 mentions - if await self.mention_check(message): - self.add_spam_message(self._msg_content) + if await self.mention_check(ctx): + self.add_spam_message(ctx.content) await discord.Message.delete(message) msg = ( - f"El mensaje del usuario {self._msg_author_mention} fue borrado por tener muchas " + f"El mensaje del usuario {ctx.author_mention} fue borrado por tener muchas " "menciones y podría ser un engaño.\nEvita `hacer click` en enlaces de " "**usuarios que no conozcas**." ) @@ -173,13 +200,13 @@ async def on_message(self, message): description=msg, colour=colors.BRAND, ) - await self._msg_channel.send(embed=embed, delete_after=300) + await ctx.channel.send(embed=embed, delete_after=300) - if await self.spam_check(message): - self.add_spam_message(self._msg_content) + if await self.spam_check(ctx): + self.add_spam_message(ctx.content) await discord.Message.delete(message) msg = ( - f"El mensaje del usuario {self._msg_author_mention} fue borrado y podría ser " + f"El mensaje del usuario {ctx.author_mention} fue borrado y podría ser " "un engaño.\nEvita `hacer click` en enlaces de **usuarios que no conozcas**." ) embed = discord.Embed( @@ -187,24 +214,22 @@ async def on_message(self, message): description=msg, colour=colors.BRAND, ) - await self._msg_channel.send(embed=embed, delete_after = 300) - - async def spam_check(self, message: discord.Message): - author = message.author + await ctx.channel.send(embed=embed, delete_after = 300) - if not isinstance(author, discord.Member): + async def spam_check(self, ctx: MessageContext): + if not isinstance(ctx.author, discord.Member): return - if not any(all(i in message.content for i in sw) for sw in SPAM_WORDS): + if not any(all(i in ctx.message.content for i in sw) for sw in SPAM_WORDS): return False - await self.alert_moderation("Alerta de SCAM", "scam") + await self.alert_moderation(ctx, "Alerta de SCAM", "scam") # Set muted role - await author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) _msg = ( - f"Usuario {author.mention} silenciado por compartir un mensaje que " + f"Usuario {ctx.author_mention} silenciado por compartir un mensaje que " "parece contener enlaces de engaño. El equipo de coordinación ha sido notificado." ) embed = discord.Embed( @@ -213,38 +238,39 @@ async def spam_check(self, message: discord.Message): colour=colors.BRAND, ) # Send message notifying the user is muted - await message.channel.send(embed=embed, delete_after = 300) + await ctx.channel.send(embed=embed, delete_after = 300) return True - async def flood_check(self, message): - logger.debug("flood_check: %s", message.id) + async def flood_check(self, ctx: MessageContext): + logger.debug("flood_check: %s", ctx.message.id) # Textless (image-only) messages are handled by attachment_check - if not self._msg_content: + if not ctx.content: return False - if self._msg_author not in self.messages.normal: - self.messages.normal[self._msg_author] = {self._msg_content: 1} + if ctx.author not in self.messages.normal: + self.messages.normal[ctx.author] = {ctx.content: 1} else: - if self._msg_content not in self.messages.normal[self._msg_author]: - self.messages.normal[self._msg_author][self._msg_content] = 1 + if ctx.content not in self.messages.normal[ctx.author]: + self.messages.normal[ctx.author][ctx.content] = 1 else: - self.messages.normal[self._msg_author][self._msg_content] += 1 - if self.messages.normal[self._msg_author][self._msg_content] >= config.FLOOD_LIMIT: - self.add_spam_message(self._msg_content) + self.messages.normal[ctx.author][ctx.content] += 1 + if self.messages.normal[ctx.author][ctx.content] >= config.FLOOD_LIMIT: + self.add_spam_message(ctx.content) await self.alert_moderation( + ctx, "Alerta de Flood", "flood", ) # Set muted role - await self._msg_author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) # Reset author counters - self.messages.normal[self._msg_author] = {} + self.messages.normal[ctx.author] = {} _msg = ( - f"Usuario {self._msg_author_mention} silenciado por enviar mensajes " + f"Usuario {ctx.author_mention} silenciado por enviar mensajes " "repetitivos. El equipo de coordinación ha sido notificado." ) embed = discord.Embed( @@ -253,7 +279,7 @@ async def flood_check(self, message): colour=colors.BRAND, ) # Send message notifying the user is muted - await self._msg_channel.send(embed=embed, delete_after = 120) + await ctx.channel.send(embed=embed, delete_after = 120) @staticmethod def _hash_bytes(data: bytes) -> str: @@ -283,7 +309,7 @@ async def _sanitize_bytes(data: bytes) -> Optional[discord.File]: logger.warning("_sanitize_bytes: could not decode image, skipping") return None - async def attachment_check(self, message: discord.Message) -> bool: + async def attachment_check(self, ctx: MessageContext) -> bool: """Detect image-based spam/scam bursts from (often compromised) accounts. Two mechanisms: @@ -295,10 +321,10 @@ async def attachment_check(self, message: discord.Message) -> bool: same images across the server. When this fires, the offending images are hashed and cached for the fast path above. """ - logger.debug("attachment_check: %s", message.id) + logger.debug("attachment_check: %s", ctx.message.id) images = [ - a for a in message.attachments + a for a in ctx.message.attachments if (a.content_type or "").startswith("image/") ] if not images: @@ -314,17 +340,18 @@ async def attachment_check(self, message: discord.Message) -> bool: digest = self._hash_bytes(data) if digest in self.messages.image_spam: await self.alert_moderation( + ctx, "Alerta de SPAM (Imagen conocida)", "known_image", image_bytes=image_bytes, ) # Set muted role - await self._msg_author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) - await discord.Message.delete(message) + await discord.Message.delete(ctx.message) msg = ( - f"El mensaje del usuario {self._msg_author_mention} fue borrado por " + f"El mensaje del usuario {ctx.author_mention} fue borrado por " "contener una imagen detectada previamente como spam.\nEl equipo de " "coordinación ha sido notificado." ) @@ -333,7 +360,7 @@ async def attachment_check(self, message: discord.Message) -> bool: description=msg, colour=colors.BRAND, ) - await self._msg_channel.send(embed=embed, delete_after=60) + await ctx.channel.send(embed=embed, delete_after=60) return True if len(images) < config.IMAGE_ATTACHMENT_LIMIT: @@ -341,37 +368,38 @@ async def attachment_check(self, message: discord.Message) -> bool: # Burst path: same author, 2+ images, 2+ different channels, short window now = time.time() - channels = self.messages.image_authors.get(self._msg_author, {}) + channels = self.messages.image_authors.get(ctx.author, {}) channels = { channel_id: ts for channel_id, ts in channels.items() if now - ts <= config.IMAGE_BURST_WINDOW } - channels[message.channel.id] = now - self.messages.image_authors[self._msg_author] = channels + channels[ctx.channel.id] = now + self.messages.image_authors[ctx.author] = channels if len(channels) < 2: return False await self.alert_moderation( + ctx, "Alerta de SPAM (Imágenes en varios canales)", "image_burst", image_bytes=image_bytes, ) # Set muted role - await self._msg_author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) # Cache the images involved so future occurrences hit the fast path for data in image_bytes: self.add_spam_image_hash(self._hash_bytes(data)) # Reset author's channel tracking now that we've acted on it - self.messages.image_authors[self._msg_author] = {} + self.messages.image_authors[ctx.author] = {} - await discord.Message.delete(message) + await discord.Message.delete(ctx.message) msg = ( - f"El mensaje del usuario {self._msg_author_mention} fue borrado por compartir " + f"El mensaje del usuario {ctx.author_mention} fue borrado por compartir " "imágenes en varios canales en poco tiempo, lo cual podría indicar una cuenta " "comprometida.\nEvita **hacer click** en enlaces o seguir instrucciones de " "imágenes de **usuarios que no conozcas**.\nEl equipo de coordinación ha sido " @@ -382,26 +410,27 @@ async def attachment_check(self, message: discord.Message) -> bool: description=msg, colour=colors.BRAND, ) - await self._msg_channel.send(embed=embed, delete_after=300) + await ctx.channel.send(embed=embed, delete_after=300) return True - async def mention_check(self, message): - logger.debug("mention_check: %s", message.id) + async def mention_check(self, ctx: MessageContext): + logger.debug("mention_check: %s", ctx.message.id) # Skip if 2 mentions or less - if (len(message.mentions) + len(message.role_mentions)) < config.MENTIONS_LIMIT: + if (len(ctx.message.mentions) + len(ctx.message.role_mentions)) < config.MENTIONS_LIMIT: return False await self.alert_moderation( + ctx, "Alerta de Flood (Menciones)", "menciones", ) # Set muted role - await self._msg_author.add_roles(self.muted_role) + await ctx.author.add_roles(self.muted_role) _msg = ( - f"Usuario {self._msg_author_mention} silenciado por hacer muchas menciones. " + f"Usuario {ctx.author_mention} silenciado por hacer muchas menciones. " "El equipo de coordinación ha sido notificado." ) embed = discord.Embed( @@ -410,7 +439,7 @@ async def mention_check(self, message): colour=colors.BRAND, ) # Send message notifying the user is muted - await self._msg_channel.send(embed=embed, delete_after = 300) + await ctx.channel.send(embed=embed, delete_after = 300) return True @@ -426,33 +455,33 @@ def add_spam_image_hash(self, digest): f.write(f"{digest}\n") self.messages.image_spam.add(digest) - async def alert_moderation(self, title, reason, image_bytes=None): + async def alert_moderation(self, ctx: MessageContext, title, reason, image_bytes=None): logger.debug("alert_moderation: %s (%s)", title, reason) d_msg = { "menciones": ( f"{self.coord_role.mention} Se detectó un mensaje con muchas menciones " - f"de {self._msg_author_mention} y se ha muteado." + f"de {ctx.author_mention} y se ha muteado." ), "flood": ( f"{self.coord_role.mention} Se detectaron mensajes repetitivos de " - f"{self._msg_author_mention} y se ha muteado." + f"{ctx.author_mention} y se ha muteado." ), "scam": ( f"{self.coord_role.mention} Se detectó un mensaje de SCAM de " - f"{self._msg_author_mention} y se ha muteado." + f"{ctx.author_mention} y se ha muteado." ), "known": ( f"{self.coord_role.mention} Se detectó un mensaje previamente reconocido " - f"como spam de {self._msg_author_mention} y se ha muteado." + f"como spam de {ctx.author_mention} y se ha muteado." ), "known_image": ( f"{self.coord_role.mention} Se detectó una imagen previamente reconocida " - f"como spam/scam de {self._msg_author_mention} y se ha muteado." + f"como spam/scam de {ctx.author_mention} y se ha muteado." ), "image_burst": ( f"{self.coord_role.mention} Se detectaron imágenes enviadas por " - f"{self._msg_author_mention} en varios canales en poco tiempo " + f"{ctx.author_mention} en varios canales en poco tiempo " "(posible cuenta comprometida) y se ha muteado." ), } @@ -465,7 +494,7 @@ async def alert_moderation(self, title, reason, image_bytes=None): # Escape backticks so message content can't break out of the inline # code span (repr(...)[1:-1] used to do this by stripping repr's # quote characters - fragile, and didn't actually escape backticks). - safe_content = self._msg_content.replace("`", "'") if self._msg_content else "(sin texto)" + safe_content = ctx.content.replace("`", "'") if ctx.content else "(sin texto)" embed.add_field(name="Mensaje", value=f"`{safe_content}`", inline=False) embed.add_field( name="En caso de ser spam", @@ -505,7 +534,7 @@ async def alert_moderation(self, title, reason, image_bytes=None): inline=False, ) - view = ModActionView(self._msg_author, self._muted_role) - thread = await self.main_mod_channel.create_thread(name=f"{title} - {self._msg_author_mention}", + view = ModActionView(ctx.author, self._muted_role) + thread = await self.main_mod_channel.create_thread(name=f"{title} - {ctx.author_mention}", auto_archive_duration=60, type=discord.ChannelType.public_thread) - await thread.send(embed=embed, view=view, files=files) \ No newline at end of file + await thread.send(embed=embed, view=view, files=files) diff --git a/tests/factories.py b/tests/factories.py index 5cdb9f0..9463205 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -201,11 +201,11 @@ def bind_commands(cog): return cog -def prime_cog(cog, message): - """Mirror the attribute setup ``FloodSpam.on_message`` does before - delegating to its individual ``*_check`` methods, so those methods can - be unit-tested directly without going through the full listener.""" - cog._msg_channel = message.channel - cog._msg_content = strip_message(message.content) - cog._msg_author = message.author - cog._msg_author_mention = message.author.mention +def make_context(message): + """Build the ``MessageContext`` ``FloodSpam.on_message`` would build + before delegating to its individual ``*_check`` methods, so those + methods can be unit-tested directly without going through the full + listener.""" + from comandos.flood import MessageContext + + return MessageContext(message=message, content=strip_message(message.content)) diff --git a/tests/test_flood.py b/tests/test_flood.py index 2844c83..8e62f63 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -5,11 +5,11 @@ from tests.factories import ( make_attachment, + make_context, make_member, make_message, make_png_bytes, make_text_channel, - prime_cog, ) @@ -20,13 +20,14 @@ class TestSpamCheck: async def test_ignores_non_member_author(self, flood_cog): message = make_message(content="discord nitro free http://evil") message.author = object() # not a discord.Member + ctx = make_context(message) - assert await flood_cog.spam_check(message) is None + assert await flood_cog.spam_check(ctx) is None async def test_no_match_returns_false(self, flood_cog): - message = make_message(content="hola a todos, buen dia") + ctx = make_context(make_message(content="hola a todos, buen dia")) - assert await flood_cog.spam_check(message) is False + assert await flood_cog.spam_check(ctx) is False @pytest.mark.parametrize( "content", @@ -38,9 +39,9 @@ async def test_no_match_returns_false(self, flood_cog): async def test_match_mutes_and_notifies(self, flood_cog, content): member = make_member(name="victima") message = make_message(content=content, author=member) - prime_cog(flood_cog, message) + ctx = make_context(message) - result = await flood_cog.spam_check(message) + result = await flood_cog.spam_check(ctx) assert result is True member.add_roles.assert_awaited_once_with(flood_cog.muted_role) @@ -54,29 +55,28 @@ async def test_match_mutes_and_notifies(self, flood_cog, content): # --------------------------------------------------------------------------- class TestFloodCheck: async def test_empty_content_is_a_noop(self, flood_cog): - message = make_message(content="") - prime_cog(flood_cog, message) + ctx = make_context(make_message(content="")) - assert await flood_cog.flood_check(message) is False + assert await flood_cog.flood_check(ctx) is False assert flood_cog.messages.normal == {} async def test_below_flood_limit_does_not_mute(self, flood_cog, config): member = make_member(name="repetidor") message = make_message(content="hola hola hola", author=member) - prime_cog(flood_cog, message) + ctx = make_context(message) for _ in range(config.FLOOD_LIMIT - 1): - await flood_cog.flood_check(message) + await flood_cog.flood_check(ctx) member.add_roles.assert_not_awaited() async def test_reaching_flood_limit_mutes_and_caches(self, flood_cog, config): member = make_member(name="repetidor") message = make_message(content="hola hola hola", author=member) - prime_cog(flood_cog, message) + ctx = make_context(message) for _ in range(config.FLOOD_LIMIT): - await flood_cog.flood_check(message) + await flood_cog.flood_check(ctx) member.add_roles.assert_awaited_once_with(flood_cog.muted_role) assert "hola hola hola" in flood_cog.messages.spam @@ -94,13 +94,11 @@ async def test_different_authors_counted_separately(self, flood_cog, config): bob = make_member(name="bob", id=2) for _ in range(config.FLOOD_LIMIT - 1): - msg = make_message(content="mismo mensaje", author=alice) - prime_cog(flood_cog, msg) - await flood_cog.flood_check(msg) + ctx = make_context(make_message(content="mismo mensaje", author=alice)) + await flood_cog.flood_check(ctx) - msg = make_message(content="mismo mensaje", author=bob) - prime_cog(flood_cog, msg) - await flood_cog.flood_check(msg) + ctx = make_context(make_message(content="mismo mensaje", author=bob)) + await flood_cog.flood_check(ctx) alice.add_roles.assert_not_awaited() bob.add_roles.assert_not_awaited() @@ -112,28 +110,27 @@ async def test_different_authors_counted_separately(self, flood_cog, config): class TestMentionCheck: async def test_below_limit_returns_false(self, flood_cog, config): mentions = [make_member(id=i) for i in range(config.MENTIONS_LIMIT - 1)] - message = make_message(mentions=mentions) - prime_cog(flood_cog, message) + ctx = make_context(make_message(mentions=mentions)) - assert await flood_cog.mention_check(message) is False + assert await flood_cog.mention_check(ctx) is False async def test_at_limit_mutes_and_alerts(self, flood_cog, config): member = make_member(name="mencionador") mentions = [make_member(id=i) for i in range(config.MENTIONS_LIMIT)] - message = make_message(author=member, mentions=mentions) - prime_cog(flood_cog, message) + ctx = make_context(make_message(author=member, mentions=mentions)) - assert await flood_cog.mention_check(message) is True + assert await flood_cog.mention_check(ctx) is True member.add_roles.assert_awaited_once_with(flood_cog.muted_role) async def test_mentions_and_role_mentions_add_up(self, flood_cog, config): member = make_member(name="mencionador") mentions = [make_member(id=1)] role_mentions = [object() for _ in range(config.MENTIONS_LIMIT - 1)] - message = make_message(author=member, mentions=mentions, role_mentions=role_mentions) - prime_cog(flood_cog, message) + ctx = make_context( + make_message(author=member, mentions=mentions, role_mentions=role_mentions) + ) - assert await flood_cog.mention_check(message) is True + assert await flood_cog.mention_check(ctx) is True # --------------------------------------------------------------------------- @@ -141,10 +138,9 @@ async def test_mentions_and_role_mentions_add_up(self, flood_cog, config): # --------------------------------------------------------------------------- class TestAttachmentCheckFastPath: async def test_no_images_returns_false(self, flood_cog): - message = make_message(attachments=[make_attachment(content_type="text/plain")]) - prime_cog(flood_cog, message) + ctx = make_context(make_message(attachments=[make_attachment(content_type="text/plain")])) - assert await flood_cog.attachment_check(message) is False + assert await flood_cog.attachment_check(ctx) is False async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( self, flood_cog, patched_message_delete @@ -158,9 +154,9 @@ async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( author=member, attachments=[make_attachment(data=data)], ) - prime_cog(flood_cog, message) + ctx = make_context(message) - assert await flood_cog.attachment_check(message) is True + assert await flood_cog.attachment_check(ctx) is True member.add_roles.assert_awaited_once_with(flood_cog.muted_role) patched_message_delete.assert_awaited_once_with(message) @@ -183,14 +179,12 @@ async def test_each_attachment_is_only_downloaded_once(self, flood_cog): channel=make_text_channel(id=1), attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], ) - prime_cog(flood_cog, first) - await flood_cog.attachment_check(first) + await flood_cog.attachment_check(make_context(first)) b1 = make_attachment(filename="b1.png", data=make_png_bytes((255, 0, 0))) b2 = make_attachment(filename="b2.png", data=make_png_bytes((0, 255, 0))) second = make_message(author=member, channel=make_text_channel(id=2), attachments=[b1, b2]) - prime_cog(flood_cog, second) - result = await flood_cog.attachment_check(second) + result = await flood_cog.attachment_check(make_context(second)) assert result is True # sanity check that the burst path actually ran b1.read.assert_awaited_once() @@ -202,9 +196,8 @@ async def test_single_channel_two_images_does_not_trigger(self, flood_cog): author=member, attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], ) - prime_cog(flood_cog, message) - assert await flood_cog.attachment_check(message) is False + assert await flood_cog.attachment_check(make_context(message)) is False member.add_roles.assert_not_awaited() async def test_single_image_across_channels_does_not_trigger(self, flood_cog): @@ -216,8 +209,7 @@ async def test_single_image_across_channels_does_not_trigger(self, flood_cog): message = make_message( author=member, channel=channel, attachments=[make_attachment()] ) - prime_cog(flood_cog, message) - assert await flood_cog.attachment_check(message) is False + assert await flood_cog.attachment_check(make_context(message)) is False member.add_roles.assert_not_awaited() @@ -233,16 +225,14 @@ async def test_two_images_two_channels_triggers_on_the_second_message( channel=channel_a, attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], ) - prime_cog(flood_cog, first) - first_result = await flood_cog.attachment_check(first) + first_result = await flood_cog.attachment_check(make_context(first)) second = make_message( author=member, channel=channel_b, attachments=[make_attachment(filename="b1.png"), make_attachment(filename="b2.png")], ) - prime_cog(flood_cog, second) - second_result = await flood_cog.attachment_check(second) + second_result = await flood_cog.attachment_check(make_context(second)) # The first channel's message is never retroactively touched - only # the message that crosses the 2-channel threshold gets acted on. @@ -268,16 +258,14 @@ async def test_images_get_cached_for_the_fast_path(self, flood_cog): channel=make_text_channel(id=1), attachments=[make_attachment(data=data_a), make_attachment(data=data_b)], ) - prime_cog(flood_cog, first) - await flood_cog.attachment_check(first) + await flood_cog.attachment_check(make_context(first)) second = make_message( author=member, channel=make_text_channel(id=2), attachments=[make_attachment(data=data_a), make_attachment(data=data_b)], ) - prime_cog(flood_cog, second) - await flood_cog.attachment_check(second) + await flood_cog.attachment_check(make_context(second)) import hashlib @@ -296,16 +284,14 @@ async def test_outside_burst_window_does_not_trigger(self, flood_cog, config, mo channel=make_text_channel(id=1), attachments=[make_attachment(filename="a1.png"), make_attachment(filename="a2.png")], ) - prime_cog(flood_cog, first) - await flood_cog.attachment_check(first) + await flood_cog.attachment_check(make_context(first)) second = make_message( author=member, channel=make_text_channel(id=2), attachments=[make_attachment(filename="b1.png"), make_attachment(filename="b2.png")], ) - prime_cog(flood_cog, second) - result = await flood_cog.attachment_check(second) + result = await flood_cog.attachment_check(make_context(second)) assert result is False member.add_roles.assert_not_awaited() @@ -320,8 +306,7 @@ async def test_same_channel_twice_is_not_two_distinct_channels(self, flood_cog): channel=channel, attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], ) - prime_cog(flood_cog, message) - result = await flood_cog.attachment_check(message) + result = await flood_cog.attachment_check(make_context(message)) assert result is False member.add_roles.assert_not_awaited() @@ -382,10 +367,9 @@ async def test_backticks_in_content_do_not_break_the_code_span(self, flood_cog): so a message containing one could break out of the inline code span in the "Mensaje" field. """ - message = make_message(content="mira este `codigo` raro") - prime_cog(flood_cog, message) + ctx = make_context(make_message(content="mira este `codigo` raro")) - await flood_cog.alert_moderation("Alerta", "scam") + await flood_cog.alert_moderation(ctx, "Alerta", "scam") thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args @@ -393,10 +377,9 @@ async def test_backticks_in_content_do_not_break_the_code_span(self, flood_cog): assert mensaje_field.value.count("`") == 2 # only the wrapping backticks async def test_empty_content_shows_a_placeholder(self, flood_cog): - message = make_message(content="") - prime_cog(flood_cog, message) + ctx = make_context(make_message(content="")) - await flood_cog.alert_moderation("Alerta", "known_image") + await flood_cog.alert_moderation(ctx, "Alerta", "known_image") thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args @@ -405,10 +388,9 @@ async def test_empty_content_shows_a_placeholder(self, flood_cog): async def test_creates_thread_and_sends_embed(self, flood_cog): member = make_member(name="alguien") - message = make_message(author=member) - prime_cog(flood_cog, message) + ctx = make_context(make_message(author=member)) - await flood_cog.alert_moderation("Alerta de prueba", "scam") + await flood_cog.alert_moderation(ctx, "Alerta de prueba", "scam") flood_cog.main_mod_channel.create_thread.assert_awaited_once() _, kwargs = flood_cog.main_mod_channel.create_thread.call_args @@ -418,17 +400,15 @@ async def test_creates_thread_and_sends_embed(self, flood_cog): thread.send.assert_awaited_once() async def test_unknown_reason_raises(self, flood_cog): - message = make_message() - prime_cog(flood_cog, message) + ctx = make_context(make_message()) with pytest.raises(KeyError): - await flood_cog.alert_moderation("Título", "no-existe") + await flood_cog.alert_moderation(ctx, "Título", "no-existe") async def test_attachments_are_forwarded_sanitized_and_spoilered(self, flood_cog): - message = make_message() - prime_cog(flood_cog, message) + ctx = make_context(make_message()) - await flood_cog.alert_moderation("Alerta", "known_image", image_bytes=[make_png_bytes()]) + await flood_cog.alert_moderation(ctx, "Alerta", "known_image", image_bytes=[make_png_bytes()]) thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args @@ -436,20 +416,18 @@ async def test_attachments_are_forwarded_sanitized_and_spoilered(self, flood_cog assert kwargs["files"][0].spoiler is True async def test_undecodable_attachment_is_skipped_not_forwarded(self, flood_cog): - message = make_message() - prime_cog(flood_cog, message) + ctx = make_context(make_message()) - await flood_cog.alert_moderation("Alerta", "known_image", image_bytes=[b"garbage" * 10]) + await flood_cog.alert_moderation(ctx, "Alerta", "known_image", image_bytes=[b"garbage" * 10]) thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args assert kwargs["files"] == [] async def test_no_attachments_means_no_warning_field(self, flood_cog): - message = make_message() - prime_cog(flood_cog, message) + ctx = make_context(make_message()) - await flood_cog.alert_moderation("Alerta", "scam") + await flood_cog.alert_moderation(ctx, "Alerta", "scam") thread = flood_cog.main_mod_channel.create_thread.return_value _, kwargs = thread.send.call_args @@ -501,8 +479,9 @@ async def test_ignores_short_textless_messages_without_attachments(self, flood_c await flood_cog.on_message(message) - # Never even gets far enough to set up per-message state. - assert flood_cog._msg_author is None + # Never even gets far enough to build a MessageContext or touch state. + message.channel.send.assert_not_awaited() + assert flood_cog.messages.image_authors == {} async def test_short_caption_with_attachments_is_still_processed(self, flood_cog): member = make_member(name="alguien") @@ -514,9 +493,10 @@ async def test_short_caption_with_attachments_is_still_processed(self, flood_cog await flood_cog.on_message(message) - # It went through the pipeline (attachment_check saw it), even though - # the caption alone would have been skipped. - assert flood_cog._msg_author is member + # It went through the pipeline (attachment_check saw it and recorded + # this channel for the burst-tracking window), even though the + # caption alone would have been skipped. + assert member in flood_cog.messages.image_authors async def test_skips_coordination_role_members(self, flood_cog): message = make_message( @@ -524,11 +504,7 @@ async def test_skips_coordination_role_members(self, flood_cog): author=make_member(name="mod", roles=[flood_cog.coord_role]), ) - result = None - try: - result = await flood_cog.on_message(message) - finally: - pass + await flood_cog.on_message(message) message.author.add_roles.assert_not_awaited() From f08114ffa56a4eab97a25116fefa6a1d68420863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Thu, 13 Aug 2026 00:09:54 +0200 Subject: [PATCH 09/12] Remove get_moderation_channel wrapper, fold Messages dataclass into FloodSpam - get_moderation_channel(bot, channel_id) was a same-signature wrapper around bot.get_channel() with a name that implied more than it did (it doesn't look up "the mod channel", it returns whatever channel id it's given). Inlined at all 5 call sites, removed from utils.py. - messages.py's Messages dataclass was only ever used by FloodSpam (spam/normal/image_spam/image_authors), accessed everywhere through an extra self.messages.x layer of indirection. Folded those four fields directly onto FloodSpam as plain attributes and deleted messages.py. No behavior change - same lookups, same state, just fewer names to track for something only one cog ever used. --- comandos/ayuda.py | 3 +-- comandos/flood.py | 44 +++++++++++++++++++++--------------------- comandos/moderacion.py | 10 +++++----- messages.py | 10 ---------- tests/test_flood.py | 22 ++++++++++----------- tests/test_utils.py | 14 +------------- utils.py | 4 ---- 7 files changed, 40 insertions(+), 67 deletions(-) delete mode 100644 messages.py diff --git a/comandos/ayuda.py b/comandos/ayuda.py index 677cf36..4375e28 100644 --- a/comandos/ayuda.py +++ b/comandos/ayuda.py @@ -3,7 +3,6 @@ import colors from configuration import Config -from utils import get_moderation_channel config = Config() @@ -20,7 +19,7 @@ async def mensaje_ayuda(self, ctx): # Check which channel combination we are using from the # configuration information - channel_mod = get_moderation_channel(self.bot, ctx.channel.id) + channel_mod = self.bot.get_channel(ctx.channel.id) if channel_mod: e = self.get_mod_help() diff --git a/comandos/flood.py b/comandos/flood.py index 3c917f3..5cc5993 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -11,7 +11,6 @@ import colors from configuration import Config -from messages import Messages from utils import strip_message from typing import Optional @@ -91,11 +90,12 @@ def __init__(self, bot): self.bot = bot self._main_mod_channel: Optional[discord.TextChannel] = None - self.messages = Messages() - self.messages.spam = config.get_spam_messages() - self.messages.normal = {} - self.messages.image_spam = config.get_spam_image_hashes() - self.messages.image_authors = {} + # Known spam/scam text and image hashes, plus the short-lived + # per-author tracking used to detect floods and image bursts. + self.spam = config.get_spam_messages() + self.normal = {} + self.image_spam = config.get_spam_image_hashes() + self.image_authors = {} self.guild = None self._coord_role: Optional[discord.Role] = None @@ -136,8 +136,8 @@ async def on_ready(self): # Remove messages every hour @tasks.loop(seconds=60 * 30) async def clear_messages(self): - self.messages.normal = {} - self.messages.image_authors = {} + self.normal = {} + self.image_authors = {} @commands.Cog.listener() async def on_message(self, message): @@ -164,7 +164,7 @@ async def on_message(self, message): if await self.flood_check(ctx): return - if ctx.content in self.messages.spam: + if ctx.content in self.spam: await self.alert_moderation( ctx, "Alerta de SPAM (Mensaje conocido)", @@ -248,14 +248,14 @@ async def flood_check(self, ctx: MessageContext): if not ctx.content: return False - if ctx.author not in self.messages.normal: - self.messages.normal[ctx.author] = {ctx.content: 1} + if ctx.author not in self.normal: + self.normal[ctx.author] = {ctx.content: 1} else: - if ctx.content not in self.messages.normal[ctx.author]: - self.messages.normal[ctx.author][ctx.content] = 1 + if ctx.content not in self.normal[ctx.author]: + self.normal[ctx.author][ctx.content] = 1 else: - self.messages.normal[ctx.author][ctx.content] += 1 - if self.messages.normal[ctx.author][ctx.content] >= config.FLOOD_LIMIT: + self.normal[ctx.author][ctx.content] += 1 + if self.normal[ctx.author][ctx.content] >= config.FLOOD_LIMIT: self.add_spam_message(ctx.content) await self.alert_moderation( ctx, @@ -267,7 +267,7 @@ async def flood_check(self, ctx: MessageContext): await ctx.author.add_roles(self.muted_role) # Reset author counters - self.messages.normal[ctx.author] = {} + self.normal[ctx.author] = {} _msg = ( f"Usuario {ctx.author_mention} silenciado por enviar mensajes " @@ -338,7 +338,7 @@ async def attachment_check(self, ctx: MessageContext) -> bool: # Fast path: any image already known to be spam/scam for data in image_bytes: digest = self._hash_bytes(data) - if digest in self.messages.image_spam: + if digest in self.image_spam: await self.alert_moderation( ctx, "Alerta de SPAM (Imagen conocida)", @@ -368,14 +368,14 @@ async def attachment_check(self, ctx: MessageContext) -> bool: # Burst path: same author, 2+ images, 2+ different channels, short window now = time.time() - channels = self.messages.image_authors.get(ctx.author, {}) + channels = self.image_authors.get(ctx.author, {}) channels = { channel_id: ts for channel_id, ts in channels.items() if now - ts <= config.IMAGE_BURST_WINDOW } channels[ctx.channel.id] = now - self.messages.image_authors[ctx.author] = channels + self.image_authors[ctx.author] = channels if len(channels) < 2: return False @@ -395,7 +395,7 @@ async def attachment_check(self, ctx: MessageContext) -> bool: self.add_spam_image_hash(self._hash_bytes(data)) # Reset author's channel tracking now that we've acted on it - self.messages.image_authors[ctx.author] = {} + self.image_authors[ctx.author] = {} await discord.Message.delete(ctx.message) msg = ( @@ -447,13 +447,13 @@ def add_spam_message(self, message): logger.info("add_spam_message: %r", message) with open(config.log_spam_file, "a") as f: f.write(f"{message}\n") - self.messages.spam.add(message) + self.spam.add(message) def add_spam_image_hash(self, digest): logger.info("add_spam_image_hash: %s", digest) with open(config.log_image_spam_file, "a") as f: f.write(f"{digest}\n") - self.messages.image_spam.add(digest) + self.image_spam.add(digest) async def alert_moderation(self, ctx: MessageContext, title, reason, image_bytes=None): logger.debug("alert_moderation: %s (%s)", title, reason) diff --git a/comandos/moderacion.py b/comandos/moderacion.py index aede552..5e3bc50 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -12,7 +12,7 @@ import colors from configuration import Config -from utils import get_moderation_channel, get_message_to_moderate, aceptar_emoji, rechazar_emoji +from utils import get_message_to_moderate, aceptar_emoji, rechazar_emoji config = Config() logger = logging.getLogger(__name__) @@ -112,7 +112,7 @@ def _is_bot(self, ctx) -> bool: return self._resolve_author(ctx).id == config.BOT_ID def _is_valid_channel(self, ctx) -> bool: - channel_mod = get_moderation_channel(self.bot, ctx.channel.id) + channel_mod = self.bot.get_channel(ctx.channel.id) return channel_mod.id == ctx.message.channel.id def get_channels_main_mod_sub(self, channel_id): @@ -125,7 +125,7 @@ async def _parse_post_id( self, ctx, message_id: Optional[int], command_name: str ) -> Optional[str]: """Parse and validate the post_id from interaction or command message.""" - channel_mod = get_moderation_channel(self.bot, ctx.channel.id) + channel_mod = self.bot.get_channel(ctx.channel.id) if isinstance(ctx, discord.Interaction) and message_id is not None: return str(message_id) @@ -151,7 +151,7 @@ async def _get_validated_post( if self._is_bot(ctx) or not self._is_valid_channel(ctx): return None - channel_mod = get_moderation_channel(self.bot, ctx.channel.id) + channel_mod = self.bot.get_channel(ctx.channel.id) post_id = await self._parse_post_id(ctx, message_id, command_name) if post_id is None: @@ -366,7 +366,7 @@ async def mostrar_mensajes(self, ctx): if self._is_bot(ctx) or not self._is_valid_channel(ctx): return - channel_mod = get_moderation_channel(self.bot, ctx.channel.id) + channel_mod = self.bot.get_channel(ctx.channel.id) _post = ctx.message.content.replace("%mod", "").strip().split() if not _post: diff --git a/messages.py b/messages.py deleted file mode 100644 index 915b7a6..0000000 --- a/messages.py +++ /dev/null @@ -1,10 +0,0 @@ -from dataclasses import dataclass, field -from typing import Set, Dict, Any - - -@dataclass -class Messages: - spam: Set = field(default_factory=set) - normal: Dict[Any, Any] = field(default_factory=dict) - image_spam: Set = field(default_factory=set) - image_authors: Dict[Any, Any] = field(default_factory=dict) diff --git a/tests/test_flood.py b/tests/test_flood.py index 8e62f63..c4ea6fe 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -58,7 +58,7 @@ async def test_empty_content_is_a_noop(self, flood_cog): ctx = make_context(make_message(content="")) assert await flood_cog.flood_check(ctx) is False - assert flood_cog.messages.normal == {} + assert flood_cog.normal == {} async def test_below_flood_limit_does_not_mute(self, flood_cog, config): member = make_member(name="repetidor") @@ -79,9 +79,9 @@ async def test_reaching_flood_limit_mutes_and_caches(self, flood_cog, config): await flood_cog.flood_check(ctx) member.add_roles.assert_awaited_once_with(flood_cog.muted_role) - assert "hola hola hola" in flood_cog.messages.spam + assert "hola hola hola" in flood_cog.spam # Counter resets after muting - assert flood_cog.messages.normal[member] == {} + assert flood_cog.normal[member] == {} # Repeated messages are behavioral spam, not a scam-link detection - # the public notice should say so consistently with the other # behavioral checks (mentions, known text/images). @@ -147,7 +147,7 @@ async def test_known_hash_mutes_and_deletes_regardless_of_channel_count( ): data = make_png_bytes() digest = __import__("hashlib").sha256(data).hexdigest() - flood_cog.messages.image_spam.add(digest) + flood_cog.image_spam.add(digest) member = make_member(name="reincidente") message = make_message( @@ -269,8 +269,8 @@ async def test_images_get_cached_for_the_fast_path(self, flood_cog): import hashlib - assert hashlib.sha256(data_a).hexdigest() in flood_cog.messages.image_spam - assert hashlib.sha256(data_b).hexdigest() in flood_cog.messages.image_spam + assert hashlib.sha256(data_a).hexdigest() in flood_cog.image_spam + assert hashlib.sha256(data_b).hexdigest() in flood_cog.image_spam async def test_outside_burst_window_does_not_trigger(self, flood_cog, config, monkeypatch): import comandos.flood as flood_module @@ -347,13 +347,13 @@ class TestAddSpamHelpers: def test_add_spam_message_persists_and_caches(self, flood_cog, isolated_logs): flood_cog.add_spam_message("mensaje malo") - assert "mensaje malo" in flood_cog.messages.spam + assert "mensaje malo" in flood_cog.spam assert "mensaje malo" in isolated_logs.log_spam_file.read_text() def test_add_spam_image_hash_persists_and_caches(self, flood_cog, isolated_logs): flood_cog.add_spam_image_hash("deadbeef") - assert "deadbeef" in flood_cog.messages.image_spam + assert "deadbeef" in flood_cog.image_spam assert "deadbeef" in isolated_logs.log_image_spam_file.read_text() @@ -481,7 +481,7 @@ async def test_ignores_short_textless_messages_without_attachments(self, flood_c # Never even gets far enough to build a MessageContext or touch state. message.channel.send.assert_not_awaited() - assert flood_cog.messages.image_authors == {} + assert flood_cog.image_authors == {} async def test_short_caption_with_attachments_is_still_processed(self, flood_cog): member = make_member(name="alguien") @@ -496,7 +496,7 @@ async def test_short_caption_with_attachments_is_still_processed(self, flood_cog # It went through the pipeline (attachment_check saw it and recorded # this channel for the burst-tracking window), even though the # caption alone would have been skipped. - assert member in flood_cog.messages.image_authors + assert member in flood_cog.image_authors async def test_skips_coordination_role_members(self, flood_cog): message = make_message( @@ -511,7 +511,7 @@ async def test_skips_coordination_role_members(self, flood_cog): async def test_known_spam_text_is_deleted_and_author_muted( self, flood_cog, patched_message_delete ): - flood_cog.messages.spam.add("mensaje ya conocido como spam") + flood_cog.spam.add("mensaje ya conocido como spam") member = make_member(name="repetidor") message = make_message(content="Mensaje YA conocido como SPAM", author=member) diff --git a/tests/test_utils.py b/tests/test_utils.py index 2ac0a48..dbd772d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,5 @@ from tests.factories import make_message -from utils import get_message_to_moderate, get_moderation_channel, strip_message +from utils import get_message_to_moderate, strip_message class TestStripMessage: @@ -25,18 +25,6 @@ def test_empty_string(self): assert strip_message("") == "" -class TestGetModerationChannel: - def test_returns_bot_get_channel_result(self): - sentinel = object() - - class FakeBot: - def get_channel(self, channel_id): - assert channel_id == 42 - return sentinel - - assert get_moderation_channel(FakeBot(), 42) is sentinel - - class TestGetMessageToModerate: def test_embed_contains_message_and_commands(self): message = make_message(content="hola, este es mi post") diff --git a/utils.py b/utils.py index 45defb9..18cf793 100644 --- a/utils.py +++ b/utils.py @@ -11,10 +11,6 @@ aceptar_emoji = "\N{WHITE HEAVY CHECK MARK}" rechazar_emoji = "\N{CROSS MARK}" -def get_moderation_channel(bot, channel_id): - channel_mod = bot.get_channel(channel_id) - return channel_mod - def get_message_to_moderate(message): msg = ( From 97b4f210ca4416597f959a464a6b80e60466d081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Thu, 13 Aug 2026 00:17:01 +0200 Subject: [PATCH 10/12] moderacion.py: drop _is_valid_channel It compared bot.get_channel(ctx.channel.id) against ctx's own channel - always true, since ctx.channel is already resolved from that same id. It never actually validated anything; the real gate on aceptar/rechazar/mod is @commands.has_role(config.MOD_ROLE). --- comandos/moderacion.py | 8 ++------ tests/factories.py | 3 --- tests/test_moderacion.py | 7 ------- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/comandos/moderacion.py b/comandos/moderacion.py index 5e3bc50..6a47e94 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -111,10 +111,6 @@ def _resolve_author(self, ctx) -> discord.User | discord.Member: def _is_bot(self, ctx) -> bool: return self._resolve_author(ctx).id == config.BOT_ID - def _is_valid_channel(self, ctx) -> bool: - channel_mod = self.bot.get_channel(ctx.channel.id) - return channel_mod.id == ctx.message.channel.id - def get_channels_main_mod_sub(self, channel_id): channel_main = self.bot.get_channel(self.channels[channel_id]["main"]) channel_mod = self.bot.get_channel(self.channels[channel_id]["mod"]) @@ -148,7 +144,7 @@ async def _get_validated_post( - Resolves channels and decodes the message Returns a ValidatedPost or None if any step fails. """ - if self._is_bot(ctx) or not self._is_valid_channel(ctx): + if self._is_bot(ctx): return None channel_mod = self.bot.get_channel(ctx.channel.id) @@ -363,7 +359,7 @@ def get_mod_pending(self, data): @commands.command(name="mod", help="Comando para listar los mensajes pendientes") @commands.has_role(config.MOD_ROLE) async def mostrar_mensajes(self, ctx): - if self._is_bot(ctx) or not self._is_valid_channel(ctx): + if self._is_bot(ctx): return channel_mod = self.bot.get_channel(ctx.channel.id) diff --git a/tests/factories.py b/tests/factories.py index 9463205..9260e94 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -152,9 +152,6 @@ def make_interaction(user=None, channel=None): interaction = MagicMock(spec=discord.Interaction) interaction.user = user if user is not None else make_member() interaction.channel = channel if channel is not None else make_text_channel() - # `_is_valid_channel` compares `channel_mod.id == ctx.message.channel.id` - # even for interactions, so this needs to line up with `.channel` too. - interaction.message = SimpleNamespace(channel=interaction.channel) interaction.response = MagicMock() interaction.response.send_message = AsyncMock() interaction.response.send_modal = AsyncMock() diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py index b6be80b..0d9ac4b 100644 --- a/tests/test_moderacion.py +++ b/tests/test_moderacion.py @@ -76,13 +76,6 @@ def test_false_for_regular_user(self, moderacion_cog): assert moderacion_cog._is_bot(ctx) is False -class TestIsValidChannel: - def test_true_when_channel_is_registered_on_the_bot(self, moderacion_cog, moderacion_channels): - ctx = make_ctx(channel=moderacion_channels["mod"]) - - assert moderacion_cog._is_valid_channel(ctx) is True - - class TestGetChannelsMainModSub: def test_resolves_the_three_channels(self, moderacion_cog, moderacion_channels): main, mod, sub = moderacion_cog.get_channels_main_mod_sub(moderacion_channels["sub"].id) From 9f6091a67e6b335e191820be22bfb841ccb5268b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Thu, 13 Aug 2026 00:21:23 +0200 Subject: [PATCH 11/12] ayuda.py: always show mod help, drop dead %encuesta help text mensaje_ayuda's channel check (bot.get_channel(ctx.channel.id), always truthy since ctx.channel already comes from that id) made get_mod_help the only reachable branch in practice. The other branch, get_main_help, documented %encuesta - a command removed in f97c8ae. Simplified to always send the mod help and deleted the dead branch/content. --- comandos/ayuda.py | 36 +----------------------------------- tests/test_ayuda.py | 37 ++++++------------------------------- 2 files changed, 7 insertions(+), 66 deletions(-) diff --git a/comandos/ayuda.py b/comandos/ayuda.py index 4375e28..607ac6e 100644 --- a/comandos/ayuda.py +++ b/comandos/ayuda.py @@ -17,16 +17,7 @@ async def mensaje_ayuda(self, ctx): if ctx.author.id == config.BOT_ID: return - # Check which channel combination we are using from the - # configuration information - channel_mod = self.bot.get_channel(ctx.channel.id) - - if channel_mod: - e = self.get_mod_help() - await channel_mod.send(embed=e) - else: - e = self.get_main_help() - await ctx.channel.send(embed=e) + await ctx.channel.send(embed=self.get_mod_help()) def get_mod_help(self): embed = discord.Embed( @@ -62,28 +53,3 @@ def get_mod_help(self): inline=False, ) return embed - - def get_main_help(self): - embed = discord.Embed( - title="Comandos Disponibles", - colour=colors.BRAND, - ) - embed.add_field( - name='`%encuesta "pregunta"`', - value=( - "Para hacer preguntas de Sí y No.\n" 'Ejemplo:\n `%encuesta "¿Te gusta el té?"`' - ), - inline=False, - ) - embed.add_field( - name='`%encuesta "pregunta" "opción a" "opción b" ...`', - value=( - "Para hacer preguntas con varias opciones.\n" - 'Ejemplo:\n `%encuesta "¿Lenguaje favorito?" "Inglés" "Español" "Python"`' - ), - inline=False, - ) - embed.set_footer( - text='Importante: La pregunta y opciones deben ir entre comillas dobles "..."' - ) - return embed diff --git a/tests/test_ayuda.py b/tests/test_ayuda.py index a81fedd..624bfd4 100644 --- a/tests/test_ayuda.py +++ b/tests/test_ayuda.py @@ -1,5 +1,5 @@ from comandos.ayuda import Ayuda -from tests.factories import bind_commands, make_bot, make_ctx, make_member, make_text_channel +from tests.factories import bind_commands, make_bot, make_ctx, make_member class TestGetModHelp: @@ -14,15 +14,6 @@ def test_lists_moderation_commands(self): assert "`%rechazar ID RAZON`" in names -class TestGetMainHelp: - def test_lists_encuesta_usage(self): - cog = Ayuda(make_bot()) - - embed = cog.get_main_help() - - assert any("encuesta" in f.name for f in embed.fields) - - class TestMensajeAyuda: async def test_ignores_the_bot_itself(self, config): cog = bind_commands(Ayuda(make_bot())) @@ -32,30 +23,14 @@ async def test_ignores_the_bot_itself(self, config): ctx.channel.send.assert_not_awaited() - async def test_sends_mod_help_inside_a_moderation_channel(self): - mod_channel = make_text_channel(id=1, name="mod") - bot = make_bot(channels={mod_channel.id: mod_channel}) - cog = bind_commands(Ayuda(bot)) - ctx = make_ctx(channel=mod_channel) + async def test_sends_mod_help(self): + cog = bind_commands(Ayuda(make_bot())) + ctx = make_ctx() await cog.mensaje_ayuda(ctx) - mod_channel.send.assert_awaited_once() - _, kwargs = mod_channel.send.call_args + ctx.channel.send.assert_awaited_once() + _, kwargs = ctx.channel.send.call_args assert kwargs["embed"].title == "Comandos Disponibles" names = [f.name for f in kwargs["embed"].fields] assert "`%mod`" in names - - async def test_sends_main_help_outside_a_moderation_channel(self): - # bot.get_channel(ctx.channel.id) returns None - not a known channel. - bot = make_bot(channels={}) - cog = bind_commands(Ayuda(bot)) - channel = make_text_channel(id=99, name="general") - ctx = make_ctx(channel=channel) - - await cog.mensaje_ayuda(ctx) - - channel.send.assert_awaited_once() - _, kwargs = channel.send.call_args - names = [f.name for f in kwargs["embed"].fields] - assert any("encuesta" in name for name in names) From a6e2b2d3f1340aa9abb992a3039034043cb339df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Sat, 15 Aug 2026 18:40:49 +0200 Subject: [PATCH 12/12] Use csv.writer for log files instead of hand-built quoting Hand-rolled f'"{value}"' quoting didn't escape embedded quotes or newlines, both common in real message/display-name content. Since log_mod_file/accepted/rejected are re-parsed with pd.read_csv on every startup, a single bad row could break loading the pending queue. Also fixes two related bugs while touching these writers: - bot.py's main log wrote 6 fields against a 7-column header (missing "command"), now written as an explicit empty field. - an empty rechazar reason used to skip the "reason" column entirely, misaligning it against log_rejected_file's fixed 8-column header; now always written (empty or not). --- bot.py | 25 ++++++++++--------- comandos/moderacion.py | 53 +++++++++++++++++++++------------------- tests/test_moderacion.py | 53 +++++++++++++++++++++++++++++++++++----- 3 files changed, 89 insertions(+), 42 deletions(-) diff --git a/bot.py b/bot.py index b3e316b..e9be85b 100644 --- a/bot.py +++ b/bot.py @@ -1,4 +1,5 @@ import asyncio +import csv import pandas as pd import discord import logging @@ -38,18 +39,20 @@ @bot.event async def on_message(message: discord.Message): - # Main log - with open(config.log_main_file, "a") as f: + # Main log. Uses csv.writer (not hand-built quoting) so a message + # containing a literal '"' or a newline - both common in real messages - + # doesn't silently corrupt the row. + with open(config.log_main_file, "a", newline="") as f: date_str = f"{datetime.now()}" - line = ( - f'"{date_str}";' - f'"{message.id}";' - f'"{message.channel}";' - f'"{message.author.id}";' - f'"{message.author}";' - f'"{message.content}"\n' - ) - f.write(line) + csv.writer(f, delimiter=";").writerow([ + date_str, + "", # command - this log covers every message, not just commands + message.id, + message.channel, + message.author.id, + message.author, + message.content, + ]) @bot.event diff --git a/comandos/moderacion.py b/comandos/moderacion.py index 6a47e94..2f67be6 100644 --- a/comandos/moderacion.py +++ b/comandos/moderacion.py @@ -1,6 +1,7 @@ import ast import asyncio import base64 +import csv import logging from datetime import datetime from dataclasses import dataclass @@ -183,26 +184,34 @@ def _log_action(self, action: str, row, post_id, moderator, reason: str = ""): """ Unified log writer for accept/reject actions. action: "aceptar" or "rechazar" + + Uses csv.writer (not hand-built quoting) so a channel/author name or + message containing a literal '"' or newline doesn't silently corrupt + the row - these files are re-parsed with pd.read_csv on every bot + startup, so a malformed row there can break loading the pending + queue. """ filename = ( config.log_accepted_file if action == "aceptar" else config.log_rejected_file ) date_str = f"{datetime.now()}" - line = ( - f'"{date_str}";' - f'"{post_id}";' - f'"{row["channel"].values[0]}";' - f'"{row["author_id"].values[0]}";' - f'"{row["author"].values[0]}";' - f'"{row["message"].values[0]}";' - f'"{moderator}"' - ) - if reason: - line += f';"{reason}"' - line += "\n" - - with open(str(filename), "a") as f: - f.write(line) + fields = [ + date_str, + post_id, + row["channel"].values[0], + row["author_id"].values[0], + row["author"].values[0], + row["message"].values[0], + moderator, + ] + # log_rejected_file's header always has a "reason" column - include + # it (even if empty) for every rechazar row, not just when reason is + # truthy, so the column count always matches the header. + if action != "aceptar": + fields.append(reason) + + with open(str(filename), "a", newline="") as f: + csv.writer(f, delimiter=";").writerow(fields) def log_on_message(self, channel_sub, author): date_str = f"{datetime.now()}" @@ -216,16 +225,10 @@ def log_on_message(self, channel_sub, author): } self.bot.data_mod = pd.concat([self.bot.data_mod, pd.DataFrame([new_data])]) - line = ( - f'"{date_str}";' - f'"{self._msg_id}";' - f'"{channel_sub}";' - f'"{author.id}";' - f'"{author}";' - f'"{self._msg_enc}"\n' - ) - with open(str(config.log_mod_file), "a") as f: - f.write(line) + with open(str(config.log_mod_file), "a", newline="") as f: + csv.writer(f, delimiter=";").writerow([ + date_str, self._msg_id, channel_sub, author.id, author, self._msg_enc, + ]) @commands.Cog.listener() async def on_ready(self): diff --git a/tests/test_moderacion.py b/tests/test_moderacion.py index 0d9ac4b..5ec9411 100644 --- a/tests/test_moderacion.py +++ b/tests/test_moderacion.py @@ -1,3 +1,4 @@ +import csv from types import SimpleNamespace from unittest.mock import AsyncMock @@ -29,6 +30,15 @@ def add_pending_row(cog, post_id, *, channel="envio-eventos", author_id=42, auth return new_row +def read_last_csv_row(path, delimiter=";"): + """isolated_logs seeds each log file with a blank line instead of the + real header, so skip empty rows and return the last one actually + written.""" + with path.open(newline="") as f: + rows = [row for row in csv.reader(f, delimiter=delimiter) if row] + return rows[-1] + + class TestMessageEncoding: def test_round_trips(self): assert _decode_message(_encode_message("hola mundo")) == "hola mundo" @@ -179,17 +189,47 @@ def test_aceptar_writes_expected_line(self, moderacion_cog, isolated_logs): moderacion_cog._log_action("aceptar", row, "1", "moderador#0") - content = isolated_logs.log_accepted_file.read_text() - assert '"1"' in content - assert '"moderador#0"' in content + fields = read_last_csv_row(isolated_logs.log_accepted_file) + assert fields[1] == "1" # post_id + assert fields[6] == "moderador#0" # moderator + assert len(fields) == 7 # no "reason" column for aceptar def test_rechazar_includes_reason(self, moderacion_cog, isolated_logs): row = pd.DataFrame([add_pending_row(moderacion_cog, post_id=2)]) moderacion_cog._log_action("rechazar", row, "2", "moderador#0", "le falta info") - content = isolated_logs.log_rejected_file.read_text() - assert '"le falta info"' in content + fields = read_last_csv_row(isolated_logs.log_rejected_file) + assert fields[-1] == "le falta info" + + def test_rechazar_without_reason_still_writes_the_reason_column( + self, moderacion_cog, isolated_logs + ): + """Regression test: an empty reason used to skip the "reason" field + entirely (``if reason: line += ...``), leaving that row one column + short of log_rejected_file's fixed 8-column header - which + pd.read_csv (run on every bot startup) can choke on. + """ + row = pd.DataFrame([add_pending_row(moderacion_cog, post_id=3)]) + + moderacion_cog._log_action("rechazar", row, "3", "moderador#0", "") + + fields = read_last_csv_row(isolated_logs.log_rejected_file) + assert len(fields) == 8 + assert fields[-1] == "" + + def test_embedded_quotes_and_delimiters_round_trip(self, moderacion_cog, isolated_logs): + """Regression test: hand-built '"{value}"' quoting didn't escape + embedded quotes/delimiters, silently corrupting the row. A proper + csv.writer round-trips this correctly. + """ + tricky_name = 'mod "raro"; con punto y coma' + row = pd.DataFrame([add_pending_row(moderacion_cog, post_id=4, author_name=tricky_name)]) + + moderacion_cog._log_action("aceptar", row, "4", tricky_name) + + fields = read_last_csv_row(isolated_logs.log_accepted_file) + assert fields[6] == tricky_name class TestLogOnMessage: @@ -202,7 +242,8 @@ def test_appends_row_and_writes_log_line(self, moderacion_cog, isolated_logs): moderacion_cog.log_on_message("envio-eventos", author) assert len(moderacion_cog.bot.data_mod) == before + 1 - assert "777" in isolated_logs.log_mod_file.read_text() + fields = read_last_csv_row(isolated_logs.log_mod_file) + assert fields[1] == "777" # ---------------------------------------------------------------------------