max / makenotwork
4 files changed,
+75 insertions,
-12 deletions
| @@ -230,6 +230,17 @@ pub const SCAN_MAX_MEMORY_BYTES: usize = 100 * 1024 * 1024; // 100 MB in-memory | |||
| 230 | 230 | // assertion below documents that intent. | |
| 231 | 231 | pub const SCAN_MAX_CONCURRENT: usize = 4; // Memory-budget ceiling on concurrent scans | |
| 232 | 232 | pub const SCAN_WORKER_COUNT: usize = 2; // Background worker tasks draining scan_jobs queue | |
| 233 | + | /// Wall-clock ceiling on the CPU-bound scan layers (content-type, structural, | |
| 234 | + | /// archive decompress, yara, sha256) as a whole. The per-layer deadlines that | |
| 235 | + | /// existed — yara 30s, clamav/urlhaus their own — did not cover the archive | |
| 236 | + | /// decompress walk, which was byte-bounded (`SCAN_ZIP_MAX_UNCOMPRESSED`) but not | |
| 237 | + | /// time-bounded, so a slow-codec archive under the ratio caps could pin one of | |
| 238 | + | /// the two scan workers for its full decompress wall-time (fuzz 2026-07-06 F1). | |
| 239 | + | /// On elapse the scan fails closed (held for review) and the worker is freed; | |
| 240 | + | /// the orphaned blocking thread runs to completion on the (large) blocking pool. | |
| 241 | + | /// Generous enough for a legitimately large object, tight enough to bound the | |
| 242 | + | /// two-worker pool against monopolization. | |
| 243 | + | pub const SCAN_CPU_LAYERS_TIMEOUT_SECS: u64 = 120; | |
| 233 | 244 | /// Retention window for terminal-state (`done`, `failed`) `scan_jobs` rows. | |
| 234 | 245 | /// Queued/running rows are operational queue state and not affected. | |
| 235 | 246 | pub const SCAN_JOB_RETENTION_DAYS: u32 = 30; |
| @@ -132,14 +132,29 @@ pub(super) async fn smart_http_info_refs( | |||
| 132 | 132 | // burst of slow clones (holding their permits during the pack transfer) | |
| 133 | 133 | // stall every *new* clone at the handshake — the budget is for the | |
| 134 | 134 | // expensive streaming half only (Run #21 Performance). | |
| 135 | + | // Bounded by a wall-clock deadline like the upload-pack/receive-pack children | |
| 136 | + | // (Run #21). advertise-refs is short but reachable UNAUTHENTICATED for public | |
| 137 | + | // repos and un-gated by the semaphore, so a wedged git backend (repo lock, | |
| 138 | + | // NFS stall, pathological ref set) could park handlers unboundedly without | |
| 139 | + | // one (fuzz 2026-07-06 C4-1). `kill_on_drop` fires the SIGKILL when the | |
| 140 | + | // timed-out future is dropped. | |
| 135 | 141 | let git_subcommand = &service["git-".len()..]; // "upload-pack" | "receive-pack" | |
| 136 | - | let output = tokio::process::Command::new("git") | |
| 142 | + | let child = tokio::process::Command::new("git") | |
| 137 | 143 | .arg(git_subcommand) | |
| 138 | 144 | .arg("--stateless-rpc") | |
| 139 | 145 | .arg("--advertise-refs") | |
| 140 | 146 | .arg(&resolved.repo_path) | |
| 141 | - | .output() | |
| 147 | + | .stdout(std::process::Stdio::piped()) | |
| 148 | + | .stderr(std::process::Stdio::null()) | |
| 149 | + | .kill_on_drop(true) | |
| 150 | + | .spawn() | |
| 151 | + | .context("spawn git smart-http advertise-refs")?; | |
| 152 | + | let deadline = std::time::Duration::from_secs(constants::GIT_SMART_HTTP_TIMEOUT_SECS); | |
| 153 | + | let output = tokio::time::timeout(deadline, child.wait_with_output()) | |
| 142 | 154 | .await | |
| 155 | + | .map_err(|_| AppError::Internal(anyhow::anyhow!( | |
| 156 | + | "git {git_subcommand} advertise-refs timed out" | |
| 157 | + | )))? | |
| 143 | 158 | .context("run git smart-http advertise-refs")?; | |
| 144 | 159 | ||
| 145 | 160 | if !output.status.success() { |
| @@ -105,12 +105,13 @@ pub(crate) async fn render_project_page( | |||
| 105 | 105 | .map(|u| u.id == db_project.user_id) | |
| 106 | 106 | .unwrap_or(false); | |
| 107 | 107 | ||
| 108 | + | // Scope the purchase lookup to the items actually rendered on this page | |
| 109 | + | // rather than fetching the viewer's entire purchase history (fuzz 2026-07-06 | |
| 110 | + | // F6). `purchased_subset` is a single `item_id = ANY($2)` roundtrip. | |
| 108 | 111 | let purchased_item_ids: std::collections::HashSet<ItemId> = if let Some(ref user) = maybe_user | |
| 109 | 112 | { | |
| 110 | - | db::transactions::get_user_purchased_item_ids(&state.db, user.id) | |
| 111 | - | .await? | |
| 112 | - | .into_iter() | |
| 113 | - | .collect() | |
| 113 | + | let item_ids: Vec<ItemId> = db_items.iter().map(|i| i.id).collect(); | |
| 114 | + | db::transactions::purchased_subset(&state.db, user.id, &item_ids).await? | |
| 114 | 115 | } else { | |
| 115 | 116 | std::collections::HashSet::new() | |
| 116 | 117 | }; |
| @@ -211,6 +211,25 @@ fn panicked_sync_result(file_size: u64) -> ScanResult { | |||
| 211 | 211 | } | |
| 212 | 212 | } | |
| 213 | 213 | ||
| 214 | + | /// Fail-closed result for a scan whose CPU layers exceeded the wall-clock | |
| 215 | + | /// deadline (`SCAN_CPU_LAYERS_TIMEOUT_SECS`) — e.g. a pathological slow-codec | |
| 216 | + | /// archive decompress. Held for review rather than passed, and the worker is | |
| 217 | + | /// freed so a crafted upload can't monopolize the two-worker scan pool | |
| 218 | + | /// (fuzz 2026-07-06 F1). | |
| 219 | + | fn timed_out_sync_result(file_size: u64) -> ScanResult { | |
| 220 | + | let layer = LayerResult { | |
| 221 | + | layer: "scan_timeout", | |
| 222 | + | verdict: LayerVerdict::Error, | |
| 223 | + | detail: Some("CPU scan layers exceeded the wall-clock deadline".to_string()), | |
| 224 | + | }; | |
| 225 | + | ScanResult { | |
| 226 | + | status: final_status(std::slice::from_ref(&layer)), | |
| 227 | + | layers: vec![layer], | |
| 228 | + | sha256: String::new(), | |
| 229 | + | file_size, | |
| 230 | + | } | |
| 231 | + | } | |
| 232 | + | ||
| 214 | 233 | /// Fail-closed result for a file too large to spool for scanning. | |
| 215 | 234 | /// | |
| 216 | 235 | /// Files above [`crate::constants::SCAN_SPOOL_MAX_BYTES`] cannot be spooled to | |
| @@ -477,12 +496,21 @@ impl ScanPipeline { | |||
| 477 | 496 | ) -> ScanResult { | |
| 478 | 497 | let file_size = input.len() as u64; | |
| 479 | 498 | ||
| 480 | - | // CPU layers + hash, off the runtime. | |
| 499 | + | // CPU layers + hash, off the runtime, under a wall-clock deadline. The | |
| 500 | + | // per-layer timeouts (yara 30s) did not cover the archive decompress | |
| 501 | + | // walk, so a slow-codec archive could pin a scan worker for its full | |
| 502 | + | // decompress time (fuzz 2026-07-06 F1). The timeout frees the worker on | |
| 503 | + | // elapse; the blocking thread runs to completion on the large blocking | |
| 504 | + | // pool (spawn_blocking is not cancellable), so the memory-budget ceiling | |
| 505 | + | // still holds while the two-worker pool is no longer monopolized. | |
| 481 | 506 | let sync_bytes = input.byte_handle(); | |
| 482 | 507 | let sync_self = std::sync::Arc::clone(&self); | |
| 483 | - | let sync_fut = tokio::task::spawn_blocking(move || { | |
| 484 | - | sync_self.run_sync_layers(sync_bytes.as_slice(), file_type) | |
| 485 | - | }); | |
| 508 | + | let sync_fut = tokio::time::timeout( | |
| 509 | + | std::time::Duration::from_secs(crate::constants::SCAN_CPU_LAYERS_TIMEOUT_SECS), | |
| 510 | + | tokio::task::spawn_blocking(move || { | |
| 511 | + | sync_self.run_sync_layers(sync_bytes.as_slice(), file_type) | |
| 512 | + | }), | |
| 513 | + | ); | |
| 486 | 514 | ||
| 487 | 515 | // ClamAV: buffered scans the in-memory bytes; spooled streams the file by | |
| 488 | 516 | // path (INSTREAM frames) so a >100 MB object isn't re-buffered. The | |
| @@ -540,8 +568,8 @@ impl ScanPipeline { | |||
| 540 | 568 | let (sync_result, clamav_result, urlhaus_result) = | |
| 541 | 569 | tokio::join!(sync_fut, clamav_fut, urlhaus_fut); | |
| 542 | 570 | let (mut layers, sha256) = match sync_result { | |
| 543 | - | Ok(v) => v, | |
| 544 | - | Err(join_err) => { | |
| 571 | + | Ok(Ok(v)) => v, | |
| 572 | + | Ok(Err(join_err)) => { | |
| 545 | 573 | tracing::error!( | |
| 546 | 574 | error = %join_err, | |
| 547 | 575 | is_panic = join_err.is_panic(), | |
| @@ -549,6 +577,14 @@ impl ScanPipeline { | |||
| 549 | 577 | ); | |
| 550 | 578 | return panicked_sync_result(file_size); | |
| 551 | 579 | } | |
| 580 | + | Err(_elapsed) => { | |
| 581 | + | tracing::error!( | |
| 582 | + | timeout_secs = crate::constants::SCAN_CPU_LAYERS_TIMEOUT_SECS, | |
| 583 | + | "scan CPU layers exceeded the wall-clock deadline; holding file for \ | |
| 584 | + | review (worker freed; blocking thread runs to completion)" | |
| 585 | + | ); | |
| 586 | + | return timed_out_sync_result(file_size); | |
| 587 | + | } | |
| 552 | 588 | }; | |
| 553 | 589 | layers.push(clamav_result); | |
| 554 | 590 | layers.push(urlhaus_result); |