# NousResearch/hermes-agent · Kanban corrupted DB silent data loss fix ## Open now - Nothing open on GitHub. These issues and PRs address a bug where a corrupted or empty Kanban board database would be silently recreated during auto-initialization, causing total data loss. The work focuses on adding guards to refuse or prevent auto-init when database corruption is detected. ## TL;DR - 4 member(s): 0 open issue(s), 1 closed issue(s), 0 open PR(s), 1 merged PR(s), 2 PR(s) closed without merging. - A fix was merged to main in PR #30862. Verify it addresses your case. - **Recommended:** read PR #30862 (and any competing PRs below) before filing. Confirm the merged change covers your symptom. ## Maintainer resolution verdict - Internal-tracker verdict over this cluster's issue members: `resolved` · deciding PR(s): `pr-30862`. This is distinct from the fix status above. Weigh it yourself. ## How this cluster was grouped - Link evidence (`STRUCTURAL_SPANNING`): every member is connected by structural evidence: shared changed files, or a closing/superseded reference. - Pairwise link density `0.50`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked. ## Closed / duplicate issues - #30687: Kanban: corrupted board DB + empty top-level DB → silent recreation with total data loss ## Prior & competing PRs - **#30862: fix(kanban): refuse corrupt db auto-init**. merged to main · fix tier `verified` - maintainer direction: category `merged` - files: `hermes_cli/kanban_db.py`, `scripts/release.py`, `tests/hermes_cli/test_kanban_db.py` - fix hunks: - `hermes_cli/kanban_db.py` ```diff @@ -75,6 +75,7 @@ import os import re import secrets +import shutil import sqlite3 import subprocess import sys ``` ```diff @@ -82,6 +83,7 @@ import logging import time from dataclasses import dataclass, field +from datetime import datetime from pathlib import Path from typing import Any, Iterable, Optional ``` ```diff @@ -1005,6 +1007,131 @@ def _validate_sqlite_header(path: Path) -> None: ) +class KanbanDbCorruptError(RuntimeError): + """Raised when an existing kanban DB file fails integrity checks. + + Fail-closed guard against silent recreation of a corrupt board file, + which would otherwise destroy the user's tasks. Carries both the + original path and the timestamped backup we made before refusing. + """ + + def __init__(self, db_path: Path, backup_path: Optional[Path], reason: str): + self.db_path = db_path + self.backup_path = backup_path + self.reason = reason + backup_str = str(backup_path) if backup_path is not None else "" + super().__init__( + f"Refusing to open corrupt kanban DB at {db_path}: {reason}. " + f"Original preserved; backup at {backup_str}." + ) + + +def _backup_corrupt_db(path: Path) -> Optional[Path]: + """Copy a corrupt DB (and its WAL/SHM sidecars) to a timestamped backup. + + Returns the backup path of the main DB file, or ``None`` if the copy + itself failed (the caller still raises loudly in that case). + + Writes are confined to the original DB's parent directory. The + backup basename is derived purely from ``path.name``, never from + caller-supplied directory segments — no traversal is possible. + """ + # Resolve once and pin the parent so subsequent path operations cannot + # escape it. ``Path.resolve()`` collapses any ``..`` segments and + # symlinks, and we only ever write inside ``parent``. + resolved = path.resolve() + parent = resolved.parent + base_name = resolved.name # basename only + stamp = datetime.now().strftime("%Y%m%d_%H%M%S") + candidate = parent / f"{base_name}.corrupt.{stamp}.bak" + # Defensive: candidate must still be inside parent after construction. + # f-string interpolation of ``base_name`` cannot escape ``parent`` + # because ``base_name`` is itself a resolved basename, but assert it + # anyway so static analyzers can see the containment guarantee. + if candidate.parent != parent: + return None + counter = 0 + while candidate.exists(): + counter += 1 + candidate = parent / f"{base_name}.corrupt.{stamp}.{counter}.bak" + if candidate.parent != parent: + return None + try: + shutil.copy2(resolved, candidate) + except OSError: + return None + for suffix in ("-wal", "-shm"): + sidecar = parent / (base_name + suffix) + if sidecar.parent != parent or not sidecar.exists(): + continue + try: + sidecar_backup = parent / (candidate.name + suffix) + if sidecar_backup.parent != parent: + continue + shutil.copy2(sidecar, sidecar_backup) + except OSError: + pass + return candidate + + +def _guard_existing_db_is_healthy(path: Path) -> None: + """Run ``PRAGMA integrity_check`` on an existing non-empty DB file. + + Opens the probe in read/write mode so SQLite can recover or + checkpoint a healthy WAL/hot-journal DB before we declare it + corrupt. If the file is malformed, copy it (and any WAL/SHM + sidecars) to a timestamped backup and raise + :class:`KanbanDbCorruptError` so callers cannot silently recreate + the schema on top of a damaged DB. + + Transient lock/busy errors (``sqlite3.OperationalError``) are NOT + treated as corruption; they propagate raw so the caller sees a + normal lock failure and no spurious ``.corrupt`` backup is made. + + No-op for missing files, zero-byte files (treated as fresh), and + paths already proven healthy this process (cache hit). + + Path-trust note: ``path`` arrives via :func:`connect`, which itself + resolves it from an explicit ``db_path`` argument, the + :func:`kanban_db_path` env-var chain, or the kanban-home default — + all sources Hermes treats as user-controlled-but-trusted on the + user's own machine. We additionally resolve the path here and + confine all filesystem writes to its parent directory so any + accidental ``..`` segments are collapsed before any I/O happens. + """ + # Resolve before any I/O. ``Path.resolve()`` normalizes ``..`` and + # symlinks, giving us a canonical path whose parent dir we can pin. + try: + resolved = path.resolve() + except OSError: + return + try: + if not resolved.exists() or resolved.stat().st_size == 0: + return + except OSError: + return + if str(resolved) in _INITIALIZED_PATHS: + return + reason: Optional[str] = None + try: + probe = sqlite3.connect(str(resolved), timeout=5, isolation_level=None) + try: + row = probe.execute("PRAGMA integrity_check").fetchone() + finally: + probe.close() + if not row or (row[0] or "").lower() != "ok": + reason = f"integrity_check returned {row[0] if row else ''!r}" + except sqlite3.OperationalError: + # Lock contention, busy, transient IO — not corruption. Let it propagate. + raise + except sqlite3.DatabaseError as exc: + reason = f"sqlite refused to open file: {exc}" + if reason is None: + return + backup = _backup_corrupt_db(resolved) + raise KanbanDbCorruptError(resolved, backup, reason) + + def connect( db_path: Optional[Path] = None, *, ``` ```diff @@ -1033,7 +1160,13 @@ def connect( else: path = kanban_db_path(board=board) path.parent.mkdir(parents=True, exist_ok=True) + # Cheap byte-level check first — catches the #29507 TLS-overwrite shape + # and other invalid-header cases without opening a sqlite connection. _validate_sqlite_header(path) + # Full integrity probe — catches corruption past the header (malformed + # pages, broken internal metadata). Cached per-path after first success + # via _INITIALIZED_PATHS so it only runs once per process per path. + _guard_existing_db_is_healthy(path) resolved = str(path.resolve()) conn = sqlite3.connect(str(path), isolation_level=None, timeout=30) try: ``` - `scripts/release.py` ```diff @@ -808,6 +808,7 @@ "[redacted:email]": "xiayh0107", "[redacted:email]": "opriz", "[redacted:email]": "xxxigm", + "[redacted:email]": "NickLarcombe", "54813621+xxxigm@users.noreply.github.com": "xxxigm", "[redacted:email]": "anniesurla", "[redacted:email]": "nv-kasikritc", ``` - `tests/hermes_cli/test_kanban_db.py` ```diff @@ -2981,3 +2981,104 @@ def test_detect_stale_does_not_tick_failure_counter(kanban_home, monkeypatch): assert "stale" in kinds, ( f"Expected 'stale' event in task_events; got {kinds!r}" ) + + +# --------------------------------------------------------------------------- +# Corruption guard (issue #30687) +# --------------------------------------------------------------------------- + +def _write_corrupt_db(path: Path) -> bytes: + """Write a kanban DB with a VALID SQLite header but malformed page content. + + This is the corruption shape the integrity guard specifically targets + (e.g. issue #29507 follow-up reports where the file's first 16 bytes + pass the header byte check but ``PRAGMA integrity_check`` then fails + because the internal pages are damaged). It's what main's header-only + validator was letting through, and what this PR adds the full guard + for. + """ + # 100-byte SQLite header (magic + minimal valid-looking fields) so the + # cheap header check passes, then deliberate garbage so sqlite refuses + # to read the file past the header. + header = b"SQLite format 3\x00" + b"\x10\x00\x02\x02\x00\x40\x20\x20" + header += b"\x00\x00\x00\x0c\x00\x00\x23\x46\x00\x00\x00\x00" + header = header.ljust(100, b"\x00") + payload = b"definitely not a valid sqlite page \x00\x01\x02\x03" * 64 + blob = header + payload + path.write_bytes(blob) + return blob + + +def test_init_db_refuses_corrupt_existing_file(tmp_path): + db_path = tmp_path / "kanban.db" + original = _write_corrupt_db(db_path) + # Ensure the cache doesn't mask the guard. + kb._INITIALIZED_PATHS.discard(str(db_path.resolve())) + + with pytest.raises(kb.KanbanDbCorruptError) as excinfo: + kb.init_db(db_path=db_path) + + err = excinfo.value + assert err.db_path == db_path + assert err.backup_path is not None + assert err.backup_path.exists() + assert err.backup_path.read_bytes() == original + # Original bytes untouched — no schema was written on top. + assert db_path.read_bytes() == original + assert str(db_path) in str(err) + assert str(err.backup_path) in str(err) + + +def test_connect_refuses_corrupt_existing_file(tmp_path): + db_path = tmp_path / "kanban.db" + _write_corrupt_db(db_path) + kb._INITIALIZED_PATHS.discard(str(db_path.resolve())) + + with pytest.raises(kb.KanbanDbCorruptError): + kb.connect(db_path=db_path) + + +def test_locked_healthy_db_does_not_classify_as_corrupt(tmp_path, monkeypatch): + """A transient lock during the probe must not produce a .corrupt backup + and must not be reported as :class:`KanbanDbCorruptError`. Raw sqlite + ``OperationalError`` (lock/busy) is acceptable and expected.""" + db_path = tmp_path / "kanban.db" + kb.init_db(db_path=db_path) + kb._INITIALIZED_PATHS.discard(str(db_path.resolve())) + + real_connect = sqlite3.connect + + def flaky_connect(*args, **kwargs): + # First call is the integrity probe — simulate a lock. + raise sqlite3.OperationalError("database is locked") + + monkeypatch.setattr(kb.sqlite3, "connect", flaky_connect) + + with pytest.raises(sqlite3.OperationalError): + kb.connect(db_path=db_path) + + # No .corrupt backup may be produced for a healthy-but-locked DB. + backups = list(tmp_path.glob("*.corrupt.*")) + assert backups == [], f"unexpected corrupt backups: {backups}" + + # And once the lock clears, normal access still works. + monkeypatch.setattr(kb.sqlite3, "connect", real_connect) + with kb.connect(db_path=db_path) as conn: + kb.create_task(conn, title="still here") + titles = [t.title for t in kb.list_tasks(conn)] + assert "still here" in titles + + +def test_init_db_allows_missing_then_healthy(tmp_path): + db_path = tmp_path / "fresh.db" + assert not db_path.exists() + kb.init_db(db_path=db_path) + assert db_path.exists() and db_path.stat().st_size > 0 + + # Idempotent on a healthy DB: data survives a second init. + with kb.connect(db_path=db_path) as conn: + kb.create_task(conn, title="keeps") + kb.init_db(db_path=db_path) + with kb.connect(db_path=db_path) as conn: + tasks = kb.list_tasks(conn) + assert [t.title for t in tasks] == ["keeps"] ``` - **#30701: fix(kanban): guard corrupted board DB auto-init (Fixes #30687)**. closed without merge - maintainer direction: category `merged_via`; implication `superseded`; refs `c4b8f5efee8cdd3186e465d940bf0e8c98849346`, `#30862` - **#30707: fix(kanban): refuse corrupt db auto-init**. closed without merge - maintainer direction: category `merged_via`; implication `superseded`; refs `c4b8f5efe`, `#30862` ## Fix status - Merged to main in PR #30862 (cited above). No release data is tracked. Verify downstream. - Fix last verified (merged) `2026-05-23`. - Subsystem: `hermes_cli`