PaperMC / PaperMC/Paper

Brigadier commands registered through `Commands#getDispatcher` require incorrect permission when executed by `Bukkit#dispatchCommand`

Open
#11,378 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: accepted type: bug version: 1.21.1
Dominant language
Java
Stars
12.7k
Forks
3.5k
Avg merge
3d 13h
Merged PRs (30d)
11

Description

Expected behavior

When I execute my /test command as a player with no permissions, it runs successfully.

Observed/Actual behavior

When I execute my /test command using Bukkit#dispatchCommand, the command fails with the message I'm sorry, but you do not have permission to ...

image

Steps/models to reproduce

I created this plugin that registers two commands:

public final class BukkitTestPlugin extends JavaPlugin {
    @Override
    public void onEnable() {
        getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> {
            Commands commands = event.registrar();
            CommandDispatcher<CommandSourceStack> dispatcher = commands.getDispatcher();

            dispatcher.register(Commands.literal("test")
                .executes(context -> {
                    context.getSource().getSender().sendMessage("You ran the test command");

                    return 1;
                })
            );

            dispatcher.register(Commands.literal("run")
                .then(Commands.argument("command", StringArgumentType.greedyString())
                    .executes(context -> {
                        String command = context.getArgument("command", String.class);

                        Bukkit.dispatchCommand(context.getSource().getSender(), command);

                        return 1;
                    })
                )
            );
        });
    }
}

The /test command can be executed without any permissions. The /run command executes the given command string using Bukkit#dispatchCommand.

Plugin and Datapack List
> plugins
[13:44:11 INFO]: Server Plugins (1):
[13:44:11 INFO]: Bukkit Plugins:
[13:44:11 INFO]:  - BukkitTestPlugin
> datapack list
[13:44:20 INFO]: There are 3 data pack(s) enabled: [vanilla (built-in)], [file/bukkit (world)], [paper (built-in)]
[13:44:20 INFO]: There are no more data packs available
Paper version
> version
[13:44:39 INFO]: Checking version, please wait...
[13:44:40 INFO]: This server is running Paper version 1.21.1-69-master@925c3b9 (2024-09-07T20:43:21Z) (Implementing API version 1.21.1-R0.1-SNAPSHOT)
You are running the latest version
Other

Server log: https://paste.gg/p/anonymous/19898849d44b4a5e884d147aeb2882ad

I'm looking into this problem to try to resolve https://github.com/JorelAli/CommandAPI/issues/583. My analysis of the Paper code that causes that issue and the one here can be found in this comment on that issue. In summary, when Bukkit#dispatchCommand is run, the BukkitBrigForwardingMap added by #8235 converts the CommandNodes in the Brigadier dispatcher into Bukkit Commands. For vanilla commands, CommandAPI commands, and my /test and /run commands mentioned above, this constructor in VanillaCommandWrapper is used:

public class VanillaCommandWrapper extends BukkitCommand {
    public VanillaCommandWrapper(Commands dispatcher, CommandNode<CommandSourceStack> vanillaCommand) {
        super(vanillaCommand.getName(), "A Mojang provided command.", vanillaCommand.getUsageText(), Collections.EMPTY_LIST);
        this.vanillaCommand = vanillaCommand;
        // Incorrectly sets the permission to, for example, `minecraft.commands.test`
        this.setPermission(VanillaCommandWrapper.getPermission(vanillaCommand));
    }
}

This constructor calls setPermission. In the case of my /test command, this means the permission minecraft.commands.test is required to run it through Bukkit#dispatchCommand, rather than being null, or no permission required.

On Spigot, the CommandAPI fixes this by looping through the CommandMap and calling setPermission again to give CommandAPI commands the correct permission String. This works because Spigot only creates the VanillaCommandWrappers once, just before the server finishes start up. The same does not work on Paper, because a new VanillaCommandWrapper is created each time a vanilla node is retrieved from the BukkitBrigForwardingMap, so calls to setPermission do not persist to the next time the Command is retrieved and used.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with VanillaCommandWrapper and the BukkitBrigForwardingMap path described in the issue, then trace how Bukkit#dispatchCommand retrieves and checks converted Brigadier commands. Compare the permission handling for custom and vanilla nodes. Done means a custom command registered through Commands#getDispatcher runs through Bukkit#dispatchCommand without an unintended permission, while vanilla command permissions remain correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.