From 499955ed202b869cf511982b1957a2b4f7b8fbbc Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:27:13 +0200 Subject: [PATCH 1/7] docs: design spec for per-recipient email rate limiting Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-26-email-rate-limiting-design.md | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md diff --git a/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md b/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md new file mode 100644 index 00000000..36ed10a1 --- /dev/null +++ b/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md @@ -0,0 +1,109 @@ +# Per-recipient email rate limiting + +## Problem + +A bug in the code, a runaway webhook (e.g. emitted in a large loop), or another form of server abuse can cause Simple Directory to send a huge number of emails to the same user. There is currently no safeguard inside `sendMail` itself — only the `/mails/contact` form has an IP-based limiter, and the auth routes have their own limiter. Everything else (the `/mails` webhook fan-out, `sendMailI18n` from invitations / auth / users / organizations workers) goes straight to the SMTP transport. + +The goal is to put a single, simple guard at the central chokepoint that protects a recipient's mailbox from being flooded, without breaking legitimate bulk sending patterns (org-wide announcements, batches of invitations). + +## Goal + +Add a daily per-recipient rate limit at the central `sendMail` function in `api/src/mails/service.ts`. When a recipient exceeds the limit, drop the email to that recipient silently and log via `internalError` for admin investigation. + +## Non-goals + +- No per-IP or per-sender limiting inside `sendMail`. The mailbox-protection use case is fully covered by per-recipient limiting. +- No retry / backoff / queueing. Silent skip is sufficient for the abuse cases we're guarding against. +- No change to the existing `/mails/contact` per-IP limiter or the auth limiter — they serve different purposes and remain in place. + +## Design + +### Defaults + +- 500 emails per rolling 24h window, per recipient email address. +- Configurable via `config.mailsRateLimit.points` and `config.mailsRateLimit.duration` (seconds), following the pattern of `config.authRateLimit`. +- Environment overrides through `api/config/custom-environment-variables.cjs` (e.g. `MAILS_RATE_LIMIT_POINTS`, `MAILS_RATE_LIMIT_DURATION`). + +### Behavior + +1. Inside `sendMail(to, params, attachments)`, before any template rendering or transport call: + - Parse `to` into individual addresses by splitting on `,`, trimming, and lowercasing each. The `/mails` webhook router joins organization members with `[...to].join(', ')`, so a single `sendMail` call can have many recipients. + - For each address, consume 1 point from the mail rate limiter (`keyPrefix: 'sd-rate-limiter-mail'`). + - Build a list of allowed addresses. For each rejected address, call `internalError('mail-rate-limited', ...)` with the address and the email subject so the source can be investigated. +2. If at least one address is allowed: + - Rebuild `to` from the allowed addresses joined with `, ` and continue with the existing template-rendering + transport-sending logic unchanged. +3. If all addresses are rejected: + - Log via `internalError` and return without invoking `mailsTransport.sendMail`. The caller does not see an error. + +### Why silent skip rather than throw + +Throwing from `sendMail` would surface to the calling route as a 5xx error. A buggy webhook in a loop would then keep retrying on failure, making the abuse worse. By skipping silently and logging at `internalError` level, the abuser stops being amplified, and the operator gets the signal they need to find the source. + +### Why per-recipient and not per-sender + +The threat model is "a single user's mailbox gets flooded". The sender is sometimes an internal worker (planned deletion notices, invitations cron) where per-sender limiting would break legitimate fan-out. Per-recipient limiting cleanly addresses the threat regardless of how many distinct call sites trigger sends. + +### Why 500 / 24h + +- An org-wide announcement to 100 members costs each recipient 1 / 500 of their daily quota — no interference with normal usage. +- A user invited to many orgs, receiving action emails from several workflows, or going through 2FA flows is well under this in any realistic scenario. +- A loop bug or runaway webhook emitting one mail per iteration hits the limit within minutes and stops being amplified to that mailbox. +- The value is configurable, so operators with unusual usage patterns can tune it. + +## Components + +### 1. Config + +Add to `api/config/default.cjs` near the existing `authRateLimit`: + +```js +mailsRateLimit: { + points: 500, + duration: 86400 +}, +``` + +Add to `api/config/custom-environment-variables.cjs` so the values can be overridden by environment variables, matching how other rate-limit / numeric values are exposed. + +Test config (`api/config/test.cjs`): leave `mailsRateLimit` at production defaults (or higher) so existing mail-touching tests are unaffected. The rate-limit test exercises the limit by overriding the limiter directly in the test (see Testing below) rather than relying on a global low limit. + +### 2. Limiter factory + +Extend `api/src/utils/limiter.ts` (the file that already exports the auth limiter) with a second lazy-initialized `RateLimiterMongo`: + +- `keyPrefix: 'sd-rate-limiter-mail'` +- `points: config.mailsRateLimit.points` +- `duration: config.mailsRateLimit.duration` + +Export a named function (e.g. `mailLimiter()`) that returns the same shape as the existing auth limiter: `(key: string) => Promise` — resolves to `true` if a point was consumed, `false` if the recipient is over the limit. Any other error from `RateLimiterMongo` rethrows, matching the existing pattern. + +### 3. `sendMail` integration + +In `api/src/mails/service.ts`, at the top of `sendMail`: + +- Parse the recipient list (split on `,`, trim, lowercase). +- For each recipient address, call `mailLimiter()(address)`. +- If the limiter returns `false`, push the address into a `rejected` list; otherwise into an `allowed` list. +- For each rejected address, call `internalError('mail-rate-limited', { to: address, subject: params.subject })`. +- If `allowed.length === 0`, call one summary `internalError` and `return` (no transport call). Return `undefined`. +- Otherwise replace the local `to` with `allowed.join(', ')` and proceed with the existing logic unchanged. + +The `events.emit('send', ...)` call (used by the maildev preview integration) should fire only for the actual send, after rate-limiting — so it stays where it is, just after the recipient filtering. + +## Testing + +New test file: `tests/mail-rate-limit.unit.spec.ts` (in-process server). The limiter is exercised at low `points` (e.g. 3) by injecting / monkey-patching at the test boundary — either by stubbing the exported limiter factory in `api/src/utils/limiter.ts`, or by constructing a fresh `RateLimiterMongo` with a unique `keyPrefix` and replacing the module-level reference in `sendMail` for the duration of the test. Pick whichever fits the existing test patterns best. + +Cases: + +1. **Single recipient over the limit:** call `sendMail` 4 times in a row to the same address with `points: 3`. Assert the SMTP transport (`mailsTransport.sendMail`) was invoked exactly 3 times, and `internalError` was logged for the 4th. +2. **Multi-recipient mixed:** pre-exhaust one recipient's bucket, then call `sendMail` with a comma-separated `to` containing both the exhausted address and a fresh one. Assert the transport is called once with `to` equal to only the fresh address. +3. **All recipients exhausted:** pre-exhaust two recipients, send to both. Assert the transport is not called and the function returns without throwing. + +Existing mail-touching tests must continue to pass unchanged. + +## Risks and edge cases + +- **The `events.emit('send', ...)`** is used by the maildev preview / observability hooks. It should not fire for skipped recipients. Keep it after rate-limit filtering. +- **Address normalization:** lowercased + trimmed only — no full RFC 5321 normalization. Good enough for limiting; an attacker crafting case variations is not the threat model (this is a defense against accidental loops, not adversarial bypass). +- **MongoDB unavailable at startup:** `RateLimiterMongo` is created lazily on first use; same behavior as the existing auth and contact limiters. From 7decffb7f66853be823fdf8bbd27e0afa534df53 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:36:50 +0200 Subject: [PATCH 2/7] docs: implementation plan for per-recipient email rate limiting Co-Authored-By: Claude Opus 4.7 (1M context) --- .../plans/2026-05-26-email-rate-limiting.md | 581 ++++++++++++++++++ 1 file changed, 581 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-26-email-rate-limiting.md diff --git a/docs/superpowers/plans/2026-05-26-email-rate-limiting.md b/docs/superpowers/plans/2026-05-26-email-rate-limiting.md new file mode 100644 index 00000000..c472e4cc --- /dev/null +++ b/docs/superpowers/plans/2026-05-26-email-rate-limiting.md @@ -0,0 +1,581 @@ +# Per-recipient email rate limiting Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a daily per-recipient rate limit at the central `sendMail` function in `api/src/mails/service.ts` so a runaway loop or buggy webhook cannot flood a user's mailbox. + +**Architecture:** A second `RateLimiterMongo` instance (alongside the existing auth limiter) keyed on `keyPrefix: 'sd-rate-limiter-mail'`, exercised inside `sendMail` before any template rendering or transport call. Rejected recipients are filtered out and logged via `internalError`; if all recipients are rejected the send is dropped silently so callers never see an error. + +**Tech Stack:** Node.js + TypeScript, Express 5, `rate-limiter-flexible` (already a dependency), MongoDB, Playwright for tests, maildev as the test SMTP server. + +**Spec:** [`docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md`](../specs/2026-05-26-email-rate-limiting-design.md) + +--- + +## File Map + +- **Create:** + - `tests/features/mails-rate-limit.api.spec.ts` — Playwright API test exercising rate-limit behavior via the `/api/mails` HTTP endpoint and maildev. + +- **Modify:** + - `api/config/type/schema.json` — add `mailsRateLimit` to `required` (line ~42) and to `properties` (around line ~484, after `authRateLimit`). + - `api/config/default.cjs` — add `mailsRateLimit` block after `authRateLimit` (around line 187). + - `api/config/custom-environment-variables.cjs` — add env-var mapping after `authRateLimit` (around line 269). + - `api/config/test.cjs` — add `mailsRateLimit` with test-friendly values. + - `api/src/utils/limiter.ts` — add a named `mailLimiter` export following the same pattern as the existing default auth limiter. + - `api/src/mails/service.ts` — apply rate-limit filtering at the top of `sendMail`. + +- **Regenerated (do not hand-edit):** + - `api/config/type/.type/index.d.ts`, `api/config/type/.type/validate.js` — regenerated by `npm run build-types` after editing `schema.json`. + +--- + +## Task 1: Add `mailsRateLimit` to the config schema and config files + +**Files:** +- Modify: `api/config/type/schema.json` (two locations) +- Modify: `api/config/default.cjs` +- Modify: `api/config/custom-environment-variables.cjs` +- Modify: `api/config/test.cjs` + +- [ ] **Step 1: Add `mailsRateLimit` to the schema's `required` list** + +In `api/config/type/schema.json`, find the top-level `"required"` array (around line 17-48). The relevant existing entry is `"authRateLimit"` at line 42. Add `"mailsRateLimit"` right after it. Result: + +```json + "authRateLimit", + "mailsRateLimit", + "plannedDeletionDelay", +``` + +- [ ] **Step 2: Add `mailsRateLimit` to the schema's `properties`** + +In the same file, find the `"authRateLimit"` block (around line 470-484): + +```json + "authRateLimit": { + "type": "object", + "required": [ + "attempts", + "duration" + ], + "properties": { + "attempts": { + "type": "integer" + }, + "duration": { + "type": "integer" + } + } + }, +``` + +Insert immediately after it (before `"plannedDeletionDelay"`): + +```json + "mailsRateLimit": { + "type": "object", + "required": [ + "points", + "duration" + ], + "properties": { + "points": { + "type": "integer" + }, + "duration": { + "type": "integer" + } + } + }, +``` + +- [ ] **Step 3: Add default values in `api/config/default.cjs`** + +Find the `authRateLimit` block (around line 184-187): + +```js + authRateLimit: { + attempts: 5, + duration: 60 + }, +``` + +Insert right after it: + +```js + // per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox + // points = max number of emails per recipient address within the rolling window of `duration` seconds + mailsRateLimit: { + points: 500, + duration: 86400 + }, +``` + +- [ ] **Step 4: Add env-var bindings in `api/config/custom-environment-variables.cjs`** + +Find the `authRateLimit` block (around line 266-269): + +```js + authRateLimit: { + attempts: 'AUTHRATELIMIT_ATTEMPTS', + duration: 'AUTHRATELIMIT_DURATION' + }, +``` + +Insert right after it: + +```js + mailsRateLimit: { + points: 'MAILSRATELIMIT_POINTS', + duration: 'MAILSRATELIMIT_DURATION' + }, +``` + +- [ ] **Step 5: Add test config override in `api/config/test.cjs`** + +Find the `authRateLimit` block in `api/config/test.cjs`: + +```js + authRateLimit: { + attempts: 100, + duration: 60 + }, +``` + +Insert right after it: + +```js + mailsRateLimit: { + // high enough that no existing test trips it; rate-limit test pre-fills the bucket directly + points: 100, + duration: 5 + }, +``` + +- [ ] **Step 6: Regenerate the compiled config types** + +Run: `npm run build-types` + +Expected: command exits 0 and rewrites `api/config/type/.type/index.d.ts` and `api/config/type/.type/validate.js`. The diff for `index.d.ts` should now include a `mailsRateLimit: { points: number; duration: number; }` field on the `ApiConfig` type. + +- [ ] **Step 7: Verify TypeScript still compiles** + +Run: `npm run check-types` + +Expected: command exits 0 with no errors. + +- [ ] **Step 8: Commit** + +```bash +git add api/config/type/schema.json api/config/type/.type/ api/config/default.cjs api/config/custom-environment-variables.cjs api/config/test.cjs +git commit -m "chore: add mailsRateLimit config for per-recipient email rate limiting" +``` + +--- + +## Task 2: Add a `mailLimiter` factory in `api/src/utils/limiter.ts` + +**Files:** +- Modify: `api/src/utils/limiter.ts` + +- [ ] **Step 1: Add the `mailLimiter` named export** + +The current file is (for reference): + +```ts +import { RateLimiterMongo, RateLimiterRes } from 'rate-limiter-flexible' +import config from '#config' +import mongo from '#mongo' + +// protect authentication routes with rate limiting to prevent brute force attacks +let _limiter: (key: string) => Promise | undefined +const limiterOptions = { + keyPrefix: 'sd-rate-limiter-auth', + points: config.authRateLimit.attempts, + duration: config.authRateLimit.duration +} + +export default () => { + if (!_limiter) { + const rateLimiterMongo = new RateLimiterMongo({ + storeClient: mongo.client, + dbName: mongo.db.databaseName, + ...limiterOptions + }) + _limiter = async (key: string) => { + try { + await rateLimiterMongo.consume(key, 1) + } catch (err) { + if (err instanceof RateLimiterRes) return false + throw err + } + return true + } + } + return _limiter +} +``` + +Replace its full contents with: + +```ts +import { RateLimiterMongo, RateLimiterRes } from 'rate-limiter-flexible' +import config from '#config' +import mongo from '#mongo' + +// protect authentication routes with rate limiting to prevent brute force attacks +let _limiter: (key: string) => Promise | undefined + +export default () => { + if (!_limiter) { + const rateLimiterMongo = new RateLimiterMongo({ + storeClient: mongo.client, + dbName: mongo.db.databaseName, + keyPrefix: 'sd-rate-limiter-auth', + points: config.authRateLimit.attempts, + duration: config.authRateLimit.duration + }) + _limiter = async (key: string) => { + try { + await rateLimiterMongo.consume(key, 1) + } catch (err) { + if (err instanceof RateLimiterRes) return false + throw err + } + return true + } + } + return _limiter +} + +// per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox +let _mailLimiter: (key: string) => Promise | undefined + +export const mailLimiter = () => { + if (!_mailLimiter) { + const rateLimiterMongo = new RateLimiterMongo({ + storeClient: mongo.client, + dbName: mongo.db.databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: config.mailsRateLimit.points, + duration: config.mailsRateLimit.duration + }) + _mailLimiter = async (key: string) => { + try { + await rateLimiterMongo.consume(key, 1) + } catch (err) { + if (err instanceof RateLimiterRes) return false + throw err + } + return true + } + } + return _mailLimiter +} +``` + +- [ ] **Step 2: Verify lint and types** + +Run: `npm run lint` + +Expected: exits 0. + +Run: `npm run check-types` + +Expected: exits 0. + +- [ ] **Step 3: Commit** + +```bash +git add api/src/utils/limiter.ts +git commit -m "feat(mails): add mailLimiter factory for per-recipient rate limiting" +``` + +--- + +## Task 3: Write the failing test for `sendMail` rate-limit behavior + +**Files:** +- Modify: `api/src/test-env.ts` (extend `GET /config` to expose two more fields) +- Create: `tests/features/mails-rate-limit.api.spec.ts` + +This is a Playwright API test that uses the running dev API + maildev (same pattern as `tests/features/mails.api.spec.ts`). It pre-fills the per-recipient rate-limit bucket directly in MongoDB via a `RateLimiterMongo` instance configured the same way `mailLimiter()` configures itself, then sends emails through `/api/mails` and verifies what reaches maildev. To know the running server's mongo URL and the live rate-limit values, the test reads them from `GET /api/test-env/config` — which currently only exposes `publicUrl` and `secretKeys`, so this task extends that response. + +- [ ] **Step 1: Extend `/api/test-env/config` to expose `mongo.url` and `mailsRateLimit`** + +In `api/src/test-env.ts`, find the route handler around line 133: + +```ts +router.get('/config', (req, res) => { + res.json({ + publicUrl: config.publicUrl, + secretKeys: config.secretKeys + }) +}) +``` + +Replace it with: + +```ts +router.get('/config', (req, res) => { + res.json({ + publicUrl: config.publicUrl, + secretKeys: config.secretKeys, + mongo: { url: config.mongo.url }, + mailsRateLimit: config.mailsRateLimit + }) +}) +``` + +- [ ] **Step 2: Create the test file** + +Create `tests/features/mails-rate-limit.api.spec.ts` with this exact content: + +```ts +import { strict as assert } from 'node:assert' +import { test } from '@playwright/test' +import { axios, testEnvAx, maildevAx, deleteAllEmails, getServerConfig } from '../support/axios.ts' +import { MongoClient } from 'mongodb' +import { RateLimiterMongo } from 'rate-limiter-flexible' + +// A unique recipient per test process avoids cross-test interference on the same bucket +const uniqueEmail = (label: string) => `rate-limit-${label}-${Date.now()}-${Math.floor(Math.random() * 1e6)}@test.com` + +let mongoClient: MongoClient +let rateLimiter: RateLimiterMongo + +test.describe('mails rate limiting', () => { + test.beforeAll(async () => { + const cfg = await getServerConfig() + mongoClient = await MongoClient.connect(cfg.mongo.url) + rateLimiter = new RateLimiterMongo({ + storeClient: mongoClient, + dbName: mongoClient.db().databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: cfg.mailsRateLimit.points, + duration: cfg.mailsRateLimit.duration + }) + // give the limiter time to ensure its underlying collection exists + await rateLimiter.get('warmup') + }) + + test.afterAll(async () => { + if (mongoClient) await mongoClient.close() + }) + + test.beforeEach(async () => { + await deleteAllEmails() + await testEnvAx.delete('/') + await testEnvAx.post('/seed') + }) + + test('Delivers email to a fresh recipient (sanity)', async () => { + const ax = await axios() + const to = uniqueEmail('fresh') + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-sanity', + text: 'hello' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-sanity') + assert.ok(email, 'expected a delivered email for fresh recipient') + assert.ok(email.to.some((t: any) => t.address === to), 'expected the fresh recipient to be in the to list') + }) + + test('Drops email to a recipient whose bucket is already full', async () => { + const ax = await axios() + const to = uniqueEmail('victim') + + const cfg = await getServerConfig() + // pre-fill the rate-limit bucket up to the limit so the next consume() rejects + await rateLimiter.set(to.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-victim', + text: 'should be dropped' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-victim') + assert.ok(!email, 'expected no email delivered to a rate-limited recipient') + }) + + test('Multi-recipient send filters out only the rate-limited address', async () => { + const ax = await axios() + const victim = uniqueEmail('multi-victim') + const fresh = uniqueEmail('multi-fresh') + + const cfg = await getServerConfig() + await rateLimiter.set(victim.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + // a single `to` string with a comma exercises sendMail's multi-recipient split path + const res = await ax.post('/api/mails', { + to: [`${victim}, ${fresh}`], + subject: 'rate-limit-mixed', + text: 'partial delivery' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-mixed') + assert.ok(email, 'expected the email to be delivered with the non-limited recipient') + const addresses = email.to.map((t: any) => t.address) + assert.ok(addresses.includes(fresh), 'expected the fresh recipient to receive the email') + assert.ok(!addresses.includes(victim), 'expected the rate-limited recipient to be filtered out') + }) +}) +``` + +Note on `getServerConfig`: it already exists in `tests/support/axios.ts` and calls `GET /api/test-env/config` to read the running server's config. It is the canonical way for an api-style test to learn the runtime config values without re-loading `test.cjs` locally. Step 1 just added the two fields the test needs. + +- [ ] **Step 3: Run the new test (expect FAIL on cases 2 and 3)** + +Run: `npm run test-base tests/features/mails-rate-limit.api.spec.ts` + +Expected: the "sanity" case passes (sendMail already works), but the "Drops email" and "Multi-recipient" cases FAIL because rate limiting is not yet integrated into `sendMail` — both emails get delivered. + +- [ ] **Step 4: Commit (red state)** + +```bash +git add api/src/test-env.ts tests/features/mails-rate-limit.api.spec.ts +git commit -m "test(mails): add failing tests for per-recipient mail rate limiting" +``` + +--- + +## Task 4: Integrate the rate limit into `sendMail` + +**Files:** +- Modify: `api/src/mails/service.ts` + +- [ ] **Step 1: Add the rate-limit filter at the top of `sendMail`** + +In `api/src/mails/service.ts`, locate the imports block at the top of the file (around lines 1-9): + +```ts +import mjml2html from 'mjml' +import microTemplate from '@data-fair/lib-utils/micro-template.js' +import { join } from 'path' +import { readFileSync, existsSync } from 'node:fs' +import config from '#config' +import { flatten } from 'flat' +import EventEmitter from 'node:events' +import mailsTransport from './transport.ts' +import { getSiteByUrl, getSiteByHost } from '#services' +``` + +Add two new imports after the existing ones: + +```ts +import { internalError } from '@data-fair/lib-node/observer.js' +import { mailLimiter } from '../utils/limiter.ts' +``` + +Then locate the `sendMail` function (starts around line 85): + +```ts +export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { + let site = params.link ? (await getSiteByUrl(params.link)) : undefined + if (!site && params.host) { + site = await getSiteByHost(params.host, params.path ?? '') + } +``` + +Replace the function signature line and insert the rate-limit block as the very first thing in the body (before any other work). The full new opening of the function: + +```ts +export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { + // per-recipient rate limiting — protects users from a bug or runaway webhook spamming their mailbox + const recipients = to.split(',').map(r => r.trim().toLowerCase()).filter(r => r.length > 0) + const allowed: string[] = [] + for (const recipient of recipients) { + if (await mailLimiter()(recipient)) { + allowed.push(recipient) + } else { + internalError('mail-rate-limited', `recipient ${recipient} exceeded mail rate limit, dropping (subject: ${params.subject})`) + } + } + if (allowed.length === 0) { + internalError('mail-rate-limited', `all recipients dropped, mail not sent (subject: ${params.subject}, original to: ${to})`) + return + } + to = allowed.join(', ') + + let site = params.link ? (await getSiteByUrl(params.link)) : undefined + if (!site && params.host) { + site = await getSiteByHost(params.host, params.path ?? '') + } +``` + +Leave the rest of the function (template rendering, `events.emit`, `mailsTransport.sendMail` call) untouched. + +- [ ] **Step 2: Run the rate-limit test (expect PASS)** + +Run: `npm run test-base tests/features/mails-rate-limit.api.spec.ts` + +Expected: all three cases pass. + +- [ ] **Step 3: Run the existing mails test suite to confirm no regression** + +Run: `npm run test-base tests/features/mails.api.spec.ts` + +Expected: all existing tests still pass. (The test-config limit is `points: 100, duration: 5` — well above what any single test sends.) + +- [ ] **Step 4: Run lint and type checks** + +Run: `npm run lint` + +Expected: exits 0. + +Run: `npm run check-types` + +Expected: exits 0. + +- [ ] **Step 5: Commit** + +```bash +git add api/src/mails/service.ts +git commit -m "feat(mails): enforce per-recipient rate limit in sendMail" +``` + +--- + +## Task 5: Final verification — run the broader test suites + +The git push hook will run the full suite. Before pushing, run the affected test groups locally so any failure is fixed in a fast feedback loop. + +- [ ] **Step 1: Run all API tests** + +Run: `npm run test-api` + +Expected: all pass. If any test that sends many mails to the same recipient fails because of the limit, raise `mailsRateLimit.points` in `api/config/test.cjs` until it passes (the limit is purely a defensive ceiling in tests). + +- [ ] **Step 2: Run all unit tests** + +Run: `npm run test-unit` + +Expected: all pass. + +- [ ] **Step 3: Sanity-check the production defaults didn't drift** + +Run: `grep -n "mailsRateLimit" api/config/default.cjs` + +Expected: shows `points: 500` and `duration: 86400` — the production defaults from the design. + +--- + +## Notes for the implementer + +- **No new dependencies.** `rate-limiter-flexible` is already in `package.json` (used by the existing auth limiter and the contact-form limiter). +- **The `events.emit('send', ...)` call inside `sendMail` stays where it is** — at line 112 (post-filtering). It only fires for actually-sent mails, which is what observability hooks expect. +- **Address normalization is lowercase + trim**, no full RFC 5321 normalization. This is good enough for accident-driven flooding (the real threat model). The test pre-fills the bucket with `.toLowerCase()` to match. +- **Why a daily window:** the goal is protecting the user's mailbox over a meaningful period. A short window would let bursts through repeatedly; a daily window caps total daily volume from a single source. `points: 500` is comfortably above any realistic legitimate volume per recipient. +- **The `/mails/contact` IP-based limiter is unrelated** — it stays in place. It guards the form against external spammers; this new limit guards mailboxes against internal bugs. They protect against different threats. From 7302094bc5657e61b90f4825c8886e7b58271c04 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:41:10 +0200 Subject: [PATCH 3/7] chore: add mailsRateLimit config for per-recipient email rate limiting Co-Authored-By: Claude Opus 4.7 (1M context) --- api/config/custom-environment-variables.cjs | 4 ++++ api/config/default.cjs | 6 ++++++ api/config/test.cjs | 5 +++++ api/config/type/schema.json | 16 ++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/api/config/custom-environment-variables.cjs b/api/config/custom-environment-variables.cjs index 03619174..00592245 100644 --- a/api/config/custom-environment-variables.cjs +++ b/api/config/custom-environment-variables.cjs @@ -267,6 +267,10 @@ module.exports = { attempts: 'AUTHRATELIMIT_ATTEMPTS', duration: 'AUTHRATELIMIT_DURATION' }, + mailsRateLimit: { + points: 'MAILSRATELIMIT_POINTS', + duration: 'MAILSRATELIMIT_DURATION' + }, oauth: { providers: jsonEnv('OAUTH_PROVIDERS'), github: { diff --git a/api/config/default.cjs b/api/config/default.cjs index 8601d530..3e98ebd6 100644 --- a/api/config/default.cjs +++ b/api/config/default.cjs @@ -185,6 +185,12 @@ module.exports = { attempts: 5, duration: 60 }, + // per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox + // points = max number of emails per recipient address within the rolling window of `duration` seconds + mailsRateLimit: { + points: 500, + duration: 86400 + }, // Example of full oauth provider configuration // oauth: {providers: [{ // id: 'github', diff --git a/api/config/test.cjs b/api/config/test.cjs index 5d75f9c7..4111655e 100644 --- a/api/config/test.cjs +++ b/api/config/test.cjs @@ -71,6 +71,11 @@ module.exports = { attempts: 100, duration: 60 }, + mailsRateLimit: { + // high enough that no existing test trips it; rate-limit test pre-fills the bucket directly + points: 100, + duration: 5 + }, anonymousAction: { // short notBefore keeps tests fast — in prod this is 8s to trip bots expiresIn: '1d', diff --git a/api/config/type/schema.json b/api/config/type/schema.json index a07d459e..d51fe86b 100644 --- a/api/config/type/schema.json +++ b/api/config/type/schema.json @@ -40,6 +40,7 @@ "saml2", "webhooks", "authRateLimit", + "mailsRateLimit", "plannedDeletionDelay", "cleanup", "avatars", @@ -482,6 +483,21 @@ } } }, + "mailsRateLimit": { + "type": "object", + "required": [ + "points", + "duration" + ], + "properties": { + "points": { + "type": "integer" + }, + "duration": { + "type": "integer" + } + } + }, "plannedDeletionDelay": { "type": "integer" }, From a585efd0f9b48f9962f724110ff0c19e8c2dea69 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:41:55 +0200 Subject: [PATCH 4/7] feat(mails): add mailLimiter factory for per-recipient rate limiting Co-Authored-By: Claude Opus 4.7 (1M context) --- api/src/utils/limiter.ts | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/api/src/utils/limiter.ts b/api/src/utils/limiter.ts index 5f97f609..c06d58c1 100644 --- a/api/src/utils/limiter.ts +++ b/api/src/utils/limiter.ts @@ -4,18 +4,15 @@ import mongo from '#mongo' // protect authentication routes with rate limiting to prevent brute force attacks let _limiter: (key: string) => Promise | undefined -const limiterOptions = { - keyPrefix: 'sd-rate-limiter-auth', - points: config.authRateLimit.attempts, - duration: config.authRateLimit.duration -} export default () => { if (!_limiter) { const rateLimiterMongo = new RateLimiterMongo({ storeClient: mongo.client, dbName: mongo.db.databaseName, - ...limiterOptions + keyPrefix: 'sd-rate-limiter-auth', + points: config.authRateLimit.attempts, + duration: config.authRateLimit.duration }) _limiter = async (key: string) => { try { @@ -29,3 +26,28 @@ export default () => { } return _limiter } + +// per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox +let _mailLimiter: (key: string) => Promise | undefined + +export const mailLimiter = () => { + if (!_mailLimiter) { + const rateLimiterMongo = new RateLimiterMongo({ + storeClient: mongo.client, + dbName: mongo.db.databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: config.mailsRateLimit.points, + duration: config.mailsRateLimit.duration + }) + _mailLimiter = async (key: string) => { + try { + await rateLimiterMongo.consume(key, 1) + } catch (err) { + if (err instanceof RateLimiterRes) return false + throw err + } + return true + } + } + return _mailLimiter +} From 14bf7486e027fd2f60e1123f60f451d4d3b6ffb5 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:43:46 +0200 Subject: [PATCH 5/7] test(mails): add failing tests for per-recipient mail rate limiting Co-Authored-By: Claude Opus 4.7 (1M context) --- api/src/test-env.ts | 4 +- tests/features/mails-rate-limit.api.spec.ts | 99 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/features/mails-rate-limit.api.spec.ts diff --git a/api/src/test-env.ts b/api/src/test-env.ts index f67a6d95..c37234a4 100644 --- a/api/src/test-env.ts +++ b/api/src/test-env.ts @@ -133,7 +133,9 @@ router.post('/clear-site-cache', async (req, res) => { router.get('/config', (req, res) => { res.json({ publicUrl: config.publicUrl, - secretKeys: config.secretKeys + secretKeys: config.secretKeys, + mongo: { url: config.mongo.url }, + mailsRateLimit: config.mailsRateLimit }) }) diff --git a/tests/features/mails-rate-limit.api.spec.ts b/tests/features/mails-rate-limit.api.spec.ts new file mode 100644 index 00000000..d72cd2b5 --- /dev/null +++ b/tests/features/mails-rate-limit.api.spec.ts @@ -0,0 +1,99 @@ +import { strict as assert } from 'node:assert' +import { test } from '@playwright/test' +import { axios, testEnvAx, maildevAx, deleteAllEmails, getServerConfig } from '../support/axios.ts' +import { MongoClient } from 'mongodb' +import { RateLimiterMongo } from 'rate-limiter-flexible' + +// A unique recipient per test process avoids cross-test interference on the same bucket +const uniqueEmail = (label: string) => `rate-limit-${label}-${Date.now()}-${Math.floor(Math.random() * 1e6)}@test.com` + +let mongoClient: MongoClient +let rateLimiter: RateLimiterMongo + +test.describe('mails rate limiting', () => { + test.beforeAll(async () => { + const cfg = await getServerConfig() + mongoClient = await MongoClient.connect(cfg.mongo.url) + rateLimiter = new RateLimiterMongo({ + storeClient: mongoClient, + dbName: mongoClient.db().databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: cfg.mailsRateLimit.points, + duration: cfg.mailsRateLimit.duration + }) + // give the limiter time to ensure its underlying collection exists + await rateLimiter.get('warmup') + }) + + test.afterAll(async () => { + if (mongoClient) await mongoClient.close() + }) + + test.beforeEach(async () => { + await deleteAllEmails() + await testEnvAx.delete('/') + await testEnvAx.post('/seed') + }) + + test('Delivers email to a fresh recipient (sanity)', async () => { + const ax = await axios() + const to = uniqueEmail('fresh') + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-sanity', + text: 'hello' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-sanity') + assert.ok(email, 'expected a delivered email for fresh recipient') + assert.ok(email.to.some((t: any) => t.address === to), 'expected the fresh recipient to be in the to list') + }) + + test('Drops email to a recipient whose bucket is already full', async () => { + const ax = await axios() + const to = uniqueEmail('victim') + + const cfg = await getServerConfig() + // pre-fill the rate-limit bucket up to the limit so the next consume() rejects + await rateLimiter.set(to.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-victim', + text: 'should be dropped' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-victim') + assert.ok(!email, 'expected no email delivered to a rate-limited recipient') + }) + + test('Multi-recipient send filters out only the rate-limited address', async () => { + const ax = await axios() + const victim = uniqueEmail('multi-victim') + const fresh = uniqueEmail('multi-fresh') + + const cfg = await getServerConfig() + await rateLimiter.set(victim.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + // a single `to` string with a comma exercises sendMail's multi-recipient split path + const res = await ax.post('/api/mails', { + to: [`${victim}, ${fresh}`], + subject: 'rate-limit-mixed', + text: 'partial delivery' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-mixed') + assert.ok(email, 'expected the email to be delivered with the non-limited recipient') + const addresses = email.to.map((t: any) => t.address) + assert.ok(addresses.includes(fresh), 'expected the fresh recipient to receive the email') + assert.ok(!addresses.includes(victim), 'expected the rate-limited recipient to be filtered out') + }) +}) From 37701e919a00732953319c268f3c7c353beaa678 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 10:57:05 +0200 Subject: [PATCH 6/7] feat(mails): enforce per-recipient rate limit in sendMail Co-Authored-By: Claude Opus 4.7 (1M context) --- api/src/mails/service.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/api/src/mails/service.ts b/api/src/mails/service.ts index b5cccc3c..5503c37e 100644 --- a/api/src/mails/service.ts +++ b/api/src/mails/service.ts @@ -7,6 +7,8 @@ import { flatten } from 'flat' import EventEmitter from 'node:events' import mailsTransport from './transport.ts' import { getSiteByUrl, getSiteByHost } from '#services' +import { internalError } from '@data-fair/lib-node/observer.js' +import { mailLimiter } from '../utils/limiter.ts' export const events = new EventEmitter() @@ -83,6 +85,22 @@ export const sendMailI18n = async (key: string, messages: any, to: string, param } export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { + // per-recipient rate limiting — protects users from a bug or runaway webhook spamming their mailbox + const recipients = to.split(',').map(r => r.trim().toLowerCase()).filter(r => r.length > 0) + const allowed: string[] = [] + for (const recipient of recipients) { + if (await mailLimiter()(recipient)) { + allowed.push(recipient) + } else { + internalError('mail-rate-limited', `recipient ${recipient} exceeded mail rate limit, dropping (subject: ${params.subject})`) + } + } + if (allowed.length === 0) { + internalError('mail-rate-limited', `all recipients dropped, mail not sent (subject: ${params.subject}, original to: ${to})`) + return + } + to = allowed.join(', ') + let site = params.link ? (await getSiteByUrl(params.link)) : undefined if (!site && params.host) { site = await getSiteByHost(params.host, params.path ?? '') From 9f3c58d386878dd620506285cb8f62dee7010219 Mon Sep 17 00:00:00 2001 From: Alban Mouton Date: Tue, 26 May 2026 11:43:34 +0200 Subject: [PATCH 7/7] chore: cleanup superpowers files --- .../plans/2026-05-26-email-rate-limiting.md | 581 ------------------ .../2026-05-26-email-rate-limiting-design.md | 109 ---- 2 files changed, 690 deletions(-) delete mode 100644 docs/superpowers/plans/2026-05-26-email-rate-limiting.md delete mode 100644 docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md diff --git a/docs/superpowers/plans/2026-05-26-email-rate-limiting.md b/docs/superpowers/plans/2026-05-26-email-rate-limiting.md deleted file mode 100644 index c472e4cc..00000000 --- a/docs/superpowers/plans/2026-05-26-email-rate-limiting.md +++ /dev/null @@ -1,581 +0,0 @@ -# Per-recipient email rate limiting Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add a daily per-recipient rate limit at the central `sendMail` function in `api/src/mails/service.ts` so a runaway loop or buggy webhook cannot flood a user's mailbox. - -**Architecture:** A second `RateLimiterMongo` instance (alongside the existing auth limiter) keyed on `keyPrefix: 'sd-rate-limiter-mail'`, exercised inside `sendMail` before any template rendering or transport call. Rejected recipients are filtered out and logged via `internalError`; if all recipients are rejected the send is dropped silently so callers never see an error. - -**Tech Stack:** Node.js + TypeScript, Express 5, `rate-limiter-flexible` (already a dependency), MongoDB, Playwright for tests, maildev as the test SMTP server. - -**Spec:** [`docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md`](../specs/2026-05-26-email-rate-limiting-design.md) - ---- - -## File Map - -- **Create:** - - `tests/features/mails-rate-limit.api.spec.ts` — Playwright API test exercising rate-limit behavior via the `/api/mails` HTTP endpoint and maildev. - -- **Modify:** - - `api/config/type/schema.json` — add `mailsRateLimit` to `required` (line ~42) and to `properties` (around line ~484, after `authRateLimit`). - - `api/config/default.cjs` — add `mailsRateLimit` block after `authRateLimit` (around line 187). - - `api/config/custom-environment-variables.cjs` — add env-var mapping after `authRateLimit` (around line 269). - - `api/config/test.cjs` — add `mailsRateLimit` with test-friendly values. - - `api/src/utils/limiter.ts` — add a named `mailLimiter` export following the same pattern as the existing default auth limiter. - - `api/src/mails/service.ts` — apply rate-limit filtering at the top of `sendMail`. - -- **Regenerated (do not hand-edit):** - - `api/config/type/.type/index.d.ts`, `api/config/type/.type/validate.js` — regenerated by `npm run build-types` after editing `schema.json`. - ---- - -## Task 1: Add `mailsRateLimit` to the config schema and config files - -**Files:** -- Modify: `api/config/type/schema.json` (two locations) -- Modify: `api/config/default.cjs` -- Modify: `api/config/custom-environment-variables.cjs` -- Modify: `api/config/test.cjs` - -- [ ] **Step 1: Add `mailsRateLimit` to the schema's `required` list** - -In `api/config/type/schema.json`, find the top-level `"required"` array (around line 17-48). The relevant existing entry is `"authRateLimit"` at line 42. Add `"mailsRateLimit"` right after it. Result: - -```json - "authRateLimit", - "mailsRateLimit", - "plannedDeletionDelay", -``` - -- [ ] **Step 2: Add `mailsRateLimit` to the schema's `properties`** - -In the same file, find the `"authRateLimit"` block (around line 470-484): - -```json - "authRateLimit": { - "type": "object", - "required": [ - "attempts", - "duration" - ], - "properties": { - "attempts": { - "type": "integer" - }, - "duration": { - "type": "integer" - } - } - }, -``` - -Insert immediately after it (before `"plannedDeletionDelay"`): - -```json - "mailsRateLimit": { - "type": "object", - "required": [ - "points", - "duration" - ], - "properties": { - "points": { - "type": "integer" - }, - "duration": { - "type": "integer" - } - } - }, -``` - -- [ ] **Step 3: Add default values in `api/config/default.cjs`** - -Find the `authRateLimit` block (around line 184-187): - -```js - authRateLimit: { - attempts: 5, - duration: 60 - }, -``` - -Insert right after it: - -```js - // per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox - // points = max number of emails per recipient address within the rolling window of `duration` seconds - mailsRateLimit: { - points: 500, - duration: 86400 - }, -``` - -- [ ] **Step 4: Add env-var bindings in `api/config/custom-environment-variables.cjs`** - -Find the `authRateLimit` block (around line 266-269): - -```js - authRateLimit: { - attempts: 'AUTHRATELIMIT_ATTEMPTS', - duration: 'AUTHRATELIMIT_DURATION' - }, -``` - -Insert right after it: - -```js - mailsRateLimit: { - points: 'MAILSRATELIMIT_POINTS', - duration: 'MAILSRATELIMIT_DURATION' - }, -``` - -- [ ] **Step 5: Add test config override in `api/config/test.cjs`** - -Find the `authRateLimit` block in `api/config/test.cjs`: - -```js - authRateLimit: { - attempts: 100, - duration: 60 - }, -``` - -Insert right after it: - -```js - mailsRateLimit: { - // high enough that no existing test trips it; rate-limit test pre-fills the bucket directly - points: 100, - duration: 5 - }, -``` - -- [ ] **Step 6: Regenerate the compiled config types** - -Run: `npm run build-types` - -Expected: command exits 0 and rewrites `api/config/type/.type/index.d.ts` and `api/config/type/.type/validate.js`. The diff for `index.d.ts` should now include a `mailsRateLimit: { points: number; duration: number; }` field on the `ApiConfig` type. - -- [ ] **Step 7: Verify TypeScript still compiles** - -Run: `npm run check-types` - -Expected: command exits 0 with no errors. - -- [ ] **Step 8: Commit** - -```bash -git add api/config/type/schema.json api/config/type/.type/ api/config/default.cjs api/config/custom-environment-variables.cjs api/config/test.cjs -git commit -m "chore: add mailsRateLimit config for per-recipient email rate limiting" -``` - ---- - -## Task 2: Add a `mailLimiter` factory in `api/src/utils/limiter.ts` - -**Files:** -- Modify: `api/src/utils/limiter.ts` - -- [ ] **Step 1: Add the `mailLimiter` named export** - -The current file is (for reference): - -```ts -import { RateLimiterMongo, RateLimiterRes } from 'rate-limiter-flexible' -import config from '#config' -import mongo from '#mongo' - -// protect authentication routes with rate limiting to prevent brute force attacks -let _limiter: (key: string) => Promise | undefined -const limiterOptions = { - keyPrefix: 'sd-rate-limiter-auth', - points: config.authRateLimit.attempts, - duration: config.authRateLimit.duration -} - -export default () => { - if (!_limiter) { - const rateLimiterMongo = new RateLimiterMongo({ - storeClient: mongo.client, - dbName: mongo.db.databaseName, - ...limiterOptions - }) - _limiter = async (key: string) => { - try { - await rateLimiterMongo.consume(key, 1) - } catch (err) { - if (err instanceof RateLimiterRes) return false - throw err - } - return true - } - } - return _limiter -} -``` - -Replace its full contents with: - -```ts -import { RateLimiterMongo, RateLimiterRes } from 'rate-limiter-flexible' -import config from '#config' -import mongo from '#mongo' - -// protect authentication routes with rate limiting to prevent brute force attacks -let _limiter: (key: string) => Promise | undefined - -export default () => { - if (!_limiter) { - const rateLimiterMongo = new RateLimiterMongo({ - storeClient: mongo.client, - dbName: mongo.db.databaseName, - keyPrefix: 'sd-rate-limiter-auth', - points: config.authRateLimit.attempts, - duration: config.authRateLimit.duration - }) - _limiter = async (key: string) => { - try { - await rateLimiterMongo.consume(key, 1) - } catch (err) { - if (err instanceof RateLimiterRes) return false - throw err - } - return true - } - } - return _limiter -} - -// per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox -let _mailLimiter: (key: string) => Promise | undefined - -export const mailLimiter = () => { - if (!_mailLimiter) { - const rateLimiterMongo = new RateLimiterMongo({ - storeClient: mongo.client, - dbName: mongo.db.databaseName, - keyPrefix: 'sd-rate-limiter-mail', - points: config.mailsRateLimit.points, - duration: config.mailsRateLimit.duration - }) - _mailLimiter = async (key: string) => { - try { - await rateLimiterMongo.consume(key, 1) - } catch (err) { - if (err instanceof RateLimiterRes) return false - throw err - } - return true - } - } - return _mailLimiter -} -``` - -- [ ] **Step 2: Verify lint and types** - -Run: `npm run lint` - -Expected: exits 0. - -Run: `npm run check-types` - -Expected: exits 0. - -- [ ] **Step 3: Commit** - -```bash -git add api/src/utils/limiter.ts -git commit -m "feat(mails): add mailLimiter factory for per-recipient rate limiting" -``` - ---- - -## Task 3: Write the failing test for `sendMail` rate-limit behavior - -**Files:** -- Modify: `api/src/test-env.ts` (extend `GET /config` to expose two more fields) -- Create: `tests/features/mails-rate-limit.api.spec.ts` - -This is a Playwright API test that uses the running dev API + maildev (same pattern as `tests/features/mails.api.spec.ts`). It pre-fills the per-recipient rate-limit bucket directly in MongoDB via a `RateLimiterMongo` instance configured the same way `mailLimiter()` configures itself, then sends emails through `/api/mails` and verifies what reaches maildev. To know the running server's mongo URL and the live rate-limit values, the test reads them from `GET /api/test-env/config` — which currently only exposes `publicUrl` and `secretKeys`, so this task extends that response. - -- [ ] **Step 1: Extend `/api/test-env/config` to expose `mongo.url` and `mailsRateLimit`** - -In `api/src/test-env.ts`, find the route handler around line 133: - -```ts -router.get('/config', (req, res) => { - res.json({ - publicUrl: config.publicUrl, - secretKeys: config.secretKeys - }) -}) -``` - -Replace it with: - -```ts -router.get('/config', (req, res) => { - res.json({ - publicUrl: config.publicUrl, - secretKeys: config.secretKeys, - mongo: { url: config.mongo.url }, - mailsRateLimit: config.mailsRateLimit - }) -}) -``` - -- [ ] **Step 2: Create the test file** - -Create `tests/features/mails-rate-limit.api.spec.ts` with this exact content: - -```ts -import { strict as assert } from 'node:assert' -import { test } from '@playwright/test' -import { axios, testEnvAx, maildevAx, deleteAllEmails, getServerConfig } from '../support/axios.ts' -import { MongoClient } from 'mongodb' -import { RateLimiterMongo } from 'rate-limiter-flexible' - -// A unique recipient per test process avoids cross-test interference on the same bucket -const uniqueEmail = (label: string) => `rate-limit-${label}-${Date.now()}-${Math.floor(Math.random() * 1e6)}@test.com` - -let mongoClient: MongoClient -let rateLimiter: RateLimiterMongo - -test.describe('mails rate limiting', () => { - test.beforeAll(async () => { - const cfg = await getServerConfig() - mongoClient = await MongoClient.connect(cfg.mongo.url) - rateLimiter = new RateLimiterMongo({ - storeClient: mongoClient, - dbName: mongoClient.db().databaseName, - keyPrefix: 'sd-rate-limiter-mail', - points: cfg.mailsRateLimit.points, - duration: cfg.mailsRateLimit.duration - }) - // give the limiter time to ensure its underlying collection exists - await rateLimiter.get('warmup') - }) - - test.afterAll(async () => { - if (mongoClient) await mongoClient.close() - }) - - test.beforeEach(async () => { - await deleteAllEmails() - await testEnvAx.delete('/') - await testEnvAx.post('/seed') - }) - - test('Delivers email to a fresh recipient (sanity)', async () => { - const ax = await axios() - const to = uniqueEmail('fresh') - const res = await ax.post('/api/mails', { - to: [to], - subject: 'rate-limit-sanity', - text: 'hello' - }, { params: { key: 'testkey' } }) - assert.equal(res.status, 200) - await new Promise(resolve => setTimeout(resolve, 100)) - const emails: any[] = (await maildevAx.get('/email')).data - const email = emails.find((m: any) => m.subject === 'rate-limit-sanity') - assert.ok(email, 'expected a delivered email for fresh recipient') - assert.ok(email.to.some((t: any) => t.address === to), 'expected the fresh recipient to be in the to list') - }) - - test('Drops email to a recipient whose bucket is already full', async () => { - const ax = await axios() - const to = uniqueEmail('victim') - - const cfg = await getServerConfig() - // pre-fill the rate-limit bucket up to the limit so the next consume() rejects - await rateLimiter.set(to.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) - - const res = await ax.post('/api/mails', { - to: [to], - subject: 'rate-limit-victim', - text: 'should be dropped' - }, { params: { key: 'testkey' } }) - assert.equal(res.status, 200) - - await new Promise(resolve => setTimeout(resolve, 100)) - const emails: any[] = (await maildevAx.get('/email')).data - const email = emails.find((m: any) => m.subject === 'rate-limit-victim') - assert.ok(!email, 'expected no email delivered to a rate-limited recipient') - }) - - test('Multi-recipient send filters out only the rate-limited address', async () => { - const ax = await axios() - const victim = uniqueEmail('multi-victim') - const fresh = uniqueEmail('multi-fresh') - - const cfg = await getServerConfig() - await rateLimiter.set(victim.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) - - // a single `to` string with a comma exercises sendMail's multi-recipient split path - const res = await ax.post('/api/mails', { - to: [`${victim}, ${fresh}`], - subject: 'rate-limit-mixed', - text: 'partial delivery' - }, { params: { key: 'testkey' } }) - assert.equal(res.status, 200) - - await new Promise(resolve => setTimeout(resolve, 100)) - const emails: any[] = (await maildevAx.get('/email')).data - const email = emails.find((m: any) => m.subject === 'rate-limit-mixed') - assert.ok(email, 'expected the email to be delivered with the non-limited recipient') - const addresses = email.to.map((t: any) => t.address) - assert.ok(addresses.includes(fresh), 'expected the fresh recipient to receive the email') - assert.ok(!addresses.includes(victim), 'expected the rate-limited recipient to be filtered out') - }) -}) -``` - -Note on `getServerConfig`: it already exists in `tests/support/axios.ts` and calls `GET /api/test-env/config` to read the running server's config. It is the canonical way for an api-style test to learn the runtime config values without re-loading `test.cjs` locally. Step 1 just added the two fields the test needs. - -- [ ] **Step 3: Run the new test (expect FAIL on cases 2 and 3)** - -Run: `npm run test-base tests/features/mails-rate-limit.api.spec.ts` - -Expected: the "sanity" case passes (sendMail already works), but the "Drops email" and "Multi-recipient" cases FAIL because rate limiting is not yet integrated into `sendMail` — both emails get delivered. - -- [ ] **Step 4: Commit (red state)** - -```bash -git add api/src/test-env.ts tests/features/mails-rate-limit.api.spec.ts -git commit -m "test(mails): add failing tests for per-recipient mail rate limiting" -``` - ---- - -## Task 4: Integrate the rate limit into `sendMail` - -**Files:** -- Modify: `api/src/mails/service.ts` - -- [ ] **Step 1: Add the rate-limit filter at the top of `sendMail`** - -In `api/src/mails/service.ts`, locate the imports block at the top of the file (around lines 1-9): - -```ts -import mjml2html from 'mjml' -import microTemplate from '@data-fair/lib-utils/micro-template.js' -import { join } from 'path' -import { readFileSync, existsSync } from 'node:fs' -import config from '#config' -import { flatten } from 'flat' -import EventEmitter from 'node:events' -import mailsTransport from './transport.ts' -import { getSiteByUrl, getSiteByHost } from '#services' -``` - -Add two new imports after the existing ones: - -```ts -import { internalError } from '@data-fair/lib-node/observer.js' -import { mailLimiter } from '../utils/limiter.ts' -``` - -Then locate the `sendMail` function (starts around line 85): - -```ts -export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { - let site = params.link ? (await getSiteByUrl(params.link)) : undefined - if (!site && params.host) { - site = await getSiteByHost(params.host, params.path ?? '') - } -``` - -Replace the function signature line and insert the rate-limit block as the very first thing in the body (before any other work). The full new opening of the function: - -```ts -export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { - // per-recipient rate limiting — protects users from a bug or runaway webhook spamming their mailbox - const recipients = to.split(',').map(r => r.trim().toLowerCase()).filter(r => r.length > 0) - const allowed: string[] = [] - for (const recipient of recipients) { - if (await mailLimiter()(recipient)) { - allowed.push(recipient) - } else { - internalError('mail-rate-limited', `recipient ${recipient} exceeded mail rate limit, dropping (subject: ${params.subject})`) - } - } - if (allowed.length === 0) { - internalError('mail-rate-limited', `all recipients dropped, mail not sent (subject: ${params.subject}, original to: ${to})`) - return - } - to = allowed.join(', ') - - let site = params.link ? (await getSiteByUrl(params.link)) : undefined - if (!site && params.host) { - site = await getSiteByHost(params.host, params.path ?? '') - } -``` - -Leave the rest of the function (template rendering, `events.emit`, `mailsTransport.sendMail` call) untouched. - -- [ ] **Step 2: Run the rate-limit test (expect PASS)** - -Run: `npm run test-base tests/features/mails-rate-limit.api.spec.ts` - -Expected: all three cases pass. - -- [ ] **Step 3: Run the existing mails test suite to confirm no regression** - -Run: `npm run test-base tests/features/mails.api.spec.ts` - -Expected: all existing tests still pass. (The test-config limit is `points: 100, duration: 5` — well above what any single test sends.) - -- [ ] **Step 4: Run lint and type checks** - -Run: `npm run lint` - -Expected: exits 0. - -Run: `npm run check-types` - -Expected: exits 0. - -- [ ] **Step 5: Commit** - -```bash -git add api/src/mails/service.ts -git commit -m "feat(mails): enforce per-recipient rate limit in sendMail" -``` - ---- - -## Task 5: Final verification — run the broader test suites - -The git push hook will run the full suite. Before pushing, run the affected test groups locally so any failure is fixed in a fast feedback loop. - -- [ ] **Step 1: Run all API tests** - -Run: `npm run test-api` - -Expected: all pass. If any test that sends many mails to the same recipient fails because of the limit, raise `mailsRateLimit.points` in `api/config/test.cjs` until it passes (the limit is purely a defensive ceiling in tests). - -- [ ] **Step 2: Run all unit tests** - -Run: `npm run test-unit` - -Expected: all pass. - -- [ ] **Step 3: Sanity-check the production defaults didn't drift** - -Run: `grep -n "mailsRateLimit" api/config/default.cjs` - -Expected: shows `points: 500` and `duration: 86400` — the production defaults from the design. - ---- - -## Notes for the implementer - -- **No new dependencies.** `rate-limiter-flexible` is already in `package.json` (used by the existing auth limiter and the contact-form limiter). -- **The `events.emit('send', ...)` call inside `sendMail` stays where it is** — at line 112 (post-filtering). It only fires for actually-sent mails, which is what observability hooks expect. -- **Address normalization is lowercase + trim**, no full RFC 5321 normalization. This is good enough for accident-driven flooding (the real threat model). The test pre-fills the bucket with `.toLowerCase()` to match. -- **Why a daily window:** the goal is protecting the user's mailbox over a meaningful period. A short window would let bursts through repeatedly; a daily window caps total daily volume from a single source. `points: 500` is comfortably above any realistic legitimate volume per recipient. -- **The `/mails/contact` IP-based limiter is unrelated** — it stays in place. It guards the form against external spammers; this new limit guards mailboxes against internal bugs. They protect against different threats. diff --git a/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md b/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md deleted file mode 100644 index 36ed10a1..00000000 --- a/docs/superpowers/specs/2026-05-26-email-rate-limiting-design.md +++ /dev/null @@ -1,109 +0,0 @@ -# Per-recipient email rate limiting - -## Problem - -A bug in the code, a runaway webhook (e.g. emitted in a large loop), or another form of server abuse can cause Simple Directory to send a huge number of emails to the same user. There is currently no safeguard inside `sendMail` itself — only the `/mails/contact` form has an IP-based limiter, and the auth routes have their own limiter. Everything else (the `/mails` webhook fan-out, `sendMailI18n` from invitations / auth / users / organizations workers) goes straight to the SMTP transport. - -The goal is to put a single, simple guard at the central chokepoint that protects a recipient's mailbox from being flooded, without breaking legitimate bulk sending patterns (org-wide announcements, batches of invitations). - -## Goal - -Add a daily per-recipient rate limit at the central `sendMail` function in `api/src/mails/service.ts`. When a recipient exceeds the limit, drop the email to that recipient silently and log via `internalError` for admin investigation. - -## Non-goals - -- No per-IP or per-sender limiting inside `sendMail`. The mailbox-protection use case is fully covered by per-recipient limiting. -- No retry / backoff / queueing. Silent skip is sufficient for the abuse cases we're guarding against. -- No change to the existing `/mails/contact` per-IP limiter or the auth limiter — they serve different purposes and remain in place. - -## Design - -### Defaults - -- 500 emails per rolling 24h window, per recipient email address. -- Configurable via `config.mailsRateLimit.points` and `config.mailsRateLimit.duration` (seconds), following the pattern of `config.authRateLimit`. -- Environment overrides through `api/config/custom-environment-variables.cjs` (e.g. `MAILS_RATE_LIMIT_POINTS`, `MAILS_RATE_LIMIT_DURATION`). - -### Behavior - -1. Inside `sendMail(to, params, attachments)`, before any template rendering or transport call: - - Parse `to` into individual addresses by splitting on `,`, trimming, and lowercasing each. The `/mails` webhook router joins organization members with `[...to].join(', ')`, so a single `sendMail` call can have many recipients. - - For each address, consume 1 point from the mail rate limiter (`keyPrefix: 'sd-rate-limiter-mail'`). - - Build a list of allowed addresses. For each rejected address, call `internalError('mail-rate-limited', ...)` with the address and the email subject so the source can be investigated. -2. If at least one address is allowed: - - Rebuild `to` from the allowed addresses joined with `, ` and continue with the existing template-rendering + transport-sending logic unchanged. -3. If all addresses are rejected: - - Log via `internalError` and return without invoking `mailsTransport.sendMail`. The caller does not see an error. - -### Why silent skip rather than throw - -Throwing from `sendMail` would surface to the calling route as a 5xx error. A buggy webhook in a loop would then keep retrying on failure, making the abuse worse. By skipping silently and logging at `internalError` level, the abuser stops being amplified, and the operator gets the signal they need to find the source. - -### Why per-recipient and not per-sender - -The threat model is "a single user's mailbox gets flooded". The sender is sometimes an internal worker (planned deletion notices, invitations cron) where per-sender limiting would break legitimate fan-out. Per-recipient limiting cleanly addresses the threat regardless of how many distinct call sites trigger sends. - -### Why 500 / 24h - -- An org-wide announcement to 100 members costs each recipient 1 / 500 of their daily quota — no interference with normal usage. -- A user invited to many orgs, receiving action emails from several workflows, or going through 2FA flows is well under this in any realistic scenario. -- A loop bug or runaway webhook emitting one mail per iteration hits the limit within minutes and stops being amplified to that mailbox. -- The value is configurable, so operators with unusual usage patterns can tune it. - -## Components - -### 1. Config - -Add to `api/config/default.cjs` near the existing `authRateLimit`: - -```js -mailsRateLimit: { - points: 500, - duration: 86400 -}, -``` - -Add to `api/config/custom-environment-variables.cjs` so the values can be overridden by environment variables, matching how other rate-limit / numeric values are exposed. - -Test config (`api/config/test.cjs`): leave `mailsRateLimit` at production defaults (or higher) so existing mail-touching tests are unaffected. The rate-limit test exercises the limit by overriding the limiter directly in the test (see Testing below) rather than relying on a global low limit. - -### 2. Limiter factory - -Extend `api/src/utils/limiter.ts` (the file that already exports the auth limiter) with a second lazy-initialized `RateLimiterMongo`: - -- `keyPrefix: 'sd-rate-limiter-mail'` -- `points: config.mailsRateLimit.points` -- `duration: config.mailsRateLimit.duration` - -Export a named function (e.g. `mailLimiter()`) that returns the same shape as the existing auth limiter: `(key: string) => Promise` — resolves to `true` if a point was consumed, `false` if the recipient is over the limit. Any other error from `RateLimiterMongo` rethrows, matching the existing pattern. - -### 3. `sendMail` integration - -In `api/src/mails/service.ts`, at the top of `sendMail`: - -- Parse the recipient list (split on `,`, trim, lowercase). -- For each recipient address, call `mailLimiter()(address)`. -- If the limiter returns `false`, push the address into a `rejected` list; otherwise into an `allowed` list. -- For each rejected address, call `internalError('mail-rate-limited', { to: address, subject: params.subject })`. -- If `allowed.length === 0`, call one summary `internalError` and `return` (no transport call). Return `undefined`. -- Otherwise replace the local `to` with `allowed.join(', ')` and proceed with the existing logic unchanged. - -The `events.emit('send', ...)` call (used by the maildev preview integration) should fire only for the actual send, after rate-limiting — so it stays where it is, just after the recipient filtering. - -## Testing - -New test file: `tests/mail-rate-limit.unit.spec.ts` (in-process server). The limiter is exercised at low `points` (e.g. 3) by injecting / monkey-patching at the test boundary — either by stubbing the exported limiter factory in `api/src/utils/limiter.ts`, or by constructing a fresh `RateLimiterMongo` with a unique `keyPrefix` and replacing the module-level reference in `sendMail` for the duration of the test. Pick whichever fits the existing test patterns best. - -Cases: - -1. **Single recipient over the limit:** call `sendMail` 4 times in a row to the same address with `points: 3`. Assert the SMTP transport (`mailsTransport.sendMail`) was invoked exactly 3 times, and `internalError` was logged for the 4th. -2. **Multi-recipient mixed:** pre-exhaust one recipient's bucket, then call `sendMail` with a comma-separated `to` containing both the exhausted address and a fresh one. Assert the transport is called once with `to` equal to only the fresh address. -3. **All recipients exhausted:** pre-exhaust two recipients, send to both. Assert the transport is not called and the function returns without throwing. - -Existing mail-touching tests must continue to pass unchanged. - -## Risks and edge cases - -- **The `events.emit('send', ...)`** is used by the maildev preview / observability hooks. It should not fire for skipped recipients. Keep it after rate-limit filtering. -- **Address normalization:** lowercased + trimmed only — no full RFC 5321 normalization. Good enough for limiting; an attacker crafting case variations is not the threat model (this is a defense against accidental loops, not adversarial bypass). -- **MongoDB unavailable at startup:** `RateLimiterMongo` is created lazily on first use; same behavior as the existing auth and contact limiters.