test(approval): cover CLI EXEC_ASK leak and fix slash-worker Path mock
Regression tests for silent pending_approval when ask-mode leaks into interactive CLI, plus a Path-typed hermes_constants mock so the slash-worker profile_home test survives per-file isolation.
This commit is contained in:
113
tests/tools/test_cli_approval_exec_ask_leak.py
Normal file
113
tests/tools/test_cli_approval_exec_ask_leak.py
Normal file
@@ -0,0 +1,113 @@
|
||||
"""Regression: interactive CLI must not lose the Dangerous Command panel.
|
||||
|
||||
When ``HERMES_EXEC_ASK`` leaks into a classic CLI process (historically via
|
||||
``import gateway.run`` setting the flag at module import), the ask/gateway
|
||||
branch used to return ``pending_approval`` immediately with no notify
|
||||
listener and skip the CLI approval callback. Users saw tools "auto-block"
|
||||
with no Approve/Deny UI.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
import tools.approval as approval_module
|
||||
from tools.approval import check_all_command_guards
|
||||
from tools.terminal_tool import set_approval_callback
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_approval_env(monkeypatch):
|
||||
for key in (
|
||||
"HERMES_EXEC_ASK",
|
||||
"HERMES_GATEWAY_SESSION",
|
||||
"HERMES_SESSION_PLATFORM",
|
||||
"HERMES_CRON_SESSION",
|
||||
"HERMES_YOLO_MODE",
|
||||
):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
monkeypatch.setenv("HERMES_INTERACTIVE", "1")
|
||||
monkeypatch.setattr(approval_module, "_YOLO_MODE_FROZEN", False)
|
||||
monkeypatch.setattr(
|
||||
approval_module,
|
||||
"_get_approval_mode",
|
||||
lambda: "manual",
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"tools.tirith_security.check_command_security",
|
||||
lambda _command: {"action": "allow", "findings": [], "summary": ""},
|
||||
)
|
||||
approval_module._session_approved.clear()
|
||||
approval_module._permanent_approved.clear()
|
||||
approval_module._pending.clear()
|
||||
set_approval_callback(None)
|
||||
yield
|
||||
set_approval_callback(None)
|
||||
|
||||
|
||||
class TestCliApprovalSurvivesExecAskLeak:
|
||||
def test_cli_callback_used_when_exec_ask_set_without_notifier(self, monkeypatch):
|
||||
"""Ask-mode with a CLI callback must prompt locally, not pending_approval."""
|
||||
monkeypatch.setenv("HERMES_EXEC_ASK", "1")
|
||||
calls = []
|
||||
|
||||
def _cb(command, description, **kwargs):
|
||||
calls.append((command, description))
|
||||
return "once"
|
||||
|
||||
set_approval_callback(_cb)
|
||||
result = check_all_command_guards("rm -rf /tmp/testdir", "local")
|
||||
|
||||
assert calls, "CLI approval callback was never invoked"
|
||||
assert result.get("status") != "pending_approval"
|
||||
assert result.get("approval_pending") is not True
|
||||
assert result.get("approved") is True
|
||||
assert result.get("user_approved") is True
|
||||
|
||||
def test_pending_approval_still_used_without_cli_callback(self, monkeypatch):
|
||||
"""Headless ask-mode without a CLI callback keeps the pending fallback."""
|
||||
monkeypatch.setenv("HERMES_EXEC_ASK", "1")
|
||||
monkeypatch.delenv("HERMES_INTERACTIVE", raising=False)
|
||||
set_approval_callback(None)
|
||||
|
||||
result = check_all_command_guards("rm -rf /tmp/testdir", "local")
|
||||
|
||||
assert result.get("approved") is False
|
||||
assert result.get("status") == "pending_approval"
|
||||
assert result.get("approval_pending") is True
|
||||
|
||||
|
||||
class TestGatewayRunImportDoesNotSetExecAsk:
|
||||
def test_importing_gateway_run_does_not_set_exec_ask(self):
|
||||
"""Incidental imports must not poison CLI ask-mode process-wide."""
|
||||
script = r"""
|
||||
import os, sys
|
||||
os.environ.pop("HERMES_EXEC_ASK", None)
|
||||
sys.path.insert(0, %r)
|
||||
# Avoid starting the gateway; only import the module for _gateway_runner_ref
|
||||
# style side imports.
|
||||
import gateway.run # noqa: F401
|
||||
print("EXEC_ASK=" + repr(os.environ.get("HERMES_EXEC_ASK")))
|
||||
""" % (str(REPO_ROOT),)
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
cwd=str(REPO_ROOT),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env={
|
||||
**os.environ,
|
||||
"HERMES_HOME": str(REPO_ROOT / ".tmp-hermes-exec-ask-import"),
|
||||
},
|
||||
timeout=60,
|
||||
)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
assert "EXEC_ASK=None" in proc.stdout, proc.stdout + proc.stderr
|
||||
@@ -1,38 +1,35 @@
|
||||
"""Tests for TUI gateway slash_worker profile_home propagation (#40677)."""
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch, call
|
||||
|
||||
import pytest
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
||||
def test_slash_worker_accepts_profile_home():
|
||||
"""_SlashWorker.__init__ accepts profile_home parameter."""
|
||||
# hermes_state evaluates get_hermes_home() / "state.db" at import time, so
|
||||
# the mock must return a Path (a bare str raises TypeError under per-file
|
||||
# subprocess isolation).
|
||||
with patch.dict("sys.modules", {
|
||||
# get_hermes_home() returns a Path in production; hermes_state.py's
|
||||
# module-level DEFAULT_DB_PATH = get_hermes_home() / "state.db" does path
|
||||
# division, so a str mock makes the import raise TypeError (str / str).
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))),
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test")),
|
||||
),
|
||||
}):
|
||||
with patch("subprocess.Popen") as mock_popen:
|
||||
mock_popen.return_value.stdout = MagicMock()
|
||||
mock_popen.return_value.stderr = MagicMock()
|
||||
|
||||
|
||||
from tui_gateway.server import _SlashWorker
|
||||
|
||||
|
||||
# Test initialization with profile_home
|
||||
worker = _SlashWorker(
|
||||
session_key="test_key",
|
||||
model="test-model",
|
||||
profile_home="/home/luke/.hermes/profiles/work"
|
||||
)
|
||||
|
||||
|
||||
# Verify Popen was called
|
||||
assert mock_popen.called
|
||||
|
||||
|
||||
# Check that HERMES_HOME was set in the environment
|
||||
call_kwargs = mock_popen.call_args[1]
|
||||
assert "env" in call_kwargs
|
||||
|
||||
Reference in New Issue
Block a user