Skip to main content

max / makenotwork

Remediate Run #21 moderate/low findings - downloads: never stream a Quarantined file, not even to its own creator (preview of Pending/HeldForReview still allowed). - discover: surface only scan_status = 'clean' items (was: anything not quarantined, which leaked Pending/HeldForReview metadata into discovery). - scanning/worker: spawn the quarantine WAM ticket instead of awaiting it, so a WAM outage can't stall malicious-object purge ~5s. - scanning/archive: per-entry compression-ratio cap (1 MB floor) so an incompressible padding entry can't dilute the aggregate ratio and mask a single bomb entry. - validate: slug-availability checks fail closed (prompt retry) on a DB error instead of reporting a possibly-taken slug as available. - scheduler + monitor: tickers skip missed ticks instead of bursting. - monitor: recover from a poisoned storage-fill mutex instead of panicking. - docengine: html-escape the video `src`, not just `alt`, in img_to_video. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-15 23:19 UTC
Commit: 832422853cd88aea463146dc5126d92d3c954663
Parent: aa2b1fe
8 files changed, +95 insertions, -27 deletions
M server/src/db/discover.rs +7 -7
@@ -192,7 +192,7 @@ pub async fn discover_items(
192 192 JOIN users u ON p.user_id = u.id
193 193 LEFT JOIN item_tags pit ON pit.item_id = i.id AND pit.is_primary = true
194 194 LEFT JOIN tags pt ON pt.id = pit.tag_id
195 - WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
195 + WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
196 196 "#,
197 197 )
198 198 } else if has_search {
@@ -219,7 +219,7 @@ pub async fn discover_items(
219 219 JOIN users u ON p.user_id = u.id
220 220 LEFT JOIN item_tags pit ON pit.item_id = i.id AND pit.is_primary = true
221 221 LEFT JOIN tags pt ON pt.id = pit.tag_id
222 - WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
222 + WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
223 223 "#,
224 224 )
225 225 } else {
@@ -245,7 +245,7 @@ pub async fn discover_items(
245 245 JOIN users u ON p.user_id = u.id
246 246 LEFT JOIN item_tags pit ON pit.item_id = i.id AND pit.is_primary = true
247 247 LEFT JOIN tags pt ON pt.id = pit.tag_id
248 - WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
248 + WHERE i.is_public = true AND i.listed = true AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE AND i.deleted_at IS NULL
249 249 "#,
250 250 )
251 251 };
@@ -295,7 +295,7 @@ pub async fn count_discover_items(
295 295 FROM items i
296 296 JOIN projects p ON i.project_id = p.id
297 297 JOIN users u ON p.user_id = u.id
298 - WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE
298 + WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE
299 299 "#,
300 300 );
301 301
@@ -507,7 +507,7 @@ pub async fn get_item_type_counts(
507 507 FROM items i
508 508 JOIN projects p ON i.project_id = p.id
509 509 JOIN users u ON p.user_id = u.id
510 - WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE
510 + WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE
511 511 "#,
512 512 );
513 513
@@ -578,7 +578,7 @@ pub async fn get_price_range_counts(
578 578 FROM items i
579 579 JOIN projects p ON i.project_id = p.id
580 580 JOIN users u ON p.user_id = u.id
581 - WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE
581 + WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE
582 582 "#,
583 583 );
584 584
@@ -648,7 +648,7 @@ pub async fn get_ai_tier_counts(
648 648 FROM items i
649 649 JOIN projects p ON i.project_id = p.id
650 650 JOIN users u ON p.user_id = u.id
651 - WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status != 'quarantined' AND u.is_sandbox = FALSE
651 + WHERE i.is_public = true AND i.listed = true AND i.deleted_at IS NULL AND p.is_public = true AND i.scan_status = 'clean' AND u.is_sandbox = FALSE
652 652 "#,
653 653 );
654 654
M server/src/monitor.rs +6 -1
@@ -110,6 +110,9 @@ pub fn spawn_monitor(
110 110 .unwrap_or(constants::HEALTH_CHECK_INTERVAL_SECS);
111 111
112 112 let mut interval = tokio::time::interval(std::time::Duration::from_secs(interval_secs));
113 + // Skip (not burst) missed ticks so a slow health check doesn't cause a
114 + // catch-up burst on the next wake.
115 + interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
113 116 interval.tick().await; // consume immediate first tick
114 117
115 118 let mut previous_status: Option<MonitorStatus> = None;
@@ -155,7 +158,9 @@ pub fn spawn_monitor(
155 158 )
156 159 });
157 160 let should_refresh = {
158 - let mut last = last_lock.lock().expect("storage-fill mutex");
161 + // Recover from a poisoned lock rather than panicking the monitor
162 + // loop — the critical section is a trivial timestamp swap.
163 + let mut last = last_lock.lock().unwrap_or_else(|e| e.into_inner());
159 164 if last.elapsed() >= STORAGE_FILL_TTL {
160 165 *last = std::time::Instant::now();
161 166 true
M server/src/routes/api/validate.rs +28 -11
@@ -96,10 +96,10 @@ pub async fn validate_project_slug(
96 96 }
97 97 };
98 98
99 - let is_taken = db::projects::get_project_by_user_and_slug(&state.db, auth.0.id, &slug)
100 - .await
101 - .map(|p| p.is_some())
102 - .unwrap_or(false);
99 + let is_taken = match db::projects::get_project_by_user_and_slug(&state.db, auth.0.id, &slug).await {
100 + Ok(p) => p.is_some(),
101 + Err(e) => return slug_check_failed(e),
102 + };
103 103
104 104 if is_taken {
105 105 let suggestions = suggest_slugs(&state.db, auth.0.id, &form.slug).await;
@@ -155,10 +155,10 @@ pub async fn validate_collection_slug(
155 155 };
156 156
157 157 let is_taken =
158 - db::collections::get_collection_by_user_and_slug(&state.db, auth.0.id, &slug)
159 - .await
160 - .map(|c| c.is_some())
161 - .unwrap_or(false);
158 + match db::collections::get_collection_by_user_and_slug(&state.db, auth.0.id, &slug).await {
159 + Ok(c) => c.is_some(),
160 + Err(e) => return slug_check_failed(e),
161 + };
162 162
163 163 Html(SlugStatusTemplate { available: !is_taken, suggestions: Vec::new() }.render_string())
164 164 }
@@ -207,9 +207,26 @@ pub async fn validate_blog_slug(
207 207 _ => return Html(String::new()),
208 208 };
209 209
210 - let is_taken = db::blog_posts::blog_post_slug_exists(&state.db, project.id, &slug)
211 - .await
212 - .unwrap_or(false);
210 + let is_taken = match db::blog_posts::blog_post_slug_exists(&state.db, project.id, &slug).await {
211 + Ok(taken) => taken,
212 + Err(e) => return slug_check_failed(e),
213 + };
213 214
214 215 Html(SlugStatusTemplate { available: !is_taken, suggestions: Vec::new() }.render_string())
215 216 }
217 +
218 + /// Fail-closed response for a slug-availability check whose DB query errored.
219 + /// Reporting "available" on error (the old `.unwrap_or(false)`) misleads the
220 + /// creator into thinking a possibly-taken slug is free; the real save then
221 + /// fails on the uniqueness constraint. Surface a retry instead (Run #21 UX LOW,
222 + /// carry-forward from #18).
223 + fn slug_check_failed(e: crate::error::AppError) -> Html<String> {
224 + tracing::warn!(error = ?e, "slug availability check failed; reporting retry");
225 + Html(
226 + SaveStatusTemplate {
227 + success: false,
228 + message: "Couldn't check availability right now — try again".to_string(),
229 + }
230 + .render_string(),
231 + )
232 + }
M server/src/routes/storage/downloads.rs +12 -5
@@ -85,9 +85,12 @@ pub(super) async fn stream_url(
85 85 }
86 86
87 87 // Only allow files with Clean scan status to be streamed.
88 - // Blocks Pending (not yet scanned), Quarantined, and HeldForReview.
89 - // Creators can stream their own HeldForReview content for preview.
90 - if item.scan_status != db::FileScanStatus::Clean && !is_creator {
88 + // A creator may preview their own Pending/HeldForReview content, but
89 + // confirmed-malicious (Quarantined) content is never streamable — not even
90 + // by its own creator (Run #21 Security LOW).
91 + if item.scan_status == db::FileScanStatus::Quarantined
92 + || (item.scan_status != db::FileScanStatus::Clean && !is_creator)
93 + {
91 94 return Err(AppError::NotFound);
92 95 }
93 96
@@ -188,8 +191,12 @@ pub(super) async fn version_download(
188 191 return Err(AppError::NotFound);
189 192 }
190 193
191 - // Only allow files with Clean scan status to be downloaded (creators can access their own)
192 - if version.scan_status != db::FileScanStatus::Clean && !is_creator {
194 + // Only allow files with Clean scan status to be downloaded. A creator may
195 + // preview their own Pending/HeldForReview version, but never a Quarantined
196 + // one — not even their own (Run #21 Security LOW).
197 + if version.scan_status == db::FileScanStatus::Quarantined
198 + || (version.scan_status != db::FileScanStatus::Clean && !is_creator)
199 + {
193 200 return Err(AppError::NotFound);
194 201 }
195 202
M server/src/scanning/archive.rs +23 -1
@@ -294,7 +294,8 @@ fn inspect_zip<R: std::io::Read + std::io::Seek>(reader: R) -> LayerResult {
294 294 };
295 295 }
296 296
297 - total_compressed += entry.compressed_size();
297 + let entry_compressed = entry.compressed_size();
298 + total_compressed += entry_compressed;
298 299 let claimed_size = entry.size();
299 300 drop(entry);
300 301
@@ -348,6 +349,27 @@ fn inspect_zip<R: std::io::Read + std::io::Seek>(reader: R) -> LayerResult {
348 349 };
349 350 total_uncompressed += actual_size;
350 351
352 + // Per-entry ratio cap. The aggregate ratio check below can be diluted by
353 + // an incompressible padding entry masking a single bomb entry, so flag a
354 + // single entry whose own ratio is extreme (Run #21 Security MODERATE).
355 + // Gated on a 1 MB floor so small highly-compressible files (text/JSON)
356 + // don't false-positive — absolute size is already bounded per-entry (the
357 + // `counted > limit` guard above) and in total (below); this is bomb
358 + // detection, not the resource cap.
359 + if entry_compressed > 0 && actual_size >= 1024 * 1024 {
360 + let entry_ratio = actual_size as f64 / entry_compressed as f64;
361 + if entry_ratio > constants::SCAN_ZIP_MAX_RATIO {
362 + return LayerResult {
363 + layer: "archive",
364 + verdict: LayerVerdict::Fail,
365 + detail: Some(format!(
366 + "Entry {} compression ratio {:.1}x exceeds limit of {:.0}x (possible ZIP bomb)",
367 + name, entry_ratio, constants::SCAN_ZIP_MAX_RATIO
368 + )),
369 + };
370 + }
371 + }
372 +
351 373 // Check for nested archives — extension check first, then magic bytes
352 374 // from the first decompression pass (no re-read needed).
353 375 let lower_name = name.to_lowercase();
M server/src/scanning/worker.rs +10 -2
@@ -301,14 +301,22 @@ async fn run_pipeline_and_decide(
301 301 .filter(|l| l.verdict == LayerVerdict::Fail)
302 302 .map(|l| l.layer)
303 303 .collect();
304 - if let Some(ref wam) = ctx.wam {
304 + if let Some(wam) = ctx.wam.clone() {
305 + // Fire-and-forget: the WAM call has a multi-second timeout, and the
306 + // verdict enforcement below (row purge + object delete) must not wait
307 + // on it — otherwise a WAM outage stalls every quarantine ~5s before
308 + // the malicious object is removed (Run #21 Performance MODERATE).
309 + // Matches the spawned-ticket pattern the checkout path already uses.
305 310 let title = format!("File quarantined: {}", job.s3_key);
306 311 let body = format!(
307 312 "Upload by user {} flagged as malicious.\n\
308 313 Failed layers: {}\nFile type: {file_type:?}\nSize: {}",
309 314 job.user_id, failed_layers.join(", "), job.file_size_bytes,
310 315 );
311 - wam.create_ticket(&title, Some(&body), "high", "malware-quarantine", Some(&job.s3_key)).await;
316 + let s3_key = job.s3_key.clone();
317 + tokio::spawn(async move {
318 + wam.create_ticket(&title, Some(&body), "high", "malware-quarantine", Some(&s3_key)).await;
319 + });
312 320 }
313 321
314 322 // Enforce the verdict by removing the malicious content. Two parts:
M server/src/scheduler/mod.rs +4
@@ -71,6 +71,10 @@ pub fn spawn_scheduler(
71 71 let mut interval = tokio::time::interval(
72 72 std::time::Duration::from_secs(constants::SCHEDULER_INTERVAL_SECS),
73 73 );
74 + // Skip (not burst) missed ticks: a long-running pass (storage recalc,
75 + // drift checks under the advisory lock) must not trigger a catch-up
76 + // burst of back-to-back ticks on the next wake.
77 + interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
74 78 interval.tick().await; // consume immediate first tick
75 79
76 80 let mut tick_count: u64 = 0;
M shared/docengine/src/media_urls.rs +5
@@ -79,6 +79,11 @@ pub fn img_to_video(html: &str) -> String {
79 79 .map(|c| c[1].to_string())
80 80 .unwrap_or_default();
81 81
82 + // Escape `src` too, not just `alt`. The regex capture is `[^"]*` and
83 + // ammonia (which ran before this) entity-escapes `"`/`<`/`>` inside
84 + // attribute values, so a breakout isn't representable today — but
85 + // don't depend on that invariant holding upstream (Run #21 UX NOTE).
86 + let src = crate::escape::html_escape(src);
82 87 if alt.is_empty() {
83 88 format!(r#"<video controls src="{}">Your browser does not support video.</video>"#, src)
84 89 } else {