fix(state): make journal mode canonical and behaviorally verified
Use database.journal_mode as the sole non-secret operator setting, preserve the vulnerable-SQLite safety gate and existing WAL databases, validate explicit DELETE results, document the active config path, and cover real SQLite openers with behavioral tests.
This commit is contained in:
@@ -61,6 +61,8 @@ def _db_path() -> Path:
|
||||
|
||||
|
||||
def _connect() -> sqlite3.Connection:
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
path = _db_path()
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
conn = sqlite3.connect(path)
|
||||
|
||||
+15
-2
@@ -1,6 +1,19 @@
|
||||
# Hermes Agent CLI Configuration
|
||||
# Copy this file to cli-config.yaml and customize as needed.
|
||||
# This file configures the CLI behavior. Environment variables in .env take precedence.
|
||||
# Copy settings from this example into ~/.hermes/config.yaml, or use
|
||||
# `hermes config set <section.key> <value>` to update the active profile.
|
||||
# This file configures CLI behavior; only documented secret environment
|
||||
# variables in .env take precedence over their corresponding settings.
|
||||
|
||||
# =============================================================================
|
||||
# Database Configuration
|
||||
# =============================================================================
|
||||
# WAL is the normal default. Hermes automatically falls back to DELETE when
|
||||
# SQLite reports that WAL is incompatible with the filesystem. Set this to
|
||||
# "delete" explicitly for deployments whose backing filesystem is not WAL
|
||||
# crash-safe, such as Linux containers bind-mounted through macOS virtiofs,
|
||||
# NFS, or SMB. Hermes will not live-downgrade a database already open in WAL.
|
||||
database:
|
||||
journal_mode: "wal" # Supported values: "wal", "delete"
|
||||
|
||||
# =============================================================================
|
||||
# Model Configuration
|
||||
|
||||
+1
-2
@@ -34,8 +34,7 @@ def _initialize_schema(conn: sqlite3.Connection) -> None:
|
||||
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA busy_timeout=5000")
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
apply_wal_with_fallback(conn, db_label="cron_executions.db")
|
||||
apply_wal_with_fallback(conn, db_label="cron/executions.db")
|
||||
conn.execute("PRAGMA synchronous=FULL")
|
||||
conn.execute(
|
||||
"""CREATE TABLE IF NOT EXISTS executions (
|
||||
|
||||
@@ -10,6 +10,12 @@ DEFAULT_CONFIG = {
|
||||
"fallback_providers": [],
|
||||
"credential_pool_strategies": {},
|
||||
"toolsets": ["hermes-cli"],
|
||||
# SQLite journal mode used by every Hermes database opener. WAL is the
|
||||
# normal default; set DELETE for weak-fsync/shared filesystems where WAL is
|
||||
# not crash-safe (for example macOS virtiofs, NFS, or SMB).
|
||||
"database": {
|
||||
"journal_mode": "wal",
|
||||
},
|
||||
# Global active chat session cap across CLI, TUI/dashboard, and messaging.
|
||||
# None/0 = unbounded.
|
||||
"max_concurrent_sessions": None,
|
||||
|
||||
+56
-30
@@ -509,23 +509,27 @@ def sqlite_source_id() -> str:
|
||||
def resolve_journal_mode() -> str:
|
||||
"""Return the configured journal mode (``wal`` or ``delete``).
|
||||
|
||||
Honors ``HERMES_JOURNAL_MODE`` env var and ``database.journal_mode``
|
||||
in config.yaml. Default is ``wal``. Set to ``delete`` on filesystems
|
||||
where WAL is not crash-safe (virtiofs, NFS, SMB, containerized-on-
|
||||
macOS) — the process can't detect the backing filesystem from inside
|
||||
a container, so this is the escape hatch (#68545).
|
||||
``database.journal_mode`` in config.yaml is the canonical operator
|
||||
setting. ``wal`` remains the default; use ``delete`` when the backing
|
||||
filesystem does not provide WAL-safe durability (for example macOS
|
||||
virtiofs, NFS, or SMB). Invalid or malformed values fail safely to the
|
||||
existing default.
|
||||
"""
|
||||
raw = os.environ.get("HERMES_JOURNAL_MODE", "").strip().lower()
|
||||
if not raw:
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
cfg = load_config() or {}
|
||||
raw = str(cfg.get("database", {}).get("journal_mode", "")).strip().lower()
|
||||
except Exception:
|
||||
pass
|
||||
if raw in ("delete", "truncate", "persist", "memory", "off"):
|
||||
return raw
|
||||
return "wal"
|
||||
try:
|
||||
from hermes_cli.config import load_config_readonly
|
||||
|
||||
config = load_config_readonly() or {}
|
||||
database = config.get("database", {})
|
||||
if not isinstance(database, dict):
|
||||
return "wal"
|
||||
raw = database.get("journal_mode", "wal")
|
||||
except Exception:
|
||||
return "wal"
|
||||
|
||||
if not isinstance(raw, str):
|
||||
return "wal"
|
||||
mode = raw.strip().lower()
|
||||
return mode if mode in ("wal", "delete") else "wal"
|
||||
|
||||
|
||||
def apply_wal_with_fallback(
|
||||
@@ -571,9 +575,18 @@ def apply_wal_with_fallback(
|
||||
_on_disk_journal_mode. That holds for both the NFS path and the
|
||||
WAL-reset vulnerability path.
|
||||
"""
|
||||
# Vulnerable SQLite: do not enable WAL on new/non-WAL files.
|
||||
configured = resolve_journal_mode()
|
||||
|
||||
# Vulnerable SQLite: do not enable WAL on new/non-WAL files. Resolve the
|
||||
# operator setting first so an explicit DELETE request still verifies that
|
||||
# SQLite actually accepted DELETE rather than silently returning MEMORY or
|
||||
# another connection-specific mode.
|
||||
if is_sqlite_wal_reset_vulnerable():
|
||||
return _apply_delete_for_wal_reset_bug(conn, db_label=db_label)
|
||||
return _apply_delete_for_wal_reset_bug(
|
||||
conn,
|
||||
db_label=db_label,
|
||||
require_delete=configured == "delete",
|
||||
)
|
||||
|
||||
# Read-only probe — no flock, no checkpoint, no WAL/SHM unlink.
|
||||
# Skipping the set-pragma prevents WAL-init from unlinking files other connections hold open.
|
||||
@@ -586,15 +599,16 @@ def apply_wal_with_fallback(
|
||||
except sqlite3.OperationalError:
|
||||
pass
|
||||
|
||||
# #68545: honor user-configured journal_mode (env/config.yaml).
|
||||
# If the user forced DELETE (e.g. for virtiofs/NFS/SMB), don't try WAL.
|
||||
_configured = resolve_journal_mode()
|
||||
if _configured != "wal":
|
||||
try:
|
||||
conn.execute(f"PRAGMA journal_mode={_configured.upper()}")
|
||||
except sqlite3.OperationalError:
|
||||
pass # mode may not be supported; leave whatever is set
|
||||
return _configured
|
||||
# #68545: honor the canonical database.journal_mode setting. Existing
|
||||
# on-disk WAL databases were returned above and are never live-downgraded.
|
||||
if configured == "delete":
|
||||
row = conn.execute("PRAGMA journal_mode=DELETE").fetchone()
|
||||
actual = str(row[0]).lower() if row else ""
|
||||
if actual != "delete":
|
||||
raise sqlite3.OperationalError(
|
||||
f"could not set configured journal_mode=delete (got {actual or 'no result'})"
|
||||
)
|
||||
return actual
|
||||
|
||||
try:
|
||||
conn.execute("PRAGMA journal_mode=WAL")
|
||||
@@ -619,11 +633,13 @@ def _apply_delete_for_wal_reset_bug(
|
||||
conn: sqlite3.Connection,
|
||||
*,
|
||||
db_label: str,
|
||||
require_delete: bool = False,
|
||||
) -> str:
|
||||
"""Avoid enabling WAL when the linked SQLite has the WAL-reset bug.
|
||||
|
||||
- Already-WAL on disk: leave WAL alone (no live downgrade) and warn.
|
||||
- Otherwise: set DELETE and warn.
|
||||
- For an explicit operator request, verify SQLite accepted DELETE.
|
||||
"""
|
||||
current = ""
|
||||
try:
|
||||
@@ -641,11 +657,21 @@ def _apply_delete_for_wal_reset_bug(
|
||||
_enforce_macos_synchronous_full(conn)
|
||||
return "wal"
|
||||
|
||||
actual = ""
|
||||
try:
|
||||
conn.execute("PRAGMA journal_mode=DELETE")
|
||||
row = conn.execute("PRAGMA journal_mode=DELETE").fetchone()
|
||||
if row and row[0] is not None:
|
||||
actual = str(row[0]).strip().lower()
|
||||
except sqlite3.OperationalError:
|
||||
# Best-effort: DELETE is usually already the default for new files.
|
||||
pass
|
||||
if require_delete:
|
||||
raise
|
||||
# Best-effort for the automatic vulnerable-runtime fallback: DELETE is
|
||||
# normally already the default for new file-backed databases.
|
||||
if require_delete and actual != "delete":
|
||||
raise sqlite3.OperationalError(
|
||||
"could not set configured journal_mode=delete "
|
||||
f"(got {actual or 'no result'})"
|
||||
)
|
||||
_log_wal_reset_bug_once(db_label, kept_wal=False)
|
||||
return "delete"
|
||||
|
||||
|
||||
@@ -54,6 +54,7 @@ class DiscordRecoveryStore:
|
||||
|
||||
def _initialize(self, conn: sqlite3.Connection) -> None:
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
apply_wal_with_fallback(conn, db_label="discord_recovery.db")
|
||||
conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS discord_messages (
|
||||
|
||||
@@ -1,84 +1,228 @@
|
||||
"""#68545: configurable journal_mode (env + config.yaml) + centralized DB openers."""
|
||||
"""Behavioral coverage for #68545's centralized journal-mode setting."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import inspect
|
||||
import os
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
|
||||
def test_resolve_journal_mode_defaults_to_wal(monkeypatch):
|
||||
def _write_config(monkeypatch: pytest.MonkeyPatch, tmp_path, config: object) -> None:
|
||||
home = tmp_path / "hermes-home"
|
||||
home.mkdir(exist_ok=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
(home / "config.yaml").write_text(
|
||||
yaml.safe_dump(config),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _configure_mode(monkeypatch: pytest.MonkeyPatch, tmp_path, mode: object) -> None:
|
||||
_write_config(monkeypatch, tmp_path, {"database": {"journal_mode": mode}})
|
||||
|
||||
|
||||
def _disable_vulnerable_gate(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
"hermes_state.is_sqlite_wal_reset_vulnerable",
|
||||
lambda **kwargs: False,
|
||||
)
|
||||
|
||||
|
||||
def test_database_journal_mode_has_a_canonical_default():
|
||||
from hermes_cli.config import DEFAULT_CONFIG
|
||||
|
||||
assert DEFAULT_CONFIG["database"]["journal_mode"] == "wal"
|
||||
|
||||
|
||||
def test_resolve_journal_mode_uses_real_database_config(monkeypatch, tmp_path):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
monkeypatch.delenv("HERMES_JOURNAL_MODE", raising=False)
|
||||
assert resolve_journal_mode() == "wal"
|
||||
_configure_mode(monkeypatch, tmp_path, "DELETE")
|
||||
assert resolve_journal_mode() == "delete"
|
||||
|
||||
|
||||
def test_resolve_journal_mode_env_override(monkeypatch):
|
||||
def test_new_nonsecret_hermes_env_override_is_not_exposed(monkeypatch, tmp_path):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
_configure_mode(monkeypatch, tmp_path, "wal")
|
||||
monkeypatch.setenv("HERMES_JOURNAL_MODE", "delete")
|
||||
assert resolve_journal_mode() == "delete"
|
||||
|
||||
|
||||
def test_resolve_journal_mode_env_truncase(monkeypatch):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
monkeypatch.setenv("HERMES_JOURNAL_MODE", "DELETE")
|
||||
assert resolve_journal_mode() == "delete"
|
||||
|
||||
|
||||
def test_resolve_journal_mode_invalid_falls_back_to_wal(monkeypatch):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
monkeypatch.setenv("HERMES_JOURNAL_MODE", "bogus")
|
||||
assert resolve_journal_mode() == "wal"
|
||||
|
||||
|
||||
def test_apply_wal_with_fallback_honors_delete_mode(monkeypatch, tmp_path):
|
||||
"""When HERMES_JOURNAL_MODE=delete, apply_wal_with_fallback must NOT set WAL."""
|
||||
@pytest.mark.parametrize("value", ["bogus", "truncate", None, 42, {"bad": "shape"}])
|
||||
def test_invalid_config_value_falls_back_to_wal(monkeypatch, tmp_path, value):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
_configure_mode(monkeypatch, tmp_path, value)
|
||||
assert resolve_journal_mode() == "wal"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("database", [[], "delete", 42, None])
|
||||
def test_malformed_database_section_falls_back_to_wal(
|
||||
monkeypatch, tmp_path, database
|
||||
):
|
||||
from hermes_state import resolve_journal_mode
|
||||
|
||||
_write_config(monkeypatch, tmp_path, {"database": database})
|
||||
assert resolve_journal_mode() == "wal"
|
||||
|
||||
|
||||
def test_apply_wal_with_fallback_honors_delete_config(monkeypatch, tmp_path):
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
monkeypatch.setenv("HERMES_JOURNAL_MODE", "delete")
|
||||
db = tmp_path / "test.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
mode = apply_wal_with_fallback(conn, db_label="test.db")
|
||||
assert mode == "delete"
|
||||
actual = conn.execute("PRAGMA journal_mode").fetchone()[0]
|
||||
assert actual.lower() == "delete"
|
||||
conn.close()
|
||||
_configure_mode(monkeypatch, tmp_path, "delete")
|
||||
_disable_vulnerable_gate(monkeypatch)
|
||||
conn = sqlite3.connect(tmp_path / "configured.db")
|
||||
try:
|
||||
assert apply_wal_with_fallback(conn, db_label="configured.db") == "delete"
|
||||
assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "delete"
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_apply_wal_with_fallback_defaults_to_wal(monkeypatch, tmp_path):
|
||||
"""Without override, apply_wal_with_fallback still sets WAL."""
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
monkeypatch.delenv("HERMES_JOURNAL_MODE", raising=False)
|
||||
# Avoid the WAL-reset vulnerability check on older SQLite versions
|
||||
# (our dev env has 3.50.4 which is flagged vulnerable, causing a
|
||||
# delete-mode fallback that makes this test environment-dependent).
|
||||
_configure_mode(monkeypatch, tmp_path, "wal")
|
||||
_disable_vulnerable_gate(monkeypatch)
|
||||
conn = sqlite3.connect(tmp_path / "default.db")
|
||||
try:
|
||||
assert apply_wal_with_fallback(conn, db_label="default.db") == "wal"
|
||||
assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal"
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_configured_delete_validates_vulnerable_sqlite_result(monkeypatch, tmp_path):
|
||||
"""The safety gate must not report DELETE when SQLite returns MEMORY."""
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
_configure_mode(monkeypatch, tmp_path, "delete")
|
||||
monkeypatch.setattr(
|
||||
"hermes_state.is_sqlite_wal_reset_vulnerable", lambda **kw: False
|
||||
"hermes_state.is_sqlite_wal_reset_vulnerable",
|
||||
lambda **kwargs: True,
|
||||
)
|
||||
db = tmp_path / "test2.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
mode = apply_wal_with_fallback(conn, db_label="test2.db")
|
||||
assert mode == "wal"
|
||||
conn.close()
|
||||
conn = sqlite3.connect(":memory:")
|
||||
try:
|
||||
with pytest.raises(sqlite3.OperationalError, match="configured.*delete"):
|
||||
apply_wal_with_fallback(conn, db_label="memory-configured.db")
|
||||
assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "memory"
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_direct_setters_use_apply_wal_with_fallback():
|
||||
"""All 5 bypass openers must route through apply_wal_with_fallback (#68545)."""
|
||||
for fpath in [
|
||||
"tools/async_delegation.py",
|
||||
"gateway/delivery_ledger.py",
|
||||
"agent/verification_evidence.py",
|
||||
"cron/executions.py",
|
||||
"plugins/platforms/discord/recovery.py",
|
||||
]:
|
||||
src = Path(fpath).read_text(encoding="utf-8")
|
||||
assert "apply_wal_with_fallback" in src, f"{fpath} must use apply_wal_with_fallback"
|
||||
assert 'PRAGMA journal_mode=WAL"' not in src, f"{fpath} must not set WAL directly"
|
||||
def test_configured_delete_never_live_downgrades_existing_wal(monkeypatch, tmp_path):
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
_configure_mode(monkeypatch, tmp_path, "delete")
|
||||
db_path = tmp_path / "existing-wal.db"
|
||||
conn = sqlite3.connect(db_path)
|
||||
try:
|
||||
assert conn.execute("PRAGMA journal_mode=WAL").fetchone()[0].lower() == "wal"
|
||||
monkeypatch.setattr(
|
||||
"hermes_state.is_sqlite_wal_reset_vulnerable",
|
||||
lambda **kwargs: True,
|
||||
)
|
||||
assert apply_wal_with_fallback(conn, db_label="existing-wal.db") == "wal"
|
||||
assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal"
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_real_db_openers_honor_configured_delete(monkeypatch, tmp_path):
|
||||
"""All helper-routed file-backed openers must behaviorally use DELETE."""
|
||||
_configure_mode(monkeypatch, tmp_path, "delete")
|
||||
_disable_vulnerable_gate(monkeypatch)
|
||||
|
||||
from agent import verification_evidence
|
||||
from cron import executions
|
||||
from gateway import delivery_ledger
|
||||
from gateway.platforms.api_server import ResponseStore
|
||||
from hermes_cli import kanban_db, projects_db
|
||||
from hermes_state import SessionDB
|
||||
from plugins.memory.holographic.store import MemoryStore
|
||||
from plugins.platforms.discord.recovery import DiscordRecoveryStore
|
||||
from tools import async_delegation
|
||||
|
||||
observed: dict[str, str] = {}
|
||||
|
||||
for name, connect in (
|
||||
("async_delegation", async_delegation._connect),
|
||||
("delivery_ledger", delivery_ledger._connect),
|
||||
("verification_evidence", verification_evidence._connect),
|
||||
):
|
||||
conn = connect()
|
||||
try:
|
||||
observed[name] = conn.execute("PRAGMA journal_mode").fetchone()[0].lower()
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
monkeypatch.setattr(executions, "EXECUTIONS_FILE", tmp_path / "cron" / "executions.db")
|
||||
executions.EXECUTIONS_FILE.parent.mkdir(parents=True, exist_ok=True)
|
||||
cron_conn = executions._connect()
|
||||
try:
|
||||
executions._initialize_schema(cron_conn)
|
||||
observed["cron_executions"] = cron_conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
cron_conn.close()
|
||||
|
||||
discord = DiscordRecoveryStore(hermes_home=tmp_path)
|
||||
observed["discord_recovery"] = discord.call(
|
||||
lambda conn: conn.execute("PRAGMA journal_mode").fetchone()[0].lower()
|
||||
)
|
||||
|
||||
session_db = SessionDB(db_path=tmp_path / "state.db")
|
||||
try:
|
||||
observed["session_db"] = session_db._conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
session_db.close()
|
||||
|
||||
kanban_conn = kanban_db.connect(db_path=tmp_path / "kanban.db")
|
||||
try:
|
||||
observed["kanban"] = kanban_conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
kanban_conn.close()
|
||||
|
||||
projects_conn = projects_db.connect(db_path=tmp_path / "projects.db")
|
||||
try:
|
||||
observed["projects"] = projects_conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
projects_conn.close()
|
||||
|
||||
holographic = MemoryStore(db_path=tmp_path / "memory_store.db")
|
||||
try:
|
||||
observed["holographic"] = holographic._conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
holographic.close()
|
||||
|
||||
response_store = ResponseStore(db_path=str(tmp_path / "response_store.db"))
|
||||
try:
|
||||
observed["response_store"] = response_store._conn.execute(
|
||||
"PRAGMA journal_mode"
|
||||
).fetchone()[0].lower()
|
||||
finally:
|
||||
response_store.close()
|
||||
|
||||
assert observed == {
|
||||
"async_delegation": "delete",
|
||||
"delivery_ledger": "delete",
|
||||
"verification_evidence": "delete",
|
||||
"cron_executions": "delete",
|
||||
"discord_recovery": "delete",
|
||||
"session_db": "delete",
|
||||
"kanban": "delete",
|
||||
"projects": "delete",
|
||||
"holographic": "delete",
|
||||
"response_store": "delete",
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user