From 9863a07af67a180228a8c6595e0e7ab2c1c1fdb7 Mon Sep 17 00:00:00 2001 From: Schrotti77 Date: Mon, 25 May 2026 01:09:10 -0700 Subject: [PATCH] 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> --- cron/scheduler.py | 25 ++++++++++++++++++++++++- tests/cron/test_scheduler.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index 6b511d38b77..37c250b67d0 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -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 diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index 62bc6b688a0..94587fccedd 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -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``