fix(thumbnails): confine cache writes to the cache directory

item_id and image_type reached the cache filename unsanitised while tag was
already being sanitised, and Path::join neither folds .. nor keeps the base
when handed an absolute path. Applies the tag's existing rule to all three
parts and adds a starts_with(cache_dir) check at the point of use, modelled on
media_server::resolve_path.

Not exploitable as shipped — server URLs must be HTTPS (auth/mod.rs) and
Android blocks cleartext, so the id would have to come from a server the user
chose to trust. This makes the write path consistent with how the rest of the
codebase already handles caller-supplied paths.

TRACES: | DR-210 | UT-204

src-tauri/src/thumbnail/cache.rs

@@ -1,7 +1,7 @@
 //! Thumbnail cache manager with LRU eviction
 
 use log::error;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::sync::Mutex;
 
@@ -28,6 +28,19 @@ impl Default for CacheConfig {
     }
 }
 
+/// Make one part of a cache filename safe to put in a path.
+///
+/// Every part of the name comes from the caller — the item id and image type are
+/// taken verbatim from Jellyfin JSON — so none of them may contribute a path
+/// separator or a `..`. The rule is the one the image tag has always used
+/// (non-alphanumerics become `_`), applied to all three parts, so values that
+/// were already safe keep producing exactly the filename they did before.
+///
+/// TRACES: | DR-210 | UT-204
+fn safe_component(value: &str) -> String {
+    value.replace(|c: char| !c.is_alphanumeric(), "_")
+}
+
 /// Thumbnail cache with LRU eviction
 pub struct ThumbnailCache {
     config: Arc<Mutex<CacheConfig>>,
@@ -50,6 +63,34 @@ impl ThumbnailCache {
         }
     }
 
