fix(review): harden plugin gateway injection
This commit is contained in:
@@ -383,7 +383,12 @@ class GatewayAuthorizationMixin:
|
||||
return per_profile[profile]
|
||||
return getattr(self, "pairing_store", None)
|
||||
|
||||
def _is_user_authorized(self, source: SessionSource) -> bool:
|
||||
def _is_user_authorized(
|
||||
self,
|
||||
source: SessionSource,
|
||||
*,
|
||||
allow_adapter_delegation: bool = True,
|
||||
) -> bool:
|
||||
"""
|
||||
Check if a user is authorized to use the bot.
|
||||
|
||||
@@ -432,9 +437,12 @@ class GatewayAuthorizationMixin:
|
||||
# SessionSource, and an explicit identity check refuses to authorize a
|
||||
# non-bool stand-in (e.g. a MagicMock attribute auto-vivifies truthy in
|
||||
# tests) — defensive against accidental fail-open.
|
||||
if source.delivered_via_upstream_relay is True or self._adapter_authorization_is_upstream(
|
||||
source.platform,
|
||||
profile=adapter_profile,
|
||||
if allow_adapter_delegation and (
|
||||
source.delivered_via_upstream_relay is True
|
||||
or self._adapter_authorization_is_upstream(
|
||||
source.platform,
|
||||
profile=adapter_profile,
|
||||
)
|
||||
):
|
||||
return True
|
||||
|
||||
@@ -630,7 +638,7 @@ class GatewayAuthorizationMixin:
|
||||
# flag (checked above), and the pairing flow remain the explicit
|
||||
# opt-ins to broader access. (#34515 follow-up: trusting "open" was a
|
||||
# fail-open.)
|
||||
if self._adapter_enforces_own_access_policy(
|
||||
if allow_adapter_delegation and self._adapter_enforces_own_access_policy(
|
||||
source.platform,
|
||||
profile=adapter_profile,
|
||||
):
|
||||
|
||||
@@ -2375,10 +2375,16 @@ class MessageEvent:
|
||||
|
||||
# Timestamps
|
||||
timestamp: datetime = field(default_factory=datetime.now)
|
||||
|
||||
# Whether this event may resolve gateway commands or pending control
|
||||
# prompts. Kept last to preserve positional construction compatibility.
|
||||
# Proactive plugin events set this to False so untrusted payload text
|
||||
# remains conversational input.
|
||||
allow_gateway_control: bool = True
|
||||
|
||||
def is_command(self) -> bool:
|
||||
"""Check if this is a command message (e.g., /new, /reset)."""
|
||||
return (self.text or "").lstrip().startswith("/")
|
||||
return self.allow_gateway_control and (self.text or "").lstrip().startswith("/")
|
||||
|
||||
def get_command(self) -> Optional[str]:
|
||||
"""Extract command name if this is a command message."""
|
||||
@@ -5958,7 +5964,8 @@ class BasePlatformAdapter(ABC):
|
||||
if not self._message_handler:
|
||||
return
|
||||
|
||||
coerce_plaintext_gateway_command(event)
|
||||
if event.allow_gateway_control:
|
||||
coerce_plaintext_gateway_command(event)
|
||||
|
||||
# Telegram topic recovery only applies to private DM topic lanes. Do
|
||||
# not submit a no-op check for group/forum/channel traffic to the
|
||||
@@ -5976,6 +5983,16 @@ class BasePlatformAdapter(ABC):
|
||||
group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True),
|
||||
thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False),
|
||||
)
|
||||
expected_session_key = str(
|
||||
(event.metadata or {}).get("gateway_session_key") or ""
|
||||
).strip()
|
||||
if expected_session_key and session_key != expected_session_key:
|
||||
logger.warning(
|
||||
"Dropping internally routed event: expected session=%s derived=%s",
|
||||
expected_session_key,
|
||||
session_key,
|
||||
)
|
||||
return
|
||||
|
||||
# On-entry self-heal: if the adapter still has an _active_sessions
|
||||
# entry for this key but the owner task has already exited (done or
|
||||
@@ -6061,7 +6078,7 @@ class BasePlatformAdapter(ABC):
|
||||
# Same shape as the /approve deadlock fix (PR #4926) — both
|
||||
# cases are "agent thread blocked on Event.wait, message must
|
||||
# reach the resolver before being treated as a new turn."
|
||||
if not cmd:
|
||||
if not cmd and event.allow_gateway_control:
|
||||
try:
|
||||
from tools import clarify_gateway as _clarify_mod
|
||||
_has_text_clarify = (
|
||||
|
||||
123
gateway/run.py
123
gateway/run.py
@@ -9048,7 +9048,24 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
# semantics); everything else appends to the overflow tail.
|
||||
pending_slot = getattr(adapter, "_pending_messages", None)
|
||||
existing = pending_slot.get(session_key) if isinstance(pending_slot, dict) else None
|
||||
if existing is not None and (
|
||||
security_metadata_keys = (
|
||||
"hermes_plugin_id",
|
||||
"hermes_plugin_injection",
|
||||
"gateway_session_key",
|
||||
"gateway_session_id",
|
||||
"gateway_session_strict",
|
||||
)
|
||||
same_security_context = existing is not None and (
|
||||
getattr(existing, "internal", False) == getattr(event, "internal", False)
|
||||
and getattr(existing, "allow_gateway_control", True)
|
||||
== getattr(event, "allow_gateway_control", True)
|
||||
and all(
|
||||
(getattr(existing, "metadata", None) or {}).get(key)
|
||||
== (getattr(event, "metadata", None) or {}).get(key)
|
||||
for key in security_metadata_keys
|
||||
)
|
||||
)
|
||||
if same_security_context and (
|
||||
getattr(existing, "message_type", None) == MessageType.PHOTO
|
||||
or event.message_type == MessageType.PHOTO
|
||||
or bool(getattr(existing, "media_urls", None))
|
||||
@@ -9180,7 +9197,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
# we deliver it ourselves (mirroring the draining-case send above).
|
||||
try:
|
||||
from tools.approval import has_blocking_approval
|
||||
if has_blocking_approval(session_key):
|
||||
if event.allow_gateway_control and has_blocking_approval(session_key):
|
||||
_raw_text = (event.text or "").strip().lower()
|
||||
_approve_words = {"approve", "yes", "ok", "okay", "confirm", "y", "👍"}
|
||||
_deny_words = {"deny", "no", "reject", "cancel", "n", "👎"}
|
||||
@@ -9245,9 +9262,12 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
# (the default busy_text_mode) aborts the active turn AND sends a "⚡
|
||||
# Interrupting current task" ack — exactly the opposite of the design
|
||||
# invariant that a completion surfaces as a NEW turn only when idle and
|
||||
# never splices into a running turn. Fall through to the base adapter,
|
||||
# which queues internal events silently (no interrupt, no ack) so they
|
||||
# cascade after the current turn finishes.
|
||||
# never splices into a running turn. Plugin events carry untrusted
|
||||
# payload text, so queue those through the gateway FIFO to keep their
|
||||
# security metadata separate from pending user input.
|
||||
if getattr(event, "internal", False) and not event.allow_gateway_control:
|
||||
self._queue_or_replace_pending_event(session_key, event)
|
||||
return True
|
||||
if getattr(event, "internal", False):
|
||||
return False
|
||||
|
||||
@@ -15226,8 +15246,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
# Otherwise control/session commands like /new or /help get silently
|
||||
# consumed as update answers instead of being dispatched normally.
|
||||
_quick_key = self._session_key_for_source(source)
|
||||
allow_gateway_control = event.allow_gateway_control
|
||||
_up_state = self._peek_session_state(_quick_key)
|
||||
if _up_state is not None and _up_state.persistent.update_prompt_pending:
|
||||
if (
|
||||
allow_gateway_control
|
||||
and _up_state is not None
|
||||
and _up_state.persistent.update_prompt_pending
|
||||
):
|
||||
raw = (event.text or "").strip()
|
||||
# Accept /approve and /deny as shorthand for yes/no
|
||||
cmd = event.get_command()
|
||||
@@ -15306,7 +15331,11 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
)
|
||||
except Exception:
|
||||
_pending_clarify = None
|
||||
if _pending_clarify is not None and _clarify_mod is not None:
|
||||
if (
|
||||
allow_gateway_control
|
||||
and _pending_clarify is not None
|
||||
and _clarify_mod is not None
|
||||
):
|
||||
_clarify_has_audio = bool(self._pending_event_audio_paths(event))
|
||||
_raw_clarify_reply = await self._prepare_clarify_reply_text(event)
|
||||
if _clarify_has_audio and not _raw_clarify_reply:
|
||||
@@ -15368,7 +15397,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
_tool_approval_live = has_blocking_approval(_quick_key)
|
||||
except Exception:
|
||||
_tool_approval_live = False
|
||||
if _pending_confirm and not _tool_approval_live:
|
||||
if allow_gateway_control and _pending_confirm and not _tool_approval_live:
|
||||
_raw_reply = (event.text or "").strip()
|
||||
# Accept bang-prefixed replies (`!always`, `!cancel`) verbatim.
|
||||
# Slack/Matrix instruction text shows the `!` prefix (typed `/`
|
||||
@@ -17153,11 +17182,38 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
plugin_id: str,
|
||||
) -> bool:
|
||||
"""Route a plugin-triggered turn through the session's live adapter."""
|
||||
if not getattr(self, "_running", False) or getattr(self, "_draining", False):
|
||||
return False
|
||||
|
||||
entry = await self.async_session_store.lookup_by_session_key(session_key)
|
||||
if entry is None or entry.origin is None:
|
||||
return False
|
||||
if not getattr(self, "_running", False) or getattr(self, "_draining", False):
|
||||
return False
|
||||
|
||||
source = dataclasses.replace(entry.origin)
|
||||
try:
|
||||
if not self._is_user_authorized(
|
||||
source,
|
||||
allow_adapter_delegation=False,
|
||||
):
|
||||
logger.warning(
|
||||
"Plugin message injection denied by current gateway authorization: "
|
||||
"plugin=%s session=%s",
|
||||
plugin_id,
|
||||
session_key,
|
||||
)
|
||||
return False
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Plugin message injection authorization check failed: "
|
||||
"plugin=%s session=%s",
|
||||
plugin_id,
|
||||
session_key,
|
||||
exc_info=True,
|
||||
)
|
||||
return False
|
||||
|
||||
adapter = self._adapter_for_source(source)
|
||||
if adapter is None:
|
||||
return False
|
||||
@@ -17167,12 +17223,22 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
message_type=MessageType.TEXT,
|
||||
source=source,
|
||||
internal=True,
|
||||
allow_gateway_control=False,
|
||||
metadata={
|
||||
"hermes_plugin_id": plugin_id,
|
||||
"hermes_plugin_injection": True,
|
||||
"gateway_session_key": session_key,
|
||||
"gateway_session_id": entry.session_id,
|
||||
"gateway_session_strict": True,
|
||||
},
|
||||
)
|
||||
await adapter.handle_message(event)
|
||||
logger.info(
|
||||
"Plugin message injection dispatched: plugin=%s session=%s session_id=%s",
|
||||
plugin_id,
|
||||
session_key,
|
||||
entry.session_id,
|
||||
)
|
||||
return True
|
||||
|
||||
def _get_cached_session_source(self, session_key: str):
|
||||
@@ -17218,12 +17284,43 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
session_key = session_entry.session_key
|
||||
pinned_session_id = str(
|
||||
(getattr(event, "metadata", None) or {}).get("gateway_session_id") or ""
|
||||
event_metadata = getattr(event, "metadata", None) or {}
|
||||
expected_session_key = str(
|
||||
event_metadata.get("gateway_session_key") or ""
|
||||
).strip()
|
||||
if pinned_session_id:
|
||||
if expected_session_key:
|
||||
derived_session_key = self._session_key_for_source(source)
|
||||
if derived_session_key != expected_session_key:
|
||||
logger.warning(
|
||||
"Dropping internally routed event after route recovery: "
|
||||
"expected session=%s derived=%s",
|
||||
expected_session_key,
|
||||
derived_session_key,
|
||||
)
|
||||
return
|
||||
|
||||
strict_session = bool(event_metadata.get("gateway_session_strict"))
|
||||
pinned_session_id = str(event_metadata.get("gateway_session_id") or "").strip()
|
||||
if strict_session:
|
||||
session_entry = await self.async_session_store.lookup_by_session_key(
|
||||
expected_session_key
|
||||
)
|
||||
if (
|
||||
session_entry is None
|
||||
or not pinned_session_id
|
||||
or session_entry.session_id != pinned_session_id
|
||||
):
|
||||
logger.warning(
|
||||
"Dropping internally routed event: expected session id=%s is no "
|
||||
"longer current for key=%s",
|
||||
pinned_session_id or "missing",
|
||||
expected_session_key or "missing",
|
||||
)
|
||||
return
|
||||
else:
|
||||
session_entry = await self.async_session_store.get_or_create_session(source)
|
||||
session_key = session_entry.session_key
|
||||
if not strict_session and pinned_session_id:
|
||||
resolved_entry = await self._resolve_async_delegation_session(
|
||||
session_entry,
|
||||
pinned_session_id,
|
||||
|
||||
@@ -149,6 +149,96 @@ def test_own_policy_allowlist_authorized_without_env_allowlist(monkeypatch, plat
|
||||
assert runner._is_user_authorized(_source(platform)) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_stored_route_cannot_reuse_prior_adapter_authorization(monkeypatch, platform):
|
||||
_clear_auth_env(monkeypatch)
|
||||
config = GatewayConfig(
|
||||
platforms={
|
||||
platform: PlatformConfig(
|
||||
enabled=True,
|
||||
extra={"dm_policy": "allowlist"},
|
||||
)
|
||||
}
|
||||
)
|
||||
runner, _adapter = _make_runner(platform, config, enforces=True)
|
||||
|
||||
assert runner._is_user_authorized(
|
||||
_source(platform),
|
||||
allow_adapter_delegation=False,
|
||||
) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_own_policy_open_dm_authorized_with_gateway_allow_all(monkeypatch, platform):
|
||||
"""Explicit ``GATEWAY_ALLOW_ALL_USERS`` unlocks ``dm_policy: open``."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
|
||||
config = GatewayConfig(
|
||||
platforms={platform: PlatformConfig(enabled=True, extra={"dm_policy": "open"})}
|
||||
)
|
||||
runner, _adapter = _make_runner(platform, config, enforces=True)
|
||||
|
||||
assert runner._is_user_authorized(_source(platform)) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_own_policy_open_dm_not_authorized_without_allowlist(monkeypatch, platform):
|
||||
"""``dm_policy: open`` forwards everyone → NOT authorization (SECURITY.md §2.6).
|
||||
|
||||
With no env allowlist and no per-platform allow-all flag, an own-policy
|
||||
adapter running ``open`` (the default) must NOT fail open: the gateway falls
|
||||
through to default-deny so the whole external network can't reach the agent.
|
||||
"""
|
||||
_clear_auth_env(monkeypatch)
|
||||
config = GatewayConfig(
|
||||
platforms={platform: PlatformConfig(enabled=True, extra={"dm_policy": "open"})}
|
||||
)
|
||||
runner, _adapter = _make_runner(platform, config, enforces=True)
|
||||
|
||||
assert runner._is_user_authorized(_source(platform)) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_own_policy_default_open_dm_is_fail_closed(monkeypatch, platform):
|
||||
"""The adapters' *default* ``open`` policy (no config at all) fails closed.
|
||||
|
||||
Operators who enable an own-policy adapter with only credentials get
|
||||
``dm_policy = "open"`` resolved on the live adapter. Simulate that resolved
|
||||
state (empty config.extra, adapter ``_dm_policy = "open"``) and confirm the
|
||||
gateway denies — the do-nothing default must not be open to the world.
|
||||
"""
|
||||
_clear_auth_env(monkeypatch)
|
||||
config = GatewayConfig(platforms={platform: PlatformConfig(enabled=True, extra={})})
|
||||
runner, adapter = _make_runner(platform, config, enforces=True)
|
||||
adapter._dm_policy = "open" # as the live adapter resolves the default
|
||||
|
||||
assert runner._is_user_authorized(_source(platform)) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_own_policy_allowlist_authorized_for_group_chat(monkeypatch, platform):
|
||||
"""A config-only ``group_policy: allowlist`` is trusted for group traffic."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
config = GatewayConfig(
|
||||
platforms={platform: PlatformConfig(enabled=True, extra={"group_policy": "allowlist"})}
|
||||
)
|
||||
runner, _adapter = _make_runner(platform, config, enforces=True)
|
||||
|
||||
assert runner._is_user_authorized(_source(platform, chat_type="group")) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", _OWN_POLICY_PLATFORMS)
|
||||
def test_own_policy_open_group_not_authorized_without_allowlist(monkeypatch, platform):
|
||||
"""``group_policy: open`` is the same fail-open class as DM open → deny."""
|
||||
_clear_auth_env(monkeypatch)
|
||||
config = GatewayConfig(
|
||||
platforms={platform: PlatformConfig(enabled=True, extra={"group_policy": "open"})}
|
||||
)
|
||||
runner, _adapter = _make_runner(platform, config, enforces=True)
|
||||
|
||||
assert runner._is_user_authorized(_source(platform, chat_type="group")) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"module_path, class_name, dm_helper",
|
||||
[
|
||||
|
||||
@@ -7,11 +7,18 @@ from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from gateway.config import Platform
|
||||
from gateway.config import GatewayConfig, Platform
|
||||
from gateway.platforms.base import (
|
||||
BasePlatformAdapter,
|
||||
MessageEvent,
|
||||
MessageType,
|
||||
PlatformConfig,
|
||||
)
|
||||
from gateway.run import GatewayRunner
|
||||
from gateway.session import SessionEntry, SessionSource
|
||||
from hermes_cli.plugins import PluginManager
|
||||
from gateway.session import SessionEntry, SessionSource, SessionStore, build_session_key
|
||||
from hermes_cli.plugins import PluginContext, PluginManager, PluginManifest
|
||||
|
||||
|
||||
def _entry(*, origin=True) -> SessionEntry:
|
||||
@@ -39,16 +46,108 @@ def _runner(entry: SessionEntry | None, adapter=None) -> GatewayRunner:
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.session_store = SimpleNamespace()
|
||||
runner._async_session_store = SimpleNamespace(
|
||||
_store=runner.session_store,
|
||||
lookup_by_session_key=AsyncMock(return_value=entry)
|
||||
_store=runner.session_store, lookup_by_session_key=AsyncMock(return_value=entry)
|
||||
)
|
||||
runner.adapters = {Platform.TELEGRAM: adapter} if adapter else {}
|
||||
runner._profile_adapters = {}
|
||||
runner._running = True
|
||||
runner._draining = False
|
||||
runner._background_tasks = set()
|
||||
runner._is_user_authorized = MagicMock(return_value=True)
|
||||
return runner
|
||||
|
||||
|
||||
class _RoutingAdapter(BasePlatformAdapter):
|
||||
def __init__(self):
|
||||
super().__init__(PlatformConfig(enabled=True, token="test"), Platform.TELEGRAM)
|
||||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
return True
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
self._mark_disconnected()
|
||||
|
||||
async def send(self, chat_id, content, reply_to=None, metadata=None):
|
||||
raise AssertionError("network send is not expected")
|
||||
|
||||
async def get_chat_info(self, chat_id):
|
||||
return {"id": chat_id, "type": "dm"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plugin_context_routes_through_live_gateway_to_existing_session(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
):
|
||||
hermes_home = tmp_path / "hermes"
|
||||
hermes_home.mkdir()
|
||||
(hermes_home / "config.yaml").write_text(
|
||||
yaml.safe_dump({
|
||||
"plugins": {"entries": {"notify-plugin": {"allow_gateway_injection": True}}}
|
||||
})
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
store = SessionStore(sessions_dir=tmp_path / "sessions", config=GatewayConfig())
|
||||
source = _entry().origin
|
||||
entry = store.get_or_create_session(source)
|
||||
adapter = _RoutingAdapter()
|
||||
adapter.set_message_handler(AsyncMock())
|
||||
adapter._active_sessions[entry.session_key] = asyncio.Event()
|
||||
pending_user_event = MessageEvent(
|
||||
text="human follow-up",
|
||||
message_type=MessageType.PHOTO,
|
||||
source=source,
|
||||
media_urls=["human.jpg"],
|
||||
media_types=["image/jpeg"],
|
||||
)
|
||||
adapter._pending_messages[entry.session_key] = pending_user_event
|
||||
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.session_store = store
|
||||
runner.adapters = {Platform.TELEGRAM: adapter}
|
||||
runner._profile_adapters = {}
|
||||
runner._gateway_loop = asyncio.get_running_loop()
|
||||
runner._running = True
|
||||
runner._draining = False
|
||||
runner._background_tasks = set()
|
||||
runner._queued_events = {}
|
||||
runner._is_user_authorized = MagicMock(return_value=True)
|
||||
adapter.set_busy_session_handler(runner._handle_active_session_busy_message)
|
||||
|
||||
manager = PluginManager()
|
||||
context = PluginContext(
|
||||
PluginManifest(name="notify-plugin", key="notify-plugin", source="user"),
|
||||
manager,
|
||||
)
|
||||
|
||||
with patch("hermes_cli.plugins.get_plugin_manager", return_value=manager):
|
||||
runner._install_plugin_message_injector()
|
||||
assert (
|
||||
context.inject_message(
|
||||
"/approve always",
|
||||
session_key=entry.session_key,
|
||||
)
|
||||
is True
|
||||
)
|
||||
task = next(iter(runner._background_tasks))
|
||||
await asyncio.gather(task, return_exceptions=True)
|
||||
await asyncio.sleep(0)
|
||||
|
||||
assert adapter._pending_messages[entry.session_key] is pending_user_event
|
||||
queued = runner._queued_events[entry.session_key][0]
|
||||
assert pending_user_event.text == "human follow-up"
|
||||
assert pending_user_event.media_urls == ["human.jpg"]
|
||||
assert pending_user_event.allow_gateway_control is True
|
||||
assert queued.text == "/approve always"
|
||||
assert queued.allow_gateway_control is False
|
||||
assert queued.metadata["gateway_session_id"] == entry.session_id
|
||||
adapter._message_handler.assert_not_awaited()
|
||||
|
||||
runner._clear_plugin_message_injector()
|
||||
assert manager.has_gateway_message_injector is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_uses_stored_origin_and_adapter_message_path():
|
||||
adapter = SimpleNamespace(handle_message=AsyncMock())
|
||||
@@ -66,11 +165,20 @@ async def test_dispatch_uses_stored_origin_and_adapter_message_path():
|
||||
event = adapter.handle_message.await_args.args[0]
|
||||
assert event.text == "check the deployment"
|
||||
assert event.internal is True
|
||||
assert event.allow_gateway_control is False
|
||||
assert event.get_command() is None
|
||||
assert event.source == entry.origin
|
||||
assert event.source is not entry.origin
|
||||
runner._is_user_authorized.assert_called_once_with(
|
||||
event.source,
|
||||
allow_adapter_delegation=False,
|
||||
)
|
||||
assert event.metadata == {
|
||||
"hermes_plugin_id": "notify-plugin",
|
||||
"hermes_plugin_injection": True,
|
||||
"gateway_session_key": entry.session_key,
|
||||
"gateway_session_id": entry.session_id,
|
||||
"gateway_session_strict": True,
|
||||
}
|
||||
|
||||
|
||||
@@ -97,17 +205,110 @@ async def test_dispatch_rejects_unroutable_session(entry, with_adapter):
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("raises", [False, True])
|
||||
async def test_dispatch_rechecks_current_authorization(raises):
|
||||
adapter = SimpleNamespace(handle_message=AsyncMock())
|
||||
runner = _runner(_entry(), adapter)
|
||||
if raises:
|
||||
runner._is_user_authorized.side_effect = RuntimeError("config unavailable")
|
||||
else:
|
||||
runner._is_user_authorized.return_value = False
|
||||
|
||||
accepted = await runner._dispatch_plugin_message_injection(
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
|
||||
assert accepted is False
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_stops_when_gateway_drains_during_lookup():
|
||||
adapter = SimpleNamespace(handle_message=AsyncMock())
|
||||
runner = _runner(_entry(), adapter)
|
||||
lookup_started = asyncio.Event()
|
||||
release_lookup = asyncio.Event()
|
||||
|
||||
async def _lookup(_session_key):
|
||||
lookup_started.set()
|
||||
await release_lookup.wait()
|
||||
return _entry()
|
||||
|
||||
runner._async_session_store.lookup_by_session_key = _lookup
|
||||
dispatch = asyncio.create_task(
|
||||
runner._dispatch_plugin_message_injection(
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
)
|
||||
|
||||
await lookup_started.wait()
|
||||
runner._draining = True
|
||||
release_lookup.set()
|
||||
|
||||
assert await dispatch is False
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_base_adapter_queues_non_control_plugin_text_for_exact_session():
|
||||
adapter = _RoutingAdapter()
|
||||
adapter.set_message_handler(AsyncMock())
|
||||
source = _entry().origin
|
||||
session_key = build_session_key(source)
|
||||
adapter._active_sessions[session_key] = asyncio.Event()
|
||||
event = MessageEvent(
|
||||
text="/approve always",
|
||||
message_type=MessageType.TEXT,
|
||||
source=source,
|
||||
internal=True,
|
||||
allow_gateway_control=False,
|
||||
metadata={"gateway_session_key": session_key},
|
||||
)
|
||||
|
||||
await adapter.handle_message(event)
|
||||
|
||||
adapter._message_handler.assert_not_awaited()
|
||||
assert adapter._pending_messages[session_key] is event
|
||||
assert adapter._active_sessions[session_key].is_set() is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_base_adapter_rejects_derived_session_mismatch():
|
||||
adapter = _RoutingAdapter()
|
||||
adapter.set_message_handler(AsyncMock())
|
||||
event = MessageEvent(
|
||||
text="ordinary input",
|
||||
source=_entry().origin,
|
||||
internal=True,
|
||||
allow_gateway_control=False,
|
||||
metadata={"gateway_session_key": "agent:main:telegram:dm:other"},
|
||||
)
|
||||
|
||||
await adapter.handle_message(event)
|
||||
|
||||
adapter._message_handler.assert_not_awaited()
|
||||
assert adapter._active_sessions == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scheduler_submits_dispatch_on_live_gateway_loop():
|
||||
runner = _runner(_entry())
|
||||
runner._gateway_loop = asyncio.get_running_loop()
|
||||
runner._dispatch_plugin_message_injection = AsyncMock(return_value=True)
|
||||
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is True
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
await asyncio.sleep(0)
|
||||
runner._dispatch_plugin_message_injection.assert_awaited_once_with(
|
||||
@@ -134,11 +335,14 @@ async def test_scheduler_ignores_same_loop_task_cancellation():
|
||||
runner._dispatch_plugin_message_injection = _wait_for_cancellation
|
||||
|
||||
try:
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is True
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
task = next(iter(runner._background_tasks))
|
||||
task.cancel()
|
||||
@@ -150,6 +354,37 @@ async def test_scheduler_ignores_same_loop_task_cancellation():
|
||||
assert callback_errors == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scheduler_logs_async_failure_without_callback_error(caplog):
|
||||
runner = _runner(_entry())
|
||||
loop = asyncio.get_running_loop()
|
||||
runner._gateway_loop = loop
|
||||
callback_errors = []
|
||||
previous_handler = loop.get_exception_handler()
|
||||
loop.set_exception_handler(lambda _loop, context: callback_errors.append(context))
|
||||
runner._dispatch_plugin_message_injection = AsyncMock(
|
||||
side_effect=RuntimeError("adapter failed")
|
||||
)
|
||||
|
||||
try:
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is True
|
||||
)
|
||||
task = next(iter(runner._background_tasks))
|
||||
await asyncio.gather(task, return_exceptions=True)
|
||||
await asyncio.sleep(0)
|
||||
finally:
|
||||
loop.set_exception_handler(previous_handler)
|
||||
|
||||
assert callback_errors == []
|
||||
assert "plugin=notify-plugin session=key" in caplog.text
|
||||
|
||||
|
||||
def test_scheduler_uses_threadsafe_bridge_outside_gateway_loop():
|
||||
runner = _runner(_entry())
|
||||
loop = MagicMock()
|
||||
@@ -164,11 +399,14 @@ def test_scheduler_uses_threadsafe_bridge_outside_gateway_loop():
|
||||
return future
|
||||
|
||||
with patch("gateway.run.safe_schedule_threadsafe", side_effect=_submit) as submit:
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is True
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
submit.assert_called_once()
|
||||
|
||||
@@ -189,11 +427,14 @@ def test_scheduler_ignores_threadsafe_future_cancellation():
|
||||
patch("gateway.run.safe_schedule_threadsafe", side_effect=_submit),
|
||||
patch("gateway.run.logger.warning") as warning,
|
||||
):
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is True
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
warning.assert_not_called()
|
||||
|
||||
@@ -205,20 +446,37 @@ def test_scheduler_rejects_stopped_or_closed_gateway():
|
||||
runner._gateway_loop = loop
|
||||
runner._running = False
|
||||
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is False
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is False
|
||||
)
|
||||
loop.call_soon_threadsafe.assert_not_called()
|
||||
|
||||
runner._running = True
|
||||
runner._gateway_loop = None
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
runner._gateway_loop = loop
|
||||
loop.is_closed.return_value = True
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is False
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is False
|
||||
)
|
||||
loop.call_soon_threadsafe.assert_not_called()
|
||||
|
||||
|
||||
@@ -228,12 +486,19 @@ def test_scheduler_rejects_submission_failure():
|
||||
loop.is_closed.return_value = False
|
||||
runner._gateway_loop = loop
|
||||
|
||||
with patch("gateway.run.safe_schedule_threadsafe", return_value=None):
|
||||
assert runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
) is False
|
||||
def _reject(coro, _target_loop, **_kwargs):
|
||||
coro.close()
|
||||
return None
|
||||
|
||||
with patch("gateway.run.safe_schedule_threadsafe", side_effect=_reject):
|
||||
assert (
|
||||
runner._schedule_plugin_message_injection(
|
||||
session_key="key",
|
||||
content="wake up",
|
||||
plugin_id="notify-plugin",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
def test_install_and_clear_gateway_injector_preserves_newer_owner():
|
||||
|
||||
@@ -68,6 +68,9 @@ class _FakeGateway:
|
||||
def _update_runtime_status(self, *_a, **_kw):
|
||||
pass
|
||||
|
||||
def _clear_plugin_message_injector(self):
|
||||
pass
|
||||
|
||||
async def _run_in_executor_with_context(self, func, *args):
|
||||
# stop() offloads agent-resource cleanup off the loop (#53175); run
|
||||
# inline in tests so the bounded-cleanup path is exercised.
|
||||
|
||||
@@ -47,7 +47,7 @@ def test_cli_running_injection_keeps_existing_interrupt_behaviour():
|
||||
)
|
||||
manager._cli_ref = cli
|
||||
|
||||
assert context.inject_message("status", role="system") is True
|
||||
assert context.inject_message("status", "system") is True
|
||||
assert cli._interrupt_queue.get_nowait() == "[system] status"
|
||||
assert cli._pending_input.empty()
|
||||
|
||||
@@ -72,10 +72,13 @@ def test_gateway_injection_requires_explicit_permission(tmp_path, monkeypatch):
|
||||
injector = MagicMock(return_value=True)
|
||||
manager.set_gateway_message_injector(object(), injector)
|
||||
|
||||
assert context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
) is False
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
injector.assert_not_called()
|
||||
|
||||
|
||||
@@ -89,10 +92,13 @@ def test_gateway_injection_does_not_treat_string_as_permission(tmp_path, monkeyp
|
||||
injector = MagicMock(return_value=True)
|
||||
manager.set_gateway_message_injector(object(), injector)
|
||||
|
||||
assert context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
) is False
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
injector.assert_not_called()
|
||||
|
||||
|
||||
@@ -105,10 +111,13 @@ def test_gateway_injection_fails_closed_when_config_cannot_be_read():
|
||||
"hermes_cli.plugins.load_config_readonly",
|
||||
side_effect=OSError("config unavailable"),
|
||||
):
|
||||
assert context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
) is False
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
injector.assert_not_called()
|
||||
|
||||
@@ -122,10 +131,13 @@ def test_gateway_injection_requires_live_host(tmp_path, monkeypatch):
|
||||
context, manager = _context()
|
||||
|
||||
assert manager.has_gateway_message_injector is False
|
||||
assert context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
) is False
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
def test_gateway_injection_passes_host_owned_plugin_identity(tmp_path, monkeypatch):
|
||||
@@ -164,7 +176,29 @@ def test_gateway_injection_returns_host_rejection(tmp_path, monkeypatch):
|
||||
MagicMock(return_value=False),
|
||||
)
|
||||
|
||||
assert context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
) is False
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
def test_gateway_injection_fails_closed_on_host_exception(tmp_path, monkeypatch):
|
||||
_write_plugin_config(
|
||||
tmp_path,
|
||||
monkeypatch,
|
||||
{"allow_gateway_injection": True},
|
||||
)
|
||||
context, manager = _context()
|
||||
injector = MagicMock(side_effect=RuntimeError("gateway unavailable"))
|
||||
manager.set_gateway_message_injector(object(), injector)
|
||||
|
||||
assert (
|
||||
context.inject_message(
|
||||
"wake up",
|
||||
session_key="agent:main:telegram:dm:42",
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
@@ -475,6 +475,10 @@ In gateway mode:
|
||||
|
||||
- `session_key` is required and must identify an existing gateway session. It is the stable routing key, not the CLI session ID.
|
||||
- Hermes reuses that session's stored platform, chat, thread, profile, and conversation history. Plugins cannot supply a new chat route through this API.
|
||||
- Hermes rechecks the stored route against the gateway's current authorisation rules before dispatch.
|
||||
- Routes that relied only on an adapter-time or upstream authorisation decision are rejected unless Hermes can revalidate them from current core allowlists, pairing, or explicit allow-all configuration.
|
||||
- Injected text is always conversational input. It cannot invoke slash commands, approve tools, or resolve pending confirmation and clarification prompts.
|
||||
- The route and conversation are pinned while dispatch is pending. Hermes drops the request if topic recovery changes the route or the session rotates before handling starts.
|
||||
- The request enters the platform adapter's normal message path. Active sessions use the existing busy-session queue rather than starting a competing turn.
|
||||
- Returns `True` when the live gateway accepts the request for asynchronous dispatch. This does not confirm that the agent turn or platform delivery has completed.
|
||||
- Returns `False` when `session_key` is omitted, the permission is not granted, or no live gateway can accept the request. Unknown or unroutable session keys discovered after asynchronous acceptance are written to the gateway log.
|
||||
|
||||
Reference in New Issue
Block a user