diff --git a/.env.example b/.env.example index fbc04b5..6c19416 100644 --- a/.env.example +++ b/.env.example @@ -18,6 +18,10 @@ POSTGRES_PASSWORD=CHANGE_ME_use_a_strong_password POSTGRES_DB=eventsnap # Connection pool size. Default 10. For a busy event (~100 guests polling the feed # + SSE + uploads at once) raise to ~30 so requests don't queue on a pool permit. +# PAIRED WITH THE DB CONTAINER'S MEMORY LIMIT: 30 backends plus Postgres 16's default +# shared_buffers is already snug in the 1G that docker-compose.yml allots the `db` +# service. If you raise this, raise `db.deploy.resources.limits.memory` with it — an +# OOM in Postgres doesn't degrade one feature, it takes the whole event down. DATABASE_MAX_CONNECTIONS=30 # ── Authentication ──────────────────────────────────────────────────────────── diff --git a/backend/migrations/020_social_rate.down.sql b/backend/migrations/020_social_rate.down.sql new file mode 100644 index 0000000..f01f7dd --- /dev/null +++ b/backend/migrations/020_social_rate.down.sql @@ -0,0 +1 @@ +DELETE FROM config WHERE key IN ('social_rate_per_min', 'social_rate_enabled'); diff --git a/backend/migrations/020_social_rate.up.sql b/backend/migrations/020_social_rate.up.sql new file mode 100644 index 0000000..71eac8f --- /dev/null +++ b/backend/migrations/020_social_rate.up.sql @@ -0,0 +1,16 @@ +-- Per-user rate limit for social writes (likes, comments, comment deletions). +-- +-- These were the only writes in the app with no limit at all. Every other mutating +-- path -- upload, join, recover, export, admin login -- carries one; social.rs +-- carried none, so the coverage was asymmetric rather than deliberately open. +-- +-- Severity is genuinely low for an invited-guest event, and the amplification worry +-- turned out to be contained: a like fans an SSE broadcast to ~100 clients, but the +-- export regeneration it could otherwise trigger is debounced (REGEN_DEBOUNCE 20s) +-- and superseded workers are inert. So this closes the gap for symmetry, not urgency, +-- and the ceiling is set high enough that no real guest will ever meet it -- a +-- double-tapping enthusiast at a wedding is not the thing being defended against. +INSERT INTO config (key, value) VALUES + ('social_rate_per_min', '120'), + ('social_rate_enabled', 'true') +ON CONFLICT (key) DO NOTHING; diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index c31a95d..d001c4d 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -127,6 +127,9 @@ pub async fn patch_config( // Same shape for /recover: the per-(ip, name) bucket is the anti-guessing control, // this only bounds a name-cycling flood in front of a cost-12 bcrypt (migration 019). ("recover_ip_rate_per_min", true, 1.0, 100_000.0), + // Aggregate ceiling on likes + comments + comment deletions, per user per minute. + // These were the only mutating endpoints with no limit at all (migration 020). + ("social_rate_per_min", true, 1.0, 100_000.0), ("quota_tolerance", false, 0.0, 1.0), ("estimated_guest_count", true, 1.0, 1_000_000.0), ]; @@ -141,6 +144,7 @@ pub async fn patch_config( // missing from this allowlist — so the switch existed in code and could never be flipped. "admin_login_rate_enabled", "recover_rate_enabled", + "social_rate_enabled", "quota_enabled", "storage_quota_enabled", "upload_count_quota_enabled", diff --git a/backend/src/handlers/social.rs b/backend/src/handlers/social.rs index d1cb7d1..a0f7db0 100644 --- a/backend/src/handlers/social.rs +++ b/backend/src/handlers/social.rs @@ -10,8 +10,40 @@ use crate::error::AppError; use crate::models::comment::{Comment, CommentDto}; use crate::models::hashtag::{self, Hashtag}; use crate::models::upload::Upload; +use crate::services::config; use crate::state::AppState; +/// Throttle a social write. Keyed PER USER, like the feed and upload limits and for the same +/// reason: at a venue every guest sits behind one NAT, so an IP key hands the whole party a +/// single bucket and the most active guest starves everyone else. +/// +/// These were the only mutating endpoints in the app with no limit at all — the coverage was +/// asymmetric, not deliberately open. The ceiling is set well above anything a real guest +/// produces; this bounds a script, not an enthusiastic double-tapper. +async fn check_social_rate(state: &AppState, user_id: Uuid) -> Result<(), AppError> { + let rate_limits_on = config::get_bool(&state.config_cache, "rate_limits_enabled", true).await; + let social_rate_on = config::get_bool(&state.config_cache, "social_rate_enabled", true).await; + if !(rate_limits_on && social_rate_on) { + return Ok(()); + } + let rate_limit = config::get_usize(&state.config_cache, "social_rate_per_min", 120).await; + // ONE bucket across likes, comments and comment deletions. Separate buckets would let a + // caller triple the aggregate write rate just by alternating between them. + state + .rate_limiter + .check_with_retry( + format!("social:{user_id}"), + rate_limit, + std::time::Duration::from_secs(60), + ) + .map_err(|retry_after_secs| { + AppError::TooManyRequests( + "Zu viele Aktionen. Bitte warte kurz und versuche es erneut.".into(), + Some(retry_after_secs), + ) + }) +} + #[derive(Serialize)] pub struct LikeResponse { /// The caller's like state *after* this toggle. The client sets `liked_by_me` from @@ -35,6 +67,7 @@ pub async fn toggle_like( if user.is_banned { return Err(AppError::Forbidden("Du bist gesperrt.".into())); } + check_social_rate(&state, auth.user_id).await?; // Event-scope: the upload must belong to the caller's event (404 otherwise), // matching the host handlers' find_by_id_and_event pattern. @@ -141,6 +174,7 @@ pub async fn add_comment( if user.is_banned { return Err(AppError::Forbidden("Du bist gesperrt.".into())); } + check_social_rate(&state, auth.user_id).await?; // Event-scope: only comment on an upload that belongs to the caller's event. Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id) @@ -216,6 +250,7 @@ pub async fn delete_comment( if auth.is_banned { return Err(AppError::Forbidden("Du bist gesperrt.".into())); } + check_social_rate(&state, auth.user_id).await?; let comment = Comment::find_by_id(&state.pool, comment_id) .await? .ok_or_else(|| AppError::NotFound("Kommentar nicht gefunden.".into()))?; diff --git a/backend/src/handlers/test_admin.rs b/backend/src/handlers/test_admin.rs index 362c61a..e3c7f3a 100644 --- a/backend/src/handlers/test_admin.rs +++ b/backend/src/handlers/test_admin.rs @@ -63,6 +63,7 @@ pub async fn truncate_all( ('export_rate_per_day', '3'), ('join_ip_rate_per_min', '60'), ('recover_ip_rate_per_min', '30'), + ('social_rate_per_min', '120'), ('quota_tolerance', '0.75'), ('estimated_guest_count', '100'), ('compression_concurrency', '2'), @@ -71,6 +72,7 @@ pub async fn truncate_all( ('feed_rate_enabled', 'false'), ('export_rate_enabled', 'false'), ('join_rate_enabled', 'false'), + ('social_rate_enabled', 'false'), ('admin_login_rate_enabled', 'false'), ('quota_enabled', 'false'), ('storage_quota_enabled', 'false'), diff --git a/docker-compose.yml b/docker-compose.yml index 7e26de9..734d6b3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -17,7 +17,15 @@ services: deploy: resources: limits: - memory: 512M + # 1G, not 512M. DATABASE_MAX_CONNECTIONS defaults to 30 for a ~100-guest event + # (feed polling + SSE + uploads at once), and 30 backends plus Postgres 16's + # default shared_buffers leaves very little headroom at 512M. An OOM here does + # not degrade one feature — it takes the event down, because every request + # path touches the database. Memory is the cheaper knob than shrinking the + # pool back and reintroducing the queueing it was raised to fix. + # + # Raising DATABASE_MAX_CONNECTIONS further means raising this too. + memory: 1G app: build: diff --git a/e2e/specs/03-feed/social-rate-limit.spec.ts b/e2e/specs/03-feed/social-rate-limit.spec.ts new file mode 100644 index 0000000..ca57223 --- /dev/null +++ b/e2e/specs/03-feed/social-rate-limit.spec.ts @@ -0,0 +1,136 @@ +/** + * Regression guard — likes, comments and comment deletions are rate limited. + * + * These were the only mutating endpoints in the app with no limit at all. Every other write path + * -- upload, join, recover, export, admin login -- carried one; `social.rs` carried none, so the + * coverage was asymmetric rather than deliberately open. + * + * Severity is genuinely low for an invited-guest event, and the amplification worry is contained: + * a like does fan an SSE broadcast to every connected client, but the export regeneration a + * comment deletion triggers is debounced (REGEN_DEBOUNCE 20s) and superseded workers are inert. So + * this closes the gap for symmetry, and the ceiling is set well above anything a real guest + * produces -- it bounds a script, not an enthusiastic double-tapper. + * + * The bucket is shared across all three actions on purpose: separate buckets would let a caller + * triple the aggregate write rate just by alternating between them. That is what the second test + * pins, and it is the part most likely to be lost in a refactor. + * + * Keyed per USER, not per IP — at a venue every guest is behind one NAT, so an IP key would hand + * the whole party one bucket. Third test. + */ +import { test, expect } from '../../fixtures/test'; +import { seedUpload } from '../../helpers/seed'; +import { BASE } from '../../helpers/env'; + +const like = (jwt: string, uploadId: string) => + fetch(`${BASE}/api/v1/upload/${uploadId}/like`, { + method: 'POST', + headers: { Authorization: `Bearer ${jwt}` }, + }); + +const comment = (jwt: string, uploadId: string, body: string) => + fetch(`${BASE}/api/v1/upload/${uploadId}/comments`, { + method: 'POST', + headers: { Authorization: `Bearer ${jwt}`, 'Content-Type': 'application/json' }, + body: JSON.stringify({ body }), + }); + +test.describe('Social — rate limit', () => { + test('a burst of likes past the ceiling returns 429 with Retry-After', async ({ + api, + adminToken, + guest, + }) => { + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + social_rate_enabled: 'true', + social_rate_per_min: '3', + }); + + const g = await guest('Tapper'); + const uploadId = await seedUpload(g.jwt); + + // Sequential, not parallel: a toggle flips state, so ordering matters for the assertion. + const statuses: number[] = []; + for (let i = 0; i < 5; i++) statuses.push((await like(g.jwt, uploadId)).status); + + expect(statuses.slice(0, 3), 'the first three are within the ceiling').toEqual([200, 200, 200]); + expect(statuses.slice(3), 'everything past it is refused').toEqual([429, 429]); + + const limited = await like(g.jwt, uploadId); + expect(limited.status).toBe(429); + expect( + limited.headers.get('retry-after'), + 'a 429 without Retry-After tells the client nothing about when to come back' + ).toBeTruthy(); + }); + + test('likes and comments share one bucket', async ({ api, adminToken, guest }) => { + // THE assertion. Per-action buckets would let a caller triple the aggregate write rate by + // alternating, which defeats the point of having a ceiling at all. + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + social_rate_enabled: 'true', + social_rate_per_min: '2', + }); + + const g = await guest('Mixer'); + const uploadId = await seedUpload(g.jwt); + + expect((await like(g.jwt, uploadId)).status).toBe(200); + expect((await comment(g.jwt, uploadId, 'schön!')).status).toBe(201); + // Two writes spent, whichever endpoints they went to. + expect( + (await comment(g.jwt, uploadId, 'noch eins')).status, + 'a comment must consume the same budget a like does' + ).toBe(429); + expect((await like(g.jwt, uploadId)).status).toBe(429); + }); + + test('one guest hitting the ceiling does not block another', async ({ + api, + adminToken, + guest, + }) => { + // Keyed per user, not per IP. Every request in this suite comes from one address, which is + // exactly the venue-NAT shape that made the /join and /feed limits turn guests away. + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + social_rate_enabled: 'true', + social_rate_per_min: '2', + }); + + const noisy = await guest('Noisy'); + const quiet = await guest('Quiet'); + const uploadId = await seedUpload(noisy.jwt); + + for (let i = 0; i < 3; i++) await like(noisy.jwt, uploadId); + expect((await like(noisy.jwt, uploadId)).status).toBe(429); + + expect( + (await like(quiet.jwt, uploadId)).status, + 'a second guest behind the same IP must have their own budget' + ).toBe(200); + }); + + test('flipping social_rate_enabled off bypasses the limit', async ({ + api, + adminToken, + guest, + }) => { + // The toggle has to actually be honoured, or the admin switch is decorative — the failure + // mode two other per-area toggles already shipped with. + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + social_rate_enabled: 'false', + social_rate_per_min: '2', + }); + + const g = await guest('Unlimited'); + const uploadId = await seedUpload(g.jwt); + + const statuses: number[] = []; + for (let i = 0; i < 6; i++) statuses.push((await like(g.jwt, uploadId)).status); + expect(statuses.every((s) => s === 200)).toBe(true); + }); +}); diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index fae72cb..aacb17c 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -84,9 +84,16 @@ { key: 'feed_rate_enabled', label: 'Feed-Limit aktiv', kind: 'bool' }, { key: 'export_rate_enabled', label: 'Export-Limit aktiv', kind: 'bool' }, { key: 'join_rate_enabled', label: 'Join-Limit aktiv', kind: 'bool' }, + { key: 'social_rate_enabled', label: 'Interaktions-Limit aktiv', kind: 'bool' }, { key: 'upload_rate_per_hour', label: 'Upload-Limit pro Stunde', kind: 'number' }, { key: 'feed_rate_per_min', label: 'Feed-Anfragen pro Minute', kind: 'number' }, - { key: 'export_rate_per_day', label: 'Export-Downloads pro Tag', kind: 'number' } + { key: 'export_rate_per_day', label: 'Export-Downloads pro Tag', kind: 'number' }, + { + key: 'social_rate_per_min', + label: 'Interaktionen pro Minute', + kind: 'number', + hint: 'Likes, Kommentare und Kommentar-Löschungen zusammen, pro Gast. Bewusst hoch angesetzt — soll ein Skript bremsen, keinen begeisterten Gast.' + } ] }, {