From 91e1bd4315c1e0ce8c0697804223b9b5119570b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Mon, 17 Aug 2026 00:13:34 +0200 Subject: [PATCH] flood.py: clean up every channel an image-spam burst touched attachment_check's burst path only deleted the one message that happened to cross the 2-channel threshold, leaving earlier copies of the same spam images up in whichever channels they were posted to first - a compromised account spraying images across channels could beat detection (and the resulting mute) to several of them before getting caught. image_authors now tracks the actual per-channel message (not just a timestamp), so once a burst is confirmed, every tracked channel gets its message deleted and the "cuenta comprometida" warning posted - not just the triggering one. Deletion tolerates a message already being gone (e.g. removed manually first) rather than crashing, and still warns that channel regardless. The mod-thread alert title now also reports how many channels were affected. --- comandos/flood.py | 40 ++++++++++++++++------ tests/test_flood.py | 83 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/comandos/flood.py b/comandos/flood.py index 5cc5993..e343584 100644 --- a/comandos/flood.py +++ b/comandos/flood.py @@ -368,21 +368,21 @@ async def attachment_check(self, ctx: MessageContext) -> bool: # Burst path: same author, 2+ images, 2+ different channels, short window now = time.time() - 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 + tracked = self.image_authors.get(ctx.author, {}) + tracked = { + channel_id: entry + for channel_id, entry in tracked.items() + if now - entry["ts"] <= config.IMAGE_BURST_WINDOW } - channels[ctx.channel.id] = now - self.image_authors[ctx.author] = channels + tracked[ctx.channel.id] = {"message": ctx.message, "ts": now} + self.image_authors[ctx.author] = tracked - if len(channels) < 2: + if len(tracked) < 2: return False await self.alert_moderation( ctx, - "Alerta de SPAM (Imágenes en varios canales)", + f"Alerta de SPAM (Imágenes en {len(tracked)} canales)", "image_burst", image_bytes=image_bytes, ) @@ -397,7 +397,11 @@ async def attachment_check(self, ctx: MessageContext) -> bool: # Reset author's channel tracking now that we've acted on it self.image_authors[ctx.author] = {} - await discord.Message.delete(ctx.message) + # Clean up every channel the burst touched, not just the message + # that happened to cross the 2-channel threshold. A compromised + # account spraying the same images across channels can beat the + # detection to several of them before the mute above lands - leaving + # those copies up defeats the point of catching this at all. msg = ( 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 " @@ -410,9 +414,23 @@ async def attachment_check(self, ctx: MessageContext) -> bool: description=msg, colour=colors.BRAND, ) - await ctx.channel.send(embed=embed, delete_after=300) + for entry in tracked.values(): + await self._delete_and_warn(entry["message"], embed) + return True + @staticmethod + async def _delete_and_warn(message: discord.Message, embed: discord.Embed): + """Delete ``message`` (tolerating it already being gone) and post + ``embed`` in its channel either way - bystanders in that channel + should still see the compromised-account warning even if the + message itself was already removed by someone else.""" + try: + await discord.Message.delete(message) + except discord.NotFound: + logger.debug("_delete_and_warn: message %s already gone", message.id) + await message.channel.send(embed=embed, delete_after=300) + async def mention_check(self, ctx: MessageContext): logger.debug("mention_check: %s", ctx.message.id) diff --git a/tests/test_flood.py b/tests/test_flood.py index c4ea6fe..6ae9aa1 100644 --- a/tests/test_flood.py +++ b/tests/test_flood.py @@ -1,4 +1,4 @@ -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock import discord import pytest @@ -234,20 +234,81 @@ async def test_two_images_two_channels_triggers_on_the_second_message( ) 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. + # The first channel's message doesn't trigger anything on its own - + # detection only fires once a second channel is seen within the + # burst window. 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) - # 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 + # But once it fires, *both* channels get cleaned up - not just the + # message that happened to cross the threshold. A compromised + # account spraying images across channels can beat detection to + # several of them before the mute lands; leaving those copies up + # would defeat the point. + patched_message_delete.assert_any_await(first) + patched_message_delete.assert_any_await(second) + assert patched_message_delete.await_count == 2 + + for channel in (channel_a, channel_b): + channel.send.assert_awaited_once() + _, kwargs = 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_alert_moderation_title_reports_the_channel_count(self, flood_cog): + # The burst fires (and resets tracking) the moment a 2nd distinct + # channel is seen, so the count in the mod-thread title is always 2 + # at the point it actually triggers. + member = make_member(name="comprometido") + + for channel_id in (1, 2): + message = make_message( + author=member, + channel=make_text_channel(id=channel_id), + attachments=[make_attachment(filename="a.png"), make_attachment(filename="b.png")], + ) + result = await flood_cog.attachment_check(make_context(message)) + + assert result is True + flood_cog.main_mod_channel.create_thread.assert_awaited_once() + _, kwargs = flood_cog.main_mod_channel.create_thread.call_args + assert "2 canales" in kwargs["name"] + + async def test_still_warns_a_channel_whose_message_was_already_deleted( + self, flood_cog, patched_message_delete + ): + """If one of the affected messages is already gone (e.g. a + moderator removed it manually first), the cleanup shouldn't crash - + bystanders in that channel should still see the warning. + """ + 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")], + ) + await flood_cog.attachment_check(make_context(first)) + + async def flaky_delete(message): + if message is first: + raise discord.NotFound(MagicMock(), "ya no existe") + + patched_message_delete.side_effect = flaky_delete + + second = make_message( + author=member, + channel=channel_b, + attachments=[make_attachment(filename="b1.png"), make_attachment(filename="b2.png")], + ) + result = await flood_cog.attachment_check(make_context(second)) + + assert result is True + channel_a.send.assert_awaited_once() + channel_b.send.assert_awaited_once() async def test_images_get_cached_for_the_fast_path(self, flood_cog): member = make_member(name="comprometido")