Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix edge case where perm names are not validated in custom Red decos #6291

Merged

Conversation

Flame442
Copy link
Member

Description of the changes

_validate_perms_dict is supposed to be used to validate that the permissions names passed to decorators such as @commands.admin_or_permissions are actual permissions. The get_decorator helper used to build these decorators currently returns a decorator that fails to validate permission names if the decorated object is a coroutine. This causes the check to never properly happen in cases where the decorator executes before @commands.command (ie, if it is on the bottom). This potentially could cause unexpected behavior if a permission name is misspelled when passed to one of these decorators.

Found by @untir_l.

Reproduction example:

import discord
from redbot.core import commands

class Test(commands.Cog):
    @commands.command()
    @commands.admin_or_permissions(NONEXISTENT_PERM=True)
    async def test(self, ctx):
        await ctx.reply("Boop!")

Already properly handled (prevents load):

import discord
from redbot.core import commands

class Test(commands.Cog):
    @commands.admin_or_permissions(NONEXISTENT_PERM=True)
    @commands.command()
    async def test(self, ctx):
        await ctx.reply("Boop!")

Have the changes in this PR been tested?

Yes

@Flame442 Flame442 added Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. High Priority High priority labels Feb 11, 2024
@Flame442 Flame442 requested a review from Jackenmen February 11, 2024 01:28
@Flame442 Flame442 added this to the 3.5.6 milestone Feb 11, 2024
@github-actions github-actions bot added the Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. label Feb 11, 2024
@Kowlin Kowlin merged commit ff09713 into Cog-Creators:V3/develop Feb 11, 2024
19 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Feb 11, 2024
@Flame442 Flame442 deleted the actually-check-permission-keys branch February 11, 2024 18:06
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Mar 22, 2024
Dav-Git pushed a commit to Dav-Git/Red-DiscordBot that referenced this pull request Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. High Priority High priority Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants