diff options
| author | 2021-05-06 19:28:18 +0200 | |
|---|---|---|
| committer | 2021-05-06 19:28:18 +0200 | |
| commit | 425b4f1d2f71b42f197dbaacd7926a5431a74a45 (patch) | |
| tree | 4a8b4124072868ef86dbf869f3e0fcfc7102c423 | |
| parent | Merge pull request #1561 from dawnofmidnight/patch-1 (diff) | |
Prevent accidental addition of users to talentpool
Bug: When asking for a review for a user that isn't currently nominated
using the `!talentpool get_review <userid>` command, the user is added
to the talentpool `watched_users` cache, causing their messages to be
relayed to the talentpool watch channel.
Steps to reproduce: Use `!talentpool get_review <user_id>`, where
`<user_id>` is the ID of a user not currently nominated. The command
will correctly reply that the user isn't nominated, but their ID will be
added as a key to the defaultdict nonetheless.
Solution: replace all regular getitem usages with `.get(<user_id>)`, as
the Reviewer should never insert IDs using the regular defaultdict path.
Additional note: I've replaced all occurrences of regular getitem access
into the defaultdict, even those that are normally not reachable with
the id of a user that's currently not nominated, to prevent a future
refactor from accidentally introducing this bug again.
| -rw-r--r-- | bot/exts/recruitment/talentpool/_review.py | 12 | 
1 files changed, 8 insertions, 4 deletions
| diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index 11aa3b62b..4ae1c5ad6 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -57,7 +57,7 @@ class Reviewer:          """Schedules a single user for review."""          log.trace(f"Scheduling review of user with ID {user_id}") -        user_data = self._pool.watched_users[user_id] +        user_data = self._pool.watched_users.get(user_id)          inserted_at = isoparse(user_data['inserted_at']).replace(tzinfo=None)          review_at = inserted_at + timedelta(days=MAX_DAYS_IN_POOL) @@ -81,14 +81,18 @@ class Reviewer:                  await message.add_reaction(reaction)          if update_database: -            nomination = self._pool.watched_users[user_id] +            nomination = self._pool.watched_users.get(user_id)              await self.bot.api_client.patch(f"{self._pool.api_endpoint}/{nomination['id']}", json={"reviewed": True})      async def make_review(self, user_id: int) -> typing.Tuple[str, Optional[Emoji]]:          """Format a generic review of a user and return it with the seen emoji."""          log.trace(f"Formatting the review of {user_id}") -        nomination = self._pool.watched_users[user_id] +        # Since `watched_users` is a defaultdict, we should take care +        # not to accidentally insert the IDs of users that have no +        # active nominated by using the `watched_users.get(user_id)` +        # instead of `watched_users[user_id]`. +        nomination = self._pool.watched_users.get(user_id)          if not nomination:              log.trace(f"There doesn't appear to be an active nomination for {user_id}")              return "", None @@ -303,7 +307,7 @@ class Reviewer:              await ctx.send(f":x: Can't find a currently nominated user with id `{user_id}`")              return False -        nomination = self._pool.watched_users[user_id] +        nomination = self._pool.watched_users.get(user_id)          if nomination["reviewed"]:              await ctx.send(":x: This nomination was already reviewed, but here's a cookie :cookie:")              return False | 
