From a69a9c351dfa7cc37b804259b19c4f4a6a3a7e44 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Fri, 28 Aug 2026 15:22:16 +1000 Subject: [PATCH] feat(telemetry): transmit the stable install_id as-is Product-owner decision, 2026-08-27: the analytical need is stable cross-window identity (retention curves, longitudinal install behaviour), which the rotating pseudonym destroyed by design. The feature has not shipped - zero consented users, zero production transmissions - so identity semantics can change without breaking any promise made to a user; existing (dev-only) consent windows carry forward unchanged. Removed in full rather than weakened in place: - shared_metrics_identity.py (salt generation/rotation, HMAC-SHA256 derivation, payload substitution) and its 19-test file. - The sender's derivation step. _freeze_identity keeps its validation role (unreadable/non-object/id-less payloads still reject rather than block the queue) and now records the raw install_id in sent_install_id; _body rewrites the payload's install_id from that frozen column, keeping byte-identical resends anchored to one recorded value. Consent surface updated in the same change: the setup wizard now states plainly that packages carry the stable profile-scoped install ID (a random UUID, no personal information, reset by deleting the shared-metrics directory). No consent was ever collected under the old wording in any shipped build. Docs A.2/A.3 rewritten as decision records rather than silently edited: A.2 records what is transmitted now and states the consequences plainly (indefinite cross-package correlation is the designed behaviour); A.3 records why rotation existed and why its removal was accepted. The main-body "must not reuse the persistent local identifier by default" escape hatch is exercised, not deleted: that paragraph required exactly this product decision, which has now been made. A.6's deletion note updated: install_id is now itself the lookup key, so a future delete-on-request needs only a service-side API, not a mapping. Tests: the two privacy assertions invert deliberately (test_the_stable_install_id_is_transmitted_as_is and the e2e wire variant); freezing/byte-identical-retry coverage unchanged. Staging E2E script now asserts transmitted == install_id. 258 targeted tests pass; ruff + footguns clean; both staging E2E harnesses green with the raw id observed on the wire (202s). --- docs/observability/relay-shared-metrics.md | 138 +++++++------- hermes_cli/observability/shared_metrics.py | 5 +- .../observability/shared_metrics_identity.py | 131 ------------- .../observability/shared_metrics_sender.py | 36 ++-- hermes_cli/setup.py | 10 +- scripts/e2e_shared_metrics_staging.py | 12 +- .../test_shared_metrics_identity.py | 179 ------------------ .../hermes_cli/test_shared_metrics_sender.py | 14 +- .../test_shared_metrics_sender_e2e.py | 7 +- 9 files changed, 119 insertions(+), 413 deletions(-) delete mode 100644 hermes_cli/observability/shared_metrics_identity.py delete mode 100644 tests/hermes_cli/test_shared_metrics_identity.py diff --git a/docs/observability/relay-shared-metrics.md b/docs/observability/relay-shared-metrics.md index cb3ea2197a..a736bf6edd 100644 --- a/docs/observability/relay-shared-metrics.md +++ b/docs/observability/relay-shared-metrics.md @@ -230,17 +230,18 @@ packages from that profile and can therefore link those local packages. Deleting `$HERMES_HOME/telemetry/shared_metrics` resets the identifier together with all aggregates and package files. -Remote delivery is opt-in and off by default. A remote exporter must not reuse -the persistent local identifier by default. It requires a separate product and -privacy decision covering consent, identity scope, rotation or keyed -pseudonymization, reset behavior, retention, and deletion. +Remote delivery is opt-in and off by default. Reusing the persistent local +identifier remotely required a separate product and privacy decision covering +consent, identity scope, reset behavior, retention, and deletion — that +decision has been made. > Those decisions are recorded in > [Appendix A](#appendix-a-remote-exporter-decisions-phase-2), and the exporter > implementing them has shipped. Collection alone still transmits nothing: the -> sender runs only when `telemetry.shared_metrics.send` is also true, and it -> transmits a rotating HMAC of the install identity rather than the identifier -> itself. +> sender runs only when `telemetry.shared_metrics.send` is also true. Each +> transmitted package carries the stable `install_id` as-is (product decision, +> 2026-08-27 — see A.2 for the record, including the superseded +> HMAC-pseudonym design). The install identity is scoped to one `HERMES_HOME`. To reset it, stop Hermes processes and remove `$HERMES_HOME/telemetry/shared_metrics`. This deliberately @@ -327,60 +328,66 @@ Local history can be up to 30 days old, and that data was collected under a promise that nothing is uploaded. Honouring consent forward-only costs at most 30 days of backlog we never had permission to send. -### A.2 Identity scope — the transmitted identifier is derived, not the local one +### A.2 Identity scope — the stable install_id is transmitted as-is -`install_id` is the persistent profile-scoped identifier described above. It is -**not transmitted**. Each package sent carries a derived value instead: +**Decision record.** The original design of this exporter (and revisions 1–8 +of this appendix) transmitted a keyed pseudonym instead of the identifier: +`HMAC-SHA256(key = locally-held rotating salt, message = install_id)`, with +the salt rotating every 30 days. On **2026-08-27**, before the feature +shipped (zero consented users, zero production transmissions), the product +owner decided the analytical need is a **stable cross-window identity** — +retention curves, longitudinal install behaviour — which rotation by design +destroys. The pseudonymization layer was removed in full rather than +weakened in place. -```text -transmitted_id = HMAC-SHA256(key = rotation_salt, message = install_id) -``` +What is transmitted now: -- `rotation_salt` is random, generated locally, and never leaves the machine. -- The derivation is one-way: the service cannot recover `install_id`. -- Within a rotation window, packages from one profile correlate — so distinct - installs remain countable, which is the primary analytical question. -- Across windows, they do not. +- Each package carries `install_id` verbatim: the persistent, profile-scoped + random UUID described above. +- It is generated locally (`uuid4`), contains no hardware, account, user, or + machine-derived information, and identifies a *profile*, not a person. +- It is stable until the user deletes the shared-metrics directory, which + regenerates it (see A.4). -This satisfies "must not reuse the persistent local identifier by default" -while keeping the data useful. Stripping the identifier entirely was rejected -because "how many installs are reporting" is the first question the data must -answer; sending `install_id` unchanged was rejected because it contradicts the -commitment made above. +Consequences stated plainly rather than papered over: -**Byte-identical resends still hold.** The derived value is computed **once**, -when the package is first prepared for sending, and stored alongside the -package (the derived id only — not a second copy of the payload, which is -recomputed deterministically from the stored package). A retry therefore -rebuilds identical bytes even if the salt rotated in between. The contract -requires this: resending a `package_id` with different content is undefined -behaviour. +- Packages from one profile correlate **indefinitely**, not per-window. + Long-term linkability of one install's daily envelope sequence is now the + designed behaviour, not a residue. +- The A.3 residue analysis of the old design (stable `resource` tuple + + contiguous periods bridging rotation windows) is moot — there is no window + boundary left to bridge. +- The setup wizard's consent language states this identity model explicitly; + it was updated in the same change that removed the derivation, so no + consent was ever collected under the old wording in any shipped build. -### A.3 Rotation +**Byte-identical resends still hold.** The transmitted id is recorded on the +row (`sent_install_id`) when the package is first prepared, and the wire body +is always rebuilt from that recorded value, so a retry rebuilds identical +bytes. The contract requires this: resending a `package_id` with different +content is undefined behaviour. (With a stable id the recorded copy is no +longer load-bearing against rotation — it remains as the audit column and as +cheap insurance against any future change to identity semantics.) -`rotation_salt` rotates on a fixed schedule (default: every 30 days, aligned to -local history retention). Rotation only affects packages prepared after it; -already-prepared packages keep their derived value so retries stay -byte-identical. +### A.3 Rotation — removed (decision record) -Rotation bounds long-term linkability without destroying short-term cohort -analysis. A profile is one identity for the length of a window, and an -unrelated identity after it. +Salt rotation was deleted together with the derivation (product decision, +2026-08-27). This section is retained as a record of what the earlier design +did and why the removal was accepted: -**What rotation does not bound.** The identifier changes; the rest of the -envelope does not. `resource` (`os_family`, `architecture`, `install_method`, -`hermes_version`) is stable and low-entropy, and `period_start` / -`period_end` are contiguous across a rotation boundary. For a common -configuration this is no help to an observer — measured against the 11 real -packages in a development outbox, every one shares the same -`arm64 / macos / git` tuple. For a **rare** configuration it is a plausible -re-identification aid: an unusual architecture or install method, combined -with an uninterrupted daily period sequence, can bridge two windows. The -claim this design makes is therefore "rotation raises the cost of long-term -correlation", not "rotation makes it impossible". Narrowing that residue -would mean coarsening `resource` or jittering period boundaries, and neither -is worth the analytical loss today — but it should be a conscious decision, -not an unexamined one. +- Rotation existed to bound long-term linkability: one identity per 30-day + window, unrelated identities across windows. +- The documented residue (see git history for the full analysis): the + envelope's stable, low-entropy `resource` tuple plus contiguous daily + periods could plausibly bridge windows for rare configurations anyway, so + the boundary was a cost-raiser, not a wall. +- The product need that killed it: cross-window continuity is precisely what + retention analysis requires. A boundary that mostly inconveniences honest + analysis while only raising costs for a determined correlator was judged + the wrong trade once stable identity became a requirement. + +There is no salt in the store, no rotation schedule, and no derived +identifier anywhere in the pipeline. ### A.4 Reset behavior @@ -388,11 +395,11 @@ Removing `$HERMES_HOME/telemetry/shared_metrics` still resets local identity, aggregates, and package files, exactly as documented above. Two honest qualifications now apply: -- Reset also discards `rotation_salt`, so subsequent packages derive a **new** - transmitted identity. Local reset does give a new remote identity. +- Reset regenerates `install_id`, so subsequent packages transmit a **new** + identity. Local reset does give a new remote identity. - Reset **cannot unsend**. Packages already transmitted remain in the ingest - service's storage under their derived identifier. There is no read-back or - delete API in the v1 contract. + service's storage under the identifier they were sent with. There is no + read-back or delete API in the v1 contract. Setting `send: false` stops transmission immediately: consent is re-read before every package, so a pass already in flight stops after the package it @@ -439,13 +446,13 @@ invent one. What a user can do: |---|---| | `send: false` | No further packages leave the machine | | `enabled: false` | Collection stops; existing local state remains | -| Remove `.../shared_metrics` | Local identity, aggregates, and files reset; future sends use a new derived identity | +| Remove `.../shared_metrics` | Local identity, aggregates, and files reset; future sends use a new install_id | | Delete already-sent data | Not self-service — requires an operator acting on the S3 bucket | -If a deletion-on-request obligation is ever taken on, it needs a lookup path -from a user to their derived identifiers. That is deliberately **not** built: -it would require retaining the mapping this design exists to avoid. Any such -change is a new product decision, not an implementation detail. +If a deletion-on-request obligation is ever taken on, the lookup path is now +direct: the user's `install_id` (readable from their local store) is the key +their data is stored under. Building the service-side delete API remains a +new product decision, not an implementation detail. ### A.7 What the outbox directory is @@ -464,7 +471,8 @@ state they were promised. Send state lives in new columns on the ### A.8 Scope note -The `install_id` field inside the package body is what gets replaced by the -derived value. No other payload field changes, nothing is added, and the -service treats the whole body as opaque. Payload schema evolution therefore -stays a sender-side concern, as before. +The `install_id` field inside the package body is transmitted as the +generator wrote it (rewritten from the row's frozen `sent_install_id`, which +records the same value). No other payload field changes, nothing is added, +and the service treats the whole body as opaque. Payload schema evolution +therefore stays a sender-side concern, as before. diff --git a/hermes_cli/observability/shared_metrics.py b/hermes_cli/observability/shared_metrics.py index ddf570b6f9..87094922d9 100644 --- a/hermes_cli/observability/shared_metrics.py +++ b/hermes_cli/observability/shared_metrics.py @@ -373,8 +373,9 @@ class SharedMetricsStore: # Earliest next attempt; enforces backoff across process restarts. ("next_attempt_at", "TEXT"), ("last_error", "TEXT"), - # The derived identifier actually transmitted, frozen on the first - # attempt so retries stay byte-identical across a salt rotation. + # The identifier actually transmitted, frozen on the first + # attempt so retries stay byte-identical. Since the 2026-08-27 + # product decision this is the stable install_id itself. # Only the ~36-byte id is stored: the body is recomputed from # payload_json, whose serialisation is deterministic. ("sent_install_id", "TEXT"), diff --git a/hermes_cli/observability/shared_metrics_identity.py b/hermes_cli/observability/shared_metrics_identity.py deleted file mode 100644 index b4f8cda8e3..0000000000 --- a/hermes_cli/observability/shared_metrics_identity.py +++ /dev/null @@ -1,131 +0,0 @@ -"""Keyed pseudonymization of the shared-metrics install identity. - -``install_id`` is a persistent, profile-scoped identifier. It is deliberately -NOT transmitted: ``docs/observability/relay-shared-metrics.md`` commits that a -remote exporter "must not reuse the persistent local identifier by default". - -Each transmitted package instead carries:: - - HMAC-SHA256(key=rotation_salt, message=install_id) - -where ``rotation_salt`` is generated locally, never leaves the machine, and -rotates on a fixed schedule. Within a rotation window the value is stable, so -distinct installs stay countable — the primary analytical question. Across -windows it changes, bounding long-term linkability. - -The derivation is one-way: the service cannot recover ``install_id`` from what -it receives. - -See Appendix A.2 and A.3 of the doc above for the decision record. -""" - -from __future__ import annotations - -import hashlib -import hmac -import secrets -import sqlite3 -from datetime import datetime, timedelta, timezone - -#: Salt lifetime. Matches local history retention so the two ages line up. -ROTATION_INTERVAL = timedelta(days=30) - -#: ``telemetry_state`` keys. The salt lives in the same store as install_id, so -#: deleting the shared-metrics directory resets both together — the documented -#: reset behaviour keeps working without a second cleanup path. -SALT_KEY = "send_rotation_salt" -SALT_ISSUED_AT_KEY = "send_rotation_salt_issued_at" - -_SALT_BYTES = 32 - - -def _isoformat(value: datetime) -> str: - return value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") - - -def _parse(value: str | None) -> datetime | None: - if not value: - return None - try: - parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) - except ValueError: - return None - if parsed.tzinfo is None: - parsed = parsed.replace(tzinfo=timezone.utc) - return parsed.astimezone(timezone.utc) - - -def _read(connection: sqlite3.Connection, key: str) -> str | None: - row = connection.execute( - "SELECT value FROM telemetry_state WHERE key = ?", (key,) - ).fetchone() - if row is None: - return None - # sqlite3.Row and plain tuples both index by position. - return str(row[0]) - - -def _write(connection: sqlite3.Connection, key: str, value: str) -> None: - connection.execute( - """ - INSERT INTO telemetry_state(key, value) VALUES (?, ?) - ON CONFLICT(key) DO UPDATE SET value = excluded.value - """, - (key, value), - ) - - -def current_salt( - connection: sqlite3.Connection, - *, - now: datetime | None = None, -) -> str: - """Return the active salt, generating or rotating it when due. - - Must be called inside a write transaction: it can write to - ``telemetry_state``. - """ - moment = now or datetime.now(timezone.utc) - salt = _read(connection, SALT_KEY) - issued_at = _parse(_read(connection, SALT_ISSUED_AT_KEY)) - - fresh = ( - salt is not None - and issued_at is not None - # Strictly within the window. A future issued_at means the clock moved - # backwards (or the value was tampered with), so the recorded age - # cannot be trusted and we reissue rather than keep using a salt of - # unknown vintage. Reissuing is the safe direction: it shortens - # linkability, and already-prepared packages keep their frozen - # identifier so retries stay byte-identical. - and issued_at <= moment < issued_at + ROTATION_INTERVAL - ) - if fresh: - return str(salt) - - salt = secrets.token_hex(_SALT_BYTES) - _write(connection, SALT_KEY, salt) - _write(connection, SALT_ISSUED_AT_KEY, _isoformat(moment)) - return salt - - -def derive_install_id(install_id: str, salt: str) -> str: - """Return the transmitted identifier for ``install_id`` under ``salt``.""" - return hmac.new( - salt.encode("utf-8"), - install_id.encode("utf-8"), - hashlib.sha256, - ).hexdigest() - - -def substitute_install_id(payload: dict, derived: str) -> dict: - """Return ``payload`` with its ``install_id`` replaced by ``derived``. - - This is the ONLY field the exporter changes. Everything else is - transmitted exactly as the generator wrote it, so payload schema evolution - stays a sender-side concern. A shallow copy is enough — only a top-level - key is replaced — and the caller's dict is left untouched. - """ - updated = dict(payload) - updated["install_id"] = derived - return updated diff --git a/hermes_cli/observability/shared_metrics_sender.py b/hermes_cli/observability/shared_metrics_sender.py index 8e6f81c4b8..9418353c9b 100644 --- a/hermes_cli/observability/shared_metrics_sender.py +++ b/hermes_cli/observability/shared_metrics_sender.py @@ -39,12 +39,6 @@ from datetime import datetime, timedelta, timezone from hermes_cli.sqlite_util import write_txn -from .shared_metrics_identity import ( - current_salt, - derive_install_id, - substitute_install_id, -) - logger = logging.getLogger(__name__) #: Contract recommends timing out at 30s and treating a timeout as retryable. @@ -432,13 +426,17 @@ class SharedMetricsSender: payload_json, now: datetime, ) -> str | None: - """Derive and persist the transmitted id, or reject an unusable row. + """Record the transmitted id on the row, or reject an unusable one. - Returns None when the package can never be sent. Rejecting rather than - raising matters: an exception here rolls back the claim transaction - and blocks every healthy package behind this one. + The stable install_id is transmitted as-is (product decision, + 2026-08-27 — see the doc's A.2). What remains of "freezing" is the + validation and the audit column: ``sent_install_id`` records exactly + what the wire will carry, and rejecting unusable rows here rather + than raising matters because an exception rolls back the claim + transaction and blocks every healthy package behind this one. """ reason = None + install_id = None try: payload = json.loads(payload_json) except (TypeError, ValueError): @@ -467,24 +465,26 @@ class SharedMetricsSender: ) return None - salt = current_salt(connection, now=now) - derived = derive_install_id(payload["install_id"], salt) connection.execute( "UPDATE package_outbox SET sent_install_id = ? WHERE package_id = ?", - (derived, package_id), + (install_id, package_id), ) - return derived + return str(install_id) # -- transmission ------------------------------------------------------ - def _body(self, payload_json: str, derived: str) -> bytes: + def _body(self, payload_json: str, transmitted_id: str) -> bytes: """Rebuild the exact bytes to send. The payload is recomputed from the stored package rather than kept as - a second copy: json.dumps with these options is deterministic, and the - only mutable input (the derived id) is frozen in the row. + a second copy: json.dumps with these options is deterministic. The + install_id is written from the frozen ``sent_install_id`` column + rather than trusted implicitly, keeping "a resend is byte-identical" + anchored to one recorded value. """ - payload = substitute_install_id(json.loads(payload_json), derived) + payload = json.loads(payload_json) + payload = dict(payload) + payload["install_id"] = transmitted_id return json.dumps(payload, indent=2, sort_keys=True).encode("utf-8") def _mark( diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index 4772929b88..2ec08da8af 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -2464,10 +2464,12 @@ def setup_telemetry(config: dict): print_success("Local shared metrics enabled.") print_info("") print_info("Sending uploads each daily package to the Nous telemetry") - print_info("service. Your profile-scoped install ID is NOT sent: packages") - print_info("carry a rotating HMAC of it instead. Only packages from the") - print_info("day you opt in onwards are ever sent, and sending can be") - print_info("turned off again at any time.") + print_info("service. Packages carry your profile-scoped install ID, a") + print_info("stable random UUID that identifies this profile across days") + print_info("(it contains no personal information and is reset by deleting") + print_info("the shared-metrics directory). Only packages from the day you") + print_info("opt in onwards are ever sent, and sending can be turned off") + print_info("again at any time.") shared_metrics["send"] = prompt_yes_no( "Send shared metrics to Nous?", default=shared_metrics.get("send") is True, diff --git a/scripts/e2e_shared_metrics_staging.py b/scripts/e2e_shared_metrics_staging.py index e0c497a342..666c9e51e9 100644 --- a/scripts/e2e_shared_metrics_staging.py +++ b/scripts/e2e_shared_metrics_staging.py @@ -171,10 +171,12 @@ def main() -> int: print(f" last_error : {row[5]}") if row[1] != "sent": failures.append(f"{row[0]} is {row[1]}: {row[5]}") - if row[4] == real_install_id: - failures.append(f"{row[0]} LEAKED the real install_id") - if not row[4] or len(str(row[4])) != 64: - failures.append(f"{row[0]} has a malformed derived id") + # Product decision 2026-08-27: the stable install_id is transmitted + # as-is; the transmitted value must be exactly the local id. + if row[4] != real_install_id: + failures.append( + f"{row[0]} transmitted {row[4]!r}, expected the install_id" + ) print() if failures: @@ -183,7 +185,7 @@ def main() -> int: print(f" ✗ {failure}") return 1 - print("PASS: every package acknowledged 202 with a derived identifier.") + print("PASS: every package acknowledged 202 with the stable install_id.") print() print("Verify the objects in S3 with the package ids above:") print(" aws s3 ls --recursive " diff --git a/tests/hermes_cli/test_shared_metrics_identity.py b/tests/hermes_cli/test_shared_metrics_identity.py deleted file mode 100644 index 1ea1d95961..0000000000 --- a/tests/hermes_cli/test_shared_metrics_identity.py +++ /dev/null @@ -1,179 +0,0 @@ -"""Tests for keyed pseudonymization of the shared-metrics install identity. - -The load-bearing property: install_id must never be transmitted, and the -value that IS transmitted must stay stable for a package even across a salt -rotation, or a retry would change the body under an already-used package_id. -""" - -from __future__ import annotations - -import sqlite3 -from datetime import datetime, timedelta, timezone - -import pytest - -from hermes_cli.observability.shared_metrics_identity import ( - ROTATION_INTERVAL, - SALT_ISSUED_AT_KEY, - SALT_KEY, - current_salt, - derive_install_id, - substitute_install_id, -) - -INSTALL_ID = "12a73e97-4de9-4766-830d-9ca1192c0420" -T0 = datetime(2026, 8, 26, 12, 0, tzinfo=timezone.utc) - - -@pytest.fixture -def connection(): - conn = sqlite3.connect(":memory:") - conn.execute( - "CREATE TABLE telemetry_state (key TEXT PRIMARY KEY, value TEXT NOT NULL)" - ) - yield conn - conn.close() - - -class TestSaltLifecycle: - def test_first_call_generates_a_salt(self, connection): - salt = current_salt(connection, now=T0) - assert len(salt) == 64 # 32 bytes hex - assert int(salt, 16) >= 0 # valid hex - - def test_salt_is_stable_within_the_window(self, connection): - first = current_salt(connection, now=T0) - later = current_salt(connection, now=T0 + timedelta(days=29, hours=23)) - assert first == later - - def test_salt_rotates_after_the_interval(self, connection): - first = current_salt(connection, now=T0) - after = current_salt(connection, now=T0 + ROTATION_INTERVAL + timedelta(seconds=1)) - assert first != after - - def test_salt_is_persisted(self, connection): - salt = current_salt(connection, now=T0) - stored = connection.execute( - "SELECT value FROM telemetry_state WHERE key = ?", (SALT_KEY,) - ).fetchone()[0] - assert stored == salt - - def test_issued_at_is_recorded(self, connection): - current_salt(connection, now=T0) - stored = connection.execute( - "SELECT value FROM telemetry_state WHERE key = ?", (SALT_ISSUED_AT_KEY,) - ).fetchone()[0] - assert stored.startswith("2026-08-26T12:00") - - def test_two_installs_get_different_salts(self): - salts = set() - for _ in range(5): - conn = sqlite3.connect(":memory:") - conn.execute( - "CREATE TABLE telemetry_state (key TEXT PRIMARY KEY, value TEXT NOT NULL)" - ) - salts.add(current_salt(conn, now=T0)) - conn.close() - assert len(salts) == 5, "salts must be random per install, not derived" - - def test_clock_rollback_reissues_rather_than_trusting_the_stamp(self, connection): - """A future issued_at means the clock moved; the age is unknowable. - - Reissuing is the safe direction — it shortens linkability rather than - extending it, and packages already prepared keep their frozen id. - """ - first = current_salt(connection, now=T0) - rolled_back = current_salt(connection, now=T0 - timedelta(days=5)) - assert rolled_back != first - - def test_corrupt_issued_at_reissues_rather_than_crashing(self, connection): - current_salt(connection, now=T0) - connection.execute( - "UPDATE telemetry_state SET value = 'not-a-date' WHERE key = ?", - (SALT_ISSUED_AT_KEY,), - ) - assert current_salt(connection, now=T0) is not None - - -class TestDerivation: - def test_derivation_is_deterministic(self): - salt = "a" * 64 - assert derive_install_id(INSTALL_ID, salt) == derive_install_id(INSTALL_ID, salt) - - def test_derivation_hides_the_install_id(self): - derived = derive_install_id(INSTALL_ID, "a" * 64) - assert INSTALL_ID not in derived - assert derived != INSTALL_ID - - def test_different_salts_give_different_values(self): - assert derive_install_id(INSTALL_ID, "a" * 64) != derive_install_id( - INSTALL_ID, "b" * 64 - ) - - def test_different_installs_give_different_values(self): - salt = "a" * 64 - assert derive_install_id(INSTALL_ID, salt) != derive_install_id("other", salt) - - def test_output_shape_is_sha256_hex(self): - derived = derive_install_id(INSTALL_ID, "a" * 64) - assert len(derived) == 64 - int(derived, 16) - - -class TestSubstitution: - def _package(self): - return { - "schema_version": "hermes.shared_metrics.v2", - "package_id": "3a63d27e-f170-4d4c-8c4d-ebd80feac592", - "install_id": INSTALL_ID, - "generated_at": "2026-08-26T01:01:25.311956Z", - "period_start": "2026-08-26T00:00:00Z", - "period_end": "2026-08-27T00:00:00Z", - "resource": {"hermes_version": "0.20.5", "os_family": "macos"}, - "metrics": [{"name": "hermes.client.active", "type": "counter", "value": 1}], - } - - def test_install_id_is_replaced(self): - result = substitute_install_id(self._package(), "derived-value") - assert result["install_id"] == "derived-value" - - def test_no_other_field_changes(self): - original = self._package() - result = substitute_install_id(original, "derived-value") - for key in original: - if key != "install_id": - assert result[key] == original[key] - - def test_the_caller_dict_is_not_mutated(self): - original = self._package() - substitute_install_id(original, "derived-value") - assert original["install_id"] == INSTALL_ID - - def test_no_fields_are_added_or_removed(self): - original = self._package() - assert set(substitute_install_id(original, "x")) == set(original) - - def test_the_raw_install_id_never_survives_substitution(self): - import json - - body = json.dumps(substitute_install_id(self._package(), "derived-value")) - assert INSTALL_ID not in body - - -class TestRetryStability: - """The property that keeps retries contract-compliant.""" - - def test_a_frozen_derived_id_survives_a_rotation(self, connection): - salt_before = current_salt(connection, now=T0) - frozen = derive_install_id(INSTALL_ID, salt_before) - - # Time passes, the salt rotates, and the package is retried. - salt_after = current_salt(connection, now=T0 + ROTATION_INTERVAL + timedelta(days=1)) - assert salt_after != salt_before - - # Rebuilding from the FROZEN value reproduces identical bytes; deriving - # afresh would not. - assert substitute_install_id({"install_id": INSTALL_ID}, frozen) == { - "install_id": frozen - } - assert derive_install_id(INSTALL_ID, salt_after) != frozen diff --git a/tests/hermes_cli/test_shared_metrics_sender.py b/tests/hermes_cli/test_shared_metrics_sender.py index c6a7455fe2..cbaff4c9ea 100644 --- a/tests/hermes_cli/test_shared_metrics_sender.py +++ b/tests/hermes_cli/test_shared_metrics_sender.py @@ -374,15 +374,19 @@ class TestConsentGate: class TestIdentity: - def test_install_id_is_never_transmitted(self, store): + def test_the_stable_install_id_is_transmitted_as_is(self, store): + """Product decision 2026-08-27: no pseudonymization. + + The wire body carries the profile-scoped install_id verbatim. This + test is the deliberate inversion of the pre-decision assertion that + the raw id never crossed the wire. + """ _add_package(store, "pkg-1", "2026-08-26") transport = FakeTransport(FakeResponse(202)) _sender(store, transport).send_pending() - raw = transport.calls[0]["payload"].decode("utf-8") - assert INSTALL_ID not in raw - assert transport.bodies[0]["install_id"] != INSTALL_ID + assert transport.bodies[0]["install_id"] == INSTALL_ID - def test_derived_id_is_frozen_on_the_row(self, store): + def test_transmitted_id_is_frozen_on_the_row(self, store): _add_package(store, "pkg-1", "2026-08-26") transport = FakeTransport(FakeResponse(503), FakeResponse(202)) _sender(store, transport).send_pending() diff --git a/tests/hermes_cli/test_shared_metrics_sender_e2e.py b/tests/hermes_cli/test_shared_metrics_sender_e2e.py index 9ff8bf9caf..85be9b2388 100644 --- a/tests/hermes_cli/test_shared_metrics_sender_e2e.py +++ b/tests/hermes_cli/test_shared_metrics_sender_e2e.py @@ -171,12 +171,11 @@ class TestRealTransport: ).fetchone()[0] assert state == "sent" - def test_the_install_id_never_crosses_the_wire(self, store, server): + def test_the_stable_install_id_crosses_the_wire_as_is(self, store, server): + """Product decision 2026-08-27: the raw install_id is transmitted.""" _add(store, "pkg-1", metrics=40) _sender(store, server).send_pending() - body = json.dumps(Ingest.received[0]["body"]) - assert INSTALL_ID not in body - assert len(Ingest.received[0]["body"]["install_id"]) == 64 + assert Ingest.received[0]["body"]["install_id"] == INSTALL_ID def test_content_type_is_json(self, store, server): _add(store, "pkg-1")