Merge pull request 'security: stop trusting the leftmost x-forwarded-for for rate limits and abuse quotas (FRE-371)' (#1) from agent-security-engineer/freno-dev:freno-dev/FRE-371-trusted-client-ip-clean into master

Reviewed-on: #1

marketing/metrics/weekly.md

@@ -1,6 +1,6 @@
 # Weekly funnel snapshot
 
-Window: **2026-10-22 16:00:00 → 2026-10-29 16:00:00 UTC** (7 days). Previous: 2026-10-15 16:00:00 → 2026-10-22 16:00:00 UTC.
+Window: **2026-10-22 18:05:00 → 2026-10-29 18:05:00 UTC** (7 days). Previous: 2026-10-15 18:05:00 → 2026-10-22 18:05:00 UTC.
 
 Generated by `bun --env-file=.env scripts/weekly-metrics.ts`; regenerated weekly, never hand-edited. The analysis is the CMO's.
 
@@ -8,10 +8,11 @@ Generated by `bun --env-file=.env scripts/weekly-metrics.ts`; regenerated weekly
 
 | Stage | This week | Last week | Change |
 |---|---:|---:|---:|
-| visit (page views) | 61 | 56 | +5 |
-| download click → DMG served (one label) † | 156 | 0 | +156 |
+| visit (nook.freno.me page views) | 0 | 0 | 0 |
+| download click | 0 | 0 | 0 |
+| DMG served | 0 | 0 | 0 |
 | trial started | 2 | 0 | +2 |
-| checkout page visit † | 0 | 0 | 0 |
+| checkout started (Stripe session) | 0 | 0 | 0 |
 | purchase (licence issued) | 0 | 0 | 0 |
 | activation | 0 | 0 | 0 |
 
@@ -21,31 +22,22 @@ Top 10 by volume, untagged excluded — the same grouping `getAnalyticsSummary.t
 
 | Source | Medium | Campaign | Rows |
 |---|---|---|---:|
-| hn | post | launch | 2 |
-| direct | — | deploycheck-direct | 1 |
-| hn | — | flushproof | 1 |
-| hn | post | deploycheck-ref | 1 |
-| r_macapps | — | week2 | 1 |
 | x | — | — | 1 |
 
-7 of 356 rows in the window carry campaign tags; the rest are untagged and therefore unattributed.
+1 of 291 rows in the window carry campaign tags; the rest are untagged and therefore unattributed.
 
 ## The one thing that moved
 
-**download click → DMG served (one label): 0 → 156 (+156).**
+**trial started: 0 → 2 (+2).**
 
 ## Before acting on these numbers
 
-- **356 of 356 rows in the window came from a loopback address** (127.0.0.1 / ::1); every one of the 12035 addressed rows the table has ever held is loopback (13 carry no address). Either this host is the only audience, or the app sits behind a local proxy and `getRequestIP` records the proxy, not the visitor. Either way the address identifies nobody — "unique visitors" collapses to one host and per-channel reach cannot be read from it.
-- **The download stage is one label, not two.** The landing-door route and the file route that serves the DMG both write `/api/downloads/thenook`, so "download click → DMG served" cannot be split: one served click writes two rows, a click that fails to resolve writes one, and a Sparkle update writes one. Of this window's 156 download rows, 156 are loopback and 117 are not browser user agents (curl, Sparkle, link-preview crawlers).
-- **Checkout start is not recorded.** `POST /api/the-nook/checkout` — the call that actually creates the Stripe session — writes nothing to analytics; the only trace is the /checkout page view reported above, which is a page, not a started checkout.
-- **There is no host dimension.** `VisitorAnalytics` records a path, not the site, so the Nook's landing page (`/` on nook.freno.me) is the same row as freno.me's root. "visit" above is site-wide page views, not Nook visits.
+- **291 of 291 rows in the window predate the FRE-366 instrumentation.** They carry no host, and their recorded address is the production proxy's loopback (127.0.0.1 / ::1) — the writers then recorded the direct peer, not the visitor. They are excluded from every host-scoped stage above (visits, and the download click/served split), and "unique visitors" over them still collapses to one host. Rows recorded since carry the site they addressed and the visitor's address truncated to a prefix (/24, or /56 for IPv6 — the FRE-369 privacy floor), so "unique visitors" counts networks, not machines.
+- **Download rows are automation-heavy.** Of this window's 137 download rows, 0 are clicks and 0 are served files; the remaining 137 predate the click/served stage label and cannot be split, so they count in neither stage. 112 of all 137 are not browser user agents (curl, Sparkle, link-preview crawlers).
 - **No conversion rate before the cohort matures.** Trial→paid belongs to the mature Day 28–35 read (FRE-197); a ratio from a partial cohort is not a metric.
 
-† Proxy or merged label — see *Before acting on these numbers* above.
-
 ## Sources
 
 - `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns.
 - `trials`, `licenses`, `activations` (the-nook Turso DB) — the paid funnel.
-- Generated 2026-10-29T16:00:00Z.
+- Generated 2026-10-29T18:05:00Z.

scripts/weekly-metrics.ts

@@ -2,6 +2,7 @@ import { createClient } from "@libsql/client/web";
 import { mkdir, writeFile } from "node:fs/promises";
 import { dirname, resolve } from "node:path";
 import {
+  ensureAnalyticsSchema,
   getWeeklyReport,
   renderWeeklyMarkdown,
   type Connections
@@ -123,6 +124,11 @@ async function main(): Promise<void> {
     nook: connect(requiredEnv("NOOK_DB_URL"), requiredEnv("NOOK_DB_TOKEN"))
   };
 
+  // The app migrates the table on boot, but this job must not depend on a
+  // deploy having happened: add any missing post-hoc columns (stage, host)
+  // through the same idempotent bootstrap path first.
+  await ensureAnalyticsSchema(connections.analytics);
+
   const report = await getWeeklyReport(connections, {
     days: options.days,
     now: options.now
@@ -141,8 +147,8 @@ async function main(): Promise<void> {
   console.log(
     `wrote ${options.out}\n` +
       `window ${report.window.start} → ${report.window.end}\n` +
-      `visit ${f.visit}, download ${f.download}, trial ${f.trial}, ` +
-      `checkout ${f.checkout}, purchase ${f.purchase}, activation ${f.activation}\n` +
+      `visit ${f.visit}, download click ${f.downloadClick}, DMG served ${f.dmgServed}, ` +
+      `trial ${f.trial}, checkout ${f.checkout}, purchase ${f.purchase}, activation ${f.activation}\n` +
       `mover: ${report.mover ? `${report.mover.label} ${report.mover.before} → ${report.mover.now}` : "none"}`
   );
 }

src/db/types.ts

@@ -160,6 +160,8 @@ export interface VisitorAnalytics {
   ttfb?: number | null;
   dom_load?: number | null;
   load_complete?: number | null;
+  stage?: string | null;
+  host?: string | null;
   created_at: string;
 }
 

src/env/server.ts

@@ -69,6 +69,12 @@ const serverEnvSchema = z.object({
   // Bearer secret for the daily lifecycle-email cron. Falls back to
   // Vercel's CRON_SECRET; unset + no CRON_SECRET = the endpoint denies all.
   NOOK_LIFECYCLE_SECRET: z.string().min(1).optional(),
+  // Daily VisitorAnalytics retention cron (/api/analytics-retention). Same
+  // shape: falls back to CRON_SECRET, and unset denies every request.
+  ANALYTICS_RETENTION_SECRET: z.string().min(1).optional(),
+  // How long a VisitorAnalytics row lives. Unset (or unusable) = 365 days;
+  // see src/server/analytics-retention.ts before shortening it.
+  ANALYTICS_RETENTION_DAYS: z.string().optional(),
   CRON_SECRET: z.string().min(1).optional()
 });
 

src/routes/api/analytics-retention.ts

@@ -0,0 +1,35 @@
+import type { APIEvent } from "@solidjs/start/server";
+import { env } from "~/env/server";
+import { ConnectionFactory } from "~/server/db-connections";
+import { runRetentionCron } from "~/server/analytics-retention";
+
+/**
+ * GET /api/analytics-retention — daily Vercel cron.
+ *
+ * Deletes `VisitorAnalytics` rows older than the retention window, so visitor
+ * data has a bound instead of accumulating forever (FRE-369, finding Q4 — the
+ * table's only cleaner used to be a manual admin call, and since FRE-366 the
+ * rows carry a truncated but still real client address).
+ *
+ * Window: `ANALYTICS_RETENTION_DAYS`, defaulting to 365 when unset or unusable
+ * (see `~/server/analytics-retention` for why the default is conservative
+ * rather than the reviewer's 90).
+ *
+ * Auth: `Authorization: Bearer <secret>` where the secret is
+ * `ANALYTICS_RETENTION_SECRET` or Vercel's `CRON_SECRET`. An unconfigured secret
+ * denies every request — an unauthenticated trigger of a bulk delete is the one
+ * outcome this endpoint must never have.
+ */
+export async function GET(event: APIEvent) {
+  const result = await runRetentionCron({
+    authorization: event.request.headers.get("authorization"),
+    secret: env.ANALYTICS_RETENTION_SECRET ?? env.CRON_SECRET,
+    configuredDays: env.ANALYTICS_RETENTION_DAYS,
+    conn: ConnectionFactory()
+  });
+
+  return new Response(JSON.stringify(result.body), {
+    status: result.status,
+    headers: { "Content-Type": "application/json" }
+  });
+}

src/routes/api/downloads/[filename].ts

@@ -95,7 +95,7 @@ export async function GET(event: APIEvent) {
 
     console.log(`✓ Serving ${filename} (${body.length} bytes)`);
 
-    await trackDownload(event, assetKeys[asset]!);
+    await trackDownload(event, assetKeys[asset]!, "served");
 
     // SAFETY: `transformToByteArray` yields `Uint8Array<ArrayBufferLike>`, which the
     // DOM lib's `BufferSource` (ArrayBuffer-backed) does not accept, though undici

src/routes/api/the-nook/checkout.ts

@@ -3,6 +3,7 @@ import { env } from "~/env/server";
 import { TURNSTILE_CONFIG } from "~/config";
 import { verifyTurnstileToken } from "~/server/fetch-utils";
 import { json, error } from "./_lib";
+import { trackRequest } from "~/server/track-download";
 
 /**
  * POST /api/the-nook/checkout
@@ -12,6 +13,14 @@ import { json, error } from "./_lib";
  * session for the one-time $10 The Nook license. Returns the hosted
  * checkout URL for the client to redirect to.
  *
+ * Analytics: a created session is the funnel's "checkout started" stage
+ * (FRE-366). Until then the only trace was the client-side /checkout page
+ * view — a page, not a started checkout. Recorded through the shared
+ * `trackRequest` writer (referrer, campaign tags, country, real client
+ * address, host) under this route's own path, best-effort: a failed
+ * analytics write never blocks a checkout, and an attempt Turnstile or
+ * Stripe rejects is correctly not a started checkout.
+ *
  * Uses raw `fetch` (Node 24 / Vercel Node functions have global fetch) — no
  * `stripe` npm dependency added.
  */
@@ -67,5 +76,14 @@ export async function POST(event: APIEvent) {
   if (!data.url) {
     return error("Stripe returned no checkout URL", 502);
   }
+
+  // The session exists: that is "checkout started". Awaited so the row is
+  // flushed before the response — a buffered write is not guaranteed to
+  // survive the response on serverless.
+  await trackRequest(event, {
+    path: "/api/the-nook/checkout",
+    method: "POST"
+  });
+
   return json({ checkoutUrl: data.url });
 }

src/routes/api/the-nook/download.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it, mock, beforeEach } from "bun:test";
+import { truncateIpAddress } from "~/server/request-client";
 
 // The route's contracts:
 //   1. Every click is recorded through the shared analytics buffer with the
@@ -55,9 +56,13 @@ mock.module("~/server/analytics", () => ({
   flushAnalytics: async () => {
     flushCalls += 1;
   },
-  // Mirrors the real enrichment contract: deviceType is derived from the UA.
+  // Mirrors the real enrichment contract: deviceType is derived from the UA,
+  // and the address is cut to its prefix by the FRE-369 privacy floor. The
+  // real `enrichAnalyticsEntry` does both, so the mirror has to apply the same
+  // truncation — otherwise this file would prove the door stores a full address.
   enrichAnalyticsEntry: (entry: Entry) => ({
     ...entry,
+    ipAddress: truncateIpAddress(entry.ipAddress as string | null),
     deviceType: String(entry.userAgent ?? "").includes("Macintosh")
       ? "desktop"
       : null
@@ -113,6 +118,10 @@ describe("GET /api/the-nook/download", () => {
     expect(logged[0]).toMatchObject({
       path: "/api/downloads/thenook",
       method: "GET",
+      // The door is the click half of the split; the file route writes
+      // "served". And the row names the site it came from (FRE-366).
+      stage: "click",
+      host: "nook.freno.me",
       referrer: "https://nook.freno.me/"
     });
     // Flushed within the request, not left to the batch timer.
@@ -137,9 +146,16 @@ describe("GET /api/the-nook/download", () => {
     expect(logged[2]!.country).toBe("JP");
   });
 
-  it("falls back to the forwarded address when no request IP is available", async () => {
-    await GET(request({ "x-forwarded-for": "198.51.100.9, 10.0.0.1" }));
-    expect(logged[0]!.ipAddress).toBe("198.51.100.9");
+  it("records the forwarded visitor below the trusted edge, truncated", async () => {
+    // Client → Cloudflare → origin proxy: the sender's spoof is first, the real
+    // client second, the edge last. And the stored value is the /24, never the
+    // address itself (FRE-369).
+    await GET(
+      request({
+        "x-forwarded-for": "198.51.100.9, 203.0.113.7, 198.51.100.1"
+      })
+    );
+    expect(logged[0]!.ipAddress).toBe("203.0.113.0");
   });
 
   it("still logs the click when the appcast cannot be read", async () => {

src/routes/api/the-nook/download.ts

@@ -13,8 +13,10 @@ import { resolveLatestDownload } from "./_download";
  * Why a redirect and not a direct S3 link: the appcast route and the DMG
  * route never logged anything, so step 2 of the funnel (download click →
  * DMG served) was invisible. Every click now lands one row in
- * VisitorAnalytics under `/api/downloads/thenook`, alongside the clicks and
- * served files of every other product — see `~/server/track-download`.
+ * VisitorAnalytics under `/api/downloads/thenook` with `stage = "click"`,
+ * distinct from the `stage = "served"` row the file route writes when the
+ * DMG is actually streamed — the split FRE-366 added, alongside the clicks
+ * and served files of every other product — see `~/server/track-download`.
  *
  * Referrer, campaign tags, edge country (with `?c=` override) and user agent
  * are read by `trackDownload`, which is shared with the other download doors.
@@ -28,7 +30,7 @@ import { resolveLatestDownload } from "./_download";
 export async function GET(event: APIEvent): Promise<Response> {
   // Recorded before resolving: a click that dies at resolution is exactly the
   // failure the funnel needs to show.
-  await trackDownload(event, "thenook");
+  await trackDownload(event, "thenook", "click");
 
   let target: string | null = null;
   try {

src/routes/api/the-nook/report.test.ts

@@ -10,6 +10,8 @@ import { describe, expect, it, mock, beforeEach } from "bun:test";
 
 const sentEmails: { to: string; subject: string; html: string }[] = [];
 const tokenAnswers: { allowed: boolean; retryAfterSec?: number }[] = [];
+/** Every rate-limit key the route derived, in order. */
+const rateKeys: string[] = [];
 
 mock.module("~/server/email", () => ({
   default: async (to: string, subject: string, html: string) => {
@@ -19,7 +21,10 @@ mock.module("~/server/email", () => ({
 }));
 
 mock.module("~/server/bug-report-rate-limit", () => ({
-  takeBugReportToken: () => tokenAnswers.shift() ?? { allowed: true }
+  takeBugReportToken: (key: string) => {
+    rateKeys.push(key);
+    return tokenAnswers.shift() ?? { allowed: true };
+  }
 }));
 
 const { POST } = await import("./report");
@@ -72,6 +77,7 @@ describe("POST /api/the-nook/report", () => {
   beforeEach(() => {
     sentEmails.length = 0;
     tokenAnswers.length = 0;
+    rateKeys.length = 0;
   });
 
   it("emails the owner a report containing the snapshot, escaped", async () => {
@@ -133,10 +139,49 @@ describe("POST /api/the-nook/report", () => {
 
   it("still succeeds when no fingerprint header exists", async () => {
     const res = await POST(
-      request(validPayload, { "x-forwarded-for": "203.0.113.9, 10.0.0.1" })
+      request(validPayload, {
+        "cf-connecting-ip": "203.0.113.9",
+        "x-forwarded-for": "203.0.113.9, 10.0.0.1"
+      })
     );
     expect(res.status).toBe(200);
     expect(sentEmails).toHaveLength(1);
+    // The quota key is the Cloudflare-recorded client, not a header hop the
+    // sender chose.
+    expect(rateKeys).toEqual(["203.0.113.9"]);
+  });
+
+  it("a rotating leftmost x-forwarded-for does not mint a fresh quota key", async () => {
+    // FRE-371: the old read keyed on the leftmost hop, which the sender
+    // controls outright — one scripted loop with a rotating header had
+    // unlimited reports. With the same Cloudflare-recorded client and a
+    // different spoofed prefix per request, the key must be stable.
+    for (const spoof of ["1.1.1.1", "2.2.2.2", "3.3.3.3"]) {
+      const res = await POST(
+        request(validPayload, {
+          "cf-connecting-ip": "203.0.113.9",
+          "x-forwarded-for": `${spoof}, 203.0.113.9, 10.0.0.1`
+        })
+      );
+      expect(res.status).toBe(200);
+    }
+
+    expect(rateKeys).toEqual([
+      "203.0.113.9",
+      "203.0.113.9",
+      "203.0.113.9"
+    ]);
+  });
+
+  it("keys on the rightmost-minus-one hop when Cloudflare's header is absent", async () => {
+    const res = await POST(
+      request(validPayload, {
+        "x-forwarded-for": "6.6.6.6, 203.0.113.9, 10.0.0.1"
+      })
+    );
+    expect(res.status).toBe(200);
+    // Never the leftmost (sender-chosen) hop.
+    expect(rateKeys).toEqual(["203.0.113.9"]);
   });
 
   it("delivers the landing page's beta request with only the email", async () => {

src/routes/api/the-nook/report.ts

@@ -3,6 +3,7 @@ import { z } from "zod";
 import sendEmail from "~/server/email";
 import { CONTACT_RECIPIENT_EMAIL } from "~/lib/contact-config";
 import { takeBugReportToken } from "~/server/bug-report-rate-limit";
+import { resolveTrustedClientIp } from "~/server/trusted-client-ip";
 import { json, error } from "./_lib";
 
 /**
@@ -103,10 +104,9 @@ export async function POST(event: APIEvent): Promise<Response> {
   )
     .trim()
     .toLowerCase();
-  const forwarded = event.request.headers.get("x-forwarded-for");
-  const rateKey =
-    fingerprint ||
-    (forwarded ? forwarded.split(",")[0]!.trim() : "unknown");
+  // FRE-371: the IP fallback must be the spoof-resistant read — a rotating
+  // leftmost x-forwarded-for used to mint unlimited report quota.
+  const rateKey = fingerprint || resolveTrustedClientIp(event);
   const token = takeBugReportToken(rateKey);
   if (!token.allowed) {
     return new Response(JSON.stringify({ error: "Too many reports" }), {

src/server/analytics-retention.test.ts

@@ -0,0 +1,200 @@
+import { beforeEach, describe, expect, it } from "bun:test";
+import { Database } from "bun:sqlite";
+import { VISITOR_ANALYTICS_DDL } from "./analytics-schema";
+import {
+  DEFAULT_ANALYTICS_RETENTION_DAYS,
+  MAX_ANALYTICS_RETENTION_DAYS,
+  isAuthorizedCron,
+  pruneVisitorAnalytics,
+  resolveRetentionDays,
+  runRetentionCron
+} from "./analytics-retention";
+
+// Why a real in-memory SQLite database: retention is a `DELETE` whose whole
+// job is a date comparison. A stub returning a canned count would pass with a
+// window that is off by a unit, inverted, or — the failure this file exists for
+// — never applied at all. Same reasoning as the backfill and schema suites, and
+// nothing is module-mocked here on purpose: bun's `mock.module` is process-wide.
+
+let db: Database;
+
+const conn = {
+  async execute(query: { sql: string; args?: unknown[] }) {
+    const statement = db.query(query.sql);
+    const args = (query.args ?? []) as never[];
+    // `DELETE … RETURNING id` answers with rows, which is how the prune counts
+    // what it removed; bun:sqlite exposes those through `.all()`.
+    if (
+      /^\s*(select|pragma|with)\b/i.test(query.sql) ||
+      /\breturning\b/i.test(query.sql)
+    ) {
+      return { rows: statement.all(...args) as unknown[], rowsAffected: 0 };
+    }
+    return {
+      rows: [] as unknown[],
+      rowsAffected: statement.run(...args).changes
+    };
+  }
+};
+
+function insert(id: string, createdAt: string) {
+  db.run(
+    `INSERT INTO VisitorAnalytics (id, path, method, created_at) VALUES (?, '/x', 'GET', ?)`,
+    [id, createdAt]
+  );
+}
+
+function rowCount(): number {
+  return (db.query(`SELECT COUNT(*) AS n FROM VisitorAnalytics`).get() as {
+    n: number;
+  }).n;
+}
+
+beforeEach(() => {
+  db = new Database(":memory:");
+  db.run(VISITOR_ANALYTICS_DDL);
+});
+
+describe("resolveRetentionDays", () => {
+  it("takes a usable configured window", () => {
+    expect(resolveRetentionDays("90")).toBe(90);
+    expect(resolveRetentionDays(30)).toBe(30);
+  });
+
+  it("falls back to the default when the value is missing or nonsense", () => {
+    expect(resolveRetentionDays(undefined)).toBe(
+      DEFAULT_ANALYTICS_RETENTION_DAYS
+    );
+    expect(resolveRetentionDays("")).toBe(DEFAULT_ANALYTICS_RETENTION_DAYS);
+    expect(resolveRetentionDays("0")).toBe(DEFAULT_ANALYTICS_RETENTION_DAYS);
+    expect(resolveRetentionDays("-5")).toBe(DEFAULT_ANALYTICS_RETENTION_DAYS);
+    expect(resolveRetentionDays("1.5")).toBe(DEFAULT_ANALYTICS_RETENTION_DAYS);
+    expect(resolveRetentionDays("all")).toBe(DEFAULT_ANALYTICS_RETENTION_DAYS);
+  });
+
+  it("refuses a window so long it means 'never prune'", () => {
+    expect(resolveRetentionDays(String(MAX_ANALYTICS_RETENTION_DAYS + 1))).toBe(
+      DEFAULT_ANALYTICS_RETENTION_DAYS
+    );
+  });
+});
+
+describe("pruneVisitorAnalytics", () => {
+  it("deletes only rows older than the window", async () => {
+    insert("young", "2020-01-01 00:00:00");
+    insert("old", "2019-01-01 00:00:00");
+    // A row exactly at `now - 365 days` boundary in the other direction: the
+    // comparison is `<`, so the recent one survives.
+    db.run(
+      `UPDATE VisitorAnalytics SET created_at = datetime('now', '-10 days') WHERE id = 'young'`
+    );
+    db.run(
+      `UPDATE VisitorAnalytics SET created_at = datetime('now', '-400 days') WHERE id = 'old'`
+    );
+
+    const deleted = await pruneVisitorAnalytics(conn, 365);
+
+    expect(deleted).toBe(1);
+    expect(rowCount()).toBe(1);
+  });
+
+  it("deletes nothing when every row is inside the window", async () => {
+    insert("a", "x");
+    db.run(
+      `UPDATE VisitorAnalytics SET created_at = datetime('now', '-1 day') WHERE id = 'a'`
+    );
+
+    expect(await pruneVisitorAnalytics(conn, 365)).toBe(0);
+    expect(rowCount()).toBe(1);
+  });
+
+  it("refuses a window it was not given a number for", async () => {
+    insert("a", "2019-01-01 00:00:00");
+
+    await expect(pruneVisitorAnalytics(conn, 0)).rejects.toThrow(/window/);
+    await expect(pruneVisitorAnalytics(conn, -30)).rejects.toThrow(/window/);
+    await expect(pruneVisitorAnalytics(conn, NaN)).rejects.toThrow(/window/);
+    // "0" is not a window either — and must not delete the table.
+    await expect(
+      pruneVisitorAnalytics(conn, "0" as unknown as number)
+    ).rejects.toThrow(/window/);
+
+    expect(rowCount()).toBe(1);
+  });
+
+  it("passes the window as a parameter, not as interpolated SQL", async () => {
+    insert("a", "2019-01-01 00:00:00");
+    const injected = "365); DROP TABLE VisitorAnalytics; --" as unknown as number;
+
+    // Truncating a non-numeric string yields NaN, which the guard refuses — so
+    // the statement never runs and the table is still there.
+    await expect(pruneVisitorAnalytics(conn, injected)).rejects.toThrow(/window/);
+    expect(rowCount()).toBe(1);
+  });
+});
+
+describe("isAuthorizedCron", () => {
+  it("matches the bearer secret and nothing else", () => {
+    expect(isAuthorizedCron("Bearer sekrit", "sekrit")).toBe(true);
+    expect(isAuthorizedCron("bearer SEKRIT".toLowerCase(), "sekrit")).toBe(true);
+    expect(isAuthorizedCron("Bearer nope", "sekrit")).toBe(false);
+    expect(isAuthorizedCron(null, "sekrit")).toBe(false);
+    expect(isAuthorizedCron("Bearer sekrit", null)).toBe(false);
+  });
+
+  it("denies everything when no secret is configured (fail closed)", () => {
+    expect(isAuthorizedCron("Bearer ", undefined)).toBe(false);
+    expect(isAuthorizedCron(undefined, "")).toBe(false);
+  });
+});
+
+describe("runRetentionCron", () => {
+  it("denies an unauthenticated trigger before touching the table", async () => {
+    insert("a", "2019-01-01 00:00:00");
+
+    const res = await runRetentionCron({
+      authorization: null,
+      secret: "sekrit",
+      configuredDays: "365",
+      conn
+    });
+
+    expect(res.status).toBe(401);
+    expect(rowCount()).toBe(1);
+  });
+
+  it("prunes on its window and reports what it removed", async () => {
+    db.run(
+      `INSERT INTO VisitorAnalytics (id, path, method, created_at) VALUES ('old','/x','GET',datetime('now','-400 days'))`
+    );
+    db.run(
+      `INSERT INTO VisitorAnalytics (id, path, method, created_at) VALUES ('new','/x','GET',datetime('now','-1 day'))`
+    );
+
+    const res = await runRetentionCron({
+      authorization: "Bearer sekrit",
+      secret: "sekrit",
+      configuredDays: "365",
+      conn
+    });
+
+    expect(res.status).toBe(200);
+    expect(res.body).toEqual({ success: true, days: 365, deleted: 1 });
+    expect(rowCount()).toBe(1);
+  });
+
+  it("says it failed rather than reporting a quiet zero", async () => {
+    db.close(); // the table is gone: the prune's statement must now throw
+
+    const res = await runRetentionCron({
+      authorization: "Bearer sekrit",
+      secret: "sekrit",
+      configuredDays: "365",
+      conn
+    });
+
+    expect(res.status).toBe(500);
+    expect(res.body.days).toBe(365);
+    expect(res.body.deleted).toBeUndefined();
+  });
+});

src/server/analytics-retention.ts

@@ -0,0 +1,129 @@
+/**
+ * Scheduled retention for `VisitorAnalytics` (FRE-369, finding Q4).
+ *
+ * The table used to grow without bound: `cleanupOldAnalytics` existed but was
+ * reachable only as a manual tRPC call, and once FRE-366 started recording a
+ * real client address instead of the proxy's loopback, "unbounded" meant
+ * unbounded personal data in a hosted third-party database. Two halves answer
+ * that — the write-time prefix in `./request-client`, and this: a bound on how
+ * long any row exists at all.
+ *
+ * The default (365 days) is deliberately conservative so enabling the schedule
+ * is not itself a destructive event: it deletes nothing on its first run
+ * against a database whose history starts inside the window, and it bounds
+ * growth from then on. Tighten it to the reviewer's suggested 90 by setting
+ * `ANALYTICS_RETENTION_DAYS=90` — that first run *will* delete months of
+ * history that the /analytics dashboard and the weekly report read, which is an
+ * operational decision, not one this file should make silently.
+ */
+
+/** Kept when `ANALYTICS_RETENTION_DAYS` is unset or unusable. */
+export const DEFAULT_ANALYTICS_RETENTION_DAYS = 365;
+
+/** Anything longer is a misconfiguration, not a policy (≈ 2 years). */
+export const MAX_ANALYTICS_RETENTION_DAYS = 730;
+
+/**
+ * The slice of a connection this module needs — the same shape
+ * `weekly-metrics.ts` pins as `MetricsConn`, so a libsql client satisfies it
+ * without a cast (and the in-memory SQLite test adapter satisfies it too).
+ */
+export interface AnalyticsConn {
+  execute(query: {
+    sql: string;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    args?: any[];
+  }): Promise<{ rows: unknown[]; rowsAffected?: number }>;
+}
+
+/**
+ * The retention window to enforce: a positive integer at most
+ * {@link MAX_ANALYTICS_RETENTION_DAYS}, else the default. Accepts the env
+ * string form and a number, because both callers hand over a different shape.
+ */
+export function resolveRetentionDays(
+  configured: string | number | undefined | null
+): number {
+  const days = typeof configured === "number" ? configured : Number(configured);
+  if (!Number.isInteger(days) || days <= 0) return DEFAULT_ANALYTICS_RETENTION_DAYS;
+  if (days > MAX_ANALYTICS_RETENTION_DAYS) return DEFAULT_ANALYTICS_RETENTION_DAYS;
+  return days;
+}
+
+/**
+ * Delete analytics rows older than `olderThanDays` and report how many went.
+ *
+ * The window is a SQL parameter, not interpolated text: `cleanupOldAnalytics`
+ * used to splice the number into the statement, which only ever sat behind a
+ * `z.number()` input on the tRPC side — one unvalidated caller away from
+ * injection. A non-integer is refused here rather than passed on, so neither
+ * path can delete on a window the code did not check.
+ */
+export async function pruneVisitorAnalytics(
+  conn: AnalyticsConn,
+  olderThanDays: number
+): Promise<number> {
+  const days = Math.trunc(Number(olderThanDays));
+  if (!Number.isInteger(days) || days <= 0) {
+    throw new Error(
+      `Refusing to prune VisitorAnalytics on a non-positive window: ${olderThanDays}`
+    );
+  }
+
+  const result = await conn.execute({
+    sql: `DELETE FROM VisitorAnalytics
+          WHERE created_at < datetime('now', ?)
+          RETURNING id`,
+    args: [`-${days} days`]
+  });
+
+  // Counted from `RETURNING id` rather than `rowsAffected`, keeping the contract
+  // `cleanupOldAnalytics` had: the caller is told how many rows actually went.
+  return result.rows.length;
+}
+
+/** Whether a cron may run: Bearer match, and an unset secret denies everything. */
+export function isAuthorizedCron(
+  authorization: string | null | undefined,
+  secret: string | undefined | null
+): boolean {
+  if (!secret) return false;
+  const provided = (authorization ?? "").replace(/^Bearer\s+/i, "").trim();
+  return Boolean(provided) && provided === secret;
+}
+
+export interface RetentionOutcome {
+  status: number;
+  body: Record<string, unknown>;
+}
+
+/**
+ * The whole cron decision, away from the route: authorise, resolve the window,
+ * prune. Returns the status and body the route should send, so the behaviour is
+ * testable without a database or an environment.
+ *
+ * Fail closed on auth (an unauthenticated trigger deleting real history is the
+ * worst thing this endpoint can do), and 500 rather than a silent zero on a
+ * failed prune — a retention job that quietly stopped pruning is exactly the
+ * state that created this issue.
+ */
+export async function runRetentionCron(options: {
+  authorization: string | null | undefined;
+  secret: string | undefined | null;
+  configuredDays: string | number | undefined | null;
+  conn: AnalyticsConn;
+}): Promise<RetentionOutcome> {
+  if (!isAuthorizedCron(options.authorization, options.secret)) {
+    return { status: 401, body: { error: "Unauthorized" } };
+  }
+
+  const days = resolveRetentionDays(options.configuredDays);
+
+  try {
+    const deleted = await pruneVisitorAnalytics(options.conn, days);
+    return { status: 200, body: { success: true, days, deleted } };
+  } catch (error) {
+    console.error("Analytics retention job failed:", error);
+    return { status: 500, body: { error: "Analytics retention job failed", days } };
+  }
+}

src/server/analytics-schema.test.ts

@@ -40,7 +40,9 @@ const sampleArgs = [
   2, // inp
   3, // ttfb
   4, // dom_load
-  5 // load_complete
+  5, // load_complete
+  "click", // stage
+  "nook.freno.me" // host
 ];
 
 beforeEach(() => {
@@ -69,7 +71,7 @@ describe("VisitorAnalytics schema", () => {
     }
     // The reverse direction matters too: a column the code writes but the
     // DDL omits is the failure this whole file exists to catch.
-    expect(VISITOR_ANALYTICS_COLUMNS.length).toBe(23);
+    expect(VISITOR_ANALYTICS_COLUMNS.length).toBe(25);
   });
 
   it("has one placeholder per inserted column", () => {
@@ -83,7 +85,7 @@ describe("VisitorAnalytics schema", () => {
 
     const row = db
       .prepare(
-        `SELECT path, country, utm_source, utm_medium, utm_campaign
+        `SELECT path, country, utm_source, utm_medium, utm_campaign, stage, host
          FROM VisitorAnalytics`
       )
       .get() as Record<string, unknown>;
@@ -93,7 +95,9 @@ describe("VisitorAnalytics schema", () => {
       country: "GB",
       utm_source: "hn",
       utm_medium: "post",
-      utm_campaign: "launch"
+      utm_campaign: "launch",
+      stage: "click",
+      host: "nook.freno.me"
     });
   });
 

src/server/analytics-schema.ts

@@ -51,6 +51,8 @@ export const VISITOR_ANALYTICS_DDL = `
     utm_source TEXT,
     utm_medium TEXT,
     utm_campaign TEXT,
+    stage TEXT,
+    host TEXT,
     FOREIGN KEY (user_id) REFERENCES User(id) ON DELETE SET NULL
   )
 `;
@@ -79,6 +81,18 @@ export const VISITOR_ANALYTICS_ADDED_COLUMNS = [
   {
     name: "utm_campaign",
     ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN utm_campaign TEXT`
+  },
+  {
+    // Which funnel stage a download row is: "click" (a door) or "served"
+    // (the file route). Null on every non-download row (FRE-366).
+    name: "stage",
+    ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN stage TEXT`
+  },
+  {
+    // The Host the client addressed, e.g. "nook.freno.me" — the site
+    // dimension the table never had (FRE-366).
+    name: "host",
+    ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN host TEXT`
   }
 ];
 
@@ -110,7 +124,9 @@ export const VISITOR_ANALYTICS_COLUMNS = [
   "inp",
   "ttfb",
   "dom_load",
-  "load_complete"
+  "load_complete",
+  "stage",
+  "host"
 ] as const;
 
 /**

src/server/analytics.ts

@@ -9,6 +9,8 @@ import {
   VISITOR_ANALYTICS_INDEXES,
   VISITOR_ANALYTICS_INSERT
 } from "./analytics-schema";
+import { truncateIpAddress } from "./request-client";
+import { pruneVisitorAnalytics } from "./analytics-retention";
 
 /**
  * Idempotent schema bootstrap, mirroring `nook.ts`. The table had no DDL in
@@ -64,6 +66,10 @@ export interface AnalyticsEntry {
   ttfb?: number | null;
   domLoad?: number | null;
   loadComplete?: number | null;
+  /** Funnel stage for download rows ("click" | "served"); null elsewhere. */
+  stage?: string | null;
+  /** Host the client addressed, e.g. "nook.freno.me" — the site dimension. */
+  host?: string | null;
 }
 
 /**
@@ -124,7 +130,9 @@ async function flushAnalyticsBuffer(): Promise<void> {
           entry.inp || null,
           entry.ttfb || null,
           entry.domLoad || null,
-          entry.loadComplete || null
+          entry.loadComplete || null,
+          entry.stage || null,
+          entry.host || null
         ]
       });
     }
@@ -258,6 +266,8 @@ export async function queryAnalytics(
     utm_medium: row.utm_medium as string | null,
     utm_campaign: row.utm_campaign as string | null,
     duration_ms: row.duration_ms as number | null,
+    stage: row.stage as string | null,
+    host: row.host as string | null,
     created_at: row.created_at as string
   }));
 }
@@ -309,6 +319,10 @@ export async function getAnalyticsSummary(days: number = 30): Promise<{
   });
   const totalApiCalls = (totalApiCallsResult.rows[0]?.count as number) || 0;
 
+  // Since the FRE-369 privacy floor, `ip_address` holds a truncated prefix
+  // (/24 for v4, /56 for v6), so this counts distinct prefixes — households and
+  // mobile exits, not machines. It was never accurate before that either: every
+  // row held the proxy's loopback, so the count was 1.
   const uniqueVisitorsResult = await conn.execute({
     sql: `SELECT COUNT(DISTINCT ip_address) as count FROM VisitorAnalytics 
           WHERE ip_address IS NOT NULL 
@@ -620,19 +634,15 @@ export async function getPerformanceStats(days: number = 30): Promise<{
   };
 }
 
+/**
+ * Delete visitor analytics older than `olderThanDays`. Reachable from tRPC
+ * (admin) and from the daily retention cron (`/api/analytics/retention`);
+ * the SQL and the input rules live in `./analytics-retention`.
+ */
 export async function cleanupOldAnalytics(
   olderThanDays: number
 ): Promise<number> {
-  const conn = ConnectionFactory();
-
-  const result = await conn.execute({
-    sql: `DELETE FROM VisitorAnalytics 
-          WHERE created_at < datetime('now', '-${olderThanDays} days')
-          RETURNING id`,
-    args: []
-  });
-
-  return result.rows.length;
+  return pruneVisitorAnalytics(ConnectionFactory(), olderThanDays);
 }
 
 function parseUserAgent(userAgent?: string): {
@@ -667,6 +677,16 @@ function parseUserAgent(userAgent?: string): {
   return { deviceType, browser, os };
 }
 
+/**
+ * Fill in what the writer did not supply, and apply the privacy floor.
+ *
+ * Every analytics entry — page views, tRPC context, downloads, checkout —
+ * passes through here, which is why the truncation lives here and not at the
+ * call sites: a future writer cannot forget it, and no caller can persist a
+ * full client address by accident (FRE-369, finding Q4). The audit log
+ * (`./audit.ts`) is a separate, pre-existing surface with its own stakes and is
+ * deliberately untouched by this change.
+ */
 export function enrichAnalyticsEntry(entry: AnalyticsEntry): AnalyticsEntry {
   const { deviceType, browser, os } = parseUserAgent(
     entry.userAgent || undefined
@@ -674,6 +694,7 @@ export function enrichAnalyticsEntry(entry: AnalyticsEntry): AnalyticsEntry {
 
   return {
     ...entry,
+    ipAddress: truncateIpAddress(entry.ipAddress),
     deviceType: entry.deviceType || deviceType,
     browser: entry.browser || browser,
     os: entry.os || os

src/server/api/routers/analytics.ts

@@ -10,7 +10,7 @@ import {
 } from "~/server/analytics";
 import { ConnectionFactory } from "~/server/database";
 import { v4 as uuid } from "uuid";
-import { getRequestIP } from "vinxi/http";
+import { resolveClientIp, requestHost } from "~/server/request-client";
 import { parseUtm } from "~/server/utm";
 import { VISITOR_ANALYTICS_INSERT } from "~/server/analytics-schema";
 
@@ -98,7 +98,8 @@ export const analyticsRouter = createTRPCRouter({
             getHeader(req.headers, "referer") ||
             getHeader(req.headers, "referrer") ||
             getHeader(ctx.event.request?.headers, "referer");
-          const ipAddress = getRequestIP(ctx.event.nativeEvent) || undefined;
+          const ipAddress = resolveClientIp(ctx.event) || undefined;
+          const host = requestHost(ctx.event) || undefined;
           const enriched = enrichAnalyticsEntry({
             userId: ctx.userId,
             path: input.path,
@@ -106,6 +107,7 @@ export const analyticsRouter = createTRPCRouter({
             userAgent,
             referrer,
             ipAddress,
+            host,
             // The client beacon sends `pathname + search`, so the page URL's
             // own query string is the campaign source; the referrer fills any
             // gap (e.g. clicks that arrive on a clean path).
@@ -145,7 +147,9 @@ export const analyticsRouter = createTRPCRouter({
               enriched.inp || null,
               enriched.ttfb || null,
               enriched.domLoad || null,
-              enriched.loadComplete || null
+              enriched.loadComplete || null,
+              enriched.stage || null,
+              enriched.host || null
             ]
           });
 

src/server/api/routers/downloads.ts

@@ -79,7 +79,7 @@ export const downloadsRouter = createTRPCRouter({
       // exactly the event the funnel needs to show. Unknown asset names are
       // skipped rather than recorded as a download that never happened.
       if (assets[input.asset_name] || latestAssets[input.asset_name]) {
-        await trackDownload(ctx.event, input.asset_name);
+        await trackDownload(ctx.event, input.asset_name, "click");
       }
 
       const bucket = env.VITE_DOWNLOAD_BUCKET_STRING;

src/server/api/utils.ts

@@ -1,7 +1,7 @@
 import { initTRPC, TRPCError } from "@trpc/server";
 import type { APIEvent } from "@solidjs/start/server";
 import { logVisit, enrichAnalyticsEntry } from "~/server/analytics";
-import { getRequestIP } from "vinxi/http";
+import { resolveClientIp, requestHost } from "~/server/request-client";
 import { getAuthPayloadFromEvent } from "~/server/auth";
 
 export type Context = {
@@ -51,7 +51,11 @@ async function createContextInner(event: APIEvent): Promise<Context> {
     getHeader(req.headers, "referer") ||
     getHeader(req.headers, "referrer") ||
     getHeader(event.request?.headers, "referer");
-  const ipAddress = getRequestIP(event.nativeEvent) || undefined;
+  // Behind the production proxy the direct peer is loopback, so the real
+  // client address comes from the forwarded header the proxy sets — see
+  // `~/server/request-client` for the trust boundary (FRE-366).
+  const ipAddress = resolveClientIp(event) || undefined;
+  const host = requestHost(event) || undefined;
 
   // Don't log the performance logging endpoint itself to avoid circular tracking
   if (!path.includes("analytics.logPerformance")) {
@@ -62,7 +66,8 @@ async function createContextInner(event: APIEvent): Promise<Context> {
         method,
         userAgent,
         referrer,
-        ipAddress
+        ipAddress,
+        host
       })
     );
   }

src/server/request-client.test.ts

@@ -0,0 +1,203 @@
+import { beforeEach, describe, expect, it, mock } from "bun:test";
+
+// The trust boundary of FRE-366, as corrected by the FRE-369 security review.
+// Two things must stay true here:
+//   1. The address we record is the visitor, not whoever typed a header first.
+//      Cloudflare and the origin proxy both *append* to x-forwarded-for, so the
+//      leftmost hop is the client's own invention — `cf-connecting-ip` (which
+//      Cloudflare overwrites) comes first, then the hop below the trusted edge.
+//   2. What reaches the database is a prefix, never a full address
+//      (`truncateIpAddress`), because a visitor row is not worth a precise
+//      location.
+// A regression on (1) re-records whatever an attacker claims; on (2) it stores
+// personal data we promised not to.
+
+let directIp: string | null = null;
+mock.module("vinxi/http", () => ({
+  getRequestIP: () => directIp
+}));
+
+const {
+  resolveClientIp,
+  isLoopbackAddress,
+  isIpAddress,
+  truncateIpAddress,
+  normaliseHost,
+  requestHost
+} = await import("./request-client");
+
+function event(
+  headers: Record<string, string> = {},
+  url = "https://nook.freno.me/downloads"
+) {
+  return {
+    request: new Request(url, { headers }),
+    nativeEvent: {}
+  } as never;
+}
+
+beforeEach(() => {
+  directIp = null;
+});
+
+describe("resolveClientIp", () => {
+  it("prefers Cloudflare's connecting address over any forwarded header", () => {
+    directIp = "127.0.0.1";
+    expect(
+      resolveClientIp(
+        event({
+          "cf-connecting-ip": "203.0.113.7",
+          "x-forwarded-for": "198.51.100.9, 203.0.113.7, 198.51.100.1"
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("takes the hop below the trusted edge — never the leftmost one", () => {
+    // client → Cloudflare → same-host origin proxy: the sender's spoof is
+    // first, the real client second, the edge last.
+    directIp = "::1";
+    expect(
+      resolveClientIp(
+        event({
+          "x-forwarded-for": "198.51.100.9, 203.0.113.7, 198.51.100.1"
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("refuses a single-hop forwarded header: nothing above it to discount", () => {
+    directIp = "127.0.0.1";
+    expect(
+      resolveClientIp(event({ "x-forwarded-for": "198.51.100.9" }))
+    ).toBe("127.0.0.1");
+  });
+
+  it("ignores a forwarded header from a real peer — it is spoofable", () => {
+    directIp = "203.0.113.7";
+    expect(
+      resolveClientIp(
+        event({
+          "cf-connecting-ip": "198.51.100.9",
+          "x-forwarded-for": "198.51.100.9, 10.0.0.1"
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("ignores a header value that is not an address", () => {
+    directIp = "127.0.0.1";
+    expect(
+      resolveClientIp(
+        event({
+          "cf-connecting-ip": "not-an-address",
+          "x-forwarded-for": "198.51.100.9, 203.0.113.7, 198.51.100.1"
+        })
+      )
+    ).toBe("203.0.113.7");
+
+    directIp = "127.0.0.1";
+    expect(
+      resolveClientIp(event({ "cf-connecting-ip": "evil.example.com" }))
+    ).toBe("127.0.0.1");
+  });
+
+  it("keeps the loopback peer when the proxy set nothing usable", () => {
+    directIp = "::ffff:127.0.0.1";
+    expect(resolveClientIp(event())).toBe("::ffff:127.0.0.1");
+  });
+
+  it("records nothing rather than inventing an address", () => {
+    expect(resolveClientIp(event())).toBeNull();
+  });
+});
+
+describe("isLoopbackAddress", () => {
+  it("covers the v4 range and every spelling of v6 loopback", () => {
+    expect(isLoopbackAddress("127.0.0.1")).toBe(true);
+    expect(isLoopbackAddress("127.0.0.53")).toBe(true);
+    expect(isLoopbackAddress("::1")).toBe(true);
+    expect(isLoopbackAddress("0:0:0:0:0:0:0:1")).toBe(true);
+    expect(isLoopbackAddress("::ffff:127.0.0.1")).toBe(true);
+    expect(isLoopbackAddress("::ffff:127.9.9.9")).toBe(true);
+  });
+
+  it("is not loopback for a real peer, or for text it cannot parse", () => {
+    expect(isLoopbackAddress("203.0.113.7")).toBe(false);
+    expect(isLoopbackAddress("10.0.0.1")).toBe(false);
+    expect(isLoopbackAddress("localhost")).toBe(false);
+    expect(isLoopbackAddress("127.0.53")).toBe(false); // not a complete address
+    expect(isLoopbackAddress("")).toBe(false);
+  });
+});
+
+describe("isIpAddress", () => {
+  it("accepts a dotted quad and an IPv6 literal", () => {
+    expect(isIpAddress("198.51.100.9")).toBe(true);
+    expect(isIpAddress("2001:db8::1")).toBe(true);
+    expect(isIpAddress("::ffff:198.51.100.9")).toBe(true);
+  });
+
+  it("rejects anything a proxy could not have meant", () => {
+    expect(isIpAddress("999.1.1.1")).toBe(false);
+    expect(isIpAddress("1.2.3")).toBe(false);
+    expect(isIpAddress("2001:db8:::1")).toBe(false);
+    expect(isIpAddress("evil.example.com")).toBe(false);
+    expect(isIpAddress("")).toBe(false);
+    expect(isIpAddress(null)).toBe(false);
+  });
+});
+
+describe("truncateIpAddress", () => {
+  it("cuts IPv4 to its /24", () => {
+    expect(truncateIpAddress("198.51.100.9")).toBe("198.51.100.0");
+    expect(truncateIpAddress("127.0.0.1")).toBe("127.0.0.0");
+  });
+
+  it("cuts IPv6 to its /56, uncompressed and stable", () => {
+    expect(
+      truncateIpAddress("2001:db8:85a3:1234:5678:9abc:defa:1234")
+    ).toBe("2001:db8:85a3:1200:0:0:0:0");
+    // The compressed spelling of the same address truncates to the same string,
+    // so `COUNT(DISTINCT ip_address)` cannot be split by how a proxy rendered it.
+    expect(truncateIpAddress("2001:db8:85a3:1234::")).toBe(
+      "2001:db8:85a3:1200:0:0:0:0"
+    );
+    expect(truncateIpAddress("::1")).toBe("0:0:0:0:0:0:0:0");
+  });
+
+  it("treats an IPv4-mapped v6 address as the v4 it is", () => {
+    expect(truncateIpAddress("::ffff:198.51.100.9")).toBe("198.51.100.0");
+  });
+
+  it("drops what it cannot parse rather than storing it", () => {
+    expect(truncateIpAddress("203.0.113.999")).toBeNull();
+    expect(truncateIpAddress("<script>alert(1)</script>")).toBeNull();
+    expect(truncateIpAddress("")).toBeNull();
+    expect(truncateIpAddress(null)).toBeNull();
+    expect(truncateIpAddress(undefined)).toBeNull();
+  });
+});
+
+describe("normaliseHost", () => {
+  it("lowercases and strips the port so the weekly host scope can match", () => {
+    expect(normaliseHost("Nook.Freno.ME:443")).toBe("nook.freno.me");
+    expect(normaliseHost("nook.freno.me.")).toBe("nook.freno.me");
+    expect(normaliseHost("[::1]:3000")).toBe("::1");
+    expect(normaliseHost("  ")).toBeNull();
+    expect(normaliseHost(null)).toBeNull();
+  });
+});
+
+describe("requestHost", () => {
+  it("reads the host the client addressed from the request URL", () => {
+    expect(requestHost(event())).toBe("nook.freno.me");
+    expect(requestHost(event({}, "https://freno.me/"))).toBe("freno.me");
+  });
+
+  it("normalises a host that carries a port", () => {
+    expect(requestHost(event({}, "https://nook.freno.me:443/downloads"))).toBe(
+      "nook.freno.me"
+    );
+  });
+});

src/server/request-client.ts

@@ -0,0 +1,282 @@
+import type { APIEvent } from "@solidjs/start/server";
+import { getRequestIP } from "vinxi/http";
+import { hostFromEventLike } from "./site-context-server";
+
+/**
+ * The client address and the site a request addressed, for the analytics
+ * writers (FRE-366).
+ *
+ * Until this existed every writer called `getRequestIP` and only looked at
+ * `x-forwarded-for` when it returned null — so behind the production proxy the
+ * recorded address was the proxy itself, and all 12,035 addressed rows in
+ * VisitorAnalytics carried 127.0.0.1 / ::1.
+ *
+ * STANDING RULE (FRE-369, the security review of that change): nothing here is
+ * a security signal. The address is a *best guess at a visitor*, not an
+ * authenticated identity — do not import it into auth, rate limiting, abuse
+ * control, fraud checks, or anything that grants or denies access. A caller
+ * that needs a security decision must re-derive trust for its own stakes; the
+ * blast radius of a wrong answer here is a poisoned analytics row, and that is
+ * the only reason the shortcuts below are acceptable.
+ *
+ * Trust boundary, stated plainly: `x-forwarded-for` is client-controlled text.
+ * It is believed only when the direct peer is a loopback address — the
+ * production shape, where a same-host proxy (nginx/caddy) terminates TLS and
+ * sets the header itself. Anywhere else the direct peer IS the client (dev,
+ * direct hit) and a forwarded header from it is ignored as spoofable.
+ *
+ * Which hop is the visitor (FRE-369 finding Q1): the request arrives as
+ * `client → Cloudflare → same-host origin proxy → app`, and both proxies
+ * *append*. So `x-forwarded-for` reaches us as
+ * `<client-supplied spoof…>, <true client>, <Cloudflare edge>` and the leftmost
+ * hop is exactly the one the sender chose — the original FRE-366 read took it
+ * and was wrong. Order of preference:
+ *
+ *   1. `cf-connecting-ip` — Cloudflare *overwrites* this header, so a
+ *      client-supplied value never survives the edge. Authoritative.
+ *   2. `x-forwarded-for`, counted from the right: drop the rightmost hop (the
+ *      Cloudflare edge address the origin proxy appended) and take the one
+ *      before it. Never the leftmost.
+ *   3. the direct peer, as before.
+ *
+ * h3's own `getRequestIP(event, { xForwardedFor: true })` also takes the
+ * leftmost hop, so it is not a fix for this and must not be swapped in.
+ *
+ * Residual caveat, deliberately not papered over: step 2 assumes the pinned
+ * topology (one trusted proxy above us, i.e. Cloudflare). Behind a *single*
+ * forwarding proxy that does not set `cf-connecting-ip`, a client that sent its
+ * own `x-forwarded-for` sits at the hop we read. That is why the value is only
+ * ever stored truncated (see `truncateIpAddress`) and why it stays out of every
+ * security path. Deploy-side precondition this repo cannot verify: the origin
+ * must be reachable only from Cloudflare (origin firewall / authenticated
+ * origin pull). If it is directly reachable, an attacker can set every header
+ * in this file and no read here can recover the truth.
+ */
+
+/** Cloudflare's "who actually connected" header: overwritten at the edge. */
+const CF_CONNECTING_IP = "cf-connecting-ip";
+const X_FORWARDED_FOR = "x-forwarded-for";
+
+/**
+ * Hops discounted from the right of `x-forwarded-for` before the value is
+ * taken: the Cloudflare edge address the origin proxy appended. Pinned to 1
+ * because there is exactly one trusted proxy between Cloudflare and the app;
+ * raise it only if the topology gains a trusted hop.
+ */
+const FORWARDED_HOPS_TO_DISCOUNT = 1;
+
+function header(event: APIEvent, name: string): string | null {
+  try {
+    return event.request?.headers?.get?.(name) ?? null;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * The visitor's address, resolved through the trust boundary above.
+ *
+ * Never store the result of this directly: `enrichAnalyticsEntry` runs every
+ * analytics entry through {@link truncateIpAddress} so a full address cannot
+ * reach the database even from a writer that forgets. Callers that need the
+ * un-truncated value must justify it in review.
+ */
+export function resolveClientIp(event: APIEvent): string | null {
+  const direct = getRequestIP(event.nativeEvent);
+
+  // A real peer is its own client: believed, and no forwarded header from it
+  // is (it would be the peer lying about who it is).
+  if (direct && !isLoopbackAddress(direct)) return direct;
+
+  // 1. Cloudflare's own record of the connecting address.
+  const connecting = cleanHop(header(event, CF_CONNECTING_IP));
+  if (isIpAddress(connecting)) return connecting;
+
+  // 2. Rightmost-minus-the-trusted-hop, never leftmost.
+  const hops = (header(event, X_FORWARDED_FOR) ?? "")
+    .split(",")
+    .map(cleanHop)
+    .filter(Boolean);
+  const index = hops.length - 1 - FORWARDED_HOPS_TO_DISCOUNT;
+  if (index >= 0) {
+    const candidate = hops[index]!;
+    if (isIpAddress(candidate)) return candidate;
+  }
+
+  // 3. Nothing trustworthy: record what we actually saw, even though it is the
+  // proxy. `readTraffic` reports these rows rather than hiding them.
+  return direct ?? null;
+}
+
+function cleanHop(value: string | null | undefined): string {
+  return (value ?? "").trim();
+}
+
+/**
+ * Whether an address is the loopback the production proxy presents.
+ *
+ * Parses rather than string-matches, so the forms a real peer can present in —
+ * `::ffff:127.0.0.53`, `0:0:0:0:0:0:0:1`, an IPv4-mapped literal — are all
+ * recognised. An address this does not parse is *not* loopback, which fails
+ * closed: forwarded headers from an unrecognised peer are ignored rather than
+ * believed.
+ */
+export function isLoopbackAddress(ip: string): boolean {
+  const bytes = addressBytes(ip);
+  if (!bytes) return false;
+  if (bytes.length === 4) return bytes[0] === 127;
+
+  // IPv6 loopback ::1 — and ::ffff:127/8, which is loopback too.
+  const mapped = ipv4FromBytes(bytes);
+  if (mapped) return isLoopbackAddress(mapped);
+  const tail = bytes[15]!;
+  return bytes.slice(0, 15).every((byte) => byte === 0) && tail === 1;
+}
+
+// ── Addresses: parse, validate, truncate ────────────────────────────────────
+
+/**
+ * The privacy floor the security review required before this ships (FRE-369,
+ * finding Q4): VisitorAnalytics must never hold a full client address. IPv4 is
+ * cut to /24, IPv6 to /56 — the prefix that identifies a household and that a
+ * rotating "privacy" IPv6 address does not actually change. Funnel stages count
+ * rows, and `uniqueVisitors` survives as a count of distinct prefixes, so
+ * nothing measured here needs the last octet.
+ *
+ * An address that does not parse is dropped rather than stored: an unparseable
+ * string is client text we cannot reason about, and keeping it would smuggle
+ * the very granularity this function exists to remove.
+ */
+export function truncateIpAddress(ip: string | null | undefined): string | null {
+  const value = cleanHop(ip);
+  if (!value) return null;
+
+  const bytes = addressBytes(value);
+  if (!bytes) return null;
+
+  const mapped = bytes.length === 16 ? ipv4FromBytes(bytes) : null;
+  if (bytes.length === 4 || mapped) {
+    const octets = (mapped ?? value).split(".").map(Number) as number[];
+    return `${octets[0]}.${octets[1]}.${octets[2]}.0`;
+  }
+
+  // /56 = keep the first 7 bytes, zero the rest. Rendered uncompressed and
+  // lowercase so one client always truncates to one stored string.
+  const kept = bytes.map((_, i) => (i >= 7 ? 0 : _));
+  const groups: string[] = [];
+  for (let i = 0; i < 16; i += 2) {
+    groups.push((((kept[i]! << 8) | kept[i + 1]!) >>> 0).toString(16));
+  }
+  return groups.join(":");
+}
+
+/** Any address this file understands: a dotted quad or an IPv6 literal. */
+export function isIpAddress(value: string | null | undefined): boolean {
+  return value !== null && value !== undefined && addressBytes(value) !== null;
+}
+
+function isIpv4(value: string): boolean {
+  const parts = value.split(".");
+  if (parts.length !== 4) return false;
+  return parts.every(
+    (part) => /^\d{1,3}$/.test(part) && Number(part) <= 255 && !/^0\d/.test(part)
+  );
+}
+
+/** 4 bytes for IPv4, 16 for IPv6 (including `::ffff:a.b.c.d`), else null. */
+function addressBytes(value: string): number[] | null {
+  const raw = value.trim().replace(/^\[|\]$/g, "").split("%")[0]!;
+  if (isIpv4(raw)) return raw.split(".").map(Number);
+  if (!raw.includes(":")) return null;
+
+  const halves = raw.split("::");
+  if (halves.length > 2) return null;
+  const compressed = halves.length === 2;
+
+  const head = groupsToBytes(halves[0] ?? "");
+  const tail = groupsToBytes(halves[1] ?? "");
+  if (!head || !tail) return null;
+
+  const total = head.length + tail.length;
+  if (compressed) {
+    if (total > 8) return null;
+    return [...head, ...new Array(8 - total).fill(0), ...tail].flatMap(split16);
+  }
+  if (total !== 8) return null;
+  return [...head, ...tail].flatMap(split16);
+}
+
+/** One side of `::` as 16-bit groups; a trailing `1.2.3.4` counts as two. */
+function groupsToBytes(side: string): number[] | null {
+  if (side === "") return [];
+  const out: number[] = [];
+  for (const part of side.split(":")) {
+    if (/^[0-9a-fA-F]{1,4}$/.test(part)) {
+      out.push(parseInt(part, 16));
+    } else if (isIpv4(part)) {
+      const o = part.split(".").map(Number);
+      out.push((o[0]! << 8) | o[1]!, (o[2]! << 8) | o[3]!);
+    } else {
+      return null;
+    }
+  }
+  return out;
+}
+
+function split16(group: number): number[] {
+  return [(group >> 8) & 0xff, group & 0xff];
+}
+
+/** The embedded IPv4 of an IPv4-mapped IPv6 address (`::ffff:1.2.3.4`), else null. */
+function ipv4FromBytes(bytes: number[]): string | null {
+  const prefix = bytes.slice(0, 10).every((byte) => byte === 0);
+  const marker = bytes[10] === 0xff && bytes[11] === 0xff;
+  if (!prefix || !marker) return null;
+  return `${bytes[12]}.${bytes[13]}.${bytes[14]}.${bytes[15]}`;
+}
+
+/**
+ * The site the request addressed — the `Host` the client asked for, normalised
+ * to `nook.freno.me`. Recorded on every analytics row so a weekly read can
+ * scope to one site; without it `/` on nook.freno.me is the same row as `/` on
+ * freno.me. Thin alias over the site-context extractor so the analytics writers
+ * share one host-parsing implementation with routing.
+ *
+ * Normalised (lowercased, port stripped) because the weekly read scopes with
+ * `host = ?`: `nook.freno.me:443` or `Nook.Freno.ME` would otherwise store a
+ * value that silently never matches the scope (FRE-369, finding Q3 — no
+ * injection risk, but a correctness one worth fixing at write time).
+ */
+export function requestHost(event: APIEvent): string | null {
+  return normaliseHost(hostFromEventLike(event)) ?? normaliseHost(urlHost(event));
+}
+
+function urlHost(event: APIEvent): string | null {
+  // `host` is a forbidden header and some server shapes drop it from
+  // `event.request.headers`; the request URL is absolute on the server and
+  // carries the same host the client addressed.
+  try {
+    const url = event.request?.url;
+    return url ? new URL(url).host || null : null;
+  } catch {
+    return null;
+  }
+}
+
+/** Lowercase, drop the port and any trailing dot; null when nothing is left. */
+export function normaliseHost(host: string | null | undefined): string | null {
+  const raw = cleanHop(host).toLowerCase();
+  if (!raw) return null;
+
+  // A bracketed IPv6 literal keeps its brackets' contents (`[::1]:3000` →
+  // `::1`); anything else drops a trailing `:port`.
+  const closing = raw.indexOf("]");
+  const withoutPort = raw.startsWith("[")
+    ? closing > 0
+      ? raw.slice(1, closing)
+      : raw.slice(1)
+    : raw.replace(/:\d+$/, "");
+
+  const trimmed = withoutPort.replace(/\.$/, "");
+  return trimmed || null;
+}

src/server/security.ts

@@ -3,6 +3,7 @@ import { getCookie, setCookie } from "vinxi/http";
 import type { H3Event } from "vinxi/http";
 import { t } from "~/server/api/utils";
 import { logAuditEvent } from "~/server/audit";
+import { resolveTrustedClientIp } from "~/server/trusted-client-ip";
 import { env } from "~/env/server";
 import {
   AUTH_CONFIG,
@@ -328,39 +329,15 @@ async function cleanupExpiredRateLimits(): Promise<void> {
 }
 
 /**
- * Get client IP address from request headers.
- * Only trusts X-Forwarded-For outside of local development (set by the Vercel
- * edge network in production; trusted in tests so the header-parsing path is
- * exercised). In local development, uses the socket address to prevent header
- * spoofing.
+ * Get client IP address for a security decision (rate limiting, audit).
+ *
+ * Thin alias over `resolveTrustedClientIp` (./trusted-client-ip), which owns
+ * the trust order: the previous leftmost `x-forwarded-for` read was
+ * client-spoofable in our append-only proxy topology, and `x-real-ip` was
+ * read unconditionally — both fixed there (FRE-371).
  */
 export function getClientIP(event: H3Event): string {
-  // In production on Vercel, X-Forwarded-For is set by the edge network
-  // and cannot be spoofed by clients. In dev, ignore it.
-  if (env.NODE_ENV !== "development") {
-    const forwarded = getHeaderValue(event, "x-forwarded-for");
-    if (forwarded) {
-      return forwarded.split(",")[0].trim();
-    }
-    const realIP = getHeaderValue(event, "x-real-ip");
-    if (realIP) {
-      return realIP;
-    }
-  }
-
-  // Fallback: try socket remote address
-  try {
-    const nodeReq = event.node.req;
-    if (nodeReq?.socket?.remoteAddress) {
-      const addr = nodeReq.socket.remoteAddress;
-      // Clean up IPv6-mapped IPv4 addresses
-      return addr.replace(/^::ffff:/, "");
-    }
-  } catch {
-    // socket access failed
-  }
-
-  return "unknown";
+  return resolveTrustedClientIp(event);
 }
 
 /**

src/server/security/rate-limit.test.ts

@@ -150,29 +150,100 @@ describe("Rate Limiting", () => {
   });
 
   describe("getClientIP", () => {
-    it("should extract IP from x-forwarded-for header", () => {
+    // FRE-371: the read is spoof-resistant now. In the pinned topology
+    // (client → Cloudflare → origin proxy → app) both proxies APPEND, so
+    // x-forwarded-for arrives as `<spoof…>, <true client>, <CF edge>` and the
+    // leftmost hop is whatever the sender typed. Trust order:
+    // cf-connecting-ip, then x-forwarded-for rightmost-minus-one, then the
+    // socket peer — never the leftmost hop, never x-real-ip.
+
+    it("takes x-forwarded-for rightmost-minus-one, never the leftmost hop", () => {
       const event = createMockEvent({
-        headers: { "x-forwarded-for": "192.168.1.1, 10.0.0.1" }
+        headers: {
+          "x-forwarded-for": "6.6.6.6, 192.168.1.1, 10.0.0.1"
+        }
       });
 
       const ip = getClientIP(event);
       expect(ip).toBe("192.168.1.1");
     });
 
-    it("should extract IP from x-real-ip header", () => {
+    it("prefers cf-connecting-ip (Cloudflare overwrites it at the edge)", () => {
+      const event = createMockEvent({
+        headers: {
+          "cf-connecting-ip": "203.0.113.7",
+          "x-forwarded-for": "6.6.6.6, 192.168.1.1, 10.0.0.1"
+        }
+      });
+
+      const ip = getClientIP(event);
+      expect(ip).toBe("203.0.113.7");
+    });
+
+    it("a rotating leftmost x-forwarded-for does not change the resolved client", () => {
+      // The spoof the old read fell for: same Cloudflare-recorded client,
+      // different sender-chosen prefix per request. The key must be stable,
+      // or each rotation mints a fresh rate-limit bucket.
+      const first = getClientIP(
+        createMockEvent({
+          headers: {
+            "cf-connecting-ip": "203.0.113.7",
+            "x-forwarded-for": "1.1.1.1, 203.0.113.7, 10.0.0.1"
+          }
+        })
+      );
+      const second = getClientIP(
+        createMockEvent({
+          headers: {
+            "cf-connecting-ip": "203.0.113.7",
+            "x-forwarded-for": "2.2.2.2, 203.0.113.7, 10.0.0.1"
+          }
+        })
+      );
+
+      expect(first).toBe("203.0.113.7");
+      expect(second).toBe(first);
+    });
+
+    it("ignores x-real-ip entirely (client-settable in this topology)", () => {
       const event = createMockEvent({
         headers: { "x-real-ip": "192.168.1.2" }
       });
 
       const ip = getClientIP(event);
-      expect(ip).toBe("192.168.1.2");
+      expect(ip).toBe("unknown");
     });
 
-    it("should prefer x-forwarded-for over x-real-ip", () => {
+    it("ignores a client-supplied single-hop x-forwarded-for with no trusted proxy above it", () => {
+      // One hop only = the sender's own invention; discounting the trusted
+      // hop leaves nothing to believe.
       const event = createMockEvent({
+        headers: { "x-forwarded-for": "6.6.6.6" }
+      });
+
+      const ip = getClientIP(event);
+      expect(ip).toBe("unknown");
+    });
+
+    it("a non-loopback socket peer is the client and beats every header", () => {
+      const event = createMockEvent({
+        remoteAddress: "198.51.100.23",
         headers: {
-          "x-forwarded-for": "192.168.1.1",
-          "x-real-ip": "192.168.1.2"
+          "cf-connecting-ip": "6.6.6.6",
+          "x-forwarded-for": "6.6.6.6, 192.168.1.1, 10.0.0.1",
+          "x-real-ip": "6.6.6.6"
+        }
+      });
+
+      const ip = getClientIP(event);
+      expect(ip).toBe("198.51.100.23");
+    });
+
+    it("a loopback socket peer lets the forwarded headers speak", () => {
+      const event = createMockEvent({
+        remoteAddress: "127.0.0.1",
+        headers: {
+          "x-forwarded-for": "6.6.6.6, 192.168.1.1, 10.0.0.1"
         }
       });
 
@@ -198,13 +269,23 @@ describe("Rate Limiting", () => {
     it("should handle IPv6 addresses", () => {
       const event = createMockEvent({
         headers: {
-          "x-forwarded-for": "2001:0db8:85a3:0000:0000:8a2e:0370:7334"
+          "x-forwarded-for":
+            "2001:0db8:85a3:0000:0000:8a2e:0370:7334, 10.0.0.1"
         }
       });
 
       const ip = getClientIP(event);
       expect(ip).toBe("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
     });
+
+    it("rejects a candidate hop that is not an address", () => {
+      const event = createMockEvent({
+        headers: { "x-forwarded-for": "not-an-ip, 10.0.0.1" }
+      });
+
+      const ip = getClientIP(event);
+      expect(ip).toBe("unknown");
+    });
   });
 
   describe("rateLimitLogin", () => {

src/server/security/test-utils.ts

@@ -14,12 +14,15 @@ export function createMockEvent(options: {
   cookies?: Record<string, string>;
   method?: string;
   url?: string;
+  /** Socket peer the server observed; omit for a peer-less synthetic event. */
+  remoteAddress?: string;
 }): H3Event {
   const {
     headers = {},
     cookies = {},
     method = "POST",
-    url = "http://localhost:3000/"
+    url = "http://localhost:3000/",
+    remoteAddress
   } = options;
 
   // Build the cookie header string from the cookies object only
@@ -49,7 +52,10 @@ export function createMockEvent(options: {
     headers: headersObj,
     node: {
       req: {
-        headers: allHeaders
+        headers: allHeaders,
+        ...(remoteAddress
+          ? { socket: { remoteAddress } }
+          : {})
       },
       res: {
         cookies: {}

src/server/site-context-server.ts

@@ -38,9 +38,10 @@ export interface ServerSiteEvent {
  * the raw Node request via `event.nativeEvent.node.req.headers`. We try
  * both so this works for API route handlers (`APIEvent`), the SSR document
  * handler (`PageEvent`), and tRPC procedures operating on the underlying
- * `H3Event`.
+ * `H3Event`. Also used by the analytics writers to record the site dimension
+ * (`host` column) on every row (FRE-366).
  */
-function hostFromEventLike(event: ServerSiteEvent | unknown): string | null {
+export function hostFromEventLike(event: ServerSiteEvent | unknown): string | null {
   // 1) Web FetchEvent / APIEvent / PageEvent → standard `Request` headers.
   try {
     const req = (event as { request?: Request } | null)?.request;

src/server/track-download.ts

@@ -1,5 +1,4 @@
 import type { APIEvent } from "@solidjs/start/server";
-import { getRequestIP } from "vinxi/http";
 import {
   logVisit,
   enrichAnalyticsEntry,
@@ -7,6 +6,7 @@ import {
 } from "~/server/analytics";
 import { parseUtm } from "~/server/utm";
 import { downloadPath } from "~/lib/download-asset";
+import { resolveClientIp, requestHost } from "~/server/request-client";
 
 /**
  * Record one download event under the canonical path `/api/downloads/<asset>`.
@@ -15,35 +15,75 @@ import { downloadPath } from "~/lib/download-asset";
  * label per asset to group on: the tRPC click proxy, the S3 file route that
  * serves Sparkle updates, and the Nook door.
  *
+ * The `stage` is what separates the doors within one asset label: "click"
+ * for a door the visitor chose (landing button, tRPC proxy), "served" for
+ * the file route that actually streams the DMG. Until FRE-366 both wrote the
+ * same unlabelled row, so "download click → DMG served" could not be split —
+ * one served click wrote two identical rows. `summariseDownloads()` and the
+ * /analytics downloads block still group on the path alone; the stage is a
+ * separate column precisely so that grouping cannot change.
+ *
  * Best-effort by design — a failed analytics write must never block a
  * download. Flushed rather than merely buffered: a download is low-volume and
  * the 30s batch timer is not guaranteed to survive the response on serverless.
+ */
+export async function trackDownload(
+  event: APIEvent,
+  asset: string,
+  stage: DownloadStage
+): Promise<void> {
+  await trackRequest(event, {
+    path: downloadPath(asset),
+    method: "GET",
+    stage
+  });
+}
+
+export type DownloadStage = "click" | "served";
+
+export interface TrackedRequest {
+  path: string;
+  method: string;
+  /** Funnel stage label; only download rows carry one today. */
+  stage?: string | null;
+}
+
+/**
+ * Record one server-side request event with the shared visitor dimensions:
+ * referrer, campaign tags, edge country, real client address and host.
+ *
+ * This is the one writer every measured door goes through — download doors
+ * via `trackDownload`, the checkout door directly — so the dimensions can
+ * never drift between doors. Same best-effort, flushed-immediately contract
+ * as `trackDownload`.
  *
  * `event.request` is expected to be a Fetch `Request` (true for Vinxi route
  * handlers and browser tRPC calls). A server-side `createCaller` hands over a
  * Node request, whose headers have no `get` — that throw is swallowed below,
- * so such a click is not recorded. No server-side caller downloads today.
+ * so such an event is not recorded. No server-side caller downloads today.
  */
-export async function trackDownload(
+export async function trackRequest(
   event: APIEvent,
-  asset: string
+  tracked: TrackedRequest
 ): Promise<void> {
   const { request } = event;
 
   try {
     const referrer =
       request.headers.get("referer") ?? request.headers.get("referrer");
-    const forwarded = request.headers.get("x-forwarded-for");
 
     await logVisit(
       enrichAnalyticsEntry({
-        path: downloadPath(asset),
-        method: "GET",
+        path: tracked.path,
+        method: tracked.method,
+        stage: tracked.stage ?? null,
         referrer,
         userAgent: request.headers.get("user-agent"),
-        ipAddress:
-          getRequestIP(event.nativeEvent) ??
-          (forwarded ? forwarded.split(",")[0]!.trim() : null),
+        // Behind the production proxy the direct peer is loopback; the real
+        // client address comes from the forwarded header the proxy sets —
+        // see `~/server/request-client` for the trust boundary.
+        ipAddress: resolveClientIp(event),
+        host: requestHost(event),
         // The click carries its own tags when a campaign links straight at the
         // door; otherwise the landing page's tags ride along in the referer.
         ...parseUtm(request.url, referrer),
@@ -56,6 +96,6 @@ export async function trackDownload(
     );
     await flushAnalytics();
   } catch (error) {
-    console.error(`Failed to log download of ${asset}:`, error);
+    console.error(`Failed to log ${tracked.method} ${tracked.path}:`, error);
   }
 }

src/server/trusted-client-ip.test.ts

@@ -0,0 +1,171 @@
+import { describe, expect, it } from "bun:test";
+
+import {
+  resolveTrustedClientIp,
+  isIpAddress,
+  isLoopbackAddress,
+  UNKNOWN_CLIENT_IP,
+  type ClientIpEventLike
+} from "./trusted-client-ip";
+
+/**
+ * Unit coverage for the security-path client-IP read (FRE-371). The caller
+ * suites (rate-limit.test.ts, report.test.ts) pin the behaviour through
+ * getClientIP and the route; this file pins the helper's own edges: the peer
+ * forms a real socket can present, the APIEvent wrapper, and failing closed.
+ */
+function event(opts: {
+  headers?: Record<string, string>;
+  remoteAddress?: string;
+  wrapped?: boolean;
+}): ClientIpEventLike {
+  const inner: ClientIpEventLike = {
+    ...(opts.remoteAddress
+      ? { node: { req: { socket: { remoteAddress: opts.remoteAddress } } } }
+      : {}),
+    request: { headers: new Headers(opts.headers ?? {}) }
+  };
+  // An APIEvent wraps its H3Event as `nativeEvent` (routes/api/* shape).
+  return opts.wrapped ? { nativeEvent: inner } : inner;
+}
+
+describe("resolveTrustedClientIp", () => {
+  it("trust order: non-loopback peer, then cf-connecting-ip, then rightmost-minus-one x-forwarded-for", () => {
+    // Peer wins.
+    expect(
+      resolveTrustedClientIp(
+        event({
+          remoteAddress: "198.51.100.23",
+          headers: {
+            "cf-connecting-ip": "203.0.113.7",
+            "x-forwarded-for": "6.6.6.6, 203.0.113.7, 10.0.0.1"
+          }
+        })
+      )
+    ).toBe("198.51.100.23");
+
+    // Loopback peer → Cloudflare's record.
+    expect(
+      resolveTrustedClientIp(
+        event({
+          remoteAddress: "127.0.0.1",
+          headers: {
+            "cf-connecting-ip": "203.0.113.7",
+            "x-forwarded-for": "6.6.6.6, 203.0.113.7, 10.0.0.1"
+          }
+        })
+      )
+    ).toBe("203.0.113.7");
+
+    // No CF header → the hop below the trusted proxy.
+    expect(
+      resolveTrustedClientIp(
+        event({
+          remoteAddress: "127.0.0.1",
+          headers: { "x-forwarded-for": "6.6.6.6, 203.0.113.7, 10.0.0.1" }
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("reads the wrapped H3Event of an APIEvent-shaped input", () => {
+    expect(
+      resolveTrustedClientIp(
+        event({
+          wrapped: true,
+          remoteAddress: "127.0.0.1",
+          headers: { "cf-connecting-ip": "203.0.113.7" }
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("never returns a truncated address", () => {
+    // The full /128 is the bucket key; a truncated one would pool attackers
+    // with their neighbours.
+    const ip = resolveTrustedClientIp(
+      event({
+        remoteAddress: "127.0.0.1",
+        headers: { "cf-connecting-ip": "2001:0db8:85a3:0000:0000:8a2e:0370:7334" }
+      })
+    );
+    expect(ip).toBe("2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+  });
+
+  it("fails closed to the stricter unknown key", () => {
+    // One hop = the sender's own invention; a non-address hop; nothing at all.
+    expect(
+      resolveTrustedClientIp(
+        event({ headers: { "x-forwarded-for": "6.6.6.6" } })
+      )
+    ).toBe(UNKNOWN_CLIENT_IP);
+    expect(
+      resolveTrustedClientIp(
+        event({ headers: { "cf-connecting-ip": "not-an-ip" } })
+      )
+    ).toBe(UNKNOWN_CLIENT_IP);
+    expect(resolveTrustedClientIp(event({}))).toBe(UNKNOWN_CLIENT_IP);
+  });
+
+  it("recognises the loopback forms a real peer presents", () => {
+    expect(
+      resolveTrustedClientIp(
+        event({
+          // IPv6-mapped IPv4 loopback from a node socket.
+          remoteAddress: "::ffff:127.0.0.1",
+          headers: { "cf-connecting-ip": "203.0.113.7" }
+        })
+      )
+    ).toBe("203.0.113.7");
+    expect(
+      resolveTrustedClientIp(
+        event({
+          remoteAddress: "::1",
+          headers: { "cf-connecting-ip": "203.0.113.7" }
+        })
+      )
+    ).toBe("203.0.113.7");
+    // A non-loopback peer that merely looks odd is still the client.
+    expect(
+      resolveTrustedClientIp(
+        event({
+          remoteAddress: "::ffff:203.0.113.7",
+          headers: { "cf-connecting-ip": "6.6.6.6" }
+        })
+      )
+    ).toBe("203.0.113.7");
+  });
+
+  it("never believes x-real-ip", () => {
+    expect(
+      resolveTrustedClientIp(
+        event({ headers: { "x-real-ip": "203.0.113.7" } })
+      )
+    ).toBe(UNKNOWN_CLIENT_IP);
+  });
+});
+
+describe("address helpers", () => {
+  it("isIpAddress", () => {
+    expect(isIpAddress("203.0.113.7")).toBe(true);
+    expect(isIpAddress("::1")).toBe(true);
+    expect(isIpAddress("::ffff:203.0.113.7")).toBe(true);
+    expect(isIpAddress("2001:db8::1")).toBe(true);
+    expect(isIpAddress("not-an-ip")).toBe(false);
+    expect(isIpAddress("256.1.1.1")).toBe(false);
+    expect(isIpAddress("")).toBe(false);
+    expect(isIpAddress(null)).toBe(false);
+    expect(isIpAddress(undefined)).toBe(false);
+  });
+
+  it("isLoopbackAddress", () => {
+    expect(isLoopbackAddress("127.0.0.1")).toBe(true);
+    expect(isLoopbackAddress("127.255.0.3")).toBe(true);
+    expect(isLoopbackAddress("::1")).toBe(true);
+    expect(isLoopbackAddress("::ffff:127.0.0.1")).toBe(true);
+    expect(isLoopbackAddress("203.0.113.7")).toBe(false);
+    expect(isLoopbackAddress("::2")).toBe(false);
+    // Unparseable input is not loopback — fails closed.
+    expect(isLoopbackAddress("garbage")).toBe(false);
+  });
+});

src/server/trusted-client-ip.ts

@@ -0,0 +1,250 @@
+/**
+ * The client address, resolved for security decisions (FRE-371).
+ *
+ * This is the ONLY client-IP read allowed to feed a security decision — rate
+ * limiting, abuse quotas, account lockout, audit trails. It exists separately
+ * from `request-client.ts`'s `resolveClientIp` (the analytics read) on
+ * purpose:
+ *
+ *   - `resolveClientIp` carries a standing rule (FRE-369 review, 1d9287a):
+ *     it never feeds a security decision. Its blast radius is a poisoned
+ *     analytics row; ours is granting or denying access.
+ *   - Rate limiting needs the UN-truncated address; the analytics module only
+ *     ever hands out truncated prefixes.
+ *   - `request-client.ts` ships on FRE-366's branch, which is under review;
+ *     a security fix must not merge-depend on an analytics diff.
+ *
+ * The address-parsing primitives below are therefore a deliberate port of the
+ * reviewed ones, not an import. If the two ever drift, this file's version is
+ * the one security paths follow.
+ *
+ * WHY the old read was spoofable: `security.ts`'s `getClientIP` took the
+ * LEFTMOST `x-forwarded-for` hop, believing the header "cannot be spoofed".
+ * In the pinned topology — client → Cloudflare → same-host origin proxy →
+ * app — both proxies *append*, so the header arrives as
+ * `<client-supplied spoof…>, <true client>, <Cloudflare edge>` and the
+ * leftmost hop is exactly the one the sender chose. Rotating
+ * `X-Forwarded-For: <random>` per request minted a fresh rate-limit bucket
+ * and unlimited bug-report quota. `x-real-ip` was read unconditionally too;
+ * nothing in this topology overwrites it, so it is equally client text and is
+ * no longer read at all.
+ *
+ * Trust order (matches the convention FRE-366 landed after its security
+ * review, re-derived here for security stakes):
+ *
+ *   1. The direct socket peer, when it is a real routable address: it IS the
+ *      client (dev, direct hit), and any forwarded header from it is the
+ *      client describing itself — ignored.
+ *   2. `cf-connecting-ip` — reachable only when the peer is the loopback
+ *      origin proxy (or unreadable, e.g. a synthetic event in tests).
+ *      Cloudflare *overwrites* this header at the edge, so a client-supplied
+ *      value never survives. Must parse as an address.
+ *   3. `x-forwarded-for` counted from the RIGHT: drop the rightmost hop (the
+ *      Cloudflare edge address the origin proxy appended) and take the hop
+ *      before it. Never the leftmost. Must parse as an address.
+ *   4. "unknown" — callers give unknown-IP keys stricter shared limits, so
+ *      an unresolvable client fails toward a tighter bucket, never a looser
+ *      one.
+ *
+ * Deploy-side precondition this code cannot verify (same as the analytics
+ * read): the origin must be reachable only through Cloudflare and the
+ * same-host proxy. If the app is directly internet-reachable, a client can
+ * set every header read here and no in-app read can recover the truth.
+ */
+
+/** Cloudflare's "who actually connected" header: overwritten at the edge. */
+const CF_CONNECTING_IP = "cf-connecting-ip";
+const X_FORWARDED_FOR = "x-forwarded-for";
+
+/**
+ * Hops discounted from the right of `x-forwarded-for` before the value is
+ * taken: the Cloudflare edge address the origin proxy appended. Pinned to 1
+ * because there is exactly one trusted proxy between Cloudflare and the app;
+ * raise it only if the topology gains a trusted hop.
+ */
+const FORWARDED_HOPS_TO_DISCOUNT = 1;
+
+/** Returned when no trustworthy address exists. Callers rate-limit it harder. */
+export const UNKNOWN_CLIENT_IP = "unknown";
+
+/**
+ * The shapes this module reads: an H3Event (`security.ts` callers) or a
+ * SolidStart APIEvent (`routes/api/*`), which wraps its H3Event as
+ * `nativeEvent`. Every field is optional because test doubles are minimal;
+ * anything unreadable simply falls through to the next trust step.
+ */
+export interface ClientIpEventLike {
+  request?: { headers?: unknown };
+  headers?: unknown;
+  node?: {
+    req?: {
+      headers?: Record<string, unknown>;
+      socket?: { remoteAddress?: string };
+    };
+  };
+  nativeEvent?: ClientIpEventLike;
+}
+
+/**
+ * The client address for a security decision, per the trust order above.
+ * Never truncated: rate-limit buckets are per-address, and truncating would
+ * let an attacker share a bucket with the innocents behind their /24.
+ */
+export function resolveTrustedClientIp(event: ClientIpEventLike): string {
+  // An APIEvent's socket and (authoritative) headers live on the wrapped
+  // H3Event; a bare APIEvent-shaped double without one is read as-is.
+  const source = event.nativeEvent ?? event;
+
+  const peer = directPeer(source);
+  if (peer && isIpAddress(peer) && !isLoopbackAddress(peer)) return peer;
+
+  // The peer is the loopback origin proxy (or unreadable): forwarded headers
+  // may be believed, in the order above.
+  const connecting = cleanHop(headerValue(source, CF_CONNECTING_IP));
+  if (isIpAddress(connecting)) return connecting;
+
+  const hops = (headerValue(source, X_FORWARDED_FOR) ?? "")
+    .split(",")
+    .map(cleanHop)
+    .filter(Boolean);
+  const index = hops.length - 1 - FORWARDED_HOPS_TO_DISCOUNT;
+  if (index >= 0 && isIpAddress(hops[index])) return hops[index]!;
+
+  return UNKNOWN_CLIENT_IP;
+}
+
+/** The socket peer, IPv6-mapped IPv4 flattened (`::ffff:1.2.3.4` → `1.2.3.4`). */
+function directPeer(event: ClientIpEventLike): string | null {
+  try {
+    const addr = event.node?.req?.socket?.remoteAddress;
+    return addr ? addr.replace(/^::ffff:/, "") : null;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Header read tolerant of every event shape in play: Fetch-style
+ * `request.headers.get`, an h3 `event.headers` (Headers or plain object), and
+ * the raw node request headers. Header names are case-insensitive per HTTP;
+ * the plain-object fallbacks lowercase to match how node and the test doubles
+ * store them.
+ */
+function headerValue(event: ClientIpEventLike, name: string): string | null {
+  try {
+    const fromHeaders = (h: unknown): string | null => {
+      if (!h) return null;
+      if (typeof (h as Headers).get === "function") {
+        return (h as Headers).get(name) ?? null;
+      }
+      if (typeof h === "object") {
+        const record = h as Record<string, unknown>;
+        const value = record[name] ?? record[name.toLowerCase()];
+        return typeof value === "string" ? value : null;
+      }
+      return null;
+    };
+
+    return (
+      fromHeaders(event.request?.headers) ??
+      fromHeaders(event.headers) ??
+      fromHeaders(event.node?.req?.headers)
+    );
+  } catch {
+    return null;
+  }
+}
+
+function cleanHop(value: string | null | undefined): string {
+  return (value ?? "").trim();
+}
+
+// ── Addresses: parse and validate ───────────────────────────────────────────
+// Ported from the reviewed `request-client.ts` (FRE-366 @ 1d9287a); see the
+// header comment for why this is a copy and not an import.
+
+/** Any address this file understands: a dotted quad or an IPv6 literal. */
+export function isIpAddress(value: string | null | undefined): boolean {
+  return value !== null && value !== undefined && addressBytes(value) !== null;
+}
+
+/**
+ * Whether an address is the loopback the production proxy presents.
+ *
+ * Parses rather than string-matches, so the forms a real peer can present in —
+ * `::ffff:127.0.0.53`, `0:0:0:0:0:0:0:1`, an IPv4-mapped literal — are all
+ * recognised. An address this does not parse is *not* loopback, which fails
+ * closed: forwarded headers from an unrecognised peer are ignored rather than
+ * believed (a non-loopback peer short-circuits before headers are read).
+ */
+export function isLoopbackAddress(ip: string): boolean {
+  const bytes = addressBytes(ip);
+  if (!bytes) return false;
+  if (bytes.length === 4) return bytes[0] === 127;
+
+  // IPv6 loopback ::1 — and ::ffff:127/8, which is loopback too.
+  const mapped = ipv4FromBytes(bytes);
+  if (mapped) return isLoopbackAddress(mapped);
+  const tail = bytes[15]!;
+  return bytes.slice(0, 15).every((byte) => byte === 0) && tail === 1;
+}
+
+function isIpv4(value: string): boolean {
+  const parts = value.split(".");
+  if (parts.length !== 4) return false;
+  return parts.every(
+    (part) => /^\d{1,3}$/.test(part) && Number(part) <= 255 && !/^0\d/.test(part)
+  );
+}
+
+/** 4 bytes for IPv4, 16 for IPv6 (including `::ffff:a.b.c.d`), else null. */
+function addressBytes(value: string): number[] | null {
+  const raw = value.trim().replace(/^\[|\]$/g, "").split("%")[0]!;
+  if (isIpv4(raw)) return raw.split(".").map(Number);
+  if (!raw.includes(":")) return null;
+
+  const halves = raw.split("::");
+  if (halves.length > 2) return null;
+  const compressed = halves.length === 2;
+
+  const head = groupsToBytes(halves[0] ?? "");
+  const tail = groupsToBytes(halves[1] ?? "");
+  if (!head || !tail) return null;
+
+  const total = head.length + tail.length;
+  if (compressed) {
+    if (total > 8) return null;
+    return [...head, ...new Array(8 - total).fill(0), ...tail].flatMap(split16);
+  }
+  if (total !== 8) return null;
+  return [...head, ...tail].flatMap(split16);
+}
+
+/** One side of `::` as 16-bit groups; a trailing `1.2.3.4` counts as two. */
+function groupsToBytes(side: string): number[] | null {
+  if (side === "") return [];
+  const out: number[] = [];
+  for (const part of side.split(":")) {
+    if (/^[0-9a-fA-F]{1,4}$/.test(part)) {
+      out.push(parseInt(part, 16));
+    } else if (isIpv4(part)) {
+      const o = part.split(".").map(Number);
+      out.push((o[0]! << 8) | o[1]!, (o[2]! << 8) | o[3]!);
+    } else {
+      return null;
+    }
+  }
+  return out;
+}
+
+function split16(group: number): number[] {
+  return [(group >> 8) & 0xff, group & 0xff];
+}
+
+/** The embedded IPv4 of an IPv4-mapped IPv6 address (`::ffff:1.2.3.4`), else null. */
+function ipv4FromBytes(bytes: number[]): string | null {
+  const prefix = bytes.slice(0, 10).every((byte) => byte === 0);
+  const marker = bytes[10] === 0xff && bytes[11] === 0xff;
+  if (!prefix || !marker) return null;
+  return `${bytes[12]}.${bytes[13]}.${bytes[14]}.${bytes[15]}`;
+}

src/server/weekly-metrics.test.ts

@@ -1,12 +1,16 @@
 import { beforeEach, describe, expect, it } from "bun:test";
 import { Database } from "bun:sqlite";
 import {
+  VISITOR_ANALYTICS_ADDED_COLUMNS,
   VISITOR_ANALYTICS_COLUMNS,
   VISITOR_ANALYTICS_DDL,
   VISITOR_ANALYTICS_INSERT
 } from "./analytics-schema";
 import {
+  NOOK_CHECKOUT_PATH,
   NOOK_DOWNLOAD_PATH,
+  NOOK_HOST,
+  ensureAnalyticsSchema,
   getWeeklyReport,
   pickMover,
   readCampaigns,
@@ -70,6 +74,8 @@ let conns: Connections;
 
 const NOW = new Date("2026-10-29T16:00:00Z");
 
+// A row as the post-FRE-366 writers record it: real client address, the host
+// it addressed, and a stage label where the row is a download.
 const row = (
   overrides: Record<string, unknown>
 ): Record<string, unknown> => ({
@@ -79,7 +85,9 @@ const row = (
   method: "GET",
   referrer: null,
   user_agent: "Mozilla/5.0 (Macintosh)",
-  ip_address: "203.0.113.9",
+  // What the writers store since the FRE-369 privacy floor: a /24 (or /56 for
+  // IPv6), never the address itself.
+  ip_address: "203.0.113.0",
   country: "GB",
   device_type: "desktop",
   browser: "safari",
@@ -96,6 +104,8 @@ const row = (
   ttfb: null,
   dom_load: null,
   load_complete: null,
+  stage: null,
+  host: NOOK_HOST,
   ...overrides
 });
 
@@ -142,20 +152,32 @@ describe("windowsFor", () => {
 });
 
 describe("readFunnel", () => {
-  it("counts each stage inside its window and nothing outside it", async () => {
-    // Page views count; API calls do not.
+  it("counts each stage by its own label inside its window", async () => {
+    // Page views on the Nook host count; other sites, API calls, legacy
+    // rows with no host, and the other window do not.
     insertAnalytics({ path: "/", created_at: THIS_WEEK });
     insertAnalytics({ path: "/downloads", created_at: THIS_WEEK });
+    insertAnalytics({ path: "/", host: "freno.me", created_at: THIS_WEEK });
+    insertAnalytics({ path: "/", host: null, created_at: THIS_WEEK });
     insertAnalytics({ path: "/api/trpc/anything", created_at: THIS_WEEK });
     insertAnalytics({ path: "/", created_at: LAST_WEEK });
-
-    // The merged download label, click and served alike.
-    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, created_at: THIS_WEEK });
-    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, created_at: THIS_WEEK });
-
-    // Checkout page doors, including the legacy /nook/checkout path.
+    // The old checkout proxy: a /checkout page view is a visit, not a
+    // started checkout.
     insertAnalytics({ path: "/checkout", created_at: THIS_WEEK });
-    insertAnalytics({ path: "/nook/checkout", created_at: THIS_WEEK });
+
+    // The download label splits by stage; a row with no stage predates the
+    // split and counts in neither.
+    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "click", created_at: THIS_WEEK });
+    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "served", created_at: THIS_WEEK });
+    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "served", created_at: THIS_WEEK });
+    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: null, created_at: THIS_WEEK });
+
+    // A created Stripe session, recorded by the checkout route itself.
+    insertAnalytics({
+      path: NOOK_CHECKOUT_PATH,
+      method: "POST",
+      created_at: THIS_WEEK
+    });
 
     // The licence DB stores ISO-8601 with milliseconds.
     nookDb
@@ -180,13 +202,12 @@ describe("readFunnel", () => {
     const window = windowsFor(NOW, 7).current;
     const funnel = await readFunnel(conns, window);
 
-    // "/checkout" and "/nook/checkout" are page views too, so they land in
-    // both the visit count and the checkout count.
     expect(funnel).toEqual({
-      visit: 4,
-      download: 2,
+      visit: 3, // "/", "/downloads" and the "/checkout" page view on the Nook host
+      downloadClick: 1,
+      dmgServed: 2,
       trial: 1,
-      checkout: 2,
+      checkout: 1,
       purchase: 1,
       activation: 1
     });
@@ -196,7 +217,8 @@ describe("readFunnel", () => {
     const window = windowsFor(NOW, 7).current;
     expect(await readFunnel(conns, window)).toEqual({
       visit: 0,
-      download: 0,
+      downloadClick: 0,
+      dmgServed: 0,
       trial: 0,
       checkout: 0,
       purchase: 0,
@@ -206,37 +228,57 @@ describe("readFunnel", () => {
 });
 
 describe("readTraffic", () => {
-  it("separates loopback and automation from browser traffic", async () => {
+  it("separates legacy rows, live loopback and automation from clean traffic", async () => {
+    // Post-instrumentation rows: host set. Two of them still loopback — in the
+    // truncated shapes the privacy floor writes — which is the shape the report
+    // must flag as "the fix is not live".
     insertAnalytics({
       path: NOOK_DOWNLOAD_PATH,
+      stage: "click",
+      created_at: THIS_WEEK
+    });
+    insertAnalytics({
+      path: NOOK_DOWNLOAD_PATH,
+      stage: "served",
+      ip_address: "127.0.0.0",
+      user_agent: "The Nook/0.13.4 Sparkle/2.9.6",
+      created_at: THIS_WEEK
+    });
+    insertAnalytics({
+      path: "/",
+      ip_address: "0:0:0:0:0:0:0:0",
+      created_at: THIS_WEEK
+    });
+    insertAnalytics({ path: "/", created_at: THIS_WEEK });
+
+    // Pre-instrumentation rows: no host, proxy's address — legacy, never
+    // counted as "the fix is not live".
+    insertAnalytics({
+      path: NOOK_DOWNLOAD_PATH,
+      stage: null,
+      host: null,
       ip_address: "127.0.0.1",
       user_agent: "curl/8.7.1",
       created_at: THIS_WEEK
     });
     insertAnalytics({
-      path: NOOK_DOWNLOAD_PATH,
+      path: "/",
+      host: null,
       ip_address: "::1",
-      user_agent: "The Nook/0.13.4 Sparkle/2.9.6",
       created_at: THIS_WEEK
     });
-    insertAnalytics({
-      path: NOOK_DOWNLOAD_PATH,
-      ip_address: "203.0.113.9",
-      user_agent: "Mozilla/5.0 (Macintosh) Safari/605.1.15",
-      created_at: THIS_WEEK
-    });
-    insertAnalytics({ path: "/", ip_address: "203.0.113.9", created_at: THIS_WEEK });
 
     const traffic = await readTraffic(conns, windowsFor(NOW, 7).current);
 
     expect(traffic).toEqual({
-      analyticsRows: 4,
-      loopbackRows: 2,
-      allTimeRows: 4,
-      allTimeAddressedRows: 4,
-      allTimeLoopbackRows: 2,
+      analyticsRows: 6,
+      hostRows: 4,
+      legacyRows: 2,
+      hostLoopbackRows: 2,
       downloadRows: 3,
-      downloadLoopbackRows: 2,
+      downloadClickRows: 1,
+      downloadServedRows: 1,
+      downloadUnstagedRows: 1,
       downloadNonBrowserRows: 2
     });
   });
@@ -288,10 +330,53 @@ describe("readCampaigns", () => {
   });
 });
 
+describe("ensureAnalyticsSchema", () => {
+  it("adds the post-hoc columns to a table that predates them, idempotently", async () => {
+    // The table as production held it before any post-hoc column existed.
+    // (`created_at` is not in the insert column list — it defaults DB-side —
+    // so the simulated table has to name it explicitly.)
+    const oldDb = new Database(":memory:");
+    const withoutAdded = [
+      ...VISITOR_ANALYTICS_COLUMNS.filter(
+        (c) => !VISITOR_ANALYTICS_ADDED_COLUMNS.some((a) => a.name === c)
+      ),
+      "created_at"
+    ];
+    oldDb.run(
+      `CREATE TABLE VisitorAnalytics (${withoutAdded.map((c) => `${c} TEXT`).join(", ")})`
+    );
+    const oldConn = adapter(oldDb);
+
+    await ensureAnalyticsSchema(oldConn);
+    await ensureAnalyticsSchema(oldConn); // a second run is a no-op
+
+    // The writers' full INSERT — stage and host included — now works.
+    const values = row({ stage: "click", path: NOOK_DOWNLOAD_PATH });
+    expect(() =>
+      oldDb
+        .prepare(VISITOR_ANALYTICS_INSERT)
+        .run(
+          ...(VISITOR_ANALYTICS_COLUMNS.map((c) => values[c]) as never[])
+        )
+    ).not.toThrow();
+    oldDb
+      .prepare(`UPDATE VisitorAnalytics SET created_at = ? WHERE id = ?`)
+      .run(THIS_WEEK, values.id as string);
+
+    // And the funnel reads its stage split from the migrated table.
+    const funnel = await readFunnel(
+      { analytics: oldConn, nook: conns.nook },
+      windowsFor(NOW, 7).current
+    );
+    expect(funnel.downloadClick).toBe(1);
+  });
+});
+
 describe("pickMover", () => {
   const base: FunnelCounts = {
     visit: 0,
-    download: 0,
+    downloadClick: 0,
+    dmgServed: 0,
     trial: 0,
     checkout: 0,
     purchase: 0,
@@ -300,12 +385,12 @@ describe("pickMover", () => {
 
   it("picks the stage with the largest absolute change", () => {
     const mover = pickMover(
-      { ...base, visit: 61, download: 156, trial: 2 },
+      { ...base, visit: 61, dmgServed: 156, trial: 2 },
       { ...base, visit: 56 }
     );
     expect(mover).toEqual({
-      key: "download",
-      label: "download click → DMG served (one label)",
+      key: "dmgServed",
+      label: "DMG served",
       now: 156,
       before: 0,
       delta: 156
@@ -314,8 +399,8 @@ describe("pickMover", () => {
 
   it("breaks ties toward the earlier stage", () => {
     const mover = pickMover(
-      { ...base, visit: 5, download: 5 },
-      { ...base, visit: 0, download: 0 }
+      { ...base, visit: 5, downloadClick: 5 },
+      { ...base, visit: 0, downloadClick: 0 }
     );
     expect(mover?.key).toBe("visit");
   });
@@ -326,20 +411,20 @@ describe("pickMover", () => {
 });
 
 describe("getWeeklyReport / renderWeeklyMarkdown", () => {
-  it("reports a week in the agreed stage order with the computed caveats", async () => {
+  it("renders seven distinct stages with no merged-label or proxy caveats", async () => {
     insertAnalytics({ path: "/", created_at: THIS_WEEK });
+    insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "click", created_at: THIS_WEEK });
     insertAnalytics({
       path: NOOK_DOWNLOAD_PATH,
-      ip_address: "127.0.0.1",
-      user_agent: "curl/8.7.1",
+      stage: "served",
       created_at: THIS_WEEK
     });
     insertAnalytics({
       path: NOOK_DOWNLOAD_PATH,
-      ip_address: "127.0.0.1",
-      user_agent: "curl/8.7.1",
+      stage: "served",
       created_at: THIS_WEEK
     });
+    insertAnalytics({ path: NOOK_CHECKOUT_PATH, method: "POST", created_at: THIS_WEEK });
     nookDb
       .prepare(`INSERT INTO trials (fingerprint, started_at) VALUES (?, ?)`)
       .run("fp-1", "2026-10-28T20:28:46.806Z");
@@ -347,21 +432,75 @@ describe("getWeeklyReport / renderWeeklyMarkdown", () => {
     const report = await getWeeklyReport(conns, { now: NOW, days: 7 });
     const markdown = renderWeeklyMarkdown(report);
 
-    expect(report.mover?.key).toBe("download");
     expect(markdown).toContain(
       "Window: **2026-10-22 16:00:00 → 2026-10-29 16:00:00 UTC** (7 days)"
     );
-    expect(markdown).toContain("| visit (page views) | 1 | 0 | +1 |");
-    expect(markdown.indexOf("visit (page views)")).toBeLessThan(
-      markdown.indexOf("download click → DMG served")
+    expect(markdown).toContain(`| visit (${NOOK_HOST} page views) | 1 | 0 | +1 |`);
+    expect(markdown).toContain("| download click | 1 | 0 | +1 |");
+    expect(markdown).toContain("| DMG served | 2 | 0 | +2 |");
+    expect(markdown).toContain("| checkout started (Stripe session) | 1 | 0 | +1 |");
+
+    const order = [
+      `visit (${NOOK_HOST} page views)`,
+      "download click",
+      "DMG served",
+      "trial started",
+      "checkout started (Stripe session)",
+      "purchase (licence issued)",
+      "activation"
+    ];
+    for (let i = 1; i < order.length; i += 1) {
+      expect(markdown.indexOf(order[i - 1]!)).toBeLessThan(
+        markdown.indexOf(order[i]!)
+      );
+    }
+
+    // The caveats this issue retired stay retired.
+    expect(markdown).not.toContain("one label");
+    expect(markdown).not.toContain("not recorded");
+    expect(markdown).not.toContain("no host dimension");
+    expect(markdown).not.toContain("†");
+
+    expect(report.mover?.key).toBe("dmgServed");
+    expect(markdown).toContain("**DMG served: 0 → 2 (+2).**");
+  });
+
+  it("states what pre-instrumentation rows cannot say, computed from the window", async () => {
+    insertAnalytics({ path: "/", created_at: THIS_WEEK });
+    // Legacy rows: no host, proxy's address, no stage label.
+    insertAnalytics({ path: "/", host: null, ip_address: "127.0.0.1", created_at: THIS_WEEK });
+    insertAnalytics({
+      path: NOOK_DOWNLOAD_PATH,
+      stage: null,
+      host: null,
+      ip_address: "127.0.0.1",
+      user_agent: "curl/8.7.1",
+      created_at: THIS_WEEK
+    });
+
+    const report = await getWeeklyReport(conns, { now: NOW, days: 7 });
+    const markdown = renderWeeklyMarkdown(report);
+
+    expect(markdown).toContain(
+      "2 of 3 rows in the window predate the FRE-366 instrumentation"
     );
-    expect(markdown.indexOf("download click → DMG served")).toBeLessThan(
-      markdown.indexOf("trial started")
+    expect(markdown).toContain("predate the click/served stage label");
+    // Legacy loopback is not "the fix is not live".
+    expect(markdown).not.toContain("still carry a loopback address");
+  });
+
+  it("flags loopback only when rows recorded since the instrumentation carry it", async () => {
+    // Both spellings a live writer can produce: the raw proxy address, and the
+    // /24 the privacy floor stores instead of it.
+    insertAnalytics({ path: "/", ip_address: "127.0.0.1", created_at: THIS_WEEK });
+    insertAnalytics({ path: "/", ip_address: "127.0.53.0", created_at: THIS_WEEK });
+
+    const report = await getWeeklyReport(conns, { now: NOW, days: 7 });
+    const markdown = renderWeeklyMarkdown(report);
+
+    expect(markdown).toContain(
+      "2 rows recorded since the instrumentation still carry a loopback address"
     );
-    expect(markdown).toContain("**download click → DMG served (one label): 0 → 2 (+2).**");
-    expect(markdown).toContain("2 of 3 rows in the window came from a loopback address");
-    expect(markdown).toContain("Checkout start is not recorded");
-    expect(markdown).toContain("There is no host dimension");
   });
 
   it("says nothing moved rather than inventing a winner", async () => {

src/server/weekly-metrics.ts

@@ -18,20 +18,33 @@
  *    whose `.env` carries nothing else. The SQL here is the same shape as the
  *    analytics summaries, so the weekly read and the dashboard cannot drift
  *    into different label sets.
- *  - Every stage carries what it actually measured. Two of the seven agreed
- *    stages share one label in production today and a third is only a page-view
- *    proxy; the report states that next to the number instead of inventing a
- *    figure (see `renderWeeklyMarkdown`).
+ *  - Every stage is measured by its own label. FRE-366 gave the funnel that:
+ *    the download door and the DMG file route record distinct `stage` values
+ *    (`click` / `served`), a created Stripe checkout session records its own
+ *    row, and every row carries the real client address and the host it
+ *    addressed, so the visit stage scopes to the Nook. Rows that predate
+ *    that instrumentation are stated next to the numbers, never silently
+ *    counted (see `renderWeeklyMarkdown`).
  *
  * No `~/env`, no `~/server/database`: the caller owns the connections, the
  * tests own an in-memory SQLite, and the script owns the libsql clients.
  */
 
 import { DOWNLOAD_PATH_PREFIX } from "~/lib/download-asset";
+import {
+  VISITOR_ANALYTICS_ADDED_COLUMNS,
+  VISITOR_ANALYTICS_DDL
+} from "~/server/analytics-schema";
 
 /** The one asset key the Nook lands under — `downloadPath("thenook")`. */
 export const NOOK_DOWNLOAD_PATH = `${DOWNLOAD_PATH_PREFIX}thenook`;
 
+/** The host the visit stage scopes to. Recorded per row since FRE-366. */
+export const NOOK_HOST = "nook.freno.me";
+
+/** The path a created Stripe checkout session records (FRE-366). */
+export const NOOK_CHECKOUT_PATH = "/api/the-nook/checkout";
+
 /** An `execute`-shaped connection: the libsql client and the test adapter both fit. */
 export interface MetricsConn {
   execute(query: {
@@ -49,7 +62,8 @@ export interface Connections {
 
 export type StageKey =
   | "visit"
-  | "download"
+  | "downloadClick"
+  | "dmgServed"
   | "trial"
   | "checkout"
   | "purchase"
@@ -58,35 +72,28 @@ export type StageKey =
 export interface StageDef {
   key: StageKey;
   label: string;
-  /** True when the number is a proxy or a merged label, not a clean stage. */
-  approximate: boolean;
 }
 
 /**
- * The seven agreed stages. `download` deliberately carries two of them: the
- * door and the file route both write `/api/downloads/thenook`, so a click and
- * the DMG it fetched are one label in the data (proved in the report's caveats).
+ * The seven agreed stages, each with its own label in the data:
+ * `downloadClick` and `dmgServed` split what used to be one merged download
+ * label (the `stage` column, FRE-366), and `checkout` reads created Stripe
+ * sessions instead of the /checkout page-view proxy.
  */
 export const STAGES: StageDef[] = [
-  { key: "visit", label: "visit (page views)", approximate: false },
-  {
-    key: "download",
-    label: "download click → DMG served (one label)",
-    approximate: true
-  },
-  { key: "trial", label: "trial started", approximate: false },
-  {
-    key: "checkout",
-    label: "checkout page visit",
-    approximate: true
-  },
-  { key: "purchase", label: "purchase (licence issued)", approximate: false },
-  { key: "activation", label: "activation", approximate: false }
+  { key: "visit", label: `visit (${NOOK_HOST} page views)` },
+  { key: "downloadClick", label: "download click" },
+  { key: "dmgServed", label: "DMG served" },
+  { key: "trial", label: "trial started" },
+  { key: "checkout", label: "checkout started (Stripe session)" },
+  { key: "purchase", label: "purchase (licence issued)" },
+  { key: "activation", label: "activation" }
 ];
 
 export interface FunnelCounts {
   visit: number;
-  download: number;
+  downloadClick: number;
+  dmgServed: number;
   trial: number;
   checkout: number;
   purchase: number;
@@ -103,13 +110,17 @@ export interface CampaignRow {
 /** Traffic-shape facts the report has to state next to the funnel. */
 export interface WindowTraffic {
   analyticsRows: number;
-  loopbackRows: number;
-  /** All-time row counts, so "every row ever" is a checked claim, not a guess. */
-  allTimeRows: number;
-  allTimeAddressedRows: number;
-  allTimeLoopbackRows: number;
+  /** Rows carrying a host — written since the FRE-366 instrumentation. */
+  hostRows: number;
+  /** Rows with no host — pre-FRE-366; their address is the proxy's loopback. */
+  legacyRows: number;
+  /** Post-instrumentation rows still recording loopback: the fix is not live. */
+  hostLoopbackRows: number;
   downloadRows: number;
-  downloadLoopbackRows: number;
+  downloadClickRows: number;
+  downloadServedRows: number;
+  /** Download rows with no stage label — they predate FRE-366 and cannot be split. */
+  downloadUnstagedRows: number;
   downloadNonBrowserRows: number;
 }
 
@@ -133,8 +144,16 @@ export interface WeeklyReport {
   generatedAt: string;
 }
 
-/** Loopback addresses — every row the live table has ever held. */
-const LOOPBACK = ["127.0.0.1", "::1", "::ffff:127.0.0.1"];
+/**
+ * A recorded address that is not a visitor, in every spelling the column can
+ * hold: the proxy the request came through (`127.0.0.1`, `::1`, an IPv4-mapped
+ * form) *and* the prefixes the FRE-369 privacy floor writes instead of them
+ * (`127.0.0.0`, all-zero IPv6). Whole `127/8` because truncation keeps the first
+ * three octets, so `127.0.53.1` and `127.0.53.0` are the same non-visitor.
+ */
+const LOOPBACK_SQL = `(ip_address LIKE '127.%'
+     OR ip_address LIKE '::ffff:127.%'
+     OR ip_address IN ('::1', '0:0:0:0:0:0:0:1', '0:0:0:0:0:0:0:0'))`;
 
 /** SQLite's `datetime()` string, UTC: `YYYY-MM-DD HH:MM:SS`. */
 export function sqlTime(date: Date): string {
@@ -183,6 +202,28 @@ async function scalar(
   return toNumber(Object.values(row)[0]);
 }
 
+/**
+ * Bring the analytics table up to the schema the readers expect, through the
+ * same idempotent bootstrap path the app server uses (`analytics.ts`): the
+ * DDL is a no-op against the live table and each post-hoc column is ALTERed
+ * in only when absent. The weekly job runs against databases the app may not
+ * have migrated yet — a fresh checkout, a restored backup, or production
+ * before the next deploy — and must render its seven stages regardless.
+ */
+export async function ensureAnalyticsSchema(conn: MetricsConn): Promise<void> {
+  await conn.execute({ sql: VISITOR_ANALYTICS_DDL });
+
+  const info = await conn.execute({
+    sql: `PRAGMA table_info(VisitorAnalytics)`
+  });
+  const present = new Set(info.rows.map((row) => String((row as any).name)));
+  for (const column of VISITOR_ANALYTICS_ADDED_COLUMNS) {
+    if (!present.has(column.name)) {
+      await conn.execute({ sql: column.ddl });
+    }
+  }
+}
+
 async function countInWindow(
   conn: MetricsConn,
   sql: string,
@@ -200,30 +241,43 @@ export async function readFunnel(
   const { analytics, nook } = conns;
   const { start, end } = window;
 
-  const visit = await countInWindow(
+  // Scoped to the Nook host (FRE-366): before the host column existed, a
+  // page view of `/` on nook.freno.me was the same row as freno.me's root.
+  // Pre-instrumentation rows carry no host and are excluded — the traffic
+  // section states how many.
+  const visit = await scalar(
     analytics,
     `SELECT COUNT(*) AS n FROM VisitorAnalytics
      WHERE created_at >= ? AND created_at < ?
-     AND path NOT LIKE '/api/%'`,
-    start,
-    end
+     AND path NOT LIKE '/api/%'
+     AND host = ?`,
+    [start, end, NOOK_HOST]
   );
 
-  const download = await scalar(
+  const downloadClick = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ?
+     AND path = ? AND stage = 'click'`,
+    [start, end, NOOK_DOWNLOAD_PATH]
+  );
+
+  const dmgServed = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ?
+     AND path = ? AND stage = 'served'`,
+    [start, end, NOOK_DOWNLOAD_PATH]
+  );
+
+  // Created Stripe checkout sessions, recorded by the checkout route itself
+  // (FRE-366) — not the /checkout page view that used to proxy for them.
+  const checkout = await scalar(
     analytics,
     `SELECT COUNT(*) AS n FROM VisitorAnalytics
      WHERE created_at >= ? AND created_at < ?
      AND path = ?`,
-    [start, end, NOOK_DOWNLOAD_PATH]
-  );
-
-  const checkout = await countInWindow(
-    analytics,
-    `SELECT COUNT(*) AS n FROM VisitorAnalytics
-     WHERE created_at >= ? AND created_at < ?
-     AND (path LIKE '/checkout%' OR path LIKE '/nook/checkout%')`,
-    start,
-    end
+    [start, end, NOOK_CHECKOUT_PATH]
   );
 
   // The licence DB stores ISO-8601 (`2026-10-28T20:28:46.806Z`); the analytics
@@ -252,7 +306,8 @@ export async function readFunnel(
 
   return {
     visit,
-    download,
+    downloadClick,
+    dmgServed,
     trial,
     checkout,
     purchase,
@@ -261,9 +316,11 @@ export async function readFunnel(
 }
 
 /**
- * Traffic shape for one window: how much of it came from this host, and how
- * much of the download label was automation. Without these two facts the
- * numbers read as demand, which they are not.
+ * Traffic shape for one window: how much of it predates the FRE-366
+ * instrumentation (no host, proxy's address), whether rows recorded since
+ * still see loopback (which would mean the forwarded-header fix is not
+ * live), and how the download label splits. Without these facts the numbers
+ * read as demand, which they are not.
  */
 export async function readTraffic(
   conns: Connections,
@@ -271,26 +328,6 @@ export async function readTraffic(
 ): Promise<WindowTraffic> {
   const { analytics } = conns;
   const { start, end } = window;
-  const placeholders = LOOPBACK.map(() => "?").join(", ");
-
-  const allTimeRows = await scalar(
-    analytics,
-    `SELECT COUNT(*) AS n FROM VisitorAnalytics`,
-    []
-  );
-
-  const allTimeAddressedRows = await scalar(
-    analytics,
-    `SELECT COUNT(*) AS n FROM VisitorAnalytics WHERE ip_address IS NOT NULL`,
-    []
-  );
-
-  const allTimeLoopbackRows = await scalar(
-    analytics,
-    `SELECT COUNT(*) AS n FROM VisitorAnalytics
-     WHERE ip_address IN (${placeholders})`,
-    [...LOOPBACK]
-  );
 
   const analyticsRows = await countInWindow(
     analytics,
@@ -300,12 +337,29 @@ export async function readTraffic(
     end
   );
 
-  const loopbackRows = await scalar(
+  const hostRows = await scalar(
     analytics,
     `SELECT COUNT(*) AS n FROM VisitorAnalytics
      WHERE created_at >= ? AND created_at < ?
-     AND ip_address IN (${placeholders})`,
-    [start, end, ...LOOPBACK]
+     AND host IS NOT NULL`,
+    [start, end]
+  );
+
+  const legacyRows = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ?
+     AND host IS NULL`,
+    [start, end]
+  );
+
+  const hostLoopbackRows = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ?
+     AND host IS NOT NULL
+     AND ${LOOPBACK_SQL}`,
+    [start, end]
   );
 
   const downloadRows = await scalar(
@@ -315,12 +369,25 @@ export async function readTraffic(
     [start, end, NOOK_DOWNLOAD_PATH]
   );
 
-  const downloadLoopbackRows = await scalar(
+  const downloadClickRows = await scalar(
     analytics,
     `SELECT COUNT(*) AS n FROM VisitorAnalytics
-     WHERE created_at >= ? AND created_at < ? AND path = ?
-     AND ip_address IN (${placeholders})`,
-    [start, end, NOOK_DOWNLOAD_PATH, ...LOOPBACK]
+     WHERE created_at >= ? AND created_at < ? AND path = ? AND stage = 'click'`,
+    [start, end, NOOK_DOWNLOAD_PATH]
+  );
+
+  const downloadServedRows = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ? AND path = ? AND stage = 'served'`,
+    [start, end, NOOK_DOWNLOAD_PATH]
+  );
+
+  const downloadUnstagedRows = await scalar(
+    analytics,
+    `SELECT COUNT(*) AS n FROM VisitorAnalytics
+     WHERE created_at >= ? AND created_at < ? AND path = ? AND stage IS NULL`,
+    [start, end, NOOK_DOWNLOAD_PATH]
   );
 
   // Anything whose UA is not a browser engine: curl, Sparkle's updater,
@@ -335,12 +402,13 @@ export async function readTraffic(
 
   return {
     analyticsRows,
-    loopbackRows,
-    allTimeRows,
-    allTimeAddressedRows,
-    allTimeLoopbackRows,
+    hostRows,
+    legacyRows,
+    hostLoopbackRows,
     downloadRows,
-    downloadLoopbackRows,
+    downloadClickRows,
+    downloadServedRows,
+    downloadUnstagedRows,
     downloadNonBrowserRows
   };
 }
@@ -483,8 +551,7 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string {
   for (const stage of STAGES) {
     const now = counts[stage.key];
     const before = previousCounts[stage.key];
-    const label = stage.approximate ? `${stage.label} †` : stage.label;
-    lines.push(`| ${label} | ${now} | ${before} | ${signed(now - before)} |`);
+    lines.push(`| ${stage.label} | ${now} | ${before} | ${signed(now - before)} |`);
   }
   lines.push("");
 
@@ -528,58 +595,47 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string {
   lines.push("## Before acting on these numbers");
   lines.push("");
 
-  if (traffic.analyticsRows > 0 && traffic.loopbackRows > 0) {
+  if (traffic.legacyRows > 0) {
     lines.push(
-      `- **${traffic.loopbackRows} of ${traffic.analyticsRows} rows in the window came from a loopback address** ` +
-        `(127.0.0.1 / ::1)` +
-        (traffic.allTimeAddressedRows > 0 &&
-        traffic.allTimeAddressedRows === traffic.allTimeLoopbackRows
-          ? `; every one of the ${traffic.allTimeAddressedRows} addressed rows the table ` +
-            `has ever held is loopback (${traffic.allTimeRows - traffic.allTimeAddressedRows} carry no address)`
-          : `; across the table's history, ${traffic.allTimeLoopbackRows} of ` +
-            `${traffic.allTimeAddressedRows} addressed rows are loopback`) +
-        `. Either this host is the only audience, or the app sits behind a local proxy ` +
-        `and \`getRequestIP\` records the proxy, not the visitor. Either way the address ` +
-        `identifies nobody — "unique visitors" collapses to one host and per-channel ` +
-        `reach cannot be read from it.`
+      `- **${traffic.legacyRows} of ${traffic.analyticsRows} rows in the window predate the FRE-366 instrumentation.** ` +
+        `They carry no host, and their recorded address is the production proxy's loopback ` +
+        `(127.0.0.1 / ::1) — the writers then recorded the direct peer, not the visitor. ` +
+        `They are excluded from every host-scoped stage above (visits, and the download ` +
+        `click/served split), and "unique visitors" over them still collapses to one host. ` +
+        `Rows recorded since carry the site they addressed and the visitor's address ` +
+        `truncated to a prefix (/24, or /56 for IPv6 — the FRE-369 privacy floor), so ` +
+        `"unique visitors" counts networks, not machines.`
+    );
+  }
+
+  if (traffic.hostLoopbackRows > 0) {
+    lines.push(
+      `- **${traffic.hostLoopbackRows} rows recorded since the instrumentation still carry a ` +
+        `loopback address** — the forwarded-header fix is not taking effect in production ` +
+        `(no \`cf-connecting-ip\`, and no usable \`x-forwarded-for\` hop). Do not trust ` +
+        `per-channel reach until this is zero.`
     );
   }
 
   if (traffic.downloadRows > 0) {
     lines.push(
-      `- **The download stage is one label, not two.** The landing-door route and the file ` +
-        `route that serves the DMG both write \`${NOOK_DOWNLOAD_PATH}\`, so ` +
-        `"download click → DMG served" cannot be split: one served click writes two rows, ` +
-        `a click that fails to resolve writes one, and a Sparkle update writes one. ` +
-        `Of this window's ${traffic.downloadRows} download rows, ` +
-        `${traffic.downloadLoopbackRows} are loopback and ` +
-        `${traffic.downloadNonBrowserRows} are not browser user agents (curl, Sparkle, ` +
-        `link-preview crawlers).`
+      `- **Download rows are automation-heavy.** Of this window's ${traffic.downloadRows} ` +
+        `download rows, ${traffic.downloadClickRows} are clicks and ` +
+        `${traffic.downloadServedRows} are served files` +
+        (traffic.downloadUnstagedRows > 0
+          ? `; the remaining ${traffic.downloadUnstagedRows} predate the click/served stage ` +
+            `label and cannot be split, so they count in neither stage`
+          : "") +
+        `. ${traffic.downloadNonBrowserRows} of all ${traffic.downloadRows} are not browser ` +
+        `user agents (curl, Sparkle, link-preview crawlers).`
     );
   }
 
-  lines.push(
-    `- **Checkout start is not recorded.** \`POST /api/the-nook/checkout\` — the call that ` +
-      `actually creates the Stripe session — writes nothing to analytics; the only trace is ` +
-      `the /checkout page view reported above, which is a page, not a started checkout.`
-  );
-
-  lines.push(
-    `- **There is no host dimension.** \`VisitorAnalytics\` records a path, not the site, so ` +
-      `the Nook's landing page (\`/\` on nook.freno.me) is the same row as freno.me's root. ` +
-      `"visit" above is site-wide page views, not Nook visits.`
-  );
-
   lines.push(
     `- **No conversion rate before the cohort matures.** Trial→paid belongs to the mature ` +
       `Day 28–35 read (FRE-197); a ratio from a partial cohort is not a metric.`
   );
   lines.push("");
-
-  lines.push(
-    "† Proxy or merged label — see *Before acting on these numbers* above."
-  );
-  lines.push("");
   lines.push("## Sources");
   lines.push("");
   lines.push("- `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns.");

vercel.json

@@ -57,6 +57,7 @@
     }
   ],
   "crons": [
+    { "path": "/api/analytics-retention", "schedule": "0 12 * * *" },
     { "path": "/api/the-nook/lifecycle", "schedule": "0 13 * * *" }
   ]
 }