diff --git a/integrations/agent-plugin-core/python/telemetry.py b/integrations/agent-plugin-core/python/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/agent-plugin-core/python/telemetry.py +++ b/integrations/agent-plugin-core/python/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/antigravity-plugin/core/telemetry.py b/integrations/antigravity-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/antigravity-plugin/core/telemetry.py +++ b/integrations/antigravity-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/claude-code-plugin/core/telemetry.py b/integrations/claude-code-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/claude-code-plugin/core/telemetry.py +++ b/integrations/claude-code-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/claude-code-plugin/tests/test_telemetry.py b/integrations/claude-code-plugin/tests/test_telemetry.py index eaa741ac5..b816b629e 100644 --- a/integrations/claude-code-plugin/tests/test_telemetry.py +++ b/integrations/claude-code-plugin/tests/test_telemetry.py @@ -309,14 +309,59 @@ def test_salt_does_not_touch_the_identity_file(isolated_env): 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. +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. - Random per call is unbounded cardinality in PostHog, which is worse than no - salt at all. + 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"))) - first = telemetry._install_salt() + + 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 = "" - assert telemetry._install_salt() == first + 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_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" diff --git a/integrations/codex-plugin/core/telemetry.py b/integrations/codex-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/codex-plugin/core/telemetry.py +++ b/integrations/codex-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/cursor-plugin/core/telemetry.py b/integrations/cursor-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/cursor-plugin/core/telemetry.py +++ b/integrations/cursor-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/kimi-plugin/core/telemetry.py b/integrations/kimi-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/kimi-plugin/core/telemetry.py +++ b/integrations/kimi-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}", diff --git a/integrations/mem0-agent-plugin/core/telemetry.py b/integrations/mem0-agent-plugin/core/telemetry.py index 239836604..2cb1056e0 100644 --- a/integrations/mem0-agent-plugin/core/telemetry.py +++ b/integrations/mem0-agent-plugin/core/telemetry.py @@ -116,36 +116,49 @@ def _install_salt() -> str: 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. + Published atomically, and there is deliberately no derived fallback. Creating + the file with O_CREAT|O_EXCL and then writing into it leaves a window where + the file exists and is empty, and a concurrent hook that reads it in that + window gets nothing. Falling back to a digest of the path would hand that + process a salt an attacker can compute, memoized for its whole run, which is + the privacy control this function exists to provide silently turning itself + off under load. The salt is written to a private temp file first and linked + into place, so the name either does not exist or already has the full value. + + Returns "" when it genuinely cannot persist. Callers omit the hash entirely + rather than emit an unsalted one. """ global _salt_cache if _salt_cache: return _salt_cache path = _salt_path() + temporary = path.with_name(f"{path.name}.{os.getpid()}.tmp") try: path.parent.mkdir(parents=True, exist_ok=True) - handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + handle = os.open(temporary, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(uuid.uuid4().hex) + stream.flush() + os.fsync(stream.fileno()) try: - with os.fdopen(handle, "w", encoding="utf-8") as stream: - stream.write(uuid.uuid4().hex) + # Atomic claim: fails if another process already published one. + # os.link rather than replace, which would clobber theirs. + os.link(temporary, path) + except FileExistsError: + pass + except OSError: + pass + finally: + try: + temporary.unlink() 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 @@ -158,10 +171,17 @@ def _scoped_digest(value: str, length: int = 16) -> str: privacy control without the salt. Salting per install keeps every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. + + Returns "" when there is no salt, so record() omits the property. An + unsalted digest over this input space is close to plaintext, and emitting one + under a name that implies it is hashed is worse than sending nothing. """ if not value: return "" - return hashlib.sha256(f"{_install_salt()}:{value}".encode("utf-8")).hexdigest()[:length] + salt = _install_salt() + if not salt: + return "" + return hashlib.sha256(f"{salt}:{value}".encode("utf-8")).hexdigest()[:length] def _safe_value(value: Any) -> Any: @@ -252,10 +272,17 @@ def record( os=sys.platform, python_version=platform.python_version(), ) + # Assigned only when the digest is real. _scoped_digest returns "" when + # the salt could not be persisted, and an empty property is worse than an + # absent one: it survives the None filter below and reads as a value. if repo is not None: - properties["repo_hash"] = _scoped_digest(getattr(repo, "identity", "")) + repo_hash = _scoped_digest(getattr(repo, "identity", "")) + if repo_hash: + properties["repo_hash"] = repo_hash if session_id: - properties["session_hash"] = _scoped_digest(session_id) + session_hash = _scoped_digest(session_id) + if session_hash: + properties["session_hash"] = session_hash line = json.dumps( { "event": f"{EVENT_PREFIX}.{event}",