fix(approvals): treat webhook platforms as notifier-blind in _presence
The TurnRunner lane registers a gateway notify callback for every inbound turn with no platform branch (run_turn_runner._run_conversation_with_approval), so on webhook/msgraph_webhook the 'no notifier' discriminator can never fire: is_ask stays True, the adapter renders no send_exec_approval surface, the inbound lane is POST -> 202 with no reader, and the turn blocks in _await_gateway_decision for the full approval timeout before failing closed — the residual stall of #100532 that the notifier derivation missed. A registered notifier only means 'someone can answer' on api_server, whose turn paths choose whether to register one. Add _NOTIFIER_BLIND_APPROVAL_PLATFORMS ({webhook, msgraph_webhook}) and drop is_ask there regardless of registration; api_server keeps the notifier test. New tests drive the gate with a registered notifier on both platforms (the suite previously had no HERMES_SESSION_PLATFORM=webhook case at all).
This commit is contained in:
@@ -1016,14 +1016,17 @@ class TestWebhookApprovalExclusion:
|
||||
class TestUnattendedPlatformExecAskLeak:
|
||||
"""A gateway process sets HERMES_EXEC_ASK=1 for EVERY session it serves, so an
|
||||
api_server/webhook turn reads as "ask" in ``_presence()`` even though the platform was
|
||||
already excluded from gateway approval contexts. Without a registered notify callback
|
||||
(non-streaming /v1/chat/completions, webhook, msgraph_webhook) the ask branch parks the
|
||||
approval as ``pending_approval`` that no client on that endpoint can ever answer, and
|
||||
``approvals.unattended_mode`` never runs — the unanswerable stall of #100532.
|
||||
already excluded from gateway approval contexts. Without a surface that can answer the
|
||||
card — non-streaming /v1/chat/completions registers no notify callback, and
|
||||
webhook/msgraph_webhook register one on every turn yet render no approval surface and
|
||||
have no reader — the ask branch blocks for the full approval timeout with no human who
|
||||
can resolve it, and ``approvals.unattended_mode`` never runs (#100532).
|
||||
|
||||
Fix: ``_presence()`` drops ``is_ask`` for unattended-platform sessions without a notify
|
||||
callback, so the unattended branch resolves instantly from ``approvals.unattended_mode``.
|
||||
``/v1/runs`` and streaming chat completions register a callback and keep the ask path.
|
||||
Fix: ``_presence()`` drops ``is_ask`` for unattended-platform sessions that no client can
|
||||
answer — no notify callback on api_server, or a notifier-blind platform (webhook,
|
||||
msgraph_webhook) where the TurnRunner lane registers one unconditionally — so the
|
||||
unattended branch resolves instantly from ``approvals.unattended_mode``. ``/v1/runs`` and
|
||||
streaming chat completions register a callback and keep the ask path.
|
||||
"""
|
||||
|
||||
def _isolate(self, monkeypatch, *, platform="api_server"):
|
||||
@@ -1100,6 +1103,48 @@ class TestUnattendedPlatformExecAskLeak:
|
||||
_, _is_cli, _is_gateway, is_ask = approval_mod._presence()
|
||||
assert is_ask is True
|
||||
|
||||
def test_webhook_registered_notifier_still_resolves_unattended(self, monkeypatch):
|
||||
"""webhook/msgraph_webhook residual of #100532: the generic TurnRunner lane registers a
|
||||
notify callback for *every* inbound turn (``_run_conversation_with_approval`` registers
|
||||
unconditionally), yet the WebhookAdapter renders no ``send_exec_approval``/``/approve``
|
||||
surface and the inbound lane is ``POST -> 202`` with no reader — so a registered
|
||||
notifier there does not mean anyone can answer. The ask leak must resolve from
|
||||
``approvals.unattended_mode`` instead of blocking the full approval timeout."""
|
||||
import tools.approval as approval_mod
|
||||
from tools.approval import check_all_command_guards
|
||||
|
||||
self._isolate(monkeypatch, platform="webhook")
|
||||
monkeypatch.setattr(approval_mod, "_smart_verdict",
|
||||
lambda *a, **k: pytest.fail("guardian ran after unattended resolution"))
|
||||
|
||||
def _no_human(data):
|
||||
pytest.fail("approval card delivered to a platform with no approval transport")
|
||||
|
||||
approval_mod.register_gateway_notify("test-unattended-ask-leak", _no_human)
|
||||
try:
|
||||
result = check_all_command_guards("pkill -9 -f soffice", "local")
|
||||
finally:
|
||||
approval_mod.unregister_gateway_notify("test-unattended-ask-leak")
|
||||
|
||||
assert result["approved"] is False
|
||||
assert result.get("status") != "pending_approval"
|
||||
assert result.get("approval_pending") is not True
|
||||
assert "unattended platform" in result["message"]
|
||||
assert "approvals.unattended_mode" in result["message"]
|
||||
|
||||
def test_msgraph_webhook_registered_notifier_drops_ask(self, monkeypatch):
|
||||
"""Same shape at the ``_presence()`` level for msgraph_webhook: the TurnRunner-registered
|
||||
notifier must not keep the ask branch alive on a notifier-blind platform."""
|
||||
import tools.approval as approval_mod
|
||||
|
||||
self._isolate(monkeypatch, platform="msgraph_webhook")
|
||||
approval_mod.register_gateway_notify("test-unattended-ask-leak", lambda data: None)
|
||||
try:
|
||||
_, _is_cli, _is_gateway, is_ask = approval_mod._presence()
|
||||
assert is_ask is False
|
||||
finally:
|
||||
approval_mod.unregister_gateway_notify("test-unattended-ask-leak")
|
||||
|
||||
|
||||
class TestNormalizationBypass:
|
||||
"""Obfuscation techniques must not bypass dangerous command detection."""
|
||||
|
||||
@@ -914,23 +914,30 @@ def _presence(approval_callback=None) -> tuple:
|
||||
HERMES_INTERACTIVE=1 for sudo prompts, and a gateway sets HERMES_EXEC_ASK=1 at startup and
|
||||
passes its environ to every external cron worker (#110932) — in neither can a human answer
|
||||
the card, so the gate must resolve from ``approvals.<ctx>_mode`` instead of parking on a
|
||||
pending approval. Unattended *platforms* keep ``is_ask`` only while the session has a
|
||||
registered approval notifier: ``/v1/runs`` and streaming chat completions bridge one
|
||||
(``approval.request`` → ``POST /v1/runs/{id}/approval``). Without a notifier no client on
|
||||
the platform can ever answer, so ``is_ask`` drops too and the unattended branch resolves
|
||||
from ``approvals.unattended_mode`` instead of parking an unanswerable card (#100532)."""
|
||||
pending approval. Unattended *platforms* keep ``is_ask`` only while a client on the session
|
||||
could actually answer: ``/v1/runs`` and streaming chat completions bridge an approval
|
||||
notifier (``approval.request`` → ``POST /v1/runs/{id}/approval``), so on api_server a
|
||||
registered notifier is the discriminator. webhook/msgraph_webhook turns run through the
|
||||
generic TurnRunner lane, which registers a notifier for *every* turn — but those adapters
|
||||
render no approval surface and the inbound lane has no reader, so there ``is_ask`` drops
|
||||
regardless and the unattended branch resolves from ``approvals.unattended_mode`` instead of
|
||||
parking an unanswerable card (#100532)."""
|
||||
approval_callback = _resolve_cli_approval_callback(approval_callback)
|
||||
is_cli, is_gateway = _is_interactive_cli(), _is_gateway_approval_context()
|
||||
is_ask = env_var_enabled("HERMES_EXEC_ASK")
|
||||
if _is_single_query_approval_context() or _is_cron_approval_context():
|
||||
is_cli = is_gateway = is_ask = False
|
||||
elif is_ask and _is_unattended_platform_approval_context() \
|
||||
and _gateway_notify_cb(get_current_session_key()) is None:
|
||||
and (_gateway_notify_cb(get_current_session_key()) is None
|
||||
or _get_session_platform() in approval_context._NOTIFIER_BLIND_APPROVAL_PLATFORMS):
|
||||
# start_gateway() sets HERMES_EXEC_ASK process-wide, so an api_server/webhook turn
|
||||
# inside that gateway still reads as "ask" here even though _is_gateway_approval_context()
|
||||
# already excluded the platform. /v1/runs and streaming chat register their notify
|
||||
# callback before the turn runs; a session with none has no surface that can resolve an
|
||||
# approval — resolve from approvals.unattended_mode, never a pending no one can answer.
|
||||
# approval. webhook/msgraph_webhook are notifier-blind: the generic TurnRunner lane
|
||||
# registers one for every turn while the adapter has no ``send_exec_approval`` and the
|
||||
# POST -> 202 lane has no reader, so a registered callback there answers nothing either
|
||||
# — resolve from approvals.unattended_mode, never a pending no one can answer.
|
||||
is_ask = False
|
||||
return approval_callback, is_cli, is_gateway, is_ask
|
||||
|
||||
|
||||
@@ -146,6 +146,15 @@ def _is_unattended_platform_approval_context() -> bool:
|
||||
return _get_session_platform() in _UNATTENDED_APPROVAL_PLATFORMS
|
||||
|
||||
|
||||
# Platforms where a *registered* gateway notify callback still does not mean a human can answer:
|
||||
# the generic TurnRunner lane registers one for every inbound turn
|
||||
# (``_run_conversation_with_approval`` registers unconditionally, no platform branch), while the
|
||||
# adapter renders no ``send_exec_approval``/``/approve`` surface and the inbound lane is a
|
||||
# fire-and-forget ``POST -> 202`` with no reader. Notifier presence is only a meaningful
|
||||
# "someone can answer" discriminator on api_server, whose turn paths choose whether to register one.
|
||||
_NOTIFIER_BLIND_APPROVAL_PLATFORMS = frozenset({"webhook", "msgraph_webhook"})
|
||||
|
||||
|
||||
def _is_single_query_approval_context() -> bool:
|
||||
"""True for a single-query (-q) session: ``hermes chat -q`` exports
|
||||
``HERMES_INTERACTIVE=1`` (so sudo password prompts work) but nobody is waiting
|
||||
|
||||
Reference in New Issue
Block a user