From c85ae811504689b1cc69d125c0b7beba01d7ec6c Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 11 Aug 2026 22:59:21 +0000 Subject: [PATCH 1/2] test: reproduce obfuscated .NN RAR failure (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a paired reproduction suite for issue #87, where an obfuscated multi-volume RAR set named `<32 hex digits>.NN` is never extracted and the job still reports Completed. Tests are grouped by role: neg_* negative controls — conventional naming works. Green today; they isolate the defect to the filename and act as the regression guard for any fix. pos_* positive reproductions — the obfuscated set. These assert the desired behaviour and therefore FAIL today, so they are #[ignore]d to keep CI green. The fix removes the attributes. guard_* false-positive guards — a naive "any numeric extension is a RAR volume" fix breaks these. They must stay green. obfuscated_rar_volumes.rs covers the detection layer: find_archives returns nothing (Extract reports "No archives found" and skips), has_usable_output classifies raw volumes as payload (which is what lets move_to_history stamp Completed), the volumes are never cleanup candidates, and parse_rar_volume rejects bare numeric extensions. par2_deobfuscation_gate.rs covers the pipeline gate. rename_to_par2_names already recovers original filenames via first-16K MD5 matching, but it sits inside the verify branch, which is skipped when articles_failed==0. The suite is an A/B on identical inputs where only articles_failed differs: the damaged download deobfuscates correctly, the healthy one keeps its .NN names. Testing that required a parseable PAR2 index, and the repo has no .par2 fixtures while CI has no par2create; rust-par2 exposes no creation API. tests/support/par2_fixture.rs synthesises spec-correct Main and FileDesc packets to fill that gap. Packet MD5s must be exact — the parser silently skips mismatched packets — so the suite self-tests the fixture and verifies its 16K hashes against compute_hash_16k. No production code changes. cargo test -p nzb-postproc # 63 passed, 6 ignored cargo test -p nzb-postproc -- --ignored # 6 failed (the reproductions) Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 1 + crates/nzb-postproc/Cargo.toml | 5 + .../tests/obfuscated_rar_volumes.rs | 294 ++++++++++++++++++ .../tests/par2_deobfuscation_gate.rs | 210 +++++++++++++ crates/nzb-postproc/tests/support/mod.rs | 3 + .../tests/support/par2_fixture.rs | 155 +++++++++ 6 files changed, 668 insertions(+) create mode 100644 crates/nzb-postproc/tests/obfuscated_rar_volumes.rs create mode 100644 crates/nzb-postproc/tests/par2_deobfuscation_gate.rs create mode 100644 crates/nzb-postproc/tests/support/mod.rs create mode 100644 crates/nzb-postproc/tests/support/par2_fixture.rs diff --git a/Cargo.lock b/Cargo.lock index 73d2844..e73e462 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1952,6 +1952,7 @@ name = "nzb-postproc" version = "0.2.6" dependencies = [ "anyhow", + "md-5 0.10.6", "nzb-core", "opentelemetry", "rust-par2", diff --git a/crates/nzb-postproc/Cargo.toml b/crates/nzb-postproc/Cargo.toml index edc7f97..c69d3c6 100644 --- a/crates/nzb-postproc/Cargo.toml +++ b/crates/nzb-postproc/Cargo.toml @@ -24,6 +24,11 @@ zip = "8" [dev-dependencies] tempfile = "3" +# Integration tests synthesise real PAR2 index files (see +# `tests/support/par2_fixture.rs`), which requires writing spec-correct +# packet MD5s. Version tracks what `rust-par2` itself parses with. +md-5 = "0.10" +tokio = { version = "1", features = ["macros", "rt-multi-thread"] } [lints.clippy] all = { level = "warn", priority = -1 } diff --git a/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs new file mode 100644 index 0000000..6f16884 --- /dev/null +++ b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs @@ -0,0 +1,294 @@ +//! Reproduction suite for issue #87: obfuscated multi-volume RAR sets named +//! `<32 hex digits>.NN` are never detected, never extracted, and still let the +//! job report Completed. +//! +//! The suite is deliberately paired: +//! +//! * `neg_*` — **negative controls**. Conventional naming (`.rar`/`.r00`, +//! `.partNNN.rar`, `.7z.001`) and true payload. These pass on `main` and +//! prove the detection layer works when the filename carries an extension, +//! so a failure in a `pos_*` test is about the *name*, not the plumbing. +//! They are also the regression guard: any fix must keep these green. +//! +//! * `pos_*` — **positive reproductions**. The obfuscated `.NN` set. +//! These assert the behaviour we want and therefore FAIL on `main`. They are +//! `#[ignore]`d so the committed suite stays green; run them with +//! `cargo test -p nzb-postproc --test obfuscated_rar_volumes -- --ignored`. +//! The fix removes the `#[ignore]` attributes. +//! +//! * `guard_*` — **false-positive guards**. A naive fix ("treat any numeric +//! extension as a RAR volume") would break these. They pass on `main` and +//! must stay green. +//! +//! Fixtures carry real RAR signature bytes so that a content-sniffing fix has +//! something valid to sniff. + +use std::fs; +use std::path::Path; + +use nzb_postproc::ArchiveType; +use nzb_postproc::detect::{ + find_archives, find_cleanup_files, has_usable_output, parse_rar_volume, +}; + +/// RAR 4.x volume signature. +const RAR4_MAGIC: &[u8] = b"Rar!\x1a\x07\x00"; +/// RAR 5.x volume signature. +const RAR5_MAGIC: &[u8] = b"Rar!\x1a\x07\x01\x00"; + +/// The obfuscated set name from the issue report. +const HASH: &str = "cfd4be79c0fb01d429c52a9f8551ee79"; + +/// Write `files` (name, contents) into a fresh temp dir. +fn make_dir(files: &[(&str, &[u8])]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + for (name, contents) in files { + fs::write(dir.path().join(name), contents).unwrap(); + } + dir +} + +/// A RAR volume body: signature + filler so the file is not empty. +fn rar_volume(magic: &[u8]) -> Vec { + let mut body = magic.to_vec(); + body.extend_from_slice(&[0u8; 64]); + body +} + +/// Count the RAR entries `find_archives` reported. +fn rar_archives(dir: &Path) -> Vec { + find_archives(dir) + .into_iter() + .filter(|(kind, _)| *kind == ArchiveType::Rar) + .map(|(_, path)| path.file_name().unwrap().to_string_lossy().into_owned()) + .collect() +} + +/// Cleanup candidates as bare filenames. +fn cleanup_names(dir: &Path) -> Vec { + find_cleanup_files(dir) + .into_iter() + .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()) + .collect() +} + +// --------------------------------------------------------------------------- +// Negative controls — conventional naming. Green on `main`, must stay green. +// --------------------------------------------------------------------------- + +#[test] +fn neg_old_style_rar_set_is_detected_extracted_and_cleaned() { + let body = rar_volume(RAR4_MAGIC); + let dir = make_dir(&[ + ("movie.rar", &body), + ("movie.r00", &body), + ("movie.r01", &body), + ("movie.par2", b""), + ]); + + // Detected, and only the first volume is offered for extraction. + assert_eq!( + rar_archives(dir.path()), + vec!["movie.rar".to_string()], + "old-style set should yield exactly the first volume" + ); + + // Every volume is a cleanup candidate. + let cleanup = cleanup_names(dir.path()); + for name in ["movie.rar", "movie.r00", "movie.r01", "movie.par2"] { + assert!( + cleanup.contains(&name.to_string()), + "{name} should be cleanup" + ); + } + + // Raw archives alone are not a usable completion. + assert!( + !has_usable_output(dir.path()).unwrap(), + "a directory of raw volumes must not count as usable output" + ); + + // Volume numbering is understood. + assert_eq!(parse_rar_volume("movie.rar").unwrap().volume_number, 0); + assert_eq!(parse_rar_volume("movie.r00").unwrap().volume_number, 1); + assert_eq!(parse_rar_volume("movie.r01").unwrap().volume_number, 2); +} + +#[test] +fn neg_new_style_part_set_is_detected() { + let body = rar_volume(RAR5_MAGIC); + let dir = make_dir(&[ + ("movie.part001.rar", &body), + ("movie.part002.rar", &body), + ("movie.part003.rar", &body), + ]); + + assert_eq!( + rar_archives(dir.path()), + vec!["movie.part001.rar".to_string()], + "new-style set should yield exactly part001" + ); + assert!(!has_usable_output(dir.path()).unwrap()); + + let parsed = parse_rar_volume("movie.part002.rar").unwrap(); + assert_eq!(parsed.set_name, "movie"); + assert_eq!(parsed.volume_number, 1); +} + +#[test] +fn neg_split_7z_set_is_detected() { + let dir = make_dir(&[ + ("archive.7z.001", b"7z\xbc\xaf\x27\x1c"), + ("archive.7z.002", b"data"), + ]); + + let archives = find_archives(dir.path()); + assert_eq!(archives.len(), 1, "only the .001 volume starts extraction"); + assert_eq!(archives[0].0, ArchiveType::SevenZip); + assert!(!has_usable_output(dir.path()).unwrap()); +} + +#[test] +fn neg_real_payload_counts_as_usable_output() { + let dir = make_dir(&[("Movie.2024.1080p.mkv", b"\x1a\x45\xdf\xa3matroska")]); + assert!( + has_usable_output(dir.path()).unwrap(), + "extracted media must count as usable output" + ); + assert!( + cleanup_names(dir.path()).is_empty(), + "payload must never be a cleanup candidate" + ); +} + +// --------------------------------------------------------------------------- +// Positive reproductions — obfuscated `.NN`. RED on `main` (issue #87). +// --------------------------------------------------------------------------- + +/// Build the obfuscated set from the issue: `.45`, `.46`, `.47` + par2. +fn obfuscated_dir() -> tempfile::TempDir { + let body = rar_volume(RAR4_MAGIC); + let dir = tempfile::tempdir().unwrap(); + for n in 45..=47u32 { + fs::write(dir.path().join(format!("{HASH}.{n}")), &body).unwrap(); + } + fs::write(dir.path().join(format!("{HASH}.par2")), b"").unwrap(); + dir +} + +#[test] +#[ignore = "reproduces issue #87: .NN volumes are not detected as a RAR set"] +fn pos_obfuscated_numeric_volumes_are_detected_as_rar() { + let dir = obfuscated_dir(); + let archives = rar_archives(dir.path()); + assert!( + !archives.is_empty(), + "obfuscated RAR set went undetected; find_archives returned nothing, \ + so the Extract stage reports \"No archives found\" and skips" + ); + assert_eq!( + archives.len(), + 1, + "exactly one volume should start extraction, got {archives:?}" + ); +} + +#[test] +#[ignore = "reproduces issue #87: has_usable_output treats raw volumes as payload"] +fn pos_obfuscated_volumes_are_not_usable_output() { + let dir = obfuscated_dir(); + assert!( + !has_usable_output(dir.path()).unwrap(), + "raw .NN volumes were reported as usable payload, which is what \ + lets move_to_history stamp the job Completed" + ); +} + +#[test] +#[ignore = "reproduces issue #87: .NN volumes are never cleaned up"] +fn pos_obfuscated_volumes_are_cleanup_candidates() { + let dir = obfuscated_dir(); + let cleanup = cleanup_names(dir.path()); + for n in 45..=47u32 { + let name = format!("{HASH}.{n}"); + assert!( + cleanup.contains(&name), + "{name} should be a cleanup candidate; got {cleanup:?}" + ); + } +} + +#[test] +#[ignore = "reproduces issue #87: parse_rar_volume rejects bare numeric extensions"] +fn pos_obfuscated_volume_numbers_are_parsed() { + let parsed = + parse_rar_volume(&format!("{HASH}.45")).expect("bare numeric RAR volume should parse"); + assert_eq!(parsed.set_name, HASH); + + let next = + parse_rar_volume(&format!("{HASH}.46")).expect("bare numeric RAR volume should parse"); + assert_eq!( + next.volume_number, + parsed.volume_number + 1, + "consecutive numeric volumes must order consecutively" + ); +} + +#[test] +#[ignore = "reproduces issue #87: RAR5-signed obfuscated volumes are equally invisible"] +fn pos_obfuscated_rar5_volumes_are_detected() { + let body = rar_volume(RAR5_MAGIC); + let dir = tempfile::tempdir().unwrap(); + for n in 1..=3u32 { + fs::write(dir.path().join(format!("{HASH}.{n:02}")), &body).unwrap(); + } + assert!( + !rar_archives(dir.path()).is_empty(), + "RAR5 obfuscated set went undetected" + ); +} + +// --------------------------------------------------------------------------- +// False-positive guards — a naive "any numeric extension is a RAR volume" fix +// breaks these. Green on `main`, must stay green. +// --------------------------------------------------------------------------- + +#[test] +fn guard_numeric_suffix_without_rar_signature_is_not_an_archive() { + // Looks like `.NN` but holds no RAR signature — e.g. a stray data + // file. Detection must not claim it. + let dir = make_dir(&[ + ( + "Concert.Recording.1987", + b"plain text notes, not an archive", + ), + ("logfile.01", b"2026-08-11 INFO started"), + ]); + assert!( + rar_archives(dir.path()).is_empty(), + "non-RAR numeric-suffixed files must not be detected as archives" + ); +} + +#[test] +fn guard_split_7z_volumes_are_not_reclassified_as_rar() { + // `.001` is numeric too. It must stay a 7z volume, not become a RAR set, + // or extraction hands the wrong file to the wrong extractor. + let dir = make_dir(&[ + ("archive.7z.001", b"7z\xbc\xaf\x27\x1c"), + ("archive.7z.002", b"more data"), + ]); + assert!( + rar_archives(dir.path()).is_empty(), + "split 7z volumes must not be classified as RAR" + ); + assert_eq!(find_archives(dir.path()).len(), 1); +} + +#[test] +fn guard_media_file_with_year_suffix_stays_payload() { + // `has_usable_output` must not start discarding real payload just because + // the name ends in digits. + let dir = make_dir(&[("Movie.Title.2024.mkv", b"\x1a\x45\xdf\xa3matroska")]); + assert!(has_usable_output(dir.path()).unwrap()); +} diff --git a/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs new file mode 100644 index 0000000..a07f41e --- /dev/null +++ b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs @@ -0,0 +1,210 @@ +//! Reproduction suite for the second half of issue #87: the PAR2-guided +//! deobfuscation that *would* fix obfuscated names never runs on a healthy +//! download. +//! +//! `rename_to_par2_names` (pipeline.rs) recovers original filenames by matching +//! each file's first-16K MD5 against the PAR2 metadata — exactly what an +//! obfuscated `.NN` set needs. But it is called from inside the verify +//! branch, and verification is skipped outright when `articles_failed == 0` +//! ("files are known-good from CRC checks"). So the one mechanism that can +//! deobfuscate the set only runs when the download was *damaged*. +//! +//! The pairing mirrors `obfuscated_rar_volumes.rs`: +//! +//! * `neg_*` — the damaged path, where the rename does run. Green on `main`; +//! proves the rename machinery itself works and is the regression guard. +//! * `pos_*` — the healthy path. Same files, same PAR2, only +//! `articles_failed` differs. FAILS on `main`; `#[ignore]`d so CI stays +//! green. Run with `-- --ignored`; the fix removes the attribute. +//! * `guard_*` — must not regress when the gate is lifted. +//! +//! Extraction and cleanup are disabled throughout so the assertions are about +//! filenames on disk and nothing else — these tests need no `unrar` binary. + +mod support; + +use std::fs; +use std::path::Path; + +use nzb_postproc::{PostProcConfig, run_pipeline}; +use support::par2_fixture::Par2Fixture; + +/// The obfuscated set name from the issue report. +const HASH: &str = "cfd4be79c0fb01d429c52a9f8551ee79"; + +/// Canonical names PAR2 knows the volumes by. +const CANONICAL: [&str; 3] = [ + "Movie.Title.2024.part001.rar", + "Movie.Title.2024.part002.rar", + "Movie.Title.2024.part003.rar", +]; + +/// Volume bodies: real RAR signature plus per-volume filler so each file has a +/// distinct 16K hash and matching is unambiguous. +fn volume_bodies() -> Vec> { + (0..3u8) + .map(|n| { + let mut body = b"Rar!\x1a\x07\x00".to_vec(); + body.extend_from_slice(&[n; 512]); + body + }) + .collect() +} + +/// A job directory holding the three volumes under `names`, plus a PAR2 index +/// that records them under their canonical names. +fn job_dir(names: &[String]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + let bodies = volume_bodies(); + + let mut fixture = Par2Fixture::new(); + for (canonical, body) in CANONICAL.iter().zip(&bodies) { + fixture = fixture.add_file(canonical, body); + } + fixture.write_index(&dir.path().join("Movie.Title.2024.par2")); + + for (name, body) in names.iter().zip(&bodies) { + fs::write(dir.path().join(name), body).unwrap(); + } + dir +} + +/// The obfuscated layout: `.45`, `.46`, `.47`. +fn obfuscated_names() -> Vec { + (45..=47u32).map(|n| format!("{HASH}.{n}")).collect() +} + +/// Pipeline config that isolates the verify/rename stage. +fn config(articles_failed: usize) -> PostProcConfig { + PostProcConfig { + articles_failed, + skip_extract: true, + cleanup_after_extract: false, + ..Default::default() + } +} + +/// Sorted non-par2 filenames in `dir`. +fn names_on_disk(dir: &Path) -> Vec { + let mut names: Vec = fs::read_dir(dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| !n.to_lowercase().ends_with(".par2")) + .collect(); + names.sort(); + names +} + +// --------------------------------------------------------------------------- +// Fixture self-test — if this breaks, every result below is meaningless. +// --------------------------------------------------------------------------- + +#[test] +fn neg_par2_fixture_is_parseable_and_names_the_files() { + let dir = job_dir(&obfuscated_names()); + let index = dir.path().join("Movie.Title.2024.par2"); + + let file_set = rust_par2::parse(&index).expect("synthesised PAR2 index must parse"); + let mut names: Vec = file_set + .files + .values() + .map(|f| f.filename.clone()) + .collect(); + names.sort(); + assert_eq!( + names, + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "synthesised PAR2 index must report the canonical filenames" + ); + + // The 16K hashes must match what the rename path computes, or matching + // would silently never fire and the gate tests would prove nothing. + for (canonical, body) in CANONICAL.iter().zip(volume_bodies()) { + let probe = dir.path().join("probe.bin"); + fs::write(&probe, &body).unwrap(); + let hash = rust_par2::compute_hash_16k(&probe).unwrap(); + assert!( + file_set + .files + .values() + .any(|f| f.filename == *canonical && f.hash_16k == hash), + "16K hash recorded for {canonical} must match compute_hash_16k" + ); + fs::remove_file(&probe).unwrap(); + } +} + +// --------------------------------------------------------------------------- +// Negative control — damaged download. Rename runs. Green on `main`. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn neg_damaged_download_deobfuscates_via_par2() { + let dir = job_dir(&obfuscated_names()); + + // articles_failed > 0 → verify branch runs → rename_to_par2_names runs. + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert_eq!( + names_on_disk(dir.path()), + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "with articles_failed > 0 the PAR2-guided rename should restore the \ + canonical names" + ); +} + +// --------------------------------------------------------------------------- +// Positive reproduction — healthy download. RED on `main` (issue #87). +// --------------------------------------------------------------------------- + +#[tokio::test] +#[ignore = "reproduces issue #87: PAR2-guided rename is gated behind articles_failed > 0"] +async fn pos_healthy_download_deobfuscates_via_par2() { + let dir = job_dir(&obfuscated_names()); + + // Identical inputs to the negative control; only articles_failed differs. + let _ = run_pipeline(dir.path(), &config(0)).await; + + assert_eq!( + names_on_disk(dir.path()), + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "a clean download of an obfuscated post kept its .NN names: \ + verification is skipped when articles_failed == 0, and the PAR2 \ + rename is nested inside that skipped branch, so the set is never \ + deobfuscated and the Extract stage finds no archives" + ); +} + +// --------------------------------------------------------------------------- +// Guards — must stay green once the gate is lifted. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn guard_correctly_named_files_are_left_alone() { + let canonical: Vec = CANONICAL.iter().map(|s| s.to_string()).collect(); + let dir = job_dir(&canonical); + + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert_eq!( + names_on_disk(dir.path()), + canonical, + "files already matching PAR2 must not be renamed" + ); +} + +#[tokio::test] +async fn guard_unrelated_files_are_not_renamed() { + // A file PAR2 knows nothing about must keep its name — lifting the gate + // must not turn the rename into a free-for-all over the job directory. + let dir = job_dir(&obfuscated_names()); + fs::write(dir.path().join("readme.nfo"), b"release notes").unwrap(); + + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert!( + dir.path().join("readme.nfo").exists(), + "a file absent from the PAR2 set must be left untouched" + ); +} diff --git a/crates/nzb-postproc/tests/support/mod.rs b/crates/nzb-postproc/tests/support/mod.rs new file mode 100644 index 0000000..c226ad9 --- /dev/null +++ b/crates/nzb-postproc/tests/support/mod.rs @@ -0,0 +1,3 @@ +//! Shared test support for `nzb-postproc` integration tests. + +pub mod par2_fixture; diff --git a/crates/nzb-postproc/tests/support/par2_fixture.rs b/crates/nzb-postproc/tests/support/par2_fixture.rs new file mode 100644 index 0000000..73e85da --- /dev/null +++ b/crates/nzb-postproc/tests/support/par2_fixture.rs @@ -0,0 +1,155 @@ +//! Synthesise real, parseable PAR2 index files for integration tests. +//! +//! The repo has no `.par2` fixtures and no `par2create` binary in CI, which +//! previously made the PAR2-guided deobfuscation path (`rename_to_par2_names`) +//! untestable end-to-end. This builder writes spec-correct PAR2 packets so +//! `rust_par2::parse` accepts them and the pipeline exercises the real code +//! path rather than a mock. +//! +//! Only the packets the pipeline actually reads are emitted: +//! +//! * **Main** — carries the slice size; `parse` returns `NoMainPacket` without it. +//! * **FileDesc** — one per file: file ID, full-file MD5, first-16K MD5, size, +//! and the *expected* filename. The 16K hash is what `rename_to_par2_names` +//! matches obfuscated files against. +//! +//! Recovery (`RecvSlic`) and slice-checksum (`IFSC`) packets are omitted: they +//! only matter for actual repair, which these tests never reach. Verification +//! will therefore report files as damaged — that is fine and expected, because +//! the assertions are about *filenames on disk*, not repair outcomes. +//! +//! Packet layout implemented here (little-endian), per the PAR 2.0 spec: +//! +//! ```text +//! 0..8 magic "PAR2\0PKT" +//! 8..16 packet length (u64, whole packet, multiple of 4) +//! 16..32 MD5 of everything from offset 32 onward +//! 32..48 recovery set ID +//! 48..64 packet type +//! 64.. body +//! ``` + +use std::path::Path; + +use md5::{Digest, Md5}; + +const MAGIC: &[u8; 8] = b"PAR2\x00PKT"; +const TYPE_MAIN: &[u8; 16] = b"PAR 2.0\x00Main\x00\x00\x00\x00"; +const TYPE_FILE_DESC: &[u8; 16] = b"PAR 2.0\x00FileDesc"; + +/// One file recorded in the recovery set. +struct FileEntry { + /// The name PAR2 considers canonical — what a rename should restore. + expected_name: String, + /// File ID (spec: MD5 of hash_16k + size + name). + file_id: [u8; 16], + hash: [u8; 16], + hash_16k: [u8; 16], + size: u64, +} + +/// Builds a PAR2 index file describing a set of files by content. +pub struct Par2Fixture { + slice_size: u64, + recovery_set_id: [u8; 16], + files: Vec, +} + +impl Par2Fixture { + /// Start a new recovery set. `slice_size` must be a multiple of 4. + pub fn new() -> Self { + Self { + slice_size: 4096, + // Fixed, not random: tests must be deterministic. + recovery_set_id: *b"rustnzbfixture01", + files: Vec::new(), + } + } + + /// Record `contents` under the canonical name PAR2 should report. + /// + /// This does not write the file to disk — the test decides whether to + /// place it under its canonical name or an obfuscated one. + pub fn add_file(mut self, expected_name: &str, contents: &[u8]) -> Self { + let hash: [u8; 16] = Md5::digest(contents).into(); + // Mirrors `rust_par2::compute_hash_16k`: MD5 of the first 16 KiB. + let head = &contents[..contents.len().min(16384)]; + let hash_16k: [u8; 16] = Md5::digest(head).into(); + let size = contents.len() as u64; + + // Spec: File ID = MD5(hash_16k || size_le || name). + let mut id_input = Vec::new(); + id_input.extend_from_slice(&hash_16k); + id_input.extend_from_slice(&size.to_le_bytes()); + id_input.extend_from_slice(expected_name.as_bytes()); + let file_id: [u8; 16] = Md5::digest(&id_input).into(); + + self.files.push(FileEntry { + expected_name: expected_name.to_string(), + file_id, + hash, + hash_16k, + size, + }); + self + } + + /// Write the index `.par2` file to `path`. + pub fn write_index(&self, path: &Path) { + let mut out = Vec::new(); + + // Main packet: slice size, file count, then the file IDs in order. + let mut main_body = Vec::new(); + main_body.extend_from_slice(&self.slice_size.to_le_bytes()); + main_body.extend_from_slice(&(self.files.len() as u32).to_le_bytes()); + for file in &self.files { + main_body.extend_from_slice(&file.file_id); + } + out.extend_from_slice(&self.packet(TYPE_MAIN, &main_body)); + + // One FileDesc packet per file. + for file in &self.files { + let mut body = Vec::new(); + body.extend_from_slice(&file.file_id); + body.extend_from_slice(&file.hash); + body.extend_from_slice(&file.hash_16k); + body.extend_from_slice(&file.size.to_le_bytes()); + body.extend_from_slice(file.expected_name.as_bytes()); + // Filename is null-padded to a multiple of 4 so the packet length + // stays 4-aligned; the parser rejects lengths that are not. + while body.len() % 4 != 0 { + body.push(0); + } + out.extend_from_slice(&self.packet(TYPE_FILE_DESC, &body)); + } + + std::fs::write(path, &out).unwrap(); + } + + /// Frame `body` as a PAR2 packet with a correct length and MD5. + /// + /// The parser recomputes the MD5 over everything from offset 32 and skips + /// packets that don't match, so this must be exact — a wrong hash makes + /// the fixture silently empty rather than failing loudly. + fn packet(&self, packet_type: &[u8; 16], body: &[u8]) -> Vec { + // `data` is what the MD5 covers: set ID + type + body. + let mut data = Vec::with_capacity(32 + body.len()); + data.extend_from_slice(&self.recovery_set_id); + data.extend_from_slice(packet_type); + data.extend_from_slice(body); + + let packet_len = (32 + data.len()) as u64; + assert!( + packet_len % 4 == 0, + "PAR2 packet length must be 4-aligned, got {packet_len}" + ); + let md5: [u8; 16] = Md5::digest(&data).into(); + + let mut packet = Vec::with_capacity(packet_len as usize); + packet.extend_from_slice(MAGIC); + packet.extend_from_slice(&packet_len.to_le_bytes()); + packet.extend_from_slice(&md5); + packet.extend_from_slice(&data); + packet + } +} From 8cb5cc9c8687a71a8adab2717eb43553b6158049 Mon Sep 17 00:00:00 2001 From: TheDancingDeveloper Date: Tue, 11 Aug 2026 23:10:30 +0000 Subject: [PATCH 2/2] fix: detect and deobfuscate .NN RAR volumes (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Obfuscated multi-volume RAR sets named `<32 hex digits>.NN` were never extracted, and the job still reported Completed with raw volumes left in the output directory. Two independent causes, both fixed here; the tests added in the previous commit are un-ignored and now pass. 1. PAR2-guided deobfuscation was gated behind damage. `rename_to_par2_names` recovers original filenames by matching each file's first-16K MD5 against PAR2 metadata — exactly what an obfuscated set needs. It was called only from inside the verify branch, and verification is skipped when articles_failed == 0 because the files are already CRC-verified. So a *damaged* download self-healed while a clean one kept its meaningless names. The rename now runs whenever the PAR2 index parses, independent of the verify decision. It is a cheap MD5 pass over file heads, not a full verification. This alone fixes the reported case for any post shipping a PAR2 set. 2. Detection matched extensions only. find_archives, find_cleanup_files and has_usable_output all keyed off filename patterns, so a bare numeric extension was invisible: no archive was found (Extract reported "No archives found" and skipped), the volumes were not cleanup candidates, and has_usable_output — defined as "anything not recognised as junk is payload" — classified them as usable output, which let move_to_history stamp Completed. Detection now reads the RAR4/RAR5 signature. Extension-widening alone would have been wrong: it misclassifies split 7z volumes and any file that merely ends in digits, so content is the deciding evidence. - has_rar_signature(path) new, public - parse_rar_volume_at(path) new, public; name-based parse plus a bare-numeric fallback gated on the signature - find_rar_files groups obfuscated volumes by stem and hands unrar the lowest-numbered one - is_cleanup_candidate_at path-aware cleanup/payload decision parse_rar_volume keeps its name-only contract and still rejects bare numerics, so the direct-unpack path in queue_manager is unchanged and cannot be fed a non-archive. Deliberately not adopted there yet: direct unpack simply skips obfuscated sets and post-processing handles them. Not addressed here: has_usable_output remains a denylist ("unrecognised => payload"). Inverting it to an allowlist would make future unknown obfuscation schemes fail loudly rather than silently, but it changes completion semantics for legitimately extensionless payload and deserves its own change. cargo test -p nzb-postproc # 70 passed, 0 ignored cargo test --workspace # green cargo fmt --all / cargo clippy --workspace --all-targets # clean Closes #87 Co-Authored-By: Claude Opus 5 (1M context) --- crates/nzb-postproc/src/detect.rs | 127 +++++++++++++++++- crates/nzb-postproc/src/lib.rs | 5 +- crates/nzb-postproc/src/pipeline.rs | 30 ++++- .../tests/obfuscated_rar_volumes.rs | 67 +++++---- .../tests/par2_deobfuscation_gate.rs | 41 +++--- 5 files changed, 214 insertions(+), 56 deletions(-) diff --git a/crates/nzb-postproc/src/detect.rs b/crates/nzb-postproc/src/detect.rs index 5d842d5..5ce4ba6 100644 --- a/crates/nzb-postproc/src/detect.rs +++ b/crates/nzb-postproc/src/detect.rs @@ -3,10 +3,58 @@ //! Scans a completed download directory to find par2 files, RAR archives, //! 7z archives, ZIP archives, and cleanup candidates. +use std::collections::BTreeMap; +use std::io::Read; use std::path::{Path, PathBuf}; use walkdir::WalkDir; +/// RAR 4.x volume signature. +const RAR4_SIGNATURE: &[u8] = b"Rar!\x1a\x07\x00"; +/// RAR 5.x volume signature. +const RAR5_SIGNATURE: &[u8] = b"Rar!\x1a\x07\x01\x00"; + +/// Returns true if the file begins with a RAR volume signature. +/// +/// Obfuscated posts strip every naming cue, so content is the only reliable +/// evidence that a `.NN` file is an archive volume. Extension matching +/// alone would misclassify unrelated numeric-suffixed files. +pub fn has_rar_signature(path: &Path) -> bool { + let Ok(mut file) = std::fs::File::open(path) else { + return false; + }; + // Read up to the longest signature; short files simply cannot match. + let mut header = [0u8; RAR5_SIGNATURE.len()]; + let mut filled = 0; + while filled < header.len() { + match file.read(&mut header[filled..]) { + Ok(0) => break, + Ok(n) => filled += n, + Err(_) => return false, + } + } + let head = &header[..filled]; + head.starts_with(RAR4_SIGNATURE) || head.starts_with(RAR5_SIGNATURE) +} + +/// Split a bare numeric extension: `"cfd4be79….45"` → `("cfd4be79…", 45)`. +/// +/// Returns `None` for anything with a non-numeric extension, and for split 7z +/// volumes (`archive.7z.001`) — those are numeric too, but they belong to the +/// 7z path and must not be reclassified as RAR. +fn split_numeric_volume(filename: &str) -> Option<(&str, u32)> { + let dot = filename.rfind('.')?; + let ext = &filename[dot + 1..]; + if ext.is_empty() || !ext.chars().all(|c| c.is_ascii_digit()) { + return None; + } + let stem = &filename[..dot]; + if stem.to_ascii_lowercase().ends_with(".7z") { + return None; + } + ext.parse::().ok().map(|num| (stem, num)) +} + /// Parsed RAR volume information: set name and volume number. #[derive(Debug, Clone, PartialEq, Eq)] pub struct RarVolumeInfo { @@ -77,6 +125,32 @@ pub fn parse_rar_volume(filename: &str) -> Option { None } +/// Parse a RAR volume from a file on disk, falling back to content inspection. +/// +/// [`parse_rar_volume`] can only judge names, so it cannot recognise an +/// obfuscated volume like `cfd4be79….45`. This variant additionally accepts a +/// bare numeric extension when the file actually begins with a RAR signature — +/// evidence a filename cannot provide. Prefer it wherever a path is available. +/// +/// The volume number for an obfuscated set is the numeric extension itself, so +/// volumes order correctly relative to one another within the set. It is not +/// comparable with the numbering [`parse_rar_volume`] assigns to conventional +/// sets, which is anchored to `.rar` = 0. +pub fn parse_rar_volume_at(path: &Path) -> Option { + let name = path.file_name().and_then(|n| n.to_str())?; + if let Some(info) = parse_rar_volume(name) { + return Some(info); + } + let (set_name, volume_number) = split_numeric_volume(name)?; + if !has_rar_signature(path) { + return None; + } + Some(RarVolumeInfo { + set_name: set_name.to_string(), + volume_number, + }) +} + /// The type of archive detected. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ArchiveType { @@ -159,6 +233,8 @@ fn is_par2_volume(name_lower: &str) -> bool { /// `unrar x`). pub fn find_rar_files(dir: &Path) -> Vec { let mut first_volumes: Vec = Vec::new(); + // Obfuscated sets keyed by set name → (lowest volume seen, its path). + let mut numeric_sets: BTreeMap = BTreeMap::new(); for entry in WalkDir::new(dir).into_iter().flatten() { let path = entry.path(); @@ -191,11 +267,34 @@ pub fn find_rar_files(dir: &Path) -> Vec { } // Plain .rar with no .partNNN — this is the first volume in old-style first_volumes.push(path.to_path_buf()); + continue; } // Old-style: .r00, .r01, etc. — we do NOT add these; the .rar file // is the first volume in old-style sets. + if parse_rar_volume(&name_lower).is_some() { + continue; + } + + // Obfuscated set: `.NN` with no recognisable extension. Only + // files that actually begin with a RAR signature qualify, so unrelated + // numeric-suffixed files (`Concert.Recording.1987`) are left alone. + // The lowest-numbered volume is treated as the one to hand to unrar. + if let Some((set_name, volume_number)) = split_numeric_volume(&name_lower) + && has_rar_signature(path) + { + numeric_sets + .entry(set_name.to_string()) + .and_modify(|(lowest, lowest_path)| { + if volume_number < *lowest { + *lowest = volume_number; + *lowest_path = path.to_path_buf(); + } + }) + .or_insert_with(|| (volume_number, path.to_path_buf())); + } } + first_volumes.extend(numeric_sets.into_values().map(|(_, path)| path)); first_volumes.sort(); first_volumes } @@ -251,7 +350,7 @@ pub fn find_cleanup_files(dir: &Path) -> Vec { None => continue, }; - if is_cleanup_candidate(&name) { + if is_cleanup_candidate_at(path, &name) { cleanup.push(path.to_path_buf()); } } @@ -273,7 +372,7 @@ pub fn has_usable_output(dir: &Path) -> std::io::Result { let Some(name) = path.file_name().and_then(|name| name.to_str()) else { continue; }; - if !is_cleanup_candidate(&name.to_ascii_lowercase()) { + if !is_cleanup_candidate_at(path, &name.to_ascii_lowercase()) { return Ok(true); } } @@ -307,8 +406,30 @@ fn is_split_7z_volume(name_lower: &str) -> bool { false } -/// Determine whether a file (by its lowercased name) is safe to clean up +/// Determine whether a file on disk is safe to clean up after successful +/// extraction, using its name and — where the name is uninformative — its +/// content. +/// +/// `name_lower` must be the lowercased file name of `path`. +/// +/// An obfuscated `.NN` volume is indistinguishable by name from an +/// ordinary file that happens to end in digits, so the RAR signature is what +/// separates junk to delete from payload to keep. Getting this wrong in either +/// direction is costly: treating payload as junk deletes it, and treating an +/// archive volume as payload lets a job report Completed with nothing usable +/// in it (issue #87). +fn is_cleanup_candidate_at(path: &Path, name_lower: &str) -> bool { + if is_cleanup_candidate(name_lower) { + return true; + } + split_numeric_volume(name_lower).is_some() && has_rar_signature(path) +} + +/// Determine whether a file (by its lowercased name alone) is safe to clean up /// after successful extraction. +/// +/// Name-only: prefer [`is_cleanup_candidate_at`] wherever a path is available, +/// 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") { diff --git a/crates/nzb-postproc/src/lib.rs b/crates/nzb-postproc/src/lib.rs index 8a35c5a..0db76ca 100644 --- a/crates/nzb-postproc/src/lib.rs +++ b/crates/nzb-postproc/src/lib.rs @@ -16,7 +16,10 @@ pub mod unpack; // need nzb-postproc as a single dependency. pub use nzb_core; -pub use detect::{ArchiveType, RarVolumeInfo, has_usable_output, parse_rar_volume}; +pub use detect::{ + ArchiveType, RarVolumeInfo, has_rar_signature, has_usable_output, parse_rar_volume, + parse_rar_volume_at, +}; pub use par2::recovery_can_cover; pub use pipeline::{PostProcConfig, PostProcResult, run_pipeline, run_pipeline_with_resources}; pub use resources::{PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot}; diff --git a/crates/nzb-postproc/src/pipeline.rs b/crates/nzb-postproc/src/pipeline.rs index 99452ce..7f75c6c 100644 --- a/crates/nzb-postproc/src/pipeline.rs +++ b/crates/nzb-postproc/src/pipeline.rs @@ -160,12 +160,34 @@ pub async fn run_pipeline_with_resources( }); } } else if config.articles_failed == 0 { + // Files are known-good from CRC checks during yEnc decode, so the + // expensive MD5 verification pass is skipped. + // + // PAR2-guided deobfuscation still has to run. Obfuscated posts arrive + // with meaningless filenames whether or not an article failed, and the + // PAR2 metadata is the only record of the real names. While this + // rename lived inside the verify branch below, a *clean* download of + // an obfuscated post was never deobfuscated: no archive was found, the + // Extract stage reported "No archives found", and the job completed + // with raw volumes on disk. A damaged download self-healed; a healthy + // one did not (issue #87). info!("Skipping PAR2 verification — zero article failures (CRC-verified)"); + let start = Instant::now(); + let message = match rust_par2::parse(&par2_files[0]) { + Ok(file_set) => { + rename_to_par2_names(&file_set, job_dir); + "Skipped — zero article failures (PAR2-guided rename applied)".to_string() + } + Err(e) => { + debug!(error = %e, "PAR2 parse failed; skipping PAR2-guided deobfuscation"); + format!("Skipped — zero article failures (PAR2 parse failed: {e})") + } + }; stages.push(StageResult { name: "Verify".to_string(), status: StageStatus::Skipped, - message: Some("Skipped — zero article failures".to_string()), - duration_secs: 0.0, + message: Some(message), + duration_secs: start.elapsed().as_secs_f64(), }); } else { let verify_start = Instant::now(); @@ -182,7 +204,9 @@ pub async fn run_pipeline_with_resources( // PAR2 expected names (common with obfuscated posts where // NZB subjects have readable names but PAR2 references // the original obfuscated filenames), rename them using - // MD5-16k hash matching before verification runs. + // MD5-16k hash matching before verification runs. The + // zero-failure branch above runs this too — verification is + // skipped there, but deobfuscation must not be. rename_to_par2_names(&file_set, job_dir); // Run verify (and repair if needed) in a single spawn_blocking call. diff --git a/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs index 6f16884..5f29501 100644 --- a/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs +++ b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs @@ -1,34 +1,29 @@ -//! Reproduction suite for issue #87: obfuscated multi-volume RAR sets named -//! `<32 hex digits>.NN` are never detected, never extracted, and still let the -//! job report Completed. +//! Detection-layer regression suite for issue #87: obfuscated multi-volume RAR +//! sets named `<32 hex digits>.NN` were never detected, never extracted, and +//! still let the job report Completed. //! //! The suite is deliberately paired: //! //! * `neg_*` — **negative controls**. Conventional naming (`.rar`/`.r00`, -//! `.partNNN.rar`, `.7z.001`) and true payload. These pass on `main` and -//! prove the detection layer works when the filename carries an extension, -//! so a failure in a `pos_*` test is about the *name*, not the plumbing. -//! They are also the regression guard: any fix must keep these green. +//! `.partNNN.rar`, `.7z.001`) and true payload. These passed before the fix +//! as well as after, so a `pos_*` failure is about the *name*, not the +//! plumbing. They are the guard against the fix breaking ordinary sets. //! -//! * `pos_*` — **positive reproductions**. The obfuscated `.NN` set. -//! These assert the behaviour we want and therefore FAIL on `main`. They are -//! `#[ignore]`d so the committed suite stays green; run them with -//! `cargo test -p nzb-postproc --test obfuscated_rar_volumes -- --ignored`. -//! The fix removes the `#[ignore]` attributes. +//! * `pos_*` — the obfuscated `.NN` set. Each of these failed before the +//! fix, one per broken layer. //! //! * `guard_*` — **false-positive guards**. A naive fix ("treat any numeric -//! extension as a RAR volume") would break these. They pass on `main` and -//! must stay green. +//! extension as a RAR volume") breaks these; they are why detection reads +//! the RAR signature instead of trusting the extension. //! -//! Fixtures carry real RAR signature bytes so that a content-sniffing fix has -//! something valid to sniff. +//! Fixtures carry real RAR signature bytes because detection now sniffs them. use std::fs; use std::path::Path; use nzb_postproc::ArchiveType; use nzb_postproc::detect::{ - find_archives, find_cleanup_files, has_usable_output, parse_rar_volume, + find_archives, find_cleanup_files, has_usable_output, parse_rar_volume, parse_rar_volume_at, }; /// RAR 4.x volume signature. @@ -73,7 +68,7 @@ fn cleanup_names(dir: &Path) -> Vec { } // --------------------------------------------------------------------------- -// Negative controls — conventional naming. Green on `main`, must stay green. +// Negative controls — conventional naming. Must stay green. // --------------------------------------------------------------------------- #[test] @@ -162,7 +157,7 @@ fn neg_real_payload_counts_as_usable_output() { } // --------------------------------------------------------------------------- -// Positive reproductions — obfuscated `.NN`. RED on `main` (issue #87). +// The obfuscated `.NN` set — each of these failed before the fix. // --------------------------------------------------------------------------- /// Build the obfuscated set from the issue: `.45`, `.46`, `.47` + par2. @@ -177,7 +172,6 @@ fn obfuscated_dir() -> tempfile::TempDir { } #[test] -#[ignore = "reproduces issue #87: .NN volumes are not detected as a RAR set"] fn pos_obfuscated_numeric_volumes_are_detected_as_rar() { let dir = obfuscated_dir(); let archives = rar_archives(dir.path()); @@ -194,7 +188,6 @@ fn pos_obfuscated_numeric_volumes_are_detected_as_rar() { } #[test] -#[ignore = "reproduces issue #87: has_usable_output treats raw volumes as payload"] fn pos_obfuscated_volumes_are_not_usable_output() { let dir = obfuscated_dir(); assert!( @@ -205,7 +198,6 @@ fn pos_obfuscated_volumes_are_not_usable_output() { } #[test] -#[ignore = "reproduces issue #87: .NN volumes are never cleaned up"] fn pos_obfuscated_volumes_are_cleanup_candidates() { let dir = obfuscated_dir(); let cleanup = cleanup_names(dir.path()); @@ -219,14 +211,22 @@ fn pos_obfuscated_volumes_are_cleanup_candidates() { } #[test] -#[ignore = "reproduces issue #87: parse_rar_volume rejects bare numeric extensions"] fn pos_obfuscated_volume_numbers_are_parsed() { - let parsed = - parse_rar_volume(&format!("{HASH}.45")).expect("bare numeric RAR volume should parse"); + // Name alone cannot distinguish an obfuscated volume from any other file + // ending in digits, so numbering is resolved through the content-aware + // entry point. `parse_rar_volume` deliberately still rejects these. + let dir = obfuscated_dir(); + assert!( + parse_rar_volume(&format!("{HASH}.45")).is_none(), + "the name-only parser must not claim bare numeric extensions" + ); + + let parsed = parse_rar_volume_at(&dir.path().join(format!("{HASH}.45"))) + .expect("bare numeric RAR volume should parse from disk"); assert_eq!(parsed.set_name, HASH); - let next = - parse_rar_volume(&format!("{HASH}.46")).expect("bare numeric RAR volume should parse"); + let next = parse_rar_volume_at(&dir.path().join(format!("{HASH}.46"))) + .expect("bare numeric RAR volume should parse from disk"); assert_eq!( next.volume_number, parsed.volume_number + 1, @@ -235,7 +235,16 @@ fn pos_obfuscated_volume_numbers_are_parsed() { } #[test] -#[ignore = "reproduces issue #87: RAR5-signed obfuscated volumes are equally invisible"] +fn guard_numeric_file_without_signature_is_not_a_volume() { + // The content check is what keeps `parse_rar_volume_at` honest. + let dir = make_dir(&[("logfile.01", b"2026-08-11 INFO started")]); + assert!( + parse_rar_volume_at(&dir.path().join("logfile.01")).is_none(), + "a numeric-suffixed file with no RAR signature is not a volume" + ); +} + +#[test] fn pos_obfuscated_rar5_volumes_are_detected() { let body = rar_volume(RAR5_MAGIC); let dir = tempfile::tempdir().unwrap(); @@ -250,7 +259,7 @@ fn pos_obfuscated_rar5_volumes_are_detected() { // --------------------------------------------------------------------------- // False-positive guards — a naive "any numeric extension is a RAR volume" fix -// breaks these. Green on `main`, must stay green. +// breaks these. They are why detection sniffs the RAR signature. // --------------------------------------------------------------------------- #[test] diff --git a/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs index a07f41e..d45bfb1 100644 --- a/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs +++ b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs @@ -1,22 +1,24 @@ -//! Reproduction suite for the second half of issue #87: the PAR2-guided -//! deobfuscation that *would* fix obfuscated names never runs on a healthy +//! Regression suite for the second half of issue #87: the PAR2-guided +//! deobfuscation that fixes obfuscated names used to be skipped on a healthy //! download. //! //! `rename_to_par2_names` (pipeline.rs) recovers original filenames by matching //! each file's first-16K MD5 against the PAR2 metadata — exactly what an -//! obfuscated `.NN` set needs. But it is called from inside the verify -//! branch, and verification is skipped outright when `articles_failed == 0` -//! ("files are known-good from CRC checks"). So the one mechanism that can -//! deobfuscate the set only runs when the download was *damaged*. +//! obfuscated `.NN` set needs. It used to be called only from inside the +//! verify branch, and verification is skipped outright when +//! `articles_failed == 0` ("files are known-good from CRC checks"), so the one +//! mechanism that can deobfuscate the set only ran when the download was +//! *damaged*. //! -//! The pairing mirrors `obfuscated_rar_volumes.rs`: +//! The fix runs the rename whenever the PAR2 index parses, independent of the +//! verify decision. These tests pin that: the pairing mirrors +//! `obfuscated_rar_volumes.rs`: //! -//! * `neg_*` — the damaged path, where the rename does run. Green on `main`; -//! proves the rename machinery itself works and is the regression guard. +//! * `neg_*` — the damaged path, where the rename always ran. Passed before +//! the fix too; proves the rename machinery itself was never the problem. //! * `pos_*` — the healthy path. Same files, same PAR2, only -//! `articles_failed` differs. FAILS on `main`; `#[ignore]`d so CI stays -//! green. Run with `-- --ignored`; the fix removes the attribute. -//! * `guard_*` — must not regress when the gate is lifted. +//! `articles_failed` differs. This is the test that failed before the fix. +//! * `guard_*` — the rename must stay targeted now that it runs on every job. //! //! Extraction and cleanup are disabled throughout so the assertions are about //! filenames on disk and nothing else — these tests need no `unrar` binary. @@ -136,7 +138,7 @@ fn neg_par2_fixture_is_parseable_and_names_the_files() { } // --------------------------------------------------------------------------- -// Negative control — damaged download. Rename runs. Green on `main`. +// Negative control — damaged download. The rename always ran here. // --------------------------------------------------------------------------- #[tokio::test] @@ -155,11 +157,10 @@ async fn neg_damaged_download_deobfuscates_via_par2() { } // --------------------------------------------------------------------------- -// Positive reproduction — healthy download. RED on `main` (issue #87). +// Healthy download — this is the case that failed before the fix. // --------------------------------------------------------------------------- #[tokio::test] -#[ignore = "reproduces issue #87: PAR2-guided rename is gated behind articles_failed > 0"] async fn pos_healthy_download_deobfuscates_via_par2() { let dir = job_dir(&obfuscated_names()); @@ -169,15 +170,15 @@ async fn pos_healthy_download_deobfuscates_via_par2() { assert_eq!( names_on_disk(dir.path()), CANONICAL.iter().map(|s| s.to_string()).collect::>(), - "a clean download of an obfuscated post kept its .NN names: \ - verification is skipped when articles_failed == 0, and the PAR2 \ - rename is nested inside that skipped branch, so the set is never \ - deobfuscated and the Extract stage finds no archives" + "a clean download of an obfuscated post kept its .NN names — the \ + PAR2-guided rename has been re-gated behind the verify branch, which \ + articles_failed == 0 skips, so the set is never deobfuscated and the \ + Extract stage finds no archives (issue #87)" ); } // --------------------------------------------------------------------------- -// Guards — must stay green once the gate is lifted. +// Guards — the rename must stay targeted now that it runs on every job. // --------------------------------------------------------------------------- #[tokio::test]