fix: isolate Matrix clients and crypto stores by device #149

Merged

jlxq0 merged 10 commits from fix/device-scoped-crypto into main 2026-09-25 08:46:51 +00:00

Conversation 0 Commits 10 Files changed 35 +5238 -1655

jlxq0 commented 2026-09-24 23:31:17 +00:00

Owner

• Copy link
• Copy Markdown

Whole repair, exact source

Isolate same-MXID device-bound OAuth clients, encrypted stores, and routing while preserving legacy Olm/Megolm keys. Upgrade matrix-sdk to 0.19.1 and rustls to 0.23.45 without advisory waivers. Bind each existing crypto store to its operator-verified MAS subject; do not infer legacy ownership from a token or .device-id. Serialize migration/mint across processes, fail closed, and make inspect-store refusals exit nonzero.

Reviewed source head: c411a409c5bdfc71c99f6e49c50387b78a2d5d78 (independent read-only security and concurrency reviews, no blocking code findings). Local exact-head fmt, Rust 1.96 check, Clippy 1.98, tests twice (352 unit + 6 integration each), cargo audit and cargo deny pass. Audit reports one existing wasm-only derivative warning, not a security advisory waiver. Full logs and 0.17 encrypted-store fixture evidence on Kanban t_c215b8d8. Remote PR CI: https://forge.oddie.app/jlxq0/matrix-mcp/actions/runs/284 (verify its final state before merge). PR CI does not build the image.

Release and activation are separate

After exact-source review and green CI, publish a release tag, require tag image CI green and verify its immutable registry digest and matching inspect-store binary/image. Do not treat this PR or a green Argo app as a release. docs/multi-device.md specifies the rollback from pre-rollout snapshot: SDK 0.17 can silently open a 0.19-migrated schema; redeploying the old image on the same volume is not rollback. The migration empties event cache and deduplicates request bookkeeping, not Olm/Megolm keys.

Clark alone owns operational activation. No old and new pod may overlap on the store volume. Before rollout Clark must inspect only a writable snapshot clone with the reviewed inspector; prove the embedded legacy owner, MAS subject binding and homeserver device-key state without disclosing pepper/key values; obtain Julian’s specific rollout approval. Until then: no volume writes, deployment, or authenticated room read. No waiver approved.

## Whole repair, exact source Isolate same-MXID device-bound OAuth clients, encrypted stores, and routing while preserving legacy Olm/Megolm keys. Upgrade matrix-sdk to 0.19.1 and rustls to 0.23.45 without advisory waivers. Bind each existing crypto store to its operator-verified MAS subject; do not infer legacy ownership from a token or `.device-id`. Serialize migration/mint across processes, fail closed, and make `inspect-store` refusals exit nonzero. Reviewed source head: `c411a409c5bdfc71c99f6e49c50387b78a2d5d78` (independent read-only security and concurrency reviews, no blocking code findings). Local exact-head fmt, Rust 1.96 check, Clippy 1.98, tests twice (352 unit + 6 integration each), cargo audit and cargo deny pass. Audit reports one existing wasm-only derivative warning, not a security advisory waiver. Full logs and 0.17 encrypted-store fixture evidence on Kanban t_c215b8d8. Remote PR CI: https://forge.oddie.app/jlxq0/matrix-mcp/actions/runs/284 (verify its final state before merge). PR CI does not build the image. ## Release and activation are separate After exact-source review and green CI, publish a release tag, require tag image CI green and verify its immutable registry digest and matching `inspect-store` binary/image. Do not treat this PR or a green Argo app as a release. `docs/multi-device.md` specifies the rollback from pre-rollout snapshot: SDK 0.17 can silently open a 0.19-migrated schema; redeploying the old image on the same volume is not rollback. The migration empties event cache and deduplicates request bookkeeping, not Olm/Megolm keys. Clark alone owns operational activation. No old and new pod may overlap on the store volume. Before rollout Clark must inspect only a writable snapshot clone with the reviewed inspector; prove the embedded legacy owner, MAS subject binding and homeserver device-key state without disclosing pepper/key values; obtain Julian’s specific rollout approval. Until then: no volume writes, deployment, or authenticated room read. No waiver approved.

jlxq0 added 1 commit 2026-09-24 23:31:17 +00:00

fix: key Matrix clients, stores and channel routes by (MXID, device) ... 1f95788524

Two device-bound grants for one account (HERMESHONOKA-MCP01 and
HERMESLUCY-MCP01 on @julian) were pointed at one per-MXID store and one
cached client. With the first device cached, the second token went through
restore_session on that client, which panics in matrix-sdk 0.17
(AlreadyInitializedError) and, under the release profile's panic = "abort",
exits the pod. Every same-device token refresh took the same path.

