fix(multiplex): a profile whose home does not resolve must not borrow the launch profile's secrets
_profile_home_or_none answered None both for "this body is the launch profile's own work" and for "this NAMED profile's home no longer resolves" (deleted or renamed mid-run, invalid name, transient OSError), and the handler factories captured that answer once. With the launch-profile fallback added earlier on this branch, an inbound message on secondary B's own bot was then handled with the LAUNCH profile's .env and frozen env — a silent cross-tenant credential fallback where origin/main raised. _routed_profile_home now returns an UNRESOLVED_PROFILE_HOME sentinel and logs a WARNING; that case binds nothing, so an unscoped get_secret raises UnscopedSecretError under multiplexing. None keeps exactly one meaning: launch-owned.
This commit is contained in:
@@ -36,6 +36,20 @@ logger = logging.getLogger("gateway.run")
|
||||
_UNSET = object() # "no per-profile human_delay snapshot": fall back to the primary's value
|
||||
|
||||
|
||||
class _UnresolvedProfileHome:
|
||||
"""A NAMED routed profile whose home does not resolve — never the same thing as ``None``
|
||||
("this body is the launch profile's own work"). Overloading ``None`` for both let an inbound
|
||||
message on a secondary's bot run with the LAUNCH profile's ``.env`` and frozen env."""
|
||||
|
||||
__slots__ = ()
|
||||
|
||||
def __repr__(self) -> str: # log/diagnostic readability
|
||||
return "<unresolved profile home>"
|
||||
|
||||
|
||||
UNRESOLVED_PROFILE_HOME = _UnresolvedProfileHome()
|
||||
|
||||
|
||||
class GatewayAdapterLifecycleMixin:
|
||||
"""Adapter lifecycle: connect/teardown, fatal recovery, reconnect watcher, multiplex profiles."""
|
||||
|
||||
@@ -1258,7 +1272,7 @@ class GatewayAdapterLifecycleMixin:
|
||||
return
|
||||
attempts += 1
|
||||
queue_info["attempts"] = attempts
|
||||
profile_home = self._profile_home_or_none(profile_name)
|
||||
profile_home = self._routed_profile_home(profile_name)
|
||||
# The attempt above already hydrated this profile's secret sources off-loop.
|
||||
with self._scope_or_null(
|
||||
functools.partial(_profile_runtime_scope, hydrate_secrets=False), profile_home):
|
||||
@@ -1369,24 +1383,40 @@ class GatewayAdapterLifecycleMixin:
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _profile_home_or_none(profile_name: str):
|
||||
def _routed_profile_home(profile_name: str):
|
||||
"""A named routed profile's home, or :data:`UNRESOLVED_PROFILE_HOME` when it does not
|
||||
resolve (deleted or renamed mid-run, invalid name, transient OSError).
|
||||
|
||||
Never ``None``: ``None`` is reserved for "this body is the launch profile's own work", and
|
||||
answering it for an unresolvable NAMED profile is what made a secondary's inbound message
|
||||
run on the launch profile's credentials.
|
||||
"""
|
||||
from hermes_cli.profiles import get_profile_dir
|
||||
try:
|
||||
return get_profile_dir(profile_name)
|
||||
except Exception:
|
||||
return None
|
||||
logger.warning(
|
||||
"Profile home for '%s' does not resolve; its handlers run with no profile scope "
|
||||
"(credential reads fail closed instead of borrowing the launch profile's)",
|
||||
profile_name, exc_info=True)
|
||||
return UNRESOLVED_PROFILE_HOME
|
||||
|
||||
@staticmethod
|
||||
def _scope_or_null(scope_factory, profile_home):
|
||||
"""``scope_factory(profile_home)`` for a known profile home, else the LAUNCH profile's own
|
||||
scope once this process multiplexes.
|
||||
"""The runtime scope for one body, by what ``profile_home`` IS:
|
||||
|
||||
``None`` here means "no routed profile for this body" — the launch profile's own work, or a
|
||||
profile name that no longer resolves. Returning a bare ``nullcontext()`` made the launch
|
||||
profile the one tenant that ran with ambient ``os.environ`` and the process home, which a
|
||||
secondary's context may have poisoned; under the one-process-per-host ruling it is a tenant
|
||||
like any other. Single-profile hosts are unchanged: the helper is a no-op until activation.
|
||||
* a home — that profile's scope;
|
||||
* ``None`` — no routed profile: the LAUNCH profile's own work, so bind the launch profile
|
||||
explicitly once this process multiplexes. A bare ``nullcontext()`` made the launch
|
||||
profile the one tenant running on ambient ``os.environ`` and the process home, which a
|
||||
secondary's context may have poisoned; under the one-process-per-host ruling it is a
|
||||
tenant like any other. Single-profile hosts are unchanged (no-op until activation);
|
||||
* :data:`UNRESOLVED_PROFILE_HOME` — a NAMED profile whose home is gone: bind nothing, so an
|
||||
unscoped ``get_secret`` raises ``UnscopedSecretError`` under multiplexing. A profile never
|
||||
borrows another profile's value, and "we cannot tell whose this is" must fail closed.
|
||||
"""
|
||||
if profile_home is UNRESOLVED_PROFILE_HOME:
|
||||
return contextlib.nullcontext()
|
||||
if profile_home is not None:
|
||||
return scope_factory(profile_home)
|
||||
from tui_gateway.launch_profile_policy import launch_profile_scope_if_multiplexed
|
||||
@@ -1395,6 +1425,8 @@ class GatewayAdapterLifecycleMixin:
|
||||
@staticmethod
|
||||
def _async_scope_or_null(scope_factory, profile_home):
|
||||
"""``async with`` twin of :meth:`_scope_or_null` (for ``_async_profile_runtime_scope``)."""
|
||||
if profile_home is UNRESOLVED_PROFILE_HOME:
|
||||
return contextlib.nullcontext()
|
||||
if profile_home is not None:
|
||||
return scope_factory(profile_home)
|
||||
from tui_gateway.launch_profile_policy import async_launch_profile_scope_if_multiplexed
|
||||
@@ -1426,7 +1458,7 @@ class GatewayAdapterLifecycleMixin:
|
||||
profile scope (auth runs BEFORE the agent-turn scope, so the profile's ``.env`` must be
|
||||
visible here)."""
|
||||
from gateway.run import _async_profile_runtime_scope
|
||||
profile_home = self._profile_home_or_none(profile_name)
|
||||
profile_home = self._routed_profile_home(profile_name)
|
||||
|
||||
async def _handler(event):
|
||||
self._canonicalize(getattr(event, "source", None), transport_profile=profile_name)
|
||||
@@ -1439,7 +1471,7 @@ class GatewayAdapterLifecycleMixin:
|
||||
"""Busy-path twin: canonicalize FIRST, then resolve busy policy under the profile scope
|
||||
(auth runs against the profile's own allowlist, same as the cold-path message handler)."""
|
||||
from gateway.run import _async_profile_runtime_scope
|
||||
profile_home = self._profile_home_or_none(profile_name)
|
||||
profile_home = self._routed_profile_home(profile_name)
|
||||
|
||||
async def _handler(event, _session_key):
|
||||
self._canonicalize(event.source, transport_profile=profile_name)
|
||||
@@ -1527,7 +1559,7 @@ class GatewayAdapterLifecycleMixin:
|
||||
def _make_profile_platform_event_handler(self, profile_name: str):
|
||||
"""Bind platform-event auth and hook dispatch to one multiplex profile."""
|
||||
from gateway.run import _profile_runtime_scope
|
||||
profile_home = self._profile_home_or_none(profile_name)
|
||||
profile_home = self._routed_profile_home(profile_name)
|
||||
|
||||
async def _handler(event, source):
|
||||
self._canonicalize(source, transport_profile=profile_name)
|
||||
|
||||
@@ -1,16 +1,22 @@
|
||||
"""The launch profile is a tenant: a body with no routed profile binds ITS scope, not ambient env.
|
||||
"""Who owns a body with no routed profile home — and who must NOT.
|
||||
|
||||
``GatewayAdapterLifecycleMixin._scope_or_null`` used to return ``contextlib.nullcontext()`` whenever
|
||||
the profile home was ``None`` — the launch profile's own handoff reclaims, reconnect attention
|
||||
flags and platform events therefore ran completely unscoped on a multiplexing host: a legitimate
|
||||
launch-profile ``get_secret`` failed closed, and anything reading process env picked up whatever a
|
||||
secondary context had left there.
|
||||
|
||||
``None`` must mean exactly one thing. A NAMED profile whose home no longer resolves answers the
|
||||
:data:`UNRESOLVED_PROFILE_HOME` sentinel instead, and binds nothing: handing it the launch
|
||||
profile's scope would serve a secondary's inbound message with the LAUNCH profile's credentials.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope
|
||||
from agent.secret_scope import get_secret
|
||||
from gateway.run_adapters import GatewayAdapterLifecycleMixin
|
||||
from agent.secret_scope import UnscopedSecretError, get_secret
|
||||
from gateway.run_adapters import UNRESOLVED_PROFILE_HOME, GatewayAdapterLifecycleMixin
|
||||
from tui_gateway import launch_profile_policy
|
||||
|
||||
POISON = "LAUNCHSCOPE_TEST_KEY"
|
||||
@@ -48,3 +54,46 @@ def test_single_profile_host_keeps_ambient_precedence(monkeypatch):
|
||||
monkeypatch.setenv(POISON, "ambient")
|
||||
with GatewayAdapterLifecycleMixin._scope_or_null(lambda home: None, None):
|
||||
assert get_secret(POISON) == "ambient"
|
||||
|
||||
|
||||
class _Runner(GatewayAdapterLifecycleMixin):
|
||||
"""Just enough runner for the production handler factory."""
|
||||
|
||||
def __init__(self, seen):
|
||||
self.seen = seen
|
||||
|
||||
async def _handle_message(self, _event):
|
||||
try:
|
||||
self.seen.append(get_secret(POISON))
|
||||
except UnscopedSecretError:
|
||||
self.seen.append("<fail-closed>")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_named_secondary_with_unresolvable_home_cannot_read_the_launch_secret(
|
||||
multiplexing_host, monkeypatch):
|
||||
"""The invariant: a profile never borrows another profile's credential value.
|
||||
|
||||
Base resolved the missing home to ``None`` and the handler ran under the LAUNCH profile's
|
||||
scope, so an inbound message on profile B's own bot read ``launch-dotenv``.
|
||||
"""
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
|
||||
def _gone(_name):
|
||||
raise FileNotFoundError("profile deleted mid-run")
|
||||
|
||||
monkeypatch.setattr(profiles_mod, "get_profile_dir", _gone)
|
||||
|
||||
seen: list[str] = []
|
||||
handler = _Runner(seen)._make_profile_message_handler("b")
|
||||
await handler(SimpleNamespace(source=None))
|
||||
|
||||
assert seen == ["<fail-closed>"], f"secondary's handler resolved a credential to {seen}"
|
||||
|
||||
|
||||
def test_unresolvable_home_is_a_sentinel_not_none(multiplexing_host, monkeypatch):
|
||||
"""Overloading ``None`` is what made the fallback silent; keep the two answers distinct."""
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
|
||||
monkeypatch.setattr(profiles_mod, "get_profile_dir", lambda _n: (_ for _ in ()).throw(OSError()))
|
||||
assert GatewayAdapterLifecycleMixin._routed_profile_home("b") is UNRESOLVED_PROFILE_HOME
|
||||
|
||||
75
tests/gateway/test_mcp_shutdown_profile_passes.py
Normal file
75
tests/gateway/test_mcp_shutdown_profile_passes.py
Normal file
@@ -0,0 +1,75 @@
|
||||
"""MCP teardown at shutdown: the caller's budget is divided, and the wildcard pass is not poisoned.
|
||||
|
||||
Each ``shutdown_mcp_servers`` call waits up to its own timeout on the MCP loop, so N per-profile
|
||||
passes at the 15s default consumed the whole 5s caller budget and the trailing WILDCARD pass — the
|
||||
only one that stops the shared loop — never ran. And the teardown worker must start from a FRESH
|
||||
context: a caller sitting inside a served profile's scope would otherwise hand its HERMES_HOME
|
||||
override to the launch-profile pass, which documents that it has none.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from gateway import run as gateway_run
|
||||
|
||||
|
||||
class _Cfg:
|
||||
multiplex_profiles = True
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def homes(tmp_path, monkeypatch):
|
||||
homes = []
|
||||
for name in ("a", "b", "c"):
|
||||
home = tmp_path / name
|
||||
home.mkdir()
|
||||
homes.append((name, home))
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "launch"))
|
||||
(tmp_path / "launch").mkdir()
|
||||
monkeypatch.setattr(gateway_run, "_multiplex_profile_homes", lambda _cfg: homes)
|
||||
return homes
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_every_pass_shares_the_callers_budget(homes, monkeypatch):
|
||||
import tools.mcp_tool_lifecycle as lifecycle
|
||||
|
||||
seen: list[tuple] = []
|
||||
|
||||
def _fake(*, scope=None, names=None, timeout=15.0):
|
||||
seen.append((scope, timeout))
|
||||
|
||||
monkeypatch.setattr(lifecycle, "shutdown_mcp_servers", _fake)
|
||||
|
||||
assert await gateway_run._shutdown_mcp_servers_nonblocking(timeout=8.0, config=_Cfg())
|
||||
|
||||
assert len(seen) == 4, f"the wildcard pass did not run: {seen}"
|
||||
assert seen[-1][0] is None, "the last pass must be the process-wide wildcard"
|
||||
budget_per_pass = 8.0 / 4
|
||||
assert all(t <= budget_per_pass + 0.01 for _scope, t in seen), (
|
||||
f"a pass could outlive the caller's whole budget: {seen}")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_teardown_thread_does_not_inherit_the_callers_home_override(tmp_path, monkeypatch):
|
||||
import tools.mcp_tool_lifecycle as lifecycle
|
||||
from hermes_constants import (
|
||||
get_hermes_home_override, reset_hermes_home_override, set_hermes_home_override)
|
||||
|
||||
launch = tmp_path / "launch"
|
||||
poison = tmp_path / "profiles" / "poison"
|
||||
for path in (launch, poison):
|
||||
path.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(launch))
|
||||
|
||||
seen: list = []
|
||||
monkeypatch.setattr(
|
||||
lifecycle, "shutdown_mcp_servers",
|
||||
lambda **_kw: seen.append(get_hermes_home_override()))
|
||||
|
||||
token = set_hermes_home_override(str(poison))
|
||||
try:
|
||||
await gateway_run._shutdown_mcp_servers_nonblocking(timeout=5.0, config=None)
|
||||
finally:
|
||||
reset_hermes_home_override(token)
|
||||
|
||||
assert seen == [None], f"the wildcard teardown ran inside {seen} instead of a fresh context"
|
||||
@@ -68,7 +68,7 @@ async def test_secondary_reconnect_loop_escalates_under_own_profile(tmp_path, mo
|
||||
|
||||
monkeypatch.setattr(runner, "_secondary_reconnect_attempt", failing_attempt)
|
||||
monkeypatch.setattr(runner, "_safe_adapter_disconnect", noop_disconnect)
|
||||
monkeypatch.setattr(runner, "_profile_home_or_none", lambda name: secondary)
|
||||
monkeypatch.setattr(runner, "_routed_profile_home", lambda name: secondary)
|
||||
monkeypatch.setattr(gateway_run, "_reconnect_backoff", lambda attempts: 0.02)
|
||||
|
||||
task = asyncio.create_task(runner._run_secondary_profile_reconnect("sec", Platform.DISCORD))
|
||||
|
||||
47
tests/hermes_cli/test_web_server_launch_env_freeze.py
Normal file
47
tests/hermes_cli/test_web_server_launch_env_freeze.py
Normal file
@@ -0,0 +1,47 @@
|
||||
"""The launch-env freeze is the LAST boot step, not the first.
|
||||
|
||||
Activation snapshots ``os.environ`` as the launch profile's credentials, and that snapshot is the
|
||||
only source for launch keys with no ``.env`` to rebuild them from (systemd ``Environment=``,
|
||||
``op run``, Compose). Taken at the TOP of ``start_server`` it missed everything the rest of boot
|
||||
injected or rotated, for the process lifetime.
|
||||
"""
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope
|
||||
from hermes_cli import web_server
|
||||
from tui_gateway import launch_profile_policy
|
||||
|
||||
LATE_KEY = "LAUNCH_FREEZE_PROBE_TOKEN"
|
||||
|
||||
|
||||
def test_boot_time_credential_injection_is_inside_the_frozen_launch_env(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(secret_scope, "_MULTIPLEX_ACTIVE", False)
|
||||
monkeypatch.setattr(launch_profile_policy, "_snapshot", None)
|
||||
monkeypatch.delenv(LATE_KEY, raising=False)
|
||||
# A two-profile host, without touching the live install's profiles/.
|
||||
monkeypatch.setattr(launch_profile_policy, "_servable_profile_homes",
|
||||
lambda: {tmp_path / "a", tmp_path / "b"})
|
||||
monkeypatch.setattr(launch_profile_policy, "_multiplex_disabled_explicitly", lambda: False)
|
||||
|
||||
# Stand-ins for the boot steps that follow the old (top-of-function) activation point. A
|
||||
# provider key injected by the auth gate / keepalive / a lifespan hook is the real case.
|
||||
def _inject(*_args, **_kwargs):
|
||||
os.environ[LATE_KEY] = "injected-during-boot"
|
||||
|
||||
monkeypatch.setattr(web_server, "_configure_auth_gate", _inject)
|
||||
monkeypatch.setattr(web_server, "_build_uvicorn_server", lambda *a, **k: (object(), object()))
|
||||
monkeypatch.setattr(web_server, "_port_bind_conflict", lambda *a, **k: False)
|
||||
served: list[bool] = []
|
||||
monkeypatch.setattr(web_server, "_run_serve", lambda *a, **k: served.append(True))
|
||||
|
||||
try:
|
||||
web_server.start_server(open_browser=False, headless=True)
|
||||
finally:
|
||||
os.environ.pop(LATE_KEY, None)
|
||||
|
||||
assert served, "start_server never reached the serve step"
|
||||
assert secret_scope.is_multiplex_active()
|
||||
assert launch_profile_policy.capture_launch_env().get(LATE_KEY) == "injected-during-boot", (
|
||||
"the launch env was frozen before boot finished injecting credentials")
|
||||
66
tests/tui_gateway/test_eager_multiplex_activation.py
Normal file
66
tests/tui_gateway/test_eager_multiplex_activation.py
Normal file
@@ -0,0 +1,66 @@
|
||||
"""Eager multi-profile activation: when a host arms the fail-closed secret guard, and when it must not.
|
||||
|
||||
``profiles_to_serve`` takes the multiplex flag as an ARGUMENT and never reads config, so the eager
|
||||
gate has to consult the operator's setting itself; and ``named_profile_has_identity`` accepts an
|
||||
empty ``.env``, which is all a crashed ``hermes profile create`` leaves behind. Both made a host
|
||||
arm a one-way, process-wide guard it was never meant to arm.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from agent import secret_scope
|
||||
from tui_gateway import launch_profile_policy
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def two_profile_host(tmp_path, monkeypatch):
|
||||
"""Fake HOME so ``profiles/`` never resolves to the live install (see hermes-agent-dev)."""
|
||||
home = tmp_path / "fakehome" / ".hermes"
|
||||
(home / "profiles" / "b").mkdir(parents=True)
|
||||
monkeypatch.setenv("HOME", str(tmp_path / "fakehome"))
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(secret_scope, "_MULTIPLEX_ACTIVE", False)
|
||||
monkeypatch.setattr(launch_profile_policy, "_snapshot", None)
|
||||
monkeypatch.delenv("GATEWAY_MULTIPLEX_PROFILES", raising=False)
|
||||
from hermes_cli.profiles import _get_profiles_root
|
||||
assert str(_get_profiles_root()).startswith(str(tmp_path))
|
||||
return home / "profiles" / "b"
|
||||
|
||||
|
||||
def test_activates_for_a_real_second_profile(two_profile_host):
|
||||
(two_profile_host / "config.yaml").write_text("{}\n", encoding="utf-8")
|
||||
assert launch_profile_policy.activate_multi_profile_hosting_eagerly() is True
|
||||
assert secret_scope.is_multiplex_active()
|
||||
|
||||
|
||||
def test_multiplex_disabled_in_config_is_honoured(two_profile_host, monkeypatch):
|
||||
"""A host that pinned ``gateway.multiplex_profiles: false`` keeps per-profile gateways AND
|
||||
per-profile credential semantics; arming the guard there breaks legitimate unscoped reads."""
|
||||
(two_profile_host / "config.yaml").write_text("{}\n", encoding="utf-8")
|
||||
from hermes_cli import config as cfg_mod
|
||||
|
||||
monkeypatch.setattr(cfg_mod, "load_config", lambda *a, **k: {"gateway": {"multiplex_profiles": False}})
|
||||
|
||||
assert launch_profile_policy.activate_multi_profile_hosting_eagerly() is False
|
||||
assert not secret_scope.is_multiplex_active()
|
||||
|
||||
|
||||
def test_a_crashed_profile_create_shell_is_not_a_second_tenant(two_profile_host):
|
||||
"""An EMPTY ``.env`` is enough for ``named_profile_has_identity`` — not for flipping the host."""
|
||||
(two_profile_host / ".env").write_text("", encoding="utf-8")
|
||||
assert launch_profile_policy.activate_multi_profile_hosting_eagerly() is False
|
||||
assert not secret_scope.is_multiplex_active()
|
||||
|
||||
|
||||
def test_unreadable_profiles_dir_fails_closed_and_says_so(two_profile_host, monkeypatch, caplog):
|
||||
"""Silently returning False left the guard off for the process lifetime with zero log lines."""
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
|
||||
def _boom(multiplex):
|
||||
raise PermissionError("profiles/ is unreadable")
|
||||
|
||||
monkeypatch.setattr(profiles_mod, "profiles_to_serve", _boom)
|
||||
|
||||
with caplog.at_level("WARNING"):
|
||||
assert launch_profile_policy.activate_multi_profile_hosting_eagerly() is True
|
||||
assert secret_scope.is_multiplex_active()
|
||||
assert any("profile homes" in r.getMessage() for r in caplog.records), caplog.text
|
||||
Reference in New Issue
Block a user