From c4e5c1ad09820d1d75faf88cd2bad7c45328d900 Mon Sep 17 00:00:00 2001 From: Ryan Wang Date: Fri, 16 Oct 2026 00:26:15 +0800 Subject: [PATCH] Fix theme name validation in MCP tools --- .../mcpserver/tools/ThemeSettingTools.java | 4 +- .../tools/ThemeSettingToolsTest.java | 66 +++++++++++++++---- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/main/java/run/halo/mcpserver/tools/ThemeSettingTools.java b/src/main/java/run/halo/mcpserver/tools/ThemeSettingTools.java index 3d06532..e1309f8 100644 --- a/src/main/java/run/halo/mcpserver/tools/ThemeSettingTools.java +++ b/src/main/java/run/halo/mcpserver/tools/ThemeSettingTools.java @@ -81,7 +81,7 @@ Mono getGroup(Map arguments) { } Mono updateGroup(Map arguments) { - var themeName = resourceName(arguments, "themeName"); + var themeName = requiredString(arguments, "themeName"); var group = requiredString(arguments, "group"); var patch = requiredPatch(arguments); var expectedVersion = requiredLong(arguments, "expectedVersion"); @@ -119,7 +119,7 @@ Mono listTemplates(Map arguments) { } Mono getTemplate(Map arguments) { - var themeName = resourceName(arguments, "themeName"); + var themeName = requiredString(arguments, "themeName"); var path = relativeHtmlPath(requiredString(arguments, "path")); return activeTheme().flatMap(theme -> { requireActiveTheme(theme, themeName); diff --git a/src/test/java/run/halo/mcpserver/tools/ThemeSettingToolsTest.java b/src/test/java/run/halo/mcpserver/tools/ThemeSettingToolsTest.java index 580616d..cbf981c 100644 --- a/src/test/java/run/halo/mcpserver/tools/ThemeSettingToolsTest.java +++ b/src/test/java/run/halo/mcpserver/tools/ThemeSettingToolsTest.java @@ -19,6 +19,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -189,18 +191,21 @@ void mapsLazyTemplateWalkFailuresToAToolError() throws IOException { } } - @Test - void readsAnHtmlTemplateRelativeToTheTemplatesDirectory() throws IOException { + @ParameterizedTest + @ValueSource(strings = {"theme-hao", "theme-echoCraft"}) + void readsAnHtmlTemplateRelativeToTheTemplatesDirectory(String themeName) throws IOException { + reset(client); + stubActiveTheme(themeName); var templates = Files.createDirectories(themeRoot().resolve("templates/modules")); var html = "
"; Files.writeString(templates.resolve("layout.html"), html); var result = data(tools.getTemplate(Map.of( - "themeName", "theme-hao", "path", "modules/layout.html")) + "themeName", themeName, "path", "modules/layout.html")) .block()); assertThat(result) - .containsEntry("themeName", "theme-hao") + .containsEntry("themeName", themeName) .containsEntry("path", "modules/layout.html") .containsEntry("html", html); } @@ -242,8 +247,11 @@ void rejectsTemplateSymlinkEscapesAndOversizedFiles() throws IOException { .verify(); } - @Test - void mergePatchesOnlyTheRequestedGroup() { + @ParameterizedTest + @ValueSource(strings = {"theme-hao", "theme-echoCraft"}) + void mergePatchesOnlyTheRequestedGroup(String themeName) { + reset(client); + stubActiveTheme(themeName); when(client.fetch(Setting.class, "theme-hao-setting")) .thenReturn(Mono.just(setting(form("basics", "Basic", Map.of())))); var configMap = configMap( @@ -265,7 +273,7 @@ void mergePatchesOnlyTheRequestedGroup() { var patch = Map.of("nested", nestedPatch, "items", List.of(4)); var result = data(tools.updateGroup(Map.of( - "themeName", "theme-hao", + "themeName", themeName, "group", "basics", "patch", patch, "expectedVersion", 0L)) @@ -281,7 +289,9 @@ void mergePatchesOnlyTheRequestedGroup() { assertThat(savedGroup.get("nested").get("add").asInt()).isEqualTo(3); assertThat(savedGroup.get("items").size()).isEqualTo(1); assertThat(savedGroup.get("items").get(0).asInt()).isEqualTo(4); - assertThat(result).containsEntry("configVersion", 1L); + assertThat(result) + .containsEntry("themeName", themeName) + .containsEntry("configVersion", 1L); assertThat(result.get("values")).isEqualTo(Map.of( "nested", Map.of("keep", 1, "add", 3), "items", List.of(4))); } @@ -338,21 +348,55 @@ void rejectsCorruptStoredValuesAndSkipsEmptyPatches() { verify(client, never()).update(any(ConfigMap.class)); } + @ParameterizedTest + @ValueSource(strings = {"", " "}) + void rejectsBlankThemeNames(String themeName) { + reset(client); + + assertThatThrownBy(() -> tools.updateGroup(Map.of("themeName", themeName))) + .satisfies(error -> assertToolError(error, "INVALID_ARGUMENT", "non-empty string")); + assertThatThrownBy(() -> tools.getTemplate(Map.of("themeName", themeName))) + .satisfies(error -> assertToolError(error, "INVALID_ARGUMENT", "non-empty string")); + } + + @Test + void rejectsThemeNamesWithDifferentCase() { + reset(client); + stubActiveTheme("theme-echoCraft"); + + StepVerifier.create(tools.updateGroup(Map.of( + "themeName", "theme-echocraft", + "group", "basics", + "patch", Map.of(), + "expectedVersion", 0L))) + .expectErrorSatisfies(error -> assertToolError(error, "CONFLICT", "active theme changed")) + .verify(); + StepVerifier.create(tools.getTemplate(Map.of( + "themeName", "theme-echocraft", "path", "index.html"))) + .expectErrorSatisfies(error -> assertToolError(error, "CONFLICT", "active theme changed")) + .verify(); + verify(client, never()).update(any(ConfigMap.class)); + } + private void stubActiveTheme() { + stubActiveTheme("theme-hao"); + } + + private void stubActiveTheme(String themeName) { when(client.fetch(ConfigMap.class, SystemSetting.SYSTEM_CONFIG)) .thenReturn(Mono.just(configMap( SystemSetting.SYSTEM_CONFIG, 1L, - Map.of(SystemSetting.Theme.GROUP, "{\"active\":\"theme-hao\"}")))); + Map.of(SystemSetting.Theme.GROUP, "{\"active\":\"" + themeName + "\"}")))); activeTheme = new Theme(); - activeTheme.setMetadata(metadata("theme-hao", 3L)); + activeTheme.setMetadata(metadata(themeName, 3L)); var spec = new Theme.ThemeSpec(); spec.setDisplayName("Hao"); spec.setVersion("1.2.3"); spec.setSettingName("theme-hao-setting"); spec.setConfigMapName("theme-hao-config"); activeTheme.setSpec(spec); - when(client.fetch(Theme.class, "theme-hao")).thenReturn(Mono.just(activeTheme)); + when(client.fetch(Theme.class, themeName)).thenReturn(Mono.just(activeTheme)); } private Path themeRoot() throws IOException {