fix(analytics): sign the acquisition cookies and bound what they can write

Both cookies were read straight into database columns after nothing more than a format check. httpOnly keeps JavaScript out of them and does nothing about curl, so the anonymous id was a string the caller picked: enough to write a first-touch row for a visitor who never existed, to file it under a channel of their choosing, and to claim that id's events at signup, since the backfill matches on the id alone. They are now signed with an HMAC over AUTH_SECRET, through Web Crypto rather than node:crypto because the proxy runs on the edge and the pages that read the cookies back run in Node. The first-touch body moved to base64url on the way: cookie values are percent-encoded and decoded by several layers that do not agree on how many times, and a payload carrying its own percent escapes comes back subtly different and takes the signature with it. Signing stops a caller choosing an id, not collecting one, since dropping the cookie and asking for the landing page again mints another. So the bot and prefetch filters moved to where the rows are written rather than only where the cookies are issued, which also fixes a returning visitor's prefetch of /register recording a signup start, and a per-client hourly ceiling now sits in front of the write. The ceiling is skipped when TRUSTED_PROXY_MODE is unset, where every caller resolves to 127.0.0.1 and the bucket would empty on real traffic long before it emptied on a flood. Four smaller things around it: - /api/events checked the flag and the origin after paying for a rate-limit write, so a host who never turned analytics on was still writing a row per anonymous POST. Both checks are free and now come first, and the limiter answers 204 rather than 429: a beacon has nobody to tell, and a flooder should not be handed the reset time. - /api/onboarding/source was keyed by IP on an authenticated route. Without TRUSTED_PROXY_MODE that is five answers an hour for the whole deployment, and with it a shared office address locks out everyone after one colleague answered. Keyed by account, like /api/onboarding/complete beside it. - The cookies took their Secure flag from request.nextUrl.protocol, which behind a TLS-terminating reverse proxy is the container-internal http address. It comes off the configured public origin now. - sanitizeLandingPath took anything that started with a slash, including from the cookie, so a hand-written one could put newlines and markup into a column an admin table may render one day. Also: the paid-account query had no LIMIT and returned every active account's name and email, the growth route answered 403 where it meant 401, and the schema claimed no free text is stored when self_reported_note holds 200 characters of it.
2026-09-11 09:36:08 +00:00 · 2026-08-01 20:29:28 +03:00
parent 7ca5abd041
commit 33c845636c
21 changed files with 711 additions and 129 deletions
@@ -1,15 +1,22 @@
 // The proxy is where a visitor gets an identity, and it is the only place that
 // can: it runs on the edge, before the page, on every document request.
 //
-// The load-bearing detail below is that the id is written to the *request* as
-// well as the response. A cookie set only on the response is invisible to the
+// Two load-bearing details below. The id is written to the *request* as well as
+// the response, because a cookie set only on the response is invisible to the
 // page rendering that same request, so the very first landing view, the one
-// carrying the campaign tags that brought the visitor, would go unrecorded.
+// carrying the campaign tags that brought the visitor, would go unrecorded. And
+// both cookies are signed, because they are read straight into database columns
+// and httpOnly stops JavaScript, not curl.

-import { describe, it, expect, afterEach, vi } from 'vitest';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { NextRequest } from 'next/server';
 import { proxy } from '@/proxy';
-import { ANONYMOUS_ID_COOKIE, FIRST_TOUCH_COOKIE, decodeFirstTouch } from '@/lib/analytics/cookies';
+import {
+  ANONYMOUS_ID_COOKIE,
+  FIRST_TOUCH_COOKIE,
+  readAnonymousIdCookie,
+  readFirstTouchCookie,
+} from '@/lib/analytics/cookies';

 const BROWSER_UA =
  'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126 Safari/537.36';
@@ -30,43 +37,51 @@ function documentRequest(
  return new NextRequest(new URL(url), { headers });
 }

