fix(state): block repair on deleted SQLite holders

Move live-holder inspection into a bounded helper and make repair fail closed when a process still owns the state database, including deleted WAL/SHM descriptors and ambiguous procfs reads.

Preserve the contributor lineage from the original four-commit review train while presenting one coherent release object on current main.

Co-authored-by: fangliquanflq <fangliquan@qq.com>
Co-authored-by: ciabata <296402666+ciabata-git@users.noreply.github.com>
This commit is contained in:
Benjamin PERRY
2026-08-30 09:52:15 +00:00
committed by kshitij
parent 5edc0c492b
commit 06d7b77b1c
7 changed files with 807 additions and 236 deletions
@@ -0,0 +1,2 @@
ciabata-git
# PR #96011 author email preserved by PR #97330
@@ -0,0 +1,2 @@
benperry6
# PR #97330 author email
+15 -178
View File
@@ -60,6 +60,7 @@ from hermes_cli.sqlite_runtime import (
)
from typing import Any, Callable, Dict, Iterator, List, Optional, Set, Tuple, TypeVar, cast
import hermes_state_holders as _state_holders
from hermes_state_common import ( # noqa: F401 (re-exported for back-compat)
_BRANCH_CHILD_SQL,
_COMPRESSION_CHILD_SQL,
@@ -3595,58 +3596,17 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]:
conn.close()
def _foreign_state_db_holders(db_path: Path) -> List[Tuple[int, str]]:
"""Compatibility delegate to the state-holder authority."""
return _state_holders.foreign_state_db_holders(db_path)
def _live_writer_holds_db(db_path: Path) -> bool:
"""True when a connection outside this call still holds ``db_path`` open.
Detection works by asking SQLite for the thing a repair actually needs and
a live writer cannot grant: ``PRAGMA locking_mode=EXCLUSIVE`` followed by
``BEGIN IMMEDIATE``. In WAL mode, entering exclusive locking mode
requires exclusive locks on the WAL index, so any other open connection —
reader or writer — makes it fail with SQLITE_BUSY. Neither statement
parses the schema, so this works on the malformed databases repair exists
to handle.
Fails **open** (returns False) on anything other than a positive
busy/locked signal: refusing to repair a database that nobody is actually
holding would strand the very self-heal path this guard protects.
Scope: the WAL-index exclusive lock is what makes this detect a holder, so
the guard is effective in WAL mode. On SQLite builds carrying the WAL-reset
bug and on NFS/SMB, Hermes deliberately runs ``state.db`` in
``journal_mode=DELETE`` (see :func:`apply_wal_with_fallback`); there a held
reader takes only a SHARED lock, ``BEGIN IMMEDIATE`` still acquires
RESERVED, and this probe returns False. In that mode repair is serialised
only by the cross-process repairer lock rather than by this holder probe.
The 2026-08 incident that motivated the guard was in WAL mode, which this
covers; broadening detection to DELETE mode is left to a follow-up.
"""
probe = None
try:
probe = _connect_repair_durable(db_path, timeout=0.0)
probe.execute("PRAGMA locking_mode=EXCLUSIVE")
probe.execute("BEGIN IMMEDIATE")
probe.execute("ROLLBACK")
return False
except sqlite3.OperationalError as exc:
lowered = str(exc).lower()
return "locked" in lowered or "busy" in lowered
except sqlite3.DatabaseError:
# Malformed/unreadable: no evidence of a live holder either way.
return False
except Exception:
return False
finally:
if probe is not None:
try:
# Drop exclusive locking mode before closing so the probe
# itself never leaves the file pinned.
probe.execute("PRAGMA locking_mode=NORMAL")
except Exception:
pass
try:
probe.close()
except Exception:
pass
"""Compatibility delegate to the repair-admission authority."""
return _state_holders.live_writer_holds_db(
db_path,
connect_repair_durable=_connect_repair_durable,
)
def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, Any]:
@@ -5114,35 +5074,8 @@ def _concrete_state_db_holder_pids(
return pids
def _read_proc_cmdline(pid: int) -> Optional[str]:
"""Read /proc/<pid>/cmdline, world-readable even when fd table is not.
Returns the cmdline as a space-joined string, or None when unreadable
(process exited, or hidepid mount).
"""
try:
with open(f"/proc/{pid}/cmdline", "rb") as f:
raw = f.read()
if not raw:
return None
return raw.replace(b"\x00", b" ").decode("utf-8", "replace").strip()
except OSError:
return None
_HERMES_CMDLINE_MARKERS = ("hermes_cli.main", "hermes_cli/main", "hermes serve",
"hermes-agent", "hermes gateway", "hermes chat")
def _looks_like_hermes(cmdline: str) -> bool:
"""Heuristic: does this cmdline look like a Hermes process?
Used to decide whether an uninspectable process (fd table unreadable
due to different user) should be treated as a potential state.db holder.
We only flag processes that look like Hermes, not every system daemon.
"""
lower = cmdline.lower()
return any(marker in lower for marker in _HERMES_CMDLINE_MARKERS)
_read_proc_argv = _state_holders._read_proc_argv
_looks_like_hermes = _state_holders._looks_like_hermes
# Lifecycle statuses surfaced by session pickers. Classification looks ONLY at
@@ -6720,104 +6653,8 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
return msg.startswith("fts5:") and "corrupt structure" in msg
def _foreign_state_db_holders(self) -> List[Tuple[int, str]]:
"""Return foreign processes holding this DB or its WAL sidecars.
Automatic FTS repair is structural maintenance, not an ordinary WAL
write. It must not run while another process remains attached: a
sidecar reset under that holder can leave the two processes writing
through different WAL inodes.
A scan failure is represented as an unknown holder. Skipping optional
automatic maintenance is safer than assuming quiescence; canonical
writes continue through the stale-FTS fail-open path.
"""
# The split-brain mechanism requires POSIX unlink semantics: Windows
# refuses to replace SQLite sidecars while another process has them
# open. Avoid psutil.open_files() there; querying arbitrary Windows
# processes can block for minutes on device-backed handles.
if _IS_WINDOWS:
return []
if psutil is None:
return [(-1, "open-file scan unavailable")]
db_path = os.path.abspath(os.fspath(self.db_path))
watched = {
_canonical_sqlite_path(db_path),
_canonical_sqlite_path(db_path + "-wal"),
_canonical_sqlite_path(db_path + "-shm"),
}
holders: List[Tuple[int, str]] = []
# On Linux, read /proc/<pid>/fd symlinks directly. psutil's
# open_files() filters through isfile_strict(), which stats the
# literal path — for an unlinked WAL sidecar the kernel returns
# "/path/state.db-wal (deleted)" and stat fails, so the entry is
# silently dropped and the split-brain holder is never seen.
# /proc readlinks preserve the "(deleted)" suffix so _canonical can
# strip it and match.
if sys.platform.startswith("linux"):
try:
own_pid = os.getpid()
for pid_str in os.listdir("/proc"):
if not pid_str.isdigit():
continue
pid = int(pid_str)
if pid == own_pid:
continue
fd_dir = f"/proc/{pid}/fd"
try:
fds = os.listdir(fd_dir)
except OSError:
# Cannot read this process's fd table (different
# user, e.g. root gateway vs user desktop).
# /proc/<pid>/cmdline is world-readable by default,
# so check whether this is a Hermes process —
# only flag uninspectable holders that look like
# another Hermes instance, not every system daemon.
cmdline = _read_proc_cmdline(pid)
if cmdline is not None and _looks_like_hermes(cmdline):
holders.append((pid, f"uninspectable holder: {cmdline[:80]}"))
continue
for fd in fds:
try:
target = os.readlink(f"{fd_dir}/{fd}")
except OSError:
continue
if _canonical_sqlite_path(target) in watched:
holders.append((pid, target))
except Exception as exc:
logger.warning(
"Could not prove state.db has no foreign holders; "
"deferring automatic FTS maintenance: %s",
exc,
)
return holders or [(-1, f"open-file scan failed: {exc}")]
return holders
# macOS / BSD: use psutil.open_files(). macOS does not use the
# "(deleted)" suffix convention, so psutil's filtering is safe here.
try:
for process in psutil.process_iter(["pid", "open_files"]):
info = process.info
pid = int(info["pid"])
if pid == os.getpid():
continue
# psutil's as_dict() converts AccessDenied to None, which
# or-() turns into an empty iteration. On macOS this is
# acceptable: the gateway/desktop topology from the issue is
# Linux-specific (systemd units running as root).
for opened in info.get("open_files") or ():
path = getattr(opened, "path", "")
if path and _canonical_sqlite_path(path) in watched:
holders.append((pid, path))
except Exception as exc:
logger.warning(
"Could not prove state.db has no foreign holders; "
"deferring automatic FTS maintenance: %s",
exc,
)
return holders or [(-1, f"open-file scan failed: {exc}")]
return holders
"""Return foreign processes holding this DB or its WAL sidecars."""
return _foreign_state_db_holders(self.db_path)
def _reap_inactive_orphan_desktop_holders(
self, holders: List[Tuple[int, str]], *, min_age_seconds: float
+291
View File
@@ -0,0 +1,291 @@
"""Process and descriptor authority for state.db structural maintenance.
This module owns the proof that no foreign process still holds the active or
an unlinked SQLite DB/WAL/SHM generation. ``hermes_state`` supplies only the
SQLite connection factory needed by the final lock probe.
"""
from __future__ import annotations
import errno
import logging
import os
import sqlite3
import sys
from pathlib import Path
from typing import Callable, List, Optional, Sequence, Set, Tuple
try: # Hard dependency, but tolerate scaffold-phase imports before pip install.
import psutil
except ImportError: # pragma: no cover - stripped/scaffold installs only
psutil = None # type: ignore[assignment]
logger = logging.getLogger(__name__)
_IS_WINDOWS = sys.platform == "win32"
_HERMES_EXECUTABLES = frozenset({"hermes", "hermes-agent", "hermes-acp"})
_HERMES_PYTHON_MODULES = frozenset({"acp_adapter", "hermes_cli.main"})
_HERMES_PYTHON_SCRIPTS = frozenset({"hermes_cli/main.py", "run_agent.py"})
_PYTHON_SHORT_OPTIONS_WITH_OPERANDS = frozenset({"Q", "W", "X"})
_PYTHON_LONG_OPTIONS_WITH_OPERANDS = frozenset(
{"--check-hash-based-pycs", "--jit"}
)
def _read_proc_argv(pid: int) -> Optional[List[str]]:
"""Read /proc/<pid>/cmdline without losing argv boundaries."""
try:
with open(f"/proc/{pid}/cmdline", "rb") as handle:
raw = handle.read()
if not raw:
return None
argv = raw.decode("utf-8", "replace").split("\x00")
if argv[-1] == "":
argv.pop()
return argv or None
except OSError:
return None
def _looks_like_python_executable(program: str) -> bool:
name = os.path.basename(program).lower().removesuffix(".exe")
for prefix in ("python", "pypy"):
if name.startswith(prefix):
suffix = name[len(prefix) :]
return not suffix or all(char.isdigit() or char == "." for char in suffix)
return False
def _python_execution_target(argv: Sequence[str]) -> Optional[Tuple[str, str]]:
"""Return the Python module or script selected by interpreter options."""
index = 1
while index < len(argv):
arg = argv[index]
if arg == "--":
index += 1
return ("script", argv[index]) if index < len(argv) else None
if arg in _PYTHON_LONG_OPTIONS_WITH_OPERANDS:
index += 2
continue
if arg.startswith("--check-hash-based-pycs=") or arg.startswith("--jit="):
index += 1
continue
if arg.startswith("--"):
index += 1
continue
if arg.startswith("-") and arg != "-":
options = arg[1:]
option_index = 0
consumed_next = False
while option_index < len(options):
option = options[option_index]
attached = options[option_index + 1 :]
if option == "c":
return None
if option == "m":
if attached:
return "module", attached
index += 1
return ("module", argv[index]) if index < len(argv) else None
if option in _PYTHON_SHORT_OPTIONS_WITH_OPERANDS:
consumed_next = not attached
break
option_index += 1
index += 2 if consumed_next else 1
continue
return "script", arg
return None
def _looks_like_hermes(argv: Sequence[str]) -> bool:
"""Return whether argv identifies a supported Hermes execution target."""
if not argv:
return False
program = os.path.basename(argv[0]).lower().removesuffix(".exe")
if program in _HERMES_EXECUTABLES:
return True
if not _looks_like_python_executable(program):
return False
target = _python_execution_target(argv)
if target is None:
return False
kind, value = target
normalized = value.lower().replace("\\", "/")
if kind == "module":
return normalized in _HERMES_PYTHON_MODULES
return any(
normalized == script or normalized.endswith(f"/{script}")
for script in _HERMES_PYTHON_SCRIPTS
)
def foreign_state_db_holders(db_path: Path) -> List[Tuple[int, str]]:
"""Return foreign holders of the DB or one of its WAL sidecars.
A scan failure is represented as an unknown holder. Structural maintenance
must not assume quiescence when an old, unlinked SQLite generation may
still be open by another process.
"""
if _IS_WINDOWS:
return []
def _canonical(path: str) -> str:
clean = path.removesuffix(" (deleted)")
return os.path.normcase(os.path.abspath(clean))
db_path_str = os.path.abspath(os.fspath(db_path))
watched = {
_canonical(db_path_str),
_canonical(db_path_str + "-wal"),
_canonical(db_path_str + "-shm"),
}
holders: List[Tuple[int, str]] = []
watched_ids: Set[Tuple[int, int]] = set()
db_dev: Optional[int] = None
for candidate in (db_path_str, db_path_str + "-wal", db_path_str + "-shm"):
try:
stat_result = os.stat(candidate)
except OSError as exc:
if exc.errno not in (errno.ENOENT, errno.ESRCH):
holders.append(
(-1, f"watched-file stat failed: {candidate}: {exc}")
)
continue
watched_ids.add((stat_result.st_dev, stat_result.st_ino))
if candidate == db_path_str:
db_dev = stat_result.st_dev
if sys.platform.startswith("linux"):
try:
own_pid = os.getpid()
for pid_str in os.listdir("/proc"):
if not pid_str.isdigit():
continue
pid = int(pid_str)
if pid == own_pid:
continue
fd_dir = f"/proc/{pid}/fd"
try:
fds = os.listdir(fd_dir)
except OSError:
argv = _read_proc_argv(pid)
if argv is not None and _looks_like_hermes(argv):
cmdline = " ".join(argv)
holders.append((pid, f"uninspectable holder: {cmdline[:80]}"))
continue
for fd in fds:
fd_path = f"{fd_dir}/{fd}"
try:
target = os.readlink(fd_path)
except OSError as exc:
if exc.errno in (errno.ENOENT, errno.ESRCH):
continue
argv = _read_proc_argv(pid)
if argv is not None and _looks_like_hermes(argv):
holders.append(
(
pid,
f"uninspectable descriptor: {fd_path}: {exc}",
)
)
continue
target_is_watched = _canonical(target) in watched
try:
fd_stat = os.stat(fd_path)
except OSError as exc:
if exc.errno in (errno.ENOENT, errno.ESRCH):
continue
if target_is_watched:
holders.append(
(pid, f"uninspectable descriptor: {target}: {exc}")
)
else:
argv = _read_proc_argv(pid)
if argv is not None and _looks_like_hermes(argv):
holders.append(
(
pid,
"uninspectable descriptor: "
f"{target}: {exc}",
)
)
continue
if (fd_stat.st_dev, fd_stat.st_ino) in watched_ids or (
target_is_watched
and target.endswith(" (deleted)")
and db_dev is not None
and fd_stat.st_dev == db_dev
):
holders.append((pid, target))
except Exception as exc:
logger.warning(
"Could not prove state.db has no foreign holders; "
"deferring structural maintenance: %s",
exc,
)
holders.append((-1, f"open-file scan failed: {exc}"))
return holders
if psutil is None:
return [(-1, "open-file scan unavailable")]
try:
for process in psutil.process_iter(["pid", "open_files"]):
info = process.info
pid = int(info["pid"])
if pid == os.getpid():
continue
for opened in info.get("open_files") or ():
path = getattr(opened, "path", "")
if path and _canonical(path) in watched:
holders.append((pid, path))
except Exception as exc:
logger.warning(
"Could not prove state.db has no foreign holders; "
"deferring structural maintenance: %s",
exc,
)
holders.append((-1, f"open-file scan failed: {exc}"))
return holders
def live_writer_holds_db(
db_path: Path,
*,
connect_repair_durable: Callable[..., sqlite3.Connection],
) -> bool:
"""Return whether repair lacks proven exclusive ownership of ``db_path``."""
foreign_holders = foreign_state_db_holders(db_path)
if any(
pid < 0
or path.startswith("uninspectable holder:")
or path.startswith("uninspectable descriptor:")
or path.endswith(" (deleted)")
for pid, path in foreign_holders
):
return True
probe = None
try:
probe = connect_repair_durable(db_path, timeout=0.0)
probe.execute("PRAGMA locking_mode=EXCLUSIVE")
probe.execute("BEGIN IMMEDIATE")
probe.execute("ROLLBACK")
return False
except sqlite3.OperationalError as exc:
lowered = str(exc).lower()
return "locked" in lowered or "busy" in lowered
except sqlite3.DatabaseError:
return False
except Exception:
return False
finally:
if probe is not None:
try:
probe.execute("PRAGMA locking_mode=NORMAL")
except Exception:
pass
try:
probe.close()
except Exception:
pass
+135 -58
View File
@@ -16,11 +16,11 @@ rebuild later, outside the failed live write/search operation.
import json
import os
import sqlite3
from types import SimpleNamespace
import pytest
import hermes_state
import hermes_state_holders
import hermes_state_schema
from hermes_state import (
FTS_REBUILD_DEFERRAL_KEY,
@@ -140,49 +140,55 @@ class TestRuntimeFtsRebuild:
}
)
def test_foreign_holder_detection_includes_deleted_wal(
self, db, tmp_path, monkeypatch
):
db_path = tmp_path / "state.db"
@pytest.mark.parametrize(
"argv",
(
("journalctl", "-u", "hermes-agent.service"),
("grep", "hermes-agent", "/var/log/syslog"),
(
"/usr/sbin/tailscaled",
"be-child",
"ssh",
"--cmd=python -m hermes_cli.main gateway",
),
("tmux", "new-session", "/opt/hermes-agent/.venv/bin/hermes gateway"),
("python3", "/opt/hermes-agent/tools/check_state.py"),
("hermes-monitor", "gateway"),
("hermesctl", "serve"),
("python3", "worker.py", "hermes_cli.main"),
("python3", "-m", "other.module", "hermes_cli.main"),
("python3", "-c", "hermes_cli.main"),
("python3", "-Icprint('hermes_cli.main')", "hermes_cli/main.py"),
),
)
def test_uninspectable_non_hermes_process_is_not_a_holder(self, argv):
assert not hermes_state_holders._looks_like_hermes(argv)
class FakePsutil:
@staticmethod
def process_iter(_attrs):
return iter(
(
SimpleNamespace(
info={
"pid": 111,
"open_files": [SimpleNamespace(path=str(db_path))],
}
),
SimpleNamespace(
info={
"pid": 222,
"open_files": [
SimpleNamespace(path=f"{db_path}-wal (deleted)")
],
}
),
SimpleNamespace(
info={
"pid": 333,
"open_files": [SimpleNamespace(path=str(tmp_path / "other.db"))],
}
),
)
)
monkeypatch.setattr(hermes_state, "psutil", FakePsutil)
monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False)
monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111)
# Force the macOS/psutil path even on Linux test runners
monkeypatch.setattr(hermes_state.sys, "platform", "darwin")
assert db._foreign_state_db_holders() == [
(222, f"{db_path}-wal (deleted)")
]
@pytest.mark.parametrize(
"argv",
(
("/usr/local/bin/hermes", "gateway"),
("/usr/local/bin/hermes-agent", "serve"),
("/usr/local/bin/hermes-acp", "--stdio"),
("/usr/bin/python3", "-m", "hermes_cli.main", "gateway"),
("/usr/bin/python3", "-m", "acp_adapter"),
("/usr/bin/python3", "-Im", "hermes_cli.main", "gateway"),
("/usr/bin/python3", "-mhermes_cli.main", "gateway"),
("/usr/bin/python3", "-W", "ignore", "-m", "hermes_cli.main"),
("/usr/bin/python3", "-Xdev", "-m", "hermes_cli.main"),
(
"/opt/hermes-agent/.venv/bin/python",
"/opt/hermes-agent/hermes_cli/main.py",
"gateway",
),
("python.exe", "--", "hermes_cli/main.py", "gateway"),
("python3", "/opt/hermes-agent/run_agent.py", "--query", "hello"),
),
)
def test_uninspectable_hermes_process_remains_a_holder(self, argv):
assert hermes_state_holders._looks_like_hermes(argv)
@pytest.mark.linux_only
def test_foreign_holder_detection_proc_readlink_deleted_wal(
self, db, tmp_path, monkeypatch
):
@@ -209,24 +215,93 @@ class TestRuntimeFtsRebuild:
other.touch()
os.symlink(str(other), str(proc_root / "333" / "fd" / "3"))
monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False)
monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111)
monkeypatch.setattr(hermes_state.sys, "platform", "linux")
monkeypatch.setattr(hermes_state_holders.os, "getpid", lambda: 111)
real_listdir = os.listdir
def _listdir(path):
if isinstance(path, str):
path = path.replace("/proc", str(proc_root))
return real_listdir(path)
monkeypatch.setattr(hermes_state.os, "listdir", _listdir)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
real_readlink = os.readlink
def _readlink(path):
path = path.replace("/proc", str(proc_root))
return real_readlink(path)
monkeypatch.setattr(hermes_state.os, "readlink", _readlink)
monkeypatch.setattr(hermes_state_holders.os, "readlink", _readlink)
real_stat = os.stat
def _stat(path, *args, **kwargs):
path_s = str(path).replace("/proc", str(proc_root))
if path_s.endswith("/222/fd/3"):
# A real /proc fd remains statable after unlink and retains
# the deleted sidecar's filesystem identity: same device as
# state.db, but an inode no live watched path can reach.
fields = list(real_stat(db_path))
fields[1] += 1000
return os.stat_result(fields)
return real_stat(path_s, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
holders = db._foreign_state_db_holders()
holders = hermes_state_holders.foreign_state_db_holders(db_path)
assert holders == [(222, db_path_wal + " (deleted)")]
@pytest.mark.linux_only
@pytest.mark.parametrize("different_device", (False, True))
def test_foreign_holder_ignores_same_path_with_different_file_identity(
self, db, tmp_path, monkeypatch, different_device
):
"""A namespace peer's different state.db is not a holder of the host's.
A peer process can appear in /proc with a string-identical path for a
different inode, either on the same filesystem or a different one.
Matching on path alone -- or on device alone -- would defer automatic
FTS maintenance forever while corruption compounds.
Identity must come from (st_dev, st_ino), not the path text.
"""
db_path = tmp_path / "state.db"
proc_root = tmp_path / "proc"
for pid in (111, 222):
(proc_root / str(pid) / "fd").mkdir(parents=True)
# PID 222 = container process holding ITS OWN state.db, which happens
# to have the identical absolute path inside its mount namespace.
guest_db = tmp_path / "guest_state.db"
guest_db.touch()
os.symlink(str(guest_db), str(proc_root / "222" / "fd" / "3"))
monkeypatch.setattr(hermes_state_holders.os, "getpid", lambda: 111)
real_listdir = os.listdir
def _listdir(path):
if isinstance(path, str):
path = path.replace("/proc", str(proc_root))
return real_listdir(path)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
# The guest fd reports the host's path (identical string), which is
# exactly what the kernel shows across mount namespaces.
def _readlink(path):
path = path.replace("/proc", str(proc_root))
if path.endswith(f"{proc_root}/222/fd/3") or "222" in path:
return str(db_path)
return os.readlink(path)
monkeypatch.setattr(hermes_state_holders.os, "readlink", _readlink)
# ...but stat()ing the descriptor resolves to the peer's own inode.
real_stat = os.stat
def _stat(path, *a, **kw):
path_s = str(path).replace("/proc", str(proc_root))
st = real_stat(path_s, *a, **kw)
if different_device and path_s.endswith("/222/fd/3"):
fields = list(st)
# os.stat_result positional layout: st_dev is index 2.
fields[2] = st.st_dev + 1000
return os.stat_result(fields)
return st
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
assert hermes_state_holders.foreign_state_db_holders(db_path) == []
@pytest.mark.linux_only
def test_foreign_holder_uninspectable_process_cmdline_fallback(
self, db, tmp_path, monkeypatch
):
@@ -241,32 +316,34 @@ class TestRuntimeFtsRebuild:
os.chmod(proc_root / "222" / "fd", 0o000)
# PID 222's cmdline is world-readable and looks like Hermes
cmdline_path = proc_root / "222" / "cmdline"
cmdline_path.write_bytes(b"python3\x00hermes_cli.main\x00chat\x00")
cmdline_path.write_bytes(
b"python3\x00-m\x00hermes_cli.main\x00chat\x00"
)
monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False)
monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111)
monkeypatch.setattr(hermes_state.sys, "platform", "linux")
monkeypatch.setattr(hermes_state_holders.os, "getpid", lambda: 111)
real_listdir = os.listdir
def _listdir(path):
if isinstance(path, str):
if path == "/proc/222/fd":
raise PermissionError(path)
path = path.replace("/proc", str(proc_root))
return real_listdir(path)
monkeypatch.setattr(hermes_state.os, "listdir", _listdir)
# _read_proc_cmdline opens /proc/<pid>/cmdline directly; redirect
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
# _read_proc_argv opens /proc/<pid>/cmdline directly; redirect
# it to our fake proc tree.
def _fake_cmdline(pid):
def _fake_argv(pid):
fake_path = str(proc_root / str(pid) / "cmdline")
try:
with open(fake_path, "rb") as f:
raw = f.read()
if not raw:
return None
return raw.replace(b"\x00", b" ").decode("utf-8", "replace").strip()
return raw.decode("utf-8", "replace").rstrip("\x00").split("\x00")
except OSError:
return None
monkeypatch.setattr(hermes_state, "_read_proc_cmdline", _fake_cmdline)
monkeypatch.setattr(hermes_state_holders, "_read_proc_argv", _fake_argv)
holders = db._foreign_state_db_holders()
holders = hermes_state_holders.foreign_state_db_holders(db_path)
# Should include PID 222 with the cmdline info
assert len(holders) == 1
assert holders[0][0] == 222
+50
View File
@@ -0,0 +1,50 @@
"""Behavioral tests for the state-holder and repair-admission authority."""
import os
import pytest
import hermes_state_holders
@pytest.mark.linux_only
def test_foreign_holder_accepts_same_inode_reached_through_an_alias(
tmp_path, monkeypatch
):
"""Descriptor identity is authoritative even when /proc spells another path."""
db_path = tmp_path / "state.db"
db_path.touch()
alias_path = tmp_path / "namespace-alias" / "state.db"
proc_root = tmp_path / "proc"
for pid in (111, 222):
(proc_root / str(pid) / "fd").mkdir(parents=True)
os.symlink(db_path, proc_root / "222" / "fd" / "3")
monkeypatch.setattr(hermes_state_holders.os, "getpid", lambda: 111)
real_listdir = os.listdir
def _listdir(path):
if isinstance(path, str):
path = path.replace("/proc", str(proc_root))
return real_listdir(path)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
def _readlink(path):
if path == "/proc/222/fd/3":
return str(alias_path)
return os.readlink(path.replace("/proc", str(proc_root)))
monkeypatch.setattr(hermes_state_holders.os, "readlink", _readlink)
real_stat = os.stat
def _stat(path, *args, **kwargs):
path = str(path).replace("/proc", str(proc_root))
return real_stat(path, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
assert hermes_state_holders.foreign_state_db_holders(db_path) == [
(222, str(alias_path))
]
@@ -18,12 +18,18 @@ live-writer guard.)
from __future__ import annotations
import errno
import select
import sqlite3
import subprocess
import sys
import uuid
from pathlib import Path
import pytest
import hermes_state
import hermes_state_holders
from hermes_state import (
SessionDB,
repair_state_db_schema,
@@ -84,6 +90,312 @@ def test_repair_refuses_while_another_connection_holds_the_db(tmp_path):
assert "live writer" in (report["error"] or "").lower()
def test_repair_checks_foreign_holders_before_opening_sqlite(tmp_path, monkeypatch):
"""A replacement pathname cannot expose locks on the deleted old inode."""
db = _make_wal_db(tmp_path)
monkeypatch.setattr(
hermes_state_holders,
"foreign_state_db_holders",
lambda _path: [(4242, f"{db}-wal (deleted)")],
)
def _unexpected_probe(*_args, **_kwargs):
pytest.fail("repair opened SQLite before excluding foreign holders")
monkeypatch.setattr(hermes_state, "_connect_repair_durable", _unexpected_probe)
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
@pytest.mark.linux_only
def test_linux_holder_scan_does_not_require_psutil(tmp_path, monkeypatch):
"""The Linux safety scan must not make psutil a repair dependency."""
monkeypatch.setattr(hermes_state_holders, "psutil", None)
holders = hermes_state_holders.foreign_state_db_holders(
tmp_path / "absent-state.db"
)
assert holders == []
@pytest.mark.linux_only
def test_incomplete_holder_scan_keeps_unknown_sentinel(tmp_path, monkeypatch):
"""A partial scan must not hide uncertainty behind an ordinary holder."""
db = tmp_path / "state.db"
db.touch()
def _listdir(path):
if path == "/proc":
return ["4242", "4343"]
if path == "/proc/4242/fd":
return ["7"]
if path == "/proc/4343/fd":
raise RuntimeError("scan interrupted")
raise AssertionError(f"unexpected scan path: {path}")
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
monkeypatch.setattr(hermes_state_holders.os, "readlink", lambda _path: str(db))
real_stat = hermes_state_holders.os.stat
def _stat(path, *args, **kwargs):
if path == "/proc/4242/fd/7":
return real_stat(db)
return real_stat(path, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
holders = hermes_state_holders.foreign_state_db_holders(db)
assert (4242, str(db)) in holders
assert any(pid < 0 and "scan interrupted" in path for pid, path in holders)
@pytest.mark.linux_only
def test_uninspectable_watched_descriptor_blocks_repair_before_sqlite(
tmp_path, monkeypatch
):
"""A watched fd whose identity cannot be read is not proven safe."""
db = _make_wal_db(tmp_path)
def _listdir(path):
if path == "/proc":
return ["4242"]
if path == "/proc/4242/fd":
return ["7"]
raise AssertionError(f"unexpected scan path: {path}")
real_stat = hermes_state_holders.os.stat
def _stat(path, *args, **kwargs):
if path == "/proc/4242/fd/7":
raise PermissionError(errno.EACCES, "descriptor denied", path)
return real_stat(path, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
monkeypatch.setattr(hermes_state_holders.os, "readlink", lambda _path: str(db))
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
def _unexpected_probe(*_args, **_kwargs):
pytest.fail("repair opened SQLite with unproven descriptor identity")
monkeypatch.setattr(hermes_state, "_connect_repair_durable", _unexpected_probe)
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
@pytest.mark.linux_only
@pytest.mark.parametrize(
("argv", "should_block"),
(
(["python3", "backup.py"], False),
(["python3", "-m", "hermes_cli.main", "gateway"], True),
),
)
def test_uninspectable_unknown_descriptor_uses_hermes_identity_at_repair_boundary(
tmp_path, monkeypatch, argv, should_block
):
"""An unknown fd target blocks only when argv identifies Hermes."""
db = _make_wal_db(tmp_path)
def _listdir(path):
if path == "/proc":
return ["4242"]
if path == "/proc/4242/fd":
return ["7"]
raise AssertionError(f"unexpected scan path: {path}")
def _readlink(path):
raise PermissionError(errno.EACCES, "descriptor denied", path)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
monkeypatch.setattr(hermes_state_holders.os, "readlink", _readlink)
monkeypatch.setattr(
hermes_state_holders,
"_read_proc_argv",
lambda _pid: argv,
)
if should_block:
def _unexpected_probe(*_args, **_kwargs):
pytest.fail("repair opened SQLite with an unproven Hermes descriptor")
monkeypatch.setattr(
hermes_state,
"_connect_repair_durable",
_unexpected_probe,
)
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
else:
real_connect = hermes_state._connect_repair_durable
probe_reached = False
def _record_probe(*args, **kwargs):
nonlocal probe_reached
probe_reached = True
return real_connect(*args, **kwargs)
monkeypatch.setattr(
hermes_state,
"_connect_repair_durable",
_record_probe,
)
report = repair_state_db_schema(db, backup=False)
assert probe_reached is True
assert "live writer" not in (report["error"] or "").lower()
@pytest.mark.linux_only
def test_uninspectable_watched_identity_blocks_alias_before_sqlite(
tmp_path, monkeypatch
):
"""A non-disappearance stat error cannot prove an aliased holder safe."""
db = _make_wal_db(tmp_path)
alias = tmp_path / "namespace-alias" / "state.db"
def _listdir(path):
if path == "/proc":
return ["4242"]
if path == "/proc/4242/fd":
return ["7"]
raise AssertionError(f"unexpected scan path: {path}")
real_stat = hermes_state_holders.os.stat
def _stat(path, *args, **kwargs):
if str(path) == str(db) and not args and not kwargs:
raise PermissionError(errno.EACCES, "watched identity denied", path)
if path == "/proc/4242/fd/7":
return real_stat(db)
return real_stat(path, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
monkeypatch.setattr(
hermes_state_holders.os, "readlink", lambda _path: str(alias)
)
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
def _unexpected_probe(*_args, **_kwargs):
pytest.fail("repair opened SQLite with an unproven watched identity")
monkeypatch.setattr(hermes_state, "_connect_repair_durable", _unexpected_probe)
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
@pytest.mark.linux_only
def test_uninspectable_alias_descriptor_for_hermes_blocks_before_sqlite(
tmp_path, monkeypatch
):
"""Hermes cannot make an aliased fd safe when its identity is unreadable."""
db = _make_wal_db(tmp_path)
alias = tmp_path / "namespace-alias" / "state.db"
def _listdir(path):
if path == "/proc":
return ["4242"]
if path == "/proc/4242/fd":
return ["7"]
raise AssertionError(f"unexpected scan path: {path}")
real_stat = hermes_state_holders.os.stat
def _stat(path, *args, **kwargs):
if path == "/proc/4242/fd/7":
raise PermissionError(errno.EACCES, "descriptor denied", path)
return real_stat(path, *args, **kwargs)
monkeypatch.setattr(hermes_state_holders.os, "listdir", _listdir)
monkeypatch.setattr(
hermes_state_holders.os, "readlink", lambda _path: str(alias)
)
monkeypatch.setattr(hermes_state_holders.os, "stat", _stat)
monkeypatch.setattr(
hermes_state_holders,
"_read_proc_argv",
lambda _pid: ["python3", "-m", "hermes_cli.main", "gateway"],
)
def _unexpected_probe(*_args, **_kwargs):
pytest.fail("repair opened SQLite with an unproven Hermes alias fd")
monkeypatch.setattr(hermes_state, "_connect_repair_durable", _unexpected_probe)
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
@pytest.mark.requires_wal
@pytest.mark.linux_only
def test_repair_refuses_while_foreign_process_holds_deleted_wal(tmp_path):
"""Reproduce the inode split that a pathname lock probe cannot observe."""
db = _make_wal_db(tmp_path)
holder_code = """
import sqlite3
import sys
conn = sqlite3.connect(sys.argv[1])
conn.execute("PRAGMA journal_mode=WAL")
conn.execute("BEGIN IMMEDIATE")
print("ready", flush=True)
sys.stdin.read(1)
conn.rollback()
conn.close()
"""
holder = subprocess.Popen(
[sys.executable, "-c", holder_code, str(db)],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
try:
assert holder.stdout is not None
readable, _, _ = select.select([holder.stdout], [], [], 10)
assert readable, "holder subprocess did not signal readiness"
assert holder.stdout.readline().strip() == "ready"
deleted = []
for suffix in ("-wal", "-shm"):
sidecar = Path(f"{db}{suffix}")
if sidecar.exists():
sidecar.unlink()
deleted.append(sidecar)
assert deleted
report = repair_state_db_schema(db, backup=False)
assert report["repaired"] is False
assert "live writer" in (report["error"] or "").lower()
finally:
if holder.poll() is None and holder.stdin is not None:
try:
holder.stdin.write("x")
holder.stdin.close()
except (BrokenPipeError, ValueError):
pass
try:
holder.wait(timeout=10)
except subprocess.TimeoutExpired:
holder.kill()
holder.wait(timeout=10)
def test_repair_proceeds_once_the_database_is_quiescent(tmp_path):
"""The guard must not deadlock repair on an exclusively-held file."""
db = _make_wal_db(tmp_path)