fix(daemon): harden import against review findings

Reject torrent file paths that escape the download root (absolute or
with parent components) before probing or linking. Cache probe results
across reconcile ticks: the lane's 25 s budget cancels the action while
one probe may take 60 s, so a multi-file torrent needs its settled
probes to survive into the next tick.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

crates/arr-daemon/src/import.rs

@@ -13,9 +13,10 @@
 //! destination already existing, and the `media_files` insert upserts on
 //! path.
 
+use std::collections::HashMap;
 use std::ffi::OsString;
 use std::io;
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 
 use arr_core::layout;
 use arr_core::policy::{evaluate, Candidate};
@@ -58,12 +59,27 @@ enum Placement {
     AlreadyPlaced,
 }
 
+/// What one probe attempt settled about one path.
+#[derive(Debug, Clone)]
+enum ProbeOutcome {
+    Media(Box<arr_probe::ProbedFile>),
+    /// A fact about the file — `.nfo`, artwork, corrupt — not the prober.
+    NotMedia,
+}
+
 /// Imports every downloaded grab: probe, second policy pass, hardlink into
 /// the §7.4 layout.
 #[derive(Debug)]
 pub struct ImportAction {
     transmission: TransmissionClient,
     prober: Prober,
+    /// Probe results by path, kept across ticks. The reconcile lane cancels
+    /// the whole action after its 25 s budget while one probe alone may take
+    /// up to 60 s, so without this a large multi-file torrent would restart
+    /// from the first file every tick and never finish. Transient state,
+    /// rebuilt by re-probing after a restart (§8); entries are dropped once
+    /// their grab settles.
+    probed: tokio::sync::Mutex<HashMap<PathBuf, ProbeOutcome>>,
 }
 
 /// A movie grab Transmission finished downloading, not yet imported.
@@ -85,6 +101,52 @@ impl ImportAction {
         Self {
             transmission,
             prober,
+            probed: tokio::sync::Mutex::new(HashMap::new()),
+        }
+    }
+
+    /// Probe every path, reusing results settled on earlier ticks, and
+    /// return the readable video files.
+    ///
+    /// Only facts about a file are cached; a prober failure — missing
+    /// binary, timeout, unparseable output — is returned so the tick retries.
+    async fn probe_all(
+        &self,
+        paths: &[PathBuf],
+    ) -> Result<Vec<arr_probe::ProbedFile>, ImportError> {
+        let mut files = Vec::new();
+        for path in paths {
+            let cached = self.probed.lock().await.get(path).cloned();
+            let outcome = if let Some(outcome) = cached {
+                outcome
+            } else {
+                let outcome = match self.prober.probe(path.clone()).await {
+                    Ok(file) => ProbeOutcome::Media(Box::new(file)),
+                    Err(error) if error.is_about_the_file() => {
+                        tracing::debug!(path = %path.display(), %error, "not a video file, skipping");
+                        ProbeOutcome::NotMedia
+                    }
+                    Err(error) => return Err(error.into()),
+                };
+                self.probed
+                    .lock()
+                    .await
+                    .insert(path.clone(), outcome.clone());
+                outcome
+            };
+            if let ProbeOutcome::Media(file) = outcome {
+                files.push(*file);
+            }
+        }
+        Ok(files)
+    }
+
+    /// Drop a settled grab's probe results — imported or blacklisted, they
+    /// will not be needed again.
+    async fn forget_probes(&self, paths: &[PathBuf]) {
+        let mut probed = self.probed.lock().await;
+        for path in paths {
+            probed.remove(path);
         }
     }
 
@@ -136,27 +198,38 @@ impl ImportAction {
             );
             return Ok(None);
         };
+        // Torrent-declared names are untrusted input: an absolute or
+        // `..`-carrying entry would escape the download root and get probed —
+        // and possibly hardlinked — from anywhere on disk.
         let paths: Vec<PathBuf> = content
             .files
             .iter()
-            .map(|file| content.download_dir.join(&file.path))
+            .filter_map(|file| {
+                let path = safe_join(&content.download_dir, &file.path);
+                if path.is_none() {
+                    tracing::warn!(
+                        grab_id = pending.grab_id,
+                        path = %file.path.display(),
+                        "torrent file path escapes the download root; skipping"
+                    );
+                }
+                path
+            })
             .collect();
 
         // No expected runtime yet: the movies table carries no TMDB runtime,
         // so feature selection is by size alone (largest readable video).