+beforeEach(() => {
+  vi.stubEnv('NEXTAUTH_SECRET', 'proxy-test-secret');
+  // getPublicOrigin prefers a configured origin over the request URL, so the
+  // tests that care about the request URL have to start from neither being set.
+  vi.stubEnv('NEXTAUTH_URL', undefined);
+  vi.stubEnv('NEXT_PUBLIC_APP_URL', undefined);
+});
+
 afterEach(() => {
  vi.unstubAllEnvs();
 });

 describe('proxy', () => {
-  it('always sets the content security policy', () => {
+  it('always sets the content security policy', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'false');

-    const response = proxy(documentRequest('https://open-frame.net/'));
+    const response = await proxy(documentRequest('https://open-frame.net/'));

    expect(response.headers.get('Content-Security-Policy')).toContain("default-src 'self'");
  });

-  it('sets no acquisition cookie at all when the flag is off', () => {
+  it('sets no acquisition cookie at all when the flag is off', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'false');

-    const response = proxy(documentRequest('https://open-frame.net/?utm_source=github'));
+    const response = await proxy(documentRequest('https://open-frame.net/?utm_source=github'));

    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined();
    expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined();
  });

-  it('gives a new visitor an id and stores what brought them', () => {
+  it('gives a new visitor an id and stores what brought them', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');
    const request = documentRequest(
      'https://open-frame.net/?utm_source=youtube&utm_medium=video&utm_campaign=launch'
    );

-    const response = proxy(request);
+    const response = await proxy(request);

-    const id = response.cookies.get(ANONYMOUS_ID_COOKIE);
-    expect(id?.value).toMatch(/^[a-z0-9]{32}$/);
-    expect(id?.httpOnly).toBe(true);
-    expect(id?.sameSite).toBe('lax');
-    expect(id?.secure).toBe(true);
+    const cookie = response.cookies.get(ANONYMOUS_ID_COOKIE);
+    expect(await readAnonymousIdCookie(cookie?.value)).toMatch(/^[a-z0-9]{32}$/);
+    expect(cookie?.httpOnly).toBe(true);
+    expect(cookie?.sameSite).toBe('lax');
+    expect(cookie?.secure).toBe(true);

-    const touch = decodeFirstTouch(response.cookies.get(FIRST_TOUCH_COOKIE)?.value);
+    const touch = await readFirstTouchCookie(response.cookies.get(FIRST_TOUCH_COOKIE)?.value);
    expect(touch).toEqual({
      channel: 'YOUTUBE',
      utmSource: 'youtube',
@@ -77,33 +92,38 @@ describe('proxy', () => {
    });

    // The page rendering this same request has to be able to read both.
-    expect(request.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBe(id?.value);
+    expect(request.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBe(cookie?.value);
    expect(request.cookies.get(FIRST_TOUCH_COOKIE)?.value).toBeDefined();
  });

-  it('classifies a visit that only carries a referrer', () => {
+  it('classifies a visit that only carries a referrer', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');

-    const response = proxy(
+    const response = await proxy(
      documentRequest('https://open-frame.net/vs/frameio', {
        headers: { referer: 'https://github.com/yusufipk/OpenFrame' },
      })
    );

-    expect(decodeFirstTouch(response.cookies.get(FIRST_TOUCH_COOKIE)?.value)).toMatchObject({
+    expect(
+      await readFirstTouchCookie(response.cookies.get(FIRST_TOUCH_COOKIE)?.value)
+    ).toMatchObject({
      channel: 'GITHUB',
      referrerHost: 'github.com',
      landingPath: '/vs/frameio',
    });
  });

-  it('does not overwrite the first touch of a returning visitor', () => {
+  it('does not overwrite the first touch of a returning visitor', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');
-    const existingId = 'a1b2c3d4e5f60718293a4b5c6d7e8f90';
+    const issued = await proxy(documentRequest('https://open-frame.net/?utm_source=github'));

-    const response = proxy(
+    const response = await proxy(
      documentRequest('https://open-frame.net/?utm_source=google', {
-        cookies: { [ANONYMOUS_ID_COOKIE]: existingId, [FIRST_TOUCH_COOKIE]: 'anything' },
+        cookies: {
+          [ANONYMOUS_ID_COOKIE]: issued.cookies.get(ANONYMOUS_ID_COOKIE)?.value ?? '',
+          [FIRST_TOUCH_COOKIE]: issued.cookies.get(FIRST_TOUCH_COOKIE)?.value ?? '',
+        },
      })
    );

@@ -111,22 +131,56 @@ describe('proxy', () => {
    expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined();
  });

-  it('replaces an id that does not look like one we issued', () => {
+  it('replaces an id it did not sign, however well formed it looks', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');

-    const response = proxy(
+    // The shape a real id has, chosen by the caller rather than issued here.
+    // Accepting it would let anyone mint visitors, and claim the events of one
+    // whose id they guessed.
+    const forged = 'a1b2c3d4e5f60718293a4b5c6d7e8f90';
+
+    const response = await proxy(
      documentRequest('https://open-frame.net/', {
-        cookies: { [ANONYMOUS_ID_COOKIE]: 'nope' },
+        cookies: { [ANONYMOUS_ID_COOKIE]: forged },
      })
    );

-    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toMatch(/^[a-z0-9]{32}$/);
+    const issued = await readAnonymousIdCookie(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value);
+    expect(issued).toMatch(/^[a-z0-9]{32}$/);
+    expect(issued).not.toBe(forged);
  });

-  it('ignores crawlers, so they never enter the visitor count', () => {
+  it('replaces an id signed with another deployment key', async () => {
+    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');
+    const issued = await proxy(documentRequest('https://open-frame.net/'));
+    const stolen = issued.cookies.get(ANONYMOUS_ID_COOKIE)?.value ?? '';
+
+    vi.stubEnv('NEXTAUTH_SECRET', 'a-different-secret');
+    const response = await proxy(
+      documentRequest('https://open-frame.net/', {
+        cookies: { [ANONYMOUS_ID_COOKIE]: stolen },
+      })
+    );
+
+    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).toBeDefined();
+    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.value).not.toBe(stolen);
+  });
+
+  it('sets nothing when there is no secret to sign with', async () => {
+    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');
+    vi.stubEnv('NEXTAUTH_SECRET', undefined);
+    vi.stubEnv('AUTH_SECRET', undefined);
+
+    const response = await proxy(documentRequest('https://open-frame.net/'));
+
+    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined();
+    expect(response.cookies.get(FIRST_TOUCH_COOKIE)).toBeUndefined();
+  });
+
+  it('ignores crawlers, so they never enter the visitor count', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');

-    const response = proxy(
+    const response = await proxy(
      documentRequest('https://open-frame.net/', {
        headers: { 'user-agent': 'Googlebot/2.1 (+http://www.google.com/bot.html)' },
      })
@@ -135,15 +189,15 @@ describe('proxy', () => {
    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined();
  });

-  it('ignores a prefetch and an API call', () => {
+  it('ignores a prefetch and an API call', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');

-    const prefetch = proxy(
+    const prefetch = await proxy(
      documentRequest('https://open-frame.net/register', {
        headers: { 'next-router-prefetch': '1' },
      })
    );
-    const api = proxy(
+    const api = await proxy(
      documentRequest('https://open-frame.net/api/projects', {
        headers: { 'sec-fetch-dest': 'empty' },
      })
@@ -153,11 +207,23 @@ describe('proxy', () => {
    expect(api.cookies.get(ANONYMOUS_ID_COOKIE)).toBeUndefined();
  });

-  it('leaves the cookie insecure on plain http, so local development works', () => {
+  it('leaves the cookie insecure on plain http, so local development works', async () => {
    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');

-    const response = proxy(documentRequest('http://localhost:3000/'));
+    const response = await proxy(documentRequest('http://localhost:3000/'));

    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.secure).toBe(false);
  });
+
+  it('keeps the cookie secure behind a TLS-terminating proxy', async () => {
+    vi.stubEnv('OPENFRAME_ENABLE_ANALYTICS', 'true');
+    // What a Docker deployment looks like from inside the container: the request
+    // arrived over http on an internal address, and only the configured origin
+    // knows the site is served over TLS.
+    vi.stubEnv('NEXTAUTH_URL', 'https://open-frame.net');
+
+    const response = await proxy(documentRequest('http://localhost:3000/'));
+
+    expect(response.cookies.get(ANONYMOUS_ID_COOKIE)?.secure).toBe(true);
+  });
 });