Skip to main content

max / makenotwork

server: close Ultra Fuzz Run #4 phases 1-5 + 41-test integration triage Phases 1-4 remediation (audit items in server/todo.md): - Phase 1: creator-tier CSRF tokens, git_ssh repo-name validation, login lockout email-flood, promo use_count over-release on cart cleanup, scanner streaming, scan_jobs retention, scan pool permit, broadcast bounded fan-out (most landed previously; gaps closed). - Phase 2: cart price math helpers, media delete-then-reupload pre-check via is_s3_key_live, pending_uploads owner pinning, TOTP step-replay cleared on enable/disable, delete_other_sessions returns ids for cache eviction, /login Manual CSRF, redirect_uri fetch_optional. - Phase 3: ota_builds partial unique index for single-running invariant with 23505-swallow in claim, build terminal write gated on status IN ('pending','running'), extract_s3_key_from_url path branch pinned to configured endpoint host, pending_2fa user_sessions tracking row visible to delete_all_sessions sweep. - Phase 4: MaybeUserUnverified short-circuits legacy sessions, sum_file_sizes_for_item two-sided clamp, license-key 23505 retry, Stripe multi-v1 sig parsing, has_active_subscription period-end defense, content_type Download text-sniff for HTML/SVG/XML/script, profile-link rel="ugc nofollow noopener", thousands separators in format_price/format_revenue. Integration test triage (41 failing -> 803 passing, 0 failing): Real code bugs surfaced during triage: - validate_key_code pinned at exactly 5 hyphenated words but the generator emits 6 (raised after a birthday-collision review). Now accepts both for backward compat with already-issued keys. - update_build_status terminal-write gate inadvertently blocked cancelling pending builds; loosened to IN ('pending','running'). - validate_auto on multi-method `with_csrf` routes rejected GETs because it never checked the request method; now short-circuits on safe methods per RFC 9110. - media_confirm duplicate-filename detection used a substring match on the wrapper Display; switched to structured SQLSTATE 23505 matching on AppError::Database(sqlx::Error::Database). Harness changes: - CF-Connecting-IP header default alongside X-Forwarded-For so extract_client_ip (production reads only the Cloudflare header) finds a value in tests. - failed_login_attempt(login, password) helper refreshes the CSRF token before posting — required by the new Manual /login posture. - drain_scan_jobs() drives the worker pool synchronously via a new public process_next_for_test entry; mirrors production flow honestly instead of forcing scanning back inline.
Co-Authored-By
Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author: Max J. <87768334+MaxJMath@users.noreply.github.com> · 2026-05-27 22:13 UTC
Commit: 46ae1095e22e34d1f5154890b93f105f0171f27b
Parent: db9d50f
47 files changed, +960 insertions, -172 deletions
M server/src/auth.rs +17
@@ -235,6 +235,23 @@
235 235 .await
236 236 .context("session error")?;
237 237
238 + // Short-circuit legacy sessions (USER_SESSION_KEY present without a
239 + // SESSION_TRACKING_KEY) to anonymous. Without this, a pre-tracking
240 + // session quietly survives `/logout-everywhere` — that sweep deletes
241 + // user_sessions rows, but a legacy session has no row to delete and
242 + // would keep rendering as logged-in on every Unverified extractor
243 + // until the cookie naturally expires.
244 + if user.is_some() {
245 + let tracking: Option<UserSessionId> = session
246 + .get(SESSION_TRACKING_KEY)
247 + .await
248 + .ok()
249 + .flatten();
250 + if tracking.is_none() {
251 + return Ok(MaybeUserUnverified(None));
252 + }
253 + }
254 +
238 255 Ok(MaybeUserUnverified(user))
239 256 }
240 257 }
M server/src/csrf.rs +21
@@ -184,6 +184,15 @@
184 184 }
185 185 }
186 186
187 + // Manual-posture runtime assertion (dev/test only): attempted via a tokio
188 + // task-local flag set in `validate_token_consuming` and checked in a per-
189 + // route layer. Backed out 2026-05-27 — false-positive density was too high:
190 + // rendered error pages return 200, rate-limit and form-extraction
191 + // short-circuit before the handler, and the audit explicitly marked this
192 + // follow-up as "not blocking — only matters if Manual grows beyond one
193 + // route". Compile-time discipline (the `CsrfManuallyValidated` witness type
194 + // bound as `_validated`) stays the convention.
195 +
187 196 /// Wrap a method-router with the Auto-posture validation layer.
188 197 /// Runs `validate_auto` on every request that reaches the route.
189 198 fn attach_auto_layer<S>(method_router: MethodRouter<S>) -> MethodRouter<S>
@@ -418,6 +427,18 @@
418 427 /// `_csrf` for authenticated users. Used by `CsrfPosture::Auto` routes
419 428 /// and by the path-allowlist fallback during the L2 migration.
420 429 async fn validate_auto(request: Request, next: Next, path: &str) -> Response {
430 + // Safe methods (RFC 9110 §9.2.1) are read-only by definition — never
431 + // CSRF-check them. This matters for multi-method routes wrapped by
432 + // `with_csrf(get(load).post(save))`: a bare GET should not require a
433 + // token (and the harness doesn't send one for GETs).
434 + if !matches!(*request.method(), axum::http::Method::POST
435 + | axum::http::Method::PUT
436 + | axum::http::Method::PATCH
437 + | axum::http::Method::DELETE)
438 + {
439 + return next.run(request).await;
440 + }
441 +
421 442 // Get session from extensions
422 443 let session = match request.extensions().get::<Session>() {
423 444 Some(s) => s.clone(),
M server/src/formatting.rs +50 -18
@@ -1,21 +1,36 @@
1 1 //! Formatting utilities: prices, file sizes, initials, slugs, CSV cells.
2 2
3 + /// Group thousands with commas (US locale). Returns the input string unchanged
4 + /// for values ≤999. Operates on a digit-only string so callers stay in i64
5 + /// arithmetic territory and don't need `f64` formatting tricks.
6 + fn group_thousands(n: u64) -> String {
7 + let s = n.to_string();
8 + let bytes = s.as_bytes();
9 + let mut out = String::with_capacity(bytes.len() + bytes.len() / 3);
10 + for (i, &b) in bytes.iter().enumerate() {
11 + if i > 0 && (bytes.len() - i) % 3 == 0 {
12 + out.push(',');
13 + }
14 + out.push(b as char);
15 + }
16 + out
17 + }
18 +
3 19 /// Format a price in cents as a human-readable dollar string or "Free".
4 20 pub fn format_price(cents: impl Into<i64>) -> String {
5 21 let cents: i64 = cents.into();
6 22 if cents == 0 {
7 - "Free".to_string()
8 - } else if cents < 0 {
9 - let abs = (cents as f64).abs();
10 - if cents % 100 == 0 {
11 - format!("-${}", (abs / 100.0) as u64)
12 - } else {
13 - format!("-${:.2}", abs / 100.0)
14 - }
15 - } else if cents % 100 == 0 {
16 - format!("${}", cents / 100)
23 + return "Free".to_string();
24 + }
25 + let neg = cents < 0;
26 + let abs = cents.unsigned_abs();
27 + let dollars = group_thousands(abs / 100);
28 + let frac = (abs % 100) as u32;
29 + let sign = if neg { "-" } else { "" };
30 + if frac == 0 {
31 + format!("{sign}${dollars}")
17 32 } else {
18 - format!("${:.2}", cents as f64 / 100.0)
33 + format!("{sign}${dollars}.{frac:02}")
19 34 }
20 35 }
21 36
@@ -23,11 +38,12 @@
23 38 ///
24 39 /// Unlike [`format_price`], this never returns "Free" -- zero revenue is "$0.00".
25 40 pub fn format_revenue(cents: i64) -> String {
26 - if cents < 0 {
27 - format!("-${:.2}", (cents as f64).abs() / 100.0)
28 - } else {
29 - format!("${:.2}", cents as f64 / 100.0)
30 - }
41 + let neg = cents < 0;
42 + let abs = cents.unsigned_abs();
43 + let dollars = group_thousands(abs / 100);
44 + let frac = (abs % 100) as u32;
45 + let sign = if neg { "-" } else { "" };
46 + format!("{sign}${dollars}.{frac:02}")
31 47 }
32 48
33 49 /// Format a byte count as a human-readable file size string.
@@ -192,7 +208,23 @@
192 208
193 209 #[test]
194 210 fn format_revenue_large_amount() {
195 - assert_eq!(format_revenue(1_000_000), "$10000.00");
211 + assert_eq!(format_revenue(1_000_000), "$10,000.00");
212 + }
213 +
214 + #[test]
215 + fn format_revenue_million_dollars() {
216 + assert_eq!(format_revenue(100_000_000), "$1,000,000.00");
217 + }
218 +
219 + #[test]
220 + fn format_price_thousands() {
221 + assert_eq!(format_price(1_234_500), "$12,345");
222 + assert_eq!(format_price(1_234_567), "$12,345.67");
223 + }
224 +
225 + #[test]
226 + fn format_price_negative_thousands() {
227 + assert_eq!(format_price(-1_234_567i64), "-$12,345.67");
196 228 }
197 229
198 230 #[test]
@@ -449,7 +481,7 @@
449 481 #[test]
450 482 fn format_price_large_value() {
451 483 // $1 billion in cents
452 - assert_eq!(format_price(100_000_000_000i64), "$1000000000");
484 + assert_eq!(format_price(100_000_000_000i64), "$1,000,000,000");
453 485 }
454 486
455 487 #[test]
M server/src/storage.rs +42 -11
@@ -530,6 +530,7 @@
530 530 url: &str,
531 531 cdn_base: Option<&str>,
532 532 bucket: Option<&str>,
533 + s3_endpoint: Option<&str>,
533 534 ) -> Option<String> {
534 535 let no_query = url.split('?').next()?;
535 536
@@ -544,17 +545,15 @@
544 545 }
545 546 }
546 547
547 - // Path-style S3: strip `https://{host}/{bucket}/`. HTTP URLs are
548 - // intentionally not handled — every CDN and S3 endpoint we use
549 - // serves over TLS, and an `http://` URL in `cover_image_url` would
550 - // be an operator-side misconfiguration we shouldn't paper over.
551 - if let Some(bucket) = bucket
552 - && let Some(rest) = no_query.strip_prefix("https://")
553 - {
554 - let path_start = rest.find('/')?;
555 - let path = &rest[path_start + 1..];
556 - let bucket_prefix = format!("{bucket}/");
557 - if let Some(key) = path.strip_prefix(&bucket_prefix)
548 + // Path-style S3: must match the configured `{endpoint}/{bucket}/` exactly.
549 + // Without the endpoint pin, the prior implementation accepted any
550 + // `https://{any-host}/{bucket}/{key}` — so an attacker-controlled URL like
551 + // `https://attacker.example/my-bucket/poisoned` would extract a real-looking
552 + // key and direct downstream code at attacker-chosen storage paths.
553 + if let (Some(bucket), Some(endpoint)) = (bucket, s3_endpoint) {
554 + let endpoint = endpoint.trim_end_matches('/');
555 + let prefix = format!("{endpoint}/{bucket}/");
556 + if let Some(key) = no_query.strip_prefix(&prefix)
558 557 && !key.is_empty()
559 558 {
560 559 return Some(key.to_string());
@@ -643,6 +642,7 @@
643 642 "https://cdn.makenot.work/projects/abc/image/cover.png",
644 643 Some("https://cdn.makenot.work"),
645 644 None,
645 + None,
646 646 );
647 647 assert_eq!(key.as_deref(), Some("projects/abc/image/cover.png"));
648 648 }
@@ -653,6 +653,7 @@
653 653 "https://cdn.makenot.work/foo/bar",
654 654 Some("https://cdn.makenot.work/"),
655 655 None,
656 + None,
656 657 );
657 658 assert_eq!(key.as_deref(), Some("foo/bar"));
658 659 }
@@ -663,6 +664,7 @@
663 664 "https://cdn.makenot.work/foo/bar?X-Amz-Signature=zzz",
664 665 Some("https://cdn.makenot.work"),
665 666 None,
667 + None,
666 668 );
667 669 assert_eq!(key.as_deref(), Some("foo/bar"));
668 670 }
@@ -673,10 +675,37 @@
673 675 "https://fsn1.your-objectstorage.com/my-bucket/u/123/image/cover.png?X-Amz=...",
674 676 None,
675 677 Some("my-bucket"),
678 + Some("https://fsn1.your-objectstorage.com"),
676 679 );
677 680 assert_eq!(key.as_deref(), Some("u/123/image/cover.png"));
678 681 }
679 682
683 + #[test]
684 + fn extract_key_path_style_rejects_attacker_host() {
685 + // Attacker-controlled host with the legitimate bucket name in the
686 + // path must NOT be accepted. The endpoint pin closes the gap.
687 + let key = extract_s3_key_from_url(
688 + "https://attacker.example/my-bucket/poisoned",
689 + None,
690 + Some("my-bucket"),
691 + Some("https://fsn1.your-objectstorage.com"),
692 + );
693 + assert_eq!(key, None);
694 + }
695 +
696 + #[test]
697 + fn extract_key_path_style_requires_endpoint() {
698 + // Without the endpoint, the path-style branch must not fire — bucket
699 + // name alone is not enough to identify a trustworthy host.
700 + let key = extract_s3_key_from_url(
701 + "https://fsn1.your-objectstorage.com/my-bucket/u/123/key",
702 + None,
703 + Some("my-bucket"),
704 + None,
705 + );
706 + assert_eq!(key, None);
707 + }
708 +
680 709 #[test]
681 710 fn extract_key_returns_none_when_no_prefix_matches() {
682 711 // Neither the CDN base nor the bucket name is present in the URL.
@@ -684,6 +713,7 @@
684 713 "https://random.example.com/foo/bar",
685 714 Some("https://cdn.makenot.work"),
686 715 Some("my-bucket"),
716 + Some("https://fsn1.your-objectstorage.com"),
687 717 );
688 718 assert_eq!(key, None);
689 719 }
@@ -696,6 +726,7 @@
696 726 "https://cdn.makenot.work/u/me/projects/x",
697 727 Some("https://cdn.makenot.work"),
698 728 None,
729 + None,
699 730 );
700 731 assert_eq!(key.as_deref(), Some("u/me/projects/x"));
701 732 }
M server/src/db/builds.rs +26 -5
@@ -230,14 +230,24 @@
230 230 .await?;
231 231 }
232 232 BuildStatus::Succeeded | BuildStatus::Failed | BuildStatus::Cancelled => {
233 - sqlx::query(
234 - "UPDATE ota_builds SET status = $2, finished_at = now(), error_message = $3 WHERE id = $1",
233 + // Gate on a non-terminal source status so the stale-build reaper
234 + // (`fail_stale_running_builds`) and a real terminal write can't
235 + // race: if the reaper just flipped the row to 'failed', the
236 + // successful builder's write must no-op rather than clobber it.
237 + // Pending IS a legitimate source — cancelling a build that never
238 + // started must still transition pending → cancelled.
239 + let result = sqlx::query(
240 + "UPDATE ota_builds SET status = $2, finished_at = now(), error_message = $3 WHERE id = $1 AND status IN ('pending', 'running')",
235 241 )
236 242 .bind(build_id)
237 243 .bind(status)
238 244 .bind(error_message)
239 245 .execute(pool)
240 246 .await?;
247 + if result.rows_affected() == 0 {
248 + tracing::warn!(build_id = %build_id, target_status = ?status,
249 + "build status terminal write skipped — row already terminal (likely reaper-set)");
250 + }
241 251 }
242 252 BuildStatus::Pending => {
243 253 sqlx::query("UPDATE ota_builds SET status = $2 WHERE id = $1")
@@ -259,7 +269,7 @@
259 269 ///; the loser simply gets no row.
260 270 #[tracing::instrument(skip_all)]
261 271 pub async fn claim_pending_build(pool: &PgPool) -> Result<Option<DbBuild>> {
262 - let build = sqlx::query_as::<_, DbBuild>(
272 + let result = sqlx::query_as::<_, DbBuild>(
263 273 r#"
264 274 UPDATE ota_builds
265 275 SET status = 'running', started_at = now()
@@ -275,9 +285,20 @@
275 285 "#,
276 286 )
277 287 .fetch_optional(pool)
278 - .await?;
288 + .await;
279 289
280 - Ok(build)
290 + // Multi-replica: the NOT EXISTS subquery races between replicas. The
291 + // `ota_builds_single_running` partial unique index is the backstop —
292 + // the loser's UPDATE surfaces as a 23505 unique violation, which means
293 + // a peer claimed first. Treat as "nothing to claim this tick".
294 + match result {
295 + Ok(build) => Ok(build),
296 + Err(sqlx::Error::Database(e)) if e.code().as_deref() == Some("23505") => {
297 + tracing::info!("claim_pending_build lost the running-slot race; another replica claimed");
298 + Ok(None)
299 + }
300 + Err(e) => Err(e.into()),
301 + }
281 302 }
282 303
283 304 /// Mark any builds that have been "running" longer than the timeout as failed.
M server/src/db/cart.rs +11
M server/src/db/git_repos.rs +1
M server/src/db/license_keys.rs +33 -12
M server/src/db/oauth.rs +9 -3
M server/src/db/pending_s3_deletions.rs +32
M server/src/db/pending_uploads.rs +7 -1
M server/src/db/sessions.rs +59 -5
M server/src/db/subscriptions.rs +6 -1
M server/src/db/totp.rs +6 -2
M server/src/db/transactions.rs +27 -3
M server/src/db/versions.rs +5 -4
M server/src/payments/webhooks.rs +25 -13
M server/src/routes/auth.rs +28 -3
M server/src/scanning/content_type.rs +42
M server/src/scanning/worker.rs +19
M server/src/scheduler/cleanup.rs +20
M server/src/validation/items.rs +5 -2
M server/templates/pages/cart.html +4 -4
M server/templates/pages/login.html +3
M server/templates/pages/user.html +1 -1
M server/tests/harness/client.rs +15 -3
M server/tests/harness/mod.rs +60
M server/tests/workflows/admin.rs +5 -5
M server/tests/workflows/adversarial_auth.rs +10 -20
M server/tests/workflows/auth.rs +11 -11
M server/tests/workflows/creator_media.rs +4
M server/tests/workflows/media_library.rs +17 -2
M server/tests/workflows/password_reset.rs +24 -12
M server/tests/workflows/promo_codes_free_access.rs +8 -2
M server/tests/workflows/scanning.rs +14 -8
M server/tests/workflows/wizards.rs +1 -1
M server/src/routes/api/projects.rs +1
M server/src/routes/storage/images.rs +1
M server/src/routes/storage/media.rs +8 -1
M server/src/templates/public/mod.rs +10
M server/src/routes/api/users/profile.rs +6 -4
M server/src/routes/api/users/sessions.rs +5 -4
M server/src/routes/pages/email_actions/links.rs +11
M server/src/routes/pages/public/two_factor.rs +57 -11
A server/migrations/131_ota_builds_single_running.sql +9
A server/migrations/132_user_sessions_kind.sql +10
A server/todo.md +174