diff --git a/CHANGELOG.md b/CHANGELOG.md index 639c49c..c0c7d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Security + +- The browser relay no longer trusts the first `X-Forwarded-For` entry for its per-visitor rate limit (`runtime-node`). Any caller could set that entry to a new value on each request and get a fresh window every time, driving unbounded parsing and forwarding under your ingest key. New option `trustProxy`, default `"auto"`: the relay reads the header right to left and takes the first public address, which is the one your load balancer, CDN or platform appended or wrote, and in Node it ignores the header entirely when the connection comes straight from a public address. On Cloudflare Workers it uses `CF-Connecting-IP`. No setup is needed behind common load balancers and hosting platforms. `trustProxy` also takes a hop count, `true` (the old leftmost behaviour) or `false`, and a new `clientIp(request)` option supplies the address yourself. Ported from the relay hardening by @rajeshaipython-stack that followed #20 and never reached `main`; its context redaction and detached cancel already reached `main` through #26 and #20, with their tests. + ## [1.10.0] - 2026-10-24 Packages: `otlp-ingester` 1.10.0, `runtime-edge` 1.3.0; `adapters/python` (copied, not published); `runtime-browser` 1.7.0, `runtime-node` 1.8.0, `runtime-next` 1.8.0 (its `/edge` entry picks up `runtime-edge` 1.3.0 through `^1.2.0`) and `runtime-cli` 1.2.0 are unchanged. Compat feature `edge_request_counts`. Request counts for services the error rate was wrong for: edge functions had errors and no requests, and Python services counted only the requests head sampling kept while every error was retained, so their error rates read about 1/sampleRate too high. Any deploy order works: an ingester before this ignores the new edge attributes (edge requests stay uncounted, as before), an older `runtime-edge` sends no mark (its summaries stay uncounted), and the Python opt-in needs only an ingester from 1.8.0. diff --git a/docs/GETTING-STARTED.md b/docs/GETTING-STARTED.md index 521da75..2c8b19a 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -428,6 +428,11 @@ clickhouse-client --password dev`. doesn't cost you debugging context. - [ ] The relay route keeps its built-in per-IP rate limit (or your WAF covers it: `perIpRateLimit: false`). +- [ ] The relay's default `trustProxy: "auto"` keys each visitor behind + load balancers, CDNs and hosting platforms. Set a hop count only for a + Node origin behind a public CDN with no load balancer (`trustProxy: 1` + behind Cloudflare), and `trustProxy: false` or `clientIp` for a fetch + handler exposed with no proxy at all. - [ ] Direct browser ingest: your CSP includes `connect-src https://your-ingester…`, and you accept that ad-blockers may drop some events (the relay avoids this). diff --git a/packages/runtime-node/README.md b/packages/runtime-node/README.md index 8621cf8..0cdb749 100644 --- a/packages/runtime-node/README.md +++ b/packages/runtime-node/README.md @@ -127,7 +127,39 @@ export const POST = createBrowserRelayFetchHandler({ ``` Options: `endpoint` (default `https://otlp.autter.dev`), `maxBodyBytes` -(default 64 KB), `onError`. +(default 64 KB), `perIpRateLimit` (default 120 requests a minute per client; +`false` when a WAF already limits the route), `trustProxy`, `redact`, +`compatCheck`, `onError`. + +**Rate-limit key and `trustProxy`.** The relay route is public, so it limits +each visitor. Any caller can write anything into `X-Forwarded-For`, so by +default (`trustProxy: "auto"`) the relay never takes that header's first entry. +Proxies append the address they saw to the right of the header, or overwrite +it, so the relay reads it right to left and takes the first public address, +skipping hops on your own network. That works behind load balancers, CDNs and +hosting platforms with no setup: + +| Where the relay runs | What `"auto"` keys | +| --- | --- | +| Node, reached directly | The connection's own address; the header is ignored | +| Node behind a load balancer or ingress on your network | The address that proxy appended | +| Next.js route handlers, Vercel, Netlify, Deno Deploy and other fetch runtimes | The rightmost public entry, which the platform's edge wrote | +| Cloudflare Workers | `CF-Connecting-IP` | + +Other settings: + +- `trustProxy: 1` (or any count) when exactly that many proxies you control + sit in front, such as a Node origin behind Cloudflare with no load balancer. +- `trustProxy: true` when every proxy in front strips client values: the + leftmost entry. +- `trustProxy: false` never reads the header: the socket address in Node, one + shared bucket in fetch runtimes. +- `clientIp: (request) => string` to supply the address yourself, for example + Deno's `info.remoteAddr` or Bun's `server.requestIP`. + +Strings from environment variables work too (`"auto"`, `"false"`, `"2"`). +A fetch handler exposed straight to the internet with no proxy in front can +still be fooled by a forged header; set `trustProxy: false` or `clientIp` there. ## 2. Server tracker diff --git a/packages/runtime-node/src/relay.ts b/packages/runtime-node/src/relay.ts index 5fdd1c5..fe24359 100644 --- a/packages/runtime-node/src/relay.ts +++ b/packages/runtime-node/src/relay.ts @@ -31,6 +31,37 @@ export interface RelayOptions { * `false` to disable (e.g. when a WAF already rate-limits). */ perIpRateLimit?: number | false; + /** + * How the per-client rate limit finds the visitor's address. A caller can + * write anything into `X-Forwarded-For`, so the relay never takes its + * first (leftmost) entry unless told to. + * + * - `"auto"` (default): works behind any load balancer, CDN or platform + * proxy without setup. Proxies append the address they saw to the right + * of the header (or overwrite it), so the relay reads it right to left + * and takes the first public address, skipping private and loopback + * hops. The Node handler only does this when the connection itself came + * from a private address (a proxy on your network); a public peer is the + * visitor and its header is ignored. On Cloudflare Workers it uses + * `CF-Connecting-IP`. + * - a number `n`: exactly `n` proxies you control sit in front; use the + * entry `n` from the right (Express `trust proxy` semantics). Use `1` + * for a Node origin behind Cloudflare with no load balancer. + * - `true`: every proxy in front is yours and strips client values; use + * the leftmost entry. + * - `false`: never read the header. The Node handler keys the socket peer; + * the fetch handler keys one shared bucket for the route. + * + * Strings from environment variables are read too (`"auto"`, `"true"`, + * `"false"`, `"0"`, `"2"`); anything else falls back to `"auto"`. + */ + trustProxy?: boolean | "auto" | number | string; + /** + * Return the visitor's address yourself (Deno's `info.remoteAddr`, Bun's + * `server.requestIP`, a header your edge sets). Wins over `trustProxy` + * when it returns a non-empty string. + */ + clientIp?: (request: Request | IncomingMessage) => string | null | undefined; /** Called when the async forward fails (default: console.warn). */ onError?: (err: unknown) => void; /** @@ -75,6 +106,153 @@ function firstForwardedFor(value: string | string[] | undefined | null): string return raw ? (raw.split(",")[0] ?? "").trim() : ""; } +// ── Client address for the rate limit ────────────────────────────────────── + +type ProxyTrust = + | { mode: "auto" } + | { mode: "never" } + | { mode: "all" } + | { mode: "hops"; hops: number }; + +const TRUST_AUTO: ProxyTrust = { mode: "auto" }; +let warnedTrustProxy = false; + +/** Read `trustProxy` (including strings from env vars) into one mode. */ +export function resolveTrustProxy(value: unknown): ProxyTrust { + if (value === undefined || value === null || value === "auto") return TRUST_AUTO; + if (value === true) return { mode: "all" }; + if (value === false || value === 0) return { mode: "never" }; + if (typeof value === "number" && Number.isInteger(value) && value >= 1 && value <= 100) { + return { mode: "hops", hops: value }; + } + if (typeof value === "string") { + const v = value.trim().toLowerCase(); + if (v === "" || v === "auto") return TRUST_AUTO; + if (v === "false" || v === "0" || v === "no" || v === "off") return { mode: "never" }; + if (v === "true" || v === "yes" || v === "on") return { mode: "all" }; + if (/^[1-9][0-9]?$/.test(v)) return { mode: "hops", hops: Number(v) }; + } + if (!warnedTrustProxy) { + warnedTrustProxy = true; + console.warn( + `[autter-runtime] relay: trustProxy ${JSON.stringify(value)} is not recognised; using "auto".`, + ); + } + return TRUST_AUTO; +} + +const IPV4_RE = /^(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}$/; + +/** One forwarded/peer entry as a bare, lowercased IP, or null when it is not one. */ +export function normalizeAddress(entry: string | null | undefined): string | null { + if (!entry) return null; + let a = entry.trim(); + if (!a || a.length > 64) return null; + const bracketed = /^\[([0-9A-Fa-f:.]+)\](?::\d{1,5})?$/.exec(a); + if (bracketed) a = bracketed[1] ?? ""; + else if (/^[\d.]+:\d{1,5}$/.test(a)) a = a.slice(0, a.lastIndexOf(":")); + a = a.toLowerCase(); + if (a.startsWith("::ffff:") && IPV4_RE.test(a.slice(7))) a = a.slice(7); + if (IPV4_RE.test(a)) return a; + if (/^[0-9a-f:]+(:(\d{1,3}\.){3}\d{1,3})?$/.test(a) && a.includes(":") && a.split(":").length <= 9) { + return a; + } + return null; +} + +/** Loopback, private, link-local, carrier-grade NAT or unique-local: a hop on your own network. */ +export function isPrivateAddress(a: string): boolean { + if (IPV4_RE.test(a)) { + const [x, y] = a.split(".").map(Number) as [number, number]; + return ( + x === 10 || + x === 127 || + x === 0 || + (x === 169 && y === 254) || + (x === 172 && y >= 16 && y <= 31) || + (x === 192 && y === 168) || + (x === 100 && y >= 64 && y <= 127) + ); + } + return a === "::1" || a === "::" || /^f[cd]/.test(a) || /^fe[89ab]/.test(a); +} + +function forwardedEntries(value: string | string[] | undefined | null): string[] { + const raw = Array.isArray(value) ? value.join(",") : value; + if (!raw) return []; + const out: string[] = []; + for (const part of raw.split(",")) { + const a = normalizeAddress(part); + if (a) out.push(a); + } + return out; +} + +/** Right to left, the first address that is not a hop on your own network. */ +function rightmostPublic(entries: string[]): string | null { + for (let i = entries.length - 1; i >= 0; i--) { + const a = entries[i]; + if (a && !isPrivateAddress(a)) return a; + } + return null; +} + +function fromCustom( + opts: RelayOptions, + request: Request | IncomingMessage, +): string | null { + if (!opts.clientIp) return null; + try { + const v = opts.clientIp(request); + return typeof v === "string" && v.trim() ? v.trim().slice(0, 64) : null; + } catch { + return null; + } +} + +/** Rate-limit key for the fetch handler, which has no peer address. */ +export function fetchClientKey(request: Request, opts: RelayOptions): string { + const custom = fromCustom(opts, request); + if (custom) return custom; + const trust = resolveTrustProxy(opts.trustProxy); + if (trust.mode === "never") return "shared"; + const header = request.headers.get("x-forwarded-for"); + if (trust.mode === "all") return firstForwardedFor(header) || "shared"; + const entries = forwardedEntries(header); + if (trust.mode === "hops") { + return entries[Math.max(0, entries.length - trust.hops)] ?? "shared"; + } + // auto. Cloudflare Workers: `request.cf` is set by the runtime, never by a + // header, so its CF-Connecting-IP is the address Cloudflare accepted. + const cf = (request as unknown as { cf?: unknown }).cf; + if (cf !== null && typeof cf === "object") { + const viaCf = normalizeAddress(request.headers.get("cf-connecting-ip")); + if (viaCf) return viaCf; + } + // Fetch runtimes sit behind the platform's edge, which appends (or + // overwrites) the address it saw: the rightmost public entry. + return rightmostPublic(entries) ?? entries[entries.length - 1] ?? "shared"; +} + +/** Rate-limit key for the Node handler. */ +export function nodeClientKey(req: IncomingMessage, opts: RelayOptions): string { + const custom = fromCustom(opts, req); + if (custom) return custom; + const trust = resolveTrustProxy(opts.trustProxy); + const peer = normalizeAddress(req.socket?.remoteAddress); + if (trust.mode === "never") return peer ?? "unknown"; + const header = req.headers["x-forwarded-for"]; + if (trust.mode === "all") return firstForwardedFor(header) || peer || "unknown"; + const entries = forwardedEntries(header); + if (trust.mode === "hops") { + return entries[Math.max(0, entries.length - trust.hops)] ?? peer ?? "unknown"; + } + // auto: a public peer is the visitor itself, whatever header it sent. + if (peer && !isPrivateAddress(peer)) return peer; + // The connection came from a proxy on your network. + return rightmostPublic(entries) ?? peer ?? entries[entries.length - 1] ?? "unknown"; +} + const DEFAULT_ENDPOINT = "https://otlp.autter.dev"; const DEFAULT_MAX_BODY = 64 * 1024; const MAX_EVENTS = 50; @@ -411,8 +589,7 @@ export function createBrowserRelayFetchHandler( return new Response(null, { status: 405 }); } if (limiter) { - const ip = - firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown"; + const ip = fetchClientKey(request, opts); if (!limiter.allow(ip)) { return new Response(JSON.stringify({ error: "rate limit exceeded" }), { status: 429, @@ -501,10 +678,7 @@ export function createBrowserRelayHandler( return; } if (limiter) { - const ip = - firstForwardedFor(req.headers["x-forwarded-for"]) || - req.socket?.remoteAddress || - "unknown"; + const ip = nodeClientKey(req, opts); if (!limiter.allow(ip)) { respond(res, 429, { error: "rate limit exceeded" }); return; diff --git a/packages/runtime-node/test/relay.test.mjs b/packages/runtime-node/test/relay.test.mjs index 7f6d51e..51e4cd1 100644 --- a/packages/runtime-node/test/relay.test.mjs +++ b/packages/runtime-node/test/relay.test.mjs @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { sanitizeBrowserPayload, createBrowserRelayFetchHandler, + createBrowserRelayHandler, } from "../dist/index.js"; function sanitizeContext(context) { @@ -253,3 +254,155 @@ test("the relay keeps valid debug ids, query-stripped, and drops the rest", () = assert.deepEqual(payload.events[0].debugIds, { "https://app.test/assets/app.js": id.toLowerCase() }); assert.equal("debugIds" in payload.events[1], false); }); + +// ── Rate-limit key source (trustProxy) ───────────────────────────────────── +// Each case sends two requests against a limit of 1: the second is 429 when +// both landed in the same bucket, and 400 (past the limit, then invalid JSON) +// when they were keyed apart. + +const fetchReq = (xff, extra = {}) => { + const headers = { ...(xff === undefined ? {} : { "x-forwarded-for": xff }), ...(extra.headers ?? {}) }; + const req = new Request("http://localhost/relay", { method: "POST", headers, body: "{" }); + if (extra.cf) Object.defineProperty(req, "cf", { value: extra.cf }); + return req; +}; + +async function fetchPair(opts, a, b) { + const handler = createBrowserRelayFetchHandler({ apiKey: "autter_rt_test", perIpRateLimit: 1, ...opts }); + return [(await handler(a)).status, (await handler(b)).status]; +} + +test("relay auto: values a caller writes left of the proxy's entry cannot bypass the limit", async () => { + // A proxy appended 203.0.113.9; the caller rotated what it put in front. + assert.deepEqual( + await fetchPair({}, fetchReq("1.1.1.1, 203.0.113.9"), fetchReq("2.2.2.2, 203.0.113.9")), + [400, 429], + ); +}); + +test("relay auto: distinct visitors behind a platform proxy keep separate windows", async () => { + // Vercel-style overwrite: one entry per request. + assert.deepEqual(await fetchPair({}, fetchReq("198.51.100.1"), fetchReq("198.51.100.2")), [400, 400]); + // Appended chain behind an internal load balancer: private hops are skipped. + assert.deepEqual( + await fetchPair({}, fetchReq("198.51.100.1, 10.0.0.2"), fetchReq("198.51.100.2, 10.0.0.2")), + [400, 400], + ); +}); + +test("relay auto: Cloudflare Workers use CF-Connecting-IP only when the runtime set request.cf", async () => { + const cf = { colo: "LHR" }; + assert.deepEqual( + await fetchPair( + {}, + fetchReq("9.9.9.9", { cf, headers: { "cf-connecting-ip": "198.51.100.1" } }), + fetchReq("9.9.9.9", { cf, headers: { "cf-connecting-ip": "198.51.100.2" } }), + ), + [400, 400], + ); + // Without request.cf the header is just something a caller sent. + assert.deepEqual( + await fetchPair( + {}, + fetchReq("9.9.9.9", { headers: { "cf-connecting-ip": "198.51.100.1" } }), + fetchReq("9.9.9.9", { headers: { "cf-connecting-ip": "198.51.100.2" } }), + ), + [400, 429], + ); +}); + +test("relay: trustProxy false keys one shared bucket in fetch runtimes, including the env string", async () => { + for (const trustProxy of [false, "false", "0"]) { + assert.deepEqual( + await fetchPair({ trustProxy }, fetchReq("198.51.100.1"), fetchReq("198.51.100.2")), + [400, 429], + `trustProxy ${JSON.stringify(trustProxy)}`, + ); + } +}); + +test("relay: trustProxy hop count reads the entry that many from the right", async () => { + // Two proxies you control appended "client, edge"; the client is 2 from the right. + assert.deepEqual( + await fetchPair( + { trustProxy: 2 }, + fetchReq("6.6.6.6, 198.51.100.1, 203.0.113.50"), + fetchReq("7.7.7.7, 198.51.100.1, 203.0.113.50"), + ), + [400, 429], + ); + assert.deepEqual( + await fetchPair({ trustProxy: "2" }, fetchReq("198.51.100.1, 203.0.113.50"), fetchReq("198.51.100.2, 203.0.113.50")), + [400, 400], + ); +}); + +test("relay: trustProxy true keeps the leftmost entry", async () => { + assert.deepEqual( + await fetchPair({ trustProxy: true }, fetchReq("198.51.100.1, 203.0.113.9"), fetchReq("198.51.100.2, 203.0.113.9")), + [400, 400], + ); +}); + +test("relay: a clientIp resolver wins over the header", async () => { + assert.deepEqual( + await fetchPair( + { clientIp: (r) => r.headers.get("x-test-ip") }, + fetchReq("198.51.100.1", { headers: { "x-test-ip": "a" } }), + fetchReq("198.51.100.2", { headers: { "x-test-ip": "a" } }), + ), + [400, 429], + ); +}); + +function nodeCall(handler, { xff, peer }) { + return new Promise((resolve) => { + const req = { + method: "POST", + headers: xff ? { "x-forwarded-for": xff } : {}, + socket: { remoteAddress: peer }, + body: "{", + }; + const res = { + statusCode: 0, + setHeader() {}, + end() { + resolve(this.statusCode); + }, + }; + handler(req, res); + }); +} + +const nodeHandler = (opts = {}) => + createBrowserRelayHandler({ apiKey: "autter_rt_test", perIpRateLimit: 1, ...opts }); + +test("relay (node) auto: a public peer is the visitor, whatever header it sends", async () => { + const handler = nodeHandler(); + assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "203.0.113.5" }), 400); + assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "203.0.113.5" }), 429); + assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "203.0.113.6" }), 400); +}); + +test("relay (node) auto: behind a proxy on your network, the proxy's entry keys the visitor", async () => { + const handler = nodeHandler(); + // The load balancer (10.0.0.5) appended 198.51.100.7; the left part is the caller's. + assert.equal(await nodeCall(handler, { xff: "1.1.1.1, 198.51.100.7", peer: "10.0.0.5" }), 400); + assert.equal(await nodeCall(handler, { xff: "2.2.2.2, 198.51.100.7", peer: "10.0.0.5" }), 429); + assert.equal(await nodeCall(handler, { xff: "198.51.100.8", peer: "10.0.0.5" }), 400); + // IPv4-mapped IPv6 peers count as private too. + assert.equal(await nodeCall(handler, { xff: "198.51.100.9", peer: "::ffff:10.0.0.5" }), 400); +}); + +test("relay (node): trustProxy false keys the socket peer", async () => { + const handler = nodeHandler({ trustProxy: false }); + assert.equal(await nodeCall(handler, { xff: "198.51.100.1", peer: "10.0.0.5" }), 400); + assert.equal(await nodeCall(handler, { xff: "198.51.100.2", peer: "10.0.0.5" }), 429); +}); + +test("relay (node): trustProxy 1 behind Cloudflare keys the address Cloudflare appended", async () => { + const handler = nodeHandler({ trustProxy: 1 }); + // Peer is a Cloudflare edge (public); Cloudflare appended the visitor. + assert.equal(await nodeCall(handler, { xff: "6.6.6.6, 198.51.100.1", peer: "172.64.0.1" }), 400); + assert.equal(await nodeCall(handler, { xff: "7.7.7.7, 198.51.100.1", peer: "172.64.0.2" }), 429); +});