mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-10 03:22:05 +00:00
fix(approval): cron jobs must not be treated as gateway context
The new _is_gateway_approval_context() widened the gateway classification to any call with HERMES_SESSION_PLATFORM bound via contextvars. But cron/scheduler.py binds that same contextvar for delivery routing on cron jobs that originate from a gateway platform (telegram/discord/etc.), so those jobs were getting routed through submit_pending with no listener — blocking indefinitely instead of honoring approvals.cron_mode. Short-circuit on HERMES_CRON_SESSION before any gateway check. Cron is always governed by cron_mode config, regardless of where the job was scheduled from. Adds regression coverage in TestCronWithGatewayOrigin and records the contributor email mapping for scripts/release.py.
This commit is contained in:
parent
526c0e018a
commit
839cdd1b05
3 changed files with 84 additions and 0 deletions
|
|
@ -905,6 +905,7 @@ AUTHOR_MAP = {
|
|||
"montbra@gmail.com": "Montbra", # PR #20897 salvage of #16189 (TUI voice PTT)
|
||||
"promptsiren@gmail.com": "firefly", # PR #18123 salvage of #16660 (ContextVars)
|
||||
"wtyopenclaw@gmail.com": "WuTianyi123", # PR #20275 salvage of #13723 (feishu markdown)
|
||||
"zhicheng.han@mathematik.uni-goettingen.de": "hanzckernel", # PR #20311 (api-server approval events)
|
||||
# pander: empty email, salvaged via PR #19665 from #16126 by @ms-alan
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -256,3 +256,77 @@ class TestCronModeInteractions:
|
|||
|
||||
result = check_dangerous_command("rm -rf /tmp/stuff", "local")
|
||||
assert result["approved"]
|
||||
|
||||
|
||||
class TestCronWithGatewayOrigin:
|
||||
"""Cron jobs originating from a gateway platform must NOT be treated as gateway.
|
||||
|
||||
cron/scheduler.py binds HERMES_SESSION_PLATFORM via contextvars for
|
||||
delivery routing (so cron output lands back in the origin chat). The
|
||||
API-server approvals work (PR #20311) made check_dangerous_command treat
|
||||
any contextvar-bound platform as a gateway session. That would route
|
||||
cron-from-telegram/discord/etc. through submit_pending with no listener,
|
||||
hanging the job instead of respecting approvals.cron_mode.
|
||||
"""
|
||||
|
||||
def test_cron_with_telegram_origin_uses_cron_mode_not_gateway(self, monkeypatch):
|
||||
"""Cron + contextvar platform=telegram + cron_mode=deny → BLOCKED, not pending."""
|
||||
monkeypatch.setenv("HERMES_CRON_SESSION", "1")
|
||||
monkeypatch.delenv("HERMES_INTERACTIVE", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False)
|
||||
monkeypatch.delenv("HERMES_YOLO_MODE", raising=False)
|
||||
monkeypatch.delenv("HERMES_EXEC_ASK", raising=False)
|
||||
|
||||
from gateway.session_context import set_session_vars, clear_session_vars
|
||||
tokens = set_session_vars(platform="telegram", chat_id="123")
|
||||
try:
|
||||
from unittest.mock import patch as mock_patch
|
||||
with mock_patch("tools.approval._get_cron_approval_mode", return_value="deny"):
|
||||
result = check_dangerous_command("rm -rf /tmp/stuff", "local")
|
||||
# Cron-mode path: BLOCKED message, NOT pending/approval_required.
|
||||
assert not result["approved"]
|
||||
assert "BLOCKED" in result["message"]
|
||||
assert "cron_mode" in result["message"]
|
||||
assert result.get("status") != "approval_required"
|
||||
finally:
|
||||
clear_session_vars(tokens)
|
||||
|
||||
def test_cron_with_telegram_origin_approve_mode_allows(self, monkeypatch):
|
||||
"""Cron + contextvar platform=telegram + cron_mode=approve → allowed via cron path."""
|
||||
monkeypatch.setenv("HERMES_CRON_SESSION", "1")
|
||||
monkeypatch.delenv("HERMES_INTERACTIVE", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False)
|
||||
monkeypatch.delenv("HERMES_YOLO_MODE", raising=False)
|
||||
monkeypatch.delenv("HERMES_EXEC_ASK", raising=False)
|
||||
|
||||
from gateway.session_context import set_session_vars, clear_session_vars
|
||||
tokens = set_session_vars(platform="discord", chat_id="456")
|
||||
try:
|
||||
from unittest.mock import patch as mock_patch
|
||||
with mock_patch("tools.approval._get_cron_approval_mode", return_value="approve"):
|
||||
result = check_dangerous_command("rm -rf /tmp/stuff", "local")
|
||||
assert result["approved"]
|
||||
# Should NOT be a gateway-approval response.
|
||||
assert result.get("status") != "approval_required"
|
||||
finally:
|
||||
clear_session_vars(tokens)
|
||||
|
||||
def test_cron_with_telegram_origin_combined_guard_uses_cron_mode(self, monkeypatch):
|
||||
"""check_all_command_guards must also honor cron_mode over gateway classification."""
|
||||
monkeypatch.setenv("HERMES_CRON_SESSION", "1")
|
||||
monkeypatch.delenv("HERMES_INTERACTIVE", raising=False)
|
||||
monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False)
|
||||
monkeypatch.delenv("HERMES_YOLO_MODE", raising=False)
|
||||
monkeypatch.delenv("HERMES_EXEC_ASK", raising=False)
|
||||
|
||||
from gateway.session_context import set_session_vars, clear_session_vars
|
||||
tokens = set_session_vars(platform="telegram", chat_id="789")
|
||||
try:
|
||||
from unittest.mock import patch as mock_patch
|
||||
with mock_patch("tools.approval._get_cron_approval_mode", return_value="deny"):
|
||||
result = check_all_command_guards("rm -rf /tmp/stuff", "local")
|
||||
assert not result["approved"]
|
||||
assert "BLOCKED" in result["message"]
|
||||
assert result.get("status") != "approval_required"
|
||||
finally:
|
||||
clear_session_vars(tokens)
|
||||
|
|
|
|||
|
|
@ -100,7 +100,16 @@ def _is_gateway_approval_context() -> bool:
|
|||
Legacy gateway integrations set HERMES_GATEWAY_SESSION in process env.
|
||||
Newer concurrent gateway paths bind HERMES_SESSION_PLATFORM via
|
||||
contextvars so approval mode does not depend on process-global flags.
|
||||
|
||||
Cron jobs are NEVER gateway-approval contexts even when they originate
|
||||
from a gateway platform (cron binds HERMES_SESSION_PLATFORM via
|
||||
contextvars for delivery routing). Cron approvals are governed by
|
||||
``approvals.cron_mode`` config, not interactive resolve — letting cron
|
||||
fall through to the gateway branch would submit a pending approval
|
||||
with no listener and block the job indefinitely.
|
||||
"""
|
||||
if os.getenv("HERMES_CRON_SESSION"):
|
||||
return False
|
||||
if os.getenv("HERMES_GATEWAY_SESSION"):
|
||||
return True
|
||||
return bool(_get_session_platform())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue