diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index d27977e..0970d9f 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -2062,6 +2062,27 @@ pub async fn h_setup_apply( } } + // Reject relative complete_dir/incomplete_dir rather than silently persisting + // them: SABnzbd allows relative paths (resolved against its own working + // directory), but rustnzb creates these directories eagerly at startup via + // create_dir_all, which resolves a relative path against *its* process CWD + // (e.g. "/" in Docker) instead of the intended download volume — producing + // a confusing crash far away from the actual misconfiguration (see #62). + if let Some(ref dir) = preview.general.complete_dir + && !std::path::Path::new(dir).is_absolute() + { + return Err(ApiError::bad_request( + "cannot apply: complete_dir must be an absolute path", + )); + } + if let Some(ref dir) = preview.general.incomplete_dir + && !std::path::Path::new(dir).is_absolute() + { + return Err(ApiError::bad_request( + "cannot apply: incomplete_dir must be an absolute path", + )); + } + let mut config = (*state.config()).clone(); // Convert imported servers → ServerConfig with fresh UUIDs diff --git a/apps/rustnzb/tests/sabnzbd_import_apply_test.rs b/apps/rustnzb/tests/sabnzbd_import_apply_test.rs new file mode 100644 index 0000000..6cce164 --- /dev/null +++ b/apps/rustnzb/tests/sabnzbd_import_apply_test.rs @@ -0,0 +1,127 @@ +//! Regression tests for the SABnzbd import apply path (issue #62 follow-up): +//! a relative complete_dir/incomplete_dir must be rejected at apply time, +//! not silently written to config.toml where it later causes a confusing +//! startup crash. + +use std::sync::Arc; + +use arc_swap::ArcSwap; +use axum::Json; +use axum::extract::State; +use http::StatusCode; +use nzb_web::auth::{CredentialStore, TokenStore}; +use nzb_web::log_buffer::LogBuffer; +use nzb_web::nzb_core::config::AppConfig; +use nzb_web::nzb_core::db::Database; +use nzb_web::nzb_core::sabnzbd_import::SabnzbdImportPreview; +use nzb_web::queue_manager::QueueManager; +use nzb_web::state::AppState; +use rustnzb::handlers::h_setup_apply; + +fn test_state() -> (Arc, tempfile::TempDir) { + let tempdir = tempfile::tempdir().expect("tempdir"); + let log_buffer = LogBuffer::default(); + let manager = QueueManager::new( + Vec::new(), + Database::open_memory().expect("database"), + tempdir.path().join("incomplete"), + tempdir.path().join("complete"), + log_buffer.clone(), + 1, + Vec::new(), + 0, + 0, + false, + 5, + false, + false, + 100.0, + 30, + ); + let state = AppState::new( + Arc::new(ArcSwap::from_pointee(AppConfig::default())), + tempdir.path().join("config.toml"), + manager, + log_buffer, + Arc::new(TokenStore::new()), + Arc::new(CredentialStore::new(tempdir.path().to_path_buf())), + ); + (Arc::new(state), tempdir) +} + +fn preview_with_dirs(complete_dir: &str, incomplete_dir: &str) -> SabnzbdImportPreview { + serde_json::from_value(serde_json::json!({ + "servers": [], + "categories": [], + "general": { + "api_key": null, + "complete_dir": complete_dir, + "incomplete_dir": incomplete_dir, + "speed_limit_bps": 0 + }, + "rss_feeds": [], + "warnings": [], + "skipped_fields": [] + })) + .expect("preview JSON should deserialize") +} + +/// Negative: a relative complete_dir must be rejected, not persisted. +#[tokio::test] +async fn apply_rejects_relative_complete_dir() { + let (state, _tempdir) = test_state(); + let preview = preview_with_dirs("Downloads", "/downloads/incomplete"); + + let error = match h_setup_apply(State(state.clone()), Json(preview)).await { + Ok(_) => panic!("expected relative complete_dir to be rejected"), + Err(e) => e, + }; + + assert_eq!(error.status(), StatusCode::BAD_REQUEST); + assert!(error.to_string().contains("complete_dir")); + assert_eq!( + state.config().general.complete_dir, + AppConfig::default().general.complete_dir, + "rejected apply must not have mutated the persisted config" + ); +} + +/// Negative: a relative incomplete_dir must be rejected, not persisted. +#[tokio::test] +async fn apply_rejects_relative_incomplete_dir() { + let (state, _tempdir) = test_state(); + let preview = preview_with_dirs("/downloads/complete", "Downloads/incomplete"); + + let error = match h_setup_apply(State(state.clone()), Json(preview)).await { + Ok(_) => panic!("expected relative incomplete_dir to be rejected"), + Err(e) => e, + }; + + assert_eq!(error.status(), StatusCode::BAD_REQUEST); + assert!(error.to_string().contains("incomplete_dir")); + assert_eq!( + state.config().general.incomplete_dir, + AppConfig::default().general.incomplete_dir, + "rejected apply must not have mutated the persisted config" + ); +} + +/// Positive: absolute complete_dir/incomplete_dir are applied normally. +#[tokio::test] +async fn apply_accepts_absolute_dirs() { + let (state, _tempdir) = test_state(); + let preview = preview_with_dirs("/downloads/complete", "/downloads/incomplete"); + + h_setup_apply(State(state.clone()), Json(preview)) + .await + .expect("absolute dirs should be accepted"); + + assert_eq!( + state.config().general.complete_dir, + std::path::PathBuf::from("/downloads/complete") + ); + assert_eq!( + state.config().general.incomplete_dir, + std::path::PathBuf::from("/downloads/incomplete") + ); +} diff --git a/crates/nzb-core/src/sabnzbd_import.rs b/crates/nzb-core/src/sabnzbd_import.rs index a6a024e..beb48c6 100644 --- a/crates/nzb-core/src/sabnzbd_import.rs +++ b/crates/nzb-core/src/sabnzbd_import.rs @@ -79,6 +79,28 @@ pub struct ImportedGeneral { // Helpers // --------------------------------------------------------------------------- +/// Warn when an imported complete_dir/incomplete_dir isn't an absolute path. +/// rustnzb creates these directories eagerly at startup via `create_dir_all`, +/// which resolves a relative path against the process's working directory +/// rather than any intended download volume — applying such a path produces +/// a confusing crash far from the actual misconfiguration (see issue #62). +fn warn_relative_dirs(general: &ImportedGeneral, warnings: &mut Vec) { + if let Some(ref dir) = general.complete_dir + && !std::path::Path::new(dir).is_absolute() + { + warnings.push(format!( + "complete_dir '{dir}' is a relative path and must be made absolute before this import can be applied" + )); + } + if let Some(ref dir) = general.incomplete_dir + && !std::path::Path::new(dir).is_absolute() + { + warnings.push(format!( + "incomplete_dir '{dir}' is a relative path and must be made absolute before this import can be applied" + )); + } +} + /// Parse SABnzbd bandwidth limit string (e.g. "50M", "1G", "500K", "0", ""). pub fn parse_bandwidth_limit(s: &str) -> u64 { let s = s.trim().trim_matches('"'); @@ -188,6 +210,7 @@ pub fn parse_sabnzbd_ini(content: &str) -> SabnzbdImportPreview { .map(|s| parse_bandwidth_limit(s)) .unwrap_or(0), }; + warn_relative_dirs(&general, &mut warnings); // --- Servers (from [servers] → [[name]]) --- let servers: Vec = sections @@ -313,6 +336,7 @@ pub fn parse_sabnzbd_api_response(json: &serde_json::Value) -> SabnzbdImportPrev .or_else(|| misc["bandwidth_limit"].as_u64()) .unwrap_or(0), }; + warn_relative_dirs(&general, &mut warnings); // --- Servers --- let servers: Vec = config["servers"] @@ -548,6 +572,77 @@ mod tests { assert_eq!(preview.warnings.len(), 1); } + #[test] + fn ini_import_warns_on_relative_complete_and_incomplete_dir() { + let preview = parse_sabnzbd_ini( + r#" + [misc] + complete_dir = Downloads + download_dir = Downloads/incomplete + "#, + ); + + assert!( + preview + .warnings + .iter() + .any(|w| w.contains("complete_dir") && w.contains("Downloads")), + "expected a relative complete_dir warning, got: {:?}", + preview.warnings + ); + assert!( + preview + .warnings + .iter() + .any(|w| w.contains("incomplete_dir") && w.contains("Downloads/incomplete")), + "expected a relative incomplete_dir warning, got: {:?}", + preview.warnings + ); + } + + #[test] + fn ini_import_does_not_warn_on_absolute_complete_and_incomplete_dir() { + let preview = parse_sabnzbd_ini( + r#" + [misc] + complete_dir = /downloads/complete + download_dir = /downloads/incomplete + "#, + ); + + assert!( + preview.warnings.is_empty(), + "expected no warnings for absolute dirs, got: {:?}", + preview.warnings + ); + } + + #[test] + fn api_import_warns_on_relative_complete_and_incomplete_dir() { + let preview = parse_sabnzbd_api_response(&serde_json::json!({ + "config": { + "misc": { "complete_dir": "Downloads", "download_dir": "Downloads/incomplete" } + } + })); + + assert!( + preview + .warnings + .iter() + .any(|w| w.contains("complete_dir") && w.contains("Downloads")), + "expected a relative complete_dir warning, got: {:?}", + preview.warnings + ); + assert!( + preview + .warnings + .iter() + .any(|w| w.contains("incomplete_dir") && w.contains("Downloads/incomplete")), + "expected a relative incomplete_dir warning, got: {:?}", + preview.warnings + ); + } + proptest! { #[test] fn arbitrary_ini_input_never_panics(input in ".{0,4096}") {