+    /// Resolve a cache filename against the cache directory, refusing anything
+    /// that lands outside it.
+    ///
+    /// `..` is folded away lexically rather than through `canonicalize`, so a
+    /// file that does not exist yet still resolves — the same approach as
+    /// `media_server::resolve_path`. `safe_component` should already have made an
+    /// escape impossible; this is the check at the point of use.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    fn resolve_in_cache_dir(&self, filename: &str) -> Result<PathBuf, String> {
+        let mut resolved = self.cache_dir.clone();
+        for part in Path::new(filename).components() {
+            match part {
+                std::path::Component::ParentDir => {
+                    resolved.pop();
+                }
+                std::path::Component::CurDir => {}
+                other => resolved.push(other),
+            }
+        }
+
+        if resolved.starts_with(&self.cache_dir) {
+            Ok(resolved)
+        } else {
+            Err("Thumbnail path escapes the cache directory".to_string())
+        }
+    }
+
     /// Check if caching is enabled
     pub fn is_enabled(&self) -> bool {
         self.config.lock().map(|c| c.enabled).unwrap_or(true)
@@ -155,6 +196,8 @@ impl ThumbnailCache {
     }
 
     /// Save thumbnail to cache
+    ///
+    /// TRACES: | DR-210 | UT-204
     // The arguments are the cache key (item/type/tag) plus the payload and its
     // dimensions — all independent scalars borrowed from the caller. A parameter
     // struct would only move the same list one level down.
@@ -173,10 +216,16 @@ impl ThumbnailCache {
             return Err("Thumbnail caching is disabled".to_string());
         }
 
-        // Generate safe filename
+        // Generate safe filename. The database keeps the *raw* key below, so the
-        let safe_tag = tag.replace(|c: char| !c.is_alphanumeric(), "_");
+        // lookup in `get_cached_path` still matches what the caller asks for; only
-        let filename = format!("{}_{}_{}.jpg", item_id, image_type, safe_tag);
+        // the on-disk name is sanitised, and the row records where it landed.
-        let file_path = self.cache_dir.join(&filename);
+        let filename = format!(
+            "{}_{}_{}.jpg",
+            safe_component(item_id),
+            safe_component(image_type),
+            safe_component(tag)
+        );
+        let file_path = self.resolve_in_cache_dir(&filename)?;
 
         // Ensure we have space (evict LRU items if needed)
         self.ensure_space(db.clone(), data.len() as u64).await?;
@@ -571,4 +620,171 @@ mod tests {
         let size = cache.get_cache_size(conn.clone()).await;
         assert!(size <= 60);
     }
+
+    /// A traversal-style `item_id` must not steer a cache write out of the cache
+    /// directory. The id reaches `save_thumbnail` verbatim from Jellyfin JSON, so
+    /// it is not ours to trust.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    #[tokio::test]
+    async fn test_save_thumbnail_confines_traversal_item_id() {
+        let (conn, temp_dir) = setup_test_db();
+        let cache = ThumbnailCache::new(temp_dir.path().to_path_buf(), CacheConfig::default());
+
+        let result = cache
+            .save_thumbnail(
+                conn.clone(),
+                "../evil",
+                "Primary",
+                "tag1",
+                b"fake image data",
+                None,
+                None,
+            )
+            .await;
+
+        // Where `../evil` lands if `..` is honoured: the cache dir's parent.
+        let escaped = temp_dir.path().join("evil_Primary_tag1.jpg");
+        assert!(
+            !escaped.exists(),
+            "wrote outside the cache directory: {}",
+            escaped.display()
+        );
+
+        // Refusing is acceptable; succeeding is too, as long as it stayed inside.
+        if let Ok(path) = result {
+            assert!(
+                path.starts_with(&cache.cache_dir) && !path.to_string_lossy().contains(".."),
+                "returned a path outside the cache directory: {}",
+                path.display()
+            );
+            assert!(path.exists());
+        }
+    }
+
+    /// `Path::join` discards the base when handed an absolute path, so an
+    /// absolute `item_id` would otherwise pick the write location outright.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    #[tokio::test]
+    async fn test_save_thumbnail_confines_absolute_item_id() {
+        let (conn, temp_dir) = setup_test_db();
+        let outside = TempDir::new().unwrap();
+        let cache = ThumbnailCache::new(temp_dir.path().to_path_buf(), CacheConfig::default());
+
+        let absolute_id = outside.path().join("evil").to_string_lossy().to_string();
+        let result = cache
+            .save_thumbnail(
+                conn.clone(),
+                &absolute_id,
+                "Primary",
+                "tag1",
+                b"fake image data",
+                None,
+                None,
+            )
+            .await;
+
+        let escaped = outside.path().join("evil_Primary_tag1.jpg");
+        assert!(
+            !escaped.exists(),
+            "wrote outside the cache directory: {}",
+            escaped.display()
+        );
+
+        if let Ok(path) = result {
+            assert!(
+                path.starts_with(&cache.cache_dir),
+                "returned a path outside the cache directory: {}",
+                path.display()
+            );
+        }
+    }
+
+    /// `image_type` is equally unsanitised, and equally caller-supplied.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    #[tokio::test]
+    async fn test_save_thumbnail_confines_traversal_image_type() {
+        let (conn, temp_dir) = setup_test_db();
+        let cache = ThumbnailCache::new(temp_dir.path().to_path_buf(), CacheConfig::default());
+
+        let path = cache
+            .save_thumbnail(
+                conn.clone(),
+                "item1",
+                "../Primary",
+                "tag1",
+                b"fake image data",
+                None,
+                None,
+            )
+            .await
+            .expect("a malformed image_type should be sanitised, not break caching");
+
+        // The file belongs directly in the cache dir — no separator from the
+        // image type may survive into the filename.
+        assert_eq!(path.parent(), Some(cache.cache_dir.as_path()));
+        assert!(path.exists());
+    }
+
+    /// Ids, types and tags that were already filesystem-safe — the overwhelming
+    /// majority — keep producing exactly the filename they did before, so
+    /// sanitising does not orphan existing cache entries.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    #[tokio::test]
+    async fn test_save_thumbnail_filename_unchanged_for_safe_values() {
+        let (conn, temp_dir) = setup_test_db();
+        let cache = ThumbnailCache::new(temp_dir.path().to_path_buf(), CacheConfig::default());
+
+        let path = cache
+            .save_thumbnail(
+                conn.clone(),
+                "a1b2c3d4e5f60718293a4b5c6d7e8f90",
+                "Primary",
+                "abcdef0123456789",
+                b"fake image data",
+                None,
+                None,
+            )
+            .await
+            .unwrap();
+
+        assert_eq!(
+            path,
+            cache
+                .cache_dir
+                .join("a1b2c3d4e5f60718293a4b5c6d7e8f90_Primary_abcdef0123456789.jpg")
+        );
+    }
+
+    /// Sanitising the filename must not desynchronise the write path from the
+    /// read path: the database keeps the raw key and the resolved path, so a
+    /// lookup after a save still finds the file that was written.
+    ///
+    /// TRACES: | DR-210 | UT-204
+    #[tokio::test]
+    async fn test_traversal_item_id_still_round_trips() {
+        let (conn, temp_dir) = setup_test_db();
+        let cache = ThumbnailCache::new(temp_dir.path().to_path_buf(), CacheConfig::default());
+
+        let saved = cache
+            .save_thumbnail(
+                conn.clone(),
+                "../evil",
+                "Primary",
+                "tag1",
+                b"fake image data",
+                None,
+                None,
+            )
+            .await
+            .unwrap();
+
+        let cached = cache
+            .get_cached_path(conn.clone(), "../evil", "Primary", "tag1")
+            .await;
+        assert_eq!(cached, Some(saved));
+    }
 }