- ClientKey { mxid, device } keys the client cache and the store; channel
  sessions are keyed by (mxid, device, MAS subject). A token only reaches
  its own device's client, and the cached client always holds the token
  being presented.
- store_layout::resolve_store decides the store before any client exists:
  the legacy per-MXID store goes to the device named by the Olm account
  inside it (never .device-id); an operator binding can only name the owner
  of an account-less store and must agree otherwise; other devices get
  devices/<hash>/ with a device-bound cipher. Ambiguous, contradicted,
  unreadable or I/O-failing cases are refused.
- Any store in which matrix-sdk would mint a new Olm account (new dir, a
  leftover dir or empty database without an account, a binding-only legacy
  store) is minted only after a typed, well-formed /keys/query answer says
  the homeserver holds no keys for the device. A "yes", a malformed body,
  reported failures, or no answer all refuse.
- A token change rebuilds its own device's client on the same store after
  the previous client drains (bounded). A different MAS subject cannot use
  or replace a cached device, and /setup only sees its own subject's.
- The device-key self-heal no longer wipes anything; it reports.
- /setup asks which device when several are connected.
- matrix-mcp inspect-store for pre-rollout checks on a copy.

Migration contract and residual risks: docs/multi-device.md.

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

jlxq0 added 6 commits 2026-09-25 08:22:19 +00:00

test: open a store root written by matrix-sdk 0.17.0 through the device-scoped cache ... 8e8e8ba8c3

Adds tests/fixtures/sdk-0.17-store: a legacy per-MXID store (HONOKA) and a
per-device store (LUCY) written by matrix-sdk 0.17.0 itself, with the same
Cargo.lock as 1f95788. The legacy store holds an Olm session with a peer
device, the peer's Megolm room key received over it, HONOKA's own outbound
session, and one ciphertext from each sender. The generator is kept beside it
as generate.rs.txt.

The new test opens a *copy* through MatrixClientCache with two bound tokens
for the same MXID and checks that each device keeps its ed25519/curve25519
identity, tokens stay isolated, a refresh rebuilds only its own device, the
Megolm and Olm sessions survive and both historical messages decrypt. It also
asserts the checked-in fixture is byte-identical afterwards.

Passes on matrix-sdk 0.17 here, so an SDK upgrade is measured against what
the released SDK wrote rather than against stores it writes for itself.

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

fix(deps): matrix-sdk 0.19.1, rustls 0.23.45, MSRV 1.96 ... bb59f34b25

Clears RUSTSEC-2026-0285 (rustls 0.23.40 -> 0.23.45) and RUSTSEC-2026-0292
(imbl-sized-chunks 0.1.3 -> 0.2.0, via imbl 7 / eyeball-im 0.9, which only
matrix-sdk 0.19 takes). Also drops bitmaps (RUSTSEC-2026-0247, -2025-0167)
and proc-macro-error2 from the tree.

matrix-sdk, matrix-sdk-crypto 0.17 -> 0.19.1; ruma 0.15 -> 0.17 (the line
0.19 re-exports). matrix-sdk 0.19.1 declares rust-version 1.96, so the MSRV
goes 1.93 -> 1.96 and the Docker builder is pinned to rust:1.96.0-bookworm by
digest (the patch tag: `1.96-bookworm` floats and was already 1.96.1). The
clippy floor stays at 1.98.0 in CI.

TLS: from 0.19 matrix-sdk's reqwest 0.13 is `rustls-no-provider` unless the
new `rustls-aws-lc-rs` feature is on, and we build it without default
features. Without the feature `Client::build` panics with "No rustls crypto
provider is configured" (checked by removing it), which `panic = "abort"`
makes a pod exit. The feature restores exactly what 0.17 did implicitly.
`an_https_homeserver_client_builds` names that cause.

API changes: `ReceiptThread` is now passed by reference; tests build an
`OlmMachine` through `OlmMachineBuilder` (`with_store` is gone). The
`MismatchedAccount` refusal in `OlmMachine::from_builder` is still there.

Checked in 0.19.1 rather than assumed, and comments updated to say so:
`restore_session` on an authenticated client still panics
(authentication/matrix/mod.rs:787) and there is still no public token
setter; E2EE media is still decrypted only under `e2e-encryption` (now in
`DefaultMediaFetcher::fetch_media_content`); the LatestEvents feed still
needs `event_cache().subscribe()` (sync.rs:363); `get_media_content` still
returns a whole `Vec<u8>`.

