fix(backup): capture memory-provider state stored outside HERMES_HOME (#50325)
hermes backup only walks HERMES_HOME, so memory providers that keep config/credentials in home-anchored dotdirs (honcho -> ~/.honcho, hindsight -> ~/.hindsight, openviking -> ~/.openviking) lost that data across a backup/import cycle — the peer IDs, session pairings, and API keys never made it into the archive. Add an optional MemoryProvider.backup_paths() hook (default []). The active provider declares its external paths; backup resolves them from config only (no init, no network), archives the ones under the home dir into a reserved _external/ subtree encoded relative to home, and import restores them to their original location with a home-anchored traversal guard and 0600 on credential-shaped files. Paths outside home are skipped as non-portable. honcho, hindsight, and openviking override the hook. E2E-validated full backup->import cycle plus 7 new tests.
This commit is contained in:
Teknium
GitHub
parent
7a8c4fe238
commit
587b5b9ac2
6 changed files with 388 additions and 2 deletions
|
|
@ -28,6 +28,7 @@ Optional hooks (override to opt in):
|
|||
on_pre_compress(messages) -> str — extract before context compression
|
||||
on_memory_write(action, target, content, metadata=None) — mirror built-in memory writes
|
||||
on_delegation(task, result, **kwargs) — parent-side observation of subagent work
|
||||
backup_paths() -> list[str] — extra on-disk paths to include in `hermes backup`
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -294,3 +295,21 @@ class MemoryProvider(ABC):
|
|||
|
||||
Use to mirror built-in memory writes to your backend.
|
||||
"""
|
||||
|
||||
def backup_paths(self) -> List[str]:
|
||||
"""Return extra on-disk paths this provider stores OUTSIDE HERMES_HOME.
|
||||
|
||||
``hermes backup`` only walks HERMES_HOME, so any provider state kept
|
||||
under ``~/.honcho``, ``~/.hindsight``, ``~/.openviking``, etc. is lost
|
||||
across a backup/import cycle unless it's declared here.
|
||||
|
||||
Return a list of absolute path strings (files or directories). The
|
||||
backup command resolves each, captures the ones that exist and live
|
||||
under the user's home directory into a reserved ``_external/`` subtree
|
||||
of the archive, and ``hermes import`` restores them to their original
|
||||
locations. Paths outside the home directory are skipped for safety.
|
||||
|
||||
MUST be callable without ``initialize()`` and without network — resolve
|
||||
from config/env only. Default returns an empty list (nothing external).
|
||||
"""
|
||||
return []
|
||||
|
|
|
|||
|
|
@ -124,6 +124,89 @@ _IMPORT_SKIP_NAMES = {
|
|||
# zipfile.open() drops Unix mode bits on extract; restore tightens these to 0600.
|
||||
_SECRET_FILE_NAMES = {".env", "auth.json", "state.db"}
|
||||
|
||||
# Reserved archive subtree for provider state that lives OUTSIDE HERMES_HOME
|
||||
# (e.g. ~/.honcho, ~/.hindsight). The active memory provider declares these via
|
||||
# MemoryProvider.backup_paths(); they're stored under this prefix encoded
|
||||
# relative to the user's home directory, and restored to their original
|
||||
# home-relative location on import. Anything not under home is skipped.
|
||||
_EXTERNAL_PREFIX = "_external/"
|
||||
|
||||
|
||||
def _collect_memory_provider_external_paths() -> List[Path]:
|
||||
"""Return existing absolute paths the active memory provider stores
|
||||
outside HERMES_HOME, resolved from config only (no network, no init).
|
||||
|
||||
Reads ``memory.provider`` from config, loads just that provider, and asks
|
||||
it for ``backup_paths()``. Returns an empty list when no external provider
|
||||
is active or the provider can't be loaded — backup must never fail because
|
||||
of a flaky plugin.
|
||||
"""
|
||||
try:
|
||||
from plugins.memory import _get_active_memory_provider, load_memory_provider
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
try:
|
||||
active = _get_active_memory_provider()
|
||||
except Exception:
|
||||
active = None
|
||||
if not active:
|
||||
return []
|
||||
|
||||
try:
|
||||
provider = load_memory_provider(active)
|
||||
except Exception:
|
||||
provider = None
|
||||
if provider is None:
|
||||
return []
|
||||
|
||||
try:
|
||||
declared = provider.backup_paths() or []
|
||||
except Exception as exc:
|
||||
logger.warning("backup_paths() failed for memory provider %r: %s", active, exc)
|
||||
return []
|
||||
|
||||
out: List[Path] = []
|
||||
seen: set = set()
|
||||
for raw in declared:
|
||||
try:
|
||||
p = Path(raw).expanduser()
|
||||
except Exception:
|
||||
continue
|
||||
if not p.exists():
|
||||
continue
|
||||
try:
|
||||
resolved = p.resolve()
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if resolved in seen:
|
||||
continue
|
||||
seen.add(resolved)
|
||||
out.append(p)
|
||||
return out
|
||||
|
||||
|
||||
def _iter_external_files(base: Path) -> List[Path]:
|
||||
"""Yield regular files under *base* (a file or a directory), skipping
|
||||
symlinks, caches, and pyc files. *base* itself may be a file."""
|
||||
files: List[Path] = []
|
||||
if base.is_file() and not base.is_symlink():
|
||||
files.append(base)
|
||||
return files
|
||||
if not base.is_dir():
|
||||
return files
|
||||
for dirpath, dirnames, filenames in os.walk(base, followlinks=False):
|
||||
dp = Path(dirpath)
|
||||
dirnames[:] = [d for d in dirnames if d not in _EXCLUDED_DIRS]
|
||||
for fname in filenames:
|
||||
fpath = dp / fname
|
||||
if fpath.is_symlink():
|
||||
continue
|
||||
if fpath.name in _EXCLUDED_NAMES or fpath.name.endswith(_EXCLUDED_SUFFIXES):
|
||||
continue
|
||||
files.append(fpath)
|
||||
return files
|
||||
|
||||
|
||||
def _should_exclude(rel_path: Path) -> bool:
|
||||
"""Return True if *rel_path* (relative to hermes root) should be skipped."""
|
||||
|
|
@ -262,12 +345,36 @@ def run_backup(args) -> None:
|
|||
|
||||
files_to_add.append((fpath, rel))
|
||||
|
||||
if not files_to_add:
|
||||
# External memory-provider state (e.g. ~/.honcho, ~/.hindsight) lives
|
||||
# outside HERMES_HOME, so the walk above never sees it. Ask the active
|
||||
# provider for its declared paths and stage them under the reserved
|
||||
# ``_external/`` arc prefix, encoded relative to the user's home dir.
|
||||
# Only paths under home are captured (security + portability); anything
|
||||
# else is skipped with a note.
|
||||
home_dir = Path.home().resolve()
|
||||
external_to_add: list[tuple[Path, str]] = [] # (absolute, arcname)
|
||||
skipped_external: list[str] = []
|
||||
for base in _collect_memory_provider_external_paths():
|
||||
try:
|
||||
base_resolved = base.resolve()
|
||||
base_resolved.relative_to(home_dir)
|
||||
except (ValueError, OSError):
|
||||
skipped_external.append(str(base))
|
||||
continue
|
||||
for fpath in _iter_external_files(base):
|
||||
try:
|
||||
rel_to_home = fpath.resolve().relative_to(home_dir)
|
||||
except (ValueError, OSError):
|
||||
continue
|
||||
arcname = _EXTERNAL_PREFIX + rel_to_home.as_posix()
|
||||
external_to_add.append((fpath, arcname))
|
||||
|
||||
if not files_to_add and not external_to_add:
|
||||
print("No files to back up.")
|
||||
return
|
||||
|
||||
# Create the zip
|
||||
file_count = len(files_to_add)
|
||||
file_count = len(files_to_add) + len(external_to_add)
|
||||
print(f"Backing up {file_count} files ...")
|
||||
|
||||
total_bytes = 0
|
||||
|
|
@ -306,6 +413,17 @@ def run_backup(args) -> None:
|
|||
if i % 500 == 0:
|
||||
print(f" {i}/{file_count} files ...")
|
||||
|
||||
# External memory-provider state, stored under the ``_external/`` arc
|
||||
# prefix. These never include ``.db`` files in practice (config/env
|
||||
# blobs), so a straight zf.write is fine.
|
||||
for abs_path, arcname in external_to_add:
|
||||
try:
|
||||
zf.write(abs_path, arcname=arcname)
|
||||
total_bytes += abs_path.stat().st_size
|
||||
except (PermissionError, OSError, ValueError) as exc:
|
||||
errors.append(f" {arcname}: {exc}")
|
||||
continue
|
||||
|
||||
elapsed = time.monotonic() - t0
|
||||
zip_size = out_path.stat().st_size
|
||||
|
||||
|
|
@ -317,6 +435,20 @@ def run_backup(args) -> None:
|
|||
print(f" Compressed: {_format_size(zip_size)}")
|
||||
print(f" Time: {elapsed:.1f}s")
|
||||
|
||||
if external_to_add:
|
||||
print(
|
||||
f"\n Included {len(external_to_add)} memory-provider file(s) "
|
||||
f"stored outside {display_hermes_home()}."
|
||||
)
|
||||
|
||||
if skipped_external:
|
||||
print(
|
||||
f"\n Skipped {len(skipped_external)} memory-provider path(s) "
|
||||
f"outside your home directory (not portable):"
|
||||
)
|
||||
for p in sorted(skipped_external)[:10]:
|
||||
print(f" {p}")
|
||||
|
||||
if skipped_dirs:
|
||||
print(f"\n Excluded directories:")
|
||||
for d in sorted(skipped_dirs):
|
||||
|
|
@ -442,10 +574,44 @@ def run_import(args) -> None:
|
|||
|
||||
errors = []
|
||||
restored = 0
|
||||
restored_external = 0
|
||||
skipped_runtime: list[str] = []
|
||||
home_dir = Path.home().resolve()
|
||||
t0 = time.monotonic()
|
||||
|
||||
for member in members:
|
||||
# External memory-provider state captured under the reserved
|
||||
# ``_external/`` arc prefix restores to its original home-relative
|
||||
# location (e.g. ~/.honcho/config.json), NOT under HERMES_HOME.
|
||||
if member.startswith(_EXTERNAL_PREFIX):
|
||||
ext_rel = member[len(_EXTERNAL_PREFIX):]
|
||||
if not ext_rel:
|
||||
continue
|
||||
target = home_dir / ext_rel
|
||||
# Security: the resolved target must stay under the home dir.
|
||||
try:
|
||||
target.resolve().relative_to(home_dir)
|
||||
except ValueError:
|
||||
errors.append(f" {member}: path traversal blocked")
|
||||
continue
|
||||
try:
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
with zf.open(member) as src, open(target, "wb") as dst:
|
||||
dst.write(src.read())
|
||||
# External provider configs commonly hold credentials.
|
||||
if target.suffix in {".json", ".env", ".conf"} or target.name in _SECRET_FILE_NAMES:
|
||||
try:
|
||||
os.chmod(target, 0o600)
|
||||
except OSError:
|
||||
pass
|
||||
restored += 1
|
||||
restored_external += 1
|
||||
except (PermissionError, OSError) as exc:
|
||||
errors.append(f" {member}: {exc}")
|
||||
if restored % 500 == 0:
|
||||
print(f" {restored}/{file_count} files ...")
|
||||
continue
|
||||
|
||||
# Strip prefix if detected
|
||||
if prefix and member.startswith(prefix):
|
||||
rel = member[len(prefix):]
|
||||
|
|
@ -494,6 +660,12 @@ def run_import(args) -> None:
|
|||
print(f"Import complete: {restored} files restored in {elapsed:.1f}s")
|
||||
print(f" Target: {display_hermes_home()}")
|
||||
|
||||
if restored_external:
|
||||
print(
|
||||
f"\n Restored {restored_external} memory-provider file(s) to "
|
||||
f"their original location(s) outside {display_hermes_home()}."
|
||||
)
|
||||
|
||||
if errors:
|
||||
print(f"\n Warnings ({len(errors)} files skipped):")
|
||||
for e in errors[:10]:
|
||||
|
|
|
|||
|
|
@ -583,6 +583,16 @@ def _resolve_bank_id_template(template: str, fallback: str, **placeholders: str)
|
|||
class HindsightMemoryProvider(MemoryProvider):
|
||||
"""Hindsight long-term memory with knowledge graph and multi-strategy retrieval."""
|
||||
|
||||
def backup_paths(self) -> List[str]:
|
||||
"""Hindsight's legacy shared config and embedded-mode profile env
|
||||
files live under ~/.hindsight (see _load_config / line ~509)."""
|
||||
try:
|
||||
from pathlib import Path
|
||||
legacy_dir = Path.home() / ".hindsight"
|
||||
return [str(legacy_dir)]
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
def __init__(self):
|
||||
self._config = None
|
||||
self._api_key = None
|
||||
|
|
|
|||
|
|
@ -191,6 +191,19 @@ ALL_TOOL_SCHEMAS = [PROFILE_SCHEMA, SEARCH_SCHEMA, REASONING_SCHEMA, CONTEXT_SCH
|
|||
class HonchoMemoryProvider(MemoryProvider):
|
||||
"""Honcho AI-native memory with dialectic Q&A and persistent user modeling."""
|
||||
|
||||
def backup_paths(self) -> List[str]:
|
||||
"""Honcho keeps its peer/session config under ~/.honcho when no
|
||||
profile-local honcho.json exists (see client.resolve_config_path)."""
|
||||
paths: List[str] = []
|
||||
try:
|
||||
from .client import resolve_global_config_path
|
||||
global_cfg = resolve_global_config_path()
|
||||
# Capture the whole ~/.honcho dir so sibling state travels with it.
|
||||
paths.append(str(global_cfg.parent))
|
||||
except Exception:
|
||||
pass
|
||||
return paths
|
||||
|
||||
def __init__(self):
|
||||
self._manager = None # HonchoSessionManager
|
||||
self._config = None # HonchoClientConfig
|
||||
|
|
|
|||
|
|
@ -1678,6 +1678,19 @@ def _run_create_profile_setup(
|
|||
class OpenVikingMemoryProvider(MemoryProvider):
|
||||
"""Full bidirectional memory via OpenViking context database."""
|
||||
|
||||
def backup_paths(self) -> List[str]:
|
||||
"""OpenViking's ovcli config lives at ~/.openviking/ovcli.conf by
|
||||
default (or OPENVIKING_CLI_CONFIG_FILE). Capture the resolved file so
|
||||
endpoint/api-key survive a backup/import cycle."""
|
||||
try:
|
||||
cfg = _resolve_ovcli_config_path()
|
||||
# The home-scoped guard in the backup walk drops anything outside
|
||||
# the user's home; an env override pointing elsewhere is skipped
|
||||
# there rather than here.
|
||||
return [str(cfg)]
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
def __init__(self):
|
||||
self._client: Optional[_VikingClient] = None
|
||||
self._endpoint = ""
|
||||
|
|
|
|||
|
|
@ -2077,3 +2077,162 @@ class TestRestoreCronJobsIfEmptied:
|
|||
result = restore_cron_jobs_if_emptied(snap_id, hermes_home=hermes_home)
|
||||
assert result is not None
|
||||
assert result["job_count"] == 2
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Memory-provider external paths (~/.honcho, ~/.hindsight, ...) — captured via
|
||||
# MemoryProvider.backup_paths() and restored to their original home-relative
|
||||
# location, NOT under HERMES_HOME. (backup/import cycle data-loss fix)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestMemoryProviderExternalPaths:
|
||||
def _make_min_tree(self, hermes_home: Path) -> None:
|
||||
hermes_home.mkdir(parents=True, exist_ok=True)
|
||||
(hermes_home / "config.yaml").write_text("model:\n provider: openrouter\n")
|
||||
(hermes_home / ".env").write_text("OPENROUTER_API_KEY=sk-test\n")
|
||||
(hermes_home / "state.db").write_bytes(b"x")
|
||||
|
||||
def test_backup_captures_external_paths_under_external_prefix(self, tmp_path, monkeypatch):
|
||||
"""Provider state under ~/.honcho is archived beneath _external/,
|
||||
encoded relative to the home directory."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
self._make_min_tree(hermes_home)
|
||||
# External provider state living OUTSIDE HERMES_HOME.
|
||||
honcho = tmp_path / ".honcho"
|
||||
honcho.mkdir()
|
||||
(honcho / "config.json").write_text('{"peer":"alice"}')
|
||||
(honcho / "sub").mkdir()
|
||||
(honcho / "sub" / "x.json").write_text('{"a":1}')
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
|
||||
import hermes_cli.backup as backup_mod
|
||||
monkeypatch.setattr(
|
||||
backup_mod, "_collect_memory_provider_external_paths", lambda: [honcho]
|
||||
)
|
||||
|
||||
out_zip = tmp_path / "backup.zip"
|
||||
backup_mod.run_backup(Namespace(output=str(out_zip)))
|
||||
|
||||
with zipfile.ZipFile(out_zip) as zf:
|
||||
names = set(zf.namelist())
|
||||
assert "_external/.honcho/config.json" in names
|
||||
assert "_external/.honcho/sub/x.json" in names
|
||||
# In-home files still present.
|
||||
assert "config.yaml" in names
|
||||
|
||||
def test_backup_skips_external_paths_outside_home(self, tmp_path, monkeypatch):
|
||||
"""A declared path outside the home dir is not portable and must be
|
||||
skipped, never archived."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
self._make_min_tree(hermes_home)
|
||||
outside = tmp_path.parent / "outside-home-secret"
|
||||
outside.mkdir(exist_ok=True)
|
||||
(outside / "leak.json").write_text('{"secret":1}')
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
|
||||
import hermes_cli.backup as backup_mod
|
||||
monkeypatch.setattr(
|
||||
backup_mod, "_collect_memory_provider_external_paths", lambda: [outside]
|
||||
)
|
||||
|
||||
out_zip = tmp_path / "backup.zip"
|
||||
backup_mod.run_backup(Namespace(output=str(out_zip)))
|
||||
|
||||
with zipfile.ZipFile(out_zip) as zf:
|
||||
names = set(zf.namelist())
|
||||
assert not any(n.startswith("_external/") for n in names)
|
||||
assert not any("leak.json" in n for n in names)
|
||||
(outside / "leak.json").unlink()
|
||||
outside.rmdir()
|
||||
|
||||
def test_import_restores_external_to_home_relative_location(self, tmp_path, monkeypatch):
|
||||
"""_external/ members restore to ~/<relpath>, not under HERMES_HOME,
|
||||
and credential-shaped files get 0600."""
|
||||
dst_home = tmp_path / "dst"
|
||||
dst_home.mkdir()
|
||||
hermes_home = dst_home / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
||||
zip_path = tmp_path / "backup.zip"
|
||||
with zipfile.ZipFile(zip_path, "w") as zf:
|
||||
zf.writestr("config.yaml", "model: {}\n")
|
||||
zf.writestr(".env", "X=1\n")
|
||||
zf.writestr("state.db", "")
|
||||
zf.writestr("_external/.honcho/config.json", '{"peer":"bob"}')
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: dst_home)
|
||||
|
||||
from hermes_cli.backup import run_import
|
||||
run_import(Namespace(zipfile=str(zip_path), force=True))
|
||||
|
||||
restored = dst_home / ".honcho" / "config.json"
|
||||
assert restored.exists()
|
||||
assert restored.read_text() == '{"peer":"bob"}'
|
||||
# Credential-shaped file tightened.
|
||||
assert (restored.stat().st_mode & 0o777) == 0o600
|
||||
# External state did NOT leak into HERMES_HOME.
|
||||
assert not (hermes_home / "_external").exists()
|
||||
|
||||
def test_import_blocks_external_path_traversal(self, tmp_path, monkeypatch):
|
||||
"""A malicious _external/ member that escapes the home dir is blocked."""
|
||||
dst_home = tmp_path / "dst"
|
||||
dst_home.mkdir()
|
||||
hermes_home = dst_home / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
sentinel = tmp_path / "PWNED"
|
||||
|
||||
zip_path = tmp_path / "backup.zip"
|
||||
with zipfile.ZipFile(zip_path, "w") as zf:
|
||||
zf.writestr("config.yaml", "model: {}\n")
|
||||
zf.writestr(".env", "X=1\n")
|
||||
zf.writestr("state.db", "")
|
||||
zf.writestr("_external/../../PWNED", "pwned")
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: dst_home)
|
||||
|
||||
from hermes_cli.backup import run_import
|
||||
run_import(Namespace(zipfile=str(zip_path), force=True))
|
||||
|
||||
assert not sentinel.exists()
|
||||
|
||||
def test_abc_backup_paths_defaults_empty(self):
|
||||
"""The ABC default returns [] so providers opt in explicitly."""
|
||||
from agent.memory_provider import MemoryProvider
|
||||
|
||||
class _Dummy(MemoryProvider):
|
||||
@property
|
||||
def name(self):
|
||||
return "dummy"
|
||||
|
||||
def is_available(self):
|
||||
return True
|
||||
|
||||
def initialize(self, session_id, **kwargs):
|
||||
pass
|
||||
|
||||
def get_tool_schemas(self):
|
||||
return []
|
||||
|
||||
assert _Dummy().backup_paths() == []
|
||||
|
||||
def test_honcho_provider_declares_global_config_dir(self, tmp_path, monkeypatch):
|
||||
"""The honcho provider's backup_paths() resolves to ~/.honcho."""
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
from plugins.memory.honcho import HonchoMemoryProvider
|
||||
|
||||
paths = HonchoMemoryProvider().backup_paths()
|
||||
assert str(tmp_path / ".honcho") in paths
|
||||
|
||||
def test_hindsight_provider_declares_legacy_dir(self, tmp_path, monkeypatch):
|
||||
"""The hindsight provider's backup_paths() resolves to ~/.hindsight."""
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
from plugins.memory.hindsight import HindsightMemoryProvider
|
||||
|
||||
paths = HindsightMemoryProvider().backup_paths()
|
||||
assert str(tmp_path / ".hindsight") in paths
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue