From 402a6377468a3544e5fe8309fac0143d64abb3b7 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 14 Aug 2026 13:34:56 -0400 Subject: [PATCH 1/7] fix: preserve Windows history during upgrades --- agent/config.py | 10 + agent/main.py | 24 +- agent/migrations.py | 373 +++++++++++++++++++++---- installer/service-setup.ps1 | 15 + tests/test_database_migration.py | 327 ++++++++++++++++------ tests/test_windows_service_security.py | 21 ++ 6 files changed, 623 insertions(+), 147 deletions(-) diff --git a/agent/config.py b/agent/config.py index bd13fcb..d6499d3 100644 --- a/agent/config.py +++ b/agent/config.py @@ -82,6 +82,7 @@ class HumWatchConfig: retention_days: int = 7 db_path: str = "./humwatch.db" data_root: Optional[str] = None + legacy_db_path: Optional[str] = None theme_override: Optional[str] = None auth_token_file: Optional[str] = None tls_certfile: Optional[str] = None @@ -121,6 +122,10 @@ def _resolve_runtime_path(self, value: Optional[str]) -> Optional[Path]: path = Path(value) return path if path.is_absolute() else self.resolved_data_root / path + @property + def resolved_legacy_db_path(self) -> Optional[Path]: + return self._resolve_runtime_path(self.legacy_db_path) + @property def resolved_auth_token_file(self) -> Optional[Path]: return self._resolve_runtime_path(self.auth_token_file) @@ -161,6 +166,7 @@ def load_config() -> HumWatchConfig: retention_days=data.get("retention_days", 7), db_path=data.get("db_path", "./humwatch.db"), data_root=data.get("data_root"), + legacy_db_path=data.get("legacy_db_path"), theme_override=data.get("theme_override"), auth_token_file=data.get("auth_token_file"), tls_certfile=data.get("tls_certfile"), @@ -193,6 +199,10 @@ def load_config() -> HumWatchConfig: if env_data_root is not None: config.data_root = env_data_root + env_legacy_db = os.environ.get("HUMWATCH_LEGACY_DB") + if env_legacy_db is not None: + config.legacy_db_path = env_legacy_db + env_auth_token_file = os.environ.get("HUMWATCH_AUTH_TOKEN_FILE") if env_auth_token_file is not None: config.auth_token_file = env_auth_token_file diff --git a/agent/main.py b/agent/main.py index 4adbb43..c7cdf93 100644 --- a/agent/main.py +++ b/agent/main.py @@ -17,7 +17,7 @@ from agent import __version__ from agent.config import PROJECT_ROOT, get_config, validate_cors_origins from agent.database import init_db, close_db -from agent.migrations import migrate_legacy_database +from agent.migrations import MigrationSafetyError, migrate_legacy_database from agent.security.auth import require_bearer_token from agent.security.limits import RequestLimitMiddleware from agent.security.tls import build_uvicorn_ssl_kwargs, validate_security_config @@ -96,7 +96,27 @@ async def lifespan(app: FastAPI): logger.info("HumWatch v%s starting on port %d", __version__, config.port) # Carry an existing database into the runtime root before opening a new one. - migrate_legacy_database(config) + # An explicit legacy source (set by the installer) that cannot be safely + # preserved raises MigrationSafetyError, which is left to propagate here + # and stop startup rather than silently adopting an empty database. + try: + migration_result = migrate_legacy_database(config) + except MigrationSafetyError as exc: + logger.critical("Refusing to start: %s", exc) + raise + + if migration_result.status == "migrated": + logger.info( + "Migrated legacy database from %s into %s (manifest: %s)", + migration_result.source, migration_result.target, migration_result.archive_manifest, + ) + elif migration_result.status == "target_exists": + log = logger.info if migration_result.archive_manifest else logger.warning + log("Legacy database migration skipped: %s", migration_result.detail) + elif migration_result.status == "failed": + logger.warning("Legacy database migration failed: %s", migration_result.detail) + else: + logger.debug("Legacy database migration skipped (%s): %s", migration_result.status, migration_result.detail) # Initialize database await init_db() diff --git a/agent/migrations.py b/agent/migrations.py index 4752586..88fc980 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -1,106 +1,355 @@ """One-time runtime state migrations for in-place upgrades.""" +import hashlib +import json import logging -import shutil +import os +import sqlite3 +from dataclasses import dataclass +from datetime import datetime, timezone from pathlib import Path -from typing import Optional +from typing import Dict, Iterable, Optional from agent.config import PROJECT_ROOT, HumWatchConfig logger = logging.getLogger("humwatch.migrations") -# SQLite writes these alongside the database in WAL mode. Leaving them behind -# next to an orphaned database loses whatever had not been checkpointed. -_SQLITE_SIDECAR_SUFFIXES = ("-wal", "-shm") - # Copies land under this suffix first so a half-written file is never mistaken # for the real database if the process dies mid-migration. _STAGING_SUFFIX = ".migrating" +# Tables a genuine 2.0 database must have. Their presence, plus at least one +# machine_info row, distinguishes a real legacy database from an empty or +# unrelated file sitting at the same path. +_REQUIRED_TABLES = ("machine_info", "metrics") + +_MANIFEST_NAME = "manifest.json" +_ARCHIVE_DIRNAME = "migration-archive" + + +class MigrationSafetyError(Exception): + """Raised when a legacy database cannot be safely preserved. + + This must stop startup rather than be swallowed: silently continuing is + exactly the failure mode (issue #14) this module exists to close. + """ + + +@dataclass(frozen=True) +class MigrationResult: + source: Path | None + target: Path + status: str + detail: str + archive_manifest: Path | None + def legacy_database_path(config: HumWatchConfig) -> Optional[Path]: """Return the pre-hardening database location, when one could exist. - Releases before the runtime data root resolved ``db_path`` against - ``PROJECT_ROOT``. An absolute ``db_path`` was always explicit, so it never - moved and never needs migrating. + An explicit ``legacy_db_path`` always wins. Otherwise, releases before + the runtime data root resolved ``db_path`` against ``PROJECT_ROOT``. An + absolute ``db_path`` was always explicit, so it never moved and never + needs migrating. """ + if config.resolved_legacy_db_path is not None: + return config.resolved_legacy_db_path if Path(config.db_path).is_absolute(): return None return PROJECT_ROOT / Path(config.db_path) -def migrate_legacy_database(config: HumWatchConfig) -> bool: +def migrate_legacy_database(config: HumWatchConfig) -> MigrationResult: """Carry a pre-hardening database into the runtime data root. - Returns whether anything was migrated. Never overwrites an existing - database, so running it twice is safe and a provisioned install is left - alone. A migration must never be able to keep the service from starting, - so every failure is a logged warning and a ``False``. + Copies via ``sqlite3.Connection.backup()`` (so committed WAL pages are + included), verifies the copy against the source, and only then swaps it + into place. Every migration is provable afterward through a manifest + written under the runtime data root. + + An explicit ``legacy_db_path`` (set by the Windows installer once it has + confirmed the pre-hardening file exists) is a strong contract: if it + cannot be safely honored, this raises ``MigrationSafetyError`` instead of + quietly adopting whatever sits at the target. An inferred legacy path is + best-effort, as it always has been, and failures there are logged and + reported through the result instead of raised. """ + explicit_source = config.resolved_legacy_db_path is not None + legacy = legacy_database_path(config) + target = config.resolved_db_path + + if legacy is None: + return MigrationResult( + None, target, "source_missing", + "No legacy database path could be determined.", None, + ) + + if target == legacy: + return MigrationResult( + legacy, target, "same_path", + "The legacy and target paths are identical; nothing to migrate.", None, + ) + + if not legacy.is_file(): + return MigrationResult( + legacy, target, "source_missing", + f"No legacy database found at {legacy}.", None, + ) + + if target.exists(): + proof = _find_manifest_proof(config, legacy, target) + if proof is not None: + return MigrationResult( + legacy, target, "target_exists", + f"Target already holds a proven prior migration from {legacy} (manifest: {proof}).", + proof, + ) + if explicit_source: + raise MigrationSafetyError(_safety_error_message(legacy, target)) + logger.warning( + "A database already exists at %s and no prior migration from %s is on " + "record; leaving the existing target untouched.", + target, legacy, + ) + return MigrationResult( + legacy, target, "target_exists", + f"Target already exists at {target} with no migration proof for {legacy}; left untouched.", + None, + ) + try: - return _copy_legacy_database(config) - except Exception: + return _copy_and_verify(config, legacy, target) + except Exception as exc: + if explicit_source: + raise MigrationSafetyError( + f"Could not safely migrate the explicit legacy database at {legacy} " + f"into {target}: {exc}" + ) from exc logger.warning( - "Could not migrate the legacy database, continuing with the runtime root as-is", - exc_info=True, + "Could not migrate the legacy database at %s, continuing with the " + "runtime root as-is", + legacy, exc_info=True, ) - return False + return MigrationResult(legacy, target, "failed", str(exc), None) -def _copy_legacy_database(config: HumWatchConfig) -> bool: - """Copy the legacy database across, then best-effort remove the original. +def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> MigrationResult: + """Back up ``legacy`` into a staging file, verify it, then swap it into place. - Copy rather than move, because the packaged service runs as ``humwatch`` - while the legacy database sits in a root-owned install tree under - ``ProtectSystem=strict``. A move has to unlink the source, and that unlink - fails, so the target never appears and every restart repeats the attempt. + The rename is bracketed by a manifest written twice: once with status + ``pending`` before the rename (fsynced, so it survives a crash), and once + with status ``migrated`` after. A crash between those two writes leaves a + completed target next to a ``pending`` manifest, which the next call + verifies and finalizes rather than treating as unproven. """ - legacy = legacy_database_path(config) - if legacy is None: - return False - - target = config.resolved_db_path - if target == legacy or target.exists() or not legacy.is_file(): - return False - target.parent.mkdir(parents=True, exist_ok=True) + staging = target.with_name(target.name + _STAGING_SUFFIX) + if staging.exists(): + staging.unlink() - # The main database is the "already migrated" sentinel, so it is copied and - # renamed last. An interrupted run leaves sidecars and staging files behind, - # which the next pass overwrites, rather than a database with no WAL. - staged = [] try: - for suffix in _SQLITE_SIDECAR_SUFFIXES + ("",): - source = legacy.with_name(legacy.name + suffix) - if not source.is_file(): - continue - final = target.with_name(target.name + suffix) - staging = final.with_name(final.name + _STAGING_SUFFIX) - shutil.copyfile(str(source), str(staging)) - staged.append((staging, final, source)) + _backup_database(legacy, staging) + row_counts = _verify_staged_database(staging) + source_counts = _table_row_counts(legacy, row_counts.keys()) + if source_counts != row_counts: + raise RuntimeError( + f"row counts differ between source {source_counts} and staged copy {row_counts}" + ) + + manifest_path = _manifest_path_for(config) + manifest = { + "source": str(legacy), + "target": str(target), + "source_digest": _sha256(legacy), + "target_digest": _sha256(staging), + "row_counts": row_counts, + "status": "pending", + "time": _utc_now_iso(), + } + _write_manifest_atomic(manifest_path, manifest) + + staging.replace(target) + + manifest["status"] = "migrated" + manifest["time"] = _utc_now_iso() + _write_manifest_atomic(manifest_path, manifest) except Exception: - for staging, _, _ in staged: - _discard(staging) + if staging.exists(): + try: + staging.unlink() + except OSError: + logger.debug("Could not remove staging file %s after a failed migration", staging) raise - for staging, final, _ in staged: - staging.replace(final) + logger.info("Migrated the legacy database from %s into %s", legacy, target) + return MigrationResult( + legacy, target, "migrated", + f"Migrated {legacy} into {target}; manifest at {manifest_path}.", + manifest_path, + ) - for _, _, source in staged: - _discard(source) - logger.info("Migrated the existing database into the protected runtime root") - return True +def _backup_database(source: Path, destination: Path) -> None: + """Copy ``source`` into ``destination`` through the SQLite backup API. + + This is a read path against ``source``: it opens read-only and never + unlinks or rewrites the source database or its WAL/SHM sidecars. Using + ``backup()`` rather than a byte-level copy means committed WAL pages are + folded into the copy even while the source connection stays open. + """ + source_conn = sqlite3.connect(f"file:{source}?mode=ro", uri=True) + try: + destination_conn = sqlite3.connect(destination) + try: + source_conn.backup(destination_conn) + finally: + destination_conn.close() + finally: + source_conn.close() -def _discard(path: Path) -> None: - """Remove a file when we are allowed to. A leftover original is harmless.""" +def _verify_staged_database(staged: Path) -> Dict[str, int]: + """Run integrity checks against a staged copy and return its row counts.""" + conn = sqlite3.connect(staged) try: - path.unlink() - except FileNotFoundError: - pass - except OSError: - # PermissionError on a root-owned install tree, EROFS on a read-only - # one. Either way the copy already landed, so the migration stands. - logger.debug("Left %s in place, it could not be removed", path) + check = conn.execute("PRAGMA quick_check").fetchone() + if check != ("ok",): + raise RuntimeError(f"staged database failed integrity check: {check}") + + existing_tables = { + row[0] for row in conn.execute("SELECT name FROM sqlite_master WHERE type='table'") + } + missing = [table for table in _REQUIRED_TABLES if table not in existing_tables] + if missing: + raise RuntimeError(f"staged database is missing required tables: {missing}") + + machine_count = conn.execute("SELECT COUNT(*) FROM machine_info").fetchone()[0] + if machine_count < 1: + raise RuntimeError("staged database has no machine_info rows") + + return _row_counts(conn, _REQUIRED_TABLES) + finally: + conn.close() + + +def _table_row_counts(db_path: Path, tables: Iterable[str]) -> Dict[str, int]: + conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + try: + return _row_counts(conn, tables) + finally: + conn.close() + + +def _row_counts(conn: sqlite3.Connection, tables: Iterable[str]) -> Dict[str, int]: + counts = {} + for table in tables: + counts[table] = conn.execute(f"SELECT COUNT(*) FROM {table}").fetchone()[0] + return counts + + +def _manifest_path_for(config: HumWatchConfig) -> Path: + return config.resolved_data_root / _ARCHIVE_DIRNAME / _utc_timestamp_dirname() / _MANIFEST_NAME + + +def _utc_timestamp_dirname() -> str: + # No colons: this becomes a directory component on a Windows install. + # Microseconds keep back-to-back migrations (as in tests) from colliding. + return datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S.%fZ") + + +def _utc_now_iso() -> str: + return datetime.now(timezone.utc).isoformat() + + +def _write_manifest_atomic(path: Path, manifest: dict) -> None: + """Write the manifest and fsync it before returning. + + The proof protocol depends on this manifest surviving a crash between the + pending write and the rename, or between the rename and the finalize + write, so the write has to be durable on disk before either boundary. + """ + path.parent.mkdir(parents=True, exist_ok=True) + data = json.dumps(manifest, indent=2, sort_keys=True) + with open(path, "w", encoding="utf-8") as handle: + handle.write(data) + handle.flush() + os.fsync(handle.fileno()) + + +def _sha256(path: Path) -> str: + digest = hashlib.sha256() + with open(path, "rb") as handle: + for chunk in iter(lambda: handle.read(1024 * 1024), b""): + digest.update(chunk) + return digest.hexdigest() + + +def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> Optional[Path]: + """Return the manifest that proves ``target`` came from migrating ``legacy``. + + A ``migrated`` manifest for this exact source/target pair is proof on its + own. A ``pending`` manifest is proof too, once verified: it means the + rename completed and only the finalize write was interrupted, so + recomputing the target's digest and row counts and comparing them against + what the manifest recorded either confirms that (and finalizes it) or + shows a genuine mismatch, which is no proof at all. + """ + archive_root = config.resolved_data_root / _ARCHIVE_DIRNAME + if not archive_root.is_dir(): + return None + + for manifest_path in sorted(archive_root.glob(f"*/{_MANIFEST_NAME}"), reverse=True): + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + continue + + if manifest.get("source") != str(legacy) or manifest.get("target") != str(target): + continue + + if manifest.get("status") == "migrated": + return manifest_path + + if manifest.get("status") == "pending": + if _verify_pending_manifest(target, manifest): + manifest["status"] = "migrated" + manifest["time"] = _utc_now_iso() + _write_manifest_atomic(manifest_path, manifest) + return manifest_path + + return None + + +def _verify_pending_manifest(target: Path, manifest: dict) -> bool: + if not target.is_file(): + return False + try: + if _sha256(target) != manifest.get("target_digest"): + return False + expected_counts = manifest.get("row_counts") or {} + actual_counts = _table_row_counts(target, expected_counts.keys()) + return actual_counts == expected_counts + except Exception: + logger.debug("Could not verify pending manifest for %s", target, exc_info=True) + return False + + +def _safety_error_message(legacy: Path, target: Path) -> str: + legacy_counts = _safe_row_counts(legacy) + target_counts = _safe_row_counts(target) + return ( + f"A database already exists at {target} (rows: {target_counts}) but an " + f"explicit legacy database at {legacy} (rows: {legacy_counts}) has no " + "recorded prior migration into it. Refusing to start rather than risk " + "silently adopting an incomplete database. If the target already holds " + "everything from the legacy database, remove the legacy file; otherwise " + "merge the two databases by hand (or move the legacy file into place " + "manually) and remove the other before restarting HumWatch." + ) + + +def _safe_row_counts(db_path: Path) -> Dict[str, int]: + try: + return _table_row_counts(db_path, _REQUIRED_TABLES) + except Exception: + return {} diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index 5e60de2..af6fd86 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -604,6 +604,21 @@ if (Test-Path (Join-Path $RuntimeRoot "tls\trusted-ca.pem")) { $environmentPaths += "HUMWATCH_TRUSTED_CA_FILE=$(Join-Path $RuntimeRoot 'tls\trusted-ca.pem')" } +# Pre-hardening releases kept the database directly under the install root +# (db_path defaulted to "./humwatch.db", resolved against the app directory +# itself). An in-place upgrade now always sets HUMWATCH_DB to an absolute +# ProgramData path, so the old inferred-legacy-path lookup in +# migrate_legacy_database never sees it: an absolute db_path was always +# explicit and never needed migrating. Passing the pre-hardening location +# here explicitly closes that gap. Only set when the file exists, so a fresh +# install (nothing to migrate) and a fully-upgraded machine (already moved) +# both preserve today's behavior. +$LegacyDatabasePath = Join-Path $AppDir "humwatch.db" +if (Test-Path -LiteralPath $LegacyDatabasePath -PathType Leaf) { + $environmentPaths += "HUMWATCH_LEGACY_DB=$LegacyDatabasePath" + Write-Log "Found a pre-hardening database at ${LegacyDatabasePath}; migration will use it explicitly." +} + # AppDirectory is the most critical setting. NSSM defaults it to the # directory containing the Application exe (python\), but we need the # install root so that "python -m agent.main" finds the agent package. diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 34bfeea..699a05f 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -1,127 +1,288 @@ """Regression tests for carrying an existing database into the runtime root.""" -import os +import json +import sqlite3 import pytest -import agent.config as config_module +import agent.migrations as migrations_module from agent.config import HumWatchConfig -from agent.migrations import migrate_legacy_database +from agent.migrations import MigrationSafetyError, migrate_legacy_database + + +def _make_legacy_db(path, machine_id="machine-a", metric_rows=1): + """Create a real 2.0-shaped SQLite database with WAL enabled.""" + path.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(path) + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("CREATE TABLE machine_info (machine_id TEXT PRIMARY KEY)") + conn.execute("CREATE TABLE metrics (timestamp TEXT, metric_name TEXT, value REAL)") + conn.execute("INSERT INTO machine_info VALUES (?)", (machine_id,)) + for i in range(metric_rows): + conn.execute( + "INSERT INTO metrics VALUES (?, 'cpu', ?)", + (f"2026-08-13T12:{i:02d}:00Z", 40 + i), + ) + conn.commit() + conn.close() + return path + + +def test_installer_absolute_destination_migrates_explicit_legacy_source(tmp_path): + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + legacy.parent.mkdir() + source = sqlite3.connect(legacy) + source.execute("PRAGMA journal_mode=WAL") + source.execute("CREATE TABLE machine_info (machine_id TEXT PRIMARY KEY)") + source.execute("CREATE TABLE metrics (timestamp TEXT, metric_name TEXT, value REAL)") + source.execute("INSERT INTO machine_info VALUES ('machine-a')") + source.execute("INSERT INTO metrics VALUES ('2026-08-13T12:00:00Z', 'cpu', 42)") + source.commit() + source.execute("INSERT INTO metrics VALUES ('2026-08-13T12:01:00Z', 'cpu', 43)") + source.commit() + config = HumWatchConfig( + db_path=str(target), + legacy_db_path=str(legacy), + data_root=str(tmp_path / "runtime"), + ) + + result = migrate_legacy_database(config) + + assert result.status == "migrated" + migrated = sqlite3.connect(target) + assert migrated.execute("PRAGMA quick_check").fetchone() == ("ok",) + assert migrated.execute("SELECT machine_id FROM machine_info").fetchone() == ("machine-a",) + assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) + assert result.archive_manifest.is_file() + assert legacy.is_file() + + manifest = json.loads(result.archive_manifest.read_text()) + assert manifest["status"] == "migrated" + assert manifest["source"] == str(legacy) + assert manifest["target"] == str(target) + assert manifest["row_counts"] == {"machine_info": 1, "metrics": 2} + + +def test_source_missing_when_explicit_legacy_path_does_not_exist(tmp_path): + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + config = HumWatchConfig( + db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime"), + ) + + result = migrate_legacy_database(config) + + assert result.status == "source_missing" + assert not target.exists() + + +def test_source_missing_when_no_legacy_path_can_be_inferred(tmp_path): + """An absolute db_path with no explicit legacy source was never resolved + against the project root, so there is nothing to look for.""" + explicit = tmp_path / "elsewhere" / "humwatch.db" + config = HumWatchConfig(db_path=str(explicit), data_root=str(tmp_path / "runtime")) + result = migrate_legacy_database(config) -def _legacy_layout(tmp_path, monkeypatch): - """Point PROJECT_ROOT at a throwaway pre-hardening install tree.""" - project_root = tmp_path / "HumWatch" - project_root.mkdir() - monkeypatch.setattr(config_module, "PROJECT_ROOT", project_root) + assert result.status == "source_missing" + assert result.source is None + assert not explicit.exists() - import agent.migrations as migrations_module - monkeypatch.setattr(migrations_module, "PROJECT_ROOT", project_root) - return project_root +def test_same_path_when_legacy_and_target_are_identical(tmp_path): + shared = tmp_path / "shared" / "humwatch.db" + _make_legacy_db(shared) + config = HumWatchConfig(db_path=str(shared), legacy_db_path=str(shared), data_root=str(tmp_path / "runtime")) + result = migrate_legacy_database(config) -def test_existing_database_moves_into_the_runtime_root(tmp_path, monkeypatch): - """An in-place upgrade must not open an empty database and lose a year of history.""" - project_root = _legacy_layout(tmp_path, monkeypatch) - (project_root / "humwatch.db").write_bytes(b"telemetry") - (project_root / "humwatch.db-wal").write_bytes(b"uncheckpointed") - (project_root / "humwatch.db-shm").write_bytes(b"index") + assert result.status == "same_path" + assert result.archive_manifest is None + + +def test_inferred_legacy_path_still_migrates_a_relative_db(tmp_path, monkeypatch): + """The pre-installer-fix path: no explicit legacy_db_path, db_path relative + to a project root that used to hold the database directly.""" + project_root = tmp_path / "HumWatch" + project_root.mkdir() + monkeypatch.setattr(migrations_module, "PROJECT_ROOT", project_root) + _make_legacy_db(project_root / "humwatch.db") data_root = tmp_path / "runtime" config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) - assert migrate_legacy_database(config) is True - assert (data_root / "humwatch.db").read_bytes() == b"telemetry" - assert (data_root / "humwatch.db-wal").read_bytes() == b"uncheckpointed" - assert (data_root / "humwatch.db-shm").read_bytes() == b"index" - assert not (project_root / "humwatch.db").exists() + result = migrate_legacy_database(config) + + assert result.status == "migrated" + assert (data_root / "humwatch.db").is_file() + # The source is never touched. + assert (project_root / "humwatch.db").is_file() -@pytest.mark.skipif( - not hasattr(os, "geteuid") or os.geteuid() == 0, - reason="Directory permissions do not restrict root, and Windows ignores the mode bits", -) -def test_a_read_only_legacy_directory_still_migrates(tmp_path, monkeypatch): - """The service runs as humwatch against a root-owned install tree. +def test_migration_is_idempotent_and_reports_target_exists_with_proof(tmp_path): + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) - Unlinking the original is not permitted there. If that failure aborted the - migration the target would never appear, every restart would retry it, and - the service would never come up. - """ - project_root = _legacy_layout(tmp_path, monkeypatch) - (project_root / "humwatch.db").write_bytes(b"telemetry") - (project_root / "humwatch.db-wal").write_bytes(b"uncheckpointed") + first = migrate_legacy_database(config) + second = migrate_legacy_database(config) + + assert first.status == "migrated" + assert second.status == "target_exists" + assert second.archive_manifest == first.archive_manifest + manifest = json.loads(second.archive_manifest.read_text()) + assert manifest["status"] == "migrated" + + +def test_bug_shaped_case_refuses_without_manifest_proof(tmp_path): + """The exact shape of issue #14: an absolute target already exists (empty), + an explicit legacy source is populated, and there is no manifest proving + the target came from migrating it. This must refuse to start.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=5) + target.parent.mkdir(parents=True) + sqlite3.connect(target).close() # an empty, unrelated database + + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + message = str(excinfo.value) + assert str(legacy) in message + assert str(target) in message + # Row counts for both sides are surfaced so the operator can judge the risk. + assert "machine_info" in message or "metrics" in message + # Nothing was touched. + assert legacy.is_file() + assert sqlite3.connect(legacy).execute("SELECT COUNT(*) FROM metrics").fetchone() == (5,) + + +def test_target_exists_without_proof_is_left_alone_for_an_inferred_source(tmp_path, monkeypatch): + """The non-explicit (best-effort) path never raises: a provisioned install + with a real target and a stray legacy file must survive untouched.""" + project_root = tmp_path / "HumWatch" + project_root.mkdir() + monkeypatch.setattr(migrations_module, "PROJECT_ROOT", project_root) + _make_legacy_db(project_root / "humwatch.db") data_root = tmp_path / "runtime" + data_root.mkdir() + (data_root / "humwatch.db").write_bytes(b"current") config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) - project_root.chmod(0o500) - try: - assert migrate_legacy_database(config) is True - finally: - project_root.chmod(0o700) + result = migrate_legacy_database(config) - assert (data_root / "humwatch.db").read_bytes() == b"telemetry" - assert (data_root / "humwatch.db-wal").read_bytes() == b"uncheckpointed" - # The original could not be removed, and that is fine. The next pass sees - # the migrated database and stops. - assert (project_root / "humwatch.db").exists() - assert migrate_legacy_database(config) is False - assert not list(data_root.glob("*.migrating")) + assert result.status == "target_exists" + assert result.archive_manifest is None + assert (data_root / "humwatch.db").read_bytes() == b"current" -def test_a_failed_copy_leaves_no_database_behind(tmp_path, monkeypatch): - """A partial migration must not publish a sentinel it cannot back up.""" - project_root = _legacy_layout(tmp_path, monkeypatch) - (project_root / "humwatch.db").write_bytes(b"telemetry") +def test_failed_status_when_backup_fails_for_an_inferred_source(tmp_path, monkeypatch): + """A best-effort (non-explicit) source must never be able to block startup.""" + project_root = tmp_path / "HumWatch" + project_root.mkdir() + monkeypatch.setattr(migrations_module, "PROJECT_ROOT", project_root) + (project_root / "humwatch.db").write_bytes(b"not a real sqlite file") data_root = tmp_path / "runtime" config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) - import agent.migrations as migrations_module + result = migrate_legacy_database(config) - def _explode(*args, **kwargs): - raise OSError("no space left on device") + assert result.status == "failed" + assert not (data_root / "humwatch.db").exists() + # The corrupt original is left exactly as it was. + assert (project_root / "humwatch.db").read_bytes() == b"not a real sqlite file" - monkeypatch.setattr(migrations_module.shutil, "copyfile", _explode) - assert migrate_legacy_database(config) is False - assert not (data_root / "humwatch.db").exists() - assert (project_root / "humwatch.db").read_bytes() == b"telemetry" +def test_failed_migration_raises_for_an_explicit_source(tmp_path): + legacy = tmp_path / "legacy" / "humwatch.db" + legacy.parent.mkdir() + legacy.write_bytes(b"not a real sqlite file") + target = tmp_path / "runtime" / "humwatch.db" + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + with pytest.raises(MigrationSafetyError): + migrate_legacy_database(config) -def test_migration_never_overwrites_an_existing_runtime_database(tmp_path, monkeypatch): - """A provisioned install with real data must survive a stray legacy file.""" - project_root = _legacy_layout(tmp_path, monkeypatch) - (project_root / "humwatch.db").write_bytes(b"stale") + assert not target.exists() + assert legacy.read_bytes() == b"not a real sqlite file" - data_root = tmp_path / "runtime" - data_root.mkdir() - (data_root / "humwatch.db").write_bytes(b"current") - config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) - assert migrate_legacy_database(config) is False - assert (data_root / "humwatch.db").read_bytes() == b"current" - assert (project_root / "humwatch.db").read_bytes() == b"stale" +def test_interrupted_migration_finalizes_pending_manifest_on_restart(tmp_path, monkeypatch): + """Kill the process after the rename but before the manifest finalize + write. The next call must verify the pending manifest against the target + that is actually on disk, finalize it, and start normally.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=3) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + original_write = migrations_module._write_manifest_atomic + calls = {"n": 0} -def test_migration_is_idempotent(tmp_path, monkeypatch): - """Every restart calls this, so a second pass must be a no-op.""" - project_root = _legacy_layout(tmp_path, monkeypatch) - (project_root / "humwatch.db").write_bytes(b"telemetry") + def _crash_before_finalize(path, manifest): + calls["n"] += 1 + if calls["n"] == 1: + original_write(path, manifest) + return + raise OSError("simulated crash before the manifest finalize write") - config = HumWatchConfig(db_path="./humwatch.db", data_root=str(tmp_path / "runtime")) + monkeypatch.setattr(migrations_module, "_write_manifest_atomic", _crash_before_finalize) - assert migrate_legacy_database(config) is True - assert migrate_legacy_database(config) is False + with pytest.raises(MigrationSafetyError): + migrate_legacy_database(config) + # The rename happened before the simulated crash: the target is real data. + assert target.is_file() + assert sqlite3.connect(target).execute("SELECT COUNT(*) FROM metrics").fetchone() == (3,) -def test_absolute_database_paths_are_left_alone(tmp_path, monkeypatch): - """An explicit absolute db_path was never resolved against the project root.""" - _legacy_layout(tmp_path, monkeypatch) - explicit = tmp_path / "elsewhere" / "humwatch.db" - config = HumWatchConfig(db_path=str(explicit), data_root=str(tmp_path / "runtime")) + monkeypatch.setattr(migrations_module, "_write_manifest_atomic", original_write) + + result = migrate_legacy_database(config) + + assert result.status == "target_exists" + assert result.archive_manifest is not None + manifest = json.loads(result.archive_manifest.read_text()) + assert manifest["status"] == "migrated" + # Legacy source is still untouched throughout. + assert legacy.is_file() + + +def test_pending_manifest_mismatch_is_not_treated_as_proof(tmp_path, monkeypatch): + """If the target on disk does not match what a pending manifest recorded, + that is not proof of anything, and an explicit source must still refuse.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + original_write = migrations_module._write_manifest_atomic + calls = {"n": 0} + + def _crash_before_finalize(path, manifest): + calls["n"] += 1 + if calls["n"] == 1: + original_write(path, manifest) + return + raise OSError("simulated crash before the manifest finalize write") + + monkeypatch.setattr(migrations_module, "_write_manifest_atomic", _crash_before_finalize) + with pytest.raises(MigrationSafetyError): + migrate_legacy_database(config) + monkeypatch.setattr(migrations_module, "_write_manifest_atomic", original_write) + + # Tamper with the target after the crash, so it no longer matches the + # pending manifest's recorded digest. + conn = sqlite3.connect(target) + conn.execute("INSERT INTO metrics VALUES ('2026-08-13T13:00:00Z', 'cpu', 99)") + conn.commit() + conn.close() + + with pytest.raises(MigrationSafetyError): + migrate_legacy_database(config) - assert migrate_legacy_database(config) is False - assert not explicit.exists() diff --git a/tests/test_windows_service_security.py b/tests/test_windows_service_security.py index 1c3901c..5616ef0 100644 --- a/tests/test_windows_service_security.py +++ b/tests/test_windows_service_security.py @@ -412,6 +412,7 @@ def test_service_setup_configures_paths_firewall_and_conditional_service_identit "ProgramData", "AppDirectory", "HUMWATCH_DB", + "HUMWATCH_LEGACY_DB", "HUMWATCH_LOG_DIR", "HUMWATCH_AUTH_TOKEN_FILE", "HUMWATCH_TLS_KEYFILE", @@ -431,6 +432,26 @@ def test_service_setup_configures_paths_firewall_and_conditional_service_identit assert "Join-Path $AppDir \"logs\"" not in source +def test_installer_passes_the_legacy_database_path_only_when_it_exists(): + """The migration bug this task closes (issue #14) was an absolute + HUMWATCH_DB destination silently defeating the inferred-legacy-path + lookup. HUMWATCH_LEGACY_DB must be set explicitly, and only when the + pre-hardening file is actually there, so a fresh install or an + already-upgraded machine sees no behavior change.""" + source = read("installer/service-setup.ps1") + + assert 'Join-Path $AppDir "humwatch.db"' in source + guard = source.split("$LegacyDatabasePath =", 1)[1] + assert "Test-Path -LiteralPath $LegacyDatabasePath -PathType Leaf" in guard + assert "HUMWATCH_LEGACY_DB=$LegacyDatabasePath" in guard + # The conditional append happens after $environmentPaths already exists, + # so a missing legacy file leaves the array, and prior upgrade behavior, + # unchanged rather than appending an empty/garbage entry. + legacy_block_index = source.index("$LegacyDatabasePath =") + environment_array_index = source.index("$environmentPaths = @(") + assert environment_array_index < legacy_block_index + + def test_service_identity_can_modify_only_dedicated_data_and_log_directories(): source = read("installer/service-setup.ps1") grant = source.split("function Set-ProtectedWritableDirectoryAcl", 1)[1].split( From f31064e93319c9967a44b69d8bee470484b782d2 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 14 Aug 2026 13:48:23 -0400 Subject: [PATCH 2/7] fix: close three review findings in the 2.0 database preservation fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Make manifest writes genuinely atomic (temp file + fsync + os.replace, plus directory fsync) so a crash mid-finalize-rewrite can no longer truncate a valid manifest and turn a successful migration into a permanent MigrationSafetyError. - Recover an unmerged WAL from a crashed legacy source by copying the database and its -wal/-shm sidecars into scratch space and checkpointing that copy, instead of relying on a read-only open of the real source (which is not reliable when there is no live connection left to have checkpointed it — exactly the crashed-service upgrade this task exists for). - Match manifest proof by normalized path (os.path.normcase + os.path.abspath) instead of exact string equality, so casing or separator drift on a Windows reinstall can't make valid proof invisible and refuse to start a healthy machine. Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 123 ++++++++++++++++++++++---- tests/test_database_migration.py | 146 ++++++++++++++++++++++++++++++- 2 files changed, 248 insertions(+), 21 deletions(-) diff --git a/agent/migrations.py b/agent/migrations.py index 88fc980..e3ed876 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -4,7 +4,9 @@ import json import logging import os +import shutil import sqlite3 +import tempfile from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -147,20 +149,24 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr if staging.exists(): staging.unlink() + source_digest = _sha256(legacy) + try: - _backup_database(legacy, staging) - row_counts = _verify_staged_database(staging) - source_counts = _table_row_counts(legacy, row_counts.keys()) - if source_counts != row_counts: - raise RuntimeError( - f"row counts differ between source {source_counts} and staged copy {row_counts}" - ) + with tempfile.TemporaryDirectory(prefix="humwatch-legacy-recovery-") as scratch_dir: + recovered = _recover_source_copy(legacy, Path(scratch_dir)) + _backup_database(recovered, staging) + row_counts = _verify_staged_database(staging) + source_counts = _table_row_counts(recovered, row_counts.keys()) + if source_counts != row_counts: + raise RuntimeError( + f"row counts differ between source {source_counts} and staged copy {row_counts}" + ) manifest_path = _manifest_path_for(config) manifest = { - "source": str(legacy), - "target": str(target), - "source_digest": _sha256(legacy), + "source": _normalize_path(legacy), + "target": _normalize_path(target), + "source_digest": source_digest, "target_digest": _sha256(staging), "row_counts": row_counts, "status": "pending", @@ -189,15 +195,45 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr ) +def _recover_source_copy(legacy: Path, scratch_dir: Path) -> Path: + """Copy ``legacy`` and its WAL/SHM sidecars into scratch space and recover them. + + A crashed legacy service can leave committed data sitting only in the + ``-wal`` file, with the main database file still holding the old state + and no live connection left to have checkpointed it. Opening the real + source read-only (``mode=ro``) to read through that WAL is not reliable + in that shape: it depends on being able to create or reuse a ``-shm`` + index, which a read-only open is not always able to do. Copying the + trio into scratch space first and opening THAT copy (a normal writable + open, since it is only a copy) lets SQLite replay the WAL the same way + a normal startup would. Every write here lands only in ``scratch_dir``; + the real source is only ever read via ``shutil.copyfile``. + """ + recovered = scratch_dir / legacy.name + shutil.copyfile(str(legacy), str(recovered)) + for suffix in ("-wal", "-shm"): + sidecar = legacy.with_name(legacy.name + suffix) + if sidecar.is_file(): + shutil.copyfile(str(sidecar), str(recovered.with_name(recovered.name + suffix))) + + conn = sqlite3.connect(recovered) + try: + conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") + conn.commit() + finally: + conn.close() + + return recovered + + def _backup_database(source: Path, destination: Path) -> None: """Copy ``source`` into ``destination`` through the SQLite backup API. - This is a read path against ``source``: it opens read-only and never - unlinks or rewrites the source database or its WAL/SHM sidecars. Using - ``backup()`` rather than a byte-level copy means committed WAL pages are - folded into the copy even while the source connection stays open. + ``source`` here is always a recovered scratch copy (see + ``_recover_source_copy``), never the real legacy file, so opening it + normally is safe. """ - source_conn = sqlite3.connect(f"file:{source}?mode=ro", uri=True) + source_conn = sqlite3.connect(source) try: destination_conn = sqlite3.connect(destination) try: @@ -262,18 +298,61 @@ def _utc_now_iso() -> str: def _write_manifest_atomic(path: Path, manifest: dict) -> None: - """Write the manifest and fsync it before returning. + """Write the manifest atomically and fsync it (and its directory) durable. The proof protocol depends on this manifest surviving a crash between the pending write and the rename, or between the rename and the finalize - write, so the write has to be durable on disk before either boundary. + write. A plain truncate-in-place write (``open(path, "w")``) is not safe + for either boundary: a crash mid-write leaves a truncated, unparseable + manifest sitting next to a target that actually migrated cleanly, and the + next startup has no way to tell that apart from real data loss. Writing + to a temp file, fsyncing it, and ``os.replace``-ing it onto the real path + means the manifest is only ever fully-old or fully-new on disk, never + partial. The directory is fsynced too, both so the file's own directory + entry is durable, and, on the first (pending) write, so the entry for the + manifest's own newly-created timestamp directory under the archive root + is durable before the rename that write is meant to precede. """ path.parent.mkdir(parents=True, exist_ok=True) data = json.dumps(manifest, indent=2, sort_keys=True) - with open(path, "w", encoding="utf-8") as handle: + tmp_path = path.with_name(path.name + ".tmp") + with open(tmp_path, "w", encoding="utf-8") as handle: handle.write(data) handle.flush() os.fsync(handle.fileno()) + os.replace(tmp_path, path) + _fsync_dir(path.parent) + _fsync_dir(path.parent.parent) + + +def _fsync_dir(dir_path: Path) -> None: + """Best-effort fsync of a directory's own entry table. + + Not supported on every platform (notably some Windows filesystems, where + a directory handle cannot be fsynced), so a failure here is swallowed: + the file-level fsync in ``_write_manifest_atomic`` already makes the + manifest's own contents durable, this only tightens the guarantee that + the directory entry pointing at it is too. + """ + try: + fd = os.open(str(dir_path), os.O_RDONLY) + try: + os.fsync(fd) + finally: + os.close(fd) + except OSError: + logger.debug("Could not fsync directory %s", dir_path, exc_info=True) + + +def _normalize_path(path) -> str: + """Return a form of ``path`` that compares equal across superficial drift. + + Manifest proof is matched against this, not the raw string, so a + reinstall to the same location with different path casing, an 8.3 short + name, or a stray trailing separator still finds its own manifest instead + of refusing to start on a healthy machine. + """ + return os.path.normcase(os.path.abspath(str(path))) def _sha256(path: Path) -> str: @@ -304,7 +383,13 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> except (OSError, json.JSONDecodeError): continue - if manifest.get("source") != str(legacy) or manifest.get("target") != str(target): + # Manifests are matched by normalized path so a Windows reinstall with + # different casing, short (8.3) names, or separator drift does not + # make otherwise-valid proof invisible. Older manifests written before + # normalization was applied at write time are normalized here too. + if _normalize_path(manifest.get("source", "")) != _normalize_path(legacy) or ( + _normalize_path(manifest.get("target", "")) != _normalize_path(target) + ): continue if manifest.get("status") == "migrated": diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 699a05f..65f3a65 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -1,7 +1,10 @@ """Regression tests for carrying an existing database into the runtime root.""" import json +import shutil import sqlite3 +import subprocess +import sys import pytest @@ -59,8 +62,8 @@ def test_installer_absolute_destination_migrates_explicit_legacy_source(tmp_path manifest = json.loads(result.archive_manifest.read_text()) assert manifest["status"] == "migrated" - assert manifest["source"] == str(legacy) - assert manifest["target"] == str(target) + assert manifest["source"] == migrations_module._normalize_path(legacy) + assert manifest["target"] == migrations_module._normalize_path(target) assert manifest["row_counts"] == {"machine_info": 1, "metrics": 2} @@ -286,3 +289,142 @@ def _crash_before_finalize(path, manifest): with pytest.raises(MigrationSafetyError): migrate_legacy_database(config) + +def test_manifest_finalize_write_is_atomic_and_does_not_truncate_on_crash(tmp_path, monkeypatch): + """A crash mid-finalize-write must never leave a truncated, unparseable + manifest sitting next to data that actually migrated cleanly. The write + goes through a temp file plus os.replace, so a failure before the + replace must leave the previous manifest contents completely intact.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + assert result.status == "migrated" + manifest_path = result.archive_manifest + original_bytes = manifest_path.read_bytes() + assert json.loads(original_bytes)["status"] == "migrated" + + def _boom(*args, **kwargs): + raise OSError("simulated crash during os.replace") + + monkeypatch.setattr(migrations_module.os, "replace", _boom) + + manifest = json.loads(original_bytes) + manifest["status"] = "migrated" # a re-finalize attempt, e.g. from a retry + with pytest.raises(OSError): + migrations_module._write_manifest_atomic(manifest_path, manifest) + + # The crash happened before the atomic replace, so the file on disk is + # byte-for-byte the previous, valid manifest -- never truncated. + assert manifest_path.read_bytes() == original_bytes + assert json.loads(manifest_path.read_text())["status"] == "migrated" + + +def test_migration_recovers_committed_data_from_an_unmerged_wal(tmp_path): + """A crashed legacy service leaves committed rows sitting only in the + -wal file, with the main database file still holding the old (empty) + state and no live connection left to have checkpointed it. Migration + must still recover every row rather than fail or adopt a stale copy.""" + legacy = tmp_path / "legacy" / "humwatch.db" + legacy.parent.mkdir(parents=True) + + # Simulate the crash with a real process exit that skips SQLite's normal + # close-time checkpoint (os._exit bypasses interpreter and C-level + # cleanup entirely, unlike a plain conn.close()). + crash_script = ( + "import sqlite3, os\n" + f"conn = sqlite3.connect(r'{legacy}')\n" + "conn.execute('PRAGMA journal_mode=WAL')\n" + "conn.execute('PRAGMA wal_autocheckpoint=0')\n" + "conn.execute('CREATE TABLE machine_info (machine_id TEXT PRIMARY KEY)')\n" + "conn.execute('CREATE TABLE metrics (timestamp TEXT, metric_name TEXT, value REAL)')\n" + "conn.execute(\"INSERT INTO machine_info VALUES ('machine-a')\")\n" + "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:00:00Z', 'cpu', 42)\")\n" + "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:01:00Z', 'cpu', 43)\")\n" + "conn.commit()\n" + "os._exit(0)\n" + ) + subprocess.run([sys.executable, "-c", crash_script], check=True) + + wal = legacy.with_name(legacy.name + "-wal") + assert wal.is_file() and wal.stat().st_size > 0, "the crash simulation did not leave an unmerged WAL" + + # Copy the file trio to a fresh directory, as an operator restoring from + # a crashed machine's disk image would. + fresh = tmp_path / "fresh" / "humwatch.db" + fresh.parent.mkdir(parents=True) + shutil.copyfile(legacy, fresh) + for suffix in ("-wal", "-shm"): + sidecar = legacy.with_name(legacy.name + suffix) + if sidecar.is_file(): + shutil.copyfile(sidecar, fresh.with_name(fresh.name + suffix)) + + target = tmp_path / "runtime" / "humwatch.db" + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(fresh), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + + assert result.status == "migrated" + migrated = sqlite3.connect(target) + assert migrated.execute("SELECT machine_id FROM machine_info").fetchone() == ("machine-a",) + assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) + # The real (crashed) source and its sidecars are never touched. + assert legacy.is_file() + assert wal.is_file() + + +def test_manifest_proof_matches_a_manifest_with_separator_normalization_drift(tmp_path): + """A manifest's recorded target string can drift from the exact string + the config resolves to (redundant "./" segments, trailing separators) + without the underlying path being any different. This is normalized away + on every platform, unlike casing, so it must always still count as + proof rather than refusing to start a healthy machine.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + assert result.status == "migrated" + + manifest = json.loads(result.archive_manifest.read_text()) + manifest["target"] = str(target.parent / "." / target.name) + manifest["source"] = str(legacy.parent / "." / legacy.name) + result.archive_manifest.write_text(json.dumps(manifest)) + + second = migrate_legacy_database(config) + + assert second.status == "target_exists" + assert second.archive_manifest == result.archive_manifest + assert json.loads(result.archive_manifest.read_text())["status"] == "migrated" + + +def test_manifest_proof_matching_folds_case_the_way_the_target_platform_does(tmp_path, monkeypatch): + """Casing drift (an 8.3 short name, or a reinstall that differs only in + case) only matters on a case-insensitive filesystem, i.e. Windows -- the + platform this guards. That behavior lives entirely in + os.path.normcase, so it is pinned directly here by forcing it to fold + case the way Windows does, independent of the host OS actually running + this test.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + assert result.status == "migrated" + + manifest = json.loads(result.archive_manifest.read_text()) + manifest["source"] = str(legacy).upper() + manifest["target"] = str(target).upper() + result.archive_manifest.write_text(json.dumps(manifest)) + + monkeypatch.setattr(migrations_module.os.path, "normcase", str.lower) + + second = migrate_legacy_database(config) + + assert second.status == "target_exists" + assert second.archive_manifest == result.archive_manifest + From 1e222f660121563f81f53af83bdfb8e23dd6f60c Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 14 Aug 2026 14:02:13 -0400 Subject: [PATCH 3/7] fix: close four findings from the whole-branch codex review - Try a checkpoint-consistent read-only backup() of the real source first (handles a live writer safely); only fall back to the scratch trio-copy when that open fails, and guard the copy window with a before/after stat check (retry once, then fail closed) so a torn db/-wal/-shm pairing can no longer silently pass verification. - Require target_exists proof to re-verify the source file's current SHA256 against the manifest's recorded source_digest, not just its path, so swapping in a different database at the same path can't ride a stale manifest. - fsync the target's parent directory right after the rename and before the manifest finalize write, so power loss between them can't leave the manifest and the target disagreeing about whether it happened. - same_path detection now uses the same normalized path comparison manifest matching already uses, instead of raw Path equality. Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 172 ++++++++++++++++++++++++---- tests/test_database_migration.py | 186 +++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+), 23 deletions(-) diff --git a/agent/migrations.py b/agent/migrations.py index e3ed876..97a9825 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -86,7 +86,7 @@ def migrate_legacy_database(config: HumWatchConfig) -> MigrationResult: "No legacy database path could be determined.", None, ) - if target == legacy: + if _normalize_path(target) == _normalize_path(legacy): return MigrationResult( legacy, target, "same_path", "The legacy and target paths are identical; nothing to migrate.", None, @@ -153,10 +153,9 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr try: with tempfile.TemporaryDirectory(prefix="humwatch-legacy-recovery-") as scratch_dir: - recovered = _recover_source_copy(legacy, Path(scratch_dir)) - _backup_database(recovered, staging) + comparable_source = _snapshot_source_into(legacy, staging, Path(scratch_dir)) row_counts = _verify_staged_database(staging) - source_counts = _table_row_counts(recovered, row_counts.keys()) + source_counts = _table_row_counts(comparable_source, row_counts.keys()) if source_counts != row_counts: raise RuntimeError( f"row counts differ between source {source_counts} and staged copy {row_counts}" @@ -175,6 +174,11 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr _write_manifest_atomic(manifest_path, manifest) staging.replace(target) + # The manifest's finalize write below is what proves this rename + # happened; without fsyncing the target's own directory entry first, + # power loss right here could keep the manifest but lose the target + # (or the reverse), so this has to land before that write. + _fsync_dir(target.parent) manifest["status"] = "migrated" manifest["time"] = _utc_now_iso() @@ -195,27 +199,120 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr ) +_SOURCE_SIDECAR_SUFFIXES = ("-wal", "-shm") + +# A live writer changing the source mid-copy gets one retry (a fresh +# before/after stat window), then the copy is treated as untrustworthy. +_TRIO_COPY_ATTEMPTS = 2 + + +def _snapshot_source_into(legacy: Path, staging: Path, scratch_dir: Path) -> Path: + """Produce a checkpoint-consistent copy of ``legacy`` at ``staging``. + + Tries a direct read-only ``backup()`` of the real source first: SQLite's + backup API takes its snapshot page-by-page while walking the source, so + it is checkpoint-consistent even against a live writer, and a live + writer is exactly the case where a read-only open reliably succeeds + (it implies an already-initialized ``-shm`` index). That open only fails + when the source has an unrecovered WAL and no live connection at all (a + crashed service) -- never because of anything this function does to the + source, since this path is read-only throughout. + + Only on that failure does this fall back to copying the db/-wal/-shm + trio into scratch space and recovering it there (see + ``_recover_source_copy``). Returns the path that was actually backed up + from, so the caller can sanity-check row counts against the same data + that produced ``staging``. + """ + try: + _backup_readonly(legacy, staging) + return legacy + except sqlite3.Error: + logger.debug( + "Read-only backup of %s failed (likely an unrecovered WAL with no " + "live connection); falling back to a scratch-space copy and checkpoint", + legacy, exc_info=True, + ) + + recovered = _recover_source_copy(legacy, scratch_dir) + if staging.exists(): + staging.unlink() + _backup_writable(recovered, staging) + return recovered + + def _recover_source_copy(legacy: Path, scratch_dir: Path) -> Path: """Copy ``legacy`` and its WAL/SHM sidecars into scratch space and recover them. - A crashed legacy service can leave committed data sitting only in the - ``-wal`` file, with the main database file still holding the old state - and no live connection left to have checkpointed it. Opening the real - source read-only (``mode=ro``) to read through that WAL is not reliable - in that shape: it depends on being able to create or reuse a ``-shm`` - index, which a read-only open is not always able to do. Copying the - trio into scratch space first and opening THAT copy (a normal writable - open, since it is only a copy) lets SQLite replay the WAL the same way - a normal startup would. Every write here lands only in ``scratch_dir``; + Only reached when a live read-only backup of the real source failed, + which means there is no live connection to the source at all (a crashed + legacy service can leave committed data sitting only in the ``-wal`` + file, with the main database file still holding the old state). Copying + the trio into scratch space and opening THAT copy (a normal writable + open, since it is only a copy) lets SQLite replay the WAL the same way a + normal startup would. Every write here lands only in ``scratch_dir``; the real source is only ever read via ``shutil.copyfile``. + + Copying the three files one at a time is not itself atomic: if some + writer checkpoints between the main-db copy and the ``-wal`` copy, the + scratch trio pairs an old main db with a newer (reset) WAL, and that torn + pairing can still pass integrity and row-count checks run against + itself, silently dropping history. This path is only meant for a source + with no live writer, so any change detected during the copy window means + something touched the source anyway and the copy cannot be trusted: + each attempt stats size and mtime for all three files immediately before + and after the copy, a mismatch gets one retry with a fresh window, and a + second mismatch fails closed instead of proceeding on an unproven copy. """ + last_error = None + for attempt in range(1, _TRIO_COPY_ATTEMPTS + 1): + before = _source_signatures(legacy) + recovered = _copy_source_trio(legacy, scratch_dir) + after = _source_signatures(legacy) + if before == after: + _checkpoint_recovered_copy(recovered) + return recovered + last_error = ( + f"source files at {legacy} changed while being copied for WAL " + f"recovery (attempt {attempt}/{_TRIO_COPY_ATTEMPTS}); a live writer may be active" + ) + logger.warning(last_error) + + raise RuntimeError( + f"{last_error}; refusing to migrate from a copy that cannot be proven consistent" + ) + + +def _source_signatures(legacy: Path) -> Dict[Path, Optional[tuple]]: + paths = [legacy] + [legacy.with_name(legacy.name + suffix) for suffix in _SOURCE_SIDECAR_SUFFIXES] + return {path: _stat_signature(path) for path in paths} + + +def _stat_signature(path: Path) -> Optional[tuple]: + try: + stat = path.stat() + return (stat.st_size, stat.st_mtime_ns) + except FileNotFoundError: + return None + + +def _copy_source_trio(legacy: Path, scratch_dir: Path) -> Path: recovered = scratch_dir / legacy.name shutil.copyfile(str(legacy), str(recovered)) - for suffix in ("-wal", "-shm"): + for suffix in _SOURCE_SIDECAR_SUFFIXES: sidecar = legacy.with_name(legacy.name + suffix) + destination = recovered.with_name(recovered.name + suffix) if sidecar.is_file(): - shutil.copyfile(str(sidecar), str(recovered.with_name(recovered.name + suffix))) + shutil.copyfile(str(sidecar), str(destination)) + elif destination.exists(): + # A leftover from a previous (retried) attempt in this same + # scratch_dir; the source no longer has this sidecar, so the + # copy should not either. + destination.unlink() + return recovered + +def _checkpoint_recovered_copy(recovered: Path) -> None: conn = sqlite3.connect(recovered) try: conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") @@ -223,17 +320,23 @@ def _recover_source_copy(legacy: Path, scratch_dir: Path) -> Path: finally: conn.close() - return recovered +def _backup_readonly(source: Path, destination: Path) -> None: + """Copy ``source`` into ``destination`` via a read-only ``backup()`` call.""" + _run_backup(sqlite3.connect(f"file:{source}?mode=ro", uri=True), destination) -def _backup_database(source: Path, destination: Path) -> None: - """Copy ``source`` into ``destination`` through the SQLite backup API. - ``source`` here is always a recovered scratch copy (see - ``_recover_source_copy``), never the real legacy file, so opening it - normally is safe. +def _backup_writable(source: Path, destination: Path) -> None: + """Copy ``source`` into ``destination`` via ``backup()``, opened normally. + + Only ever called with a recovered scratch copy as ``source`` (see + ``_recover_source_copy``), never the real legacy file, so a normal + (writable) open is safe. """ - source_conn = sqlite3.connect(source) + _run_backup(sqlite3.connect(source), destination) + + +def _run_backup(source_conn: sqlite3.Connection, destination: Path) -> None: try: destination_conn = sqlite3.connect(destination) try: @@ -367,7 +470,12 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> """Return the manifest that proves ``target`` came from migrating ``legacy``. A ``migrated`` manifest for this exact source/target pair is proof on its - own. A ``pending`` manifest is proof too, once verified: it means the + own, PROVIDED the source file at that path still hashes to what the + manifest recorded. Without that check, replacing the file at ``legacy`` + with a different database (same path, different content -- nothing we + ever do, since the source is never written by this module) would let a + stale manifest vouch for a target that never saw the new file's history + at all. A ``pending`` manifest is proof too, once verified: it means the rename completed and only the finalize write was interrupted, so recomputing the target's digest and row counts and comparing them against what the manifest recorded either confirms that (and finalizes it) or @@ -377,6 +485,8 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> if not archive_root.is_dir(): return None + current_source_digest = _current_source_digest(legacy) + for manifest_path in sorted(archive_root.glob(f"*/{_MANIFEST_NAME}"), reverse=True): try: manifest = json.loads(manifest_path.read_text(encoding="utf-8")) @@ -392,6 +502,14 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> ): continue + if manifest.get("source_digest") != current_source_digest: + logger.debug( + "Manifest %s matches %s by path but not by content; the file " + "there has changed since that migration, so it is not proof.", + manifest_path, legacy, + ) + continue + if manifest.get("status") == "migrated": return manifest_path @@ -405,6 +523,14 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> return None +def _current_source_digest(legacy: Path) -> Optional[str]: + try: + return _sha256(legacy) + except OSError: + logger.debug("Could not hash %s to verify manifest proof", legacy, exc_info=True) + return None + + def _verify_pending_manifest(target: Path, manifest: dict) -> bool: if not target.is_file(): return False diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 65f3a65..249f1ae 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -428,3 +428,189 @@ def test_manifest_proof_matching_folds_case_the_way_the_target_platform_does(tmp assert second.status == "target_exists" assert second.archive_manifest == result.archive_manifest + + +def test_scratch_copy_guard_fails_closed_when_source_changes_mid_copy(tmp_path, monkeypatch): + """The trio-copy fallback (db, -wal, -shm copied one file at a time) is + not itself an atomic snapshot: a writer that changes the source between + the pre-copy and post-copy stat check could pair mismatched files that + still pass every downstream check run against themselves. Force the + fallback path (as an unreadable/unrecovered WAL would), then make every + copy attempt observe drift, and confirm this fails closed rather than + silently accepting an unproven copy.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=2) + + def _boom_readonly(*args, **kwargs): + raise sqlite3.OperationalError("simulated unreadable/unrecovered WAL") + + monkeypatch.setattr(migrations_module, "_backup_readonly", _boom_readonly) + + original_copy = migrations_module._copy_source_trio + + def _copy_then_mutate(legacy_path, scratch_dir): + recovered = original_copy(legacy_path, scratch_dir) + # Simulate a writer touching the source during the copy window: by + # the time the caller's post-copy stat runs, the source no longer + # matches what the pre-copy stat recorded. + with open(legacy_path, "ab") as handle: + handle.write(b"x") + return recovered + + monkeypatch.setattr(migrations_module, "_copy_source_trio", _copy_then_mutate) + + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + message = str(excinfo.value).lower() + assert "changed" in message or "inconsistent" in message + # Nothing was published: no target, and the (mutated, but never + # unlinked/rewritten by us) source is still there. + assert not target.exists() + assert legacy.is_file() + + +def test_migration_recovers_committed_data_from_an_unmerged_wal_still_passes_with_the_guard(tmp_path): + """The crashed-WAL recovery case from the previous fix pass must keep + working with the new primary-path-first, guarded-fallback design.""" + legacy = tmp_path / "legacy" / "humwatch.db" + legacy.parent.mkdir(parents=True) + + crash_script = ( + "import sqlite3, os\n" + f"conn = sqlite3.connect(r'{legacy}')\n" + "conn.execute('PRAGMA journal_mode=WAL')\n" + "conn.execute('PRAGMA wal_autocheckpoint=0')\n" + "conn.execute('CREATE TABLE machine_info (machine_id TEXT PRIMARY KEY)')\n" + "conn.execute('CREATE TABLE metrics (timestamp TEXT, metric_name TEXT, value REAL)')\n" + "conn.execute(\"INSERT INTO machine_info VALUES ('machine-a')\")\n" + "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:00:00Z', 'cpu', 42)\")\n" + "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:01:00Z', 'cpu', 43)\")\n" + "conn.commit()\n" + "os._exit(0)\n" + ) + subprocess.run([sys.executable, "-c", crash_script], check=True) + + wal = legacy.with_name(legacy.name + "-wal") + assert wal.is_file() and wal.stat().st_size > 0, "the crash simulation did not leave an unmerged WAL" + + fresh = tmp_path / "fresh" / "humwatch.db" + fresh.parent.mkdir(parents=True) + shutil.copyfile(legacy, fresh) + for suffix in ("-wal", "-shm"): + sidecar = legacy.with_name(legacy.name + suffix) + if sidecar.is_file(): + shutil.copyfile(sidecar, fresh.with_name(fresh.name + suffix)) + + target = tmp_path / "runtime" / "humwatch.db" + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(fresh), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + + assert result.status == "migrated" + migrated = sqlite3.connect(target) + assert migrated.execute("SELECT machine_id FROM machine_info").fetchone() == ("machine-a",) + assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) + assert legacy.is_file() + assert wal.is_file() + + +def test_target_exists_proof_requires_current_source_digest_to_match(tmp_path): + """A manifest matching by path alone is not enough: if the file at the + legacy path has since become a genuinely different database (this module + never writes to the source, so that only happens if something else + replaced it), the old manifest must not vouch for the target, and an + explicit source with no other proof must refuse to start.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, machine_id="machine-a", metric_rows=2) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + first = migrate_legacy_database(config) + assert first.status == "migrated" + + # Replace the file at the same path with a genuinely different database. + legacy.unlink() + for suffix in ("-wal", "-shm"): + sidecar = legacy.with_name(legacy.name + suffix) + if sidecar.exists(): + sidecar.unlink() + _make_legacy_db(legacy, machine_id="machine-b", metric_rows=9) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + message = str(excinfo.value) + assert str(legacy) in message + assert str(target) in message + # The original, already-migrated target is left exactly as it was. + migrated = sqlite3.connect(target) + assert migrated.execute("SELECT machine_id FROM machine_info").fetchone() == ("machine-a",) + assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) + + +def test_target_parent_directory_is_fsynced_after_the_rename(tmp_path, monkeypatch): + """The rename that publishes the target has to be durable before the + manifest finalize write claims it happened, or power loss between them + could keep the manifest but lose the target. Pin that the target's + parent directory is fsynced, and that it happens after the rename but + before the finalize write.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + events = [] + original_replace = migrations_module.Path.replace + original_fsync_dir = migrations_module._fsync_dir + original_write = migrations_module._write_manifest_atomic + + def _tracked_replace(self, other): + if str(self).endswith(migrations_module._STAGING_SUFFIX): + events.append("rename") + return original_replace(self, other) + + def _tracked_fsync_dir(dir_path): + if dir_path == target.parent: + events.append("fsync_target_parent") + return original_fsync_dir(dir_path) + + def _tracked_write(path, manifest): + if manifest.get("status") == "migrated": + events.append("finalize_write") + return original_write(path, manifest) + + monkeypatch.setattr(migrations_module.Path, "replace", _tracked_replace) + monkeypatch.setattr(migrations_module, "_fsync_dir", _tracked_fsync_dir) + monkeypatch.setattr(migrations_module, "_write_manifest_atomic", _tracked_write) + + result = migrate_legacy_database(config) + + assert result.status == "migrated" + assert "fsync_target_parent" in events + rename_index = events.index("rename") + fsync_index = events.index("fsync_target_parent") + finalize_index = events.index("finalize_write") + assert rename_index < fsync_index < finalize_index + + +def test_same_path_detection_folds_case_the_way_the_target_platform_does(tmp_path, monkeypatch): + """same_path detection must use the same normalized comparison manifest + matching uses, or a differently-cased spelling of the same file (an 8.3 + short name on the real OS this guards) raises a spurious + MigrationSafetyError instead of recognizing there is nothing to do.""" + shared = tmp_path / "shared" / "humwatch.db" + _make_legacy_db(shared) + config = HumWatchConfig( + db_path=str(shared).upper(), legacy_db_path=str(shared), data_root=str(tmp_path / "runtime"), + ) + + monkeypatch.setattr(migrations_module.os.path, "normcase", str.lower) + + result = migrate_legacy_database(config) + + assert result.status == "same_path" + assert result.archive_manifest is None From 9a3bfe690dac7a8ffb5513289fdca24fc516aeb9 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 14 Aug 2026 15:10:28 -0400 Subject: [PATCH 4/7] fix: close two Luna review findings and a test duplicate - Build SQLite read-only URIs from Path.as_uri() instead of raw string interpolation. A plain f"file:{path}?mode=ro" breaks on Windows paths containing backslashes, a drive-letter colon, or a literal %/?/#, and a failed proof-digest recompute at startup meant a healthy already-migrated machine would refuse to start. - Refuse a symlink (or reparse point, on the installer side) planted at the legacy db path or its -wal/-shm sidecars, which could otherwise redirect the migration read outside the application tree. Routes through the same MigrationSafetyError (explicit source) / "failed" (inferred source) paths as every other guard. - Collapse a near-identical duplicate of the unmerged-WAL recovery test that a previous fix pass added instead of just re-verifying the original. Declined by the controller and not implemented: hashing WAL bytes into source_digest. Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 51 ++++++++++- installer/service-setup.ps1 | 13 ++- tests/test_database_migration.py | 150 +++++++++++++++++++++---------- 3 files changed, 162 insertions(+), 52 deletions(-) diff --git a/agent/migrations.py b/agent/migrations.py index 97a9825..e5942cc 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -6,6 +6,7 @@ import os import shutil import sqlite3 +import stat import tempfile from dataclasses import dataclass from datetime import datetime, timezone @@ -144,6 +145,8 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr completed target next to a ``pending`` manifest, which the next call verifies and finalizes rather than treating as unproven. """ + _require_regular_file(legacy, "legacy database") + target.parent.mkdir(parents=True, exist_ok=True) staging = target.with_name(target.name + _STAGING_SUFFIX) if staging.exists(): @@ -290,12 +293,34 @@ def _source_signatures(legacy: Path) -> Dict[Path, Optional[tuple]]: def _stat_signature(path: Path) -> Optional[tuple]: try: - stat = path.stat() - return (stat.st_size, stat.st_mtime_ns) + info = path.stat() + return (info.st_size, info.st_mtime_ns) except FileNotFoundError: return None +def _require_regular_file(path: Path, description: str) -> None: + """Refuse a symlink (or other special file) at ``path``. + + A symlink planted at the legacy db path -- or its ``-wal``/``-shm`` + sidecars -- could redirect a migration read to anywhere on disk outside + the application tree (a reparse point does the same on Windows, guarded + separately at the installer level). ``lstat`` inspects ``path`` itself + rather than following it, which is the only way to actually catch this: + ``Path.is_file()``/``Path.stat()`` both follow symlinks and would see + straight through to whatever the symlink points at. + """ + try: + info = path.lstat() + except FileNotFoundError: + return + if not stat.S_ISREG(info.st_mode): + raise RuntimeError( + f"{description} at {path} is not a regular file (symlink or other " + "special file); refusing to read through it" + ) + + def _copy_source_trio(legacy: Path, scratch_dir: Path) -> Path: recovered = scratch_dir / legacy.name shutil.copyfile(str(legacy), str(recovered)) @@ -303,6 +328,7 @@ def _copy_source_trio(legacy: Path, scratch_dir: Path) -> Path: sidecar = legacy.with_name(legacy.name + suffix) destination = recovered.with_name(recovered.name + suffix) if sidecar.is_file(): + _require_regular_file(sidecar, "legacy database sidecar") shutil.copyfile(str(sidecar), str(destination)) elif destination.exists(): # A leftover from a previous (retried) attempt in this same @@ -323,7 +349,7 @@ def _checkpoint_recovered_copy(recovered: Path) -> None: def _backup_readonly(source: Path, destination: Path) -> None: """Copy ``source`` into ``destination`` via a read-only ``backup()`` call.""" - _run_backup(sqlite3.connect(f"file:{source}?mode=ro", uri=True), destination) + _run_backup(sqlite3.connect(_readonly_sqlite_uri(source), uri=True), destination) def _backup_writable(source: Path, destination: Path) -> None: @@ -372,7 +398,7 @@ def _verify_staged_database(staged: Path) -> Dict[str, int]: def _table_row_counts(db_path: Path, tables: Iterable[str]) -> Dict[str, int]: - conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + conn = sqlite3.connect(_readonly_sqlite_uri(db_path), uri=True) try: return _row_counts(conn, tables) finally: @@ -458,6 +484,23 @@ def _normalize_path(path) -> str: return os.path.normcase(os.path.abspath(str(path))) +def _readonly_sqlite_uri(path: Path) -> str: + """Build a valid SQLite URI for a read-only open of ``path``. + + A plain ``f"file:{path}?mode=ro"`` breaks on Windows: backslash path + separators, a drive letter's colon, and any literal ``%``, ``?``, or + ``#`` in the path are not valid in SQLite's URI syntax. A perfectly + healthy database at a path containing any of those can fail to open at + all, and a failed proof-digest recompute at startup means a healthy, + already-migrated machine refuses to start every time from then on. + ``Path.as_uri()`` produces the correct ``file:///C:/...`` form with + proper percent-escaping (including of ``%``, ``?``, and ``#`` + themselves), and it requires an absolute path, which every path this + module hands to it already is. + """ + return f"{Path(path).as_uri()}?mode=ro" + + def _sha256(path: Path) -> str: digest = hashlib.sha256() with open(path, "rb") as handle: diff --git a/installer/service-setup.ps1 b/installer/service-setup.ps1 index af6fd86..222949e 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -615,8 +615,17 @@ if (Test-Path (Join-Path $RuntimeRoot "tls\trusted-ca.pem")) { # both preserve today's behavior. $LegacyDatabasePath = Join-Path $AppDir "humwatch.db" if (Test-Path -LiteralPath $LegacyDatabasePath -PathType Leaf) { - $environmentPaths += "HUMWATCH_LEGACY_DB=$LegacyDatabasePath" - Write-Log "Found a pre-hardening database at ${LegacyDatabasePath}; migration will use it explicitly." + # A reparse point (symlink or junction) planted at this path by a local + # user before the elevated installer runs could redirect the migration + # read to anywhere on disk. Test-Path alone follows reparse points, so + # the attributes have to be checked directly before trusting this path. + $legacyAttributes = (Get-Item -LiteralPath $LegacyDatabasePath -Force).Attributes + if (($legacyAttributes -band [IO.FileAttributes]::ReparsePoint) -eq 0) { + $environmentPaths += "HUMWATCH_LEGACY_DB=$LegacyDatabasePath" + Write-Log "Found a pre-hardening database at ${LegacyDatabasePath}; migration will use it explicitly." + } else { + Write-Log "Skipped a reparse point at ${LegacyDatabasePath}; not treating it as the pre-hardening database." + } } # AppDirectory is the most critical setting. NSSM defaults it to the diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 249f1ae..795daa6 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -1,6 +1,7 @@ """Regression tests for carrying an existing database into the runtime root.""" import json +import os import shutil import sqlite3 import subprocess @@ -326,7 +327,9 @@ def test_migration_recovers_committed_data_from_an_unmerged_wal(tmp_path): """A crashed legacy service leaves committed rows sitting only in the -wal file, with the main database file still holding the old (empty) state and no live connection left to have checkpointed it. Migration - must still recover every row rather than fail or adopt a stale copy.""" + must still recover every row rather than fail or adopt a stale copy. + Also covers that this keeps working under the primary-path-first, + guarded-fallback snapshot design added in a later fix pass.""" legacy = tmp_path / "legacy" / "humwatch.db" legacy.parent.mkdir(parents=True) @@ -473,51 +476,6 @@ def _copy_then_mutate(legacy_path, scratch_dir): assert legacy.is_file() -def test_migration_recovers_committed_data_from_an_unmerged_wal_still_passes_with_the_guard(tmp_path): - """The crashed-WAL recovery case from the previous fix pass must keep - working with the new primary-path-first, guarded-fallback design.""" - legacy = tmp_path / "legacy" / "humwatch.db" - legacy.parent.mkdir(parents=True) - - crash_script = ( - "import sqlite3, os\n" - f"conn = sqlite3.connect(r'{legacy}')\n" - "conn.execute('PRAGMA journal_mode=WAL')\n" - "conn.execute('PRAGMA wal_autocheckpoint=0')\n" - "conn.execute('CREATE TABLE machine_info (machine_id TEXT PRIMARY KEY)')\n" - "conn.execute('CREATE TABLE metrics (timestamp TEXT, metric_name TEXT, value REAL)')\n" - "conn.execute(\"INSERT INTO machine_info VALUES ('machine-a')\")\n" - "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:00:00Z', 'cpu', 42)\")\n" - "conn.execute(\"INSERT INTO metrics VALUES ('2026-08-13T12:01:00Z', 'cpu', 43)\")\n" - "conn.commit()\n" - "os._exit(0)\n" - ) - subprocess.run([sys.executable, "-c", crash_script], check=True) - - wal = legacy.with_name(legacy.name + "-wal") - assert wal.is_file() and wal.stat().st_size > 0, "the crash simulation did not leave an unmerged WAL" - - fresh = tmp_path / "fresh" / "humwatch.db" - fresh.parent.mkdir(parents=True) - shutil.copyfile(legacy, fresh) - for suffix in ("-wal", "-shm"): - sidecar = legacy.with_name(legacy.name + suffix) - if sidecar.is_file(): - shutil.copyfile(sidecar, fresh.with_name(fresh.name + suffix)) - - target = tmp_path / "runtime" / "humwatch.db" - config = HumWatchConfig(db_path=str(target), legacy_db_path=str(fresh), data_root=str(tmp_path / "runtime")) - - result = migrate_legacy_database(config) - - assert result.status == "migrated" - migrated = sqlite3.connect(target) - assert migrated.execute("SELECT machine_id FROM machine_info").fetchone() == ("machine-a",) - assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) - assert legacy.is_file() - assert wal.is_file() - - def test_target_exists_proof_requires_current_source_digest_to_match(tmp_path): """A manifest matching by path alone is not enough: if the file at the legacy path has since become a genuinely different database (this module @@ -614,3 +572,103 @@ def test_same_path_detection_folds_case_the_way_the_target_platform_does(tmp_pat assert result.status == "same_path" assert result.archive_manifest is None + + +def test_migration_handles_source_paths_with_percent_and_space_characters(tmp_path): + """A plain f"file:{path}?mode=ro" string is not a valid SQLite URI when + the path contains characters SQLite's URI parser treats specially: a + literal %, ?, or # (and, on Windows, a backslash or drive-letter colon). + Every read-only open in this module has to go through proper URI + construction (Path.as_uri()) instead, or a perfectly healthy database at + such a path can fail to open -- and for a proof-digest recompute at + startup, that means a healthy already-migrated machine refuses to + start. Exercises the read-only backup() path (first migration) and + manifest proof matching (second call) over such a path.""" + legacy = tmp_path / "legacy 100% done" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=2) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + result = migrate_legacy_database(config) + + assert result.status == "migrated" + migrated = sqlite3.connect(target) + assert migrated.execute("SELECT COUNT(*) FROM metrics").fetchone() == (2,) + + second = migrate_legacy_database(config) + + assert second.status == "target_exists" + assert second.archive_manifest == result.archive_manifest + + +def test_migration_refuses_a_symlinked_legacy_database_for_an_explicit_source(tmp_path): + """A symlink planted at the legacy db path could redirect the migration + read to anywhere on disk. An explicit source (the installer-set + HUMWATCH_LEGACY_DB case this task exists for) must refuse outright + rather than silently follow it.""" + real = tmp_path / "real" / "humwatch.db" + _make_legacy_db(real) + linked = tmp_path / "legacy" / "humwatch.db" + linked.parent.mkdir(parents=True) + os.symlink(real, linked) + + target = tmp_path / "runtime" / "humwatch.db" + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(linked), data_root=str(tmp_path / "runtime")) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + assert "not a regular file" in str(excinfo.value) or "symlink" in str(excinfo.value).lower() + assert not target.exists() + # Neither the symlink nor the real file it points at were touched. + assert linked.is_symlink() + assert real.is_file() + + +def test_migration_reports_failed_for_a_symlinked_legacy_database_from_an_inferred_source(tmp_path, monkeypatch): + """The same guard on the best-effort (inferred, non-explicit) path must + never be able to block startup -- it reports failed rather than + raising, the same routing as every other failure on this path.""" + project_root = tmp_path / "HumWatch" + project_root.mkdir() + monkeypatch.setattr(migrations_module, "PROJECT_ROOT", project_root) + + real = tmp_path / "real" / "humwatch.db" + _make_legacy_db(real) + os.symlink(real, project_root / "humwatch.db") + + data_root = tmp_path / "runtime" + config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) + + result = migrate_legacy_database(config) + + assert result.status == "failed" + assert not (data_root / "humwatch.db").exists() + assert (project_root / "humwatch.db").is_symlink() + assert real.is_file() + + +def test_scratch_recovery_refuses_a_symlinked_wal_sidecar(tmp_path, monkeypatch): + """The same guard applies to the -wal/-shm sidecars in the scratch + trio-copy fallback, not just the main db file.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=2) + + real_wal = tmp_path / "real.wal" + real_wal.write_bytes(b"not really WAL content, just needs to exist") + linked_wal = legacy.with_name(legacy.name + "-wal") + os.symlink(real_wal, linked_wal) + + def _boom_readonly(*args, **kwargs): + raise sqlite3.OperationalError("simulated unreadable/unrecovered WAL") + + monkeypatch.setattr(migrations_module, "_backup_readonly", _boom_readonly) + + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + assert "not a regular file" in str(excinfo.value) or "symlink" in str(excinfo.value).lower() + assert not target.exists() From cd33acc4f89169f0105e57c42c639671e16c92da Mon Sep 17 00:00:00 2001 From: Static Date: Sat, 15 Aug 2026 21:16:14 -0400 Subject: [PATCH 5/7] docs: stop claiming 8.3 alias tolerance the normalizer does not have Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/agent/migrations.py b/agent/migrations.py index e5942cc..a826969 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -477,9 +477,11 @@ def _normalize_path(path) -> str: """Return a form of ``path`` that compares equal across superficial drift. Manifest proof is matched against this, not the raw string, so a - reinstall to the same location with different path casing, an 8.3 short - name, or a stray trailing separator still finds its own manifest instead - of refusing to start on a healthy machine. + reinstall to the same location with different path casing or a stray + trailing separator still finds its own manifest instead of refusing to + start on a healthy machine. Windows 8.3 short-name aliases are NOT + expanded: a mismatch there fails safe (visible refusal with merge + guidance), and the installer only ever supplies long-form paths. """ return os.path.normcase(os.path.abspath(str(path))) From 3a8b82dd25fb668d42b1bfad8f99ce15b0163133 Mon Sep 17 00:00:00 2001 From: Static Date: Sat, 15 Aug 2026 21:23:23 -0400 Subject: [PATCH 6/7] fix: refuse finalized proof when the current target is not a migrated database Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 37 +++++++++++++++++ tests/test_database_migration.py | 70 ++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/agent/migrations.py b/agent/migrations.py index a826969..89ef9b3 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -556,6 +556,25 @@ def _find_manifest_proof(config: HumWatchConfig, legacy: Path, target: Path) -> continue if manifest.get("status") == "migrated": + # Do NOT compare target_digest or row_counts here: those were + # recorded at migration time, but the live service mutates the + # target from first boot onward and retention pruning shrinks + # row counts over time, so an equality check would false-refuse + # a healthy, long-running machine. This only guards against the + # target having been swapped out from under a surviving + # manifest (e.g. a rollback/restore to a fresh empty database + # while the manifest and legacy file survive): it must still + # open as SQLite, have the required tables, and have at least + # one machine_info row. + if not _target_looks_like_a_migrated_database(target): + logger.warning( + "Manifest %s matches %s/%s by path and source content, but " + "the current target no longer looks like a migrated " + "database (missing tables or no machine_info rows); " + "treating it as unproven.", + manifest_path, legacy, target, + ) + continue return manifest_path if manifest.get("status") == "pending": @@ -590,6 +609,24 @@ def _verify_pending_manifest(target: Path, manifest: dict) -> bool: return False +def _target_looks_like_a_migrated_database(target: Path) -> bool: + """Check that ``target`` still plausibly holds a migrated 2.0 database. + + Used only to gate a FINALIZED manifest's proof (see the ``migrated`` + branch in ``_find_manifest_proof``); deliberately shallow, since it is + not meant to detect drift from what the manifest recorded, only that + the target hasn't been swapped out for something unrelated. + """ + if not target.is_file(): + return False + try: + counts = _table_row_counts(target, _REQUIRED_TABLES) + except Exception: + logger.debug("Could not inspect %s to confirm manifest proof", target, exc_info=True) + return False + return counts.get("machine_info", 0) >= 1 + + def _safety_error_message(legacy: Path, target: Path) -> str: legacy_counts = _safe_row_counts(legacy) target_counts = _safe_row_counts(target) diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 795daa6..12da1d4 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -165,6 +165,76 @@ def test_bug_shaped_case_refuses_without_manifest_proof(tmp_path): assert sqlite3.connect(legacy).execute("SELECT COUNT(*) FROM metrics").fetchone() == (5,) +def test_finalized_manifest_is_not_proof_when_target_was_replaced_with_an_empty_database(tmp_path): + """A finalized manifest and the legacy file can both survive a target + rollback/restore (e.g. the target database file gets swapped for a + fresh empty one). The manifest's source_digest check alone can't catch + this -- the legacy file never changed -- so the target itself must be + inspected too. This is the exact shape of issue #14's sibling: proof + must not be accepted from a manifest whose target no longer looks like + a migrated database.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=3) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + first = migrate_legacy_database(config) + assert first.status == "migrated" + + # Simulate a rollback/restore: the target is replaced with a fresh, + # empty sqlite file. The manifest and the legacy source are untouched. + target.unlink() + sqlite3.connect(target).close() + assert sqlite3.connect(target).execute( + "SELECT COUNT(*) FROM sqlite_master WHERE type='table'" + ).fetchone() == (0,) + + with pytest.raises(MigrationSafetyError) as excinfo: + migrate_legacy_database(config) + + assert str(legacy) in str(excinfo.value) + assert str(target) in str(excinfo.value) + # The empty replacement target is left exactly as it was found. + assert sqlite3.connect(target).execute( + "SELECT COUNT(*) FROM sqlite_master WHERE type='table'" + ).fetchone() == (0,) + + +def test_finalized_manifest_still_proves_target_after_normal_service_life(tmp_path): + """A finalized manifest must NOT be refused just because the live + service has since mutated the target: retention pruning shrinks row + counts, and new metrics get inserted, so the target's row counts will + never again match what the manifest recorded at migration time. Proof + must still be accepted as long as the target still looks like a real + migrated database.""" + legacy = tmp_path / "legacy" / "humwatch.db" + target = tmp_path / "runtime" / "humwatch.db" + _make_legacy_db(legacy, metric_rows=3) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + + first = migrate_legacy_database(config) + assert first.status == "migrated" + + # Simulate normal service life: new metrics land, retention prunes old + # ones, net row count differs from what the manifest recorded. + conn = sqlite3.connect(target) + conn.execute("DELETE FROM metrics") + for i in range(10): + conn.execute( + "INSERT INTO metrics VALUES (?, 'cpu', ?)", + (f"2026-08-14T09:{i:02d}:00Z", 50 + i), + ) + conn.commit() + conn.close() + + second = migrate_legacy_database(config) + + assert second.status == "target_exists" + assert second.archive_manifest == first.archive_manifest + # The target's real, mutated state is left untouched. + assert sqlite3.connect(target).execute("SELECT COUNT(*) FROM metrics").fetchone() == (10,) + + def test_target_exists_without_proof_is_left_alone_for_an_inferred_source(tmp_path, monkeypatch): """The non-explicit (best-effort) path never raises: a provisioned install with a real target and a stray legacy file must survive untouched.""" From b85898cb7288f9659d29ced7f53d37ec03ec36bf Mon Sep 17 00:00:00 2001 From: Static Date: Sat, 15 Aug 2026 21:28:34 -0400 Subject: [PATCH 7/7] fix: make the archive directory entry durable before publishing the target Co-Authored-By: Claude Fable 5 --- agent/migrations.py | 10 ++++++++++ tests/test_database_migration.py | 31 +++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/agent/migrations.py b/agent/migrations.py index 89ef9b3..83b626b 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -175,6 +175,16 @@ def _copy_and_verify(config: HumWatchConfig, legacy: Path, target: Path) -> Migr "time": _utc_now_iso(), } _write_manifest_atomic(manifest_path, manifest) + # On first migration this pending write just created the + # migration-archive directory itself (mkdir(parents=True), inside + # _write_manifest_atomic); that call's own fsyncs only cover entries + # INSIDE migration-archive (the timestamp subdir and the manifest + # file), never the directory entry for migration-archive itself + # within the data root. Without fsyncing the data root here, power + # loss right after the rename below could keep the published target + # while losing every trace that the archive directory -- and thus + # the manifest proving the migration -- exists at all. + _fsync_dir(config.resolved_data_root) staging.replace(target) # The manifest's finalize write below is what proves this rename diff --git a/tests/test_database_migration.py b/tests/test_database_migration.py index 12da1d4..bfe23b2 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -585,11 +585,23 @@ def test_target_parent_directory_is_fsynced_after_the_rename(tmp_path, monkeypat manifest finalize write claims it happened, or power loss between them could keep the manifest but lose the target. Pin that the target's parent directory is fsynced, and that it happens after the rename but - before the finalize write.""" + before the finalize write. + + Also pin the data-root fsync added for the archive directory's own + entry: on first migration, the pending manifest write creates the + migration-archive directory itself (mkdir(parents=True)), and that + directory's entry lives in the data root, not inside migration-archive + where the existing fsyncs already reach. Without a data-root fsync + landing after the pending write and before the rename, power loss right + after publishing the target could lose every trace that the archive + directory (and the manifest proving the migration) exists at all. The + target is nested a level below the data root here so the two fsync + targets are distinguishable events rather than the same directory.""" legacy = tmp_path / "legacy" / "humwatch.db" - target = tmp_path / "runtime" / "humwatch.db" + data_root = tmp_path / "runtime" + target = data_root / "db" / "humwatch.db" _make_legacy_db(legacy) - config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(tmp_path / "runtime")) + config = HumWatchConfig(db_path=str(target), legacy_db_path=str(legacy), data_root=str(data_root)) events = [] original_replace = migrations_module.Path.replace @@ -602,12 +614,16 @@ def _tracked_replace(self, other): return original_replace(self, other) def _tracked_fsync_dir(dir_path): - if dir_path == target.parent: + if dir_path == data_root: + events.append("fsync_data_root") + elif dir_path == target.parent: events.append("fsync_target_parent") return original_fsync_dir(dir_path) def _tracked_write(path, manifest): - if manifest.get("status") == "migrated": + if manifest.get("status") == "pending": + events.append("pending_write") + elif manifest.get("status") == "migrated": events.append("finalize_write") return original_write(path, manifest) @@ -619,10 +635,13 @@ def _tracked_write(path, manifest): assert result.status == "migrated" assert "fsync_target_parent" in events + assert "fsync_data_root" in events + pending_index = events.index("pending_write") + data_root_fsync_index = events.index("fsync_data_root") rename_index = events.index("rename") fsync_index = events.index("fsync_target_parent") finalize_index = events.index("finalize_write") - assert rename_index < fsync_index < finalize_index + assert pending_index < data_root_fsync_index < rename_index < fsync_index < finalize_index def test_same_path_detection_folds_case_the_way_the_target_platform_does(tmp_path, monkeypatch):