fix(approval): every unattended context clears leaked presence vars
Widen the cron-only clearing to `_unattended_contexts()`: a webhook / api_server session running inside a gateway inherits HERMES_EXEC_ASK=1 exactly like an external cron worker does, and `_presence()` returning is_ask=True sent it to the gateway-decision branch with no notifier — a pending card nobody can answer — instead of `approvals.unattended_mode`. Same class as #110932, one predicate. Test trimmed to two invariants (cron / webhook leak → cleared; interactive keeps presence); the launch-path comment in cron/scheduler.py names the env-fallback consumers instead of an internal incident log.
This commit is contained in:
@@ -3261,12 +3261,10 @@ def _launch_external_cron_worker(job: dict) -> bool:
|
||||
finally:
|
||||
_reset_fire_secret_scope(fire_scope_tokens)
|
||||
worker_env = systemd_user_bus_env(worker_env)
|
||||
# Cron workers are unattended: presence vars inherited from a gateway that
|
||||
# set them at runtime (start_gateway sets HERMES_EXEC_ASK; interactive
|
||||
# launches set the rest) invert the approval gate — `_is_interactive_cli()`
|
||||
# sees HERMES_INTERACTIVE=1 and `approvals.cron_mode` is never consulted for
|
||||
# `terminal`, so the run hangs 10-30s on a pending card nobody can answer
|
||||
# (measured 2026-09-14: ms197 cron left 6 claims stranded; fab-swarm #105).
|
||||
# Unattended worker: the gateway sets HERMES_EXEC_ASK at startup (interactive launches set
|
||||
# the other two), and an inherited presence var makes every env-fallback consumer in the
|
||||
# child (`_is_interactive_cli`, sudo prompting, `check_cronjob_requirements`) believe a
|
||||
# human is present to answer (#110932).
|
||||
for _presence_var in (
|
||||
"HERMES_INTERACTIVE",
|
||||
"HERMES_GATEWAY_SESSION",
|
||||
|
||||
@@ -1,52 +1,38 @@
|
||||
"""Cron approval context must never resolve as interactive (layer-2 fix for #110932).
|
||||
"""Unattended approval contexts never resolve as interactive (#110932).
|
||||
|
||||
Layer 1 (#110942) strips presence vars at the launch path. This test pins the
|
||||
deeper invariant: even if HERMES_INTERACTIVE / HERMES_EXEC_ASK leak into a cron
|
||||
worker by some other route, `_approval_transport()` must not treat the context
|
||||
as CLI-interactive — nobody can answer the card, so the gate must fall through
|
||||
to `approvals.cron_mode` instead of hanging.
|
||||
A gateway sets HERMES_EXEC_ASK=1 at startup and hands its environ to every external cron
|
||||
worker; interactive launches export HERMES_INTERACTIVE=1. Inside cron (or a programmatic
|
||||
platform session) nobody can answer the card, so ``_presence()`` must clear the trio and let
|
||||
the gate resolve from ``approvals.cron_mode`` / ``approvals.unattended_mode``.
|
||||
"""
|
||||
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from tools import approval as approval_mod
|
||||
from tools.approval_context import set_hermes_interactive_context
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def leaked_presence(monkeypatch):
|
||||
monkeypatch.setenv("HERMES_INTERACTIVE", "1")
|
||||
monkeypatch.setenv("HERMES_EXEC_ASK", "1")
|
||||
# Explicitly NOT a gateway session: gateway handling is a separate path.
|
||||
monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False)
|
||||
# Mark the approval decision as running inside cron (session env is the
|
||||
# fallback path used by external workers; contextvars win in-process).
|
||||
monkeypatch.setenv("HERMES_CRON_SESSION", "1")
|
||||
|
||||
|
||||
def test_cron_context_is_not_cli_even_with_leaked_interactive(monkeypatch, leaked_presence):
|
||||
callback, is_cli, is_gateway, is_ask = approval_mod._presence()
|
||||
assert is_cli is False, "cron context must not resolve as interactive CLI"
|
||||
assert is_gateway is False, "cron context must not resolve as gateway"
|
||||
assert is_ask is False, "HERMES_EXEC_ASK leak must not make cron ask"
|
||||
|
||||
|
||||
def test_non_cron_interactive_still_cli(monkeypatch):
|
||||
# Guard against over-broad fix: a real interactive session stays interactive.
|
||||
monkeypatch.setenv("HERMES_GATEWAY_SESSION", "1")
|
||||
monkeypatch.delenv("HERMES_CRON_SESSION", raising=False)
|
||||
monkeypatch.setenv("HERMES_INTERACTIVE", "1")
|
||||
callback, is_cli, is_gateway, is_ask = approval_mod._presence()
|
||||
assert is_cli is True
|
||||
monkeypatch.delenv("HERMES_SINGLE_QUERY_SESSION", raising=False)
|
||||
monkeypatch.delenv("HERMES_SESSION_PLATFORM", raising=False)
|
||||
|
||||
|
||||
def test_cron_contextvar_interactive_also_not_cli(monkeypatch, leaked_presence):
|
||||
# The contextvar binding is stronger than env; cron must win over it too.
|
||||
token = set_hermes_interactive_context(True)
|
||||
try:
|
||||
callback, is_cli, is_gateway, is_ask = approval_mod._presence()
|
||||
assert is_cli is False
|
||||
finally:
|
||||
from tools.approval_context import reset_hermes_interactive_context
|
||||
reset_hermes_interactive_context(token)
|
||||
@pytest.mark.parametrize(
|
||||
"unattended_env",
|
||||
[{"HERMES_CRON_SESSION": "1"}, {"HERMES_SESSION_PLATFORM": "webhook"}],
|
||||
ids=["cron", "webhook"],
|
||||
)
|
||||
def test_unattended_context_clears_leaked_presence(monkeypatch, leaked_presence, unattended_env):
|
||||
for key, value in unattended_env.items():
|
||||
monkeypatch.setenv(key, value)
|
||||
_, is_cli, is_gateway, is_ask = approval_mod._presence()
|
||||
assert (is_cli, is_gateway, is_ask) == (False, False, False)
|
||||
|
||||
|
||||
def test_interactive_session_keeps_presence(monkeypatch, leaked_presence):
|
||||
_, is_cli, is_gateway, is_ask = approval_mod._presence()
|
||||
assert (is_cli, is_gateway, is_ask) == (True, True, True)
|
||||
|
||||
@@ -867,21 +867,18 @@ def _human_decision(spec: _GateSpec, *, command: str, description: str,
|
||||
|
||||
|
||||
def _presence(approval_callback=None) -> tuple:
|
||||
"""``(approval_callback, is_cli, is_gateway, is_ask)`` for the current context. Single-query
|
||||
(-q) exports HERMES_INTERACTIVE=1 but nobody answers prompts, and HERMES_EXEC_ASK has no
|
||||
human either — both are cleared so single_query_mode actually takes effect."""
|
||||
"""``(approval_callback, is_cli, is_gateway, is_ask)`` for the current context.
|
||||
|
||||
Every unattended context (single-query ``-q``, cron, programmatic platforms) clears the
|
||||
presence trio: ``hermes chat -q`` exports HERMES_INTERACTIVE=1 for sudo prompts, a gateway
|
||||
sets HERMES_EXEC_ASK=1 at startup and passes its environ to every external cron worker
|
||||
(#110932), and a webhook session inherits that same HERMES_EXEC_ASK — in none of them can a
|
||||
human answer the card, so the gate must resolve from ``approvals.<ctx>_mode`` instead of
|
||||
parking on a pending approval."""
|
||||
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():
|
||||
is_cli = is_gateway = is_ask = False
|
||||
if _is_cron_approval_context():
|
||||
# Cron workers are unattended: nobody can answer an approval card. Presence
|
||||
# vars (HERMES_INTERACTIVE / HERMES_EXEC_ASK) can leak in via any env-passing
|
||||
# launch path — treat them as advisory here so the gate consults
|
||||
# ``approvals.cron_mode`` instead of hanging on a pending card (#110932).
|
||||
# Mirrors the single-query clearing above and the cron exclusion inside
|
||||
# ``_is_gateway_approval_context``.
|
||||
if _unattended_contexts():
|
||||
is_cli = is_gateway = is_ask = False
|
||||
return approval_callback, is_cli, is_gateway, is_ask
|
||||
|
||||
|
||||
Reference in New Issue
Block a user