-        let selection = match self.prober.select_feature(paths, None).await {
-            Ok(selection) => selection,
+        let mut candidates = self.probe_all(&paths).await?;
+        candidates.sort_by_key(|candidate| std::cmp::Reverse(candidate.size));
+        let Some(feature) = candidates.into_iter().next() else {
             // §5.7 "corrupt, wrong content": nothing in the torrent is a
             // readable video file.
-            Err(arr_probe::Error::NoCandidates) => {
-                return self
-                    .hard_fail(database, pending, "no readable video file")
-                    .await
-                    .map(Some)
-            }
-            Err(error) => return Err(error.into()),
+            self.forget_probes(&paths).await;
+            return self
+                .hard_fail(database, pending, "no readable video file")
+                .await
+                .map(Some);
         };
-        let feature = selection.feature;
 
         // §5.6 second phase of truth: same policy, real evidence.
         let evaluation = evaluate(
@@ -168,10 +241,11 @@ impl ImportAction {
         );
         let waiver: Option<Rule> = match evaluation.verdict {
             Verdict::Rejected(rule) => {
+                self.forget_probes(&paths).await;
                 return self
                     .hard_fail(database, pending, &rule.name())
                     .await
-                    .map(Some)
+                    .map(Some);
             }
             Verdict::Waived(rule) => Some(rule),
             Verdict::Eligible => None,
@@ -200,6 +274,7 @@ impl ImportAction {
             tokio::task::spawn_blocking(move || place(&source_path, &link_target)).await??;
 
         record_import(database, pending, &feature, waiver.as_ref(), &destination).await?;
+        self.forget_probes(&paths).await;
         let path_text = destination.to_string_lossy().into_owned();
         tracing::info!(
             grab_id = pending.grab_id,
@@ -389,6 +464,24 @@ fn probed_json(media: &ProbedMedia) -> serde_json::Value {
     })
 }
 
+/// Join a torrent-declared file path onto the download root, refusing
+/// anything that could land outside it: absolute paths, drive prefixes and
+/// `..` components. `None` means the entry is hostile or malformed.
+fn safe_join(root: &Path, declared: &Path) -> Option<PathBuf> {
+    let mut clean = PathBuf::new();
+    for component in declared.components() {
+        match component {
+            Component::Normal(part) => clean.push(part),
+            Component::CurDir => {}
+            Component::RootDir | Component::Prefix(_) | Component::ParentDir => return None,
+        }
+    }
+    if clean.as_os_str().is_empty() {
+        return None;
+    }
+    Some(root.join(clean))
+}
+
 /// Hardlink `source` to `destination`, falling back to copy on `EXDEV` only
 /// (§7.2). No configuration flag.
 fn place(source: &Path, destination: &Path) -> Result<Placement, ImportError> {
@@ -498,6 +591,17 @@ mod tests {
     }
 
     async fn harness(media_json: &str) -> Harness {
+        harness_with(
+            media_json,
+            json!([
+                {"name": "Dune/Dune.mkv", "length": 13, "bytesCompleted": 13},
+                {"name": "Dune/Dune.nfo", "length": 10, "bytesCompleted": 10}
+            ]),
+        )
+        .await
+    }
+
+    async fn harness_with(media_json: &str, files: serde_json::Value) -> Harness {
         let dir = tempfile::tempdir().unwrap();
         let downloads = dir.path().join("downloads");
         let library = dir.path().join("library");
@@ -548,10 +652,7 @@ mod tests {
                 "arguments": {"torrents": [{
                     "hashString": INFOHASH,
                     "downloadDir": downloads.to_string_lossy(),
-                    "files": [
-                        {"name": "Dune/Dune.mkv", "length": 13, "bytesCompleted": 13},
-                        {"name": "Dune/Dune.nfo", "length": 10, "bytesCompleted": 10}
-                    ]
+                    "files": files
                 }]}
             })))
             .mount(&server)
@@ -700,6 +801,110 @@ mod tests {
         assert!(outcomes.is_empty());
     }
 
+    /// Torrent-declared names are untrusted: absolute and `..`-carrying
+    /// entries are skipped, and the import proceeds from what remains.
+    #[tokio::test]
+    async fn hostile_torrent_paths_never_leave_the_download_root() {
+        let h = harness_with(
+            HDR10_PROBE,
+            json!([
+                {"name": "../outside.mkv", "length": 13, "bytesCompleted": 13},
+                {"name": "/tmp/absolute.mkv", "length": 13, "bytesCompleted": 13},
+                {"name": "Dune/Dune.mkv", "length": 13, "bytesCompleted": 13}
+            ]),
+        )
+        .await;
+
+        let outcomes = h.action.tick(&h.database).await.unwrap();
+
+        assert_eq!(outcomes.len(), 1);
+        assert!(expected_library_file(&h.library).is_file());
+        let files: i64 = sqlx::query_scalar("SELECT count(*) FROM media_files")
+            .fetch_one(h.database.pool())
+            .await
+            .unwrap();
+        assert_eq!(files, 1, "only the safe path is imported");
+    }
+
+    /// A torrent whose every entry escapes the root has nothing importable:
+    /// hard fail, not an escape.
+    #[tokio::test]
+    async fn a_torrent_of_only_hostile_paths_hard_fails() {
+        let h = harness_with(
+            HDR10_PROBE,
+            json!([{"name": "../../etc/passwd", "length": 13, "bytesCompleted": 13}]),
+        )
+        .await;
+
+        let outcomes = h.action.tick(&h.database).await.unwrap();
+
+        assert_eq!(outcomes.len(), 1);
+        let reason: String = sqlx::query_scalar("SELECT reason FROM blacklist")
+            .fetch_one(h.database.pool())
+            .await
+            .unwrap();
+        assert_eq!(reason, "no readable video file");
+        assert!(std::fs::read_dir(&h.library).unwrap().next().is_none());
+    }
+
+    #[test]
+    fn safe_join_refuses_escapes_and_keeps_normal_paths() {
+        let root = Path::new("/downloads");
+        assert_eq!(
+            safe_join(root, Path::new("Dune/./Dune.mkv")),
+            Some(PathBuf::from("/downloads/Dune/Dune.mkv"))
+        );
+        assert_eq!(safe_join(root, Path::new("../outside.mkv")), None);
+        assert_eq!(safe_join(root, Path::new("Dune/../../outside.mkv")), None);
+        assert_eq!(safe_join(root, Path::new("/etc/passwd")), None);
+        assert_eq!(safe_join(root, Path::new("")), None);
+    }
+
+    /// The reconcile lane cancels the action after 25 s while one probe may
+    /// take 60 s, so results settled on one tick must survive to the next —
+    /// otherwise a large torrent restarts from file one forever.
+    #[tokio::test]
+    async fn probe_results_are_reused_across_calls() {
+        let dir = tempfile::tempdir().unwrap();
+        let media = dir.path().join("film.mkv");
+        std::fs::write(&media, b"bytes").unwrap();
+        let counter = dir.path().join("count");
+        let script = dir.path().join("ffprobe");
+        std::fs::write(
+            &script,
+            format!(
+                "#!/bin/sh\necho x >> {}\ncat <<'PROBE_EOF'\n{HDR10_PROBE}\nPROBE_EOF\n",
+                counter.display()
+            ),
+        )
+        .unwrap();
+        let mut permissions = std::fs::metadata(&script).unwrap().permissions();
+        permissions.set_mode(0o755);
+        std::fs::set_permissions(&script, permissions).unwrap();
+
+        let action = ImportAction::new(
+            TransmissionClient::new("http://127.0.0.1:1").unwrap(),
+            Prober::new().with_binary(&script),
+        );
+        let paths = vec![media];
+
+        action.probe_all(&paths).await.unwrap();
+        action.probe_all(&paths).await.unwrap();
+        assert_eq!(
+            std::fs::read_to_string(&counter).unwrap().lines().count(),
+            1,
+            "the second call reuses the first call's result"
+        );
+
+        action.forget_probes(&paths).await;
+        action.probe_all(&paths).await.unwrap();
+        assert_eq!(
+            std::fs::read_to_string(&counter).unwrap().lines().count(),
+            2,
+            "a settled grab's entries are dropped"
+        );
+    }
+
     /// The `EXDEV` fallback path lands whole files via rename (§7.2).
     #[test]
     fn the_copy_fallback_lands_a_whole_file_and_cleans_up() {