From b23cc32f33f31f6733251a7ace287149d69e052a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Wed, 15 Jul 2026 14:57:18 +0800 Subject: [PATCH] fix(wiki): make global starter-pack templates read-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Global transformation templates (workspace_id IS NULL, e.g. the 7 built-in starter packs made global by V165) were shared across every workspace but not actually read-only: any workspace member could edit or delete them, mutating/affecting all workspaces, with deletes unrecoverable (Flyway seed runs once). - Controller: reject update/delete of null-workspace templates with 403 (err.wiki.global_template_readonly); read/apply paths unchanged. - Service: defense-in-depth — update/delete also reject global templates, guarding non-HTTP callers (WikiTool LLM entry points). delete() now checks the entity before deleting instead of deleting blindly. - findByName: add deterministic ORDER BY (workspace_id IS NULL) ASC so a workspace-local template wins over a same-named global one (was LIMIT 1 with no ordering). Consistent across H2/MySQL/Kingbase. - i18n: new err.wiki.global_template_readonly (zh + en). - Tests: +2 controller mock tests (403 on update/delete, no service write), +2 E2E tests (global template stays intact; findByName prefers local). Tests: 10/10 green (4 controller + 6 E2E). --- .../WikiTransformationController.java | 17 ++++++ .../service/WikiTransformationService.java | 27 ++++++++- .../src/main/resources/messages.properties | 3 + .../src/main/resources/messages_en.properties | 3 + .../WikiTransformationControllerTest.java | 37 ++++++++++++ ...ransformationStarterPackGlobalE2ETest.java | 60 +++++++++++++++++++ 6 files changed, 146 insertions(+), 1 deletion(-) diff --git a/mateclaw-server/src/main/java/vip/mate/wiki/controller/WikiTransformationController.java b/mateclaw-server/src/main/java/vip/mate/wiki/controller/WikiTransformationController.java index e4c9c7fd..f9e0f63d 100644 --- a/mateclaw-server/src/main/java/vip/mate/wiki/controller/WikiTransformationController.java +++ b/mateclaw-server/src/main/java/vip/mate/wiki/controller/WikiTransformationController.java @@ -86,6 +86,7 @@ public class WikiTransformationController { WikiTransformationEntity existing = transformationService.getById(id); if (existing == null) return R.fail(404, "Transformation not found"); verifyTemplateWorkspace(existing, workspaceId); + rejectGlobalTemplateMutation(existing); return R.ok(transformationService.update(id, body)); } @@ -96,6 +97,7 @@ public class WikiTransformationController { WikiTransformationEntity existing = transformationService.getById(id); if (existing != null) { verifyTemplateWorkspace(existing, workspaceId); + rejectGlobalTemplateMutation(existing); transformationService.delete(id); } return R.ok(); @@ -273,4 +275,19 @@ public class WikiTransformationController { throw new MateClawException("err.common.wrong_workspace", 403, "Resource does not belong to current workspace"); } } + + /** + * Global templates ({@code workspace_id IS NULL}, e.g. the built-in starter + * pack) are shared across every workspace, so they must stay read-only on + * the write/delete paths — a mutation by one workspace would affect all of + * them, and a delete is unrecoverable (the Flyway seed runs once). + * {@link #verifyTemplateWorkspace} intentionally allows null-workspace on + * read/apply paths; this guard only covers mutation endpoints. + */ + private void rejectGlobalTemplateMutation(WikiTransformationEntity t) { + if (t.getWorkspaceId() == null) { + throw new MateClawException("err.wiki.global_template_readonly", 403, + "Built-in global templates are read-only"); + } + } } diff --git a/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiTransformationService.java b/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiTransformationService.java index af71db3b..baf1130d 100644 --- a/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiTransformationService.java +++ b/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiTransformationService.java @@ -5,6 +5,7 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import vip.mate.exception.MateClawException; import vip.mate.wiki.model.WikiTransformationEntity; import vip.mate.wiki.model.WikiTransformationRunEntity; import vip.mate.wiki.repository.WikiTransformationMapper; @@ -77,7 +78,10 @@ public class WikiTransformationService { .and(ws -> ws.eq(WikiTransformationEntity::getWorkspaceId, workspaceId) .or().isNull(WikiTransformationEntity::getWorkspaceId)) .eq(WikiTransformationEntity::getName, name) - .last("LIMIT 1")); + // Deterministic: prefer the workspace-local row over a same-named + // global one, then newer-first. (workspace_id IS NULL) ASC puts the + // non-null workspace-scoped row first — consistent across H2/MySQL/Kingbase. + .last("ORDER BY (workspace_id IS NULL) ASC, update_time DESC LIMIT 1")); return Optional.ofNullable(global); } @@ -135,6 +139,10 @@ public class WikiTransformationService { if (entity == null) { throw new IllegalArgumentException("Transformation not found: " + id); } + // Defense in depth: global templates are system-owned / read-only. + // The controller already rejects this, but this also guards callers that + // bypass the controller (e.g. the WikiTool LLM entry points). + rejectGlobalTemplateMutation(entity); if (patch.getTitle() != null) entity.setTitle(patch.getTitle()); if (patch.getDescription() != null) entity.setDescription(patch.getDescription()); if (patch.getPromptTemplate() != null) entity.setPromptTemplate(patch.getPromptTemplate()); @@ -219,6 +227,11 @@ public class WikiTransformationService { @Transactional public void delete(Long id) { + WikiTransformationEntity entity = transformationMapper.selectById(id); + if (entity == null) { + return; + } + rejectGlobalTemplateMutation(entity); transformationMapper.deleteById(id); } @@ -288,4 +301,16 @@ public class WikiTransformationService { "name must be 3-64 chars, lowercase letters / digits / hyphens (start and end alphanumeric)"); } } + + /** + * Global templates ({@code workspace_id IS NULL}) are shared across all + * workspaces and seeded once by Flyway, so they are read-only: a mutation + * by one workspace hits everyone, and a delete is unrecoverable. + */ + private static void rejectGlobalTemplateMutation(WikiTransformationEntity entity) { + if (entity.getWorkspaceId() == null) { + throw new MateClawException("err.wiki.global_template_readonly", 403, + "Built-in global templates are read-only"); + } + } } diff --git a/mateclaw-server/src/main/resources/messages.properties b/mateclaw-server/src/main/resources/messages.properties index f5853324..b9b95a06 100644 --- a/mateclaw-server/src/main/resources/messages.properties +++ b/mateclaw-server/src/main/resources/messages.properties @@ -323,6 +323,9 @@ err.wiki.vision.no_provider=\u672a\u914d\u7f6e\u53ef\u7528\u7684\u56fe\u7247\u8b err.wiki.vision.provider_failed=\u56fe\u7247\u8bc6\u522b provider \u8c03\u7528\u5931\u8d25 err.wiki.vision.all_failed=\u6240\u6709\u56fe\u7247\u8bc6\u522b provider \u8c03\u7528\u5931\u8d25 +# --- Wiki transformation: global starter pack is read-only --- +err.wiki.global_template_readonly=\u5185\u7f6e\u5168\u5c40\u6a21\u677f\u53ea\u8bfb\uff0c\u4e0d\u53ef\u7f16\u8f91\u6216\u5220\u9664 + # --- Chat: assistant stop / interrupt placeholders --- chat.stopMarker.userAborted=[\u5df2\u88ab\u7528\u6237\u4e2d\u6b62] diff --git a/mateclaw-server/src/main/resources/messages_en.properties b/mateclaw-server/src/main/resources/messages_en.properties index 35769b96..70cea359 100644 --- a/mateclaw-server/src/main/resources/messages_en.properties +++ b/mateclaw-server/src/main/resources/messages_en.properties @@ -330,6 +330,9 @@ err.wiki.vision.no_provider=No image vision provider is configured err.wiki.vision.provider_failed=Image vision provider call failed err.wiki.vision.all_failed=All image vision providers failed +# --- Wiki transformation: global starter pack is read-only --- +err.wiki.global_template_readonly=Built-in global templates are read-only + # --- Chat: assistant stop / interrupt placeholders --- chat.stopMarker.userAborted=[Stopped by user] diff --git a/mateclaw-server/src/test/java/vip/mate/wiki/controller/WikiTransformationControllerTest.java b/mateclaw-server/src/test/java/vip/mate/wiki/controller/WikiTransformationControllerTest.java index fe9c7f87..d9f1e6a0 100644 --- a/mateclaw-server/src/test/java/vip/mate/wiki/controller/WikiTransformationControllerTest.java +++ b/mateclaw-server/src/test/java/vip/mate/wiki/controller/WikiTransformationControllerTest.java @@ -3,6 +3,7 @@ package vip.mate.wiki.controller; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import vip.mate.common.result.R; +import vip.mate.exception.MateClawException; import vip.mate.wiki.model.WikiTransformationEntity; import vip.mate.wiki.model.WikiTransformationRunEntity; import vip.mate.wiki.service.WikiKnowledgeBaseService; @@ -13,7 +14,12 @@ import vip.mate.wiki.service.WikiTransformationService; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class WikiTransformationControllerTest { @@ -53,4 +59,35 @@ class WikiTransformationControllerTest { assertEquals(400, response.getCode()); } + + @Test + void updateGlobalTemplateThrows403() { + WikiTransformationEntity global = new WikiTransformationEntity(); + global.setId(1000004001L); + global.setWorkspaceId(null); // global starter pack + when(transformationService.getById(1000004001L)).thenReturn(global); + + MateClawException ex = assertThrows(MateClawException.class, () -> + controller.update(1000004001L, new WikiTransformationEntity(), 999L)); + + assertEquals(403, ex.getCode()); + assertEquals("err.wiki.global_template_readonly", ex.getMsgKey()); + // The mutating service call must never be reached — no partial write. + verify(transformationService, never()).update(anyLong(), any()); + } + + @Test + void deleteGlobalTemplateThrows403() { + WikiTransformationEntity global = new WikiTransformationEntity(); + global.setId(1000004001L); + global.setWorkspaceId(null); + when(transformationService.getById(1000004001L)).thenReturn(global); + + MateClawException ex = assertThrows(MateClawException.class, () -> + controller.delete(1000004001L, 999L)); + + assertEquals(403, ex.getCode()); + assertEquals("err.wiki.global_template_readonly", ex.getMsgKey()); + verify(transformationService, never()).delete(anyLong()); + } } diff --git a/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiTransformationStarterPackGlobalE2ETest.java b/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiTransformationStarterPackGlobalE2ETest.java index 8facec06..7273c1a8 100644 --- a/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiTransformationStarterPackGlobalE2ETest.java +++ b/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiTransformationStarterPackGlobalE2ETest.java @@ -4,6 +4,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import vip.mate.exception.MateClawException; import vip.mate.wiki.model.WikiKnowledgeBaseEntity; import vip.mate.wiki.model.WikiTransformationEntity; import vip.mate.wiki.repository.WikiKnowledgeBaseMapper; @@ -11,11 +12,14 @@ import vip.mate.wiki.repository.WikiTransformationMapper; import java.time.LocalDateTime; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -128,4 +132,60 @@ class WikiTransformationStarterPackGlobalE2ETest { assertTrue(names.containsAll(STARTER_PACK), "listByWorkspace for an arbitrary workspace should still include the global starter pack"); } + + @Test + @DisplayName("Global starter-pack templates are read-only: update/delete rejected (regression for #456)") + void globalTemplateIsReadOnly() { + // Pick a real seeded global template (workspace_id NULL). + WikiTransformationEntity before = service.listByWorkspace(1L).stream() + .filter(t -> t.getWorkspaceId() == null) + .findFirst() + .orElseThrow(() -> new AssertionError("expected at least one global template after V165")); + long id = before.getId(); + String originalPrompt = before.getPromptTemplate(); + + // update must be rejected, and the stored row must be untouched afterwards. + WikiTransformationEntity patch = new WikiTransformationEntity(); + patch.setTitle("tampered title"); + MateClawException updateEx = assertThrows(MateClawException.class, + () -> service.update(id, patch)); + assertEquals(403, updateEx.getCode()); + WikiTransformationEntity afterUpdate = service.getById(id); + assertEquals(before.getTitle(), afterUpdate.getTitle(), + "global template title must not change after a rejected update"); + assertEquals(originalPrompt, afterUpdate.getPromptTemplate()); + + // delete must be rejected too; the row must still be present. + MateClawException deleteEx = assertThrows(MateClawException.class, + () -> service.delete(id)); + assertEquals(403, deleteEx.getCode()); + assertTrue(service.getById(id) != null, "global template must not be deleted"); + } + + @Test + @DisplayName("findByName prefers a workspace-local template over a same-named global one") + void findByNamePrefersWorkspaceLocalOverGlobal() { + long ws = 555555L; + long kb = newKb(ws); + // A starter-pack name exists globally; create a workspace-wide clone with the same name. + String sharedName = "contract-risk-extract"; + WikiTransformationEntity local = new WikiTransformationEntity(); + long localId = SEQ.incrementAndGet(); + local.setId(localId); + local.setKbId(null); + local.setWorkspaceId(ws); + local.setName(sharedName); + local.setTitle("local override"); + local.setPromptTemplate("local prompt"); + local.setEnabled(true); + local.setCreateTime(LocalDateTime.now()); + local.setUpdateTime(LocalDateTime.now()); + local.setDeleted(0); + transformationMapper.insert(local); + + Optional hit = service.findByName(kb, ws, sharedName); + assertTrue(hit.isPresent()); + assertEquals(localId, hit.get().getId(), + "findByName must return the workspace-local template, not the global starter pack"); + } }