From 6b4d2b780e1e2bf4cd80a46196375f5ba86f6a42 Mon Sep 17 00:00:00 2001 From: utkarsh240799 Date: Tue, 24 Mar 2026 17:52:18 +0530 Subject: [PATCH] fix: prevent in-place mutation of metadata dicts in _create_memory and _create_procedural_memory Addresses #2648. The metadata dict passed by callers was mutated in-place inside _create_memory and _create_procedural_memory (both sync and async), causing unsafe behavior under concurrent/async usage. Co-Authored-By: Claude Opus 4.6 (1M context) --- mem0/memory/main.py | 42 ++++---- tests/memory/test_main.py | 195 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 20 deletions(-) diff --git a/mem0/memory/main.py b/mem0/memory/main.py index c9b67c75c..320caa189 100644 --- a/mem0/memory/main.py +++ b/mem0/memory/main.py @@ -1180,24 +1180,24 @@ class Memory(MemoryBase): else: embeddings = self.embedding_model.embed(data, memory_action="add") memory_id = str(uuid.uuid4()) - metadata = metadata or {} - metadata["data"] = data - metadata["hash"] = hashlib.md5(data.encode()).hexdigest() - metadata["created_at"] = datetime.now(timezone.utc).isoformat() + new_metadata = deepcopy(metadata) if metadata is not None else {} + new_metadata["data"] = data + new_metadata["hash"] = hashlib.md5(data.encode()).hexdigest() + new_metadata["created_at"] = datetime.now(timezone.utc).isoformat() self.vector_store.insert( vectors=[embeddings], ids=[memory_id], - payloads=[metadata], + payloads=[new_metadata], ) self.db.add_history( memory_id, None, data, "ADD", - created_at=metadata.get("created_at"), - actor_id=metadata.get("actor_id"), - role=metadata.get("role"), + created_at=new_metadata.get("created_at"), + actor_id=new_metadata.get("actor_id"), + role=new_metadata.get("role"), ) return memory_id @@ -1231,9 +1231,10 @@ class Memory(MemoryBase): if metadata is None: raise ValueError("Metadata cannot be done for procedural memory.") - metadata["memory_type"] = MemoryType.PROCEDURAL.value + new_metadata = deepcopy(metadata) + new_metadata["memory_type"] = MemoryType.PROCEDURAL.value embeddings = self.embedding_model.embed(procedural_memory, memory_action="add") - memory_id = self._create_memory(procedural_memory, {procedural_memory: embeddings}, metadata=metadata) + memory_id = self._create_memory(procedural_memory, {procedural_memory: embeddings}, metadata=new_metadata) capture_event("mem0._create_procedural_memory", self, {"memory_id": memory_id, "sync_type": "sync"}) result = {"results": [{"id": memory_id, "memory": procedural_memory, "event": "ADD"}]} @@ -2278,16 +2279,16 @@ class AsyncMemory(MemoryBase): embeddings = await asyncio.to_thread(self.embedding_model.embed, data, memory_action="add") memory_id = str(uuid.uuid4()) - metadata = metadata or {} - metadata["data"] = data - metadata["hash"] = hashlib.md5(data.encode()).hexdigest() - metadata["created_at"] = datetime.now(timezone.utc).isoformat() + new_metadata = deepcopy(metadata) if metadata is not None else {} + new_metadata["data"] = data + new_metadata["hash"] = hashlib.md5(data.encode()).hexdigest() + new_metadata["created_at"] = datetime.now(timezone.utc).isoformat() await asyncio.to_thread( self.vector_store.insert, vectors=[embeddings], ids=[memory_id], - payloads=[metadata], + payloads=[new_metadata], ) await asyncio.to_thread( @@ -2296,9 +2297,9 @@ class AsyncMemory(MemoryBase): None, data, "ADD", - created_at=metadata.get("created_at"), - actor_id=metadata.get("actor_id"), - role=metadata.get("role"), + created_at=new_metadata.get("created_at"), + actor_id=new_metadata.get("actor_id"), + role=new_metadata.get("role"), ) return memory_id @@ -2347,9 +2348,10 @@ class AsyncMemory(MemoryBase): if metadata is None: raise ValueError("Metadata cannot be done for procedural memory.") - metadata["memory_type"] = MemoryType.PROCEDURAL.value + new_metadata = deepcopy(metadata) + new_metadata["memory_type"] = MemoryType.PROCEDURAL.value embeddings = await asyncio.to_thread(self.embedding_model.embed, procedural_memory, memory_action="add") - memory_id = await self._create_memory(procedural_memory, {procedural_memory: embeddings}, metadata=metadata) + memory_id = await self._create_memory(procedural_memory, {procedural_memory: embeddings}, metadata=new_metadata) capture_event("mem0._create_procedural_memory", self, {"memory_id": memory_id, "sync_type": "async"}) result = {"results": [{"id": memory_id, "memory": procedural_memory, "event": "ADD"}]} diff --git a/tests/memory/test_main.py b/tests/memory/test_main.py index 3b178f243..a7555292a 100644 --- a/tests/memory/test_main.py +++ b/tests/memory/test_main.py @@ -188,6 +188,201 @@ async def test_async_update_memory_uses_utc_timestamps(mocker): _assert_utc_timestamp(payload["updated_at"]) +class TestMetadataNotMutated: + """Tests that metadata dicts passed to memory methods are not mutated in-place (issue #2648).""" + + def test_create_memory_does_not_mutate_metadata(self, mocker): + memory = _build_memory_instance(mocker, Memory) + original_metadata = {"user_id": "test_user", "category": "sports"} + metadata_copy = original_metadata.copy() + + memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=original_metadata) + + assert original_metadata == metadata_copy, ( + f"_create_memory mutated the caller's metadata dict: {original_metadata} != {metadata_copy}" + ) + + def test_create_memory_stores_correct_payload(self, mocker): + memory = _build_memory_instance(mocker, Memory) + metadata = {"user_id": "test_user", "category": "sports"} + + memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=metadata) + + payload = memory.vector_store.insert.call_args.kwargs["payloads"][0] + assert payload["data"] == "test data" + assert payload["user_id"] == "test_user" + assert payload["category"] == "sports" + assert "hash" in payload + assert "created_at" in payload + + def test_create_memory_with_none_metadata(self, mocker): + memory = _build_memory_instance(mocker, Memory) + + memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=None) + + payload = memory.vector_store.insert.call_args.kwargs["payloads"][0] + assert payload["data"] == "test data" + assert "hash" in payload + + def test_create_memory_shared_metadata_across_calls(self, mocker): + """Verify that sharing a metadata dict between multiple _create_memory calls is safe.""" + memory = _build_memory_instance(mocker, Memory) + shared_metadata = {"user_id": "test_user"} + + memory._create_memory("first memory", {"first memory": [0.1, 0.2, 0.3]}, metadata=shared_metadata) + memory._create_memory("second memory", {"second memory": [0.4, 0.5, 0.6]}, metadata=shared_metadata) + + assert shared_metadata == {"user_id": "test_user"}, "shared metadata was mutated across calls" + + # Verify each call got the correct data + first_payload = memory.vector_store.insert.call_args_list[0].kwargs["payloads"][0] + second_payload = memory.vector_store.insert.call_args_list[1].kwargs["payloads"][0] + assert first_payload["data"] == "first memory" + assert second_payload["data"] == "second memory" + + def test_create_memory_preserves_role_and_actor_id_in_history(self, mocker): + """Verify that role and actor_id from metadata flow through to add_history after deepcopy.""" + memory = _build_memory_instance(mocker, Memory) + metadata = {"user_id": "test_user", "role": "assistant", "actor_id": "bot-1"} + + memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=metadata) + + # Verify the payload stored in vector store has all fields + payload = memory.vector_store.insert.call_args.kwargs["payloads"][0] + assert payload["role"] == "assistant" + assert payload["actor_id"] == "bot-1" + assert payload["user_id"] == "test_user" + assert payload["data"] == "test data" + + # Verify add_history received the correct role and actor_id + history_call = memory.db.add_history.call_args + assert history_call.kwargs["role"] == "assistant" + assert history_call.kwargs["actor_id"] == "bot-1" + + # And the original metadata is still untouched + assert metadata == {"user_id": "test_user", "role": "assistant", "actor_id": "bot-1"} + + def test_create_memory_with_nested_metadata_not_mutated(self, mocker): + """Verify deepcopy protects nested structures in metadata.""" + memory = _build_memory_instance(mocker, Memory) + metadata = {"user_id": "test_user", "tags": ["important", "urgent"], "config": {"key": "val"}} + import copy + metadata_snapshot = copy.deepcopy(metadata) + + memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=metadata) + + assert metadata == metadata_snapshot, "Nested metadata structures were mutated" + + def test_update_memory_does_not_mutate_metadata(self, mocker): + memory = _build_memory_instance(mocker, Memory) + memory.vector_store.get.return_value = MagicMock( + payload={"data": "old data", "user_id": "test_user", "created_at": "2026-01-01T00:00:00+00:00"} + ) + original_metadata = {"category": "updated"} + metadata_copy = original_metadata.copy() + + memory._update_memory("mem-id", "new data", {"new data": [0.1, 0.2, 0.3]}, metadata=original_metadata) + + assert original_metadata == metadata_copy, ( + f"_update_memory mutated the caller's metadata dict: {original_metadata} != {metadata_copy}" + ) + + def test_add_to_vector_store_no_infer_does_not_mutate_metadata(self, mocker): + """Verify _add_to_vector_store with infer=False doesn't leak metadata between messages.""" + memory = _build_memory_instance(mocker, Memory) + memory.embedding_model.embed.return_value = [0.1, 0.2, 0.3] + + original_metadata = {"user_id": "test_user"} + metadata_copy = original_metadata.copy() + + messages = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi there", "name": "bot-1"}, + ] + + result = memory._add_to_vector_store(messages, original_metadata, filters={}, infer=False) + + # Metadata should not be mutated + assert original_metadata == metadata_copy, ( + f"_add_to_vector_store mutated the caller's metadata: {original_metadata}" + ) + + # Should have created 2 memories + assert len(result) == 2 + assert result[0]["role"] == "user" + assert result[1]["role"] == "assistant" + assert result[1]["actor_id"] == "bot-1" + + # Verify each insert got distinct payloads with correct roles + insert_calls = memory.vector_store.insert.call_args_list + first_payload = insert_calls[0].kwargs["payloads"][0] + second_payload = insert_calls[1].kwargs["payloads"][0] + assert first_payload["role"] == "user" + assert "actor_id" not in first_payload # user message has no name + assert second_payload["role"] == "assistant" + assert second_payload["actor_id"] == "bot-1" + + @pytest.mark.asyncio + async def test_async_create_memory_does_not_mutate_metadata(self, mocker): + memory = _build_memory_instance(mocker, AsyncMemory) + original_metadata = {"user_id": "test_user", "category": "sports"} + metadata_copy = original_metadata.copy() + + await memory._create_memory("test data", {"test data": [0.1, 0.2, 0.3]}, metadata=original_metadata) + + assert original_metadata == metadata_copy, ( + f"async _create_memory mutated the caller's metadata dict: {original_metadata} != {metadata_copy}" + ) + + @pytest.mark.asyncio + async def test_async_create_memory_shared_metadata_across_calls(self, mocker): + memory = _build_memory_instance(mocker, AsyncMemory) + shared_metadata = {"user_id": "test_user"} + + await memory._create_memory("first memory", {"first memory": [0.1, 0.2, 0.3]}, metadata=shared_metadata) + await memory._create_memory("second memory", {"second memory": [0.4, 0.5, 0.6]}, metadata=shared_metadata) + + assert shared_metadata == {"user_id": "test_user"}, "shared metadata was mutated across async calls" + + @pytest.mark.asyncio + async def test_async_add_to_vector_store_no_infer_does_not_mutate_metadata(self, mocker): + """Verify async _add_to_vector_store with infer=False doesn't leak metadata between messages.""" + memory = _build_memory_instance(mocker, AsyncMemory) + memory.embedding_model.embed.return_value = [0.1, 0.2, 0.3] + + original_metadata = {"user_id": "test_user"} + metadata_copy = original_metadata.copy() + + messages = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi there", "name": "bot-1"}, + ] + + result = await memory._add_to_vector_store(messages, original_metadata, effective_filters={}, infer=False) + + assert original_metadata == metadata_copy, ( + f"async _add_to_vector_store mutated the caller's metadata: {original_metadata}" + ) + assert len(result) == 2 + assert result[0]["role"] == "user" + assert result[1]["role"] == "assistant" + + @pytest.mark.asyncio + async def test_async_update_memory_does_not_mutate_metadata(self, mocker): + memory = _build_memory_instance(mocker, AsyncMemory) + memory.vector_store.get.return_value = MagicMock( + payload={"data": "old data", "user_id": "test_user", "created_at": "2026-01-01T00:00:00+00:00"} + ) + original_metadata = {"category": "updated"} + metadata_copy = original_metadata.copy() + + await memory._update_memory("mem-id", "new data", {"new data": [0.1, 0.2, 0.3]}, metadata=original_metadata) + + assert original_metadata == metadata_copy, ( + f"async _update_memory mutated the caller's metadata dict: {original_metadata} != {metadata_copy}" + ) + + def test_normalize_iso_timestamp_to_utc_preserves_naive_values(): assert _normalize_iso_timestamp_to_utc("2026-03-18T00:00:00") == "2026-03-18T00:00:00"