diff --git a/mem0/memory/main.py b/mem0/memory/main.py index f4f3c4bf0..1e0fc0c44 100644 --- a/mem0/memory/main.py +++ b/mem0/memory/main.py @@ -135,18 +135,30 @@ _SENSITIVE_SUFFIXES = ( ENTITY_PARAMS = frozenset({"user_id", "agent_id", "run_id"}) DELETE_ALL_BATCH_SIZE = 1000 -# Tenant-scoping fields that update() must never let caller-supplied metadata overwrite (issues #4490, #6277). +# Tenant-scoping fields that caller-supplied metadata must never set, on either the +# creation or the update path (issues #4490, #6277, #6655). _IDENTITY_KEYS = ENTITY_PARAMS | {"actor_id"} -def _strip_identity_keys(metadata: Dict[str, Any], existing_payload: Dict[str, Any]) -> Dict[str, Any]: - """Drop identity keys from caller metadata; they are immutable after creation (issues #4490, #6277).""" +def _strip_identity_keys( + metadata: Dict[str, Any], + existing_payload: Dict[str, Any], + *, + context: str = "update()", +) -> Dict[str, Any]: + """Drop identity keys from caller metadata; scope is set by the entity params, not metadata. + + On the update path `existing_payload` carries the memory's current scope, so + re-sending an identical value is silently accepted; only a changed value warns. + On the creation path there is no prior payload, so pass an empty dict and every + identity key present in `metadata` is dropped with a warning. + """ clean = {} for key, value in metadata.items(): if key not in _IDENTITY_KEYS: clean[key] = value elif value != existing_payload.get(key): - logger.warning(f"update(): ignoring metadata['{key}'] - identity fields are immutable after creation") + logger.warning(f"{context}: ignoring metadata['{key}'] - identity fields cannot be set through metadata") return clean @@ -315,7 +327,9 @@ def _build_filters_and_metadata( for flexible session scoping and optionally narrows queries to a specific `actor_id`. It returns two dicts: 1. `base_metadata_template`: Used as a template for metadata when storing new memories. - It includes all provided session identifier(s) and any `input_metadata`. + It includes all provided session identifier(s) and any `input_metadata`. Identity + scope is set from the entity params only; identity keys in `input_metadata` are + dropped, so freeform metadata cannot place a memory into an unrequested scope. 2. `effective_query_filters`: Used for querying existing memories. It includes all provided session identifier(s), any `input_filters`, and a resolved actor identifier for targeted filtering if specified by any actor-related inputs. @@ -343,7 +357,12 @@ def _build_filters_and_metadata( scoped to the provided session(s) and potentially a resolved actor. """ - base_metadata_template = deepcopy(input_metadata) if input_metadata else {} + # Identity scope is set below from the entity params only. Stripping the keys here + # stops caller metadata from placing a memory into a scope the caller did not pass, + # which the re-pins below cannot prevent for a param that was left unset (issue #6655). + base_metadata_template = ( + _strip_identity_keys(deepcopy(input_metadata), {}, context="add()") if input_metadata else {} + ) effective_query_filters = deepcopy(input_filters) if input_filters else {} # ---------- validate and add all provided session ids ---------- diff --git a/tests/memory/test_main.py b/tests/memory/test_main.py index d00e953b0..cea7e3842 100644 --- a/tests/memory/test_main.py +++ b/tests/memory/test_main.py @@ -453,6 +453,95 @@ def test_update_memory_metadata_cannot_change_identity_fields(mocker, caplog): assert "ignoring metadata['user_id']" in caplog.text +_ATTACKER_ADD_METADATA = { + "agent_id": "victim-agent", + "run_id": "victim-run", + "actor_id": "victim-actor", + "category": "sports", +} + + +def _captured_add_metadata(memory, mocker, **add_kwargs): + """Run add() with the pipeline stubbed and return the metadata template it produced.""" + captured = {} + + def _capture(messages, metadata, filters, infer, **kwargs): + captured.update(metadata) + return [] + + mocker.patch.object(memory, "_add_to_vector_store", side_effect=_capture) + memory.add("I like coffee", infer=False, **add_kwargs) + return captured + + +def test_add_metadata_cannot_set_identity_fields(mocker, caplog): + """Regression (issue #6655): add() metadata must not inject identity scope. + + The caller scopes by user_id only, so the agent_id/run_id re-pins in + _build_filters_and_metadata never fire and cannot defend the payload. + """ + memory = _build_memory_instance(mocker, Memory) + + with caplog.at_level(logging.WARNING, logger="mem0.memory.main"): + metadata = _captured_add_metadata( + memory, mocker, user_id="attacker", metadata=dict(_ATTACKER_ADD_METADATA) + ) + + assert metadata["user_id"] == "attacker" + for key in ("agent_id", "run_id", "actor_id"): + assert key not in metadata, f"{key} was injected through add() metadata" + # Non-identity metadata is untouched. + assert metadata["category"] == "sports" + assert "ignoring metadata['agent_id']" in caplog.text + + +@pytest.mark.asyncio +async def test_async_add_metadata_cannot_set_identity_fields(mocker): + """Async counterpart of test_add_metadata_cannot_set_identity_fields.""" + memory = _build_memory_instance(mocker, AsyncMemory) + captured = {} + + async def _capture(messages, metadata, filters, infer, **kwargs): + captured.update(metadata) + return [] + + mocker.patch.object(memory, "_add_to_vector_store", side_effect=_capture) + await memory.add( + "I like coffee", + user_id="attacker", + metadata=dict(_ATTACKER_ADD_METADATA), + infer=False, + ) + + assert captured["user_id"] == "attacker" + for key in ("agent_id", "run_id", "actor_id"): + assert key not in captured, f"{key} was injected through async add() metadata" + assert captured["category"] == "sports" + + +def test_add_entity_params_still_set_scope(mocker): + """The documented top-level params remain the only way to set scope.""" + memory = _build_memory_instance(mocker, Memory) + + metadata = _captured_add_metadata( + memory, mocker, user_id="u1", agent_id="a1", run_id="r1", metadata={"category": "sports"} + ) + + assert metadata["user_id"] == "u1" + assert metadata["agent_id"] == "a1" + assert metadata["run_id"] == "r1" + assert metadata["category"] == "sports" + + +def test_add_without_metadata_is_unaffected(mocker): + """No metadata argument means no stripping and no behaviour change.""" + memory = _build_memory_instance(mocker, Memory) + + metadata = _captured_add_metadata(memory, mocker, user_id="u1") + + assert metadata == {"user_id": "u1"} + + @pytest.mark.asyncio async def test_async_update_memory_metadata_cannot_change_identity_fields(mocker): """Async counterpart of test_update_memory_metadata_cannot_change_identity_fields."""