diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx index cda386b60..c224b2f0b 100644 --- a/docs/contributing/canary-rollouts.mdx +++ b/docs/contributing/canary-rollouts.mdx @@ -116,9 +116,11 @@ during the window*, not the instantaneous rate. Two practical consequences: far tighter than a 60-day one. - **The percentage is not the safety mechanism.** It bounds *initial* exposure and decays from there. Per-render verification and per-install circuit - breakers are what actually bound harm — and note that a breaker's state - lives in the same config file as the id, so a wipe loses both and the - install can re-enrol into a path that already failed it. + breakers are what actually bound harm. A breaker's tripped state lives in + `install-state.json`, separate from `config.json` and merged back in on every + read, so a `config.json` re-mint cannot re-enrol an install into a path that + already failed it. Deleting `~/.hyperframes` clears both, deliberately — + that is the user's reset. For *measurement* — "is this feature better?" — churn is harmless: re-bucketing is random, so it adds noise, not bias. It is specifically the blast-radius @@ -223,14 +225,33 @@ bucketing unit is a dedicated seed in `install-state.json`, inherited across telemetry id and never emitted. It does not outlive `~/.hyperframes`. **A CLI-launched Studio adopts the CLI's decisions rather than re-deriving -them.** The CLI publishes `window.__HF_CLI_CANARY_DECISIONS` — a plain -`{ name: boolean }` map — and Studio takes it as authoritative over its own -seed, URL override and the registry percentage. Re-deriving cannot agree in -the cases that matter: telemetry off (the CLI resolves `telemetry_opt_out`, -but Studio's opt-out is a separate localStorage flag it cannot see), an -`HF_CANARY_*` override (env vars never cross into the browser), or no seed -injected (Studio falls back to a different unit, so a different bucket). One -render spanning both surfaces must not run half-enrolled. +them.** The CLI publishes `window.__HF_CLI_CANARY_DECISIONS` — a +`{ name: { enabled, forced } }` map. Re-deriving cannot agree in the cases +that matter: telemetry off (the CLI resolves `telemetry_opt_out`, but Studio's +opt-out is a separate localStorage flag it cannot see), an `HF_CANARY_*` +override (env vars never cross into the browser), or no seed injected (Studio +falls back to a different unit, so a different bucket). One render spanning +both surfaces must not run half-enrolled. + +`forced` carries the provenance, and the precedence follows from it — highest +first: + +1. A **forced** CLI decision (`HF_CANARY_*`). Wins outright, including over + this profile's opt-out, exactly as a local URL override does. +2. A local `?hf_canary_*=` override, same reasoning. +3. **This profile's telemetry opt-out.** Checked before any percentage + decision: the two surfaces have independent opt-outs, and CLI telemetry + being on says nothing about whether this browser profile agreed to be + measured. A cohort roll must never enrol an opted-out profile. +4. The CLI's percentage decision. +5. Local evaluation (standalone Studio, or a canary the CLI did not publish). + +Identity is treated differently from decisions. `__HF_CLI_DISTINCT_ID` and +`__HF_CLI_BUCKET_SEED` are injected only for a loopback `Host` — a page that +rebinds its hostname to `127.0.0.1` would otherwise read them as same-origin. +The decisions map is not identifying, so it is published regardless, which +keeps a LAN preview (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`) in agreement with the +CLI instead of silently re-deriving. The decisions map is published even when telemetry is off — that is the case it exists for. It is safe to expose where the seed is not: booleans about diff --git a/packages/cli/src/server/studioServer.test.ts b/packages/cli/src/server/studioServer.test.ts index cb5ce853c..f11d0dee8 100644 --- a/packages/cli/src/server/studioServer.test.ts +++ b/packages/cli/src/server/studioServer.test.ts @@ -74,22 +74,11 @@ describe("host guarding on identity-bearing responses", () => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); - // A rebound origin can point its own hostname at 127.0.0.1 and read - // responses as same-origin. Guarding only /api/telemetry-identity left the - // SPA route as an open side door: fetching `/` returned the same distinct - // id and bucket seed inline in the HTML. - it("omits identity injection from the SPA response for a hostile Host", async () => { - server = createStudioServer({ projectDir: tmpProject() }); - const res = await server.app.request("/", { headers: { host: "evil.example.com" } }); - const html = await res.text(); - expect(html).not.toContain("__HF_CLI_DISTINCT_ID"); - expect(html).not.toContain("__HF_CLI_BUCKET_SEED"); - expect(html).not.toContain("__HF_CLI_CANARY_DECISIONS"); - // Studio still loads — only the identity block is withheld. (The env - // script is empty here: it only emits with VITE_STUDIO_* vars set.) - expect(res.status).toBe(200); - expect(html).toContain(""); - }); + // NOTE: the SPA-injection branch itself is covered in telemetryIdentity.test.ts + // via buildStudioHeadScriptsForHost. It cannot be asserted here: this route + // only reaches the injection branch when packages/studio/dist is built, + // which is true locally and false in the CI test lane, so a route-level + // assertion on the returned HTML passes on a dev box and fails in CI. it("refuses the identity endpoint for a hostile Host", async () => { server = createStudioServer({ projectDir: tmpProject() }); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 210ce82f7..e026679c4 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -17,7 +17,7 @@ import { } from "./runtimeSource.js"; import { VERSION as version } from "../version.js"; import { - buildStudioHeadScripts, + buildStudioHeadScriptsForHost, isLoopbackHost, resolveCliTelemetryDistinctId, } from "./telemetryIdentity.js"; @@ -818,13 +818,13 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // Host-guarded for the same reason /api/telemetry-identity is, and it has // to be checked HERE too: guarding only the endpoint leaves this route as // an open side door, since a rebound origin can simply fetch `/` and read - // the same distinct id and seed out of the returned HTML. Untrusted Host - // still gets a working Studio — it just gets the env script alone, with no - // identity, no seed, and no canary decisions. - const trustedHost = isLoopbackHost(c.req.header("host")); - const headScript = trustedHost - ? buildStudioHeadScripts(buildRuntimeEnvScript()) - : buildRuntimeEnvScript(); + // the same distinct id and seed out of the returned HTML. + // + // Only IDENTITY is withheld from an untrusted Host. The canary decisions + // map still goes out — it is non-identifying, and a LAN/remote Studio + // (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`) needs it to stay in agreement with + // the CLI. See buildStudioHeadScriptsForHost. + const headScript = buildStudioHeadScriptsForHost(buildRuntimeEnvScript(), c.req.header("host")); if (headScript) { html = html.replace("", `${headScript}`); } diff --git a/packages/cli/src/server/telemetryIdentity.test.ts b/packages/cli/src/server/telemetryIdentity.test.ts index 4599fa3b3..83ea864c1 100644 --- a/packages/cli/src/server/telemetryIdentity.test.ts +++ b/packages/cli/src/server/telemetryIdentity.test.ts @@ -25,6 +25,7 @@ const { buildCliIdentityScript, buildStudioHeadScripts, isLoopbackHost, + buildStudioHeadScriptsForHost, } = await import("./telemetryIdentity.js"); describe("resolveCliTelemetryDistinctId", () => { @@ -206,3 +207,47 @@ describe("isLoopbackHost (DNS-rebinding guard on the identity endpoint)", () => expect(isLoopbackHost(host)).toBe(false); }); }); + +describe("buildStudioHeadScriptsForHost — Host split", () => { + const ENV = ""; + + beforeEach(() => { + shouldTrack.mockReset(); + readConfig.mockReset(); + canaryDecisions.mockReset(); + shouldTrack.mockReturnValue(true); + readConfig.mockReturnValue({ anonymousId: "machine-uuid", bucketSeed: "seed-uuid" }); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: true, forced: false } }); + }); + + it("publishes identity and decisions on a loopback Host", () => { + const head = buildStudioHeadScriptsForHost(ENV, "127.0.0.1:5173"); + expect(head).toContain("__HF_CLI_DISTINCT_ID"); + expect(head).toContain("__HF_CLI_BUCKET_SEED"); + expect(head).toContain("__HF_CLI_CANARY_DECISIONS"); + }); + + // DNS rebinding: identity must not be readable from a hostile origin. + it("withholds identity and seed from a hostile Host", () => { + const head = buildStudioHeadScriptsForHost(ENV, "evil.example.com"); + expect(head).not.toContain("__HF_CLI_DISTINCT_ID"); + expect(head).not.toContain("__HF_CLI_BUCKET_SEED"); + }); + + // ...but the decisions map is NOT identifying, and withholding it would send + // a supported LAN preview (HYPERFRAMES_PREVIEW_HOST=0.0.0.0) back to + // re-deriving locally and disagreeing with the CLI. + it.each(["evil.example.com", "192.168.1.10:5173", "my-dev-box.local:5173", undefined])( + "still publishes canary decisions for non-loopback Host %s", + (host) => { + const head = buildStudioHeadScriptsForHost(ENV, host); + expect(head).toContain("__HF_CLI_CANARY_DECISIONS"); + expect(head).not.toContain("__HF_CLI_DISTINCT_ID"); + }, + ); + + it("always keeps the env script, whatever the Host", () => { + expect(buildStudioHeadScriptsForHost(ENV, "evil.example.com")).toContain("__HF_STUDIO_ENV__"); + expect(buildStudioHeadScriptsForHost(ENV, "localhost")).toContain("__HF_STUDIO_ENV__"); + }); +}); diff --git a/packages/cli/src/server/telemetryIdentity.ts b/packages/cli/src/server/telemetryIdentity.ts index ad53b54a7..f8d027274 100644 --- a/packages/cli/src/server/telemetryIdentity.ts +++ b/packages/cli/src/server/telemetryIdentity.ts @@ -119,10 +119,16 @@ function resolveCliCanaryDecisions(): Record | null { * or browser history. Empty string only when there is nothing at all to * publish. */ -export function buildCliIdentityScript(): string { +export function buildCliIdentityScript(options: { includeIdentity?: boolean } = {}): string { + const { includeIdentity = true } = options; const parts: string[] = []; - const cliId = resolveCliTelemetryDistinctId(); + // Identity is the only part gated on a trusted Host. The decisions map below + // is not identifying, and withholding it would push a LAN/remote Studio + // (`HYPERFRAMES_PREVIEW_HOST=0.0.0.0`, an explicitly supported mode) back to + // re-deriving locally — reopening exactly the CLI/Studio disagreement this + // whole mechanism exists to close. + const cliId = includeIdentity ? resolveCliTelemetryDistinctId() : null; if (cliId) { parts.push(`window.__HF_CLI_DISTINCT_ID=${encodeInlineScriptValue(cliId)};`); const seed = resolveCliBucketSeed(); @@ -154,6 +160,21 @@ export function buildCliIdentityScript(): string { * ordering in one pure, tested function guards against a future `` inject * silently landing ahead of the identity script and reintroducing a boot race. */ -export function buildStudioHeadScripts(envScript: string): string { - return `${buildCliIdentityScript()}${envScript}`; +export function buildStudioHeadScripts( + envScript: string, + options: { includeIdentity?: boolean } = {}, +): string { + return `${buildCliIdentityScript(options)}${envScript}`; +} + +/** + * The `` scripts for a request, given its `Host`. + * + * The Host split lives here rather than in the route so it is testable + * without a Studio bundle on disk — the route's own test can only reach the + * injection branch when `packages/studio/dist` happens to be built, which is + * true locally and false in the CI test lane. + */ +export function buildStudioHeadScriptsForHost(envScript: string, host: string | undefined): string { + return buildStudioHeadScripts(envScript, { includeIdentity: isLoopbackHost(host) }); } diff --git a/packages/cli/src/telemetry/config.test.ts b/packages/cli/src/telemetry/config.test.ts index 9f0dd1c29..09b42ff46 100644 --- a/packages/cli/src/telemetry/config.test.ts +++ b/packages/cli/src/telemetry/config.test.ts @@ -494,3 +494,62 @@ describe("a tripped breaker survives even total marker+seed corruption", () => { expect(readConfig().stateFileCorrupt).toBe(true); }); }); + +describe("the breaker latch is authoritative from install-state", () => { + let readConfig: typeof import("./config.js").readConfig; + let writeConfigWithResult: typeof import("./config.js").writeConfigWithResult; + let CONFIG_PATH: typeof import("./config.js").CONFIG_PATH; + let STATE_PATH: typeof import("./config.js").STATE_PATH; + + beforeEach(async () => { + fsState.files.clear(); + vi.resetModules(); + ({ readConfig, writeConfigWithResult, CONFIG_PATH, STATE_PATH } = await import("./config.js")); + }); + + // The stale-writer race: a concurrent process rewrites config.json from a + // snapshot taken before the breaker fired, clearing the flag there. Without + // merging the latch on read, the next process re-enrols a machine whose + // router already failed. + it("rehydrates a latched breaker when config.json says otherwise", () => { + fsState.files.set( + STATE_PATH, + JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z", deParallelRouterTrialFired: true }), + ); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + expect(readConfig().deParallelRouterTrialFired).toBe(true); + }); + + it("does not invent a latch when neither store has one", () => { + fsState.files.set(STATE_PATH, JSON.stringify({ markerAt: "2026-07-28T00:00:00.000Z" })); + fsState.files.set( + CONFIG_PATH, + JSON.stringify({ telemetryEnabled: true, anonymousId: "id", bucketSeed: "seed" }), + ); + expect(readConfig().deParallelRouterTrialFired).toBeUndefined(); + }); + + it("reports mirrored:false when the state write fails but config.json lands", async () => { + const config = readConfig(); + const fs = await import("node:fs"); + const { __resetInstallStateSyncForTests } = await import("./config.js"); + __resetInstallStateSyncForTests(); + fsState.files.delete(STATE_PATH); + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + if (String(path).startsWith(STATE_PATH)) throw new Error("EACCES"); + fsState.files.set(String(path), String(content)); + }); + + config.deParallelRouterTrialFired = true; + // The config write still succeeds — a failed mirror must never break it — + // but the caller can now see the safety fact reached only one store. + expect(writeConfigWithResult(config)).toEqual({ ok: true, mirrored: false }); + + vi.mocked(fs.writeFileSync).mockImplementation((path, content) => { + fsState.files.set(String(path), String(content)); + }); + }); +}); diff --git a/packages/cli/src/telemetry/config.ts b/packages/cli/src/telemetry/config.ts index ffbbb365f..30eb92451 100644 --- a/packages/cli/src/telemetry/config.ts +++ b/packages/cli/src/telemetry/config.ts @@ -147,6 +147,29 @@ function backfillBucketSeed(config: HyperframesConfig): void { if (!write.ok) warnSeedBackfillFailed(write.error); } +// The latch is monotonic — once tripped it never untrips — so one read per +// process is enough, and readConfig is hot (every command, every render). +let latchFromStateFile: boolean | undefined; + +/** + * Is the breaker latched according to install-state? + * + * Merged into EVERY effective read, which is what makes install-state + * authoritative for the latch rather than merely a mirror of config.json. + * Without this a failed mirror, or a stale concurrent writer that rewrites + * config.json from a pre-trip snapshot, leaves state latched and config + * false — and the next process happily re-enrols a machine whose router + * already failed. Verifying the write is not enough on its own; the read has + * to prefer the safety fact. + */ +function installStateLatchedFired(): boolean { + if (latchFromStateFile === undefined) { + const state = readInstallState(); + latchFromStateFile = isInstallState(state) && state.deParallelRouterTrialFired === true; + } + return latchFromStateFile; +} + /** Narrow the parse result to a usable record. */ function isInstallState(value: InstallState | InstallStateMiss): value is InstallState { return typeof value !== "string"; @@ -249,18 +272,27 @@ function nextInstallState( return next; } -function syncInstallState(config: HyperframesConfig): void { +/** @returns false if the mirror could not be written this call. */ +/** The write half, split out to keep the memo bookkeeping legible. */ +function applyInstallState(config: HyperframesConfig, wantFired: boolean): void { + const read = readInstallState(); + const state = isInstallState(read) ? read : null; + const next = nextInstallState(state, config); + if (next !== null) writeInstallState(next); + stateMarkerSynced = true; + stateFiredSynced = wantFired || state?.deParallelRouterTrialFired === true; + if (wantFired) latchFromStateFile = true; +} + +function syncInstallState(config: HyperframesConfig): boolean { const wantFired = config.deParallelRouterTrialFired === true; - if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return; + if (stateMarkerSynced && (stateFiredSynced || !wantFired)) return true; try { - const read = readInstallState(); - const state = isInstallState(read) ? read : null; - const next = nextInstallState(state, config); - if (next !== null) writeInstallState(next); - stateMarkerSynced = true; - stateFiredSynced = wantFired || state?.deParallelRouterTrialFired === true; + applyInstallState(config, wantFired); + return true; } catch { // Leave the memo unset so a later write retries. + return false; } } @@ -504,7 +536,10 @@ export function readConfig(): HyperframesConfig { // non-boolean/non-number JSON value (e.g. the STRING "false", which is // truthy in JS) for these two fields specifically, since they're read // with a bare truthy check at the call site (review finding). - deParallelRouterTrialFired: parsed.deParallelRouterTrialFired === true ? true : undefined, + // `|| installStateLatchedFired()` — a latch recorded in install-state + // wins over an untripped config.json, never the reverse. + deParallelRouterTrialFired: + parsed.deParallelRouterTrialFired === true || installStateLatchedFired() ? true : undefined, deParallelRouterTrialRenderCount: typeof parsed.deParallelRouterTrialRenderCount === "number" ? parsed.deParallelRouterTrialRenderCount @@ -575,7 +610,11 @@ export function writeConfig(config: HyperframesConfig): boolean { return writeConfigWithResult(config).ok; } -export type ConfigWriteResult = { ok: true } | { ok: false; error: string }; +export type ConfigWriteResult = + // `mirrored: false` means config.json landed but the install-state mirror + // did not. Not a write failure — the latch is merged back in on read — but + // callers persisting a tripped breaker may want to retry or warn. + { ok: true; mirrored?: false } | { ok: false; error: string }; /** * Persist config and retain the failure reason for user-facing commands that @@ -589,9 +628,13 @@ export function writeConfigWithResult(config: HyperframesConfig): ConfigWriteRes renameSync(tmpFile, CONFIG_FILE); cachedConfig = { ...config }; // Mirror into the install-state file (marker + tripped breaker) so no - // breaker write site has to remember to do it. - syncInstallState(config); - return { ok: true }; + // breaker write site has to remember to do it. A failed mirror does NOT + // fail the config write — config.json landed, and the latch is merged + // back in on read — but it is reported rather than swallowed so a caller + // persisting a tripped breaker can tell the safety fact only reached one + // of the two stores. + const mirrored = syncInstallState(config); + return mirrored ? { ok: true } : { ok: true, mirrored: false }; } catch (error) { // Non-fatal — telemetry should never break the CLI return { ok: false, error: normalizeErrorMessage(error) }; diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts index c6bc0c5d0..802a4aaf2 100644 --- a/packages/studio/src/telemetry/canary.test.ts +++ b/packages/studio/src/telemetry/canary.test.ts @@ -6,6 +6,14 @@ import { evaluateCanary } from "@hyperframes/core/canary"; // Pin the registry: real entries move as rollouts ramp, and these tests are // about the BINDING (does the browser supply the right three inputs?), not // about whichever canaries happen to be live today. +// The policy reads import.meta.env.DEV, which vitest sets true — without +// this every case would resolve to telemetry_opt_out. Controlled explicitly +// so each test states the privacy posture it is exercising. +const policyState = { allowed: true }; +vi.mock("./policy", () => ({ + browserTelemetryAllowed: () => policyState.allowed, +})); + vi.mock("@hyperframes/core/canary-registry", async () => { const actual = await vi.importActual( "@hyperframes/core/canary-registry", @@ -43,6 +51,7 @@ function setSearch(search: string): void { } beforeEach(() => { + policyState.allowed = true; localStorage.clear(); sessionStorage.clear(); setSearch(""); @@ -198,6 +207,7 @@ describe("telemetry opt-out is canary opt-out", () => { const OPT_OUT_KEY = "hyperframes-studio:telemetryDisabled"; it("does not enrol an opted-out browser profile", () => { + policyState.allowed = false; localStorage.setItem(OPT_OUT_KEY, "1"); // on-everywhere is at 100% — it would be on for everyone otherwise. expect(resolveCanary("on-everywhere")).toEqual({ @@ -207,17 +217,20 @@ describe("telemetry opt-out is canary opt-out", () => { }); it("never buckets an opted-out profile — no cohort is assigned at all", () => { + policyState.allowed = false; localStorage.setItem(OPT_OUT_KEY, "1"); expect(resolveCanary("on-everywhere").bucket).toBeUndefined(); }); it("still honours an explicit URL override", () => { + policyState.allowed = false; localStorage.setItem(OPT_OUT_KEY, "1"); setSearch("?hf_canary_off_everywhere=on"); expect(resolveCanary("off-everywhere")).toEqual({ enabled: true, reason: "forced_on" }); }); it("reports every canary as false when opted out", () => { + policyState.allowed = false; localStorage.setItem(OPT_OUT_KEY, "1"); expect(canaryEventProperties()).toEqual({ "$feature/canary-on-everywhere": "false", @@ -281,6 +294,7 @@ describe("CLI-launched Studio adopts the CLI's decisions", () => { // opt-outs, and CLI telemetry being on says nothing about this profile. describe("precedence against Studio's own opt-out", () => { beforeEach(() => { + policyState.allowed = false; localStorage.setItem(OPT_OUT_KEY, "1"); }); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts index a8ef6a644..8c90f8088 100644 --- a/packages/studio/src/telemetry/canary.ts +++ b/packages/studio/src/telemetry/canary.ts @@ -41,7 +41,7 @@ import { } from "@hyperframes/core/canary"; import { CANARIES, findCanary } from "@hyperframes/core/canary-registry"; import { resolveStudioDistinctId } from "./distinctId"; -import { isOptedOut } from "./config"; +import { browserTelemetryAllowed } from "./policy"; import { safeSessionStorage } from "../utils/safeStorage"; /** `my-feature` → `hf_canary_my_feature`, the query param and storage key. */ @@ -212,7 +212,7 @@ function decideStudioCanary(name: string): CanaryDecision { const override = readOverride(definition.name); if (override === undefined) { - if (isOptedOut()) return { enabled: false, reason: "telemetry_opt_out" }; + if (!browserTelemetryAllowed()) return { enabled: false, reason: "telemetry_opt_out" }; if (fromCli !== undefined) return cohortOutcome(fromCli.enabled); } diff --git a/packages/studio/src/telemetry/client.ts b/packages/studio/src/telemetry/client.ts index f43aa3486..511571bb4 100644 --- a/packages/studio/src/telemetry/client.ts +++ b/packages/studio/src/telemetry/client.ts @@ -4,7 +4,8 @@ // All calls are fire-and-forget; telemetry must never break the studio UI. // --------------------------------------------------------------------------- -import { getAnonymousId, hasShownNotice, isOptedOut, markNoticeShown } from "./config"; +import { getAnonymousId, hasShownNotice, markNoticeShown } from "./config"; +import { browserTelemetryAllowed } from "./policy"; import { getBrowserSystemMeta } from "./system"; import { canaryEventProperties } from "./canary"; @@ -25,46 +26,11 @@ let eventQueue: QueuedEvent[] = []; let flushTimer: ReturnType | null = null; let telemetryEnabled: boolean | null = null; -function isDoNotTrackOn(): boolean { - return typeof navigator !== "undefined" && navigator.doNotTrack === "1"; -} - -function isApiKeyConfigured(): boolean { - return POSTHOG_API_KEY.startsWith("phc_"); -} - -// VITE_HYPERFRAMES_NO_TELEMETRY mirrors the CLI's HYPERFRAMES_NO_TELEMETRY=1 -// opt-out so HeyGen's own dev/CI builds can suppress telemetry from the studio -// bundle the same way. Vite injects it at build time. Match the CLI's -// affirmative privacy-control spellings. -// `import.meta.env` may be undefined in non-Vite bundlers (Next.js Turbopack). -function isBuildTimeOptOut(): boolean { - try { - const v = import.meta.env.VITE_HYPERFRAMES_NO_TELEMETRY as string | undefined; - return v !== undefined && ["1", "true", "yes", "on"].includes(v.trim().toLowerCase()); - } catch { - return false; - } -} - -// `import.meta.env.DEV` is true under `vite dev` / `vite preview`. Auto-suppress -// so developers running `hyperframes preview` don't pollute production telemetry. -function isViteDevMode(): boolean { - try { - return import.meta.env.DEV === true; - } catch { - return false; - } -} - export function shouldTrack(): boolean { if (telemetryEnabled !== null) return telemetryEnabled; - telemetryEnabled = - isApiKeyConfigured() && - !isBuildTimeOptOut() && - !isViteDevMode() && - !isOptedOut() && - !isDoNotTrackOn(); + // Delegated to telemetry/policy.ts so this transport, the older `studio:*` + // transport, and canary enrolment cannot drift apart again. + telemetryEnabled = browserTelemetryAllowed(); return telemetryEnabled; } diff --git a/packages/studio/src/telemetry/policy.test.ts b/packages/studio/src/telemetry/policy.test.ts new file mode 100644 index 000000000..e11bbbfae --- /dev/null +++ b/packages/studio/src/telemetry/policy.test.ts @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Each control the browser telemetry policy enforces, asserted individually. +// This is the SSOT that `telemetry/client.ts`, `utils/studioTelemetry.ts` and +// canary enrolment all consult, so a gap here is a gap in all three — which is +// how `navigator.doNotTrack` and Vite dev mode came to suppress one transport +// but not the other, nor enrolment. + +const DOCUMENTED_OPT_OUT = "hyperframes-studio:telemetryDisabled"; +const LEGACY_OPT_OUT = "hf-studio-telemetry-opt-out"; + +describe("browserTelemetryAllowed", () => { + let browserTelemetryAllowed: typeof import("./policy").browserTelemetryAllowed; + + beforeEach(async () => { + localStorage.clear(); + vi.resetModules(); + // vitest sets import.meta.env.DEV; the policy suppresses under it, so the + // baseline has to be an explicitly production-like env. + vi.stubEnv("DEV", false); + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", ""); + Object.defineProperty(navigator, "doNotTrack", { value: null, configurable: true }); + ({ browserTelemetryAllowed } = await import("./policy")); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("allows telemetry with no control set", () => { + expect(browserTelemetryAllowed()).toBe(true); + }); + + it("refuses when the documented localStorage key is set", () => { + localStorage.setItem(DOCUMENTED_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); + + // Anyone already opted out this way must never be quietly re-enabled by the + // move to the documented key. + it("refuses when the legacy localStorage key is set", () => { + localStorage.setItem(LEGACY_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it("refuses when navigator.doNotTrack is on", () => { + Object.defineProperty(navigator, "doNotTrack", { value: "1", configurable: true }); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it("refuses under Vite dev mode", async () => { + vi.stubEnv("DEV", true); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(false); + }); + + it.each(["1", "true", "yes", "on", " ON "])( + "refuses when VITE_HYPERFRAMES_NO_TELEMETRY=%s", + async (value) => { + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", value); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(false); + }, + ); + + it("ignores an unset or unrelated VITE_HYPERFRAMES_NO_TELEMETRY value", async () => { + vi.stubEnv("VITE_HYPERFRAMES_NO_TELEMETRY", "0"); + vi.resetModules(); + ({ browserTelemetryAllowed } = await import("./policy")); + expect(browserTelemetryAllowed()).toBe(true); + }); + + it("is not memoized — a mid-session opt-out takes effect immediately", () => { + expect(browserTelemetryAllowed()).toBe(true); + localStorage.setItem(DOCUMENTED_OPT_OUT, "1"); + expect(browserTelemetryAllowed()).toBe(false); + }); +}); diff --git a/packages/studio/src/telemetry/policy.ts b/packages/studio/src/telemetry/policy.ts new file mode 100644 index 000000000..89f42ff71 --- /dev/null +++ b/packages/studio/src/telemetry/policy.ts @@ -0,0 +1,93 @@ +// --------------------------------------------------------------------------- +// Browser telemetry policy — the single answer to "may this profile be +// measured?", shared by every transport and by canary enrolment. +// +// This exists because the answer was previously duplicated and the copies had +// drifted: `telemetry/client.ts` enforced five controls, the older +// `utils/studioTelemetry.ts` transport enforced one (its own localStorage +// key), and canary evaluation enforced a different one. So a profile with +// `navigator.doNotTrack` set, or a Vite dev build, still emitted `studio:*` +// events AND could be bucketed into a rollout — under controls the public +// docs say disable both. +// +// Deliberately imports only `./config` (localStorage helpers). Nothing here +// may import a transport or the canary module: both of those import this, and +// the whole point is one definition with no cycle. +// --------------------------------------------------------------------------- + +import { isOptedOut } from "./config"; + +// Write-only PostHog project key, safe to embed in client code. Duplicated +// from client.ts intentionally — the eligibility check must not drag the +// transport (and its queue/timer state) into modules that only need the +// policy. +const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; + +/** Legacy opt-out key predating `telemetry/config.ts`. Still honoured so + * anyone already opted out is never quietly re-enabled. */ +const LEGACY_OPT_OUT_KEY = "hf-studio-telemetry-opt-out"; + +function isLegacyOptedOut(): boolean { + try { + return localStorage.getItem(LEGACY_OPT_OUT_KEY) === "1"; + } catch { + return false; + } +} + +function isDoNotTrackOn(): boolean { + return typeof navigator !== "undefined" && navigator.doNotTrack === "1"; +} + +function isApiKeyConfigured(): boolean { + return POSTHOG_API_KEY.startsWith("phc_"); +} + +// VITE_HYPERFRAMES_NO_TELEMETRY mirrors the CLI's HYPERFRAMES_NO_TELEMETRY=1 +// opt-out so HeyGen's own dev/CI builds can suppress telemetry from the studio +// bundle the same way. Vite injects it at build time. Match the CLI's +// affirmative privacy-control spellings. +// `import.meta.env` may be undefined in non-Vite bundlers (Next.js Turbopack). +function isBuildTimeOptOut(): boolean { + try { + const v = import.meta.env.VITE_HYPERFRAMES_NO_TELEMETRY as string | undefined; + return v !== undefined && ["1", "true", "yes", "on"].includes(v.trim().toLowerCase()); + } catch { + return false; + } +} + +// `import.meta.env.DEV` is true under `vite dev` / `vite preview`. Auto-suppress +// so developers running `hyperframes preview` don't pollute production telemetry. +function isViteDevMode(): boolean { + try { + return import.meta.env.DEV === true; + } catch { + return false; + } +} + +/** + * May this browser profile be measured at all? + * + * Governs BOTH sending events and canary enrolment. Enrolment is part of + * measurement, not separate from it: a profile that reports nothing cannot be + * compared against anyone, so bucketing it changes that user's code path for + * no signal. The one documented exception is an explicit `HF_CANARY_*` / + * `?hf_canary_*=` override, which callers apply before consulting this. + * + * Not memoized — `isOptedOut()` reads localStorage, which a user can flip in + * DevTools mid-session, and the per-call cost is a couple of property reads. + * Callers that must stay stable within a session memoize their own result + * (canary decisions do; the transports intentionally do not). + */ +export function browserTelemetryAllowed(): boolean { + return ( + isApiKeyConfigured() && + !isBuildTimeOptOut() && + !isViteDevMode() && + !isOptedOut() && + !isLegacyOptedOut() && + !isDoNotTrackOn() + ); +} diff --git a/packages/studio/src/utils/studioTelemetry.test.ts b/packages/studio/src/utils/studioTelemetry.test.ts index a50b0bbb6..b7eb42cf6 100644 --- a/packages/studio/src/utils/studioTelemetry.test.ts +++ b/packages/studio/src/utils/studioTelemetry.test.ts @@ -6,18 +6,24 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; // canary work established: the documented opt-out did not silence it, and its // events carried no cohort assignment. These pin both. -const DOCUMENTED_OPT_OUT = "hyperframes-studio:telemetryDisabled"; -const LEGACY_OPT_OUT = "hf-studio-telemetry-opt-out"; - vi.mock("../telemetry/canary", () => ({ canaryEventProperties: () => ({ "$feature/canary-test-one": "true" }), })); +// One shared policy now governs this transport, telemetry/client.ts and +// canary enrolment. Exercised directly here so each case names the control +// under test rather than relying on ambient import.meta.env. +const policyState = { allowed: true }; +vi.mock("../telemetry/policy", () => ({ + browserTelemetryAllowed: () => policyState.allowed, +})); + describe("studioTelemetry — shared opt-out and canary properties", () => { let trackStudioEvent: typeof import("./studioTelemetry").trackStudioEvent; let fetchMock: ReturnType; beforeEach(async () => { + policyState.allowed = true; localStorage.clear(); vi.resetModules(); vi.useFakeTimers(); @@ -40,19 +46,15 @@ describe("studioTelemetry — shared opt-out and canary properties", () => { return parsed.batch ?? []; } - it("honours the documented opt-out key", async () => { - // Previously only the legacy key was checked, so a user who opted out the - // documented way kept emitting every `studio:*` event. - localStorage.setItem(DOCUMENTED_OPT_OUT, "1"); - trackStudioEvent("thing_happened"); - expect(await sentEvents()).toHaveLength(0); - }); - - it("still honours the legacy opt-out key", async () => { - // Anyone already opted out this way must not be quietly re-enabled. - localStorage.setItem(LEGACY_OPT_OUT, "1"); + // Every control the shared policy enforces — documented key, legacy key, + // navigator.doNotTrack, VITE_HYPERFRAMES_NO_TELEMETRY, Vite dev mode, API + // key eligibility. Before the policy was shared this transport honoured + // only the legacy key, so all of the others still emitted `studio:*`. + it("sends nothing when the shared policy refuses", async () => { + policyState.allowed = false; trackStudioEvent("thing_happened"); expect(await sentEvents()).toHaveLength(0); + expect(fetchMock).not.toHaveBeenCalled(); }); it("attaches canary assignments to every event", async () => { diff --git a/packages/studio/src/utils/studioTelemetry.ts b/packages/studio/src/utils/studioTelemetry.ts index 83b4fe2ed..373127d0c 100644 --- a/packages/studio/src/utils/studioTelemetry.ts +++ b/packages/studio/src/utils/studioTelemetry.ts @@ -1,5 +1,5 @@ import { resolveStudioDistinctId } from "../telemetry/distinctId"; -import { isOptedOut } from "../telemetry/config"; +import { browserTelemetryAllowed } from "../telemetry/policy"; import { canaryEventProperties } from "../telemetry/canary"; // PostHog public ingest key — write-only, safe to ship in the client bundle @@ -29,21 +29,14 @@ function getDistinctId(): string { } /** - * Honours BOTH opt-out keys. - * - * This path predates telemetry/config.ts and shipped its own key, so the - * documented `hyperframes-studio:telemetryDisabled` was silently ignored here - * — `studio:*` events kept flowing for anyone who opted out the documented - * way. The legacy key stays honoured so nobody who already opted out gets - * quietly re-enabled by this fix. + * This path predates telemetry/config.ts and enforced only its own + * localStorage key, so `navigator.doNotTrack`, VITE_HYPERFRAMES_NO_TELEMETRY, + * Vite dev mode and the documented `hyperframes-studio:telemetryDisabled` all + * failed to silence `studio:*` events. Now one shared policy governs every + * transport — including the legacy key, which it still honours. */ function isEnabled(): boolean { - if (isOptedOut()) return false; - try { - return localStorage.getItem("hf-studio-telemetry-opt-out") !== "1"; - } catch { - return true; - } + return browserTelemetryAllowed(); } function getSessionProperties(): EventProperties {