feat(bible-texts): rate-limit the anonymous bulk-texts endpoint

[henrique221]

Closes #199.

Decision (Kasey, 2026-06-26)

Yes, I believe the 1200 chapter bulk return is due to a mobile sync feature.

Anonymous access to the bible-texts/books reads is intentional, so per the agreed plan the bulk endpoint stays unauthenticated, gets its intent documented, and gains a rate limit as an abuse guard (not a substitute for auth).

What changed

• New rateLimit middleware (src/middlewares/rate-limit.ts) — minimal fixed-window, per-client-IP (first x-forwarded-for hop; Azure App Service sets it), in-memory per process. Returns 429 { message } with Retry-After, consistent with the global error envelope. better-auth already rate-limits /api/auth/*; this covers anonymous API routes and is reusable if other open endpoints appear.
• Applied to POST /bibles/{bibleId}/bulk-texts at 20 req/min per client — a full mobile sync needs 1–2 requests (up to 1200 chapters each), so this is generous for legit clients while blunting mass scraping. The 429 is documented in the OpenAPI responses, and the route description now states the intentional-anonymous decision.
• Unit tests for the middleware: under/over limit, window reset, per-IP isolation, multi-hop x-forwarded-for.
• Proposal docs updated: bulk-texts decision recorded in 01-auth-open-domains.md, the Azure Blob decision for the async pipeline recorded in 03-… (Async USFM export pipeline hardening (pre-enablement) #196), and the README ticket table statuses refreshed.

Notes / limits of the guard

• In-memory ⇒ per-instance limits (not a distributed quota). Fine for an abuse guard; if the API scales out, each instance allows 20/min.
• Clients behind one NAT share a bucket — at 20/min that's still comfortable for legitimate sync traffic.
• Other anonymous single-chapter reads (GET .../chapters/{n}/texts, books, bible-books) are left un-limited for now; the bulk endpoint was the scraping primitive. Easy to extend with the same middleware if desired.

Test plan

• npm run typecheck
• npm run lint
• npm run test — 85/85 pass (5 new middleware tests)

Screenshots

1) OpenAPI reference — endpoint documents the decision + 429

The Scalar reference for POST /bibles/{bibleId}/bulk-texts now states the endpoint is intentionally anonymous (mobile sync feature) and rate-limited, and the 429 response ("Rate limit exceeded (20 requests per minute per client)") is part of the documented contract alongside 200/400/500.

---

2) Live behavior — 20 allowed, then 429 with Retry-After

22 sequential anonymous requests from one client against the running API (seeded DB): requests 1–20 return 200 (first returns Genesis 1's 31 verses), request 21+ returns 429 with Retry-After: 60 and the standard {"message":"Too many requests"} envelope.

---

Summary by CodeRabbit

Summary by CodeRabbit

• New Features

  • Added fixed-window, per-client IP rate limiting to anonymous bulk Bible text download requests.
  • When limits are exceeded, requests now return 429 Too Many Requests with Retry-After guidance.

• Documentation

  • Updated security/export hardening proposals and ticket status with finalized decisions and remaining QA notes.

• Tests

  • Added automated tests covering rate-limit enforcement, forwarded-IP behavior, window reset, and 429 responses.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 28230420-a061-4729-890d-a524a996f2fc

📥 Commits

Reviewing files that changed from the base of the PR and between 75f4bfb and b397984.

📒 Files selected for processing (2)

• src/middlewares/rate-limit.test.ts
• src/middlewares/rate-limit.ts

---

📝 Walkthrough

Walkthrough

Adds a fixed-window in-memory rate limiter for anonymous bulk bible-texts requests, wires it into the route with 429/OpenAPI updates, and refreshes proposal docs to record the anonymous-read and export-storage decisions.

Changes

Rate limiting implementation and rollout

Layer / File(s)	Summary
Rate-limit middleware implementation src/middlewares/rate-limit.ts	Adds RateLimitOptions/Bucket types, client IP normalization, and an exported rateLimit(options) middleware with fixed-window per-client limiting, 429 responses, Retry-After, and bucket cleanup.
Middleware test coverage src/middlewares/rate-limit.test.ts	Adds Vitest tests covering under-limit allowance, over-limit blocking, window reset, per-IP isolation, forwarded-IP bucket selection, and port stripping.
Bulk bible-texts route wiring src/domains/bibles/bible-texts/bible-texts.route.ts	Imports and applies rateLimit({ windowMs: 60_000, max: 20 }) to the bulk texts route, adds a 429 TOO_MANY_REQUESTS OpenAPI response, and updates the route description.
Security hardening proposal doc updates docs/proposals/security-and-export-hardening/01-auth-open-domains.md, docs/proposals/security-and-export-hardening/03-async-export-pipeline-hardening.md, docs/proposals/security-and-export-hardening/README.md	Marks the anonymous-read follow-up as decided with rate-limiting rationale, records the Azure Blob Storage + signed URLs decision, and updates the tickets status table.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant BulkTextsRoute
  participant rateLimit
  participant Logger

  Client->>BulkTextsRoute: POST /bibles/{bibleId}/bulk-texts
  BulkTextsRoute->>rateLimit: middleware check
  alt request under limit
    rateLimit->>BulkTextsRoute: next()
    BulkTextsRoute->>Client: 200 response
  else request over limit
    rateLimit->>Logger: warn limit exceeded
    rateLimit->>Client: 429 Too Many Requests + Retry-After
  end

Loading

Possibly related issues

• #210: The new rate-limit.ts implementation adds trusted-proxy/IP bucketing behavior and a bounded in-memory bucket store, which matches the issue’s follow-up concerns.

Suggested reviewers: kaseywright, Joel-Joseph-George

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding rate limiting to the anonymous bulk-texts endpoint.
Linked Issues check	✅ Passed	The PR documents anonymous bulk-texts access as intentional and adds per-client rate limiting, matching #199's approved path.
Out of Scope Changes check	✅ Passed	No clearly unrelated changes appear; the docs, middleware, route updates, and tests all support the same rate-limited anonymous-access decision.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/199-bulk-texts-rate-limit

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds a lightweight, reusable rate-limiting middleware to protect an intentionally unauthenticated “bulk texts” endpoint from scraping/abuse, and records the related security/export hardening decisions in proposal docs.

Changes:

• Introduces an in-memory fixed-window rateLimit middleware keyed by client IP.
• Applies rate limiting (20 req/min) to POST /bibles/{bibleId}/bulk-texts and documents the 429 response in OpenAPI.
• Updates security/export hardening proposal docs to reflect reviewer decisions and ticket statuses.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/middlewares/rate-limit.ts	New in-memory fixed-window rate limiting middleware for anonymous routes.
src/middlewares/rate-limit.test.ts	Unit tests covering limit enforcement, reset behavior, and XFF parsing.
src/domains/bibles/bible-texts/bible-texts.route.ts	Applies the limiter to the bulk-texts endpoint and documents 429 in OpenAPI.
docs/proposals/security-and-export-hardening/README.md	Updates ticket status table with recent decisions/merges.
docs/proposals/security-and-export-hardening/03-async-export-pipeline-hardening.md	Records decision to use Azure Blob + signed URLs for async exports.
docs/proposals/security-and-export-hardening/01-auth-open-domains.md	Records decision to keep certain reads anonymous and rate-limit bulk-texts.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/proposals/security-and-export-hardening/01-auth-open-domains.md`:
- Around line 53-59: Clarify that the bulk POST /bibles/{bibleId}/bulk-texts
limiter in rate-limit.ts is instance-local, not a global cluster-wide cap.
Update the proposal text to say the fixed-window guard in
src/middlewares/rate-limit.ts is per-process/per-replica and only enforces
limits within a single app instance. Keep the “intentionally anonymous” note,
but avoid implying a distributed 20 rpm limit unless a shared backend is added.

In `@src/domains/bibles/bible-texts/bible-texts.route.ts`:
- Around line 121-124: The 429 response in the OpenAPI contract is incomplete:
the rate limit behavior from rateLimit() includes a Retry-After header and is
keyed by client IP, not a generic client. Update the 429 response definition in
bible-texts.route.ts to document “per client IP” and add the Retry-After
response header alongside the JSON body so the published contract matches the
middleware behavior.

In `@src/middlewares/rate-limit.ts`:
- Around line 22-29: The clientKey helper is using request-supplied
X-Forwarded-For and X-Real-IP values, which can be spoofed to bypass the rate
limiter and inflate the bucket map. Update clientKey in
src/middlewares/rate-limit.ts to derive the key only from a trusted proxy
boundary or the framework’s trusted remote address source, and avoid falling
back to untrusted headers for client identity.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ceb2daf8-72e6-4901-a744-20e413568d6c

📥 Commits

Reviewing files that changed from the base of the PR and between f2ca17e and ee9acaa.

📒 Files selected for processing (6)

• docs/proposals/security-and-export-hardening/01-auth-open-domains.md
• docs/proposals/security-and-export-hardening/03-async-export-pipeline-hardening.md
• docs/proposals/security-and-export-hardening/README.md
• src/domains/bibles/bible-texts/bible-texts.route.ts
• src/middlewares/rate-limit.test.ts
• src/middlewares/rate-limit.ts

[kaseywright]

Only Items that I have are nits and really only a concern as the project moves into scaling behind a load balancer:

1. Hard-coded constants. MAX_BUCKETS = 10_000 and the route-level windowMs: 60_000, max: 20 are not configurable. For a minimal guard this is fine, but we will want to expose them through env vars when the limiter is reused.
2. The assumption that x-forwarded-for is always appended by a trusted proxy might be problematic later. We would want to make this configurable.
3. Rate-limit hits are not logged. Adding a logger.warn for 429s would help detect abuse and debug false positives.

[henrique221]

Thanks for the review @kaseywright! Handled as follows:

• 3 (log 429s) — done in b75d9f0: every rate-limit hit now logger.warns the client key, path, limit, and window, so hits land in App Insights for abuse detection / false-positive debugging (test asserts it).
• 1 (env-configurable constants) and 2 (configurable trusted-proxy assumption) — agreed; since both only bite once the limiter is reused / we're behind more proxy layers, I tracked them together in Rate limiter: env-configurable limits + configurable trusted-proxy assumption (scale-out follow-up) #210 (with a TODO in the code pointing at it) rather than growing this PR.

Heads-up: the push likely dismissed your approval — re-approve when convenient. 🙏

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/middlewares/rate-limit.test.ts (1)

85-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding coverage for the MAX_BUCKETS eviction path.

src/middlewares/rate-limit.ts (lines 67-78) added sweep-then-evict-oldest logic when the bucket map hits 10k entries, but no test in this file exercises that branch. It's non-trivial (nested loops, break condition), worth a targeted regression test.

Example test sketch

it('evicts oldest buckets once MAX_BUCKETS is reached', async () => {
  const middleware = rateLimit({ windowMs: 60_000, max: 1 });
  const next = vi.fn();

  for (let i = 0; i < 10_000; i++) {
    await middleware(createContext({ 'x-forwarded-for': `10.0.${i >> 8}.${i & 0xff}` }), next);
  }

  // The oldest client's bucket should have been evicted, allowing a fresh request through
  const revived = createContext({ 'x-forwarded-for': '10.0.0.0' });
  await middleware(revived, next);
  expect(revived.json).not.toHaveBeenCalled();
});

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/middlewares/rate-limit.test.ts` around lines 85 - 117, Add a regression
test in rate-limit.test.ts that covers the MAX_BUCKETS eviction path in
rateLimit by driving the middleware past the 10,000-bucket threshold and then
verifying the oldest bucket is evicted and can be reused. Use the existing
createContext, rateLimit, and next setup, and target the sweep-then-evict logic
in rateLimit so the nested loop/break branch is exercised.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/middlewares/rate-limit.test.ts`:
- Around line 85-117: Add a regression test in rate-limit.test.ts that covers
the MAX_BUCKETS eviction path in rateLimit by driving the middleware past the
10,000-bucket threshold and then verifying the oldest bucket is evicted and can
be reused. Use the existing createContext, rateLimit, and next setup, and target
the sweep-then-evict logic in rateLimit so the nested loop/break branch is
exercised.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 57e70478-8c1f-4f70-93e6-1916093a58d4

📥 Commits

Reviewing files that changed from the base of the PR and between ee9acaa and 75f4bfb.

📒 Files selected for processing (4)

• docs/proposals/security-and-export-hardening/01-auth-open-domains.md
• src/domains/bibles/bible-texts/bible-texts.route.ts
• src/middlewares/rate-limit.test.ts
• src/middlewares/rate-limit.ts