Fix: chown directories created during caching to PUID/PGID (#178)

core/app.py

@@ -26,7 +26,7 @@
 from core import __version__
 from core.config import ConfigManager
 from core.logging_config import LoggingManager, reset_warning_error_flag
-from core.system_utils import SystemDetector, FileUtils, SingleInstanceLock, get_disk_usage, get_array_direct_path, detect_zfs, set_zfs_prefixes, format_bytes
+from core.system_utils import SystemDetector, FileUtils, SingleInstanceLock, get_disk_usage, get_array_direct_path, detect_zfs, set_zfs_prefixes, format_bytes, create_dir_with_ownership
 from core.plex_api import PlexManager, OnDeckItem
 from core.file_operations import MultiPathModifier, SiblingFileFinder, FileFilter, FileMover, PlexcachedRestorer, CacheTimestampTracker, WatchlistTracker, OnDeckTracker, CachePriorityManager, PlexcachedMigration, get_media_identity, find_matching_plexcached, is_directory_level_file
 from core.pinned_media import PinnedMediaTracker, resolve_pins_to_paths
@@ -801,7 +801,9 @@ def _ensure_cache_path_exists(self, cache_path: str) -> None:
         """Ensure a cache directory exists, creating it if necessary."""
         if not os.path.exists(cache_path):
             try:
-                os.makedirs(cache_path, exist_ok=True)
+                # Honor PUID/PGID so a cache dir created while running as root
+                # isn't left root:root.
+                create_dir_with_ownership(cache_path)
                 logging.info(f"[CONFIG] Created missing cache directory: {cache_path}")
             except OSError as e:
                 raise FileNotFoundError(f"Cannot create cache directory {cache_path}: {e}")
@@ -1749,7 +1751,7 @@ def _handle_upgrade_plexcached(self, old_path: str, new_path: str,
                 if not os.path.isfile(new_plexcached) and os.path.isfile(new_cache_path):
                     try:
                         new_array_dir = os.path.dirname(new_array_path)
-                        os.makedirs(new_array_dir, exist_ok=True)
+                        create_dir_with_ownership(new_array_dir, new_cache_path)
                         shutil.copy2(new_cache_path, new_plexcached)
                         # Verify size match
                         src_size = os.path.getsize(new_cache_path)
@@ -2659,7 +2661,7 @@ def _run_smart_eviction(self, needed_space_bytes: int = 0) -> tuple:
                             import shutil
                             try:
                                 array_direct_dir = os.path.dirname(array_direct_path)
-                                os.makedirs(array_direct_dir, exist_ok=True)
+                                create_dir_with_ownership(array_direct_dir, cache_path)
                                 shutil.copy2(cache_path, array_direct_path)
                                 logging.debug(f"Copied upgraded file to array: {array_direct_path}")
                                 # Verify copy succeeded using array-direct path
@@ -2695,7 +2697,7 @@ def _run_smart_eviction(self, needed_space_bytes: int = 0) -> tuple:
                                     # If we copy to /mnt/user/, Unraid's cache policy may put the file
                                     # back on cache (if shareUseCache=yes), causing data loss
                                     array_direct_dir = os.path.dirname(array_direct_path)
-                                    os.makedirs(array_direct_dir, exist_ok=True)
+                                    create_dir_with_ownership(array_direct_dir, cache_path)
                                     shutil.copy2(cache_path, array_direct_path)
                                     logging.info(f"[EVICTION] Created array copy before eviction: {os.path.basename(array_direct_path)}")
                                     # Verify copy succeeded using array-direct path

core/file_operations.py

@@ -16,7 +16,7 @@
 import re
 
 from core.logging_config import get_console_lock
-from core.system_utils import resolve_user0_to_disk, get_disk_free_space_bytes, get_disk_number_from_path, get_array_direct_path, format_bytes
+from core.system_utils import resolve_user0_to_disk, get_disk_free_space_bytes, get_disk_number_from_path, get_array_direct_path, format_bytes, create_dir_with_ownership
 
 if TYPE_CHECKING:
     from core.config import PathMapping
@@ -2364,10 +2364,10 @@ def _migrate_single_file(self, args: Tuple[str, str, str]) -> int:
                 self._active_files[thread_id] = (filename, file_size)
                 self._print_progress()
 
-            # Ensure directory exists
+            # Ensure directory exists with PUID/PGID ownership (running as root,
+            # a raw makedirs would leave new array folders root:root).
             array_dir = os.path.dirname(plexcached_file)
-            if not os.path.exists(array_dir):
-                os.makedirs(array_dir, exist_ok=True)
+            create_dir_with_ownership(array_dir, cache_file)
 
             # Copy cache file to array as .plexcached (preserving ownership on Linux)
             if self.is_unraid:
@@ -3109,7 +3109,7 @@ def _create_symlink(self, symlink_path: str, target_path: str) -> bool:
                 os.remove(symlink_path)
             parent_dir = os.path.dirname(symlink_path)
             if parent_dir and not os.path.isdir(parent_dir):
-                os.makedirs(parent_dir, exist_ok=True)
+                create_dir_with_ownership(parent_dir, target_path)
             os.symlink(target_path, symlink_path)
             logging.debug(f"Created symlink: {symlink_path} -> {target_path}")
             return True
@@ -5342,7 +5342,10 @@ def combined_stop_check():
                     display_src = self._translate_to_host_path(cache_file) if self.file_utils.is_docker else None
                     array_direct_file = get_array_direct_path(array_file)
                     array_direct_dir = os.path.dirname(array_direct_file)
-                    os.makedirs(array_direct_dir, exist_ok=True)
+                    # Create with PUID/PGID ownership — running as root, a raw
+                    # makedirs would leave new array folders root:root and block
+                    # Sonarr/Radarr from writing into them.
+                    self.file_utils.create_directory_with_permissions(array_direct_dir, cache_file)
 
                     def combined_stop_check():
                         if self._stop_requested:
@@ -5373,7 +5376,9 @@ def combined_stop_check():
                 # CRITICAL: Copy to /mnt/user0/ (array direct), NOT /mnt/user/ (FUSE)
                 array_direct_file = get_array_direct_path(array_file)
                 array_direct_dir = os.path.dirname(array_direct_file)
-                os.makedirs(array_direct_dir, exist_ok=True)
+                # Create with PUID/PGID ownership (see note above) so new array
+                # folders aren't left root:root when running as root.
+                self.file_utils.create_directory_with_permissions(array_direct_dir, cache_file)
 
                 def combined_stop_check():
                     if self._stop_requested:
@@ -5530,7 +5535,7 @@ def _create_symlink(self, symlink_path: str, target_path: str) -> bool:
             # Ensure parent directory exists
             parent_dir = os.path.dirname(symlink_path)
             if parent_dir and not os.path.isdir(parent_dir):
-                os.makedirs(parent_dir, exist_ok=True)
+                create_dir_with_ownership(parent_dir, target_path)
 
             os.symlink(target_path, symlink_path)
             logging.debug(f"Created symlink: {symlink_path} -> {target_path}")

core/system_utils.py

@@ -109,6 +109,91 @@ def get_array_direct_path(user_share_path: str) -> str:
     return user_share_path
 
 
+def create_dir_with_ownership(
+    path: str,
+    src_file_for_permissions: Optional[str] = None,
+    permissions: int = 0o777,
+) -> None:
+    """Create ``path`` (and any missing parents) with correct ownership/permissions.
+
+    The container runs as root so it can move files of any ownership, but raw
+    ``os.makedirs()`` then leaves new directories owned by ``root:root`` with a
+    umask-derived mode (typically 0755). On Unraid that blocks Sonarr/Radarr from
+    writing into newly created library folders. This helper chowns/chmods every
+    newly created directory level so they match PUID/PGID (or the source file's
+    owner when PUID/PGID are unset), the same way file copies are handled.
+
+    Owner resolution: PUID/PGID environment variables take precedence; otherwise
+    the owner of ``src_file_for_permissions`` is used (when it exists). chown/chmod
+    are Linux-only and best-effort — failures (e.g. filesystem without ownership
+    support) are logged at DEBUG and never raised. No-op if ``path`` already exists.
+
+    Args:
+        path: Directory path to create.
+        src_file_for_permissions: File whose owner is used when PUID/PGID are unset.
+        permissions: Mode applied to newly created directories (default 0o777).
+    """
+    if os.path.exists(path):
+        return
+
+    # Non-Linux (Windows dev/tests): just create the tree; no POSIX ownership.
+    if not hasattr(os, "chown"):
+        os.makedirs(path, exist_ok=True)
+        return
+
+    # Resolve target ownership: PUID/PGID env override, else source file's owner.
+    target_uid: Optional[int] = None
+    target_gid: Optional[int] = None
+    for env_name, setter in (("PUID", "uid"), ("PGID", "gid")):
+        env_val = os.environ.get(env_name)
+        if env_val:
+            try:
+                if setter == "uid":
+                    target_uid = int(env_val)
+                else:
+                    target_gid = int(env_val)
+            except ValueError:
+                pass  # Invalid env value — fall back to source ownership below.
+
+    if (target_uid is None or target_gid is None) and src_file_for_permissions:
+        try:
+            stat_info = os.stat(src_file_for_permissions)
+            if target_uid is None:
+                target_uid = stat_info.st_uid
+            if target_gid is None:
+                target_gid = stat_info.st_gid
+        except OSError:
+            pass  # Source missing — leave any unresolved id as None (skip chown).
+
+    # Track every directory level we are about to create so we can chown each one,
+    # not just the leaf (os.makedirs only returns after creating the whole chain).
+    dirs_to_create = []
+    current = path
+    while current and not os.path.exists(current):
+        dirs_to_create.append(current)
+        parent = os.path.dirname(current)
+        if parent == current:  # Reached filesystem root.
+            break
+        current = parent
+    dirs_to_create.reverse()  # Closest existing ancestor downward.
+
+    original_umask = os.umask(0)
+    try:
+        os.makedirs(path, exist_ok=True)
+        for dir_path in dirs_to_create:
+            if target_uid is not None and target_gid is not None:
+                try:
+                    os.chown(dir_path, target_uid, target_gid)
+                except (PermissionError, OSError) as e:
+                    logging.debug(f"Could not set directory ownership for {dir_path}: {e}")
+            try:
+                os.chmod(dir_path, permissions)
+            except (PermissionError, OSError) as e:
+                logging.debug(f"Could not set directory permissions for {dir_path}: {e}")
+    finally:
+        os.umask(original_umask)
+
+
 def parse_size_bytes(size_str: str) -> int:
     """Parse a human-readable size string and return bytes.
 
@@ -934,53 +1019,14 @@ def create_directory_with_permissions(self, path: str, src_file_for_permissions:
 
         If PUID/PGID environment variables are set, those values are used for ownership.
         Otherwise, the source file's ownership is used.
+
+        Thin instance wrapper around the module-level create_dir_with_ownership()
+        so the same logic is shared with callers that have no FileUtils instance
+        (e.g. the web service layer).
         """
         logging.debug(f"Creating directory with permissions: {path}")
-
-        if not os.path.exists(path):
-            if self.is_linux:
-                # Get the permissions of the source file
-                stat_info = os.stat(src_file_for_permissions)
-                src_uid = stat_info.st_uid
-                src_gid = stat_info.st_gid
-
-                # Use PUID/PGID if set, otherwise use source ownership
-                target_uid = self.puid if self.puid is not None else src_uid
-                target_gid = self.pgid if self.pgid is not None else src_gid
-
-                # Find the first existing ancestor directory
-                # We need to track which directories we create so we can chown them all
-                dirs_to_create = []
-                current = path
-                while current and not os.path.exists(current):
-                    dirs_to_create.append(current)
-                    parent = os.path.dirname(current)
-                    if parent == current:  # Reached root
-                        break
-                    current = parent
-
-                # Reverse so we create from closest existing ancestor downward
-                dirs_to_create.reverse()
-
-                original_umask = os.umask(0)
-                os.makedirs(path, exist_ok=True)
-
-                # Set ownership and permissions on ALL newly created directories
-                for dir_path in dirs_to_create:
-                    try:
-                        os.chown(dir_path, target_uid, target_gid)
-                    except (PermissionError, OSError) as e:
-                        logging.debug(f"Could not set directory ownership for {dir_path}: {e}")
-
-                    try:
-                        os.chmod(dir_path, self.permissions)
-                    except (PermissionError, OSError) as e:
-                        logging.debug(f"Could not set directory permissions for {dir_path}: {e}")
-
-                os.umask(original_umask)
-                logging.debug(f"Directory created with permissions (Linux): {path} ({len(dirs_to_create)} level(s), uid={target_uid}, gid={target_gid})")
-            else:  # Windows platform
-                os.makedirs(path, exist_ok=True)
-                logging.debug(f"Directory created (Windows): {path}")
-        else:
-            logging.debug(f"Directory already exists: {path}") 
+        if os.path.exists(path):
+            logging.debug(f"Directory already exists: {path}")
+            return
+        create_dir_with_ownership(path, src_file_for_permissions, permissions=self.permissions)
+        logging.debug(f"Directory created with permissions: {path}")

tests/test_system_utils.py

@@ -9,7 +9,7 @@
 import sys
 from datetime import datetime, timedelta
 from pathlib import Path
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 # conftest.py handles fcntl/apscheduler mocking and path setup
 from core.system_utils import (
@@ -18,6 +18,7 @@
     translate_host_to_container_path,
     remove_from_exclude_file,
     remove_from_timestamps_file,
+    create_dir_with_ownership,
 )
 
 
@@ -262,3 +263,79 @@ def test_preserves_other_entries(self, tmp_path):
         assert len(data) == 2
         assert "/mnt/cache/a.mkv" in data
         assert "/mnt/cache/c.mkv" in data
+
+
+# ============================================================================
+# TestCreateDirWithOwnership
+# ============================================================================
+
+class TestCreateDirWithOwnership:
+    """Tests for create_dir_with_ownership().
+
+    Guards the fix for #178: directories created while running as root must be
+    chowned to PUID/PGID so they don't end up root:root and block Sonarr/Radarr.
+    """
+
+    def test_creates_nested_tree(self, tmp_path, monkeypatch):
+        """Creates the full directory chain when it doesn't exist."""
+        monkeypatch.delenv("PUID", raising=False)
+        monkeypatch.delenv("PGID", raising=False)
+        target = tmp_path / "a" / "b" / "c"
+
+        create_dir_with_ownership(str(target))
+
+        assert target.is_dir()
+
+    def test_noop_when_exists(self, tmp_path):
+        """Existing directory is left untouched and no chown is attempted."""
+        existing = tmp_path / "already"
+        existing.mkdir()
+
+        with patch("core.system_utils.os.chown", create=True) as mock_chown:
+            create_dir_with_ownership(str(existing), str(existing))
+
+        mock_chown.assert_not_called()
+
+    def test_chowns_each_created_level_to_puid_pgid(self, tmp_path, monkeypatch):
+        """PUID/PGID env values are applied to every newly created level."""
+        monkeypatch.setenv("PUID", "99")
+        monkeypatch.setenv("PGID", "100")
+        src = tmp_path / "src.mkv"
+        src.write_text("data")
+
+        # tmp_path/dest exists already; only show/season are newly created.
+        dest_root = tmp_path / "dest"
+        dest_root.mkdir()
+        target = dest_root / "Show" / "Season 01"
+
+        with patch("core.system_utils.os.chown", create=True) as mock_chown, \
+                patch("core.system_utils.os.chmod"):
+            create_dir_with_ownership(str(target), str(src))
+
+        assert target.is_dir()
+        chowned = {call.args[0] for call in mock_chown.call_args_list}
+        assert str(target) in chowned
+        assert str(dest_root / "Show") in chowned
+        # The pre-existing ancestor must NOT be re-chowned.
+        assert str(dest_root) not in chowned
+        # Every chown targets the configured PUID/PGID.
+        for call in mock_chown.call_args_list:
+            assert call.args[1:] == (99, 100)
+
+    def test_invalid_puid_falls_back_to_source_owner(self, tmp_path, monkeypatch):
+        """A non-numeric PUID is ignored; the source file's owner is used."""
+        monkeypatch.setenv("PUID", "notanumber")
+        monkeypatch.delenv("PGID", raising=False)
+        src = tmp_path / "src.mkv"
+        src.write_text("data")
+        src_stat = os.stat(str(src))
+        target = tmp_path / "dest" / "Show"
+
+        with patch("core.system_utils.os.chown", create=True) as mock_chown, \
+                patch("core.system_utils.os.chmod"):
+            create_dir_with_ownership(str(target), str(src))
+
+        assert target.is_dir()
+        assert mock_chown.call_args_list, "chown should be attempted"
+        for call in mock_chown.call_args_list:
+            assert call.args[1:] == (src_stat.st_uid, src_stat.st_gid)

tools/audit_cache.py

@@ -17,7 +17,7 @@
 if PROJECT_ROOT_INIT not in sys.path:
     sys.path.insert(0, PROJECT_ROOT_INIT)
 
-from core.system_utils import get_array_direct_path
+from core.system_utils import get_array_direct_path, create_dir_with_ownership
 from core.file_operations import VIDEO_EXTENSIONS, SUBTITLE_EXTENSIONS, MEDIA_EXTENSIONS
 
 # Get script directory and resolve project root
@@ -426,8 +426,9 @@ def sync_to_array(dry_run=True):
             print(f"    -> {array_path}")
         else:
             try:
-                # Create destination directory if needed
-                os.makedirs(array_dir, exist_ok=True)
+                # Create destination directory if needed (honor PUID/PGID so new
+                # array folders aren't left root:root)
+                create_dir_with_ownership(array_dir, cache_path)
 
                 # Use rsync to copy file (preserves permissions, shows progress)
                 cmd = ['rsync', '-avh', '--progress', cache_path, array_path]