From 01715c7d6dca8a85ac03a840416876b1b271f5b7 Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Mon, 1 Jun 2026 15:05:49 +0300 Subject: [PATCH] feat(api): per-user rate limits on admin account endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the L1 finding from the final security review. The six admin-class write endpoints under /api/account had no rate limits, so a compromised admin session (or a script run in a loop) could spam invite creation / role flips / member removes fast enough to flood the DB or audit logs. Adds a new `RATE_LIMITS.adminAction` bucket — 30/min/user — and wires it into: - POST /api/account/invitations → admin:inviteCreate - DELETE /api/account/invitations/[id] → admin:inviteRevoke - PATCH /api/account/members/[userId] → admin:memberRole - DELETE /api/account/members/[userId] → admin:memberRemove - POST /api/account/transfer-ownership → admin:transferOwnership - PATCH /api/account → admin:rename Each endpoint keys its own bucket so spamming one doesn't starve another. 30/min is comfortably above any realistic human pace (the Members tab is a clicks-only UI; a legit admin would struggle to hit ten of these per minute by hand) while still bounding accidental abuse. The bucket joins the existing send / broadcast / react / invitationPeek / invitationRedeem buckets in `RATE_LIMITS` — same in-memory fixed-window limiter, same `rateLimitResponse` helper for the 429 shape. Severity in the final review: LOW. The admin-trust model means this isn't blocking — but the cost of adding it is six near-identical 4-line additions and zero net risk, so worth landing for hardening. Verification - `npm run lint` → 0 errors (same 19 pre-existing warnings) - `npm run typecheck` → clean - `npm test` → 366/366 pass Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/account/invitations/[id]/route.ts | 12 ++++++++++++ src/app/api/account/invitations/route.ts | 15 +++++++++++++++ src/app/api/account/members/[userId]/route.ts | 19 +++++++++++++++++++ src/app/api/account/route.ts | 15 +++++++++++++++ .../api/account/transfer-ownership/route.ts | 15 +++++++++++++++ src/lib/rate-limit.ts | 7 +++++++ 6 files changed, 83 insertions(+) diff --git a/src/app/api/account/invitations/[id]/route.ts b/src/app/api/account/invitations/[id]/route.ts index 7679b8a..117acae 100644 --- a/src/app/api/account/invitations/[id]/route.ts +++ b/src/app/api/account/invitations/[id]/route.ts @@ -16,6 +16,11 @@ import { NextResponse } from "next/server"; import { requireRole, toErrorResponse } from "@/lib/auth/account"; +import { + checkRateLimit, + rateLimitResponse, + RATE_LIMITS, +} from "@/lib/rate-limit"; export async function DELETE( _request: Request, @@ -23,6 +28,13 @@ export async function DELETE( ) { try { const ctx = await requireRole("admin"); + + const limit = checkRateLimit( + `admin:inviteRevoke:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const { id } = await params; // No `eq('account_id', ctx.accountId)` — the RLS policy diff --git a/src/app/api/account/invitations/route.ts b/src/app/api/account/invitations/route.ts index 5ce3de5..76affed 100644 --- a/src/app/api/account/invitations/route.ts +++ b/src/app/api/account/invitations/route.ts @@ -27,6 +27,11 @@ import { inviteUrl, } from "@/lib/auth/invitations"; import { isAccountRole } from "@/lib/auth/roles"; +import { + checkRateLimit, + rateLimitResponse, + RATE_LIMITS, +} from "@/lib/rate-limit"; // Resolve the base URL we publish invite links under. // @@ -115,6 +120,16 @@ export async function POST(request: Request) { try { const ctx = await requireRole("admin"); + // 30/min per user. The Members tab is a clicks-only UI so any + // legitimate admin is far below this; the cap exists to keep + // a script run in a loop or a compromised admin session from + // flooding `account_invitations` with rows. + const limit = checkRateLimit( + `admin:inviteCreate:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const body = (await request.json().catch(() => null)) as | { role?: unknown; expiresInDays?: unknown; label?: unknown } | null; diff --git a/src/app/api/account/members/[userId]/route.ts b/src/app/api/account/members/[userId]/route.ts index 2ecc912..253123e 100644 --- a/src/app/api/account/members/[userId]/route.ts +++ b/src/app/api/account/members/[userId]/route.ts @@ -19,6 +19,11 @@ import type { PostgrestError } from "@supabase/supabase-js"; import { requireRole, toErrorResponse } from "@/lib/auth/account"; import { isAccountRole } from "@/lib/auth/roles"; +import { + checkRateLimit, + rateLimitResponse, + RATE_LIMITS, +} from "@/lib/rate-limit"; // Map known SQLSTATEs from the RPCs (see migration 018) onto HTTP // statuses. The `error.code` field is the SQLSTATE; the `message` @@ -43,6 +48,13 @@ export async function PATCH( ) { try { const ctx = await requireRole("admin"); + + const limit = checkRateLimit( + `admin:memberRole:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const { userId } = await params; const body = (await request.json().catch(() => null)) as @@ -88,6 +100,13 @@ export async function DELETE( ) { try { const ctx = await requireRole("admin"); + + const limit = checkRateLimit( + `admin:memberRemove:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const { userId } = await params; const { data, error } = await ctx.supabase.rpc("remove_account_member", { diff --git a/src/app/api/account/route.ts b/src/app/api/account/route.ts index 8759ba7..2f4cf81 100644 --- a/src/app/api/account/route.ts +++ b/src/app/api/account/route.ts @@ -18,6 +18,11 @@ import { getCurrentAccount, toErrorResponse, } from "@/lib/auth/account"; +import { + checkRateLimit, + rateLimitResponse, + RATE_LIMITS, +} from "@/lib/rate-limit"; export async function GET() { try { @@ -37,6 +42,16 @@ export async function PATCH(request: Request) { try { const ctx = await requireRole("admin"); + // Per-user limit on admin-class mutations. Bounds accidental + // abuse (script run in a loop) and a compromised admin session + // spamming renames. Each admin endpoint keys its own bucket so + // one route doesn't starve another. + const limit = checkRateLimit( + `admin:rename:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const body = (await request.json().catch(() => null)) as | { name?: unknown } | null; diff --git a/src/app/api/account/transfer-ownership/route.ts b/src/app/api/account/transfer-ownership/route.ts index 89831c2..c84b839 100644 --- a/src/app/api/account/transfer-ownership/route.ts +++ b/src/app/api/account/transfer-ownership/route.ts @@ -22,6 +22,11 @@ import { NextResponse } from "next/server"; import type { PostgrestError } from "@supabase/supabase-js"; import { requireRole, toErrorResponse } from "@/lib/auth/account"; +import { + checkRateLimit, + rateLimitResponse, + RATE_LIMITS, +} from "@/lib/rate-limit"; function rpcErrorToResponse(err: PostgrestError): NextResponse { if (err.code === "42501") { @@ -54,6 +59,16 @@ export async function POST(request: Request) { // on the obvious "admin trying to transfer" case. const ctx = await requireRole("owner"); + // Rate-limit owner-only transfers. Legitimate use is one click + // every few months at most; a script run in a loop would + // produce a noisy audit trail. 30/min is well above any human + // pace and bounds the noise. + const limit = checkRateLimit( + `admin:transferOwnership:${ctx.userId}`, + RATE_LIMITS.adminAction, + ); + if (!limit.success) return rateLimitResponse(limit); + const body = (await request.json().catch(() => null)) as | { newOwnerUserId?: unknown } | null; diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 90daecf..9fc737e 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -134,6 +134,13 @@ export const RATE_LIMITS = { * successful redemption mutates two profiles and an invite row, so * the abuse surface is "spam join attempts." */ invitationRedeem: { limit: 10, windowMs: 60_000 }, + /** Admin-only account / member-management actions: create/revoke + * invitation, rename account, change member role, remove member, + * transfer ownership. 30/min per user is comfortably above any + * realistic legitimate use (the Members tab is a clicks-only UI) + * while still bounding accidental abuse from a script run in a + * loop or a compromised admin session spamming role flips. */ + adminAction: { limit: 30, windowMs: 60_000 }, } as const; /** Test-only helper. Clears the in-memory state so unit tests don't