fix(profile): gate bg-review memory tool on memory_enabled (#54937 layer 2)
background_review hardcoded enabled_toolsets=["memory", "skills"] in the review fork's whitelist, so a skill-review fork on a profile with memory_enabled: false still granted the LLM the built-in MEMORY.md read/write tool — contaminating a profile that opted out of built-in memory. The flag was already in scope (review_agent._memory_enabled). Include "memory" only when _memory_enabled or _user_profile_enabled (USER.md also needs the tool). Layer 1 of #54937 (the path leak) is fixed by this PR's thread-context propagation: get_memory_dir() is already per-call on main, so once the bg-review thread inherits the profile override its writes land in the right profile (verified). This commit closes the remaining whitelist layer.
This commit is contained in:
parent
1f1d346ced
commit
437dcacbbf
2 changed files with 79 additions and 1 deletions
|
|
@ -725,10 +725,17 @@ def _run_review_in_thread(
|
|||
clear_thread_tool_whitelist,
|
||||
)
|
||||
|
||||
# Gate the built-in memory tool on the profile's memory_enabled flag.
|
||||
# Hardcoding ["memory", "skills"] granted the review LLM the MEMORY.md
|
||||
# read/write tool even when a profile set memory_enabled: false,
|
||||
# contaminating a memory-disabled profile (#54937 layer 2).
|
||||
review_toolsets = ["skills"]
|
||||
if review_agent._memory_enabled or review_agent._user_profile_enabled:
|
||||
review_toolsets.insert(0, "memory")
|
||||
review_whitelist = {
|
||||
t["function"]["name"]
|
||||
for t in get_tool_definitions(
|
||||
enabled_toolsets=["memory", "skills"],
|
||||
enabled_toolsets=review_toolsets,
|
||||
quiet_mode=True,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -156,3 +156,74 @@ def test_background_review_agent_tools_are_limited():
|
|||
assert "delegate_task" not in expected_tools
|
||||
assert "web_search" not in expected_tools
|
||||
assert "execute_code" not in expected_tools
|
||||
|
||||
|
||||
def test_background_review_excludes_memory_when_disabled():
|
||||
"""A memory-disabled profile must NOT get the memory tool in the review fork.
|
||||
|
||||
Regression for #54937 layer 2: the whitelist hardcoded ["memory", "skills"],
|
||||
so a skill-review fork on a profile with memory_enabled=false still granted
|
||||
the LLM the MEMORY.md read/write tool, contaminating a profile that opted
|
||||
out of built-in memory. The whitelist must gate "memory" on the flag.
|
||||
"""
|
||||
import run_agent
|
||||
from hermes_cli import plugins as _plugins
|
||||
|
||||
captured = {}
|
||||
|
||||
def _capture_whitelist(whitelist, deny_msg_fmt=None):
|
||||
captured["whitelist"] = set(whitelist)
|
||||
raise RuntimeError("stop after capturing whitelist")
|
||||
|
||||
agent = _make_agent_stub(run_agent.AIAgent)
|
||||
agent._memory_enabled = False
|
||||
agent._user_profile_enabled = False
|
||||
|
||||
def _no_init(self, *args, **kwargs):
|
||||
return None
|
||||
|
||||
with patch.object(run_agent.AIAgent, "__init__", _no_init), \
|
||||
patch.object(_plugins, "set_thread_tool_whitelist", _capture_whitelist), \
|
||||
patch("threading.Thread", _SyncThread):
|
||||
agent._spawn_background_review(
|
||||
messages_snapshot=[],
|
||||
review_memory=False,
|
||||
review_skills=True,
|
||||
)
|
||||
|
||||
whitelist = captured["whitelist"]
|
||||
# Skill tools still allowed...
|
||||
assert "skill_manage" in whitelist
|
||||
assert "skill_view" in whitelist
|
||||
# ...but the built-in memory tool must be gated out.
|
||||
assert "memory" not in whitelist
|
||||
|
||||
|
||||
def test_background_review_includes_memory_when_user_profile_enabled():
|
||||
"""user_profile_enabled alone (USER.md) still needs the memory tool."""
|
||||
import run_agent
|
||||
from hermes_cli import plugins as _plugins
|
||||
|
||||
captured = {}
|
||||
|
||||
def _capture_whitelist(whitelist, deny_msg_fmt=None):
|
||||
captured["whitelist"] = set(whitelist)
|
||||
raise RuntimeError("stop after capturing whitelist")
|
||||
|
||||
agent = _make_agent_stub(run_agent.AIAgent)
|
||||
agent._memory_enabled = False
|
||||
agent._user_profile_enabled = True
|
||||
|
||||
def _no_init(self, *args, **kwargs):
|
||||
return None
|
||||
|
||||
with patch.object(run_agent.AIAgent, "__init__", _no_init), \
|
||||
patch.object(_plugins, "set_thread_tool_whitelist", _capture_whitelist), \
|
||||
patch("threading.Thread", _SyncThread):
|
||||
agent._spawn_background_review(
|
||||
messages_snapshot=[],
|
||||
review_memory=True,
|
||||
review_skills=False,
|
||||
)
|
||||
|
||||
assert "memory" in captured["whitelist"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue