Merge branch 'master' into feature/cooldown-tag

13 files changed, 179 insertions, 76 deletions

diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
new file mode 100644
index 000000000..8760b35ec
--- /dev/null
+++ b/.github/workflows/codeql-analysis.yml
@@ -0,0 +1,32 @@
+name: "Code scanning - action"
+
+on:
+  push:
+  pull_request:
+  schedule:
+    - cron: '0 12 * * *'
+
+jobs:
+  CodeQL-Build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+      with:
+        fetch-depth: 2
+
+    - run: git checkout HEAD^2
+      if: ${{ github.event_name == 'pull_request' }}
+
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v1
+      with:
+        languages: python
+
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v1
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v1
diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py
index 1d9fddb12..4ebc831e1 100644
--- a/bot/cogs/filtering.py
+++ b/bot/cogs/filtering.py
@@ -1,6 +1,8 @@
+import asyncio
 import logging
 import re
-from typing import Optional, Union
+from datetime import datetime, timedelta
+from typing import List, Optional, Union
 
 import discord.errors
 from dateutil.relativedelta import relativedelta
@@ -14,6 +16,7 @@ from bot.constants import (
     Channels, Colours,
     Filter, Icons, URLs
 )
+from bot.utils.redis_cache import RedisCache
 
 log = logging.getLogger(__name__)
 
@@ -40,6 +43,8 @@ TOKEN_WATCHLIST_PATTERNS = [
 ]
 WATCHLIST_PATTERNS = WORD_WATCHLIST_PATTERNS + TOKEN_WATCHLIST_PATTERNS
 
+DAYS_BETWEEN_ALERTS = 3
+
 
 def expand_spoilers(text: str) -> str:
     """Return a string containing all interpretations of a spoilered message."""
@@ -52,8 +57,12 @@ def expand_spoilers(text: str) -> str:
 class Filtering(Cog):
     """Filtering out invites, blacklisting domains, and warning us of certain regular expressions."""
 
+    # Redis cache mapping a user ID to the last timestamp a bad nickname alert was sent
+    name_alerts = RedisCache()
+
     def __init__(self, bot: Bot):
         self.bot = bot
