Revxrsal / Lamp

A powerful, extendable, flexible yet simple to use commands annotation framework.
MIT License
171 stars 33 forks source link

Entity Selector Issue #90

Closed FigT closed 3 months ago

FigT commented 3 months ago

Description

I'm trying to disable @ entity selectors in commands for users without permission. This works for tab auto-complete, however, it doesn't prevent their usage. I tried to make a custom Player value resolver to prevent this, but found the resolvers stack, and cannot be replaced.

Scenario

I have a simple message command, however, with the entity selectors it can be abused to find the nearest player, among other things. As shown the tab complete works fine, but the entity selectors can still be used,
(screenshot without permissions)

Screenshot 2024-03-28 at 4 00 12 PM

(screenshot with op)

Screenshot 2024-03-28 at 4 01 51 PM

Conclusion

What's the best way to go about this? Ability to replace value resolvers? A method to disable entity selectors? I'm willing to submit a PR, just wanting your thoughts on the best path forward. :)

Revxrsal commented 3 months ago

You can disable this feature by calling BukkitBrigadier.disableNativePlayerCompletions(). So something like this:

commandHandler.getBrigadier().ifPresent(brigadier -> {
    brigadier.disableNativePlayerCompletions();
    brigadier.register();
});

Do note that the default Player resolver also has basic support for selectors too. You can overwrite it with something like:

commandHandler.registerValueResolver(/* priority in the stack */ 0, Player.class, context -> { ... });
FigT commented 3 months ago
commandHandler.getBrigadier().ifPresent(brigadier -> {
  brigadier.disableNativePlayerCompletions();
  brigadier.register();
});

I see, I didn't realize this was a method.

But, as you mentioned the default Player resolver still has some support for selectors, so following your example, I tried to disable that, however, this seems to break the sender resolver, causing all Player arguments needing to be provided. I tried circumventing this by checking if the argument is equal to an arbitrary string (self) and using the @Default annotation, however, this is clunky and still allows you to specify a player, and autocompletes. Code snippets and screenshot below.


registration:

this.commandHandler.registerValueResolver(0, Player.class, context -> {
    String name = context.pop();

    if (name.equalsIgnoreCase("self")) {
        return ((BukkitCommandActor) context.actor()).requirePlayer();
    }

    Player player = Bukkit.getPlayer(name);

    if (player == null) {
        throw new CommandErrorException("Player not found: '" + name + "'");
    }

    return player;
});

this.commandHandler.register(...);

this.commandHandler.getBrigadier().ifPresent(brigadier -> {
    brigadier.disableNativePlayerCompletion();
    brigadier.register();
});

command:

    @Command("test")
    private void handleMsgTest(@Default("self") Player player) {
        player.sendMessage("This is a test, " + player.getName());
    }

in-game:

FigT commented 3 months ago

@Revxrsal just bumping this in case you didn’t see

Revxrsal commented 3 months ago

Sender resolver has zero priority, setting the priority to 1 should work.