crypto: auto-heal split-brain device_keys state on for_user #88

Open

jlxq0 wants to merge 1 commit from crypto/auto-heal-device-keys into main

jlxq0 commented

2026-05-21 09:59:09 +00:00

Owner

Root cause

matrix-rust-sdk's per-account shared flag (a.k.a. device_keys_published) latches true on the first successful /keys/upload and is never re-verified against the homeserver. If the upload was actually rejected — e.g. /setup attempting its own unbound bearer, a transient network blip, or a later admin-side device wipe — the SDK keeps operating against a device that doesn't exist server-side. Every subsequent /keys/upload is short-circuited (keys_for_upload() returns None while shared == true); the device shows up "Unverified — doesn't support encryption" in Element even though OTKs are uploaded; /setup/recover's self-sign attaches to local keys that never landed on Synapse.

Repro'd on 2026-05-21: MATRIXMCP2 had 50 OTKs in e2e_one_time_keys_json and zero rows in e2e_device_keys_json. matrix-rust-sdk emitted the canonical log line "Our own device might have been deleted" on every sync but no remediation triggered.

What this PR does

MatrixClientCache::for_user now post-verifies (Client::encryption().get_user_devices(...)) once per cached client. On a negative result it evicts, wipes the per-MXID store subdirectory, and recurses exactly once. A second negative after rebuild surfaces as an error rather than looping. The wipe is scoped to one MXID's hash directory under the store root — never the root itself, never another user's data.

src/matrix_client.rs: refactor for_user → for_user_inner(retry: bool), new helpers verify_or_heal and verify_device_keys_published, new wipe_user_store on the cache, new keys_verified: AtomicBool on CachedClient. New tests for the wipe (scope + idempotency) and the metric.
src/metrics.rs: new matrix_mcp_client_cache_self_heal_total{reason="..."} IntCounterVec. Single reason label for now (device_keys_missing); the design leaves room for future heal causes.
src/setup.rs: comment refresh — the existing contains() precondition stays as defense-in-depth; for_user is now the primary guard.

Why scoped per-MXID wipe is the right level

The split-brain is fundamentally per-user crypto state; nothing else in the store should be reset.
matrix-rust-sdk exposes no public way to flip shared back to false. The pickled Account is a single encrypted blob in kv[key="account"]; surgical mutation would require per-SDK-version schema understanding and is more fragile than a scoped rebuild.
Key backup restores Megolm history within seconds (~12s for 924 keys / 31 rooms in our repro). Olm sessions re-establish lazily and transparently. From the user's perspective /setup/recover just takes 30s longer; no error, no manual intervention.

Why not upstream this to matrix-rust-sdk

A public Encryption::republish_device_keys() or periodic verification of the shared flag against /keys/query would make this workaround vestigial. Worth filing eventually; not in scope for this PR.

Observability

Paired with argocd PR #551 which adds a Grafana Loki alert on the SDK warning "Our own device might have been deleted". The combination is: alert fires when the split-brain happens in the wild → the next /mcp tool call for that user auto-heals → metric increments → alert resolves on its own.

🤖 Generated with Claude Code

## Root cause matrix-rust-sdk's per-account `shared` flag (a.k.a. `device_keys_published`) latches `true` on the first successful `/keys/upload` and is never re-verified against the homeserver. If the upload was actually rejected — e.g. /setup attempting its own unbound bearer, a transient network blip, or a later admin-side device wipe — the SDK keeps operating against a device that doesn't exist server-side. Every subsequent `/keys/upload` is short-circuited (`keys_for_upload()` returns `None` while `shared == true`); the device shows up "Unverified — doesn't support encryption" in Element even though OTKs are uploaded; `/setup/recover`'s self-sign attaches to local keys that never landed on Synapse. Repro'd on 2026-05-21: `MATRIXMCP2` had 50 OTKs in `e2e_one_time_keys_json` and zero rows in `e2e_device_keys_json`. matrix-rust-sdk emitted the canonical log line `"Our own device might have been deleted"` on every sync but no remediation triggered. ## What this PR does `MatrixClientCache::for_user` now post-verifies (`Client::encryption().get_user_devices(...)`) once per cached client. On a negative result it evicts, wipes the per-MXID store subdirectory, and recurses exactly once. A second negative after rebuild surfaces as an error rather than looping. The wipe is scoped to one MXID's hash directory under the store root — never the root itself, never another user's data. - `src/matrix_client.rs`: refactor `for_user` → `for_user_inner(retry: bool)`, new helpers `verify_or_heal` and `verify_device_keys_published`, new `wipe_user_store` on the cache, new `keys_verified: AtomicBool` on `CachedClient`. New tests for the wipe (scope + idempotency) and the metric. - `src/metrics.rs`: new `matrix_mcp_client_cache_self_heal_total{reason="..."}` `IntCounterVec`. Single reason label for now (`device_keys_missing`); the design leaves room for future heal causes. - `src/setup.rs`: comment refresh — the existing `contains()` precondition stays as defense-in-depth; for_user is now the primary guard. ## Why scoped per-MXID wipe is the right level - The split-brain is fundamentally per-user crypto state; nothing else in the store should be reset. - matrix-rust-sdk exposes no public way to flip `shared` back to `false`. The pickled `Account` is a single encrypted blob in `kv[key="account"]`; surgical mutation would require per-SDK-version schema understanding and is more fragile than a scoped rebuild. - Key backup restores Megolm history within seconds (~12s for 924 keys / 31 rooms in our repro). Olm sessions re-establish lazily and transparently. From the user's perspective `/setup/recover` just takes 30s longer; no error, no manual intervention. ## Why not upstream this to matrix-rust-sdk A public `Encryption::republish_device_keys()` or periodic verification of the `shared` flag against `/keys/query` would make this workaround vestigial. Worth filing eventually; not in scope for this PR. ## Observability Paired with [argocd PR #551](https://github.com/thehansogroup/argocd/pull/551) which adds a Grafana Loki alert on the SDK warning `"Our own device might have been deleted"`. The combination is: alert fires when the split-brain happens in the wild → the next /mcp tool call for that user auto-heals → metric increments → alert resolves on its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jlxq0 added 1 commit

2026-05-21 09:59:09 +00:00

crypto: auto-heal split-brain device_keys state on for_user

CI / cargo (pull_request) Successful in 45s

Details

CI / docker (pull_request) Successful in 1m7s

Details

4c55886cca

When matrix-rust-sdk's local `device_keys_published` flag is latched to
true but Synapse doesn't actually have our device's keys (e.g. an earlier
/keys/upload was rejected with the SDK still flipping the flag),
subsequent /keys/upload calls are skipped forever and the device stays
invisible to the homeserver. The SDK exposes no public API to clear the
flag, so the only recovery is to wipe the per-MXID crypto store and let
the SDK rebuild from scratch.

`MatrixClientCache::for_user` now post-verifies via /keys/query once per
cached client. On a negative result it evicts, wipes the per-MXID store
directory, and recurses exactly once. A second negative after rebuild
errors out instead of looping. The wipe is scoped to one MXID's
subdirectory under the store root — never the root itself, never
another user's data.

A new `matrix_mcp_client_cache_self_heal_total{reason}` counter records
each self-heal so we can alert on it in Loki/Grafana. The /setup
precondition stays as defense-in-depth.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>