diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 228a17d1e3..bce42f23b5 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -66,6 +66,23 @@ from typing import Any, Dict, List, Optional # (no prefix / multiplexing off → handle as the default profile). _PROFILE_REJECTED = object() + +def _prefix_names_served_profile(profile: str) -> bool: + """True when a /p// prefix names the profile this gateway serves. + + Single-profile (non-multiplex) gateways historically ignored the prefix + and answered every /p// request from their own profile's config — + which silently served the gateway owner's toolsets/capabilities under + another profile's URL (#91583 defect 2). Only a self-referential prefix + may fall through; anything else must be rejected. Fail closed. + """ + try: + from hermes_cli.profiles import profile_matches_home + + return profile_matches_home(profile) + except Exception: + return False + # Profile selected by the /p// URL prefix for the current request. # Set by the profile-prefix middleware; read by handlers / _run_agent. _api_request_profile: ContextVar[Optional[str]] = ContextVar( @@ -2082,12 +2099,13 @@ class APIServerAdapter(BasePlatformAdapter): Returns: - ``None`` when no profile prefix is present, or when multiplexing - is off and the prefix names this process's own profile (the - request is already scoped to the only agent served here). + is off and the prefix names this gateway's own profile (the + request is handled as the serving profile). - the profile name (str) when present, multiplexing is on, and the profile is one this gateway serves. - - ``_PROFILE_REJECTED`` when a prefix is present but names a profile - this process cannot serve (handler/middleware returns 404). + - ``_PROFILE_REJECTED`` when a prefix is present but the profile is + unknown/unconfigured, or names a profile this single-profile + gateway does not serve (handler/middleware returns 404). """ profile = (request.match_info.get("profile") or "").strip() if not profile: @@ -2095,25 +2113,19 @@ class APIServerAdapter(BasePlatformAdapter): runner = getattr(self, "gateway_runner", None) cfg = getattr(runner, "config", None) if not getattr(cfg, "multiplex_profiles", False): - # A prefix names a specific agent. With multiplexing off this - # process serves exactly one, so honor the prefix only when it - # names that one (peers address single-profile daemons this way - # without knowing the host's topology). Anything else must fail - # closed: answering as the local profile would deliver the - # request to a DIFFERENT agent than the one addressed — silent - # misdelivery, strictly worse than a 404. Observed live (Aug - # 2026): `hermes peer dm mini/researcher` answered by the mini's - # default agent, with no error anywhere. - try: - from hermes_cli.profiles import get_active_profile_name - - own = get_active_profile_name() - except Exception: - return _PROFILE_REJECTED - if profile == own: - # Scoped to self: same handling as no prefix at all. - return None - return _PROFILE_REJECTED + # Prefix supplied but multiplexing is off. Only a self-referential + # prefix (naming the profile this gateway already serves) may fall + # through to the bare route. Silently ignoring ANY prefix served + # the gateway owner's config/toolsets/capabilities under another + # profile's URL — cross-profile capability leakage (#91583 + # defect 2) and silently misdelivered peer DMs (observed live: + # `hermes peer dm mini/researcher` answered by the mini's default + # agent) — so anything else fails closed as unknown. + return ( + None + if _prefix_names_served_profile(profile) + else _PROFILE_REJECTED + ) try: from hermes_cli.profiles import profiles_to_serve diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index bc3ebe0968..99eefcd634 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -564,12 +564,14 @@ class WebhookAdapter(BasePlatformAdapter): """Resolve + validate the /p// URL prefix on a webhook request. Returns: - - ``None`` when no profile prefix is present, or multiplexing is off - (the prefix is ignored, request handled as the default profile). + - ``None`` when no profile prefix is present, or when multiplexing + is off and the prefix names this gateway's own profile (the + request is handled as the serving profile). - the profile name (str) when present, multiplexing is on, and the profile is one this gateway serves. - ``_PROFILE_REJECTED`` when a prefix is present but the profile is - unknown/unconfigured (handler returns 404). + unknown/unconfigured, or names a profile this single-profile + gateway does not serve (handler returns 404). """ profile = (request.match_info.get("profile") or "").strip() if not profile: @@ -577,9 +579,19 @@ class WebhookAdapter(BasePlatformAdapter): runner = self.gateway_runner cfg = getattr(runner, "config", None) if not getattr(cfg, "multiplex_profiles", False): - # Prefix supplied but multiplexing is off — ignore it, behave as - # the single-profile gateway (don't 404 a would-be valid route). - return None + # Prefix supplied but multiplexing is off. Only a self-referential + # prefix (naming this gateway's own profile) may fall through to + # the bare route; anything else fails closed — silently ignoring + # the prefix served the gateway owner's routes/config under + # another profile's URL (#91583 defect 2). + try: + from hermes_cli.profiles import profile_matches_home + + if profile_matches_home(profile): + return None + except Exception: + pass + return _PROFILE_REJECTED try: from hermes_cli.profiles import profiles_to_serve served = { diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 6e83a824f3..fbcd9cbd87 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -387,6 +387,38 @@ def profile_exists(name: str) -> bool: return get_profile_dir(canon).is_dir() +def profile_matches_home(name: str, home: "Path | None" = None) -> bool: + """Return True when *name* refers to the profile served from *home*. + + ``home`` defaults to the process's current Hermes home + (:func:`hermes_constants.get_hermes_home`). Used by single-profile + gateways to decide whether a ``/p//`` URL prefix is + self-referential (safe to serve on the bare route) or names a *different* + profile — in which case the request must fail closed rather than silently + resolve config/toolsets from the gateway owner (#91583 defect 2). + + Invalid profile names return False (fail closed). + """ + try: + target = get_profile_dir(name) + except Exception: + return False + if home is None: + try: + from hermes_constants import get_hermes_home + + home = get_hermes_home() + except Exception: + return False + try: + return ( + Path(target).expanduser().resolve(strict=False) + == Path(home).expanduser().resolve(strict=False) + ) + except Exception: + return False + + def list_profile_names() -> List[str]: """Cheap name-only profile listing: ``default`` plus profile dirs. diff --git a/tests/gateway/test_multiplex_toolsets_profile_isolation.py b/tests/gateway/test_multiplex_toolsets_profile_isolation.py new file mode 100644 index 0000000000..86a8f320de --- /dev/null +++ b/tests/gateway/test_multiplex_toolsets_profile_isolation.py @@ -0,0 +1,204 @@ +"""Per-profile toolset isolation on the multiplexed api_server (#91583 defect 2). + +Two defects covered: + +1. With multiplexing ON, ``/p//v1/toolsets`` must reflect profile *x*'s own + ``platform_toolsets.api_server`` — for both the gateway-owning default + profile and a secondary profile — never the listener owner's config. + +2. With multiplexing OFF, a ``/p//`` prefix used to be silently + ignored, serving the gateway owner's config under another profile's URL. + It must now be rejected (404); only a self-referential prefix (naming the + profile this gateway actually serves) falls through. + +E2E-style: real profile homes under a temp HERMES root, real config.yaml +files read through the canonical loaders, and real aiohttp request routing +(TestClient) through the profile-prefix middleware. No mocked config reads. +""" +from __future__ import annotations + +import pytest + +aiohttp = pytest.importorskip("aiohttp") +from aiohttp import web # noqa: E402 +from aiohttp.test_utils import TestClient, TestServer # noqa: E402 + +from gateway.config import GatewayConfig, PlatformConfig # noqa: E402 +from gateway.platforms.api_server import ( # noqa: E402 + APIServerAdapter, + _PROFILE_REJECTED, +) + +OWNER_KEY = "owner-key-1234567890abcdef" +LOKAJ_KEY = "lokaj-key-1234567890abcdef" + + +@pytest.fixture() +def hermes_root(tmp_path, monkeypatch): + """Two real profile homes: default (owner) and 'lokaj' (secondary).""" + root = tmp_path / "hermes" + lokaj = root / "profiles" / "lokaj" + lokaj.mkdir(parents=True) + (root / "config.yaml").write_text( + "platform_toolsets:\n api_server: [web, file]\n", + encoding="utf-8", + ) + (lokaj / "config.yaml").write_text( + "platform_toolsets:\n api_server: [web, file, computer_use]\n", + encoding="utf-8", + ) + (lokaj / ".env").write_text(f"API_SERVER_KEY={LOKAJ_KEY}\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(root)) + # get_default_hermes_root memoizes per (native_home, env_home) pair, so + # the env change alone re-keys it; no cache reset needed. + return root + + +def _make_adapter(multiplex: bool) -> APIServerAdapter: + cfg = PlatformConfig(enabled=True, extra={"key": OWNER_KEY}) + adapter = APIServerAdapter(cfg) + + class _Runner: + config = GatewayConfig(multiplex_profiles=multiplex) + + adapter.gateway_runner = _Runner() + return adapter + + +def _make_app(adapter: APIServerAdapter) -> web.Application: + """Mirror connect()'s wiring: middleware + native and /p/ mirror routes.""" + app = web.Application(middlewares=[adapter._make_profile_prefix_middleware()]) + app["api_server_adapter"] = adapter + for method, path, handler in adapter._http_route_table(): + app.router.add_route(method, path, handler) + app.router.add_route(method, f"/p/{{profile}}{path}", handler) + return app + + +async def _enabled_toolsets(cli: TestClient, path: str, key: str): + resp = await cli.get(path, headers={"Authorization": f"Bearer {key}"}) + if resp.status != 200: + return resp.status, None + body = await resp.json() + return resp.status, {d["name"] for d in body["data"] if d["enabled"]} + + +class TestMultiplexOnToolsetIsolation: + @pytest.mark.asyncio + async def test_each_profile_sees_its_own_toolsets(self, hermes_root): + adapter = _make_adapter(multiplex=True) + async with TestClient(TestServer(_make_app(adapter))) as cli: + # Owner (default) profile: bare route and /p/default mirror. + for path in ("/v1/toolsets", "/p/default/v1/toolsets"): + status, enabled = await _enabled_toolsets(cli, path, OWNER_KEY) + assert status == 200, path + assert "computer_use" not in enabled, path + assert {"web", "file"} <= enabled, path + + # Secondary profile: its own config, its own key. + status, enabled = await _enabled_toolsets( + cli, "/p/lokaj/v1/toolsets", LOKAJ_KEY + ) + assert status == 200 + # The exact repro from #91583 defect 2: computer_use enabled in + # lokaj's config must be reported enabled under /p/lokaj/… + assert "computer_use" in enabled + + @pytest.mark.asyncio + async def test_owner_key_does_not_open_secondary_profile(self, hermes_root): + """Cross-profile auth stays closed: owner key must not read lokaj.""" + adapter = _make_adapter(multiplex=True) + async with TestClient(TestServer(_make_app(adapter))) as cli: + status, _ = await _enabled_toolsets( + cli, "/p/lokaj/v1/toolsets", OWNER_KEY + ) + assert status == 401 + + @pytest.mark.asyncio + async def test_unknown_profile_is_404(self, hermes_root): + adapter = _make_adapter(multiplex=True) + async with TestClient(TestServer(_make_app(adapter))) as cli: + resp = await cli.get( + "/p/ghost/v1/toolsets", + headers={"Authorization": f"Bearer {OWNER_KEY}"}, + ) + assert resp.status == 404 + + +class TestMultiplexOffPrefixFailsClosed: + """Single-profile gateways must not serve another profile's URL.""" + + def test_foreign_prefix_rejected(self, hermes_root): + adapter = _make_adapter(multiplex=False) + + class _Req: + match_info = {"profile": "lokaj"} + + assert adapter._resolve_request_profile(_Req()) is _PROFILE_REJECTED + + def test_self_referential_prefix_falls_through(self, hermes_root): + """/p/default/ on the default-profile gateway keeps working.""" + adapter = _make_adapter(multiplex=False) + + class _Req: + match_info = {"profile": "default"} + + assert adapter._resolve_request_profile(_Req()) is None + + def test_own_named_profile_prefix_falls_through(self, hermes_root, monkeypatch): + """A gateway launched FOR profile lokaj accepts /p/lokaj/…""" + monkeypatch.setenv( + "HERMES_HOME", str(hermes_root / "profiles" / "lokaj") + ) + adapter = _make_adapter(multiplex=False) + + class _Req: + match_info = {"profile": "lokaj"} + + assert adapter._resolve_request_profile(_Req()) is None + + @pytest.mark.asyncio + async def test_foreign_prefix_is_404_end_to_end(self, hermes_root): + adapter = _make_adapter(multiplex=False) + async with TestClient(TestServer(_make_app(adapter))) as cli: + resp = await cli.get( + "/p/lokaj/v1/toolsets", + headers={"Authorization": f"Bearer {OWNER_KEY}"}, + ) + assert resp.status == 404 + # Bare route unaffected. + status, enabled = await _enabled_toolsets( + cli, "/v1/toolsets", OWNER_KEY + ) + assert status == 200 + assert "computer_use" not in enabled + + +class TestWebhookMultiplexOffPrefixFailsClosed: + """Same bug class in the webhook adapter's prefix resolver.""" + + def _adapter(self, multiplex: bool): + from gateway.platforms.webhook import WebhookAdapter, _PROFILE_REJECTED + + class _Runner: + config = GatewayConfig(multiplex_profiles=multiplex) + + adapter = WebhookAdapter.__new__(WebhookAdapter) + adapter.gateway_runner = _Runner() + return adapter, _PROFILE_REJECTED + + def test_foreign_prefix_rejected(self, hermes_root): + adapter, rejected = self._adapter(multiplex=False) + + class _Req: + match_info = {"profile": "lokaj"} + + assert adapter._resolve_request_profile(_Req()) is rejected + + def test_self_referential_prefix_falls_through(self, hermes_root): + adapter, _rejected = self._adapter(multiplex=False) + + class _Req: + match_info = {"profile": "default"} + + assert adapter._resolve_request_profile(_Req()) is None