From b38e645a66b76693ebc0cf0febc63187ab7a8b2f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 9 Apr 2021 18:52:01 -0700 Subject: AntiSpam: prevent attempts to punish a user multiple times A user may manage to send multiple message that violate filters before the mute is applied. Because of a race condition, subsequent punish attempts did not detect the mute role exists and therefore proceeded to apply another mute. To avoid the race condition, abort any subsequent punish attempts while one is already ongoing for a given user. It could be possible to wait instead of abort, but the first attempt failing very likely means subsequent attempts would fail too. Fixes #902 --- bot/exts/filters/antispam.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index af8528a68..c9052b138 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -3,7 +3,7 @@ import logging from collections.abc import Mapping from dataclasses import dataclass, field from datetime import datetime, timedelta -from operator import itemgetter +from operator import attrgetter, itemgetter from typing import Dict, Iterable, List, Set from discord import Colour, Member, Message, NotFound, Object, TextChannel @@ -18,6 +18,7 @@ from bot.constants import ( ) from bot.converters import Duration from bot.exts.moderation.modlog import ModLog +from bot.utils import lock from bot.utils.messages import format_user, send_attachments @@ -211,6 +212,7 @@ class AntiSpam(Cog): await self.maybe_delete_messages(channel, relevant_messages) break + @lock.lock_arg("antispam.punish", "member", attrgetter("id")) async def punish(self, msg: Message, member: Member, reason: str) -> None: """Punishes the given member for triggering an antispam rule.""" if not any(role.id == self.muted_role.id for role in member.roles): -- cgit v1.2.3 From 73b49b5b4d8f545da4d42b644907a34826757b3e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 9 Apr 2021 18:59:49 -0700 Subject: AntiSpam: create tasks in a safer manner Name the tasks and use `scheduling.create_task` to ensure exceptions are caught. --- bot/exts/filters/antispam.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index c9052b138..7555e25a2 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -18,7 +18,7 @@ from bot.constants import ( ) from bot.converters import Duration from bot.exts.moderation.modlog import ModLog -from bot.utils import lock +from bot.utils import lock, scheduling from bot.utils.messages import format_user, send_attachments @@ -115,7 +115,7 @@ class AntiSpam(Cog): self.message_deletion_queue = dict() - self.bot.loop.create_task(self.alert_on_validation_error()) + self.bot.loop.create_task(self.alert_on_validation_error(), name="AntiSpam.alert_on_validation_error") @property def mod_log(self) -> ModLog: @@ -192,7 +192,10 @@ class AntiSpam(Cog): if channel.id not in self.message_deletion_queue: log.trace(f"Creating queue for channel `{channel.id}`") self.message_deletion_queue[message.channel.id] = DeletionContext(channel) - self.bot.loop.create_task(self._process_deletion_context(message.channel.id)) + scheduling.create_task( + self._process_deletion_context(message.channel.id), + name=f"AntiSpam._process_deletion_context({message.channel.id})" + ) # Add the relevant of this trigger to the Deletion Context await self.message_deletion_queue[message.channel.id].add( @@ -202,11 +205,9 @@ class AntiSpam(Cog): ) for member in members: - - # Fire it off as a background task to ensure - # that the sleep doesn't block further tasks - self.bot.loop.create_task( - self.punish(message, member, full_reason) + scheduling.create_task( + self.punish(message, member, full_reason), + name=f"AntiSpam.punish(message={message.id}, member={member.id}, rule={rule_name})" ) await self.maybe_delete_messages(channel, relevant_messages) -- cgit v1.2.3 From 475bd2124d56f6a59933b79b2f22c2b6c8896a25 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 14 Apr 2021 00:19:25 +0100 Subject: Use a paginated embed to output multiple snowflakes Previously each snowflake passed to the command would have their own embed, which may cause the bot to send many embeds if a staff unknowingly passed it a bunch of snowflakes. This change makes sure that we don't run into rate limits on the bot by sending all of the snowflakes in one embed. --- bot/exts/utils/utils.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/bot/exts/utils/utils.py b/bot/exts/utils/utils.py index cae7f2593..60383996d 100644 --- a/bot/exts/utils/utils.py +++ b/bot/exts/utils/utils.py @@ -162,17 +162,26 @@ class Utils(Cog): if len(snowflakes) > 1 and await has_no_roles_check(ctx, *STAFF_ROLES): raise BadArgument("Cannot process more than one snowflake in one invocation.") + embed = Embed( + colour=Colour.blue() + ) + embed.set_author( + name=f"Snowflake{'s'[:len(snowflakes)^1]}", # Deals with pluralisation + icon_url="https://github.com/twitter/twemoji/blob/master/assets/72x72/2744.png?raw=true" + ) + + lines = [] for snowflake in snowflakes: created_at = snowflake_time(snowflake) - embed = Embed( - description=f"**Created at {created_at}** ({time_since(created_at, max_units=3)}).", - colour=Colour.blue() - ) - embed.set_author( - name=f"Snowflake: {snowflake}", - icon_url="https://github.com/twitter/twemoji/blob/master/assets/72x72/2744.png?raw=true" - ) - await ctx.send(embed=embed) + lines.append(f"**{snowflake}**\nCreated at {created_at} ({time_since(created_at, max_units=3)}).") + + await LinePaginator.paginate( + lines, + ctx=ctx, + embed=embed, + max_lines=5, + max_size=1000 + ) @command(aliases=("poll",)) @has_any_role(*MODERATION_ROLES, Roles.project_leads, Roles.domain_leads) -- cgit v1.2.3 From f9fb8631ce8568e0c9f15ea4ff0977e722ede3ba Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 14 Apr 2021 10:23:38 +0100 Subject: Require at least one snowflake to be provided. --- bot/exts/utils/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/exts/utils/utils.py b/bot/exts/utils/utils.py index 60383996d..0fe0cab78 100644 --- a/bot/exts/utils/utils.py +++ b/bot/exts/utils/utils.py @@ -162,6 +162,9 @@ class Utils(Cog): if len(snowflakes) > 1 and await has_no_roles_check(ctx, *STAFF_ROLES): raise BadArgument("Cannot process more than one snowflake in one invocation.") + if not snowflakes: + raise BadArgument("At least one snowflake must be provided.") + embed = Embed( colour=Colour.blue() ) -- cgit v1.2.3 From 347927cc6d86e852959db716b6de31c6a886640d Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 14 Apr 2021 16:54:27 +0100 Subject: Catch NotFound errors when trying to delete the invocation message when cleaning This often happens during a raid, when an int e script is added to ban & clean messages. Since the invocation message will be deleted on the first run, we should except subsequent NotFound errors. --- bot/exts/utils/clean.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 8acaf9131..cb662e852 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -3,7 +3,7 @@ import random import re from typing import Iterable, Optional -from discord import Colour, Embed, Message, TextChannel, User +from discord import Colour, Embed, Message, TextChannel, User, errors from discord.ext import commands from discord.ext.commands import Cog, Context, group, has_any_role @@ -115,7 +115,11 @@ class Clean(Cog): # Delete the invocation first self.mod_log.ignore(Event.message_delete, ctx.message.id) - await ctx.message.delete() + try: + await ctx.message.delete() + except errors.NotFound: + # Invocation message has already been deleted + log.info("Tried to delete invocation message, but it was already deleted.") messages = [] message_ids = [] -- cgit v1.2.3 From 56ff78ae70986dfbc01878e666a6aaf753739668 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 14 Apr 2021 17:48:49 +0100 Subject: Refactor embed to use just one line --- bot/exts/utils/utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/exts/utils/utils.py b/bot/exts/utils/utils.py index 0fe0cab78..8d9d27c64 100644 --- a/bot/exts/utils/utils.py +++ b/bot/exts/utils/utils.py @@ -165,9 +165,7 @@ class Utils(Cog): if not snowflakes: raise BadArgument("At least one snowflake must be provided.") - embed = Embed( - colour=Colour.blue() - ) + embed = Embed(colour=Colour.blue()) embed.set_author( name=f"Snowflake{'s'[:len(snowflakes)^1]}", # Deals with pluralisation icon_url="https://github.com/twitter/twemoji/blob/master/assets/72x72/2744.png?raw=true" -- cgit v1.2.3