The id is scoped by {@code channelId} so the same sender reaching two + * different workspaces' same-type channels (e.g. two separate wecom channels) + * no longer collapses into one shared conversation row. {@code channelId} is + * the {@code ChannelEntity} primary key, which binds to exactly one workspace. + * + *
Format: {@code {channelType}:{channelId}:{chatId|senderId}}. When
+ * {@code channelId} is null (defensive; the routed channel row always has an
+ * id) the legacy {@code {channelType}:{identifier}} form is used so nothing
+ * NPEs — those ids remain workspace-ambiguous but that path is not reachable
+ * for a persisted channel.
+ */
+ private String buildConversationId(ChannelMessage message, Long channelId) {
String identifier = message.getChatId() != null ? message.getChatId() : message.getSenderId();
- return message.getChannelType() + ":" + identifier;
+ if (channelId == null) {
+ return message.getChannelType() + ":" + identifier;
+ }
+ return message.getChannelType() + ":" + channelId + ":" + identifier;
}
/**
diff --git a/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java b/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java
index b79340ac..0d0f6299 100644
--- a/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java
+++ b/mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java
@@ -1212,7 +1212,8 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
if (isGroup && chatId != null) {
shortSuffix = resolveGroupSessionSuffix(chatId);
}
- String conversationId = buildConversationId(shortSuffix, senderOpenId, isGroup);
+ String conversationId = buildConversationId(shortSuffix, senderOpenId, isGroup,
+ channelEntity != null ? channelEntity.getId() : null);
String stagedUploadPath = null;
if (isFileMessage) {
@@ -1731,14 +1732,22 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
* {@code senderId} is the full open id. Mirror that exactly:
* {@code groups → feishu:{shortSuffix}}, {@code DMs → feishu:{senderOpenId}}.
*/
- static String buildConversationId(String shortSuffix, String senderOpenId, boolean isGroup) {
+ static String buildConversationId(String shortSuffix, String senderOpenId, boolean isGroup,
+ Long channelId) {
// The routed ChannelMessage carries chatId = (isGroup ? shortSuffix : null);
// the router then falls back to senderId when that chatId is null. Mirror both
// steps so the storage id matches the runtime id in every case (including the
// degenerate group-with-no-suffix path).
String routedChatId = isGroup ? shortSuffix : null;
String identifier = routedChatId != null ? routedChatId : senderOpenId;
- return identifier != null ? CHANNEL_TYPE + ":" + identifier : null;
+ if (identifier == null) {
+ return null;
+ }
+ // Mirror ChannelMessageRouter#buildConversationId: scope the id by channelId so
+ // the same sender on two workspaces' feishu channels never shares a conversation.
+ return channelId != null
+ ? CHANNEL_TYPE + ":" + channelId + ":" + identifier
+ : CHANNEL_TYPE + ":" + identifier;
}
// ==================== Per-chat recent file cache ====================
diff --git a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java
index 41c7ed98..20261639 100644
--- a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java
+++ b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java
@@ -320,15 +320,22 @@ public class ToolGuardCardHandler implements FeishuCardHandler {
private static ChannelMessage buildSynthetic(String commandText, String clickerOpenId,
PendingApproval pending,
P2CardActionTriggerData data) {
- // pending.conversationId looks like "feishu: The router derives its id from the routed {@code ChannelMessage}, whose
* {@code chatId} is {@code (isGroup ? shortSuffix : null)} and whose {@code senderId}
- * is the full open id, via {@code feishu:{chatId != null ? chatId : senderId}}.
+ * is the full open id, via {@code feishu:{channelId}:{chatId != null ? chatId : senderId}}.
+ * The {@code channelId} segment scopes the id to one channel row (hence one workspace)
+ * so the same sender on two workspaces' feishu channels never collides into one id.
*/
class FeishuConversationIdAlignmentTest {
private static final String CHANNEL = FeishuChannelAdapter.CHANNEL_TYPE; // "feishu"
private static final String SHORT_SUFFIX = "cli2_abcd1234";
private static final String SENDER = "ou_user0123456789";
+ private static final Long CHANNEL_ID = 2056987497408438273L;
/** Mirror of ChannelMessageRouter#buildConversationId against the routed message. */
- private static String routerConversationId(String shortSuffix, String senderId, boolean isGroup) {
+ private static String routerConversationId(String shortSuffix, String senderId, boolean isGroup,
+ Long channelId) {
String routedChatId = isGroup ? shortSuffix : null;
String identifier = routedChatId != null ? routedChatId : senderId;
- return identifier != null ? CHANNEL + ":" + identifier : null;
+ if (identifier == null) {
+ return null;
+ }
+ return channelId != null
+ ? CHANNEL + ":" + channelId + ":" + identifier
+ : CHANNEL + ":" + identifier;
}
@Test
void group_usesShortSuffix() {
- assertEquals(CHANNEL + ":" + SHORT_SUFFIX,
- FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true));
+ assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SHORT_SUFFIX,
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID));
}
@Test
void dm_usesSenderOpenIdAndIgnoresShortSuffix() {
- assertEquals(CHANNEL + ":" + SENDER,
- FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false));
+ assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SENDER,
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID));
}
@Test
void group_nullShortSuffix_fallsBackToSender() {
// Degenerate group path: routed chatId is null, so the router (and this helper)
// fall back to the sender open id — never null when a sender is present.
- assertEquals(CHANNEL + ":" + SENDER,
- FeishuChannelAdapter.buildConversationId(null, SENDER, true));
+ assertEquals(CHANNEL + ":" + CHANNEL_ID + ":" + SENDER,
+ FeishuChannelAdapter.buildConversationId(null, SENDER, true, CHANNEL_ID));
}
@Test
void dm_nullSender_returnsNull() {
- assertNull(FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false));
+ assertNull(FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID));
+ }
+
+ @Test
+ void nullChannelId_fallsBackToLegacyTwoSegmentForm() {
+ // Defensive: a null channelId (unreachable for a persisted channel row) keeps
+ // the legacy {channelType}:{identifier} form so nothing NPEs.
+ assertEquals(CHANNEL + ":" + SENDER,
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, null));
}
@Test
void matchesRouterFormula_acrossCases() {
// Group and DM, with and without a short suffix — the storage id must equal
// the id the router computes for the routed ChannelMessage in every case.
- assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, true),
- FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true));
- assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, false),
- FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false));
- assertEquals(routerConversationId(null, SENDER, true),
- FeishuChannelAdapter.buildConversationId(null, SENDER, true));
- assertEquals(routerConversationId(SHORT_SUFFIX, null, false),
- FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false));
+ assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID),
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, true, CHANNEL_ID));
+ assertEquals(routerConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID),
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, SENDER, false, CHANNEL_ID));
+ assertEquals(routerConversationId(null, SENDER, true, CHANNEL_ID),
+ FeishuChannelAdapter.buildConversationId(null, SENDER, true, CHANNEL_ID));
+ assertEquals(routerConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID),
+ FeishuChannelAdapter.buildConversationId(SHORT_SUFFIX, null, false, CHANNEL_ID));
}
}
diff --git a/mateclaw-server/src/test/java/vip/mate/channel/wecom/WeComInboundConversationIdTest.java b/mateclaw-server/src/test/java/vip/mate/channel/wecom/WeComInboundConversationIdTest.java
index bc5a27cc..56a72c71 100644
--- a/mateclaw-server/src/test/java/vip/mate/channel/wecom/WeComInboundConversationIdTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/channel/wecom/WeComInboundConversationIdTest.java
@@ -20,38 +20,45 @@ import static org.junit.jupiter.api.Assertions.*;
* different conversationId — and the {@code /api/v1/chat/files/{convId}/...}
* endpoint's owner check fails for every fetch (403 → broken images).
*
- * The format both produce: {@code wecom:{chatId}} for groups,
- * {@code wecom:{senderId}} for 1:1 — no {@code group:} infix.
+ * The format both produce: {@code wecom:{channelId}:{chatId}} for groups,
+ * {@code wecom:{channelId}:{senderId}} for 1:1 — no {@code group:} infix. The
+ * {@code channelId} segment scopes the id to one channel row (hence one
+ * workspace) so the same sender on two workspaces' wecom channels never
+ * collides into one conversation.
*/
class WeComInboundConversationIdTest {
+ private static final Long CHANNEL_ID = 2056987497408438273L;
+
private static String inboundConversationId(String senderId, String chatId, String chatType) throws Exception {
Method m = WeComChannelAdapter.class.getDeclaredMethod(
- "inboundConversationId", String.class, String.class, String.class);
+ "inboundConversationId", String.class, String.class, String.class, Long.class);
m.setAccessible(true);
- return (String) m.invoke(null, senderId, chatId, chatType);
+ return (String) m.invoke(null, senderId, chatId, chatType, CHANNEL_ID);
+ }
+
+ /** Mirror of ChannelMessageRouter#buildConversationId for cross-checking. */
+ private static String routerConversationId(String identifier) {
+ return "wecom:" + CHANNEL_ID + ":" + identifier;
}
@Test
- @DisplayName("group → wecom:{chatId} (no 'group:' infix, matches router)")
+ @DisplayName("group → wecom:{channelId}:{chatId} (no 'group:' infix, matches router)")
void groupChatIdFormat() throws Exception {
- // The bug fix: previously returned "wecom:group:abc" which mismatched
- // the router's "wecom:abc" — quoted-image fileUrls hit a 403 because
- // isConversationOwner couldn't find a "wecom:group:abc" row in
- // mate_conversation.
- assertEquals("wecom:group-abc",
+ // The channelId segment scopes the id to one channel/workspace; the
+ // group branch still uses chatId (no "group:" infix) to match the router.
+ assertEquals("wecom:" + CHANNEL_ID + ":group-abc",
inboundConversationId("XuZhanFu", "group-abc", "group"));
}
@Test
- @DisplayName("1:1 → wecom:{senderId} (chatId is irrelevant in single chats)")
+ @DisplayName("1:1 → wecom:{channelId}:{senderId} (chatId is irrelevant in single chats)")
void singleChatSenderFormat() throws Exception {
- // Single-chat case never had the bug because both adapter and
- // router fell back to senderId — pin it so a future refactor of
- // either side doesn't accidentally diverge.
- assertEquals("wecom:XuZhanFu",
+ // Single-chat case: both adapter and router fall back to senderId; pin it
+ // so a future refactor of either side doesn't accidentally diverge.
+ assertEquals("wecom:" + CHANNEL_ID + ":XuZhanFu",
inboundConversationId("XuZhanFu", null, "single"));
- assertEquals("wecom:XuZhanFu",
+ assertEquals("wecom:" + CHANNEL_ID + ":XuZhanFu",
inboundConversationId("XuZhanFu", "ignored-when-single", "single"));
}
@@ -59,19 +66,13 @@ class WeComInboundConversationIdTest {
@DisplayName("matches ChannelMessageRouter.buildConversationId for both group and 1:1")
void matchesRouterFormat() throws Exception {
// Router's identifier picker:
- // chatId != null → "{channelType}:{chatId}" (group)
- // chatId == null → "{channelType}:{senderId}" (single)
+ // chatId != null → "{channelType}:{channelId}:{chatId}" (group)
+ // chatId == null → "{channelType}:{channelId}:{senderId}" (single)
// Inbound side passes chatId for groups, null/ignored for 1:1.
// Both must arrive at the same string, exact-equal.
-
- // group: router gets chatId from the ChannelMessage builder
- String routerGroup = "wecom" + ":" + "group-xyz";
- assertEquals(routerGroup,
+ assertEquals(routerConversationId("group-xyz"),
inboundConversationId("Alice", "group-xyz", "group"));
-
- // single: router falls back to senderId (chatId is null on the message)
- String routerSingle = "wecom" + ":" + "Alice";
- assertEquals(routerSingle,
+ assertEquals(routerConversationId("Alice"),
inboundConversationId("Alice", null, "single"));
}
}
diff --git a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceOwnershipWorkspaceTest.java b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceOwnershipWorkspaceTest.java
index b1df1669..97b9a121 100644
--- a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceOwnershipWorkspaceTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceOwnershipWorkspaceTest.java
@@ -22,6 +22,7 @@ import vip.mate.workspace.conversation.repository.MessageMapper;
import vip.mate.workspace.core.service.WorkspaceService;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
@@ -240,6 +241,38 @@ class ConversationServiceOwnershipWorkspaceTest {
return u;
}
+ // ------------------------------------------------------------------
+ // getOrCreateConversation cross-workspace defense (channel-scoping)
+ // ------------------------------------------------------------------
+
+ @Test
+ @DisplayName("getOrCreate: existing row in another workspace → rejected, no insert")
+ void getOrCreateCrossWorkspaceRejected() {
+ // An existing conversation owned by workspace A; a caller from workspace B
+ // resolving the same id must be refused rather than silently writing into A.
+ when(conversationMapper.selectOne(any()))
+ .thenReturn(conv(ALICE_CONV, "alice", WS_TENANT_A));
+
+ assertThatThrownBy(() ->
+ service.getOrCreateConversation(ALICE_CONV, 1L, "alice", WS_TENANT_B))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("工作区");
+ verify(conversationMapper, never()).insert(any(ConversationEntity.class));
+ }
+
+ @Test
+ @DisplayName("getOrCreate: existing row in the same workspace → returned, no throw")
+ void getOrCreateSameWorkspaceReturns() {
+ when(conversationMapper.selectOne(any()))
+ .thenReturn(conv(ALICE_CONV, "alice", WS_TENANT_A));
+
+ ConversationEntity got =
+ service.getOrCreateConversation(ALICE_CONV, 1L, "alice", WS_TENANT_A);
+
+ assertThat(got.getConversationId()).isEqualTo(ALICE_CONV);
+ verify(conversationMapper, never()).insert(any(ConversationEntity.class));
+ }
+
private static ConversationEntity conv(String conversationId, String username, Long workspaceId) {
ConversationEntity c = new ConversationEntity();
c.setConversationId(conversationId);