Paper icon indicating copy to clipboard operation
Paper copied to clipboard

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

Open willkroboth opened this issue 1 year ago • 0 comments

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.

willkroboth avatar Sep 08 '24 18:09 willkroboth