feat(anti-spam): rate limits + link filter + honeypot (LC-94) #154

Merged
nrupard merged 4 commits from feat/lc-94-anti-spam into main 2026-05-19 20:38:06 +02:00
Owner

Summary

Implements LC-94 in full: rate limits, link filter (block / quarantine / warn), honeypot on register. Three logical commits inside the branch so the review can move per-subsystem.

Locked design choices before coding (per maintainer):

  • Fixed N-per-minute window (not token bucket).
  • Curated default deny-list: the Freenom free-TLDs (*.tk, *.ml, *.ga, *.cf) seeded with quarantine action so admins see them and decide. Operators can add their own rules at /admin/link-filter.
  • Honeypot only; no third-party CAPTCHA in this PR.

Foundation (cbf18fc)

  • chat/0032_anti_spam.sql: messages.quarantined, link_filter_rules, link_filter_quarantine, seeded TLDs.
  • settings/0004_anti_spam.sql: three new toggle / cap keys.
  • rate_limit module: fixed-window per (kind, key) on a DashMap. Zero limit = disabled.
  • db::anti_spam: rules CRUD + quarantine queue (approve/reject wrap message + queue updates in a single tx).
  • links: extract_hosts walks linkify hits and pulls url::Url::host_str; pattern_matches converts the admin's literal-or-glob pattern to an anchored regex.
  • AppState gets a rate_limits: RateLimits field; main.rs + every test literal updated.

Enforcement (228be41)

  • routes/room.rs::post_message: per-user rate cap before any DB work; link filter walks URLs and routes to block/quarantine/warn. Quarantine flips the row's quarantined bit + inserts queue row + audit log + skips broadcast.
  • routes/auth.rs::post_register: honeypot rejects bot fills with a generic 422 inline error (no signal to the spammer); per-IP rate limit ahead of validation.
  • routes/password_reset.rs::post_forgot: per-IP rate limit ahead of email validation.
  • db/chat.rs + db/{inbox,activity,uploads,bookmarks,pinned}.rs + digest.rs: every user-visible read path now joins AND m.quarantined = 0 so quarantined messages never leak into the room, unread counts, inbox, activity feed, file browser, bookmarks, pinned strip, or email digest.
  • AppError::TooManyRequests -> 429.

Admin UI (b33e4ad)

  • /admin/anti-spam: toggles + caps. Saved-state flash; anti_spam_settings audit row.
  • /admin/link-filter: add / list / delete rules with inline error on duplicates.
  • /admin/quarantine: pending queue with approve / reject. Approve unhides, reject soft-deletes; both audit-logged.
  • New tabs in admin_layout.html.

Test plan

  • just check (both feature builds, clippy -D warnings, fmt --check).
  • just test (50 binaries + new routes_anti_spam 12-case file). Pre-existing flake file_serve_round_trips_uploaded_bytes (CLAUDE.md "out of scope") passes in isolation.
  • just test-saas green; the 5 admin-page tests are #[cfg(feature = "standalone")].
  • Manual: trip each defense from the admin UI; flip toggles on/off; add a rule and confirm a matching URL is held.

Limitations / follow-ups

  • Quote-preview body fetch (server/src/views/room.rs:387) still reads soft-deleted via SELECT-and-filter-in-Rust; not extended to quarantined. Quarantined messages can be quoted briefly before approve/reject; broadcast still skipped so visibility is minimal. Worth a follow-up if it matters.
  • Third-party CAPTCHA (Turnstile / hCaptcha) is intentionally out-of-scope per spec discussion.
