From 95d4fc27e23c1e05888915c142051c7c28235a95 Mon Sep 17 00:00:00 2001 From: Saket Aryan Date: Tue, 15 Sep 2026 00:25:12 +0530 Subject: [PATCH] fix(plugins): make the telemetry salt stable, its own file, and memoized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found three ways the first cut produced worse data than no salt at all. All three came from keeping the salt as a key in the identity dict and doing an unlocked read-modify-write. Hooks are short-lived separate processes firing on every tool call, and people run more than one agent window, so several processes would read {}, each mint its own uuid4, and each hash with it. One repository hashed several ways in the window before a writer won. resolve_distinct_id holds a copy of that same dict across a network call to /v1/ping/ with a 5s timeout, so whichever write landed second erased the other's key: losing the salt changes repo_hash mid-stream, losing the email fires a second $identify and splits the person. _write_identity swallows OSError, and nothing memoized, so on a read-only or full data directory every single event got a brand-new random salt — unbounded cardinality in PostHog, which is strictly worse than the unsalted value it replaced. The salt now lives in its own file claimed with O_CREAT|O_EXCL, so exactly one process wins and the losers read the winner's value, and it is memoized per process. When it cannot be persisted the fallback is derived from the data directory path: stable for the machine rather than random per call. Its own file also means record() no longer creates telemetry-identity.json as a side effect. is_first_run keys off that file, so the first cut would have silently suppressed the install event — a production metric change hidden in a docs PR. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb --- .../agent-plugin-core/python/telemetry.py | 62 ++++++++++++++++--- .../antigravity-plugin/core/telemetry.py | 62 ++++++++++++++++--- .../claude-code-plugin/core/telemetry.py | 62 ++++++++++++++++--- .../tests/test_telemetry.py | 47 ++++++++++++++ integrations/codex-plugin/core/telemetry.py | 62 ++++++++++++++++--- integrations/cursor-plugin/core/telemetry.py | 62 ++++++++++++++++--- integrations/kimi-plugin/core/telemetry.py | 62 ++++++++++++++++--- .../mem0-agent-plugin/core/telemetry.py | 62 ++++++++++++++++--- 8 files changed, 425 insertions(+), 56 deletions(-) diff --git a/integrations/agent-plugin-core/python/telemetry.py b/integrations/agent-plugin-core/python/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/agent-plugin-core/python/telemetry.py +++ b/integrations/agent-plugin-core/python/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/antigravity-plugin/core/telemetry.py b/integrations/antigravity-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/antigravity-plugin/core/telemetry.py +++ b/integrations/antigravity-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/claude-code-plugin/core/telemetry.py b/integrations/claude-code-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/claude-code-plugin/core/telemetry.py +++ b/integrations/claude-code-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/claude-code-plugin/tests/test_telemetry.py b/integrations/claude-code-plugin/tests/test_telemetry.py index 0ae280b19..eaa741ac5 100644 --- a/integrations/claude-code-plugin/tests/test_telemetry.py +++ b/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,49 @@ 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_salt_is_stable_when_it_cannot_be_persisted(isolated_env, monkeypatch): + """A read-only data dir must degrade to a weaker salt, not to random-per-call. + + Random per call is unbounded cardinality in PostHog, which is worse than no + salt at all. + """ + telemetry._salt_cache = "" + monkeypatch.setattr(telemetry.os, "open", lambda *a, **k: (_ for _ in ()).throw(OSError("read-only"))) + first = telemetry._install_salt() + telemetry._salt_cache = "" + assert telemetry._install_salt() == first diff --git a/integrations/codex-plugin/core/telemetry.py b/integrations/codex-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/codex-plugin/core/telemetry.py +++ b/integrations/codex-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/cursor-plugin/core/telemetry.py b/integrations/cursor-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/cursor-plugin/core/telemetry.py +++ b/integrations/cursor-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/kimi-plugin/core/telemetry.py b/integrations/kimi-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/kimi-plugin/core/telemetry.py +++ b/integrations/kimi-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: diff --git a/integrations/mem0-agent-plugin/core/telemetry.py b/integrations/mem0-agent-plugin/core/telemetry.py index 1b6bd854e..239836604 100644 --- a/integrations/mem0-agent-plugin/core/telemetry.py +++ b/integrations/mem0-agent-plugin/core/telemetry.py @@ -34,6 +34,7 @@ from typing import Any import memory_core +_salt_cache: str = "" _harness: str = "generic" _source_tag: str = "MEM0_PLUGIN" _PRIVATE_KEYS = { @@ -92,15 +93,60 @@ def _digest(value: str, length: int = 16) -> str: return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length] +def _salt_path() -> Path: + return memory_core.data_dir() / "telemetry-salt" + + def _install_salt() -> str: - """Random per-install salt, created on first use and kept in the identity file.""" - identity = _read_identity() - salt = identity.get("salt") - if not salt: - salt = uuid.uuid4().hex - identity["salt"] = salt - _write_identity(identity) - return salt + """Random per-install salt, created once and memoized for the process. + + Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in + the identity file. Three reasons, all of which produced wrong data when this + lived in the identity dict: + + - Hooks are short-lived separate processes firing on every tool call, and + people run more than one agent window. A read-modify-write would let each + process mint its own salt, so one repository would hash several ways in the + window before a writer won. + - resolve_distinct_id holds a copy of the identity dict across a network call + to /v1/ping/, so whichever write landed second erased the other's key — + losing either the salt (repo_hash changes mid-stream) or the email (a + second $identify, splitting the person). + - Touching the identity file from record() would create it, and is_first_run + keys off that file, so recording an event would silently suppress the + install event. + + On a read-only or full data directory the fallback is derived from the data + directory path: stable for the machine rather than random per call, so the + failure mode is a weaker salt and not unbounded cardinality in PostHog. + """ + global _salt_cache + if _salt_cache: + return _salt_cache + + path = _salt_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + except OSError: + pass + except FileExistsError: + pass + except OSError: + # Cannot persist. Stable-per-machine beats random-per-call. + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache + + try: + _salt_cache = path.read_text(encoding="utf-8").strip() + except OSError: + _salt_cache = "" + if not _salt_cache: + _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest() + return _salt_cache def _scoped_digest(value: str, length: int = 16) -> str: