From dec24feaa059baadf1a3c74aa99afc398b09a360 Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Fri, 10 Jul 2026 17:30:24 +0200 Subject: [PATCH] fix(web-next): enforce manifest rate limits on the production path bindAllProduction injected NoopRateLimit, so the sign-in budgets the auth manifest declares were never enforced in production (audit finding A4/B3). The production ctx now binds InMemoryRateLimit seeded from the manifest's rateLimit budgets (manifest stays the source of truth); dev-seed intentionally keeps Noop so local iteration never throttles. A regression test drives sign-in through the REAL auth production binder + app router and asserts the 6th failed attempt returns TOO_MANY_REQUESTS while other IPs stay unaffected. In-memory counters are per-process; multi-instance deployments need a shared IRateLimit backend. Co-Authored-By: Claude Fable 5 --- .../server/bind-production.rate-limit.test.ts | 97 +++++++++++++++++++ .../src/server/bind-production.test.ts | 30 ++++++ apps/web-next/src/server/bind-production.ts | 27 +++++- 3 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 apps/web-next/src/server/bind-production.rate-limit.test.ts diff --git a/apps/web-next/src/server/bind-production.rate-limit.test.ts b/apps/web-next/src/server/bind-production.rate-limit.test.ts new file mode 100644 index 0000000..36eebc2 --- /dev/null +++ b/apps/web-next/src/server/bind-production.rate-limit.test.ts @@ -0,0 +1,97 @@ +// A4/B3 regression: the PRODUCTION binder must enforce the auth manifest's +// rate-limit budgets. Unlike bind-production.test.ts (which mocks every +// feature binder), this file runs the REAL auth production binder against a +// stubbed Payload local API and drives sign-in through the app router. +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@repo/core-cms", () => ({ default: Promise.resolve({}) })); + +const payloadStub = vi.hoisted(() => ({ + secret: "test-secret", + jobs: { queue: vi.fn() }, + // No user ever matches → every sign-in fails and consumes budget. + find: vi.fn(async () => ({ docs: [] })), + findByID: vi.fn(async () => null), + create: vi.fn(async ({ data }: { data: Record }) => data), +})); +vi.mock("payload", () => ({ getPayload: vi.fn(async () => payloadStub) })); + +// Other features are irrelevant here — mock their binders so this test only +// boots the auth production path. +vi.mock("@repo/blog/di/bind-production", () => ({ + bindProductionBlog: vi.fn(), +})); +vi.mock("@repo/marketing-pages/di/bind-production", () => ({ + bindProductionMarketingPages: vi.fn(), +})); +vi.mock("@repo/navigation/di/bind-production", () => ({ + bindProductionNavigation: vi.fn(), +})); +vi.mock("@repo/media/di/bind-production", () => ({ + bindProductionMedia: vi.fn(), +})); + +describe("bindAllProduction rate limiting (A4/B3)", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + }); + + it("returns TOO_MANY_REQUESTS once the manifest ip budget is exhausted", async () => { + const { bindAllProduction } = await import("./bind-production"); + await bindAllProduction(); + + const { appRouter } = await import("@repo/core-api"); + const { authManifest } = await import("@repo/auth"); + const caller = appRouter.createCaller({ clientIp: "203.0.113.7" }); + + const attempt = () => + caller.auth.signIn({ username: "ghost", password: "wrong-password" }); + + // The manifest is the budget's source of truth: 5 failed attempts pass + // through (UNAUTHORIZED), the 6th trips the ip bucket. + const ipBudget = authManifest.useCases.signIn.rateLimit.find( + (b) => b.name === "ip", + ); + expect(ipBudget).toBeDefined(); + for (let i = 0; i < ipBudget!.budget; i++) { + await expect(attempt()).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + } + await expect(attempt()).rejects.toMatchObject({ + code: "TOO_MANY_REQUESTS", + }); + }); + + it("keeps other client IPs unaffected by an exhausted bucket", async () => { + const { bindAllProduction } = await import("./bind-production"); + await bindAllProduction(); + + const { appRouter } = await import("@repo/core-api"); + const { authManifest } = await import("@repo/auth"); + + const throttled = appRouter.createCaller({ clientIp: "198.51.100.9" }); + const ipBudget = authManifest.useCases.signIn.rateLimit.find( + (b) => b.name === "ip", + )!; + for (let i = 0; i < ipBudget.budget; i++) { + await expect( + throttled.auth.signIn({ + username: `user${i}x`, + password: "wrong-password", + }), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + } + await expect( + throttled.auth.signIn({ username: "user0x", password: "wrong-password" }), + ).rejects.toMatchObject({ code: "TOO_MANY_REQUESTS" }); + + // A different IP still gets an ordinary auth failure, not a throttle. + const fresh = appRouter.createCaller({ clientIp: "192.0.2.55" }); + await expect( + fresh.auth.signIn({ + username: "someoneelse", + password: "wrong-password", + }), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + }); +}); diff --git a/apps/web-next/src/server/bind-production.test.ts b/apps/web-next/src/server/bind-production.test.ts index 2b153a93..216b7d5 100644 --- a/apps/web-next/src/server/bind-production.test.ts +++ b/apps/web-next/src/server/bind-production.test.ts @@ -91,6 +91,25 @@ describe("bindAllProduction", () => { expect(ctx.bus).toBeUndefined(); expect(ctx.queue).toBeInstanceOf(PayloadJobQueue); }); + + it("binds a real InMemoryRateLimit seeded from the auth manifest (A4)", async () => { + const { bindAllProduction } = await import("./bind-production"); + const { bindProductionAuth } = + await import("@repo/auth/di/bind-production"); + const { InMemoryRateLimit } = await import("@repo/core-shared/rate-limit"); + + await bindAllProduction(); + + const ctx = vi.mocked(bindProductionAuth).mock.calls[0]![0]; + expect(ctx.rateLimit).toBeInstanceOf(InMemoryRateLimit); + // The manifest budgets must be resolvable — an unknown budget throws. + await expect(ctx.rateLimit!.consume("ip", "smoke")).resolves.toMatchObject({ + allowed: true, + }); + await expect( + ctx.rateLimit!.consume("account", "smoke"), + ).resolves.toMatchObject({ allowed: true }); + }); }); describe("bindAllDevSeed", () => { @@ -110,6 +129,17 @@ describe("bindAllDevSeed", () => { expect(ctx.bus).toBeUndefined(); expect(ctx.queue).toBeInstanceOf(InMemoryJobQueue); }); + + it("keeps the no-op rate limiter on the dev-seed path (never throttles locally)", async () => { + const { bindAllDevSeed } = await import("./bind-production"); + const { bindDevSeedAuth } = await import("@repo/auth/di/bind-dev-seed"); + const { NoopRateLimit } = await import("@repo/core-shared/rate-limit"); + + await bindAllDevSeed(); + + const ctx = vi.mocked(bindDevSeedAuth).mock.calls[0]![0]; + expect(ctx.rateLimit).toBeInstanceOf(NoopRateLimit); + }); }); describe("bindAll dispatcher", () => { diff --git a/apps/web-next/src/server/bind-production.ts b/apps/web-next/src/server/bind-production.ts index 4dce47a..1e97eba 100644 --- a/apps/web-next/src/server/bind-production.ts +++ b/apps/web-next/src/server/bind-production.ts @@ -16,7 +16,12 @@ import { PayloadJobQueue, type IJobQueue, } from "@repo/core-shared/jobs"; -import { NoopRateLimit } from "@repo/core-shared/rate-limit"; +import { + InMemoryRateLimit, + NoopRateLimit, + type RateLimitBudget, +} from "@repo/core-shared/rate-limit"; +import { authManifest } from "@repo/auth"; import { bindProductionBlog } from "@repo/blog/di/bind-production"; import { bindProductionAuth } from "@repo/auth/di/bind-production"; import { bindProductionMarketingPages } from "@repo/marketing-pages/di/bind-production"; @@ -81,6 +86,18 @@ function resolveJobsDevSeed(): { queue: IJobQueue } { return { queue }; } +/** + * Collect every per-use-case rate-limit budget declared in the feature + * manifests. Budgets are the manifests' source of truth (auth declares + * signIn ip/account budgets today); add further manifests here as features + * declare `rateLimit` entries. + */ +function collectManifestRateLimitBudgets(): RateLimitBudget[] { + return Object.values(authManifest.useCases).flatMap((useCase) => + "rateLimit" in useCase && useCase.rateLimit ? [...useCase.rateLimit] : [], + ); +} + /** * Production path: swap each feature's mock repository binding for the real * Payload-backed one. Constructs `new XRepository(config, tracer, logger)` per @@ -96,7 +113,11 @@ export async function bindAllProduction(): Promise { tracer, logger, queue, - rateLimit: new NoopRateLimit(), + // Real limiter in production (audit finding A4/B3): budgets come from the + // feature manifests, so manifest edits change enforcement without touching + // this file. In-memory ⇒ per-process counters; multi-instance deployments + // need a shared backend behind IRateLimit. + rateLimit: new InMemoryRateLimit(collectManifestRateLimitBudgets()), }; bindProductionAuth(ctx); @@ -119,6 +140,8 @@ export async function bindAllDevSeed(): Promise { tracer, logger, queue, + // Dev seed intentionally keeps the no-op limiter so local iteration and + // seeded demos are never throttled; production binds InMemoryRateLimit. rateLimit: new NoopRateLimit(), };