permissions_for not working well with new text in voice feature

[lexicalunit]

Summary

in GuildChannel.permissions_for: read_messages can be False when manage_channels is True

Reproduction Steps

On a server that has set up permissions to deny text in voice for voice channels, my bot is reporting an incorrect permission for perms.manage_channels. I was able to trace the issue to a particular guard clause in GuildChannel.permissions_for.

Stripped down to the bare minimum, the code in the bot looks like this:

def bot_can_delete_channel(channel: discord.abc.GuildChannel) -> bool:
    perms = channel.permissions_for(channel.guild.me)
    return perms.manage_channels

Since the text in voice channels feature, this function is now erroneously returning False in the case where the bot actually does have permissions to delete the channel. This only happens if the server has set up permissions to deny text in voice chat for their voice channels (the server basically wants to opt-out of this feature).

Minimal Reproducible Code

def bot_can_delete_channel(channel: discord.abc.GuildChannel) -> bool:
    perms = channel.permissions_for(channel.guild.me)
    return perms.manage_channels

Expected Results

This is the issue: https://github.com/Rapptz/discord.py/blob/dc50736bfc3340d7b999d9f165808f8dcb8f1a60/discord/abc.py#L772

I confirmed by copy/pasting this function into my own codebase and commenting out this guard clause. The bot is now able to properly delete voice channels again.

The issue is that, for these voice channels, base.read_messages is False. But base.manage_channels is True.

Actual Results

What happens now is that if I rely on GuildChannel.permissions_for as it is currently written, I'm not able to correctly identify voice channels that I have permission to delete, even when I actually do have permissions to delete it.

Intents

3276543

System Information

- Python v3.10.4-final
- discord.py v2.0.0-alpha
    - discord.py pkg_resources: v2.0.0a4448+g13c725f1
- aiohttp v3.8.1
- system info: Darwin 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:37 PDT 2022; root:xnu-8020.121.3~4/RELEASE_ARM64_T6000

Checklist

• [X] I have searched the open issues for duplicates.
• [X] I have shown the entire traceback, if possible.
• [X] I have removed my token from display, if visible.

Additional Context

No response

[Rapptz]

I have a fix for this locally but I consider this to be a bug (or oddity really) on Discord's end that I need elaboration on before I commit to it.

I am aware that the TiV permission situation is a bit incorrect right now (for example, blocking connect blocks read_message_history) but I need more information from the devs.

[Rapptz]

FYI, I have just tried this with a test channel with view_channel = False, connect = True, manage_channel = True and I got a 403 when I tried to edit it.

[lexicalunit]

In this case the voice channel text perms are being set as off on the category that the voice channels are in, rather than applied directly on the voice channels themselves. Sorry, didn't realize that would matter.

[Rapptz]

The category permissions shouldn't matter so I'm not sure about that. Regardless I made a category with view_channel = False and a voice channel with connect = True and manage_channel = True and I could not delete it.

[lexicalunit]

Ok, let me ask the server for more information...

[lexicalunit]

I also can't replicate this issue on my own server. But I can see in the logs in production. Here's the instrumented code I'm using. It's just GuildChannel.permissions_for() with a bunch of extra logging:

