From 3115d706dc9730a459e693c10d5e5c043ec3e920 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 22:47:05 -0700 Subject: [PATCH 1/9] Fix Windows input identity and private cache protection --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 4 + src/styleprofile/cache.py | 15 ++- src/styleprofile/cache_acl.py | 193 ++++++++++++++++++++++++++++ src/styleprofile/corpus/reading.py | 27 ++-- tests/test_cache.py | 38 +++--- tests/test_evaluate.py | 4 +- tests/test_lean_profile.py | 1 + tests/test_windows_release.py | 195 +++++++++++++++++++++++++++++ 9 files changed, 454 insertions(+), 25 deletions(-) create mode 100644 src/styleprofile/cache_acl.py create mode 100644 tests/test_windows_release.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d52cf2..27174ad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,7 +100,7 @@ jobs: - run: uv sync --locked - name: Confirm spaCy is absent run: uv run python -c "import importlib.util, sys; sys.exit(importlib.util.find_spec('spacy') is not None)" - - run: uv run pytest -n auto tests/test_platforms.py tests/test_module_ownership.py tests/test_report_compatibility.py tests/test_cli_onboarding.py tests/test_input_robustness.py tests/test_demo.py tests/test_library_start.py tests/test_build_speed.py tests/test_concise_output.py tests/test_cache.py::test_a_relative_xdg_cache_home_is_ignored tests/test_throughput.py::test_memory_jobs_follow_physical_memory + - run: uv run pytest -n auto tests/test_platforms.py tests/test_windows_release.py tests/test_module_ownership.py tests/test_report_compatibility.py tests/test_cli_onboarding.py tests/test_input_robustness.py tests/test_demo.py tests/test_library_start.py tests/test_build_speed.py tests/test_concise_output.py tests/test_cache.py::test_a_relative_xdg_cache_home_is_ignored tests/test_throughput.py::test_memory_jobs_follow_physical_memory - run: uv run pytest 'tests/test_snapshots.py::test_snapshot[surface-demo]' tests/test_snapshots.py::test_demo_snapshot_is_independent_of_disk_cache package: diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf9f57..23a3145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,6 +180,10 @@ then `styleprofile setup` for the parser-based metrics. ### Fixed +- **Windows paths and cache privacy.** Directory IDs and ordering are portable, home + directory names stay out of saved reports, and cache files use native private ACLs. + A cache that cannot be protected is left out while measurements continue. + - **References need at least two chunks.** Build refuses smaller references and suggests adding documents or reducing the window size. Incomparable library results are not judged, and any CLI fail flag rejects them with exit status 3. Too-short texts still diff --git a/src/styleprofile/cache.py b/src/styleprofile/cache.py index e4e2a66..7564903 100644 --- a/src/styleprofile/cache.py +++ b/src/styleprofile/cache.py @@ -29,6 +29,7 @@ import time import zlib from collections.abc import Callable, Sequence +from contextlib import closing from dataclasses import dataclass from functools import cache from importlib import import_module @@ -199,6 +200,7 @@ def __init__( import queue self.path = path if path is not None else cache_dir() / FILENAME + self._owned_directory = path is None or self.path.parent == cache_dir() self.max_bytes = max_bytes self.clock = clock self.problem: str | None = None @@ -250,7 +252,15 @@ def _open_or_reset(self) -> sqlite3.Connection | None: def _open(self) -> sqlite3.Connection: import sqlite3 + if sys.platform == "win32": + from styleprofile.cache_acl import check_cache_paths + + check_cache_paths(self.path, owned_directory=self._owned_directory) self.path.parent.mkdir(parents=True, exist_ok=True, mode=0o700) + if sys.platform == "win32": + from styleprofile.cache_acl import protect_cache + + protect_cache(self.path, owned_directory=self._owned_directory) # The file holds pattern counts of the user's texts: private, as they are. if not self.path.exists(): os.close(os.open(self.path, os.O_WRONLY | os.O_CREAT, 0o600)) @@ -526,9 +536,10 @@ def describe(path: Path | None = None) -> tuple[Path, int, int, str | None]: problem = f"{existing} is not writable" if base.exists(): try: - with sqlite3.connect(f"file:{base}?mode=ro", uri=True, timeout=TIMEOUT_S) as db: + with closing( + sqlite3.connect(base.absolute().as_uri() + "?mode=ro", uri=True, timeout=TIMEOUT_S) + ) as db: entries = db.execute("SELECT COUNT(*) FROM entries").fetchone()[0] - db.close() except sqlite3.Error as error: problem = problem or str(error) entries = 0 diff --git a/src/styleprofile/cache_acl.py b/src/styleprofile/cache_acl.py new file mode 100644 index 0000000..2e39d1a --- /dev/null +++ b/src/styleprofile/cache_acl.py @@ -0,0 +1,193 @@ +"""Windows cache privacy, using native ACLs without an optional dependency. + +Only the app's cache directory and its own database files are changed. A caller's +parent directory is inspected, never rewritten. Failure disables caching upstream. +""" + +from __future__ import annotations + +import ctypes +import os +import re +import stat +from pathlib import Path +from typing import Any + +_native: Any = ctypes + + +class _WindowsACL: + def __init__(self) -> None: + # Imported lazily: scores and non-Windows runs do not load Windows libraries. + from ctypes import wintypes + + self.api: Any = _native.WinDLL("advapi32", use_last_error=True) + self.kernel: Any = _native.WinDLL("kernel32", use_last_error=True) + pointer = ctypes.c_void_p + self.kernel.GetCurrentProcess.restype = wintypes.HANDLE + self.kernel.CloseHandle.argtypes = [wintypes.HANDLE] + self.kernel.LocalFree.argtypes = [pointer] + self.kernel.LocalFree.restype = pointer + signatures = { + "OpenProcessToken": ( + [wintypes.HANDLE, wintypes.DWORD, ctypes.POINTER(pointer)], + wintypes.BOOL, + ), + "GetTokenInformation": ( + [pointer, ctypes.c_int, pointer, wintypes.DWORD, ctypes.POINTER(wintypes.DWORD)], + wintypes.BOOL, + ), + "ConvertSidToStringSidW": ([pointer, ctypes.POINTER(pointer)], wintypes.BOOL), + "ConvertStringSecurityDescriptorToSecurityDescriptorW": ( + [wintypes.LPCWSTR, wintypes.DWORD, ctypes.POINTER(pointer), pointer], + wintypes.BOOL, + ), + "ConvertSecurityDescriptorToStringSecurityDescriptorW": ( + [pointer, wintypes.DWORD, wintypes.DWORD, ctypes.POINTER(pointer), pointer], + wintypes.BOOL, + ), + "GetNamedSecurityInfoW": ( + [ + wintypes.LPCWSTR, + ctypes.c_int, + wintypes.DWORD, + ctypes.POINTER(pointer), + pointer, + pointer, + pointer, + ctypes.POINTER(pointer), + ], + wintypes.DWORD, + ), + "SetFileSecurityW": ([wintypes.LPCWSTR, wintypes.DWORD, pointer], wintypes.BOOL), + } + for name, (arguments, result) in signatures.items(): + function = getattr(self.api, name) + function.argtypes = arguments + function.restype = result + token = pointer() + self._check( + self.api.OpenProcessToken(self.kernel.GetCurrentProcess(), 8, ctypes.byref(token)) + ) + try: + size = wintypes.DWORD() + self.api.GetTokenInformation(token, 1, None, 0, ctypes.byref(size)) + if not size.value: + self._check(False) + data = ctypes.create_string_buffer(size.value) + self._check(self.api.GetTokenInformation(token, 1, data, size, ctypes.byref(size))) + self.user = self._sid(pointer.from_buffer(data).value) + finally: + self.kernel.CloseHandle(token) + + @staticmethod + def _check(success: Any) -> None: + if not success: + raise _native.WinError(_native.get_last_error()) + + def _sid(self, value: Any) -> str: + text = ctypes.c_void_p() + self._check(self.api.ConvertSidToStringSidW(value, ctypes.byref(text))) + try: + return ctypes.wstring_at(text) + finally: + self.kernel.LocalFree(text) + + def read(self, path: Path) -> tuple[str, str]: + owner, descriptor = ctypes.c_void_p(), ctypes.c_void_p() + status = self.api.GetNamedSecurityInfoW( + str(path), 1, 5, ctypes.byref(owner), None, None, None, ctypes.byref(descriptor) + ) + if status: + raise _native.WinError(status) + text = ctypes.c_void_p() + try: + self._check( + self.api.ConvertSecurityDescriptorToStringSecurityDescriptorW( + descriptor, 1, 4, ctypes.byref(text), None + ) + ) + return self._sid(owner), ctypes.wstring_at(text) + finally: + if text.value: + self.kernel.LocalFree(text) + self.kernel.LocalFree(descriptor) + + def protect(self, path: Path, *, directory: bool) -> None: + owner, _ = self.read(path) + if owner not in {self.user, "S-1-5-18", "S-1-5-32-544"}: + raise PermissionError(f"the cache path belongs to another user: {path}") + inherit = "OICI" if directory else "" + dacl = "D:P" + "".join(f"(A;{inherit};FA;;;{sid})" for sid in (self.user, "SY", "BA")) + descriptor = ctypes.c_void_p() + self._check( + self.api.ConvertStringSecurityDescriptorToSecurityDescriptorW( + dacl, 1, ctypes.byref(descriptor), None + ) + ) + try: + # SetFileSecurity changes this object; unlike SetNamedSecurityInfo, it + # does not propagate new permissions to unrelated existing children. + self._check(self.api.SetFileSecurityW(str(path), 0x80000004, descriptor)) + finally: + self.kernel.LocalFree(descriptor) + self.verify(path) + + def verify(self, path: Path) -> None: + owner, dacl = self.read(path) + if owner not in {self.user, "S-1-5-18", "S-1-5-32-544"}: + raise PermissionError(f"the cache path belongs to another user: {path}") + allowed = {self.user, "SY", "BA", "S-1-5-18", "S-1-5-32-544"} + entries = re.findall(r"\(([^()]*)\)", dacl) + user_access = False + for entry in entries: + fields = entry.split(";") + if len(fields) != 6: + raise PermissionError(f"the cache ACL cannot be verified: {path}") + kind, flags, rights, _, _, sid = fields + if kind == "D": + continue + if kind != "A" or sid not in allowed: + raise PermissionError(f"the cache ACL grants access beyond this user: {path}") + if sid == self.user and "IO" not in flags and rights in {"FA", "GA", "0x1f01ff"}: + user_access = True + if not entries or not user_access: + raise PermissionError(f"the cache ACL does not grant this user full access: {path}") + + +def check_cache_paths(path: Path, *, owned_directory: bool) -> list[Path]: + """Refuse existing reparse points before creating directories or changing ACLs.""" + directories = [path.parent] + if ( + owned_directory + and path.parent.name == "Cache" + and path.parent.parent.name == "styleprofile" + ): + directories.insert(0, path.parent.parent) + files = [ + Path(f"{path}{suffix}") + for suffix in ("", "-journal", "-wal", "-shm") + if os.path.lexists(f"{path}{suffix}") + ] + for item in [*directories, *files]: + try: + info = item.lstat() + except FileNotFoundError: + continue + if stat.S_ISLNK(info.st_mode) or getattr(info, "st_file_attributes", 0) & 0x400: + raise PermissionError(f"the cache path is a link or reparse point: {item}") + return files + + +def protect_cache(path: Path, *, owned_directory: bool) -> None: + """Protect the owned directory and existing database/sidecars, or fail closed.""" + files = check_cache_paths(path, owned_directory=owned_directory) + acl = _WindowsACL() + if owned_directory: + if path.parent.name == "Cache" and path.parent.parent.name == "styleprofile": + acl.protect(path.parent.parent, directory=True) + acl.protect(path.parent, directory=True) + else: + acl.verify(path.parent) + for item in files: + acl.protect(item, directory=False) diff --git a/src/styleprofile/corpus/reading.py b/src/styleprofile/corpus/reading.py index a2b595e..f796731 100644 --- a/src/styleprofile/corpus/reading.py +++ b/src/styleprofile/corpus/reading.py @@ -9,7 +9,7 @@ from collections import Counter from collections.abc import Sequence from dataclasses import dataclass, field -from pathlib import Path, PurePosixPath +from pathlib import Path, PurePath, PurePosixPath from styleprofile.core import Note, NoteCode, StyleProfileError from styleprofile.corpus.ids import _GROUPED, _PART_SUFFIX, _RECORD, document_of, group_id @@ -256,7 +256,17 @@ def expand_path(value: str) -> Path: """A path as typed with ``~`` expanded, as an error rather than a crash when it names an unknown user (``~other``).""" try: - return Path(value).expanduser() + expanded = Path(value).expanduser() + user = re.split(r"[/\\]", value, maxsplit=1)[0] + # Windows infers another user's path without checking that home exists. + if ( + os.name == "nt" + and user.startswith("~") + and len(user) > 1 + and not Path(user).expanduser().is_dir() + ): + raise StyleProfileError(f"{value}: cannot find that user's home", code="not_found") + return expanded except RuntimeError as error: raise StyleProfileError(f"{value}: {error}", code="not_found") from error @@ -286,11 +296,14 @@ def root_name(value: str) -> str: OTHER_HOMES = (Path("/root"),) -def _is_home(absolute: Path) -> bool: +def _is_home(absolute: PurePath) -> bool: """Whether ``absolute`` is a home directory, whose name is a user name.""" - if absolute == Path(os.path.abspath(Path.home())) or absolute in OTHER_HOMES: + root = absolute.parents[-1] if absolute.parents else absolute + if absolute == Path(os.path.abspath(Path.home())) or absolute in ( + root / home.name for home in OTHER_HOMES + ): return True - return absolute.parent in HOME_PARENTS + return absolute.parent in (root / parent.name for parent in HOME_PARENTS) @dataclass @@ -502,7 +515,7 @@ def _walk(path: Path) -> tuple[list[Path], list[Path], list[str]]: readable: list[Path] = [] skipped: list[Path] = [] generated: list[str] = [] - for item in sorted(path.rglob("*")): + for item in sorted(path.rglob("*"), key=lambda item: item.as_posix()): parts = item.relative_to(path).parts if not item.is_file() or any( part.startswith(".") or part in SKIPPED_DIRS for part in parts[:-1] @@ -618,7 +631,7 @@ def load_chunks( ) sources = _name_sources(value, path, files, names) for item, source in zip(files, sources, strict=True): - chunk_id = str(item.relative_to(path)) + chunk_id = item.relative_to(path).as_posix() label = str(Path(value) / chunk_id) chunks.extend( _file_chunks( diff --git a/tests/test_cache.py b/tests/test_cache.py index 44d1887..d29cc77 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -125,7 +125,7 @@ def counting_spans(parts: Any, whole: Any) -> Any: monkeypatch.setattr(measure, "measure_spans", counting_spans) warm = sp.build(WRITER, settings, contrast=CONTRAST, cache=True, passages=True) - assert counting.texts == [] # every chunk and piece came from the cache + assert counting.texts == [], (cold.notes, warm.notes) # every chunk and piece came from cache assert spans == [] # and every document read in parts for drift calibration assert "drift" in (warm.report.get("calibration") or {}) cold.save(tmp_path / "cold.json") @@ -200,7 +200,7 @@ def test_adding_a_document_measures_only_the_new_one( counting = Counting(monkeypatch) grown = sp.build(corpus, settings, cache=True) new_windows = window(load_chunks([str(new)]), settings.window_words) - assert len(counting.texts) == len(new_windows) + assert len(counting.texts) == len(new_windows), grown.notes # And the profile is the one a build without the cache gives. cold = sp.build(corpus, settings, cache=False) assert dumps_report(grown.report) == dumps_report(cold.report) @@ -261,8 +261,18 @@ def test_a_relative_xdg_cache_home_is_ignored(monkeypatch: pytest.MonkeyPatch) - def test_the_cache_file_is_private(cache_home: Path) -> None: sp.build(WRITER, sp.Settings(syntax=False), cache=True) - assert cache_home.stat().st_mode & 0o077 == 0 - assert cache_home.parent.stat().st_mode & 0o077 == 0 + if sys.platform == "win32": + from styleprofile.cache_acl import _WindowsACL + + acl = _WindowsACL() + acl.verify(cache_home) + acl.verify(cache_home.parent) + journal = Path(f"{cache_home}-journal") + if journal.exists(): + acl.verify(journal) + else: + assert cache_home.stat().st_mode & 0o077 == 0 + assert cache_home.parent.stat().st_mode & 0o077 == 0 # Size and sharing. @@ -441,19 +451,19 @@ def test_scoring_leaves_nothing_in_the_cache_unless_asked( assert caching.describe(cache_home)[2] > entries -@pytest.mark.skipif( - hasattr(os, "geteuid") and os.geteuid() == 0, - reason="root can write to folders regardless of permission bits", -) def test_the_cache_command_says_when_the_cache_is_unavailable( - cache_home: Path, capsys: pytest.CaptureFixture[str] + cache_home: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: cache_home.parent.mkdir(parents=True) - cache_home.parent.chmod(0o500) - try: - assert main(["cache"]) == 0 - finally: - cache_home.parent.chmod(0o700) + # Windows chmod changes the read-only flag, not directory write access. Inject + # the access-check result to exercise the same unavailable-command contract. + access = caching.os.access + monkeypatch.setattr( + caching.os, + "access", + lambda path, mode: False if Path(path) == cache_home.parent else access(path, mode), + ) + assert main(["cache"]) == 0 out = capsys.readouterr().out assert "unavailable:" in out and "not writable" in out diff --git a/tests/test_evaluate.py b/tests/test_evaluate.py index d81fc78..f875833 100644 --- a/tests/test_evaluate.py +++ b/tests/test_evaluate.py @@ -233,7 +233,9 @@ def test_an_edit_of_a_duplicate_draft_file_pairs_with_the_copy_kept(tmp_path: Pa import styleprofile as sp author, drafts = _corpus(tmp_path) - (drafts / "draft5.md").write_text((drafts / "draft1.md").read_text(encoding="utf-8")) + (drafts / "draft5.md").write_text( + (drafts / "draft1.md").read_text(encoding="utf-8"), encoding="utf-8" + ) edited = tmp_path / "edited" _edit(drafts, edited, _strip_dashes) (edited / "draft1.md").unlink() diff --git a/tests/test_lean_profile.py b/tests/test_lean_profile.py index a42bc4e..baef931 100644 --- a/tests/test_lean_profile.py +++ b/tests/test_lean_profile.py @@ -165,6 +165,7 @@ def test_home_and_filesystem_roots_save_as_input( home = corpus / user # a home directory named like the user, as on most systems shutil.copytree(corpus / "writer", home) monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("USERPROFILE", str(home)) assert root_name("~") == root_name(str(home)) == root_name("/") == "input" assert root_name("~/old-maps.md") == "old-maps.md" diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py new file mode 100644 index 0000000..ad324f5 --- /dev/null +++ b/tests/test_windows_release.py @@ -0,0 +1,195 @@ +"""Release regressions for native Windows paths and private measurement caches.""" + +from __future__ import annotations + +import ctypes +import sys +from pathlib import Path, PureWindowsPath + +import pytest + +import styleprofile as sp +from styleprofile import cache as caching +from styleprofile.cache_acl import _WindowsACL +from styleprofile.reports import dumps_report + +ROOT = Path(__file__).resolve().parent.parent + + +def test_cache_protection_failure_leaves_measurements_unchanged( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from styleprofile import cache_acl + + expected = sp.build(ROOT / "examples/writer", sp.Settings(syntax=False), cache=False) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + monkeypatch.setattr(caching.sys, "platform", "win32") + + def denied(path: Path, *, owned_directory: bool) -> None: + raise PermissionError("cannot protect the cache ACL") + + monkeypatch.setattr(cache_acl, "protect_cache", denied) + actual = sp.build(ROOT / "examples/writer", sp.Settings(syntax=False), cache=True) + assert dumps_report(actual.report) == dumps_report(expected.report) + assert any(note.code is sp.NoteCode.CACHE_UNAVAILABLE for note in actual.notes) + assert not (tmp_path / "styleprofile" / caching.FILENAME).exists() + + +@pytest.mark.parametrize("sid", ["WD", "BU", "S-1-5-21-123-456-789-1000"]) +def test_acl_validation_rejects_other_principals(sid: str, monkeypatch: pytest.MonkeyPatch) -> None: + # Exercise the actual validation independently of host OS and localized account names. + acl = object.__new__(_WindowsACL) + acl.user = "S-1-5-21-1-2-3-1000" + monkeypatch.setattr( + acl, "read", lambda path: (acl.user, f"D:(A;;FA;;;{acl.user})(A;;FR;;;{sid})") + ) + with pytest.raises(PermissionError, match="beyond this user"): + acl.verify(Path("cache")) + + +@pytest.mark.skipif(sys.platform != "win32", reason="native Windows ACL integration") +@pytest.mark.parametrize("environment", ["XDG_CACHE_HOME", "LOCALAPPDATA"]) +def test_windows_cache_override_is_private_without_changing_parent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, environment: str +) -> None: + # Explicitly grant Everyone access to the caller's override. The app must only + # protect its own child, not rewrite these unrelated parent permissions. + acl = _WindowsACL() + descriptor = ctypes.c_void_p() + acl._check( + acl.api.ConvertStringSecurityDescriptorToSecurityDescriptorW( + f"D:P(A;OICI;FA;;;{acl.user})(A;OICI;FA;;;WD)", 1, ctypes.byref(descriptor), None + ) + ) + try: + acl._check(acl.api.SetFileSecurityW(str(tmp_path), 0x80000004, descriptor)) + finally: + acl.kernel.LocalFree(descriptor) + before = acl.read(tmp_path) + monkeypatch.delenv("XDG_CACHE_HOME", raising=False) + monkeypatch.setenv(environment, str(tmp_path)) + settings = sp.Settings(syntax=False) + cold = sp.build(ROOT / "examples/writer", settings, cache=True) + warm = sp.build(ROOT / "examples/writer", settings, cache=True) + assert dumps_report(cold.report) == dumps_report(warm.report) + assert not any( + note.code is sp.NoteCode.CACHE_UNAVAILABLE for note in (*cold.notes, *warm.notes) + ) + path = caching.cache_dir() / caching.FILENAME + acl.verify(path.parent) + if environment == "LOCALAPPDATA": + acl.verify(path.parent.parent) + acl.verify(path) + journal = Path(f"{path}-journal") + if journal.exists(): + acl.verify(journal) + assert acl.read(tmp_path) == before + + +def test_cache_description_handles_uri_characters_and_relative_paths( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + path = Path("cache #1?.sqlite3") if sys.platform != "win32" else Path("cache #1.sqlite3") + with caching.MeasurementCache(path) as store: + store.put(b"one", 3, {"value": 1}) + assert caching.describe(path)[2:] == (1, None) + assert caching.clear(path) + + +def test_cache_description_closes_a_connection_when_a_query_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import sqlite3 + + path = tmp_path / "empty.sqlite3" + path.touch() + closed = [] + + class BrokenQuery: + def execute(self, query: str) -> None: + raise sqlite3.OperationalError("no entries table") + + def close(self) -> None: + closed.append(True) + + monkeypatch.setattr(sqlite3, "connect", lambda *args, **kwargs: BrokenQuery()) + assert caching.describe(path)[2:] == (0, "no entries table") + assert closed == [True] + + +@pytest.mark.parametrize("linked", ["directory", "database", "journal"]) +def test_cache_acl_refuses_links_before_touching_unrelated_targets( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, linked: str +) -> None: + from styleprofile import cache_acl + + target = tmp_path / "unrelated" + target.mkdir() + original = target / "private.txt" + original.write_text("unrelated data", encoding="utf-8") + folder = tmp_path / "styleprofile" + if linked == "directory": + folder.symlink_to(target, target_is_directory=True) + else: + folder.mkdir() + suffix = "-journal" if linked == "journal" else "" + Path(f"{folder / caching.FILENAME}{suffix}").symlink_to(original) + + def unexpected_acl() -> None: + pytest.fail("refusal must precede any Windows ACL access or mutation") + + monkeypatch.setattr(cache_acl, "_WindowsACL", unexpected_acl) + with pytest.raises(PermissionError, match="link or reparse point"): + cache_acl.protect_cache(folder / caching.FILENAME, owned_directory=True) + assert original.read_text(encoding="utf-8") == "unrelated data" + + +@pytest.mark.skipif(sys.platform != "win32", reason="native Windows junction integration") +def test_windows_cache_junction_is_refused_and_scoring_continues( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import subprocess + + target = tmp_path / "unrelated" + target.mkdir() + acl = _WindowsACL() + before = acl.read(target) + subprocess.run( + ["cmd", "/c", "mklink", "/J", str(tmp_path / "styleprofile"), str(target)], + check=True, + capture_output=True, + ) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + settings = sp.Settings(syntax=False) + expected = sp.build(ROOT / "examples/writer", settings, cache=False) + actual = sp.build(ROOT / "examples/writer", settings, cache=True) + assert dumps_report(actual.report) == dumps_report(expected.report) + assert any(note.code is sp.NoteCode.CACHE_UNAVAILABLE for note in actual.notes) + assert not (target / caching.FILENAME).exists() + assert acl.read(target) == before + + +def test_windows_owned_root_link_is_refused_before_creating_cache_children( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "unrelated" + target.mkdir() + (tmp_path / "styleprofile").symlink_to(target, target_is_directory=True) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + monkeypatch.setattr(caching.sys, "platform", "win32") + # The native default has a Cache child below the owned styleprofile root. + store = caching.MeasurementCache(tmp_path / "styleprofile/Cache" / caching.FILENAME) + store._owned_directory = True + assert store.fetch(b"one") is None + store.close() + assert store.problem and "link or reparse point" in store.problem + assert list(target.iterdir()) == [] + + +@pytest.mark.parametrize("home", ["C:/Users/alice", "D:/Users/bob", "D:/home/bob", "C:/root"]) +def test_home_identity_does_not_depend_on_current_drive(home: str) -> None: + from styleprofile.corpus.reading import _is_home + + assert _is_home(PureWindowsPath(home)) + assert not _is_home(PureWindowsPath(home) / "posts") From 318f5d067d75d2674d9fc2f9ba1ee657d0e7cd82 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:06:39 -0700 Subject: [PATCH 2/9] fix: verify native cache ACEs and refuse linked targets --- src/styleprofile/cache_acl.py | 76 ++++++++++++++++++------- tests/test_windows_release.py | 102 +++++++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/src/styleprofile/cache_acl.py b/src/styleprofile/cache_acl.py index 2e39d1a..7c06d36 100644 --- a/src/styleprofile/cache_acl.py +++ b/src/styleprofile/cache_acl.py @@ -8,7 +8,6 @@ import ctypes import os -import re import stat from pathlib import Path from typing import Any @@ -60,6 +59,7 @@ def __init__(self) -> None: wintypes.DWORD, ), "SetFileSecurityW": ([wintypes.LPCWSTR, wintypes.DWORD, pointer], wintypes.BOOL), + "GetAce": ([pointer, wintypes.DWORD, ctypes.POINTER(pointer)], wintypes.BOOL), } for name, (arguments, result) in signatures.items(): function = getattr(self.api, name) @@ -93,13 +93,24 @@ def _sid(self, value: Any) -> str: finally: self.kernel.LocalFree(text) - def read(self, path: Path) -> tuple[str, str]: - owner, descriptor = ctypes.c_void_p(), ctypes.c_void_p() + def _read(self, path: Path) -> tuple[Any, Any, Any]: + owner, dacl, descriptor = ctypes.c_void_p(), ctypes.c_void_p(), ctypes.c_void_p() status = self.api.GetNamedSecurityInfoW( - str(path), 1, 5, ctypes.byref(owner), None, None, None, ctypes.byref(descriptor) + str(path), + 1, + 5, + ctypes.byref(owner), + None, + ctypes.byref(dacl), + None, + ctypes.byref(descriptor), ) if status: raise _native.WinError(status) + return owner, dacl, descriptor + + def read(self, path: Path) -> tuple[str, str]: + owner, _, descriptor = self._read(path) text = ctypes.c_void_p() try: self._check( @@ -131,27 +142,47 @@ def protect(self, path: Path, *, directory: bool) -> None: self._check(self.api.SetFileSecurityW(str(path), 0x80000004, descriptor)) finally: self.kernel.LocalFree(descriptor) - self.verify(path) + self.verify(path, directory=directory) + + def _entries(self, path: Path) -> tuple[str, list[tuple[int, int, int, str]]]: + """Read numeric SIDs and access masks, without SDDL's account aliases.""" + owner, dacl, descriptor = self._read(path) + try: + if not dacl.value: # A null DACL grants everyone full access. + raise PermissionError(f"the cache has no access control list: {path}") + # ACL's AceCount is a WORD at offset 4 (the fixed ACL header is 8 bytes). + count = ctypes.c_uint16.from_address(dacl.value + 4).value + entries = [] + for index in range(count): + ace = ctypes.c_void_p() + self._check(self.api.GetAce(dacl, index, ctypes.byref(ace))) + address = ace.value + if address is None: + raise PermissionError(f"the cache ACL cannot be verified: {path}") + kind = ctypes.c_ubyte.from_address(address).value + flags = ctypes.c_ubyte.from_address(address + 1).value + if kind != 0: # Only ordinary ACCESS_ALLOWED_ACE has this layout. + raise PermissionError(f"the cache ACL cannot be verified: {path}") + mask = ctypes.c_uint32.from_address(address + 4).value + entries.append((kind, flags, mask, self._sid(address + 8))) + return self._sid(owner), entries + finally: + self.kernel.LocalFree(descriptor) - def verify(self, path: Path) -> None: - owner, dacl = self.read(path) + def verify(self, path: Path, *, directory: bool = False) -> None: + owner, entries = self._entries(path) if owner not in {self.user, "S-1-5-18", "S-1-5-32-544"}: raise PermissionError(f"the cache path belongs to another user: {path}") - allowed = {self.user, "SY", "BA", "S-1-5-18", "S-1-5-32-544"} - entries = re.findall(r"\(([^()]*)\)", dacl) + allowed = {self.user, "S-1-5-18", "S-1-5-32-544"} user_access = False - for entry in entries: - fields = entry.split(";") - if len(fields) != 6: - raise PermissionError(f"the cache ACL cannot be verified: {path}") - kind, flags, rights, _, _, sid = fields - if kind == "D": - continue - if kind != "A" or sid not in allowed: + user_inheritance = False + for kind, flags, rights, sid in entries: + if kind != 0 or sid not in allowed: raise PermissionError(f"the cache ACL grants access beyond this user: {path}") - if sid == self.user and "IO" not in flags and rights in {"FA", "GA", "0x1f01ff"}: + if sid == self.user and not flags & 8 and rights in {0x1F01FF, 0x10000000}: user_access = True - if not entries or not user_access: + user_inheritance |= flags & 3 == 3 + if not entries or not user_access or (directory and not user_inheritance): raise PermissionError(f"the cache ACL does not grant this user full access: {path}") @@ -169,13 +200,16 @@ def check_cache_paths(path: Path, *, owned_directory: bool) -> list[Path]: for suffix in ("", "-journal", "-wal", "-shm") if os.path.lexists(f"{path}{suffix}") ] - for item in [*directories, *files]: + # Check the entire path chain: lstat of a leaf does not expose a linked ancestor. + for item in dict.fromkeys([*directories, *path.parent.parents, *files]): try: info = item.lstat() except FileNotFoundError: continue if stat.S_ISLNK(info.st_mode) or getattr(info, "st_file_attributes", 0) & 0x400: raise PermissionError(f"the cache path is a link or reparse point: {item}") + if item in files and info.st_nlink != 1: + raise PermissionError(f"the cache path has multiple hard links: {item}") return files @@ -188,6 +222,6 @@ def protect_cache(path: Path, *, owned_directory: bool) -> None: acl.protect(path.parent.parent, directory=True) acl.protect(path.parent, directory=True) else: - acl.verify(path.parent) + acl.verify(path.parent, directory=True) for item in files: acl.protect(item, directory=False) diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index ad324f5..3633f63 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -41,12 +41,37 @@ def test_acl_validation_rejects_other_principals(sid: str, monkeypatch: pytest.M acl = object.__new__(_WindowsACL) acl.user = "S-1-5-21-1-2-3-1000" monkeypatch.setattr( - acl, "read", lambda path: (acl.user, f"D:(A;;FA;;;{acl.user})(A;;FR;;;{sid})") + acl, + "_entries", + lambda path: (acl.user, [(0, 0, 0x1F01FF, acl.user), (0, 0, 0x120089, sid)]), ) with pytest.raises(PermissionError, match="beyond this user"): acl.verify(Path("cache")) +def test_acl_validation_rejects_denied_access(monkeypatch: pytest.MonkeyPatch) -> None: + acl = object.__new__(_WindowsACL) + acl.user = "S-1-5-21-1-2-3-1000" + monkeypatch.setattr( + acl, + "_entries", + lambda path: (acl.user, [(1, 0, 0x1F01FF, acl.user), (0, 0, 0x1F01FF, acl.user)]), + ) + with pytest.raises(PermissionError): + acl.verify(Path("cache")) + + +def test_explicit_cache_parent_requires_private_inheritance( + monkeypatch: pytest.MonkeyPatch, +) -> None: + acl = object.__new__(_WindowsACL) + acl.user = "S-1-5-21-1-2-3-1000" + monkeypatch.setattr(acl, "_entries", lambda path: (acl.user, [(0, 0, 0x1F01FF, acl.user)])) + acl.verify(Path("cache")) + with pytest.raises(PermissionError): + acl.verify(Path("cache"), directory=True) + + @pytest.mark.skipif(sys.platform != "win32", reason="native Windows ACL integration") @pytest.mark.parametrize("environment", ["XDG_CACHE_HOME", "LOCALAPPDATA"]) def test_windows_cache_override_is_private_without_changing_parent( @@ -74,7 +99,7 @@ def test_windows_cache_override_is_private_without_changing_parent( assert dumps_report(cold.report) == dumps_report(warm.report) assert not any( note.code is sp.NoteCode.CACHE_UNAVAILABLE for note in (*cold.notes, *warm.notes) - ) + ), (cold.notes, warm.notes) path = caching.cache_dir() / caching.FILENAME acl.verify(path.parent) if environment == "LOCALAPPDATA": @@ -93,6 +118,7 @@ def test_cache_description_handles_uri_characters_and_relative_paths( path = Path("cache #1?.sqlite3") if sys.platform != "win32" else Path("cache #1.sqlite3") with caching.MeasurementCache(path) as store: store.put(b"one", 3, {"value": 1}) + assert store.problem is None, store.problem assert caching.describe(path)[2:] == (1, None) assert caching.clear(path) @@ -193,3 +219,75 @@ def test_home_identity_does_not_depend_on_current_drive(home: str) -> None: assert _is_home(PureWindowsPath(home)) assert not _is_home(PureWindowsPath(home) / "posts") + + +@pytest.mark.parametrize("suffix", ["", "-journal", "-wal", "-shm"]) +def test_cache_hard_links_are_refused_before_acl_changes( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, suffix: str +) -> None: + import os + + from styleprofile import cache_acl + + original = tmp_path / "unrelated.txt" + original.write_text("unrelated data", encoding="utf-8") + folder = tmp_path / "styleprofile" + folder.mkdir() + path = folder / caching.FILENAME + os.link(original, Path(f"{path}{suffix}")) + monkeypatch.setattr(cache_acl, "_WindowsACL", lambda: pytest.fail("must refuse before ACL")) + with pytest.raises(PermissionError, match="multiple hard links"): + cache_acl.protect_cache(path, owned_directory=True) + assert original.read_text(encoding="utf-8") == "unrelated data" + + +def test_cache_linked_ancestor_is_refused_before_creating_children( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "unrelated" + target.mkdir() + (tmp_path / "override").symlink_to(target, target_is_directory=True) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "override")) + monkeypatch.setattr(caching.sys, "platform", "win32") + with caching.MeasurementCache() as store: + assert store.fetch(b"one") is None + assert store.problem and "link or reparse point" in store.problem + assert list(target.iterdir()) == [] + + +@pytest.mark.skipif(sys.platform != "win32", reason="native Windows ACL integration") +def test_native_acl_preserves_unrelated_children_and_reads_numeric_user(tmp_path: Path) -> None: + acl = _WindowsACL() + folder = tmp_path / "styleprofile" + folder.mkdir() + child = folder / "unrelated.txt" + child.write_text("unrelated data", encoding="utf-8") + before = acl.read(child) + acl.protect(folder, directory=True) + assert acl.read(child) == before + owner, entries = acl._entries(folder) + assert owner == acl.user + assert {entry[3] for entry in entries} == {acl.user, "S-1-5-18", "S-1-5-32-544"} + acl.verify(folder, directory=True) + + +@pytest.mark.skipif(sys.platform != "win32", reason="native Windows ACL integration") +def test_native_denied_acl_is_not_reported_as_usable(tmp_path: Path) -> None: + acl = _WindowsACL() + path = tmp_path / "denied.txt" + path.touch() + descriptor = ctypes.c_void_p() + acl._check( + acl.api.ConvertStringSecurityDescriptorToSecurityDescriptorW( + f"D:P(D;;FW;;;{acl.user})(A;;FA;;;{acl.user})", 1, ctypes.byref(descriptor), None + ) + ) + try: + acl._check(acl.api.SetFileSecurityW(str(path), 0x80000004, descriptor)) + finally: + acl.kernel.LocalFree(descriptor) + try: + with pytest.raises(PermissionError): + acl.verify(path) + finally: + acl.protect(path, directory=False) From 048a5c731fa0c53afc5c424a4048c4053bca89a2 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:10:04 -0700 Subject: [PATCH 3/9] test: preserve native owner and expose ACL failure diagnostics --- tests/test_windows_release.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index 3633f63..ff62df5 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -97,9 +97,14 @@ def test_windows_cache_override_is_private_without_changing_parent( cold = sp.build(ROOT / "examples/writer", settings, cache=True) warm = sp.build(ROOT / "examples/writer", settings, cache=True) assert dumps_report(cold.report) == dumps_report(warm.report) - assert not any( - note.code is sp.NoteCode.CACHE_UNAVAILABLE for note in (*cold.notes, *warm.notes) - ), (cold.notes, warm.notes) + assert ( + "\n".join( + note.message + for note in (*cold.notes, *warm.notes) + if note.code is sp.NoteCode.CACHE_UNAVAILABLE + ) + == "" + ) path = caching.cache_dir() / caching.FILENAME acl.verify(path.parent) if environment == "LOCALAPPDATA": @@ -262,11 +267,12 @@ def test_native_acl_preserves_unrelated_children_and_reads_numeric_user(tmp_path folder.mkdir() child = folder / "unrelated.txt" child.write_text("unrelated data", encoding="utf-8") + original_owner = acl.read(folder)[0] before = acl.read(child) acl.protect(folder, directory=True) assert acl.read(child) == before owner, entries = acl._entries(folder) - assert owner == acl.user + assert owner == original_owner # Elevated Windows creation can choose Administrators. assert {entry[3] for entry in entries} == {acl.user, "S-1-5-18", "S-1-5-32-544"} acl.verify(folder, directory=True) From f52e60a9afdd9e0730c6f7fe62eec102d60213d6 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:11:48 -0700 Subject: [PATCH 4/9] test: capture native SQLite failure codes for cache regression --- tests/test_windows_release.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index ff62df5..cdfabad 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -91,11 +91,20 @@ def test_windows_cache_override_is_private_without_changing_parent( finally: acl.kernel.LocalFree(descriptor) before = acl.read(tmp_path) + failures: list[str] = [] + fail = caching.MeasurementCache._fail + + def capture_failure(self: caching.MeasurementCache, error: BaseException | str) -> None: + failures.append(f"{error!r}: {getattr(error, 'sqlite_errorname', '')}") + fail(self, error) + + monkeypatch.setattr(caching.MeasurementCache, "_fail", capture_failure) monkeypatch.delenv("XDG_CACHE_HOME", raising=False) monkeypatch.setenv(environment, str(tmp_path)) settings = sp.Settings(syntax=False) cold = sp.build(ROOT / "examples/writer", settings, cache=True) warm = sp.build(ROOT / "examples/writer", settings, cache=True) + assert failures == [], "\n".join(failures) assert dumps_report(cold.report) == dumps_report(warm.report) assert ( "\n".join( From a023f90b1670e38c1302761739234a7c965468b6 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:15:29 -0700 Subject: [PATCH 5/9] fix: select persistent journal before cache initialization writes --- src/styleprofile/cache.py | 5 ++++- tests/test_windows_release.py | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/styleprofile/cache.py b/src/styleprofile/cache.py index 7564903..cc41465 100644 --- a/src/styleprofile/cache.py +++ b/src/styleprofile/cache.py @@ -272,13 +272,16 @@ def _open(self) -> sqlite3.Connection: # time (4 KB at a time took several times longer on a busy macOS disk), and # pages freed by pruning go back to the file system. connection.execute(f"PRAGMA page_size={PAGE_SIZE}") + # Set the connection's journal policy before auto_vacuum, which can write + # even when its value is unchanged. A new connection defaults to DELETE: + # on Windows it cannot delete a journal another PERSIST connection holds. + connection.execute("PRAGMA journal_mode=PERSIST") connection.execute("PRAGMA auto_vacuum=INCREMENTAL") new = connection.execute("PRAGMA user_version").fetchall()[0][0] != FORMAT # A rollback journal kept between writes rather than deleted: a run that only # reads opens one file and writes none, and writes don't create a file each time. # On a busy disk, creating and growing files is what is slow: a write-ahead log # made reading a warm cache 0.1-0.3 s slower. - connection.execute("PRAGMA journal_mode=PERSIST") connection.execute(f"PRAGMA journal_size_limit={JOURNAL_LIMIT}") # A cache need not survive a power cut, so writes are never synced to disk; a # file damaged that way is started afresh (``_open_or_reset``). diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index cdfabad..a20c194 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -158,6 +158,27 @@ def close(self) -> None: assert closed == [True] +def test_cache_reader_and_writer_keep_the_persistent_journal(tmp_path: Path) -> None: + path = tmp_path / "cache.sqlite3" + with caching.MeasurementCache(path) as store: + assert store.fetch(b"one") is None # Keep the reader connection open. + store.put(b"one", 3, {"value": 1}) + assert store.problem is None, store.problem + with caching.MeasurementCache(path) as warm: + assert warm.fetch(b"one") is not None + warm.put(b"two", 4, {"value": 2}) + assert warm.problem is None, warm.problem + with caching.MeasurementCache(path) as again: + assert again.fetch(b"one") is not None + assert again.fetch(b"two") is not None + db = again._connection + assert db is not None + assert db.execute("PRAGMA journal_mode").fetchone()[0] == "persist" + assert db.execute("PRAGMA page_size").fetchone()[0] == caching.PAGE_SIZE + assert db.execute("PRAGMA auto_vacuum").fetchone()[0] == 2 + assert again.problem is None, again.problem + + @pytest.mark.parametrize("linked", ["directory", "database", "journal"]) def test_cache_acl_refuses_links_before_touching_unrelated_targets( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, linked: str From c4aa17b3b70bdc338d22fcaea2680a4da3316a15 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:17:41 -0700 Subject: [PATCH 6/9] fix: preserve component order across corpus platforms --- src/styleprofile/corpus/reading.py | 2 +- tests/test_windows_release.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/styleprofile/corpus/reading.py b/src/styleprofile/corpus/reading.py index f796731..c6407f7 100644 --- a/src/styleprofile/corpus/reading.py +++ b/src/styleprofile/corpus/reading.py @@ -515,7 +515,7 @@ def _walk(path: Path) -> tuple[list[Path], list[Path], list[str]]: readable: list[Path] = [] skipped: list[Path] = [] generated: list[str] = [] - for item in sorted(path.rglob("*"), key=lambda item: item.as_posix()): + for item in sorted(path.rglob("*"), key=lambda item: item.parts): parts = item.relative_to(path).parts if not item.is_file() or any( part.startswith(".") or part in SKIPPED_DIRS for part in parts[:-1] diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index a20c194..cfac2d5 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -16,6 +16,16 @@ ROOT = Path(__file__).resolve().parent.parent +def test_portable_walk_preserves_posix_component_order(tmp_path: Path) -> None: + from styleprofile.corpus.reading import load_chunks + + (tmp_path / "a").mkdir() + for name in ("a/inside.md", "a.md", "Z.md", "b.md"): + (tmp_path / name).write_text("A short document to read.", encoding="utf-8") + chunks = load_chunks([str(tmp_path)]) + assert [chunk.id for chunk in chunks] == ["Z.md", "a/inside.md", "a.md", "b.md"] + + def test_cache_protection_failure_leaves_measurements_unchanged( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From ae43617bb74b6e1b0d07ee1b72ea99a2ec7866b1 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:20:18 -0700 Subject: [PATCH 7/9] test: use native read-only cache failure on Windows --- tests/test_cache.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_cache.py b/tests/test_cache.py index d29cc77..a72a8b8 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -379,13 +379,20 @@ def test_a_damaged_cache_in_a_read_only_folder_is_left_out( settings = sp.Settings(syntax=False) expected = dumps_report(sp.build(WRITER, settings, cache=False).report) cache_home.parent.mkdir(parents=True) - cache_home.write_bytes(b"\x00garbage" * 1000) # not a database, and cannot be replaced + damaged = b"\x00garbage" * 1000 + cache_home.write_bytes(damaged) # not a database, and cannot be replaced cache_home.parent.chmod(0o500) + if sys.platform == "win32": + # Windows directory chmod does not deny writes. A read-only file cannot be + # deleted to reset this damaged database, regardless of its parent's mode. + cache_home.chmod(0o400) try: profile = sp.build(WRITER, settings, cache=True) finally: cache_home.parent.chmod(0o700) + cache_home.chmod(0o600) assert dumps_report(profile.report) == expected + assert cache_home.read_bytes() == damaged [note] = _notes(profile) assert str(cache_home) in note.message From 8161745acf1a866ec38e2330319baf57d7d79619 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:25:17 -0700 Subject: [PATCH 8/9] ci: require Windows suite after exact native pass --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 27174ad..e74eacd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,9 +54,6 @@ jobs: name: test (Python 3.11, ${{ matrix.os }}, syntax=${{ matrix.syntax }}) runs-on: ${{ matrix.os }} timeout-minutes: 15 - # Pre-existing Windows full-suite failures are listed in this PR; WP-3 and WP-5 regressions - # remain required in windows-platforms below. - continue-on-error: ${{ matrix.os == 'windows-latest' }} strategy: fail-fast: false matrix: From 96ce2727e6056efb81b650782818f3e9a6677610 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Sun, 4 Oct 2026 23:44:13 -0700 Subject: [PATCH 9/9] Stop cache writers after failures --- src/styleprofile/cache.py | 6 ++-- tests/test_windows_release.py | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/styleprofile/cache.py b/src/styleprofile/cache.py index cc41465..69bdf0f 100644 --- a/src/styleprofile/cache.py +++ b/src/styleprofile/cache.py @@ -429,8 +429,10 @@ def close(self) -> None: self.flush() if self._writer is not None: - # A writer already given up on is not waited for (it is a daemon thread). - if self.problem is None and self._hand(None): + # A failed writer still drains its queue. Give it the stop signal too, + # so it closes its connection instead of waiting forever for another batch. + # Both handing off and waiting remain bounded for an unresponsive writer. + if self._hand(None): self._wait_for_writer(self._writer) self._writer = None if self._connection is not None: diff --git a/tests/test_windows_release.py b/tests/test_windows_release.py index cfac2d5..d47dc66 100644 --- a/tests/test_windows_release.py +++ b/tests/test_windows_release.py @@ -189,6 +189,67 @@ def test_cache_reader_and_writer_keep_the_persistent_journal(tmp_path: Path) -> assert again.problem is None, again.problem +@pytest.mark.parametrize("failure", ["open", "write", "reader"]) +def test_failed_cache_writer_stops_and_closes_its_connection( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, failure: str +) -> None: + import sqlite3 + import threading + + store = caching.MeasurementCache(tmp_path / "cache.sqlite3") + opened = threading.Event() + closed = threading.Event() + failed = threading.Event() + fail = store._fail + + def record_failure(error: BaseException | str) -> None: + fail(error) + failed.set() + + monkeypatch.setattr(store, "_fail", record_failure) + + class Connection: + def __enter__(self) -> Connection: + return self + + def __exit__(self, *args: object) -> None: + pass + + def executemany(self, *args: object) -> None: + if failure == "write": + raise sqlite3.OperationalError("cache write denied") + + def close(self) -> None: + closed.set() + + def opening() -> Connection: + opened.set() + if failure == "open": + raise PermissionError("cannot protect the cache ACL") + return Connection() + + monkeypatch.setattr(store, "_open", opening) + store.put(b"one", 3, {"value": 1}) + store.flush() + writer = store._writer + assert writer is not None + try: + assert opened.wait(2), "cache writer did not open" + if failure == "reader": + store._fail("cache read denied") + assert failed.wait(2), "cache writer did not record the failure" + assert store.problem is not None + store.close() + writer.join(2) + assert not writer.is_alive(), "failed cache writer survived close" + assert failure == "open" or closed.is_set(), "writer connection was not closed" + finally: + # Also reap the writer on the unfixed source, retaining a useful failing test. + if writer.is_alive(): + store._writes.put(None, timeout=2) + writer.join(2) + + @pytest.mark.parametrize("linked", ["directory", "database", "journal"]) def test_cache_acl_refuses_links_before_touching_unrelated_targets( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, linked: str