Multiple off-by-one errors and silent reminder deletion with no user notification in reminders.py #3507

Activity

1. oliveman-au commented on May 5, 2026

   

   oliveman-au

   on May 5, 2026

   Last edited by oliveman-au

   ContributorAuthor

   More actions

   Additional bugs found in bot/exts/utils/reminders.py

   ---

   Bug 1: Off-by-one — users can create 6 reminders despite a limit of 5

   Current code (reminders.py line 478):

   MAXIMUM_REMINDERS = 5
   
   if len(active_reminders) > MAXIMUM_REMINDERS:
       await send_denial(ctx, "You have too many active reminders!")
       return

   > 5 only evaluates to True once the user already has 6 reminders in the database — meaning the 6th is always written before the check can block it. The denial never fires on the 6th attempt.

   Fix:

   if len(active_reminders) >= MAXIMUM_REMINDERS:
       await send_denial(ctx, "You have too many active reminders!")
       return

   Steps to reproduce (as a regular member in #bot-commands):

   1. Run each of the following one at a time:

   !remind new 7d reminder 1
   !remind new 7d reminder 2
   !remind new 7d reminder 3
   !remind new 7d reminder 4
   !remind new 7d reminder 5
   !remind new 7d reminder 6
   

   2. All 6 succeed with a confirmation embed — the denial is never shown
   3. Run !remind list to confirm all 6 are saved

   Expected: the bot sends a denial on the 6th attempt and does not save it

   ---

   Bug 2: Off-by-one — users can delete 6 reminders at once despite a limit of 5

   The docstring explicitly states the limit is 5, but the same > vs >= mistake is present:

   Current code (reminders.py line 660-663):

   async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:
       """Delete up to (and including) 5 of your active reminders."""
       if len(ids) > 5:
           await send_denial(ctx, "You can only delete a maximum of 5 reminders at once.")
           return

   > 5 is only True when 6 or more IDs are passed, so passing exactly 6 IDs is accepted and all 6 are deleted. The docstring and the behaviour directly contradict each other.

   Fix:

   async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:
       """Delete up to (and including) 5 of your active reminders."""
       if len(ids) >= 5:
           await send_denial(ctx, "You can only delete a maximum of 5 reminders at once.")
           return

   Steps to reproduce (as a regular member in #bot-commands):

   1. First create 6 reminders using the steps in Bug 1 above
   2. Run !remind list and copy the 6 reminder IDs
   3. Run:

   !remind delete <id1> <id2> <id3> <id4> <id5> <id6>
   

   4. The bot confirms all 6 were deleted — no denial is shown

   Expected: the bot sends a denial when more than 5 IDs are provided and deletes nothing

   ---

   Bug 3: Reminders are silently deleted with no user notification when the bot loses channel cache

   Current code (reminders.py line 248-258):

   def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel]:
       """Ensure reminder channel can be fetched otherwise delete the reminder."""
       channel = self.bot.get_channel(reminder["channel_id"])
       is_valid = True
       if not channel:
           is_valid = False
           log.info(
               f"Reminder {reminder['id']} invalid: "
               f"Channel {reminder['channel_id']}={channel}."
           )
           scheduling.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}"))
   
       return is_valid, channel

   self.bot.get_channel() only checks the local cache — it does not make a Discord API call. If the bot restarts or a channel temporarily falls out of cache during an outage, get_channel() returns None even though the channel still exists and is perfectly valid. The reminder is then permanently deleted from the database with no DM, no message, and no indication to the user that their reminder was lost. The content is gone with no way to recover it.

   Fix: use fetch_channel() to fall back to the Discord API before giving up, and DM the user if the reminder truly cannot be delivered:

   async def ensure_valid_reminder(self, reminder: dict) -> tuple[bool, discord.TextChannel | None]:
       """Ensure reminder channel can be fetched otherwise notify the user and delete the reminder."""
       channel = self.bot.get_channel(reminder["channel_id"])
       if not channel:
           try:
               channel = await self.bot.fetch_channel(reminder["channel_id"])
           except discord.HTTPException:
               log.info(
                   f"Reminder {reminder['id']} invalid: "
                   f"Channel {reminder['channel_id']} could not be fetched."
               )
               user = self.bot.get_user(reminder["author"])
               if user:
                   await user.send(
                       f"Your reminder could not be delivered because the channel it was set in is no longer accessible.\n"
                       f"**Reminder content:** {reminder['content']}"
                   )
               scheduling.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}"))
               return False, None
   
       return True, channel

   Steps to reproduce:

   1. Set a reminder with !remind new 10m check this
   2. Have the bot restart before the 10 minutes are up
   3. When the reminder time arrives, nothing happens — the reminder was silently deleted on restart and the user receives no notification

   Expected: the user receives a DM with their reminder content explaining it could not be delivered, so they can recreate it
2. oliveman-au commented on May 5, 2026

   

   oliveman-au

   on May 5, 2026

   ContributorAuthor

   More actions

   I'd love to open a PR to fix all of the bugs outlined above if I'm able to be assigned. I'm happy to provide image proof reproducing each one if needed before any work begins. Thanks for your time!
3. oliveman-au

   changed the title [-]Off-by-one error allows regular users to create 6 reminders despite a limit of 5[/-] [+]Multiple off-by-one errors and silent reminder deletion with no user notification in reminders.py[/+] on May 5, 2026
4. wookie184 commented on May 5, 2026

   

   wookie184

   on May 5, 2026

   Contributor

   More actions

   1 and 2 are simple and sound good.

   For 3, generally all channels should be cached, so it isn't a massive issue exactly as described, but it is still an issue as the channel may simply not exist. This has an existing issue at #3015. I think DMing sounds sensible. People can also react to reminders to also be reminded, should they be DMed as well?

   Also, we have a utility for fetching channel if not cached, https://bot-core.pythondiscord.com/output/pydis_core.utils.channel.html#pydis_core.utils.channel.get_or_fetch_channel, you can search the codebase for examples of how it's used.

   Overall sounds good, I'll approve and assign you.
5. wookie184

   added

   status: approvedThe issue has received a core developer's approvalThe issue has received a core developer's approval

   on May 5, 2026
6. wookie184

   assigned

   oliveman-au

   on May 5, 2026
7. oliveman-au commented on May 7, 2026

   

   oliveman-au

   on May 7, 2026

   ContributorAuthor

   More actions

   PR can be found here - #3509
8. wookie184

   linked a pull request that will close this issueFix off-by-one errors and silent reminder deletion in reminders.py #3509on Jun 13, 2026