Commit Graph

7 Commits

Author SHA1 Message Date
Saket Aryan 2ed501a43c Merge branch 'pr1/telemetry-privacy-docs' into pr2/source-at-record-time 2026-09-16 20:33:57 +05:30
Saket Aryan 7710a4e180 fix(plugins): publish the salt atomically, and omit the hash when there is none
Review finding from @kartik-mem0 on this PR.

O_CREAT|O_EXCL then write leaves a window where the salt file exists and is
empty. Hooks are short-lived processes firing on every tool call and people run
several agent windows, so a concurrent reader lands in that window, reads
nothing, and falls back to a digest of the salt file's own path, memoized for
its whole run. That path is guessable, so the race silently replaced the privacy
control with something an attacker can compute, and hashed the same repository
two ways depending on timing.

The value is now written to a private temp file, fsynced, and published with
os.link, which is atomic and fails if another process already published one.
Link rather than replace, so losing the race adopts their salt instead of
clobbering it. The temp file is removed either way.

The derived fallback is gone rather than fixed. _scoped_digest returns "" when
there is no salt and record() omits the property, because an unsalted digest
over a git remote or a home-directory path is close to plaintext, and shipping
one under a name that says hash is worse than sending nothing.

Three tests: the racing reader never sees the name half-written, a second writer
adopts the first's salt and leaves no temp file, and an unwritable data
directory drops the property instead of emitting a weak one. The old test
asserted the fallback behaviour and is replaced.

265 passed, 8 skipped.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
2026-09-16 20:33:54 +05:30
Saket Aryan 59627f2a6c Merge branch 'pr1/telemetry-privacy-docs' into pr2/source-at-record-time
# Conflicts:
#	integrations/agent-plugin-core/python/telemetry.py
#	integrations/antigravity-plugin/core/telemetry.py
#	integrations/claude-code-plugin/core/telemetry.py
#	integrations/codex-plugin/core/telemetry.py
#	integrations/cursor-plugin/core/telemetry.py
#	integrations/kimi-plugin/core/telemetry.py
#	integrations/mem0-agent-plugin/core/telemetry.py
2026-09-15 00:32:51 +05:30
Saket Aryan 95d4fc27e2 fix(plugins): make the telemetry salt stable, its own file, and memoized
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
2026-09-15 00:25:12 +05:30
Saket Aryan f9c566aa16 fix(plugins): report the plugin that produced the event, not the one that sent it
harness is set when an event is recorded; source was set when its batch was
sent. Both came from module globals that stay at "generic" and "MEM0_PLUGIN"
until telemetry.init() runs, and two processes in the pipeline never run it:

- `python3 telemetry.py`, the detached sender spawn_flush() starts at session
  start, after every skill command, and when the MCP server exits. Everything it
  delivered was labelled source=MEM0_PLUGIN. Only batches flush_worker.py
  happened to drain got the real host.
- mcp_server.py, which records every manual search as harness=generic.

All six Python plugins ship the same files, so source could not tell any of them
apart and MCP searches from every plugin landed in one generic bucket. The
portable plugin is worse: it has no flush_worker at all, so its only sender is
the uninitialised one and 100% of its events were mislabelled.

Two changes. record() stamps source beside harness, so the sending process stops
mattering — flush() already spreads per-event properties last, so a per-event
source wins over any sender default. And the build generates core/_harness_id.py
per host, seeding both modules at import, so identity no longer depends on an
entrypoint remembering to call init(). The build already computed HARNESS_ID and
spent it only on skill templating, and bundle_drift already diffs core/
byte-for-byte, so --check catches drift for free.

Deliberately not adding MEM0_PLUGIN_HARNESS to the six manifests: they sit
outside the --sync and --check boundary, which is the property that caused this.

Also unifies two defaults that disagreed. configure_harness derived
`<host>_plugin` while telemetry.init derived `MEM0_<HOST>_PLUGIN`, so a third
value existed. It was unreachable only because hook_runner never calls flush();
moving source into record() would have made it live.

Events now carry a uuid so a resend can be collapsed.

The suite stayed green through all of this because the only tests live under one
host, behind a conftest that calls init() at import. New tests run in real
subprocesses with no init, and cover the portable plugin, which would pass a
native-only test vacuously.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
2026-09-15 00:17:53 +05:30
Saket Aryan 0d37619f24 fix(plugins): say what telemetry actually sends, and salt the hashes
The plugin README promises "anonymous usage events" and the telemetry module's
docstring says it sends only "salted hashes". Neither is true.

resolve_distinct_id() exchanges the API key for the account email and sends that
as the distinct_id on every event. Installing the plugin requires an API key, so
this is nearly every user. That is probably the behaviour we want — the Python
SDK and the CLI attribute the same way — but the description has to match it.

repo_hash and session_hash were unsalted SHA-256 cut to 16 hex characters.
repo.identity is a git remote URL, or `local:<absolute path>` when there is no
remote, which normally contains the account username. Sixteen unsalted hex
characters over that input space is enumerable, so the hash was not a privacy
control at all.

Salted per install, with the salt kept in the identity file. That preserves
every within-account join the analytics actually use and gives up only
cross-machine joins on the same repository, which nothing computes. Since the
distinct_id is already the email, the hash was never buying privacy from us —
only from whoever obtains the data later, which is exactly what the salt fixes.

Also corrects deepseek-plugin's README and source comment, which told readers
ZAPIER and STRANDS were already in the backend's KNOWN_EVENT_SOURCES allowlist.
Neither was.

Adds a Telemetry section to docs/integrations/claude-code.mdx, which had none.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
2026-09-15 00:17:26 +05:30
Kartik 73e7b8763a refactor(integrations): shared agent plugin runtimes and native adapters (#7203) 2026-09-08 23:32:25 +05:30