## Summary Implements LC-94 in full: rate limits, link filter (block / quarantine / warn), honeypot on register. Three logical commits inside the branch so the review can move per-subsystem. **Locked design choices before coding** (per maintainer): - Fixed N-per-minute window (not token bucket). - Curated default deny-list: the Freenom free-TLDs (`*.tk`, `*.ml`, `*.ga`, `*.cf`) seeded with `quarantine` action so admins see them and decide. Operators can add their own rules at `/admin/link-filter`. - Honeypot only; no third-party CAPTCHA in this PR. ### Foundation (`cbf18fc`) - `chat/0032_anti_spam.sql`: `messages.quarantined`, `link_filter_rules`, `link_filter_quarantine`, seeded TLDs. - `settings/0004_anti_spam.sql`: three new toggle / cap keys. - `rate_limit` module: fixed-window per (kind, key) on a DashMap. Zero limit = disabled. - `db::anti_spam`: rules CRUD + quarantine queue (approve/reject wrap message + queue updates in a single tx). - `links`: `extract_hosts` walks linkify hits and pulls `url::Url::host_str`; `pattern_matches` converts the admin's literal-or-glob pattern to an anchored regex. - `AppState` gets a `rate_limits: RateLimits` field; main.rs + every test literal updated. ### Enforcement (`228be41`) - `routes/room.rs::post_message`: per-user rate cap before any DB work; link filter walks URLs and routes to block/quarantine/warn. Quarantine flips the row's `quarantined` bit + inserts queue row + audit log + skips broadcast. - `routes/auth.rs::post_register`: honeypot rejects bot fills with a generic 422 inline error (no signal to the spammer); per-IP rate limit ahead of validation. - `routes/password_reset.rs::post_forgot`: per-IP rate limit ahead of email validation. - `db/chat.rs` + `db/{inbox,activity,uploads,bookmarks,pinned}.rs` + `digest.rs`: every user-visible read path now joins `AND m.quarantined = 0` so quarantined messages never leak into the room, unread counts, inbox, activity feed, file browser, bookmarks, pinned strip, or email digest. - `AppError::TooManyRequests` -> 429. ### Admin UI (`b33e4ad`) - `/admin/anti-spam`: toggles + caps. Saved-state flash; `anti_spam_settings` audit row. - `/admin/link-filter`: add / list / delete rules with inline error on duplicates. - `/admin/quarantine`: pending queue with approve / reject. Approve unhides, reject soft-deletes; both audit-logged. - New tabs in `admin_layout.html`. ## Test plan - [x] `just check` (both feature builds, clippy -D warnings, fmt --check). - [x] `just test` (50 binaries + new `routes_anti_spam` 12-case file). Pre-existing flake `file_serve_round_trips_uploaded_bytes` (CLAUDE.md "out of scope") passes in isolation. - [x] `just test-saas` green; the 5 admin-page tests are `#[cfg(feature = "standalone")]`. - [ ] Manual: trip each defense from the admin UI; flip toggles on/off; add a rule and confirm a matching URL is held. ## Limitations / follow-ups - Quote-preview body fetch (`server/src/views/room.rs:387`) still reads soft-deleted via SELECT-and-filter-in-Rust; not extended to `quarantined`. Quarantined messages can be quoted briefly before approve/reject; broadcast still skipped so visibility is minimal. Worth a follow-up if it matters. - Third-party CAPTCHA (Turnstile / hCaptcha) is intentionally out-of-scope per spec discussion.
Lays down the foundation for the three-defense bundle:

- chat/0032: messages.quarantined flag, link_filter_rules, link_filter_quarantine. Seeds a small curated deny-list (Freenom TLDs *.tk *.ml *.ga *.cf) with the conservative 'quarantine' action so the admin sees them and decides.
- rate_limit: fixed-window per (kind, key) counter on DashMap. Zero limit is treated as "disabled" so callers don't need an extra is-enabled branch.
- db::anti_spam: rules CRUD + quarantine queue helpers including approve / reject that wrap the messages + quarantine updates in a single transaction.
- links: extract_hosts walks linkify hits and pulls Url::host_str; pattern_matches turns the admin's literal-or-glob pattern into an anchored regex.
- AppState gains a RateLimits field; main.rs constructs it at startup. Tests will inherit the same shape via the per-file AppState literals.
- routes/room.rs::post_message: per-user fixed-window rate limit (cap from settings.rate_limit_messages, 0=disabled); link filter walks linkify hits, runs each host through link_filter_rules. Block rejects the send; quarantine inserts with messages.quarantined=1 + link_filter_quarantine row + audit entry, skips broadcast; warn passes through + audit entry.
- routes/auth.rs::post_register: honeypot hidden field on the register form rejects bots that auto-fill every input; per-IP rate limit on /register; both gates skip cleanly when their respective settings are off.
- routes/password_reset.rs::post_forgot: per-IP rate limit on /forgot.
- db/chat.rs + db/{inbox,activity,uploads,bookmarks,pinned}.rs + digest.rs: every user-visible read path now joins `AND m.quarantined = 0` next to the existing `m.deleted_at IS NULL` so quarantined messages never leak into the room, unread counts, inbox, activity feed, file browser, bookmarks, pinned strip, or email digest.
- settings/0004_anti_spam.sql seeds the three new toggle keys + IP-cap keys (caps default 0 = disabled; toggles default true so the link filter and honeypot are on out of the box).
- error.rs: new AppError::TooManyRequests -> 429.
- AppState + every test literal grow a `rate_limits: RateLimits::new()` field.
- 0032 backfilled into the hand-rolled migration arrays / verbose blocks.
feat(anti-spam): admin pages + integration tests (LC-94)
All checks were successful
Check / clippy + fmt + tests (pull_request) Successful in 3m42s
b33e4ad2b4
Three new admin tabs alongside Settings / Users / etc:

