Files
freno-dev/src/server/request-client.test.ts
Paperclip Agent 1d9287a199 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.
2026-10-29 18:05:44 +00:00

204 lines
6.7 KiB
TypeScript

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"
);
});
});