mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-03 07:21:54 +00:00
fix(cron): layer agent.disabled_toolsets onto cron baseline (#25752)
The bug: cron/scheduler.py:_resolve_cron_enabled_toolsets returns an LLM-supplied per-job enabled_toolsets verbatim. The disabled_toolsets passed to AIAgent was a hardcoded [cronjob, messaging, clarify] that ignored agent.disabled_toolsets from config.yaml. An LLM could call cronjob(action='add', enabled_toolsets=['terminal','file'], prompt='...') and the cron-spawned agent would receive terminal+file even when the operator had globally disabled them. Fix: new _resolve_cron_disabled_toolsets() helper that ALWAYS layers agent.disabled_toolsets on top of the cron baseline. AIAgent's disabled_toolsets takes precedence over enabled_toolsets, so this stops the bypass regardless of what the per-job override contains. This is the disabled-side fix. Three concurrent PRs (#25842, #25815, #25780) proposed intersection-side variants on _resolve_cron_enabled_toolsets; this fix is more robust because it stops the leak at the precedence boundary AIAgent itself enforces, not at a layer above. Regression test reproduces the issue's PoC exactly: config.yaml has agent.disabled_toolsets=[terminal,file]; cron job has enabled_toolsets=[web,terminal,file]; assertion: AIAgent receives disabled_toolsets containing terminal AND file. Salvaged from PR #25786 by @Schrotti77. Simplified the implementation: dropped a 23-line _normalize_toolset_list() helper (handled str/tuple/ set/garbage input shapes) in favor of the existing convention (agent_cfg.get('disabled_toolsets') or []) used elsewhere in the codebase. YAML always parses these as lists; the elaborate normalizer was theatre for shapes we never produce. Closes #25752 Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
This commit is contained in:
parent
a6b0414ea0
commit
9863a07af6
2 changed files with 60 additions and 1 deletions
|
|
@ -57,6 +57,29 @@ class CronPromptInjectionBlocked(Exception):
|
|||
"""
|
||||
|
||||
|
||||
def _resolve_cron_disabled_toolsets(cfg: dict) -> list[str]:
|
||||
"""Toolsets a cron-spawned agent must never receive.
|
||||
|
||||
Three protected toolsets are always disabled in cron context:
|
||||
- ``cronjob`` — would let a cron-spawned agent schedule more cron jobs
|
||||
- ``messaging`` — interactive, needs a live gateway session
|
||||
- ``clarify`` — interactive, blocks waiting for user input
|
||||
|
||||
User-level ``agent.disabled_toolsets`` from config.yaml is layered on top
|
||||
so per-job ``enabled_toolsets`` cannot bypass policy that applies to
|
||||
ordinary agent runs (#25752 — LLM-supplied enabled_toolsets was widening
|
||||
past config.yaml's denylist).
|
||||
"""
|
||||
disabled = ["cronjob", "messaging", "clarify"]
|
||||
agent_cfg = (cfg or {}).get("agent") or {}
|
||||
user_disabled = agent_cfg.get("disabled_toolsets") or []
|
||||
for name in user_disabled:
|
||||
name = str(name).strip()
|
||||
if name and name not in disabled:
|
||||
disabled.append(name)
|
||||
return disabled
|
||||
|
||||
|
||||
def _resolve_cron_enabled_toolsets(job: dict, cfg: dict) -> list[str] | None:
|
||||
"""Resolve the toolset list for a cron job.
|
||||
|
||||
|
|
@ -1574,7 +1597,7 @@ def _run_job_impl(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
|||
provider_sort=pr.get("sort"),
|
||||
openrouter_min_coding_score=(_cfg.get("openrouter") or {}).get("min_coding_score"),
|
||||
enabled_toolsets=_resolve_cron_enabled_toolsets(job, _cfg),
|
||||
disabled_toolsets=["cronjob", "messaging", "clarify"],
|
||||
disabled_toolsets=_resolve_cron_disabled_toolsets(_cfg),
|
||||
quiet_mode=True,
|
||||
# Cron jobs should always inherit the user's SOUL.md identity from
|
||||
# HERMES_HOME. When a workdir is configured, also inject project
|
||||
|
|
|
|||
|
|
@ -1021,6 +1021,42 @@ class TestRunJobSessionPersistence:
|
|||
kwargs = mock_agent_cls.call_args.kwargs
|
||||
assert kwargs["enabled_toolsets"] == ["web", "terminal", "file"]
|
||||
|
||||
def test_run_job_disabled_toolsets_layer_user_config_on_baseline(self, tmp_path):
|
||||
"""agent.disabled_toolsets must be honoured in cron — issue #25752.
|
||||
|
||||
The bug: per-job enabled_toolsets was returned verbatim, letting an
|
||||
LLM-supplied cronjob() call re-enable tools the operator had globally
|
||||
disabled. The fix: ALWAYS include agent.disabled_toolsets in the
|
||||
disabled_toolsets passed to AIAgent, on top of the cron baseline
|
||||
(cronjob/messaging/clarify). AIAgent's disabled_toolsets takes
|
||||
precedence over enabled_toolsets, so this stops the bypass.
|
||||
"""
|
||||
(tmp_path / "config.yaml").write_text(
|
||||
"agent:\n"
|
||||
" disabled_toolsets:\n"
|
||||
" - terminal\n"
|
||||
" - file\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
job = {
|
||||
"id": "policy-job",
|
||||
"name": "test",
|
||||
"prompt": "hello",
|
||||
"enabled_toolsets": ["web", "terminal", "file"],
|
||||
}
|
||||
fake_db, patches = self._make_run_job_patches(tmp_path)
|
||||
with patches[0], patches[1], patches[2], patches[3], patches[4], \
|
||||
patch("run_agent.AIAgent") as mock_agent_cls:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run_conversation.return_value = {"final_response": "ok"}
|
||||
mock_agent_cls.return_value = mock_agent
|
||||
run_job(job)
|
||||
|
||||
kwargs = mock_agent_cls.call_args.kwargs
|
||||
assert set(kwargs["disabled_toolsets"]) >= {
|
||||
"cronjob", "messaging", "clarify", "terminal", "file",
|
||||
}
|
||||
|
||||
def test_run_job_enabled_toolsets_resolves_from_platform_config_when_not_set(self, tmp_path):
|
||||
"""When a job has no explicit enabled_toolsets, the scheduler now
|
||||
resolves them from ``hermes tools`` platform config for ``cron``
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue