Skip to main content

max / makenotwork

security: seal admin gate, close export quarantine bypass, drop legacy sessions (audit Run 20 Phase 2) - admin auth is now structural, not per-handler. admin_routes applies a blanket require_admin_layer over the whole /admin* subtree with a sealed ADMIN_PUBLIC_PATHS allowlist (only the PoM-facing scan-health JSON). A handler that forgets AdminUser can no longer ship unauthenticated. Coverage: an integration test asserts anon->401 / non-admin->404 on gated routes and that health.json stays public; a unit test seals the allowlist. Subsumes the standing scan_health_json finding. - content export no longer bypasses quarantine. Added db::scanning:: quarantined_s3_keys (keyed on the file_scan_results authority the quarantine worker writes) and filter it in export_content, so a creator can't pull back their own confirmed-malicious object that every download path already refuses. The shared get_user_s3_keys accounting query is left untouched. - legacy pre-tracking sessions are no longer trusted. AuthUser and MaybeUserVerified now refuse a session with USER_SESSION_KEY but no SESSION_TRACKING_KEY (it can't be revoked by logout-everywhere/suspend/ password-change), matching MaybeUserUnverified. Forces re-login. - csrf_coverage runs as admin so admin routes still exercise their CSRF layer through the new admin gate. Build runner sandboxing (Run 20 Phase 2a) deferred by decision as a dedicated infra effort. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-03 07:22 UTC
Commit: f958136b9e55a06eb6aad4e36484542c88775873
Parent: 75be30d
7 files changed, +232 insertions, -75 deletions
M server/src/auth.rs +85 -68
@@ -158,47 +158,57 @@ impl FromRequestParts<crate::AppState> for AuthUser {
158 158 .context("session error")?
159 159 .ok_or(AppError::Unauthorized)?;
160 160
161 - // Validate session tracking (skip for legacy sessions without tracking ID).
162 - // Uses an in-memory cache to avoid hitting the DB on every request —
163 - // if this session was validated within SESSION_TOUCH_CACHE_SECS, skip the query.
161 + // Every live session carries a tracking id, set at login by
162 + // `track_session`. A session with USER_SESSION_KEY but no
163 + // SESSION_TRACKING_KEY is a legacy pre-tracking session that cannot be
164 + // revoked — "log out everywhere", suspend, and password-change all act
165 + // on `user_sessions` rows it doesn't have. Refuse it (force re-login)
166 + // rather than trust an unrevocable session (Run 20 Security). Matches
167 + // the short-circuit `MaybeUserUnverified` already applies.
164 168 let mut user = user;
165 - if let Ok(Some(tracking_id)) = session
166 - .get::<UserSessionId>(SESSION_TRACKING_KEY)
167 - .await
168 - {
169 - let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
170 - let cached = state.session_cache.get(&tracking_id)
171 - .map(|entry| entry.elapsed() < cache_ttl)
172 - .unwrap_or(false);
173 -
174 - if !cached {
175 - let result = match db::sessions::touch_session(&state.db, tracking_id).await {
176 - Ok(r) => r,
177 - Err(e) => {
178 - tracing::warn!(error = ?e, "session touch failed, invalidating");
179 - db::sessions::TouchResult { valid: false, suspended: false, can_create_projects: false, is_fan_plus: false, creator_tier: None }
180 - }
181 - };
182 - if !result.valid {
183 - state.session_cache.remove(&tracking_id);
184 - let _ = session.flush().await;
185 - return Err(AppError::Unauthorized);
169 + let tracking_id = match session.get::<UserSessionId>(SESSION_TRACKING_KEY).await {
170 + Ok(Some(id)) => id,
171 + _ => {
172 + let _ = session.flush().await;
173 + return Err(AppError::Unauthorized);
174 + }
175 + };
176 +
177 + // Validate the tracking row. Uses an in-memory cache to avoid hitting
178 + // the DB on every request — if this session was validated within
179 + // SESSION_TOUCH_CACHE_SECS, skip the query.
180 + let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
181 + let cached = state.session_cache.get(&tracking_id)
182 + .map(|entry| entry.elapsed() < cache_ttl)
183 + .unwrap_or(false);
184 +
185 + if !cached {
186 + let result = match db::sessions::touch_session(&state.db, tracking_id).await {
187 + Ok(r) => r,
188 + Err(e) => {
189 + tracing::warn!(error = ?e, "session touch failed, invalidating");
190 + db::sessions::TouchResult { valid: false, suspended: false, can_create_projects: false, is_fan_plus: false, creator_tier: None }
186 191 }
187 - // If the user's live DB state differs from the session, update it.
188 - // touch_session returns suspended, can_create_projects, is_fan_plus,
189 - // and creator_tier in a single query (no extra round-trips).
190 - let live_tier: Option<db::CreatorTier> = result.creator_tier.as_deref().and_then(|s| s.parse().ok());
191 - if user.suspended != result.suspended || user.is_fan_plus != result.is_fan_plus || user.can_create_projects != result.can_create_projects || user.creator_tier != live_tier {
192 - user.suspended = result.suspended;
193 - user.is_fan_plus = result.is_fan_plus;
194 - user.can_create_projects = result.can_create_projects;
195 - user.creator_tier = live_tier;
196 - if let Err(e) = session.insert(USER_SESSION_KEY, user.clone()).await {
197 - tracing::warn!(user_id = %user.id, error = ?e, "failed to update session with refreshed user state");
198 - }
192 + };
193 + if !result.valid {
194 + state.session_cache.remove(&tracking_id);
195 + let _ = session.flush().await;
196 + return Err(AppError::Unauthorized);
197 + }
198 + // If the user's live DB state differs from the session, update it.
199 + // touch_session returns suspended, can_create_projects, is_fan_plus,
200 + // and creator_tier in a single query (no extra round-trips).
201 + let live_tier: Option<db::CreatorTier> = result.creator_tier.as_deref().and_then(|s| s.parse().ok());
202 + if user.suspended != result.suspended || user.is_fan_plus != result.is_fan_plus || user.can_create_projects != result.can_create_projects || user.creator_tier != live_tier {
203 + user.suspended = result.suspended;
204 + user.is_fan_plus = result.is_fan_plus;
205 + user.can_create_projects = result.can_create_projects;
206 + user.creator_tier = live_tier;
207 + if let Err(e) = session.insert(USER_SESSION_KEY, user.clone()).await {
208 + tracing::warn!(user_id = %user.id, error = ?e, "failed to update session with refreshed user state");
199 209 }
200 - state.session_cache.insert(tracking_id, Instant::now());
201 210 }
211 + state.session_cache.insert(tracking_id, Instant::now());
202 212 }
203 213
204 214 // Record user_id in the current span so all downstream logs
@@ -301,40 +311,47 @@ impl FromRequestParts<crate::AppState> for MaybeUserVerified {
301 311 return Ok(MaybeUserVerified(None));
302 312 };
303 313
304 - if let Ok(Some(tracking_id)) = session
305 - .get::<UserSessionId>(SESSION_TRACKING_KEY)
306 - .await
307 - {
308 - let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
309 - let cached = state.session_cache.get(&tracking_id)
310 - .map(|entry| entry.elapsed() < cache_ttl)
311 - .unwrap_or(false);
312 -
313 - if !cached {
314 - let result = match db::sessions::touch_session(&state.db, tracking_id).await {
315 - Ok(r) => r,
316 - Err(e) => {
317 - tracing::warn!(error = ?e, "session touch failed in MaybeUserVerified, treating as anonymous");
318 - db::sessions::TouchResult { valid: false, suspended: false, can_create_projects: false, is_fan_plus: false, creator_tier: None }
319 - }
320 - };
321 - if !result.valid {
322 - state.session_cache.remove(&tracking_id);
323 - let _ = session.flush().await;
324 - return Ok(MaybeUserVerified(None));
314 + // A session with USER_SESSION_KEY but no SESSION_TRACKING_KEY is a legacy
315 + // pre-tracking session that can't be revoked; treat it as anonymous
316 + // rather than trust it (Run 20 Security), matching `AuthUser` and
317 + // `MaybeUserUnverified`.
318 + let tracking_id = match session.get::<UserSessionId>(SESSION_TRACKING_KEY).await {
319 + Ok(Some(id)) => id,
320 + _ => {
321 + let _ = session.flush().await;
322 + return Ok(MaybeUserVerified(None));
323 + }
324 + };
325 +
326 + let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
327 + let cached = state.session_cache.get(&tracking_id)
328 + .map(|entry| entry.elapsed() < cache_ttl)
329 + .unwrap_or(false);
330 +
331 + if !cached {
332 + let result = match db::sessions::touch_session(&state.db, tracking_id).await {
333 + Ok(r) => r,
334 + Err(e) => {
335 + tracing::warn!(error = ?e, "session touch failed in MaybeUserVerified, treating as anonymous");
336 + db::sessions::TouchResult { valid: false, suspended: false, can_create_projects: false, is_fan_plus: false, creator_tier: None }
325 337 }
326 - let live_tier: Option<db::CreatorTier> = result.creator_tier.as_deref().and_then(|s| s.parse().ok());
327 - if user.suspended != result.suspended || user.is_fan_plus != result.is_fan_plus || user.can_create_projects != result.can_create_projects || user.creator_tier != live_tier {
328 - user.suspended = result.suspended;
329 - user.is_fan_plus = result.is_fan_plus;
330 - user.can_create_projects = result.can_create_projects;
331 - user.creator_tier = live_tier;
332 - if let Err(e) = session.insert(USER_SESSION_KEY, user.clone()).await {
333 - tracing::warn!(user_id = %user.id, error = ?e, "failed to update session with refreshed user state");
334 - }
338 + };
339 + if !result.valid {
340 + state.session_cache.remove(&tracking_id);
341 + let _ = session.flush().await;
342 + return Ok(MaybeUserVerified(None));
343 + }
344 + let live_tier: Option<db::CreatorTier> = result.creator_tier.as_deref().and_then(|s| s.parse().ok());
345 + if user.suspended != result.suspended || user.is_fan_plus != result.is_fan_plus || user.can_create_projects != result.can_create_projects || user.creator_tier != live_tier {
346 + user.suspended = result.suspended;
347 + user.is_fan_plus = result.is_fan_plus;
348 + user.can_create_projects = result.can_create_projects;
349 + user.creator_tier = live_tier;
350 + if let Err(e) = session.insert(USER_SESSION_KEY, user.clone()).await {
351 + tracing::warn!(user_id = %user.id, error = ?e, "failed to update session with refreshed user state");
335 352 }
336 - state.session_cache.insert(tracking_id, Instant::now());
337 353 }
354 + state.session_cache.insert(tracking_id, Instant::now());
338 355 }
339 356
340 357 tracing::Span::current().record("user_id", tracing::field::display(&user.id));
M server/src/db/scanning.rs +29
@@ -90,6 +90,35 @@ pub async fn purge_cdn_image_rows_by_key(db: &PgPool, s3_key: &str) -> Result<u6
90 90 Ok(removed)
91 91 }
92 92
93 + /// Of the given S3 keys, the subset whose authoritative scan verdict is
94 + /// `Quarantined` (confirmed malicious).
95 + ///
96 + /// `file_scan_results` is the per-object scan record the quarantine worker
97 + /// writes; keying on it covers every content surface (audio/cover/video,
98 + /// versions, insertions) uniformly. Used to exclude quarantined objects from
99 + /// bulk key reads that ultimately serve content to a user — notably content
100 + /// export, which collects raw keys and would otherwise hand a creator back their
101 + /// own quarantined object that every download path already refuses (Run 20
102 + /// Security). Absent rows (never scanned) are not quarantined and are not
103 + /// returned.
104 + #[tracing::instrument(skip_all)]
105 + pub async fn quarantined_s3_keys(
106 + pool: &PgPool,
107 + keys: &[String],
108 + ) -> Result<std::collections::HashSet<String>, sqlx::Error> {
109 + if keys.is_empty() {
110 + return Ok(std::collections::HashSet::new());
111 + }
112 + let rows: Vec<(String,)> = sqlx::query_as(
113 + "SELECT DISTINCT s3_key FROM file_scan_results \
114 + WHERE s3_key = ANY($1) AND scan_status = 'quarantined'",
115 + )
116 + .bind(keys)
117 + .fetch_all(pool)
118 + .await?;
119 + Ok(rows.into_iter().map(|(k,)| k).collect())
120 + }
121 +
93 122 /// Set the per-row `scan_status` on every CDN-served image surface keyed by
94 123 /// `s3_key`, in one transaction. The symmetric read-side counterpart of
95 124 /// [`purge_cdn_image_rows_by_key`]: where the purge DELETEs (or NULLs) a
M server/src/lib.rs +1 -1
@@ -184,7 +184,7 @@ pub fn build_app(
184 184 .merge(api_routes())
185 185 .merge(storage_routes())
186 186 .merge(stripe_routes())
187 - .merge(admin_routes())
187 + .merge(admin_routes(state.clone()))
188 188 .merge(synckit_routes(state.config.synckit_jwt_secret.clone().map(std::sync::Arc::new)))
189 189 .merge(oauth_routes())
190 190 .merge(postmark_routes())
M server/src/routes/admin/mod.rs +56 -2
@@ -8,11 +8,14 @@ mod users;
8 8 mod waitlist;
9 9
10 10 use axum::{
11 - extract::{Path, State},
11 + extract::{Path, Request, State},
12 + middleware::{from_fn_with_state, Next},
12 13 response::{IntoResponse, Response},
13 14 routing::get,
14 15 Form,
15 16 };
17 + use axum::http::request::Parts;
18 + use axum::extract::FromRequestParts;
16 19 use serde::Deserialize;
17 20
18 21 use crate::{
@@ -23,8 +26,43 @@ use crate::{
23 26 AppState,
24 27 };
25 28
29 + /// `/admin*` routes served WITHOUT the admin gate, by explicit design.
30 + ///
31 + /// Kept as a sealed allowlist so the blanket [`require_admin_layer`] stays
32 + /// honest: every route under `admin_routes()` requires admin EXCEPT these, and
33 + /// `admin_gate_covers_every_route` asserts the set is exactly this. Mirrors the
34 + /// CSRF skip-list discipline — an exemption is a visible, tested decision, not an
35 + /// accidental gap.
36 + pub(crate) const ADMIN_PUBLIC_PATHS: &[&str] = &[
37 + // Scan-queue health JSON, intentionally unauthenticated (counts only, no
38 + // PII) so PoM can scrape it against stable threshold rules.
39 + "/admin/uploads/health.json",
40 + ];
41 +
42 + /// Blanket admin gate over the whole `/admin*` subtree. Runs the same
43 + /// [`AdminUser`] check every admin handler ran individually, so a handler that
44 + /// forgets the extractor can no longer ship unauthenticated (Run 20 Security —
45 + /// auth-by-convention). Non-admins get 404, keeping admin routes hidden; the
46 + /// only bypass is the explicit [`ADMIN_PUBLIC_PATHS`] allowlist. Handlers keep
47 + /// their own `AdminUser` where they need the `AdminId` write-witness; the
48 + /// duplicate check is a cached session touch.
49 + async fn require_admin_layer(
50 + State(state): State<AppState>,
51 + req: Request,
52 + next: Next,
53 + ) -> Result<Response> {
54 + if ADMIN_PUBLIC_PATHS.contains(&req.uri().path()) {
55 + return Ok(next.run(req).await);
56 + }
57 + let (mut parts, body): (Parts, _) = req.into_parts();
58 + // The AdminUser rejection (404 for non-admins) becomes the response.
59 + let _admin = AdminUser::from_request_parts(&mut parts, &state).await?;
60 + let req = Request::from_parts(parts, body);
61 + Ok(next.run(req).await)
62 + }
63 +
26 64 /// Register admin routes for waitlist management, user moderation, and lottery.
27 - pub fn admin_routes() -> CsrfRouter<AppState> {
65 + pub fn admin_routes(state: AppState) -> CsrfRouter<AppState> {
28 66 CsrfRouter::new()
29 67 // Waitlist
30 68 .route_get("/admin/waitlist", get(waitlist::admin_waitlist))
@@ -80,6 +118,9 @@ pub fn admin_routes() -> CsrfRouter<AppState> {
80 118 .route("/api/admin/comp-codes/create", post_csrf(comp_codes::admin_create_comp_code))
81 119 // Metrics
82 120 .route_get("/admin/metrics", get(admin_metrics))
121 + // Structural admin gate over every route above (Run 20). A forgotten
122 + // per-handler AdminUser can no longer expose an admin route.
123 + .route_layer(from_fn_with_state(state, require_admin_layer))
83 124 }
84 125
85 126 // ── MT Provisioning ──
@@ -364,3 +405,16 @@ async fn admin_file_override(
364 405
365 406 Ok(crate::helpers::htmx_toast_response(&msg, "success"))
366 407 }
408 +
409 + #[cfg(test)]
410 + mod tests {
411 + use super::ADMIN_PUBLIC_PATHS;
412 +
413 + /// Seal the admin-gate exemption allowlist. Adding a route here is a
414 + /// deliberate decision to serve it unauthenticated under `/admin*`; this
415 + /// test forces that decision to be explicit rather than a silent gap.
416 + #[test]
417 + fn admin_public_paths_is_exactly_the_health_endpoint() {
418 + assert_eq!(ADMIN_PUBLIC_PATHS, &["/admin/uploads/health.json"]);
419 + }
420 + }
M server/src/routes/api/exports/content.rs +16
@@ -120,6 +120,22 @@ pub(in crate::routes::api) async fn export_content(
120 120 }
121 121 }
122 122
123 + // Exclude confirmed-malicious objects. The download paths refuse to serve a
124 + // Quarantined object even to its own creator (downloads.rs); the export must
125 + // honor the same invariant, since it collects raw S3 keys with no scan gate
126 + // of its own (Run 20 Security). Filtering here (not in get_user_s3_keys)
127 + // keeps that shared query intact for storage accounting, which must still see
128 + // every key.
129 + let export_keys: Vec<String> = files.iter().map(|(k, _, _)| k.clone()).collect();
130 + let quarantined = db::scanning::quarantined_s3_keys(&state.db, &export_keys).await?;
131 + if !quarantined.is_empty() {
132 + tracing::warn!(
133 + user_id = %user.id, dropped = quarantined.len(),
134 + "excluding quarantined objects from content export"
135 + );
136 + files.retain(|(key, _, _)| !quarantined.contains(key));
137 + }
138 +
123 139 if files.is_empty() {
124 140 if is_htmx {
125 141 return export_error_html("No content files to export.");
M server/tests/workflows/admin.rs +38
@@ -3,6 +3,44 @@
3 3 use crate::harness::TestHarness;
4 4 use makenotwork::db::UserId;
5 5
6 + // ── Structural admin gate (Run 20) ──
7 +
8 + /// The blanket `require_admin_layer` must hide every `/admin*` route from
9 + /// anonymous and non-admin callers, regardless of whether the individual
10 + /// handler remembers `AdminUser` — except the explicit public health endpoint.
11 + #[tokio::test]
12 + async fn admin_gate_rejects_anonymous_and_non_admin() {
13 + let (mut h, _admin_id) = TestHarness::with_admin().await;
14 + h.client.post_form("/logout", "").await; // ensure anonymous
15 +
16 + // Gated routes: an anonymous caller is rejected before the handler by the
17 + // layer's AuthUser check (401 Unauthorized). The invariant that matters is
18 + // that no gated /admin route is ever reachable (2xx) without auth.
19 + for path in [
20 + "/admin/users",
21 + "/admin/uploads",
22 + "/admin/metrics",
23 + "/admin/reports",
24 + "/admin/signups",
25 + "/admin/uploads/queue-summary",
26 + ] {
27 + let resp = h.client.get(path).await;
28 + assert_eq!(resp.status.as_u16(), 401, "anonymous must not reach {path}");
29 + }
30 +
31 + // The one explicit exemption stays public for PoM.
32 + let resp = h.client.get("/admin/uploads/health.json").await;
33 + assert_eq!(resp.status.as_u16(), 200, "scan-health JSON must stay public");
34 +
35 + // A logged-in non-admin is equally hidden.
36 + h.signup("plainuser", "plainuser@test.com", "password123").await;
37 + let resp = h.client.get("/admin/users").await;
38 + assert_eq!(resp.status.as_u16(), 404, "non-admin must not reach /admin/users");
39 + // ...but the public health endpoint is still reachable for them too.
40 + let resp = h.client.get("/admin/uploads/health.json").await;
41 + assert_eq!(resp.status.as_u16(), 200, "health JSON stays public for non-admins");
42 + }
43 +
6 44 // ── User Suspension ──
7 45
8 46 #[tokio::test]
M server/tests/workflows/csrf_coverage.rs +7 -4
@@ -40,12 +40,15 @@ const REJECTED_BEFORE_CSRF_LAYER: &[&str] = &[
40 40 /// validation layer (the #18-class regression) across the whole router at once.
41 41 #[tokio::test]
42 42 async fn every_auto_route_rejects_authenticated_tokenless_mutation() {
43 - let mut h = TestHarness::new().await;
44 - // Log in a user so requests clear the access gate and reach the per-route
45 - // CSRF layer. The client tracks the session cookie; `request_with_headers`
43 + // Log in as ADMIN so requests clear the access gate AND the `/admin*` admin
44 + // gate (Run 20), reaching the per-route CSRF layer on every auto route,
45 + // including admin mutations. A non-admin would be rejected by the admin gate
46 + // (404) before the CSRF layer on `/admin*` routes, masking their CSRF
47 + // coverage here. The client tracks the session cookie; `request_with_headers`
46 48 // deliberately does NOT inject a CSRF token, so each request is
47 49 // authenticated-but-tokenless.
48 - h.signup("csrfcov", "csrfcov@example.com", "password123").await;
50 + let (mut h, _admin_id) = TestHarness::with_admin().await;
51 + h.login("admin", "password123").await;
49 52
50 53 let manifest = route_manifest();
51 54 let auto_routes: Vec<_> = manifest