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..83b626b 100644 --- a/agent/migrations.py +++ b/agent/migrations.py @@ -1,106 +1,658 @@ """One-time runtime state migrations for in-place upgrades.""" +import hashlib +import json import logging +import os import shutil +import sqlite3 +import stat +import tempfile +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 _normalize_path(target) == _normalize_path(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 + _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(): + staging.unlink() + + source_digest = _sha256(legacy) - # 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)) + with tempfile.TemporaryDirectory(prefix="humwatch-legacy-recovery-") as scratch_dir: + comparable_source = _snapshot_source_into(legacy, staging, Path(scratch_dir)) + row_counts = _verify_staged_database(staging) + 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}" + ) + + manifest_path = _manifest_path_for(config) + manifest = { + "source": _normalize_path(legacy), + "target": _normalize_path(target), + "source_digest": source_digest, + "target_digest": _sha256(staging), + "row_counts": row_counts, + "status": "pending", + "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 + # 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() + _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) +_SOURCE_SIDECAR_SUFFIXES = ("-wal", "-shm") - logger.info("Migrated the existing database into the protected runtime root") - return True +# 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 _discard(path: Path) -> None: - """Remove a file when we are allowed to. A leftover original is harmless.""" +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. + + 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: + 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: - path.unlink() + info = path.lstat() except FileNotFoundError: - pass + 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)) + for suffix in _SOURCE_SIDECAR_SUFFIXES: + 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 + # 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)") + conn.commit() + finally: + conn.close() + + +def _backup_readonly(source: Path, destination: Path) -> None: + """Copy ``source`` into ``destination`` via a read-only ``backup()`` call.""" + _run_backup(sqlite3.connect(_readonly_sqlite_uri(source), uri=True), destination) + + +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. + """ + _run_backup(sqlite3.connect(source), destination) + + +def _run_backup(source_conn: sqlite3.Connection, destination: Path) -> None: + try: + destination_conn = sqlite3.connect(destination) + try: + source_conn.backup(destination_conn) + finally: + destination_conn.close() + finally: + source_conn.close() + + +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: + 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(_readonly_sqlite_uri(db_path), 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 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. 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) + 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: - # 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) + 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 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))) + + +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: + 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, 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 + 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 + + 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")) + except (OSError, json.JSONDecodeError): + continue + + # 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("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": + # 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": + 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 _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 + 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 _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) + 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..222949e 100644 --- a/installer/service-setup.ps1 +++ b/installer/service-setup.ps1 @@ -604,6 +604,30 @@ 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) { + # 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 # 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..bfe23b2 100644 --- a/tests/test_database_migration.py +++ b/tests/test_database_migration.py @@ -1,127 +1,763 @@ """Regression tests for carrying an existing database into the runtime root.""" +import json import os +import shutil +import sqlite3 +import subprocess +import sys 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"] == migrations_module._normalize_path(legacy) + assert manifest["target"] == migrations_module._normalize_path(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")) -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) + result = migrate_legacy_database(config) - import agent.migrations as migrations_module + assert result.status == "source_missing" + assert result.source is None + assert not explicit.exists() - 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")) -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") + result = migrate_legacy_database(config) - data_root = tmp_path / "runtime" - config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) + assert result.status == "same_path" + assert result.archive_manifest is None - 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() +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") -@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. + data_root = tmp_path / "runtime" + config = HumWatchConfig(db_path="./humwatch.db", data_root=str(data_root)) - 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") + 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() + + +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")) + + 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_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.""" + 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 - - def _explode(*args, **kwargs): - raise OSError("no space left on device") - - monkeypatch.setattr(migrations_module.shutil, "copyfile", _explode) + result = migrate_legacy_database(config) - assert migrate_legacy_database(config) is False + assert result.status == "failed" assert not (data_root / "humwatch.db").exists() - assert (project_root / "humwatch.db").read_bytes() == b"telemetry" - + # The corrupt original is left exactly as it was. + assert (project_root / "humwatch.db").read_bytes() == b"not a real sqlite file" + + +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) + + assert not target.exists() + assert legacy.read_bytes() == b"not a real sqlite file" + + +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 _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) + + # 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,) + + 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) + + +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. + 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) + + # 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 + + + +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_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. + + 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" + 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(data_root)) + + 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 == 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") == "pending": + events.append("pending_write") + elif 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 + 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 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): + """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 + + +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) -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") + real = tmp_path / "real" / "humwatch.db" + _make_legacy_db(real) + os.symlink(real, 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)) - 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" + 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_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") - config = HumWatchConfig(db_path="./humwatch.db", data_root=str(tmp_path / "runtime")) +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) - assert migrate_legacy_database(config) is True - assert migrate_legacy_database(config) is False + 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") -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, "_backup_readonly", _boom_readonly) - assert migrate_legacy_database(config) is False - assert not explicit.exists() + 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() 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(