Merge pull request #1798 from python-discord/ignore-infra-mods-errors

Prevent role hierarchy issues with infractions

5 files changed, 22 insertions, 7 deletions

diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py
index b58b09250..a7f7dcb7f 100644
--- a/bot/exts/moderation/infraction/infractions.py
+++ b/bot/exts/moderation/infraction/infractions.py
@@ -315,6 +315,10 @@ class Infractions(InfractionScheduler, commands.Cog):
     @respect_role_hierarchy(member_arg=2)
     async def apply_kick(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None:
         """Apply a kick infraction with kwargs passed to `post_infraction`."""
+        if user.top_role >= ctx.me.top_role:
+            await ctx.send(":x: I can't kick users above or equal to me in the role hierarchy.")
+            return
+
         infraction = await _utils.post_infraction(ctx, user, "kick", reason, active=False, **kwargs)
         if infraction is None:
             return
@@ -341,6 +345,10 @@ class Infractions(InfractionScheduler, commands.Cog):
 
         Will also remove the banned user from the Big Brother watch list if applicable.
         """
+        if isinstance(user, Member) and user.top_role >= ctx.me.top_role:
+            await ctx.send(":x: I can't ban users above or equal to me in the role hierarchy.")
+            return
+
         # In the case of a permanent ban, we don't need get_active_infractions to tell us if one is active
         is_temporary = kwargs.get("expires_at") is not None
         active_infraction = await _utils.get_active_infraction(ctx, user, "ban", is_temporary)
@@ -520,7 +528,7 @@ class Infractions(InfractionScheduler, commands.Cog):
     async def cog_command_error(self, ctx: Context, error: Exception) -> None:
         """Send a notification to the invoking context on a Union failure."""
         if isinstance(error, commands.BadUnionArgument):
-            if discord.User in error.converters or discord.Member in error.converters:
+            if discord.User in error.converters or Member in error.converters:
                 await ctx.send(str(error.errors[0]))
                 error.handled = True
 
diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py
index aa2fd367b..17cde68f6 100644
--- a/bot/exts/moderation/infraction/superstarify.py
+++ b/bot/exts/moderation/infraction/superstarify.py
@@ -133,6 +133,10 @@ class Superstarify(InfractionScheduler, Cog):
         An optional reason can be provided, which would be added to a message stating their old nickname
         and linking to the nickname policy.
         """
+        if member.top_role >= ctx.me.top_role:
+            await ctx.send(":x: I can't starify users above or equal to me in the role hierarchy.")
+            return
+
         if await _utils.get_active_infraction(ctx, member, "superstar"):
             return
 
diff --git a/tests/bot/exts/backend/sync/test_users.py b/tests/bot/exts/backend/sync/test_users.py
index 88f1b2f52..2fc97af2d 100644
--- a/tests/bot/exts/backend/sync/test_users.py
+++ b/tests/bot/exts/backend/sync/test_users.py
@@ -12,7 +12,7 @@ def fake_user(**kwargs):
     kwargs.setdefault("id", 43)
     kwargs.setdefault("name", "bob the test man")
     kwargs.setdefault("discriminator", 1337)
-    kwargs.setdefault("roles", [666])
+    kwargs.setdefault("roles", [helpers.MockRole(id=666)])
     kwargs.setdefault("in_guild", True)
 
     return kwargs
diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py
index aeff734dc..4d01e18a5 100644
--- a/tests/bot/exts/moderation/infraction/test_infractions.py
+++ b/tests/bot/exts/moderation/infraction/test_infractions.py
@@ -15,12 +15,13 @@ class TruncationTests(unittest.IsolatedAsyncioTestCase):
     """Tests for ban and kick command reason truncation."""
 
     def setUp(self):
+        self.me = MockMember(id=7890, roles=[MockRole(id=7890, position=5)])
         self.bot = MockBot()
         self.cog = Infractions(self.bot)
-        self.user = MockMember(id=1234, top_role=MockRole(id=3577, position=10))
-        self.target = MockMember(id=1265, top_role=MockRole(id=9876, position=0))
+        self.user = MockMember(id=1234, roles=[MockRole(id=3577, position=10)])
+        self.target = MockMember(id=1265, roles=[MockRole(id=9876, position=1)])
         self.guild = MockGuild(id=4567)
-        self.ctx = MockContext(bot=self.bot, author=self.user, guild=self.guild)
+        self.ctx = MockContext(me=self.me, bot=self.bot, author=self.user, guild=self.guild)
 
     @patch("bot.exts.moderation.infraction._utils.get_active_infraction")
     @patch("bot.exts.moderation.infraction._utils.post_infraction")
@@ -66,8 +67,8 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase):
 
     def setUp(self):
         self.bot = MockBot()
-        self.mod = MockMember(top_role=10)
-        self.user = MockMember(top_role=1, roles=[MockRole(id=123456)])
+        self.mod = MockMember(roles=[MockRole(id=7890123, position=10)])
+        self.user = MockMember(roles=[MockRole(id=123456, position=1)])
         self.guild = MockGuild()
         self.ctx = MockContext(bot=self.bot, author=self.mod)
         self.cog = Infractions(self.bot)
diff --git a/tests/helpers.py b/tests/helpers.py
index 47f06f292..83b9b2363 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -235,6 +235,7 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin
         self.roles = [MockRole(name="@everyone", position=1, id=0)]
         if roles:
             self.roles.extend(roles)
+        self.top_role = max(self.roles)
 
         if 'mention' not in kwargs:
             self.mention = f"@{self.name}"
@@ -442,6 +443,7 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock):
 
     def __init__(self, **kwargs) -> None:
         super().__init__(**kwargs)
+        self.me = kwargs.get('me', MockMember())
         self.bot = kwargs.get('bot', MockBot())
         self.guild = kwargs.get('guild', MockGuild())
         self.author = kwargs.get('author', MockMember())