Warn once on unconfigured proxy; fix trusted-proxy examples

Rhys Sullivan committed 1 month ago

Commit 0ff9d4f1 parent 3fb9a487 files · +172 −38Browse files

Unified · Split

7 files

.changeset/selfhost-auth-client-ip.md +3 −1
apps/docs/hosted/docker.mdx +23 −10
apps/host-selfhost/.env.example +8 −4
apps/host-selfhost/src/auth/better-auth.ts +2 −5
apps/host-selfhost/src/auth/client-ip.test.ts +71 −10
apps/host-selfhost/src/auth/client-ip.ts +64 −7
apps/host-selfhost/src/config.ts +1 −1

.changeset/selfhost-auth-client-ip.md+3−1

@@ -3,4 +3,6 @@ · 2 unmodified lines
3	3	"executor": patch
4	4	---
5	5
6		−Key the self-host's sign-in rate limit on the real client IP. The server now stamps the connecting address on every auth request, so a directly exposed instance limits each client separately with no configuration and a client cannot spoof its address. Behind a reverse proxy, set `EXECUTOR_TRUSTED_PROXY_HEADER` (the header the proxy sets, e.g. `cf-connecting-ip` or `x-real-ip`) and `EXECUTOR_TRUSTED_PROXIES` (the IPs or CIDR ranges the proxy connects from) together; the header is honoured only on connections from those addresses. Half-configured or malformed values refuse to boot.
	6	+Key the self-host's sign-in rate limit on the real client IP. The server now stamps the connecting address on every auth request, so a directly exposed instance limits each client separately with no configuration and a client cannot spoof its address.
	7	+
	8	+Behaviour change for proxied deployments: `x-forwarded-for` is no longer read on its own, because any client could set it. If Executor runs behind a reverse proxy, set both `EXECUTOR_TRUSTED_PROXY_HEADER` (the header the proxy sets, e.g. `cf-connecting-ip` or `x-real-ip`) and `EXECUTOR_TRUSTED_PROXIES` (the proxy's own IPs or CIDR ranges), otherwise all users share one sign-in bucket. The server logs one warning naming both variables when an auth request carries a proxy header and neither is set. List only the proxy addresses, never a range that also contains your users; if every hop is trusted no client IP is found and the bucket is shared again. Half-configured or malformed values refuse to boot.

apps/docs/hosted/docker.mdx+23−10

@@ -118,25 +118,38 @@ browsers reach the container directly, the IP is the connecting address and · 117 unmodified lines
118	118	nothing needs configuring.
119	119
120	120	Behind Caddy, nginx, Cloudflare, or another reverse proxy, every connection
121		−comes from the proxy. Tell Executor which header the proxy sets to the real
122		−client IP, and which addresses the proxy connects from:
	121	+comes from the proxy. If Executor runs behind a reverse proxy, set both
	122	+variables, otherwise all users share one sign-in bucket. Tell Executor which
	123	+header the proxy sets to the real client IP, and which addresses the proxy
	124	+connects from:
123	125
124	126	```bash
125	127	-e EXECUTOR_TRUSTED_PROXY_HEADER=x-real-ip \
126		−-e EXECUTOR_TRUSTED_PROXIES=10.0.0.0/8,172.16.0.0/12
	128	+-e EXECUTOR_TRUSTED_PROXIES=172.18.0.2
127	129	```
128	130
129	131	Set both together. The header is only honoured on connections from one of the
130	132	listed addresses, so a client that reaches the container directly cannot spoof
131		−it. Use `cf-connecting-ip` with Cloudflare's published IP ranges, `x-real-ip`
132		−for nginx (`proxy_set_header X-Real-IP $remote_addr;`), or `x-forwarded-for`
133		−for Caddy. A malformed entry refuses to boot rather than silently pooling every
	133	+it. List only the proxy's own addresses (its IP, or the compose network it
	134	+shares with Executor, such as `172.18.0.0/24`), never a range that also
	135	+contains your users: every listed address counts as a proxy hop, and if every
	136	+hop is trusted Better Auth finds no client and falls back to one shared bucket.
	137	+Use `cf-connecting-ip` with Cloudflare's published IP ranges, `x-real-ip` for
	138	+nginx (`proxy_set_header X-Real-IP $remote_addr;`), or `x-forwarded-for` for
	139	+Caddy. A malformed entry refuses to boot rather than silently pooling every
