diff --git a/paper-api/src/main/java/io/papermc/paper/InternalAPIBridge.java b/paper-api/src/main/java/io/papermc/paper/InternalAPIBridge.java index 422fdd93d1..65e36495bd 100644 --- a/paper-api/src/main/java/io/papermc/paper/InternalAPIBridge.java +++ b/paper-api/src/main/java/io/papermc/paper/InternalAPIBridge.java @@ -1,5 +1,6 @@ package io.papermc.paper; +import io.papermc.paper.command.brigadier.CommandSourceStack; import io.papermc.paper.world.damagesource.CombatEntry; import io.papermc.paper.world.damagesource.FallLocationType; import net.kyori.adventure.util.Services; @@ -11,6 +12,8 @@ import org.jetbrains.annotations.ApiStatus; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +import java.util.function.Predicate; + /** * Static bridge to the server internals. *

@@ -73,5 +76,15 @@ public interface InternalAPIBridge { * @return combat entry */ CombatEntry createCombatEntry(DamageSource damageSource, float damage, @Nullable FallLocationType fallLocationType, float fallDistance); + + /** + * Causes this predicate to be considered restricted. + * Applying this to a command node prevents this command from being executed from an + * unattended context, such as click events. + * + * @param predicate wrapped predicate + * @return wrapped predicate + */ + Predicate restricted(Predicate predicate); } diff --git a/paper-api/src/main/java/io/papermc/paper/command/brigadier/Commands.java b/paper-api/src/main/java/io/papermc/paper/command/brigadier/Commands.java index 8664429248..3c2d08662d 100644 --- a/paper-api/src/main/java/io/papermc/paper/command/brigadier/Commands.java +++ b/paper-api/src/main/java/io/papermc/paper/command/brigadier/Commands.java @@ -5,6 +5,7 @@ import com.mojang.brigadier.arguments.ArgumentType; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.InternalAPIBridge; import io.papermc.paper.plugin.bootstrap.BootstrapContext; import io.papermc.paper.plugin.bootstrap.PluginBootstrap; import io.papermc.paper.plugin.configuration.PluginMeta; @@ -13,6 +14,7 @@ import io.papermc.paper.plugin.lifecycle.event.registrar.Registrar; import java.util.Collection; import java.util.Collections; import java.util.Set; +import java.util.function.Predicate; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Unmodifiable; import org.jspecify.annotations.NullMarked; @@ -85,6 +87,22 @@ public interface Commands extends Registrar { return RequiredArgumentBuilder.argument(name, argumentType); } + /** + * Creates a restricted {@link Predicate} that wraps the given predicate. + *

+ * A restricted predicate prevents execution in unattended contexts, such as from + * chat click events. A warning is shown on the client before executing the command. + *

+ * This is used by vanilla to prevent invocation of sensitive commands (like op) from + * players without their knowledge. + * + * @param predicate the original predicate to wrap + * @return a new predicate with restricted execution behavior + */ + static Predicate restricted(final Predicate predicate) { + return InternalAPIBridge.get().restricted(predicate); + } + /** * Gets the underlying {@link CommandDispatcher}. * diff --git a/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch b/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch index a3538d58eb..1910b261ab 100644 --- a/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch +++ b/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch @@ -1,6 +1,13 @@ --- a/net/minecraft/commands/Commands.java +++ b/net/minecraft/commands/Commands.java -@@ -176,6 +_,11 @@ +@@ -170,12 +_,18 @@ + + @Override + public boolean isRestricted(CommandNode node) { ++ if (node.getRequirement() instanceof PermissionSource.RestrictedMarker) return true; // Paper - restricted api + return node.getRequirement() instanceof PermissionCheck permissionCheck && permissionCheck.requiredLevel() > 0; + } + }; private final CommandDispatcher dispatcher = new CommandDispatcher<>(); public Commands(Commands.CommandSelection selection, CommandBuildContext context) { @@ -55,71 +62,20 @@ this.dispatcher.setConsumer(ExecutionCommandSource.resultConsumer()); } -@@ -289,9 +_,41 @@ - return new ParseResults<>(commandContextBuilder, parseResults.getReader(), parseResults.getExceptions()); - } - -+ // CraftBukkit start -+ public void dispatchServerCommand(CommandSourceStack sender, String command) { -+ com.google.common.base.Joiner joiner = com.google.common.base.Joiner.on(" "); -+ if (command.startsWith("/")) { -+ command = command.substring(1); -+ } -+ -+ org.bukkit.event.server.ServerCommandEvent event = new org.bukkit.event.server.ServerCommandEvent(sender.getBukkitSender(), command); -+ org.bukkit.Bukkit.getPluginManager().callEvent(event); -+ if (event.isCancelled()) { -+ return; -+ } -+ command = event.getCommand(); -+ -+ String[] args = command.split(" "); -+ if (args.length == 0) return; // Paper - empty commands shall not be dispatched -+ -+ // Paper - Fix permission levels for command blocks -+ -+ // Handle vanilla commands; // Paper - handled in CommandNode/CommandDispatcher -+ -+ String newCommand = joiner.join(args); -+ this.performPrefixedCommand(sender, newCommand, newCommand); -+ } -+ // CraftBukkit end -+ - public void performPrefixedCommand(CommandSourceStack source, String command) { -+ // CraftBukkit start -+ this.performPrefixedCommand(source, command, command); -+ } -+ -+ public void performPrefixedCommand(CommandSourceStack source, String command, String label) { - command = trimOptionalPrefix(command); -- this.performCommand(this.dispatcher.parse(command, source), command); -+ this.performCommand(this.dispatcher.parse(command, source), command, label); -+ // CraftBukkit end - } - - public static String trimOptionalPrefix(String command) { -@@ -299,9 +_,20 @@ +@@ -299,6 +_,13 @@ } public void performCommand(ParseResults parseResults, String command) { -+ // CraftBukkit start -+ this.performCommand(parseResults, command, command); ++ // Paper start ++ this.performCommand(parseResults, command, false); + } + -+ public void performCommand(ParseResults parseResults, String command, String label) { -+ // CraftBukkit end -+ // Paper start -+ this.performCommand(parseResults, command, label, false); -+ } -+ public void performCommand(ParseResults parseResults, String command, String label, boolean throwCommandError) { ++ public void performCommand(ParseResults parseResults, String command, boolean throwCommandError) { ++ org.spigotmc.AsyncCatcher.catchOp("Cannot perform command async"); + // Paper end CommandSourceStack commandSourceStack = parseResults.getContext().getSource(); Profiler.get().push(() -> "/" + command); -- ContextChain contextChain = finishParsing(parseResults, command, commandSourceStack); -+ ContextChain contextChain = this.finishParsing(parseResults, command, commandSourceStack, label); // CraftBukkit // Paper - Add UnknownCommandEvent - - try { - if (contextChain != null) { + ContextChain contextChain = finishParsing(parseResults, command, commandSourceStack); @@ -313,9 +_,10 @@ ); } @@ -133,12 +89,12 @@ StackTraceElement[] stackTrace = var12.getStackTrace(); for (int i = 0; i < Math.min(stackTrace.length, 3); i++) { -@@ -341,18 +_,22 @@ +@@ -341,13 +_,17 @@ } @Nullable - private static ContextChain finishParsing(ParseResults parseResults, String command, CommandSourceStack source) { -+ private ContextChain finishParsing(ParseResults parseResults, String command, CommandSourceStack source, String label) { // CraftBukkit // Paper - Add UnknownCommandEvent ++ private ContextChain finishParsing(ParseResults parseResults, String command, CommandSourceStack source) { try { validateParseResults(parseResults); return ContextChain.tryFlatten(parseResults.getContext().build(command)) @@ -153,12 +109,6 @@ if (var7.getInput() != null && var7.getCursor() >= 0) { int min = Math.min(var7.getInput().length(), var7.getCursor()); MutableComponent mutableComponent = Component.empty() - .withStyle(ChatFormatting.GRAY) -- .withStyle(style -> style.withClickEvent(new ClickEvent.SuggestCommand("/" + command))); -+ .withStyle(style -> style.withClickEvent(new ClickEvent.SuggestCommand("/" + label))); // CraftBukkit // Paper - if (min > 10) { - mutableComponent.append(CommonComponents.ELLIPSIS); - } @@ -364,7 +_,17 @@ } diff --git a/paper-server/patches/sources/net/minecraft/commands/PermissionSource.java.patch b/paper-server/patches/sources/net/minecraft/commands/PermissionSource.java.patch index d71a268e02..0d8a8570bb 100644 --- a/paper-server/patches/sources/net/minecraft/commands/PermissionSource.java.patch +++ b/paper-server/patches/sources/net/minecraft/commands/PermissionSource.java.patch @@ -1,16 +1,18 @@ --- a/net/minecraft/commands/PermissionSource.java +++ b/net/minecraft/commands/PermissionSource.java -@@ -9,9 +_,20 @@ +@@ -9,9 +_,22 @@ return this.hasPermission(2); } - public record Check(@Override int requiredLevel) implements PermissionCheck { -+ public record Check(@Override int requiredLevel, java.util.concurrent.atomic.AtomicReference> vanillaNode) implements PermissionCheck { // Paper -+ // Paper start - Vanilla Command permission checking ++ // Paper start - Vanilla Command permission checking & expose restricted API ++ interface RestrictedMarker { } ++ ++ public record Check(@Override int requiredLevel, java.util.concurrent.atomic.AtomicReference> vanillaNode) implements PermissionCheck { + public Check(int requiredLevel) { + this(requiredLevel, new java.util.concurrent.atomic.AtomicReference<>()); + } -+ // Paper end - Vanilla Command permission checking ++ // Paper end - Vanilla Command permission checking & expose restricted API @Override public boolean test(T source) { + // Paper start - Vanilla Command permission checking diff --git a/paper-server/patches/sources/net/minecraft/server/dedicated/DedicatedServer.java.patch b/paper-server/patches/sources/net/minecraft/server/dedicated/DedicatedServer.java.patch index 9e68c7056d..faf3ec703a 100644 --- a/paper-server/patches/sources/net/minecraft/server/dedicated/DedicatedServer.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/dedicated/DedicatedServer.java.patch @@ -273,7 +273,7 @@ } @Override -@@ -291,13 +_,23 @@ +@@ -291,12 +_,20 @@ } public void handleConsoleInput(String msg, CommandSourceStack source) { @@ -284,23 +284,19 @@ public void handleConsoleInputs() { - while (!this.consoleInput.isEmpty()) { - ConsoleInput consoleInput = this.consoleInput.remove(0); -- this.getCommands().performPrefixedCommand(consoleInput.source, consoleInput.msg); + // Paper start - Perf: use proper queue -+ ConsoleInput servercommand; -+ while ((servercommand = this.serverCommandQueue.poll()) != null) { ++ ConsoleInput consoleInput; ++ while ((consoleInput = this.serverCommandQueue.poll()) != null) { + // Paper end - Perf: use proper queue + // CraftBukkit start - ServerCommand for preprocessing -+ org.bukkit.event.server.ServerCommandEvent event = new org.bukkit.event.server.ServerCommandEvent(this.console, servercommand.msg); ++ org.bukkit.event.server.ServerCommandEvent event = new org.bukkit.event.server.ServerCommandEvent(this.console, consoleInput.msg); + this.server.getPluginManager().callEvent(event); + if (event.isCancelled()) continue; -+ servercommand = new ConsoleInput(event.getCommand(), servercommand.source); -+ -+ // this.getCommands().performPrefixedCommand(servercommand.source, servercommand.msg); // Called in dispatchServerCommand -+ this.server.dispatchServerCommand(this.console, servercommand); ++ consoleInput = new ConsoleInput(event.getCommand(), consoleInput.source); + // CraftBukkit end + this.getCommands().performPrefixedCommand(consoleInput.source, consoleInput.msg); } } - @@ -430,7 +_,11 @@ @Override public boolean enforceSecureProfile() { @@ -314,7 +310,7 @@ } @Override -@@ -515,14 +_,54 @@ +@@ -515,14 +_,53 @@ @Override public String getPluginNames() { @@ -365,8 +361,7 @@ + if (event.isCancelled()) { + return; + } -+ ConsoleInput serverCommand = new ConsoleInput(event.getCommand(), wrapper); -+ this.server.dispatchServerCommand(event.getSender(), serverCommand); ++ this.getCommands().performPrefixedCommand(wrapper, event.getCommand()); + }); + return rconConsoleSource.getCommandResponse(); + // CraftBukkit end diff --git a/paper-server/patches/sources/net/minecraft/world/level/BaseCommandBlock.java.patch b/paper-server/patches/sources/net/minecraft/world/level/BaseCommandBlock.java.patch index 4d22b3d012..73f2de213d 100644 --- a/paper-server/patches/sources/net/minecraft/world/level/BaseCommandBlock.java.patch +++ b/paper-server/patches/sources/net/minecraft/world/level/BaseCommandBlock.java.patch @@ -12,12 +12,18 @@ public int getSuccessCount() { return this.successCount; -@@ -108,7 +_,7 @@ +@@ -108,7 +_,13 @@ this.successCount++; } }); - server.getCommands().performPrefixedCommand(commandSourceStack, this.command); -+ server.getCommands().dispatchServerCommand(commandSourceStack, this.command); // CraftBukkit ++ // Paper start - ServerCommandEvent ++ org.bukkit.event.server.ServerCommandEvent event = new org.bukkit.event.server.ServerCommandEvent(commandSourceStack.getBukkitSender(), net.minecraft.commands.Commands.trimOptionalPrefix(this.command)); ++ if (!event.callEvent()) { ++ return true; ++ } ++ server.getCommands().performPrefixedCommand(commandSourceStack, event.getCommand()); ++ // Paper end - ServerCommandEvent } catch (Throwable var6) { CrashReport crashReport = CrashReport.forThrowable(var6, "Executing command block"); CrashReportCategory crashReportCategory = crashReport.addCategory("Command to be executed"); diff --git a/paper-server/src/main/java/io/papermc/paper/PaperServerInternalAPIBridge.java b/paper-server/src/main/java/io/papermc/paper/PaperServerInternalAPIBridge.java index d693356269..61468be56c 100644 --- a/paper-server/src/main/java/io/papermc/paper/PaperServerInternalAPIBridge.java +++ b/paper-server/src/main/java/io/papermc/paper/PaperServerInternalAPIBridge.java @@ -1,10 +1,12 @@ package io.papermc.paper; +import io.papermc.paper.command.brigadier.CommandSourceStack; import io.papermc.paper.world.damagesource.CombatEntry; import io.papermc.paper.world.damagesource.FallLocationType; import io.papermc.paper.world.damagesource.PaperCombatEntryWrapper; import io.papermc.paper.world.damagesource.PaperCombatTrackerWrapper; import net.minecraft.Optionull; +import net.minecraft.commands.PermissionSource; import net.minecraft.world.damagesource.FallLocation; import org.bukkit.block.Biome; import org.bukkit.craftbukkit.block.CraftBiome; @@ -16,6 +18,7 @@ import org.bukkit.damage.DamageSource; import org.bukkit.entity.LivingEntity; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +import java.util.function.Predicate; @NullMarked public class PaperServerInternalAPIBridge implements InternalAPIBridge { @@ -71,4 +74,16 @@ public class PaperServerInternalAPIBridge implements InternalAPIBridge { damageSource, damage, fallLocation, fallDistance )); } + + @Override + public Predicate restricted(final Predicate predicate) { + record RestrictedPredicate(Predicate predicate) implements Predicate, PermissionSource.RestrictedMarker { + @Override + public boolean test(final CommandSourceStack commandSourceStack) { + return this.predicate.test(commandSourceStack); + } + } + + return new RestrictedPredicate(predicate); + } } diff --git a/paper-server/src/main/java/io/papermc/paper/command/brigadier/bukkit/BukkitBrigForwardingMap.java b/paper-server/src/main/java/io/papermc/paper/command/brigadier/bukkit/BukkitBrigForwardingMap.java index 4ee648f967..fd349089f0 100644 --- a/paper-server/src/main/java/io/papermc/paper/command/brigadier/bukkit/BukkitBrigForwardingMap.java +++ b/paper-server/src/main/java/io/papermc/paper/command/brigadier/bukkit/BukkitBrigForwardingMap.java @@ -20,6 +20,7 @@ import java.util.Spliterator; import java.util.function.Consumer; import java.util.stream.Stream; import org.bukkit.command.Command; +import org.bukkit.craftbukkit.command.VanillaCommandWrapper; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -96,7 +97,7 @@ public class BukkitBrigForwardingMap extends HashMap { public Command put(String key, Command value) { Command old = this.get(key); this.getDispatcher().getRoot().removeCommand(key); // Override previous command - if (value instanceof PluginVanillaCommandWrapper wrapper && wrapper.getName().equals(key)) { + if (value instanceof VanillaCommandWrapper wrapper && wrapper.getName().equals(key)) { // Don't break when some plugin tries to remove and add back a plugin command registered with modern API... this.getDispatcher().getRoot().addChild((CommandNode) wrapper.vanillaCommand); } else { diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java index 0d955ee431..03dfcb4665 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -302,7 +302,6 @@ public final class CraftServer implements Server { public CraftDataPackManager dataPackManager; private final CraftServerTickManager serverTickManager; private final CraftServerLinks serverLinks; - public boolean playerCommandState; private boolean printSaveWarning; private CraftIconCache icon; private boolean overrideAllCommandBlockCommands = false; @@ -973,41 +972,13 @@ public final class CraftServer implements Server { return this.playerList; } - // NOTE: Should only be called from DedicatedServer.ah() - public boolean dispatchServerCommand(CommandSender sender, ConsoleInput serverCommand) { - if (sender instanceof Conversable) { - Conversable conversable = (Conversable) sender; - - if (conversable.isConversing()) { - conversable.acceptConversationInput(serverCommand.msg); - return true; - } - } - try { - this.playerCommandState = true; - return this.dispatchCommand(sender, serverCommand.msg); - } catch (Exception ex) { - this.getLogger().log(Level.WARNING, "Unexpected exception while parsing console command \"" + serverCommand.msg + '"', ex); - return false; - } finally { - this.playerCommandState = false; - } - } - @Override - public boolean dispatchCommand(CommandSender sender, String commandLine) { - Preconditions.checkArgument(sender != null, "sender cannot be null"); + public boolean dispatchCommand(CommandSender rawSender, String commandLine) { + Preconditions.checkArgument(rawSender != null, "sender cannot be null"); Preconditions.checkArgument(commandLine != null, "commandLine cannot be null"); org.spigotmc.AsyncCatcher.catchOp("Command Dispatched Async: " + commandLine); // Spigot // Paper - Include command in error message + CommandSourceStack sourceStack = VanillaCommandWrapper.getListener(rawSender); - if (this.commandMap.dispatch(sender, commandLine)) { - return true; - } - - return this.dispatchCommand(VanillaCommandWrapper.getListener(sender), commandLine); - } - - public boolean dispatchCommand(CommandSourceStack sourceStack, String commandLine) { net.minecraft.commands.Commands commands = this.getHandle().getServer().getCommands(); com.mojang.brigadier.CommandDispatcher dispatcher = commands.getDispatcher(); com.mojang.brigadier.ParseResults results = dispatcher.parse(commandLine, sourceStack); @@ -1017,7 +988,12 @@ public final class CraftServer implements Server { Command target = this.commandMap.getCommand(args[0].toLowerCase(java.util.Locale.ENGLISH)); try { - commands.performCommand(results, commandLine, commandLine, true); + if (results.getContext().getNodes().isEmpty()) { + return false; + } + Commands.validateParseResults(results); + commands.performCommand(results, commandLine, true); + return true; } catch (CommandException ex) { new com.destroystokyo.paper.event.server.ServerExceptionEvent(new com.destroystokyo.paper.exception.ServerCommandException(ex, target, sender, args)).callEvent(); // Paper throw ex; @@ -1026,9 +1002,6 @@ public final class CraftServer implements Server { new com.destroystokyo.paper.event.server.ServerExceptionEvent(new com.destroystokyo.paper.exception.ServerCommandException(ex, target, sender, args)).callEvent(); // Paper throw new CommandException(msg, ex); } - // Paper end - - return false; } @Override @@ -2608,7 +2581,7 @@ public final class CraftServer implements Server { } public void checkSaveState() { - if (this.playerCommandState || this.printSaveWarning || this.console.autosavePeriod <= 0) { + if (this.printSaveWarning || this.console.autosavePeriod <= 0) { return; } this.printSaveWarning = true; diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java b/paper-server/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java index 7876e09b91..f93af795d5 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/command/VanillaCommandWrapper.java @@ -50,7 +50,7 @@ public class VanillaCommandWrapper extends BukkitCommand { // Paper if (!this.testPermission(sender)) return true; CommandSourceStack source = VanillaCommandWrapper.getListener(sender); - this.commands().performPrefixedCommand(source, this.toDispatcher(args, this.getName()), this.toDispatcher(args, commandLabel)); // Paper + this.commands().performPrefixedCommand(source, this.toDispatcher(args, this.getName())); return true; }