From fc8efca49d6e6feaf33cdbf4094c0c2af7963519 Mon Sep 17 00:00:00 2001 From: Afonso Coutinho Date: Wed, 3 Jun 2026 05:29:14 +0100 Subject: [PATCH] fix: backup import drops a user's memory when its text matches another user's (#1743) --- routes/backup_routes.py | 7 ++- tests/test_backup_import_cross_user_dedup.py | 60 ++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 tests/test_backup_import_cross_user_dedup.py diff --git a/routes/backup_routes.py b/routes/backup_routes.py index b165fcc..2b92a15 100644 --- a/routes/backup_routes.py +++ b/routes/backup_routes.py @@ -77,7 +77,12 @@ def setup_backup_routes(memory_manager, preset_manager, skills_manager) -> APIRo # ── Memories ── if "memories" in body and isinstance(body["memories"], list): existing = memory_manager.load_all() - existing_texts = {e.get("text", "").strip().lower() for e in existing} + # Dedup against THIS user's own memories only. Using every tenant's + # rows (load_all) meant a memory whose text matched any other + # user's was silently skipped, so the importing user lost their own + # data. The full store is still saved back below. + existing_texts = {e.get("text", "").strip().lower() + for e in existing if e.get("owner") == user} added = 0 for mem in body["memories"]: if not isinstance(mem, dict) or not mem.get("text"): diff --git a/tests/test_backup_import_cross_user_dedup.py b/tests/test_backup_import_cross_user_dedup.py new file mode 100644 index 0000000..2df5936 --- /dev/null +++ b/tests/test_backup_import_cross_user_dedup.py @@ -0,0 +1,60 @@ +"""Backup import must dedup memories against the importing user only. + +import_data deduped incoming memories against memory_manager.load_all() +(every tenant\'s rows), so a memory whose text matched ANY other user\'s +memory was silently skipped - the importing user lost their own data. The +dedup must be scoped to the caller\'s own memories. The full multi-tenant +store is still saved back. +""" +import asyncio +from types import SimpleNamespace +from unittest.mock import MagicMock + +import routes.backup_routes as br + + +class _Req: + def __init__(self, body): + self._body = body + + async def json(self): + return self._body + + +def _setup(monkeypatch, store, user="alice"): + monkeypatch.setattr(br, "require_admin", lambda request: None) + monkeypatch.setattr(br, "get_current_user", lambda request: user) + + mem = MagicMock() + mem.load_all.return_value = list(store) + saved = {} + mem.save.side_effect = lambda entries: saved.__setitem__("entries", entries) + + skills = MagicMock() + skills.load_all.return_value = [] + router = br.setup_backup_routes(mem, MagicMock(), skills) + endpoint = None + for r in router.routes: + if r.path == "/api/import" and "POST" in getattr(r, "methods", set()): + endpoint = r.endpoint + assert endpoint is not None + return endpoint, saved + + +def test_user_can_import_memory_matching_another_users_text(monkeypatch): + # bob already has "buy milk"; alice imports her own "Buy Milk". + endpoint, saved = _setup(monkeypatch, [{"text": "buy milk", "owner": "bob"}]) + body = {"memories": [{"text": "Buy Milk"}]} + asyncio.run(endpoint(_Req(body))) + texts_by_owner = {(e.get("owner"), e.get("text")) for e in saved["entries"]} + assert ("alice", "Buy Milk") in texts_by_owner # not dropped as a "duplicate" + assert ("bob", "buy milk") in texts_by_owner # other tenant preserved + + +def test_users_own_duplicate_is_still_skipped(monkeypatch): + endpoint, saved = _setup(monkeypatch, [{"text": "buy milk", "owner": "alice"}]) + body = {"memories": [{"text": "Buy Milk"}]} + asyncio.run(endpoint(_Req(body))) + alice_milk = [e for e in saved["entries"] + if e.get("owner") == "alice" and e.get("text", "").lower() == "buy milk"] + assert len(alice_milk) == 1 # the real duplicate is still deduped