analytics: fix the two FRE-369 findings in the funnel instrumentation
The security review of 39d874a (FRE-369) found the address fix recorded
the wrong thing twice. Both are fixed here; the branch is the same one
under review, this commit does not rewrite it.
Q1 — the first x-forwarded-for hop is attacker-chosen text. In
client → Cloudflare → same-host proxy → app, both proxies *append*, so
the header arrives as `<client spoof…>, <true client>, <CF edge>` and
`split(",")[0]` took the sender's own invention — the loopback-peer gate
was sound, the hop selection defeated it. Preference is now
`cf-connecting-ip` (Cloudflare overwrites it, so a client value never
survives the edge), then x-forwarded-for counted from the right with one
trusted hop discounted, then the direct peer. Every candidate must parse
as an address. h3's `getRequestIP(…, { xForwardedFor: true })` has the
same leftmost flaw and is documented as not a fix.
Q4 — the table held full client addresses indefinitely. Every analytics
entry now passes through `truncateIpAddress` in `enrichAnalyticsEntry`
(IPv4 /24, IPv6 /56, unparseable dropped), so no writer can persist a
full address by accident. Retention got the schedule it never had:
`/api/analytics/retention` on a daily Vercel cron, Bearer-authenticated
and fail-closed, with the window parameterised (the old
`cleanupOldAnalytics` spliced the number into the SQL). Default 365 days
rather than the review's 90 so enabling the cron is not itself a
destructive event — the tighter window is an operational decision, named
on the task.
Also from the review: the module doc now forbids feeding this helper into
any security decision; `isLoopbackAddress` parses instead of
string-matching (`::ffff:127.9.9.9`, `0:0:0:0:0:0:0:1`); and `host` is
normalised (lowercased, port stripped) so `nook.freno.me:443` cannot
silently miss the `host = ?` scope. The weekly loopback caveat matches
the truncated shapes writers produce now, and says plainly that "unique
visitors" counts prefixes.
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Weekly funnel snapshot
|
# Weekly funnel snapshot
|
||||||
|
|
||||||
Window: **2026-10-22 17:01:33 → 2026-10-29 17:01:33 UTC** (7 days). Previous: 2026-10-15 17:01:33 → 2026-10-22 17:01:33 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.
|
Generated by `bun --env-file=.env scripts/weekly-metrics.ts`; regenerated weekly, never hand-edited. The analysis is the CMO's.
|
||||||
|
|
||||||
@@ -32,7 +32,7 @@ Top 10 by volume, untagged excluded — the same grouping `getAnalyticsSummary.t
|
|||||||
|
|
||||||
## Before acting on these numbers
|
## Before acting on these numbers
|
||||||
|
|
||||||
- **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 real client address and the site they addressed.
|
- **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).
|
- **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.
|
- **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.
|
||||||
|
|
||||||
@@ -40,4 +40,4 @@ Top 10 by volume, untagged excluded — the same grouping `getAnalyticsSummary.t
|
|||||||
|
|
||||||
- `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns.
|
- `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns.
|
||||||
- `trials`, `licenses`, `activations` (the-nook Turso DB) — the paid funnel.
|
- `trials`, `licenses`, `activations` (the-nook Turso DB) — the paid funnel.
|
||||||
- Generated 2026-10-29T17:01:33Z.
|
- Generated 2026-10-29T18:05:00Z.
|
||||||
|
|||||||
6
src/env/server.ts
vendored
6
src/env/server.ts
vendored
@@ -69,6 +69,12 @@ const serverEnvSchema = z.object({
|
|||||||
// Bearer secret for the daily lifecycle-email cron. Falls back to
|
// Bearer secret for the daily lifecycle-email cron. Falls back to
|
||||||
// Vercel's CRON_SECRET; unset + no CRON_SECRET = the endpoint denies all.
|
// Vercel's CRON_SECRET; unset + no CRON_SECRET = the endpoint denies all.
|
||||||
NOOK_LIFECYCLE_SECRET: z.string().min(1).optional(),
|
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()
|
CRON_SECRET: z.string().min(1).optional()
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
35
src/routes/api/analytics-retention.ts
Normal file
35
src/routes/api/analytics-retention.ts
Normal file
@@ -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" }
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -1,4 +1,5 @@
|
|||||||
import { describe, expect, it, mock, beforeEach } from "bun:test";
|
import { describe, expect, it, mock, beforeEach } from "bun:test";
|
||||||
|
import { truncateIpAddress } from "~/server/request-client";
|
||||||
|
|
||||||
// The route's contracts:
|
// The route's contracts:
|
||||||
// 1. Every click is recorded through the shared analytics buffer with the
|
// 1. Every click is recorded through the shared analytics buffer with the
|
||||||
@@ -55,9 +56,13 @@ mock.module("~/server/analytics", () => ({
|
|||||||
flushAnalytics: async () => {
|
flushAnalytics: async () => {
|
||||||
flushCalls += 1;
|
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) => ({
|
enrichAnalyticsEntry: (entry: Entry) => ({
|
||||||
...entry,
|
...entry,
|
||||||
|
ipAddress: truncateIpAddress(entry.ipAddress as string | null),
|
||||||
deviceType: String(entry.userAgent ?? "").includes("Macintosh")
|
deviceType: String(entry.userAgent ?? "").includes("Macintosh")
|
||||||
? "desktop"
|
? "desktop"
|
||||||
: null
|
: null
|
||||||
@@ -141,9 +146,16 @@ describe("GET /api/the-nook/download", () => {
|
|||||||
expect(logged[2]!.country).toBe("JP");
|
expect(logged[2]!.country).toBe("JP");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("falls back to the forwarded address when no request IP is available", async () => {
|
it("records the forwarded visitor below the trusted edge, truncated", async () => {
|
||||||
await GET(request({ "x-forwarded-for": "198.51.100.9, 10.0.0.1" }));
|
// Client → Cloudflare → origin proxy: the sender's spoof is first, the real
|
||||||
expect(logged[0]!.ipAddress).toBe("198.51.100.9");
|
// 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 () => {
|
it("still logs the click when the appcast cannot be read", async () => {
|
||||||
|
|||||||
200
src/server/analytics-retention.test.ts
Normal file
200
src/server/analytics-retention.test.ts
Normal file
@@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
129
src/server/analytics-retention.ts
Normal file
129
src/server/analytics-retention.ts
Normal file
@@ -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 } };
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -9,6 +9,8 @@ import {
|
|||||||
VISITOR_ANALYTICS_INDEXES,
|
VISITOR_ANALYTICS_INDEXES,
|
||||||
VISITOR_ANALYTICS_INSERT
|
VISITOR_ANALYTICS_INSERT
|
||||||
} from "./analytics-schema";
|
} 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
|
* Idempotent schema bootstrap, mirroring `nook.ts`. The table had no DDL in
|
||||||
@@ -317,6 +319,10 @@ export async function getAnalyticsSummary(days: number = 30): Promise<{
|
|||||||
});
|
});
|
||||||
const totalApiCalls = (totalApiCallsResult.rows[0]?.count as number) || 0;
|
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({
|
const uniqueVisitorsResult = await conn.execute({
|
||||||
sql: `SELECT COUNT(DISTINCT ip_address) as count FROM VisitorAnalytics
|
sql: `SELECT COUNT(DISTINCT ip_address) as count FROM VisitorAnalytics
|
||||||
WHERE ip_address IS NOT NULL
|
WHERE ip_address IS NOT NULL
|
||||||
@@ -628,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(
|
export async function cleanupOldAnalytics(
|
||||||
olderThanDays: number
|
olderThanDays: number
|
||||||
): Promise<number> {
|
): Promise<number> {
|
||||||
const conn = ConnectionFactory();
|
return pruneVisitorAnalytics(ConnectionFactory(), olderThanDays);
|
||||||
|
|
||||||
const result = await conn.execute({
|
|
||||||
sql: `DELETE FROM VisitorAnalytics
|
|
||||||
WHERE created_at < datetime('now', '-${olderThanDays} days')
|
|
||||||
RETURNING id`,
|
|
||||||
args: []
|
|
||||||
});
|
|
||||||
|
|
||||||
return result.rows.length;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function parseUserAgent(userAgent?: string): {
|
function parseUserAgent(userAgent?: string): {
|
||||||
@@ -675,6 +677,16 @@ function parseUserAgent(userAgent?: string): {
|
|||||||
return { deviceType, browser, os };
|
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 {
|
export function enrichAnalyticsEntry(entry: AnalyticsEntry): AnalyticsEntry {
|
||||||
const { deviceType, browser, os } = parseUserAgent(
|
const { deviceType, browser, os } = parseUserAgent(
|
||||||
entry.userAgent || undefined
|
entry.userAgent || undefined
|
||||||
@@ -682,6 +694,7 @@ export function enrichAnalyticsEntry(entry: AnalyticsEntry): AnalyticsEntry {
|
|||||||
|
|
||||||
return {
|
return {
|
||||||
...entry,
|
...entry,
|
||||||
|
ipAddress: truncateIpAddress(entry.ipAddress),
|
||||||
deviceType: entry.deviceType || deviceType,
|
deviceType: entry.deviceType || deviceType,
|
||||||
browser: entry.browser || browser,
|
browser: entry.browser || browser,
|
||||||
os: entry.os || os
|
os: entry.os || os
|
||||||
|
|||||||
@@ -1,19 +1,30 @@
|
|||||||
import { beforeEach, describe, expect, it, mock } from "bun:test";
|
import { beforeEach, describe, expect, it, mock } from "bun:test";
|
||||||
|
|
||||||
// The trust boundary of FRE-366: x-forwarded-for is client-controlled text,
|
// The trust boundary of FRE-366, as corrected by the FRE-369 security review.
|
||||||
// believed only when the direct peer is the loopback production proxy. These
|
// Two things must stay true here:
|
||||||
// tests pin that boundary — a regression here either re-records the proxy as
|
// 1. The address we record is the visitor, not whoever typed a header first.
|
||||||
// the visitor (unique visitors collapses to one host) or lets any client
|
// Cloudflare and the origin proxy both *append* to x-forwarded-for, so the
|
||||||
// spoof its address.
|
// 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;
|
let directIp: string | null = null;
|
||||||
mock.module("vinxi/http", () => ({
|
mock.module("vinxi/http", () => ({
|
||||||
getRequestIP: () => directIp
|
getRequestIP: () => directIp
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const { resolveClientIp, isLoopbackAddress, requestHost } = await import(
|
const {
|
||||||
"./request-client"
|
resolveClientIp,
|
||||||
);
|
isLoopbackAddress,
|
||||||
|
isIpAddress,
|
||||||
|
truncateIpAddress,
|
||||||
|
normaliseHost,
|
||||||
|
requestHost
|
||||||
|
} = await import("./request-client");
|
||||||
|
|
||||||
function event(
|
function event(
|
||||||
headers: Record<string, string> = {},
|
headers: Record<string, string> = {},
|
||||||
@@ -30,26 +41,68 @@ beforeEach(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("resolveClientIp", () => {
|
describe("resolveClientIp", () => {
|
||||||
it("takes the first forwarded hop when the direct peer is the loopback proxy", () => {
|
it("prefers Cloudflare's connecting address over any forwarded header", () => {
|
||||||
directIp = "127.0.0.1";
|
directIp = "127.0.0.1";
|
||||||
expect(
|
expect(
|
||||||
resolveClientIp(event({ "x-forwarded-for": "198.51.100.9, 10.0.0.1" }))
|
resolveClientIp(
|
||||||
).toBe("198.51.100.9");
|
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";
|
directIp = "::1";
|
||||||
expect(
|
expect(
|
||||||
resolveClientIp(event({ "x-forwarded-for": "198.51.100.10" }))
|
resolveClientIp(
|
||||||
).toBe("198.51.100.10");
|
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", () => {
|
it("ignores a forwarded header from a real peer — it is spoofable", () => {
|
||||||
directIp = "203.0.113.7";
|
directIp = "203.0.113.7";
|
||||||
expect(
|
expect(
|
||||||
resolveClientIp(event({ "x-forwarded-for": "198.51.100.9" }))
|
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");
|
).toBe("203.0.113.7");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("keeps the loopback peer when the proxy set no forwarded header", () => {
|
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";
|
directIp = "::ffff:127.0.0.1";
|
||||||
expect(resolveClientIp(event())).toBe("::ffff:127.0.0.1");
|
expect(resolveClientIp(event())).toBe("::ffff:127.0.0.1");
|
||||||
});
|
});
|
||||||
@@ -60,13 +113,79 @@ describe("resolveClientIp", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("isLoopbackAddress", () => {
|
describe("isLoopbackAddress", () => {
|
||||||
it("covers the v4 range, the shorthand and the v6 forms", () => {
|
it("covers the v4 range and every spelling of v6 loopback", () => {
|
||||||
expect(isLoopbackAddress("127.0.0.1")).toBe(true);
|
expect(isLoopbackAddress("127.0.0.1")).toBe(true);
|
||||||
expect(isLoopbackAddress("127.0.0.53")).toBe(true);
|
expect(isLoopbackAddress("127.0.0.53")).toBe(true);
|
||||||
expect(isLoopbackAddress("::1")).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.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("203.0.113.7")).toBe(false);
|
||||||
expect(isLoopbackAddress("10.0.0.1")).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();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -75,4 +194,10 @@ describe("requestHost", () => {
|
|||||||
expect(requestHost(event())).toBe("nook.freno.me");
|
expect(requestHost(event())).toBe("nook.freno.me");
|
||||||
expect(requestHost(event({}, "https://freno.me/"))).toBe("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"
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,65 +3,280 @@ import { getRequestIP } from "vinxi/http";
|
|||||||
import { hostFromEventLike } from "./site-context-server";
|
import { hostFromEventLike } from "./site-context-server";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The real client address behind the production proxy (FRE-366).
|
* The client address and the site a request addressed, for the analytics
|
||||||
|
* writers (FRE-366).
|
||||||
*
|
*
|
||||||
* Until this existed every analytics writer called `getRequestIP` and only
|
* Until this existed every writer called `getRequestIP` and only looked at
|
||||||
* looked at `x-forwarded-for` when it returned null — so behind the
|
* `x-forwarded-for` when it returned null — so behind the production proxy the
|
||||||
* production proxy the recorded address was the proxy itself, and all
|
* recorded address was the proxy itself, and all 12,035 addressed rows in
|
||||||
* 12,035 addressed rows in VisitorAnalytics carried 127.0.0.1 / ::1.
|
* VisitorAnalytics carried 127.0.0.1 / ::1.
|
||||||
*
|
*
|
||||||
* Trust boundary, stated plainly: `x-forwarded-for` is client-controlled
|
* STANDING RULE (FRE-369, the security review of that change): nothing here is
|
||||||
* text. It is believed only when the direct peer is a loopback address —
|
* a security signal. The address is a *best guess at a visitor*, not an
|
||||||
* the production shape, where a same-host proxy (nginx/caddy) terminates
|
* authenticated identity — do not import it into auth, rate limiting, abuse
|
||||||
* TLS and sets the header itself. Anywhere else the direct peer IS the
|
* control, fraud checks, or anything that grants or denies access. A caller
|
||||||
* client (dev, direct hit), and a forwarded header from it is ignored as
|
* that needs a security decision must re-derive trust for its own stakes; the
|
||||||
* spoofable. The first hop of the header is taken; later hops are the
|
* blast radius of a wrong answer here is a poisoned analytics row, and that is
|
||||||
* client's own claims.
|
* 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 {
|
export function resolveClientIp(event: APIEvent): string | null {
|
||||||
const direct = getRequestIP(event.nativeEvent);
|
const direct = getRequestIP(event.nativeEvent);
|
||||||
|
|
||||||
if (direct && !isLoopbackAddress(direct)) {
|
// A real peer is its own client: believed, and no forwarded header from it
|
||||||
return direct;
|
// is (it would be the peer lying about who it is).
|
||||||
}
|
if (direct && !isLoopbackAddress(direct)) return direct;
|
||||||
|
|
||||||
const forwarded = event.request?.headers?.get?.("x-forwarded-for");
|
// 1. Cloudflare's own record of the connecting address.
|
||||||
const first = forwarded?.split(",")[0]?.trim();
|
const connecting = cleanHop(header(event, CF_CONNECTING_IP));
|
||||||
if (first) {
|
if (isIpAddress(connecting)) return connecting;
|
||||||
return first;
|
|
||||||
|
// 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;
|
return direct ?? null;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function isLoopbackAddress(ip: string): boolean {
|
function cleanHop(value: string | null | undefined): string {
|
||||||
return (
|
return (value ?? "").trim();
|
||||||
ip === "::1" ||
|
|
||||||
ip === "::ffff:127.0.0.1" ||
|
|
||||||
ip === "127.0.0.1" ||
|
|
||||||
ip.startsWith("127.")
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The site the request addressed — the `Host` the client asked for, e.g.
|
* Whether an address is the loopback the production proxy presents.
|
||||||
* `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
|
* Parses rather than string-matches, so the forms a real peer can present in —
|
||||||
* `/` on freno.me. Thin alias over the site-context extractor so the
|
* `::ffff:127.0.0.53`, `0:0:0:0:0:0:0:1`, an IPv4-mapped literal — are all
|
||||||
* analytics writers share one host-parsing implementation with routing.
|
* 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 {
|
export function requestHost(event: APIEvent): string | null {
|
||||||
const header = hostFromEventLike(event);
|
return normaliseHost(hostFromEventLike(event)) ?? normaliseHost(urlHost(event));
|
||||||
if (header) return header;
|
}
|
||||||
|
|
||||||
|
function urlHost(event: APIEvent): string | null {
|
||||||
// `host` is a forbidden header and some server shapes drop it from
|
// `host` is a forbidden header and some server shapes drop it from
|
||||||
// `event.request.headers`; the request URL is absolute on the server and
|
// `event.request.headers`; the request URL is absolute on the server and
|
||||||
// carries the same host the client addressed.
|
// carries the same host the client addressed.
|
||||||
try {
|
try {
|
||||||
const url = event.request?.url;
|
const url = event.request?.url;
|
||||||
if (url) return new URL(url).host || null;
|
return url ? new URL(url).host || null : null;
|
||||||
} catch {
|
} catch {
|
||||||
/* noop */
|
|
||||||
}
|
|
||||||
return null;
|
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;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -85,7 +85,9 @@ const row = (
|
|||||||
method: "GET",
|
method: "GET",
|
||||||
referrer: null,
|
referrer: null,
|
||||||
user_agent: "Mozilla/5.0 (Macintosh)",
|
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",
|
country: "GB",
|
||||||
device_type: "desktop",
|
device_type: "desktop",
|
||||||
browser: "safari",
|
browser: "safari",
|
||||||
@@ -227,8 +229,9 @@ describe("readFunnel", () => {
|
|||||||
|
|
||||||
describe("readTraffic", () => {
|
describe("readTraffic", () => {
|
||||||
it("separates legacy rows, live loopback and automation from clean traffic", async () => {
|
it("separates legacy rows, live loopback and automation from clean traffic", async () => {
|
||||||
// Post-instrumentation rows: host set. One of them still loopback — the
|
// Post-instrumentation rows: host set. Two of them still loopback — in the
|
||||||
// shape the report must flag as "the fix is not live".
|
// truncated shapes the privacy floor writes — which is the shape the report
|
||||||
|
// must flag as "the fix is not live".
|
||||||
insertAnalytics({
|
insertAnalytics({
|
||||||
path: NOOK_DOWNLOAD_PATH,
|
path: NOOK_DOWNLOAD_PATH,
|
||||||
stage: "click",
|
stage: "click",
|
||||||
@@ -237,10 +240,15 @@ describe("readTraffic", () => {
|
|||||||
insertAnalytics({
|
insertAnalytics({
|
||||||
path: NOOK_DOWNLOAD_PATH,
|
path: NOOK_DOWNLOAD_PATH,
|
||||||
stage: "served",
|
stage: "served",
|
||||||
ip_address: "127.0.0.1",
|
ip_address: "127.0.0.0",
|
||||||
user_agent: "The Nook/0.13.4 Sparkle/2.9.6",
|
user_agent: "The Nook/0.13.4 Sparkle/2.9.6",
|
||||||
created_at: THIS_WEEK
|
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 });
|
insertAnalytics({ path: "/", created_at: THIS_WEEK });
|
||||||
|
|
||||||
// Pre-instrumentation rows: no host, proxy's address — legacy, never
|
// Pre-instrumentation rows: no host, proxy's address — legacy, never
|
||||||
@@ -263,10 +271,10 @@ describe("readTraffic", () => {
|
|||||||
const traffic = await readTraffic(conns, windowsFor(NOW, 7).current);
|
const traffic = await readTraffic(conns, windowsFor(NOW, 7).current);
|
||||||
|
|
||||||
expect(traffic).toEqual({
|
expect(traffic).toEqual({
|
||||||
analyticsRows: 5,
|
analyticsRows: 6,
|
||||||
hostRows: 3,
|
hostRows: 4,
|
||||||
legacyRows: 2,
|
legacyRows: 2,
|
||||||
hostLoopbackRows: 1,
|
hostLoopbackRows: 2,
|
||||||
downloadRows: 3,
|
downloadRows: 3,
|
||||||
downloadClickRows: 1,
|
downloadClickRows: 1,
|
||||||
downloadServedRows: 1,
|
downloadServedRows: 1,
|
||||||
@@ -482,13 +490,16 @@ describe("getWeeklyReport / renderWeeklyMarkdown", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("flags loopback only when rows recorded since the instrumentation carry it", async () => {
|
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.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 report = await getWeeklyReport(conns, { now: NOW, days: 7 });
|
||||||
const markdown = renderWeeklyMarkdown(report);
|
const markdown = renderWeeklyMarkdown(report);
|
||||||
|
|
||||||
expect(markdown).toContain(
|
expect(markdown).toContain(
|
||||||
"1 rows recorded since the instrumentation still carry a loopback address"
|
"2 rows recorded since the instrumentation still carry a loopback address"
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -144,8 +144,16 @@ export interface WeeklyReport {
|
|||||||
generatedAt: string;
|
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`. */
|
/** SQLite's `datetime()` string, UTC: `YYYY-MM-DD HH:MM:SS`. */
|
||||||
export function sqlTime(date: Date): string {
|
export function sqlTime(date: Date): string {
|
||||||
@@ -320,7 +328,6 @@ export async function readTraffic(
|
|||||||
): Promise<WindowTraffic> {
|
): Promise<WindowTraffic> {
|
||||||
const { analytics } = conns;
|
const { analytics } = conns;
|
||||||
const { start, end } = window;
|
const { start, end } = window;
|
||||||
const placeholders = LOOPBACK.map(() => "?").join(", ");
|
|
||||||
|
|
||||||
const analyticsRows = await countInWindow(
|
const analyticsRows = await countInWindow(
|
||||||
analytics,
|
analytics,
|
||||||
@@ -351,8 +358,8 @@ export async function readTraffic(
|
|||||||
`SELECT COUNT(*) AS n FROM VisitorAnalytics
|
`SELECT COUNT(*) AS n FROM VisitorAnalytics
|
||||||
WHERE created_at >= ? AND created_at < ?
|
WHERE created_at >= ? AND created_at < ?
|
||||||
AND host IS NOT NULL
|
AND host IS NOT NULL
|
||||||
AND ip_address IN (${placeholders})`,
|
AND ${LOOPBACK_SQL}`,
|
||||||
[start, end, ...LOOPBACK]
|
[start, end]
|
||||||
);
|
);
|
||||||
|
|
||||||
const downloadRows = await scalar(
|
const downloadRows = await scalar(
|
||||||
@@ -595,7 +602,9 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string {
|
|||||||
`(127.0.0.1 / ::1) — the writers then recorded the direct peer, not the visitor. ` +
|
`(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 ` +
|
`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. ` +
|
`click/served split), and "unique visitors" over them still collapses to one host. ` +
|
||||||
`Rows recorded since carry the real client address and the site they addressed.`
|
`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.`
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -603,8 +612,8 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string {
|
|||||||
lines.push(
|
lines.push(
|
||||||
`- **${traffic.hostLoopbackRows} rows recorded since the instrumentation still carry a ` +
|
`- **${traffic.hostLoopbackRows} rows recorded since the instrumentation still carry a ` +
|
||||||
`loopback address** — the forwarded-header fix is not taking effect in production ` +
|
`loopback address** — the forwarded-header fix is not taking effect in production ` +
|
||||||
`(or the proxy is not setting \`x-forwarded-for\`). Do not trust per-channel reach ` +
|
`(no \`cf-connecting-ip\`, and no usable \`x-forwarded-for\` hop). Do not trust ` +
|
||||||
`until this is zero.`
|
`per-channel reach until this is zero.`
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -57,6 +57,7 @@
|
|||||||
}
|
}
|
||||||
],
|
],
|
||||||
"crons": [
|
"crons": [
|
||||||
|
{ "path": "/api/analytics-retention", "schedule": "0 12 * * *" },
|
||||||
{ "path": "/api/the-nook/lifecycle", "schedule": "0 13 * * *" }
|
{ "path": "/api/the-nook/lifecycle", "schedule": "0 13 * * *" }
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user