Merge branch 'main' into fix/key-backup-honest-count

Author: CybotTM

skills/matrix-communication/references/setup-guide.md

@@ -166,6 +166,43 @@ uv run skills/matrix-communication/scripts/matrix-key-backup.py --recovery-key "
 
 ## Troubleshooting
 
+**A token you found somewhere is valid — that does not make it yours.**
+
+When the configured token stops working, the tempting next step is the token
+lying in a secrets file, an env var, or another tool's config. It authenticates,
+`whoami` answers, calls succeed. It can still be the wrong credential, because a
+token carries a `device_id` and that device may be a human's running client.
+
+Before adopting any credential you did not create here:
+
+```bash
+HS=$(python3 -c "import json,os;print(json.load(open(os.path.expanduser('~/.config/matrix/config.json')))['homeserver'])")
+curl -s -H "Authorization: Bearer $TOKEN" "$HS/_matrix/client/v3/account/whoami"
+curl -s -H "Authorization: Bearer $TOKEN" "$HS/_matrix/client/v3/devices" \
+  | python3 -c "import json,sys;[print(d['device_id'], '-', d.get('display_name')) for d in json.load(sys.stdin)['devices']]"
+```
+
+If the `device_id` belongs to a device whose display name reads like a human
+client — "Element Desktop: Windows", "Element X Android", "FluffyChat android" —
+stop. That is someone's session, not an agent credential.
+
+What happens if you use it anyway: the nio store creates its own olm account for
+that device id, so one device now has two crypto identities. The client that
+owns it starts failing to decrypt messages, including its own, and SAS
+verification against the account's other devices fails with "expected key did
+not match" — even after switching to a proper device, because the damage is on
+the server-side device keys.
+
+The only sanctioned path to an agent credential is `matrix-e2ee-setup.py`, which
+logs in fresh and gets a device of its own.
+
+**Recovery, if a foreign token was already used with the E2EE scripts:** treat
+that device's crypto identity as spent. Log the agent device out
+(`matrix-e2ee-setup.py --logout`, which touches only that device's files), set
+up a fresh one, re-import the room keys, and verify again. The human client
+whose device was hijacked needs a logout and a fresh login of its own; its local
+session state cannot be repaired from this side.
+
 **E2EE setup fails with "Invalid username or password":**
 
 If your password contains special characters (`!`, `$`, `\`, etc.), bash may mangle them:

skills/matrix-communication/scripts/_lib/__init__.py

@@ -27,6 +27,7 @@
     load_credentials,
     restore_login_checked,
     save_credentials,
+    store_files_for,
 )
 
 # Formatting
@@ -88,5 +89,6 @@
     "save_credentials",
     # Formatting
     "shorten_service_urls",
+    "store_files_for",
     "suppress_nio_logging",
 ]

skills/matrix-communication/scripts/_lib/e2ee.py

@@ -114,15 +114,52 @@ def restore_login_checked(client, user_id: str, device_id: str, access_token: st
         raise SystemExit(f"Error: {hint}") from exc
 
 
-def delete_credentials():
-    """Remove stored device credentials and key store files."""
+def store_files_for(user_id: str, device_id: str) -> list[Path]:
+    """Every store file belonging to one device.
+
+    nio names them ``{user_id}_{device_id}.<suffix>`` - the database plus the
+    blacklisted/ignored/trusted device lists. The trailing dot matters: without
+    it a device id that is a prefix of another would collect the other's files
+    too.
+    """
+    prefix = f"{user_id}_{device_id}."
+    return sorted(p for p in get_store_path().iterdir() if p.name.startswith(prefix))
+
+
+def delete_credentials(purge_all: bool = False) -> list[str]:
+    """Remove the stored credentials and the store files of THAT device.
+
+    Returns the names of the files removed, so the caller can say what it did.
+
+    The store directory is shared by every device ever set up here. Globbing
+    ``*.db`` and ``*_devices`` across it - which this did - means logging one
+    device out destroys the megolm history of all the others. That happened:
+    a logout of a broken device took a months-old 25 MB store with it, and only
+    a server-side key backup made the rooms readable again.
+
+    ``purge_all`` restores the old sweep for the case where you do want the
+    directory emptied. ``backup_key.json`` is never touched either way; it is
+    not device-scoped and re-importing keys depends on it.
+    """
+    removed: list[str] = []
+    creds = load_credentials()
     creds_path = get_credentials_path()
+
+    if purge_all:
+        store_path = get_store_path()
+        targets = sorted([*store_path.glob("*.db"), *store_path.glob("*_devices")])
+    elif creds and creds.get("user_id") and creds.get("device_id"):
+        targets = store_files_for(creds["user_id"], creds["device_id"])
+    else:
+        # No credentials to scope by. Removing nothing beats removing everything.
+        targets = []
+
+    for path in targets:
+        path.unlink()
+        removed.append(path.name)
+
     if creds_path.exists():
         creds_path.unlink()
+        removed.append(creds_path.name)
 
-    # Also remove key store databases
-    store_path = get_store_path()
-    for db_file in store_path.glob("*.db"):
-        db_file.unlink()
-    for key_file in store_path.glob("*_devices"):
-        key_file.unlink()
+    return removed

skills/matrix-communication/scripts/_lib/test_e2ee.py

@@ -1,4 +1,4 @@
-"""Tests for `_lib.e2ee` store-error diagnosis.
+"""Tests for `_lib.e2ee`: store-error diagnosis and scoped credential deletion.
 
 The skill directory contains a hyphen (`matrix-communication`) so it is not
 importable as a package; run the file directly or use unittest discovery:
@@ -16,13 +16,23 @@
 package and breaks `urllib` on the way in.
 """
 
+import json
 import os
+import pathlib
+import shutil
 import sys
+import tempfile
 import unittest
 
 sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
 
-from e2ee import explain_store_error, restore_login_checked
+import e2ee
+from e2ee import (
+    delete_credentials,
+    explain_store_error,
+    restore_login_checked,
+    store_files_for,
+)
 
 
 class OlmAccountError(Exception):
@@ -86,5 +96,78 @@ def test_unrelated_error_is_reraised_untouched(self):
             restore_login_checked(client, "@u:example.org", "DEVICE", "syt_token")
 
 
+class StoreScopingTests(unittest.TestCase):
+    """--logout must take one device's files and leave every other device alone.
+
+    Regression for #81: the old code globbed `*.db` and `*_devices` across the
+    shared store directory, so logging one device out destroyed the megolm
+    history of all of them.
+    """
+
+    USER = "@user:example.org"
+    MINE = "DEVICEAAAA"
+    OTHER = "DEVICEBBBB"
+
+    def setUp(self):
+        self.store = pathlib.Path(tempfile.mkdtemp())
+        self.addCleanup(shutil.rmtree, self.store, True)
+        real = e2ee.get_store_path
+        e2ee.get_store_path = lambda: self.store
+        self.addCleanup(setattr, e2ee, "get_store_path", real)
+
+        for device in (self.MINE, self.OTHER):
+            for suffix in (
+                "db",
+                "blacklisted_devices",
+                "ignored_devices",
+                "trusted_devices",
+            ):
+                (self.store / f"{self.USER}_{device}.{suffix}").write_text("x")
+
+        # Not device-scoped, and the key import depends on it.
+        (self.store / "backup_key.json").write_text("{}")
+        (self.store / "credentials.json").write_text(
+            json.dumps({"user_id": self.USER, "device_id": self.MINE})
+        )
+
+    def _names(self):
+        return sorted(p.name for p in self.store.iterdir())
+
+    def test_store_files_for_selects_one_device(self):
+        names = sorted(p.name for p in store_files_for(self.USER, self.MINE))
+        self.assertEqual(len(names), 4)
+        self.assertTrue(all(self.MINE in n for n in names))
+
+    def test_store_files_for_does_not_match_a_prefix_device_id(self):
+        """A device id that is a prefix of another must not collect its files."""
+        (self.store / f"{self.USER}_{self.MINE}EXTRA.db").write_text("x")
+        names = [p.name for p in store_files_for(self.USER, self.MINE)]
+        self.assertNotIn(f"{self.USER}_{self.MINE}EXTRA.db", names)
+
+    def test_logout_removes_only_this_device(self):
+        removed = delete_credentials()
+
+        self.assertIn("credentials.json", removed)
+        self.assertEqual(len([n for n in removed if self.MINE in n]), 4)
+
+        left = self._names()
+        self.assertEqual(len([n for n in left if self.OTHER in n]), 4)
+        self.assertIn("backup_key.json", left)
+        self.assertNotIn("credentials.json", left)
+
+    def test_purge_all_removes_every_device(self):
+        delete_credentials(purge_all=True)
+        left = self._names()
+        self.assertEqual([n for n in left if n.endswith("_devices")], [])
+        self.assertEqual([n for n in left if n.endswith(".db")], [])
+        self.assertIn("backup_key.json", left)
+
+    def test_without_credentials_nothing_is_removed(self):
+        """No credentials means no device to scope by - deleting nothing is right."""
+        (self.store / "credentials.json").unlink()
+        self.assertEqual(delete_credentials(), [])
+        self.assertEqual(len(self._names()), 9)
+
+
 if __name__ == "__main__":
     unittest.main(verbosity=2)

skills/matrix-communication/scripts/matrix-e2ee-setup.py

@@ -18,12 +18,17 @@
     matrix-e2ee-setup.py PASSWORD     # Password as argument
     matrix-e2ee-setup.py --status
     matrix-e2ee-setup.py --logout
+    matrix-e2ee-setup.py --logout --purge-all
     matrix-e2ee-setup.py --help
 
 Options:
-    --status    Check if E2EE device is set up
-    --logout    Remove stored device credentials
-    --help      Show this help
+    --status      Check if E2EE device is set up
+    --logout      Remove the credentials and this device's store files
+    --purge-all   With --logout: wipe every other device's store files too
+    --help        Show this help
+
+--logout is scoped to the device in credentials.json. The store directory is
+shared by every device ever set up here, and their megolm history lives in it.
 """
 
 import asyncio
@@ -138,7 +143,15 @@ def main():
     )
     parser.add_argument("--status", action="store_true", help="Check E2EE setup status")
     parser.add_argument(
-        "--logout", action="store_true", help="Remove stored device credentials"
+        "--logout",
+        action="store_true",
+        help="Remove the stored credentials and this device's store files",
+    )
+    parser.add_argument(
+        "--purge-all",
+        action="store_true",
+        help="With --logout: also delete the store files of every OTHER device "
+        "in the store directory (destroys their megolm history)",
     )
     parser.add_argument("--json", action="store_true", help="Output as JSON")
     parser.add_argument("--debug", action="store_true", help="Show debug info")
@@ -169,11 +182,27 @@ def main():
     if args.logout:
         creds = load_credentials()
         if creds:
-            delete_credentials()
+            removed = delete_credentials(purge_all=args.purge_all)
             if args.json:
-                print(json.dumps({"success": True, "message": "Credentials removed"}))
+                print(
+                    json.dumps(
+                        {
+                            "success": True,
+                            "message": "Credentials removed",
+                            "device_id": creds.get("device_id"),
+                            "removed": removed,
+                        }
+                    )
+                )
             else:
-                print("E2EE device credentials removed.")
+                scope = (
+                    "every device in the store"
+                    if args.purge_all
+                    else f"device {creds.get('device_id')}"
+                )
+                print(f"E2EE credentials removed for {scope}:")
+                for name in removed:
+                    print(f"  {name}")
                 print("Note: The device still exists on the server.")
                 print("To fully remove it, go to Element > Settings > Sessions")
         else: