diff --git a/server/src/__tests__/invite-rate-limit-route.test.ts b/server/src/__tests__/invite-rate-limit-route.test.ts new file mode 100644 index 0000000000..9cfae606cd --- /dev/null +++ b/server/src/__tests__/invite-rate-limit-route.test.ts @@ -0,0 +1,120 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createInviteRateLimiter } from "../services/invite-rate-limit.js"; + +function createSelectChain(rows: unknown[]) { + const query = { + then(resolve: (value: unknown[]) => unknown) { + return Promise.resolve(rows).then(resolve); + }, + leftJoin() { + return query; + }, + orderBy() { + return query; + }, + where() { + return query; + }, + }; + return { + from() { + return query; + }, + }; +} + +function createDbStub(...selectResponses: unknown[][]) { + let selectCall = 0; + return { + select() { + const rows = selectResponses[selectCall] ?? []; + selectCall += 1; + return createSelectChain(rows); + }, + }; +} + +async function createApp(db: Record) { + const [{ accessRoutes }, { errorHandler }] = await Promise.all([ + import("../routes/access.js"), + import("../middleware/index.js"), + ]); + const app = express(); + app.use((req, _res, next) => { + (req as any).actor = { type: "anon" }; + next(); + }); + app.use( + "/api", + accessRoutes(db as any, { + deploymentMode: "local_trusted", + deploymentExposure: "private", + bindHost: "127.0.0.1", + allowedHostnames: [], + inviteRateLimiter: createInviteRateLimiter({ + maxRequests: 1, + windowMs: 60_000, + now: () => 1_000, + }), + }), + ); + app.use(errorHandler); + return app; +} + +describe("invite-token endpoint rate limiting", () => { + beforeEach(() => { + vi.resetModules(); + }); + + it("returns 429 once the per-IP threshold is exceeded", async () => { + // No invite row -> route would 404, but the rate-limit middleware runs first + // and short-circuits on the second request. + const app = await createApp(createDbStub([], [], [], [], [])); + + const first = await request(app).get( + "/api/invites/pcp_invite_aaaaaaaaaaaaaaaaaaaaaa", + ); + expect(first.status).toBe(404); + + const limited = await request(app).get( + "/api/invites/pcp_invite_aaaaaaaaaaaaaaaaaaaaaa", + ); + expect(limited.status).toBe(429); + expect(limited.headers["retry-after"]).toBe("60"); + expect(limited.body).toMatchObject({ + error: "Too many invite requests", + details: { retryAfterSeconds: 60 }, + }); + }); + + it("also rate-limits the accept sub-route", async () => { + const app = await createApp(createDbStub([], [], [], [], [])); + + await request(app).get("/api/invites/pcp_invite_bbbbbbbbbbbbbbbbbbbbbb"); + + const limited = await request(app) + .post("/api/invites/pcp_invite_bbbbbbbbbbbbbbbbbbbbbb/accept") + .send({}); + expect(limited.status).toBe(429); + }); + + it("ignores client-supplied X-Forwarded-For — spoofed IPs do not reset the budget", async () => { + // `trust proxy` is unset here (Express default: trust nothing), so the + // rate-limit key is the socket's remote address. Rotating fake + // X-Forwarded-For values must NOT mint a fresh per-IP budget. + const app = await createApp(createDbStub([], [], [], [], [])); + + const first = await request(app) + .get("/api/invites/pcp_invite_cccccccccccccccccccccc") + .set("x-forwarded-for", "1.1.1.1"); + expect(first.status).toBe(404); + + const spoofed = await request(app) + .get("/api/invites/pcp_invite_cccccccccccccccccccccc") + .set("x-forwarded-for", "1.1.1.2"); + expect(spoofed.status).toBe(429); + }); +}); diff --git a/server/src/__tests__/invite-rate-limit.test.ts b/server/src/__tests__/invite-rate-limit.test.ts new file mode 100644 index 0000000000..b6ec6a4719 --- /dev/null +++ b/server/src/__tests__/invite-rate-limit.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from "vitest"; +import { createInviteRateLimiter } from "../services/invite-rate-limit.js"; + +describe("createInviteRateLimiter", () => { + it("allows requests up to the limit then blocks with a retry-after", () => { + const limiter = createInviteRateLimiter({ + maxRequests: 3, + windowMs: 60_000, + now: () => 1_000, + }); + + expect(limiter.consume("1.2.3.4").allowed).toBe(true); + expect(limiter.consume("1.2.3.4").allowed).toBe(true); + expect(limiter.consume("1.2.3.4").allowed).toBe(true); + + const blocked = limiter.consume("1.2.3.4"); + expect(blocked.allowed).toBe(false); + expect(blocked.remaining).toBe(0); + expect(blocked.retryAfterSeconds).toBe(60); + }); + + it("tracks each IP independently", () => { + const limiter = createInviteRateLimiter({ + maxRequests: 1, + windowMs: 60_000, + now: () => 1_000, + }); + + expect(limiter.consume("1.1.1.1").allowed).toBe(true); + expect(limiter.consume("1.1.1.1").allowed).toBe(false); + expect(limiter.consume("2.2.2.2").allowed).toBe(true); + }); + + it("forgets hits once the window has elapsed", () => { + let current = 1_000; + const limiter = createInviteRateLimiter({ + maxRequests: 1, + windowMs: 60_000, + now: () => current, + }); + + expect(limiter.consume("9.9.9.9").allowed).toBe(true); + expect(limiter.consume("9.9.9.9").allowed).toBe(false); + current += 60_001; + expect(limiter.consume("9.9.9.9").allowed).toBe(true); + }); +}); diff --git a/server/src/__tests__/invite-token-entropy.test.ts b/server/src/__tests__/invite-token-entropy.test.ts new file mode 100644 index 0000000000..c6370392b4 --- /dev/null +++ b/server/src/__tests__/invite-token-entropy.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { createInviteToken } from "../routes/access.js"; + +const PREFIX = "pcp_invite_"; + +describe("createInviteToken", () => { + it("keeps the human-readable pcp_invite_ prefix", () => { + expect(createInviteToken().startsWith(PREFIX)).toBe(true); + }); + + it("carries at least 128 bits of randomness", () => { + const suffix = createInviteToken().slice(PREFIX.length); + // base64url over 32 random bytes => 43 chars, 256 bits of entropy. Each + // base64url char is 6 bits, so >= 22 chars guarantees >= 128 bits. + expect(suffix.length).toBeGreaterThanOrEqual(22); + expect(suffix).toMatch(/^[A-Za-z0-9_-]+$/); + const bits = suffix.length * 6; + expect(bits).toBeGreaterThanOrEqual(128); + }); + + it("produces unique, non-repeating tokens", () => { + const tokens = new Set( + Array.from({ length: 1000 }, () => createInviteToken()), + ); + expect(tokens.size).toBe(1000); + }); +}); diff --git a/server/src/errors.ts b/server/src/errors.ts index e502d7e8bc..6abad9c7ee 100644 --- a/server/src/errors.ts +++ b/server/src/errors.ts @@ -32,3 +32,7 @@ export function conflict(message: string, details?: unknown) { export function unprocessable(message: string, details?: unknown) { return new HttpError(422, message, details); } + +export function tooManyRequests(message = "Too many requests", details?: unknown) { + return new HttpError(429, message, details); +} diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index b341c71780..39889d19b2 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -53,8 +53,13 @@ import { conflict, notFound, unauthorized, - badRequest + badRequest, + tooManyRequests } from "../errors.js"; +import { + createInviteRateLimiter, + type InviteRateLimiter, +} from "../services/invite-rate-limit.js"; import { logger } from "../middleware/logger.js"; import { validate } from "../middleware/validate.js"; import { collectReachableInterfaceHosts } from "../runtime-api.js"; @@ -90,8 +95,12 @@ function hashToken(token: string) { } const INVITE_TOKEN_PREFIX = "pcp_invite_"; -const INVITE_TOKEN_ALPHABET = "abcdefghijklmnopqrstuvwxyz0123456789"; -const INVITE_TOKEN_SUFFIX_LENGTH = 8; +// 32 random bytes = 256 bits of entropy, base64url-encoded (43 chars). The +// invite token is public (anyone with the link can GET/accept the invite), so +// it must not be brute-forceable. The previous 8-char base36 suffix carried only +// ~41 bits, which is online-enumerable. The token is stored hashed (sha256) in +// `invites.tokenHash`; the raw value is only returned once on creation. +const INVITE_TOKEN_ENTROPY_BYTES = 32; const INVITE_TOKEN_MAX_RETRIES = 5; const COMPANY_INVITE_TTL_MS = 72 * 60 * 60 * 1000; const INVITE_RESOLUTION_DNS_TIMEOUT_MS = 3_000; @@ -101,12 +110,8 @@ type MemberGrantPayload = { scope?: Record | null; }; -function createInviteToken() { - const bytes = randomBytes(INVITE_TOKEN_SUFFIX_LENGTH); - let suffix = ""; - for (let idx = 0; idx < INVITE_TOKEN_SUFFIX_LENGTH; idx += 1) { - suffix += INVITE_TOKEN_ALPHABET[bytes[idx]! % INVITE_TOKEN_ALPHABET.length]; - } +export function createInviteToken() { + const suffix = randomBytes(INVITE_TOKEN_ENTROPY_BYTES).toString("base64url"); return `${INVITE_TOKEN_PREFIX}${suffix}`; } @@ -2598,6 +2603,7 @@ export function accessRoutes( bindHost: string; allowedHostnames: string[]; inviteResolutionNetwork?: Partial; + inviteRateLimiter?: InviteRateLimiter; } ) { const router = Router(); @@ -2608,6 +2614,37 @@ export function accessRoutes( ? { ...defaultInviteResolutionNetwork, ...opts.inviteResolutionNetwork } : inviteResolutionNetwork; + // Per-IP rate limit for the public, unauthenticated invite-token endpoints + // (`/invites/:token*`). The token is looked up by hash, so without a limit the + // token space would be online-enumerable. Applied as a router-level middleware + // so every current and future `/invites/:token` sub-route is covered. + // + // The key is deliberately NOT `requestIp()`: that helper prefers the + // client-supplied `X-Forwarded-For` header (fine for log/audit fields, + // but trivially spoofable as a rate-limit key — rotating fake XFF values + // would mint a fresh budget per request). `req.ip` honors Express's + // `trust proxy` setting (configured from TRUST_PROXY in app.ts, default: + // trust nothing), so it is the socket's remote address unless the + // operator explicitly trusts a proxy — an unforgeable key either way. + const inviteRateLimiter = opts.inviteRateLimiter ?? createInviteRateLimiter(); + router.use("/invites/:token", (req, res, next) => { + const result = inviteRateLimiter.consume( + req.ip || req.socket?.remoteAddress || "unknown", + ); + res.setHeader("X-RateLimit-Limit", String(result.limit)); + res.setHeader("X-RateLimit-Remaining", String(result.remaining)); + if (!result.allowed) { + res.setHeader("Retry-After", String(result.retryAfterSeconds)); + next( + tooManyRequests("Too many invite requests", { + retryAfterSeconds: result.retryAfterSeconds, + }), + ); + return; + } + next(); + }); + async function assertInstanceAdmin(req: Request) { if (req.actor.type !== "board") throw unauthorized(); if (isLocalImplicit(req)) return; diff --git a/server/src/services/invite-rate-limit.ts b/server/src/services/invite-rate-limit.ts new file mode 100644 index 0000000000..e89afc522b --- /dev/null +++ b/server/src/services/invite-rate-limit.ts @@ -0,0 +1,79 @@ +// Generic per-IP sliding-window rate limiter for the public, unauthenticated +// invite-token endpoints (`/invites/:token*`). These routes accept a token in +// the URL and look it up by hash, so without a limit the token space is +// online-enumerable. The limiter is in-memory and therefore per-process; for a +// horizontally-scaled deployment it bounds enumeration per instance, which is +// the meaningful protection here (a brute-force still has to defeat the limit on +// every replica). It intentionally has no external dependency. + +export const INVITE_RATE_LIMIT_WINDOW_MS = 60_000; +export const INVITE_RATE_LIMIT_MAX_REQUESTS = 20; + +export type InviteRateLimitResult = { + allowed: boolean; + limit: number; + remaining: number; + retryAfterSeconds: number; +}; + +export type InviteRateLimiter = { + consume(ip: string): InviteRateLimitResult; +}; + +export function createInviteRateLimiter(options: { + windowMs?: number; + maxRequests?: number; + now?: () => number; +} = {}): InviteRateLimiter { + const windowMs = options.windowMs ?? INVITE_RATE_LIMIT_WINDOW_MS; + const maxRequests = options.maxRequests ?? INVITE_RATE_LIMIT_MAX_REQUESTS; + const now = options.now ?? Date.now; + const hitsByKey = new Map(); + let lastSweep = 0; + + // Periodically drop keys whose hits have all aged out so a stream of unique + // spoofable IPs can't grow the map without bound. + function sweep(currentTime: number) { + if (currentTime - lastSweep < windowMs) return; + lastSweep = currentTime; + const cutoff = currentTime - windowMs; + for (const [key, hits] of hitsByKey) { + const recent = hits.filter((hit) => hit > cutoff); + if (recent.length === 0) hitsByKey.delete(key); + else hitsByKey.set(key, recent); + } + } + + return { + consume(ip: string) { + const currentTime = now(); + sweep(currentTime); + const cutoff = currentTime - windowMs; + const key = ip || "unknown"; + const recentHits = (hitsByKey.get(key) ?? []).filter((hit) => hit > cutoff); + + if (recentHits.length >= maxRequests) { + const oldestHit = recentHits[0] ?? currentTime; + hitsByKey.set(key, recentHits); + return { + allowed: false, + limit: maxRequests, + remaining: 0, + retryAfterSeconds: Math.max( + 1, + Math.ceil((oldestHit + windowMs - currentTime) / 1000), + ), + }; + } + + recentHits.push(currentTime); + hitsByKey.set(key, recentHits); + return { + allowed: true, + limit: maxRequests, + remaining: Math.max(0, maxRequests - recentHits.length), + retryAfterSeconds: 0, + }; + }, + }; +} diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index 6984f42974..f2798cc634 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -865,19 +865,16 @@ export function buildHostServices( }; const INVITE_TOKEN_PREFIX = "pcp_invite_"; - const INVITE_TOKEN_ALPHABET = "abcdefghijklmnopqrstuvwxyz0123456789"; - const INVITE_TOKEN_SUFFIX_LENGTH = 8; + // 256 bits of entropy, base64url-encoded. Keep in sync with createInviteToken + // in routes/access.ts. The token is public, so it must not be brute-forceable. + const INVITE_TOKEN_ENTROPY_BYTES = 32; const INVITE_TOKEN_MAX_RETRIES = 5; const COMPANY_INVITE_TTL_MS = 72 * 60 * 60 * 1000; const hashToken = (token: string) => createHash("sha256").update(token).digest("hex"); const createInviteToken = () => { - const bytes = randomBytes(INVITE_TOKEN_SUFFIX_LENGTH); - let suffix = ""; - for (let idx = 0; idx < INVITE_TOKEN_SUFFIX_LENGTH; idx += 1) { - suffix += INVITE_TOKEN_ALPHABET[bytes[idx]! % INVITE_TOKEN_ALPHABET.length]; - } + const suffix = randomBytes(INVITE_TOKEN_ENTROPY_BYTES).toString("base64url"); return `${INVITE_TOKEN_PREFIX}${suffix}`; };