From 83474215648cfb5fa0b102f3edb55196e3c0e5cd Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Wed, 9 Sep 2026 09:52:26 +0000 Subject: [PATCH 1/5] test: uplift deterministic compatibility harness --- CONTRIBUTING.md | 14 + Cargo.lock | 1 + apps/rustnzb/src/handlers.rs | 11 +- ci/run | 4 +- ci/tasks/harness | 12 + crates/nzb-postproc/src/unpack.rs | 166 ++++++++++- crates/nzb-web/Cargo.toml | 1 + crates/nzb-web/src/dir_watcher.rs | 86 +++++- crates/nzb-web/src/fetch_guard.rs | 132 ++++++++- crates/nzb-web/src/queue_manager.rs | 18 +- crates/nzb-web/src/rss_monitor.rs | 130 ++++++--- crates/nzb-web/src/sabnzbd_compat.rs | 92 +++++++ crates/nzb-web/tests/harness/mod.rs | 140 +++++++++- crates/nzb-web/tests/harness/nzb_fixture.rs | 65 +++++ crates/nzb-web/tests/harness_catalog.rs | 36 +++ .../nzb-web/tests/harness_failure_matrix.rs | 257 ++++++++++++++++++ crates/nzb-web/tests/workflow_fixtures.rs | 83 ++++++ docs/DEVELOPMENT.md | 37 +++ 18 files changed, 1213 insertions(+), 72 deletions(-) create mode 100755 ci/tasks/harness create mode 100644 crates/nzb-web/tests/harness_catalog.rs create mode 100644 crates/nzb-web/tests/harness_failure_matrix.rs create mode 100644 crates/nzb-web/tests/workflow_fixtures.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8a9bac60..53996eb9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,6 +34,20 @@ npm test -- --watch=false npm run build -- --configuration=production ``` +Compatibility and security regression tests are deterministic and must remain +network-independent. Use the focused harness gates while iterating: + +```bash +cargo test -p nzb-web --tests --locked +cargo test -p rustnzb --tests --locked +cargo test -p nzb-postproc --tests --locked +``` + +Golden responses are reviewed as API contract changes: keep dynamic type +markers for timestamps, rates, paths, and generated identifiers, and update +the fixture README when the capture source changes. Do not add credentials, +provider URLs, private hostnames, or personal paths to fixtures or logs. + The containerized task interface in [`ci/run`](ci/run) provides local parity with selected build tasks. See [docs/DEVELOPMENT.md](docs/DEVELOPMENT.md) for the supported commands. diff --git a/Cargo.lock b/Cargo.lock index 78ddc46c..e7c90bf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,6 +1890,7 @@ dependencies = [ "base64 0.23.1", "chrono", "feed-rs", + "flate2", "governor", "hex", "http", diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 3a169a62..0c5f44cd 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -15,12 +15,12 @@ use serde::{Deserialize, Serialize}; static HTTP_CLIENT: std::sync::LazyLock = std::sync::LazyLock::new(|| { reqwest::Client::builder() .timeout(std::time::Duration::from_secs(30)) + .redirect(reqwest::redirect::Policy::none()) .build() .expect("Failed to build shared HTTP client") }); const MAX_NZB_DECOMPRESSED_BYTES: u64 = 100 * 1024 * 1024; -const MAX_FETCH_BODY_BYTES: usize = 100 * 1024 * 1024; #[cfg(feature = "webdav")] use nzb_web::nzb_core::config::DavConfig; @@ -32,7 +32,9 @@ use nzb_web::nzb_core::nzb_parser; use nzb_web::nzb_core::sabnzbd_import; use nzb_web::error::ApiError; -use nzb_web::fetch_guard::{build_fetch_client, read_response_bytes_limited, validate_fetch_url}; +use nzb_web::fetch_guard::{ + MAX_FETCH_BODY_BYTES, build_fetch_client, read_response_bytes_limited, validate_fetch_url, +}; use nzb_web::log_buffer::LogEntry; use nzb_web::state::AppState; @@ -1939,9 +1941,8 @@ pub async fn h_import_sabnzbd_api( ))); } - let json: serde_json::Value = resp - .json() - .await + let body = read_response_bytes_limited(resp, MAX_FETCH_BODY_BYTES).await?; + let json: serde_json::Value = serde_json::from_slice(&body) .map_err(|e| ApiError::from(anyhow::anyhow!("Invalid JSON from SABnzbd: {e}")))?; let preview = sabnzbd_import::parse_sabnzbd_api_response(&json); diff --git a/ci/run b/ci/run index 77f22af5..b58c4422 100755 --- a/ci/run +++ b/ci/run @@ -7,7 +7,7 @@ cold=false usage() { printf '%s\n' 'usage: ./ci/run [--cold-cache] TASK [TASK_ARGS...]' >&2 - printf '%s\n' 'tasks: fmt check test clippy frontend-test frontend-audit frontend-build e2e desktop-test' >&2 + printf '%s\n' 'tasks: fmt check test clippy harness frontend-test frontend-audit frontend-build e2e desktop-test' >&2 printf '%s\n' ' build-linux build-linux-arm64 build-windows package-release' >&2 printf '%s\n' ' verify-image-pins build-ci-images build-image smoke-image' >&2 exit 2 @@ -27,7 +27,7 @@ lock_value() { } case "$task" in - fmt|check|test|clippy|frontend-test|frontend-audit|frontend-build|verify-image-pins) image_key=CORE_IMAGE ;; + fmt|check|test|clippy|harness|frontend-test|frontend-audit|frontend-build|verify-image-pins) image_key=CORE_IMAGE ;; e2e) image_key=E2E_IMAGE ;; desktop-test) image_key=DESKTOP_IMAGE ;; build-linux|build-linux-arm64|build-windows|package-release) image_key=CROSS_IMAGE ;; diff --git a/ci/tasks/harness b/ci/tasks/harness new file mode 100755 index 00000000..3af41703 --- /dev/null +++ b/ci/tasks/harness @@ -0,0 +1,12 @@ +#!/bin/sh +set -eu +. "$(dirname "$0")/lib.sh" +trap cleanup_frontend EXIT +task_start harness cargo +task_target_dir harness +prepare_placeholder_frontend + +cargo test -p nzb-web --tests --locked +cargo test -p nzb-postproc --tests --locked +cargo test -p rustnzb --tests --locked +show_sccache_stats diff --git a/crates/nzb-postproc/src/unpack.rs b/crates/nzb-postproc/src/unpack.rs index 8ab633e2..bc6e5b40 100644 --- a/crates/nzb-postproc/src/unpack.rs +++ b/crates/nzb-postproc/src/unpack.rs @@ -119,6 +119,53 @@ fn newly_extracted_files( Ok(files) } +fn safe_zip_output_path(root: &Path, name: &str) -> anyhow::Result { + let normalized = name.replace('\\', "/"); + let has_drive_prefix = normalized.as_bytes().get(1) == Some(&b':'); + if normalized.is_empty() + || normalized.starts_with('/') + || has_drive_prefix + || normalized.split('/').any(|component| component == "..") + { + anyhow::bail!("ZIP archive contains unsafe path `{name}`"); + } + + let output = normalized + .split('/') + .filter(|component| !component.is_empty() && *component != ".") + .fold(root.to_path_buf(), |path, component| path.join(component)); + if !output.starts_with(root) { + anyhow::bail!("ZIP archive path escapes extraction directory: `{name}`"); + } + Ok(output) +} + +fn reject_symlinked_path(root: &Path, path: &Path) -> anyhow::Result<()> { + let relative = path + .strip_prefix(root) + .map_err(|_| anyhow::anyhow!("ZIP archive path is outside extraction directory"))?; + let mut current = root.to_path_buf(); + if let Ok(metadata) = std::fs::symlink_metadata(¤t) + && metadata.file_type().is_symlink() + { + anyhow::bail!("ZIP extraction directory is a symbolic link"); + } + + for component in relative.components() { + current.push(component.as_os_str()); + let Ok(metadata) = std::fs::symlink_metadata(¤t) else { + continue; + }; + if metadata.file_type().is_symlink() { + anyhow::bail!("ZIP archive path crosses a symbolic link"); + } + if current != path && !metadata.is_dir() { + anyhow::bail!("ZIP archive path crosses a non-directory"); + } + } + Ok(()) +} + /// Extract RAR archives in a directory. /// /// If `password` is `Some`, it is passed to the extractor (`-p` for unrar, @@ -281,14 +328,32 @@ pub async fn extract_zip(zip_file: &Path, output_dir: &Path) -> anyhow::Result, @@ -69,8 +73,11 @@ impl DirWatcher { } fn is_nzb_file(path: &Path) -> bool { - path.extension().is_some_and(|ext| ext == "nzb") - || path.to_str().is_some_and(|s| s.ends_with(".nzb.gz")) + path.extension().is_some_and(|ext| ext == "nzb") || Self::is_gz_nzb(path) + } + + fn is_gz_nzb(path: &Path) -> bool { + path.to_str().is_some_and(|s| s.ends_with(".nzb.gz")) } async fn process_existing_files(&self) { @@ -93,7 +100,7 @@ impl DirWatcher { async fn process_file(&self, path: &Path) { info!(file = %path.display(), "Processing NZB from watch directory"); - let data = match std::fs::read(path) { + let raw_data = match Self::read_limited(path) { Ok(d) => d, Err(e) => { warn!(error = %e, file = %path.display(), "Failed to read NZB file"); @@ -101,11 +108,37 @@ impl DirWatcher { } }; - let name = path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("unknown") - .to_string(); + let data = if Self::is_gz_nzb(path) { + let decoder = GzDecoder::new(raw_data.as_slice()); + let mut decompressed = Vec::new(); + if let Err(error) = decoder + .take((MAX_WATCHED_NZB_BYTES as u64).saturating_add(1)) + .read_to_end(&mut decompressed) + { + warn!(error = %error, file = %path.display(), "Failed to decompress watched NZB"); + return; + } + if decompressed.len() > MAX_WATCHED_NZB_BYTES { + warn!(file = %path.display(), limit = MAX_WATCHED_NZB_BYTES, "Decompressed watched NZB exceeds the input limit"); + return; + } + decompressed + } else { + raw_data + }; + + let name = if Self::is_gz_nzb(path) { + path.file_name() + .and_then(|name| name.to_str()) + .and_then(|name| name.strip_suffix(".nzb.gz")) + .unwrap_or("unknown") + .to_string() + } else { + path.file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("unknown") + .to_string() + }; match crate::nzb_core::nzb_parser::parse_nzb(&name, &data) { Ok(mut job) => { @@ -142,6 +175,34 @@ impl DirWatcher { } } } + + fn read_limited(path: &Path) -> std::io::Result> { + let metadata = std::fs::symlink_metadata(path)?; + if metadata.file_type().is_symlink() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "watched NZB symlinks are not supported", + )); + } + if metadata.len() > MAX_WATCHED_NZB_BYTES as u64 { + return Err(std::io::Error::new( + std::io::ErrorKind::FileTooLarge, + format!("watched NZB exceeds the {MAX_WATCHED_NZB_BYTES} byte limit"), + )); + } + + let file = std::fs::File::open(path)?; + let mut data = Vec::new(); + file.take((MAX_WATCHED_NZB_BYTES as u64).saturating_add(1)) + .read_to_end(&mut data)?; + if data.len() > MAX_WATCHED_NZB_BYTES { + return Err(std::io::Error::new( + std::io::ErrorKind::FileTooLarge, + format!("watched NZB exceeds the {MAX_WATCHED_NZB_BYTES} byte limit"), + )); + } + Ok(data) + } } #[cfg(test)] @@ -155,4 +216,13 @@ mod tests { assert!(!DirWatcher::is_nzb_file(Path::new("release.NZB"))); assert!(!DirWatcher::is_nzb_file(Path::new("release.txt"))); } + + #[test] + fn bounded_file_reader_rejects_oversized_input() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("oversized.nzb"); + std::fs::write(&path, vec![b'x'; MAX_WATCHED_NZB_BYTES + 1]).unwrap(); + let error = DirWatcher::read_limited(&path).unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::FileTooLarge); + } } diff --git a/crates/nzb-web/src/fetch_guard.rs b/crates/nzb-web/src/fetch_guard.rs index c032bab1..ca06e0c1 100644 --- a/crates/nzb-web/src/fetch_guard.rs +++ b/crates/nzb-web/src/fetch_guard.rs @@ -13,6 +13,9 @@ use std::net::{IpAddr, SocketAddr}; use crate::error::ApiError; +/// Maximum body size accepted by URL-backed NZB and feed workflows. +pub const MAX_FETCH_BODY_BYTES: usize = 100 * 1024 * 1024; + #[derive(Debug)] pub struct FetchUrlPlan { pub url: reqwest::Url, @@ -92,7 +95,11 @@ pub async fn validate_fetch_url(raw_url: &str) -> Result /// Build a reqwest client pinned to the addresses validated in `plan`, so a /// hostname cannot re-resolve to an internal address after the check. pub fn build_fetch_client(plan: &FetchUrlPlan) -> Result { - let mut builder = reqwest::Client::builder().timeout(std::time::Duration::from_secs(30)); + let mut builder = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + // Redirect targets are not covered by the original DNS validation. + // Refuse redirects so a public URL cannot bounce into a private host. + .redirect(reqwest::redirect::Policy::none()); if let Some((host, addrs)) = &plan.resolved_addrs { builder = builder.resolve_to_addrs(host, addrs.as_slice()); } @@ -107,6 +114,16 @@ pub async fn read_response_bytes_limited( mut response: reqwest::Response, max_bytes: usize, ) -> Result, ApiError> { + if response + .content_length() + .is_some_and(|length| length > max_bytes as u64) + { + return Err(ApiError::from(anyhow::anyhow!( + "Fetched body exceeds the {} MB limit", + max_bytes / 1024 / 1024 + ))); + } + let mut body = Vec::new(); while let Some(chunk) = response .chunk() @@ -127,15 +144,39 @@ pub async fn read_response_bytes_limited( fn is_globally_routable(ip: IpAddr) -> bool { match ip { IpAddr::V4(v4) => { - !v4.is_loopback() + let [first, second, ..] = v4.octets(); + let this_network = first == 0; + let shared_address_space = first == 100 && (64..=127).contains(&second); + let benchmarking_space = first == 198 && (18..=19).contains(&second); + let reserved_zero_block = first == 192 && second == 0; + let multicast = (224..=239).contains(&first); + let reserved_future_use = first >= 240; + !this_network + && !v4.is_loopback() && !v4.is_private() && !v4.is_link_local() && !v4.is_broadcast() && !v4.is_unspecified() && !v4.is_documentation() + && !shared_address_space + && !benchmarking_space + && !reserved_zero_block + && !multicast + && !reserved_future_use } IpAddr::V6(v6) => { - !v6.is_loopback() && !v6.is_unspecified() && !v6.is_multicast() && !v6.is_unique_local() + let first = v6.segments()[0]; + let mapped_is_global = v6 + .to_ipv4_mapped() + .is_none_or(|mapped| is_globally_routable(IpAddr::V4(mapped))); + !v6.is_loopback() + && !v6.is_unspecified() + && !v6.is_multicast() + && !v6.is_unique_local() + && (first & 0xffc0) != 0xfe80 + && (first & 0xffc0) != 0xfec0 + && !(first == 0x2001 && v6.segments()[1] == 0x0db8) + && mapped_is_global } } } @@ -174,4 +215,89 @@ mod tests { let err = validate_fetch_url("file:///etc/passwd").await.unwrap_err(); assert!(err.to_string().contains("not allowed")); } + + #[tokio::test] + async fn validate_fetch_url_rejects_special_use_address_ranges() { + for url in [ + "http://100.64.0.1/file.nzb", + "http://198.18.0.1/file.nzb", + "http://192.0.0.1/file.nzb", + "http://0.1.2.3/file.nzb", + "http://224.0.0.1/file.nzb", + "http://240.0.0.1/file.nzb", + "http://[fe80::1]/file.nzb", + "http://[2001:db8::1]/file.nzb", + ] { + let error = validate_fetch_url(url) + .await + .expect_err("special-use address must be rejected"); + assert!( + error.to_string().contains("private/reserved"), + "{url}: {error}" + ); + } + } + + async fn one_shot_http_response(response: &'static str) -> reqwest::Url { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind local fixture"); + let address = listener.local_addr().expect("local fixture address"); + tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.expect("accept local fixture"); + let mut request = [0; 1024]; + let _ = socket.read(&mut request).await; + let _ = socket.write_all(response.as_bytes()).await; + let _ = socket.shutdown().await; + }); + format!("http://{address}/fixture").parse().unwrap() + } + + #[tokio::test] + async fn pinned_client_uses_validated_address_and_does_not_follow_redirects() { + let url = one_shot_http_response( + "HTTP/1.1 302 Found\r\nLocation: http://127.0.0.1/private\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .await; + let address = url.port().expect("fixture port"); + let plan = FetchUrlPlan { + url: url.clone(), + resolved_addrs: Some(( + "fixture.invalid".into(), + vec![std::net::SocketAddr::from(([127, 0, 0, 1], address))], + )), + }; + let client = build_fetch_client(&plan).expect("build pinned client"); + let response = client + .get(url) + .header("host", "fixture.invalid") + .send() + .await + .expect("request local fixture"); + assert_eq!(response.status(), reqwest::StatusCode::FOUND); + } + + #[tokio::test] + async fn response_body_limit_is_enforced_incrementally() { + let url = one_shot_http_response( + "HTTP/1.1 200 OK\r\nContent-Length: 8\r\nConnection: close\r\n\r\n12345678", + ) + .await; + let plan = FetchUrlPlan { + url: url.clone(), + resolved_addrs: None, + }; + let response = build_fetch_client(&plan) + .expect("build fixture client") + .get(url) + .send() + .await + .expect("request body fixture"); + let error = read_response_bytes_limited(response, 4) + .await + .expect_err("oversized body must be rejected"); + assert!(error.to_string().contains("exceeds")); + } } diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index 282e6252..e875c9d8 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -1192,7 +1192,11 @@ impl QueueManager { state.job.articles_failed = checkpoint.articles_failed; state.job.files_completed = checkpoint.files_completed; for file in &mut state.job.files { - if let Some(segments) = checkpoint.files.get(&file.id) { + let segments = checkpoint + .files + .get(&file.filename) + .or_else(|| checkpoint.files.get(&file.id)); + if let Some(segments) = segments { let mut fbd: u64 = 0; for article in &mut file.articles { if segments.contains(&article.segment_number) { @@ -2148,7 +2152,11 @@ impl QueueManager { .filter(|a| a.downloaded) .map(|a| a.segment_number) .collect(); - (f.id.clone(), downloaded_segments) + // File IDs are generated while parsing an NZB and + // therefore change after a restart. The filename is + // stable across parses; accept the old ID key while + // reading checkpoints written by older versions. + (f.filename.clone(), downloaded_segments) }) .collect(), downloaded_bytes: state.job.downloaded_bytes, @@ -3309,7 +3317,11 @@ impl QueueManager { job.files_completed = checkpoint.files_completed; for file in &mut job.files { - if let Some(segments) = checkpoint.files.get(&file.id) { + let segments = checkpoint + .files + .get(&file.filename) + .or_else(|| checkpoint.files.get(&file.id)); + if let Some(segments) = segments { let mut file_bytes_downloaded: u64 = 0; for article in &mut file.articles { if segments.contains(&article.segment_number) { diff --git a/crates/nzb-web/src/rss_monitor.rs b/crates/nzb-web/src/rss_monitor.rs index 7164e5e0..79f387e8 100644 --- a/crates/nzb-web/src/rss_monitor.rs +++ b/crates/nzb-web/src/rss_monitor.rs @@ -6,6 +6,9 @@ use arc_swap::ArcSwap; use chrono::Utc; use tracing::{info, warn}; +use crate::fetch_guard::{ + MAX_FETCH_BODY_BYTES, build_fetch_client, read_response_bytes_limited, validate_fetch_url, +}; use crate::nzb_core::config::{AppConfig, RssFeedConfig}; use crate::nzb_core::models::{Priority, RssItem}; @@ -85,11 +88,6 @@ impl RssMonitor { // Migrate legacy seen file on first run self.migrate_seen_json(); - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(30)) - .build() - .expect("Failed to create HTTP client"); - loop { let cfg = self.config.load(); let feeds = &cfg.rss_feeds; @@ -99,7 +97,7 @@ impl RssMonitor { continue; } - if let Err(e) = self.check_feed(&client, feed).await { + if let Err(e) = self.check_feed(feed).await { warn!(feed = %feed.name, error = %e, "RSS feed check failed"); } } @@ -127,22 +125,31 @@ impl RssMonitor { } } - async fn check_feed( - &self, - client: &reqwest::Client, - feed: &RssFeedConfig, - ) -> anyhow::Result<()> { + async fn check_feed(&self, feed: &RssFeedConfig) -> anyhow::Result<()> { info!(feed = %feed.name, url = %feed.url, "Checking RSS feed"); - let response = client.get(&feed.url).send().await?; - let body = response.bytes().await?; + let feed_plan = validate_fetch_url(&feed.url) + .await + .map_err(|error| anyhow::anyhow!(error.to_string()))?; + let feed_client = + build_fetch_client(&feed_plan).map_err(|error| anyhow::anyhow!(error.to_string()))?; + let response = feed_client.get(feed_plan.url).send().await?; + if !response.status().is_success() { + anyhow::bail!("HTTP {}", response.status()); + } + let body = read_response_bytes_limited(response, MAX_FETCH_BODY_BYTES) + .await + .map_err(|error| anyhow::anyhow!(error.to_string()))?; let parsed = feed_rs::parser::parse(&body[..])?; // Compile filter regex if provided - let filter = feed - .filter_regex - .as_ref() - .and_then(|r| regex::Regex::new(r).ok()); + let filter = match feed.filter_regex.as_deref() { + None => None, + Some(pattern) => Some( + Self::compile_filter(pattern) + .ok_or_else(|| anyhow::anyhow!("invalid RSS filter expression"))?, + ), + }; // Load download rules for this feed let rules = self @@ -198,6 +205,17 @@ impl RssMonitor { // Batch insert all items in one transaction (single DB lock) let items_for_insert: Vec = pending.iter().map(|p| p.item.clone()).collect(); + let downloaded_ids: HashSet = pending + .iter() + .filter(|pending| { + self.queue_manager + .rss_item_get(&pending.item.id) + .ok() + .flatten() + .is_some_and(|item| item.downloaded) + }) + .map(|pending| pending.item.id.clone()) + .collect(); let new_items = self .queue_manager .rss_items_batch_upsert(&items_for_insert) @@ -205,9 +223,14 @@ impl RssMonitor { // Now process auto-downloads for newly inserted items only // (batch_upsert uses INSERT OR IGNORE so only new items get inserted) + let mut handled_ids = HashSet::new(); for p in &pending { let Some(ref url) = p.nzb_url else { continue }; + if downloaded_ids.contains(&p.item.id) || !handled_ids.insert(p.item.id.clone()) { + continue; + } + // Feed-level filter must pass (if set) let passes_filter = match filter { Some(ref re) => re.is_match(&p.title), @@ -219,7 +242,7 @@ impl RssMonitor { // Check download rules let matched_rule = rules.iter().find(|r| { - regex::Regex::new(&r.match_regex) + Self::compile_filter(&r.match_regex) .map(|re| re.is_match(&p.title)) .unwrap_or(false) }); @@ -244,26 +267,10 @@ impl RssMonitor { continue; } - // Skip if already downloaded (existing item in DB) - if self - .queue_manager - .rss_item_exists(&p.item.id) - .unwrap_or(false) - { - // Item existed before this batch — already processed previously - // Check if it was newly inserted by seeing if it's in our new count - // Actually, we can just check the downloaded flag - if let Ok(Some(existing)) = self.queue_manager.rss_item_get(&p.item.id) - && existing.downloaded - { - continue; - } - } - info!(feed = %feed.name, title = %p.title, url = %url, "Auto-downloading RSS item"); match self - .fetch_and_enqueue(client, url, &p.title, feed, category.as_deref(), priority) + .fetch_and_enqueue(url, &p.title, feed, category.as_deref(), priority) .await { Ok(()) => { @@ -285,6 +292,17 @@ impl RssMonitor { Ok(()) } + fn compile_filter(pattern: &str) -> Option { + const MAX_PATTERN_BYTES: usize = 512; + if pattern.len() > MAX_PATTERN_BYTES { + return None; + } + regex::RegexBuilder::new(pattern) + .size_limit(1024 * 1024) + .build() + .ok() + } + /// Extract NZB URL from a feed entry's links or media content. fn extract_nzb_url(entry: &feed_rs::model::Entry) -> Option { entry @@ -314,18 +332,24 @@ impl RssMonitor { async fn fetch_and_enqueue( &self, - client: &reqwest::Client, url: &str, name: &str, feed: &RssFeedConfig, category: Option<&str>, priority: i32, ) -> anyhow::Result<()> { - let response = client.get(url).send().await?; + let plan = validate_fetch_url(url) + .await + .map_err(|error| anyhow::anyhow!(error.to_string()))?; + let client = + build_fetch_client(&plan).map_err(|error| anyhow::anyhow!(error.to_string()))?; + let response = client.get(plan.url).send().await?; if !response.status().is_success() { anyhow::bail!("HTTP {}", response.status()); } - let data = response.bytes().await?; + let data = read_response_bytes_limited(response, MAX_FETCH_BODY_BYTES) + .await + .map_err(|error| anyhow::anyhow!(error.to_string()))?; let mut job = crate::nzb_core::nzb_parser::parse_nzb(name, &data)?; @@ -354,7 +378,7 @@ impl RssMonitor { std::fs::create_dir_all(&job.work_dir)?; - self.queue_manager.add_job(job, Some(data.to_vec()))?; + self.queue_manager.add_job(job, Some(data))?; Ok(()) } } @@ -433,4 +457,32 @@ mod tests { assert!(item.downloaded_at.is_some()); } } + + #[test] + fn feed_filters_fail_closed_at_syntax_and_size_limits() { + assert!(RssMonitor::compile_filter(r"release-[0-9]+").is_some()); + assert!(RssMonitor::compile_filter("(").is_none()); + assert!(RssMonitor::compile_filter(&"x".repeat(513)).is_none()); + } + + #[tokio::test] + async fn feed_checks_reject_local_targets_before_network_access() { + let temp = tempfile::tempdir().expect("tempdir"); + let (monitor, _) = monitor(temp.path().to_path_buf()); + let feed = RssFeedConfig { + name: "local-feed".into(), + url: "http://127.0.0.1:9/feed.xml".into(), + poll_interval_secs: 900, + category: None, + filter_regex: None, + enabled: true, + auto_download: true, + }; + + let error = monitor + .check_feed(&feed) + .await + .expect_err("local addresses must be rejected"); + assert!(error.to_string().contains("private/reserved")); + } } diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index c48b4bc2..eacdc493 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -2724,6 +2724,98 @@ mod tests { assert_eq!(response["scripts"], serde_json::json!(["None"])); } + #[tokio::test] + async fn rejected_requests_keep_the_error_envelope() { + let test_state = test_state(); + + for provided in [None, Some("wrong-key")] { + let response = validate_api_key(&test_state.state, provided) + .expect_err("invalid credentials must be rejected") + .0; + assert_eq!(response["status"], serde_json::json!(false)); + assert!(response["error"].is_string()); + } + + let response = dispatch_mode( + &test_state.state, + "unknown-contract-mode", + &SabApiRequest::default(), + ) + .0; + assert_eq!(response["status"], serde_json::json!(false)); + assert!(response["error"].as_str().unwrap().contains("Unknown mode")); + } + + #[tokio::test] + async fn representative_read_modes_keep_stable_top_level_types() { + let test_state = test_state(); + + let config = dispatch_mode(&test_state.state, "get_config", &SabApiRequest::default()).0; + assert!(config["config"]["misc"]["complete_dir"].is_string()); + assert!(config["config"]["categories"].is_array()); + + let categories = dispatch_mode(&test_state.state, "get_cats", &SabApiRequest::default()).0; + assert!(categories["categories"].is_array()); + + let scripts = dispatch_mode(&test_state.state, "get_scripts", &SabApiRequest::default()).0; + assert!(scripts["scripts"].is_array()); + } + + #[tokio::test] + async fn addfile_reports_success_and_parse_errors_as_json() { + let test_state = test_state(); + let success = dispatch_post( + &test_state.state, + "addfile".into(), + None, + None, + None, + Some(("contract.nzb".into(), SAMPLE_NZB.as_bytes().to_vec())), + None, + None, + SabApiRequest::default(), + ) + .await + .expect("addfile response") + .0; + assert_eq!(success["status"], serde_json::json!(true)); + assert_eq!(success["nzo_ids"].as_array().unwrap().len(), 1); + + let missing = dispatch_post( + &test_state.state, + "addfile".into(), + None, + None, + None, + None, + None, + None, + SabApiRequest::default(), + ) + .await + .expect("missing-file response") + .0; + assert_eq!(missing["status"], serde_json::json!(false)); + assert!(missing["error"].is_string()); + + let malformed = dispatch_post( + &test_state.state, + "addfile".into(), + None, + None, + None, + Some(("malformed.nzb".into(), b"not an nzb".to_vec())), + None, + None, + SabApiRequest::default(), + ) + .await + .expect("malformed-file response") + .0; + assert_eq!(malformed["status"], serde_json::json!(false)); + assert!(malformed["error"].is_string()); + } + /// SABnzbd's real `_api_queue_delete` accepts a comma-separated `value` /// list, removing every matching job in one call. #[tokio::test] diff --git a/crates/nzb-web/tests/harness/mod.rs b/crates/nzb-web/tests/harness/mod.rs index a82d9060..9766ff8b 100644 --- a/crates/nzb-web/tests/harness/mod.rs +++ b/crates/nzb-web/tests/harness/mod.rs @@ -28,6 +28,7 @@ use nzb_web::nzb_core::config::ServerConfig; use nzb_web::nzb_core::db::Database; use nzb_web::nzb_core::models::{JobStatus, NzbJob}; use nzb_web::nzb_core::nzb_parser; +use nzb_web::nzb_postproc::PostProcLimits; use nzb_web::queue_manager::QueueManager; use tempfile::TempDir; @@ -90,24 +91,32 @@ impl ServerProfile { /// independently optional; defaults are tuned for fast, deterministic tests. pub struct HarnessBuilder { servers: Vec, + server_configs: Vec, + database_path: Option, + state_dir: Option, article_timeout_secs: u64, max_active_downloads: usize, abort_hopeless: bool, early_failure_check: bool, required_completion_pct: f64, speed_limit_bps: u64, + postproc_limits: PostProcLimits, } impl HarnessBuilder { pub fn new() -> Self { Self { servers: Vec::new(), + server_configs: Vec::new(), + database_path: None, + state_dir: None, article_timeout_secs: 30, max_active_downloads: 5, abort_hopeless: true, early_failure_check: true, required_completion_pct: 100.0, speed_limit_bps: 0, + postproc_limits: PostProcLimits::default(), } } @@ -116,6 +125,60 @@ impl HarnessBuilder { self } + pub fn with_server_config(mut self, config: ServerConfig) -> Self { + self.server_configs.push(config); + self + } + + pub fn with_database_path(mut self, path: PathBuf) -> Self { + self.database_path = Some(path); + self + } + + pub fn with_state_dir(mut self, path: PathBuf) -> Self { + self.state_dir = Some(path); + self + } + + /// Happy-path profile: one healthy provider and the production-like + /// completion policy used by smoke tests. + pub fn happy_path(server: ServerProfile) -> Self { + Self::new().with_server(server) + } + + /// Retry profile: short article deadlines expose reconnect and failover + /// behavior without making the test wait through production timeouts. + pub fn retrying(server: ServerProfile) -> Self { + Self::new().with_server(server).article_timeout(3) + } + + /// Pause/resume profile: keep one active download so control-plane tests + /// can observe a pause boundary deterministically. + pub fn pause_resume(server: ServerProfile) -> Self { + Self::new().with_server(server).max_active_downloads(1) + } + + /// Cancellation profile: a single worker makes slot-release assertions + /// independent of scheduler width. + pub fn cancellation(server: ServerProfile) -> Self { + Self::new().with_server(server).max_active_downloads(1) + } + + /// Hopeless-job profile: enable the failure watchdog and use a compact + /// article deadline so silent providers converge quickly. + pub fn hopeless(server: ServerProfile) -> Self { + Self::new() + .with_server(server) + .article_timeout(2) + .abort_hopeless(true) + } + + /// Restart-recovery profile: a single active worker makes checkpoint and + /// requeue assertions independent of scheduler width. + pub fn restart_recovery(server: ServerProfile) -> Self { + Self::new().with_server(server).max_active_downloads(1) + } + pub fn article_timeout(mut self, secs: u64) -> Self { self.article_timeout_secs = secs; self @@ -131,27 +194,69 @@ impl HarnessBuilder { self } - /// Build the engine. Creates temp dirs, an in-memory database, and a - /// fully-wired `QueueManager` whose worker pool is already running. + pub fn early_failure_check(mut self, enabled: bool) -> Self { + self.early_failure_check = enabled; + self + } + + pub fn required_completion_pct(mut self, percentage: f64) -> Self { + self.required_completion_pct = percentage; + self + } + + pub fn speed_limit_bps(mut self, bytes_per_second: u64) -> Self { + self.speed_limit_bps = bytes_per_second; + self + } + + pub fn postproc_limits(mut self, limits: PostProcLimits) -> Self { + self.postproc_limits = limits; + self + } + + /// Build the engine. Creates isolated directories and a fully-wired + /// `QueueManager` whose worker pool is already running. pub fn build(self) -> TestEngine { init_test_tracing(); - let tempdir = TempDir::new().expect("create tempdir"); - let incomplete_dir = tempdir.path().join("incomplete"); - let complete_dir = tempdir.path().join("complete"); + let tempdir = self + .state_dir + .is_none() + .then(|| TempDir::new().expect("create tempdir")); + let root = self.state_dir.clone().unwrap_or_else(|| { + tempdir + .as_ref() + .expect("temporary state") + .path() + .to_path_buf() + }); + std::fs::create_dir_all(&root).expect("create harness state directory"); + let incomplete_dir = root.join("incomplete"); + let complete_dir = root.join("complete"); std::fs::create_dir_all(&incomplete_dir).expect("create incomplete_dir"); std::fs::create_dir_all(&complete_dir).expect("create complete_dir"); - let db = Database::open_memory().expect("open in-memory db"); - let server_configs: Vec = - self.servers.iter().map(|p| p.config.clone()).collect(); + let db = self + .database_path + .as_deref() + .map(Database::open) + .transpose() + .expect("open harness database") + .unwrap_or_else(|| Database::open_memory().expect("open in-memory db")); + let server_configs: Vec = self + .servers + .iter() + .map(|p| p.config.clone()) + .chain(self.server_configs) + .collect(); - let queue_manager = QueueManager::new( + let queue_manager = QueueManager::new_with_postproc_limits( server_configs, db, incomplete_dir.clone(), complete_dir.clone(), LogBuffer::default(), self.max_active_downloads, + self.postproc_limits, Vec::new(), // categories 0, // min_free_space self.speed_limit_bps, @@ -190,7 +295,7 @@ impl Default for HarnessBuilder { pub struct TestEngine { pub queue_manager: Arc, _servers: Vec, - _tempdir: TempDir, + _tempdir: Option, pub incomplete_dir: PathBuf, pub complete_dir: PathBuf, } @@ -229,6 +334,14 @@ impl TestEngine { self.snapshot().jobs.into_iter().find(|j| j.id == id) } + pub fn history_status(&self, id: &str) -> Option { + self.queue_manager + .history_get(id) + .ok() + .flatten() + .map(|entry| entry.status) + } + /// Poll `predicate` against fresh snapshots until it returns `true` or /// the timeout elapses. Returns `true` on success. Polls every 100 ms. pub async fn wait_for(&self, timeout: Duration, mut predicate: F) -> bool @@ -260,7 +373,10 @@ impl TestEngine { .iter() .find(|j| j.id == job_id) .map(|j| statuses.contains(&j.status)) - .unwrap_or(false) + .unwrap_or_else(|| { + self.history_status(job_id) + .is_some_and(|status| statuses.contains(&status)) + }) }) .await } @@ -291,6 +407,7 @@ pub struct JobView { pub articles_failed: usize, pub downloaded_bytes: u64, pub total_bytes: u64, + pub error_message: Option, } impl From for JobView { @@ -304,6 +421,7 @@ impl From for JobView { articles_failed: j.articles_failed, downloaded_bytes: j.downloaded_bytes, total_bytes: j.total_bytes, + error_message: j.error_message, } } } diff --git a/crates/nzb-web/tests/harness/nzb_fixture.rs b/crates/nzb-web/tests/harness/nzb_fixture.rs index 2d4f596b..accef76c 100644 --- a/crates/nzb-web/tests/harness/nzb_fixture.rs +++ b/crates/nzb-web/tests/harness/nzb_fixture.rs @@ -14,8 +14,11 @@ //! // into harness::yenc_articles for the mock config //! ``` +use std::collections::HashMap; use std::fmt::Write; +use nzb_nntp::testutil::MockConfig; + #[derive(Default)] pub struct NzbFixture<'a> { name: String, @@ -36,6 +39,68 @@ pub struct BuiltFixture<'a> { pub articles: Vec<(&'a str, &'a [u8], String)>, } +/// Owned, reusable fixture cases for tests that need to hand the same input +/// to several providers or restart a queue manager. All bytes are generated +/// from literals, so the catalog never reads the network or wall clock. +#[derive(Clone, Debug)] +pub struct FixtureCase { + pub name: String, + pub xml: Vec, + pub articles: HashMap>, +} + +impl FixtureCase { + pub fn mock_config(&self) -> MockConfig { + MockConfig { + articles: self.articles.clone(), + ..MockConfig::default() + } + } +} + +/// Stable fixture catalog shared by harness profiles and contract tests. +pub struct FixtureCatalog; + +impl FixtureCatalog { + fn from_fixture(fixture: BuiltFixture<'_>, name: &str) -> FixtureCase { + let articles = fixture + .articles + .iter() + .map(|(id, body, filename)| { + let (encoded, _) = + yenc_simd::encode_article(body, filename, 1, 1, 0, body.len() as u64); + ((*id).to_string(), encoded) + }) + .collect(); + FixtureCase { + name: name.into(), + xml: fixture.xml, + articles, + } + } + + pub fn single() -> FixtureCase { + let fixture = NzbFixture::new("catalog-single") + .add_file("catalog.txt", &[("catalog-single-1@test", b"catalog body")]) + .build(); + Self::from_fixture(fixture, "catalog-single") + } + + pub fn multi_segment() -> FixtureCase { + let fixture = NzbFixture::new("catalog-multi") + .add_file( + "catalog.bin", + &[ + ("catalog-multi-1@test", b"first"), + ("catalog-multi-2@test", b"second"), + ("catalog-multi-3@test", b"third"), + ], + ) + .build(); + Self::from_fixture(fixture, "catalog-multi") + } +} + impl<'a> NzbFixture<'a> { pub fn new(name: &str) -> Self { Self { diff --git a/crates/nzb-web/tests/harness_catalog.rs b/crates/nzb-web/tests/harness_catalog.rs new file mode 100644 index 00000000..58a12484 --- /dev/null +++ b/crates/nzb-web/tests/harness_catalog.rs @@ -0,0 +1,36 @@ +//! Catalog and profile invariants for deterministic integration tests. + +mod harness; + +use harness::nzb_fixture::FixtureCatalog; +use harness::{HarnessBuilder, ServerProfile}; +use nzb_nntp::testutil::MockConfig; + +type ProfileFactory = fn(ServerProfile) -> HarnessBuilder; + +#[tokio::test] +async fn catalog_cases_are_reproducible_and_profiles_are_explicit() { + let first = FixtureCatalog::single(); + let second = FixtureCatalog::single(); + assert_eq!(first.name, second.name); + assert_eq!(first.xml, second.xml); + assert_eq!(first.articles, second.articles); + assert_eq!(first.articles.len(), 1); + + let multi = FixtureCatalog::multi_segment(); + assert_eq!(multi.articles.len(), 3); + assert!(String::from_utf8_lossy(&multi.xml).contains("catalog.bin")); + + let profiles: [(&str, ProfileFactory); 6] = [ + ("happy", HarnessBuilder::happy_path), + ("retry", HarnessBuilder::retrying), + ("pause", HarnessBuilder::pause_resume), + ("cancel", HarnessBuilder::cancellation), + ("hopeless", HarnessBuilder::hopeless), + ("restart", HarnessBuilder::restart_recovery), + ]; + for (name, profile) in profiles { + let server = ServerProfile::start(name, MockConfig::default(), 1).await; + let _ = profile(server); + } +} diff --git a/crates/nzb-web/tests/harness_failure_matrix.rs b/crates/nzb-web/tests/harness_failure_matrix.rs new file mode 100644 index 00000000..180f272d --- /dev/null +++ b/crates/nzb-web/tests/harness_failure_matrix.rs @@ -0,0 +1,257 @@ +//! Table-driven NNTP failure and lifecycle invariants. + +mod harness; + +use std::collections::HashMap; +use std::path::PathBuf; +use std::time::Duration; + +use harness::nzb_fixture::{FixtureCatalog, NzbFixture}; +use harness::{HarnessBuilder, ServerProfile, yenc_articles}; +use nzb_nntp::testutil::MockConfig; +use nzb_web::nzb_core::db::Database; +use nzb_web::nzb_core::models::JobStatus; +use nzb_web::nzb_core::nzb_parser; + +#[tokio::test] +async fn transient_failures_recover_without_duplicate_completion() { + let body = b"recoverable"; + let fixture = NzbFixture::new("failure-matrix") + .add_file("payload.bin", &[("failure-matrix-1", body)]) + .build(); + let triples = fixture + .articles + .iter() + .map(|(id, bytes, name)| (*id, *bytes, name.as_str())) + .collect::>(); + let mut sequences = HashMap::new(); + sequences.insert( + "failure-matrix-1".to_string(), + std::collections::VecDeque::from([(400, "temporary failure".into())]), + ); + let server = ServerProfile::start( + "failure-matrix", + MockConfig { + articles: yenc_articles(&triples), + article_response_sequences: Some(std::sync::Arc::new(parking_lot::Mutex::new( + sequences, + ))), + ..MockConfig::default() + }, + 1, + ) + .await; + let engine = HarnessBuilder::new().with_server(server).build(); + let id = engine + .submit_nzb_xml("failure-matrix", fixture.xml) + .unwrap(); + assert!( + engine + .wait_for_status(&id, Duration::from_secs(10), &[JobStatus::Completed]) + .await + ); + let history = engine + .queue_manager + .history_get(&id) + .expect("history query") + .expect("completed history"); + assert_eq!(history.status, JobStatus::Completed); + assert_eq!(history.downloaded_bytes, history.total_bytes); +} + +#[tokio::test] +async fn cancellation_releases_connection_slots_and_removes_active_job() { + let fixture = FixtureCatalog::single(); + let server = ServerProfile::start( + "cancel", + MockConfig { + articles: fixture.mock_config().articles, + hang_after_command: Some("ARTICLE".into()), + ..MockConfig::default() + }, + 1, + ) + .await; + let engine = HarnessBuilder::cancellation(server) + .article_timeout(2) + .build(); + let id = engine + .submit_nzb_xml(&fixture.name, fixture.xml) + .expect("submit fixture"); + assert!( + engine + .wait_for_status(&id, Duration::from_secs(3), &[JobStatus::Downloading]) + .await + ); + + engine + .queue_manager + .remove_job(&id) + .expect("cancel active job"); + assert!(engine.job(&id).is_none()); + assert!( + tokio::time::timeout(Duration::from_secs(3), async { + loop { + if engine.queue_manager.connection_total() == 0 { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .is_ok(), + "cancelled jobs must release all NNTP slots" + ); +} + +#[tokio::test] +async fn pause_and_resume_preserve_progress_until_single_completion() { + let fixture = FixtureCatalog::multi_segment(); + let server = ServerProfile::start( + "pause-resume", + MockConfig { + articles: fixture.articles.clone(), + response_delay: Some(Duration::from_millis(120)), + ..MockConfig::default() + }, + 1, + ) + .await; + let engine = HarnessBuilder::pause_resume(server) + .article_timeout(5) + .build(); + let id = engine + .submit_nzb_xml(&fixture.name, fixture.xml) + .expect("submit fixture"); + assert!( + engine + .wait_for_status(&id, Duration::from_secs(5), &[JobStatus::Downloading]) + .await + ); + + engine + .queue_manager + .pause_job(&id) + .expect("pause active job"); + assert!( + engine + .wait_for_status(&id, Duration::from_secs(3), &[JobStatus::Paused]) + .await + ); + let paused = engine.job(&id).expect("paused job"); + tokio::time::sleep(Duration::from_millis(250)).await; + let still_paused = engine.job(&id).expect("paused job remains queued"); + assert_eq!(still_paused.status, JobStatus::Paused); + assert_eq!(still_paused.articles_downloaded, paused.articles_downloaded); + + engine + .queue_manager + .resume_job(&id) + .expect("resume paused job"); + assert!( + engine + .wait_for_status(&id, Duration::from_secs(10), &[JobStatus::Completed]) + .await + ); + assert_eq!(engine.history_status(&id), Some(JobStatus::Completed)); +} + +#[tokio::test] +async fn authentication_failure_waits_for_recovery_without_leaking_connections() { + let fixture = FixtureCatalog::single(); + let mut server = ServerProfile::start( + "auth-failure", + MockConfig { + articles: fixture.mock_config().articles, + auth_required: true, + fail_auth: true, + ..MockConfig::default() + }, + 1, + ) + .await; + server.config.username = Some("user".into()); + server.config.password = Some("pass".into()); + let engine = HarnessBuilder::hopeless(server).build(); + let id = engine + .submit_nzb_xml(&fixture.name, fixture.xml) + .expect("submit fixture"); + assert!( + engine + .wait_for(Duration::from_secs(8), |snapshot| { + snapshot.job(&id).is_some_and(|job| { + job.status == JobStatus::Downloading && job.error_message.is_some() + }) + }) + .await + ); + assert_eq!(engine.queue_manager.connected_snapshot()[0].1, 0); +} + +#[tokio::test] +async fn restart_restores_checkpoint_and_skips_completed_article() { + let fixture = NzbFixture::new("restart-matrix") + .add_file( + "restart.bin", + &[("restart-1", b"first"), ("restart-2", b"second")], + ) + .build(); + let triples = fixture + .articles + .iter() + .map(|(id, bytes, name)| (*id, *bytes, name.as_str())) + .collect::>(); + let mut overrides = HashMap::new(); + overrides.insert("restart-1".to_string(), 430); + let server = ServerProfile::start( + "restart", + MockConfig { + articles: yenc_articles(&triples), + article_response_overrides: overrides, + ..MockConfig::default() + }, + 1, + ) + .await; + let state = tempfile::tempdir().expect("restart state"); + let database_path = state.path().join("queue.sqlite"); + let incomplete_dir = state.path().join("incomplete"); + let complete_dir = state.path().join("complete"); + std::fs::create_dir_all(&incomplete_dir).unwrap(); + std::fs::create_dir_all(&complete_dir).unwrap(); + let mut job = nzb_parser::parse_nzb("restart-matrix", &fixture.xml).unwrap(); + job.status = JobStatus::Downloading; + job.work_dir = incomplete_dir.join(&job.id); + job.output_dir = complete_dir.join(&job.name); + let job_id = job.id.clone(); + let db = Database::open(&database_path).unwrap(); + db.queue_insert(&job).unwrap(); + db.queue_store_nzb_data(&job_id, &fixture.xml).unwrap(); + db.queue_store_job_data( + &job_id, + &serde_json::to_vec(&serde_json::json!({ + "files": {"restart.bin": [1]}, + "downloaded_bytes": 5, + "articles_downloaded": 1, + "articles_failed": 0, + "files_completed": 0 + })) + .unwrap(), + ) + .unwrap(); + + let engine = HarnessBuilder::restart_recovery(server) + .with_database_path(database_path) + .with_state_dir(PathBuf::from(state.path())) + .build(); + engine.queue_manager.restore_from_db().unwrap(); + + assert!( + engine + .wait_for_status(&job_id, Duration::from_secs(10), &[JobStatus::Completed]) + .await + ); + let history = engine.queue_manager.history_get(&job_id).unwrap().unwrap(); + assert_eq!(history.status, JobStatus::Completed); + assert_eq!(history.downloaded_bytes, history.total_bytes); +} diff --git a/crates/nzb-web/tests/workflow_fixtures.rs b/crates/nzb-web/tests/workflow_fixtures.rs new file mode 100644 index 00000000..a2e28a42 --- /dev/null +++ b/crates/nzb-web/tests/workflow_fixtures.rs @@ -0,0 +1,83 @@ +//! Local filesystem and feed fixture policy checks. + +use std::io::Write; +use std::path::Path; +use std::time::Duration; + +use flate2::Compression; +use flate2::write::GzEncoder; +use nzb_web::dir_watcher::DirWatcher; +use nzb_web::log_buffer::LogBuffer; +use nzb_web::nzb_core::db::Database; +use nzb_web::nzb_core::models::JobStatus; +use nzb_web::queue_manager::QueueManager; + +#[test] +fn gzip_fixture_is_deterministic_and_uses_the_watch_folder_suffix() { + let input = b""; + let mut encoder = GzEncoder::new(Vec::new(), Compression::default()); + encoder.write_all(input).unwrap(); + let compressed = encoder.finish().unwrap(); + let mut second_encoder = GzEncoder::new(Vec::new(), Compression::default()); + second_encoder.write_all(input).unwrap(); + let second_compressed = second_encoder.finish().unwrap(); + assert!(!compressed.is_empty()); + assert_eq!(compressed, second_compressed); + assert!( + Path::new("release.nzb.gz") + .to_string_lossy() + .ends_with(".nzb.gz") + ); +} + +#[tokio::test] +async fn existing_gzip_nzb_is_imported_once_and_moved_to_processed() { + let temp = tempfile::tempdir().unwrap(); + let watch_dir = temp.path().join("watch"); + let incomplete = temp.path().join("incomplete"); + let complete = temp.path().join("complete"); + std::fs::create_dir_all(&watch_dir).unwrap(); + let source = br#"alt.testwatched-1@test"#; + let mut encoder = GzEncoder::new(Vec::new(), Compression::default()); + encoder.write_all(source).unwrap(); + let compressed = encoder.finish().unwrap(); + let input = watch_dir.join("watched.nzb.gz"); + std::fs::write(&input, compressed).unwrap(); + + let queue = QueueManager::new( + Vec::new(), + Database::open_memory().unwrap(), + incomplete.clone(), + complete, + LogBuffer::default(), + 1, + Vec::new(), + 0, + 0, + false, + 5, + true, + true, + 100.0, + 2, + ); + let watcher = DirWatcher::new(watch_dir.clone(), queue.clone()); + let watcher_task = tokio::spawn(watcher.run()); + let imported = tokio::time::timeout(Duration::from_secs(2), async { + loop { + if queue.queue_size() == 1 { + break; + } + tokio::task::yield_now().await; + } + }) + .await; + watcher_task.abort(); + assert!( + imported.is_ok(), + "watch folder did not enqueue the gzip NZB" + ); + assert!(watch_dir.join("processed/watched.nzb.gz").exists()); + assert!(!input.exists()); + assert_eq!(queue.get_jobs()[0].status, JobStatus::Downloading); +} diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 7cc05738..237f1ed0 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -54,6 +54,7 @@ useful where a local Docker environment is available: ./ci/run check ./ci/run test ./ci/run clippy +./ci/run harness ./ci/run frontend-test ./ci/run e2e ./ci/run build-image rustnzb:local @@ -70,3 +71,39 @@ frontend build directories and must not be committed. - Browser journeys and Playwright coverage live in `e2e/`. - The deterministic NNTP fixture is in `crates/mock-nntp-server/`. - `benchnzb/` is a benchmark harness, not a substitute for correctness tests. + +### Deterministic compatibility harness + +The compatibility test layers are intentionally local and deterministic: + +```bash +cargo test -p nzb-web --tests --locked +cargo test -p rustnzb --tests --locked +cargo test -p nzb-postproc --tests --locked +``` + +The reusable fixtures live under `crates/nzb-web/tests/harness/` and the +checked-in response contract lives under +`crates/nzb-web/tests/fixtures/`. Update a golden only when the +wire contract changes, preserve dynamic `$type:*` markers, and include a test +that proves the changed mode's complete response shape. NNTP, URL, feed, and +watch-folder tests use loopback fixtures or temporary directories; correctness +tests must never contact a live provider. Every fixture owns its temporary +state and drops it at test completion. Tests that depend on an implementation +not yet present should be marked as an explicit implementation-gated plan +rather than weakened to pass. + +The harness gate covers the compatibility and failure matrix as one explicit +review target. The matrix is intentionally split by ownership: + +| Area | Current deterministic coverage | Implementation-gated extension | +| --- | --- | --- | +| API envelopes | Read modes, uploads, errors, filtering, paging, and golden fixtures | Add a fixture whenever a supported response field changes | +| NNTP lifecycle | Retry, pause/resume, cancellation, authentication failure, failover, and restart checkpoints | Add provider-specific protocol cases only when the production state machine gains them | +| URL and feed input | Scheme/address validation, redirect resistance, body limits, feed filters, and duplicate suppression | Add parser fixtures for newly accepted feed formats | +| Post-processing | Nested archives, path safety, cleanup, password diagnostics, repair, and resource limits | Tar and hardlink semantics remain explicit implementation gates until supported | +| Import and watch workflows | Multipart import, URL import, watch-folder ingestion, and compressed input limits | Add a workflow fixture before exposing a new ingestion source | + +Focused gates are suitable for local iteration; the full workspace commands +above remain the review gate. Run a focused test three times when changing +timing-sensitive queue behavior to catch flakes before broadening the loop. From 01a6af24e12c708b4ed15fdb3809aec61f1f5d9b Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:04:26 +0000 Subject: [PATCH 2/5] feat: complete compatibility backlog --- Cargo.lock | 38 +- Cargo.toml | 5 +- apps/rustnzb/config.example.toml | 10 + apps/rustnzb/src/handlers.rs | 101 +- apps/rustnzb/src/server.rs | 15 +- apps/rustnzb/tests/api_contracts.rs | 76 ++ apps/rustnzb/tests/api_mock_download_test.rs | 19 + apps/rustnzb/tests/e2e_full_pipeline.rs | 21 + benchnzb/METHODOLOGY.md | 16 + benchnzb/OVERVIEW.md | 4 + benchnzb/src/clients/rustnzb.rs | 60 +- benchnzb/src/report.rs | 10 +- benchnzb/src/runner.rs | 81 +- benchnzb/src/stress.rs | 8 +- crates/nzb-core/Cargo.toml | 2 +- crates/nzb-core/src/config.rs | 49 + crates/nzb-core/src/db.rs | 42 +- crates/nzb-core/src/lib.rs | 1 + crates/nzb-core/src/models.rs | 3 + crates/nzb-core/src/path.rs | 58 ++ crates/nzb-core/src/sabnzbd_import.rs | 3 + crates/nzb-dispatch/Cargo.toml | 2 +- crates/nzb-dispatch/src/download_engine.rs | 18 +- crates/nzb-postproc/Cargo.toml | 5 +- crates/nzb-postproc/src/detect.rs | 14 +- crates/nzb-postproc/src/lib.rs | 13 +- crates/nzb-postproc/src/pipeline.rs | 127 ++- crates/nzb-postproc/src/unpack.rs | 253 ++++- crates/nzb-web/Cargo.toml | 4 +- crates/nzb-web/src/auth.rs | 60 +- crates/nzb-web/src/queue_manager.rs | 928 +++++++++++++++++-- crates/nzb-web/src/rss_monitor.rs | 18 + crates/nzb-web/src/sabnzbd_compat.rs | 79 +- crates/nzb-web/src/startup.rs | 8 + 34 files changed, 1931 insertions(+), 220 deletions(-) create mode 100644 crates/nzb-core/src/path.rs diff --git a/Cargo.lock b/Cargo.lock index e7c90bf5..6209acb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -773,6 +773,16 @@ dependencies = [ "uuid", ] +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -1763,7 +1773,7 @@ dependencies = [ [[package]] name = "nzb-core" -version = "0.2.16" +version = "0.2.17" dependencies = [ "anyhow", "chrono", @@ -1864,13 +1874,14 @@ dependencies = [ [[package]] name = "nzb-postproc" -version = "0.2.6" +version = "0.2.7" dependencies = [ "anyhow", "md-5 0.11.0", "nzb-core", "opentelemetry", "rust-par2", + "tar", "tempfile", "thiserror 2.0.20", "tokio", @@ -1881,7 +1892,7 @@ dependencies = [ [[package]] name = "nzb-web" -version = "0.4.20" +version = "0.4.21" dependencies = [ "anyhow", "arc-swap", @@ -3220,6 +3231,17 @@ dependencies = [ "libc", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "tempfile" version = "3.27.0" @@ -4232,6 +4254,16 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ffae5123b2d3fc086436f8834ae3ab053a283cfac8fe0a0b8eaae044768a4c4" +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix", +] + [[package]] name = "yenc-simd" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index a5443778..e050052b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,6 +78,7 @@ rustls-pki-types = "1" quick-xml = "0.41" regex = "1" walkdir = "2" +tar = "0.4" flate2 = "1" tokio-socks = "0.5" notify = "7" @@ -87,11 +88,11 @@ unicode-normalization = "0.1" # Shared NZB crates nzb-web = { version = "0.4.20", path = "crates/nzb-web", features = ["groups-db"] } nzb-nntp = { version = "0.2.22", path = "crates/nzb-nntp" } -nzb-core = { version = "0.2.16", path = "crates/nzb-core", features = ["groups-db"] } +nzb-core = { version = "0.2.17", path = "crates/nzb-core", features = ["groups-db"] } nzb-decode = { version = "0.1.2", path = "crates/nzb-decode" } nzb-news = { version = "0.1.12", path = "crates/nzb-news" } nzb-dispatch = { version = "0.2.6", path = "crates/nzb-dispatch" } -nzb-postproc = { version = "0.2.6", path = "crates/nzb-postproc" } +nzb-postproc = { version = "0.2.7", path = "crates/nzb-postproc" } mock-nntp-server = { path = "crates/mock-nntp-server" } rust-par2 = { version = "0.1.3" } yenc-simd = { version = "0.1.1" } diff --git a/apps/rustnzb/config.example.toml b/apps/rustnzb/config.example.toml index c4a26916..843304ac 100644 --- a/apps/rustnzb/config.example.toml +++ b/apps/rustnzb/config.example.toml @@ -10,6 +10,8 @@ cache_size = 524288000 # 500 MB log_level = "info" # log_file = "data/rustnzb.log" # history_retention = 100 # Number of NZBs to keep in history (omit or 0 = keep all) +# auto_sort_remaining_pct = false # Keep queued items ordered by remaining work +# rss_downloaded_item_expiry_days = 30 # Remove downloaded RSS records after this many days # Independent post-processing jobs overlap downloads. Separate repair and # extraction gates allow those stages to overlap safely across different jobs. # These limits are applied at process start. @@ -18,6 +20,12 @@ max_repair_workers = 1 max_extract_workers = 1 direct_unpack = true # Extract RAR volumes while downloading when unrar is available max_nested_archive_depth = 5 # 0=outer archive only; protects against unbounded nesting +# Post-processing hooks run without a shell, with a bounded timeout and output capture. +# scripts_dir = "/data/scripts" +# script_success = "on-success.sh" +# script_failure = "on-failure.sh" +script_timeout_secs = 300 +script_max_output_bytes = 1048576 # NNTP servers — add as many as needed, ordered by priority # Use the web UI "Servers" tab to add/edit servers, or uncomment below: @@ -64,6 +72,8 @@ post_processing = 3 # 0=none, 1=repair, 2=unpack, 3=repair+unpack name = "movies" output_dir = "movies" post_processing = 3 +# cleanup_patterns = ["*.nfo", "sample/*"] +# unwanted_extensions = [".sfv", ".jpg"] [[categories]] name = "tv" diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 0c5f44cd..e1f4f208 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -93,6 +93,17 @@ pub struct MoveJobBody { pub position: usize, } +#[derive(Deserialize)] +pub struct SortQueueBody { + /// Sort in ascending remaining percentage order when true. + #[serde(default = "default_sort_ascending")] + pub ascending: bool, +} + +fn default_sort_ascending() -> bool { + true +} + #[derive(Deserialize, Serialize)] pub struct HistoryRetentionBody { pub retention: Option, @@ -372,7 +383,9 @@ fn enqueue_nzb( let qm = &state.queue_manager; job.work_dir = qm.incomplete_dir().join(&job.id); - job.output_dir = qm.complete_dir().join(&job.category).join(&job.name); + job.output_dir = qm + .output_dir_for(&job.category, &job.name) + .map_err(ApiError::from)?; std::fs::create_dir_all(&job.work_dir).map_err(|e| { ApiError::from(anyhow::anyhow!( @@ -456,6 +469,17 @@ pub async fn h_queue_set_priority( Ok(Json(SimpleResponse { status: true })) } +/// POST /api/queue/sort -- Stable sort by remaining work percentage. +pub async fn h_queue_sort( + State(state): State>, + Json(body): Json, +) -> Result, ApiError> { + state + .queue_manager + .sort_by_remaining_percentage(body.ascending); + Ok(Json(SimpleResponse { status: true })) +} + // --------------------------------------------------------------------------- // Add URL handler // --------------------------------------------------------------------------- @@ -683,15 +707,11 @@ pub async fn h_history_retry( .map_err(ApiError::from)? .ok_or_else(|| ApiError::from(anyhow::anyhow!("No NZB data stored for this entry")))?; - // Re-parse the NZB - let mut job = nzb_parser::parse_nzb(&entry.name, &nzb_data).map_err(ApiError::from)?; - - job.category = entry.category.clone(); - - // Set working directories let qm = &state.queue_manager; - job.work_dir = qm.incomplete_dir().join(&job.id); - job.output_dir = qm.complete_dir().join(&job.category).join(&job.name); + let retry_data = qm.history_get_retry_data(&id).map_err(ApiError::from)?; + let job = qm + .prepare_retry_job(&entry, &nzb_data, retry_data.as_deref()) + .map_err(ApiError::from)?; std::fs::create_dir_all(&job.work_dir).map_err(|e| { ApiError::from(anyhow::anyhow!( @@ -1216,7 +1236,7 @@ pub async fn h_disk_guards_get( })) } -/// PUT /api/config/disk-guards -- Update disk guard settings (persisted; restart to apply). +/// PUT /api/config/disk-guards -- Update disk guard settings. pub async fn h_disk_guards_set( State(state): State>, Json(body): Json, @@ -1224,6 +1244,9 @@ pub async fn h_disk_guards_set( let mut config = (*state.config()).clone(); config.general.min_free_space_bytes = body.min_free_space_bytes; config.general.abort_hopeless = body.abort_hopeless; + state + .queue_manager + .set_min_free_space(body.min_free_space_bytes); state.update_config(config).map_err(ApiError::from)?; Ok(Json(SimpleResponse { status: true })) } @@ -1359,11 +1382,10 @@ pub async fn h_rss_item_download( } job.work_dir = state.queue_manager.incomplete_dir().join(&job.id); - job.output_dir = if let Some(ref cat) = item.category { - state.queue_manager.complete_dir().join(cat).join(&job.name) - } else { - state.queue_manager.complete_dir().join(&job.name) - }; + job.output_dir = state + .queue_manager + .output_dir_for(&job.category, &job.name) + .map_err(ApiError::from)?; std::fs::create_dir_all(&job.work_dir).map_err(|e| { ApiError::from(anyhow::anyhow!( @@ -1509,6 +1531,13 @@ pub struct UpdateGeneralBody { pub max_extract_workers: Option, pub history_retention: Option>, pub rss_history_limit: Option>, + pub auto_sort_remaining_pct: Option, + pub rss_downloaded_item_expiry_days: Option>, + pub scripts_dir: Option, + pub script_success: Option, + pub script_failure: Option, + pub script_timeout_secs: Option, + pub script_max_output_bytes: Option, } /// PUT /api/config/general -- Update general settings. @@ -1565,6 +1594,48 @@ pub async fn h_general_update( let _ = state.queue_manager.rss_items_prune(limit); } } + if let Some(enabled) = body.auto_sort_remaining_pct { + state.queue_manager.set_auto_sort_remaining_pct(enabled); + config.general.auto_sort_remaining_pct = enabled; + } + if let Some(days) = body.rss_downloaded_item_expiry_days { + config.general.rss_downloaded_item_expiry_days = days; + } + if let Some(directory) = body.scripts_dir { + config.general.scripts_dir = if directory.is_empty() { + None + } else { + Some(directory.into()) + }; + } + if let Some(script) = body.script_success { + config.general.script_success = if script.is_empty() { + None + } else { + Some(script.into()) + }; + } + if let Some(script) = body.script_failure { + config.general.script_failure = if script.is_empty() { + None + } else { + Some(script.into()) + }; + } + if let Some(timeout) = body.script_timeout_secs { + config.general.script_timeout_secs = timeout.max(1); + } + if let Some(max_output) = body.script_max_output_bytes { + config.general.script_max_output_bytes = max_output; + } + + state.queue_manager.set_postproc_scripts( + config.general.scripts_dir.clone(), + config.general.script_success.clone(), + config.general.script_failure.clone(), + config.general.script_timeout_secs, + config.general.script_max_output_bytes, + ); state.update_config(config).map_err(ApiError::from)?; Ok(Json(SimpleResponse { status: true })) diff --git a/apps/rustnzb/src/server.rs b/apps/rustnzb/src/server.rs index ee32bd2c..b6c6b78f 100644 --- a/apps/rustnzb/src/server.rs +++ b/apps/rustnzb/src/server.rs @@ -161,6 +161,7 @@ pub fn build_router(state: Arc) -> Router { .route("/queue/{id}/pause", post(handlers::h_queue_pause)) .route("/queue/{id}/resume", post(handlers::h_queue_resume)) .route("/queue/{id}/move", post(handlers::h_queue_move)) + .route("/queue/sort", post(handlers::h_queue_sort)) .route("/queue/{id}/priority", put(handlers::h_queue_set_priority)) .route( "/queue/{id}/category", @@ -321,9 +322,19 @@ pub fn build_router(state: Arc) -> Router { return Err(ApiError::unauthorized()); } - // If no credentials configured, allow all requests (setup_required state) + // Before first-boot setup, only the setup endpoints may be + // reached. Treating the entire protected router as public + // would expose queue/config mutation while the wizard is open. if !credential_store.has_credentials() { - return Ok(next.run(request).await); + let path = request.uri().path(); + if path == "/setup/status" + || path.starts_with("/setup/") + || path == "/api/setup/status" + || path.starts_with("/api/setup/") + { + return Ok(next.run(request).await); + } + return Err(ApiError::unauthorized()); } // Try Bearer token first diff --git a/apps/rustnzb/tests/api_contracts.rs b/apps/rustnzb/tests/api_contracts.rs index f9843e13..d831fc89 100644 --- a/apps/rustnzb/tests/api_contracts.rs +++ b/apps/rustnzb/tests/api_contracts.rs @@ -157,10 +157,79 @@ async fn protected_routes_reject_missing_credentials_and_preserve_auth_contracts ); } +#[tokio::test] +async fn first_boot_only_exposes_setup_and_setup_is_single_use() { + let app = start_app(false).await; + let client = reqwest::Client::new(); + + assert_eq!( + client + .get(format!("{}/api/status", app.base_url)) + .send() + .await + .unwrap() + .status(), + reqwest::StatusCode::UNAUTHORIZED + ); + assert_eq!( + client + .get(format!("{}/api/setup/status", app.base_url)) + .send() + .await + .unwrap() + .status(), + reqwest::StatusCode::OK + ); + + let setup = client + .post(format!("{}/api/auth/setup", app.base_url)) + .json(&serde_json::json!({"username":"owner","password":"secret"})) + .send() + .await + .unwrap(); + assert_eq!(setup.status(), reqwest::StatusCode::OK); + let access = setup.json::().await.unwrap()["access_token"] + .as_str() + .unwrap() + .to_string(); + + assert_eq!( + client + .get(format!("{}/api/status", app.base_url)) + .bearer_auth(&access) + .send() + .await + .unwrap() + .status(), + reqwest::StatusCode::OK + ); + assert_eq!( + client + .post(format!("{}/api/auth/setup", app.base_url)) + .json(&serde_json::json!({"username":"other","password":"secret"})) + .send() + .await + .unwrap() + .status(), + reqwest::StatusCode::FORBIDDEN + ); +} + #[tokio::test] async fn config_routes_validate_duplicates_and_persist_successful_updates() { let app = start_app(false).await; let client = reqwest::Client::new(); + let setup = client + .post(format!("{}/api/auth/setup", app.base_url)) + .json(&serde_json::json!({"username":"owner","password":"secret"})) + .send() + .await + .unwrap(); + assert_eq!(setup.status(), reqwest::StatusCode::OK); + let access = setup.json::().await.unwrap()["access_token"] + .as_str() + .unwrap() + .to_string(); let server = serde_json::json!({ "id":"", "name":"Primary", "host":" news.example.test ", "port":563, "ssl":true, "ssl_verify":true, "username":"", "password":"", "connections":8, @@ -171,6 +240,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .post(format!("{}/api/config/servers", app.base_url)) + .bearer_auth(&access) .json(&server) .send() .await @@ -180,6 +250,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { ); let servers = client .get(format!("{}/api/config/servers", app.base_url)) + .bearer_auth(&access) .send() .await .unwrap() @@ -195,6 +266,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .post(format!("{}/api/config/categories", app.base_url)) + .bearer_auth(&access) .json(&category) .send() .await @@ -205,6 +277,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .post(format!("{}/api/config/categories", app.base_url)) + .bearer_auth(&access) .json(&category) .send() .await @@ -217,6 +290,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .post(format!("{}/api/config/rss-feeds", app.base_url)) + .bearer_auth(&access) .json(&feed) .send() .await @@ -227,6 +301,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .put(format!("{}/api/config/speed-limit", app.base_url)) + .bearer_auth(&access) .json(&serde_json::json!({"speed_limit_bps":1234})) .send() .await @@ -237,6 +312,7 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { assert_eq!( client .get(format!("{}/api/config/speed-limit", app.base_url)) + .bearer_auth(&access) .send() .await .unwrap() diff --git a/apps/rustnzb/tests/api_mock_download_test.rs b/apps/rustnzb/tests/api_mock_download_test.rs index db557e0e..cae3bfa9 100644 --- a/apps/rustnzb/tests/api_mock_download_test.rs +++ b/apps/rustnzb/tests/api_mock_download_test.rs @@ -28,6 +28,20 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let app = start_test_server(vec![config]).await; let client = reqwest::Client::new(); + let setup = client + .post(format!("{}/api/auth/setup", app.base_url)) + .json(&serde_json::json!({ + "username": "mock-test", + "password": "mock-test-password" + })) + .send() + .await + .expect("auth setup failed"); + assert_eq!(setup.status(), 200); + let access = setup.json::().await.unwrap()["access_token"] + .as_str() + .expect("auth setup should return an access token") + .to_string(); let part = reqwest::multipart::Part::bytes(fixture.xml.clone()) .file_name("mock-download.nzb") @@ -37,6 +51,7 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let resp = client .post(format!("{}/api/queue/add", app.base_url)) + .bearer_auth(&access) .multipart(form) .send() .await @@ -50,6 +65,7 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let history_entry = loop { let status: serde_json::Value = client .get(format!("{}/api/status", app.base_url)) + .bearer_auth(&access) .send() .await .expect("status request failed") @@ -66,6 +82,7 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let history: serde_json::Value = client .get(format!("{}/api/history?limit=10", app.base_url)) + .bearer_auth(&access) .send() .await .expect("history request failed") @@ -96,6 +113,7 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let idle_status: serde_json::Value = client .get(format!("{}/api/status", app.base_url)) + .bearer_auth(&access) .send() .await .expect("idle status request failed") @@ -113,6 +131,7 @@ async fn upload_nzb_downloads_via_mock_server_and_reaches_history() { let queue_after_history: serde_json::Value = client .get(format!("{}/api/queue", app.base_url)) + .bearer_auth(&access) .send() .await .expect("queue request after history failed") diff --git a/apps/rustnzb/tests/e2e_full_pipeline.rs b/apps/rustnzb/tests/e2e_full_pipeline.rs index ec498efb..2f6d2619 100644 --- a/apps/rustnzb/tests/e2e_full_pipeline.rs +++ b/apps/rustnzb/tests/e2e_full_pipeline.rs @@ -15,10 +15,25 @@ async fn test_upload_nzb_and_verify_queue() { let app = start_test_server(Vec::new()).await; let client = reqwest::Client::new(); let base_url = &app.base_url; + let setup = client + .post(format!("{}/api/auth/setup", base_url)) + .json(&serde_json::json!({ + "username": "pipeline-test", + "password": "pipeline-test-password" + })) + .send() + .await + .expect("auth setup failed"); + assert_eq!(setup.status(), 200); + let access = setup.json::().await.unwrap()["access_token"] + .as_str() + .expect("auth setup should return an access token") + .to_string(); // 1. Verify server is up: GET /api/status let resp = client .get(format!("{}/api/status", base_url)) + .bearer_auth(&access) .send() .await .expect("Failed to reach server"); @@ -30,6 +45,7 @@ async fn test_upload_nzb_and_verify_queue() { // 2. Verify queue is initially empty let resp = client .get(format!("{}/api/queue", base_url)) + .bearer_auth(&access) .send() .await .unwrap(); @@ -62,6 +78,7 @@ async fn test_upload_nzb_and_verify_queue() { "{}/api/queue/add?category=test&priority=1", base_url )) + .bearer_auth(&access) .multipart(form) .send() .await @@ -76,6 +93,7 @@ async fn test_upload_nzb_and_verify_queue() { // 4. Verify job appears in queue let resp = client .get(format!("{}/api/queue", base_url)) + .bearer_auth(&access) .send() .await .unwrap(); @@ -198,6 +216,7 @@ async fn test_upload_nzb_and_verify_queue() { // 11. Verify now 2 jobs in queue let resp = client .get(format!("{}/api/queue", base_url)) + .bearer_auth(&access) .send() .await .unwrap(); @@ -242,6 +261,7 @@ async fn test_upload_nzb_and_verify_queue() { let first_job_id = queue["jobs"][0]["id"].as_str().unwrap(); let resp = client .delete(format!("{}/api/queue/{}", base_url, first_job_id)) + .bearer_auth(&access) .send() .await .unwrap(); @@ -251,6 +271,7 @@ async fn test_upload_nzb_and_verify_queue() { // 15. Verify the remaining queue (may have 1 job or 0 depending on timing) let resp = client .get(format!("{}/api/queue", base_url)) + .bearer_auth(&access) .send() .await .unwrap(); diff --git a/benchnzb/METHODOLOGY.md b/benchnzb/METHODOLOGY.md index eb580937..ee7a2385 100644 --- a/benchnzb/METHODOLOGY.md +++ b/benchnzb/METHODOLOGY.md @@ -84,6 +84,22 @@ Both clients are configured for raw download throughput with post-processing dis | Speed limit | None | None | | NNTP server | synth-nntp (shared) | synth-nntp (shared) | +### Cache and connection sweep + +The v1 runner accepts two environment variables for controlled RustNZB +experiments: + +```bash +RUSTNZB_BENCH_CACHE_BYTES=268435456 \ +RUSTNZB_BENCH_CONNECTIONS=8 \ +./run.sh --scenarios verify +``` + +The runner applies the cache setting through the RustNZB configuration API, +updates the benchmark NNTP server connection count on every run, and records +both values in each JSON and CSV result. This keeps comparisons reproducible +when direct-write buffering or connection defaults change. + ## Known Differences The following differences between clients are inherent to their architecture and are not corrected for in the test: diff --git a/benchnzb/OVERVIEW.md b/benchnzb/OVERVIEW.md index 01027cb3..9aa02f4b 100644 --- a/benchnzb/OVERVIEW.md +++ b/benchnzb/OVERVIEW.md @@ -31,6 +31,10 @@ metadata. Runs SABnzbd and rustnzb sequentially against the same NZB, using a file-backed mock NNTP server. Measures download speed, post-processing time, CPU, memory, network, and disk I/O. +RustNZB cache and NNTP connection settings can be swept without editing the +fixture config. Set `RUSTNZB_BENCH_CACHE_BYTES` and +`RUSTNZB_BENCH_CONNECTIONS`; both values are stored with every result row. + ### How it works 1. `run.sh` seeds configs, launches Docker Compose (4 containers) diff --git a/benchnzb/src/clients/rustnzb.rs b/benchnzb/src/clients/rustnzb.rs index 35df3410..3cbf3b1b 100644 --- a/benchnzb/src/clients/rustnzb.rs +++ b/benchnzb/src/clients/rustnzb.rs @@ -9,6 +9,7 @@ pub struct StatusSummary { pub struct RustnzbClient { url: String, + access_token: Option, http: reqwest::Client, } @@ -16,10 +17,55 @@ impl RustnzbClient { pub fn new(url: &str) -> Self { Self { url: url.trim_end_matches('/').to_string(), + access_token: None, http: reqwest::Client::new(), } } + pub fn get(&self, url: String) -> reqwest::RequestBuilder { + self.authorize(self.http.get(url)) + } + + pub fn post(&self, url: String) -> reqwest::RequestBuilder { + self.authorize(self.http.post(url)) + } + + pub fn put(&self, url: String) -> reqwest::RequestBuilder { + self.authorize(self.http.put(url)) + } + + fn authorize(&self, request: reqwest::RequestBuilder) -> reqwest::RequestBuilder { + match &self.access_token { + Some(token) => request.bearer_auth(token), + None => request, + } + } + + /// Create an isolated first-boot account and retain its short-lived token. + /// Benchmark state is disposable, so no credential or API key is stored in + /// the repository or passed through the benchmark configuration. + pub async fn initialize_auth(&mut self) -> Result<()> { + let suffix = rand::random::(); + let response: serde_json::Value = self + .http + .post(format!("{}/api/auth/setup", self.url)) + .json(&serde_json::json!({ + "username": format!("benchmark-{suffix:x}"), + "password": format!("benchmark-{suffix:032x}"), + })) + .timeout(std::time::Duration::from_secs(15)) + .send() + .await? + .error_for_status()? + .json() + .await?; + self.access_token = response["access_token"].as_str().map(str::to_string); + if self.access_token.is_none() { + anyhow::bail!("rustnzb auth setup did not return an access token"); + } + Ok(()) + } + pub async fn add_nzb(&self, data: &[u8], filename: &str) -> Result<()> { let part = reqwest::multipart::Part::bytes(data.to_vec()) .file_name(filename.to_string()) @@ -27,7 +73,6 @@ impl RustnzbClient { let form = reqwest::multipart::Form::new().part("nzbfile", part); let resp = self - .http .post(format!("{}/api/queue/add", self.url)) .multipart(form) .timeout(std::time::Duration::from_secs(30)) @@ -42,7 +87,6 @@ impl RustnzbClient { pub async fn all_finished(&self) -> Result { // Queue empty = download phase done, but check history for post-processing let queue: serde_json::Value = self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -62,7 +106,6 @@ impl RustnzbClient { // Check history let history: serde_json::Value = self - .http .get(format!("{}/api/history", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -86,7 +129,6 @@ impl RustnzbClient { /// successful benchmark result. pub async fn terminal_status(&self) -> Result> { let queue: serde_json::Value = self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -101,7 +143,6 @@ impl RustnzbClient { } let history: serde_json::Value = self - .http .get(format!("{}/api/history", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -117,7 +158,6 @@ impl RustnzbClient { pub async fn progress_fraction(&self) -> f64 { let queue: serde_json::Value = match self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -152,7 +192,6 @@ impl RustnzbClient { pub async fn download_speed(&self) -> f64 { let queue: serde_json::Value = match self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -171,7 +210,6 @@ impl RustnzbClient { pub async fn get_stage_timing(&self) -> Result { let history: serde_json::Value = self - .http .get(format!("{}/api/history", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -219,7 +257,6 @@ impl RustnzbClient { /// Fetch internal metrics from the history API after job completion. pub async fn get_internal_metrics(&self) -> Result { let history: serde_json::Value = self - .http .get(format!("{}/api/history", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -289,6 +326,7 @@ impl RustnzbClient { pub fn clone_client(&self) -> Self { Self { url: self.url.clone(), + access_token: self.access_token.clone(), http: self.http.clone(), } } @@ -296,7 +334,6 @@ impl RustnzbClient { /// Get queue size (number of jobs in queue). pub async fn queue_size(&self) -> Result { let queue: serde_json::Value = self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -310,7 +347,6 @@ impl RustnzbClient { /// Get status summary from the API. pub async fn get_status(&self) -> Result { let status: serde_json::Value = self - .http .get(format!("{}/api/status", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -319,7 +355,6 @@ impl RustnzbClient { .await?; let queue: serde_json::Value = self - .http .get(format!("{}/api/queue", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() @@ -343,7 +378,6 @@ impl RustnzbClient { /// Get count of history entries. pub async fn history_count(&self) -> Result { let history: serde_json::Value = self - .http .get(format!("{}/api/history", self.url)) .timeout(std::time::Duration::from_secs(10)) .send() diff --git a/benchnzb/src/report.rs b/benchnzb/src/report.rs index c1a2595c..d6a02328 100644 --- a/benchnzb/src/report.rs +++ b/benchnzb/src/report.rs @@ -30,7 +30,7 @@ pub fn write_csv( ) -> Result<()> { let path = dir.join(format!("benchmark_{timestamp}.csv")); let mut out = String::from( - "scenario,test_type,client,total_bytes,total_sec,download_sec,par2_sec,unpack_sec,\ + "scenario,test_type,client,total_bytes,direct_write_cache_bytes,nntp_connections,total_sec,download_sec,par2_sec,unpack_sec,\ avg_speed_mbps,peak_speed_mbps,cpu_avg,cpu_peak,mem_avg_mb,mem_peak_mb,\ net_rx_avg_mbps,net_rx_peak_mbps,disk_write_avg_mbps,disk_write_peak_mbps,\ iowait_avg,iowait_peak,\ @@ -49,13 +49,15 @@ pub fn write_csv( (0.0, 0, 0) }; out.push_str(&format!( - "{},{},{},{},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},\ + "{},{},{},{},{},{},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},{:.2},\ {:.2},{:.2},{:.2},{:.2},{:.4},{:.4},\ - {:.2},{},{},{:?},{},{},{},{},{},{},{}\n", + {:.2},{},{},{:?},{},{},{},{},{},{},{},{}\n", r.scenario, r.test_type, r.client, r.total_bytes, + r.direct_write_cache_bytes, + r.nntp_connections, r.total_sec, r.download_sec, r.par2_sec, @@ -75,7 +77,7 @@ pub fn write_csv( int_dl, int_art_ok, int_art_fail, - r.outcome, + format!("{:?}", r.outcome), r.payload_verified, r.peak_work_dir_bytes, r.fixture_metrics.payload_bytes_served, diff --git a/benchnzb/src/runner.rs b/benchnzb/src/runner.rs index aedd0d95..aa2713d8 100644 --- a/benchnzb/src/runner.rs +++ b/benchnzb/src/runner.rs @@ -46,6 +46,12 @@ pub struct ClientResult { pub scenario_description: String, pub test_type: String, pub total_bytes: u64, + /// Runtime knobs recorded with every result so cache/connection changes + /// cannot be mistaken for an application improvement. + #[serde(default = "default_benchmark_cache_bytes")] + pub direct_write_cache_bytes: u64, + #[serde(default = "default_benchmark_connections")] + pub nntp_connections: usize, pub outcome: BenchmarkOutcome, pub payload_verified: bool, pub peak_work_dir_bytes: u64, @@ -180,7 +186,7 @@ pub async fn run(scenario_selector: String, data_dir: PathBuf, results_dir: Path // Wait for services tracing::info!("Waiting for services..."); - let rnzb = RustnzbClient::new(config::RUSTNZB_API); + let mut rnzb = RustnzbClient::new(config::RUSTNZB_API); wait_for_service("mock-nntp", "http://mock-nntp:8080/health", 120).await?; let sab = SabnzbdClient::from_runtime_config(config::SABNZBD_API, &docker_client).await?; @@ -188,11 +194,13 @@ pub async fn run(scenario_selector: String, data_dir: PathBuf, results_dir: Path sab.configure_mock_server().await?; wait_for_service( "rustnzb", - &format!("{}/api/status", config::RUSTNZB_API), + &format!("{}/api/health", config::RUSTNZB_API), 120, ) .await?; - bootstrap_rustnzb_mock_server().await?; + rnzb.initialize_auth().await?; + bootstrap_rustnzb_mock_server(&rnzb).await?; + configure_rustnzb_benchmark_settings(&rnzb).await?; // Resolve container IDs for metrics and log capture metrics.resolve_container_id("sabnzbd").await; @@ -378,6 +386,8 @@ async fn run_client( scenario_description: sc.description.clone(), test_type: sc.test_type.to_string(), total_bytes: sc.total_size, + direct_write_cache_bytes: runtime_benchmark_settings().0, + nntp_connections: runtime_benchmark_settings().1, outcome: BenchmarkOutcome::SubmissionFailed, payload_verified: false, peak_work_dir_bytes: 0, @@ -583,6 +593,38 @@ async fn run_client( result } +fn runtime_benchmark_settings() -> (u64, usize) { + let cache = std::env::var("RUSTNZB_BENCH_CACHE_BYTES") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or_else(default_benchmark_cache_bytes); + let connections = std::env::var("RUSTNZB_BENCH_CONNECTIONS") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or_else(default_benchmark_connections); + (cache, connections) +} + +fn default_benchmark_cache_bytes() -> u64 { + 524_288_000 +} + +fn default_benchmark_connections() -> usize { + 20 +} + +async fn configure_rustnzb_benchmark_settings(client: &RustnzbClient) -> Result<()> { + let (cache_size, _) = runtime_benchmark_settings(); + client + .put(format!("{}/api/config/general", config::RUSTNZB_API)) + .json(&serde_json::json!({"cache_size": cache_size})) + .timeout(std::time::Duration::from_secs(15)) + .send() + .await? + .error_for_status()?; + Ok(()) +} + async fn reset_fixture_stats() -> Result<()> { reqwest::Client::new() .post("http://mock-nntp:8080/reset-stats") @@ -596,21 +638,31 @@ async fn reset_fixture_stats() -> Result<()> { /// the service is ready. This avoids relying on image/bootstrap timing for a /// bind-mounted TOML file and verifies the workload has a real server before /// timing any job. -async fn bootstrap_rustnzb_mock_server() -> Result<()> { - let http = reqwest::Client::new(); +async fn bootstrap_rustnzb_mock_server(client: &RustnzbClient) -> Result<()> { let endpoint = format!("{}/api/config/servers", config::RUSTNZB_API); - let existing: Vec = http - .get(&endpoint) + let existing: Vec = client + .get(endpoint.clone()) .timeout(std::time::Duration::from_secs(15)) .send() .await? .error_for_status()? .json() .await?; - if !existing + let (_, connections) = runtime_benchmark_settings(); + if let Some(existing_server) = existing .iter() - .any(|server| server["id"].as_str() == Some("benchmark-mock")) + .find(|server| server["id"].as_str() == Some("benchmark-mock")) { + let mut server = existing_server.clone(); + server["connections"] = serde_json::json!(connections); + client + .put(format!("{endpoint}/benchmark-mock")) + .json(&server) + .timeout(std::time::Duration::from_secs(15)) + .send() + .await? + .error_for_status()?; + } else { let server = serde_json::json!({ "id": "benchmark-mock", "name": "Benchmark mock NNTP", @@ -620,7 +672,7 @@ async fn bootstrap_rustnzb_mock_server() -> Result<()> { "ssl_verify": false, "username": "bench", "password": "bench", - "connections": 20, + "connections": connections, "priority": 0, "enabled": true, "retention": 0, @@ -633,7 +685,8 @@ async fn bootstrap_rustnzb_mock_server() -> Result<()> { "trusted_fingerprint": null, "connect_timeout_secs": 30, }); - http.post(&endpoint) + client + .post(endpoint.clone()) .json(&server) .timeout(std::time::Duration::from_secs(15)) .send() @@ -641,8 +694,8 @@ async fn bootstrap_rustnzb_mock_server() -> Result<()> { .error_for_status()?; } - let configured: Vec = http - .get(&endpoint) + let configured: Vec = client + .get(endpoint) .timeout(std::time::Duration::from_secs(15)) .send() .await? @@ -656,7 +709,7 @@ async fn bootstrap_rustnzb_mock_server() -> Result<()> { }) { anyhow::bail!("rustnzb benchmark mock NNTP server was not configured"); } - tracing::info!("rustnzb mock NNTP server configured"); + tracing::info!(connections, "rustnzb mock NNTP server configured"); Ok(()) } diff --git a/benchnzb/src/stress.rs b/benchnzb/src/stress.rs index d742623f..ea7d8687 100644 --- a/benchnzb/src/stress.rs +++ b/benchnzb/src/stress.rs @@ -156,8 +156,12 @@ pub async fn run(cfg: StressConfig) -> Result<()> { wait_for_sabnzbd(&sab, 120).await?; StressClient::Sabnzbd(sab) } else { - wait_for_service("rustnzb", &format!("{RUSTNZB_API}/api/status"), 120).await?; - StressClient::Rustnzb(crate::clients::rustnzb::RustnzbClient::new(RUSTNZB_API)) + wait_for_service("rustnzb", &format!("{RUSTNZB_API}/api/health"), 120).await?; + { + let mut rustnzb = crate::clients::rustnzb::RustnzbClient::new(RUSTNZB_API); + rustnzb.initialize_auth().await?; + StressClient::Rustnzb(rustnzb) + } }; // Clear any stale state diff --git a/crates/nzb-core/Cargo.toml b/crates/nzb-core/Cargo.toml index d96f19ac..565d5cfa 100644 --- a/crates/nzb-core/Cargo.toml +++ b/crates/nzb-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nzb-core" -version = "0.2.16" +version = "0.2.17" edition = "2024" description = "Shared models, config, NZB parser, and SQLite database for NZB clients" license = "MIT" diff --git a/crates/nzb-core/src/config.rs b/crates/nzb-core/src/config.rs index 0605dca2..c5a56f23 100644 --- a/crates/nzb-core/src/config.rs +++ b/crates/nzb-core/src/config.rs @@ -108,6 +108,27 @@ pub struct GeneralConfig { /// 0 = no timeout. Default: 30. #[serde(default = "default_article_timeout_secs")] pub article_timeout_secs: u64, + /// Sort queued jobs by remaining percentage whenever progress changes. + #[serde(default)] + pub auto_sort_remaining_pct: bool, + /// Remove downloaded RSS records after this many days. None keeps them. + #[serde(default)] + pub rss_downloaded_item_expiry_days: Option, + /// Optional directory containing post-processing scripts. + #[serde(default)] + pub scripts_dir: Option, + /// Script run after a successful post-processing job. + #[serde(default)] + pub script_success: Option, + /// Script run after a failed post-processing job. + #[serde(default)] + pub script_failure: Option, + /// Maximum runtime for a post-processing script. + #[serde(default = "default_script_timeout_secs")] + pub script_timeout_secs: u64, + /// Maximum captured output retained from a post-processing script. + #[serde(default = "default_script_output_bytes")] + pub script_max_output_bytes: usize, } fn default_rss_history_limit() -> Option { @@ -152,6 +173,14 @@ pub fn normalize_history_retention(limit: Option) -> Option { limit.filter(|max| *max > 0) } +fn default_script_timeout_secs() -> u64 { + 300 +} + +fn default_script_output_bytes() -> usize { + 1024 * 1024 +} + impl Default for GeneralConfig { fn default() -> Self { Self { @@ -179,6 +208,13 @@ impl Default for GeneralConfig { early_failure_check: true, required_completion_pct: default_required_completion_pct(), article_timeout_secs: default_article_timeout_secs(), + auto_sort_remaining_pct: false, + rss_downloaded_item_expiry_days: None, + scripts_dir: None, + script_success: None, + script_failure: None, + script_timeout_secs: default_script_timeout_secs(), + script_max_output_bytes: default_script_output_bytes(), } } } @@ -249,6 +285,12 @@ pub struct CategoryConfig { pub output_dir: Option, /// Post-processing level: 0=none, 1=repair, 2=unpack, 3=repair+unpack pub post_processing: u8, + /// Filename or relative-path glob patterns to remove after unpacking. + #[serde(default)] + pub cleanup_patterns: Vec, + /// Extensions to remove after unpacking, including or omitting the dot. + #[serde(default)] + pub unwanted_extensions: Vec, } impl Default for CategoryConfig { @@ -257,6 +299,8 @@ impl Default for CategoryConfig { name: "Default".into(), output_dir: None, post_processing: 3, + cleanup_patterns: Vec::new(), + unwanted_extensions: Vec::new(), } } } @@ -284,6 +328,9 @@ pub struct RssFeedConfig { /// Ignored when filter_regex is set (use download rules instead). #[serde(default)] pub auto_download: bool, + /// Ignore entries older than this many days. None disables age filtering. + #[serde(default)] + pub max_age_days: Option, } fn default_poll_interval() -> u64 { @@ -557,6 +604,7 @@ mod tests { name: "movies".into(), output_dir: Some("/movies".into()), post_processing: 3, + ..CategoryConfig::default() }); assert!(cfg.category("Default").is_some()); @@ -584,6 +632,7 @@ mod tests { name: "movies".into(), output_dir: None, post_processing: 3, + ..CategoryConfig::default() }); assert_eq!(cfg.find_category_or_default("movies").name, "movies"); assert_eq!(cfg.find_category_or_default("unknown").name, "Default"); diff --git a/crates/nzb-core/src/db.rs b/crates/nzb-core/src/db.rs index 3b452111..ddc1688a 100644 --- a/crates/nzb-core/src/db.rs +++ b/crates/nzb-core/src/db.rs @@ -309,6 +309,17 @@ impl Database { )?; } + if version < 9 { + info!("Applying database migration v9: per-article retry outcomes"); + self.conn.execute_batch( + " + ALTER TABLE history ADD COLUMN retry_data BLOB; + DELETE FROM schema_version; + INSERT INTO schema_version (version) VALUES (9); + ", + )?; + } + Ok(()) } @@ -469,8 +480,8 @@ impl Database { let server_stats_json = serde_json::to_string(&entry.server_stats).unwrap_or_default(); self.conn.execute( "INSERT INTO history (id, name, category, status, total_bytes, downloaded_bytes, - added_at, completed_at, download_time_secs, output_dir, stages, error_message, nzb_data, server_stats) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14)", + added_at, completed_at, download_time_secs, output_dir, stages, error_message, nzb_data, server_stats, retry_data) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)", params![ entry.id, entry.name, @@ -486,6 +497,7 @@ impl Database { entry.error_message, entry.nzb_data, server_stats_json, + entry.retry_data, ], )?; @@ -580,6 +592,7 @@ impl Database { server_stats, // Don't load actual blob in list - just note if it exists nzb_data: if has_nzb != 0 { Some(Vec::new()) } else { None }, + retry_data: None, }) })? .collect::, _>>()?; @@ -601,6 +614,20 @@ impl Database { } } + /// Get persisted per-article outcomes for a history retry. + pub fn history_get_retry_data(&self, id: &str) -> Result>, NzbError> { + let result = self.conn.query_row( + "SELECT retry_data FROM history WHERE id = ?1", + params![id], + |row| row.get::<_, Option>>(0), + ); + match result { + Ok(data) => Ok(data), + Err(rusqlite::Error::QueryReturnedNoRows) => Ok(None), + Err(e) => Err(NzbError::Database(e)), + } + } + /// Enforce history retention limit by deleting oldest entries. /// /// A limit of `0` is treated as "keep all" and is a no-op: the @@ -650,6 +677,7 @@ impl Database { error_message: row.get(11)?, server_stats, nzb_data: None, + retry_data: None, }) }); @@ -893,6 +921,15 @@ impl Database { Ok(deleted) } + /// Remove downloaded RSS items older than `cutoff` and return the count. + pub fn rss_items_expire_downloaded(&self, cutoff: &str) -> Result { + Ok(self.conn.execute( + "DELETE FROM rss_items WHERE downloaded = 1 AND downloaded_at IS NOT NULL + AND downloaded_at < ?1", + params![cutoff], + )?) + } + fn map_rss_item(&self, row: &rusqlite::Row<'_>) -> rusqlite::Result { Ok(RssItem { id: row.get(0)?, @@ -1073,6 +1110,7 @@ mod tests { error_message: None, server_stats: Vec::new(), nzb_data: None, + retry_data: None, } } diff --git a/crates/nzb-core/src/lib.rs b/crates/nzb-core/src/lib.rs index 56beda5d..61389794 100644 --- a/crates/nzb-core/src/lib.rs +++ b/crates/nzb-core/src/lib.rs @@ -5,6 +5,7 @@ pub mod error; pub mod groups_db; pub mod models; pub mod nzb_parser; +pub mod path; pub mod sabnzbd_import; pub use config::AppConfig; diff --git a/crates/nzb-core/src/models.rs b/crates/nzb-core/src/models.rs index b12ab03d..bbffaeaa 100644 --- a/crates/nzb-core/src/models.rs +++ b/crates/nzb-core/src/models.rs @@ -205,6 +205,9 @@ pub struct HistoryEntry { /// Raw NZB XML data (for retry) #[serde(skip_serializing)] pub nzb_data: Option>, + /// Serialized per-article outcomes used for missing-only retry. + #[serde(skip_serializing)] + pub retry_data: Option>, } /// Immutable statistics ledger row recorded when a job leaves the queue. diff --git a/crates/nzb-core/src/path.rs b/crates/nzb-core/src/path.rs new file mode 100644 index 00000000..0cf3d803 --- /dev/null +++ b/crates/nzb-core/src/path.rs @@ -0,0 +1,58 @@ +//! Path validation shared by download and post-processing boundaries. + +use std::path::{Path, PathBuf}; + +/// Join an untrusted archive or API supplied relative name beneath `root`. +/// Backslashes are treated as separators on every platform. +pub fn safe_join(root: &Path, name: &str) -> Option { + let normalized = name.replace('\\', "/"); + if normalized.is_empty() + || normalized.starts_with('/') + || normalized.as_bytes().get(1) == Some(&b':') + || normalized.chars().any(char::is_control) + { + return None; + } + + let mut output = root.to_path_buf(); + for component in normalized.split('/') { + match component { + "" | "." => {} + ".." => return None, + component => output.push(component), + } + } + output.starts_with(root).then_some(output) +} + +/// Validate a user supplied directory or category component. +/// These values are intentionally a single path component. +pub fn safe_component(value: &str) -> Option<&str> { + if value.is_empty() + || value == "." + || value == ".." + || value.contains('/') + || value.contains('\\') + || value.chars().any(char::is_control) + { + return None; + } + Some(value) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn safe_join_rejects_cross_platform_escape_paths() { + let root = Path::new("/tmp/job"); + assert!(safe_join(root, "folder/file.txt").is_some()); + for path in ["../outside", r"..\outside", "/etc/passwd", r"C:\temp"] { + assert!(safe_join(root, path).is_none(), "{path}"); + } + assert!(safe_component("movies").is_some()); + assert!(safe_component("../outside").is_none()); + assert!(safe_component(r"movies\tv").is_none()); + } +} diff --git a/crates/nzb-core/src/sabnzbd_import.rs b/crates/nzb-core/src/sabnzbd_import.rs index beb48c6c..5b3535d0 100644 --- a/crates/nzb-core/src/sabnzbd_import.rs +++ b/crates/nzb-core/src/sabnzbd_import.rs @@ -244,6 +244,7 @@ pub fn parse_sabnzbd_ini(content: &str) -> SabnzbdImportPreview { .filter(|s| !s.is_empty()) .map(std::path::PathBuf::from), post_processing: kv.get("pp").and_then(|s| s.parse().ok()).unwrap_or(3), + ..CategoryConfig::default() } }) .collect(); @@ -279,6 +280,7 @@ pub fn parse_sabnzbd_ini(content: &str) -> SabnzbdImportPreview { filter_regex, enabled: kv.get("enable").map(|s| parse_ini_bool(s)).unwrap_or(true), auto_download: false, + max_age_days: None, }) }) .collect(); @@ -424,6 +426,7 @@ pub fn parse_sabnzbd_api_response(json: &serde_json::Value) -> SabnzbdImportPrev .as_u64() .or_else(|| c["pp"].as_str().and_then(|p| p.parse().ok())) .unwrap_or(3) as u8, + ..CategoryConfig::default() } }) .collect() diff --git a/crates/nzb-dispatch/Cargo.toml b/crates/nzb-dispatch/Cargo.toml index d6e9bf49..f1447b2f 100644 --- a/crates/nzb-dispatch/Cargo.toml +++ b/crates/nzb-dispatch/Cargo.toml @@ -10,7 +10,7 @@ readme = "README.md" [dependencies] nzb-nntp = { version = "0.2.22", path = "../nzb-nntp" } nzb-decode = { version = "0.1.2", path = "../nzb-decode" } -nzb-core = { version = "0.2.16", path = "../nzb-core" } +nzb-core = { version = "0.2.17", path = "../nzb-core" } tokio = { version = "1", features = ["full"] } async-trait = "0.1" diff --git a/crates/nzb-dispatch/src/download_engine.rs b/crates/nzb-dispatch/src/download_engine.rs index 02db34e2..502b7ac0 100644 --- a/crates/nzb-dispatch/src/download_engine.rs +++ b/crates/nzb-dispatch/src/download_engine.rs @@ -2660,9 +2660,19 @@ pub(crate) fn build_job_submission( let assembler = Arc::new(FileAssembler::new()); for file in &job.files { let output_path = job.work_dir.join(&file.filename); - if let Err(e) = - assembler.register_file(&job.id, &file.id, output_path, file.articles.len() as u32) - { + let completed_segments: Vec = file + .articles + .iter() + .filter(|article| article.downloaded) + .map(|article| article.segment_number) + .collect(); + if let Err(e) = assembler.register_file_with_completed_segments( + &job.id, + &file.id, + output_path, + file.articles.len() as u32, + &completed_segments, + ) { error!(file = %file.filename, "Failed to register file for assembly: {e}"); } } @@ -2680,7 +2690,7 @@ pub(crate) fn build_job_submission( filename: file.filename.clone(), message_id: article.message_id.clone(), segment_number: article.segment_number, - tried_servers: Vec::new(), + tried_servers: article.tried_servers.clone(), provider_outcomes: HashMap::new(), tries_on_current: 0, }) diff --git a/crates/nzb-postproc/Cargo.toml b/crates/nzb-postproc/Cargo.toml index f2a64729..dcc368cd 100644 --- a/crates/nzb-postproc/Cargo.toml +++ b/crates/nzb-postproc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nzb-postproc" -version = "0.2.6" +version = "0.2.7" edition = "2024" description = "Post-processing pipeline: PAR2 verify/repair, archive extraction" license = "MIT" @@ -12,7 +12,7 @@ default = [] groups-db = ["nzb-core/groups-db"] [dependencies] -nzb-core = { version = "0.2.16", path = "../nzb-core" } +nzb-core = { version = "0.2.17", path = "../nzb-core" } tokio = { version = "1", features = ["rt", "sync", "process"] } tracing = "0.1" opentelemetry.workspace = true @@ -21,6 +21,7 @@ thiserror = "2" rust-par2 = "0.1" walkdir = "2" zip = "8" +tar.workspace = true [dev-dependencies] tempfile = "3" diff --git a/crates/nzb-postproc/src/detect.rs b/crates/nzb-postproc/src/detect.rs index 5ce4ba61..a59ad3c9 100644 --- a/crates/nzb-postproc/src/detect.rs +++ b/crates/nzb-postproc/src/detect.rs @@ -1,7 +1,7 @@ //! File detection helpers for post-processing. //! //! Scans a completed download directory to find par2 files, RAR archives, -//! 7z archives, ZIP archives, and cleanup candidates. +//! 7z archives, TAR archives, ZIP archives, and cleanup candidates. use std::collections::BTreeMap; use std::io::Read; @@ -156,6 +156,7 @@ pub fn parse_rar_volume_at(path: &Path) -> Option { pub enum ArchiveType { Rar, SevenZip, + Tar, Zip, } @@ -164,6 +165,7 @@ impl std::fmt::Display for ArchiveType { match self { Self::Rar => write!(f, "RAR"), Self::SevenZip => write!(f, "7z"), + Self::Tar => write!(f, "TAR"), Self::Zip => write!(f, "ZIP"), } } @@ -309,7 +311,7 @@ pub fn find_archives(dir: &Path) -> Vec<(ArchiveType, PathBuf)> { archives.push((ArchiveType::Rar, path)); } - // 7z (including split volumes), and ZIP + // 7z (including split volumes), TAR, and ZIP for entry in WalkDir::new(dir).into_iter().flatten() { let path = entry.path(); if !path.is_file() { @@ -325,6 +327,8 @@ pub fn find_archives(dir: &Path) -> Vec<(ArchiveType, PathBuf)> { } else if is_split_7z_first_volume(&name) { // Split 7z: .7z.001 is the first volume — 7z handles the rest archives.push((ArchiveType::SevenZip, path.to_path_buf())); + } else if name.ends_with(".tar") { + archives.push((ArchiveType::Tar, path.to_path_buf())); } else if name.ends_with(".zip") { archives.push((ArchiveType::Zip, path.to_path_buf())); } @@ -432,7 +436,11 @@ fn is_cleanup_candidate_at(path: &Path, name_lower: &str) -> bool { /// so obfuscated volumes are caught too. fn is_cleanup_candidate(name: &str) -> bool { // Par2 files: .par2 - if name.ends_with(".par2") || name.ends_with(".zip") || name.ends_with(".7z") { + if name.ends_with(".par2") + || name.ends_with(".zip") + || name.ends_with(".7z") + || name.ends_with(".tar") + { return true; } diff --git a/crates/nzb-postproc/src/lib.rs b/crates/nzb-postproc/src/lib.rs index 0db76caa..b1ece59e 100644 --- a/crates/nzb-postproc/src/lib.rs +++ b/crates/nzb-postproc/src/lib.rs @@ -1,9 +1,9 @@ -//! Post-processing pipeline: par2 verify/repair, RAR/7z/ZIP extraction, cleanup. +//! Post-processing pipeline: par2 verify/repair, RAR/7z/TAR/ZIP extraction, cleanup. //! //! This crate contains: -//! - `detect` — File detection helpers (par2, RAR, 7z, ZIP, cleanup candidates) +//! - `detect` — File detection helpers (par2, RAR, 7z, TAR, ZIP, cleanup candidates) //! - `par2` — Native PAR2 verify/repair via `rust-par2` -//! - `unpack` — RAR extraction (unrar), 7z (7z binary), ZIP (zip crate) +//! - `unpack` — RAR/7z extraction (system tools), TAR/ZIP (native crates) //! - `pipeline` — Orchestrate: verify -> repair -> extract -> cleanup pub mod detect; @@ -21,6 +21,9 @@ pub use detect::{ parse_rar_volume_at, }; pub use par2::recovery_can_cover; -pub use pipeline::{PostProcConfig, PostProcResult, run_pipeline, run_pipeline_with_resources}; +pub use pipeline::{ + PostProcConfig, PostProcResult, run_pipeline, run_pipeline_with_cleanup, + run_pipeline_with_resources, +}; pub use resources::{PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot}; -pub use unpack::find_unrar; +pub use unpack::{extract_tar, find_unrar}; diff --git a/crates/nzb-postproc/src/pipeline.rs b/crates/nzb-postproc/src/pipeline.rs index 7f75c6c3..17a5e568 100644 --- a/crates/nzb-postproc/src/pipeline.rs +++ b/crates/nzb-postproc/src/pipeline.rs @@ -17,7 +17,7 @@ use tracing::{debug, error, info, warn}; use crate::detect::{ArchiveType, find_archives, find_cleanup_files, find_par2_files}; use crate::par2::par2_repair; use crate::resources::PostProcResourcePool; -use crate::unpack::{extract_7z, extract_rar, extract_zip}; +use crate::unpack::{extract_7z, extract_rar, extract_tar, extract_zip}; fn increment_counter(name: &'static str) { opentelemetry::global::meter_provider() @@ -115,6 +115,17 @@ pub async fn run_pipeline_with_resources( job_dir: &Path, config: &PostProcConfig, resources: Option<&Arc>, +) -> PostProcResult { + run_pipeline_with_cleanup(job_dir, config, resources, &[], &[]).await +} + +/// Run the pipeline with optional category cleanup rules. +pub async fn run_pipeline_with_cleanup( + job_dir: &Path, + config: &PostProcConfig, + resources: Option<&Arc>, + cleanup_patterns: &[String], + unwanted_extensions: &[String], ) -> PostProcResult { let mut stages: Vec = Vec::new(); let mut pipeline_ok = true; @@ -434,7 +445,14 @@ pub async fn run_pipeline_with_resources( // Stage 4: Cleanup // ------------------------------------------------------------------ if pipeline_ok && config.cleanup_after_extract { - let result = run_cleanup_stage(job_dir, &extracted_archives); + let cleanup_root = config.output_dir.as_deref().unwrap_or(job_dir); + let result = run_cleanup_stage_with_rules( + job_dir, + cleanup_root, + &extracted_archives, + cleanup_patterns, + unwanted_extensions, + ); stages.push(result); } @@ -640,6 +658,7 @@ async fn run_extract_stage( let result = match archive_type { ArchiveType::Rar => extract_rar(path, output_dir, password).await, ArchiveType::SevenZip => extract_7z(path, output_dir, password).await, + ArchiveType::Tar => extract_tar(path, output_dir).await, ArchiveType::Zip => extract_zip(path, output_dir).await, }; @@ -712,15 +731,108 @@ async fn run_extract_stage( ) } +#[cfg(test)] fn run_cleanup_stage(job_dir: &Path, extracted_archives: &[PathBuf]) -> StageResult { + run_cleanup_stage_with_rules(job_dir, job_dir, extracted_archives, &[], &[]) +} + +fn wildcard_match(pattern: &str, value: &str) -> bool { + let (mut pattern_index, mut value_index) = (0usize, 0usize); + let mut star = None; + let mut star_value = 0usize; + let pattern = pattern.as_bytes(); + let value = value.as_bytes(); + + while value_index < value.len() { + if pattern_index < pattern.len() + && (pattern[pattern_index] == value[value_index] || pattern[pattern_index] == b'?') + { + pattern_index += 1; + value_index += 1; + } else if pattern_index < pattern.len() && pattern[pattern_index] == b'*' { + star = Some(pattern_index); + pattern_index += 1; + star_value = value_index; + } else if let Some(star_index) = star { + pattern_index = star_index + 1; + star_value += 1; + value_index = star_value; + } else { + return false; + } + } + while pattern_index < pattern.len() && pattern[pattern_index] == b'*' { + pattern_index += 1; + } + pattern_index == pattern.len() +} + +fn run_cleanup_stage_with_rules( + job_dir: &Path, + cleanup_root: &Path, + extracted_archives: &[PathBuf], + cleanup_patterns: &[String], + unwanted_extensions: &[String], +) -> StageResult { let start = Instant::now(); let mut files = find_cleanup_files(job_dir); files.extend( extracted_archives .iter() - .filter(|path| path.is_file()) + .filter(|path| { + std::fs::symlink_metadata(path) + .map(|metadata| metadata.file_type().is_file()) + .unwrap_or(false) + }) .cloned(), ); + + let normalized_extensions: Vec = unwanted_extensions + .iter() + .map(|extension| { + let extension = extension.trim().to_ascii_lowercase(); + if extension.starts_with('.') { + extension + } else { + format!(".{extension}") + } + }) + .filter(|extension| extension.len() > 1) + .collect(); + let patterns: Vec = cleanup_patterns + .iter() + .map(|pattern| pattern.replace('\\', "/").to_ascii_lowercase()) + .filter(|pattern| !pattern.is_empty()) + .collect(); + for root in [job_dir, cleanup_root] { + for entry in walkdir::WalkDir::new(root).into_iter().flatten() { + let path = entry.path(); + let Ok(metadata) = std::fs::symlink_metadata(path) else { + continue; + }; + if !metadata.file_type().is_file() { + continue; + } + let Ok(relative) = path.strip_prefix(root) else { + continue; + }; + let relative = relative.to_string_lossy().replace('\\', "/"); + let filename = path + .file_name() + .map(|name| name.to_string_lossy().to_string()) + .unwrap_or_default(); + if normalized_extensions + .iter() + .any(|extension| filename.to_ascii_lowercase().ends_with(extension)) + || patterns.iter().any(|pattern| { + wildcard_match(pattern, &relative.to_ascii_lowercase()) + || wildcard_match(pattern, &filename.to_ascii_lowercase()) + }) + { + files.push(path.to_path_buf()); + } + } + } files.sort(); files.dedup(); @@ -737,6 +849,15 @@ fn run_cleanup_stage(job_dir: &Path, extracted_archives: &[PathBuf]) -> StageRes let mut errors = 0u32; for path in &files { + let under_allowed_root = path.starts_with(job_dir) || path.starts_with(cleanup_root); + let is_regular_file = std::fs::symlink_metadata(path) + .map(|metadata| metadata.file_type().is_file()) + .unwrap_or(false); + if !under_allowed_root || !is_regular_file { + warn!(file = %path.display(), "Skipping cleanup path outside job roots or through a link"); + errors += 1; + continue; + } match std::fs::remove_file(path) { Ok(()) => { removed += 1; diff --git a/crates/nzb-postproc/src/unpack.rs b/crates/nzb-postproc/src/unpack.rs index bc6e5b40..e01fba7b 100644 --- a/crates/nzb-postproc/src/unpack.rs +++ b/crates/nzb-postproc/src/unpack.rs @@ -1,10 +1,11 @@ -//! Archive extraction: RAR, 7z, ZIP. +//! Archive extraction: RAR, 7z, TAR, ZIP. //! //! - RAR: Shell out to `unrar` binary //! - 7z: Shell out to `7z`/`7zz`/`7za` binary //! - ZIP: Uses std::fs + zip crate use std::collections::HashSet; +use std::io::Write; use std::path::{Path, PathBuf}; use std::process::Stdio; @@ -107,6 +108,36 @@ fn output_files(root: &Path) -> std::io::Result> { Ok(files) } +/// Reject links and non-directory path components left by an external +/// extractor. Native formats are checked before each write; this is the +/// equivalent postcondition for unrar/7z, whose archive member lists are not +/// exposed through a stable API. +fn validate_extraction_tree(root: &Path) -> anyhow::Result<()> { + let mut directories = vec![root.to_path_buf()]; + while let Some(directory) = directories.pop() { + for entry in std::fs::read_dir(&directory)? { + let entry = entry?; + let path = entry.path(); + let file_type = entry.file_type()?; + if file_type.is_symlink() { + anyhow::bail!( + "archive extraction produced a symbolic link `{}`", + path.display() + ); + } + if file_type.is_dir() { + directories.push(path); + } else if !file_type.is_file() { + anyhow::bail!( + "archive extraction produced unsupported path `{}`", + path.display() + ); + } + } + } + Ok(()) +} + fn newly_extracted_files( output_dir: &Path, before: &HashSet, @@ -119,36 +150,20 @@ fn newly_extracted_files( Ok(files) } -fn safe_zip_output_path(root: &Path, name: &str) -> anyhow::Result { - let normalized = name.replace('\\', "/"); - let has_drive_prefix = normalized.as_bytes().get(1) == Some(&b':'); - if normalized.is_empty() - || normalized.starts_with('/') - || has_drive_prefix - || normalized.split('/').any(|component| component == "..") - { - anyhow::bail!("ZIP archive contains unsafe path `{name}`"); - } - - let output = normalized - .split('/') - .filter(|component| !component.is_empty() && *component != ".") - .fold(root.to_path_buf(), |path, component| path.join(component)); - if !output.starts_with(root) { - anyhow::bail!("ZIP archive path escapes extraction directory: `{name}`"); - } - Ok(output) +fn safe_archive_output_path(root: &Path, name: &str, kind: &str) -> anyhow::Result { + nzb_core::path::safe_join(root, name) + .ok_or_else(|| anyhow::anyhow!("{kind} archive contains unsafe path `{name}`")) } fn reject_symlinked_path(root: &Path, path: &Path) -> anyhow::Result<()> { let relative = path .strip_prefix(root) - .map_err(|_| anyhow::anyhow!("ZIP archive path is outside extraction directory"))?; + .map_err(|_| anyhow::anyhow!("archive path is outside extraction directory"))?; let mut current = root.to_path_buf(); if let Ok(metadata) = std::fs::symlink_metadata(¤t) && metadata.file_type().is_symlink() { - anyhow::bail!("ZIP extraction directory is a symbolic link"); + anyhow::bail!("archive extraction directory is a symbolic link"); } for component in relative.components() { @@ -157,10 +172,10 @@ fn reject_symlinked_path(root: &Path, path: &Path) -> anyhow::Result<()> { continue; }; if metadata.file_type().is_symlink() { - anyhow::bail!("ZIP archive path crosses a symbolic link"); + anyhow::bail!("archive path crosses a symbolic link"); } if current != path && !metadata.is_dir() { - anyhow::bail!("ZIP archive path crosses a non-directory"); + anyhow::bail!("archive path crosses a non-directory"); } } Ok(()) @@ -218,6 +233,10 @@ pub async fn extract_rar( let combined = format!("{stdout}\n{stderr}"); let success = output.status.success(); + if success { + validate_extraction_tree(output_dir)?; + } + if !success { // Detect password-protected archives (unrar exit code 255 + password prompt) let is_encrypted = combined.contains("Enter password") @@ -286,6 +305,10 @@ pub async fn extract_7z( let combined = format!("{stdout}\n{stderr}"); let success = output.status.success(); + if success { + validate_extraction_tree(output_dir)?; + } + if !success { let is_encrypted = SEVENZ_PASSWORD_PATTERNS .iter() @@ -341,7 +364,7 @@ pub async fn extract_zip(zip_file: &Path, output_dir: &Path) -> anyhow::Result anyhow::Result anyhow::Result { + info!(file = %tar_file.display(), dest = %output_dir.display(), "Extracting TAR"); + let tar_path = tar_file.to_path_buf(); + let out_path = output_dir.to_path_buf(); + tokio::task::spawn_blocking(move || -> anyhow::Result { + let file = std::fs::File::open(&tar_path)?; + let mut archive = tar::Archive::new(file); + std::fs::create_dir_all(&out_path)?; + let mut extracted = Vec::new(); + let mut output_paths = HashSet::new(); + + for entry in archive.entries()? { + let mut entry = entry?; + let entry_path = entry.path()?.to_string_lossy().into_owned(); + let entry_type = entry.header().entry_type(); + if entry_type.is_symlink() { + anyhow::bail!("TAR archive contains unsupported symbolic link `{entry_path}`"); + } + if entry_type.is_hard_link() { + anyhow::bail!("TAR archive contains unsupported hard link `{entry_path}`"); + } + + let outpath = safe_archive_output_path(&out_path, &entry_path, "TAR")?; + if !output_paths.insert(outpath.clone()) { + anyhow::bail!("TAR archive contains duplicate output path `{entry_path}`"); + } + + if entry_type.is_dir() { + reject_symlinked_path(&out_path, &outpath)?; + std::fs::create_dir_all(&outpath)?; + continue; + } + if !entry_type.is_file() { + anyhow::bail!("TAR archive contains unsupported entry `{entry_path}`"); + } + + reject_symlinked_path(&out_path, &outpath)?; + if let Some(parent) = outpath.parent() { + std::fs::create_dir_all(parent)?; + } + let mut outfile = std::fs::File::create(&outpath)?; + std::io::copy(&mut entry, &mut outfile)?; + outfile.flush()?; + extracted.push(outpath.to_string_lossy().into_owned()); + } + + Ok(UnpackResult { + success: true, + files_extracted: extracted, + output: String::new(), + error_output: String::new(), + }) + }) + .await? +} + pub fn find_unrar() -> Option { for name in &["unrar", "unrar-free", "rar"] { if which_exists(name) { @@ -408,6 +491,7 @@ fn which_exists(name: &str) -> bool { #[cfg(test)] mod tests { use super::*; + use std::fs; use std::io::Write; #[tokio::test] @@ -542,6 +626,125 @@ mod tests { assert!(error.contains("symbolic link"), "{error}"); } + fn write_tar(path: &Path, entries: &[(&str, &[u8])]) { + let file = std::fs::File::create(path).unwrap(); + let mut builder = tar::Builder::new(file); + for (name, contents) in entries { + let mut header = tar::Header::new_gnu(); + header.set_path(name).unwrap(); + header.set_size(contents.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder.append(&header, *contents).unwrap(); + } + builder.finish().unwrap(); + } + + fn write_raw_tar(path: &Path, entries: &[(&str, &[u8])]) { + let file = std::fs::File::create(path).unwrap(); + let mut builder = tar::Builder::new(file); + for (name, contents) in entries { + let mut header = tar::Header::new_gnu(); + let name_bytes = name.as_bytes(); + assert!(name_bytes.len() <= 100); + header.as_mut_bytes()[..100].fill(0); + header.as_mut_bytes()[..name_bytes.len()].copy_from_slice(name_bytes); + header.set_size(contents.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + builder.append(&header, *contents).unwrap(); + } + builder.finish().unwrap(); + } + + #[tokio::test] + async fn test_extract_tar_valid_and_large_file() { + let dir = tempfile::tempdir().unwrap(); + let tar_path = dir.path().join("payload.tar"); + let output = dir.path().join("output"); + let large = vec![b'x'; 128 * 1024]; + write_tar( + &tar_path, + &[("nested/hello.txt", b"hello"), ("large.bin", &large)], + ); + + let result = extract_tar(&tar_path, &output).await.unwrap(); + assert!(result.success); + assert_eq!(fs::read(output.join("nested/hello.txt")).unwrap(), b"hello"); + assert_eq!( + fs::metadata(output.join("large.bin")).unwrap().len(), + large.len() as u64 + ); + } + + #[tokio::test] + async fn test_extract_tar_rejects_traversal_and_duplicate_paths() { + for (index, names) in [vec!["../../outside.txt"], vec!["same.txt", "same.txt"]] + .into_iter() + .enumerate() + { + let dir = tempfile::tempdir().unwrap(); + let tar_path = dir.path().join(format!("unsafe-{index}.tar")); + let entries: Vec<(&str, &[u8])> = names + .iter() + .map(|name| (*name, b"data".as_slice())) + .collect(); + if names.iter().any(|name| name.contains("..")) { + write_raw_tar(&tar_path, &entries); + } else { + write_tar(&tar_path, &entries); + } + let error = extract_tar(&tar_path, &dir.path().join("output")) + .await + .unwrap_err() + .to_string(); + assert!( + error.contains("unsafe path") || error.contains("duplicate output path"), + "{error}" + ); + assert!(!dir.path().join("outside.txt").exists()); + } + } + + #[cfg(unix)] + #[tokio::test] + async fn test_extract_tar_rejects_symlink_and_hardlink_entries() { + let dir = tempfile::tempdir().unwrap(); + let tar_path = dir.path().join("links.tar"); + let file = fs::File::create(&tar_path).unwrap(); + let mut builder = tar::Builder::new(file); + let mut symlink_header = tar::Header::new_gnu(); + symlink_header.set_entry_type(tar::EntryType::Symlink); + symlink_header.set_path("link").unwrap(); + symlink_header.set_link_name("outside").unwrap(); + symlink_header.set_size(0); + symlink_header.set_cksum(); + builder.append(&symlink_header, &[][..]).unwrap(); + builder.finish().unwrap(); + let error = extract_tar(&tar_path, &dir.path().join("output")) + .await + .unwrap_err() + .to_string(); + assert!(error.contains("symbolic link"), "{error}"); + + let hardlink_path = dir.path().join("hardlink.tar"); + let file = fs::File::create(&hardlink_path).unwrap(); + let mut builder = tar::Builder::new(file); + let mut hardlink_header = tar::Header::new_gnu(); + hardlink_header.set_entry_type(tar::EntryType::Link); + hardlink_header.set_path("copy").unwrap(); + hardlink_header.set_link_name("original").unwrap(); + hardlink_header.set_size(0); + hardlink_header.set_cksum(); + builder.append(&hardlink_header, &[][..]).unwrap(); + builder.finish().unwrap(); + let error = extract_tar(&hardlink_path, &dir.path().join("hardlink-output")) + .await + .unwrap_err() + .to_string(); + assert!(error.contains("hard link"), "{error}"); + } + #[test] fn test_unpack_result_fields() { let result = UnpackResult { diff --git a/crates/nzb-web/Cargo.toml b/crates/nzb-web/Cargo.toml index ba3a9736..a3f2d4de 100644 --- a/crates/nzb-web/Cargo.toml +++ b/crates/nzb-web/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nzb-web" -version = "0.4.20" +version = "0.4.21" edition = "2024" description = "Usenet download engine: queue management, download orchestration, and background services" license = "MIT" @@ -13,7 +13,7 @@ groups-db = ["nzb-postproc/groups-db"] [dependencies] nzb-nntp = { version = "0.2.22", path = "../nzb-nntp" } nzb-decode = { version = "0.1.2", path = "../nzb-decode" } -nzb-postproc = { version = "0.2.6", path = "../nzb-postproc" } +nzb-postproc = { version = "0.2.7", path = "../nzb-postproc" } nzb-dispatch = { version = "0.2.6", path = "../nzb-dispatch" } axum = { version = "0.8", features = ["multipart"] } tower = "0.5" diff --git a/crates/nzb-web/src/auth.rs b/crates/nzb-web/src/auth.rs index dbc1cd33..15255825 100644 --- a/crates/nzb-web/src/auth.rs +++ b/crates/nzb-web/src/auth.rs @@ -111,6 +111,12 @@ impl TokenStore { self.refresh_tokens.write().remove(refresh_token); } + /// Revoke every session after credentials change. + pub fn revoke_all(&self) { + self.access_tokens.write().clear(); + self.refresh_tokens.write().clear(); + } + pub fn cleanup_expired(&self) { let now = Instant::now(); self.access_tokens @@ -177,6 +183,30 @@ impl CredentialStore { Ok(()) } + /// Set credentials exactly once. The check and write are serialized so + /// two first-boot setup requests cannot race into different accounts. + pub fn initialize_credentials(&self, creds: StoredCredentials) -> Result<(), std::io::Error> { + let mut current = self.credentials.write(); + if current.is_some() { + return Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + "credentials already configured", + )); + } + let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; + if let Some(parent) = self.file_path.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(&self.file_path, &json)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&self.file_path, std::fs::Permissions::from_mode(0o600))?; + } + *current = Some(creds); + Ok(()) + } + pub fn validate(&self, username: &str, password: &str) -> bool { match &*self.credentials.read() { Some(creds) => { @@ -238,11 +268,6 @@ pub async fn h_auth_setup( State(state): State, Json(req): Json, ) -> impl IntoResponse { - // Only allow if no credentials exist yet - if state.credential_store.has_credentials() { - return (StatusCode::FORBIDDEN, "credentials already configured").into_response(); - } - if req.username.is_empty() || req.password.is_empty() { return ( StatusCode::BAD_REQUEST, @@ -251,15 +276,20 @@ pub async fn h_auth_setup( .into_response(); } - match state.credential_store.set_credentials(StoredCredentials { - username: req.username, - password: req.password, - }) { + match state + .credential_store + .initialize_credentials(StoredCredentials { + username: req.username, + password: req.password, + }) { Ok(_) => { // Create tokens for the new user so they're immediately logged in let tokens = state.token_store.create_tokens(); (StatusCode::OK, Json(tokens)).into_response() } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + (StatusCode::FORBIDDEN, "credentials are already configured").into_response() + } Err(e) => ( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to save credentials: {e}"), @@ -300,9 +330,19 @@ pub async fn h_auth_change_credentials( username: req.new_username.unwrap_or(current_creds.username), password: req.new_password.unwrap_or(current_creds.password), }; + if new_creds.username.is_empty() || new_creds.password.is_empty() { + return ( + StatusCode::BAD_REQUEST, + "username and password cannot be empty", + ) + .into_response(); + } match state.credential_store.set_credentials(new_creds) { - Ok(_) => StatusCode::OK.into_response(), + Ok(_) => { + state.token_store.revoke_all(); + StatusCode::OK.into_response() + } Err(e) => ( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to save credentials: {e}"), diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index e875c9d8..4aff63c2 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -5,6 +5,7 @@ //! to interact with. use std::collections::{HashMap, HashSet}; +use std::io; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::time::{Duration, Instant}; @@ -12,6 +13,7 @@ use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use parking_lot::Mutex; use serde::{Deserialize, Serialize}; +use tokio::io::{AsyncRead, AsyncReadExt}; use tokio::sync::{broadcast, mpsc}; use tracing::{debug, error, info, warn}; @@ -21,7 +23,7 @@ use crate::nzb_core::models::*; use crate::nzb_core::nzb_parser; use nzb_postproc::{ PostProcConfig, PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot, - has_usable_output, parse_rar_volume, run_pipeline_with_resources, + has_usable_output, parse_rar_volume, run_pipeline_with_cleanup, }; use crate::direct_unpack::DirectUnpacker; @@ -30,7 +32,12 @@ use nzb_dispatch::{ BandwidthConfig, BandwidthLimiter, DispatchEngine, DispatchHandle, ProgressUpdate, }; -fn cleanup_terminal_work_dir(job_id: &str, work_dir: &std::path::Path, final_status: JobStatus) { +fn cleanup_terminal_work_dir( + job_id: &str, + work_dir: &std::path::Path, + final_status: JobStatus, + retain_for_retry: bool, +) { if !work_dir.exists() { return; } @@ -38,6 +45,10 @@ fn cleanup_terminal_work_dir(job_id: &str, work_dir: &std::path::Path, final_sta let cleanup_result = match final_status { // Failed downloads can be retried from their retained NZB history, so // retaining raw articles only leaks disk without improving recovery. + JobStatus::Failed if retain_for_retry => { + info!(job_id, work_dir = %work_dir.display(), "Retaining partial job files for missing-article retry"); + return; + } JobStatus::Failed => std::fs::remove_dir_all(work_dir), // A successful job must not lose files if an output move failed. Only // remove the directory after the move/pipeline has left it empty. @@ -129,11 +140,17 @@ pub struct GlobalStatisticsData { /// Get free disk space for a path (returns 0 on error). fn get_disk_free(path: &std::path::Path) -> u64 { + let mut candidate = path.to_path_buf(); + while !candidate.exists() { + if !candidate.pop() { + return 0; + } + } #[cfg(unix)] { use std::ffi::CString; use std::mem::MaybeUninit; - let c_path = match CString::new(path.to_string_lossy().as_bytes()) { + let c_path = match CString::new(candidate.to_string_lossy().as_bytes()) { Ok(p) => p, Err(_) => return 0, }; @@ -149,11 +166,119 @@ fn get_disk_free(path: &std::path::Path) -> u64 { } #[cfg(not(unix))] { - let _ = path; + let _ = candidate; 0 } } +fn disk_space_available(threshold: u64, paths: &[&std::path::Path]) -> bool { + threshold == 0 || paths.iter().all(|path| get_disk_free(path) >= threshold) +} + +#[derive(Debug, Clone, Default)] +struct PostProcScriptConfig { + scripts_dir: Option, + success: Option, + failure: Option, + timeout: Duration, + max_output_bytes: usize, +} + +fn resolve_script_path( + scripts_dir: Option<&std::path::Path>, + configured: &std::path::Path, +) -> io::Result { + let candidate = if configured.is_absolute() { + configured.to_path_buf() + } else { + let root = scripts_dir.ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "relative post-processing scripts require scripts_dir", + ) + })?; + crate::nzb_core::path::safe_join(root, &configured.to_string_lossy()).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "post-processing script path is unsafe", + ) + })? + }; + let resolved = std::fs::canonicalize(candidate)?; + if !std::fs::metadata(&resolved)?.is_file() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "post-processing script is not a regular file", + )); + } + if let Some(root) = scripts_dir { + let root = std::fs::canonicalize(root)?; + if !resolved.starts_with(root) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "post-processing script is outside scripts_dir", + )); + } + } + Ok(resolved) +} + +async fn read_script_output( + reader: R, + max_output_bytes: usize, +) -> io::Result<(Vec, bool)> { + let mut output = Vec::new(); + let mut limited = reader.take(max_output_bytes.saturating_add(1) as u64); + limited.read_to_end(&mut output).await?; + let truncated = output.len() > max_output_bytes; + output.truncate(max_output_bytes); + Ok((output, truncated)) +} + +fn regular_output_files(root: &std::path::Path) -> Vec { + let mut pending = vec![root.to_path_buf()]; + let mut files = Vec::new(); + while let Some(directory) = pending.pop() { + let Ok(entries) = std::fs::read_dir(directory) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + let Ok(metadata) = std::fs::symlink_metadata(&path) else { + continue; + }; + if metadata.file_type().is_symlink() { + continue; + } + if metadata.is_dir() { + pending.push(path); + } else if metadata.is_file() { + files.push(path); + } + } + } + files.sort(); + files +} + +fn script_output_message(stdout: &(Vec, bool), stderr: &(Vec, bool)) -> String { + let mut message = String::from_utf8_lossy(&stdout.0).trim().to_string(); + let error = String::from_utf8_lossy(&stderr.0).trim().to_string(); + if !error.is_empty() { + if !message.is_empty() { + message.push_str("; "); + } + message.push_str(&error); + } + if stdout.1 || stderr.1 { + if !message.is_empty() { + message.push_str("; "); + } + message.push_str("output truncated"); + } + message +} + // --------------------------------------------------------------------------- // Job checkpoint for resume support // --------------------------------------------------------------------------- @@ -172,6 +297,113 @@ struct JobCheckpoint { articles_failed: usize, /// Number of files completed files_completed: usize, + /// Full article outcomes. This was added after the original segment-only + /// checkpoint so history retries can distinguish missing articles from + /// articles that were already written before a failure. + #[serde(default)] + articles: HashMap>, + /// Retained partial work directory for missing-only history retry. + #[serde(default)] + work_dir: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct ArticleCheckpoint { + message_id: String, + segment_number: u32, + bytes: u64, + downloaded: bool, + data_begin: Option, + data_size: Option, + crc32: Option, + tried_servers: Vec, + tries: u32, +} + +fn checkpoint_for_job(job: &NzbJob) -> JobCheckpoint { + JobCheckpoint { + files: job + .files + .iter() + .map(|file| { + ( + file.filename.clone(), + file.articles + .iter() + .filter(|article| article.downloaded) + .map(|article| article.segment_number) + .collect(), + ) + }) + .collect(), + downloaded_bytes: job.downloaded_bytes, + articles_downloaded: job.articles_downloaded, + articles_failed: job.articles_failed, + files_completed: job.files_completed, + articles: job + .files + .iter() + .map(|file| { + ( + file.filename.clone(), + file.articles + .iter() + .map(|article| ArticleCheckpoint { + message_id: article.message_id.clone(), + segment_number: article.segment_number, + bytes: article.bytes, + downloaded: article.downloaded, + data_begin: article.data_begin, + data_size: article.data_size, + crc32: article.crc32, + tried_servers: article.tried_servers.clone(), + tries: article.tries, + }) + .collect(), + ) + }) + .collect(), + work_dir: Some(job.work_dir.clone()), + } +} + +fn apply_checkpoint(job: &mut NzbJob, checkpoint: &JobCheckpoint) { + job.downloaded_bytes = checkpoint.downloaded_bytes; + job.articles_downloaded = checkpoint.articles_downloaded; + job.articles_failed = checkpoint.articles_failed; + job.files_completed = checkpoint.files_completed; + for file in &mut job.files { + let outcomes = checkpoint.articles.get(&file.filename); + let segments = checkpoint + .files + .get(&file.filename) + .or_else(|| checkpoint.files.get(&file.id)); + let mut file_bytes: u64 = 0; + for article in &mut file.articles { + if let Some(outcome) = outcomes.and_then(|items| { + items.iter().find(|item| { + item.message_id == article.message_id + || item.segment_number == article.segment_number + }) + }) { + article.downloaded = outcome.downloaded; + article.data_begin = outcome.data_begin; + article.data_size = outcome.data_size; + article.crc32 = outcome.crc32; + article.tried_servers = outcome.tried_servers.clone(); + article.tries = outcome.tries; + } else if segments.is_some_and(|items| items.contains(&article.segment_number)) { + // Checkpoints written before article outcomes existed only + // recorded downloaded segment numbers. + article.downloaded = true; + } + if article.downloaded { + file_bytes = file_bytes.saturating_add(article.data_size.unwrap_or(article.bytes)); + } + } + file.bytes_downloaded = file_bytes; + file.assembled = file.articles.iter().all(|article| article.downloaded); + } } // --------------------------------------------------------------------------- @@ -715,12 +947,16 @@ pub struct QueueManager { add_tx: broadcast::Sender, /// Max concurrent active downloads (0 = unlimited). max_active_downloads: AtomicUsize, + /// Automatically keep the queue ordered by remaining percentage. + auto_sort_remaining_pct: AtomicBool, + /// Optional bounded post-processing hooks. + postproc_scripts: Mutex, /// Stage-specific resource gates shared by all post-processing jobs. postproc_resources: Arc, /// Category configs for post-processing decisions. categories: Mutex>, /// Minimum free disk space in bytes before pausing downloads. - min_free_space: u64, + min_free_space: AtomicU64, /// Bandwidth limiter for throttling downloads. bandwidth: Arc, /// Whether direct unpack (RAR extraction during download) is enabled. @@ -839,9 +1075,11 @@ impl QueueManager { log_buffer: Some(log_buffer), add_tx, max_active_downloads: AtomicUsize::new(max_active_downloads), + auto_sort_remaining_pct: AtomicBool::new(false), + postproc_scripts: Mutex::new(PostProcScriptConfig::default()), postproc_resources: PostProcResourcePool::new(postproc_limits), categories: Mutex::new(categories), - min_free_space, + min_free_space: AtomicU64::new(min_free_space), bandwidth, direct_unpack_enabled: AtomicBool::new(direct_unpack), max_nested_archive_depth, @@ -993,6 +1231,62 @@ impl QueueManager { self.start_next_queued(); } + /// Enable or disable automatic remaining-percentage ordering. + pub fn set_auto_sort_remaining_pct(&self, enabled: bool) { + self.auto_sort_remaining_pct + .store(enabled, Ordering::Relaxed); + } + + pub fn auto_sort_remaining_pct(&self) -> bool { + self.auto_sort_remaining_pct.load(Ordering::Relaxed) + } + + /// Configure the optional success and failure hooks used after + /// post-processing. Script paths are resolved and confined when a job + /// invokes them; keeping the raw config here allows live updates without + /// rebuilding the queue manager. + pub fn set_postproc_scripts( + &self, + scripts_dir: Option, + success: Option, + failure: Option, + timeout_secs: u64, + max_output_bytes: usize, + ) { + *self.postproc_scripts.lock() = PostProcScriptConfig { + scripts_dir, + success, + failure, + timeout: Duration::from_secs(timeout_secs.max(1)), + max_output_bytes, + }; + } + + /// Stable sort of the queue by remaining work percentage. The original + /// queue order is retained for equal percentages, which keeps repeated + /// manual sorts deterministic and avoids active-job churn. + pub fn sort_by_remaining_percentage(&self, ascending: bool) { + let jobs = self.jobs.lock(); + let mut order = self.job_order.lock(); + order.sort_by(|left, right| { + let remaining = |id: &String| { + jobs.get(id).map_or((0u64, 1u64), |state| { + let total = state.job.total_bytes.max(1); + (total.saturating_sub(state.job.downloaded_bytes), total) + }) + }; + let (left_remaining, left_total) = remaining(left); + let (right_remaining, right_total) = remaining(right); + let ordering = (left_remaining as u128 * right_total as u128) + .cmp(&(right_remaining as u128 * left_total as u128)); + if ascending { + ordering + } else { + ordering.reverse() + } + }); + } + /// Get max active downloads. pub fn get_max_active_downloads(&self) -> usize { self.max_active_downloads.load(Ordering::Relaxed) @@ -1095,6 +1389,32 @@ impl QueueManager { mut job: NzbJob, nzb_data: Option>, ) -> crate::nzb_core::Result<()> { + if crate::nzb_core::path::safe_component(&job.category).is_none() { + return Err(crate::nzb_core::NzbError::Other( + "category must be a single safe path component".to_string(), + )); + } + crate::nzb_core::path::safe_component(&job.name).ok_or_else(|| { + crate::nzb_core::NzbError::Other( + "job name must be a single safe path component".to_string(), + ) + })?; + let complete_root = self.complete_dir(); + let configured_root = self + .categories + .lock() + .iter() + .find(|category| category.name == job.category) + .and_then(|category| category.output_dir.clone()); + let output_is_allowed = job.output_dir.starts_with(&complete_root) + || configured_root + .as_ref() + .is_some_and(|root| job.output_dir.starts_with(root)); + if !output_is_allowed { + return Err(crate::nzb_core::NzbError::Other( + "job output directory is outside configured storage roots".to_string(), + )); + } // Ensure work directory exists std::fs::create_dir_all(&job.work_dir)?; @@ -1163,6 +1483,43 @@ impl QueueManager { Ok(()) } + /// Rebuild a history job for retry. Newer history rows carry a checkpoint + /// and retain a partial work directory when at least one article was + /// written, so the dispatcher can enqueue only unresolved articles and + /// append them to the existing assembled files. Older rows, and rows + /// whose partial directory is gone, deliberately fall back to a full + /// retry. + pub fn prepare_retry_job( + &self, + entry: &HistoryEntry, + nzb_data: &[u8], + retry_data: Option<&[u8]>, + ) -> crate::nzb_core::Result { + let mut job = nzb_parser::parse_nzb(&entry.name, nzb_data)?; + job.category = entry.category.clone(); + job.output_dir = self.output_dir_for(&job.category, &job.name)?; + job.work_dir = self.incomplete_dir().join(&job.id); + + if entry.status == JobStatus::Failed + && let Some(data) = retry_data + && let Ok(checkpoint) = serde_json::from_slice::(data) + && let Some(work_dir) = checkpoint.work_dir.as_ref() + && std::fs::canonicalize(self.incomplete_dir()) + .ok() + .zip(std::fs::canonicalize(work_dir).ok()) + .is_some_and(|(root, retained)| retained.starts_with(root)) + && std::fs::symlink_metadata(work_dir) + .map(|metadata| metadata.file_type().is_dir()) + .unwrap_or(false) + { + apply_checkpoint(&mut job, &checkpoint); + job.articles_failed = 0; + job.work_dir = work_dir.clone(); + } + + Ok(job) + } + /// Launch the download task for a job that is already in the jobs map /// with status `Downloading`. /// @@ -1187,29 +1544,7 @@ impl QueueManager { && let Ok(checkpoint) = serde_json::from_slice::(&cp_data) { - state.job.downloaded_bytes = checkpoint.downloaded_bytes; - state.job.articles_downloaded = checkpoint.articles_downloaded; - state.job.articles_failed = checkpoint.articles_failed; - state.job.files_completed = checkpoint.files_completed; - for file in &mut state.job.files { - let segments = checkpoint - .files - .get(&file.filename) - .or_else(|| checkpoint.files.get(&file.id)); - if let Some(segments) = segments { - let mut fbd: u64 = 0; - for article in &mut file.articles { - if segments.contains(&article.segment_number) { - article.downloaded = true; - fbd += article.bytes; - } - } - file.bytes_downloaded = fbd; - if file.articles.iter().all(|a| a.downloaded) { - file.assembled = true; - } - } - } + apply_checkpoint(&mut state.job, &checkpoint); info!( job_id = %job_id, name = %state.job.name, @@ -1237,13 +1572,18 @@ impl QueueManager { }; // Pre-flight disk space check - let free = get_disk_free(&self.incomplete_dir.lock()); - if self.min_free_space > 0 && free > 0 && free < self.min_free_space { + let incomplete_dir = self.incomplete_dir(); + let output_dir = job.output_dir.clone(); + let free = get_disk_free(&incomplete_dir); + if !disk_space_available( + self.min_free_space(), + [incomplete_dir.as_path(), output_dir.as_path()].as_slice(), + ) { warn!( job_id = %job_id, free_bytes = free, - min_free_space = self.min_free_space, - "Paused job due to low disk space" + min_free_space = self.min_free_space(), + "Paused job due to low disk space on a job storage volume" ); let mut jobs = self.jobs.lock(); if let Some(state) = jobs.get_mut(job_id) { @@ -1534,6 +1874,9 @@ impl QueueManager { self.persist_job_progress(&job_id); last_db_update = Instant::now(); } + if self.auto_sort_remaining_pct() { + self.sort_by_remaining_percentage(true); + } } ProgressUpdate::ArticleFailed { file_id, @@ -1547,6 +1890,23 @@ impl QueueManager { if let Some(state) = jobs.get_mut(&job_id) { state.job.articles_failed += 1; + if let Some(article) = state + .job + .files + .iter_mut() + .find(|file| file.id == file_id) + .and_then(|file| { + file.articles + .iter_mut() + .find(|article| article.segment_number == segment_number) + }) + { + article.tries = article.tries.saturating_add(1); + if !article.tried_servers.contains(&failure.server_id) { + article.tried_servers.push(failure.server_id.clone()); + } + } + // Update per-server failed stats let sid = &failure.server_id; let stats = &mut state.job.server_stats; @@ -1865,6 +2225,41 @@ impl QueueManager { ) }; + // Repair and extraction can write to both the incomplete and the + // category output volumes. Apply the same guard to both paths before + // any post-processing work begins. + if !disk_space_available( + self.min_free_space(), + [work_dir.as_path(), output_dir.as_path()].as_slice(), + ) { + let mut jobs = self.jobs.lock(); + if let Some(state) = jobs.get_mut(job_id) { + let message = "Insufficient free disk space for post-processing".to_string(); + state.job.status = JobStatus::Failed; + state.job.error_message = Some(message.clone()); + self.move_to_history( + state, + vec![StageResult { + name: "Disk".into(), + status: StageStatus::Failed, + message: Some(message), + duration_secs: 0.0, + }], + ); + } + drop(jobs); + self.persist_job_progress(job_id); + self.start_next_queued(); + let qm = Arc::clone(self); + let jid = job_id.to_string(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(8)).await; + qm.jobs.lock().remove(&jid); + qm.job_order.lock().retain(|id| id != &jid); + }); + return; + } + // Wait for direct unpack to finish (if active). It may still be // extracting the last volume when the download completes. let direct_unpack_success = if let Some(du) = direct_unpacker { @@ -1902,6 +2297,18 @@ impl QueueManager { "Running post-processing pipeline" ); + let (cleanup_patterns, unwanted_extensions) = self + .categories + .lock() + .iter() + .find(|configured| configured.name == category) + .map(|configured| { + ( + configured.cleanup_patterns.clone(), + configured.unwanted_extensions.clone(), + ) + }) + .unwrap_or_default(); let config = PostProcConfig { cleanup_after_extract: true, output_dir: Some(output_dir.clone()), @@ -1912,9 +2319,14 @@ impl QueueManager { max_nested_archive_depth: self.max_nested_archive_depth, }; - let result = - run_pipeline_with_resources(&work_dir, &config, Some(&self.postproc_resources)) - .await; + let result = run_pipeline_with_cleanup( + &work_dir, + &config, + Some(&self.postproc_resources), + &cleanup_patterns, + &unwanted_extensions, + ) + .await; info!( job_id = %job_id, @@ -1950,6 +2362,36 @@ impl QueueManager { Vec::new() }; + // Run the configured hook after the final status is known. Its output + // is bounded and the hook cannot change the job's filesystem roots. + let script_status = { + let jobs = self.jobs.lock(); + jobs.get(job_id).map(|state| { + if state.job.status == JobStatus::Failed { + JobStatus::Failed + } else { + JobStatus::Completed + } + }) + }; + let script_stage = match script_status { + Some(status) => self.run_postproc_script(job_id, status).await, + None => None, + }; + let mut stages = stages; + if let Some(stage) = script_stage { + if stage.status == StageStatus::Failed { + let mut jobs = self.jobs.lock(); + if let Some(state) = jobs.get_mut(job_id) { + state.job.status = JobStatus::Failed; + if state.job.error_message.is_none() { + state.job.error_message = stage.message.clone(); + } + } + } + stages.push(stage); + } + // Move to history with real stage results { let mut jobs = self.jobs.lock(); @@ -1974,6 +2416,123 @@ impl QueueManager { }); } + async fn run_postproc_script( + &self, + job_id: &str, + final_status: JobStatus, + ) -> Option { + let (job, script_config) = { + let jobs = self.jobs.lock(); + let state = jobs.get(job_id)?; + (state.job.clone(), self.postproc_scripts.lock().clone()) + }; + let configured = match final_status { + JobStatus::Completed => script_config.success, + JobStatus::Failed => script_config.failure, + _ => None, + }?; + let started = Instant::now(); + let script = match resolve_script_path(script_config.scripts_dir.as_deref(), &configured) { + Ok(path) => path, + Err(error) => { + return Some(StageResult { + name: "Script".into(), + status: StageStatus::Failed, + message: Some(format!("Unable to resolve post-processing script: {error}")), + duration_secs: started.elapsed().as_secs_f64(), + }); + } + }; + + let files = regular_output_files(&job.output_dir); + let file_list = files + .iter() + .map(|path| path.to_string_lossy()) + .collect::>() + .join("\n"); + let mut command = tokio::process::Command::new(&script); + command + .current_dir(&job.output_dir) + .env("SAB_STATUS", final_status.to_string()) + .env("SAB_JOB", &job.name) + .env("SAB_CAT", &job.category) + .env("SAB_FILENAME", &job.name) + .env("SAB_COMPLETE", &job.output_dir) + .env("SAB_BYTES", job.total_bytes.to_string()) + .env("SAB_BYTES_DOWNLOADED", job.downloaded_bytes.to_string()) + .env("SAB_FILES", file_list) + .env("RUSTNZB_STATUS", final_status.to_string()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .kill_on_drop(true); + let mut child = match command.spawn() { + Ok(child) => child, + Err(error) => { + return Some(StageResult { + name: "Script".into(), + status: StageStatus::Failed, + message: Some(format!("Unable to start post-processing script: {error}")), + duration_secs: started.elapsed().as_secs_f64(), + }); + } + }; + let stdout = child.stdout.take(); + let stderr = child.stderr.take(); + let max_output_bytes = script_config.max_output_bytes; + let result = async { + let stdout_reader = async { + match stdout { + Some(reader) => read_script_output(reader, max_output_bytes).await, + None => Ok((Vec::new(), false)), + } + }; + let stderr_reader = async { + match stderr { + Some(reader) => read_script_output(reader, max_output_bytes).await, + None => Ok((Vec::new(), false)), + } + }; + let (stdout, stderr) = tokio::join!(stdout_reader, stderr_reader); + let stdout = stdout?; + let stderr = stderr?; + let status = child.wait().await?; + Ok::<_, io::Error>((status, stdout, stderr)) + }; + + match tokio::time::timeout(script_config.timeout, result).await { + Ok(Ok((status, stdout, stderr))) if status.success() => Some(StageResult { + name: "Script".into(), + status: StageStatus::Success, + message: Some(script_output_message(&stdout, &stderr)), + duration_secs: started.elapsed().as_secs_f64(), + }), + Ok(Ok((status, stdout, stderr))) => Some(StageResult { + name: "Script".into(), + status: StageStatus::Failed, + message: Some(format!( + "Post-processing script exited with {status}: {}", + script_output_message(&stdout, &stderr) + )), + duration_secs: started.elapsed().as_secs_f64(), + }), + Ok(Err(error)) => Some(StageResult { + name: "Script".into(), + status: StageStatus::Failed, + message: Some(format!("Post-processing script failed: {error}")), + duration_secs: started.elapsed().as_secs_f64(), + }), + Err(_) => Some(StageResult { + name: "Script".into(), + status: StageStatus::Failed, + message: Some(format!( + "Post-processing script exceeded {} second timeout", + script_config.timeout.as_secs() + )), + duration_secs: started.elapsed().as_secs_f64(), + }), + } + } + /// Move a job's files to output and insert a history entry. fn move_to_history(&self, state: &mut JobState, mut stages: Vec) { let move_start = Instant::now(); @@ -2015,8 +2574,32 @@ impl QueueManager { if let Ok(entries) = std::fs::read_dir(&state.job.work_dir) { for entry in entries.flatten() { let path = entry.path(); - if path.is_file() { - let dest = state.job.output_dir.join(entry.file_name()); + let is_regular = std::fs::symlink_metadata(&path) + .map(|metadata| metadata.file_type().is_file()) + .unwrap_or(false); + let Some(dest) = crate::nzb_core::path::safe_join( + &state.job.output_dir, + &entry.file_name().to_string_lossy(), + ) else { + warn!( + job_id = %state.job.id, + file = %path.display(), + "Refusing to move file with an unsafe output name" + ); + continue; + }; + if is_regular { + if std::fs::symlink_metadata(&dest) + .map(|metadata| metadata.file_type().is_symlink()) + .unwrap_or(false) + { + warn!( + job_id = %state.job.id, + file = %dest.display(), + "Refusing to replace symlink in output directory" + ); + continue; + } if let Err(e) = std::fs::rename(&path, &dest) { if let Err(e2) = std::fs::copy(&path, &dest) { warn!( @@ -2045,6 +2628,7 @@ impl QueueManager { state.job.status = final_status; // Insert into history with real stage results + let retry_data = serde_json::to_vec(&checkpoint_for_job(&state.job)).ok(); let history_entry = HistoryEntry { id: state.job.id.clone(), name: state.job.name.clone(), @@ -2060,6 +2644,7 @@ impl QueueManager { error_message: state.job.error_message.clone(), server_stats: state.job.server_stats.clone(), nzb_data: state.nzb_data.clone(), + retry_data, }; let db = self.db.lock(); @@ -2112,7 +2697,19 @@ impl QueueManager { drop(db); if history_persisted { - cleanup_terminal_work_dir(&state.job.id, &state.job.work_dir, final_status); + let retain_for_retry = final_status == JobStatus::Failed + && state.nzb_data.is_some() + && state + .job + .files + .iter() + .any(|file| file.articles.iter().any(|article| article.downloaded)); + cleanup_terminal_work_dir( + &state.job.id, + &state.job.work_dir, + final_status, + retain_for_retry, + ); } else { warn!( job_id = %state.job.id, @@ -2140,30 +2737,7 @@ impl QueueManager { } // Build and store checkpoint of downloaded article segments - let checkpoint = JobCheckpoint { - files: state - .job - .files - .iter() - .map(|f| { - let downloaded_segments: Vec = f - .articles - .iter() - .filter(|a| a.downloaded) - .map(|a| a.segment_number) - .collect(); - // File IDs are generated while parsing an NZB and - // therefore change after a restart. The filename is - // stable across parses; accept the old ID key while - // reading checkpoints written by older versions. - (f.filename.clone(), downloaded_segments) - }) - .collect(), - downloaded_bytes: state.job.downloaded_bytes, - articles_downloaded: state.job.articles_downloaded, - articles_failed: state.job.articles_failed, - files_completed: state.job.files_completed, - }; + let checkpoint = checkpoint_for_job(&state.job); if let Ok(data) = serde_json::to_vec(&checkpoint) && let Err(e) = db.queue_store_job_data(job_id, &data) @@ -2509,6 +3083,7 @@ impl QueueManager { error_message: state.job.error_message.clone(), server_stats: state.job.server_stats.clone(), nzb_data: state.nzb_data.clone(), + retry_data: None, }; if let Err(e) = db.history_insert(&history_entry) { error!(job_id = %id, "Failed to insert history for removed failed job: {e}"); @@ -2549,7 +3124,14 @@ impl QueueManager { .find(|(_, s)| s.job.id == id || s.job.id.starts_with(id)); match state { Some((_, s)) => { + crate::nzb_core::path::safe_component(new_name).ok_or_else(|| { + crate::nzb_core::NzbError::Other( + "job name must be a single safe path component".to_string(), + ) + })?; + let output_dir = self.output_dir_for(&s.job.category, new_name)?; s.job.name = new_name.to_string(); + s.job.output_dir = output_dir; info!(job_id = %id, new_name = %new_name, "Job renamed"); Ok(()) } @@ -2559,6 +3141,19 @@ impl QueueManager { /// Change a job's category in the queue. pub fn change_job_category(&self, id: &str, category: &str) -> crate::nzb_core::Result<()> { + if crate::nzb_core::path::safe_component(category).is_none() { + return Err(crate::nzb_core::NzbError::Other( + "category must be a single safe path component".to_string(), + )); + } + let job_name = self + .jobs + .lock() + .iter() + .find(|(_, state)| state.job.id == id || state.job.id.starts_with(id)) + .map(|(_, state)| state.job.name.clone()) + .ok_or_else(|| crate::nzb_core::NzbError::JobNotFound(id.to_string()))?; + let output_dir = self.output_dir_for(category, &job_name)?; let mut jobs = self.jobs.lock(); let state = jobs .iter_mut() @@ -2567,8 +3162,7 @@ impl QueueManager { Some((_, s)) => { s.job.category = category.to_string(); // Update the output directory to match the new category - let complete_dir = self.complete_dir.lock().join(category).join(&s.job.name); - s.job.output_dir = complete_dir; + s.job.output_dir = output_dir; info!(job_id = %id, category = %category, "Job category changed"); Ok(()) } @@ -2853,9 +3447,79 @@ impl QueueManager { *self.complete_dir.lock() = dir; } + /// Resolve a category and job name to the configured output directory. + /// Both values originate from API/NZB input, so they must remain single + /// path components before they are joined to a trusted configured root. + pub fn output_dir_for( + &self, + category: &str, + name: &str, + ) -> crate::nzb_core::Result { + crate::nzb_core::path::safe_component(category).ok_or_else(|| { + crate::nzb_core::NzbError::Other("category must be a single safe path component".into()) + })?; + crate::nzb_core::path::safe_component(name).ok_or_else(|| { + crate::nzb_core::NzbError::Other("job name must be a single safe path component".into()) + })?; + + let categories = self.categories.lock(); + let category_config = categories + .iter() + .find(|configured| configured.name == category); + if let Some(base) = category_config.and_then(|configured| configured.output_dir.as_ref()) { + let root = if base.is_absolute() { + base.clone() + } else { + crate::nzb_core::path::safe_join(&self.complete_dir(), &base.to_string_lossy()) + .ok_or_else(|| { + crate::nzb_core::NzbError::Other("category output path is unsafe".into()) + })? + }; + return crate::nzb_core::path::safe_join(&root, name).ok_or_else(|| { + crate::nzb_core::NzbError::Other("category output path is unsafe".into()) + }); + } + let category_dir = crate::nzb_core::path::safe_join(&self.complete_dir(), category) + .ok_or_else(|| { + crate::nzb_core::NzbError::Other("category output path is unsafe".into()) + })?; + crate::nzb_core::path::safe_join(&category_dir, name) + .ok_or_else(|| crate::nzb_core::NzbError::Other("job output path is unsafe".into())) + } + + /// Return every configured filesystem root that may receive job data. + /// Relative category roots are resolved below the complete directory. + fn disk_guard_paths(&self) -> Vec { + let complete = self.complete_dir(); + let mut paths = vec![self.incomplete_dir(), complete.clone()]; + for category in self.categories.lock().iter() { + let Some(root) = category.output_dir.as_ref() else { + continue; + }; + let resolved = if root.is_absolute() { + root.clone() + } else if let Some(resolved) = + crate::nzb_core::path::safe_join(&complete, &root.to_string_lossy()) + { + resolved + } else { + continue; + }; + if !paths.contains(&resolved) { + paths.push(resolved); + } + } + paths + } + /// Get the minimum free disk space threshold. pub fn min_free_space(&self) -> u64 { - self.min_free_space + self.min_free_space.load(Ordering::Relaxed) + } + + /// Update the disk guard threshold for both preflight and periodic checks. + pub fn set_min_free_space(&self, bytes: u64) { + self.min_free_space.store(bytes, Ordering::Relaxed); } /// Lock the database and execute a closure with direct access. @@ -3100,6 +3764,12 @@ impl QueueManager { db.history_get_nzb_data(id) } + /// Get per-article retry outcomes persisted with a history entry. + pub fn history_get_retry_data(&self, id: &str) -> crate::nzb_core::Result>> { + let db = self.db.lock(); + db.history_get_retry_data(id) + } + /// Remove a history entry. pub fn history_remove(&self, id: &str) -> crate::nzb_core::Result<()> { let db = self.db.lock(); @@ -3197,6 +3867,12 @@ impl QueueManager { db.rss_items_prune(keep) } + /// Expire downloaded RSS records older than the supplied RFC3339 cutoff. + pub fn rss_items_expire_downloaded(&self, cutoff: &str) -> crate::nzb_core::Result { + let db = self.db.lock(); + db.rss_items_expire_downloaded(cutoff) + } + /// List all RSS download rules. pub fn rss_rule_list(&self) -> crate::nzb_core::Result> { let db = self.db.lock(); @@ -3311,30 +3987,7 @@ impl QueueManager { if let Some(ref data) = checkpoint_data { match serde_json::from_slice::(data) { Ok(checkpoint) => { - job.downloaded_bytes = checkpoint.downloaded_bytes; - job.articles_downloaded = checkpoint.articles_downloaded; - job.articles_failed = checkpoint.articles_failed; - job.files_completed = checkpoint.files_completed; - - for file in &mut job.files { - let segments = checkpoint - .files - .get(&file.filename) - .or_else(|| checkpoint.files.get(&file.id)); - if let Some(segments) = segments { - let mut file_bytes_downloaded: u64 = 0; - for article in &mut file.articles { - if segments.contains(&article.segment_number) { - article.downloaded = true; - file_bytes_downloaded += article.bytes; - } - } - file.bytes_downloaded = file_bytes_downloaded; - if file.articles.iter().all(|a| a.downloaded) { - file.assembled = true; - } - } - } + apply_checkpoint(&mut job, &checkpoint); let remaining = job .article_count @@ -3644,16 +4297,17 @@ impl QueueManager { info!(total_nntp_connections = total, "NNTP connection summary"); } } - if tick_count.is_multiple_of(30) && qm.min_free_space > 0 { - let free = get_disk_free(&qm.incomplete_dir.lock()); - if free > 0 - && free < qm.min_free_space + if tick_count.is_multiple_of(30) && qm.min_free_space() > 0 { + let paths = qm.disk_guard_paths(); + let path_refs: Vec<_> = paths.iter().map(std::path::PathBuf::as_path).collect(); + let free = paths.first().map_or(0, |path| get_disk_free(path)); + if !disk_space_available(qm.min_free_space(), &path_refs) && !qm.globally_paused.load(Ordering::Relaxed) { warn!( free_bytes = free, - min_free_space = qm.min_free_space, - "Low disk space, auto-pausing downloads" + min_free_space = qm.min_free_space(), + "Low disk space on a configured storage volume, auto-pausing downloads" ); qm.pause_all(); } @@ -3733,6 +4387,88 @@ mod global_pause_tests { manager.job_order.lock().push(id); } + #[tokio::test] + async fn remaining_percentage_sort_is_stable_and_does_not_change_status() { + let (manager, tempdir) = manager(); + let mut first = job("first", JobStatus::Downloading, tempdir.path()); + first.total_bytes = 100; + first.downloaded_bytes = 50; + let mut second = job("second", JobStatus::Queued, tempdir.path()); + second.total_bytes = 200; + second.downloaded_bytes = 100; + let mut third = job("third", JobStatus::Queued, tempdir.path()); + third.total_bytes = 100; + third.downloaded_bytes = 10; + insert_job(&manager, first); + insert_job(&manager, second); + insert_job(&manager, third); + + manager.sort_by_remaining_percentage(true); + assert_eq!( + manager + .job_order + .lock() + .iter() + .map(String::as_str) + .collect::>(), + vec!["first", "second", "third"] + ); + assert_eq!( + manager.get_job("first").unwrap().status, + JobStatus::Downloading + ); + + manager.sort_by_remaining_percentage(false); + assert_eq!( + manager + .job_order + .lock() + .iter() + .map(String::as_str) + .collect::>(), + vec!["third", "first", "second"] + ); + } + + #[test] + fn script_paths_are_confined_to_the_script_directory() { + let dir = tempfile::tempdir().unwrap(); + let scripts = dir.path().join("scripts"); + std::fs::create_dir_all(&scripts).unwrap(); + let script = scripts.join("success.sh"); + std::fs::write(&script, b"#!/bin/sh\nexit 0\n").unwrap(); + + let resolved = + resolve_script_path(Some(&scripts), std::path::Path::new("success.sh")).unwrap(); + assert_eq!(resolved, std::fs::canonicalize(script).unwrap()); + assert!(resolve_script_path(Some(&scripts), std::path::Path::new("../escape.sh")).is_err()); + assert!(resolve_script_path(None, std::path::Path::new("success.sh")).is_err()); + } + + #[tokio::test] + async fn script_output_is_bounded_and_captured() { + let (manager, tempdir) = manager(); + let script = tempdir.path().join("script.sh"); + std::fs::write(&script, b"#!/bin/sh\nprintf '1234567890'\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + manager.set_postproc_scripts(None, Some(script), None, 2, 4); + let mut completed = job("script-job", JobStatus::Completed, tempdir.path()); + completed.output_dir = tempdir.path().join("output"); + std::fs::create_dir_all(&completed.output_dir).unwrap(); + insert_job(&manager, completed); + + let stage = manager + .run_postproc_script("script-job", JobStatus::Completed) + .await + .unwrap(); + assert_eq!(stage.status, StageStatus::Success); + assert!(stage.message.unwrap().contains("output truncated")); + } + #[tokio::test] async fn active_queue_view_excludes_terminal_jobs() { let (manager, tempdir) = manager(); diff --git a/crates/nzb-web/src/rss_monitor.rs b/crates/nzb-web/src/rss_monitor.rs index 79f387e8..5ed8bcee 100644 --- a/crates/nzb-web/src/rss_monitor.rs +++ b/crates/nzb-web/src/rss_monitor.rs @@ -112,6 +112,15 @@ impl RssMonitor { info!(pruned, "Pruned old RSS items"); } + if let Some(days) = cfg.general.rss_downloaded_item_expiry_days { + let cutoff = (Utc::now() - chrono::Duration::days(days as i64)).to_rfc3339(); + if let Ok(expired) = self.queue_manager.rss_items_expire_downloaded(&cutoff) + && expired > 0 + { + info!(expired, "Expired downloaded RSS items"); + } + } + // Use the minimum poll interval across all enabled feeds, defaulting to 15 min let interval = feeds .iter() @@ -185,6 +194,14 @@ impl RssMonitor { .unwrap_or(0); let published_at = entry.published.or(entry.updated); + if let Some(max_age_days) = feed.max_age_days + && let Some(published_at) = published_at + && now.signed_duration_since(published_at).num_seconds() + > (max_age_days as i64).saturating_mul(86_400) + { + continue; + } + pending.push(PendingItem { item: RssItem { id: entry.id.clone(), @@ -477,6 +494,7 @@ mod tests { filter_regex: None, enabled: true, auto_download: true, + max_age_days: None, }; let error = monitor diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index eacdc493..f2939e28 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -189,7 +189,15 @@ async fn handle_addurl( let qm = &state.queue_manager; job.work_dir = qm.incomplete_dir().join(&job.id); - job.output_dir = qm.complete_dir().join(&job.category).join(&job.name); + job.output_dir = match qm.output_dir_for(&job.category, &job.name) { + Ok(path) => path, + Err(error) => { + return Ok(Json(serde_json::json!({ + "status": false, + "error": error.to_string() + }))); + } + }; let nzo_id = format!("SABnzbd_nzo_{}", &job.id[..12.min(job.id.len())]); let job_name = job.name.clone(); @@ -485,7 +493,15 @@ async fn dispatch_post( let qm = &state.queue_manager; job.work_dir = qm.incomplete_dir().join(&job.id); - job.output_dir = qm.complete_dir().join(&job.category).join(&job.name); + job.output_dir = match qm.output_dir_for(&job.category, &job.name) { + Ok(path) => path, + Err(error) => { + return Ok(Json(serde_json::json!({ + "status": false, + "error": error.to_string() + }))); + } + }; let nzo_id = format!("SABnzbd_nzo_{}", &job.id[..12.min(job.id.len())]); let job_name = job.name.clone(); @@ -592,7 +608,7 @@ fn dispatch_mode(state: &AppState, mode: &str, req: &SabApiRequest) -> Json filesystem.py::list_scripts). - "get_scripts" => Json(serde_json::json!({ "scripts": ["None"] })), + "get_scripts" => handle_get_scripts(state), "change_cat" => handle_change_cat(state, req), @@ -624,6 +640,31 @@ fn dispatch_mode(state: &AppState, mode: &str, req: &SabApiRequest) -> Json Json { + let config = state.config(); + let mut scripts = Vec::new(); + if let Some(directory) = config.general.scripts_dir.as_ref() + && let Ok(entries) = std::fs::read_dir(directory) + { + for entry in entries.flatten() { + let path = entry.path(); + if entry + .file_type() + .map(|kind| kind.is_file()) + .unwrap_or(false) + && path.file_name().and_then(|name| name.to_str()).is_some() + { + scripts.push(path.file_name().unwrap().to_string_lossy().into_owned()); + } + } + } + scripts.sort(); + if scripts.is_empty() { + scripts.push("None".to_string()); + } + Json(serde_json::json!({ "scripts": scripts })) +} + /// Return the stable subset of SABnzbd's full-status dashboard contract. /// /// SAB-compatible clients inspect this response as a capability/status @@ -720,6 +761,16 @@ fn handle_queue(state: &AppState, req: &SabApiRequest) -> Json return handle_queue_priority(state, req), Some("rename") => return handle_queue_rename(state, req), Some("purge") => return handle_queue_purge(state), + Some("sort") => { + let ascending = !matches!( + req.value.as_deref(), + Some(value) + if value.eq_ignore_ascii_case("descending") + || value.eq_ignore_ascii_case("desc") + ); + qm.sort_by_remaining_percentage(ascending); + return Json(serde_json::json!({ "status": true })); + } Some("change_complete_action") => return Json(serde_json::json!({ "status": true })), _ => {} } @@ -1410,7 +1461,16 @@ fn handle_retry(state: &AppState, req: &SabApiRequest) -> Json data, + Err(error) => { + return Json(serde_json::json!({ "status": false, "error": error.to_string() })); + } + }; + let job = match state + .queue_manager + .prepare_retry_job(&entry, &data, retry_data.as_deref()) + { Ok(job) => job, Err(error) => { return Json(serde_json::json!({ @@ -1419,13 +1479,6 @@ fn handle_retry(state: &AppState, req: &SabApiRequest) -> Json Date: Wed, 9 Sep 2026 22:36:59 +0000 Subject: [PATCH 3/5] chore: refresh desktop lockfile --- desktop/src-tauri/Cargo.lock | 39 +++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 63f94b69..61c29aea 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1421,6 +1421,16 @@ dependencies = [ "rustc_version", ] +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -3005,7 +3015,7 @@ dependencies = [ [[package]] name = "nzb-core" -version = "0.2.16" +version = "0.2.17" dependencies = [ "anyhow", "chrono", @@ -3082,12 +3092,13 @@ dependencies = [ [[package]] name = "nzb-postproc" -version = "0.2.6" +version = "0.2.7" dependencies = [ "anyhow", "nzb-core", "opentelemetry", "rust-par2", + "tar", "thiserror 2.0.18", "tokio", "tracing", @@ -3097,7 +3108,7 @@ dependencies = [ [[package]] name = "nzb-web" -version = "0.4.20" +version = "0.4.21" dependencies = [ "anyhow", "arc-swap", @@ -3106,6 +3117,7 @@ dependencies = [ "base64 0.23.1", "chrono", "feed-rs", + "flate2", "governor", "hex", "http", @@ -5101,6 +5113,17 @@ dependencies = [ "syn 2.0.118", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "target-lexicon" version = "0.12.16" @@ -6970,6 +6993,16 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix", +] + [[package]] name = "yenc-simd" version = "0.1.1" From 943d06a8dbdf95a55b4d945166ca0667a00e1e1f Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:08:22 +0000 Subject: [PATCH 4/5] fix: confine credential storage to data directory --- crates/nzb-web/src/auth.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/nzb-web/src/auth.rs b/crates/nzb-web/src/auth.rs index 15255825..585d5dcb 100644 --- a/crates/nzb-web/src/auth.rs +++ b/crates/nzb-web/src/auth.rs @@ -143,6 +143,13 @@ pub struct CredentialStore { impl CredentialStore { pub fn new(config_dir: PathBuf) -> Self { + // Startup creates the configured data directory before constructing + // this store. Canonicalizing it here confines the credential file to + // that existing directory and removes traversal or symlinked-parent + // ambiguity from the subsequent writes. + let config_dir = config_dir.canonicalize().unwrap_or_else(|error| { + panic!("credential store data directory must exist before startup: {error}") + }); let file_path = config_dir.join("credentials.json"); let credentials = if file_path.exists() { match std::fs::read_to_string(&file_path) { @@ -168,10 +175,6 @@ impl CredentialStore { pub fn set_credentials(&self, creds: StoredCredentials) -> Result<(), std::io::Error> { let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; - // Create parent directory if needed - if let Some(parent) = self.file_path.parent() { - std::fs::create_dir_all(parent)?; - } std::fs::write(&self.file_path, &json)?; // Set file permissions to owner-only on unix #[cfg(unix)] @@ -194,9 +197,6 @@ impl CredentialStore { )); } let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; - if let Some(parent) = self.file_path.parent() { - std::fs::create_dir_all(parent)?; - } std::fs::write(&self.file_path, &json)?; #[cfg(unix)] { From 67ed88e76fd0ddb739cee1bd2eaac8a438f37b64 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:26:40 +0000 Subject: [PATCH 5/5] fix: harden credential file writes --- crates/nzb-web/src/auth.rs | 62 ++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/crates/nzb-web/src/auth.rs b/crates/nzb-web/src/auth.rs index 585d5dcb..790ab4a7 100644 --- a/crates/nzb-web/src/auth.rs +++ b/crates/nzb-web/src/auth.rs @@ -3,6 +3,13 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::{Duration, Instant}; +#[cfg(unix)] +use std::fs::File; +#[cfg(unix)] +use std::io::Write; +#[cfg(unix)] +use std::os::fd::{AsRawFd, FromRawFd}; + use parking_lot::RwLock; use serde::{Deserialize, Serialize}; @@ -138,7 +145,10 @@ pub struct StoredCredentials { pub struct CredentialStore { credentials: RwLock>, + #[cfg(not(unix))] file_path: PathBuf, + #[cfg(unix)] + directory: File, } impl CredentialStore { @@ -151,6 +161,10 @@ impl CredentialStore { panic!("credential store data directory must exist before startup: {error}") }); let file_path = config_dir.join("credentials.json"); + #[cfg(unix)] + let directory = File::open(&config_dir).unwrap_or_else(|error| { + panic!("credential store data directory must be readable: {error}") + }); let credentials = if file_path.exists() { match std::fs::read_to_string(&file_path) { Ok(contents) => serde_json::from_str(&contents).ok(), @@ -161,7 +175,10 @@ impl CredentialStore { }; Self { credentials: RwLock::new(credentials), + #[cfg(not(unix))] file_path, + #[cfg(unix)] + directory, } } @@ -173,15 +190,41 @@ impl CredentialStore { self.credentials.read().clone() } - pub fn set_credentials(&self, creds: StoredCredentials) -> Result<(), std::io::Error> { - let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; - std::fs::write(&self.file_path, &json)?; - // Set file permissions to owner-only on unix + fn persist(&self, json: &[u8]) -> Result<(), std::io::Error> { #[cfg(unix)] { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&self.file_path, std::fs::Permissions::from_mode(0o600))?; + // The directory handle is opened from the canonical data + // directory at startup. The fixed filename never comes from a + // request or configuration value, and O_NOFOLLOW prevents a + // pre-existing credentials symlink from redirecting the write. + let flags = + libc::O_WRONLY | libc::O_CREAT | libc::O_TRUNC | libc::O_CLOEXEC | libc::O_NOFOLLOW; + let fd = unsafe { + libc::openat( + self.directory.as_raw_fd(), + c"credentials.json".as_ptr(), + flags, + 0o600, + ) + }; + if fd < 0 { + return Err(std::io::Error::last_os_error()); + } + let mut file = unsafe { File::from_raw_fd(fd) }; + file.write_all(json)?; + if unsafe { libc::fchmod(file.as_raw_fd(), 0o600) } != 0 { + return Err(std::io::Error::last_os_error()); + } + file.sync_all() } + + #[cfg(not(unix))] + std::fs::write(&self.file_path, json) + } + + pub fn set_credentials(&self, creds: StoredCredentials) -> Result<(), std::io::Error> { + let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; + self.persist(json.as_bytes())?; *self.credentials.write() = Some(creds); Ok(()) } @@ -197,12 +240,7 @@ impl CredentialStore { )); } let json = serde_json::to_string_pretty(&creds).map_err(std::io::Error::other)?; - std::fs::write(&self.file_path, &json)?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&self.file_path, std::fs::Permissions::from_mode(0o600))?; - } + self.persist(json.as_bytes())?; *current = Some(creds); Ok(()) }