hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(memory): enforce independent built-in store permissions

Browse Source 
Normalize malformed memory config during initialization and bind per-target write permissions to the session MemoryStore so direct and staged writes cannot update a disabled built-in store.
This commit is contained in:
kshitijk4poor
2026-08-20 11:35:22 +05:30
committed by Teknium
parent c809d964d4
commit 2cf7b36e11
7 changed files with 292 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
agent/agent_init.py
View File

@@ -1811,9 +1811,12 @@ def init_agent(
_memory_toolset_requested = "memory" in (agent.enabled_toolsets or [])
if not skip_memory or _memory_toolset_requested:
try:
from tools.memory_tool import get_builtin_memory_store_flags
from tools.memory_tool import (
get_builtin_memory_config,
get_builtin_memory_store_flags,
)
mem_config = _agent_cfg.get("memory", {})
mem_config = get_builtin_memory_config(_agent_cfg)
agent._memory_enabled, agent._user_profile_enabled = get_builtin_memory_store_flags(
_agent_cfg
)
@@ -1823,6 +1826,8 @@ def init_agent(
agent._memory_store = MemoryStore(
memory_char_limit=mem_config.get("memory_char_limit", 2200),
user_char_limit=mem_config.get("user_char_limit", 1375),
memory_enabled=agent._memory_enabled,
user_profile_enabled=agent._user_profile_enabled,
)
agent._memory_store.load_from_disk()
except Exception:

114
tests/agent/test_builtin_memory_disabled_surface.py
View File

@@ -12,6 +12,9 @@ These tests exercise the real resolution chain (config on disk → check_fn →
``get_tool_definitions``) against a temp ``HERMES_HOME``, not mocks.
"""
import json
from unittest.mock import patch
import pytest
import yaml
@@ -46,6 +49,14 @@ def hermes_home(tmp_path, monkeypatch):
return home
def _memory_tool_definition():
return next(
tool["function"]
for tool in get_tool_definitions(enabled_toolsets=["memory"], quiet_mode=True)
if tool["function"]["name"] == "memory"
)
def _memory_tool_names():
tools = get_tool_definitions(enabled_toolsets=["memory"], quiet_mode=True)
return {tool["function"]["name"] for tool in tools}
@@ -62,13 +73,19 @@ class TestBuiltinMemoryToolAvailability:
_write_memory_config(
hermes_home, memory_enabled=False, user_profile_enabled=True
)
assert "memory" in _memory_tool_names()
definition = _memory_tool_definition()
assert definition["parameters"]["properties"]["target"]["enum"] == ["user"]
assert "only 'user' is enabled" in definition["description"]
assert "only 'memory' is enabled" not in definition["description"]
def test_tool_present_when_only_memory_enabled(self, hermes_home):
_write_memory_config(
hermes_home, memory_enabled=True, user_profile_enabled=False
)
assert "memory" in _memory_tool_names()
definition = _memory_tool_definition()
assert definition["parameters"]["properties"]["target"]["enum"] == ["memory"]
assert "only 'memory' is enabled" in definition["description"]
assert "only 'user' is enabled" not in definition["description"]
def test_tool_present_by_default(self, hermes_home):
"""No config file at all must not strip a working tool."""
@@ -87,6 +104,42 @@ class TestBuiltinMemoryToolAvailability:
{"memory": {"memory_enabled": "false", "user_profile_enabled": "false"}}
) == (False, False)
def test_schema_reuses_availability_flag_snapshot(self, monkeypatch):
"""One definition pass must not reread config between check and schema."""
from tools import memory_tool as memory_tool_module
calls = 0
def _flags():
nonlocal calls
calls += 1
return (False, True) if calls == 1 else (True, False)
monkeypatch.setattr(memory_tool_module, "get_builtin_memory_store_flags", _flags)
definition = _memory_tool_definition()
assert calls == 1
assert definition["parameters"]["properties"]["target"]["enum"] == ["user"]
def test_unavailable_snapshot_cannot_survive_failed_recheck(self, monkeypatch):
from tools import memory_tool as memory_tool_module
monkeypatch.setattr(
memory_tool_module,
"get_builtin_memory_store_flags",
lambda: (False, False),
)
assert memory_tool_module.check_memory_requirements() is False
def _boom():
raise RuntimeError("config read failed")
monkeypatch.setattr(memory_tool_module, "get_builtin_memory_store_flags", _boom)
with pytest.raises(RuntimeError, match="config read failed"):
memory_tool_module.check_memory_requirements()
assert memory_tool_module._memory_surface_flags.get() is None
def test_config_flip_updates_tool_without_manual_cache_clear(self, hermes_home):
_write_memory_config(
hermes_home, memory_enabled=False, user_profile_enabled=False
@@ -110,6 +163,63 @@ class TestBuiltinMemoryToolAvailability:
)
assert memory_tool_module.check_memory_requirements() is True
def test_malformed_memory_section_normalizes_to_defaults(self):
from tools.memory_tool import (
get_builtin_memory_config,
get_builtin_memory_store_flags,
)
config = {"memory": "not-a-mapping"}
assert get_builtin_memory_config(config) == {}
assert get_builtin_memory_store_flags(config) == (True, True)
class TestIndependentStoreWriteGates:
def _store(self, *, memory_enabled, user_profile_enabled):
from tools.memory_tool import MemoryStore
store = MemoryStore(
memory_char_limit=500,
user_char_limit=500,
memory_enabled=memory_enabled,
user_profile_enabled=user_profile_enabled,
)
store.load_from_disk()
return store
def test_user_profile_only_rejects_memory_write(self, hermes_home):
from tools.memory_tool import memory_tool
store = self._store(memory_enabled=False, user_profile_enabled=True)
denied = json.loads(memory_tool(action="add", target="memory", content="fact", store=store))
allowed = json.loads(memory_tool(action="add", target="user", content="pref", store=store))
assert denied["success"] is False
assert denied["target"] == "memory"
assert allowed["success"] is True
def test_memory_only_rejects_user_profile_write(self, hermes_home):
from tools.memory_tool import memory_tool
store = self._store(memory_enabled=True, user_profile_enabled=False)
denied = json.loads(memory_tool(action="add", target="user", content="pref", store=store))
allowed = json.loads(memory_tool(action="add", target="memory", content="fact", store=store))
assert denied["success"] is False
assert denied["target"] == "user"
assert allowed["success"] is True
def test_staged_write_cannot_bypass_target_gate(self, hermes_home):
from tools.memory_tool import apply_memory_pending
store = self._store(memory_enabled=False, user_profile_enabled=True)
result = apply_memory_pending(
{"action": "add", "target": "memory", "content": "fact"}, store
)
assert result["success"] is False
assert result["target"] == "memory"
class TestExternalProviderSurvivesBuiltinDisable:
"""Dropping the built-in tool must not drop the external provider's tools.

31
tests/run_agent/test_run_agent.py
View File

@@ -179,6 +179,37 @@ def test_direct_session_db_flushes_share_marker_claim(agent):
assert db.rows == ["exactly once"]
def test_malformed_memory_config_still_builds_default_store():
"""A non-mapping memory section must not leave an advertised dead tool."""
malformed = {"memory": "not-a-mapping"}
with (
patch(
"hermes_cli.config.load_config_readonly",
return_value=malformed,
),
patch(
"run_agent.get_tool_definitions",
return_value=_make_tool_defs("memory"),
),
patch("run_agent.check_toolset_requirements", return_value={}),
patch("run_agent.OpenAI"),
):
agent = AIAgent(
api_key="test-k...7890",
base_url="https://openrouter.ai/api/v1",
quiet_mode=True,
skip_context_files=True,
skip_memory=True,
enabled_toolsets=["memory"],
)
assert agent._memory_enabled is True
assert agent._user_profile_enabled is True
assert agent._memory_store is not None
assert agent._memory_store.memory_enabled is True
assert agent._memory_store.user_profile_enabled is True
@pytest.fixture()
def agent_with_memory_tool():
"""Agent whose valid_tool_names includes 'memory'."""

23
tests/tools/test_write_approval.py
View File

@@ -108,22 +108,27 @@ def test_cli_memory_approve_without_live_agent_uses_fresh_store(hermes_home, cap
assert any("remember the launch date" in e for e in reloaded.memory_entries)
def test_load_on_disk_store_honors_configured_char_limits(hermes_home, monkeypatch):
"""load_on_disk_store() must read memory.memory_char_limit /
user_char_limit from config so approvals applied without a live agent
enforce the SAME caps as the live agent (agent_init.py). Falls back to
defaults when config can't be loaded.
"""
def test_load_on_disk_store_honors_configured_limits_and_permissions(hermes_home, monkeypatch):
"""Fresh approval stores must match the live agent's limits and target gates."""
from tools.memory_tool import load_on_disk_store
# Config override path: helper picks up the configured limits.
# Config override path: helper picks up configured limits and store flags.
monkeypatch.setattr(
"hermes_cli.config.load_config",
lambda: {"memory": {"memory_char_limit": 999, "user_char_limit": 444}},
lambda: {
"memory": {
"memory_char_limit": 999,
"user_char_limit": 444,
"memory_enabled": False,
"user_profile_enabled": True,
}
},
)
store = load_on_disk_store()
assert store.memory_char_limit == 999
assert store.user_char_limit == 444
assert store.memory_enabled is False
assert store.user_profile_enabled is True
# Failure path: config raises → defaults, never blows up.
def _boom():
@@ -133,6 +138,8 @@ def test_load_on_disk_store_honors_configured_char_limits(hermes_home, monkeypat
fallback = load_on_disk_store()
assert fallback.memory_char_limit == 2200
assert fallback.user_char_limit == 1375
assert fallback.memory_enabled is True
assert fallback.user_profile_enabled is True
# ---------------------------------------------------------------------------

127
tools/memory_tool.py
View File

@@ -23,10 +23,12 @@ Design:
- Frozen snapshot pattern: system prompt is stable, tool responses show live state
"""
import copy
import json
import logging
import time
from contextlib import contextmanager
from contextvars import ContextVar
from pathlib import Path
from hermes_constants import get_hermes_home
from typing import Dict, Any, List, Optional, Tuple
@@ -47,6 +49,14 @@ except ImportError:
logger = logging.getLogger(__name__)
# One tool-definition pass must use one config decision for both availability
# and the dynamic target schema. ContextVar keeps concurrent profile/session
# builds isolated while allowing the check_fn result to flow to the immediately
# following dynamic_schema_overrides call in ToolRegistry.get_definitions().
_memory_surface_flags: ContextVar[Optional[Tuple[bool, bool]]] = ContextVar(
"memory_surface_flags", default=None
)
# Where memory files live — resolved dynamically so profile overrides
# (HERMES_HOME env var changes) are always respected. The old module-level
# constant was cached at import time and could go stale if a profile switch
@@ -163,17 +173,30 @@ class MemoryStore:
# turn to budget exhaustion and suppress the user's reply (issue #42405).
_MAX_CONSOLIDATION_FAILURES_PER_TURN = 3
def __init__(self, memory_char_limit: int = 2200, user_char_limit: int = 1375):
def __init__(
self,
memory_char_limit: int = 2200,
user_char_limit: int = 1375,
*,
memory_enabled: bool = True,
user_profile_enabled: bool = True,
):
self.memory_entries: List[str] = []
self.user_entries: List[str] = []
self.memory_char_limit = memory_char_limit
self.user_char_limit = user_char_limit
self.memory_enabled = memory_enabled
self.user_profile_enabled = user_profile_enabled
# Frozen snapshot for system prompt -- set once at load_from_disk()
self._system_prompt_snapshot: Dict[str, str] = {"memory": "", "user": ""}
# Per-turn counter of failed at-capacity consolidation attempts; reset
# at each turn boundary by reset_consolidation_failures() (#42405).
self._consolidation_failures = 0
def target_enabled(self, target: str) -> bool:
"""Return whether this session's selected built-in store is writable."""
return self.user_profile_enabled if target == "user" else self.memory_enabled
def reset_consolidation_failures(self) -> None:
"""Reset the per-turn consolidation-failure counter (call at turn start)."""
self._consolidation_failures = 0
@@ -900,10 +923,14 @@ def load_on_disk_store() -> "MemoryStore":
"""
memory_char_limit = 2200
user_char_limit = 1375
memory_enabled = True
user_profile_enabled = True
try:
from hermes_cli.config import load_config
mem_cfg = (load_config() or {}).get("memory", {}) or {}
config = load_config() or {}
mem_cfg = get_builtin_memory_config(config)
memory_enabled, user_profile_enabled = get_builtin_memory_store_flags(config)
memory_char_limit = int(mem_cfg.get("memory_char_limit", memory_char_limit))
user_char_limit = int(mem_cfg.get("user_char_limit", user_char_limit))
except Exception:
@@ -912,6 +939,8 @@ def load_on_disk_store() -> "MemoryStore":
store = MemoryStore(
memory_char_limit=memory_char_limit,
user_char_limit=user_char_limit,
memory_enabled=memory_enabled,
user_profile_enabled=user_profile_enabled,
)
store.load_from_disk()
return store
@@ -1093,8 +1122,9 @@ def memory_tool(
if target is None:
target = "memory"
if target not in {"memory", "user"}:
return tool_error(f"Invalid target '{target}'. Use 'memory' or 'user'.", success=False)
target_error = _memory_target_error(store, target)
if target_error is not None:
return json.dumps(target_error)
# --- Batch path -------------------------------------------------------
if operations:
@@ -1144,12 +1174,13 @@ def memory_tool(
return json.dumps(result, ensure_ascii=False)
def get_builtin_memory_store_flags(config: Optional[Dict[str, Any]] = None) -> Tuple[bool, bool]:
"""Return ``(memory_enabled, user_profile_enabled)`` from resolved config.
def get_builtin_memory_config(config: Optional[Dict[str, Any]] = None) -> Dict[str, Any]:
"""Return a normalized built-in memory config mapping.
``agent_init`` uses this same predicate to decide whether to construct the
``MemoryStore`` that backs the tool. Missing or unreadable config fails open
so availability checks do not remove a tool that would otherwise work.
Missing, unreadable, or malformed sections become an empty mapping, whose
missing flags resolve to the enabled defaults. ``agent_init`` consumes this
same normalized section so tool availability and store construction cannot
diverge.
"""
if config is None:
try:
@@ -1158,27 +1189,42 @@ def get_builtin_memory_store_flags(config: Optional[Dict[str, Any]] = None) -> T
config = load_config_readonly()
except Exception:
logger.debug("Could not read memory config for availability", exc_info=True)
return True, True
return {}
section = config.get("memory") if isinstance(config, dict) else None
if not isinstance(section, dict):
return True, True
return section if isinstance(section, dict) else {}
def get_builtin_memory_store_flags(config: Optional[Dict[str, Any]] = None) -> Tuple[bool, bool]:
"""Return ``(memory_enabled, user_profile_enabled)`` from resolved config."""
section = get_builtin_memory_config(config)
return (
is_truthy_value(section.get("memory_enabled"), default=True),
is_truthy_value(section.get("user_profile_enabled"), default=True),
)
def builtin_memory_stores_enabled() -> bool:
"""Return whether either built-in store (MEMORY.md / USER.md) is enabled."""
memory_enabled, user_profile_enabled = get_builtin_memory_store_flags()
return memory_enabled or user_profile_enabled
@no_cache_check_fn
def check_memory_requirements() -> bool:
"""Available unless both built-in memory stores are disabled in config."""
return builtin_memory_stores_enabled()
"""Snapshot store flags and report whether the built-in tool is available."""
_memory_surface_flags.set(None)
flags = get_builtin_memory_store_flags()
_memory_surface_flags.set(flags)
return flags[0] or flags[1]
def _memory_target_error(store: "MemoryStore", target: str) -> Optional[Dict[str, Any]]:
"""Return a shared validation error for an invalid or disabled target."""
if target not in {"memory", "user"}:
return {"success": False, "error": f"Invalid memory target '{target}'."}
if store.target_enabled(target):
return None
label = "USER.md" if target == "user" else "MEMORY.md"
return {
"success": False,
"error": f"Built-in {label} writes are disabled in memory config.",
"target": target,
}
def apply_memory_pending(payload: Dict[str, Any], store: "MemoryStore") -> Dict[str, Any]:
@@ -1189,6 +1235,9 @@ def apply_memory_pending(payload: Dict[str, Any], store: "MemoryStore") -> Dict[
"""
action = payload.get("action")
target = payload.get("target", "memory")
target_error = _memory_target_error(store, target)
if target_error is not None:
return target_error
content = payload.get("content") or ""
old_text = payload.get("old_text") or ""
if action == "batch":
@@ -1276,6 +1325,43 @@ MEMORY_SCHEMA = {
}
def _build_memory_schema_overrides() -> Dict[str, Any]:
"""Narrow the advertised target surface using the availability snapshot."""
flags = _memory_surface_flags.get()
_memory_surface_flags.set(None)
if flags is None:
flags = get_builtin_memory_store_flags()
memory_enabled, user_profile_enabled = flags
targets = []
if memory_enabled:
targets.append("memory")
if user_profile_enabled:
targets.append("user")
parameters = copy.deepcopy(MEMORY_SCHEMA["parameters"])
target_schema = parameters["properties"]["target"]
target_schema["enum"] = targets
description = MEMORY_SCHEMA["description"]
if targets == ["memory"]:
target_schema["description"] = "The enabled built-in store: 'memory' for personal notes."
description = description.replace(
"TARGETS: 'user' = who the user is (name, role, preferences, style). 'memory' = your "
"notes (environment, conventions, tool quirks, lessons).",
"TARGET: only 'memory' is enabled for personal notes (environment, conventions, "
"tool quirks, lessons).",
)
elif targets == ["user"]:
target_schema["description"] = "The enabled built-in store: 'user' for user profile."
description = description.replace(
"TARGETS: 'user' = who the user is (name, role, preferences, style). 'memory' = your "
"notes (environment, conventions, tool quirks, lessons).",
"TARGET: only 'user' is enabled for user profile facts (name, role, preferences, style).",
)
return {"description": description, "parameters": parameters}
# --- Registry ---
from tools.registry import registry, tool_error
@@ -1293,6 +1379,7 @@ registry.register(
store=kw.get("store")),
check_fn=check_memory_requirements,
emoji="🧠",
dynamic_schema_overrides=_build_memory_schema_overrides,
)

36
tools/registry.py
View File

@@ -327,31 +327,29 @@ def check_fn_cache_scope() -> Optional[str]:
return CHECK_FN_CACHE_BYPASS
def _run_check_fn_uncached(fn: Callable, *, unresolved_scope: bool = False) -> bool:
"""Run an availability check without cache/grace handling."""
try:
return bool(fn())
except Exception:
detail = " while profile cache scope was unresolved" if unresolved_scope else ""
logger.warning(
"check_fn %s raised%s; dependent tools will be unavailable this turn",
getattr(fn, "__qualname__", fn),
detail,
exc_info=True,
)
return False
def _check_fn_cached(fn: Callable) -> bool:
"""Return bool(fn()), TTL-cached across calls."""
now = time.monotonic()
if fn in _NO_CACHE_CHECK_FNS:
try:
return bool(fn())
except Exception:
logger.warning(
"check_fn %s raised; dependent tools will be unavailable this turn",
getattr(fn, "__qualname__", fn),
exc_info=True,
)
return False
return _run_check_fn_uncached(fn)
scope = check_fn_cache_scope()
if scope == CHECK_FN_CACHE_BYPASS:
try:
return bool(fn())
except Exception:
logger.warning(
"check_fn %s raised while profile cache scope was unresolved; "
"dependent tools will be unavailable this turn",
getattr(fn, "__qualname__", fn),
exc_info=True,
)
return False
return _run_check_fn_uncached(fn, unresolved_scope=True)
cache_key = (fn, scope)
with _check_fn_cache_lock:
_prune_check_fn_caches(now)

5
website/docs/user-guide/features/memory.md
View File

@@ -251,8 +251,9 @@ external provider tools too.
With only `memory_enabled: false` (user profile still on), the tool stays —
it backs the profile store — but the system prompt swaps the full memory
guidance for a narrower profile-only block, so the model is only instructed to
save user-profile facts and never steered at the disabled notes store.
guidance for a narrower profile-only block. The tool schema advertises only the
`user` target, and direct or staged writes to disabled `MEMORY.md` are rejected.
The inverse configuration advertises only `memory` and rejects `USER.md` writes.
## Controlling memory writes (`write_approval`)