From 6dc809d833212a1b5ff75786bfacfb4581dac1a2 Mon Sep 17 00:00:00 2001 From: Fuwn Date: Tue, 2 Jun 2026 01:56:02 +0000 Subject: fix(security): make rate limiting real; limit the click counter (L8/L10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rateLimit.ts built a new in-memory RateLimiter inside the function on every call, so its store was always empty and it never limited anything. Rewrite as module-level singletons: auth/mutation/read IP classes returning 429, plus a click-counter limiter keyed on (IP, badge) so a viewer browsing many different badges isn't throttled while hammering one badge is capped. Wire the counter into PUT /api/badges?incrementClickCount (L10). Add a pluggable RateLimiterStore seam (in-memory default, Upstash/Vercel KV ready) and document the serverless per-region caveat. Add docs/vercel-firewall.md with the dashboard WAF rate-limit rule spec (Hobby 1-rule vs Pro) for coarse per-IP edge protection — keys the app limiters can't express at the edge. Verified locally: same-badge hits 200,200,429,429…; a different badge stays 200; a no-origin hit is rate-checked then 401. --- docs/vercel-firewall.md | 43 +++++++++++++++++++++++++++ src/lib/Error/rateLimit.ts | 64 +++++++++++++++++++++++++++++++++++----- src/routes/api/badges/+server.ts | 9 +++++- 3 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 docs/vercel-firewall.md diff --git a/docs/vercel-firewall.md b/docs/vercel-firewall.md new file mode 100644 index 00000000..0d3fdbff --- /dev/null +++ b/docs/vercel-firewall.md @@ -0,0 +1,43 @@ +# Vercel WAF — rate-limiting rules + +Coarse, per-IP abuse protection that runs at the edge, complementing the +app-level limiters in `src/lib/Error/rateLimit.ts` (which handle what the WAF +cannot — per-user windows and the per-(IP, badge) click counter). + +These rules are **not committable config** — Vercel WAF rules are managed in the +dashboard (or via Terraform / the REST API), not `vercel.json`. This file is the +spec to enter under **Project → Firewall → Configure → + New Rule**. + +Caveats from the platform: + +- Counters are **per region** — traffic spread across regions can exceed a + single-region limit. +- Counting keys on Hobby/Pro are **IP / JA4 only** (no app data like a badge id; + that is why the click counter is limited app-side instead). +- Window: min **10s**, max **10min** (Hobby/Pro). +- **Hobby allows only 1 rule per project**; Pro allows 40. +- Start each rule's action as **Log** to observe real traffic, then switch to + **Deny (429)** once the thresholds look right. + +## Hobby (single rule) + +| Field | Value | +| --- | --- | +| Name | `api-mutations-ratelimit` | +| If | `Request Path` starts with `/api/` **AND** `Request Method` is one of `POST`, `PUT`, `DELETE` | +| Then | Rate Limit — Fixed Window | +| Window / Limit | `60s` / `100` requests | +| Key | `IP` | +| Action | `Deny` (429) | + +## Pro (targeted rules) + +| Name | If | Window / Limit | Key | Action | +| --- | --- | --- | --- | --- | +| `auth-ratelimit` | Path starts with `/api/oauth/` | `60s` / `20` | IP | Deny | +| `mutations-ratelimit` | Path is one of `/api/badges`, `/api/preferences`, `/api/configuration` **or** starts with `/api/notifications/`; Method `POST`/`PUT`/`DELETE` | `60s` / `60` | IP | Deny | +| `graphql-ratelimit` | Path is `/graphql`; Method `POST` | `60s` / `60` | IP | Deny | + +Public GET reads and the click counter are left to the CDN cache and the +app-level limiters respectively; add a coarse `/api/*` GET rule only if you see +read abuse. diff --git a/src/lib/Error/rateLimit.ts b/src/lib/Error/rateLimit.ts index 636bdd5d..d3508aba 100644 --- a/src/lib/Error/rateLimit.ts +++ b/src/lib/Error/rateLimit.ts @@ -1,12 +1,62 @@ import type { RequestEvent } from "@sveltejs/kit"; -import { RateLimiter } from "sveltekit-rate-limiter/server"; +import { + RateLimiter, + type RateLimiterStore, +} from "sveltekit-rate-limiter/server"; -export const checkRateLimit = async (event: RequestEvent) => { - const limiter = new RateLimiter({ rates: { IP: [5, "s"] } }); +// Storage. The library's default store is in-memory and PER INSTANCE. On +// Vercel's serverless runtime, counters do not aggregate across lambda +// instances or regions, so these limiters are best-effort backstops there. +// Provision a shared store (Upstash Redis / Vercel KV) implementing +// RateLimiterStore and return it here for durable limits. Coarse per-IP abuse +// protection also runs at the edge via the Vercel WAF (see +// docs/vercel-firewall.md); these app-level limiters add what the WAF cannot +// express — per-user windows and the per-(IP, badge) click counter. +const createStore = (): RateLimiterStore | undefined => { + // e.g. `if (env.UPSTASH_REDIS_REST_URL) return new UpstashStore(...)` + return undefined; +}; - await limiter.cookieLimiter?.preflight(event); +const store = createStore(); - if (await limiter.isLimited(event)) return new Response("rate-limited"); +// Module-level singletons: the counters must persist across requests. (The +// previous implementation built a new RateLimiter on every call, so its store +// was always empty and it never actually limited anything.) +const limiters = { + auth: new RateLimiter({ IP: [10, "m"], store }), + mutation: new RateLimiter({ IP: [30, "m"], store }), + read: new RateLimiter({ IP: [60, "m"], store }), +} satisfies Record; - return null; -}; +export type RateLimitClass = keyof typeof limiters; + +const tooManyRequests = () => + new Response("Too Many Requests", { status: 429 }); + +export const checkRateLimit = async ( + event: RequestEvent, + limit: RateLimitClass = "mutation", +): Promise => + (await limiters[limit].isLimited(event)) ? tooManyRequests() : null; + +// Click counter: bound inflation of a single badge without throttling a viewer +// browsing many DIFFERENT badges. Keyed on (IP, badge) — which the edge WAF +// can't express, since it keys on IP alone. +const clickCounterLimiter = new RateLimiter({ + store, + plugins: [ + { + rate: [2, "m"], + hash: async (event) => { + const badgeId = event.url.searchParams.get("incrementClickCount"); + + return badgeId ? `click:${event.getClientAddress()}:${badgeId}` : true; + }, + }, + ], +}); + +export const checkClickCounterLimit = async ( + event: RequestEvent, +): Promise => + (await clickCounterLimiter.isLimited(event)) ? tooManyRequests() : null; diff --git a/src/routes/api/badges/+server.ts b/src/routes/api/badges/+server.ts index 10b63125..a4212f40 100644 --- a/src/routes/api/badges/+server.ts +++ b/src/routes/api/badges/+server.ts @@ -15,6 +15,7 @@ import { } from "$lib/Database/SB/User/badges"; import { decodeAuthCookieOrNull } from "$lib/Effect/authCookie"; import { decodeRequestJsonOrThrow } from "$lib/Effect/requestBody"; +import { checkClickCounterLimit } from "$lib/Error/rateLimit"; import { appOrigin, appOriginHeaders } from "$lib/Utility/appOrigin"; import { isOwnerOrPrivileged } from "$lib/Utility/authorisation"; import privilegedUser from "$lib/Utility/privilegedUser"; @@ -53,8 +54,14 @@ export const DELETE = async ({ url, cookies }) => { return await badges(identity.id); }; -export const PUT = async ({ cookies, url, request }) => { +export const PUT = async (event) => { + const { cookies, url, request } = event; + if (url.searchParams.get("incrementClickCount") || undefined) { + const limited = await checkClickCounterLimit(event); + + if (limited) return limited; + if (request.headers.get("origin") !== appOrigin()) return unauthorised(); await incrementClickCount( -- cgit v1.2.3