From 7e037dce888edee919bd62ca304d70112b6ce7df Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Tue, 25 Aug 2026 11:32:38 -0600 Subject: [PATCH 1/3] fix: verify Windows filename safety --- .gitattributes | 3 ++ README.md | 2 +- ROADMAP.md | 11 ++++--- rename_plan.py | 19 ++++++++---- tests/test_rename_plan.py | 61 +++++++++++++++++++++++++++++++++++---- tests/test_renamer.py | 4 ++- 6 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..3450e66 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +# yamllint requires LF line endings, including on Windows checkouts. +*.yml text eol=lf +*.yaml text eol=lf diff --git a/README.md b/README.md index 0c71f5f..4f079b9 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ Available variables: `$date` `$performer` `$title` `$studio` `$height` | `$date $performer - $title [$studio]` | `2016-12-29 Eva Lovia - Her Fantasy Ball [Sneaky Sex].mp4` | Notes: -- Illegal Windows filename characters are stripped automatically. `#` and `,` are also stripped even though they are legal on Windows — edit the character-stripping regex in the script to preserve them. +- Illegal Windows filename characters, leading/trailing ASCII spaces, and trailing periods are stripped automatically. Reserved device names (including `CON`, `NUL`, `COM1`–`COM9`, and `LPT1`–`LPT9`) block the plan. `#` and `,` are also stripped even though they are legal on Windows — edit the character-stripping regex in the script to preserve them. - Heights of 2160 and 4320 are shown as `4k` and `8k`; others as `p` (e.g. `1080p`). - If a scene has more than 3 performers, `$performer` is omitted. This applies before the optional `FEMALE_ONLY` filter. diff --git a/ROADMAP.md b/ROADMAP.md index 983a073..9532c93 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -43,7 +43,7 @@ This is the first `ROADMAP.md`, but it reconciles the June 2026 historical audit ### R0-3 — Complete Windows filename rules and normalization collision checks -- **Status:** Implemented 2026-08-23; pending Windows-runtime verification. +- **Status:** Completed 2026-08-25. Windows-runtime verification with Python 3.14 exercised temporary-file renames and confirmed filename sanitization for invalid/control characters, ASCII whitespace/period normalization, standard and superscript device names, and normalized destination collisions. - **Source:** `BUG-001`, `TEST-002` - **Action:** Centralize filename sanitization/validation for control characters, reserved device basenames, trailing periods/spaces, existing punctuation policy, and post-normalization collisions. - **Reason / expected effect:** Turns predictable Windows rename failures into deterministic plan errors or safe normalized names. @@ -275,7 +275,7 @@ Any future branch deletion requires refreshed branch/PR/worktree/unique-commit e |---|---|---|---|---|---|---|---| | R0-1 | Authoritative validated plan/apply | `REL-001`, `ARCH-001`, `FEAT-001` | Complete | Medium | Current semantics regression tests | Phase 0 | Dry run and apply consume identical plan; all blocking conflicts detected before mutation | | R0-2 | Privacy-safe local configuration | `SEC-3`, `FEAT-003` | Complete | Medium | Configuration precedence decision | Phase 0 | No private defaults tracked; clean setup works; missing config fails clearly | -| R0-3 | Complete Windows filename rules | `BUG-001`, `TEST-002` | Verification pending | Medium | Normalization policy | Phase 0 | Platform rule suite passes; normalization collisions block safely | +| R0-3 | Complete Windows filename rules | `BUG-001`, `TEST-002` | Complete | Medium | Normalization policy | Phase 0 | Platform rule suite passes; normalization collisions block safely | | R1-1 | SQLite/filesystem integration fixture | `TEST-002`, `REL-001` | Complete | Medium | Plan interface | Phase 1 | Real queries and multi-file/collision paths pass in CI | | R1-2 | Versioned run manifest | `REL-002`, `FEAT-001` | Complete | Medium | R0-1 | Phase 1 | Every run has attributable operation states, recovery checkpoints, and a completion marker | | R2-1 | Explicit planner/executor/DB boundaries | `ARCH-001` | Complete | Medium | R0-1, R1-1 | Phase 2 | No global cursor required by tests; behavior unchanged | @@ -306,7 +306,6 @@ Any future branch deletion requires refreshed branch/PR/worktree/unique-commit e ## Recommended Execution Order -1. **R0-3:** Run the existing filename rule suite on Windows before describing Windows runtime support as complete. -2. **R3-2:** Prepare a release candidate; publish, tag, or release only after explicit approval and clean matrix/security checks. -3. **G2:** Update the GitHub homepage only after a separately approved public destination decision. -4. **X-1:** Separately validate Stash API/plugin demand and constraints; promote only with evidence. +1. **R3-2:** Prepare a release candidate; publish, tag, or release only after explicit approval and clean matrix/security checks. +2. **G2:** Update the GitHub homepage only after a separately approved public destination decision. +3. **X-1:** Separately validate Stash API/plugin demand and constraints; promote only with evidence. diff --git a/rename_plan.py b/rename_plan.py index 0f5a05b..fb05a88 100644 --- a/rename_plan.py +++ b/rename_plan.py @@ -21,6 +21,8 @@ "NUL", *(f"COM{number}" for number in range(1, 10)), *(f"LPT{number}" for number in range(1, 10)), + *(f"COM{number}" for number in "¹²³"), + *(f"LPT{number}" for number in "¹²³"), } INVALID_FILENAME_CHARS = re.compile(r'[\\/:"*?<>|#,\x00-\x1f]+') @@ -60,13 +62,14 @@ def sanitize_filename(filename: str) -> str: """Return a Windows-safe filename component or raise ``ValueError``. The existing punctuation policy also strips ``#`` and ``,``. Control - characters and trailing periods/spaces are removed. Reserved Windows - basenames are rejected rather than silently renamed to an unrelated file. + characters plus leading/trailing ASCII spaces and trailing periods are + removed. Reserved Windows basenames are rejected rather than silently + renamed to an unrelated file. """ - cleaned = INVALID_FILENAME_CHARS.sub("", filename).rstrip(". ") + cleaned = INVALID_FILENAME_CHARS.sub("", filename).lstrip(" ").rstrip(". ") if not cleaned or not any(character.isalnum() for character in cleaned): raise ValueError("filename is empty after Windows normalization") - stem = cleaned.split(".", 1)[0].upper() + stem = cleaned.split(".", 1)[0].rstrip(" ").upper() if stem in WINDOWS_RESERVED_NAMES: raise ValueError("filename uses reserved Windows basename: {}".format(stem)) return cleaned @@ -117,6 +120,12 @@ def read_plan(path: str | os.PathLike[str]) -> RenamePlan: return plan +def _normalized_destination(destination: str) -> str: + """Return the case-insensitive Windows comparison form for a destination.""" + parent, filename = os.path.split(os.path.normpath(destination)) + return os.path.join(parent, filename.lstrip(" ").rstrip(". ")).casefold() + + def validate_plan(plan: RenamePlan) -> tuple[PlanIssue, ...]: """Validate filesystem and cross-operation safety without modifying files.""" issues: list[PlanIssue] = [] @@ -167,7 +176,7 @@ def validate_plan(plan: RenamePlan) -> tuple[PlanIssue, ...]: "destination already exists", ) ) - normalized = os.path.normpath(operation.destination).rstrip(". ").casefold() + normalized = _normalized_destination(operation.destination) other = destinations.get(normalized) if other is not None: issues.append( diff --git a/tests/test_rename_plan.py b/tests/test_rename_plan.py index affa7b5..4734f8f 100644 --- a/tests/test_rename_plan.py +++ b/tests/test_rename_plan.py @@ -19,11 +19,50 @@ class TestFilenameSanitization(unittest.TestCase): - def test_strips_control_characters_and_trailing_dot_space(self): - self.assertEqual(sanitize_filename("Title\x00 .mp4. "), "Title .mp4") - - def test_rejects_reserved_windows_names_with_extensions(self): - for name in ("CON.mp4", "nul", "COM1.txt", "LPT9 "): + def test_normalizes_windows_unsafe_characters_and_whitespace(self): + cases = ( + ("Title\x00 .mp4. ", "Title .mp4"), + (" Title.mp4", "Title.mp4"), + ('Title<>:"/\\|?*#, .mp4', "Title .mp4"), + ) + for source, expected in cases: + with self.subTest(source=source): + self.assertEqual(sanitize_filename(source), expected) + + def test_rejects_all_reserved_windows_names_with_extensions(self): + reserved_names = ( + "CON", + "PRN", + "AUX", + "NUL", + *(f"COM{number}" for number in range(1, 10)), + *(f"LPT{number}" for number in range(1, 10)), + "COM¹", + "COM²", + "COM³", + "LPT¹", + "LPT²", + "LPT³", + ) + for reserved_name in reserved_names: + for suffix in ("", ".mp4", " .txt"): + name = reserved_name + suffix + with self.subTest(name=name): + with self.assertRaises(ValueError): + sanitize_filename(name) + + @unittest.skipUnless(os.name == "nt", "Windows filesystem verification") + def test_sanitized_name_can_be_used_by_the_windows_filesystem(self): + with tempfile.TemporaryDirectory() as directory: + source = os.path.join(directory, "source.mp4") + destination = os.path.join(directory, sanitize_filename(" Title\x00 .mp4. ")) + with open(source, "w", encoding="utf-8") as source_file: + source_file.write("test") + os.rename(source, destination) + self.assertTrue(os.path.isfile(destination)) + + def test_rejects_empty_filename_after_normalization(self): + for name in (" . ", "\x00"): with self.subTest(name=name): with self.assertRaises(ValueError): sanitize_filename(name) @@ -67,6 +106,18 @@ def test_normalized_duplicate_destinations_block_a_plan(self): ) self.assertIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) + def test_leading_space_normalization_collision_blocks_a_plan(self): + other_source = os.path.join(self.tempdir.name, "other.mp4") + with open(other_source, "w", encoding="utf-8") as other_file: + other_file.write("test") + plan = create_plan( + ( + RenameOperation("1", self.source, self.destination), + RenameOperation("2", other_source, os.path.join(self.tempdir.name, " new.mp4")), + ) + ) + self.assertIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) + def test_write_read_and_apply_require_an_unchanged_valid_plan(self): plan = create_plan((RenameOperation("1", self.source, self.destination),)) plan_path = os.path.join(self.tempdir.name, "plan.json") diff --git a/tests/test_renamer.py b/tests/test_renamer.py index 9fcd829..fd91a4f 100644 --- a/tests/test_renamer.py +++ b/tests/test_renamer.py @@ -115,7 +115,9 @@ def test_edit_db_uses_a_short_lived_handle_and_never_applies(self): finally: os.chdir(original_cwd) self.assertEqual(len(plan.operations), 1) - self.assertEqual(plan.operations[0].destination, "/fixture/Fixture Title.mp4") + self.assertEqual( + plan.operations[0].destination, os.path.join("/fixture", "Fixture Title.mp4") + ) manager.__enter__.assert_called_once_with() manager.__exit__.assert_called_once() From 3bbd747435606acdfac14d99d8ed5be57a4db7c2 Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Tue, 25 Aug 2026 12:14:41 -0600 Subject: [PATCH 2/3] fix: validate saved-plan destinations --- rename_plan.py | 23 +++++++++++++++++++++++ tests/test_rename_plan.py | 10 ++++++++++ 2 files changed, 33 insertions(+) diff --git a/rename_plan.py b/rename_plan.py index fb05a88..47d279b 100644 --- a/rename_plan.py +++ b/rename_plan.py @@ -126,6 +126,18 @@ def _normalized_destination(destination: str) -> str: return os.path.join(parent, filename.lstrip(" ").rstrip(". ")).casefold() +def _destination_filename_error(destination: str) -> str | None: + """Return a safety error when a destination basename is not Windows-safe.""" + filename = os.path.basename(destination) + try: + sanitized = sanitize_filename(filename) + except ValueError as error: + return str(error) + if sanitized != filename: + return "destination filename changes under Windows normalization" + return None + + def validate_plan(plan: RenamePlan) -> tuple[PlanIssue, ...]: """Validate filesystem and cross-operation safety without modifying files.""" issues: list[PlanIssue] = [] @@ -142,6 +154,17 @@ def validate_plan(plan: RenamePlan) -> tuple[PlanIssue, ...]: ) ) continue + filename_error = _destination_filename_error(operation.destination) + if filename_error: + issues.append( + PlanIssue( + operation.scene_id, + operation.source, + operation.destination, + "invalid_destination", + filename_error, + ) + ) if operation.source == operation.destination: continue source_directory = os.path.abspath(os.path.dirname(operation.source)) diff --git a/tests/test_rename_plan.py b/tests/test_rename_plan.py index 4734f8f..de95f4c 100644 --- a/tests/test_rename_plan.py +++ b/tests/test_rename_plan.py @@ -118,6 +118,16 @@ def test_leading_space_normalization_collision_blocks_a_plan(self): ) self.assertIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) + def test_persisted_reserved_windows_destination_blocks_a_plan(self): + plan = create_plan( + (RenameOperation("1", self.source, os.path.join(self.tempdir.name, "COM¹.mp4")),) + ) + plan_path = os.path.join(self.tempdir.name, "plan.json") + write_plan(plan, plan_path) + issues = validate_plan(read_plan(plan_path)) + self.assertEqual([issue.code for issue in issues], ["invalid_destination"]) + self.assertIn("reserved Windows basename", issues[0].message) + def test_write_read_and_apply_require_an_unchanged_valid_plan(self): plan = create_plan((RenameOperation("1", self.source, self.destination),)) plan_path = os.path.join(self.tempdir.name, "plan.json") From d41784e7efd508d80130d691b945afa148bbc821 Mon Sep 17 00:00:00 2001 From: Zachary Williams Date: Tue, 25 Aug 2026 13:03:51 -0600 Subject: [PATCH 3/3] fix: match Windows destination comparison --- rename_plan.py | 3 ++- tests/test_rename_plan.py | 19 +++++++++++++++++++ tests/test_renamer.py | 1 + 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/rename_plan.py b/rename_plan.py index 47d279b..cee8584 100644 --- a/rename_plan.py +++ b/rename_plan.py @@ -7,6 +7,7 @@ from datetime import UTC, datetime import hashlib import json +import ntpath import os from pathlib import Path import re @@ -123,7 +124,7 @@ def read_plan(path: str | os.PathLike[str]) -> RenamePlan: def _normalized_destination(destination: str) -> str: """Return the case-insensitive Windows comparison form for a destination.""" parent, filename = os.path.split(os.path.normpath(destination)) - return os.path.join(parent, filename.lstrip(" ").rstrip(". ")).casefold() + return ntpath.normcase(os.path.join(parent, filename.lstrip(" ").rstrip(". "))) def _destination_filename_error(destination: str) -> str | None: diff --git a/tests/test_rename_plan.py b/tests/test_rename_plan.py index de95f4c..3a41616 100644 --- a/tests/test_rename_plan.py +++ b/tests/test_rename_plan.py @@ -20,6 +20,7 @@ class TestFilenameSanitization(unittest.TestCase): def test_normalizes_windows_unsafe_characters_and_whitespace(self): + """Normalize forbidden characters and Windows-trimmed ASCII whitespace.""" cases = ( ("Title\x00 .mp4. ", "Title .mp4"), (" Title.mp4", "Title.mp4"), @@ -30,6 +31,7 @@ def test_normalizes_windows_unsafe_characters_and_whitespace(self): self.assertEqual(sanitize_filename(source), expected) def test_rejects_all_reserved_windows_names_with_extensions(self): + """Reject standard and superscript Windows device basenames.""" reserved_names = ( "CON", "PRN", @@ -53,6 +55,7 @@ def test_rejects_all_reserved_windows_names_with_extensions(self): @unittest.skipUnless(os.name == "nt", "Windows filesystem verification") def test_sanitized_name_can_be_used_by_the_windows_filesystem(self): + """Rename a temporary file to a sanitized Windows-safe destination.""" with tempfile.TemporaryDirectory() as directory: source = os.path.join(directory, "source.mp4") destination = os.path.join(directory, sanitize_filename(" Title\x00 .mp4. ")) @@ -62,6 +65,7 @@ def test_sanitized_name_can_be_used_by_the_windows_filesystem(self): self.assertTrue(os.path.isfile(destination)) def test_rejects_empty_filename_after_normalization(self): + """Reject names with no usable characters after normalization.""" for name in (" . ", "\x00"): with self.subTest(name=name): with self.assertRaises(ValueError): @@ -107,6 +111,7 @@ def test_normalized_duplicate_destinations_block_a_plan(self): self.assertIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) def test_leading_space_normalization_collision_blocks_a_plan(self): + """Block destinations that Windows treats as equal after trimming.""" other_source = os.path.join(self.tempdir.name, "other.mp4") with open(other_source, "w", encoding="utf-8") as other_file: other_file.write("test") @@ -118,7 +123,21 @@ def test_leading_space_normalization_collision_blocks_a_plan(self): ) self.assertIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) + def test_windows_case_normalization_preserves_sharp_s_distinction(self): + """Allow distinct NTFS names that Unicode case folding would merge.""" + other_source = os.path.join(self.tempdir.name, "other.mp4") + with open(other_source, "w", encoding="utf-8") as other_file: + other_file.write("test") + plan = create_plan( + ( + RenameOperation("1", self.source, os.path.join(self.tempdir.name, "Straße.mp4")), + RenameOperation("2", other_source, os.path.join(self.tempdir.name, "Strasse.mp4")), + ) + ) + self.assertNotIn("duplicate_destination", {issue.code for issue in validate_plan(plan)}) + def test_persisted_reserved_windows_destination_blocks_a_plan(self): + """Reject a reserved device name after reading a digest-valid plan.""" plan = create_plan( (RenameOperation("1", self.source, os.path.join(self.tempdir.name, "COM¹.mp4")),) ) diff --git a/tests/test_renamer.py b/tests/test_renamer.py index fd91a4f..7cf8464 100644 --- a/tests/test_renamer.py +++ b/tests/test_renamer.py @@ -99,6 +99,7 @@ def test_discovery_can_stop_after_first(self): class TestCompatibilityRenderer(unittest.TestCase): def test_edit_db_uses_a_short_lived_handle_and_never_applies(self): + """Keep the compatibility renderer read-only and platform-neutral.""" manager = MagicMock() database = manager.__enter__.return_value database.cursor.fetchall.return_value = [