Skip to main content

max / makenotwork

Harden caching, malware quarantine, and account-cleanup fail modes /audit Run 13 security & storage cold spots: - Cache-Control: a public route is only stamped shared-cacheable when the response is truly viewer-independent. Downgrade to private when the response sets a cookie or the viewer is authenticated, so a CDN under a "cache everything" rule can't serve one user's personalized /u /p /i /c page (incl. their CSRF token) to another. Session read reuses the handler's already-loaded session (no extra DB hit). - Malware quarantine: never report a successful quarantine while the object is still live. Track removal (delete or durable enqueue); if neither succeeds, return an error so the scan-job retry budget re-runs and the entity is held, instead of marking the job done with malware at origin. - Account cleanup: abort (not unwrap_or_default) when listing a user's projects/sync-apps fails, so a transient DB error can't CASCADE-delete the user while orphaning their S3 objects. - Scan pipeline: hold (not pass Clean) when a file exceeds the YARA prefix cap and no ClamAV full-file backstop is configured — closes a tail-of-file evasion in YARA-only deployments. New unit tests: cache_control_value matrix, yara_tail_unscanned. cargo clippy --lib and lib tests clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-01 21:09 UTC
Commit: a6fd76d0f2a92ddf8383df747aab7628f857aca5
Parent: ce0a459
4 files changed, +195 insertions, -36 deletions
M server/src/metrics.rs +85 -10
@@ -30,27 +30,44 @@ pub async fn render(State(handle): State<PrometheusHandle>) -> impl IntoResponse
30 30 ///
31 31 /// Public content pages get CDN-friendly caching (Cloudflare caches for 60s,
32 32 /// browsers always revalidate). Dashboard, API, and auth routes get no caching.
33 + ///
34 + /// A public route is only stamped shared-cacheable when the response is truly
35 + /// viewer-independent: a response that sets a cookie (per-client session state)
36 + /// or was rendered for an authenticated viewer (owner controls + a per-session
37 + /// CSRF token baked into `base.html`) is downgraded to `private`. Without this a
38 + /// shared CDN under a "cache everything" rule could serve one user's
39 + /// personalized `/u /p /i /c` page — including their CSRF token — to another
40 + /// (ultra-fuzz Run 13 cross-cutting). Reading the session *after* the handler
41 + /// runs reuses the already-loaded session, so this adds no extra DB round-trip.
33 42 pub async fn cache_control_middleware(request: Request, next: Next) -> Response {
43 + use axum::http::header::{CACHE_CONTROL, SET_COOKIE};
44 +
34 45 let path = request.uri().path().to_string();
46 + let public = is_public_page(&path);
47 + // Clone the session handle before consuming the request; only read it for
48 + // public routes (where the caching decision depends on the viewer).
49 + let session = if public {
50 + request.extensions().get::<tower_sessions::Session>().cloned()
51 + } else {
52 + None
53 + };
54 +
35 55 let mut response = next.run(request).await;
36 56
37 57 // Don't override if a handler already set Cache-Control
38 - if response.headers().contains_key(axum::http::header::CACHE_CONTROL) {
58 + if response.headers().contains_key(CACHE_CONTROL) {
39 59 return response;
40 60 }
41 61
42 - let value = if is_public_page(&path) {
43 - // CDN caches 60s, browser always revalidates, stale content served while refreshing
44 - "public, max-age=0, s-maxage=60, stale-while-revalidate=300"
45 - } else if path.starts_with("/api/") || path.starts_with("/stripe/") || path.starts_with("/postmark/") {
46 - "no-store"
47 - } else {
48 - // Dashboard, admin, auth — private, always revalidate
49 - "private, no-cache"
62 + let authed = match session {
63 + Some(ref s) => crate::auth::session_user(s).await.is_some(),
64 + None => false,
50 65 };
66 + let sets_cookie = response.headers().contains_key(SET_COOKIE);
67 + let value = cache_control_value(&path, public, sets_cookie, authed);
51 68
52 69 response.headers_mut().insert(
53 - axum::http::header::CACHE_CONTROL,
70 + CACHE_CONTROL,
54 71 axum::http::HeaderValue::from_static(value),
55 72 );
56 73
@@ -65,6 +82,30 @@ pub async fn cache_control_middleware(request: Request, next: Next) -> Response
65 82 response
66 83 }
67 84
85 + /// The `Cache-Control` value for a response given its route class and whether
86 + /// the response turned out to be viewer-independent.
87 + ///
88 + /// `public` is `is_public_page(path)` (passed in so the caller computes it once).
89 + /// A public route is only shared-cacheable when it neither sets a cookie nor was
90 + /// served to an authenticated viewer; otherwise it is per-client and must stay
91 + /// `private`.
92 + fn cache_control_value(path: &str, public: bool, sets_cookie: bool, authed: bool) -> &'static str {
93 + if public {
94 + if sets_cookie || authed {
95 + // Per-client (cookie) or personalized (authed) — never shared-cache.
96 + "private, no-cache"
97 + } else {
98 + // CDN caches 60s, browser always revalidates, stale served while refreshing.
99 + "public, max-age=0, s-maxage=60, stale-while-revalidate=300"
100 + }
101 + } else if path.starts_with("/api/") || path.starts_with("/stripe/") || path.starts_with("/postmark/") {
102 + "no-store"
103 + } else {
104 + // Dashboard, admin, auth — private, always revalidate
105 + "private, no-cache"
106 + }
107 + }
108 +
68 109 /// Returns true for public content pages that benefit from CDN caching.
69 110 fn is_public_page(path: &str) -> bool {
70 111 // `/economics` is a viewer-independent marketing page (aggregate counts only);
@@ -530,4 +571,38 @@ mod tests {
530 571 assert_eq!(status_class(500), "5xx");
531 572 assert_eq!(status_class(100), "other");
532 573 }
574 +
575 + #[test]
576 + fn public_page_anonymous_is_cdn_cacheable() {
577 + let v = cache_control_value("/u/alice", true, false, false);
578 + assert!(v.starts_with("public"), "anonymous public page should be CDN-cacheable: {v}");
579 + }
580 +
581 + #[test]
582 + fn public_page_authed_viewer_is_private() {
583 + // An authenticated viewer's /u /p /i /c page carries owner controls + a
584 + // per-session CSRF token — never shared-cache it (Run 13 cross-cutting).
585 + assert_eq!(cache_control_value("/i/thing", true, false, true), "private, no-cache");
586 + assert_eq!(cache_control_value("/p/proj", true, false, true), "private, no-cache");
587 + }
588 +
589 + #[test]
590 + fn public_page_that_sets_a_cookie_is_private() {
591 + // A response minting a session (Set-Cookie) is per-client; the CSRF token
592 + // it just created must not be served to anyone else.
593 + assert_eq!(cache_control_value("/u/alice", true, true, false), "private, no-cache");
594 + }
595 +
596 + #[test]
597 + fn api_routes_are_no_store() {
598 + assert_eq!(cache_control_value("/api/items", false, false, false), "no-store");
599 + assert_eq!(cache_control_value("/stripe/webhook", false, false, false), "no-store");
600 + assert_eq!(cache_control_value("/postmark/inbound", false, false, false), "no-store");
601 + }
602 +
603 + #[test]
604 + fn private_routes_default_to_no_cache() {
605 + assert_eq!(cache_control_value("/dashboard", false, false, true), "private, no-cache");
606 + assert_eq!(cache_control_value("/settings", false, false, false), "private, no-cache");
607 + }
533 608 }
M server/src/scanning/mod.rs +51 -10
@@ -166,6 +166,13 @@ fn final_status(layers: &[LayerResult]) -> FileScanStatus {
166 166 }
167 167 }
168 168
169 + /// Whether a file's tail would go unscanned — true when it exceeds the YARA
170 + /// prefix cap while no ClamAV full-file backstop is configured. Such a file must
171 + /// be held for review rather than certified Clean on a prefix-only scan.
172 + fn yara_tail_unscanned(data_len: usize, clamav_configured: bool) -> bool {
173 + data_len > crate::constants::SCAN_YARA_MAX_BYTES && !clamav_configured
174 + }
175 +
169 176 /// Fail-closed result for when a CPU scan layer panics on crafted input.
170 177 ///
171 178 /// The in-process parsers (goblin, yara-x, zip, content-type) run over fully
@@ -585,17 +592,39 @@ impl ScanPipeline {
585 592 // mmap resident. ClamAV (streamed, uncapped) is the full-file
586 593 // backstop, so scanning a generous prefix bounds peak RAM without
587 594 // surrendering the floor.
588 - let yara_input = if data.len() > crate::constants::SCAN_YARA_MAX_BYTES {
589 - tracing::warn!(
590 - full_bytes = data.len(),
591 - capped_bytes = crate::constants::SCAN_YARA_MAX_BYTES,
592 - "YARA input capped; scanning prefix only (ClamAV scans the full file)"
593 - );
594 - &data[..crate::constants::SCAN_YARA_MAX_BYTES]
595 + if data.len() > crate::constants::SCAN_YARA_MAX_BYTES {
596 + if yara_tail_unscanned(data.len(), self.clamav_socket.is_some()) {
597 + // No ClamAV means there is NO full-file backstop, so the
598 + // bytes past the YARA prefix would go entirely unscanned —
599 + // a tail-of-file evasion (append the payload past the cap
600 + // and it passes Clean). Hold the file rather than certify
601 + // an unscanned tail (ultra-fuzz Run 13 Storage). `yara` is
602 + // FailClosed, so this Error → HeldForReview.
603 + tracing::warn!(
604 + full_bytes = data.len(),
605 + capped_bytes = crate::constants::SCAN_YARA_MAX_BYTES,
606 + "YARA input would be capped but no ClamAV full-file backstop is configured; holding file (tail unscanned)"
607 + );
608 + LayerResult {
609 + layer: "yara",
610 + verdict: LayerVerdict::Error,
611 + detail: Some(format!(
612 + "tail unscanned: {} B exceeds the {} B YARA prefix and no ClamAV full-file scan is configured",
613 + data.len(),
614 + crate::constants::SCAN_YARA_MAX_BYTES
615 + )),
616 + }
617 + } else {
618 + tracing::warn!(
619 + full_bytes = data.len(),
620 + capped_bytes = crate::constants::SCAN_YARA_MAX_BYTES,
621 + "YARA input capped; scanning prefix only (ClamAV scans the full file)"
622 + );
623 + yara::scan_with_yara(rules, &data[..crate::constants::SCAN_YARA_MAX_BYTES])
624 + }
595 625 } else {
596 - data
597 - };
598 - yara::scan_with_yara(rules, yara_input)
626 + yara::scan_with_yara(rules, data)
627 + }
599 628 }
600 629 None => LayerResult {
601 630 layer: "yara",
@@ -952,6 +981,18 @@ mod tests {
952 981 }
953 982
954 983 #[test]
984 + fn yara_tail_unscanned_only_without_backstop() {
985 + use crate::constants::SCAN_YARA_MAX_BYTES;
986 + // Over the cap with no ClamAV → tail unscanned, must hold.
987 + assert!(yara_tail_unscanned(SCAN_YARA_MAX_BYTES + 1, false));
988 + // Over the cap but ClamAV present → ClamAV scans the full file.
989 + assert!(!yara_tail_unscanned(SCAN_YARA_MAX_BYTES + 1, true));
990 + // Within the cap → YARA scanned the whole file regardless of ClamAV.
991 + assert!(!yara_tail_unscanned(SCAN_YARA_MAX_BYTES, false));
992 + assert!(!yara_tail_unscanned(1024, false));
993 + }
994 +
995 + #[test]
955 996 fn final_status_clean_when_all_pass() {
956 997 let layers = vec![
957 998 pass("content_type"),
M server/src/scanning/worker.rs +40 -10
@@ -351,6 +351,11 @@ async fn run_pipeline_and_decide(
351 351 }
352 352 }
353 353
354 + // Track whether the malicious object was actually removed (deleted now,
355 + // or durably enqueued for deletion). A kind that doesn't purge objects
356 + // has nothing to remove here, so it counts as removed. If removal fails
357 + // we must NOT report a successful quarantine (see the check below).
358 + let mut object_removed = !kind.quarantine_purges_object();
354 359 if kind.quarantine_purges_object() {
355 360 // Sanctioned direct delete: the quarantine worker must immediately
356 361 // purge a confirmed-malicious object (the durable queue would park a
@@ -358,10 +363,13 @@ async fn run_pipeline_and_decide(
358 363 // API requires.
359 364 let auth = crate::storage::S3DeleteAuthority::new();
360 365 match ctx.s3.delete_object(&auth, &crate::storage::S3Key::from_stored(&job.s3_key)).await {
361 - Ok(()) => tracing::warn!(
362 - s3_key = %job.s3_key, target_kind = %kind.as_str(),
363 - "purged quarantined object from storage"
364 - ),
366 + Ok(()) => {
367 + tracing::warn!(
368 + s3_key = %job.s3_key, target_kind = %kind.as_str(),
369 + "purged quarantined object from storage"
370 + );
371 + object_removed = true;
372 + }
365 373 Err(e) => {
366 374 tracing::error!(
367 375 s3_key = %job.s3_key, target_kind = %kind.as_str(), error = %e,
@@ -373,30 +381,52 @@ async fn run_pipeline_and_decide(
373 381 // queue will finish the job. For gated kinds (Item/Version/
374 382 // Media) the entity row still references the key — the queue
375 383 // would park it indefinitely — so enqueuing is futile and the
376 - // WAM ticket above is the escalation path for manual removal.
384 + // job must be retried (below) to re-attempt the direct delete.
377 385 if row_deleted {
378 - if let Err(enqueue_err) = db::pending_s3_deletions::enqueue_deletions(
386 + match db::pending_s3_deletions::enqueue_deletions(
379 387 &ctx.db,
380 388 &[(job.s3_key.clone(), "main".to_string())],
381 389 "malware_quarantine",
382 390 )
383 391 .await
384 392 {
385 - tracing::error!(
393 + Ok(_) => {
394 + tracing::warn!(
395 + s3_key = %job.s3_key,
396 + "quarantined object enqueued for durable deletion after direct purge failed"
397 + );
398 + object_removed = true;
399 + }
400 + Err(enqueue_err) => tracing::error!(
386 401 s3_key = %job.s3_key, error = %enqueue_err,
387 - "FAILED to enqueue quarantined object for durable deletion; object may persist at origin until manual removal"
388 - );
402 + "FAILED to enqueue quarantined object for durable deletion; will retry the scan job"
403 + ),
389 404 }
390 405 } else {
391 406 tracing::error!(
392 407 s3_key = %job.s3_key, target_kind = %kind.as_str(),
393 - "quarantined object still referenced by its entity row; durable queue cannot delete it — relying on the malware-quarantine WAM ticket for manual removal"
408 + "quarantined object still referenced by its entity row; durable queue cannot delete it — will retry the scan job"
394 409 );
395 410 }
396 411 }
397 412 }
398 413 }
399 414
415 + // Never report a successful quarantine while the malware is still live at
416 + // origin. If we could neither delete nor durably enqueue the object,
417 + // return an error: `process_job` leaves the job un-done (the scan-job
418 + // retry budget re-runs the quarantine — a transient S3 delete resolves on
419 + // retry) and holds the entity for review meanwhile; the WAM ticket above
420 + // is the manual-removal escalation if retries are exhausted (ultra-fuzz
421 + // Run 13 Storage: quarantine must not fail-to-success).
422 + if !object_removed {
423 + return Err(format!(
424 + "quarantine incomplete: malicious object {} could neither be purged nor enqueued for deletion",
425 + job.s3_key
426 + )
427 + .into());
428 + }
429 +
400 430 // Evict any edge-cached copy of the now-deleted object. Origin deletion
401 431 // above stops origin serving, but Cloudflare caches cdn.makenot.work/{key}
402 432 // immutably for up to a year, so a previously-fetched malicious URL keeps
M server/src/scheduler/cleanup.rs +19 -6
@@ -89,12 +89,25 @@ async fn cleanup_user_s3_and_delete(state: &AppState, user_id: db::UserId, event
89 89 // Resolve everything we need from the DB once — the enqueue list and the
90 90 // delete list must come from the same snapshot, or a project/app created
91 91 // between calls will be enqueued but not deleted (or vice versa).
92 - let project_ids = db::projects::get_project_ids_for_user(&state.db, user_id)
93 - .await
94 - .unwrap_or_default();
95 - let sync_apps = db::synckit::get_sync_apps_by_creator(&state.db, user_id)
96 - .await
97 - .unwrap_or_default();
92 + // A transient DB error here must ABORT, not degrade to an empty list: an
93 + // empty key set would enqueue nothing for `projects/{pid}/` / `{app_id}/`,
94 + // then the CASCADE delete below would drop the rows and leave those S3
95 + // objects orphaned forever. Return false so the sweep retries next tick
96 + // (ultra-fuzz Run 13 Storage: orphan-on-transient-error).
97 + let project_ids = match db::projects::get_project_ids_for_user(&state.db, user_id).await {
98 + Ok(v) => v,
99 + Err(e) => {
100 + tracing::error!(error = ?e, %user_id, "{label}: failed to list projects; aborting cleanup to avoid orphaning S3 objects");
101 + return false;
102 + }
103 + };
104 + let sync_apps = match db::synckit::get_sync_apps_by_creator(&state.db, user_id).await {
105 + Ok(v) => v,
106 + Err(e) => {
107 + tracing::error!(error = ?e, %user_id, "{label}: failed to list sync apps; aborting cleanup to avoid orphaning S3 objects");
108 + return false;
109 + }
110 + };
98 111
99 112 let user_prefix = format!("{user_id}/");
100 113 let main = crate::storage::S3Bucket::Main.as_str().to_string();