Skip to content

Add support for unsafe MiniMessage tags with permission checks #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vLuckyyy wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for unsafe MiniMessage tags with permission checks #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
| `chatformatter.translatable` | `<lang>` |
| `chatformatter.keybind` | `<key>` |
| `chatformatter.nbt` | `<nbt>` |
| `chatformatter.unsafe` | **Required** to use unsafe tags (`<click>`, `<hover>`, etc.) in addition to the specific tag permission. |
| `chatformatter.reload` | `/chatformatter reload` |
| `chatformatter.receiveupdates` | receive update announcements for this plugin |

Expand Down
24 changes: 21 additions & 3 deletions chatformatter-core/src/main/java/com/eternalcode/formatter/ChatHandlerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
class ChatHandlerImpl implements ChatHandler {

private static final String PERMISSION_ALL = "chatformatter.*";
private static final String PERMISSION_UNSAFE = "chatformatter.unsafe";
private static final Map<String, TagResolver> TAG_RESOLVERS_BY_PERMISSION = new ImmutableMap.Builder<String, TagResolver>()
.put("chatformatter.decorations.*", StandardTags.decorations())
.put("chatformatter.decorations.bold", StandardTags.decorations(TextDecoration.BOLD))
Expand Down Expand Up @@ -142,19 +143,36 @@ private TagResolver.Single messagePlaceholder(Player sender, String rawMessage)
}

