diff --git a/apps/facilitator/README.md b/apps/facilitator/README.md index 2c14206f..f0131925 100644 --- a/apps/facilitator/README.md +++ b/apps/facilitator/README.md @@ -12,7 +12,49 @@ discovery routes. Design: [the M1 spine](../../docs/architecture/m1-spine.md) (A - `SETTLE_TIMEOUT_MS` (20 s, or the budget when that is shorter; at least 1 s, at most the budget): how long a repeat of a sent payment waits for it to turn final. - `NETWORK=stellar:pubnet` refuses to start, whatever the store: `/settle` would spend there - with no spend budgets (0024), no rate limits (0023) and no channel lease (0025) yet. + with no spend budgets (0024) and no channel lease (0025) yet, and the store's tables would + grow without bound until pruning (0050) lands. + +## Callers and rate limits + +Every route is open: no caller authenticates, and there are no API keys (task 0060). `/verify` +and `/settle` send no CORS headers, so a browser page can't call them; a seller's server calls +them for its buyers. + +Each client has a budget per route and window. Past it the route answers 429 +`{"error":"too_many_requests"}` with `Retry-After` in seconds; the other routes still answer. +Every request counts, a malformed or oversized one too. `/supported` has no limit. + +| Setting | Default | Meaning | +| ---------------------- | -------- | ----------------------------------------------------------------------------------- | +| `RATE_LIMIT_WINDOW_MS` | 60 000 | the window, aligned to the clock: every count starts again at its end | +| `RATE_LIMIT_VERIFY` | 600 | `/verify` requests per window and client | +| `RATE_LIMIT_SETTLE` | 600 | `/settle` requests per window and client; at least `/verify`'s | +| `RATE_LIMIT_DISCOVERY` | 120 | `/discovery/*` requests per window and client | +| `BODY_LIMIT_BYTES` | 65 536 | a larger request body is a 413 | +| `CLIENT_IP` | required | `cloudflare`: the `CF-Connecting-IP` header; `socket`: the connection's own address | + +- **A client is its IP**, an IPv6 one its /56. The seller's server calls `/verify` and `/settle` + for all of its buyers, so a budget is a budget per seller: a seller above about 10 paid requests + a second needs higher values. `/settle`'s budget is never below `/verify`'s, since the seller + settles after serving: a payment refused there was work given away. The spend budgets (AD-18), + not these, bound the fees the facilitator pays. +- **Sellers on Cloudflare Workers share one budget.** A Worker's request to another Cloudflare + zone arrives with `CF-Connecting-IP: 2a06:98c0:3600::103` for every Worker, and sellers behind + one cloud NAT share its address the same way. A per-seller budget needs API keys (0060). +- **`CLIENT_IP` is required**, so it can't be forgotten behind the tunnel, where every client + would share the tunnel's address and one budget. `cloudflare` reads `CF-Connecting-IP`, which + Cloudflare sets on every request and refuses from a client; `socket` takes the connection's own + address, for runs with no proxy in front (local runs and tests). Any other value refuses to + start, and so does a leftover `TRUSTED_PROXY_HOPS`. `X-Forwarded-For` is never read: a client + can put any address in it. +- **With `cloudflare`, a request without the header, or with anything but one address in it,** is a + 500 and an error log line: only `cloudflared` should reach the service. +- **Windows are aligned to the clock**, so a client can send up to twice its budget across a + window's end, and every blocked client is released at the same moment. +- **The counters live in the store** (`rate_limits` with `STORE=postgres`). A store error is a 500, + logged. A client over its budget is answered from the process until its window ends, without a + store write. ## The store diff --git a/apps/facilitator/package.json b/apps/facilitator/package.json index b8994772..9781b3d6 100644 --- a/apps/facilitator/package.json +++ b/apps/facilitator/package.json @@ -25,6 +25,7 @@ "@x402/extensions": "~2.28.0", "@x402/stellar": "~2.28.0", "express": "^5.2.1", + "express-rate-limit": "^8.7.0", "pg": "^8.23.1", "undici": "~7.30.0", "zod": "^3.25.76" diff --git a/apps/facilitator/scripts/settle-smoke.ts b/apps/facilitator/scripts/settle-smoke.ts index 479d7937..3d66e96b 100644 --- a/apps/facilitator/scripts/settle-smoke.ts +++ b/apps/facilitator/scripts/settle-smoke.ts @@ -85,6 +85,7 @@ if (created.status !== "SUCCESS") throw new Error(`channel setup ${created.statu const facilitatorServer = await main( { FACILITATOR_SECRET: facilitator.secret(), + CLIENT_IP: "socket", CHANNELS: channelKeys.map((keypair) => keypair.publicKey()).join(","), PORT: "0", LOG_LEVEL: "warn", diff --git a/apps/facilitator/scripts/verify-smoke.ts b/apps/facilitator/scripts/verify-smoke.ts index 8aa646ec..4050f14e 100644 --- a/apps/facilitator/scripts/verify-smoke.ts +++ b/apps/facilitator/scripts/verify-smoke.ts @@ -81,6 +81,7 @@ async function verifyWith( // A fresh facilitator key: verify never spends from it. const config = parseConfig({ FACILITATOR_SECRET: Keypair.random().secret(), + CLIENT_IP: "socket", CHANNELS: channels, LOG_LEVEL: "info", MIN_VALIDITY_LEDGERS: process.env.MIN_VALIDITY_LEDGERS, diff --git a/apps/facilitator/src/config.ts b/apps/facilitator/src/config.ts index 86bbd149..0be42dd7 100644 --- a/apps/facilitator/src/config.ts +++ b/apps/facilitator/src/config.ts @@ -178,11 +178,23 @@ const envSchema = z LEASE_TTL_MS: ms(1_000).default("30000"), BODY_LIMIT_BYTES: int(1_024).default("65536"), - /** Proxy hops in front of the service whose `X-Forwarded-For` entries are trusted (AD-11). */ - TRUSTED_PROXY_HOPS: int(0, 10).default("0"), + /** + * Where the client's IP comes from (AD-11). `cloudflare`: the `CF-Connecting-IP` header, which + * Cloudflare sets on every request and refuses from a client; only `cloudflared` reaches the + * service (ADR 0011). `socket`: the connection's own address, with no proxy in front, as in + * local runs and tests. Required: forgotten behind the tunnel, every client would share the + * tunnel's address and one budget. `X-Forwarded-For` is never read. + */ + CLIENT_IP: oneOf(["cloudflare", "socket"]), + /** Replaced by `CLIENT_IP`; refused, so a leftover can't look like it still applies. */ + TRUSTED_PROXY_HOPS: z.string().optional(), RATE_LIMIT_WINDOW_MS: ms(1_000).default("60000"), - RATE_LIMIT_VERIFY: int(1).default("120"), - RATE_LIMIT_SETTLE: int(1).default("60"), + /** + * Requests per window and client. The seller's server calls `/verify` and `/settle` for all of + * its buyers, so these are budgets per seller; fees are bounded by the spend budgets (AD-18). + */ + RATE_LIMIT_VERIFY: int(1).default("600"), + RATE_LIMIT_SETTLE: int(1).default("600"), RATE_LIMIT_DISCOVERY: int(1).default("120"), /** Rolling window of every spend budget (AD-18); a store window, not a timer. */ @@ -229,15 +241,25 @@ const envSchema = z message: "must be at most SETTLE_BUDGET_MS", path: ["SETTLE_TIMEOUT_MS"], }) + // The seller settles after serving the resource, so a payment that passed `/verify` and is + // refused at `/settle` was work given away. + .refine((env) => env.RATE_LIMIT_SETTLE >= env.RATE_LIMIT_VERIFY, { + message: "must be at least RATE_LIMIT_VERIFY", + path: ["RATE_LIMIT_SETTLE"], + }) + .refine((env) => env.TRUSTED_PROXY_HOPS === undefined, { + message: "is replaced by CLIENT_IP", + path: ["TRUSTED_PROXY_HOPS"], + }) .refine((env) => env.STORE !== "memory" || env.NETWORK === STELLAR_NETWORKS.testnet, { message: "must be postgres outside testnet", path: ["STORE"], }) - // `/settle` spends on pubnet, so not before spend budgets, rate limits and the channel lease. - // The task that lands the last of them lifts this. + // `/settle` spends on pubnet, so not before spend budgets and the channel lease, nor while the + // store's tables grow without bound. The task that lands the last of them lifts this. .refine((env) => env.NETWORK !== STELLAR_NETWORKS.pubnet, { message: - "stellar:pubnet is refused until spend budgets (0024), rate limits (0023) and the channel lease (0025) land", + "stellar:pubnet is refused until spend budgets (0024), the channel lease (0025) and store pruning (0050) land", path: ["NETWORK"], }) .refine((env) => env.DATABASE_URL !== undefined || env.STORE !== "postgres", { @@ -282,7 +304,7 @@ export interface Config { }; http: { bodyLimitBytes: number; - trustedProxyHops: number; + clientIp: "cloudflare" | "socket"; rateLimit: { windowMs: number; verify: number; settle: number; discovery: number }; }; spend: { @@ -360,7 +382,7 @@ export function parseConfig(env: Record): Config { }, http: { bodyLimitBytes: e.BODY_LIMIT_BYTES, - trustedProxyHops: e.TRUSTED_PROXY_HOPS, + clientIp: e.CLIENT_IP, rateLimit: { windowMs: e.RATE_LIMIT_WINDOW_MS, verify: e.RATE_LIMIT_VERIFY, diff --git a/apps/facilitator/src/http/app.ts b/apps/facilitator/src/http/app.ts index 94e45d45..975c18ef 100644 --- a/apps/facilitator/src/http/app.ts +++ b/apps/facilitator/src/http/app.ts @@ -9,6 +9,7 @@ import type { SettlementHooks } from "../settlement/hooks.js"; import type { Settle } from "../settlement/settle.js"; import type { Verify, VerifyTrace } from "../settlement/verify.js"; import { EXTENSION_RESPONSES_HEADER, encodeExtensionResponses } from "./extension-responses.js"; +import { rateLimit } from "./rate-limit.js"; /** What the composition root hands to the HTTP adapter. Lanes add their services here. */ export interface AppDeps { @@ -39,14 +40,28 @@ const notImplemented: RequestHandler = (_req, res) => { */ export const MAX_BODY_DEPTH = 256; -/** Whether `value` nests deeper than `max` arrays and objects, counted without recursion. */ -function nestsDeeper(value: unknown, max: number): boolean { +/** + * A NUL, or a lone UTF-16 surrogate (`\p{Cs}` matches only an unpaired one under the `u` flag). + * PostgreSQL refuses a NUL in `text` and `jsonb`, and `jsonb` a lone surrogate, so either would + * fail the settlement's store write with a 500. + */ +const UNSTORABLE_TEXT = /[\0\p{Cs}]/u; + +/** + * Whether `value` nests deeper than `max` arrays and objects, or holds a string or key matching + * {@link UNSTORABLE_TEXT}; walked without recursion. + */ +function refusedBody(value: unknown, max: number): boolean { const stack: [unknown, number][] = [[value, 0]]; while (stack.length > 0) { const [item, depth] = stack.pop()!; + if (typeof item === "string" && UNSTORABLE_TEXT.test(item)) return true; if (item === null || typeof item !== "object") continue; if (depth >= max) return true; - for (const child of Object.values(item)) stack.push([child, depth + 1]); + for (const [key, child] of Object.entries(item)) { + if (UNSTORABLE_TEXT.test(key)) return true; + stack.push([child, depth + 1]); + } } return false; } @@ -57,11 +72,12 @@ function nestsDeeper(value: unknown, max: number): boolean { * * The schema takes any non-empty `amount`, and upstream's `BigInt(amount)` throws on anything but * an integer, which it reports as `unexpected_verify_error`: our 500. So `amount` must be digits. - * A body nested deeper than {@link MAX_BODY_DEPTH} is a 400 too, before anything recurses on it. + * A body nested deeper than {@link MAX_BODY_DEPTH} is a 400 too, before anything recurses on it, + * and so is a NUL or a lone surrogate anywhere in it. */ export function parsePaymentRequest(body: unknown): VerifyRequest | undefined { if (typeof body !== "object" || body === null) return undefined; - if (nestsDeeper(body, MAX_BODY_DEPTH)) return undefined; + if (refusedBody(body, MAX_BODY_DEPTH)) return undefined; const { x402Version, paymentPayload, paymentRequirements } = body as Record; const requirements = PaymentRequirementsV2Schema.safeParse(paymentRequirements); const valid = @@ -74,12 +90,14 @@ export function parsePaymentRequest(body: unknown): VerifyRequest | undefined { /** * The inbound HTTP adapter (AD-13). `/discovery/resources` answers 501 until its task lands - * (0032). There is a body size limit and - * no CORS anywhere (spine "HTTP hardening"); rate limits come in 0023. + * (0032). There is a body size limit and no CORS anywhere (spine "HTTP hardening"). `/verify`, + * `/settle` and `/discovery/*` each have a rate limit (AD-11), applied before the body is read, so + * a malformed or oversized request counts too; `/supported` has none. */ export function createApp({ config, logger, + stores, verify, settle, extensionResponses, @@ -87,10 +105,19 @@ export function createApp({ }: AppDeps): Express { const app = express(); app.disable("x-powered-by"); - app.use(express.json({ limit: config.http.bodyLimitBytes })); + const limit = (route: string, budget: number) => + rateLimit({ + route, + limit: budget, + windowMs: config.http.rateLimit.windowMs, + clientIp: config.http.clientIp, + store: stores.rateLimits, + logger, + }); + const json = express.json({ limit: config.http.bodyLimitBytes }); // A verify outcome, valid or not, is a 200 with an x402 body; a thrown error is a 500. - app.post("/verify", async (req, res) => { + app.post("/verify", limit("verify", config.http.rateLimit.verify), json, async (req, res) => { const request = parsePaymentRequest(req.body); if (!request) { res.status(400).json({ error: code(400) }); @@ -112,7 +139,7 @@ export function createApp({ }); // Like `/verify`: any settle outcome is a 200 with an x402 body (AD-6). A thrown error, such as // upstream verify failing or the store down before the claim, is a 500. - app.post("/settle", async (req, res) => { + app.post("/settle", limit("settle", config.http.rateLimit.settle), json, async (req, res) => { const request = parsePaymentRequest(req.body); if (!request) { res.status(400).json({ error: code(400) }); @@ -157,6 +184,7 @@ export function createApp({ app.get("/supported", (_req, res) => { res.json(supported); }); + app.use("/discovery", limit("discovery", config.http.rateLimit.discovery)); app.get("/discovery/resources", notImplemented); app.use((_req, res) => { diff --git a/apps/facilitator/src/http/rate-limit.ts b/apps/facilitator/src/http/rate-limit.ts new file mode 100644 index 00000000..1e66cd1b --- /dev/null +++ b/apps/facilitator/src/http/rate-limit.ts @@ -0,0 +1,73 @@ +import { isIP } from "node:net"; +import type { RequestHandler } from "express"; +import { ipKeyGenerator } from "express-rate-limit"; +import type { Logger } from "../logger.js"; +import type { RateLimitStore } from "../ports/index.js"; + +export interface RateLimit { + /** The route's name in the counter's key and the logs: `verify`, `settle`, `discovery`. */ + route: string; + /** Requests per window and client. */ + limit: number; + windowMs: number; + /** `cloudflare`: the `CF-Connecting-IP` header; `socket`: the connection's own address. */ + clientIp: "cloudflare" | "socket"; + store: RateLimitStore; + logger: Logger; +} + +/** + * One route's budget (AD-11): a fixed window per client, counted in `RateLimitStore`, and a 429 + * past it. Every request counts, a malformed one too. An IPv6 client is counted per /56, so one + * /64 can't rotate past the budget. A store error fails the request with a 500. + * + * A client over its budget is remembered in the process until its window ends and answered + * without a store hit, so its flood can't hold the store's few connections from everyone else. + */ +export function rateLimit({ + route, + limit, + windowMs, + clientIp, + store, + logger, +}: RateLimit): RequestHandler { + /** Clients over budget, with the end of their window in milliseconds since the epoch. */ + const blocked = new Map(); + + return async (req, res, next) => { + const ip = clientIp === "cloudflare" ? req.get("cf-connecting-ip") : req.socket.remoteAddress; + if (!ip || isIP(ip) === 0) { + // Only `cloudflared` reaches the service, and Cloudflare sets the header to one address on + // every request. Anything else never reaches the store; the value is the caller's, so it isn't + // logged. + logger.error(ip ? "client_ip_invalid" : "client_ip_missing", { + route, + source: clientIp, + }); + res.status(500).json({ error: "internal_error" }); + return; + } + const client = ipKeyGenerator(ip); + const now = Date.now(); + + let resetAt = blocked.get(client); + if (resetAt === undefined || resetAt <= now) { + // A store error rejects, and Express answers it with a 500. + const hit = await store.hit(`${route}:ip:${client}`, windowMs, now); + if (hit.count <= limit) { + next(); + return; + } + resetAt = hit.resetAt; + // Requests already in flight find the budget spent too; only the first one logs. + const logged = (blocked.get(client) ?? 0) > now; + for (const [key, end] of blocked) if (end <= now) blocked.delete(key); + blocked.set(client, resetAt); + // `ip` is the address to block at the edge; `client` is what was counted (an IPv6 /56). + if (!logged) logger.warn("rate_limited", { route, ip, client, limit, windowMs }); + } + res.setHeader("Retry-After", String(Math.max(1, Math.ceil((resetAt - now) / 1000)))); + res.status(429).json({ error: "too_many_requests" }); + }; +} diff --git a/apps/facilitator/test/build-verifier.test.ts b/apps/facilitator/test/build-verifier.test.ts index 4c248676..2a3b7d9b 100644 --- a/apps/facilitator/test/build-verifier.test.ts +++ b/apps/facilitator/test/build-verifier.test.ts @@ -46,6 +46,7 @@ const CURRENT = 1_000; const config = parseConfig({ FACILITATOR_SECRET: secret, + CLIENT_IP: "socket", CHANNELS: channel, RPC_URLS: "https://rpc-a.example,https://rpc-b.example", MAX_FEE_STROOPS: "300000", diff --git a/apps/facilitator/test/config.test.ts b/apps/facilitator/test/config.test.ts index 394eb867..0eb32c57 100644 --- a/apps/facilitator/test/config.test.ts +++ b/apps/facilitator/test/config.test.ts @@ -13,15 +13,19 @@ const SECRET = keypair(1).secret(); const CHANNEL_A = keypair(2).publicKey(); const CHANNEL_B = keypair(3).publicKey(); -const minimal = { FACILITATOR_SECRET: SECRET, CHANNELS: `${CHANNEL_A}, ${CHANNEL_B}` }; -/** Pubnet needs the PostgreSQL store, and is refused until 0023, 0024 and 0025 land. */ +const minimal = { + FACILITATOR_SECRET: SECRET, + CHANNELS: `${CHANNEL_A}, ${CHANNEL_B}`, + CLIENT_IP: "socket", +}; +/** Pubnet needs the PostgreSQL store, and is refused until 0024, 0025 and 0050 land. */ const pubnet = { NETWORK: "stellar:pubnet", STORE: "postgres", DATABASE_URL: "postgres://db.internal/x402", }; const PUBNET_REFUSED = - "NETWORK stellar:pubnet is refused until spend budgets (0024), rate limits (0023) and the channel lease (0025) land"; + "NETWORK stellar:pubnet is refused until spend budgets (0024), the channel lease (0025) and store pruning (0050) land"; function problems(env: Record): string[] { try { @@ -52,8 +56,8 @@ describe("parseConfig", () => { }, http: { bodyLimitBytes: 65_536, - trustedProxyHops: 0, - rateLimit: { windowMs: 60_000, verify: 120, settle: 60, discovery: 120 }, + clientIp: "socket", + rateLimit: { windowMs: 60_000, verify: 600, settle: 600, discovery: 120 }, }, spend: { windowMs: 86_400_000, @@ -88,10 +92,10 @@ describe("parseConfig", () => { SETTLE_BUDGET_MS: "60000", LEASE_TTL_MS: "32000", BODY_LIMIT_BYTES: "70000", - TRUSTED_PROXY_HOPS: "2", + CLIENT_IP: "cloudflare", RATE_LIMIT_WINDOW_MS: "61000", RATE_LIMIT_VERIFY: "121", - RATE_LIMIT_SETTLE: "62", + RATE_LIMIT_SETTLE: "122", RATE_LIMIT_DISCOVERY: "123", SPEND_WINDOW_MS: "3600000", SPEND_GLOBAL_STROOPS: "11", @@ -123,8 +127,8 @@ describe("parseConfig", () => { }, http: { bodyLimitBytes: 70_000, - trustedProxyHops: 2, - rateLimit: { windowMs: 61_000, verify: 121, settle: 62, discovery: 123 }, + clientIp: "cloudflare", + rateLimit: { windowMs: 61_000, verify: 121, settle: 122, discovery: 123 }, }, spend: { windowMs: 3_600_000, @@ -142,16 +146,22 @@ describe("parseConfig", () => { const config = parseConfig({ FACILITATOR_SECRET: `${SECRET}\n`, CHANNELS: ` ${CHANNEL_A} `, + CLIENT_IP: " socket ", RPC_URLS: "", PORT: " ", }); expect(config.facilitatorSecret).toBe(SECRET); expect(config.channels).toEqual([CHANNEL_A]); + expect(config.http.clientIp).toBe("socket"); expect(config.port).toBe(4021); }); it("names missing required values", () => { - expect(problems({})).toEqual(["FACILITATOR_SECRET is required", "CHANNELS is required"]); + expect(problems({})).toEqual([ + "FACILITATOR_SECRET is required", + "CHANNELS is required", + "CLIENT_IP is required", + ]); }); it("names invalid values without echoing them", () => { @@ -256,6 +266,37 @@ describe("parseConfig", () => { ).toMatchObject({ maxFeeStroops: 201n, inclusionFeeStroops: 100n }); }); + it("keeps the /settle budget at or above /verify's: the seller calls both, settle after serving", () => { + expect(problems({ ...minimal, RATE_LIMIT_VERIFY: "100", RATE_LIMIT_SETTLE: "99" })).toEqual([ + "RATE_LIMIT_SETTLE must be at least RATE_LIMIT_VERIFY", + ]); + expect(problems({ ...minimal, RATE_LIMIT_VERIFY: "700" })).toEqual([ + "RATE_LIMIT_SETTLE must be at least RATE_LIMIT_VERIFY", + ]); + expect( + parseConfig({ ...minimal, RATE_LIMIT_VERIFY: "100", RATE_LIMIT_SETTLE: "100" }).http + .rateLimit, + ).toMatchObject({ verify: 100, settle: 100 }); + }); + + it("requires CLIENT_IP: cloudflare behind the tunnel, socket with no proxy in front", () => { + // Forgotten behind a proxy, every client would share the proxy's address and one budget. + expect(problems({ ...minimal, CLIENT_IP: undefined })).toEqual(["CLIENT_IP is required"]); + for (const value of ["cf-connecting-ip", "x-forwarded-for", "Cloudflare"]) { + expect(problems({ ...minimal, CLIENT_IP: value })).toEqual([ + "CLIENT_IP must be one of cloudflare, socket", + ]); + } + expect(parseConfig({ ...minimal, CLIENT_IP: "cloudflare" }).http.clientIp).toBe("cloudflare"); + }); + + it("refuses a leftover TRUSTED_PROXY_HOPS", () => { + // Left set, it would look like the client IP still came from X-Forwarded-For. + expect(problems({ ...minimal, TRUSTED_PROXY_HOPS: "1" })).toEqual([ + "TRUSTED_PROXY_HOPS is replaced by CLIENT_IP", + ]); + }); + it("ignores a leftover CATALOG_ALLOW_HTTP: every network catalogs http now (0053)", () => { expect(parseConfig({ ...minimal, CATALOG_ALLOW_HTTP: "true" })).toEqual(parseConfig(minimal)); expect( @@ -307,7 +348,7 @@ describe("parseConfig", () => { ]); }); - it("refuses stellar:pubnet with nothing else wrong, until 0023, 0024 and 0025 land", () => { + it("refuses stellar:pubnet with nothing else wrong, until 0024, 0025 and 0050 land", () => { // `/settle` would spend there with allow-all spend hooks, no rate limit and no lease. expect(problems({ ...minimal, ...pubnet, RPC_URLS: "https://rpc.example" })).toEqual([ PUBNET_REFUSED, diff --git a/apps/facilitator/test/fee-agreement.test.ts b/apps/facilitator/test/fee-agreement.test.ts index 4d68ea68..9ed4836c 100644 --- a/apps/facilitator/test/fee-agreement.test.ts +++ b/apps/facilitator/test/fee-agreement.test.ts @@ -123,6 +123,7 @@ const poolRpc = { describe("the fee ceiling at /verify and in the pool (AD-10)", () => { const config = parseConfig({ FACILITATOR_SECRET: facilitator.secret(), + CLIENT_IP: "socket", CHANNELS: channel, RPC_URLS: "http://127.0.0.1:1", MAX_FEE_STROOPS: String(MAX_FEE), @@ -191,6 +192,7 @@ describe("real upstream with a down first RPC URL (AD-3)", () => { it("answers from the second URL, and leaves the shared client on the first (decision 22)", async () => { const config = parseConfig({ FACILITATOR_SECRET: facilitator.secret(), + CLIENT_IP: "socket", CHANNELS: channel, // Nothing listens on port 1: upstream's simulation gets a refused connection. RPC_URLS: `http://127.0.0.1:1,${rpcUrl}`, diff --git a/apps/facilitator/test/fees.test.ts b/apps/facilitator/test/fees.test.ts index db4742a9..53e2d170 100644 --- a/apps/facilitator/test/fees.test.ts +++ b/apps/facilitator/test/fees.test.ts @@ -6,6 +6,7 @@ import { atCurrentBid, feeSettings } from "../src/settlement/fees.js"; const env = { FACILITATOR_SECRET: Keypair.random().secret(), + CLIENT_IP: "socket", CHANNELS: Keypair.random().publicKey(), }; diff --git a/apps/facilitator/test/main.test.ts b/apps/facilitator/test/main.test.ts index 49b3c69a..d6e21cd4 100644 --- a/apps/facilitator/test/main.test.ts +++ b/apps/facilitator/test/main.test.ts @@ -24,6 +24,7 @@ const secret = Keypair.random().secret(); const facilitator = Keypair.fromSecret(secret).publicKey(); const env = { FACILITATOR_SECRET: secret, + CLIENT_IP: "socket", CHANNELS: Keypair.random().publicKey(), BODY_LIMIT_BYTES: "1024", }; @@ -143,7 +144,7 @@ describe("startup with a bad config", () => { const error = await main(pubnet).catch((e: unknown) => e); expect(error).toBeInstanceOf(ConfigError); expect((error as ConfigError).problems).toEqual([ - "NETWORK stellar:pubnet is refused until spend budgets (0024), rate limits (0023) and the channel lease (0025) land", + "NETWORK stellar:pubnet is refused until spend budgets (0024), the channel lease (0025) and store pruning (0050) land", ]); }); }); diff --git a/apps/facilitator/test/postgres.test.ts b/apps/facilitator/test/postgres.test.ts index cd9b3c87..3d225437 100644 --- a/apps/facilitator/test/postgres.test.ts +++ b/apps/facilitator/test/postgres.test.ts @@ -346,6 +346,7 @@ describe.skipIf(!process.env.TEST_DATABASE_URL && !process.env.CI)("PostgreSQL s STORE: "postgres", DATABASE_URL: urlFor(schema), FACILITATOR_SECRET: bootSecret, + CLIENT_IP: "socket", CHANNELS: bootChannel, RPC_URLS: rpc.url, PORT: "0", @@ -413,6 +414,7 @@ describe.skipIf(!process.env.TEST_DATABASE_URL && !process.env.CI)("PostgreSQL s STORE: "postgres", DATABASE_URL: urlFor(schema), FACILITATOR_SECRET: bootSecret, + CLIENT_IP: "socket", CHANNELS: bootChannel, RPC_URLS: rpc.url, PORT: "0", @@ -441,6 +443,7 @@ describe.skipIf(!process.env.TEST_DATABASE_URL && !process.env.CI)("PostgreSQL s STORE: "postgres", DATABASE_URL: urlFor(schema), FACILITATOR_SECRET: bootSecret, + CLIENT_IP: "socket", CHANNELS: bootChannel, RPC_URLS: rpc.url, PORT: "0", diff --git a/apps/facilitator/test/rate-limit.test.ts b/apps/facilitator/test/rate-limit.test.ts new file mode 100644 index 00000000..0417c2e8 --- /dev/null +++ b/apps/facilitator/test/rate-limit.test.ts @@ -0,0 +1,223 @@ +import { Keypair } from "@stellar/stellar-sdk"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { MemoryRateLimitStore, memoryStores } from "../src/adapters/memory.js"; +import { parseConfig } from "../src/config.js"; +import { supportedResponse } from "../src/http/supported.js"; +import { jsonLogger } from "../src/logger.js"; +import { start, type Started } from "../src/main.js"; +import type { RateLimitStore, Stores } from "../src/ports/index.js"; + +let app: Started | undefined; +afterEach(async () => { + await app?.close(); + app = undefined; +}); + +/** + * A facilitator with a budget of 2 on each limited route; `/verify` and `/settle` are stubs. Windows + * are aligned to the clock, so a test's requests share one only if no boundary falls between them: + * an hour's window makes that all but certain. + */ +async function serve(env: Record = {}, stores: Stores = memoryStores()) { + const config = parseConfig({ + FACILITATOR_SECRET: Keypair.random().secret(), + CLIENT_IP: "socket", + CHANNELS: Keypair.random().publicKey(), + PORT: "0", + RATE_LIMIT_VERIFY: "2", + RATE_LIMIT_SETTLE: "2", + RATE_LIMIT_DISCOVERY: "2", + RATE_LIMIT_WINDOW_MS: "3600000", + ...env, + }); + const lines: string[] = []; + app = await start({ + config, + logger: jsonLogger("debug", (line) => lines.push(line)), + stores, + verify: async () => ({ isValid: true }), + settle: async () => ({ + response: { success: false, transaction: "", network: config.network }, + }), + extensionResponses: () => undefined, + supported: supportedResponse(config.network, Keypair.random().publicKey()), + }); + const url = `http://127.0.0.1:${app.port}`; + /** A POST of an empty object: a 400, which counts like any other request. */ + const post = (path: string, headers: Record = {}) => + fetch(`${url}${path}`, { + method: "POST", + headers: { "content-type": "application/json", ...headers }, + body: "{}", + }); + return { url, lines, post }; +} + +/** A rate-limit store whose `hit` is a spy over the memory one. */ +function spiedStores(): { stores: Stores; hit: ReturnType> } { + const memory = new MemoryRateLimitStore(); + const hit = vi.fn((...args) => memory.hit(...args)); + return { stores: { ...memoryStores(), rateLimits: { hit } }, hit }; +} + +describe("the rate limiter", () => { + it("answers 429 with Retry-After past a route's budget, on that route only", async () => { + const { url, post } = await serve(); + expect((await post("/verify")).status).toBe(400); + expect((await post("/verify")).status).toBe(400); + const limited = await post("/verify"); + expect([limited.status, await limited.json()]).toEqual([429, { error: "too_many_requests" }]); + const retryAfter = Number(limited.headers.get("retry-after")); + expect(retryAfter).toBeGreaterThanOrEqual(1); + expect(retryAfter).toBeLessThanOrEqual(3600); + // No counting headers: upstream's client reads only Retry-After. + expect(limited.headers.get("x-ratelimit-limit")).toBeNull(); + expect(limited.headers.get("ratelimit")).toBeNull(); + + expect((await post("/settle")).status).toBe(400); + for (let i = 0; i < 5; i++) expect((await fetch(`${url}/supported`)).status).toBe(200); + }); + + it("limits /discovery/* with its own budget", async () => { + const { url } = await serve(); + expect((await fetch(`${url}/discovery/resources`)).status).toBe(501); + expect((await fetch(`${url}/discovery/resources`)).status).toBe(501); + expect((await fetch(`${url}/discovery/resources`)).status).toBe(429); + expect((await fetch(`${url}/discovery/search`)).status).toBe(429); + }); + + it("ignores X-Forwarded-For: with CLIENT_IP=socket the client is the connection's address", async () => { + const { post } = await serve(); + const forged = ["1.2.3.4", "5.6.7.8", "9.9.9.9"]; + const statuses = []; + for (const ip of forged) + statuses.push((await post("/settle", { "x-forwarded-for": ip })).status); + expect(statuses).toEqual([400, 400, 429]); + }); + + it("keys the client by CF-Connecting-IP with CLIENT_IP=cloudflare, never by X-Forwarded-For", async () => { + const { post } = await serve({ CLIENT_IP: "cloudflare" }); + const from = (ip: string, forwarded: string) => + post("/verify", { "cf-connecting-ip": ip, "x-forwarded-for": forwarded }); + expect((await from("203.0.113.7", "1.1.1.1")).status).toBe(400); + expect((await from("203.0.113.7", "2.2.2.2")).status).toBe(400); + expect((await from("203.0.113.7", "3.3.3.3")).status).toBe(429); + expect((await from("203.0.113.8", "3.3.3.3")).status).toBe(400); + }); + + it("counts IPv6 clients per /56, so one /64 can't rotate past the budget", async () => { + const { post } = await serve({ CLIENT_IP: "cloudflare" }); + const from = (ip: string) => post("/verify", { "cf-connecting-ip": ip }); + expect((await from("2001:db8:0:1::1")).status).toBe(400); + expect((await from("2001:db8:0:2::9")).status).toBe(400); + expect((await from("2001:db8:0:ff::5")).status).toBe(429); + expect((await from("2001:db8:0:100::1")).status).toBe(400); + }); + + it("answers 500 and logs it when the configured header is missing", async () => { + const { lines, post } = await serve({ CLIENT_IP: "cloudflare" }); + const response = await post("/verify"); + expect([response.status, await response.json()]).toEqual([500, { error: "internal_error" }]); + expect(lines.map((line) => JSON.parse(line))).toContainEqual( + expect.objectContaining({ event: "client_ip_missing", source: "cloudflare" }), + ); + }); + + it("answers 500 without a store hit when the header holds anything but one address", async () => { + const { stores, hit } = spiedStores(); + const { lines, post } = await serve({ CLIENT_IP: "cloudflare" }, stores); + // A header sent twice arrives joined; a value this long wouldn't fit the store's key index. + for (const value of ["203.0.113.7, 203.0.113.8", "a".repeat(8_000), "localhost"]) { + const response = await post("/verify", { "cf-connecting-ip": value }); + expect([response.status, await response.json()]).toEqual([500, { error: "internal_error" }]); + } + expect(hit).not.toHaveBeenCalled(); + const invalid = lines + .map((line) => JSON.parse(line)) + .filter((e) => e.event === "client_ip_invalid"); + expect(invalid).toHaveLength(3); + // The value is the caller's, so the log never carries it. + expect(lines.join("")).not.toContain("aaaa"); + }); + + it("answers 500 and logs the cause when the store fails", async () => { + const failing: RateLimitStore = { + hit: async () => { + throw new Error("database down"); + }, + }; + const { lines, post } = await serve({}, { ...memoryStores(), rateLimits: failing }); + const response = await post("/settle"); + expect([response.status, await response.json()]).toEqual([500, { error: "internal_error" }]); + expect(lines.join("")).toContain("http_error"); + expect(lines.join("")).toContain("database down"); + }); + + it("answers a client over budget from memory, so its flood never reaches the store", async () => { + const { stores, hit } = spiedStores(); + const { lines, post } = await serve({ CLIENT_IP: "cloudflare" }, stores); + const from = (ip: string) => post("/verify", { "cf-connecting-ip": ip }); + for (let i = 0; i < 2; i++) expect((await from("203.0.113.7")).status).toBe(400); + for (let i = 0; i < 10; i++) expect((await from("203.0.113.7")).status).toBe(429); + // The two within budget, and the one that found it spent. + expect(hit).toHaveBeenCalledTimes(3); + expect((await from("203.0.113.8")).status).toBe(400); + expect(hit).toHaveBeenCalledTimes(4); + + // One line per client and window, not one per refused request. + const limited = lines.map((line) => JSON.parse(line)).filter((e) => e.event === "rate_limited"); + expect(limited).toEqual([ + expect.objectContaining({ route: "verify", client: "203.0.113.7", limit: 2 }), + ]); + }); + + it("logs one rate_limited line for a burst of requests already in flight", async () => { + // A store that answers after 20 ms, as a database does, so every request is in flight at once. + const memory = new MemoryRateLimitStore(); + const slow: RateLimitStore = { + hit: async (...args) => { + await new Promise((resolve) => setTimeout(resolve, 20)); + return memory.hit(...args); + }, + }; + const { lines, post } = await serve( + { CLIENT_IP: "cloudflare" }, + { ...memoryStores(), rateLimits: slow }, + ); + const burst = await Promise.all( + Array.from({ length: 20 }, () => post("/verify", { "cf-connecting-ip": "203.0.113.7" })), + ); + expect(burst.filter((response) => response.status === 429)).toHaveLength(18); + const limited = lines.map((line) => JSON.parse(line)).filter((e) => e.event === "rate_limited"); + expect(limited).toHaveLength(1); + }); + + it("logs the full address next to the /56 it counts for an IPv6 client", async () => { + const { lines, post } = await serve({ CLIENT_IP: "cloudflare" }); + for (let i = 0; i < 3; i++) await post("/verify", { "cf-connecting-ip": "2001:db8:0:1::7" }); + const limited = lines.map((line) => JSON.parse(line)).filter((e) => e.event === "rate_limited"); + expect(limited).toEqual([ + expect.objectContaining({ ip: "2001:db8:0:1::7", client: "2001:db8::/56" }), + ]); + }); + + it("counts a body over the size limit like any other request", async () => { + const { url } = await serve({ BODY_LIMIT_BYTES: "1024" }); + const big = () => + fetch(`${url}/settle`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ pad: "x".repeat(2_000) }), + }); + expect((await big()).status).toBe(413); + expect((await big()).status).toBe(413); + expect((await big()).status).toBe(429); + }); + + it("hands the store the route, the client and the configured window", async () => { + const { stores, hit } = spiedStores(); + const { post } = await serve({ CLIENT_IP: "cloudflare", RATE_LIMIT_WINDOW_MS: "5000" }, stores); + await post("/settle", { "cf-connecting-ip": "203.0.113.7" }); + expect(hit).toHaveBeenCalledWith("settle:ip:203.0.113.7", 5_000, expect.any(Number)); + }); +}); diff --git a/apps/facilitator/test/settle-pool.test.ts b/apps/facilitator/test/settle-pool.test.ts index 0f400c42..a2bd4803 100644 --- a/apps/facilitator/test/settle-pool.test.ts +++ b/apps/facilitator/test/settle-pool.test.ts @@ -96,6 +96,7 @@ function setup( ) { const config = parseConfig({ FACILITATOR_SECRET: facilitator.secret(), + CLIENT_IP: "socket", CHANNELS: channel, MAX_FEE_STROOPS: String(MAX_FEE), INCLUSION_FEE_STROOPS: String(BID), @@ -262,6 +263,7 @@ describe("main()'s composition (compose)", () => { function composed() { const config = parseConfig({ FACILITATOR_SECRET: facilitator.secret(), + CLIENT_IP: "socket", CHANNELS: channel, MAX_FEE_STROOPS: String(MAX_FEE), }); diff --git a/apps/facilitator/test/settle-route.test.ts b/apps/facilitator/test/settle-route.test.ts index bb20d2fa..64af9023 100644 --- a/apps/facilitator/test/settle-route.test.ts +++ b/apps/facilitator/test/settle-route.test.ts @@ -46,6 +46,7 @@ describe("POST /settle", () => { beforeAll(async () => { const env = { FACILITATOR_SECRET: Keypair.random().secret(), + CLIENT_IP: "socket", CHANNELS: Keypair.random().publicKey(), }; const config = { ...parseConfig(env), port: 0 }; @@ -199,6 +200,27 @@ describe("POST /settle", () => { expect(lines.join("")).toContain("extension_responses_failed"); }); + it("answers 400 for a NUL or a lone surrogate in any string or key, without settling", async () => { + // PostgreSQL refuses a NUL in text and jsonb, and jsonb a lone surrogate: a 500 before money + // moves, instead of the client's 400. + for (const text of ["a\u0000b", "a\ud800b"]) { + const bodies = [ + { ...request, paymentRequirements: { ...requirements, payTo: text } }, + { ...request, paymentPayload: { ...paymentPayload, extensions: { note: text } } }, + { ...request, paymentPayload: { ...paymentPayload, extensions: { [text]: 1 } } }, + ]; + for (const body of bodies) { + const response = await post(body); + expect([response.status, await response.json()]).toEqual([400, { error: "bad_request" }]); + } + } + expect(settle).not.toHaveBeenCalled(); + // A pair of surrogates is one character, and well formed. + settle.mockResolvedValueOnce({ response: success, record }); + const emoji = { ...paymentPayload, extensions: { note: "\ud83d\ude00" } }; + expect((await post({ ...request, paymentPayload: emoji })).status).toBe(200); + }); + it("answers 400 for a bad body, without settling", async () => { const response = await post({ ...request, x402Version: 1 }); expect([response.status, await response.json()]).toEqual([400, { error: "bad_request" }]); diff --git a/apps/facilitator/test/verify-route.test.ts b/apps/facilitator/test/verify-route.test.ts index 0684a512..005b232b 100644 --- a/apps/facilitator/test/verify-route.test.ts +++ b/apps/facilitator/test/verify-route.test.ts @@ -36,6 +36,7 @@ describe("POST /verify", () => { beforeAll(async () => { const env = { FACILITATOR_SECRET: Keypair.random().secret(), + CLIENT_IP: "socket", CHANNELS: Keypair.random().publicKey(), }; const config = { ...parseConfig(env), port: 0 }; @@ -164,6 +165,31 @@ describe("POST /verify", () => { expect(verify).toHaveBeenCalledTimes(1); }); + it("answers 400 for a NUL or a lone surrogate in any string or key, without verifying", async () => { + for (const text of ["a\u0000b", "a\ud800b"]) { + const bodies = [ + { ...request, paymentRequirements: { ...requirements, payTo: text } }, + { ...request, paymentPayload: { ...paymentPayload, extensions: { [text]: 1 } } }, + ]; + for (const body of bodies) { + const response = await post(body); + expect([response.status, await response.json()]).toEqual([400, { error: "bad_request" }]); + } + } + expect(verify).not.toHaveBeenCalled(); + }); + + it("sends no CORS headers", async () => { + verify.mockResolvedValueOnce({ isValid: true, payer: "GPAYER" }); + const response = await fetch(url, { + method: "POST", + headers: { "content-type": "application/json", origin: "https://other.example" }, + body: JSON.stringify(request), + }); + expect(response.status).toBe(200); + expect(response.headers.get("access-control-allow-origin")).toBeNull(); + }); + it("answers 500 and logs the error when verification throws", async () => { verify.mockRejectedValueOnce(new Error("rpc down")); const response = await post(request); diff --git a/deploy/mainnet.env.example b/deploy/mainnet.env.example index dd66932f..b5f275ab 100644 --- a/deploy/mainnet.env.example +++ b/deploy/mainnet.env.example @@ -9,6 +9,9 @@ FACILITATOR_SECRET= # Comma-separated channel addresses (G…), never secrets. The service refuses to start unless each # has its master key at weight 0 and the facilitator key as its only signer CHANNELS= +# Required. Where the rate limits take the client's IP from: cloudflare (the CF-Connecting-IP header, +# behind Cloudflare Tunnel) or socket (no proxy in front). Mainnet's proxy is decided in 0038 +CLIENT_IP= # Required outside testnet: settlements must survive restarts. Run `pnpm migrate` before each deploy. STORE=postgres # postgres:// URL of the migrated database (use a secret store in production; never logged) diff --git a/deploy/testnet.env.example b/deploy/testnet.env.example index 3ca65702..10440fdd 100644 --- a/deploy/testnet.env.example +++ b/deploy/testnet.env.example @@ -9,6 +9,9 @@ FACILITATOR_SECRET= # Comma-separated channel addresses (G…), never secrets. The service refuses to start unless each # has its master key at weight 0 and the facilitator key as its only signer CHANNELS= +# Required. Where the rate limits take the client's IP from: cloudflare (the CF-Connecting-IP header, +# behind Cloudflare Tunnel) or socket (no proxy in front) +CLIENT_IP=cloudflare # postgres for the public testnet (0026), since memory loses settlements, rate-limit windows and # spend reservations on restart; memory (the default) is for local runs. postgres needs # DATABASE_URL and `pnpm migrate` diff --git a/docs/architecture/m1-spine.md b/docs/architecture/m1-spine.md index 798aaba8..e7505e0b 100644 --- a/docs/architecture/m1-spine.md +++ b/docs/architecture/m1-spine.md @@ -202,8 +202,8 @@ graph TD - **Prevents:** one caller exhausting the pool or our fee budget, and limits implemented differently per route. - **Rule:** - One middleware, with separate budgets for `/verify`, `/settle` and `/discovery/*`. Exceeding a budget returns HTTP 429. - - **Client IP.** It comes from the header the edge sets (`CF-Connecting-IP` behind Cloudflare Tunnel, ADR 0011), never from a raw `X-Forwarded-For`. - - **API key.** An optional per-caller API key is accepted as an alternative limiter key. It is not required in T1. + - **Client IP.** It comes only from the header the edge sets (`CF-Connecting-IP` behind Cloudflare Tunnel, ADR 0011; `CLIENT_IP=cloudflare`), never from `X-Forwarded-For`. An IPv6 client is counted per /56. + - **API key.** An optional per-caller API key as an alternative limiter key is not built in T1 (task 0060): without a registry of issued keys, a caller's own key would bypass the limit. - **Storage.** Counters go through `RateLimitStore`. - **Payer, recipient and asset.** Limits on these are AD-18's job, not the IP limiter's. @@ -352,7 +352,7 @@ graph TD | Config | read and validated once at startup in `apps/facilitator` (zod); passed down through constructors. RPC URLs come from config, a fallback list is allowed, and public testnet RPC is acceptable for T1 | | Time and I/O in tests | `rpc`, `now` and `sleep` injected; store ports faked in memory | | Logging and metrics | structured, one event per line, carrying `network`, settlement key and the transaction hash where known; secrets and full XDR are never logged; pool `onEvent` forwarded to metrics | -| HTTP hardening | request body size limit; no CORS on `/verify` and `/settle`; trusted proxy hops set from config | +| HTTP hardening | request body size limit; no CORS on `/verify` and `/settle`; client IP header set from config | | Testing | unit tests with fakes in each package; facilitator integration tests against in-memory stores and `fake-rpc`; testnet runs (AD-9 token matrix, AD-15 gate) record their transaction hashes | | Test helpers | a package shares its fakes through a `/testing` subpath (e.g. `@stellar-x402/signer-pool/testing`); a lint rule keeps `src` from importing any of them | diff --git a/docs/planning/m1-epics.md b/docs/planning/m1-epics.md index ba0aea84..954bd064 100644 --- a/docs/planning/m1-epics.md +++ b/docs/planning/m1-epics.md @@ -77,7 +77,7 @@ guide, the two example integrations, the security review report, uptime targets, - **FR11:** Only one process at a time settles through a network's channel set (a lease), and shutdown waits until in-flight settlements are final or recorded as pending. - **FR12:** Rate limiting has separate budgets for `/verify`, `/settle` and `/discovery/*`, keyed - by client IP (from trusted proxy hops) or an optional API key, and returns HTTP 429 when a + by client IP (from the configured client IP header; the API key is task 0060), and returns HTTP 429 when a budget is exceeded. Caller authentication and rate limiting are documented. - **FR13:** Spend budgets (global, per payer, per `payTo`, per asset) and circuit breakers (per asset, per `payTo`) stop fee drain. Over budget, `/settle` returns `success: false` and submits @@ -498,11 +498,11 @@ So that one caller can't exhaust the channel pool or the service. **Given** a request with a forged `X-Forwarded-For` **When** the limiter keys it -**Then** the client IP comes only from the configured trusted proxy hops +**Then** the client IP comes only from the configured client IP header **Given** a request with an API key **When** the limiter keys it -**Then** the key is used instead of the IP +**Then** the key is used instead of the IP (task 0060) **And** requests without a key still work, so the gate proxy can call keyless **Given** the HTTP layer diff --git a/lore/1-tasks/active/0023_FEATURE_rate-limiting-and-http-hardening.md b/lore/1-tasks/active/0023_FEATURE_rate-limiting-and-http-hardening.md index 913900c4..a47ff9b4 100644 --- a/lore/1-tasks/active/0023_FEATURE_rate-limiting-and-http-hardening.md +++ b/lore/1-tasks/active/0023_FEATURE_rate-limiting-and-http-hardening.md @@ -89,19 +89,33 @@ test before its code. | # | What is done | From | vs the task | Who does the same (checked) | Code · test | | --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------- | --------------------------------------------------------------- | --------------------------------------------------------------------------- | ----------- | -| 1 | Separate budgets for `/verify`, `/settle` and `/discovery/*`, one counter per route (key prefixes `verify:`, `settle:`, `discovery:`); over budget is a 429 on that route only. The limiter runs before body parsing, so 400 and 413 answers count. `/supported` and unknown paths are not limited | task AC 1, agent | matches AC 1 | AD-11 | | -| 2 | Defaults 600 `/verify`, 600 `/settle`, 120 `/discovery/*` per 60 s; the config refuses a `/settle` budget below `/verify`'s. The seller's server calls both routes, so a budget per IP is a budget per seller, and `@x402/express` 2.28 settles after the seller's handler ran (`dist/esm/index.mjs:322-374`): a 429 on `/settle` throws away work already done. Fee drain is bounded by the spend budgets (0024), not by this limiter | grilling | changes the defaults (120/60/120) | the gate harness makes one payment per scenario (`conformance/README.md`) | | -| 3 | The client IP is the `CF-Connecting-IP` header, which Cloudflare sets and refuses from a client (measured through a quick tunnel, cloudflared 2026.10.3: a forged one answers `error code: 1000`). `CLIENT_IP_HEADER` replaces `TRUSTED_PROXY_HOPS`; unset, the IP is the socket address. Configured but missing on a request: 500 and an error log, since only `cloudflared` reaches the service | grilling (the header), agent (the config shape) | changes the implementation note (numeric `trust proxy`, hops 1) | ADR 0011 as revised by 0039 (PR #24); Cloudflare's header docs recommend it | | -| 4 | IPv6 addresses are counted per /56 (`ipKeyGenerator`, the library default), so one /64 can't rotate past the limit | agent | beyond it | express-rate-limit 8 default | | -| 5 | No API key in this task: no key registry exists, and a raw caller key gives each key its own counter, which bypasses the limit. AC 3 moves to 0060 | grilling | changes AC 3 | AD-11 and ADR 0007: optional, not required in Tranche 1 | | -| 6 | Store unreachable: the request fails with 500 `internal_error` and the cause is logged (`passOnStoreError: false`). `/settle` fails without the database anyway | grilling | decides the open option | | | -| 7 | In front of the store, the process remembers a key over its budget until its window ends and answers 429 without a database hit, so one client's flood can't hold the 3-connection `hit` pool | grilling (devil's advocate review) | decides the open option | | | -| 8 | `express-rate-limit` 8.7.0 (already in the lockfile through `@modelcontextprotocol/sdk`) with one adapter instance per limiter onto `RateLimitStore.hit`; its `decrement` and `resetKey` throw, its logger is ours | agent | matches "one middleware" | MCP SDK uses the same library | | -| 9 | A 429 answers `{"error":"too_many_requests"}` with `Retry-After`; no `X-RateLimit-*` or `RateLimit-*` headers. Upstream's client throws on a non-200 `/verify` or `/settle` and honours `Retry-After` only on `/supported` | agent | beyond it | | | -| 10 | A NUL (`\u0000`) or a lone surrogate in any string or key of a `/verify` or `/settle` body is a 400, checked in `parsePaymentRequest` | task | matches the implementation note | | | -| 11 | No CORS on any route, as today; a test pins that `/verify` and `/settle` send no CORS headers | task AC 4 | matches AC 4 | | | -| 12 | The pubnet refusal names what is still missing: spend budgets (0024), the channel lease (0025) and store pruning (0050) | grilling | adds 0050 | | | -| 13 | The facilitator README documents caller authentication (none, every route is keyless) and the limits | task AC 5 | matches AC 5 | | | +| 1 | Separate budgets for `/verify`, `/settle` and `/discovery/*`, one counter per route (key prefixes `verify:`, `settle:`, `discovery:`); over budget is a 429 on that route only. The limiter runs before body parsing, so 400 and 413 answers count. `/discovery` is limited as a prefix, every method and path under it; `/supported` and paths outside the three routes are not limited | task AC 1, agent | matches AC 1 | AD-11 | `http/app.ts`, `http/rate-limit.ts` · `rate-limit.test.ts` "answers 429 … on that route only", "limits /discovery/*" | +| 2 | Defaults 600 `/verify`, 600 `/settle`, 120 `/discovery/*` per 60 s; the config refuses a `/settle` budget below `/verify`'s. The seller's server calls both routes, so a budget per IP is a budget per seller, and `@x402/express` 2.28 settles after the seller's handler ran (`dist/esm/index.mjs:322-374`): a 429 on `/settle` throws away work already done. Fee drain is bounded by the spend budgets (0024), not by this limiter | grilling | changes the defaults (120/60/120) | the gate harness makes one payment per scenario (`conformance/README.md`) | `config.ts` · `config.test.ts` "keeps the /settle budget at or above /verify's", "applies testnet defaults" | +| 3 | The client IP is the `CF-Connecting-IP` header, which Cloudflare sets and refuses from a client (measured through a quick tunnel, cloudflared 2026.10.3: a forged one answers `error code: 1000`). `CLIENT_IP` replaces `TRUSTED_PROXY_HOPS`, is required and takes one of two values: `cloudflare` (that header) or `socket` (the connection's own address, with no proxy in front). Required and closed after review: a free header name let a forgotten setting or another header share one budget or trust a forgeable one. With `cloudflare`, a request without the header, or holding anything but one IP address: 500 and an error log without the value, and no store hit | grilling (the header), agent (the config shape) | changes the implementation note (numeric `trust proxy`, hops 1) | ADR 0011 as revised by 0039 (PR #24); Cloudflare's header docs recommend it | `config.ts`, `http/rate-limit.ts` · `rate-limit.test.ts` "keys the client by CF-Connecting-IP with CLIENT_IP=cloudflare", "ignores X-Forwarded-For", "answers 500 … header is missing", "answers 500 without a store hit when the header holds anything but one address"; `config.test.ts` "requires CLIENT_IP …", "refuses a leftover TRUSTED_PROXY_HOPS" | +| 4 | IPv6 addresses are counted per /56 (`ipKeyGenerator`, the library default), so one /64 can't rotate past the limit | agent | beyond it | express-rate-limit 8 default | `http/rate-limit.ts` · `rate-limit.test.ts` "counts IPv6 clients per /56" | +| 5 | No API key in this task: no key registry exists, and a raw caller key gives each key its own counter, which bypasses the limit. AC 3 moves to 0060 | grilling | changes AC 3 | AD-11 and ADR 0007: optional, not required in Tranche 1 | — · — | +| 6 | Store unreachable: the request fails with 500 `internal_error` and the cause is logged: the middleware's rejection reaches `onError`. `/settle` fails without the database anyway | grilling | decides the open option | | `http/rate-limit.ts` (Express answers the rejection) · `rate-limit.test.ts` "answers 500 and logs the cause when the store fails" | +| 7 | In front of the store, the process remembers a key over its budget until its window ends and answers 429 without a database hit, so one client's flood can't hold the 3-connection `hit` pool | grilling (devil's advocate review) | decides the open option | | `http/rate-limit.ts` · `rate-limit.test.ts` "answers a client over budget from memory" | +| 8 | Our own middleware of about 40 lines, with only `ipKeyGenerator` from `express-rate-limit` 8.7.0 (already in the lockfile through `@modelcontextprotocol/sdk`). The library's middleware needed an adapter onto `RateLimitStore` with `decrement` and `resetKey` that throw, one instance per limiter (`ERR_ERL_STORE_REUSE`), its headers off and its logger rerouted: more code than the counting it would replace | agent | matches "one middleware" | MCP SDK uses the same library | `http/rate-limit.ts` · all of `rate-limit.test.ts` | +| 9 | A 429 answers `{"error":"too_many_requests"}` with `Retry-After`; no `X-RateLimit-*` or `RateLimit-*` headers. Upstream's client throws on a non-200 `/verify` or `/settle` and honours `Retry-After` only on `/supported` | agent | beyond it | | `http/rate-limit.ts` · `rate-limit.test.ts` "answers 429 with Retry-After …" | +| 10 | A NUL (`\u0000`) or a lone surrogate in any string or key of a `/verify` or `/settle` body is a 400, checked in `parsePaymentRequest` | task | matches the implementation note | | `http/app.ts` · `verify-route.test.ts`, `settle-route.test.ts` "answers 400 for a NUL or a lone surrogate …" | +| 11 | No CORS on any route, as today; a test pins that `/verify` and `/settle` send no CORS headers | task AC 4 | matches AC 4 | | — · `verify-route.test.ts`, `settle-route.test.ts` "sends no CORS headers" | +| 12 | The pubnet refusal names what is still missing: spend budgets (0024), the channel lease (0025) and store pruning (0050) | grilling | adds 0050 | | `config.ts` · `config.test.ts`, `main.test.ts` pubnet refusal | +| 13 | The facilitator README documents caller authentication (none, every route is keyless) and the limits | task AC 5 | matches AC 5 | | `README.md` "Callers and rate limits" · — | + +Decision 7, measured on a local PostgreSQL 14 with the `hit` pool of 3: one client sends a burst +over 100 connections against a budget of 10, while 10 other clients each send one request on their +own connections. + +| Burst | With the in-process filter | Without it | +| ------ | -------------------------------- | ----------------------------------- | +| 2 000 | 25 store hits, others ≤ 267 ms | 2 010 store hits, others ≤ 110 ms | +| 20 000 | 34 store hits, others ≤ 312 ms | 20 010 store hits, others ≤ 114 ms | + +No errors either way: a local database keeps up with one write per request, so the filter isn't +needed to keep other clients answered here. It takes a flood's writes off the database (about +600 times fewer), which shares its server with the settlements. The other clients' time is the +event loop answering the burst, not the store. ## Not yet decided diff --git a/lore/1-tasks/backlog/0060_FEATURE_api-key-limiter.md b/lore/1-tasks/backlog/0060_FEATURE_api-key-limiter.md index 33678dd7..b576c90a 100644 --- a/lore/1-tasks/backlog/0060_FEATURE_api-key-limiter.md +++ b/lore/1-tasks/backlog/0060_FEATURE_api-key-limiter.md @@ -25,6 +25,11 @@ left it out: no key registry exists, and a raw caller-supplied key gives each ke which bypasses the limit. Pick this up when a caller needs a budget apart from its IP, such as sellers sharing one egress IP. +Needed before the public testnet (0026): every seller on Cloudflare Workers reaches a facilitator +behind Cloudflare with the same `CF-Connecting-IP`, `2a06:98c0:3600::103` (Cloudflare's HTTP +request headers reference, cross-zone Worker subrequests), so they all share one budget. Sellers +behind one cloud NAT share theirs the same way. + ## Implementation - `API_KEYS` in the config: SHA-256 hashes of the issued keys, never the keys. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3274d118..d2b62456 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -68,6 +68,9 @@ importers: express: specifier: ^5.2.1 version: 5.2.1 + express-rate-limit: + specifier: ^8.7.0 + version: 8.7.0(express@5.2.1) pg: specifier: ^8.23.1 version: 8.23.1