134	140	user into one bucket.
135	141
136		−Without this, an instance behind a proxy shares one sign-in bucket across all
137		−users. Docker Desktop and Docker's userland proxy also present every connection
138		−from one internal address; put a reverse proxy in front and configure it as
139		−above.
	142	+Executor never trusts `x-forwarded-for` or a similar header on its own, because
	143	+any client could set it. When an auth request carries one of those headers and
	144	+no trusted proxy is configured, the server logs one warning naming these two
	145	+variables.
	146	+
	147	+On Linux, Docker's default iptables NAT preserves the client IP on a published
	148	+port, so a container reached directly needs nothing. Docker Desktop (macOS and
	149	+Windows) routes published ports through Docker's userland proxy, as does
	150	+loopback traffic from the Docker host itself, so every connection arrives from
	151	+one internal address; there, put a reverse proxy on the same Docker network in
	152	+front of Executor and configure it as above.
140	153
141	154	## Connect an agent
142	155

apps/host-selfhost/.env.example+8−4

@@ -42,11 +42,15 @@ · 41 unmodified lines
42	42	#
43	43	# Behind a reverse proxy (Caddy, nginx, Cloudflare, ...) every connection comes
44	44	# from the proxy, so name the header the proxy sets to the real client IP and
45		−# the addresses the proxy connects from. Set BOTH; the header is only honoured
46		−# on connections from those addresses, so it cannot be spoofed by a client that
47		−# reaches the container directly.
	45	+# the addresses the proxy connects from. Set BOTH, otherwise all users share
	46	+# one sign-in bucket. The header is only honoured on connections from those
	47	+# addresses, so it cannot be spoofed by a client that reaches the container
	48	+# directly. List only the proxy's own addresses (its IP, or the compose network
	49	+# it shares with Executor, e.g. 172.18.0.0/24), never a range that also
	50	+# contains your users: if every hop is trusted no client IP is found and
	51	+# everyone shares one bucket again.
48	52	# EXECUTOR_TRUSTED_PROXY_HEADER=x-real-ip
49		−# EXECUTOR_TRUSTED_PROXIES=10.0.0.0/8,172.16.0.0/12
	53	+# EXECUTOR_TRUSTED_PROXIES=172.18.0.2
50	54	#
51	55	# Set the exact string "true" only when something in front of Executor
52	56	# rate-limits sign-ins instead.

apps/host-selfhost/src/auth/better-auth.ts+2−5

@@ -14,7 +14,7 @@ import { LibsqlDialect, type LibsqlDialectConfig } from "@libsql/kysely-libsql"; · 13 unmodified lines
14	14	import { Context } from "effect";
15	15
16	16	import { loadConfig } from "../config";
17		−import { clientIpHeaders } from "./client-ip";
	17	+import { clientIpAddressOptions } from "./client-ip";
18	18	import { seedOrgAndAdmin } from "./seed";
19	19	import { consumeInviteCode, ensureInviteCodeTable, findRedeemableCode } from "./invites";
20	20	import { isAdmitted, isOAuthCallback, ssoProviderConfig } from "./sso";
@@ -139,10 +139,7 @@ const makeAuthOptions = (client: Client, getOrganizationId: () => string, gate?: · 118 unmodified lines
139	139	// `trustedProxies` lets Better Auth strip known hops from a forwarded
140	140	// chain; the stamper has already dropped the proxy header on any
141	141	// connection that did not come from one of those addresses.
142		− ipAddress: {
143		− ipAddressHeaders: clientIpHeaders(config.trustedProxy),
144		− ...(config.trustedProxy ? { trustedProxies: [...config.trustedProxy.proxies] } : {}),
145		− },
	142	+ ipAddress: clientIpAddressOptions(config.trustedProxy),
146	143	},
147	144	// Better Auth's own limiter is on in production and off in development.
148	145	// Only an explicit opt-out is passed through, so that environment default

apps/host-selfhost/src/auth/client-ip.test.ts+71−10