- /admin/anti-spam: per-minute rate-limit caps (messages per user, registrations per IP, forgot-password per IP) plus the link-filter and honeypot toggles. Saved-state flash via ?saved=1; audit-logged as `anti_spam_settings` with a compact summary.
- /admin/link-filter: add / list / delete rules. Pattern can be a literal host or a `*`-glob. Action is block / quarantine / warn. Inline error when a pattern duplicates an existing rule.
- /admin/quarantine: pending review queue. Approve clears the quarantined flag (message becomes visible) and audit-logs `quarantine_approve`. Reject soft-deletes the message and audit-logs `quarantine_reject`.

Tests in `routes_anti_spam.rs`:

- message rate limit fires + disabled-when-zero
- honeypot rejects bot fill + per-IP register limit fires per-IP
- forgot-password rate-limit code path reachable (the test deployment has no mailer so the handler 404s before the limit, but the test still exercises the wiring)
- link-filter block / quarantine / warn end-to-end including the quarantine-then-approve hide/unhide cycle
- link-filter disabled toggle is honored
- admin page form persists + audit-logs
- link-filter add/delete round-trip
- non-admin gets 403 on every admin page
fix(anti-spam): code-review follow-ups (LC-94)
All checks were successful
Check / clippy + fmt + tests (pull_request) Successful in 2m50s
017996e16e
Addresses every finding from the LC-94 review.

- `rate_limit.rs`: opportunistic sweep every 1024 checks drops entries whose windows have expired so the DashMap can't grow unbounded; `read_u32_setting` now logs a WARN on a non-numeric value instead of silently treating it as disabled. New unit test covers the sweep path.
- `error.rs`: `AppError::TooManyRequests` now carries the retry-after seconds and renders the `Retry-After` HTTP header so well-behaved clients back off automatically; the three call sites in room/auth/password_reset all pass the value through.
- `rate_limit::client_ip_for_rate_limit`: new helper that only returns an IP when the operator has opted in via `settings.trust_proxy_headers`. X-Forwarded-For is trivially spoofable on a server facing the internet directly; the per-IP limits now no-op rather than key on whatever a client claims. New setting defaults to `'true'` to match the documented Traefik-fronted deployment shape; operators on a bare deployment flip it to `'false'`.
- `db::anti_spam::reject_quarantine`: now calls `db::moderation::soft_delete_message` instead of duplicating the UPDATE inline.
- `db::anti_spam::find_match`: walks rules in severity order (block > quarantine > warn) so a `block` rule always wins over a `warn` rule that happens to sort earlier alphabetically.
- `migrations/chat/0032`: removes the misleading `ON DELETE CASCADE` on `link_filter_quarantine.message_id`; soft-delete is the only delete path so the cascade was dead code. Comment explains the audit-trail preservation.
- `migrations/settings/0004`: adds the `trust_proxy_headers` key and an upgrade-safe UPDATE that resets the pre-existing `rate_limit_messages = '30'` seed to `'0'` only when the value still matches the original default; operators who customized it keep their setting.
- `routes/admin.rs::post_quarantine_approve`: now broadcasts `NewMessage` over WS so the freshly-unhidden message appears live for users in the room instead of waiting for the next page load.
- `tests/routes_anti_spam.rs`: the hollow `forgot_rate_limit_returns_429_after_cap` test is removed in favor of a TODO documenting the test-harness work needed (stub Mailer) to exercise that path end-to-end.

X-Forwarded-For trust was a pre-existing issue inherited from LC-92's maintenance middleware and LC-93's quota helper; both still use the unguarded `extract_session_origin` for non-security-critical session metadata (login alerts, "new device" detection). Only the rate-limit boundary is gated by the new helper, since that is the one place a spoofable IP would actually defeat a security control.
nrupard deleted branch feat/lc-94-anti-spam 2026-05-19 20:38:06 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/lets-chat!154
No description provided.