Stores: the 0.17 store-root fixture from the previous commit opens through
the device-scoped cache with both devices' keys, both tokens isolated, the
Olm session and Megolm history intact and both old messages decrypting.
Opening migrates the schema in place (see the docs commit).

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

fix(deps): clear the remaining advisories without ignores ... 9d86baf91d

`cargo deny check` (advisories included) failed on the base commit with
eight errors. After the matrix-sdk upgrade four remained, all of them
pre-existing; this clears them rather than listing them in `ignore`:

- anyhow 1.0.102 -> 1.0.104: RUSTSEC-2026-0190 (unsound downcast_mut).
- chacha20 0.10.0 -> 0.10.2 (yanked; via vodozemac/hpke).
- spin 0.9.8 -> 0.9.9 (yanked; via axum-extra/multer).
- opus 0.3 -> 0.4: 0.4 binds libopus through `opusic-sys` instead of
  `audiopus_sys`, which is unmaintained with no safe upgrade
  (RUSTSEC-2026-0150). The public API is unchanged and
  `transcoder_produces_ogg_stream` passes. `opusic-sys`'s default `bundled`
  feature builds libopus with CMake and links it statically unconditionally,
  so `OPUS_STATIC=1` (an `audiopus_sys` switch) is removed from the
  Dockerfile and CI. A 1.96.0 release build without it has no libopus in
  `ldd` and no dynamic `opus_` symbols. The Cargo.toml comment already said
  `opusic-sys`; it was wrong for 0.3 and is right now.

Lock-only for the first three; no source change.

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

fix(store): serialise matrix-sdk opens of one store directory ... 07fb1ee78a

Opening a store written by an older matrix-sdk migrates its schema, and two
concurrent opens of one 0.17 crypto database under 0.19 both start the same
migration step; the loser fails with `duplicate column name: info` and its
device is refused as "could not read the Olm account".

That is the ordinary shape of the first requests after this upgrade:
`resolve_store` reads the legacy store's owner for *every* device of an
account, so two agents on one MXID reconnecting together open the legacy
crypto database at the same moment. Fail-closed, and a retry succeeds, but
it lands on a user's tool call. It surfaced as the 0.17-fixture test going
red only when its two devices' opens overlapped.

`stored_account` now holds a per-directory async lock
(`store_layout::open_guard`) across `SqliteCryptoStore::open`.
`concurrent_first_opens_of_a_0_17_store_all_succeed` opens a copy of the
fixture six times at once: 5/5 runs failed with the migration error before
the lock and 5/5 pass with it. The client build takes no such lock, because
it cannot race: the building device's own `resolve_store` has already
migrated the crypto database under the lock, and builds of one directory
are serialised by the per-key build lock. A build-side guard was tried and
removed as unpinnable. The reasoning is on `OPEN_LOCKS`.

Test helper: `tree_digest` skips `-shm` files and files that vanish between
listing and reading. SQLite removes `-wal`/`-shm` in the background when a
dropped store's last pooled connection closes.

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

docs: the matrix-sdk 0.19 migration contract, measured ... 7e8a29c421

Opening a 0.17-written store with 0.19.1 migrates it in place (crypto
17->19, state 15->16, event cache 14->18 and reset, media unchanged), read
from each database's `kv` version on a copy of the fixture. 0.17 then opens
the migrated copy without refusing and still decrypts pre-upgrade history,
so "the old image starts" is not evidence that rolling back is safe.

docs/multi-device.md: `inspect-store` on the 0.19 image is a write on the
copy, so use a fresh copy per run. Rollback is to the step 0 snapshot, not
the previous image on the same volume, and restoring it costs whatever the
new image wrote, including LUCY's device store, whose keys are then on the
homeserver. New section "Upgrading matrix-sdk" with the table, what was
checked to survive, the event-cache reset, the migration race fixed in the
previous commit, and why `high_entropy_passphrase` is not used. Residual
risks gain the unsupported in-place rollback and a pre-existing
shutdown-time panic in deadpool-sync, seen with both SDKs.

`inspect-store` prints that it migrates older databases and should run on a
copy; `stored_account`'s doc says the migration is now expected.
`reading_the_account_of_a_0_17_store_writes_the_crypto_database_and_nothing_else`
pins "a write, crypto database only" so the docs cannot drift back to
read-only. It does not claim to show a migration, since a WAL checkpoint
alone changes the file.

