From 796aded2da2f9bea69521d19328ef6fba6a373a3 Mon Sep 17 00:00:00 2001 From: Holden Date: Mon, 15 Jun 2026 01:02:28 +0000 Subject: [PATCH] fix: log+skip on symlink TOCTOU in execute_jobs (v0.5.11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v0.5.10 compound guard 'isdir and not islink' silently skipped the rmtree when person_dir was a symlink-to-directory, then let makedirs follow the symlink — allowing crop writes outside output_dir with no diagnostic. Replace with an explicit islink pre-check that logs an error and continues, matching the ValueError path from _safe_person_dir. --- CHANGELOG.md | 6 ++++++ pyproject.toml | 2 +- winnow/executor.py | 10 ++++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa792cd..3329866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.11] - 2026-06-15 + +### Fixed + +- **`execute_jobs` symlink TOCTOU gap closed**: the v0.5.10 guard `os.path.isdir(person_dir) and not os.path.islink(person_dir)` silently skipped the wipe when `person_dir` was a symlink-to-directory, then called `os.makedirs` which followed the symlink — allowing crop writes to land outside `output_dir` with no log or skip. The guard is replaced by an explicit pre-check: if `os.path.islink(person_dir)` is True, log an error and `continue`, matching the established `ValueError` pattern from `_safe_person_dir`. The `isdir` / `rmtree` block is restored to its original simple form. + ## [0.5.10] - 2026-06-15 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 2db5d42..b7ffd2a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "winnow" -version = "0.5.10" +version = "0.5.11" 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" diff --git a/winnow/executor.py b/winnow/executor.py index 0954045..6d58aa3 100644 --- a/winnow/executor.py +++ b/winnow/executor.py @@ -107,10 +107,12 @@ def execute_jobs(jobs: list[dict]) -> None: logger.error(str(e)) continue # Face crops are transient (uploaded then discarded); wipe before each run. - # Exclude symlinks explicitly: os.path.isdir follows them and returns True - # for a symlink-to-directory, but shutil.rmtree raises OSError on a - # top-level symlink rather than deleting through it. - if os.path.isdir(person_dir) and not os.path.islink(person_dir): + # A symlink could appear here via a TOCTOU race after _safe_person_dir + # returned — writing through it would land crops outside output_dir. + if os.path.islink(person_dir): + logger.error("person_dir %s became a symlink after path check — skipping job", person_dir) + continue + if os.path.isdir(person_dir): shutil.rmtree(person_dir) os.makedirs(person_dir, exist_ok=True)