fix(matrix): pin the E2EE crypto store per profile at connect(), not import
The multiplex gateway imports plugins/platforms/matrix/adapter.py once, so
the module-level _STORE_DIR/_CRYPTO_DB_PATH resolved against the root
HERMES_HOME for every profile: all bots' Olm identities landed in one
crypto.db and inbound E2EE failed with "no session found" (#89168).
connect() runs inside _profile_runtime_scope, so resolve the store dir
there via get_hermes_dir (honors the context-local HERMES_HOME) and cache
it on the instance -- diagnostics and error-log paths read outside the
scope then still report the store actually in use. Mirrors the
pairing-store fix (a6397c379).
Salvage of #89169 (per-call resolvers collapsed into one cached resolve;
dead `_CRYPTO_DB_PATH = None` alias dropped -- no external importers).
Also routes the last raw MATRIX_HOMESERVER read in check_matrix_requirements
through _startup_env_secret like its token/password neighbours (#69943).
Fixes #89168
Co-authored-by: Michael Short <18595461+mjshorty@users.noreply.github.com>
This commit is contained in:
@@ -594,13 +594,14 @@ def _resolve_max_message_length(config) -> int:
|
||||
# Back-compat alias for callers/tests that import the module constant.
|
||||
MAX_MESSAGE_LENGTH = DEFAULT_MAX_MESSAGE_LENGTH
|
||||
|
||||
# Store directory for E2EE keys and sync state.
|
||||
# Uses get_hermes_home() so each profile gets its own Matrix store.
|
||||
# Store directory for E2EE keys and sync state. Resolved per adapter in
|
||||
# ``connect()`` (see ``_resolve_store_dir``), NOT at module scope: the
|
||||
# multiplex gateway imports this module once, so a module-level constant
|
||||
# would pin the root HERMES_HOME for every profile and all bots' Olm
|
||||
# identities would collide in one crypto.db (#89168). Mirrors the
|
||||
# pairing-store fix (a6397c379).
|
||||
from hermes_constants import get_hermes_dir as _get_hermes_dir
|
||||
|
||||
_STORE_DIR = _get_hermes_dir("platforms/matrix/store", "matrix/store")
|
||||
_CRYPTO_DB_PATH = _STORE_DIR / "crypto.db"
|
||||
|
||||
# Grace period: ignore messages older than this many seconds before startup.
|
||||
_STARTUP_GRACE_SECONDS = 5
|
||||
|
||||
@@ -1019,7 +1020,7 @@ def check_matrix_requirements() -> bool:
|
||||
"""
|
||||
token = _startup_env_secret("MATRIX_ACCESS_TOKEN")
|
||||
password = _startup_env_secret("MATRIX_PASSWORD")
|
||||
homeserver = os.getenv("MATRIX_HOMESERVER", "")
|
||||
homeserver = _startup_env_secret("MATRIX_HOMESERVER")
|
||||
|
||||
if not token and not password:
|
||||
logger.debug("Matrix: neither MATRIX_ACCESS_TOKEN nor MATRIX_PASSWORD set")
|
||||
@@ -1188,6 +1189,23 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
max_message_length = DEFAULT_MAX_MESSAGE_LENGTH
|
||||
_split_threshold = DEFAULT_MAX_MESSAGE_LENGTH - 100
|
||||
|
||||
def _resolve_store_dir(self) -> Path:
|
||||
"""Pin this adapter's crypto-store directory to the active profile.
|
||||
|
||||
Called from ``connect()``, which the multiplex gateway runs inside
|
||||
``_profile_runtime_scope`` -- ``get_hermes_dir`` honors that
|
||||
context-local HERMES_HOME, so each profile's adapter gets its own
|
||||
store. Cached on the instance so later reads (diagnostics, error
|
||||
logs) outside the scope still report the store actually in use.
|
||||
"""
|
||||
self._store_dir = _get_hermes_dir("platforms/matrix/store", "matrix/store")
|
||||
return self._store_dir
|
||||
|
||||
@property
|
||||
def _crypto_db_path(self) -> Path:
|
||||
store_dir = self._store_dir or _get_hermes_dir("platforms/matrix/store", "matrix/store")
|
||||
return store_dir / "crypto.db"
|
||||
|
||||
def __init__(self, config: PlatformConfig):
|
||||
super().__init__(config, Platform.MATRIX)
|
||||
|
||||
@@ -1218,6 +1236,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
|
||||
self._client: Any = None # mautrix.client.Client
|
||||
self._crypto_db: Any = None # mautrix.util.async_db.Database
|
||||
self._store_dir: Optional[Path] = None # pinned per profile in connect()
|
||||
self._sync_task: Optional[asyncio.Task] = None
|
||||
self._invite_join_tasks: Dict[str, asyncio.Task] = {}
|
||||
self._closing = False
|
||||
@@ -1673,7 +1692,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
"Matrix: server has different identity keys for device %s — "
|
||||
"local crypto state is stale. Delete %s and restart.",
|
||||
client.device_id,
|
||||
_CRYPTO_DB_PATH,
|
||||
str(self._crypto_db_path),
|
||||
)
|
||||
return False
|
||||
|
||||
@@ -1729,8 +1748,9 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
logger.error("Matrix: homeserver URL not configured")
|
||||
return False
|
||||
|
||||
# Ensure store dir exists for E2EE key persistence.
|
||||
_STORE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
# Ensure store dir exists for E2EE key persistence (resolved here,
|
||||
# inside the profile scope, so multiplexed profiles never share it).
|
||||
self._resolve_store_dir().mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Create the HTTP API layer.
|
||||
client_session = _create_matrix_session(self._proxy_url)
|
||||
@@ -1887,7 +1907,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
from mautrix.crypto.store.asyncpg import PgCryptoStore
|
||||
from mautrix.util.async_db import Database
|
||||
|
||||
_STORE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
self._store_dir.mkdir(parents=True, exist_ok=True)
|
||||
except Exception as exc:
|
||||
if self._e2ee_mode == "optional":
|
||||
logger.warning(
|
||||
@@ -1908,7 +1928,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
if self._encryption:
|
||||
try:
|
||||
# Remove legacy pickle file from pre-SQLite era.
|
||||
legacy_pickle = _STORE_DIR / "crypto_store.pickle"
|
||||
legacy_pickle = self._store_dir / "crypto_store.pickle"
|
||||
if legacy_pickle.exists():
|
||||
logger.info(
|
||||
"Matrix: removing legacy crypto_store.pickle (migrated to SQLite)"
|
||||
@@ -1916,7 +1936,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
legacy_pickle.unlink()
|
||||
|
||||
crypto_db = Database.create(
|
||||
f"sqlite:///{_CRYPTO_DB_PATH}",
|
||||
f"sqlite:///{self._crypto_db_path}",
|
||||
upgrade_table=PgCryptoStore.upgrade_table,
|
||||
)
|
||||
await crypto_db.start()
|
||||
@@ -2044,7 +2064,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
client.crypto = olm
|
||||
logger.info(
|
||||
"Matrix: E2EE enabled (store: %s%s)",
|
||||
str(_CRYPTO_DB_PATH),
|
||||
str(self._crypto_db_path),
|
||||
f", device_id={client.device_id}" if client.device_id else "",
|
||||
)
|
||||
except Exception as exc:
|
||||
@@ -2288,7 +2308,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
||||
"mode": self._e2ee_mode,
|
||||
"enabled": bool(self._encryption),
|
||||
"deps_available": _check_e2ee_deps(),
|
||||
"crypto_store_path": str(_CRYPTO_DB_PATH),
|
||||
"crypto_store_path": str(self._crypto_db_path),
|
||||
"recovery_key_configured": bool(
|
||||
_scoped_recovery_key().strip()
|
||||
),
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
"""Matrix crypto store must be pinned per profile at connect(), not at import.
|
||||
|
||||
Under ``gateway.multiplex_profiles`` one process imports
|
||||
``plugins.platforms.matrix.adapter`` once; the old module-level
|
||||
``_STORE_DIR``/``_CRYPTO_DB_PATH`` resolved against the root HERMES_HOME at
|
||||
import time, so every profile's adapter opened the SAME crypto.db and inbound
|
||||
E2EE failed with "no session found" (#89168). ``connect()`` calls
|
||||
``_resolve_store_dir()`` inside ``_profile_runtime_scope`` (context-local
|
||||
HERMES_HOME), so resolving there -- and caching on the instance -- gives each
|
||||
profile its own store. Exercised via ``_resolve_store_dir`` directly so the
|
||||
test needs no mautrix install.
|
||||
"""
|
||||
from gateway.config import PlatformConfig
|
||||
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||
from plugins.platforms.matrix import adapter as matrix_adapter
|
||||
|
||||
|
||||
def _make_adapter() -> matrix_adapter.MatrixAdapter:
|
||||
return matrix_adapter.MatrixAdapter(
|
||||
PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_token",
|
||||
extra={"homeserver": "https://matrix.example.org", "user_id": "@bot:example.org"},
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def test_store_dir_pinned_to_each_profile_home(tmp_path):
|
||||
"""Two profiles resolving in one process get two stores, and each
|
||||
adapter keeps reporting its own store after the scope is gone."""
|
||||
stores = {}
|
||||
for profile in ("accountant", "engineering-lead"):
|
||||
home = tmp_path / "profiles" / profile
|
||||
home.mkdir(parents=True)
|
||||
adapter = _make_adapter()
|
||||
token = set_hermes_home_override(str(home))
|
||||
try:
|
||||
adapter._resolve_store_dir().mkdir(parents=True, exist_ok=True)
|
||||
finally:
|
||||
reset_hermes_home_override(token)
|
||||
# Cached on the instance: correct even when read outside the scope.
|
||||
path = adapter.get_diagnostics()["e2ee"]["crypto_store_path"]
|
||||
assert path.startswith(str(home)), f"store not profile-scoped: {path}"
|
||||
assert adapter._store_dir.is_dir()
|
||||
stores[profile] = path
|
||||
|
||||
assert stores["accountant"] != stores["engineering-lead"]
|
||||
Reference in New Issue
Block a user