@@ -1,7 +1,13 @@
1	1	import { describe, expect, it } from "@effect/vitest";
2	2	import { Option } from "effect";
3	3
4		−import { CLIENT_IP_HEADER, clientIpHeaders, makeClientIpStamper, parseIpRange } from "./client-ip";
	4	+import {
	5	+ CLIENT_IP_HEADER,
	6	+ PROXY_HINT_HEADERS,
	7	+ clientIpAddressOptions,
	8	+ makeClientIpStamper,
	9	+ parseIpRange,
	10	+} from "./client-ip";
5	11
6	12	const signIn = (headers: Record<string, string> = {}) =>
7	13	new Request("https://host.example/api/auth/sign-in/email", {
@@ -41,21 +47,26 @@ describe("parseIpRange", () => { · 33 unmodified lines
41	47	);
42	48	});
43	49
44		−describe("clientIpHeaders", () => {
45		− it("reads only the stamped header with no proxy configured", () => {
46		− expect(clientIpHeaders(undefined)).toEqual([CLIENT_IP_HEADER]);
	50	+describe("clientIpAddressOptions", () => {
	51	+ it("reads only the stamped header, with no trusted proxies, when no proxy is configured", () => {
	52	+ expect(clientIpAddressOptions(undefined)).toStrictEqual({
	53	+ ipAddressHeaders: [CLIENT_IP_HEADER],
	54	+ });
47	55	});
48	56
49		− it("reads the proxy header first, then the stamped header", () => {
50		− expect(clientIpHeaders({ header: "cf-connecting-ip", proxies: ["10.0.0.0/8"] })).toEqual([
51		− "cf-connecting-ip",
52		− CLIENT_IP_HEADER,
53		− ]);
	57	+ it("reads the proxy header first, then the stamped header, and trusts the proxy ranges", () => {
	58	+ expect(
	59	+ clientIpAddressOptions({ header: "cf-connecting-ip", proxies: ["10.0.0.0/8", "192.0.2.10"] }),
	60	+ ).toStrictEqual({
	61	+ ipAddressHeaders: ["cf-connecting-ip", CLIENT_IP_HEADER],
	62	+ trustedProxies: ["10.0.0.0/8", "192.0.2.10"],
	63	+ });
54	64	});
55	65	});
56	66
57	67	describe("makeClientIpStamper without a trusted proxy", () => {
58		− const stamp = makeClientIpStamper(undefined);
	68	+ // The warning path has its own suite below; keep it out of this one's output.
	69	+ const stamp = makeClientIpStamper(undefined, { warn: () => {} });
59	70
60	71	it("overwrites a client-supplied header with the socket peer address", async () => {
61	72	const out = stamp(signIn({ [CLIENT_IP_HEADER]: "203.0.113.9" }), Option.some("198.51.100.4"));
@@ -118,3 +129,53 @@ describe("makeClientIpStamper behind a trusted proxy", () => { · 56 unmodified lines
118	129	expect(out.headers.has(CLIENT_IP_HEADER)).toBe(false);
119	130	});
120	131	});
	132	+
	133	+describe("unconfigured-proxy warning", () => {
	134	+ const stamperWithWarnings = (trustedProxy?: { header: string; proxies: string[] }) => {
	135	+ const warnings: string[] = [];
	136	+ const stamp = makeClientIpStamper(trustedProxy, {
	137	+ warn: (message) => {
	138	+ warnings.push(message);
	139	+ },
	140	+ });
	141	+ return { stamp, warnings };
	142	+ };
	143	+
	144	+ it.each(PROXY_HINT_HEADERS)(
	145	+ "fires once for %s with no trusted proxy configured, naming both variables",
	146	+ (header) => {
	147	+ const { stamp, warnings } = stamperWithWarnings();
	148	+ stamp(signIn({ [header]: "203.0.113.9" }), Option.some("172.18.0.2"));
	149	+ expect(warnings).toHaveLength(1);
	150	+ expect(warnings[0]).toContain(header);
	151	+ expect(warnings[0]).toContain("EXECUTOR_TRUSTED_PROXY_HEADER");
	152	+ expect(warnings[0]).toContain("EXECUTOR_TRUSTED_PROXIES");
	153	+
	154	+ stamp(signIn({ [header]: "203.0.113.9" }), Option.some("172.18.0.2"));
	155	+ stamp(signIn({ "x-forwarded-for": "203.0.113.10" }), Option.some("172.18.0.2"));
	156	+ expect(warnings).toHaveLength(1);
	157	+ },
	158	+ );
	159	+
	160	+ it("does not fire for a request without a proxy-style header", () => {
	161	+ const { stamp, warnings } = stamperWithWarnings();
	162	+ stamp(signIn(), Option.some("198.51.100.4"));
	163	+ stamp(signIn({ [CLIENT_IP_HEADER]: "203.0.113.9" }), Option.some("198.51.100.4"));
	164	+ expect(warnings).toHaveLength(0);
	165	+ });
	166	+
	167	+ it("does not fire when a trusted proxy is configured", () => {
	168	+ const { stamp, warnings } = stamperWithWarnings({
	169	+ header: "x-real-ip",
	170	+ proxies: ["172.18.0.2"],
	171	+ });
	172	+ // From the proxy, with the configured header plus the extras nginx sends.
	173	+ stamp(
	174	+ signIn({ "x-real-ip": "203.0.113.9", "x-forwarded-for": "203.0.113.9" }),
	175	+ Option.some("172.18.0.2"),
	176	+ );
	177	+ // Direct from an untrusted peer asserting a proxy header.
	178	+ stamp(signIn({ "x-forwarded-for": "203.0.113.9" }), Option.some("198.51.100.4"));
	179	+ expect(warnings).toHaveLength(0);
	180	+ });
	181	+});

apps/host-selfhost/src/auth/client-ip.ts+64−7

@@ -20,11 +20,28 @@ import type { TrustedProxyConfig } from "../config"; · 19 unmodified lines
20	20	// it connects from (EXECUTOR_TRUSTED_PROXIES). That header is honoured only
21	21	// on connections from one of those addresses and stripped otherwise, so a
22	22	// client that reaches the container directly cannot assert a proxy header.
	23	+//
	24	+// Better Auth's own default reads `x-forwarded-for` from anyone. That is not
	25	+// restored here: a client could rotate the header to dodge the limit. Behind
	26	+// an unconfigured proxy every user therefore shares one bucket, so the stamper
	27	+// warns the operator once when it sees a proxy-style header in that state.
23	28	// ---------------------------------------------------------------------------
24	29
25	30	/** Server-stamped socket peer address. Never trusted from the client. */
26	31	export const CLIENT_IP_HEADER = "x-executor-client-ip";
27	32
	33	+/**
	34	+ * Headers a reverse proxy commonly sets to the client IP. Seeing one with no
	35	+ * trusted proxy configured is the signature of a proxied deployment that has
	36	+ * not told Executor about its proxy.
	37	+ */
	38	+export const PROXY_HINT_HEADERS = [
	39	+ "x-forwarded-for",
	40	+ "x-real-ip",
	41	+ "cf-connecting-ip",
	42	+ "true-client-ip",
	43	+] as const;
	44	+
28	45	export interface IpRange {
29	46	readonly address: string;
30	47	readonly prefix: number;
@@ -64,13 +81,39 @@ const isTrustedPeer = (list: BlockList, address: string): boolean => { · 33 unmodified lines
64	81	return version !== 0 && list.check(address, version === 6 ? "ipv6" : "ipv4");
65	82	};
66	83
67		−/**
68		− * Header names Better Auth walks, in order, to find the client IP. The proxy's
69		− * header leads when one is configured; the server-stamped socket address is the
70		− * fallback for requests that did not come through the proxy.
71		− */
72		−export const clientIpHeaders = (trustedProxy: TrustedProxyConfig \| undefined): string[] =>
73		− trustedProxy ? [trustedProxy.header, CLIENT_IP_HEADER] : [CLIENT_IP_HEADER];
	84	+/** The `advanced.ipAddress` block handed to Better Auth. */
	85	+export interface ClientIpAddressOptions {
	86	+ /**
	87	+ * Header names Better Auth walks, in order, to find the client IP. The
	88	+ * proxy's header leads when one is configured; the server-stamped socket
	89	+ * address is the fallback for requests that did not come through the proxy.
	90	+ */
	91	+ readonly ipAddressHeaders: string[];
	92	+ /**
	93	+ * Hops Better Auth strips from the right of a forwarded chain. Only present
	94	+ * with a configured proxy; when every hop in a header is trusted, Better
	95	+ * Auth finds no client in it and moves to the next header.
	96	+ */
	97	+ readonly trustedProxies?: string[];
	98	+}
	99	+
	100	+export const clientIpAddressOptions = (
	101	+ trustedProxy: TrustedProxyConfig \| undefined,
	102	+): ClientIpAddressOptions =>
	103	+ trustedProxy
	104	+ ? {
	105	+ ipAddressHeaders: [trustedProxy.header, CLIENT_IP_HEADER],
	106	+ trustedProxies: [...trustedProxy.proxies],
	107	+ }
	108	+ : { ipAddressHeaders: [CLIENT_IP_HEADER] };
	109	+
	110	+export interface ClientIpStamperOptions {
	111	+ /** Where the one-time unconfigured-proxy warning goes. Defaults to console.warn. */
	112	+ readonly warn?: (message: string) => void;
	113	+}
	114	+
	115	+const unconfiguredProxyWarning = (header: string): string =>
	116	+ `[executor] An auth request carried ${header}, but no trusted proxy is configured, so every user behind that proxy shares one sign-in rate-limit bucket. Set EXECUTOR_TRUSTED_PROXY_HEADER and EXECUTOR_TRUSTED_PROXIES so the limit keys on the real client IP.`;
74	117
75	118	/**
76	119	* Build the per-request rewrite that stamps the socket peer address onto
@@ -78,13 +121,20 @@ export const clientIpHeaders = (trustedProxy: TrustedProxyConfig \| undefined): s · 1 unmodified line
78	121	* drops the trusted-proxy header unless the peer is a configured proxy. The
79	122	* `remoteAddress` is the TCP peer as reported by the HTTP server, not anything
80	123	* read from the request.
	124	+ *
	125	+ * With no trusted proxy configured, the first request that carries one of
	126	+ * PROXY_HINT_HEADERS triggers a single operator warning for the stamper's
	127	+ * lifetime (one stamper per server).
81	128	*/
82	129	export const makeClientIpStamper = (
83	130	trustedProxy: TrustedProxyConfig \| undefined,
	131	+ options: ClientIpStamperOptions = {},
84	132	): ((request: Request, remoteAddress: Option.Option<string>) => Request) => {
85	133	const proxies = trustedProxy
86	134	? blockListOf(trustedProxy.proxies.flatMap((entry) => parseIpRange(entry) ?? []))
87	135	: undefined;
	136	+ const warn = options.warn ?? ((message: string) => console.warn(message));
	137	+ let warnedUnconfiguredProxy = false;
88	138	return (request, remoteAddress) => {
89	139	const headers = new Headers(request.headers);
90	140	const peer = Option.getOrUndefined(remoteAddress);
@@ -96,6 +146,13 @@ export const makeClientIpStamper = ( · 5 unmodified lines
96	146	if (trustedProxy && proxies && !(peer && isTrustedPeer(proxies, peer))) {
97	147	headers.delete(trustedProxy.header);
98	148	}
	149	+ if (!trustedProxy && !warnedUnconfiguredProxy) {
	150	+ const hint = PROXY_HINT_HEADERS.find((header) => headers.has(header));
	151	+ if (hint) {
	152	+ warnedUnconfiguredProxy = true;
	153	+ warn(unconfiguredProxyWarning(hint));
	154	+ }
	155	+ }
99	156	return new Request(request, { headers });
100	157	};
101	158	};

apps/host-selfhost/src/config.ts+1−1

@@ -323,7 +323,7 @@ const resolveTrustedProxy = (): TrustedProxyConfig \| undefined => { · 322 unmodified lines
323	323	if (invalid.length > 0) {
324	324	// oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: refuse to boot on a malformed operator knob
325	325	throw new Error(
326		− `EXECUTOR_TRUSTED_PROXIES contains ${invalid.map((entry) => JSON.stringify(entry)).join(", ")}; each entry must be an IP address or CIDR range (e.g. "10.0.0.0/8")`,
	326	+ `EXECUTOR_TRUSTED_PROXIES contains ${invalid.map((entry) => JSON.stringify(entry)).join(", ")}; each entry must be an IP address or CIDR range (e.g. "172.18.0.2" or "172.18.0.0/24")`,
327	327	);
328	328	}
329	329	return { header, proxies };