fix: recover MLS conversations after key-package exhaustion - WPB-27395 🍒 - #5119
fix: recover MLS conversations after key-package exhaustion - WPB-27395 🍒#5119johnxnguyen wants to merge 2 commits into
Conversation
…95 (#5074) # Conflicts: # wire-ios-data-model/Source/Model/Conversation/ZMConversation+Messaging.swift
![semgrep-code-wireapp[bot]](https://avatars.githubusercontent.com/u/16047324?s=60&v=4){kind=small}
| let conversation = ZMConversation.insertNewObject(in: uiMOC) | ||
| conversation.remoteIdentifier = UUID() | ||
| conversation.domain = self.localDomain | ||
| conversation.mlsGroupID = .random() |
There was a problem hiding this comment.
Semgrep identified an issue, but thinks it may be safe to ignore.
The App uses an insecure Random Number Generator.
Why this might be safe to ignore:
This match is in a unit test, where
.random()is only used to create test data for a conversation group ID. The rule is intended to catch insecure randomness in security-sensitive production code, and this usage does not affect real application security.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by ios_insecure_random_no_generator.
You can view more details about this finding in the Semgrep AppSec Platform.
|
Semgrep found 1 The App uses an insecure Random Number Generator. |
There was a problem hiding this comment.
Pull request overview
This PR adds an automatic recovery path for pending MLS (and mixed-protocol) conversations after quick sync, targeting scenarios where offline clients miss Welcome messages due to key-package exhaustion. It integrates recovery into the sync lifecycle and updates the MLS data layer and tests to support the new behavior.
Changes:
- Trigger pending-conversation recovery after incremental sync completes (foreground/active only), alongside key-package refills.
- Add
recoverPendingConversationBatchIfNeeded()toMLSServiceInterfaceand implement a batched recovery mechanism inMLSService. - Update Core Data fetch predicates to support filtering by message protocol (MLS + mixed) and exclude remotely-deleted conversations; update mocks/tests accordingly.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wire-ios-sync-engine/Tests/Source/UserSession/ZMUserSessionTests.swift | Updates quick-sync test to assert pending-conversation recovery behavior while active vs background. |
| wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift | Runs pending-conversation recovery in a loop after quick sync when app is active. |
| wire-ios-data-model/Tests/MLS/MLSServiceTests.swift | Adds/adjusts tests for batched pending recovery and key-package backend-count gating. |
| wire-ios-data-model/Support/Sourcery/generated/AutoMockable.generated.swift | Adds mock support for recoverPendingConversationBatchIfNeeded(). |
| wire-ios-data-model/Source/Model/Conversation/ZMConversation+Messaging.swift | Extends MLS-status fetch to filter by protocol(s), require MLS group ID, and exclude deleted conversations. |
| wire-ios-data-model/Source/MLS/MLSServiceInterface.swift | Adds the new recovery API to the MLS service protocol. |
| wire-ios-data-model/Source/MLS/MLSService.swift | Implements batched pending recovery + adjusts key-package upload behavior when pending recovery exists. |
| wire-ios-data-model/Source/MLS/MLSClientManager.swift | Removes performPendingJoins() call from MLS client update path. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
|
|
||
| // Return whether another batch should be processed immediately. | ||
| return recovered == batch.count && pendingConversations.count > batch.count |
Test Results 7 files 988 suites 10m 26s ⏱️ Results for commit 70d30ef. Summary: workflow run #32005999502 |
Issue
Cherrypick of #5074
Testing
See original PR.
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: