Skip to main content

max / makenotwork

Harden ClamAV to fail-closed; drop dead code; anti-spray rate-limit Sec-S1: a trusted upload whose clamav layer errored could pass on reduced AV coverage in the window before the runtime health probe observed clamd down. Fail closed on the first clamav error instead: any clamav error now holds a trusted upload for review. Removes the probe / clamav_healthy apparatus the window required; repurposes the fail-open metric and WAM ticket into a degraded-hold alert so a clamd outage stays visible. Sec cleanup: resolve_action was genuinely unused (deleted); get_history and get_active_actions are wired into the user dashboard, so their stale #[allow(dead_code)] is removed. Sec-MINOR: with no SyncKit secret configured, the rate-limit extractor parsed the unauthenticated `app` claim into its own bucket, letting forged tokens spray buckets. Collapse it to the nil sentinel instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-25 03:33 UTC
Commit: 5dd4ce3aa2da1180cc99dcab968237518b768f29
Parent: 6db89d5
7 files changed, +105 insertions, -223 deletions
M server/src/constants.rs -5
@@ -262,11 +262,6 @@ pub const SCAN_MALWAREBAZAAR_TIMEOUT_SECS: u64 = 5;
262 262 /// hung/blackholed host so a stalled connect can't pin a scan worker (Perf-S2).
263 263 pub const SCAN_HTTP_CONNECT_TIMEOUT_SECS: u64 = 5;
264 264 pub const SCAN_CLAMAV_TIMEOUT_SECS: u64 = 30;
265 - // How often the background probe pings clamd to maintain the runtime liveness
266 - // flag. The boot gate proves clamd was up at startup; this catches a death
267 - // AFTER boot so a configured-but-erroring clamav scan holds otherwise-clean
268 - // uploads for review instead of passing them on zero AV coverage (FailOpen).
269 - pub const SCAN_CLAMAV_HEALTH_PROBE_SECS: u64 = 30;
270 265
271 266 // -- Invite system --
272 267 pub const INVITES_ENABLED: bool = true;
M server/src/db/moderation.rs +1 -25
@@ -1,12 +1,10 @@
1 1 //! Moderation action history: append-only audit trail of all moderation events.
2 - //!
3 - //! Some functions (get_history, resolve_action) are not yet wired into routes.
4 2
5 3 use chrono::{DateTime, Utc};
6 4 use sqlx::PgPool;
7 5
8 6 use super::{ModerationActionId, ModerationActionType, UserId};
9 - use crate::error::{AppError, Result};
7 + use crate::error::Result;
10 8
11 9 /// A moderation action record.
12 10 #[derive(Debug, sqlx::FromRow)]
@@ -51,7 +49,6 @@ pub async fn create_action(
51 49 }
52 50
53 51 /// Get active (unresolved) moderation actions for a user.
54 - #[allow(dead_code)]
55 52 #[tracing::instrument(skip_all)]
56 53 pub async fn get_active_actions(
57 54 pool: &PgPool,
@@ -72,7 +69,6 @@ pub async fn get_active_actions(
72 69 }
73 70
74 71 /// Get full moderation history for a user (active + resolved).
75 - #[allow(dead_code)]
76 72 #[tracing::instrument(skip_all)]
77 73 pub async fn get_history(
78 74 pool: &PgPool,
@@ -93,26 +89,6 @@ pub async fn get_history(
93 89 Ok(actions)
94 90 }
95 91
96 - /// Resolve a moderation action (e.g., warning acknowledged, suspension lifted).
97 - #[allow(dead_code)]
98 - #[tracing::instrument(skip_all)]
99 - pub async fn resolve_action(pool: &PgPool, action_id: ModerationActionId) -> Result<()> {
100 - let result = sqlx::query(
101 - "UPDATE moderation_actions SET resolved_at = NOW() WHERE id = $1",
102 - )
103 - .bind(action_id)
104 - .execute(pool)
105 - .await?;
106 -
107 - // A zero-row update means the action id was stale; report it rather than
108 - // returning a false success to the caller.
109 - if result.rows_affected() == 0 {
110 - return Err(AppError::NotFound);
111 - }
112 -
113 - Ok(())
114 - }
115 -
116 92 /// Resolve all active actions of a given type for a user.
117 93 /// Used when unsuspending (resolves the suspension action).
118 94 #[tracing::instrument(skip_all)]
M server/src/main.rs -16
@@ -391,17 +391,12 @@ async fn main() {
391 391 // Start scan worker pool. Only meaningful if a scanner is configured;
392 392 // otherwise enqueue_scan_for never enqueues (trust-gate fast path).
393 393 if let (Some(scanner), Some(s3_for_workers)) = (state.scanner.clone(), state.s3.clone()) {
394 - // Runtime ClamAV liveness: starts healthy (the boot gate already proved
395 - // clamd was up), maintained by the probe spawned below.
396 - let clamav_healthy = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true));
397 - let clamav_socket = scanner.clamav_socket().map(str::to_string);
398 394 let scan_ctx = std::sync::Arc::new(makenotwork::scanning::worker::WorkerContext {
399 395 db: state.db.clone(),
400 396 s3: s3_for_workers,
401 397 pipeline: scanner,
402 398 scan_semaphore: state.scan_semaphore.clone(),
403 399 wam: state.wam.clone(),
404 - clamav_healthy: clamav_healthy.clone(),
405 400 cloudflare: makenotwork::cloudflare::CloudflarePurger::from_env(),
406 401 cdn_base_url: state.config.cdn_base_url.as_deref().map(std::sync::Arc::from),
407 402 });
@@ -410,17 +405,6 @@ async fn main() {
410 405 makenotwork::scanning::worker::spawn_pool(worker_count, scan_ctx, worker_shutdown_rx);
411 406 tracing::info!(worker_count, "scan worker pool started");
412 407
413 - // Post-boot clamd liveness probe — flips clean verdicts to held-for-review
414 - // if clamd dies after startup (the boot gate can't catch that).
415 - if let Some(socket) = clamav_socket {
416 - makenotwork::scanning::worker::spawn_clamav_health_probe(
417 - socket,
418 - clamav_healthy,
419 - shutdown_tx.subscribe(),
420 - );
421 - tracing::info!("clamav runtime health probe started");
422 - }
423 -
424 408 let report = makenotwork::scanning::spool::reap_all(
425 409 std::path::Path::new(makenotwork::constants::SCAN_SPOOL_DIR),
426 410 );
M server/src/metrics.rs +2 -2
@@ -257,8 +257,8 @@ pub fn record_sanitizer_rejection(kind: &'static str) {
257 257 /// one reduced-AV-coverage acceptance path). Scraped at `/metrics` and paired
258 258 /// with a WAM ticket so the window is observable rather than log-only
259 259 /// (ultra-fuzz Run 6 R6-Sec-L1, 3rd appearance).
260 - pub fn record_clamav_fail_open() {
261 - counter!("clamav_fail_open_total").increment(1);
260 + pub fn record_clamav_degraded_hold() {
261 + counter!("clamav_degraded_hold_total").increment(1);
262 262 }
263 263
264 264 /// Axum middleware that implements idempotency keys for POST endpoints.
M server/src/rate_limit.rs +19 -21
@@ -1,7 +1,6 @@
1 1 //! Rate limiting: Cloudflare-aware IP extraction, per-app SyncKit extraction,
2 2 //! and governor config builders.
3 3
4 - use base64::Engine;
5 4 use tower_governor::errors::GovernorError;
6 5 use tower_governor::key_extractor::KeyExtractor;
7 6
@@ -96,20 +95,6 @@ impl SyncAppKeyExtractor {
96 95 .map(|data| data.claims.app)
97 96 }
98 97
99 - /// Unverified payload parse — only used when no secret is configured.
100 - fn parse_app_unverified(token: &str) -> Option<SyncAppId> {
101 - let payload_b64 = token.split('.').nth(1)?;
102 - let payload_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD
103 - .decode(payload_b64)
104 - .or_else(|_| base64::engine::general_purpose::STANDARD.decode(payload_b64))
105 - .ok()?;
106 -
107 - #[derive(serde::Deserialize)]
108 - struct AppClaim {
109 - app: SyncAppId,
110 - }
111 - serde_json::from_slice::<AppClaim>(&payload_bytes).ok().map(|c| c.app)
112 - }
113 98 }
114 99
115 100 impl KeyExtractor for SyncAppKeyExtractor {
@@ -130,7 +115,12 @@ impl KeyExtractor for SyncAppKeyExtractor {
130 115
131 116 let app = match &self.secret {
132 117 Some(secret) => Self::verify_app(secret, token),
133 - None => Self::parse_app_unverified(token),
118 + // No secret configured: the token's `app` claim is unauthenticated, so
119 + // we must NOT mint a per-app bucket from it — that would let forged
120 + // tokens spray fresh buckets. Collapse to the nil sentinel; the
121 + // IP-layer limiter backstops volume and sync routes are inert without a
122 + // secret anyway (Sec-MINOR, Run 9).
123 + None => None,
134 124 };
135 125
136 126 // A verified app gets its own bucket; anything that fails verification
@@ -277,6 +267,7 @@ pub fn synckit_app_rate_limiter_ms(
277 267 #[cfg(test)]
278 268 mod tests {
279 269 use super::*;
270 + use base64::Engine;
280 271 use axum::http::Request;
281 272 use tower_governor::key_extractor::KeyExtractor;
282 273
@@ -316,8 +307,10 @@ mod tests {
316 307 }
317 308
318 309 #[test]
319 - fn unverified_extracts_app_id_from_jwt_when_no_secret() {
320 - // No secret configured (SyncKit off) → fall back to unverified parse.
310 + fn no_secret_collapses_unverified_app_to_nil() {
311 + // No secret configured (SyncKit off): the `app` claim is unauthenticated,
312 + // so it must NOT become its own bucket — the extractor collapses it to the
313 + // nil sentinel rather than trusting a forgeable UUID (Sec-MINOR, Run 9).
321 314 let app_id = SyncAppId::new();
322 315 let token = fake_jwt(&app_id);
323 316
@@ -327,7 +320,7 @@ mod tests {
327 320 .unwrap();
328 321
329 322 let extracted = SyncAppKeyExtractor::new(None).extract(&req).unwrap();
330 - assert_eq!(extracted, app_id);
323 + assert_eq!(extracted, SyncAppId::nil());
331 324 }
332 325
333 326 #[test]
@@ -457,7 +450,11 @@ mod tests {
457 450 }
458 451
459 452 #[test]
460 - fn different_apps_get_different_keys() {
453 + fn unsigned_apps_collapse_to_nil_when_no_secret() {
454 + // With no secret configured the `app` claim is unauthenticated, so two
455 + // DIFFERENT forged tokens must NOT mint two distinct buckets — both
456 + // collapse to the shared nil sentinel. Otherwise an attacker sprays fresh
457 + // per-app rate-limit buckets by fabricating app UUIDs (Sec-MINOR, Run 9).
461 458 let app1 = SyncAppId::new();
462 459 let app2 = SyncAppId::new();
463 460
@@ -473,6 +470,7 @@ mod tests {
473 470 let extractor = SyncAppKeyExtractor::new(None);
474 471 let key1 = extractor.extract(&req1).unwrap();
475 472 let key2 = extractor.extract(&req2).unwrap();
476 - assert_ne!(key1, key2);
473 + assert_eq!(key1, SyncAppId::nil());
474 + assert_eq!(key2, SyncAppId::nil());
477 475 }
478 476 }
M server/src/scanning/worker.rs +83 -151
@@ -7,7 +7,6 @@
7 7 //!
8 8 //! See `docs/scan-pipeline-audit.md` § 4.4 for the architecture.
9 9
10 - use std::sync::atomic::{AtomicBool, Ordering};
11 10 use std::sync::Arc;
12 11 use std::time::Duration;
13 12
@@ -40,14 +39,6 @@ pub struct WorkerContext {
40 39 pub pipeline: Arc<ScanPipeline>,
41 40 pub scan_semaphore: Arc<Semaphore>,
42 41 pub wam: Option<WamClient>,
43 - /// Runtime ClamAV liveness, maintained by [`spawn_clamav_health_probe`].
44 - /// `false` once the periodic probe finds clamd unreachable; a
45 - /// configured-but-erroring clamav scan then holds otherwise-clean uploads
46 - /// for review instead of passing them. `true` when clamav is healthy or
47 - /// not configured (so the overlay is a no-op without a probe). The boot
48 - /// gate only proves clamd was up at startup; this closes the post-boot
49 - /// "clamd died, FailOpen passes everything" window.
50 - pub clamav_healthy: Arc<AtomicBool>,
51 42 /// Cloudflare edge-cache purger, `None` when `CF_API_TOKEN`/`CF_ZONE_ID`
52 43 /// aren't configured. On quarantine we delete the origin object and, if a
53 44 /// purger is present, evict its `cdn_base_url`-prefixed URL from the edge so
@@ -71,59 +62,34 @@ pub struct WorkerContext {
71 62 ///
72 63 /// Otherwise the pipeline returned `Clean`: a trusted uploader normally passes;
73 64 /// an untrusted one always routes to admin review. The ClamAV degraded-mode
74 - /// overlay adds one exception — if the runtime probe has found clamd DOWN *and*
75 - /// this scan's clamav (transport) layer errored (FailOpen made `final_status`
76 - /// skip it → Clean), even a trusted upload is held rather than passed on zero AV
77 - /// coverage. A single transient error while the probe still reports clamd
78 - /// healthy does NOT trip this — only a sustained outage the probe has observed.
65 + /// overlay adds one exception — the clamav layer is `FailOpen` (a transport
66 + /// error makes `final_status` skip it → Clean), but accepting a trusted upload
67 + /// on zero AV coverage is the one fail-open we refuse, so ANY clamav error holds
68 + /// the file for admin review rather than passing it (Run 9 Sec-S1: fail closed on
69 + /// the first error instead of waiting for a runtime probe to observe a sustained
70 + /// outage — the probe-lag window is gone).
79 71 fn resolve_pass_status(
80 72 pipeline_status: FileScanStatus,
81 73 is_trusted: bool,
82 - clamav_down: bool,
83 74 layers: &[LayerResult],
84 75 ) -> FileScanStatus {
85 76 if pipeline_status == FileScanStatus::HeldForReview {
86 77 return FileScanStatus::HeldForReview;
87 78 }
88 - let clamav_errored = layers
89 - .iter()
90 - .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error);
91 - if is_trusted && !(clamav_down && clamav_errored) {
79 + if is_trusted && !clamav_layer_errored(layers) {
92 80 FileScanStatus::Clean
93 81 } else {
94 82 FileScanStatus::HeldForReview
95 83 }
96 84 }
97 85
98 - /// Periodically ping clamd and maintain the shared liveness flag consumed by
99 - /// [`resolve_pass_status`]. Spawned once at startup when clamav is configured.
100 - /// Observes `shutdown_rx` for graceful exit.
101 - pub fn spawn_clamav_health_probe(
102 - socket: String,
103 - healthy: Arc<AtomicBool>,
104 - mut shutdown_rx: tokio::sync::watch::Receiver<()>,
105 - ) {
106 - tokio::spawn(async move {
107 - let mut interval =
108 - tokio::time::interval(Duration::from_secs(constants::SCAN_CLAMAV_HEALTH_PROBE_SECS));
109 - interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
110 - loop {
111 - tokio::select! {
112 - _ = interval.tick() => {
113 - let ok = super::clamav::ping(&socket).await.is_ok();
114 - let prev = healthy.swap(ok, Ordering::Relaxed);
115 - if prev != ok {
116 - if ok {
117 - tracing::warn!("clamav health probe: clamd RECOVERED — scans resume normal FailOpen posture");
118 - } else {
119 - tracing::error!("clamav health probe: clamd UNREACHABLE — trusted uploads whose clamav layer errors are now held for review");
120 - }
121 - }
122 - }
123 - _ = shutdown_rx.changed() => break,
124 - }
125 - }
126 - });
86 + /// True if the clamav layer reported a transport/scan error on this file. Under
87 + /// the layer's `FailOpen` policy `final_status` skips such an error, so the trust
88 + /// overlay re-reads it to fail closed for trusted uploads.
89 + fn clamav_layer_errored(layers: &[LayerResult]) -> bool {
90 + layers
91 + .iter()
92 + .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error)
127 93 }
128 94
129 95 /// Spawn `n` scan workers on the current tokio runtime. Each worker drains
@@ -431,41 +397,35 @@ async fn run_pipeline_and_decide(
431 397
432 398 // Pipeline returned Clean or HeldForReview. Apply the uploader-trust
433 399 // overlay (untrusted users always route to admin review) plus the ClamAV
434 - // degraded-mode overlay (a trusted upload is held when clamd is known-down
435 - // and this scan's clamav layer errored). See `resolve_pass_status`.
400 + // degraded-mode overlay (a trusted upload whose clamav layer errored is held
401 + // rather than passed on reduced AV coverage). See `resolve_pass_status`.
436 402 let is_trusted = db::users::is_upload_trusted(&ctx.db, job.user_id).await?;
437 - let clamav_down = !ctx.clamav_healthy.load(Ordering::Relaxed);
438 - let status = resolve_pass_status(result.status, is_trusted, clamav_down, &result.layers);
439 -
440 - // Visibility for the narrow fail-open window: a trusted upload passes Clean
441 - // while its clamav layer errored but the health probe has NOT yet observed
442 - // clamd down. resolve_pass_status deliberately lets this through (a single
443 - // transient error must not hold every trusted upload), but it is the one
444 - // path where a file is accepted on reduced AV coverage without a hold — so
445 - // an operator must be able to see it rather than have it pass silently
446 - // (ultra-fuzz Run 4 M-Sec4).
447 - if clamav_fail_open_occurred(status, is_trusted, clamav_down, &result.layers) {
403 + let status = resolve_pass_status(result.status, is_trusted, &result.layers);
404 +
405 + // Visibility: an otherwise-clean trusted upload that we held *because* its
406 + // clamav layer errored. We now fail closed on the first such error (Run 9
407 + // Sec-S1), so this is no longer a silent acceptance — but a clamd outage that
408 + // starts holding trusted uploads is still operationally important, so surface
409 + // it via a metric (scraped) and a WAM ticket (active alert).
410 + if clamav_degraded_hold_occurred(result.status, is_trusted, &result.layers) {
448 411 tracing::warn!(
449 412 s3_key = %job.s3_key,
450 413 user_id = %job.user_id,
451 - "clamav layer errored while the health probe still reports clamd up; \
452 - trusted upload passed on reduced AV coverage (fail-open window)"
414 + "clamav layer errored on a trusted upload; held for review on reduced AV coverage"
453 415 );
454 - // Metric (scraped) + WAM ticket (active alert): close R6-Sec-L1's repeated
455 - // "log-only" finding so the reduced-coverage window is operationally visible.
456 - crate::metrics::record_clamav_fail_open();
416 + crate::metrics::record_clamav_degraded_hold();
457 417 if let Some(wam) = ctx.wam.clone() {
458 418 let body = format!(
459 - "A trusted upload passed Clean while the clamav layer errored and the \
460 - health probe had not yet observed clamd down (fail-open window).\n\n\
419 + "A trusted upload was held for review because its clamav layer errored \
420 + (reduced AV coverage — clamd may be unreachable).\n\n\
461 421 s3_key: {}\nuser_id: {}",
462 422 job.s3_key, job.user_id
463 423 );
464 424 wam.create_ticket(
465 - "ClamAV fail-open: trusted upload accepted on reduced AV coverage",
425 + "ClamAV degraded: trusted upload held on reduced AV coverage",
466 426 Some(&body),
467 427 "medium",
468 - "clamav-fail-open",
428 + "clamav-degraded-hold",
469 429 Some(&job.s3_key),
470 430 )
471 431 .await;
@@ -475,22 +435,15 @@ async fn run_pipeline_and_decide(
475 435 Ok(status)
476 436 }
477 437
478 - /// The one fail-open acceptance path: a trusted upload resolved `Clean` while its
479 - /// clamav layer errored but the health probe had not yet observed clamd down.
480 - /// Extracted as a pure predicate so the condition is unit-tested rather than
481 - /// living only inline (ultra-fuzz Run 6 R6-Sec-L1).
482 - fn clamav_fail_open_occurred(
483 - status: FileScanStatus,
438 + /// True when an otherwise-clean trusted upload was held *because* its clamav layer
439 + /// errored (the file would have passed but for the AV-coverage gap). Extracted as
440 + /// a pure predicate so the condition is unit-tested rather than living only inline.
441 + fn clamav_degraded_hold_occurred(
442 + pipeline_status: FileScanStatus,
484 443 is_trusted: bool,
485 - clamav_down: bool,
486 444 layers: &[crate::scanning::LayerResult],
487 445 ) -> bool {
488 - status == FileScanStatus::Clean
489 - && is_trusted
490 - && !clamav_down
491 - && layers
492 - .iter()
493 - .any(|l| l.layer == "clamav" && l.verdict == LayerVerdict::Error)
446 + pipeline_status == FileScanStatus::Clean && is_trusted && clamav_layer_errored(layers)
494 447 }
495 448
496 449 /// Update the per-entity `scan_status` column for the kinds that have one.
@@ -541,83 +494,29 @@ mod tests {
541 494 const CLEAN: FileScanStatus = FileScanStatus::Clean;
542 495
543 496 #[test]
544 - fn trusted_passes_when_clamav_healthy() {
497 + fn trusted_passes_when_clamav_clean() {
545 498 let layers = [layer("clamav", LayerVerdict::Pass)];
546 - assert_eq!(resolve_pass_status(CLEAN, true, false, &layers), FileScanStatus::Clean);
499 + assert_eq!(resolve_pass_status(CLEAN, true, &layers), FileScanStatus::Clean);
547 500 }
548 501
549 502 #[test]
550 - fn trusted_held_when_clamav_down_and_errored() {
551 - // The vulnerability window: clamd died post-boot (probe says down) AND
552 - // this scan's clamav (transport) layer errored (FailOpen would otherwise
553 - // skip it). final_status saw FailOpen → Clean; the overlay holds it.
503 + fn trusted_held_on_first_clamav_error() {
504 + // Sec-S1 (Run 9): fail closed on the FIRST clamav error. The clamav layer
505 + // is FailOpen (final_status skipped the error → Clean), but a trusted
506 + // upload on zero AV coverage is held for review immediately — no waiting
507 + // for a runtime probe to observe a sustained outage.
554 508 let layers = [layer("clamav", LayerVerdict::Error)];
555 509 assert_eq!(
556 - resolve_pass_status(CLEAN, true, true, &layers),
510 + resolve_pass_status(CLEAN, true, &layers),
557 511 FileScanStatus::HeldForReview
558 512 );
559 513 }
560 514
561 515 #[test]
562 - fn trusted_passes_when_down_flag_set_but_clamav_did_not_error() {
563 - // Probe flag stale/racing but the clamav layer actually passed — not a
564 - // coverage gap, so don't hold.
565 - let layers = [layer("clamav", LayerVerdict::Pass)];
566 - assert_eq!(resolve_pass_status(CLEAN, true, true, &layers), FileScanStatus::Clean);
567 - }
568 -
569 - // ── clamav_fail_open_occurred (R6-Sec-L1 observability predicate) ──
570 -
571 - #[test]
572 - fn fail_open_detected_for_trusted_errored_while_probe_up() {
573 - let layers = [layer("clamav", LayerVerdict::Error)];
574 - assert!(clamav_fail_open_occurred(CLEAN, true, false, &layers));
575 - }
576 -
577 - #[test]
578 - fn fail_open_not_detected_for_untrusted_upload() {
579 - // Untrusted uploads are held on a clamav error, never fail-open.
580 - let layers = [layer("clamav", LayerVerdict::Error)];
581 - assert!(!clamav_fail_open_occurred(CLEAN, false, false, &layers));
582 - }
583 -
584 - #[test]
585 - fn fail_open_not_detected_when_probe_already_down() {
586 - // resolve_pass_status already held this case; it is not a silent window.
587 - let layers = [layer("clamav", LayerVerdict::Error)];
588 - assert!(!clamav_fail_open_occurred(CLEAN, true, true, &layers));
589 - }
590 -
591 - #[test]
592 - fn fail_open_not_detected_when_clamav_passed() {
593 - let layers = [layer("clamav", LayerVerdict::Pass)];
594 - assert!(!clamav_fail_open_occurred(CLEAN, true, false, &layers));
595 - }
596 -
597 - #[test]
598 - fn fail_open_not_detected_when_pipeline_not_clean() {
599 - let layers = [layer("clamav", LayerVerdict::Error)];
600 - assert!(!clamav_fail_open_occurred(
601 - FileScanStatus::HeldForReview,
602 - true,
603 - false,
604 - &layers
605 - ));
606 - }
607 -
608 - #[test]
609 - fn trusted_passes_on_transient_error_while_probe_healthy() {
610 - // Single transient clamav error but the probe still reports healthy —
611 - // don't flood the review queue on a blip.
612 - let layers = [layer("clamav", LayerVerdict::Error)];
613 - assert_eq!(resolve_pass_status(CLEAN, true, false, &layers), FileScanStatus::Clean);
614 - }
615 -
616 - #[test]
617 516 fn untrusted_always_held() {
618 517 let layers = [layer("clamav", LayerVerdict::Pass)];
619 518 assert_eq!(
620 - resolve_pass_status(CLEAN, false, false, &layers),
519 + resolve_pass_status(CLEAN, false, &layers),
621 520 FileScanStatus::HeldForReview
622 521 );
623 522 }
@@ -625,12 +524,11 @@ mod tests {
625 524 #[test]
626 525 fn trusted_held_when_pipeline_already_held_not_downgraded() {
627 526 // CHRONIC S1: a reachable-but-incomplete clamav scan (or any FailClosed
628 - // layer error) makes final_status = HeldForReview. A trusted uploader on
629 - // a HEALTHY clamd must NOT have that downgraded to Clean — the prior code
630 - // ignored the pipeline status and would have passed it.
527 + // layer error) makes final_status = HeldForReview. A trusted uploader must
528 + // NOT have that downgraded to Clean.
631 529 let layers = [layer("clamav_incomplete", LayerVerdict::Error)];
632 530 assert_eq!(
633 - resolve_pass_status(FileScanStatus::HeldForReview, true, false, &layers),
531 + resolve_pass_status(FileScanStatus::HeldForReview, true, &layers),
634 532 FileScanStatus::HeldForReview
635 533 );
636 534 }
@@ -642,9 +540,43 @@ mod tests {
642 540 // uploaders too.
643 541 let layers = [layer("scan_size_limit", LayerVerdict::Error)];
644 542 assert_eq!(
645 - resolve_pass_status(FileScanStatus::HeldForReview, true, false, &layers),
543 + resolve_pass_status(FileScanStatus::HeldForReview, true, &layers),
646 544 FileScanStatus::HeldForReview
647 545 );
648 546 }
547 +
548 + // ── clamav_degraded_hold_occurred (observability predicate) ──
549 +
550 + #[test]
551 + fn degraded_hold_detected_for_trusted_clamav_error() {
552 + let layers = [layer("clamav", LayerVerdict::Error)];
553 + assert!(clamav_degraded_hold_occurred(CLEAN, true, &layers));
554 + }
555 +
556 + #[test]
557 + fn degraded_hold_not_for_untrusted_upload() {
558 + // Untrusted uploads are always held regardless of clamav; the degraded
559 + // metric tracks only the trusted-upload coverage-gap case.
560 + let layers = [layer("clamav", LayerVerdict::Error)];
561 + assert!(!clamav_degraded_hold_occurred(CLEAN, false, &layers));
562 + }
563 +
564 + #[test]
565 + fn degraded_hold_not_when_clamav_passed() {
566 + let layers = [layer("clamav", LayerVerdict::Pass)];
567 + assert!(!clamav_degraded_hold_occurred(CLEAN, true, &layers));
568 + }
569 +
570 + #[test]
571 + fn degraded_hold_not_when_pipeline_not_clean() {
572 + // A file the pipeline already held (FailClosed) wasn't held *because* of
573 + // the clamav coverage gap, so it is not a degraded-hold event.
574 + let layers = [layer("clamav", LayerVerdict::Error)];
575 + assert!(!clamav_degraded_hold_occurred(
576 + FileScanStatus::HeldForReview,
577 + true,
578 + &layers
579 + ));
580 + }
649 581 }
650 582
M server/tests/harness/mod.rs -3
@@ -545,9 +545,6 @@ impl TestHarness {
545 545 pipeline: deps.pipeline.clone(),
546 546 scan_semaphore: deps.semaphore.clone(),
547 547 wam: None,
548 - // Tests run with clamav healthy (no probe); the degraded overlay is
549 - // a no-op.
550 - clamav_healthy: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)),
551 548 // No Cloudflare purge in tests; quarantine still deletes from origin.
552 549 cloudflare: None,
553 550 cdn_base_url: None,