fix(plugins): say what telemetry actually sends, and salt the hashes (#7322)

integrations/claude-code-plugin/tests/test_telemetry.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import os
 import sys
 from pathlib import Path
 from unittest.mock import patch
@@ -273,3 +274,114 @@ def test_spawn_flush_does_nothing_without_a_spool(isolated_env):
     with patch.object(telemetry.subprocess, "Popen") as popen:
         assert telemetry.spawn_flush() is True
         popen.assert_called_once()
+
+
+def test_salt_is_stable_across_processes(isolated_env):
+    """Hooks are separate short-lived processes; one repo must hash one way.
+
+    An unlocked read-modify-write let each process mint its own salt, so a
+    repository hashed several ways in the window before one writer won.
+    """
+    import subprocess as sp
+
+    core = str(Path(__file__).resolve().parents[1] / "core")
+    script = (
+        f"import sys; sys.path.insert(0, {core!r})\n"
+        "import telemetry\n"
+        "print(telemetry._install_salt())"
+    )
+    env = {**os.environ, "MEM0_CODE_DATA_DIR": str(memory_core.data_dir())}
+    salts = {
+        sp.run([sys.executable, "-c", script], capture_output=True, text=True, env=env).stdout.strip()
+        for _ in range(4)
+    }
+    assert len(salts) == 1, f"one repo hashed {len(salts)} ways: {salts}"
+
+
+def test_salt_does_not_touch_the_identity_file(isolated_env):
+    """The identity file is is_first_run's marker and the sender's email store.
+
+    Writing the salt into it would create it from record(), suppressing the
+    install event, and would race resolve_distinct_id, which holds a stale copy
+    of that dict across a network call.
+    """
+    telemetry._install_salt()
+    assert not telemetry._identity_path().exists()
+
+
+def test_no_salt_means_no_hash_rather_than_an_unsalted_one(isolated_env, monkeypatch):
+    """A read-only data dir drops the property; it must not emit a weak digest.
+
+    The previous fallback was a digest of the salt file's own path, which an
+    attacker can compute, memoized for the whole process. A property named
+    repo_hash carrying an effectively unsalted digest is worse than no property:
+    it reads as protected and is not.
+    """
+    telemetry._salt_cache = ""
+    monkeypatch.setattr(telemetry.os, "open", lambda *a, **k: (_ for _ in ()).throw(OSError("read-only")))
+
+    assert telemetry._install_salt() == ""
+    assert telemetry._scoped_digest("git@github.com:acme/secret.git") == ""
+
+
+def test_a_half_written_salt_is_never_visible_to_another_process(isolated_env, monkeypatch):
+    """The window this closes: file created, value not yet written.
+
+    O_CREAT|O_EXCL then write leaves the name present and empty in between. A
+    hook reading it there used to get "", fall back to the path digest and cache
+    that for its whole run, so the same repo hashed two ways depending on timing.
+    Publishing by link means the name either does not exist or is complete.
+    """
+    telemetry._salt_cache = ""
+    salt_path = telemetry._salt_path()
+    observed = []
+
+    real_link = telemetry.os.link
+
+    def observing_link(source, target):
+        # Stand where the racing reader stands: after the temp file is written,
+        # before the real name exists.
+        observed.append(salt_path.exists())
+        return real_link(source, target)
+
+    monkeypatch.setattr(telemetry.os, "link", observing_link)
+    salt = telemetry._install_salt()
+
+    assert observed == [False], "the salt name existed before it held a value"
+    assert len(salt) == 32
+    assert salt_path.read_text(encoding="utf-8").strip() == salt
+
+
+def test_a_filesystem_without_hardlinks_still_gets_a_salt(isolated_env, monkeypatch):
+    """Publishing by link must not become a silent loss of the hashes.
+
+    Some network mounts and container volumes reject os.link. Returning ""
+    there would drop repo_hash and session_hash on every run for that whole
+    cohort, which is a bigger loss than the narrow race the link closes.
+    """
+    telemetry._salt_cache = ""
+    monkeypatch.setattr(
+        telemetry.os, "link", lambda src, dst: (_ for _ in ()).throw(OSError(38, "not implemented"))
+    )
+
+    salt = telemetry._install_salt()
+
+    assert len(salt) == 32, "no salt on a filesystem without hardlinks"
+    assert telemetry._salt_path().read_text(encoding="utf-8").strip() == salt
+    assert telemetry._scoped_digest("git@github.com:acme/x.git") != ""
+    assert not list(telemetry._salt_path().parent.glob("telemetry-salt.*.tmp"))
+
+
+def test_a_concurrent_writer_does_not_clobber_the_published_salt(isolated_env):
+    """Second process to finish must adopt the first one's salt, not replace it.
+
+    os.link rather than os.replace is what makes losing the race harmless.
+    """
+    telemetry._salt_cache = ""
+    first = telemetry._install_salt()
+
+    telemetry._salt_cache = ""
+    second = telemetry._install_salt()
+
+    assert second == first
+    assert not list(telemetry._salt_path().parent.glob("telemetry-salt.*.tmp")), "temp file left behind"