From f896ea0a4b31f17a679c538efca9362c16623476 Mon Sep 17 00:00:00 2001 From: Sagnik Ghosh Date: Sun, 25 Oct 2026 16:22:04 +0530 Subject: [PATCH 1/2] fix(runtime-node): relay rate limit no longer trusts X-Forwarded-For by default The browser relay keyed its per-IP rate limit on the first X-Forwarded-For entry, which any caller can set. Rotating it gave a fresh window on every request, so the limit did not bound parsing and forwarding under the server's ingest key. New option `trustProxy` (only the boolean `true` enables it). Without it the Node handler keys the real socket peer and the fetch handler keys one shared bucket for the route. Behind a proxy that overwrites the header, set `trustProxy: true` to key each client. Ported from the relay hardening by rajeshaipython-stack that followed #20 on the fork and never reached main. Its context redaction and detached oversize cancel already landed through #26 and #20. Co-authored-by: rajeshaipython-stack Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++ docs/GETTING-STARTED.md | 4 + packages/runtime-node/README.md | 23 +++++- packages/runtime-node/src/relay.ts | 26 ++++++- packages/runtime-node/test/relay.test.mjs | 91 +++++++++++++++++++++++ 5 files changed, 147 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 639c49c..7035069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Security + +- The browser relay no longer trusts `X-Forwarded-For` for its per-IP rate limit unless you opt in (`runtime-node`). Any caller could set that header 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` (only the boolean `true` enables it). **Behaviour change:** without it, the Node handler keys the real socket peer and the fetch handler keys one shared bucket for the route, so a relay behind a load balancer, CDN or platform proxy shares one 120-a-minute window until it sets `trustProxy: true`. 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..04ae06e 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -428,6 +428,10 @@ 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`). +- [ ] Behind a load balancer, CDN or platform proxy that overwrites + `X-Forwarded-For`, the relay sets `trustProxy: true`, so the limit + keys each client instead of the proxy (or, in fetch runtimes, one + shared bucket for the whole route). - [ ] 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..48209a4 100644 --- a/packages/runtime-node/README.md +++ b/packages/runtime-node/README.md @@ -127,7 +127,28 @@ 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 client. By default it never trusts the `X-Forwarded-For` header, because +any caller can set it to a fresh value on every request and walk past the +limit. Without it, the Node handler keys the real socket peer, and the fetch +handler (which has no peer address) keys one shared bucket for the whole +route. Behind a load balancer, CDN or platform proxy that overwrites +`X-Forwarded-For`, every request arrives from the proxy, so set +`trustProxy: true` to key the real client instead: + +```ts +createBrowserRelayFetchHandler({ + apiKey: process.env.AUTTER_RUNTIME_KEY!, + trustProxy: true, // only behind a proxy you control that overwrites X-Forwarded-For +}); +``` + +Only the boolean `true` turns it on: a string such as `"false"` from an +environment variable keeps the safe default. ## 2. Server tracker diff --git a/packages/runtime-node/src/relay.ts b/packages/runtime-node/src/relay.ts index 5fdd1c5..f6036d9 100644 --- a/packages/runtime-node/src/relay.ts +++ b/packages/runtime-node/src/relay.ts @@ -31,6 +31,17 @@ export interface RelayOptions { * `false` to disable (e.g. when a WAF already rate-limits). */ perIpRateLimit?: number | false; + /** + * Trust the client-supplied `X-Forwarded-For` header when keying the per-IP + * rate limit. Off by default: the header is spoofable, so a caller could + * rotate it to bypass the window and drive unbounded parsing/forwarding + * under the server's ingest key. Enable ONLY behind a proxy/CDN you control + * that overwrites the header. When off, the fetch handler keys a single + * shared bucket, and the Node handler keys the real socket peer address. + * Only a strict boolean `true` enables it — a truthy string such as the + * common `process.env.TRUST_PROXY === "false"` slip stays on the safe path. + */ + trustProxy?: boolean; /** Called when the async forward fails (default: console.warn). */ onError?: (err: unknown) => void; /** @@ -411,8 +422,15 @@ export function createBrowserRelayFetchHandler( return new Response(null, { status: 405 }); } if (limiter) { + // Only honor X-Forwarded-For behind an explicitly trusted proxy — + // otherwise a caller could spoof a fresh IP per request to bypass + // the window. With no trusted peer source in a fetch runtime, fall + // back to one shared bucket (a conservative global limit). const ip = - firstForwardedFor(request.headers.get("x-forwarded-for")) || "unknown"; + opts.trustProxy === true + ? firstForwardedFor(request.headers.get("x-forwarded-for")) || + "unknown" + : "shared"; if (!limiter.allow(ip)) { return new Response(JSON.stringify({ error: "rate limit exceeded" }), { status: 429, @@ -501,8 +519,12 @@ export function createBrowserRelayHandler( return; } if (limiter) { + // Prefer the real socket peer; only trust X-Forwarded-For when the + // caller has explicitly opted into a trusted-proxy deployment. const ip = - firstForwardedFor(req.headers["x-forwarded-for"]) || + (opts.trustProxy === true + ? firstForwardedFor(req.headers["x-forwarded-for"]) + : "") || req.socket?.remoteAddress || "unknown"; if (!limiter.allow(ip)) { diff --git a/packages/runtime-node/test/relay.test.mjs b/packages/runtime-node/test/relay.test.mjs index 7f6d51e..2a8818e 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,93 @@ 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) ───────────────────────────────────── + +const xffRequest = (ip) => + new Request("http://localhost/relay", { + method: "POST", + headers: { "x-forwarded-for": ip }, + body: "{", + }); + +test("relay: spoofed X-Forwarded-For cannot bypass the rate limit by default", async () => { + const handler = createBrowserRelayFetchHandler({ + apiKey: "autter_rt_test", + perIpRateLimit: 1, + }); + const first = await handler(xffRequest("1.1.1.1")); + const second = await handler(xffRequest("2.2.2.2")); + assert.equal(first.status, 400); // passed the limit, then invalid JSON + assert.equal(second.status, 429); // shared bucket: a fresh spoofed IP is not a fresh window +}); + +test("relay: a truthy but non-true trustProxy stays on the safe shared bucket", async () => { + const handler = createBrowserRelayFetchHandler({ + apiKey: "autter_rt_test", + perIpRateLimit: 1, + // a common config slip: an env string "false" is truthy but must NOT + // enable forwarded-header trust + trustProxy: "false", + }); + const first = await handler(xffRequest("1.1.1.1")); + const second = await handler(xffRequest("2.2.2.2")); + assert.equal(first.status, 400); + assert.equal(second.status, 429); +}); + +test("relay: trustProxy: true keys distinct X-Forwarded-For clients apart", async () => { + const handler = createBrowserRelayFetchHandler({ + apiKey: "autter_rt_test", + perIpRateLimit: 1, + trustProxy: true, + }); + const a = await handler(xffRequest("1.1.1.1")); + const b = await handler(xffRequest("2.2.2.2")); + const a2 = await handler(xffRequest("1.1.1.1")); + assert.notEqual(a.status, 429); + assert.notEqual(b.status, 429); + assert.equal(a2.status, 429); // the same client still hits its own window +}); + +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); + }); +} + +test("relay (node): keys the socket peer, not a spoofed X-Forwarded-For, by default", async () => { + const handler = createBrowserRelayHandler({ + apiKey: "autter_rt_test", + perIpRateLimit: 1, + }); + assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 400); + // same peer, new spoofed header: still the same window + assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.5" }), 429); + // a different real peer gets its own window + assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.6" }), 400); +}); + +test("relay (node): trustProxy: true keys the forwarded client behind a proxy", async () => { + const handler = createBrowserRelayHandler({ + apiKey: "autter_rt_test", + perIpRateLimit: 1, + trustProxy: true, + }); + assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 400); + assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.5" }), 400); + assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 429); +}); From 754e08d2870c17f5e0604a1af0ee05ba372b79b6 Mon Sep 17 00:00:00 2001 From: Sagnik Ghosh Date: Sun, 25 Oct 2026 16:33:21 +0530 Subject: [PATCH 2/2] feat(runtime-node): relay picks the visitor's address automatically on any host The strict default from the previous commit kept the limit safe but put every visitor behind a proxy into one window. `trustProxy` now defaults to "auto": read X-Forwarded-For right to left and take the first public address, which the nearest load balancer, CDN or platform appended or wrote. In Node, a connection straight from a public address is the visitor and its header is ignored. On Cloudflare Workers, use CF-Connecting-IP when the runtime set request.cf. `trustProxy` also takes a hop count, `true` (leftmost entry) and `false`, including the same values as strings from env vars, and a new `clientIp(request)` option supplies the address directly. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/GETTING-STARTED.md | 9 +- packages/runtime-node/README.md | 45 +++-- packages/runtime-node/src/relay.ts | 204 +++++++++++++++++++--- packages/runtime-node/test/relay.test.mjs | 178 +++++++++++++------ 5 files changed, 332 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7035069..c0c7d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Security -- The browser relay no longer trusts `X-Forwarded-For` for its per-IP rate limit unless you opt in (`runtime-node`). Any caller could set that header 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` (only the boolean `true` enables it). **Behaviour change:** without it, the Node handler keys the real socket peer and the fetch handler keys one shared bucket for the route, so a relay behind a load balancer, CDN or platform proxy shares one 120-a-minute window until it sets `trustProxy: true`. 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. +- 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 diff --git a/docs/GETTING-STARTED.md b/docs/GETTING-STARTED.md index 04ae06e..2c8b19a 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -428,10 +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`). -- [ ] Behind a load balancer, CDN or platform proxy that overwrites - `X-Forwarded-For`, the relay sets `trustProxy: true`, so the limit - keys each client instead of the proxy (or, in fetch runtimes, one - shared bucket for the whole route). +- [ ] 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 48209a4..0cdb749 100644 --- a/packages/runtime-node/README.md +++ b/packages/runtime-node/README.md @@ -132,23 +132,34 @@ Options: `endpoint` (default `https://otlp.autter.dev`), `maxBodyBytes` `compatCheck`, `onError`. **Rate-limit key and `trustProxy`.** The relay route is public, so it limits -each client. By default it never trusts the `X-Forwarded-For` header, because -any caller can set it to a fresh value on every request and walk past the -limit. Without it, the Node handler keys the real socket peer, and the fetch -handler (which has no peer address) keys one shared bucket for the whole -route. Behind a load balancer, CDN or platform proxy that overwrites -`X-Forwarded-For`, every request arrives from the proxy, so set -`trustProxy: true` to key the real client instead: - -```ts -createBrowserRelayFetchHandler({ - apiKey: process.env.AUTTER_RUNTIME_KEY!, - trustProxy: true, // only behind a proxy you control that overwrites X-Forwarded-For -}); -``` - -Only the boolean `true` turns it on: a string such as `"false"` from an -environment variable keeps the safe default. +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 f6036d9..fe24359 100644 --- a/packages/runtime-node/src/relay.ts +++ b/packages/runtime-node/src/relay.ts @@ -32,16 +32,36 @@ export interface RelayOptions { */ perIpRateLimit?: number | false; /** - * Trust the client-supplied `X-Forwarded-For` header when keying the per-IP - * rate limit. Off by default: the header is spoofable, so a caller could - * rotate it to bypass the window and drive unbounded parsing/forwarding - * under the server's ingest key. Enable ONLY behind a proxy/CDN you control - * that overwrites the header. When off, the fetch handler keys a single - * shared bucket, and the Node handler keys the real socket peer address. - * Only a strict boolean `true` enables it — a truthy string such as the - * common `process.env.TRUST_PROXY === "false"` slip stays on the safe path. + * 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; + 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; /** @@ -86,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; @@ -422,15 +589,7 @@ export function createBrowserRelayFetchHandler( return new Response(null, { status: 405 }); } if (limiter) { - // Only honor X-Forwarded-For behind an explicitly trusted proxy — - // otherwise a caller could spoof a fresh IP per request to bypass - // the window. With no trusted peer source in a fetch runtime, fall - // back to one shared bucket (a conservative global limit). - const ip = - opts.trustProxy === true - ? firstForwardedFor(request.headers.get("x-forwarded-for")) || - "unknown" - : "shared"; + const ip = fetchClientKey(request, opts); if (!limiter.allow(ip)) { return new Response(JSON.stringify({ error: "rate limit exceeded" }), { status: 429, @@ -519,14 +678,7 @@ export function createBrowserRelayHandler( return; } if (limiter) { - // Prefer the real socket peer; only trust X-Forwarded-For when the - // caller has explicitly opted into a trusted-proxy deployment. - const ip = - (opts.trustProxy === true - ? 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 2a8818e..51e4cd1 100644 --- a/packages/runtime-node/test/relay.test.mjs +++ b/packages/runtime-node/test/relay.test.mjs @@ -256,51 +256,103 @@ test("the relay keeps valid debug ids, query-stripped, and drops the rest", () = }); // ── 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 xffRequest = (ip) => - new Request("http://localhost/relay", { - method: "POST", - headers: { "x-forwarded-for": ip }, - body: "{", - }); +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; +}; -test("relay: spoofed X-Forwarded-For cannot bypass the rate limit by default", async () => { - const handler = createBrowserRelayFetchHandler({ - apiKey: "autter_rt_test", - perIpRateLimit: 1, - }); - const first = await handler(xffRequest("1.1.1.1")); - const second = await handler(xffRequest("2.2.2.2")); - assert.equal(first.status, 400); // passed the limit, then invalid JSON - assert.equal(second.status, 429); // shared bucket: a fresh spoofed IP is not a fresh window +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: a truthy but non-true trustProxy stays on the safe shared bucket", async () => { - const handler = createBrowserRelayFetchHandler({ - apiKey: "autter_rt_test", - perIpRateLimit: 1, - // a common config slip: an env string "false" is truthy but must NOT - // enable forwarded-header trust - trustProxy: "false", - }); - const first = await handler(xffRequest("1.1.1.1")); - const second = await handler(xffRequest("2.2.2.2")); - assert.equal(first.status, 400); - assert.equal(second.status, 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: trustProxy: true keys distinct X-Forwarded-For clients apart", async () => { - const handler = createBrowserRelayFetchHandler({ - apiKey: "autter_rt_test", - perIpRateLimit: 1, - trustProxy: true, - }); - const a = await handler(xffRequest("1.1.1.1")); - const b = await handler(xffRequest("2.2.2.2")); - const a2 = await handler(xffRequest("1.1.1.1")); - assert.notEqual(a.status, 429); - assert.notEqual(b.status, 429); - assert.equal(a2.status, 429); // the same client still hits its own window +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 }) { @@ -322,25 +374,35 @@ function nodeCall(handler, { xff, peer }) { }); } -test("relay (node): keys the socket peer, not a spoofed X-Forwarded-For, by default", async () => { - const handler = createBrowserRelayHandler({ - apiKey: "autter_rt_test", - perIpRateLimit: 1, - }); - assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 400); - // same peer, new spoofed header: still the same window - assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.5" }), 429); - // a different real peer gets its own window - assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.6" }), 400); +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): trustProxy: true keys the forwarded client behind a proxy", async () => { - const handler = createBrowserRelayHandler({ - apiKey: "autter_rt_test", - perIpRateLimit: 1, - trustProxy: true, - }); - assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 400); - assert.equal(await nodeCall(handler, { xff: "2.2.2.2", peer: "10.0.0.5" }), 400); - assert.equal(await nodeCall(handler, { xff: "1.1.1.1", peer: "10.0.0.5" }), 429); +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); });