+        self.name_lock = asyncio.Lock()
 
         staff_mistake_str = "If you believe this was a mistake, please let staff know!"
         self.filters = {
@@ -112,6 +121,7 @@ class Filtering(Cog):
     async def on_message(self, msg: Message) -> None:
         """Invoke message filter for new messages."""
         await self._filter_message(msg)
+        await self.check_bad_words_in_name(msg.author)
 
     @Cog.listener()
     async def on_message_edit(self, before: Message, after: Message) -> None:
@@ -126,6 +136,55 @@ class Filtering(Cog):
             delta = relativedelta(after.edited_at, before.edited_at).microseconds
         await self._filter_message(after, delta)
 
+    @staticmethod
+    def get_name_matches(name: str) -> List[re.Match]:
+        """Check bad words from passed string (name). Return list of matches."""
+        matches = []
+        for pattern in WATCHLIST_PATTERNS:
+            if match := pattern.search(name):
+                matches.append(match)
+        return matches
+
+    async def check_send_alert(self, member: Member) -> bool:
+        """When there is less than 3 days after last alert, return `False`, otherwise `True`."""
+        if last_alert := await self.name_alerts.get(member.id):
+            last_alert = datetime.utcfromtimestamp(last_alert)
+            if datetime.utcnow() - timedelta(days=DAYS_BETWEEN_ALERTS) < last_alert:
+                log.trace(f"Last alert was too recent for {member}'s nickname.")
+                return False
+
+        return True
+
+    async def check_bad_words_in_name(self, member: Member) -> None:
+        """Send a mod alert every 3 days if a username still matches a watchlist pattern."""
+        # Use lock to avoid race conditions
+        async with self.name_lock:
+            # Check whether the users display name contains any words in our blacklist
+            matches = self.get_name_matches(member.display_name)
+
+            if not matches or not await self.check_send_alert(member):
+                return
+
+            log.info(f"Sending bad nickname alert for '{member.display_name}' ({member.id}).")
+
+            log_string = (
+                f"**User:** {member.mention} (`{member.id}`)\n"
+                f"**Display Name:** {member.display_name}\n"
+                f"**Bad Matches:** {', '.join(match.group() for match in matches)}"
+            )
+
+            await self.mod_log.send_log_message(
+                icon_url=Icons.token_removed,
+                colour=Colours.soft_red,
+                title="Username filtering alert",
+                text=log_string,
+                channel_id=Channels.mod_alerts,
+                thumbnail=member.avatar_url
+            )
+
+            # Update time when alert sent
+            await self.name_alerts.set(member.id, datetime.utcnow().timestamp())
+
     async def _filter_message(self, msg: Message, delta: Optional[int] = None) -> None:
         """Filter the input message to see if it violates any of our rules, and then respond accordingly."""
         # Should we filter this message?
diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py
index 70cef339a..6ff285c37 100644
--- a/bot/cogs/help_channels.py
+++ b/bot/cogs/help_channels.py
@@ -22,7 +22,7 @@ log = logging.getLogger(__name__)
 
 ASKING_GUIDE_URL = "https://pythondiscord.com/pages/asking-good-questions/"
 MAX_CHANNELS_PER_CATEGORY = 50
-EXCLUDED_CHANNELS = (constants.Channels.how_to_get_help,)
+EXCLUDED_CHANNELS = (constants.Channels.how_to_get_help, constants.Channels.cooldown)
 
 HELP_CHANNEL_TOPIC = """
 This is a Python help channel. You can claim your own help channel in the Python Help: Available category.
diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py
index 9d28030d9..41472c64c 100644
--- a/bot/cogs/moderation/modlog.py
+++ b/bot/cogs/moderation/modlog.py
@@ -555,6 +555,10 @@ class ModLog(Cog, name="ModLog"):
         channel = message.channel
         author = message.author
 
+        # Ignore DMs.
+        if not message.guild:
+            return
+
         if message.guild.id != GuildConstant.id or channel.id in GuildConstant.modlog_blacklist:
             return
 
diff --git a/bot/cogs/site.py b/bot/cogs/site.py
index e61cd5003..ac29daa1d 100644
--- a/bot/cogs/site.py
+++ b/bot/cogs/site.py
@@ -33,7 +33,7 @@ class Site(Cog):
         embed.colour = Colour.blurple()
         embed.description = (
             f"[Our official website]({url}) is an open-source community project "
-            "created with Python and Flask. It contains information about the server "
+            "created with Python and Django. It contains information about the server "
             "itself, lets you sign up for upcoming events, has its own wiki, contains "
             "a list of valuable learning resources, and much more."
         )
diff --git a/bot/cogs/stats.py b/bot/cogs/stats.py
index 4ebb6423c..d42f55466 100644
--- a/bot/cogs/stats.py
+++ b/bot/cogs/stats.py
@@ -36,7 +36,8 @@ class Stats(Cog):
         if message.guild.id != Guild.id:
             return
 
-        if message.channel.category.id == Categories.modmail:
+        cat = getattr(message.channel, "category", None)
+        if cat is not None and cat.id == Categories.modmail:
             if message.channel.id != Channels.incidents:
                 # Do not report modmail channels to stats, there are too many
                 # of them for interesting statistics to be drawn out of this.
diff --git a/bot/constants.py b/bot/constants.py
index b31a9c99e..470221369 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -389,6 +389,7 @@ class Channels(metaclass=YAMLGetter):
     attachment_log: int
     big_brother_logs: int
     bot_commands: int
+    cooldown: int
     defcon: int
     dev_contrib: int
     dev_core: int
diff --git a/bot/converters.py b/bot/converters.py
index 72c46fdf0..4deb59f87 100644
--- a/bot/converters.py
+++ b/bot/converters.py
@@ -217,7 +217,10 @@ class Duration(Converter):
         delta = relativedelta(**duration_dict)
         now = datetime.utcnow()
 
-        return now + delta
+        try:
+            return now + delta
+        except ValueError:
+            raise BadArgument(f"`{duration}` results in a datetime outside the supported range.")
 
 
 class ISODateTime(Converter):
diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py
index de80cee84..354e987b9 100644
--- a/bot/utils/redis_cache.py
+++ b/bot/utils/redis_cache.py
@@ -100,16 +100,7 @@ class RedisCache:
 
     def _set_namespace(self, namespace: str) -> None:
         """Try to set the namespace, but do not permit collisions."""
-        # We need a unique namespace, to prevent collisions. This loop
-        # will try appending underscores to the end of the namespace until
-        # it finds one that is unique.
-        #
-        # For example, if `john` and `john_`  are both taken, the namespace will
-        # be `john__` at the end of this loop.
-        while namespace in self._namespaces:
-            namespace += "_"
-
-        log.trace(f"RedisCache setting namespace to {self._namespace}")
+        log.trace(f"RedisCache setting namespace to {namespace}")
         self._namespaces.append(namespace)
         self._namespace = namespace
 
diff --git a/config-default.yml b/config-default.yml
index 2c85f5ef3..3388e5f78 100644
--- a/config-default.yml
+++ b/config-default.yml
@@ -142,6 +142,7 @@ guild:
 
         # Python Help: Available
         how_to_get_help:    704250143020417084
+        cooldown:           720603994149486673
 
         # Logs
         attachment_log:     &ATTACH_LOG     649243850006855680
@@ -297,6 +298,7 @@ filter:
         - 613425648685547541  # Discord Developers
         - 185590609631903755  # Blender Hub
         - 420324994703163402  # /r/FlutterDev
+        - 488751051629920277  # Python Atlanta
 
     domain_blacklist:
         - pornhub.com
diff --git a/docker-compose.yml b/docker-compose.yml
index 9884e35f0..cff7d33d6 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -17,6 +17,14 @@ services:
     ports:
       - "127.0.0.1:6379:6379"
 
+  snekbox:
+    image: pythondiscord/snekbox:latest
+    init: true
+    ipc: none
+    ports:
+     - "127.0.0.1:8060:8060"
+    privileged: true
+
   web:
     image: pythondiscord/site:latest
     command: ["run", "--debug"]
@@ -47,6 +55,7 @@ services:
     depends_on:
       - web
       - redis
+      - snekbox
     environment:
       BOT_TOKEN: ${BOT_TOKEN}
       BOT_API_KEY: badbot13m0n8f570f942013fc818f234916ca531
diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py
index ca8cb6825..c42111f3f 100644
--- a/tests/bot/test_converters.py
+++ b/tests/bot/test_converters.py
@@ -1,5 +1,5 @@
-import asyncio
 import datetime
+import re
 import unittest
 from unittest.mock import MagicMock, patch
 
@@ -16,7 +16,7 @@ from bot.converters import (
 )
 
 
-class ConverterTests(unittest.TestCase):
+class ConverterTests(unittest.IsolatedAsyncioTestCase):
     """Tests our custom argument converters."""
 
     @classmethod
@@ -26,7 +26,7 @@ class ConverterTests(unittest.TestCase):
 
         cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00')
 
-    def test_tag_content_converter_for_valid(self):
+    async def test_tag_content_converter_for_valid(self):
         """TagContentConverter should return correct values for valid input."""
         test_values = (
             ('hello', 'hello'),
@@ -35,10 +35,10 @@ class ConverterTests(unittest.TestCase):
 
         for content, expected_conversion in test_values:
             with self.subTest(content=content, expected_conversion=expected_conversion):
-                conversion = asyncio.run(TagContentConverter.convert(self.context, content))
+                conversion = await TagContentConverter.convert(self.context, content)
                 self.assertEqual(conversion, expected_conversion)
 
-    def test_tag_content_converter_for_invalid(self):
+    async def test_tag_content_converter_for_invalid(self):
         """TagContentConverter should raise the proper exception for invalid input."""
         test_values = (
             ('', "Tag contents should not be empty, or filled with whitespace."),
@@ -47,10 +47,10 @@ class ConverterTests(unittest.TestCase):
 
         for value, exception_message in test_values:
             with self.subTest(tag_content=value, exception_message=exception_message):
-                with self.assertRaises(BadArgument, msg=exception_message):
-                    asyncio.run(TagContentConverter.convert(self.context, value))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await TagContentConverter.convert(self.context, value)
 
-    def test_tag_name_converter_for_valid(self):
+    async def test_tag_name_converter_for_valid(self):
         """TagNameConverter should return the correct values for valid tag names."""
         test_values = (
             ('tracebacks', 'tracebacks'),
@@ -60,10 +60,10 @@ class ConverterTests(unittest.TestCase):
 
         for name, expected_conversion in test_values:
             with self.subTest(name=name, expected_conversion=expected_conversion):
-                conversion = asyncio.run(TagNameConverter.convert(self.context, name))
+                conversion = await TagNameConverter.convert(self.context, name)
                 self.assertEqual(conversion, expected_conversion)
 
-    def test_tag_name_converter_for_invalid(self):
+    async def test_tag_name_converter_for_invalid(self):
         """TagNameConverter should raise the correct exception for invalid tag names."""
         test_values = (
             ('👋', "Don't be ridiculous, you can't use that character!"),
@@ -75,29 +75,29 @@ class ConverterTests(unittest.TestCase):
 
         for invalid_name, exception_message in test_values:
             with self.subTest(invalid_name=invalid_name, exception_message=exception_message):
-                with self.assertRaises(BadArgument, msg=exception_message):
-                    asyncio.run(TagNameConverter.convert(self.context, invalid_name))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await TagNameConverter.convert(self.context, invalid_name)
 
-    def test_valid_python_identifier_for_valid(self):
+    async def test_valid_python_identifier_for_valid(self):
         """ValidPythonIdentifier returns valid identifiers unchanged."""
         test_values = ('foo', 'lemon')
 
         for name in test_values:
             with self.subTest(identifier=name):
-                conversion = asyncio.run(ValidPythonIdentifier.convert(self.context, name))
+                conversion = await ValidPythonIdentifier.convert(self.context, name)
                 self.assertEqual(name, conversion)
 
-    def test_valid_python_identifier_for_invalid(self):
+    async def test_valid_python_identifier_for_invalid(self):
         """ValidPythonIdentifier raises the proper exception for invalid identifiers."""
         test_values = ('nested.stuff', '#####')
 
         for name in test_values:
             with self.subTest(identifier=name):
                 exception_message = f'`{name}` is not a valid Python identifier'
-                with self.assertRaises(BadArgument, msg=exception_message):
-                    asyncio.run(ValidPythonIdentifier.convert(self.context, name))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await ValidPythonIdentifier.convert(self.context, name)
 
-    def test_duration_converter_for_valid(self):
+    async def test_duration_converter_for_valid(self):
         """Duration returns the correct `datetime` for valid duration strings."""
         test_values = (
             # Simple duration strings
@@ -159,35 +159,35 @@ class ConverterTests(unittest.TestCase):
                 mock_datetime.utcnow.return_value = self.fixed_utc_now
 
                 with self.subTest(duration=duration, duration_dict=duration_dict):
-                    converted_datetime = asyncio.run(converter.convert(self.context, duration))
+                    converted_datetime = await converter.convert(self.context, duration)
                     self.assertEqual(converted_datetime, expected_datetime)
 
-    def test_duration_converter_for_invalid(self):
+    async def test_duration_converter_for_invalid(self):
         """Duration raises the right exception for invalid duration strings."""
         test_values = (
             # Units in wrong order
-            ('1d1w'),
-            ('1s1y'),
+            '1d1w',
+            '1s1y',
 
             # Duplicated units
-            ('1 year 2 years'),
-            ('1 M 10 minutes'),
+            '1 year 2 years',
+            '1 M 10 minutes',
 
             # Unknown substrings
-            ('1MVes'),
-            ('1y3breads'),
+            '1MVes',
+            '1y3breads',
 
             # Missing amount
-            ('ym'),
+            'ym',
 
             # Incorrect whitespace
-            (" 1y"),
-            ("1S "),
-            ("1y  1m"),
+            " 1y",
+            "1S ",
+            "1y  1m",
 
             # Garbage
-            ('Guido van Rossum'),
-            ('lemon lemon lemon lemon lemon lemon lemon'),
+            'Guido van Rossum',
+            'lemon lemon lemon lemon lemon lemon lemon',
         )
 
         converter = Duration()
@@ -195,10 +195,21 @@ class ConverterTests(unittest.TestCase):
         for invalid_duration in test_values:
             with self.subTest(invalid_duration=invalid_duration):
                 exception_message = f'`{invalid_duration}` is not a valid duration string.'
-                with self.assertRaises(BadArgument, msg=exception_message):
-                    asyncio.run(converter.convert(self.context, invalid_duration))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await converter.convert(self.context, invalid_duration)
 
-    def test_isodatetime_converter_for_valid(self):
+    @patch("bot.converters.datetime")
+    async def test_duration_converter_out_of_range(self, mock_datetime):
+        """Duration converter should raise BadArgument if datetime raises a ValueError."""
+        mock_datetime.__add__.side_effect = ValueError
+        mock_datetime.utcnow.return_value = mock_datetime
+
+        duration = f"{datetime.MAXYEAR}y"
+        exception_message = f"`{duration}` results in a datetime outside the supported range."
+        with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+            await Duration().convert(self.context, duration)
+
+    async def test_isodatetime_converter_for_valid(self):
         """ISODateTime converter returns correct datetime for valid datetime string."""
         test_values = (
             # `YYYY-mm-ddTHH:MM:SSZ` | `YYYY-mm-dd HH:MM:SSZ`
@@ -243,37 +254,37 @@ class ConverterTests(unittest.TestCase):
 
         for datetime_string, expected_dt in test_values:
             with self.subTest(datetime_string=datetime_string, expected_dt=expected_dt):
-                converted_dt = asyncio.run(converter.convert(self.context, datetime_string))
+                converted_dt = await converter.convert(self.context, datetime_string)
                 self.assertIsNone(converted_dt.tzinfo)
                 self.assertEqual(converted_dt, expected_dt)
 
-    def test_isodatetime_converter_for_invalid(self):
+    async def test_isodatetime_converter_for_invalid(self):
         """ISODateTime converter raises the correct exception for invalid datetime strings."""
         test_values = (
             # Make sure it doesn't interfere with the Duration converter
-            ('1Y'),
-            ('1d'),
-            ('1H'),
+            '1Y',
+            '1d',
+            '1H',
 
             # Check if it fails when only providing the optional time part
-            ('10:10:10'),
-            ('10:00'),
+            '10:10:10',
+            '10:00',
 
             # Invalid date format
-            ('19-01-01'),
+            '19-01-01',
 
             # Other non-valid strings
-            ('fisk the tag master'),
+            'fisk the tag master',
         )
 
         converter = ISODateTime()
         for datetime_string in test_values:
             with self.subTest(datetime_string=datetime_string):
                 exception_message = f"`{datetime_string}` is not a valid ISO-8601 datetime string"
-                with self.assertRaises(BadArgument, msg=exception_message):
-                    asyncio.run(converter.convert(self.context, datetime_string))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await converter.convert(self.context, datetime_string)
 
-    def test_hush_duration_converter_for_valid(self):
+    async def test_hush_duration_converter_for_valid(self):
         """HushDurationConverter returns correct value for minutes duration or `"forever"` strings."""
         test_values = (
             ("0", 0),
@@ -286,10 +297,10 @@ class ConverterTests(unittest.TestCase):
         converter = HushDurationConverter()
         for minutes_string, expected_minutes in test_values:
             with self.subTest(minutes_string=minutes_string, expected_minutes=expected_minutes):
-                converted = asyncio.run(converter.convert(self.context, minutes_string))
+                converted = await converter.convert(self.context, minutes_string)
                 self.assertEqual(expected_minutes, converted)
 
-    def test_hush_duration_converter_for_invalid(self):
+    async def test_hush_duration_converter_for_invalid(self):
         """HushDurationConverter raises correct exception for invalid minutes duration strings."""
         test_values = (
             ("16", "Duration must be at most 15 minutes."),
@@ -299,5 +310,5 @@ class ConverterTests(unittest.TestCase):
         converter = HushDurationConverter()
         for invalid_minutes_string, exception_message in test_values:
             with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message):
-                with self.assertRaisesRegex(BadArgument, exception_message):
-                    asyncio.run(converter.convert(self.context, invalid_minutes_string))
+                with self.assertRaisesRegex(BadArgument, re.escape(exception_message)):
+                    await converter.convert(self.context, invalid_minutes_string)
diff --git a/tests/bot/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py
index 8c1a40640..e5d6e4078 100644
--- a/tests/bot/utils/test_redis_cache.py
+++ b/tests/bot/utils/test_redis_cache.py
@@ -44,16 +44,6 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase):
         with self.assertRaises(RuntimeError):
             await bad_cache.set("test", "me_up_deadman")
 
-    def test_namespace_collision(self):
-        """Test that we prevent colliding namespaces."""
-        bob_cache_1 = RedisCache()
-        bob_cache_1._set_namespace("BobRoss")
-        self.assertEqual(bob_cache_1._namespace, "BobRoss")
-
-        bob_cache_2 = RedisCache()
-        bob_cache_2._set_namespace("BobRoss")
-        self.assertEqual(bob_cache_2._namespace, "BobRoss_")
-
     async def test_set_get_item(self):
         """Test that users can set and get items from the RedisDict."""
         test_cases = (