private TagResolver providePermittedTags(Player player) {
List<TagResolver> tagResolvers = new ArrayList<>();
boolean isUnsafeAllowed = player.hasPermission(PERMISSION_UNSAFE);

if (player.hasPermission(PERMISSION_ALL)) {
if (isUnsafeAllowed && player.hasPermission(PERMISSION_ALL)) {
return TagResolver.standard();
}

List<TagResolver> tagResolvers = new ArrayList<>();

for (Map.Entry<String, TagResolver> entry : TAG_RESOLVERS_BY_PERMISSION.entrySet()) {
if (player.hasPermission(entry.getKey())) {
String permission = entry.getKey();

if (this.isUnsafePermission(permission) && !isUnsafeAllowed) {
continue;
}

if (player.hasPermission(permission) || player.hasPermission(PERMISSION_ALL)) {
tagResolvers.add(entry.getValue());
}
}

return TagResolver.resolver(tagResolvers);
}

private boolean isUnsafePermission(String permission) {
return permission.contains("click")
|| permission.contains("hover")
|| permission.contains("insertion")
|| permission.contains("score")
|| permission.contains("selector")
|| permission.contains("nbt");
}
Comment on lines +169 to +176
Copy link

@gemini-code-assist gemini-code-assist bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of String.contains() to check for unsafe permissions is a bit fragile. It could lead to false positives if a new, safe permission happens to contain one of these substrings (e.g., chatformatter.no-click-events). Using a switch statement for an exact match on the permission string would be more robust and efficient.

    private boolean isUnsafePermission(String permission) {
        switch (permission) {
            case "chatformatter.hover":
            case "chatformatter.click":
            case "chatformatter.insertion":
            case "chatformatter.score":
            case "chatformatter.selector":
            case "chatformatter.nbt":
                return true;
            default:
                return false;
        }
    }


}
108 changes: 108 additions & 0 deletions chatformatter-core/src/test/java/com/eternalcode/formatter/ChatHandlerSafeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package com.eternalcode.formatter;

import com.eternalcode.formatter.config.PluginConfig;
import com.eternalcode.formatter.placeholder.PlaceholderRegistry;
import com.eternalcode.formatter.rank.ChatRankProvider;
import com.eternalcode.formatter.template.TemplateService;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.minimessage.MiniMessage;
import net.kyori.adventure.text.serializer.gson.GsonComponentSerializer;
import org.bukkit.entity.Player;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class ChatHandlerSafeTest {

private PluginConfig config;
private ChatHandlerImpl chatHandler;
private Player player;

@BeforeEach
void setup() {
config = new PluginConfig();
ChatRankProvider rankProvider = mock(ChatRankProvider.class);
PlaceholderRegistry placeholderRegistry = mock(PlaceholderRegistry.class);
TemplateService templateService = mock(TemplateService.class);

when(rankProvider.getRank(any())).thenReturn("default");
when(placeholderRegistry.format(anyString(), any())).thenAnswer(inv -> {
String s = inv.getArgument(0);
return s.replace("{message}", "<message>")
.replace("{displayname}", "Player")
.replace("{member}", "")
.replace("$hoverName(Player)", "");
});
when(templateService.applyTemplates(anyString())).thenAnswer(inv -> inv.getArgument(0));

MiniMessage miniMessage = MiniMessage.miniMessage();

chatHandler = new ChatHandlerImpl(miniMessage, config, rankProvider, placeholderRegistry, templateService);
player = mock(Player.class);
}

@Test
void testUnsafeTagsWithoutPermission() {
when(player.hasPermission("chatformatter.click")).thenReturn(true);
when(player.hasPermission("chatformatter.unsafe")).thenReturn(false);
when(player.getDisplayName()).thenReturn("Player");
when(player.getName()).thenReturn("Player");

String rawMessage = "<click:run_command:'/op me'>Click me</click>";
String jsonMessage = GsonComponentSerializer.gson().serialize(Component.text(rawMessage));

ChatMessage chatMessage = new ChatMessage(player, Optional.empty(), jsonMessage);

ChatRenderedMessage result = chatHandler.process(chatMessage);
String json = result.jsonMessage();

assertFalse(json.contains("\"clickEvent\""),
"Should not contain click event without unsafe permission. JSON: " + json);
}

@Test
void testUnsafeTagsWithPermission() {
when(player.hasPermission("chatformatter.click")).thenReturn(true);
when(player.hasPermission("chatformatter.unsafe")).thenReturn(true);
when(player.getDisplayName()).thenReturn("Player");
when(player.getName()).thenReturn("Player");

String rawMessage = "<click:run_command:'/op me'>Click me</click>";
String jsonMessage = GsonComponentSerializer.gson().serialize(Component.text(rawMessage));

ChatMessage chatMessage = new ChatMessage(player, Optional.empty(), jsonMessage);

ChatRenderedMessage result = chatHandler.process(chatMessage);
String json = result.jsonMessage();

assertTrue(json.contains("\"clickEvent\""), "Should contain click event with unsafe permission. JSON: " + json);
}

@Test
void testWildcardPermission() {
when(player.hasPermission("chatformatter.*")).thenReturn(true);
when(player.hasPermission("chatformatter.unsafe")).thenReturn(true);
when(player.getDisplayName()).thenReturn("Player");
when(player.getName()).thenReturn("Player");

String rawMessage = "<click:run_command:'/op me'>Click me</click>";
String jsonMessage = GsonComponentSerializer.gson().serialize(Component.text(rawMessage));

ChatMessage chatMessage = new ChatMessage(player, Optional.empty(), jsonMessage);

ChatRenderedMessage result = chatHandler.process(chatMessage);
String json = result.jsonMessage();

assertTrue(json.contains("\"clickEvent\""),
"Should contain click event with wildcard permission. JSON: " + json);
}
}
7 changes: 6 additions & 1 deletion chatformatter-paper-plugin/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ bukkit {
"chatformatter.selector",
"chatformatter.keybind",
"chatformatter.newline",
"chatformatter.rainbow"
"chatformatter.rainbow",
"chatformatter.unsafe"
)
default = Default.OP
}
Expand Down Expand Up @@ -69,6 +70,10 @@ bukkit {
register("chatformatter.keybind") { default = Default.OP }
register("chatformatter.newline") { default = Default.OP }
register("chatformatter.rainbow") { default = Default.OP }
register("chatformatter.unsafe") {
description = "Allows using unsafe MiniMessage tags like <click> and <hover>"
default = Default.OP
}

register("chatformatter.color.*") {
children = listOf(
Expand Down
Loading