From 326a307d9e8638b67636f4f728702e938223bd10 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:09:24 +0000 Subject: [PATCH 1/4] Publish the server's commands and group access for the staff panel The LuckPerms bridge now also sends every registered command with its plugin, aliases, description and permission, and whether each LuckPerms group's permissions allow it on this server. Bukkit defaults fill in where LuckPerms has no node. It checks each minute and uploads when the list changes, or every ten minutes. Co-Authored-By: Claude Opus 5.5 --- .../tfmcweb/api/LuckPermsBridgeClient.java | 19 ++ .../tfmcweb/luckperms/BridgeStore.java | 9 + .../tfmcweb/luckperms/CommandCatalogue.java | 254 ++++++++++++++++++ .../tfmcweb/luckperms/LuckPermsBridge.java | 62 ++++- .../luckperms/LuckPermsBridgeStore.java | 22 ++ .../api/LuckPermsBridgeClientTest.java | 20 ++ .../luckperms/CommandCatalogueTest.java | 208 ++++++++++++++ .../luckperms/LuckPermsBridgeStoreTest.java | 30 +++ .../luckperms/LuckPermsBridgeTest.java | 104 ++++++- 9 files changed, 723 insertions(+), 5 deletions(-) create mode 100644 src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java create mode 100644 src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java diff --git a/src/main/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClient.java b/src/main/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClient.java index b90d9e4..e7864b3 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClient.java +++ b/src/main/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClient.java @@ -31,6 +31,7 @@ public final class LuckPermsBridgeClient { static final String SNAPSHOT_UNCHANGED = "/luckperms/plugin/snapshot/unchanged"; static final String CHANGES = "/luckperms/plugin/changes"; static final String RESULTS = "/luckperms/plugin/changes/results"; + static final String COMMANDS = "/luckperms/plugin/commands"; private LuckPermsBridgeClient() {} @@ -370,6 +371,24 @@ public static CallResult putSnapshot(String json) { return CallResult.success(); } + /** Replaces the site's command list for the Commands tab. */ + public static CallResult putCommands(String json) { + GatewayResult raw = ProvinceSystemGateway.requestBytes( + "PUT", + COMMANDS, + json.getBytes(StandardCharsets.UTF_8), + "application/json" + ); + if (!raw.ok) { + return CallResult.fail(raw.status, raw.error); + } + JsonObject root = object(raw.body); + if (root == null || !isTrue(root.get("ok"))) { + return CallResult.fail(raw.status, "Command list was not accepted"); + } + return CallResult.success(); + } + public static UnchangedResult snapshotUnchanged(String hash, long revision) { JsonObject body = new JsonObject(); body.addProperty("hash", hash); diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/BridgeStore.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/BridgeStore.java index 0751bee..8e544f3 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/BridgeStore.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/BridgeStore.java @@ -1,5 +1,8 @@ package net.tfminecraft.tfmcweb.luckperms; +import java.util.Map; +import java.util.function.Function; + import com.google.gson.JsonObject; import net.tfminecraft.tfmcweb.api.LuckPermsBridgeClient.Change; @@ -32,4 +35,10 @@ public interface BridgeStore { * saves once, or changes nothing. Never throws for a refused change. */ ChangeResult apply(Change change); + + /** + * @return each group's check on this server: true or false as LuckPerms resolves the + * permission, or null when the group has no node for it + */ + Map> groupChecks(); } diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java new file mode 100644 index 0000000..6141f6b --- /dev/null +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java @@ -0,0 +1,254 @@ +package net.tfminecraft.tfmcweb.luckperms; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import org.bukkit.Bukkit; +import org.bukkit.command.Command; +import org.bukkit.command.PluginIdentifiableCommand; +import org.bukkit.permissions.Permission; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.java.JavaPlugin; + +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; + +/** + * Every command this server knows, and which LuckPerms groups its permission allows, + * for the staff panel's Commands tab. Only the command's own permission is checked; + * plugins may refuse a command for other reasons when it runs. + */ +public final class CommandCatalogue { + + static final long READ_TIMEOUT_SECONDS = 10; + static final int TEXT_MAX = 300; + private static final String DEFAULT_USAGE = "/"; + + /** One registered command. Aliases exclude the name; shadowed means the plain name runs another plugin's command. */ + public record Entry( + String plugin, + String name, + List aliases, + String description, + String usage, + String permission, + boolean shadowed + ) {} + + /** What the main thread reads: commands, plugin versions, and permissions every non-op player holds by default. */ + public record Server(List commands, Map versions, Map defaults) {} + + private CommandCatalogue() {} + + /** Reads on the main thread and waits for it, from the bridge's worker thread. */ + static Server readOnMainThread(JavaPlugin plugin) throws Exception { + Future read = Bukkit.getScheduler().callSyncMethod(plugin, CommandCatalogue::read); + try { + return read.get(READ_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } finally { + read.cancel(false); + } + } + + /** Main thread only: the command map and plugin manager are not thread-safe. */ + static Server read() { + return read( + Bukkit.getCommandMap().getKnownCommands(), + Bukkit.getPluginManager().getPlugins(), + Bukkit.getPluginManager().getDefaultPermissions(false), + Bukkit.getPluginManager()::getPermission + ); + } + + static Server read( + Map known, + Plugin[] plugins, + Set defaultPermissions, + Function lookup + ) { + Map names = new HashMap<>(); + Map versions = new TreeMap<>(); + for (Plugin plugin : plugins) { + names.put(plugin.getName().toLowerCase(Locale.ROOT), plugin.getName()); + versions.put(plugin.getName(), plugin.getPluginMeta().getVersion()); + } + return new Server(entries(known, names), versions, defaults(defaultPermissions, lookup)); + } + + /** Groups the command map's labels by command, so each command appears once with its aliases. */ + static List entries(Map known, Map pluginNames) { + Map> labels = new IdentityHashMap<>(); + Map namespaces = new IdentityHashMap<>(); + for (Map.Entry entry : known.entrySet()) { + Command command = entry.getValue(); + if (command == null) { + continue; + } + labels.computeIfAbsent(command, ignored -> new ArrayList<>()); + String key = entry.getKey().toLowerCase(Locale.ROOT); + int colon = key.indexOf(':'); + if (colon < 0) { + labels.get(command).add(key); + } else { + namespaces.putIfAbsent(command, key.substring(0, colon)); + } + } + List entries = new ArrayList<>(); + for (Map.Entry> entry : labels.entrySet()) { + Command command = entry.getKey(); + String name = command.getName().toLowerCase(Locale.ROOT); + TreeSet aliases = new TreeSet<>(entry.getValue()); + boolean shadowed = !aliases.remove(name); + entries.add(new Entry( + owner(command, namespaces.get(command), pluginNames), + name, + List.copyOf(aliases), + text(command.getDescription()), + usage(command.getUsage(), name), + text(command.getPermission()), + shadowed + )); + } + entries.sort(Comparator.comparing(Entry::plugin, String.CASE_INSENSITIVE_ORDER) + .thenComparing(Entry::name) + .thenComparing(Entry::shadowed)); + return entries; + } + + private static String owner(Command command, String namespace, Map pluginNames) { + if (command instanceof PluginIdentifiableCommand owned) { + return owned.getPlugin().getName(); + } + if (namespace == null) { + return "Other"; + } + String plugin = pluginNames.get(namespace); + if (plugin != null) { + return plugin; + } + return namespace.substring(0, 1).toUpperCase(Locale.ROOT) + namespace.substring(1); + } + + /** Drops the usage Bukkit and Paper fill in when a plugin sets none. */ + private static String usage(String usage, String name) { + String text = text(usage); + if (text == null || text.equals(DEFAULT_USAGE) || text.equalsIgnoreCase("/" + name)) { + return null; + } + return text; + } + + private static String text(String text) { + if (text == null || text.isBlank()) { + return null; + } + String trimmed = text.strip(); + return trimmed.length() > TEXT_MAX ? trimmed.substring(0, TEXT_MAX) : trimmed; + } + + /** Bukkit's defaults for a non-op player: each default permission and, recursively, its children. */ + static Map defaults(Set permissions, Function lookup) { + Map values = new HashMap<>(); + for (Permission permission : permissions) { + addDefault(permission.getName(), true, values, lookup); + } + return values; + } + + private static void addDefault(String name, boolean value, Map values, Function lookup) { + String key = name.toLowerCase(Locale.ROOT); + if (values.containsKey(key)) { + return; + } + values.put(key, value); + Permission permission = lookup.apply(key); + if (permission == null) { + return; + } + for (Map.Entry child : permission.getChildren().entrySet()) { + // As Bukkit does: a child set false under a denied parent ends up allowed. + addDefault(child.getKey(), child.getValue() == value, values, lookup); + } + } + + /** + * @param checks each group's LuckPerms answer: true, false, or null when LuckPerms has no node + * @return {@code {"groups": [...], "plugins": [...], "commands": [...]}} in a stable order; each + * command's {@code access} has one {@code y} or {@code n} per group, or is null with no permission + */ + static JsonObject payload(Server server, Map> checks) { + Map> groups = new TreeMap<>(checks); + JsonArray groupNames = new JsonArray(); + groups.keySet().forEach(groupNames::add); + Map owners = new LinkedHashMap<>(); + JsonArray commands = new JsonArray(); + for (Entry entry : server.commands()) { + owners.put(entry.plugin(), Boolean.TRUE); + JsonObject json = new JsonObject(); + json.addProperty("plugin", entry.plugin()); + json.addProperty("name", entry.name()); + JsonArray aliases = new JsonArray(); + entry.aliases().forEach(aliases::add); + json.add("aliases", aliases); + json.addProperty("description", entry.description()); + json.addProperty("usage", entry.usage()); + json.addProperty("permission", entry.permission()); + json.addProperty("shadowed", entry.shadowed()); + json.addProperty("access", access(entry.permission(), groups.values(), server.defaults())); + commands.add(json); + } + JsonArray plugins = new JsonArray(); + for (String name : owners.keySet()) { + JsonObject plugin = new JsonObject(); + plugin.addProperty("name", name); + plugin.addProperty("version", server.versions().get(name)); + plugins.add(plugin); + } + JsonObject root = new JsonObject(); + root.add("groups", groupNames); + root.add("plugins", plugins); + root.add("commands", commands); + return root; + } + + /** Paper allows a command when any of its {@code ;}-separated permissions is held. */ + static String access(String permission, Iterable> groups, Map defaults) { + if (permission == null) { + return null; + } + StringBuilder out = new StringBuilder(); + for (Function group : groups) { + out.append(allowed(permission, group, defaults) ? 'y' : 'n'); + } + return out.toString(); + } + + private static boolean allowed(String permission, Function group, Map defaults) { + for (String part : permission.split(";")) { + String node = part.strip().toLowerCase(Locale.ROOT); + if (node.isEmpty()) { + continue; + } + Boolean value = group.apply(node); + if (value == null) { + value = defaults.getOrDefault(node, Boolean.FALSE); + } + if (value) { + return true; + } + } + return false; + } +} diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridge.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridge.java index 5d3660b..792abc2 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridge.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridge.java @@ -41,6 +41,15 @@ public final class LuckPermsBridge { static final int REMEMBERED_RESULTS = 500; /** Short, so a reload never stalls the main thread; an older pass that outlives it is harmless. */ static final long STOP_WAIT_SECONDS = 1; + /** Commands change rarely: check each minute, upload when changed or every ten minutes. */ + static final long COMMANDS_CHECK_MILLIS = 60_000L; + static final long COMMANDS_RESEND_MILLIS = 600_000L; + + /** Reads the server's commands; blocks the worker until the main thread answers. */ + @FunctionalInterface + public interface CommandReader { + CommandCatalogue.Server read() throws Exception; + } @FunctionalInterface public interface StoreOpener { @@ -51,6 +60,7 @@ public interface StoreOpener { private final StoreOpener opener; private final Supplier workers; private final LongSupplier clock; + private final CommandReader commands; /** Recent results by change id: a guard so a change seen twice is never applied twice. */ private final Map remembered = new LinkedHashMap<>() { @@ -81,23 +91,38 @@ private record Run(long generation, BridgeStore store, boolean applying, long sn private long stateGeneration = -1L; private long nextSnapshotAt; private String uploadedHash; + private long nextCommandsAt; + private String commandsHash; + private long commandsUploadedAt; /** Last revision handed out; survives reloads, and the clock carries it across restarts. */ private long lastRevision; public LuckPermsBridge(JavaPlugin plugin) { - this(plugin, LuckPermsBridge::openLuckPerms, LuckPermsBridge::newWorker, System::currentTimeMillis); + this( + plugin, + LuckPermsBridge::openLuckPerms, + LuckPermsBridge::newWorker, + System::currentTimeMillis, + mainThreadReader(plugin) + ); + } + + static CommandReader mainThreadReader(JavaPlugin plugin) { + return () -> CommandCatalogue.readOnMainThread(plugin); } LuckPermsBridge( JavaPlugin plugin, StoreOpener opener, Supplier workers, - LongSupplier clock + LongSupplier clock, + CommandReader commands ) { this.plugin = plugin; this.opener = opener; this.workers = workers; this.clock = clock; + this.commands = commands; } /** @@ -220,6 +245,9 @@ private void tick(Run run) { stateGeneration = run.generation(); nextSnapshotAt = 0L; uploadedHash = null; + // Wait a minute, so a starting server has registered every plugin's commands. + nextCommandsAt = clock.getAsLong() + COMMANDS_CHECK_MILLIS; + commandsHash = null; failing.clear(); } long now = clock.getAsLong(); @@ -235,6 +263,36 @@ private void tick(Run run) { nextSnapshotAt = now + run.snapshotMillis(); publish(run); } + if (current(run) && now >= nextCommandsAt) { + nextCommandsAt = now + COMMANDS_CHECK_MILLIS; + publishCommands(run, now); + } + } + } + + private void publishCommands(Run run, long now) { + try { + JsonObject data = CommandCatalogue.payload(commands.read(), run.store().groupChecks()); + if (!current(run)) { + return; + } + String hash = sha256(data.toString()); + if (hash.equals(commandsHash) && now - commandsUploadedAt < COMMANDS_RESEND_MILLIS) { + return; + } + data.addProperty("server", TFMCWeb.getRealmId()); + data.addProperty("generated_at", now / 1000L); + data.addProperty("hash", hash); + CallResult upload = LuckPermsBridgeClient.putCommands(data.toString()); + if (!upload.ok) { + fail("commands", "[luckperms] command list upload failed (" + describe(upload.status) + ")", null); + return; + } + commandsHash = hash; + commandsUploadedAt = now; + recover("commands"); + } catch (Exception e) { + fail("commands", "[luckperms] command list failed", e); } } diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStore.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStore.java index ec63b19..514a8f8 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStore.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStore.java @@ -9,6 +9,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.UUID; import java.util.function.Function; import java.util.function.LongSupplier; @@ -24,6 +25,7 @@ import net.luckperms.api.LuckPermsProvider; import net.luckperms.api.actionlog.Action; import net.luckperms.api.actionlog.ActionLogger; +import net.luckperms.api.cacheddata.CachedPermissionData; import net.luckperms.api.messaging.MessagingService; import net.luckperms.api.model.data.DataMutateResult; import net.luckperms.api.model.data.NodeMap; @@ -35,8 +37,10 @@ import net.luckperms.api.node.NodeBuilder; import net.luckperms.api.node.NodeEqualityPredicate; import net.luckperms.api.node.matcher.NodeMatcher; +import net.luckperms.api.query.QueryOptions; import net.luckperms.api.track.Track; import net.luckperms.api.track.TrackManager; +import net.luckperms.api.util.Tristate; import net.tfminecraft.tfmcweb.api.LuckPermsBridgeClient.Change; import net.tfminecraft.tfmcweb.api.LuckPermsBridgeClient.ChangeResult; import net.tfminecraft.tfmcweb.api.LuckPermsBridgeClient.NodeSpec; @@ -143,6 +147,24 @@ public JsonObject snapshot() { return root; } + @Override + public Map> groupChecks() { + QueryOptions options = LuckPermsProvider.get().getContextManager().getStaticQueryOptions(); + Map> checks = new TreeMap<>(); + for (Group group : groups.getLoadedGroups()) { + CachedPermissionData data = group.getCachedData().getPermissionData(options); + checks.put(group.getName(), permission -> answer(data.checkPermission(permission))); + } + return checks; + } + + static Boolean answer(Tristate value) { + if (value == Tristate.UNDEFINED) { + return null; + } + return Boolean.valueOf(value.asBoolean()); + } + @Override public ChangeResult apply(Change change) { if (change.ops.isEmpty()) { diff --git a/src/test/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClientTest.java b/src/test/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClientTest.java index 8c36d8d..5a752c2 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClientTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/api/LuckPermsBridgeClientTest.java @@ -60,6 +60,26 @@ class LuckPermsBridgeClientTest { assertTrue(unconfigured.error.contains("not configured")); } + @Test void commandListIsUploadedAndRefusalsAreReported() throws Exception { + http.reply(200, "{\"ok\":true}"); + assertTrue(LuckPermsBridgeClient.putCommands("{\"hash\":\"é\"}").ok); + var request = http.request(); + assertEquals("PUT", request.method()); + assertEquals("/luckperms/plugin/commands", request.path()); + assertEquals("{\"hash\":\"é\"}", request.body()); + assertEquals("application/json", request.contentType()); + http.reply(200, "{\"ok\":false}"); + CallResult refused = LuckPermsBridgeClient.putCommands("{}"); + assertFalse(refused.ok); + assertEquals("Command list was not accepted", refused.error); + http.request(); + http.reply(400, "{\"detail\":\"bad_commands\"}"); + CallResult bad = LuckPermsBridgeClient.putCommands("{}"); + assertEquals(400, bad.status); + assertEquals("bad_commands", bad.error); + http.request(); + } + @Test void unchangedCheckTellsCurrentFromNeedFull() throws Exception { http.reply(200, "{\"ok\":true}"); UnchangedResult current = LuckPermsBridgeClient.snapshotUnchanged("abc", 7L); diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java new file mode 100644 index 0000000..60d2dbf --- /dev/null +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java @@ -0,0 +1,208 @@ +package net.tfminecraft.tfmcweb.luckperms; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import com.google.gson.JsonObject; +import io.papermc.paper.plugin.configuration.PluginMeta; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; +import net.tfminecraft.tfmcweb.luckperms.CommandCatalogue.Entry; +import net.tfminecraft.tfmcweb.luckperms.CommandCatalogue.Server; +import org.bukkit.Bukkit; +import org.bukkit.command.Command; +import org.bukkit.command.CommandMap; +import org.bukkit.command.CommandSender; +import org.bukkit.command.PluginIdentifiableCommand; +import org.bukkit.permissions.Permission; +import org.bukkit.permissions.PermissionDefault; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.java.JavaPlugin; +import org.bukkit.scheduler.BukkitScheduler; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +class CommandCatalogueTest { + + static class Plain extends Command { + Plain(String name) { + super(name); + } + + @Override + public boolean execute(CommandSender sender, String label, String[] args) { + return true; + } + } + + static final class Owned extends Plain implements PluginIdentifiableCommand { + final Plugin owner; + + Owned(String name, Plugin owner) { + super(name); + this.owner = owner; + } + + @Override + public Plugin getPlugin() { + return owner; + } + } + + static Plugin plugin(String name, String version) { + Plugin plugin = mock(Plugin.class); + PluginMeta meta = mock(PluginMeta.class); + when(plugin.getName()).thenReturn(name); + when(plugin.getPluginMeta()).thenReturn(meta); + when(meta.getVersion()).thenReturn(version); + return plugin; + } + + @Test void groupsLabelsByCommandAndNamesTheOwningPlugin() { + Plugin disguises = plugin("LibsDisguises", "11.0.14"); + Owned clone = new Owned("DisguiseClone", disguises); + clone.setPermission("libsdisguises.seecmd.disguiseclone"); + clone.setDescription(" Copy a disguise. "); + Plain tp = new Plain("tp"); + tp.setPermission("minecraft.command.teleport"); + tp.setUsage("/tp "); + Plain region = new Plain("region"); + region.setDescription(" "); + Plain me = new Plain("me"); + Plain loose = new Plain("loose"); + loose.setUsage("/"); + Plain other = new Plain("other"); + Map known = new LinkedHashMap<>(); + known.put("disguiseclone", clone); + known.put("dclone", clone); + known.put("libsdisguises:disguiseclone", clone); + known.put("libsdisguises:dclone", clone); + known.put("TP", tp); + known.put("minecraft:tp", tp); + known.put("worldguard:region", region); + known.put("rg", region); + known.put("rpcharacters:me", me); + known.put("gone", null); + known.put("loose", loose); + known.put("custom:other", other); + + List entries = CommandCatalogue.entries(known, Map.of("worldguard", "WorldGuard")); + + assertEquals(List.of( + new Entry("Custom", "other", List.of(), null, null, null, true), + new Entry("LibsDisguises", "disguiseclone", List.of("dclone"), "Copy a disguise.", null, + "libsdisguises.seecmd.disguiseclone", false), + new Entry("Minecraft", "tp", List.of(), null, "/tp ", "minecraft.command.teleport", false), + new Entry("Other", "loose", List.of(), null, null, null, false), + new Entry("Rpcharacters", "me", List.of(), null, null, null, true), + new Entry("WorldGuard", "region", List.of("rg"), null, null, null, true) + ), entries); + } + + @Test void longTextIsCut() { + Plain command = new Plain("long"); + command.setDescription("x".repeat(CommandCatalogue.TEXT_MAX + 5)); + Entry entry = CommandCatalogue.entries(Map.of("long", command), Map.of()).get(0); + assertEquals(CommandCatalogue.TEXT_MAX, entry.description().length()); + } + + @Test void defaultsFollowBukkitChildInheritance() { + Permission seecmd = new Permission("libsdisguises.seecmd", PermissionDefault.TRUE, + Map.of("libsdisguises.seecmd.disguise", true, "libsdisguises.seecmd.hidden", false)); + Permission hidden = new Permission("libsdisguises.seecmd.hidden", PermissionDefault.OP, + Map.of("deep.denied", false, "deep.allowed", true)); + Permission loop = new Permission("loop", PermissionDefault.TRUE, Map.of("libsdisguises.seecmd", true)); + Map registered = new HashMap<>(); + for (Permission permission : List.of(seecmd, hidden, loop)) { + registered.put(permission.getName(), permission); + } + + Map defaults = CommandCatalogue.defaults(Set.of(seecmd, loop), registered::get); + + assertEquals(Map.of( + "libsdisguises.seecmd", true, + "libsdisguises.seecmd.disguise", true, + "libsdisguises.seecmd.hidden", false, + "deep.denied", true, + "deep.allowed", false, + "loop", true + ), defaults); + } + + @Test void accessAsksLuckPermsThenFallsBackToDefaults() { + Function staff = permission -> permission.startsWith("admin.") ? Boolean.TRUE : null; + Function player = permission -> permission.equals("blocked") ? Boolean.FALSE : null; + Map defaults = Map.of("blocked", true, "open", true); + + assertNull(CommandCatalogue.access(null, List.of(staff, player), defaults)); + assertEquals("yn", CommandCatalogue.access("admin.x", List.of(staff, player), defaults)); + assertEquals("yy", CommandCatalogue.access("OPEN", List.of(staff, player), defaults)); + assertEquals("yn", CommandCatalogue.access("blocked", List.of(staff, player), defaults)); + assertEquals("yn", CommandCatalogue.access(" ; nothing;admin.y", List.of(staff, player), defaults)); + } + + @Test void payloadListsGroupsPluginsAndCommandsInAStableOrder() { + Server server = new Server( + List.of( + new Entry("Essentials", "anvil", List.of("eanvil"), "Opens an anvil.", null, "essentials.anvil", false), + new Entry("Essentials", "god", List.of(), null, "/god [player]", "essentials.god", false), + new Entry("Minecraft", "me", List.of(), null, null, null, true) + ), + Map.of("Essentials", "2.22.0"), + Map.of() + ); + Map> checks = new LinkedHashMap<>(); + checks.put("staff", permission -> Boolean.TRUE); + checks.put("default", permission -> permission.equals("essentials.anvil") ? Boolean.TRUE : null); + + JsonObject payload = CommandCatalogue.payload(server, checks); + + assertEquals("[\"default\",\"staff\"]", payload.getAsJsonArray("groups").toString()); + assertEquals("[{\"name\":\"Essentials\",\"version\":\"2.22.0\"},{\"name\":\"Minecraft\",\"version\":null}]", + payload.getAsJsonArray("plugins").toString()); + assertEquals("{\"plugin\":\"Essentials\",\"name\":\"anvil\",\"aliases\":[\"eanvil\"]," + + "\"description\":\"Opens an anvil.\",\"usage\":null,\"permission\":\"essentials.anvil\",\"shadowed\":false," + + "\"access\":\"yy\"}", payload.getAsJsonArray("commands").get(0).toString()); + assertEquals("ny", payload.getAsJsonArray("commands").get(1).getAsJsonObject().get("access").getAsString()); + assertEquals("/god [player]", payload.getAsJsonArray("commands").get(1).getAsJsonObject().get("usage").getAsString()); + assertTrue(payload.getAsJsonArray("commands").get(2).getAsJsonObject().get("shadowed").getAsBoolean()); + } + + @Test void readsTheLiveServerOnTheMainThread() throws Exception { + Plugin core = plugin("TFMCCore", "3.5.3"); + Owned tfmc = new Owned("tfmc", core); + Permission open = new Permission("tfmc.open", PermissionDefault.TRUE); + CommandMap map = mock(CommandMap.class); + when(map.getKnownCommands()).thenReturn(Map.of("tfmc", tfmc, "tfmccore:tfmc", tfmc)); + PluginManager manager = mock(PluginManager.class); + when(manager.getPlugins()).thenReturn(new Plugin[] {core}); + when(manager.getDefaultPermissions(false)).thenReturn(Set.of(open)); + when(manager.getPermission("tfmc.open")).thenReturn(open); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + JavaPlugin web = mock(JavaPlugin.class); + CompletableFuture answer = new CompletableFuture<>(); + when(scheduler.callSyncMethod(eq(web), any())).thenAnswer(call -> { + answer.complete(call.>getArgument(1).call()); + return answer; + }); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getCommandMap).thenReturn(map); + bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); + + Server server = CommandCatalogue.readOnMainThread(web); + + assertEquals(List.of(new Entry("TFMCCore", "tfmc", List.of(), null, null, null, false)), server.commands()); + assertEquals(Map.of("TFMCCore", "3.5.3"), server.versions()); + assertEquals(Map.of("tfmc.open", true), server.defaults()); + } + } +} diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStoreTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStoreTest.java index e73aa05..b090ee5 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStoreTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeStoreTest.java @@ -57,6 +57,12 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.function.Function; +import net.luckperms.api.cacheddata.CachedDataManager; +import net.luckperms.api.cacheddata.CachedPermissionData; +import net.luckperms.api.context.ContextManager; +import net.luckperms.api.query.QueryOptions; +import net.luckperms.api.util.Tristate; import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; @@ -691,4 +697,28 @@ static List keys(JsonObject holder) { assertEquals("save_failed", store.apply(change("track", "broken", op("delete_track"))).error); verifyNoInteractions(actions, messaging); } + + @Test void groupChecksAskEachGroupOnThisServer() { + QueryOptions options = mock(QueryOptions.class); + ContextManager contexts = mock(ContextManager.class); + when(contexts.getStaticQueryOptions()).thenReturn(options); + when(LuckPermsProvider.get().getContextManager()).thenReturn(contexts); + Group staff = mock(Group.class); + when(staff.getName()).thenReturn("staff"); + CachedDataManager cached = mock(CachedDataManager.class); + CachedPermissionData permissions = mock(CachedPermissionData.class); + when(staff.getCachedData()).thenReturn(cached); + when(cached.getPermissionData(options)).thenReturn(permissions); + when(permissions.checkPermission("a")).thenReturn(Tristate.TRUE); + when(permissions.checkPermission("b")).thenReturn(Tristate.FALSE); + when(permissions.checkPermission("c")).thenReturn(Tristate.UNDEFINED); + when(groups.getLoadedGroups()).thenReturn(Set.of(staff)); + + Map> checks = store().groupChecks(); + + assertEquals(Set.of("staff"), checks.keySet()); + assertEquals(Boolean.TRUE, checks.get("staff").apply("a")); + assertEquals(Boolean.FALSE, checks.get("staff").apply("b")); + assertNull(checks.get("staff").apply("c")); + } } diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeTest.java index e851ca6..5c60b6d 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/LuckPermsBridgeTest.java @@ -11,11 +11,13 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; import net.luckperms.api.LuckPerms; @@ -62,6 +64,10 @@ class LuckPermsBridgeTest { List> posts; CallResult uploadAnswer = CallResult.success(); CallResult postAnswer = CallResult.success(); + /** Command lists the site accepted. */ + List commandUploads; + CallResult commandsAnswer = CallResult.success(); + CommandCatalogue.Server commandList = server("tfmc"); /** In-memory LuckPerms: the snapshot is whatever {@link #data} holds. */ static final class RecordingStore implements BridgeStore { @@ -93,6 +99,10 @@ static JsonObject data(String group) { return data.deepCopy(); } + @Override public Map> groupChecks() { + return Map.of("default", permission -> null, "staff", permission -> Boolean.TRUE); + } + @Override public ChangeResult apply(Change change) { duringApply.run(); if (boom.contains(change.id)) { @@ -134,6 +144,13 @@ static JsonObject data(String group) { } return uploadAnswer; }); + commandUploads = new ArrayList<>(); + api.when(() -> LuckPermsBridgeClient.putCommands(anyString())).thenAnswer(call -> { + if (commandsAnswer.ok) { + commandUploads.add(call.getArgument(0)); + } + return commandsAnswer; + }); api.when(() -> LuckPermsBridgeClient.snapshotUnchanged(anyString(), anyLong())) .thenReturn(UnchangedResult.answered(true)); api.when(() -> LuckPermsBridgeClient.postResults(anyCollection())).thenAnswer(call -> { @@ -142,7 +159,7 @@ static JsonObject data(String group) { }); store = new RecordingStore(); clock = new AtomicLong(1_791_321_779_000L); - bridge = new LuckPermsBridge(plugin, () -> store, () -> worker, clock::get); + bridge = new LuckPermsBridge(plugin, () -> store, () -> worker, clock::get, () -> commandList); Cache.luckPermsBridgePublish = true; Cache.luckPermsBridgeApply = true; Cache.luckPermsBridgePollSeconds = 3; @@ -171,6 +188,14 @@ void queue(Change... changes) { api.when(LuckPermsBridgeClient::listChanges).thenReturn(ChangesResult.success(List.of(changes))); } + static CommandCatalogue.Server server(String command) { + return new CommandCatalogue.Server( + List.of(new CommandCatalogue.Entry("TFMCCore", command, List.of(), null, null, "tfmc.use", false)), + Map.of("TFMCCore", "3.5.3"), + Map.of() + ); + } + JsonObject lastUpload() { return JsonParser.parseString(uploads.getLast()).getAsJsonObject(); } @Test void staysIdleUntilEnabledAndLuckPermsIsInstalled() { @@ -234,10 +259,10 @@ void queue(Change... changes) { } @Test void unavailableLuckPermsLeavesTheBridgeOff() { - bridge = new LuckPermsBridge(plugin, () -> null, () -> worker, clock::get); + bridge = new LuckPermsBridge(plugin, () -> null, () -> worker, clock::get, () -> commandList); bridge.refresh(); verify(logger).warning(contains("unavailable")); - bridge = new LuckPermsBridge(plugin, () -> { throw new IllegalStateException("not loaded"); }, () -> worker, clock::get); + bridge = new LuckPermsBridge(plugin, () -> { throw new IllegalStateException("not loaded"); }, () -> worker, clock::get, () -> commandList); bridge.refresh(); verify(logger).log(eq(Level.WARNING), contains("unavailable"), any(IllegalStateException.class)); assertFalse(bridge.isRunning()); @@ -252,6 +277,8 @@ void queue(Change... changes) { LuckPermsBridge real = new LuckPermsBridge(plugin); real.refresh(); assertFalse(real.isRunning()); + // No scheduler outside a server: the real reader fails rather than blocking. + assertThrows(NullPointerException.class, () -> LuckPermsBridge.mainThreadReader(plugin).read()); } @Test void workerIsOneNamedDaemonThread() throws Exception { @@ -530,4 +557,75 @@ void queue(Change... changes) { assertTrue(messages.stream().anyMatch(line -> line.contains("luckperms-bridge: ") && line.endsWith("bridge=off (LuckPerms unavailable)"))); assertTrue(messages.stream().anyMatch(line -> line.contains("luckperms-bridge: ") && line.contains("bridge=on last-snapshot=never"))); } + + JsonObject lastCommands() { return JsonParser.parseString(commandUploads.getLast()).getAsJsonObject(); } + + @Test void commandListWaitsAMinuteThenUploadsWhenChangedOrEveryTenMinutes() { + Cache.luckPermsBridgeApply = false; + start(); + tick(); + clock.addAndGet(59_999L); + tick(); + assertTrue(commandUploads.isEmpty()); + clock.addAndGet(1L); + tick(); + JsonObject first = lastCommands(); + assertEquals("main", first.get("server").getAsString()); + assertEquals(1_791_321_839L, first.get("generated_at").getAsLong()); + assertEquals(64, first.get("hash").getAsString().length()); + assertEquals("[\"default\",\"staff\"]", first.getAsJsonArray("groups").toString()); + JsonObject command = first.getAsJsonArray("commands").get(0).getAsJsonObject(); + assertEquals("tfmc", command.get("name").getAsString()); + assertEquals("ny", command.get("access").getAsString()); + + clock.addAndGet(60_000L); + tick(); + assertEquals(1, commandUploads.size()); + commandList = server("guide"); + clock.addAndGet(60_000L); + tick(); + assertEquals(2, commandUploads.size()); + assertEquals("guide", lastCommands().getAsJsonArray("commands").get(0).getAsJsonObject().get("name").getAsString()); + clock.addAndGet(600_000L); + tick(); + assertEquals(3, commandUploads.size()); + } + + @Test void commandListFailuresAreLoggedOnceAndRetried() { + Cache.luckPermsBridgeApply = false; + commandsAnswer = CallResult.fail(503, "down"); + start(); + tick(); + clock.addAndGet(60_000L); + tick(); + clock.addAndGet(60_000L); + tick(); + verify(logger, times(1)).warning("[luckperms] command list upload failed (HTTP 503)"); + commandsAnswer = CallResult.success(); + clock.addAndGet(60_000L); + tick(); + assertEquals(1, commandUploads.size()); + verify(logger).info("[luckperms] commands working again"); + + IllegalStateException boom = new IllegalStateException("timed out"); + bridge = new LuckPermsBridge(plugin, () -> store, () -> worker, clock::get, () -> { throw boom; }); + bridge.refresh(); + tick(); + clock.addAndGet(60_000L); + tick(); + verify(logger).log(Level.WARNING, "[luckperms] command list failed", boom); + } + + @Test void supersededCommandReadUploadsNothing() { + Cache.luckPermsBridgeApply = false; + bridge = new LuckPermsBridge(plugin, () -> store, () -> worker, clock::get, () -> { + bridge.stop(); + return commandList; + }); + bridge.refresh(); + tick(); + clock.addAndGet(60_000L); + tick(); + assertTrue(commandUploads.isEmpty()); + } } From 4e7bc40e96d229492239707c87b86fdbe5a57393 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:35:19 +0000 Subject: [PATCH 2/4] Join Paper's Brigadier wrappers into one entry per command Paper wraps each Brigadier label afresh on every read of the command map, so labels are now joined by owner and name: vanilla aliases by the node they redirect to, plugin aliases by the aliases Paper lists on the main label, and unlinked wrappers take the namespace of their namespaced copy. Default permissions now win over a child set false elsewhere, as in LuckPerms, and access reads the whole permission. Co-Authored-By: Claude Opus 5.5 --- .../tfmcweb/luckperms/CommandCatalogue.java | 163 ++++++++++++++---- .../luckperms/CommandCatalogueTest.java | 79 ++++++++- 2 files changed, 210 insertions(+), 32 deletions(-) diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java index 6141f6b..64d09de 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.List; @@ -14,6 +15,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.function.Function; +import java.lang.reflect.Field; import org.bukkit.Bukkit; import org.bukkit.command.Command; @@ -23,6 +25,7 @@ import org.bukkit.plugin.java.JavaPlugin; import com.google.gson.JsonArray; +import com.mojang.brigadier.tree.CommandNode; import com.google.gson.JsonObject; /** @@ -34,7 +37,10 @@ public final class CommandCatalogue { static final long READ_TIMEOUT_SECONDS = 10; static final int TEXT_MAX = 300; + /** Sent for display only; access is decided on the whole permission. */ + static final int PERMISSION_MAX = 2048; private static final String DEFAULT_USAGE = "/"; + private static final String VANILLA_WRAPPER = "VanillaCommandWrapper"; /** One registered command. Aliases exclude the name; shadowed means the plain name runs another plugin's command. */ public record Entry( @@ -87,52 +93,115 @@ static Server read( return new Server(entries(known, names), versions, defaults(defaultPermissions, lookup)); } - /** Groups the command map's labels by command, so each command appears once with its aliases. */ - static List entries(Map known, Map pluginNames) { - Map> labels = new IdentityHashMap<>(); + /** + * Groups the command map's labels into one entry per command and owner. Paper wraps each + * Brigadier label in its own object, so labels are joined by name: a vanilla alias by the + * node it redirects to, a plugin alias by the aliases Paper lists on its main label. + */ + static List entries(Map live, Map pluginNames) { + // Paper's map wraps Brigadier nodes afresh on every read, so read it once. + Map known = new LinkedHashMap<>(live); Map namespaces = new IdentityHashMap<>(); + // A copy under "ns:label" names the namespace of an unlinked wrapper of the same kind. + Map namespaceByLabel = new HashMap<>(); + for (Map.Entry entry : known.entrySet()) { + String key = entry.getKey().toLowerCase(Locale.ROOT); + int colon = key.indexOf(':'); + if (entry.getValue() != null && colon > 0) { + String namespace = key.substring(0, colon); + namespaces.putIfAbsent(entry.getValue(), namespace); + namespaceByLabel.putIfAbsent(sameKind(entry.getValue(), key.substring(colon + 1)), namespace); + } + } + Map owners = new IdentityHashMap<>(); + for (Command command : known.values()) { + if (command == null || owners.containsKey(command)) { + continue; + } + String name = command.getName().toLowerCase(Locale.ROOT); + String namespace = name.indexOf(':') > 0 ? name.substring(0, name.indexOf(':')) : namespaces.get(command); + if (namespace == null) { + namespace = namespaceByLabel.get(sameKind(command, bare(name))); + } + owners.put(command, owner(command, namespace, pluginNames)); + } + Map aliasOf = new HashMap<>(); for (Map.Entry entry : known.entrySet()) { Command command = entry.getValue(); if (command == null) { continue; } - labels.computeIfAbsent(command, ignored -> new ArrayList<>()); - String key = entry.getKey().toLowerCase(Locale.ROOT); - int colon = key.indexOf(':'); - if (colon < 0) { - labels.get(command).add(key); - } else { - namespaces.putIfAbsent(command, key.substring(0, colon)); + String owner = owners.get(command); + String name = bare(command.getName()); + for (String alias : command.getAliases()) { + if (!bare(alias).equals(name)) { + aliasOf.putIfAbsent(owner + ':' + bare(alias), name); + } + } + } + Map main = new TreeMap<>(); + Map> labels = new HashMap<>(); + for (Map.Entry entry : known.entrySet()) { + Command command = entry.getValue(); + if (command == null) { + continue; + } + String owner = owners.get(command); + String label = bare(command.getName()); + String name = redirectTarget(command); + if (name == null) { + name = aliasOf.getOrDefault(owner + ':' + label, label); + } + String id = owner + ':' + name; + boolean plainKey = entry.getKey().indexOf(':') < 0; + // Prefer the main label's plain wrapper, for its description and usage. + if (!main.containsKey(id) || label.equals(name) && plainKey) { + main.put(id, command); + } + TreeSet plain = labels.computeIfAbsent(id, ignored -> new TreeSet<>()); + if (plainKey) { + plain.add(entry.getKey().toLowerCase(Locale.ROOT)); } } List entries = new ArrayList<>(); - for (Map.Entry> entry : labels.entrySet()) { - Command command = entry.getKey(); - String name = command.getName().toLowerCase(Locale.ROOT); - TreeSet aliases = new TreeSet<>(entry.getValue()); + for (Map.Entry entry : main.entrySet()) { + Command command = entry.getValue(); + String owner = owners.get(command); + String name = entry.getKey().substring(owner.length() + 1); + TreeSet aliases = labels.get(entry.getKey()); boolean shadowed = !aliases.remove(name); entries.add(new Entry( - owner(command, namespaces.get(command), pluginNames), + owner, name, List.copyOf(aliases), - text(command.getDescription()), + text(command.getDescription(), TEXT_MAX), usage(command.getUsage(), name), - text(command.getPermission()), + text(command.getPermission(), Integer.MAX_VALUE), shadowed )); } entries.sort(Comparator.comparing(Entry::plugin, String.CASE_INSENSITIVE_ORDER) - .thenComparing(Entry::name) - .thenComparing(Entry::shadowed)); + .thenComparing(Entry::name)); return entries; } + /** The label without a namespace, lower case. */ + static String bare(String label) { + String lower = label.toLowerCase(Locale.ROOT); + return lower.substring(lower.indexOf(':') + 1); + } + + /** Wrappers of one command share class, description and permission; /version exists twice otherwise. */ + private static String sameKind(Command command, String label) { + return command.getClass().getName() + '\n' + label + '\n' + command.getDescription() + '\n' + command.getPermission(); + } + private static String owner(Command command, String namespace, Map pluginNames) { if (command instanceof PluginIdentifiableCommand owned) { return owned.getPlugin().getName(); } if (namespace == null) { - return "Other"; + return VANILLA_WRAPPER.equals(command.getClass().getSimpleName()) ? "Minecraft" : "Other"; } String plugin = pluginNames.get(namespace); if (plugin != null) { @@ -141,45 +210,77 @@ private static String owner(Command command, String namespace, Map type = command.getClass(); type != Command.class; type = type.getSuperclass()) { + for (Field field : type.getDeclaredFields()) { + if (!CommandNode.class.isAssignableFrom(field.getType())) { + continue; + } + try { + field.setAccessible(true); + CommandNode node = (CommandNode) field.get(command); + CommandNode target = node == null ? null : node.getRedirect(); + return target == null ? null : bare(target.getName()); + } catch (ReflectiveOperationException | RuntimeException e) { + return null; + } + } + } + return null; + } + /** Drops the usage Bukkit and Paper fill in when a plugin sets none. */ private static String usage(String usage, String name) { - String text = text(usage); - if (text == null || text.equals(DEFAULT_USAGE) || text.equalsIgnoreCase("/" + name)) { + String text = text(usage, TEXT_MAX); + if (text == null || text.equals(DEFAULT_USAGE) || bare(text.startsWith("/") ? text.substring(1) : text).equals(name)) { return null; } return text; } - private static String text(String text) { + private static String text(String text, int max) { if (text == null || text.isBlank()) { return null; } String trimmed = text.strip(); - return trimmed.length() > TEXT_MAX ? trimmed.substring(0, TEXT_MAX) : trimmed; + return trimmed.length() > max ? trimmed.substring(0, max) : trimmed; } - /** Bukkit's defaults for a non-op player: each default permission and, recursively, its children. */ + /** + * Bukkit's defaults for a non-op player, as LuckPerms resolves them: each default permission + * is granted, and the children it brings only fill permissions nothing else set. + */ static Map defaults(Set permissions, Function lookup) { Map values = new HashMap<>(); for (Permission permission : permissions) { - addDefault(permission.getName(), true, values, lookup); + addChildren(permission.getName(), true, values, lookup, new HashSet<>()); + } + for (Permission permission : permissions) { + values.put(permission.getName().toLowerCase(Locale.ROOT), Boolean.TRUE); } return values; } - private static void addDefault(String name, boolean value, Map values, Function lookup) { + private static void addChildren( + String name, + boolean value, + Map values, + Function lookup, + Set seen + ) { String key = name.toLowerCase(Locale.ROOT); - if (values.containsKey(key)) { + if (!seen.add(key)) { return; } - values.put(key, value); + values.putIfAbsent(key, value); Permission permission = lookup.apply(key); if (permission == null) { return; } for (Map.Entry child : permission.getChildren().entrySet()) { // As Bukkit does: a child set false under a denied parent ends up allowed. - addDefault(child.getKey(), child.getValue() == value, values, lookup); + addChildren(child.getKey(), child.getValue() == value, values, lookup, seen); } } @@ -204,7 +305,7 @@ static JsonObject payload(Server server, Map> json.add("aliases", aliases); json.addProperty("description", entry.description()); json.addProperty("usage", entry.usage()); - json.addProperty("permission", entry.permission()); + json.addProperty("permission", text(entry.permission(), PERMISSION_MAX)); json.addProperty("shadowed", entry.shadowed()); json.addProperty("access", access(entry.permission(), groups.values(), server.defaults())); commands.add(json); diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java index 60d2dbf..a519e35 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java @@ -5,9 +5,12 @@ import static org.mockito.Mockito.*; import com.google.gson.JsonObject; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.tree.CommandNode; import io.papermc.paper.plugin.configuration.PluginMeta; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -57,6 +60,23 @@ public Plugin getPlugin() { } } + /** Named like Paper's wrapper: one object per label, holding that label's Brigadier node. */ + static final class VanillaCommandWrapper extends Command { + final String note = "not a node"; + final CommandNode node; + + VanillaCommandWrapper(String label, CommandNode node) { + super(label); + this.node = node; + setPermission("minecraft.command." + CommandCatalogue.bare(label)); + } + + @Override + public boolean execute(CommandSender sender, String label, String[] args) { + return true; + } + } + static Plugin plugin(String name, String version) { Plugin plugin = mock(Plugin.class); PluginMeta meta = mock(PluginMeta.class); @@ -119,7 +139,7 @@ static Plugin plugin(String name, String version) { Map.of("libsdisguises.seecmd.disguise", true, "libsdisguises.seecmd.hidden", false)); Permission hidden = new Permission("libsdisguises.seecmd.hidden", PermissionDefault.OP, Map.of("deep.denied", false, "deep.allowed", true)); - Permission loop = new Permission("loop", PermissionDefault.TRUE, Map.of("libsdisguises.seecmd", true)); + Permission loop = new Permission("loop", PermissionDefault.TRUE, Map.of("libsdisguises.seecmd", true, "loop", false)); Map registered = new HashMap<>(); for (Permission permission : List.of(seecmd, hidden, loop)) { registered.put(permission.getName(), permission); @@ -205,4 +225,61 @@ static Plugin plugin(String name, String version) { assertEquals(Map.of("tfmc.open", true), server.defaults()); } } + + @Test void joinsPaperWrappersByRedirectAndListedAliases() { + CommandNode teleport = LiteralArgumentBuilder.literal("teleport").executes(c -> 1).build(); + CommandNode tp = LiteralArgumentBuilder.literal("tp").redirect(teleport).build(); + Plugin probe = plugin("Probe", "1"); + Owned brig = new Owned("probebrig", probe); + brig.setAliases(List.of("pb1", "probe:pb1")); + Owned brigNamespaced = new Owned("probe:probebrig", probe); + brigNamespaced.setAliases(List.of("pb1", "probe:pb1")); + Map known = new LinkedHashMap<>(); + known.put("probe:pb1", new Owned("probe:pb1", probe)); + known.put("pb1", new Owned("pb1", probe)); + known.put("probe:probebrig", brigNamespaced); + known.put("probebrig", brig); + known.put("tp", new VanillaCommandWrapper("tp", tp)); + known.put("minecraft:tp", new VanillaCommandWrapper("minecraft:tp", tp)); + known.put("teleport", new VanillaCommandWrapper("teleport", teleport)); + known.put("minecraft:teleport", new VanillaCommandWrapper("minecraft:teleport", teleport)); + known.put("minecraft:me", new VanillaCommandWrapper("minecraft:me", null)); + + assertEquals(List.of( + new Entry("Minecraft", "me", List.of(), null, null, "minecraft.command.me", true), + new Entry("Minecraft", "teleport", List.of("tp"), null, null, "minecraft.command.teleport", false), + new Entry("Probe", "probebrig", List.of("pb1"), null, null, null, false) + ), CommandCatalogue.entries(known, Map.of())); + } + + @Test void unreadableNodesHaveNoRedirect() { + @SuppressWarnings("unchecked") + CommandNode broken = mock(CommandNode.class); + when(broken.getRedirect()).thenThrow(new IllegalStateException("shadow")); + assertNull(CommandCatalogue.redirectTarget(new VanillaCommandWrapper("x", broken))); + assertNull(CommandCatalogue.redirectTarget(new Plain("y"))); + } + + @Test void aDefaultPermissionBeatsAChildSetFalseElsewhere() { + Permission parent = new Permission("example.parent", PermissionDefault.TRUE, Map.of("example.child", false)); + Permission child = new Permission("example.child", PermissionDefault.TRUE); + Map registered = Map.of(parent.getName(), parent, child.getName(), child); + Set roots = new LinkedHashSet<>(List.of(parent, child)); + + assertEquals(Map.of("example.parent", true, "example.child", true), + CommandCatalogue.defaults(roots, registered::get)); + } + + @Test void accessReadsTheWholePermissionButOnlyDisplaysPartOfIt() { + String permission = "example.denied;".repeat(150) + "example.allowed"; + Function member = node -> node.equals("example.allowed") ? Boolean.TRUE : null; + Server server = new Server(List.of(new Entry("Example", "long", List.of(), null, null, permission, false)), + Map.of(), Map.of()); + + JsonObject command = CommandCatalogue.payload(server, Map.of("member", member)) + .getAsJsonArray("commands").get(0).getAsJsonObject(); + + assertEquals("y", command.get("access").getAsString()); + assertEquals(CommandCatalogue.PERMISSION_MAX, command.get("permission").getAsString().length()); + } } From 6aca36ebc83bfb5e9174f88f2d9f2cf128512168 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:47:05 +0000 Subject: [PATCH 3/4] Join a listed alias only when it runs the same command An alias list can outlive the alias when another command later takes the label. A label now joins the command that lists it only when both are Brigadier labels with the same executor or the same subcommands. Co-Authored-By: Claude Opus 5.5 --- .../tfmcweb/luckperms/CommandCatalogue.java | 45 ++++++++++-- .../luckperms/CommandCatalogueTest.java | 70 ++++++++++++++++--- 2 files changed, 100 insertions(+), 15 deletions(-) diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java index 64d09de..f69b1b0 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java @@ -125,7 +125,7 @@ static List entries(Map live, Map plugin } owners.put(command, owner(command, namespace, pluginNames)); } - Map aliasOf = new HashMap<>(); + Map aliasOf = new HashMap<>(); for (Map.Entry entry : known.entrySet()) { Command command = entry.getValue(); if (command == null) { @@ -135,7 +135,7 @@ static List entries(Map live, Map plugin String name = bare(command.getName()); for (String alias : command.getAliases()) { if (!bare(alias).equals(name)) { - aliasOf.putIfAbsent(owner + ':' + bare(alias), name); + aliasOf.putIfAbsent(owner + ':' + bare(alias), command); } } } @@ -150,7 +150,9 @@ static List entries(Map live, Map plugin String label = bare(command.getName()); String name = redirectTarget(command); if (name == null) { - name = aliasOf.getOrDefault(owner + ':' + label, label); + Command listing = aliasOf.get(owner + ':' + label); + // An alias list can outlive the alias: only join a label that runs the same command. + name = listing != null && sameCommand(listing, command) ? bare(listing.getName()) : label; } String id = owner + ':' + name; boolean plainKey = entry.getKey().indexOf(':') < 0; @@ -212,6 +214,36 @@ private static String owner(Command command, String namespace, Map node.getRedirect() == null ? null : bare(node.getRedirect().getName()), null); + } + + /** Two Brigadier labels with the same executor or the same subcommands. */ + static boolean sameCommand(Command first, Command second) { + return withNode(first, a -> withNode(second, b -> sameNode(a, b), false), false); + } + + private static boolean sameNode(CommandNode a, CommandNode b) { + if (a.getCommand() != null) { + return a.getCommand() == b.getCommand(); + } + List> children = List.copyOf(a.getChildren()); + List> others = List.copyOf(b.getChildren()); + if (children.isEmpty() || children.size() != others.size()) { + return false; + } + for (int i = 0; i < children.size(); i++) { + if (children.get(i) != others.get(i)) { + return false; + } + } + return true; + } + + /** + * Applies {@code use} to the Brigadier node a Paper wrapper holds, read by reflection. + * Other commands, and nodes that refuse to be read, give {@code fallback}. + */ + private static T withNode(Command command, Function, T> use, T fallback) { for (Class type = command.getClass(); type != Command.class; type = type.getSuperclass()) { for (Field field : type.getDeclaredFields()) { if (!CommandNode.class.isAssignableFrom(field.getType())) { @@ -220,14 +252,13 @@ static String redirectTarget(Command command) { try { field.setAccessible(true); CommandNode node = (CommandNode) field.get(command); - CommandNode target = node == null ? null : node.getRedirect(); - return target == null ? null : bare(target.getName()); + return node == null ? fallback : use.apply(node); } catch (ReflectiveOperationException | RuntimeException e) { - return null; + return fallback; } } } - return null; + return fallback; } /** Drops the usage Bukkit and Paper fill in when a plugin sets none. */ diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java index a519e35..cc5f1d1 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java @@ -77,6 +77,33 @@ public boolean execute(CommandSender sender, String label, String[] args) { } } + /** Like Paper's plugin wrapper: a Brigadier node owned by a plugin. */ + static final class BrigWrapper extends Command implements PluginIdentifiableCommand { + final CommandNode node; + final Plugin owner; + + BrigWrapper(String label, CommandNode node, Plugin owner, String... aliases) { + super(label); + this.node = node; + this.owner = owner; + setAliases(List.of(aliases)); + } + + @Override + public boolean execute(CommandSender sender, String label, String[] args) { + return true; + } + + @Override + public Plugin getPlugin() { + return owner; + } + } + + static CommandNode literal(String label, com.mojang.brigadier.Command run) { + return LiteralArgumentBuilder.literal(label).executes(run).build(); + } + static Plugin plugin(String name, String version) { Plugin plugin = mock(Plugin.class); PluginMeta meta = mock(PluginMeta.class); @@ -230,15 +257,13 @@ static Plugin plugin(String name, String version) { CommandNode teleport = LiteralArgumentBuilder.literal("teleport").executes(c -> 1).build(); CommandNode tp = LiteralArgumentBuilder.literal("tp").redirect(teleport).build(); Plugin probe = plugin("Probe", "1"); - Owned brig = new Owned("probebrig", probe); - brig.setAliases(List.of("pb1", "probe:pb1")); - Owned brigNamespaced = new Owned("probe:probebrig", probe); - brigNamespaced.setAliases(List.of("pb1", "probe:pb1")); + com.mojang.brigadier.Command run = c -> 1; Map known = new LinkedHashMap<>(); - known.put("probe:pb1", new Owned("probe:pb1", probe)); - known.put("pb1", new Owned("pb1", probe)); - known.put("probe:probebrig", brigNamespaced); - known.put("probebrig", brig); + known.put("probe:pb1", new BrigWrapper("probe:pb1", literal("probe:pb1", run), probe)); + known.put("pb1", new BrigWrapper("pb1", literal("pb1", run), probe)); + known.put("probe:probebrig", new BrigWrapper("probe:probebrig", literal("probe:probebrig", run), probe, + "pb1", "probe:pb1")); + known.put("probebrig", new BrigWrapper("probebrig", literal("probebrig", run), probe, "pb1", "probe:pb1")); known.put("tp", new VanillaCommandWrapper("tp", tp)); known.put("minecraft:tp", new VanillaCommandWrapper("minecraft:tp", tp)); known.put("teleport", new VanillaCommandWrapper("teleport", teleport)); @@ -282,4 +307,33 @@ static Plugin plugin(String name, String version) { assertEquals("y", command.get("access").getAsString()); assertEquals(CommandCatalogue.PERMISSION_MAX, command.get("permission").getAsString().length()); } + + @Test void aStaleAliasDoesNotSwallowAnotherCommand() { + Plugin first = plugin("First", "1"); + Owned alpha = new Owned("alpha", first); + alpha.setAliases(List.of("beta")); + alpha.setPermission("first.alpha"); + Owned beta = new Owned("beta", first); + beta.setPermission("first.beta"); + CommandNode child = LiteralArgumentBuilder.literal("list").executes(c -> 1).build(); + CommandNode sub = LiteralArgumentBuilder.literal("sub").then(child).build(); + CommandNode s = LiteralArgumentBuilder.literal("s").then(child).build(); + Map known = new LinkedHashMap<>(); + known.put("alpha", alpha); + known.put("beta", beta); + known.put("gamma", new BrigWrapper("gamma", literal("gamma", c -> 1), first, "delta")); + known.put("delta", new BrigWrapper("delta", literal("delta", c -> 2), first)); + known.put("sub", new BrigWrapper("sub", sub, first, "s", "bare", "empty", "other", "twin")); + known.put("twin", new BrigWrapper("twin", LiteralArgumentBuilder.literal("twin").then(literal("list", c -> 1)) + .build(), first)); + known.put("s", new BrigWrapper("s", s, first)); + known.put("bare", new Owned("bare", first)); + known.put("empty", new BrigWrapper("empty", LiteralArgumentBuilder.literal("empty").build(), first)); + known.put("other", new BrigWrapper("other", LiteralArgumentBuilder.literal("other").then(literal("x", c -> 1)) + .then(literal("y", c -> 1)).build(), first)); + + assertEquals( + List.of("alpha[]", "bare[]", "beta[]", "delta[]", "empty[]", "gamma[]", "other[]", "sub[s]", "twin[]"), + CommandCatalogue.entries(known, Map.of()).stream().map(e -> e.name() + e.aliases()).toList()); + } } From 5638c93e3f5b07dfbe009708c2a5344f8e4b6294 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:52:28 +0000 Subject: [PATCH 4/4] Compare subcommands only when neither label has its own executor Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/tfmcweb/luckperms/CommandCatalogue.java | 2 +- .../tfmcweb/luckperms/CommandCatalogueTest.java | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java index f69b1b0..ce3a40a 100644 --- a/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java +++ b/src/main/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogue.java @@ -223,7 +223,7 @@ static boolean sameCommand(Command first, Command second) { } private static boolean sameNode(CommandNode a, CommandNode b) { - if (a.getCommand() != null) { + if (a.getCommand() != null || b.getCommand() != null) { return a.getCommand() == b.getCommand(); } List> children = List.copyOf(a.getChildren()); diff --git a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java index cc5f1d1..8797daa 100644 --- a/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java +++ b/src/test/java/net/tfminecraft/tfmcweb/luckperms/CommandCatalogueTest.java @@ -323,7 +323,9 @@ static Plugin plugin(String name, String version) { known.put("beta", beta); known.put("gamma", new BrigWrapper("gamma", literal("gamma", c -> 1), first, "delta")); known.put("delta", new BrigWrapper("delta", literal("delta", c -> 2), first)); - known.put("sub", new BrigWrapper("sub", sub, first, "s", "bare", "empty", "other", "twin")); + known.put("sub", new BrigWrapper("sub", sub, first, "s", "bare", "empty", "other", "twin", "runs")); + known.put("runs", new BrigWrapper("runs", LiteralArgumentBuilder.literal("runs").executes(c -> 3).then(child) + .build(), first, "sub")); known.put("twin", new BrigWrapper("twin", LiteralArgumentBuilder.literal("twin").then(literal("list", c -> 1)) .build(), first)); known.put("s", new BrigWrapper("s", s, first)); @@ -333,7 +335,8 @@ static Plugin plugin(String name, String version) { .then(literal("y", c -> 1)).build(), first)); assertEquals( - List.of("alpha[]", "bare[]", "beta[]", "delta[]", "empty[]", "gamma[]", "other[]", "sub[s]", "twin[]"), + List.of("alpha[]", "bare[]", "beta[]", "delta[]", "empty[]", "gamma[]", "other[]", "runs[]", "sub[s]", + "twin[]"), CommandCatalogue.entries(known, Map.of()).stream().map(e -> e.name() + e.aliases()).toList()); } }