Defer key bundle downloads on server-initiated joins (MSC4509) - #5426
Open
ara4n wants to merge 4 commits into
Open
Defer key bundle downloads on server-initiated joins (MSC4509)#5426ara4n wants to merge 4 commits into
ara4n wants to merge 4 commits into
Conversation
ara4n
requested review from
MidhunSureshR and
t3chguy
and removed request for
a team
July 16, 2026 11:27
Member
|
@ara4n where can we read about this |
Member
|
msc4509 it appears. Please can such unspecced features be gated behind some sort of opt-in from the application (which in turn should gate it behind a labs flag) |
If the server joins us to a room we were invited to (e.g. auto-accepting a knock, synapse#16307), joinRoom never runs, so the MSC4268 key bundle sent by the inviter was silently dropped ('Not yet accepting key bundle for room where we are not awaiting a bundle') and pre-join history stayed undecryptable. Watch our own invite->join transitions and run the same markRoomAsPendingKeyBundle/maybeAcceptKeyBundle dance as joinRoom.
An accepted knock can be auto-joined straight past the invited state (the invite coalesced out of sync), and the invite-time record is in-memory only, so recover the inviter from the join event's unsigned.prev_sender — the same Synapse extension getDMInviter already relies on. Mirrors the equivalent rust-sdk fix.
A server-initiated join (e.g. an auto-accepted knock) lands on all of our user's devices in the same sync instant, so each would eagerly download the MSC4268 room key bundle. Instead, only download it when the server designates this device as the claimer (a new org.matrix.msc4509.key_bundle_claim to-device message, only trusted from our own user), or lazily upon the first undecryptable event in the room. Client-initiated joins via joinRoom keep the eager behaviour. The MSC4509 behaviour is unstable, so it is opt-in via the new unstableMSC4509KeyBundleClaim client option: when unset (the default), key_bundle_claim messages are ignored and server-initiated joins keep the eager per-device download.
ara4n
force-pushed
the
matthew/auto-join-key-bundle
branch
from
July 24, 2026 01:09
0b22fe2 to
3176257
Compare
Covers the eager (default) path incl. the unsigned.prev_sender fallback, and the MSC4509 deferral: designation claims, designations racing ahead of the join, the undecryptable-event fallback, sender/room_id validation, leave cleanup, and the error paths.
richvdh
self-requested a review
August 6, 2026 09:09
richvdh
requested changes
Aug 6, 2026
Comment on lines
+1328
to
+1340
| /** Map from room ID to the user who invited us, for rooms with a pending invite. | ||
| * Used to do the key-bundle bookkeeping if the server joins us to the room itself | ||
| * (e.g. auto-accepting a knock, synapse#16307) rather than via `joinRoom`. */ | ||
| private readonly pendingKeyBundleInviters = new Map<string, string>(); | ||
|
|
||
| /** Map from room ID to inviter, for rooms the server joined us to whose key bundle | ||
| * we are deferring (MSC4509): the bundle is only downloaded if the server designates | ||
| * us as the claimer, or lazily on the first undecryptable event in the room. */ | ||
| private readonly lazyKeyBundleRooms = new Map<string, string>(); | ||
|
|
||
| /** Room IDs for which an `org.matrix.msc4509.key_bundle_claim` designation arrived | ||
| * before we processed the corresponding join. */ | ||
| private readonly earlyKeyBundleClaims = new Set<string>(); |
Member
There was a problem hiding this comment.
MatrixClient is already well over 7000 lines long and has far too many properties. Please can we move all the logic and these fields out to a different class, which can be gated as whole behind unstableMSC4509KeyBundleClaim?
| // MSC4509: the server may designate this device as the one which should eagerly | ||
| // download a room key bundle after a server-initiated join (see above). Only our | ||
| // own homeserver can deliver a to-device message with our own user as sender. | ||
| this.on(ClientEvent.ToDeviceEvent, (event) => { |
Member
There was a problem hiding this comment.
As Sonar points out, ToDeviceEvent is deprecated.
Comment on lines
+1506
to
+1507
| // MSC4509 disabled: download eagerly, as `joinRoom` would. | ||
| await cryptoBackend.maybeAcceptKeyBundle(member.roomId, finalInviter); |
Member
There was a problem hiding this comment.
Let's not sneak this change of behaviour in as part of implementing an unstable MSC.
| * If the room's join hasn't been processed yet (a designation racing ahead of the | ||
| * join in the same sync), remember the designation for the join handler instead. | ||
| */ | ||
| private claimLazyKeyBundle(roomId: string, reason: string): void { |
Member
There was a problem hiding this comment.
This is a somewhat misleading name, given it won't actually "claim" the bundle if we haven't been invited yet.
| this.logger.info(`Claiming deferred key bundle for ${roomId}: ${reason}`); | ||
| cryptoBackend.maybeAcceptKeyBundle(roomId, inviter).catch((e) => { | ||
| this.logger.error(`Error accepting deferred key bundle for room ${roomId}:`, e); | ||
| }); |
Member
There was a problem hiding this comment.
Presumably this means that if the attempt to download the bundle fails (eg the server gives a 500), we'll never actually do the download? Is that a known problem?
Comment on lines
+1477
to
+1479
| if (member.membership === KnownMembership.Invite) { | ||
| const inviter = event.getSender(); | ||
| if (inviter) this.pendingKeyBundleInviters.set(member.roomId, inviter); |
Member
There was a problem hiding this comment.
This seems like it will fail in the case of a gappy sync, since we might not see the invite event?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A server-initiated join (e.g. an auto-accepted knock) lands on all of our user's devices in the same sync instant, so each would eagerly download the MSC4268 room key bundle. Instead, only download it when the server designates this device as the claimer (a new org.matrix.msc4509.key_bundle_claim to-device message, only trusted from our own user), or lazily upon the first undecryptable event in the room. Client-initiated joins via joinRoom keep the eager behaviour.
The MSC4509 behaviour is unstable, so it is opt-in via the new
unstableMSC4509KeyBundleClaimclient option: when unset (the default), key_bundle_claim messages are ignored and server-initiated joins keep the eager per-device download. Element Web enables it behind ashare_history_on_autojoinfeature flag (PR to follow).