diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx index 5d11c0c6e..cda386b60 100644 --- a/docs/contributing/canary-rollouts.mdx +++ b/docs/contributing/canary-rollouts.mdx @@ -80,7 +80,9 @@ tooling, so whoever operates the telemetry backend can split any metric by cohort with **nothing configured server-side** — while the decision itself still happens locally and offline, which the render path requires. (Assignments ride the same anonymous, opt-out telemetry pipeline as every -other event; disabling telemetry disables the reporting, not the enrolment.) +other event — and disabling telemetry disables the **enrolment**, not just the +reporting: an opted-out install is never bucketed at all. See the note at the +top of this page.) Two details worth knowing: diff --git a/packages/cli/src/server/studioServer.test.ts b/packages/cli/src/server/studioServer.test.ts index 466317aff..cb5ce853c 100644 --- a/packages/cli/src/server/studioServer.test.ts +++ b/packages/cli/src/server/studioServer.test.ts @@ -57,3 +57,57 @@ describe("createStudioServer autoProxy plumbing", () => { expect(server.adapter.autoProxy).toBe(true); }); }); + +describe("host guarding on identity-bearing responses", () => { + const dirs: string[] = []; + let server: StudioServer | undefined; + + function tmpProject(): string { + const dir = mkdtempSync(join(tmpdir(), "hf-studio-host-test-")); + dirs.push(dir); + return dir; + } + + afterEach(() => { + server?.watcher.close(); + server = undefined; + 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(""); + }); + + it("refuses the identity endpoint for a hostile Host", async () => { + server = createStudioServer({ projectDir: tmpProject() }); + const res = await server.app.request("/api/telemetry-identity", { + headers: { host: "evil.example.com" }, + }); + expect(res.status).toBe(403); + expect(await res.text()).not.toContain('distinctId":"'); + }); + + it("serves the identity endpoint on a loopback Host", async () => { + server = createStudioServer({ projectDir: tmpProject() }); + const res = await server.app.request("/api/telemetry-identity", { + headers: { host: "127.0.0.1:5173" }, + }); + expect(res.status).toBe(200); + // The seed is no longer served here at all — Studio gets decisions + // injected instead, so nothing needs it over HTTP. + expect(Object.keys((await res.json()) as object)).toEqual(["distinctId"]); + }); +}); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 742bee1d2..210ce82f7 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -814,7 +814,17 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // Inject before the studio bundle runs. Identity script first (see // buildStudioHeadScripts) so the CLI distinct id is on `window` by the time // telemetry init reads it. - const headScript = buildStudioHeadScripts(buildRuntimeEnvScript()); + // + // 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(); 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 98728555b..4599fa3b3 100644 --- a/packages/cli/src/server/telemetryIdentity.test.ts +++ b/packages/cli/src/server/telemetryIdentity.test.ts @@ -8,7 +8,7 @@ const shouldTrack = vi.fn(); const readConfig = vi.fn(); // Pinned rather than using the real registry, so these string assertions // don't move every time a canary is added, ramped, or retired. -const canaryDecisions = vi.fn<() => Record>(); +const canaryDecisions = vi.fn<() => Record>(); vi.mock("../telemetry/client.js", () => ({ shouldTrack: (...args: unknown[]) => shouldTrack(...args), @@ -99,10 +99,11 @@ describe("buildCliIdentityScript", () => { // stops Studio evaluating independently and enrolling anyway. it("still publishes canary decisions when telemetry is off, but no identity", () => { shouldTrack.mockReturnValue(false); - canaryDecisions.mockReturnValue({ "de-parallel-router": false }); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: false, forced: false } }); const script = buildCliIdentityScript(); expect(script).toBe( - '', + "', ); expect(script).not.toContain("__HF_CLI_DISTINCT_ID"); expect(script).not.toContain("__HF_CLI_BUCKET_SEED"); @@ -111,17 +112,20 @@ describe("buildCliIdentityScript", () => { it("publishes decisions alongside the identity when telemetry is on", () => { shouldTrack.mockReturnValue(true); readConfig.mockReturnValue({ anonymousId: "machine-uuid", bucketSeed: "seed-uuid" }); - canaryDecisions.mockReturnValue({ "de-parallel-router": true }); + canaryDecisions.mockReturnValue({ "de-parallel-router": { enabled: true, forced: true } }); expect(buildCliIdentityScript()).toBe( '', + "window.__HF_CLI_CANARY_DECISIONS=" + + '{"de-parallel-router":{"enabled":true,"forced":true}};', ); }); it("escapes a canary name that tries to close the script tag", () => { shouldTrack.mockReturnValue(false); - canaryDecisions.mockReturnValue({ "