AGENTS.md: the media-decryption and restore_session bullets re-checked
against 0.19.1; build floor 1.96 with a patch-pinned builder; new pitfalls
for the `rustls-aws-lc-rs` feature, migrations and fixtures from the old
SDK, and `cargo update -p` stopping short of an advisory's fixed version.
CHANGELOG: Security entry.

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

docs: say what the 0.19 crypto migrations delete ... 119093fc4f

The migration section said nothing but the event cache is deleted. Crypto
migration 18 (UPDATE OR REPLACE into a new unique column) collapses
duplicate outgoing key/secret requests, and 19 clears the sliding-sync
position. Neither touches keys or sessions; both are deletions and the doc
now says so, with where it was read.

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

jlxq0 referenced this pull request from a commit 2026-09-25 08:43:29 +00:00

docs(setup): the store is no longer wiped when device keys are missing

jlxq0 added 3 commits 2026-09-25 08:43:29 +00:00

fix(store): durable subject binding, cross-process store lock, failing inspect exit ... 7aa5bc8c80

Three review findings on 119093f, each reproduced before the fix:

1. The MAS subject that owns an (MXID, device) lived only in the client
   cache, so after a restart a different MAS account presenting a token for
   the same device was handed its store. Reproduced by
   after_a_restart_another_subject_cannot_open_a_devices_store
   (pre-fix: unwrap_err on Ok(Client)). Each store now records its owning
   subject in `matrix-mcp-subject` (store_layout::check_subject), checked
   under the account lock on every build. The server writes it only
   immediately before minting a new account (atomic write + rename, never
   overwrites), so the recorded subject is the one that made the keys. A
   store that already holds an account and has no record, which includes
   every legacy and pre-existing per-device store, is refused for every
   subject until an operator records the owner: first-to-arrive is the
   guess the record exists to prevent. No store is wiped, moved or
   re-keyed; a refusal writes nothing. Tokens of the recorded subject,
   older or refreshed, are unaffected.

2. The per-directory lock from 07fb1ee was an in-process mutex, invisible
   to a second process on the same volume. Six concurrent `inspect-store`
   processes on one 0.17 fixture copy reproduced `Failed to run
   migrations: duplicate column name: info` through it. It is replaced by
   `flock(2)` on `<root>/<sha256(mxid)[..32]>.lock` (store_layout::
   AccountLock), taken once per account and held across resolution, the
   subject check, the mint probe, the mint and `restore_session`, so
   neither a migration nor a same-device mint can run twice at once. The
   kernel releases it when the holder exits or is killed, so it cannot go
   stale; a live holder makes waiters refuse retryably after 30 s. It does
   not cover a 0.17 pod (which does not take it) or two processes serving
   one device after both have built; that is the documented non-overlap
   precondition.

3. `inspect-store` printed REFUSED and exited 0. It now exits non-zero on
   any refusal, reports each store's subject record (never the subject),
   and takes `--subject` to check one.

tests/inspect_store.rs runs the real binary against fixture copies: exit
codes for four refusal shapes and a clean run, no pepper in output, and six
concurrent processes on one 0.17 store (3 rounds). Against 119093f it fails
4/6; with this change 6/6 pass. The checked-in fixture is only copied.

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

docs(setup): the store is no longer wiped when device keys are missing ... a90a848c1f

The comment on /setup/recover's precondition said `for_user` wipes the
per-MXID store and rebuilds when Synapse lacks the device's keys, so a
cached client proved the keys were published. Automatic wipes were removed
in #149: `check_keys` logs, counts reason="wipe_disabled" and leaves repair
to an operator. The comment now says so, including that a cached client is
therefore not proof of published keys. Comment only.

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

docs: subject records, the cross-process lock and inspect exit codes ... c411a409c5

docs/multi-device.md: new rollout steps 0a (the first upgrade must not
overlap an old pod, which does not take the lock; with the check to run)
and 1a (record the owning MAS subject for every store holding an account,
from MAS rather than from a log; verify with `inspect-store --subject`).
Step 1's expected output and exit status are updated; the "binding is in
memory only" paragraph is replaced; residual risks gain the lock's limits
(open-time only, filesystem support). AGENTS.md and CHANGELOG record all
three.

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

jlxq0 merged commit c411a409c5 into main 2026-09-25 08:46:51 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked

Held on a named, checkable blocker: an issue number, a credential or a measurement. Not "held".

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

waiting-on-julian

Blocked on Julian. The ask is in the issue.

  

wontfix

This will not be worked on

No labels

blocked

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

waiting-on-julian

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

jlxq0/matrix-mcp!149

No description provided.