mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(matrix): device_id fallback prevents E2EE init failure on fresh bot accounts
- Resolve device_id via query_keys({mxid: []}) when whoami() returns None
- Guard _verify_device_keys_on_server and _reverify_keys_after_upload
against None/unverified device_id to prevent 'device_keys values must
be a list of strings' serialization failure
- Disconnect existing client before reconnect to prevent dual OlmMachine
instances on the same crypto store
Re-targeted from #39779 (legacy gateway/platforms/matrix.py) onto the
migrated plugins/platforms/matrix/adapter.py path following the
2026-06-20 adapter migration. Logic unchanged from original fix.
242 tests passing (233 upstream + 9 new).
This commit is contained in:
parent
2d8d08cae6
commit
9048457eab
2 changed files with 440 additions and 0 deletions
|
|
@ -806,6 +806,7 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
self._device_id: str = config.extra.get("device_id", "") or os.getenv(
|
||||
"MATRIX_DEVICE_ID", ""
|
||||
)
|
||||
self._device_id_unverified: bool = False
|
||||
|
||||
self._client: Any = None # mautrix.client.Client
|
||||
self._crypto_db: Any = None # mautrix.util.async_db.Database
|
||||
|
|
@ -1039,6 +1040,12 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
self, client: Any, local_ed25519: str
|
||||
) -> bool:
|
||||
"""Re-query the server after share_keys() and verify our ed25519 key matches."""
|
||||
if not client.device_id or self._device_id_unverified:
|
||||
logger.warning(
|
||||
"Matrix: skipping post-upload key verification — "
|
||||
"device_id not yet established"
|
||||
)
|
||||
return True
|
||||
try:
|
||||
resp = await client.query_keys({client.mxid: [client.device_id]})
|
||||
dk = getattr(resp, "device_keys", {}) or {}
|
||||
|
|
@ -1065,6 +1072,12 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
Returns True if keys are valid or were successfully re-uploaded.
|
||||
Returns False if verification fails (caller should refuse E2EE).
|
||||
"""
|
||||
if not client.device_id or self._device_id_unverified:
|
||||
logger.warning(
|
||||
"Matrix: skipping device key verification — "
|
||||
"device_id not yet established"
|
||||
)
|
||||
return True
|
||||
try:
|
||||
resp = await client.query_keys({client.mxid: [client.device_id]})
|
||||
except Exception as exc:
|
||||
|
|
@ -1139,6 +1152,12 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
"""Connect to the Matrix homeserver and start syncing."""
|
||||
if self._client is not None:
|
||||
try:
|
||||
await self.disconnect()
|
||||
except Exception as exc:
|
||||
logger.warning("Matrix: error disconnecting before reconnect: %s", exc)
|
||||
|
||||
from mautrix.api import HTTPAPI
|
||||
from mautrix.client import Client
|
||||
from mautrix.client.state_store import MemoryStateStore, MemorySyncStore
|
||||
|
|
@ -1189,6 +1208,36 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
if effective_device_id:
|
||||
client.device_id = effective_device_id
|
||||
|
||||
if not client.device_id:
|
||||
try:
|
||||
dev_resp = await client.query_keys({client.mxid: []})
|
||||
all_devices = (
|
||||
(getattr(dev_resp, "device_keys", {}) or {})
|
||||
.get(str(client.mxid)) or {}
|
||||
)
|
||||
if len(all_devices) == 1:
|
||||
client.device_id = next(iter(all_devices))
|
||||
elif len(all_devices) == 0:
|
||||
logger.warning(
|
||||
"Matrix: no devices found for %s — "
|
||||
"key verification will be skipped",
|
||||
client.mxid,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"Matrix: device list query failed: %s", exc
|
||||
)
|
||||
|
||||
if not client.device_id:
|
||||
logger.warning(
|
||||
"Matrix: device_id could not be resolved for %s. "
|
||||
"Set MATRIX_DEVICE_ID for full key verification. "
|
||||
"E2EE will proceed without server-side device "
|
||||
"key confirmation.",
|
||||
client.mxid,
|
||||
)
|
||||
self._device_id_unverified = True
|
||||
|
||||
logger.info(
|
||||
"Matrix: using access token for %s%s",
|
||||
self._user_id or "(unknown user)",
|
||||
|
|
|
|||
|
|
@ -4740,3 +4740,394 @@ class TestMatrixDeadInviteHandling:
|
|||
|
||||
result = await self.adapter._join_room_by_id("!brokenleave:example.org")
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Device ID resolution when whoami returns None
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestDeviceIdNoneResolution:
|
||||
"""connect() should resolve device_id when whoami returns None."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_none_device_id_resolved_via_query_keys(self):
|
||||
"""query_keys({mxid: []}) with exactly one device should adopt that ID."""
|
||||
from plugins.platforms.matrix.adapter import MatrixAdapter
|
||||
|
||||
config = PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_access_token",
|
||||
extra={
|
||||
"homeserver": "https://matrix.example.org",
|
||||
"user_id": "@bot:example.org",
|
||||
"encryption": True,
|
||||
},
|
||||
)
|
||||
adapter = MatrixAdapter(config)
|
||||
|
||||
fake_mautrix_mods = _make_fake_mautrix()
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.device_id = None
|
||||
mock_client.state_store = MagicMock()
|
||||
mock_client.sync_store = MagicMock()
|
||||
mock_client.crypto = None
|
||||
mock_client.whoami = AsyncMock(return_value=MagicMock(
|
||||
user_id="@bot:example.org", device_id=None,
|
||||
))
|
||||
|
||||
resolve_resp = MagicMock()
|
||||
resolve_dev = MagicMock()
|
||||
resolve_dev.keys = {"ed25519:RESOLVED_DEV": "fake_ed25519_key"}
|
||||
resolve_resp.device_keys = {"@bot:example.org": {"RESOLVED_DEV": resolve_dev}}
|
||||
|
||||
verify_resp = MagicMock()
|
||||
verify_dev = MagicMock()
|
||||
verify_dev.keys = {"ed25519:RESOLVED_DEV": "fake_ed25519_key"}
|
||||
verify_resp.device_keys = {"@bot:example.org": {"RESOLVED_DEV": verify_dev}}
|
||||
mock_client.query_keys = AsyncMock(side_effect=[resolve_resp, verify_resp])
|
||||
|
||||
mock_client.sync = AsyncMock(return_value={"rooms": {"join": {"!room:server": {}}}})
|
||||
mock_client.add_event_handler = MagicMock()
|
||||
mock_client.handle_sync = MagicMock(return_value=[])
|
||||
mock_client.api = MagicMock()
|
||||
mock_client.api.token = "syt_test_access_token"
|
||||
mock_client.api.session = MagicMock()
|
||||
mock_client.api.session.close = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.load = AsyncMock()
|
||||
mock_olm.share_keys = AsyncMock()
|
||||
mock_olm.share_keys_min_trust = None
|
||||
mock_olm.send_keys_min_trust = None
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_ed25519_key"}
|
||||
|
||||
fake_mautrix_mods["mautrix.client"].Client = MagicMock(return_value=mock_client)
|
||||
fake_mautrix_mods["mautrix.crypto"].OlmMachine = MagicMock(return_value=mock_olm)
|
||||
|
||||
import plugins.platforms.matrix.adapter as matrix_mod
|
||||
with patch.object(matrix_mod, "_check_e2ee_deps", return_value=True):
|
||||
with patch.dict("sys.modules", fake_mautrix_mods):
|
||||
with patch.object(adapter, "_refresh_dm_cache", AsyncMock()):
|
||||
with patch.object(adapter, "_sync_loop", AsyncMock(return_value=None)):
|
||||
result = await adapter.connect()
|
||||
|
||||
assert result is True
|
||||
assert adapter._device_id_unverified is False
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_none_device_id_sets_unverified_flag_when_no_devices(self):
|
||||
"""query_keys returns zero devices → _device_id_unverified = True."""
|
||||
from plugins.platforms.matrix.adapter import MatrixAdapter
|
||||
|
||||
config = PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_access_token",
|
||||
extra={
|
||||
"homeserver": "https://matrix.example.org",
|
||||
"user_id": "@bot:example.org",
|
||||
"encryption": True,
|
||||
},
|
||||
)
|
||||
adapter = MatrixAdapter(config)
|
||||
|
||||
fake_mautrix_mods = _make_fake_mautrix()
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.device_id = None
|
||||
mock_client.state_store = MagicMock()
|
||||
mock_client.sync_store = MagicMock()
|
||||
mock_client.crypto = None
|
||||
mock_client.whoami = AsyncMock(return_value=MagicMock(
|
||||
user_id="@bot:example.org", device_id=None,
|
||||
))
|
||||
|
||||
resolve_resp = MagicMock()
|
||||
resolve_resp.device_keys = {"@bot:example.org": {}}
|
||||
mock_client.query_keys = AsyncMock(return_value=resolve_resp)
|
||||
|
||||
mock_client.sync = AsyncMock(return_value={"rooms": {"join": {"!room:server": {}}}})
|
||||
mock_client.add_event_handler = MagicMock()
|
||||
mock_client.handle_sync = MagicMock(return_value=[])
|
||||
mock_client.api = MagicMock()
|
||||
mock_client.api.token = "syt_test_access_token"
|
||||
mock_client.api.session = MagicMock()
|
||||
mock_client.api.session.close = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.load = AsyncMock()
|
||||
mock_olm.share_keys = AsyncMock()
|
||||
mock_olm.share_keys_min_trust = None
|
||||
mock_olm.send_keys_min_trust = None
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_ed25519_key"}
|
||||
|
||||
fake_mautrix_mods["mautrix.client"].Client = MagicMock(return_value=mock_client)
|
||||
fake_mautrix_mods["mautrix.crypto"].OlmMachine = MagicMock(return_value=mock_olm)
|
||||
|
||||
import plugins.platforms.matrix.adapter as matrix_mod
|
||||
with patch.object(matrix_mod, "_check_e2ee_deps", return_value=True):
|
||||
with patch.dict("sys.modules", fake_mautrix_mods):
|
||||
with patch.object(adapter, "_refresh_dm_cache", AsyncMock()):
|
||||
with patch.object(adapter, "_sync_loop", AsyncMock(return_value=None)):
|
||||
await adapter.connect()
|
||||
|
||||
assert adapter._device_id_unverified is True
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_none_device_id_sets_unverified_flag_when_multiple_devices(self):
|
||||
"""query_keys returns multiple devices → _device_id_unverified = True."""
|
||||
from plugins.platforms.matrix.adapter import MatrixAdapter
|
||||
|
||||
config = PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_access_token",
|
||||
extra={
|
||||
"homeserver": "https://matrix.example.org",
|
||||
"user_id": "@bot:example.org",
|
||||
"encryption": True,
|
||||
},
|
||||
)
|
||||
adapter = MatrixAdapter(config)
|
||||
|
||||
fake_mautrix_mods = _make_fake_mautrix()
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.device_id = None
|
||||
mock_client.state_store = MagicMock()
|
||||
mock_client.sync_store = MagicMock()
|
||||
mock_client.crypto = None
|
||||
mock_client.whoami = AsyncMock(return_value=MagicMock(
|
||||
user_id="@bot:example.org", device_id=None,
|
||||
))
|
||||
|
||||
resolve_resp = MagicMock()
|
||||
resolve_resp.device_keys = {"@bot:example.org": {
|
||||
"DEV_A": {"keys": {"ed25519:DEV_A": "key_a"}},
|
||||
"DEV_B": {"keys": {"ed25519:DEV_B": "key_b"}},
|
||||
}}
|
||||
mock_client.query_keys = AsyncMock(return_value=resolve_resp)
|
||||
|
||||
mock_client.sync = AsyncMock(return_value={"rooms": {"join": {"!room:server": {}}}})
|
||||
mock_client.add_event_handler = MagicMock()
|
||||
mock_client.handle_sync = MagicMock(return_value=[])
|
||||
mock_client.api = MagicMock()
|
||||
mock_client.api.token = "syt_test_access_token"
|
||||
mock_client.api.session = MagicMock()
|
||||
mock_client.api.session.close = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.load = AsyncMock()
|
||||
mock_olm.share_keys = AsyncMock()
|
||||
mock_olm.share_keys_min_trust = None
|
||||
mock_olm.send_keys_min_trust = None
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_ed25519_key"}
|
||||
|
||||
fake_mautrix_mods["mautrix.client"].Client = MagicMock(return_value=mock_client)
|
||||
fake_mautrix_mods["mautrix.crypto"].OlmMachine = MagicMock(return_value=mock_olm)
|
||||
|
||||
import plugins.platforms.matrix.adapter as matrix_mod
|
||||
with patch.object(matrix_mod, "_check_e2ee_deps", return_value=True):
|
||||
with patch.dict("sys.modules", fake_mautrix_mods):
|
||||
with patch.object(adapter, "_refresh_dm_cache", AsyncMock()):
|
||||
with patch.object(adapter, "_sync_loop", AsyncMock(return_value=None)):
|
||||
await adapter.connect()
|
||||
|
||||
assert adapter._device_id_unverified is True
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_none_device_id_sets_unverified_flag_when_query_keys_raises(self):
|
||||
"""query_keys raising an exception should not propagate — set flag."""
|
||||
from plugins.platforms.matrix.adapter import MatrixAdapter
|
||||
|
||||
config = PlatformConfig(
|
||||
enabled=True,
|
||||
token="syt_test_access_token",
|
||||
extra={
|
||||
"homeserver": "https://matrix.example.org",
|
||||
"user_id": "@bot:example.org",
|
||||
"encryption": True,
|
||||
},
|
||||
)
|
||||
adapter = MatrixAdapter(config)
|
||||
|
||||
fake_mautrix_mods = _make_fake_mautrix()
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.device_id = None
|
||||
mock_client.state_store = MagicMock()
|
||||
mock_client.sync_store = MagicMock()
|
||||
mock_client.crypto = None
|
||||
mock_client.whoami = AsyncMock(return_value=MagicMock(
|
||||
user_id="@bot:example.org", device_id=None,
|
||||
))
|
||||
|
||||
mock_client.query_keys = AsyncMock(side_effect=Exception("server unavailable"))
|
||||
|
||||
mock_client.sync = AsyncMock(return_value={"rooms": {"join": {"!room:server": {}}}})
|
||||
mock_client.add_event_handler = MagicMock()
|
||||
mock_client.handle_sync = MagicMock(return_value=[])
|
||||
mock_client.api = MagicMock()
|
||||
mock_client.api.token = "syt_test_access_token"
|
||||
mock_client.api.session = MagicMock()
|
||||
mock_client.api.session.close = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.load = AsyncMock()
|
||||
mock_olm.share_keys = AsyncMock()
|
||||
mock_olm.share_keys_min_trust = None
|
||||
mock_olm.send_keys_min_trust = None
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_ed25519_key"}
|
||||
|
||||
fake_mautrix_mods["mautrix.client"].Client = MagicMock(return_value=mock_client)
|
||||
fake_mautrix_mods["mautrix.crypto"].OlmMachine = MagicMock(return_value=mock_olm)
|
||||
|
||||
import plugins.platforms.matrix.adapter as matrix_mod
|
||||
with patch.object(matrix_mod, "_check_e2ee_deps", return_value=True):
|
||||
with patch.dict("sys.modules", fake_mautrix_mods):
|
||||
with patch.object(adapter, "_refresh_dm_cache", AsyncMock()):
|
||||
with patch.object(adapter, "_sync_loop", AsyncMock(return_value=None)):
|
||||
try:
|
||||
await adapter.connect()
|
||||
except Exception:
|
||||
pytest.fail("connect() raised — exception should be caught")
|
||||
|
||||
assert adapter._device_id_unverified is True
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
class TestVerifyDeviceKeysGuards:
|
||||
"""_verify_device_keys_on_server and _reverify_keys_after_upload guards."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_skips_when_device_id_unverified_flag_set(self):
|
||||
adapter = _make_adapter()
|
||||
adapter._device_id_unverified = True
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.device_id = "SOME_DEVICE"
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.query_keys = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_key"}
|
||||
|
||||
result = await adapter._verify_device_keys_on_server(mock_client, mock_olm)
|
||||
|
||||
assert result is True
|
||||
mock_client.query_keys.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_skips_when_client_device_id_is_none(self):
|
||||
adapter = _make_adapter()
|
||||
adapter._device_id_unverified = False
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.device_id = None
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.query_keys = AsyncMock()
|
||||
|
||||
mock_olm = MagicMock()
|
||||
mock_olm.account = MagicMock()
|
||||
mock_olm.account.identity_keys = {"ed25519": "fake_key"}
|
||||
|
||||
result = await adapter._verify_device_keys_on_server(mock_client, mock_olm)
|
||||
|
||||
assert result is True
|
||||
mock_client.query_keys.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reverify_skips_when_device_id_unverified_flag_set(self):
|
||||
adapter = _make_adapter()
|
||||
adapter._device_id_unverified = True
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.device_id = "SOME_DEVICE"
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.query_keys = AsyncMock()
|
||||
|
||||
result = await adapter._reverify_keys_after_upload(mock_client, "fake_ed25519")
|
||||
|
||||
assert result is True
|
||||
mock_client.query_keys.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reverify_skips_when_client_device_id_is_none(self):
|
||||
adapter = _make_adapter()
|
||||
adapter._device_id_unverified = False
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.device_id = None
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.query_keys = AsyncMock()
|
||||
|
||||
result = await adapter._reverify_keys_after_upload(mock_client, "fake_ed25519")
|
||||
|
||||
assert result is True
|
||||
mock_client.query_keys.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Reconnect-disconnect guard
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestMatrixReconnectDisconnect:
|
||||
"""connect() must disconnect existing client before reconnecting."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_calls_disconnect_when_client_already_set(self):
|
||||
"""When self._client is set, connect() should call disconnect() first."""
|
||||
adapter = _make_adapter()
|
||||
|
||||
adapter._client = MagicMock()
|
||||
adapter._client.api = MagicMock()
|
||||
adapter._client.api.session = MagicMock()
|
||||
adapter._client.api.session.close = AsyncMock()
|
||||
adapter._client.whoami = AsyncMock()
|
||||
|
||||
adapter.disconnect = AsyncMock()
|
||||
|
||||
fake_mautrix_mods = _make_fake_mautrix()
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.mxid = "@bot:example.org"
|
||||
mock_client.device_id = None
|
||||
mock_client.state_store = MagicMock()
|
||||
mock_client.sync_store = MagicMock()
|
||||
mock_client.crypto = None
|
||||
mock_client.whoami = AsyncMock(return_value=MagicMock(
|
||||
user_id="@bot:example.org", device_id="NEW_DEV",
|
||||
))
|
||||
mock_client.query_keys = AsyncMock()
|
||||
mock_client.sync = AsyncMock(return_value={"rooms": {"join": {"!room:server": {}}}})
|
||||
mock_client.add_event_handler = MagicMock()
|
||||
mock_client.handle_sync = MagicMock(return_value=[])
|
||||
mock_client.api = MagicMock()
|
||||
mock_client.api.token = "syt_test_access_token"
|
||||
mock_client.api.session = MagicMock()
|
||||
mock_client.api.session.close = AsyncMock()
|
||||
|
||||
fake_mautrix_mods["mautrix.client"].Client = MagicMock(return_value=mock_client)
|
||||
|
||||
import plugins.platforms.matrix.adapter as matrix_mod
|
||||
with patch.dict("sys.modules", fake_mautrix_mods):
|
||||
with patch.object(adapter, "_refresh_dm_cache", AsyncMock()):
|
||||
with patch.object(adapter, "_sync_loop", AsyncMock(return_value=None)):
|
||||
await adapter.connect()
|
||||
|
||||
adapter.disconnect.assert_awaited_once()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue