fix(matrix): refresh stale peer device lists before encrypted sends#61126
Open
chrisplough wants to merge 1 commit into
Open
fix(matrix): refresh stale peer device lists before encrypted sends#61126chrisplough wants to merge 1 commit into
chrisplough wants to merge 1 commit into
Conversation
mautrix-python only re-fetches a peer's device list when the homeserver
reports that peer in device_lists.changed during /sync
(OlmMachine.handle_device_lists); there is no /keys/changes catch-up or
manual invalidation path. If that signal is missed — gateway downtime
during a peer's device rotation, crypto-store recreation, or a
Conduit-family homeserver that fails to emit it — the cached device list
is stale forever: outbound megolm sessions are shared only to the peer's
dead device and the peer can never decrypt ("No one-time keys nor device
keys got when trying to share keys" on their side). Own-device recovery
does not cover this; the failing direction is encrypting TO a rotated
peer.
Route every encrypted send path (text, media, edits, reactions,
emotes/notices) through a single _send_room_event() chokepoint that
performs a throttled, best-effort /keys/query (include_untracked=True,
under the OlmMachine's _fetch_keys_lock) for the room's other members
before sharing the megolm session. Throttle is configurable via
matrix.device_refresh_seconds in config.yaml (default 300, 0 disables),
bridged to MATRIX_DEVICE_REFRESH_SECONDS like the other matrix keys.
Refresh failures never block the send; logging is debug-level only.
Same class of fix as matrix-rust-sdk's mark_all_tracked_users_as_dirty
(matrix-rust-sdk#3965).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 task
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
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.
What does this PR do?
Fixes a Matrix E2EE bug where a peer who rotates their device can permanently lose the ability to decrypt the bot's messages, because mautrix-python never re-fetches the peer's device list once the homeserver's one-shot notification is missed.
Problem. mautrix-python only invalidates its cached device list for another user when the homeserver reports that user in
device_lists.changedin a/syncresponse (OlmMachine.handle_device_listsis the only code path). It has no catch-up (/keys/changesafter a gap) and no manual invalidation path. If that single signal is missed, the cache is stale forever: outbound megolm sessions are shared only to the peer's dead device, and every message the bot sends is undecryptable for that peer. The peer's client shows the classic "unable to decrypt" and their key-share requests fail with "No one-time keys nor device keys got when trying to share keys".The signal gets missed in realistic, recurring ways:
/syncresponse (sync token advances) and fully applying itsdevice_listssection. The entry is spent and is not replayed on the next sync.device_lists.changedfor the rotation. This is a known long-standing bug class in the Conduit family: legacy Conduit (open since 2022, famedly/conduit#325), conduwuit (unmaintained), and Tuwunel before 1.6.1 (device_lists.changed never included in /sync responses after cross-signing key upload or device key changes matrix-construct/tuwunel#377, fixed 2026-05). On those servers the tracked-but-stale state is reached on every peer rotation._share_group_sessiononly fetches keys for users whose cached device dict is absent entirely. Multiple users in [Bug]: Matrix gateway unable to decrypt message #13891 are sitting in exactly this state today.Why own-device fixes don't cover this. #14956 (and
_verify_device_keys_on_servergenerally) handles the bot's OWN device keys being stale or missing on the server. This bug is the opposite direction: the bot's device is perfectly healthy; what is stale is the bot's cached view of the PEER's devices when encrypting TO them. That is why several users in #13891 still report the exact "No one-time keys nor device keys got when trying to share keys" symptom after #14956 landed.Precedent. matrix-rust-sdk hit the same class and added an explicit invalidation API,
mark_all_tracked_users_as_dirty(matrix-rust-sdk PR: matrix-org/matrix-rust-sdk#3965). mautrix-python has no equivalent, so this fix performs a throttled proactive re-query at the send boundary instead.The fix. All outbound events that mautrix may encrypt (text, media, edits, reactions, emotes/notices) already funnel through
Client.send_message_event. This PR routes the adapter's six call sites through a single new chokepoint,MatrixAdapter._send_room_event(), which first runs_refresh_encrypted_room_devices():matrix.device_refresh_seconds(config.yaml, default 300,0disables). Bridged to the internalMATRIX_DEVICE_REFRESH_SECONDSenv var by_apply_yaml_config, same as the other matrix keys; docs point at config.yaml.crypto._fetch_keys(members, include_untracked=True).include_untracked=Trueis load-bearing: it also repairs members mautrix never tracked (crypto-store recreation case)._fetch_keys_lockaround the query (hasattr-guarded, degrades gracefully if a future mautrix renames it) so the out-of-band/keys/querycannot race a sync-driven one.When a rotation IS detected, mautrix's own
_process_fetched_keys->on_devices_changeddrops the outbound group session, so the very next send re-shares keys to the peer's current device set. No new session churn otherwise.Related Issue
Fixes #13891
Type of Change
Changes Made
plugins/platforms/matrix/adapter.py_refresh_encrypted_room_devices()(throttled, locked, best-effort peer device-list re-query)._send_room_event()chokepoint; the sixsend_message_eventcall sites (send()incl. its E2EE retry,edit_message(),_upload_and_send(),_send_reaction(),_send_simple_message()) now route through it.__init__readsdevice_refresh_secondsfromconfig.extrawith the establishedMATRIX_DEVICE_REFRESH_SECONDSenv bridge fallback;_apply_yaml_configbridges the config.yaml key.hermes_cli/config.py:matrix.device_refresh_seconds: 300added toDEFAULT_CONFIG(additive key, no_config_versionbump needed per AGENTS.md).website/docs/user-guide/messaging/matrix.md: one bullet in the E2EE section describing the refresh and the config knob.tests/gateway/test_matrix_device_refresh.py: 18 new tests (see below).Out of scope on purpose:
_standalone_send()(cron delivery) is a raw Client-Server API plaintext sender with no E2EE, so it is not part of this bug class.How to Test
Reproduce on current main. The deterministic variant simulates the tracked-but-stale state directly (it is reachable through any of the paths above; this makes it reproducible in minutes on any homeserver):
MATRIX_E2EE_MODE=required, and a peer account (Element or a second Hermes instance) sharing an encrypted room. Exchange a few messages so the peer is tracked in the crypto store.crypto.db, the peer's cached device row still lists only the old (now dead) device — verify withsqlite3 ... "SELECT device_id FROM crypto_device WHERE user_id='@peer:...'". (If your stop/rotate timing happened to deliver thedevice_lists.changed, delete the peer's new-device row to restore the stale state — this is exactly the state crash-loss or a Conduit-family server leaves behind.)device_lists.changedfor local users.device_refresh_secondslater) re-queries the peer's devices, mautrix drops the outbound session, and the message re-shares to the current device set. The peer decrypts normally.Test suite:
pytest tests/gateway/test_matrix_device_refresh.py -q(18 passed) covering: refresh before an encrypted send once the interval elapsed; throttling within the interval;device_refresh_seconds: 0disables; a_fetch_keysexception never breaks the send; tracked and untracked members both requeried withinclude_untracked=Trueand own user excluded; unencrypted rooms skipped;_fetch_keys_lockheld around the query; every sibling send path (text, edit, media, reaction, emote/notice, E2EE retry) routed through the chokepoint; config precedence (extra > env bridge > default) and the_apply_yaml_configbridge. Existing matrix suites (tests/gateway/test_matrix*.py,tests/hermes_cli/test_config.py,tests/hermes_cli/test_setup_matrix_e2ee.py) pass unchanged.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (matrix + config areas run locally; see How to Test)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/A (matrix keys live inDEFAULT_CONFIGand the matrix docs page, matching the existing matrix settings;cli-config.yaml.examplehas no matrix block)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Debug log line after a rotation is picked up:
Disclosure: this fix was developed and validated in production during an AI-assisted engineering session — a Hermes deployment doing agent-to-agent E2EE hit the tracked-but-stale state during a churn-heavy rollout (repeated device rotations while gateways restarted), and this refresh cured it immediately and durably. For honesty: we could not pin our incident on the homeserver — a controlled test on Tuwunel 1.8.0 showed it emits
device_lists.changedcorrectly — which is precisely the point of fixing this client-side: the stale state arises from more than one path, and mautrix has no recovery from any of them. Ported to current main and adapted to the contribution rubric; a human reviewed everything before submission.