From f70e56cfc3c0cf931de525686077d510b582d42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Thu, 18 Jun 2026 11:38:09 +0800 Subject: [PATCH] fix(conversation): exclude malformed conversationIds from admin list/page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conversationId ending in ":" (e.g. webchat:: with empty visitorId, from older webchat versions) leaks into the admin console via the 'webchat:%' username LIKE, then 500/403s on open because the trailing ":" makes some reverse proxies strip the path tail — landing a GET on the @DeleteMapping variant of /{conversationId} (issue #369). Add applyMalformedIdGuard — a NOT LIKE '%:' clause — to both listConversations (lenient + strict overloads) and pageConversations so these rows never surface. isConversationOwner already rejects unknown ids with 403, so no change is needed on the direct-access endpoints; once the rows are out of the lists, admin can no longer reach them. The two existing strict/non-admin assertions changed from "no LIKE keyword" to "no webchat:% param value" — applyMalformedIdGuard emits a NOT LIKE itself, so the LIKE keyword is now present in every query. Tests cover the guard on lenient, page, and strict paths. --- .../conversation/ConversationService.java | 13 ++++ ...versationServiceWebchatVisibilityTest.java | 73 +++++++++++++++++-- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java index c578f3d5..5599aefb 100644 --- a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java +++ b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java @@ -150,6 +150,7 @@ public class ConversationService { boolean includeWebchat = includeChannelPrincipals && isGlobalAdmin(username); LambdaQueryWrapper wrapper = new LambdaQueryWrapper() .and(w -> applyOwnerScope(w, username, includeWebchat)) + .and(this::applyMalformedIdGuard) .isNull(ConversationEntity::getParentConversationId) .orderByDesc(ConversationEntity::getPinned) .orderByDesc(ConversationEntity::getLastActiveTime); @@ -203,6 +204,17 @@ public class ConversationService { } } + /** + * Exclude rows whose conversationId ends in ":" — malformed (e.g. + * {@code webchat::} with empty visitorId, from older versions). + * Showing them in the console surfaces threads that 500/403 on open + * because the trailing ":" makes some reverse proxies strip the path + * tail (issue #369). + */ + private void applyMalformedIdGuard(LambdaQueryWrapper w) { + w.notLike(ConversationEntity::getConversationId, "%:"); + } + /** * Whether the user is a global admin (role=admin), resolved from the DB — * never from client-controlled data. Gates webchat row visibility in the @@ -239,6 +251,7 @@ public class ConversationService { // conversation (issue #344), so non-admins must not see those rows. LambdaQueryWrapper wrapper = new LambdaQueryWrapper() .and(w -> applyOwnerScope(w, username, isGlobalAdmin(username))) + .and(this::applyMalformedIdGuard) .isNull(ConversationEntity::getParentConversationId) .orderByDesc(ConversationEntity::getPinned) .orderByDesc(ConversationEntity::getLastActiveTime); diff --git a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java index 355a44f8..48a4c73b 100644 --- a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java +++ b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceWebchatVisibilityTest.java @@ -79,7 +79,7 @@ class ConversationServiceWebchatVisibilityTest { } @Test - @DisplayName("lenient list, non-admin: excludes webchat principals (no LIKE clause)") + @DisplayName("lenient list, non-admin: excludes webchat principals (no 'webchat:%' param)") void lenientListNonAdminExcludesWebchat() { when(authService.findByUsername("alice")).thenReturn(user("member")); ArgumentCaptor> captor = @@ -88,12 +88,14 @@ class ConversationServiceWebchatVisibilityTest { service.listConversations("alice", 1L, true); - String sql = captor.getValue().getTargetSql(); - assertThat(sql).doesNotContainIgnoringCase("like"); + // The malformed-id guard still emits a NOT LIKE, so we assert on the + // param value instead of the LIKE keyword. + assertThat(captor.getValue().getParamNameValuePairs().values()) + .doesNotContain("webchat:%"); } @Test - @DisplayName("strict list excludes webchat principals (no LIKE clause, no role lookup)") + @DisplayName("strict list excludes webchat principals (no 'webchat:%' param, no role lookup)") void strictListExcludesWebchat() { ArgumentCaptor> captor = ArgumentCaptor.forClass(LambdaQueryWrapper.class); @@ -101,8 +103,8 @@ class ConversationServiceWebchatVisibilityTest { service.listConversations("admin", 1L); // strict 2-arg - String sql = captor.getValue().getTargetSql(); - assertThat(sql).doesNotContainIgnoringCase("like"); + assertThat(captor.getValue().getParamNameValuePairs().values()) + .doesNotContain("webchat:%"); } @Test @@ -133,8 +135,63 @@ class ConversationServiceWebchatVisibilityTest { service.pageConversations("alice", 1L, 1, 20, null); - String sql = captor.getValue().getTargetSql(); - assertThat(sql).doesNotContainIgnoringCase("like"); + assertThat(captor.getValue().getParamNameValuePairs().values()) + .doesNotContain("webchat:%"); + } + + // ------------------------------------------------------------------ + // Malformed conversationId guard — rows whose id ends in ":" (e.g. an + // empty-visitorId webchat thread) are filtered out of every admin list + // query, regardless of role. Surfacing them triggers 500/403 on open + // because the trailing ":" confuses some reverse proxies (issue #369). + // ------------------------------------------------------------------ + + @Test + @DisplayName("lenient list: applies NOT LIKE '%:' guard to filter malformed ids") + void lenientListAppliesMalformedIdGuard() { + when(authService.findByUsername("admin")).thenReturn(user("admin")); + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectList(captor.capture())).thenReturn(List.of()); + + service.listConversations("admin", 1L, true); + + // Assert on the rendered SQL (not the param values, which MyBatis-Plus + // percent-escapes internally) so the test stays independent of that + // implementation detail. + String sql = captor.getValue().getTargetSql().toLowerCase(); + assertThat(sql).contains("not like"); + assertThat(sql).contains("conversation_id"); + } + + @Test + @DisplayName("page query: applies the same NOT LIKE '%:' guard") + void pageAppliesMalformedIdGuard() { + when(authService.findByUsername("admin")).thenReturn(user("admin")); + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectPage(any(Page.class), captor.capture())) + .thenReturn(new Page<>()); + + service.pageConversations("admin", 1L, 1, 20, null); + + String sql = captor.getValue().getTargetSql().toLowerCase(); + assertThat(sql).contains("not like"); + assertThat(sql).contains("conversation_id"); + } + + @Test + @DisplayName("strict list also applies the guard — malformed ids never leak to owner-only views") + void strictListAppliesMalformedIdGuard() { + ArgumentCaptor> captor = + ArgumentCaptor.forClass(LambdaQueryWrapper.class); + when(conversationMapper.selectList(captor.capture())).thenReturn(List.of()); + + service.listConversations("admin", 1L); // strict 2-arg + + String sql = captor.getValue().getTargetSql().toLowerCase(); + assertThat(sql).contains("not like"); + assertThat(sql).contains("conversation_id"); } private static UserEntity user(String role) {