def permissions_for(
    channel: discord.abc.GuildChannel,
    obj: Union[discord.Member, discord.Role],
    /,
) -> discord.Permissions:
    """
    Copy/paste of code for GuildChannel.permissions_for() from discord.py source.

    Then instrumeted with additional logging.
    """
    from discord.abc import _Overwrites

    if channel.guild.owner_id == obj.id:
        logger.info("permission_for (%s): obj is guild owner", channel.id)
        return discord.Permissions.all()

    default = channel.guild.default_role
    base = discord.Permissions(default.permissions.value)
    logger.info("permission_for (%s): base permissions: %s", channel.id, str(base))

    # Handle the role case first
    if isinstance(obj, discord.Role):
        logger.info(
            "permission_for (%s): obj is a role with permissions: %s",
            channel.id,
            str(obj._permissions),
        )
        base.value |= obj._permissions

        if base.administrator:
            logger.info("permission_for (%s): obj is administrator", channel.id)
            return discord.Permissions.all()

        # Apply @everyone allow/deny first since it's special
        try:
            maybe_everyone = channel._overwrites[0]
            if maybe_everyone.id == channel.guild.id:
                base.handle_overwrite(allow=maybe_everyone.allow, deny=maybe_everyone.deny)
                logger.info(
                    "permission_for (%s): applied @everyone allow/deny: %s",
                    channel.id,
                    str(base),
                )
        except IndexError:
            pass

        if obj.is_default():
            logger.info("permission_for (%s): obj is default", channel.id)
            return base

        overwrite = discord.utils.get(channel._overwrites, type=_Overwrites.ROLE, id=obj.id)
        logger.info("permission_for (%s): role overwrite: %s", channel.id, str(overwrite))
        if overwrite is not None:
            logger.info(
                "permission_for (%s): role overwrite allow: %s",
                channel.id,
                str(overwrite.allow),
            )
            logger.info(
                "permission_for (%s): role overwrite deny: %s",
                channel.id,
                str(overwrite.deny),
            )
            base.handle_overwrite(overwrite.allow, overwrite.deny)
            logger.info("permission_for (%s): applied role overrides: %s", channel.id, str(base))

        return base

    roles = obj._roles
    logger.info("permission_for (%s): roles: %s", channel.id, str(roles))
    get_role = channel.guild.get_role

    # Apply guild roles that the member has.
    for role_id in roles:
        role = get_role(role_id)
        if role is not None:
            logger.info(
                "permission_for (%s): adding permissions for role: %s",
                channel.id,
                str(role),
            )
            base.value |= role._permissions
            logger.info("permission_for (%s): updated permissions: %s", channel.id, str(base))

    # Guild-wide Administrator -> True for everything
    # Bypass all channel-specific overrides
    if base.administrator:
        logger.info("permission_for (%s): obj is an administrator", channel.id)
        return discord.Permissions.all()

    # Apply @everyone allow/deny first since it's special
    try:
        maybe_everyone = channel._overwrites[0]
        if maybe_everyone.id == channel.guild.id:
            base.handle_overwrite(allow=maybe_everyone.allow, deny=maybe_everyone.deny)
            remaining_overwrites = channel._overwrites[1:]
        else:
            remaining_overwrites = channel._overwrites
    except IndexError:
        remaining_overwrites = channel._overwrites
    logger.info(
        "permission_for (%s): permissions after @everyone applied: %s",
        channel.id,
        str(base),
    )

    denies = 0
    allows = 0

    # Apply channel specific role permission overwrites
    logger.info(
        "permission_for (%s): remaining overwrites: %s",
        channel.id,
        str([o._asdict() for o in remaining_overwrites]),
    )
    for overwrite in remaining_overwrites:
        if overwrite.is_role() and roles.has(overwrite.id):
            denies |= overwrite.deny
            allows |= overwrite.allow
            logger.info(
                "permission_for (%s): allowing channel specific overwrite: %s",
                channel.id,
                str(overwrite.allow),
            )
            logger.info(
                "permission_for (%s): denying channel specific overwrite: %s",
                channel.id,
                str(overwrite.deny),
            )

    base.handle_overwrite(allow=allows, deny=denies)
    logger.info(
        "permission_for (%s): base permissions after overwrites: %s",
        channel.id,
        str(base),
    )

    # Apply member specific permission overwrites
    for overwrite in remaining_overwrites:
        if overwrite.is_member() and overwrite.id == obj.id:
            base.handle_overwrite(allow=overwrite.allow, deny=overwrite.deny)
            logger.info(
                "permission_for (%s): applied @everyone allow/deny: %s",
                channel.id,
                str(base),
            )
            break

    # if you can't send a message in a channel then you can't have certain
    # permissions as well
    if not base.send_messages:
        logger.info("permission_for (%s): obj can not send messages", channel.id)
        base.send_tts_messages = False
        base.mention_everyone = False
        base.embed_links = False
        base.attach_files = False

    # if you can't read a channel then you have no permissions there
    if not base.read_messages:
        logger.info("permission_for (%s): obj can not read messages", channel.id)
        denied = discord.Permissions.all_channel()
        base.value &= ~denied.value

    if obj.is_timed_out():
        logger.info("permission_for (%s): obj is timed out", channel.id)
        # Timeout leads to every permission except VIEW_CHANNEL and READ_MESSAGE_HISTORY
        # being explicitly denied
        # N.B.: This *must* come last, because it's a conclusive mask
        base.value &= discord.Permissions._timeout_mask()

    return base

