Skip to main content

max / makenotwork

Remediate audit Run 22 fix-first findings; bump to 0.10.8 - auth: unify password length cap on char count across all six sites (login/OAuth/SyncKit switched from byte cap), fixing a permanent self-lockout for multibyte passwords. Shared validation::password_too_long. - scanning: add scan_jobs.heartbeat_at (migration 167); worker beats every 30s and reap_stuck/stuck_count key off COALESCE(heartbeat_at, started_at), so a slow-but-progressing scan is no longer reaped and double-processed. - stripe webhook: route the four charged-but-undelivered WAM escalations through the shutdown-drained bg pool instead of raw tokio::spawn so a ticket survives a mid-deploy restart. - health: gate the live subsystem fan-out behind is_admin (anon gets the cached monitor snapshot), bound the S3 probe at 5s, reconcile docs. Tests: 6 new regression tests (password unit + multibyte login, 4-test scan-job reaper layer suite, health admin-gate); ~300 tests across touched suites green; clean clippy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-04 21:42 UTC
Commit: bfd4533c17f47fb7627243e74b45429de88e0973
Parent: e6502a7
19 files changed, +463 insertions, -63 deletions
M server/Cargo.lock +1 -1
@@ -4318,7 +4318,7 @@ dependencies = [
4318 4318
4319 4319 [[package]]
4320 4320 name = "makenotwork"
4321 - version = "0.10.7"
4321 + version = "0.10.8"
4322 4322 dependencies = [
4323 4323 "ammonia",
4324 4324 "anyhow",
M server/Cargo.toml +1 -1
@@ -1,6 +1,6 @@
1 1 [package]
2 2 name = "makenotwork"
3 - version = "0.10.7"
3 + version = "0.10.8"
4 4 edition = "2024"
5 5 license-file = "LICENSE"
6 6
A server/migrations/167_scan_jobs_heartbeat.sql +22
@@ -0,0 +1,22 @@
1 + -- Liveness heartbeat for the scan-job reaper.
2 + --
3 + -- Before this, `reap_stuck` reset any `running` job whose `started_at` was
4 + -- older than STUCK_JOB_SECS (300s). But `started_at` is stamped at claim time
5 + -- and spans the full S3 download (up to the spool ceiling) plus the scan, so a
6 + -- legitimately slow large scan crossed the threshold *while still running*: a
7 + -- second worker re-claimed it, both scanned the same object, and each claim
8 + -- incremented `attempts` toward MAX_SCAN_ATTEMPTS -- force-retiring a valid file
9 + -- to `failed` and stranding the entity at HeldForReview (audit Run 22).
10 + --
11 + -- The running worker now bumps `heartbeat_at` on a cadence well under
12 + -- STUCK_JOB_SECS; the reaper and the PoM stuck-count key off
13 + -- COALESCE(heartbeat_at, started_at), so only a job whose worker has actually
14 + -- stopped touching it (a crashed or hung process) is reaped. A slow-but-
15 + -- progressing scan keeps its heartbeat fresh and is never reclaimed. The COALESCE
16 + -- fallback to started_at keeps pre-migration in-flight rows (heartbeat_at NULL)
17 + -- reapable on the old clock.
18 + ALTER TABLE scan_jobs ADD COLUMN IF NOT EXISTS heartbeat_at TIMESTAMPTZ;
19 +
20 + -- No new index: the existing partial index `idx_scan_jobs_running`
21 + -- (WHERE status = 'running') already bounds the reaper's candidate set to the
22 + -- handful of in-flight rows, which it then filters by heartbeat.
M server/src/db/mod.rs +1 -1
@@ -38,7 +38,7 @@ pub mod passkeys; // pub so the integration test crate can exercise the layer di
38 38 pub(crate) mod health;
39 39 pub(crate) mod monitor;
40 40 pub(crate) mod scanning;
41 - pub(crate) mod scan_jobs;
41 + pub mod scan_jobs; // pub so the integration test crate can exercise the reaper/heartbeat layer directly
42 42 pub(crate) mod scan_admin_actions;
43 43 pub(crate) mod content_insertions;
44 44 pub mod invites; // pub so the integration test crate can exercise redemption races directly
M server/src/db/scan_jobs.rs +41 -10
@@ -119,6 +119,11 @@ pub struct ScanJob {
119 119 pub attempts: i32,
120 120 pub enqueued_at: DateTime<Utc>,
121 121 pub started_at: Option<DateTime<Utc>>,
122 + /// Last liveness beat from the worker running this job. Bumped on a cadence
123 + /// well under STUCK_JOB_SECS; the reaper keys off this (falling back to
124 + /// `started_at`) so a slow-but-progressing scan isn't mistaken for a
125 + /// crashed worker. See `bump_heartbeat` and migration 167.
126 + pub heartbeat_at: Option<DateTime<Utc>>,
122 127 pub completed_at: Option<DateTime<Utc>>,
123 128 pub last_error: Option<String>,
124 129 }
@@ -191,7 +196,8 @@ pub async fn claim_next(db: &PgPool) -> Result<Option<ScanJob>, sqlx::Error> {
191 196 UPDATE scan_jobs
192 197 SET status = 'running',
193 198 attempts = attempts + 1,
194 - started_at = NOW()
199 + started_at = NOW(),
200 + heartbeat_at = NOW()
195 201 WHERE id = (SELECT id FROM next)
196 202 RETURNING *
197 203 "#,
@@ -240,14 +246,36 @@ pub async fn mark_failed(db: &PgPool, job_id: Uuid, err: &str) -> Result<(), sql
240 246 Ok(())
241 247 }
242 248
243 - /// Reset jobs that have been stuck in `running` longer than `max_age_secs`.
249 + /// Refresh a running job's liveness heartbeat.
250 + ///
251 + /// Called periodically by the worker that owns the job so `reap_stuck` (and the
252 + /// PoM stuck-count) can distinguish a slow-but-progressing scan (fresh beat)
253 + /// from a crashed or hung worker (stale beat). The `status = 'running'` guard
254 + /// makes a late beat a no-op once the job has already finished or been reaped —
255 + /// it never resurrects a terminal row's timestamp.
256 + #[tracing::instrument(skip_all, fields(%job_id))]
257 + pub async fn bump_heartbeat(db: &PgPool, job_id: Uuid) -> Result<(), sqlx::Error> {
258 + sqlx::query("UPDATE scan_jobs SET heartbeat_at = NOW() WHERE id = $1 AND status = 'running'")
259 + .bind(job_id)
260 + .execute(db)
261 + .await?;
262 + Ok(())
263 + }
264 +
265 + /// Reset jobs whose worker has gone silent for longer than `max_age_secs`.
244 266 ///
245 267 /// Run on worker startup and on a timer to recover from a previous-process crash
246 268 /// mid-scan: the row would otherwise stay `running` forever and never be
247 - /// re-claimed. A job whose `attempts` has already reached `MAX_SCAN_ATTEMPTS` is
248 - /// retired to `failed` (each claim increments `attempts`, so a scan that
249 - /// reliably wedges the process trips the budget); otherwise it returns to
250 - /// `queued` for another attempt.
269 + /// re-claimed. Liveness is measured from the job's last heartbeat
270 + /// (`COALESCE(heartbeat_at, started_at)`), NOT from `started_at` alone — a
271 + /// legitimately slow scan (large object over a slow S3 link) can run past
272 + /// `max_age_secs` while still making progress, and reaping *that* let a second
273 + /// worker double-process the same object and inflate `attempts` toward
274 + /// MAX_SCAN_ATTEMPTS until a valid file was force-retired (audit Run 22). The
275 + /// running worker bumps `heartbeat_at` on a cadence well under `max_age_secs`,
276 + /// so only a crashed/hung process (no fresh beat) crosses the threshold. A job
277 + /// whose `attempts` has already reached `MAX_SCAN_ATTEMPTS` is retired to
278 + /// `failed`; otherwise it returns to `queued` for another attempt.
251 279 #[tracing::instrument(skip_all)]
252 280 pub async fn reap_stuck(db: &PgPool, max_age_secs: i64) -> Result<u64, sqlx::Error> {
253 281 let affected = sqlx::query(
@@ -255,12 +283,13 @@ pub async fn reap_stuck(db: &PgPool, max_age_secs: i64) -> Result<u64, sqlx::Err
255 283 UPDATE scan_jobs
256 284 SET status = CASE WHEN attempts >= $2 THEN 'failed' ELSE 'queued' END,
257 285 started_at = NULL,
286 + heartbeat_at = NULL,
258 287 completed_at = CASE WHEN attempts >= $2 THEN NOW() ELSE completed_at END,
259 288 last_error = CASE WHEN attempts >= $2
260 289 THEN 'exceeded max scan attempts (reaped from running)'
261 290 ELSE last_error END
262 291 WHERE status = 'running'
263 - AND started_at < NOW() - ($1 || ' seconds')::interval
292 + AND COALESCE(heartbeat_at, started_at) < NOW() - ($1 || ' seconds')::interval
264 293 "#,
265 294 )
266 295 .bind(max_age_secs.to_string())
@@ -318,11 +347,13 @@ pub async fn running_count(db: &PgPool) -> Result<i64, sqlx::Error> {
318 347 .await
319 348 }
320 349
321 - /// Count of running jobs that have been in flight longer than `max_age_secs`.
322 - /// Used by PoM to alert on stuck workers.
350 + /// Count of running jobs whose worker has gone silent longer than
351 + /// `max_age_secs`. Used by PoM to alert on stuck workers. Keyed off the
352 + /// heartbeat (falling back to `started_at`) so a slow-but-alive scan isn't
353 + /// counted as stuck — matching `reap_stuck`'s liveness definition.
323 354 pub async fn stuck_count(db: &PgPool, max_age_secs: i64) -> Result<i64, sqlx::Error> {
324 355 sqlx::query_scalar::<_, i64>(
325 - "SELECT COUNT(*) FROM scan_jobs WHERE status = 'running' AND started_at < NOW() - ($1 || ' seconds')::interval",
356 + "SELECT COUNT(*) FROM scan_jobs WHERE status = 'running' AND COALESCE(heartbeat_at, started_at) < NOW() - ($1 || ' seconds')::interval",
326 357 )
327 358 .bind(max_age_secs.to_string())
328 359 .fetch_one(db)
M server/src/routes/api/users/profile.rs +2 -2
@@ -161,7 +161,7 @@ pub(in crate::routes::api) async fn update_password(
161 161
162 162 // Validate new password
163 163 let password_len = req.new_password.chars().count();
164 - if password_len < 8 {
164 + if password_len < crate::validation::limits::PASSWORD_MIN {
165 165 if is_htmx {
166 166 return Ok(Html(SaveStatusTemplate {
167 167 success: false,
@@ -172,7 +172,7 @@ pub(in crate::routes::api) async fn update_password(
172 172 "New password must be at least 8 characters".to_string(),
173 173 ));
174 174 }
175 - if password_len > 128 {
175 + if crate::validation::password_too_long(&req.new_password) {
176 176 if is_htmx {
177 177 return Ok(Html(SaveStatusTemplate {
178 178 success: false,
M server/src/routes/auth.rs +4 -2
@@ -177,8 +177,10 @@ async fn login_handler(
177 177 ));
178 178 }
179 179
180 - // Cap password length to match signup validation (prevents Argon2 DoS with huge inputs)
181 - if form.password.len() > 128 {
180 + // Cap password length to match signup validation (prevents Argon2 DoS with
181 + // huge inputs). MUST use the same char-count metric as signup, or a valid
182 + // multibyte password (<=128 chars, >128 bytes) is silently rejected here.
183 + if crate::validation::password_too_long(&form.password) {
182 184 return return_error("Invalid username/email or password");
183 185 }
184 186
M server/src/routes/oauth.rs +4 -2
@@ -498,8 +498,10 @@ async fn authorize_post(
498 498 ));
499 499 }
500 500
501 - // Cap password length to prevent DoS via Argon2 on very long inputs
502 - if password.len() > 128 {
501 + // Cap password length to prevent DoS via Argon2 on very long inputs.
502 + // Same char-count metric as signup (bytes would lock out multibyte
503 + // passwords that were accepted at signup).
504 + if crate::validation::password_too_long(password) {
503 505 return Ok(render_authorize_error(
504 506 Some(csrf_token),
505 507 session_user,
M server/src/routes/pages/email_actions/password.rs +2 -2
@@ -196,10 +196,10 @@ pub(super) async fn reset_password_handler(
196 196
197 197 // Validate password length
198 198 let password_len = form.password.chars().count();
199 - if password_len < 8 {
199 + if password_len < crate::validation::limits::PASSWORD_MIN {
200 200 return return_error("Password must be at least 8 characters");
201 201 }
202 - if password_len > 128 {
202 + if crate::validation::password_too_long(&form.password) {
203 203 return return_error("Password must be 128 characters or fewer");
204 204 }
205 205
M server/src/routes/pages/public/health/mod.rs +58 -27
@@ -1,7 +1,10 @@
1 1 //! System health dashboard and JSON monitoring endpoint.
2 2 //!
3 3 //! Two layers:
4 - //! - `GET /health` (HTML); runs full live checks (DB queries, S3 probe, endpoint self-tests)
4 + //! - `GET /health` (HTML); the full live fan-out (DB queries, S3 probe, endpoint
5 + //! self-tests) is admin-only. Anonymous/non-admin callers get a minimal page
6 + //! whose status is read from the cached monitor snapshot — the admin gate runs
7 + //! before the fan-out so an unauthenticated request can't trigger it.
5 8 //! - `GET /api/health` (JSON); reads cached results from the background monitor's database.
6 9 //! Fast (<10ms), no live probes. This is what PoM and other external services should poll.
7 10
@@ -24,6 +27,11 @@ use crate::{
24 27 AppState,
25 28 };
26 29
30 + /// Per-subsystem ceiling for the outbound probes in the admin health fan-out,
31 + /// so one wedged dependency can't stall the dashboard on a longer client-level
32 + /// timeout.
33 + const HEALTH_PROBE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
34 +
27 35 /// Format a [`std::time::Duration`] as a human-readable uptime string.
28 36 fn format_uptime(d: std::time::Duration) -> String {
29 37 let total_secs = d.as_secs();
@@ -297,10 +305,17 @@ async fn collect_health(state: &AppState) -> HealthData {
297 305 let pool_idle = state.db.num_idle();
298 306 let pool_active = pool_max.saturating_sub(pool_idle as u32);
299 307
300 - // S3 storage status with connectivity check
308 + // S3 storage status with connectivity check. Bound the one outbound network
309 + // probe in this fan-out so a wedged S3 endpoint degrades to "unreachable" in
310 + // a few seconds rather than stalling the admin dashboard on the client-level
311 + // 60s operation timeout (audit Run 22). The DB probes are already bounded by
312 + // the pooled-connection `statement_timeout`, and the PoM fetch has its own.
301 313 let storage_configured = state.s3.is_some();
302 314 let s3_reachable = if let Some(ref s3) = state.s3 {
303 - s3.check_connectivity().await.is_ok()
315 + matches!(
316 + tokio::time::timeout(HEALTH_PROBE_TIMEOUT, s3.check_connectivity()).await,
317 + Ok(res) if res.is_ok()
318 + )
304 319 } else {
305 320 false
306 321 };
@@ -517,20 +532,27 @@ async fn collect_health(state: &AppState) -> HealthData {
517 532 /// Render the system health dashboard with database, storage, and service status.
518 533 ///
519 534 /// The detailed dashboard — infra config, DB pool internals, business counts,
520 - /// Stripe mode, S3 bucket/region — is admin-only. Anonymous and non-admin callers
521 - /// get a minimal, reachable status page (HTTP 200) so uptime monitors (PoM polls
522 - /// this URL) stay green without the operational-intel disclosure. `/api/health`
523 - /// remains the minimal JSON probe.
535 + /// Stripe mode, S3 bucket/region — is admin-only and runs the live subsystem
536 + /// fan-out (`collect_health`). Anonymous and non-admin callers get a minimal,
537 + /// reachable status page (HTTP 200) whose status comes from the background
538 + /// monitor's cached snapshot, NOT a live probe.
539 + ///
540 + /// The admin gate runs BEFORE `collect_health` so an unauthenticated request
541 + /// cannot trigger the ~10-op sweep (DB counts, S3 connectivity, session probe,
542 + /// outbound PoM fetch) — that was an anon-reachable amplifier (audit Run 22).
543 + /// Uptime monitors (including PoM) should poll `GET /api/health`, the cheap
544 + /// cached JSON probe; this HTML page is a human-facing status view, and its
545 + /// non-admin branch now reads the same cached snapshot `/api/health` does.
524 546 #[tracing::instrument(skip_all, name = "health::health")]
525 547 pub(super) async fn health(
526 548 State(state): State<AppState>,
527 549 session: Session,
528 550 crate::auth::MaybeUserVerified(maybe_user): crate::auth::MaybeUserVerified,
529 551 ) -> Result<axum::response::Response> {
530 - let data = collect_health(&state).await;
531 -
532 552 let is_admin = maybe_user.as_ref().is_some_and(|u| u.is_admin);
533 553 if !is_admin {
554 + // Cheap cached status only — no live fan-out for unauthenticated callers.
555 + let (overall, _db_ok) = cached_health_status(&state).await;
534 556 // `label()` is a controlled &'static str from OverallStatus — no user input.
535 557 let body = axum::response::Html(format!(
536 558 "<!doctype html><html lang=\"en\"><head><meta charset=\"utf-8\">\
@@ -538,11 +560,13 @@ pub(super) async fn health(
538 560 <title>MakeNotWork status</title></head>\
539 561 <body style=\"font-family:system-ui,sans-serif;max-width:32rem;margin:4rem auto;padding:0 1rem\">\
540 562 <h1>MakeNotWork</h1><p>{}</p></body></html>",
541 - data.overall.label()
563 + overall.label()
542 564 ));
543 565 return Ok(body.into_response());
544 566 }
545 567
568 + let data = collect_health(&state).await;
569 +
546 570 let now = chrono::Utc::now();
547 571
548 572 let pool_utilization = if data.db_pool_max > 0 {
@@ -667,39 +691,46 @@ pub(super) async fn health(
667 691 // short git sha is a public commit ref (the server source is git-public), not a
668 692 // secret.
669 693
670 - /// `GET /api/health`: fast JSON health endpoint.
694 + /// Overall status from the background monitor's most recent snapshot, with a
695 + /// single `SELECT 1` liveness fallback before the first monitor tick has landed.
671 696 ///
672 - /// Reads the latest snapshot from the background monitor's database instead of
673 - /// running live probes. Returns 200 if operational or degraded, 503 if error.
674 - #[tracing::instrument(skip_all, name = "health::health_json")]
675 - pub(super) async fn health_json(
676 - State(state): State<AppState>,
677 - ) -> impl IntoResponse {
678 - // Read the latest snapshot from the background monitor (single DB row)
697 + /// This is the cheap read shared by `GET /api/health` and the anonymous
698 + /// `/health` HTML page, so both agree on status without either running the live
699 + /// `collect_health` subsystem fan-out. Returns `(overall, db_ok)`.
700 + async fn cached_health_status(state: &AppState) -> (OverallStatus, bool) {
679 701 let latest = db::monitor::get_recent_health_history(&state.db, 1)
680 702 .await
681 703 .unwrap_or_default();
682 -
683 - // Use cached monitor data only — no live probes (fast <10ms as documented).
684 - // Falls back to a single DB probe only when no monitor snapshots exist yet
685 - // (fresh startup before the first monitor tick).
686 - let (overall, db_ok) = if let Some(snap) = latest.first() {
704 + if let Some(snap) = latest.first() {
687 705 let status = match snap.status.as_str() {
688 706 "operational" => OverallStatus::Operational,
689 707 "degraded" => OverallStatus::Degraded,
690 708 _ => OverallStatus::Error,
691 709 };
692 - let db_healthy = status != OverallStatus::Error;
693 - (status, db_healthy)
710 + let db_ok = status != OverallStatus::Error;
711 + (status, db_ok)
694 712 } else {
695 - // No monitor data yet — single minimal probe
713 + // No monitor data yet — single minimal probe.
696 714 let db_ok = sqlx::query_scalar::<_, i32>("SELECT 1")
697 715 .fetch_one(&state.db)
698 716 .await
699 717 .is_ok();
700 718 let status = if db_ok { OverallStatus::Operational } else { OverallStatus::Error };
701 719 (status, db_ok)
702 - };
720 + }
721 + }
722 +
723 + /// `GET /api/health`: fast JSON health endpoint.
724 + ///
725 + /// Reads the latest snapshot from the background monitor's database instead of
726 + /// running live probes. Returns 200 if operational or degraded, 503 if error.
727 + #[tracing::instrument(skip_all, name = "health::health_json")]
728 + pub(super) async fn health_json(
729 + State(state): State<AppState>,
730 + ) -> impl IntoResponse {
731 + // Cached monitor data only — no live probes (fast <10ms as documented),
732 + // with a single-`SELECT 1` fallback before the first monitor tick.
733 + let (overall, db_ok) = cached_health_status(&state).await;
703 734
704 735 let http_status = if overall == OverallStatus::Error {
705 736 StatusCode::SERVICE_UNAVAILABLE
M server/src/routes/pages/public/join_wizard.rs +2 -2
@@ -156,10 +156,10 @@ pub async fn step_account_create(
156 156 }
157 157
158 158 let password_len = form.password.chars().count();
159 - if password_len < 8 {
159 + if password_len < crate::validation::limits::PASSWORD_MIN {
160 160 return return_error(Some("password"), "Password must be at least 8 characters");
161 161 }
162 - if password_len > 128 {
162 + if crate::validation::password_too_long(&form.password) {
163 163 return return_error(Some("password"), "Password must be 128 characters or fewer");
164 164 }
165 165
M server/src/routes/stripe/webhook/checkout.rs +17 -7
@@ -39,7 +39,12 @@ async fn escalate_if_orphaned_session(
39 39 let sid = session_id.to_string();
40 40 let pi = payment_intent_id.to_string();
41 41 let label = label.to_string();
42 - tokio::spawn(async move {
42 + // Route through the shutdown-drained background pool, NOT a raw
43 + // tokio::spawn: this ticket is the only thing that pages a human
44 + // about a charged-but-undelivered buyer, so it must not be dropped
45 + // when the process is restarted mid-flight during a deploy (Run 9
46 + // pattern; audit Run 22).
47 + state.bg.spawn("stripe orphaned-session ticket", async move {
43 48 let body = format!(
44 49 "Checkout session {sid} (payment_intent {pi}, {label}) completed at Stripe but has \
45 50 NO transaction — the pending row(s) were never created. The buyer was charged and \
@@ -56,9 +61,11 @@ async fn escalate_if_orphaned_session(
56 61 /// reported session totals. Our line items are server-built, so the credited
57 62 /// total should equal Stripe's pre-tax subtotal and the session should be USD.
58 63 /// A currency mismatch or an amount mismatch is logged loudly and escalated to
59 - /// WAM (fire-and-forget so it never runs inside an open DB transaction); the
60 - /// server-recorded amount stays authoritative either way. Shared by the
61 - /// purchase, cart, and guest completion handlers so the three can't drift.
64 + /// WAM via the shutdown-drained background pool (so it never runs inside an open
65 + /// DB transaction, yet survives a mid-deploy restart rather than being dropped
66 + /// like a raw tokio::spawn — audit Run 22); the server-recorded amount stays
67 + /// authoritative either way. Shared by the purchase, cart, and guest completion
68 + /// handlers so the three can't drift.
62 69 fn reconcile_checkout_amount(
63 70 state: &AppState,
64 71 session_id: &str,
@@ -79,7 +86,7 @@ fn reconcile_checkout_amount(
79 86 let session_id = session_id.to_string();
80 87 let currency = currency.to_string();
81 88 let label = label.to_string();
82 - tokio::spawn(async move {
89 + state.bg.spawn("stripe non-usd-session ticket", async move {
83 90 let body = format!(
84 91 "Checkout session {session_id} ({label}) settled in {currency}, not USD. The \
85 92 server credits its own USD amount, but investigate how a non-USD session was created."
@@ -100,7 +107,7 @@ fn reconcile_checkout_amount(
100 107 if let Some(wam) = state.wam.clone() {
101 108 let session_id = session_id.to_string();
102 109 let label = label.to_string();
103 - tokio::spawn(async move {
110 + state.bg.spawn("stripe amount-mismatch ticket", async move {
104 111 let body = format!(
105 112 "Credited amount {credited_cents} cents != Stripe session subtotal {subtotal} cents \
106 113 (session {session_id}, {label}). The server amount is authoritative; investigate a \
@@ -678,7 +685,10 @@ pub(super) async fn handle_tip_checkout_completed(
678 685 if let Some(wam) = state.wam.clone() {
679 686 let sid = session_id.clone();
680 687 let pi = pi.to_string();
681 - tokio::spawn(async move {
688 + // Drained background pool, not a raw tokio::spawn: a
689 + // charged-but-undelivered tip ticket must survive a
690 + // mid-deploy restart (audit Run 22).
691 + state.bg.spawn("stripe orphaned-tip ticket", async move {
682 692 let body = format!(
683 693 "Tip checkout session {sid} (payment_intent {pi}) completed at Stripe but has \
684 694 NO tip row — the pending tip was never created. The tipper was charged and the \
M server/src/routes/synckit/auth.rs +3 -2
@@ -58,8 +58,9 @@ pub(super) async fn sync_auth(
58 58 .ok_or(AppError::Unauthorized)?;
59 59
60 60 // Reject oversized passwords early (before user lookup — no timing leak
61 - // since this branch doesn't touch the DB or run Argon2).
62 - if req.password.len() > 128 {
61 + // since this branch doesn't touch the DB or run Argon2). Same char-count
62 + // metric as signup so a valid multibyte password isn't rejected here.
63 + if crate::validation::password_too_long(&req.password) {
63 64 let _ = verify_password_async("dummy".to_string(), DUMMY_HASH.clone()).await;
64 65 return Err(AppError::Unauthorized);
65 66 }
M server/src/scanning/worker.rs +48 -4
@@ -24,14 +24,58 @@ use super::{LayerResult, LayerVerdict, ScanPipeline, ScanResult};
24 24 /// Worker poll interval when the queue is empty.
25 25 const IDLE_POLL_INTERVAL: Duration = Duration::from_millis(500);
26 26
27 - /// How long a `running` job can sit before the reaper resets it to `queued`.
28 - /// A scan that legitimately takes longer than this is an outlier and warrants
29 - /// admin attention anyway.
27 + /// How long a `running` job can go without a heartbeat before the reaper resets
28 + /// it. This is measured from the worker's last liveness beat (see
29 + /// [`HEARTBEAT_INTERVAL`]), not from claim time, so a genuinely slow-but-
30 + /// progressing scan is never reaped — only a crashed or hung worker crosses it.
30 31 const STUCK_JOB_SECS: i64 = 300;
31 32
33 + /// Cadence at which the worker running a job refreshes its `heartbeat_at`. Must
34 + /// be comfortably smaller than [`STUCK_JOB_SECS`] (here ~10x) so a live job that
35 + /// is merely slow keeps beating well inside the reaper's window.
36 + const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(30);
37 +
32 38 /// Cadence at which any worker tries to reap stuck jobs.
33 39 const REAPER_INTERVAL: Duration = Duration::from_secs(60);
34 40
41 + /// Aborts a spawned task when dropped, so a job's heartbeat companion never
42 + /// outlives the job — including when `process_job` returns early with an error.
43 + struct AbortOnDrop(tokio::task::JoinHandle<()>);
44 +
45 + impl Drop for AbortOnDrop {
46 + fn drop(&mut self) {
47 + self.0.abort();
48 + }
49 + }
50 +
51 + /// Run `process_job` while a companion task keeps the job's `heartbeat_at`
52 + /// fresh. The reaper distinguishes a slow-but-alive scan (recent beat) from a
53 + /// crashed worker (stale beat) purely by this heartbeat, so a large scan that
54 + /// runs past `STUCK_JOB_SECS` is no longer reclaimed and double-processed. The
55 + /// companion is aborted the moment the job finishes (via `AbortOnDrop`),
56 + /// error or not, so it cannot bump a job that has already left `running`.
57 + async fn process_job_with_heartbeat(
58 + ctx: &WorkerContext,
59 + job: ScanJob,
60 + ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
61 + let job_id = job.id;
62 + let hb_db = ctx.db.clone();
63 + let heartbeat = tokio::spawn(async move {
64 + let mut ticker = tokio::time::interval(HEARTBEAT_INTERVAL);
65 + // The claim already stamped heartbeat_at, so skip the immediate first
66 + // tick and beat one interval from now.
67 + ticker.tick().await;
68 + loop {
69 + ticker.tick().await;
70 + if let Err(e) = db::scan_jobs::bump_heartbeat(&hb_db, job_id).await {
71 + tracing::warn!(%job_id, error = %e, "scan heartbeat bump failed");
72 + }
73 + }
74 + });
75 + let _guard = AbortOnDrop(heartbeat);
76 + process_job(ctx, job).await
77 + }
78 +
35 79 /// Shared dependencies the worker pool needs.
36 80 pub struct WorkerContext {
37 81 pub db: PgPool,
@@ -116,7 +160,7 @@ pub fn spawn_pool(n: usize, ctx: Arc<WorkerContext>, shutdown_rx: tokio::sync::w
116 160 match db::scan_jobs::claim_next(&ctx.db).await {
117 161 Ok(Some(job)) => {
118 162 let job_id = job.id;
119 - if let Err(e) = process_job(&ctx, job).await {
163 + if let Err(e) = process_job_with_heartbeat(&ctx, job).await {
120 164 tracing::error!(worker_id, %job_id, error = %e, "scan job failed");
121 165 if let Err(e2) = db::scan_jobs::mark_failed(&ctx.db, job_id, &e.to_string()).await {
122 166 tracing::error!(worker_id, %job_id, error = %e2, "failed to mark job failed");
M server/src/validation/mod.rs +39
@@ -94,6 +94,25 @@ pub mod limits {
94 94 // Item sections
95 95 pub const SECTION_TITLE_MAX: usize = 100;
96 96 pub const SECTION_BODY_MAX: usize = 100_000;
97 + // Passwords. Measured in Unicode scalar values (`chars().count()`), NOT
98 + // bytes. Every password path — signup, reset, profile change, login, OAuth
99 + // login, and SyncKit auth — MUST cap by this same metric and value; a
100 + // byte-based cap on the login side rejects (permanently locks out) any
101 + // account whose password is <=128 chars but >128 bytes, which multibyte
102 + // passphrases routinely are. Route the upper-bound check through
103 + // `super::password_too_long`.
104 + pub const PASSWORD_MIN: usize = 8;
105 + pub const PASSWORD_MAX: usize = 128;
106 + }
107 +
108 + /// True when a password exceeds the maximum length.
109 + ///
110 + /// Measured in Unicode scalar values (`chars().count()`), never bytes — see
111 + /// [`limits::PASSWORD_MAX`]. This is the single upper-bound check shared by the
112 + /// set-side paths (signup / reset / profile) and the credential-verification
113 + /// paths (login / OAuth / SyncKit) so the metric can never drift back apart.
114 + pub fn password_too_long(password: &str) -> bool {
115 + password.chars().count() > limits::PASSWORD_MAX
97 116 }
98 117
99 118 /// Validate a slug: 2-100 chars, alphanumeric + hyphens.
@@ -294,6 +313,26 @@ mod tests {
294 313 assert!(validate_sync_row_id(&"a".repeat(256)).is_err());
295 314 }
296 315
316 + #[test]
317 + fn test_password_too_long_measures_chars_not_bytes() {
318 + // Regression (audit Run 22): the cap is Unicode scalar count, not bytes.
319 + // A 128-char password is accepted even when its UTF-8 encoding exceeds
320 + // 128 bytes — otherwise multibyte passphrases that pass signup would be
321 + // rejected at login (permanent self-lockout).
322 + assert!(!password_too_long(&"a".repeat(limits::PASSWORD_MAX)));
323 + assert!(password_too_long(&"a".repeat(limits::PASSWORD_MAX + 1)));
324 +
325 + // 128 CJK chars = 128 chars (at the cap) but 384 bytes — must NOT be
326 + // flagged too long.
327 + let cjk = "\u{4f60}".repeat(limits::PASSWORD_MAX);
328 + assert_eq!(cjk.chars().count(), limits::PASSWORD_MAX);
329 + assert!(cjk.len() > limits::PASSWORD_MAX, "test string must exceed the byte cap");
330 + assert!(!password_too_long(&cjk));
331 +
332 + // One char over the cap is rejected regardless of byte width.
333 + assert!(password_too_long(&"\u{4f60}".repeat(limits::PASSWORD_MAX + 1)));
334 + }
335 +
297 336 // ── Edge cases (test-fuzz) ──
298 337
299 338 #[test]
M server/tests/workflows/admin.rs +28
@@ -41,6 +41,34 @@ async fn admin_gate_rejects_anonymous_and_non_admin() {
41 41 assert_eq!(resp.status.as_u16(), 200, "health JSON stays public for non-admins");
42 42 }
43 43
44 + /// `/health` stays publicly reachable (200) but the live subsystem fan-out is
45 + /// admin-only, gated BEFORE the probes run. An anonymous caller gets a minimal
46 + /// cached-status page — never the admin dashboard — so an unauthenticated
47 + /// request can't amplify into the ~10-op DB/S3/PoM sweep (audit Run 22).
48 + #[tokio::test]
49 + async fn health_gates_full_dashboard_to_admins() {
50 + let (mut h, _admin_id) = TestHarness::with_admin().await;
51 +
52 + // Anonymous: minimal reachable status page, no admin fan-out content.
53 + h.client.post_form("/logout", "").await;
54 + let anon = h.client.get("/health").await;
55 + assert_eq!(anon.status.as_u16(), 200, "health must stay publicly reachable");
56 + assert!(anon.text.contains("MakeNotWork"), "anon gets the minimal status page");
57 + assert!(
58 + !anon.text.contains("System Health"),
59 + "anon must NOT receive the admin fan-out dashboard"
60 + );
61 +
62 + // Admin: full live dashboard.
63 + h.login("admin", "password123").await;
64 + let admin = h.client.get("/health").await;
65 + assert_eq!(admin.status.as_u16(), 200);
66 + assert!(
67 + admin.text.contains("System Health"),
68 + "admin gets the full health dashboard"
69 + );
70 + }
71 +
44 72 // ── User Suspension ──
45 73
46 74 #[tokio::test]
M server/tests/workflows/auth.rs +31
@@ -3,6 +3,37 @@
3 3 use crate::harness::TestHarness;
4 4
5 5 #[tokio::test]
6 + async fn multibyte_password_register_then_login() {
7 + // Regression (audit Run 22): signup capped password length by char count
8 + // (`chars().count()`) while login/OAuth/SyncKit capped by BYTE count
9 + // (`.len()`). A password of <=128 chars but >128 bytes — any multibyte
10 + // passphrase near the cap — registered fine, then was silently rejected at
11 + // every subsequent login: a permanent, undiagnosable self-lockout.
12 + //
13 + // 65 CJK chars = 65 chars (well under the 128-char signup cap) = 195 bytes
14 + // (over the old 128-byte login cap). Under the bug, signup succeeds but the
15 + // login below never establishes a session.
16 + let password = "\u{4f60}".repeat(65); // 你 x65
17 + assert_eq!(password.chars().count(), 65);
18 + assert!(password.len() > 128, "test password must exceed 128 bytes");
19 +
20 + let mut h = TestHarness::new().await;
21 + let _user_id = h.signup("mb_user", "mb@example.com", &password).await;
22 + h.client.post_form("/logout", "").await;
23 +
24 + // Log back in with the identical multibyte password.
25 + h.login("mb_user", &password).await;
26 +
27 + // A failed login renders 200 with an error but sets no session, so the
28 + // definitive check is that the authenticated dashboard is reachable.
29 + let resp = h.client.get("/dashboard").await;
30 + assert_eq!(
31 + resp.status, 200,
32 + "multibyte-password account must be able to log back in"
33 + );
34 + }
35 +
36 + #[tokio::test]
6 37 async fn signup_login_logout_flow() {
7 38 let mut h = TestHarness::new().await;
8 39
A server/tests/workflows/db_scan_jobs_layer.rs +158
@@ -0,0 +1,158 @@
1 + //! DB-layer contract tests for the scan-job reaper's liveness heartbeat.
2 + //!
3 + //! Audit Run 22: `reap_stuck` keyed off claim-time `started_at`, which spans the
4 + //! full S3 download + scan, so a slow-but-progressing large scan was reaped and
5 + //! double-processed — each re-claim inflating `attempts` until a valid file was
6 + //! force-retired to `failed` and the entity stranded at HeldForReview. The
7 + //! reaper now keys off `heartbeat_at` (falling back to `started_at`). These pin
8 + //! that a fresh beat spares a long-running job, a stale beat is still reaped, the
9 + //! NULL fallback works, and `bump_heartbeat` refreshes liveness without
10 + //! resurrecting a terminal row.
11 +
12 + use crate::harness::TestHarness;
13 + use makenotwork::db::scan_jobs::{self, ScanTargetKind};
14 + use makenotwork::db::UserId;
15 + use makenotwork::storage::FileType;
16 + use uuid::Uuid;
17 +
18 + async fn seed_user(h: &TestHarness, name: &str) -> UserId {
19 + let hash = makenotwork::auth::hash_password("password123").expect("hash");
20 + sqlx::query_scalar::<_, UserId>(
21 + "INSERT INTO users (username, email, password_hash, email_verified)
22 + VALUES ($1, $2, $3, true) RETURNING id",
23 + )
24 + .bind(name)
25 + .bind(format!("{name}@test.com"))
26 + .bind(&hash)
27 + .fetch_one(&h.db)
28 + .await
29 + .expect("seed user")
30 + }
31 +
32 + async fn status_of(h: &TestHarness, job: Uuid) -> String {
33 + sqlx::query_scalar::<_, String>("SELECT status FROM scan_jobs WHERE id = $1")
34 + .bind(job)
35 + .fetch_one(&h.db)
36 + .await
37 + .expect("status")
38 + }
39 +
40 + /// Enqueue one job and claim it (→ running, `started_at` and `heartbeat_at`
41 + /// both stamped NOW()).
42 + async fn enqueue_and_claim(h: &TestHarness, user: UserId, key: &str) -> Uuid {
43 + scan_jobs::enqueue(
44 + &h.db,
45 + ScanTargetKind::Item,
46 + Uuid::new_v4(),
47 + key,
48 + FileType::Download,
49 + user,
50 + 1000,
51 + )
52 + .await
53 + .expect("enqueue");
54 + let job = scan_jobs::claim_next(&h.db)
55 + .await
56 + .expect("claim")
57 + .expect("a queued job to claim");
58 + assert_eq!(job.status, "running");
59 + assert!(job.heartbeat_at.is_some(), "claim stamps heartbeat_at");
60 + job.id
61 + }
62 +
63 + #[tokio::test]
64 + async fn reaper_spares_slow_but_alive_job() {
65 + let h = TestHarness::new().await;
66 + let user = seed_user(&h, "reaper_alive").await;
67 + let job = enqueue_and_claim(&h, user, "k/alive").await;
68 +
69 + // A job running far past the stuck window but whose worker is still beating:
70 + // old started_at, fresh heartbeat. This is the exact case that used to be
71 + // reaped and double-processed.
72 + sqlx::query(
73 + "UPDATE scan_jobs SET started_at = NOW() - interval '1 hour', heartbeat_at = NOW() WHERE id = $1",
74 + )
75 + .bind(job)
76 + .execute(&h.db)
77 + .await
78 + .expect("backdate started, fresh beat");
79 +
80 + let reaped = scan_jobs::reap_stuck(&h.db, 300).await.expect("reap");
81 + assert_eq!(reaped, 0, "a job with a fresh heartbeat must not be reaped");
82 + assert_eq!(status_of(&h, job).await, "running");
83 + }
84 +
85 + #[tokio::test]
86 + async fn reaper_reaps_crashed_worker() {
87 + let h = TestHarness::new().await;
88 + let user = seed_user(&h, "reaper_dead").await;
89 + let job = enqueue_and_claim(&h, user, "k/dead").await;
90 +
91 + // Worker crashed: no more beats.
92 + sqlx::query("UPDATE scan_jobs SET heartbeat_at = NOW() - interval '1 hour' WHERE id = $1")
93 + .bind(job)
94 + .execute(&h.db)
95 + .await
96 + .expect("stale beat");
97 +
98 + let reaped = scan_jobs::reap_stuck(&h.db, 300).await.expect("reap");
99 + assert_eq!(reaped, 1, "a stale-heartbeat job must be reaped");
100 + // attempts after a single claim = 1 < MAX_SCAN_ATTEMPTS, so it returns to
101 + // the queue for another attempt rather than being retired to failed.
102 + assert_eq!(status_of(&h, job).await, "queued");
103 + }
104 +
105 + #[tokio::test]
106 + async fn reaper_falls_back_to_started_at_when_no_heartbeat() {
107 + // Any row with heartbeat_at NULL (e.g. one in flight across the migration)
108 + // stays reapable on the old started_at clock via COALESCE.
109 + let h = TestHarness::new().await;
110 + let user = seed_user(&h, "reaper_null").await;
111 + let job = enqueue_and_claim(&h, user, "k/null").await;
112 +
113 + sqlx::query(
114 + "UPDATE scan_jobs SET heartbeat_at = NULL, started_at = NOW() - interval '1 hour' WHERE id = $1",
115 + )
116 + .bind(job)
117 + .execute(&h.db)
118 + .await
119 + .expect("null beat, old start");
120 +
121 + let reaped = scan_jobs::reap_stuck(&h.db, 300).await.expect("reap");
122 + assert_eq!(reaped, 1, "null heartbeat falls back to started_at");
123 + }
124 +
125 + #[tokio::test]
126 + async fn bump_heartbeat_refreshes_and_never_resurrects() {
127 + let h = TestHarness::new().await;
128 + let user = seed_user(&h, "reaper_bump").await;
129 + let job = enqueue_and_claim(&h, user, "k/bump").await;
130 +
131 + // A stale beat would be reaped...
132 + sqlx::query("UPDATE scan_jobs SET heartbeat_at = NOW() - interval '1 hour' WHERE id = $1")
133 + .bind(job)
134 + .execute(&h.db)
135 + .await
136 + .expect("stale");
137 + // ...but a bump refreshes it, so the reaper leaves it alone.
138 + scan_jobs::bump_heartbeat(&h.db, job).await.expect("bump");
139 + let reaped = scan_jobs::reap_stuck(&h.db, 300).await.expect("reap");
140 + assert_eq!(reaped, 0, "bump_heartbeat must refresh liveness");
141 + assert_eq!(status_of(&h, job).await, "running");
142 +
143 + // A late bump after the job leaves `running` is a no-op — it must never
144 + // resurrect a terminal row's heartbeat.
145 + sqlx::query("UPDATE scan_jobs SET status = 'done', heartbeat_at = NULL WHERE id = $1")
146 + .bind(job)
147 + .execute(&h.db)
148 + .await
149 + .expect("mark done");
150 + scan_jobs::bump_heartbeat(&h.db, job).await.expect("late bump");
151 + let still_null: bool =
152 + sqlx::query_scalar("SELECT heartbeat_at IS NULL FROM scan_jobs WHERE id = $1")
153 + .bind(job)
154 + .fetch_one(&h.db)
155 + .await
156 + .expect("hb null check");
157 + assert!(still_null, "bump must not touch a job that has left running");
158 + }
M server/tests/workflows/mod.rs +1
@@ -98,6 +98,7 @@ mod cart;
98 98 mod bundles;
99 99 mod idempotency;
100 100 mod db_items_layer;
101 + mod db_scan_jobs_layer;
101 102 mod db_payments_layer;
102 103 mod db_synckit_billing_layer;
103 104 mod db_transactions_layer;