From ecc3e61a3d8921650c47c03e21a1ecc8ca4cf872 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:30:44 -0700 Subject: [PATCH 1/3] CL-6494: raise the sign-in rate limit and give it a human-readable message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit better-auth's built-in special rule for /sign-in* is 3 attempts per 10 seconds. When it can't resolve a trustworthy client IP, every signed-out visitor on that path shares one bucket, so one person mistyping a password can lock out everyone else signing in — including, in practice, the owner locking themselves out. Raises the sign-in rate limit to 10 attempts per 60 seconds (SIGNIN_RATE_LIMIT_WINDOW_SECONDS/MAX), applied the same in every environment rather than branched on NODE_ENV, consistent with this file's existing rejection of NODE_ENV-inferred auth behavior. This is safe regardless of how client-IP resolution lands: it relieves the immediate lockout on its own by giving real users, and a lone local developer sharing an unresolved-IP bucket, more headroom to retry before hitting the wall. Also gives a rate-limited sign-in/sign-up/social response a human-readable message naming what happened and when to retry, replacing better-auth's bare "Too many requests" body. --- .env.example | 8 +++ apps/hub/src/config.ts | 30 +++++++++++ apps/hub/src/index.ts | 9 ++++ apps/hub/test/chat-mount.test.ts | 1 + apps/hub/test/composition.test.ts | 69 +++++++++++++++++++++++++ apps/hub/test/config.test.ts | 14 +++++ apps/hub/test/credential-cipher.test.ts | 1 + apps/hub/test/eval-runs-mount.test.ts | 1 + apps/hub/test/presence-mount.test.ts | 1 + apps/hub/test/routine-mount.test.ts | 1 + apps/hub/test/slack-tag-mount.test.ts | 1 + apps/web/src/session.test.ts | 67 ++++++++++++++++++++++++ apps/web/src/session.ts | 20 +++++++ 13 files changed, 223 insertions(+) create mode 100644 apps/web/src/session.test.ts diff --git a/.env.example b/.env.example index d445f9e2d..d2d784c28 100644 --- a/.env.example +++ b/.env.example @@ -68,6 +68,14 @@ HUB_STATIC_DIR=../web/dist # SIGNUP_RATE_LIMIT_WINDOW_SECONDS=60 # SIGNUP_RATE_LIMIT_MAX=5 +# Per-IP rate limit on email sign-in. Defaults to 10 attempts per 60 +# seconds when unset — deliberately looser than better-auth's built-in +# 3-per-10-seconds default, which is tight enough that one shared bucket +# (a client IP that can't be resolved) locks out every signed-out +# visitor, local dev included. See CL-6494. +# SIGNIN_RATE_LIMIT_WINDOW_SECONDS=60 +# SIGNIN_RATE_LIMIT_MAX=10 + # Self-serve signup mode. Hub default is closed (owner adds users or # shares a copy-link invite). Local `bun run dev` opens signup when this # is unset so the first admin can seed — set closed explicitly to test diff --git a/apps/hub/src/config.ts b/apps/hub/src/config.ts index d12270ba9..a5b0a1b2b 100644 --- a/apps/hub/src/config.ts +++ b/apps/hub/src/config.ts @@ -76,6 +76,12 @@ const HubEnv = type({ "SIGNUP_RATE_LIMIT_MAX?": type(/^[1-9]\d*$/).describe( "the maximum sign-ups a single IP may make per window, e.g. 5", ), + "SIGNIN_RATE_LIMIT_WINDOW_SECONDS?": type(/^[1-9]\d*$/).describe( + "the per-IP sign-in rate-limit window, in seconds, e.g. 60; overrides better-auth's built-in 10-second/3-attempt default, which is too tight for a person retyping a password", + ), + "SIGNIN_RATE_LIMIT_MAX?": type(/^[1-9]\d*$/).describe( + "the maximum sign-in attempts a single IP may make per window, e.g. 10", + ), "WORKBENCH_SIGNUP?": type("'open' | 'closed'").describe( "open = self-serve email signup allowed; closed (default) = owner adds users or copy-link invite only", ), @@ -194,6 +200,18 @@ const HubEnv = type({ const DEFAULT_SIGNUP_RATE_LIMIT_WINDOW_SECONDS = 60; const DEFAULT_SIGNUP_RATE_LIMIT_MAX = 5; +// better-auth's own built-in special rule for /sign-in* is 3 attempts per +// 10 seconds, keyed per client IP. That's the bucket the shared-IP-fallback +// bug (CL-6494) starves: when the IP can't be resolved every signed-out +// visitor -- in production, or the one developer on a local box -- shares +// it. Raising it here, applied the same in every environment rather than +// branched on NODE_ENV (this file already rejects inferring auth behavior +// from NODE_ENV — see `rateLimit.enabled` below), gives real users room to +// mistype a password and gives a lone local developer room to keep working +// even while the IP genuinely can't be resolved, without loosening +// production brute-force resistance in any meaningful way. +const DEFAULT_SIGNIN_RATE_LIMIT_WINDOW_SECONDS = 60; +const DEFAULT_SIGNIN_RATE_LIMIT_MAX = 10; /** * Production default for `WORKBENCH_CHAT_IDLE_REAP_MS`: 30 minutes, @@ -293,6 +311,10 @@ export type HubConfig = { readonly windowSeconds: number; readonly max: number; }; + readonly signInRateLimit: { + readonly windowSeconds: number; + readonly max: number; + }; /** Self-serve signup. Default closed — see docs/TENANCY.md. */ readonly signupMode: "open" | "closed"; /** Domains allowed when signupMode is open. Empty = any domain. */ @@ -599,6 +621,14 @@ export function readHubConfig( ? Number(parsed.SIGNUP_RATE_LIMIT_MAX) : DEFAULT_SIGNUP_RATE_LIMIT_MAX, }, + signInRateLimit: { + windowSeconds: parsed.SIGNIN_RATE_LIMIT_WINDOW_SECONDS + ? Number(parsed.SIGNIN_RATE_LIMIT_WINDOW_SECONDS) + : DEFAULT_SIGNIN_RATE_LIMIT_WINDOW_SECONDS, + max: parsed.SIGNIN_RATE_LIMIT_MAX + ? Number(parsed.SIGNIN_RATE_LIMIT_MAX) + : DEFAULT_SIGNIN_RATE_LIMIT_MAX, + }, envProviderKeys: envProviderKeysFrom(parsed), envProviderBaseUrls: envProviderBaseUrlsFrom(parsed), envCredentialPlantAdmin: { diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index e70fed21d..a170b205b 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -372,6 +372,7 @@ const MAX_TARBALL_BYTES = 10 * 1024 * 1024; // carry an unpublished scope anyway). const TENANT_PREFIX = "/api/tenants/:tenantId"; const SIGN_UP_EMAIL_PATH = "/sign-up/email"; +const SIGN_IN_EMAIL_PATH = "/sign-in/email"; // Chat residents carry a real hub-driven idle-reap again (reversing // CL-5477's removal): the sidecar's own park/wake scheme it was meant to // replace has itself been retired in favor of a simpler reap-and-relaunch @@ -564,6 +565,14 @@ export async function createHub(config: HubConfig) { window: config.signupRateLimit.windowSeconds, max: config.signupRateLimit.max, }, + // Overrides better-auth's built-in special rule for /sign-in* + // (3 attempts / 10 seconds) — see `DEFAULT_SIGNIN_RATE_LIMIT_MAX`'s + // doc comment in config.ts for why this is raised the same way in + // every environment instead of a dev-only carve-out. + [SIGN_IN_EMAIL_PATH]: { + window: config.signInRateLimit.windowSeconds, + max: config.signInRateLimit.max, + }, }, }, // No mailer is wired up anywhere in this stack, so better-auth can diff --git a/apps/hub/test/chat-mount.test.ts b/apps/hub/test/chat-mount.test.ts index 00ce5e935..ff14af98a 100644 --- a/apps/hub/test/chat-mount.test.ts +++ b/apps/hub/test/chat-mount.test.ts @@ -29,6 +29,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], diff --git a/apps/hub/test/composition.test.ts b/apps/hub/test/composition.test.ts index 6fb87820b..c17d42402 100644 --- a/apps/hub/test/composition.test.ts +++ b/apps/hub/test/composition.test.ts @@ -31,6 +31,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], @@ -172,6 +173,74 @@ describeIfDb("extension mounting", () => { }); }); +describeIfDb("auth rate limiting resolves the client IP (CL-6494)", () => { + function signInAttempt( + hub: Awaited>, + ip: string, + ) { + return hub.app.request("/api/auth/sign-in/email", { + method: "POST", + headers: { "content-type": "application/json", "x-forwarded-for": ip }, + body: JSON.stringify({ + email: "nobody@example.com", + password: "wrong-password", + }), + }); + } + + test("two distinct client IPs get independent sign-in buckets", async () => { + const hub = await createHub({ + ...config, + signInRateLimit: { windowSeconds: 60, max: 2 }, + }); + closers.push(hub.close); + + // Exhausts IP A's bucket (max: 2). + await signInAttempt(hub, "203.0.113.10"); + await signInAttempt(hub, "203.0.113.10"); + const throttledA = await signInAttempt(hub, "203.0.113.10"); + expect(throttledA.status).toBe(429); + + // IP B's very first request is untouched by A's bucket. + const freshB = await signInAttempt(hub, "203.0.113.20"); + expect(freshB.status).not.toBe(429); + }); + + test("a rate-limited sign-in carries a retry hint and a human-readable message", async () => { + const hub = await createHub({ + ...config, + signInRateLimit: { windowSeconds: 60, max: 1 }, + }); + closers.push(hub.close); + + await signInAttempt(hub, "203.0.113.30"); + const throttled = await signInAttempt(hub, "203.0.113.30"); + + expect(throttled.status).toBe(429); + expect(throttled.headers.get("x-retry-after")).not.toBeNull(); + const body = (await throttled.json()) as { message: string }; + expect(body.message).toMatch(/\S/); + }); + + test("a resolvable x-forwarded-for means the shared-bucket boot warning never fires", async () => { + const warnSpy = spyOn(console, "warn"); + const hub = await createHub(config); + closers.push(hub.close); + + await signInAttempt(hub, "203.0.113.40"); + + const sharedBucketWarning = warnSpy.mock.calls.some((call) => + call.some( + (arg) => + typeof arg === "string" && + arg.includes("falling back to a single shared per-path bucket"), + ), + ); + expect(sharedBucketWarning).toBe(false); + warnSpy.mockRestore(); + }); +}); + describeIfDb("dev-mode email verification", () => { test("ALLOW_UNVERIFIED_EMAILS auto-verifies a fresh self-serve signup, so it never 403s on the unverified-email gate", async () => { const hub = await createHub({ diff --git a/apps/hub/test/config.test.ts b/apps/hub/test/config.test.ts index 2e6bf2e80..7e113ffda 100644 --- a/apps/hub/test/config.test.ts +++ b/apps/hub/test/config.test.ts @@ -32,6 +32,7 @@ describe("readHubConfig", () => { signupMode: "closed", allowedEmailDomains: [], signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, allowPlaintextSecrets: false, allowUnverifiedEmails: false, sidecarProvisioners: [], @@ -183,6 +184,19 @@ describe("readHubConfig", () => { expect(config.signupRateLimit).toEqual({ windowSeconds: 30, max: 2 }); }); + test("the sign-in rate limit is configurable and defaults well above better-auth's built-in 3-per-10-seconds special rule (CL-6494)", () => { + expect(readHubConfig(validEnv).signInRateLimit).toEqual({ + windowSeconds: 60, + max: 10, + }); + const config = readHubConfig({ + ...validEnv, + SIGNIN_RATE_LIMIT_WINDOW_SECONDS: "30", + SIGNIN_RATE_LIMIT_MAX: "2", + }); + expect(config.signInRateLimit).toEqual({ windowSeconds: 30, max: 2 }); + }); + test("WORKBENCH_SIGNUP defaults closed and accepts open", () => { expect(readHubConfig(validEnv).signupMode).toBe("closed"); expect( diff --git a/apps/hub/test/credential-cipher.test.ts b/apps/hub/test/credential-cipher.test.ts index 80531afbc..86e3aebb0 100644 --- a/apps/hub/test/credential-cipher.test.ts +++ b/apps/hub/test/credential-cipher.test.ts @@ -18,6 +18,7 @@ const baseConfig: HubConfig = { hubDataDir: ".data/hub", hubStaticDir: "apps/hub/public", signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], diff --git a/apps/hub/test/eval-runs-mount.test.ts b/apps/hub/test/eval-runs-mount.test.ts index 9cab3edb0..8ce304fd5 100644 --- a/apps/hub/test/eval-runs-mount.test.ts +++ b/apps/hub/test/eval-runs-mount.test.ts @@ -30,6 +30,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], diff --git a/apps/hub/test/presence-mount.test.ts b/apps/hub/test/presence-mount.test.ts index bb594479c..b20f8b55c 100644 --- a/apps/hub/test/presence-mount.test.ts +++ b/apps/hub/test/presence-mount.test.ts @@ -29,6 +29,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], diff --git a/apps/hub/test/routine-mount.test.ts b/apps/hub/test/routine-mount.test.ts index a5bd3afdb..cce307862 100644 --- a/apps/hub/test/routine-mount.test.ts +++ b/apps/hub/test/routine-mount.test.ts @@ -30,6 +30,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, signupMode: "closed", allowedEmailDomains: [], diff --git a/apps/hub/test/slack-tag-mount.test.ts b/apps/hub/test/slack-tag-mount.test.ts index 3a1c1859a..bf30b49bf 100644 --- a/apps/hub/test/slack-tag-mount.test.ts +++ b/apps/hub/test/slack-tag-mount.test.ts @@ -40,6 +40,7 @@ const config: HubConfig = { hubDataDir: path.join(root, "data"), hubStaticDir: staticDir, signupRateLimit: { windowSeconds: 60, max: 5 }, + signInRateLimit: { windowSeconds: 60, max: 10 }, socialProviders: {}, allowUnverifiedEmails: true, sidecarProvisioners: [], diff --git a/apps/web/src/session.test.ts b/apps/web/src/session.test.ts new file mode 100644 index 000000000..8845032e8 --- /dev/null +++ b/apps/web/src/session.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { signIn, signInSocial, signUp } from "./session"; + +describe("rate-limited auth responses", () => { + const realFetch = globalThis.fetch; + + afterEach(() => { + globalThis.fetch = realFetch; + }); + + function stub429(retryAfterSeconds: string | null): void { + globalThis.fetch = ((_input: RequestInfo | URL, _init?: RequestInit) => + Promise.resolve( + new Response( + JSON.stringify({ + message: "Too many requests. Please try again later.", + }), + { + status: 429, + headers: { + "content-type": "application/json", + ...(retryAfterSeconds === null + ? {} + : { "X-Retry-After": retryAfterSeconds }), + }, + }, + ), + )) as typeof fetch; + } + + test("signIn surfaces the retry countdown from X-Retry-After in consumer language", async () => { + stub429("30"); + const result = await signIn("alice@example.com", "hunter2"); + expect(result).toEqual({ + ok: false, + message: "Too many sign-in attempts. Try again in 30 seconds.", + }); + }); + + test("signUp shares the same 429 handling as signIn — same underlying bucket", async () => { + stub429("5"); + const result = await signUp("alice@example.com", "hunter2"); + expect(result).toEqual({ + ok: false, + message: "Too many sign-in attempts. Try again in 5 seconds.", + }); + }); + + test("falls back to a generic wait message when no retry countdown is given", async () => { + stub429(null); + const result = await signIn("alice@example.com", "hunter2"); + expect(result).toEqual({ + ok: false, + message: "Too many sign-in attempts. Please wait a moment and try again.", + }); + }); + + test("signInSocial gets the same consumer-language 429 message", async () => { + stub429("12"); + const result = await signInSocial("github"); + expect(result).toEqual({ + ok: false, + message: "Too many sign-in attempts. Try again in 12 seconds.", + }); + }); +}); diff --git a/apps/web/src/session.ts b/apps/web/src/session.ts index d23f179be..38f188731 100644 --- a/apps/web/src/session.ts +++ b/apps/web/src/session.ts @@ -64,6 +64,24 @@ export type AuthResult = const FailureBody = type({ message: "string" }); +/** + * better-auth answers a rate-limited request with a bare 429 and an + * `X-Retry-After` header (seconds). Every auth entry point in this file + * shares this so sign-in, sign-up, and social sign-in all tell the person + * what happened and when it's worth trying again, instead of surfacing + * better-auth's generic "Too many requests" body. + */ +function rateLimitedResult(response: Response): AuthResult { + const retryAfterSeconds = Number(response.headers.get("x-retry-after")); + return { + ok: false, + message: + Number.isFinite(retryAfterSeconds) && retryAfterSeconds > 0 + ? `Too many sign-in attempts. Try again in ${retryAfterSeconds} second${retryAfterSeconds === 1 ? "" : "s"}.` + : "Too many sign-in attempts. Please wait a moment and try again.", + }; +} + async function postAuth( path: string, body: Record, @@ -74,6 +92,7 @@ async function postAuth( headers: { "content-type": "application/json" }, body: JSON.stringify(body), }); + if (response.status === 429) return rateLimitedResult(response); const payload: unknown = await response.json().catch(() => null); if (!response.ok) { const failure = FailureBody(payload); @@ -186,6 +205,7 @@ export async function signInSocial( callbackURL: window.location.origin, }), }); + if (response.status === 429) return rateLimitedResult(response); const payload: unknown = await response.json().catch(() => null); if (!response.ok) { const failure = FailureBody(payload); From ba02600eb979b6f868cda9d866bc2631d104fbd6 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:32:26 -0700 Subject: [PATCH 2/3] CL-6494: key sign-in rate limiting on the target account, not client IP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Peer review of the sign-in rate limit correctly blocked the IP-trust half of this change on two grounds: 1. Railway's docs (specs-and-limits) list X-Real-IP as the header its edge sets for the client's address — not x-forwarded-for, which the previous comment here asserted without a source. That claim was unverified and likely wrong, which would have made the client-IP config a no-op in production. 2. More seriously: better-auth's getIPFromHeader trusts a single-value IP header verbatim whenever no trustedProxies is configured, and Railway's private networking lets any same-project service — this deployment's agent-driven sidecars included — reach this hub directly at .railway.internal, bypassing the edge entirely. A caller on that network can send a fresh forged IP header on every request and get an independent rate-limit bucket each time, a complete bypass of sign-in brute-force protection from inside our own trust boundary. Swapping in X-Real-IP would only fix finding 1 while leaving this wide open, since that header is exactly as forgeable over the internal network. better-auth has no extension point that reaches this: `customRules` can only override a matched path's window/max (or opt it out via `false`) — it never sees or changes the rate-limit key — and `customStorage` only ever receives the already-computed `ip|path` key. Neither can key on the request body. So sign-in enforcement now lives entirely in a small dedicated limiter (sign-in-rate-limit.ts) keyed on the normalized target email, with `customRules["/sign-in/email"]` set to `false` to fully disable better-auth's own IP-keyed rule for this path rather than run a second, weaker mechanism beside it. An attacker rotating IP headers still cannot exceed the budget for the one account they're actually attacking, which is the threat brute-force limiting exists to stop; client IP is deliberately not composed into the key, since this deployment has no way to tell an edge-forwarded request from a forged one, and folding an untrustworthy IP in would only let the same forged-header trick defeat this limiter too. Guards against becoming a way to lock a known user out of their own account: window and count stay short/generous (60s / 10, config.signInRateLimit), so a forced lockout self-heals within the window and a real user mistyping a password is unaffected. Upstream note for better-auth: a `customRules` (or pre-consume) hook that can see the parsed request body, or override the key itself, would let this be expressed natively instead of living beside the built-in limiter. X-Real-IP replaces the removed x-forwarded-for config for what it still legitimately helps with: sign-up's coarse, closed-by-default throttling, where the same private-network gap is a low-stakes, documented tradeoff rather than a brute-force bypass. --- .env.example | 11 +- apps/hub/src/index.ts | 73 ++++++++++-- apps/hub/src/sign-in-rate-limit.test.ts | 59 ++++++++++ apps/hub/src/sign-in-rate-limit.ts | 108 ++++++++++++++++++ apps/hub/test/composition.test.ts | 144 ++++++++++++++---------- 5 files changed, 320 insertions(+), 75 deletions(-) create mode 100644 apps/hub/src/sign-in-rate-limit.test.ts create mode 100644 apps/hub/src/sign-in-rate-limit.ts diff --git a/.env.example b/.env.example index d2d784c28..df8159bbb 100644 --- a/.env.example +++ b/.env.example @@ -68,11 +68,12 @@ HUB_STATIC_DIR=../web/dist # SIGNUP_RATE_LIMIT_WINDOW_SECONDS=60 # SIGNUP_RATE_LIMIT_MAX=5 -# Per-IP rate limit on email sign-in. Defaults to 10 attempts per 60 -# seconds when unset — deliberately looser than better-auth's built-in -# 3-per-10-seconds default, which is tight enough that one shared bucket -# (a client IP that can't be resolved) locks out every signed-out -# visitor, local dev included. See CL-6494. +# Per-account rate limit on email sign-in, keyed on the target email +# rather than client IP (client IP can't be trusted as a sign-in key in +# this deployment — see sign-in-rate-limit.ts). Defaults to 10 attempts +# per 60 seconds when unset — deliberately looser than better-auth's +# built-in 3-per-10-seconds default, which is tight enough that a mistyped +# password can lock an account out mid-window. See CL-6494. # SIGNIN_RATE_LIMIT_WINDOW_SECONDS=60 # SIGNIN_RATE_LIMIT_MAX=10 diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index a170b205b..d8082ed70 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -327,6 +327,7 @@ import { import { betterAuth } from "better-auth"; import { createBenchSessionMinter } from "./bench-session"; +import { createSignInAttemptLimiter } from "./sign-in-rate-limit"; import { drizzleAdapter } from "better-auth/adapters/drizzle"; import { type Context, Hono, type Next } from "hono"; @@ -554,6 +555,25 @@ export async function createHub(config: HubConfig) { database: drizzleAdapter(db, { provider: "pg" }), emailAndPassword: { enabled: true }, socialProviders: config.socialProviders, + // Client-IP resolution for the sign-up rate limit below. Railway's docs + // (docs.railway.com/networking/public-networking/specs-and-limits) list + // `X-Real-IP` as the header its edge sets for the client's address — + // that's the only claim about it this codebase can actually stand + // behind. It is deliberately NOT relied on for sign-in: Railway's + // private networking lets any same-project service (sidecars included) + // reach this hub directly, bypassing the edge, and with no + // `trustedProxies` configured (Railway publishes no stable edge CIDR + // list to populate one with) a single-value header is trusted verbatim + // regardless of who set it. That's an acceptable, low-stakes gap for + // sign-up's coarse throttling — a closed-by-default, operator-gated + // path — but not for brute-force resistance on sign-in, which is why + // sign-in has its own account-keyed limiter instead (see + // `sign-in-rate-limit.ts`). + advanced: { + ipAddress: { + ipAddressHeaders: ["x-real-ip"], + }, + }, rateLimit: { // Explicit and always on: better-auth's own default only enables // this in production (`enabled ?? isProduction`), which would @@ -565,14 +585,13 @@ export async function createHub(config: HubConfig) { window: config.signupRateLimit.windowSeconds, max: config.signupRateLimit.max, }, - // Overrides better-auth's built-in special rule for /sign-in* - // (3 attempts / 10 seconds) — see `DEFAULT_SIGNIN_RATE_LIMIT_MAX`'s - // doc comment in config.ts for why this is raised the same way in - // every environment instead of a dev-only carve-out. - [SIGN_IN_EMAIL_PATH]: { - window: config.signInRateLimit.windowSeconds, - max: config.signInRateLimit.max, - }, + // `false` fully disables better-auth's own built-in special rule + // for /sign-in* (3 attempts / 10 seconds, keyed on the client IP + // above) rather than leaving it running in parallel as a second, + // weaker mechanism: that IP key is exactly what CL-6494's + // private-network bypass defeats, so enforcement for this path + // lives entirely in `signInAttemptLimiter` below instead. + [SIGN_IN_EMAIL_PATH]: false, }, }, // No mailer is wired up anywhere in this stack, so better-auth can @@ -595,6 +614,13 @@ export async function createHub(config: HubConfig) { } : undefined, }); + // Account-keyed sign-in rate limit (CL-6494) — see `sign-in-rate-limit.ts` + // for why this replaces better-auth's own IP-keyed sign-in enforcement + // entirely rather than composing with it. + const signInAttemptLimiter = createSignInAttemptLimiter( + config.signInRateLimit.windowSeconds, + config.signInRateLimit.max, + ); const { signingKey, agentRepoStore, assetService } = await createBootAssetWiring({ db, dataDir: config.hubDataDir }); const baseLookups = createHubSessionLookups({ db, agentRepoStore }); @@ -986,6 +1012,37 @@ export async function createHub(config: HubConfig) { } } } + // Account-keyed sign-in brute-force protection (CL-6494) — see + // `sign-in-rate-limit.ts` for why this fully replaces better-auth's + // own IP-keyed enforcement for this path instead of running beside + // it. + if (c.req.method === "POST" && c.req.path.endsWith(SIGN_IN_EMAIL_PATH)) { + let email = ""; + try { + const body: unknown = await c.req.raw.clone().json(); + if ( + body !== null && + typeof body === "object" && + "email" in body && + typeof (body as { email: unknown }).email === "string" + ) { + email = (body as { email: string }).email; + } + } catch { + email = ""; + } + const decision = signInAttemptLimiter.consume(email); + if (!decision.allowed) { + return c.json( + { + error: "rate_limited", + message: `Too many sign-in attempts. Try again in ${decision.retryAfterSeconds} second${decision.retryAfterSeconds === 1 ? "" : "s"}.`, + }, + 429, + { "X-Retry-After": decision.retryAfterSeconds.toString() }, + ); + } + } return auth.handler(c.req.raw); }, db, diff --git a/apps/hub/src/sign-in-rate-limit.test.ts b/apps/hub/src/sign-in-rate-limit.test.ts new file mode 100644 index 000000000..3b998f6fb --- /dev/null +++ b/apps/hub/src/sign-in-rate-limit.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from "bun:test"; +import { createSignInAttemptLimiter } from "./sign-in-rate-limit.ts"; + +describe("createSignInAttemptLimiter", () => { + test("the Nth attempt against one account past the configured max is rejected", () => { + const limiter = createSignInAttemptLimiter(60, 2); + + expect(limiter.consume("victim@example.com").allowed).toBe(true); + expect(limiter.consume("victim@example.com").allowed).toBe(true); + const throttled = limiter.consume("victim@example.com"); + + expect(throttled.allowed).toBe(false); + }); + + test("a rejected attempt reports how many seconds remain in the window", () => { + const limiter = createSignInAttemptLimiter(60, 1); + + limiter.consume("victim@example.com"); + const throttled = limiter.consume("victim@example.com"); + + expect(throttled.allowed).toBe(false); + if (!throttled.allowed) { + expect(throttled.retryAfterSeconds).toBeGreaterThan(0); + expect(throttled.retryAfterSeconds).toBeLessThanOrEqual(60); + } + }); + + test("rotating the caller-supplied identity per attempt does not grow the budget for the targeted account", () => { + // The whole point of keying on the account instead of client IP: + // a caller that varies some other, attacker-chosen value per request + // (a forged IP header, in production) still can't outrun the budget + // for the one email it's actually attacking, because the key is the + // email — nothing about a rotated header changes it. + const limiter = createSignInAttemptLimiter(60, 3); + + expect(limiter.consume("victim@example.com").allowed).toBe(true); + expect(limiter.consume("victim@example.com").allowed).toBe(true); + expect(limiter.consume("victim@example.com").allowed).toBe(true); + expect(limiter.consume("victim@example.com").allowed).toBe(false); + expect(limiter.consume("victim@example.com").allowed).toBe(false); + }); + + test("two distinct accounts get independent budgets", () => { + const limiter = createSignInAttemptLimiter(60, 1); + + expect(limiter.consume("alice@example.com").allowed).toBe(true); + expect(limiter.consume("alice@example.com").allowed).toBe(false); + + // Bob's own budget is untouched by Alice's exhausted one. + expect(limiter.consume("bob@example.com").allowed).toBe(true); + }); + + test("email matching is case- and whitespace-insensitive, so it can't be sidestepped by casing/padding", () => { + const limiter = createSignInAttemptLimiter(60, 1); + + expect(limiter.consume("Victim@Example.com").allowed).toBe(true); + expect(limiter.consume(" victim@example.com ").allowed).toBe(false); + }); +}); diff --git a/apps/hub/src/sign-in-rate-limit.ts b/apps/hub/src/sign-in-rate-limit.ts new file mode 100644 index 000000000..d0e1ae785 --- /dev/null +++ b/apps/hub/src/sign-in-rate-limit.ts @@ -0,0 +1,108 @@ +// Account-keyed sign-in attempt limiter (CL-6494). +// +// better-auth's own rate limiter keys solely on client IP (falling back to +// one shared bucket when no IP resolves), configured via +// `advanced.ipAddress` + `rateLimit.customRules`. That key cannot be this +// deployment's sign-in defense: Railway's private networking lets any +// same-project service — including sidecars that run agent-driven shell +// commands — reach this hub directly at `.railway.internal`, +// bypassing Railway's edge entirely. better-auth's `getIPFromHeader` trusts +// a single-value IP header verbatim whenever no `trustedProxies` is +// configured, and Railway's anycast edge publishes no stable CIDR list to +// populate one with. A caller on the private network can therefore send a +// fresh forged IP header on every request and get an independent +// rate-limit bucket each time — a complete bypass of brute-force +// protection, reachable from inside our own trust boundary. +// +// better-auth has no native extension point that fixes this: `customRules` +// can only override a matched path's `window`/`max` (or opt the path out +// entirely via `false`) — it never gets to change the key. `customStorage` +// only ever receives the already-computed `ip|path` key. Neither sees the +// request body, so neither can key on anything but that IP. This limiter +// is therefore deliberately separate from better-auth's engine — +// `index.ts` sets `customRules["/sign-in/email"]` to `false`, fully +// disabling better-auth's native, IP-keyed enforcement for this one path, +// rather than attempting to bend its extension points to a job they don't +// reach. Upstream note: better-auth would need a `customRules` (or +// pre-consume) hook that can see the parsed request body, or override the +// key itself, to express account-keyed limiting natively. +// +// Keyed on the normalized target email instead: that value isn't +// attacker-chosen the way a header is. An attacker rotating IP headers +// still cannot exceed the budget for the one account they're actually +// trying to break into, which is the threat brute-force limiting exists to +// stop. Client IP is deliberately not composed into the key: this +// deployment has no way to tell an edge-forwarded request from one that +// arrived over the private network with a forged header, so a +// header-derived IP is not a genuinely trustworthy signal here, and +// folding it into the key would only let the same forged-header trick +// defeat this limiter too, exactly as it defeats better-auth's. Once +// Railway traffic can be verifiably split (a stable edge CIDR list, or +// private networking segregated away from `/api/auth`), an IP-composed +// *secondary* per-source budget could be layered on top of this one, to +// also blunt one source spraying many different accounts. +// +// Guards against the account key itself becoming a way to lock a known +// user out of their own account: the window is short and the count is +// generous (60s / 10 by default — `config.signInRateLimit`), so a lockout +// an attacker forces self-heals within the window and never compounds +// across windows, while a real user mistyping a password a few times in a +// row is never affected. + +const MAX_TRACKED_EMAILS = 50_000; + +export type SignInAttemptDecision = + | { readonly allowed: true } + | { readonly allowed: false; readonly retryAfterSeconds: number }; + +export type SignInAttemptLimiter = { + consume(email: string): SignInAttemptDecision; +}; + +function normalizeEmail(email: string): string { + return email.trim().toLowerCase(); +} + +export function createSignInAttemptLimiter( + windowSeconds: number, + max: number, +): SignInAttemptLimiter { + const windowMs = windowSeconds * 1000; + const buckets = new Map(); + + function pruneExpiredAndOverflow(now: number): void { + for (const [key, bucket] of buckets) { + if (now - bucket.windowStart >= windowMs) buckets.delete(key); + } + if (buckets.size <= MAX_TRACKED_EMAILS) return; + let overflow = buckets.size - MAX_TRACKED_EMAILS; + for (const key of buckets.keys()) { + if (overflow <= 0) break; + buckets.delete(key); + overflow -= 1; + } + } + + return { + consume(email: string): SignInAttemptDecision { + const now = Date.now(); + pruneExpiredAndOverflow(now); + const key = normalizeEmail(email); + const bucket = buckets.get(key); + if (!bucket || now - bucket.windowStart >= windowMs) { + buckets.set(key, { count: 1, windowStart: now }); + return { allowed: true }; + } + if (bucket.count >= max) { + return { + allowed: false, + retryAfterSeconds: Math.ceil( + (bucket.windowStart + windowMs - now) / 1000, + ), + }; + } + bucket.count += 1; + return { allowed: true }; + }, + }; +} diff --git a/apps/hub/test/composition.test.ts b/apps/hub/test/composition.test.ts index c17d42402..3b4ed00ab 100644 --- a/apps/hub/test/composition.test.ts +++ b/apps/hub/test/composition.test.ts @@ -173,73 +173,93 @@ describeIfDb("extension mounting", () => { }); }); -describeIfDb("auth rate limiting resolves the client IP (CL-6494)", () => { - function signInAttempt( - hub: Awaited>, - ip: string, - ) { - return hub.app.request("/api/auth/sign-in/email", { - method: "POST", - headers: { "content-type": "application/json", "x-forwarded-for": ip }, - body: JSON.stringify({ - email: "nobody@example.com", - password: "wrong-password", - }), +describeIfDb( + "sign-in rate limiting is keyed on the target account, not client IP (CL-6494)", + () => { + // A forged/rotating `x-forwarded-for` is exactly what a caller reaching + // this hub over Railway's private network (bypassing the edge) can + // send on every request — the header this suite deliberately varies + // per attempt below to prove it buys the attacker nothing. + function signInAttempt( + hub: Awaited>, + email: string, + forgedIp: string, + ) { + return hub.app.request("/api/auth/sign-in/email", { + method: "POST", + headers: { + "content-type": "application/json", + "x-forwarded-for": forgedIp, + }, + body: JSON.stringify({ email, password: "wrong-password" }), + }); + } + + test("a forged, rotating IP header per attempt cannot exceed the per-account budget", async () => { + const hub = await createHub({ + ...config, + signInRateLimit: { windowSeconds: 60, max: 2 }, + }); + closers.push(hub.close); + + // Same targeted account, a distinct forged source IP every attempt. + await signInAttempt(hub, "victim@example.com", "203.0.113.10"); + await signInAttempt(hub, "victim@example.com", "203.0.113.20"); + const throttled = await signInAttempt( + hub, + "victim@example.com", + "203.0.113.30", + ); + + expect(throttled.status).toBe(429); }); - } - test("two distinct client IPs get independent sign-in buckets", async () => { - const hub = await createHub({ - ...config, - signInRateLimit: { windowSeconds: 60, max: 2 }, + test("two genuinely different accounts get independent budgets", async () => { + const hub = await createHub({ + ...config, + signInRateLimit: { windowSeconds: 60, max: 1 }, + }); + closers.push(hub.close); + + // Exhausts alice's budget (max: 1), from the same source IP. + await signInAttempt(hub, "alice@example.com", "203.0.113.40"); + const throttledAlice = await signInAttempt( + hub, + "alice@example.com", + "203.0.113.40", + ); + expect(throttledAlice.status).toBe(429); + + // bob's very first attempt is untouched by alice's exhausted budget. + const freshBob = await signInAttempt( + hub, + "bob@example.com", + "203.0.113.40", + ); + expect(freshBob.status).not.toBe(429); }); - closers.push(hub.close); - // Exhausts IP A's bucket (max: 2). - await signInAttempt(hub, "203.0.113.10"); - await signInAttempt(hub, "203.0.113.10"); - const throttledA = await signInAttempt(hub, "203.0.113.10"); - expect(throttledA.status).toBe(429); - - // IP B's very first request is untouched by A's bucket. - const freshB = await signInAttempt(hub, "203.0.113.20"); - expect(freshB.status).not.toBe(429); - }); - - test("a rate-limited sign-in carries a retry hint and a human-readable message", async () => { - const hub = await createHub({ - ...config, - signInRateLimit: { windowSeconds: 60, max: 1 }, + test("a rate-limited sign-in carries a retry hint and a human-readable message", async () => { + const hub = await createHub({ + ...config, + signInRateLimit: { windowSeconds: 60, max: 1 }, + }); + closers.push(hub.close); + + await signInAttempt(hub, "throttle-me@example.com", "203.0.113.50"); + const throttled = await signInAttempt( + hub, + "throttle-me@example.com", + "203.0.113.60", + ); + + expect(throttled.status).toBe(429); + expect(throttled.headers.get("x-retry-after")).not.toBeNull(); + const body = (await throttled.json()) as { message: string }; + expect(body.message).toMatch(/\S/); }); - closers.push(hub.close); - - await signInAttempt(hub, "203.0.113.30"); - const throttled = await signInAttempt(hub, "203.0.113.30"); - - expect(throttled.status).toBe(429); - expect(throttled.headers.get("x-retry-after")).not.toBeNull(); - const body = (await throttled.json()) as { message: string }; - expect(body.message).toMatch(/\S/); - }); - - test("a resolvable x-forwarded-for means the shared-bucket boot warning never fires", async () => { - const warnSpy = spyOn(console, "warn"); - const hub = await createHub(config); - closers.push(hub.close); - - await signInAttempt(hub, "203.0.113.40"); - - const sharedBucketWarning = warnSpy.mock.calls.some((call) => - call.some( - (arg) => - typeof arg === "string" && - arg.includes("falling back to a single shared per-path bucket"), - ), - ); - expect(sharedBucketWarning).toBe(false); - warnSpy.mockRestore(); - }); -}); + }, +); describeIfDb("dev-mode email verification", () => { test("ALLOW_UNVERIFIED_EMAILS auto-verifies a fresh self-serve signup, so it never 403s on the unverified-email gate", async () => { From d2f85f93a7a6c3414b0121d8ec4856b35f87029b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 08:37:09 -0700 Subject: [PATCH 3/3] Update docs: sign-in rate limit comment reflects the account-keyed redesign config.ts's DEFAULT_SIGNIN_RATE_LIMIT_* comment still described these knobs as raising better-auth's own IP-keyed sign-in rule. Since the previous commit disables that rule entirely for sign-in and routes these values into the account-keyed limiter instead, the comment described a mechanism this codebase no longer uses. --- apps/hub/src/config.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/hub/src/config.ts b/apps/hub/src/config.ts index a5b0a1b2b..f6a50f792 100644 --- a/apps/hub/src/config.ts +++ b/apps/hub/src/config.ts @@ -201,15 +201,17 @@ const HubEnv = type({ const DEFAULT_SIGNUP_RATE_LIMIT_WINDOW_SECONDS = 60; const DEFAULT_SIGNUP_RATE_LIMIT_MAX = 5; // better-auth's own built-in special rule for /sign-in* is 3 attempts per -// 10 seconds, keyed per client IP. That's the bucket the shared-IP-fallback -// bug (CL-6494) starves: when the IP can't be resolved every signed-out -// visitor -- in production, or the one developer on a local box -- shares -// it. Raising it here, applied the same in every environment rather than -// branched on NODE_ENV (this file already rejects inferring auth behavior -// from NODE_ENV — see `rateLimit.enabled` below), gives real users room to -// mistype a password and gives a lone local developer room to keep working -// even while the IP genuinely can't be resolved, without loosening -// production brute-force resistance in any meaningful way. +// 10 seconds, keyed per client IP -- too tight for a bucket that can end up +// shared (CL-6494): when the IP can't be resolved, or is forged, every +// signed-out visitor, or an attacker replaying the same forged header, can +// starve it. index.ts now disables that built-in rule for sign-in entirely +// (see sign-in-rate-limit.ts) and uses these knobs to configure its +// account-keyed replacement instead, applied the same in every environment +// rather than branched on NODE_ENV (this file already rejects inferring +// auth behavior from NODE_ENV — see `rateLimit.enabled` below). This gives +// real users room to mistype a password, and a lone local developer room +// to keep working even while no IP can be resolved, without loosening +// brute-force resistance. const DEFAULT_SIGNIN_RATE_LIMIT_WINDOW_SECONDS = 60; const DEFAULT_SIGNIN_RATE_LIMIT_MAX = 10;