All three are the same mistake in different clothes: a limit keyed on an IP that, behind the venue's NAT, is the entire party plus the host. * join_ip_rate_per_min was raised 60 -> 300 last round and it never took effect. A config default is only a fallback for a MISSING key, and migration 017 seeds this one, so the seed won and the raise was dead code on every real install. Migration 030 raises the seeded value the way 015 already did for upload_rate_per_hour. The e2e guard could not see this: it fires 12 concurrent joins, which is green at 60 and at 300 alike. * /recover's per-(IP, name) bucket charged EVERY request, including successful ones, and refused before verifying the PIN. Its ceiling clamps to 4. So four POSTs naming "Braut Sophie" with PIN 0000, from any phone on the venue wifi, locked Sophie out of her own recovery for fifteen minutes WITH THE CORRECT PIN — and four more every fifteen minutes sustained it indefinitely, at a rate far under every volume ceiling above it. The benign version needs no attacker: the host mistypes their own PIN four times. Hosts are promoted guests whose only credential is that PIN, and /recover is their only way back after losing a session. Now it counts failures, and a spent budget changes what a FAILURE answers instead of refusing outright. Guessing is bounded exactly as before — wrong PINs are what spend it — with the per-account lockout underneath. * /admin/login's pre-verify ceiling had the same shape, and the escape hatch was circular: admin_login_rate_enabled is only flippable through PATCH /admin/config, which needs the session being refused. One phone posting twice a minute cost the operator moderation, gallery release and every config key, including the ones that would undo it. Exceeding the ceiling now shortens the hash-permit wait rather than refusing: the CPU bound was always the semaphore, never this bucket, so a flood still sheds itself while a correct password gets a truthful answer. Adds a regression test that reads the value a fresh database actually ends up with, by replaying the migrations — the drift that made the first bullet invisible is not otherwise detectable from the code.
252 lines
11 KiB
Rust
252 lines
11 KiB
Rust
//! Reads of the runtime-tunable `config` table, fronted by an in-memory cache.
|
|
//!
|
|
//! Each handler used to keep a small local copy of these helpers; consolidating them
|
|
//! here means one place to add a parser, one place to mock for tests, and one place to
|
|
//! find when a key changes. New keys do not require code changes — they're picked up
|
|
//! the next time the cache reloads.
|
|
//!
|
|
//! ## Why a cache
|
|
//!
|
|
//! The `config` table is effectively static during an event, yet it was the busiest
|
|
//! query in the system: every request re-read each key with its own `SELECT`
|
|
//! (an upload touched it ~8 times). Against the small connection pool that was the
|
|
//! throughput ceiling. [`ConfigCache`] loads the whole table once and serves reads
|
|
//! from memory.
|
|
//!
|
|
//! ## Consistency contract
|
|
//!
|
|
//! Correctness comes from **synchronous invalidation on every write**, not from the
|
|
//! TTL. The two runtime write paths — the admin `PATCH /admin/config` handler and the
|
|
//! test-mode truncate/reseed — both call [`ConfigCache::invalidate`] after committing,
|
|
//! so the *next* read reloads from the DB and sees the new value immediately. The
|
|
//! [`RELOAD_TTL`] is only a safety net for out-of-band changes (e.g. a migration or a
|
|
//! manual DB edit); it is deliberately short but never the primary mechanism.
|
|
//!
|
|
//! Values are read with a default fallback so the app still starts if a key is missing
|
|
//! (e.g. during a migration window). Production seeds keys via migrations 005 and 009.
|
|
|
|
use std::collections::HashMap;
|
|
use std::sync::{Arc, RwLock};
|
|
use std::time::{Duration, Instant};
|
|
|
|
use sqlx::PgPool;
|
|
|
|
/// How long a loaded snapshot is trusted before the next read reloads it. This is a
|
|
/// backstop for out-of-band DB changes only — every in-process write invalidates the
|
|
/// cache synchronously, so tests that PATCH-then-assert never depend on this expiring.
|
|
const RELOAD_TTL: Duration = Duration::from_secs(30);
|
|
|
|
struct Snapshot {
|
|
values: HashMap<String, String>,
|
|
loaded_at: Instant,
|
|
}
|
|
|
|
/// In-memory cache of the entire `config` table. Cheap to `clone` (shares the pool and
|
|
/// the `Arc`), so it lives in `AppState` and every handler reads through it.
|
|
#[derive(Clone)]
|
|
pub struct ConfigCache {
|
|
pool: PgPool,
|
|
inner: Arc<RwLock<Option<Snapshot>>>,
|
|
}
|
|
|
|
impl ConfigCache {
|
|
pub fn new(pool: PgPool) -> Self {
|
|
Self {
|
|
pool,
|
|
inner: Arc::new(RwLock::new(None)),
|
|
}
|
|
}
|
|
|
|
/// Drop the cached snapshot so the next read reloads the whole table from the DB.
|
|
/// Call this after any write to the `config` table (admin PATCH, test reseed).
|
|
pub fn invalidate(&self) {
|
|
*self.inner.write().unwrap() = None;
|
|
}
|
|
|
|
/// Return the fresh snapshot if one is loaded and still within [`RELOAD_TTL`].
|
|
fn fresh_snapshot(&self) -> Option<HashMap<String, String>> {
|
|
let guard = self.inner.read().unwrap();
|
|
match guard.as_ref() {
|
|
Some(snap) if snap.loaded_at.elapsed() < RELOAD_TTL => Some(snap.values.clone()),
|
|
_ => None,
|
|
}
|
|
}
|
|
|
|
/// Read one key, loading the whole table into the cache on a miss/expiry. On a DB
|
|
/// error we return `None` (callers fall back to their default) without poisoning
|
|
/// the cache.
|
|
async fn get_raw(&self, key: &str) -> Option<String> {
|
|
if let Some(values) = self.fresh_snapshot() {
|
|
return values.get(key).cloned();
|
|
}
|
|
|
|
// Cache miss or stale — reload the entire table in one query.
|
|
let rows: Vec<(String, String)> = match sqlx::query_as::<_, (String, String)>(
|
|
"SELECT key, value FROM config",
|
|
)
|
|
.fetch_all(&self.pool)
|
|
.await
|
|
{
|
|
Ok(rows) => rows,
|
|
Err(e) => {
|
|
tracing::warn!(error = ?e, "config reload failed; using defaults for this read");
|
|
return None;
|
|
}
|
|
};
|
|
|
|
let values: HashMap<String, String> = rows.into_iter().collect();
|
|
let result = values.get(key).cloned();
|
|
*self.inner.write().unwrap() = Some(Snapshot {
|
|
values,
|
|
loaded_at: Instant::now(),
|
|
});
|
|
result
|
|
}
|
|
}
|
|
|
|
pub async fn get_str(cache: &ConfigCache, key: &str, default: &str) -> String {
|
|
cache
|
|
.get_raw(key)
|
|
.await
|
|
.unwrap_or_else(|| default.to_string())
|
|
}
|
|
|
|
pub async fn get_i64(cache: &ConfigCache, key: &str, default: i64) -> i64 {
|
|
cache
|
|
.get_raw(key)
|
|
.await
|
|
.and_then(|v| v.parse().ok())
|
|
.unwrap_or(default)
|
|
}
|
|
|
|
pub async fn get_usize(cache: &ConfigCache, key: &str, default: usize) -> usize {
|
|
cache
|
|
.get_raw(key)
|
|
.await
|
|
.and_then(|v| v.parse().ok())
|
|
.unwrap_or(default)
|
|
}
|
|
|
|
pub async fn get_f64(cache: &ConfigCache, key: &str, default: f64) -> f64 {
|
|
cache
|
|
.get_raw(key)
|
|
.await
|
|
.and_then(|v| v.parse().ok())
|
|
.unwrap_or(default)
|
|
}
|
|
|
|
/// Parses common truthy spellings used by both the migration seeds and the admin form.
|
|
/// Accepts `true/false`, `1/0`, `yes/no`, `on/off` — case-insensitive. Anything else
|
|
/// returns `default`.
|
|
pub async fn get_bool(cache: &ConfigCache, key: &str, default: bool) -> bool {
|
|
let Some(raw) = cache.get_raw(key).await else {
|
|
return default;
|
|
};
|
|
match raw.trim().to_ascii_lowercase().as_str() {
|
|
"true" | "1" | "yes" | "on" => true,
|
|
"false" | "0" | "no" | "off" => false,
|
|
_ => default,
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod seed_tests {
|
|
/// The value a fresh database actually ends up with for `key`, by replaying the migrations.
|
|
///
|
|
/// This exists because a `config::get_*` default is only a fallback for a MISSING key, and the
|
|
/// migrations seed nearly every key there is. So the literal in the handler is dead code on any
|
|
/// real install, and changing it changes nothing — which is exactly what happened to
|
|
/// `join_ip_rate_per_min`: it was raised 60 → 300 in `auth/handlers.rs` to stop one QR-code
|
|
/// burst from locking the venue out of `/join`, shipped, and did nothing at all, because
|
|
/// migration 017 seeds 60 and the seed wins. Nothing in the test suite could see it: the e2e
|
|
/// regression guard fires 12 concurrent joins, which is green at 60 and at 300 alike.
|
|
fn effective_seed(key: &str) -> Option<String> {
|
|
let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("migrations");
|
|
let mut files: Vec<_> = std::fs::read_dir(&dir)
|
|
.expect("migrations directory")
|
|
.filter_map(|e| e.ok().map(|e| e.path()))
|
|
.filter(|p| p.to_string_lossy().ends_with(".up.sql"))
|
|
.collect();
|
|
// Version order: migrations are applied in filename order and later ones override.
|
|
files.sort();
|
|
|
|
let mut value: Option<String> = None;
|
|
for path in files {
|
|
let sql = std::fs::read_to_string(&path).expect("readable migration");
|
|
for line in sql.lines() {
|
|
let line = line.trim();
|
|
if line.starts_with("--") {
|
|
continue;
|
|
}
|
|
// Seed form: ('key', 'value')
|
|
if let Some(rest) = line.strip_prefix(&format!("('{key}',"))
|
|
&& let Some(v) = rest.split('\'').nth(1)
|
|
{
|
|
value = Some(v.to_string());
|
|
}
|
|
// Update form: UPDATE config SET value = 'new' WHERE key = 'key' AND value = 'old'
|
|
if line.starts_with("UPDATE config SET value")
|
|
&& line.contains(&format!("key = '{key}'"))
|
|
&& let Some(new) = line.split('\'').nth(1)
|
|
{
|
|
let scoped_to = line
|
|
.rsplit_once("AND value = '")
|
|
.and_then(|(_, tail)| tail.split('\'').next().map(|s| s.to_string()));
|
|
// Only applies if the current value still matches the scope it was written for.
|
|
if scoped_to.is_none() || scoped_to.as_deref() == value.as_deref() {
|
|
value = Some(new.to_string());
|
|
}
|
|
}
|
|
}
|
|
}
|
|
value
|
|
}
|
|
|
|
/// The invariant, not the number: `join_ip:{ip}` is keyed on an address the WHOLE VENUE shares
|
|
/// behind NAT, and `/join` is the one screen with no auto-retry. A ceiling near the size of the
|
|
/// party is a ceiling on the party. Asserted against the effective seed rather than the code
|
|
/// default precisely because the code default is what silently did not apply.
|
|
#[test]
|
|
fn the_join_ceiling_a_real_install_gets_is_sized_for_a_whole_venue_arriving_at_once() {
|
|
let seeded = effective_seed("join_ip_rate_per_min")
|
|
.expect("join_ip_rate_per_min must be seeded by a migration");
|
|
let seeded: usize = seeded.parse().expect("numeric");
|
|
assert!(
|
|
seeded >= 300,
|
|
"a fresh database ends up with join_ip_rate_per_min = {seeded}. Every guest shares one \
|
|
NAT address, so this is the ceiling for the entire party scanning one QR code. Raise \
|
|
it with a value-scoped UPDATE migration (see 030) — changing the default in \
|
|
auth/handlers.rs does nothing, because the seed wins."
|
|
);
|
|
}
|
|
|
|
/// Pins the other half of the same trap: the seeded value must not exceed the ceiling the
|
|
/// handler clamps to, or an operator reading `GET /admin/config` sees a number that is not the
|
|
/// one being enforced.
|
|
#[test]
|
|
fn the_seeded_recover_name_ceiling_is_within_what_the_handler_will_honour() {
|
|
let seeded = effective_seed("recover_name_rate_per_15min")
|
|
.expect("recover_name_rate_per_15min must be seeded by a migration");
|
|
let seeded: usize = seeded.parse().expect("numeric");
|
|
assert!(
|
|
seeded <= crate::auth::handlers::RECOVER_NAME_CEILING_MAX,
|
|
"seeded recover_name_rate_per_15min = {seeded} exceeds RECOVER_NAME_CEILING_MAX = {}; \
|
|
the handler clamps at the point of use, so the advertised value would be a lie.",
|
|
crate::auth::handlers::RECOVER_NAME_CEILING_MAX
|
|
);
|
|
}
|
|
|
|
/// The parser itself, against a value migration 015 really does change. Without this a bug in
|
|
/// `effective_seed` makes both tests above vacuously green.
|
|
#[test]
|
|
fn the_seed_parser_follows_a_value_through_a_later_update_migration() {
|
|
assert_eq!(
|
|
effective_seed("upload_rate_per_hour").as_deref(),
|
|
Some("100"),
|
|
"005 seeds 10 and 015 raises it to 100; reading 10 here means the UPDATE form is not \
|
|
being applied, and every assertion built on this helper is worthless."
|
|
);
|
|
assert_eq!(effective_seed("no_such_key_anywhere"), None);
|
|
}
|
|
}
|