fix: correct path traversal docstring, total_raw inflation, and config re-stat (v0.5.8)

- executor.py: fix _safe_person_dir docstring — realpath+startswith is the
  load-bearing traversal guard; islink is a supplementary early-exit for the
  symlink sub-case only. The previous comment "checking after realpath would be
  too late" implied islink was the primary guard, which is backwards.

- immich_api.py: move total_raw accumulation to after the dead-end-page break
  so all-garbage pages don't inflate the count and produce misleading
  "N total, 0 recent" output. Mixed pages (some valid, some non-dict) still
  count page_count so transient schema issues don't shrink MIN_FACE_COUNT below
  threshold. Add warning when a RequestException interrupts pagination mid-way
  so operators know total_raw is a lower bound.

- config.py: eliminate residual TOCTOU — change `if config_file.exists():` to
  `if _data_cfg_exists or config_file.exists():` so _data_cfg is never
  stat'd twice (the v0.5.7 fix cached the first check but not the second).

CHANGELOG.md

@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.5.8] - 2026-06-15
+
+### Fixed
+
+- **`_safe_person_dir` docstring corrected**: the previous comment stated "checking after realpath would be too late because realpath follows the link first," which implied `islink` was the primary security guard. The load-bearing path-traversal check is `realpath + startswith`; it rejects both `../../` traversal and symlinks. The `islink` check is a supplementary early-exit that provides a cleaner error message for the symlink sub-case only.
+
+- **`total_raw` not inflated by all-garbage pages**: `fetch_all_assets` previously added `page_count` to `total_raw` before checking whether any valid dict items existed. A page returning only non-dict items would inflate `total_raw` and produce a misleading "N total, 0 recent" display. `total_raw` now accumulates only after `valid_assets` is confirmed non-empty, so all-garbage pages break without contributing. Mixed pages (some valid, some non-dict) still count `page_count` so transient schema glitches on a partial page don't cause `MIN_FACE_COUNT` to incorrectly skip a real person.
+
+- **Pagination interruption warning**: when a `RequestException` breaks pagination mid-way (page > 1), a `WARNING` is now logged noting that `total_raw` is a lower bound. Previously the exception was logged at `ERROR` with no indication that the `MIN_FACE_COUNT` comparison was using a partial count.
+
+- **Config TOCTOU residue**: `config.py` line 135 re-stat'd `_data_cfg` when it was the selected config file, creating a second TOCTOU window after the fix in v0.5.7. The check is now `if _data_cfg_exists or config_file.exists():` — the primary path is never stat'd again, and the legacy path is stat'd at most once.
+
 ## [0.5.7] - 2026-06-15
 
 ### Fixed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "winnow"
-version = "0.5.7"
+version = "0.5.8"
 description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition."
 license = "AGPL-3.0-or-later"
 requires-python = ">=3.13"

winnow/config.py

@@ -132,7 +132,9 @@ class _Config:
                 _data_cfg,
             )
         config_file = _data_cfg if _data_cfg_exists else _LEGACY_CONFIG_FILE
-        if config_file.exists():
+        # _data_cfg_exists already confirmed the primary path — avoid re-stat.
+        # The short-circuit means the legacy path is stat'd at most once here.
+        if _data_cfg_exists or config_file.exists():
             try:
                 data = json.loads(config_file.read_text())
                 if self.IMMICH_URL is None:

winnow/executor.py

@@ -42,9 +42,11 @@ def _safe_person_dir(output_dir: str, person_name: str) -> str:
     """Return the output subdirectory for a person, raising ValueError on path traversal.
 
     os.path.join silently discards output_dir when person_name is absolute,
-    and '../..' sequences resolve outside the tree. Both are rejected here.
+    and '../..' sequences resolve outside the tree. Both are rejected by the
-    Symlinks on the raw (unresolved) path are also rejected — checking after
+    realpath+startswith guard, which is the load-bearing security check.
-    realpath would be too late because realpath follows the link first.
+    The islink check below provides an earlier, cleaner error message for the
+    symlink sub-case; it is redundant with (not a replacement for) the
+    realpath+startswith traversal check.
     """
     raw = os.path.join(output_dir, person_name)
     if os.path.islink(raw):

winnow/immich_api.py

@@ -86,9 +86,11 @@ def merge_people(survivor_id: str, merge_ids: list[str]) -> bool:
 def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
     """Fetch all assets for a person with pagination.
 
-    Returns (assets, raw_total) where assets is the list of valid dict items
+    Returns (assets, total_raw) where assets is the list of valid dict items
-    and raw_total is the total item count seen before non-dict filtering.
+    and total_raw is the raw item count across pages that had at least one valid
-    raw_total may exceed len(assets) if Immich returned non-dict items.
+    dict. All-garbage pages (every item non-dict) stop pagination and are not
+    counted. If pagination is interrupted by a network error, total_raw is a
+    lower bound (a warning is logged).
     """
     name = person.get("name", "Unknown")
     person_id = person.get("id")
@@ -101,7 +103,7 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
     logger.debug("Fetching assets for %s...", name)
 
     assets: list[dict] = []
-    total_raw = 0  # items seen across all pages before non-dict filtering
+    total_raw = 0  # raw item count across pages that yielded at least one valid dict
     for page in range(1, MAX_PAGES + 1):
         try:
             resp = requests.post(
@@ -122,7 +124,6 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
                 page_assets = page_assets.get("items", [])
 
             page_count = len(page_assets)  # raw count for termination check before filtering
-            total_raw += page_count
 
             # Single pass: partition valid assets from unexpected non-dict items
             valid_assets, skipped_count = [], 0
@@ -142,6 +143,11 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
                     )
                 break
 
+            # Count page_count (not just valid items) so that non-dict items from a
+            # transient schema issue on a mixed page don't cause MIN_FACE_COUNT to
+            # skip a real person. Pages where every item is a non-dict are excluded —
+            # they indicate a structural problem and break above without contributing.
+            total_raw += page_count
             assets.extend(valid_assets)
             logger.debug("Fetched page %s, total: %s", page, len(assets))
 
@@ -150,6 +156,11 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
 
         except (requests.RequestException, ValueError) as e:
             logger.error("Exception fetching assets for %s (page %s): %s", name, page, e)
+            if page > 1:
+                logger.warning(
+                    "%s: pagination interrupted at page %s — total_raw=%s may undercount actual assets",
+                    name, page, total_raw,
+                )
             break
 
     return assets, total_raw

winnow/jobs.py

@@ -268,8 +268,8 @@ def auto_configure(people: list[dict]) -> list[dict]:
         rprint(f"  {name}: {total_raw} total, {len(recent_assets)} recent")
 
         # MIN_FACE_COUNT guard: skip people with too few Immich assets.
-        # Uses total_raw (pre-filter count) so non-dict items from a transient
+        # Uses total_raw so that non-dict items from a transient Immich schema
-        # Immich schema issue don't cause a person to be skipped incorrectly.
+        # issue on a mixed page don't shrink the count below the threshold.
         # Done here (after fetch) rather than upfront because Immich v2.7.5+
         # dropped assetCount from the /api/people response.
         if min_face_count > 0 and total_raw < min_face_count: