Skip to content

Commit

Permalink
Several command permission fixes
Browse files Browse the repository at this point in the history
 - Attach permission checks to the first argument (so the literal
   command name) rather than the last argument. This fixes commands
   showing up when they shouldn't.

 - HelpingArgumentBuilder now inherits permissions of its leaf nodes.
   This only really impacts the "track" subcommand.

 - Don't autocomplete the computer selector for the "queue" subcommand.
   As everyone has permission for this command, it's possible to find
   all computer ids and labels in the world.

   I'm in mixed minds about this, but don't think this is an exploit -
   computer ids/labels are sent to in-range players so shouldn't be
   considered secret - but worth patching none-the-less.
  • Loading branch information
SquidDev committed Jul 6, 2023
1 parent 4bbde8c commit 5d71770
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@

import com.mojang.brigadier.CommandDispatcher;
import com.mojang.brigadier.arguments.StringArgumentType;
import com.mojang.brigadier.builder.RequiredArgumentBuilder;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
import com.mojang.brigadier.suggestion.Suggestions;
import dan200.computercraft.core.computer.ComputerSide;
import dan200.computercraft.core.metrics.Metrics;
import dan200.computercraft.shared.command.arguments.ComputersArgumentType;
import dan200.computercraft.shared.command.text.TableBuilder;
import dan200.computercraft.shared.computer.core.ComputerFamily;
import dan200.computercraft.shared.computer.core.ServerComputer;
Expand Down Expand Up @@ -169,7 +172,10 @@ public static void register(CommandDispatcher<CommandSourceStack> dispatcher) {

.then(command("queue")
.requires(UserLevel.ANYONE)
.arg("computer", manyComputers())
.arg(
RequiredArgumentBuilder.<CommandSourceStack, ComputersArgumentType.ComputersSupplier>argument("computer", manyComputers())
.suggests((context, builder) -> Suggestions.empty())
)
.argManyValue("args", StringArgumentType.string(), Collections.emptyList())
.executes((ctx, args) -> {
var computers = getComputersArgument(ctx, "computer");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,29 @@ public boolean test(CommandSourceStack source) {
return source.hasPermission(toLevel());
}

/**
* Take the union of two {@link UserLevel}s.
* <p>
* This satisfies the property that for all sources {@code s}, {@code a.test(s) || b.test(s) == (a ∪ b).test(s)}.
*
* @param left The first user level to take the union of.
* @param right The second user level to take the union of.
* @return The union of two levels.
*/
public static UserLevel union(UserLevel left, UserLevel right) {
if (left == right) return left;

// x ∪ ANYONE = ANYONE
if (left == ANYONE || right == ANYONE) return ANYONE;

// x ∪ OWNER = OWNER
if (left == OWNER) return right;
if (right == OWNER) return left;

// At this point, we have x != y and x, y ∈ { OP, OWNER_OP }.
return OWNER_OP;
}

private static boolean isOwner(CommandSourceStack source) {
var server = source.getServer();
var sender = source.getEntity();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,15 @@ public CommandBuilder<S> requires(Predicate<S> predicate) {
return this;
}

public CommandBuilder<S> arg(String name, ArgumentType<?> type) {
args.add(RequiredArgumentBuilder.argument(name, type));
public CommandBuilder<S> arg(ArgumentBuilder<S, ?> arg) {
args.add(arg);
return this;
}

public CommandBuilder<S> arg(String name, ArgumentType<?> type) {
return arg(RequiredArgumentBuilder.argument(name, type));
}

public <T> CommandNodeBuilder<S, ArgCommand<S, List<T>>> argManyValue(String name, ArgumentType<T> type, List<T> empty) {
return argMany(name, type, () -> empty);
}
Expand All @@ -74,7 +78,7 @@ private <T> CommandNodeBuilder<S, ArgCommand<S, List<T>>> argMany(String name, R

return command -> {
// The node for no arguments
var tail = tail(ctx -> command.run(ctx, empty.get()));
var tail = setupTail(ctx -> command.run(ctx, empty.get()));

// The node for one or more arguments
ArgumentBuilder<S, ?> moreArg = RequiredArgumentBuilder
Expand All @@ -83,7 +87,7 @@ private <T> CommandNodeBuilder<S, ArgCommand<S, List<T>>> argMany(String name, R

// Chain all of them together!
tail.then(moreArg);
return link(tail);
return buildTail(tail);
};
}

Expand All @@ -94,20 +98,16 @@ private static <T> List<T> getList(CommandContext<?> context, String name) {

@Override
public CommandNode<S> executes(Command<S> command) {
if (args.isEmpty()) throw new IllegalStateException("Cannot have empty arg chain builder");

return link(tail(command));
return buildTail(setupTail(command));
}

private ArgumentBuilder<S, ?> tail(Command<S> command) {
var defaultTail = args.get(args.size() - 1);
defaultTail.executes(command);
if (requires != null) defaultTail.requires(requires);
return defaultTail;
private ArgumentBuilder<S, ?> setupTail(Command<S> command) {
return args.get(args.size() - 1).executes(command);
}

private CommandNode<S> link(ArgumentBuilder<S, ?> tail) {
private CommandNode<S> buildTail(ArgumentBuilder<S, ?> tail) {
for (var i = args.size() - 2; i >= 0; i--) tail = args.get(i).then(tail);
if (requires != null) tail.requires(requires);
return tail.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.mojang.brigadier.context.CommandContext;
import com.mojang.brigadier.tree.CommandNode;
import com.mojang.brigadier.tree.LiteralCommandNode;
import dan200.computercraft.shared.command.UserLevel;
import net.minecraft.ChatFormatting;
import net.minecraft.commands.CommandSourceStack;
import net.minecraft.network.chat.ClickEvent;
Expand All @@ -18,6 +19,8 @@
import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.function.Predicate;
import java.util.stream.Stream;

import static dan200.computercraft.core.util.Nullability.assertNonNull;
import static dan200.computercraft.shared.command.text.ChatHelpers.coloured;
Expand All @@ -37,6 +40,29 @@ public static HelpingArgumentBuilder choice(String literal) {
return new HelpingArgumentBuilder(literal);
}

@Override
public LiteralArgumentBuilder<CommandSourceStack> requires(Predicate<CommandSourceStack> requirement) {
throw new IllegalStateException("Cannot use requires on a HelpingArgumentBuilder");
}

@Override
public Predicate<CommandSourceStack> getRequirement() {
// The requirement of this node is the union of all child's requirements.
var requirements = Stream.concat(
children.stream().map(ArgumentBuilder::getRequirement),
getArguments().stream().map(CommandNode::getRequirement)
).toList();

// If all requirements are a UserLevel, take the union of those instead.
var userLevel = UserLevel.OWNER;
for (var requirement : requirements) {
if (!(requirement instanceof UserLevel level)) return x -> requirements.stream().anyMatch(y -> y.test(x));
userLevel = UserLevel.union(userLevel, level);
}

return userLevel;
}

@Override
public LiteralArgumentBuilder<CommandSourceStack> executes(final Command<CommandSourceStack> command) {
throw new IllegalStateException("Cannot use executes on a HelpingArgumentBuilder");
Expand Down Expand Up @@ -80,9 +106,7 @@ private LiteralCommandNode<CommandSourceStack> buildImpl(String id, String comma
helpCommand.node = node;

// Set up a /... help command
var helpNode = LiteralArgumentBuilder.<CommandSourceStack>literal("help")
.requires(x -> getArguments().stream().anyMatch(y -> y.getRequirement().test(x)))
.executes(helpCommand);
var helpNode = LiteralArgumentBuilder.<CommandSourceStack>literal("help").executes(helpCommand);

// Add all normal command children to this and the help node
for (var child : getArguments()) {
Expand Down

0 comments on commit 5d71770

Please sign in to comment.