From 527d7be5c58ed51353295ccf13fe859c76cc5e29 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Thu, 20 Aug 2026 10:37:35 -0400 Subject: [PATCH] Make the Tests workflow able to fail on Windows, and skip two POSIX-only tests The Tests workflow reported SUCCESS on Windows while its own log read "2 failed, 1114 passed". On Windows a multi-line `run:` block executes under pwsh, where only the last command decides the step's result -- so `coverage xml` succeeding hid `pytest` failing. Linux and macOS use bash -e and did catch it, which is why this was invisible. `shell: bash` on that step fixes it, and the same hazard is fixed in python-ci.yml's install step. An audit of every Windows-capable multi-line step found no others; the installer smoke steps are Unix-guarded and already set -euo pipefail. A check that cannot go red is worse than no check, because it is trusted -- and this one has been trusted while six Windows tests failed underneath it. Two of those six are now skipped on Windows: they assert POSIX mode 0o600, which cannot hold where os.chmod only toggles a read-only flag and st_mode reads back 0o666. Skipped rather than relaxed, because the property is real and a weakened assertion would answer it falsely. Whether these files -- the semantic index among them, which holds stored secrets -- are actually protected from other users on Windows is an ACL question, now recorded. The remaining four are TUI tests that assume POSIX paths. They are recorded rather than patched: reproducing them needs a Windows host, and guessing at path normalization and palette-argument parsing would mean changing tests to match an assumption rather than a behaviour. They also interact with the undeclared-textual finding -- those tests SKIP everywhere except Windows, the one platform they were not written for. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN --- .github/workflows/python-ci.yml | 3 +++ .github/workflows/tests.yml | 7 +++++++ TODO.md | 36 ++++++++++++++++++++++++++++++++ src/tests/test_atomic_write.py | 12 +++++++++++ src/tests/test_semantic_index.py | 13 ++++++++++++ 5 files changed, 71 insertions(+) diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index 1ad15a9b..ba4d6314 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -116,6 +116,9 @@ jobs: restore-keys: | ${{ runner.os }}-pip- - name: Install dependencies + # See tests.yml: on Windows a multi-line block runs under pwsh, where + # a failure in any but the last command is silently ignored. + shell: bash run: | python -m pip install --upgrade pip pip install --require-hashes -r requirements.lock diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 47d4ccdb..d7c8470e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -35,6 +35,13 @@ jobs: - name: Run deterministic regression tests run: poetry run pytest -q src/tests/test_deterministic_artifact_regression.py - name: Run tests with coverage + # `shell: bash` is load-bearing, not tidiness. Without it Windows runs + # this block under pwsh, where only the LAST command decides whether + # the step failed -- so `coverage xml` succeeding hid pytest failing, + # and this job reported success on Windows while reading + # "2 failed, 1114 passed". A check that cannot go red is worse than no + # check, because it is trusted. + shell: bash run: | poetry run coverage run -m pytest poetry run coverage xml diff --git a/TODO.md b/TODO.md index 64c5f867..07aa3727 100644 --- a/TODO.md +++ b/TODO.md @@ -253,6 +253,42 @@ everything still open, in the order it should be tackled. deliberately with the sync round-trip re-validated against a real relay rather than folded into unrelated work. (Found 2026-08-20 on the first CI run of this branch.) + - [ ] **Four TUI tests fail on Windows, and only Windows ever runs + them.** Surfaced 2026-08-20 once the Tests workflow could report + a Windows failure at all (see below). All four assume POSIX + paths: `test_tui2_textual_palette_profiles_and_settings` and + `test_tui3_change_password_and_seed_backup_flows_use_vault_service` + compare against literal `/tmp/...` strings while the app echoes + the platform-normalized `\tmp\...`; + `test_tui2_textual_copy_command_for_core_and_advanced_fields` + dies with FileNotFoundError on an `export-field` target; and + `test_tui2_textual_palette_notes_tags_fields_and_doc_export` + asserts False on a doc-export path. + + They are left failing rather than patched blind: reproducing them + needs a Windows host, and guessing at path-normalization and + palette-argument parsing would mean changing tests to match an + assumption instead of a behaviour. + + Note how this interacts with the textual item below: because + textual is undeclared, these tests SKIP on Linux and macOS and run + only on Windows, which is the one platform they were not written + for. Declaring textual would make them run where they were + written, which is probably the first move. + - [ ] **Are vault files protected from other users on Windows?** + `test_atomic_write_permissions` and + `test_index_files_are_not_readable_by_other_users` assert mode + 0o600 and are now skipped on Windows, because POSIX mode bits do + not exist there -- `os.chmod` toggles a read-only flag and + `st_mode` reads back 0o666. Skipped rather than relaxed on + purpose: a weakened assertion would answer the question falsely. + + The question itself is open and worth answering. On Windows these + files inherit directory ACLs rather than carrying an explicit + owner-only mode, and the semantic index in particular holds + stored secrets. Either verify the inherited ACL is user-scoped + and assert THAT on Windows, or set an explicit ACL and assert it. + (Recorded 2026-08-20.) - [ ] **`textual` is an undeclared dependency, so the entire TUI test surface has never run in CI.** It appears in neither `[tool.poetry.dependencies]`, `[tool.poetry.extras]`, nor the dev diff --git a/src/tests/test_atomic_write.py b/src/tests/test_atomic_write.py index 3771790e..09577dcb 100644 --- a/src/tests/test_atomic_write.py +++ b/src/tests/test_atomic_write.py @@ -1,6 +1,7 @@ import json import pytest from multiprocessing import Process +import sys from pathlib import Path from utils.atomic_write import atomic_write @@ -77,6 +78,17 @@ def test_atomic_write_preserves_existing_on_failure(tmp_path: Path) -> None: assert dest_path.read_text() == "original content" +@pytest.mark.skipif( + sys.platform == "win32", + reason=( + "POSIX mode bits do not exist on Windows: os.chmod only toggles the " + "read-only flag and st_mode reads back 0o666, so this assertion cannot " + "hold there. It is SKIPPED rather than relaxed because the property it " + "checks is real -- whether these files are protected from other users " + "on Windows is an open question about ACLs, recorded in TODO.md, and " + "a weakened assertion would answer it falsely." + ), +) def test_atomic_write_permissions(tmp_path: Path) -> None: """File permissions should be set to 0o600 (owner read/write only).""" import stat diff --git a/src/tests/test_semantic_index.py b/src/tests/test_semantic_index.py index a255da75..45900cc2 100644 --- a/src/tests/test_semantic_index.py +++ b/src/tests/test_semantic_index.py @@ -1,5 +1,9 @@ from __future__ import annotations +import sys + +import pytest + from pathlib import Path from types import SimpleNamespace @@ -166,6 +170,15 @@ def test_stored_secrets_are_never_written_to_the_index(tmp_path): assert "DEPLOY_TOKEN" in raw +@pytest.mark.skipif( + sys.platform == "win32", + reason=( + "POSIX mode bits do not exist on Windows -- see the same skip in " + "test_atomic_write.py. The semantic index holds stored secrets, so " + "whether it is protected on Windows matters and is recorded in " + "TODO.md rather than papered over here." + ), +) def test_index_files_are_not_readable_by_other_users(tmp_path): """Every other file in a profile is 0600; these were created at the umask.""" from seedpass.core.semantic_index import SemanticIndex