From 3e34cf450883eaa4e548fdebb6e4f47905393b0f Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Apr 2026 10:13:22 +0000 Subject: [PATCH 1/6] do not send remove commit to removed members --- integration/test/MLS/Util.hs | 3 ++- .../src/Wire/ConversationStore/MLS/Types.hs | 4 ++++ .../Wire/ConversationSubsystem/MLS/Message.hs | 21 ++++++++++++------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/integration/test/MLS/Util.hs b/integration/test/MLS/Util.hs index a30031f6362..6fdc23d0a9d 100644 --- a/integration/test/MLS/Util.hs +++ b/integration/test/MLS/Util.hs @@ -682,7 +682,8 @@ consumingMessages mlsProtocol mp = Codensity $ \k -> do -- at this point we know that every new user has been added to the -- conversation - for_ (zip clients wss) $ \((cid, t), ws) -> case t of + let clients' = filter (\(ci, _) -> ci `Set.notMember` conv.membersToBeRemoved) clients + for_ (zip clients' wss) $ \((cid, t), ws) -> case t of MLSNotificationMessageTag -> when (conv.epoch > 0) $ void $ diff --git a/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs b/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs index 2573cfdd0fd..f3e620ed1de 100644 --- a/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs +++ b/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Types.hs @@ -174,6 +174,10 @@ cmSingleton cid idx = (cidQualifiedUser cid) (Map.singleton (ciClient cid) idx) +-- | Construct the subset of the first client map consisting of users that are present in the second. +cmIntersect :: ClientMap a -> ClientMap b -> ClientMap a +cmIntersect (ClientMap m1) (ClientMap m2) = ClientMap (Map.intersection m1 m2) + -- | Inform a handler for 'POST /conversations/list-ids' if the MLS global team -- conversation and the MLS self-conversation should be included in the -- response. diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs index 1059711effd..a5c116c2273 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs @@ -35,6 +35,7 @@ import Data.Set qualified as Set import Data.Tagged import Data.Text.Lazy qualified as LT import Data.Tuple.Extra +import Debug.Trace import Galley.Types.Error import Imports import Polysemy @@ -297,7 +298,7 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do senderIdentity <- getSenderIdentity qusr c bundle.sender lConvOrSub - (events, newClients) <- handleGroupInfoMismatch lConvOrSubId bundle $ lowerCodensity $ do + (events, newClients, lConvOrSub') <- handleGroupInfoMismatch lConvOrSubId bundle $ lowerCodensity $ do (events, newClients) <- case senderIdentity.index of Just _ -> do -- extract added/removed clients from bundle @@ -341,11 +342,18 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do action bundle.commit.value.path pure ([], mempty) - lift $ do + lConvOrSub' <- lift $ do updateOutOfSyncFlag senderIdentity.client lConvOrSub storeGroupInfo convOrSub.id (GroupInfoData bundle.groupInfo.raw) - propagateMessage qusr (Just c) lConvOrSub conn bundle.rawMessage convOrSub.members - pure (events, newClients) + -- reload conversation from db to make sure we have an up-to-date list of members + lConvOrSub' <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId + let convOrSub' = tUnqualified lConvOrSub' + mems = cmIntersect (void convOrSub.members) convOrSub'.members + traceM $ "original: " <> show convOrSub.members + traceM $ "intersected: " <> show mems + propagateMessage qusr (Just c) lConvOrSub conn bundle.rawMessage mems + pure lConvOrSub' + pure (events, newClients, lConvOrSub') -- send welcome messages for_ bundle.welcome $ \welcome -> @@ -353,11 +361,8 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do -- send application message for_ bundle.appMessage $ \msg -> do - -- reload conversation from db to make sure we have an up-to-date list of members - lConvOrSub' <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId let convOrSub' = tUnqualified lConvOrSub' - propagateMessage qusr (Just c) lConvOrSub' conn msg.rawMessage $ - void convOrSub'.members + propagateMessage qusr (Just c) lConvOrSub' conn msg.rawMessage convOrSub'.members pure events From 7f5d5c74f85ff97fcdf4489646cfffd4ba0ffdf7 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 30 Apr 2026 08:16:04 +0000 Subject: [PATCH 2/6] fix wrong client/socket association --- integration/test/MLS/Util.hs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/test/MLS/Util.hs b/integration/test/MLS/Util.hs index 6fdc23d0a9d..cdbd6fcee2c 100644 --- a/integration/test/MLS/Util.hs +++ b/integration/test/MLS/Util.hs @@ -682,8 +682,8 @@ consumingMessages mlsProtocol mp = Codensity $ \k -> do -- at this point we know that every new user has been added to the -- conversation - let clients' = filter (\(ci, _) -> ci `Set.notMember` conv.membersToBeRemoved) clients - for_ (zip clients' wss) $ \((cid, t), ws) -> case t of + let cssNoToBeRemoved = filter (\((ci, _), _) -> ci `Set.notMember` conv.membersToBeRemoved) (zip clients wss) + for_ cssNoToBeRemoved $ \((cid, t), ws) -> case t of MLSNotificationMessageTag -> when (conv.epoch > 0) $ void $ @@ -718,7 +718,7 @@ consumeMessageNoExternal cs cid mp = consumeMessageWithPredicate isNewMLSMessage where -- the backend (correctly) reacts to a commit removing someone from a parent conversation with a -- remove proposal, however, we don't want to consume this here - isNewMLSMessageNotifButNoProposal :: Value -> App Bool + isNewMLSMessageNotifButNoProposal :: (HasCallStack) => Value -> App Bool isNewMLSMessageNotifButNoProposal n = do isRelevantNotif <- isNewMLSMessageNotif n &&~ isNotifConvId mp.convId n if isRelevantNotif From 59bf9a2aeb7a1f404392d1785c4bed35ed486d9d Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 30 Apr 2026 08:16:33 +0000 Subject: [PATCH 3/6] remove debug prints --- .../src/Wire/ConversationSubsystem/MLS/Message.hs | 3 --- 1 file changed, 3 deletions(-) diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs index a5c116c2273..dde9267c7e8 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs @@ -35,7 +35,6 @@ import Data.Set qualified as Set import Data.Tagged import Data.Text.Lazy qualified as LT import Data.Tuple.Extra -import Debug.Trace import Galley.Types.Error import Imports import Polysemy @@ -349,8 +348,6 @@ postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do lConvOrSub' <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId let convOrSub' = tUnqualified lConvOrSub' mems = cmIntersect (void convOrSub.members) convOrSub'.members - traceM $ "original: " <> show convOrSub.members - traceM $ "intersected: " <> show mems propagateMessage qusr (Just c) lConvOrSub conn bundle.rawMessage mems pure lConvOrSub' pure (events, newClients, lConvOrSub') From 79c528abe312405ec0e77a3785c23cb674c989f9 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 30 Apr 2026 08:40:41 +0000 Subject: [PATCH 4/6] changelog --- changelog.d/3-bug-fixes/WPB-21359 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3-bug-fixes/WPB-21359 diff --git a/changelog.d/3-bug-fixes/WPB-21359 b/changelog.d/3-bug-fixes/WPB-21359 new file mode 100644 index 00000000000..4c0a7892df3 --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-21359 @@ -0,0 +1 @@ +Fix: Inconsistent removal messages across local and federated conversation members From 0bf12b955a14e96c68b3afd4b8f9423ea8fed50b Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Tue, 5 May 2026 08:37:14 +0200 Subject: [PATCH 5/6] Add federated removal test --- integration/test/Test/MLS.hs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index de871aae26e..567168f1200 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -1209,3 +1209,13 @@ testGroupIdParseError = do resp.json %. "label" `shouldMatch` "mls-protocol-error" msg <- resp.json %. "message" & asString assertBool "unexpected error message" $ "Could not parse group ID:" `isPrefixOf` msg + +testFederatedRemove :: (HasCallStack) => App () +testFederatedRemove = do + [alice, amy, bob] <- createAndConnectUsers [OwnDomain, OwnDomain, OtherDomain] + [alice1, amy1, bob1] <- traverse (createMLSClient def) [alice, amy, bob] + traverse_ (uploadNewKeyPackage def) [amy1, bob1] + convId <- createNewGroup def alice1 + + void $ createAddCommit alice1 convId [amy, bob] >>= sendAndConsumeCommitBundle + void $ createRemoveCommit alice1 convId [amy1, bob1] >>= sendAndConsumeCommitBundle From 45c4cb5ccc469b1bca1465a2113f98e4720c62f8 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 5 May 2026 09:02:09 +0000 Subject: [PATCH 6/6] hi ci