fix(wiki): make global starter-pack templates read-only

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).
This commit is contained in:
倪程伟 2026-07-15 14:57:18 +08:00 committed by GitHub
parent fbfb8982cc
commit b23cc32f33
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 146 additions and 1 deletions

View File

@ -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");
}
}
}

View File

@ -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");
}
}
}

View File

@ -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]

View File

@ -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]

View File

@ -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());
}
}

View File

@ -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<WikiTransformationEntity> 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");
}
}