And here's an example log where the bot is unable to delete a voice channel when it should be able to. Removing the if not base.send_messages in this case allows the logic to proceed and the bot is then able to delete the channel upon retry. I manually formatted the remaining overwrites log line to make it easier to read:

permission_for (1004477396126277642): base permissions: <Permissions value=1071698529857>
permission_for (1004477396126277642): roles: SnowflakeList('Q', [991074067148840973])
permission_for (1004477396126277642): adding permissions for role: SpellBot
permission_for (1004477396126277642): updated permissions: <Permissions value=1071967104593>
permission_for (1004477396126277642): permissions after @everyone applied: <Permissions value=1071967103569>
permission_for (1004477396126277642): remaining overwrites: [{
  'id': 991074067148840973,
  'allow': '1049600',
  'deny': '0',
  'type': 'role'
}, {
  'id': 975812570130829403,
  'allow': '0',
  'deny': '2099264',
  'type': 'role'
}, {
  'id': 757457384351465553,
  'allow': '1049600',
  'deny': '0',
  'type': 'role'
}, {
  'id': 757466024156332033,
  'allow': '1049600',
  'deny': '0',
  'type': 'role'
}, {
  'id': 725510263251402832,
  'allow': '1049600',
  'deny': '0',
  'type': 'member'
}]
permission_for (1004477396126277642): base permissions after overwrites: <Permissions value=1071967103569>
permission_for (1004477396126277642): obj can not read messages
bot permissions (1004477396126277642): 549822922752
no permissions to delete channel (1004477396126277642)

The issue seems to happen when the base permissions object is set to 1071967103569 in the log line:

permission_for (1004477396126277642): permissions after @everyone applied: <Permissions value=1071967103569>

After asking a mod about how they have permissions set up on their server, this is their explanation:

• The only denied perm on the voice channels category for @everyone is View category.
• The bot role has Send Text Message: Approved
• Are there text channels in this category: No
• Then why the Send Text Message Deny: Because this is required to stop Text-in-Voice, which has it's own permission slide in a channel, but which does not have it's own dedicated permission slider on a category level.
• As long as we don't put actual text channels in the category, this is currently our 'solution' to prevent people from using text in voice.

[Rapptz]

For some reason the role overwrite is not being applied. I think the reason for this is because the overwrites type key is, for some reason, a string instead of the expected one from Discord where it's an integer. So checks like is_role() and is_member() are failing because the type field is garbage. Which begs the question, why is the overwrite type a string?

[lexicalunit]

Huh... 🤔 Yeah that is wild... how the heck did that happen? I also can't repro that either. Running the bot locally and using my debug guild I see logs like this, where the type field is a Literal[0, 1] as expected:

remaining overwrites: [{'id': 1004556460421816330, 'allow': '1049600', 'deny': '0', 'type': 0}, {'id': 727735010114666578, 'allow': '1049600', 'deny': '0', 'type': 1}]

Grepping my production logs I see a smattering of both cases. Sometimes type is an integer, other times it's a string:

Aug 04 12:28:38 [INFO] - permission_for (754486765414252645): remaining overwrites: [{'id': 304276877588168705, 'allow': '334496784', 'deny': '0', 'type': 0}, {'id': 846228614877413426, 'allow': '0', 'deny': '103081314304', 'type': 0}, {'id': 304276767177310208, 'allow': '1049600', 'deny': '0', 'type': 0}, {'id': 150000998855868416, 'allow': '550081922576', 'deny': '0', 'type': 1}, {'id': 1003050079470506037, 'allow': '1048576', 'deny': '1024', 'type': 0}, {'id': 106905246323785728, 'allow': '66061840', 'deny': '0', 'type': 1}, {'id': 262832477930913793, 'allow': '16778256', 'deny': '0', 'type': 1}]
Aug 04 12:28:38 [INFO] - permission_for (1004793325783613451): remaining overwrites: [{'id': 783179920116678676, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 833885646291992646, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 829770754751987733, 'allow': '1049600', 'deny': '2048', 'type': 'role'}, {'id': 793749129871753226, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748898346827796, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 765545451951751209, 'allow': '1123344', 'deny': '0', 'type': 'role'}, {'id': 371808717731266562, 'allow': '1049600', 'deny': '2048', 'type': 'role'}, {'id': 783180041541910548, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 1003050079470506037, 'allow': '806423568', 'deny': '0', 'type': 'role'}, {'id': 496336161233698847, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748633471811606, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748993082523659, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 919760367863885924, 'allow': '1051648', 'deny': '0', 'type': 'role'}]
Aug 04 12:28:38 [INFO] - permission_for (1003280634787147858): remaining overwrites: [{'id': 793748993082523659, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 496336161233698847, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793748633471811606, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793749129871753226, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 783179920116678676, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 783180041541910548, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 1003050079470506037, 'allow': '806423568', 'deny': '0', 'type': 0}, {'id': 829770754751987733, 'allow': '1049600', 'deny': '2048', 'type': 0}, {'id': 371808717731266562, 'allow': '1049600', 'deny': '2048', 'type': 0}, {'id': 765545451951751209, 'allow': '1123344', 'deny': '0', 'type': 0}, {'id': 919760367863885924, 'allow': '1051648', 'deny': '0', 'type': 0}, {'id': 833885646291992646, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793748898346827796, 'allow': '3146752', 'deny': '2560', 'type': 0}]
Aug 04 12:28:38 [INFO] - permission_for (1003280634787147858): remaining overwrites: [{'id': 793748993082523659, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 496336161233698847, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793748633471811606, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793749129871753226, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 783179920116678676, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 783180041541910548, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 1003050079470506037, 'allow': '806423568', 'deny': '0', 'type': 0}, {'id': 829770754751987733, 'allow': '1049600', 'deny': '2048', 'type': 0}, {'id': 371808717731266562, 'allow': '1049600', 'deny': '2048', 'type': 0}, {'id': 765545451951751209, 'allow': '1123344', 'deny': '0', 'type': 0}, {'id': 919760367863885924, 'allow': '1051648', 'deny': '0', 'type': 0}, {'id': 833885646291992646, 'allow': '3146752', 'deny': '2560', 'type': 0}, {'id': 793748898346827796, 'allow': '3146752', 'deny': '2560', 'type': 0}]
Aug 04 12:28:43 [INFO] - permission_for (1004793325783613451): remaining overwrites: [{'id': 783179920116678676, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 833885646291992646, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 829770754751987733, 'allow': '1049600', 'deny': '2048', 'type': 'role'}, {'id': 793749129871753226, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748898346827796, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 765545451951751209, 'allow': '1123344', 'deny': '0', 'type': 'role'}, {'id': 371808717731266562, 'allow': '1049600', 'deny': '2048', 'type': 'role'}, {'id': 783180041541910548, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 1003050079470506037, 'allow': '806423568', 'deny': '0', 'type': 'role'}, {'id': 496336161233698847, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748633471811606, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 793748993082523659, 'allow': '3146752', 'deny': '2560', 'type': 'role'}, {'id': 919760367863885924, 'allow': '1051648', 'deny': '0', 'type': 'role'}]

I'm seeing the incorrect string type in multiple servers, not just the one that reported issues with voice channel deletion. So it's something that isn't just affecting that one server.

[Rapptz]

I'm going to be frank here.

1. I am aware that you are using a Discord proxy based off of previous discussions.
2. The Discord API has a default version, v6, if no version is passed in the URL. In the future this will change to a more recent version.
3. The v6 API uses strings for overwrites. The v8+ API uses integer types, which is what discord.py expects.

My guess on this is that a misconfigured proxy and fetching channels (using REST, which would use the proxy v6 API) vs receiving them in the gateway (which is using the v10 API) is what's causing this discrepancy. I do not support monkeypatching or Discord proxies because they make issues like this (and others) pop up.

I can't exactly confirm that this is the case, but I also do not think the API will just randomly give these as strings without the version discrepancy. If my guess is incorrect then I believe this issue should be reported in the discord-api-docs repo to see what's going on since it's definitely not a library issue.

[Erk-]

@lexicalunit I assume the reason is that you have monkey patched d.py to use the v7 API which uses integers for permissions compared to newer versions which uses strings to get around some technical limitations. So unless you can reproduce this on newer api versions I think it is a invalid bug.

[lexicalunit]

I'm going to remove the patch and the proxy now, they don't seem to provide the features I was looking for anyway. Will re-open only if I can reproduce it 👍🏻

[lexicalunit]

Confirmed this was due to the monkey patch you pointed out @Erk- -- Thanks all! 🤘