From 8c5e846536c8b35bb5276027ac8a103e25386390 Mon Sep 17 00:00:00 2001 From: Atakan Date: Mon, 27 Jul 2026 01:44:39 +0300 Subject: [PATCH] fix(gateway): bind HTTP auth to routed profiles --- gateway/platforms/api_server.py | 53 +++++- gateway/platforms/webhook.py | 35 ++++ .../test_api_server_multiplex_secret_scope.py | 168 ++++++++++++++++++ tests/gateway/test_webhook_adapter.py | 128 +++++++++++++ website/docs/user-guide/messaging/webhooks.md | 6 + .../docs/user-guide/multi-profile-gateways.md | 18 ++ 6 files changed, 405 insertions(+), 3 deletions(-) diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 209542745efc..0346fcc25761 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -1508,6 +1508,30 @@ class APIServerAdapter(BasePlatformAdapter): # Auth helper # ------------------------------------------------------------------ + def _expected_api_key(self) -> str: + """Return the API key authorized for the URL-selected profile.""" + profile = _api_request_profile.get() + if not profile or profile == "default": + return self._api_key + + try: + from agent.secret_scope import get_secret + from hermes_cli.auth import has_usable_secret + + key = get_secret("API_SERVER_KEY", "") or "" + if not has_usable_secret(key, min_length=16): + return "" + return key + except Exception as exc: + # Fail closed if the profile scope or strength guard cannot resolve + # the credential. Do not log the key or exception text. + logger.warning( + "Failed to resolve a usable profile-scoped API_SERVER_KEY for %r: %s", + profile, + type(exc).__name__, + ) + return "" + def _check_auth(self, request: "web.Request") -> Optional["web.Response"]: """ Validate Bearer token from Authorization header. @@ -1516,8 +1540,31 @@ class APIServerAdapter(BasePlatformAdapter): connect() refuses to start the API server without API_SERVER_KEY, so the no-key branch only exists for tests or unsupported manual wiring. """ - if not self._api_key: - return None + profile = _api_request_profile.get() + is_named_profile = bool(profile and profile != "default") + expected_key = self._expected_api_key() + if not expected_key: + # Preserve the historical no-key test/manual-wiring behavior only + # for the default listener. Named profiles must fail closed rather + # than inherit the listener owner's key. + if not is_named_profile: + return None + logger.warning( + "API server rejected request for profile %r: no profile-scoped " + "API_SERVER_KEY is configured; %s", + profile, + self._request_audit_log_suffix(request), + ) + return web.json_response( + { + "error": { + "message": "Invalid gateway API key (API_SERVER_KEY)", + "type": "gateway_auth_error", + "code": "gateway_auth_failed", + } + }, + status=401, + ) auth_header = request.headers.get("Authorization", "") if auth_header.startswith("Bearer "): @@ -1528,7 +1575,7 @@ class APIServerAdapter(BasePlatformAdapter): # otherwise crash this handler (500) instead of returning a clean # 401. Encoding both sides keeps the timing-safe comparison and # matches web_server.py's dashboard-token check. - if hmac.compare_digest(token.encode(), self._api_key.encode()): + if hmac.compare_digest(token.encode(), expected_key.encode()): return None # Auth OK logger.warning( diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 85ccedd13ca6..1124666e7589 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -559,6 +559,28 @@ class WebhookAdapter(BasePlatformAdapter): return _PROFILE_REJECTED return profile + @staticmethod + def _route_allows_profile( + route_config: dict, + request_profile: Optional[str], + ) -> bool: + """Return whether a route is bound to the URL-selected profile. + + Omitting ``profile`` keeps a route on the default profile. An explicit + null, blank, or non-string value is malformed and fails closed. + """ + if "profile" not in route_config: + configured_profile = "default" + else: + configured_profile = route_config.get("profile") + if not isinstance(configured_profile, str): + return False + configured_profile = configured_profile.strip() + if not configured_profile: + return False + effective_profile = request_profile or "default" + return configured_profile == effective_profile + async def _handle_webhook(self, request: "web.Request") -> "web.Response": """POST /webhooks/{route_name} — receive and process a webhook event.""" # Hot-reload dynamic subscriptions on each request (mtime-gated, cheap) @@ -579,6 +601,19 @@ class WebhookAdapter(BasePlatformAdapter): {"error": f"Unknown route: {route_name}"}, status=404 ) + if not self._route_allows_profile(route_config, profile): + effective_profile = profile or "default" + logger.warning( + "[webhook] Route %s is not authorized for profile %r", + route_name, + effective_profile, + ) + # Match the unknown-route response so callers cannot use profile + # mismatches to enumerate route bindings. + return web.json_response( + {"error": f"Unknown route: {route_name}"}, status=404 + ) + # Disabled routes are kept in the subscriptions file (so the dashboard # can re-enable them) but reject incoming events. Default-enabled: # only an explicit ``enabled: false`` turns a route off, matching the diff --git a/tests/gateway/test_api_server_multiplex_secret_scope.py b/tests/gateway/test_api_server_multiplex_secret_scope.py index c30d868ffb15..bb4fbff145de 100644 --- a/tests/gateway/test_api_server_multiplex_secret_scope.py +++ b/tests/gateway/test_api_server_multiplex_secret_scope.py @@ -79,3 +79,171 @@ class TestProfileScopeDefaultFallback: with adapter._profile_scope("worker"): assert ss.get_secret("OPENROUTER_BASE_URL") == "https://worker.example/v1" assert ss.current_secret_scope() is None + +# Regression coverage for #72041: profile-bound API authentication +class TestProfileScopedApiAuthentication: + @staticmethod + def _request(token: str): + from types import SimpleNamespace + + return SimpleNamespace( + headers={"Authorization": f"Bearer {token}"}, + remote="127.0.0.1", + transport=None, + method="GET", + path_qs="/p/worker/v1/models", + ) + + def test_named_profile_rejects_default_listener_key( + self, adapter, tmp_path, monkeypatch + ): + from gateway.platforms.api_server import _api_request_profile + + profile_home = tmp_path / "profiles" / "worker" + profile_home.mkdir(parents=True) + profile_key = "worker-profile-api-key-123456" + default_key = "default-listener-api-key-123456" + (profile_home / ".env").write_text( + f"API_SERVER_KEY={profile_key}\n", + encoding="utf-8", + ) + monkeypatch.setattr( + "hermes_cli.profiles.get_profile_dir", + lambda name: profile_home, + ) + adapter._api_key = default_key + ss.set_multiplex_active(True) + + profile_token = _api_request_profile.set("worker") + try: + with adapter._profile_scope("worker"): + assert adapter._check_auth(self._request(profile_key)) is None + + rejected = adapter._check_auth(self._request(default_key)) + assert rejected is not None + assert rejected.status == 401 + finally: + _api_request_profile.reset(profile_token) + + def test_named_profile_without_key_fails_closed( + self, adapter, tmp_path, monkeypatch + ): + from gateway.platforms.api_server import _api_request_profile + + profile_home = tmp_path / "profiles" / "worker" + profile_home.mkdir(parents=True) + default_key = "default-listener-api-key-123456" + monkeypatch.setattr( + "hermes_cli.profiles.get_profile_dir", + lambda name: profile_home, + ) + adapter._api_key = default_key + ss.set_multiplex_active(True) + + profile_token = _api_request_profile.set("worker") + try: + with adapter._profile_scope("worker"): + rejected = adapter._check_auth(self._request(default_key)) + assert rejected is not None + assert rejected.status == 401 + finally: + _api_request_profile.reset(profile_token) + + +@pytest.mark.asyncio +async def test_profile_middleware_binds_auth_before_handler( + adapter, tmp_path, monkeypatch +): + from aiohttp import web + from aiohttp.test_utils import TestClient, TestServer + from gateway.config import GatewayConfig + from gateway.platforms.api_server import _api_request_profile + + worker_home = tmp_path / "profiles" / "worker" + worker_home.mkdir(parents=True) + profile_key = "a" * 32 + default_key = "b" * 32 + (worker_home / ".env").write_text( + f"API_SERVER_KEY={profile_key}\n", encoding="utf-8" + ) + adapter._api_key = default_key + adapter.gateway_runner = type( + "_Runner", (), {"config": GatewayConfig(multiplex_profiles=True)} + )() + monkeypatch.setattr( + "hermes_cli.profiles.profiles_to_serve", + lambda multiplex: [("default", tmp_path), ("worker", worker_home)], + ) + monkeypatch.setattr( + "hermes_cli.profiles.get_profile_dir", + lambda name: tmp_path if name == "default" else worker_home, + ) + ss.set_multiplex_active(True) + + async def authenticated(request): + auth_error = adapter._check_auth(request) + if auth_error is not None: + return auth_error + return web.json_response( + {"profile": _api_request_profile.get() or "default"} + ) + + app = web.Application( + middlewares=[adapter._make_profile_prefix_middleware()] + ) + app.router.add_get("/v1/test", authenticated) + app.router.add_get("/p/{profile}/v1/test", authenticated) + + async with TestClient(TestServer(app)) as client: + default_response = await client.get( + "/v1/test", + headers={"Authorization": f"Bearer {default_key}"}, + ) + assert default_response.status == 200 + + default_alias = await client.get( + "/p/default/v1/test", + headers={"Authorization": f"Bearer {default_key}"}, + ) + assert default_alias.status == 200 + + rejected = await client.get( + "/p/worker/v1/test", + headers={"Authorization": f"Bearer {default_key}"}, + ) + assert rejected.status == 401 + + accepted = await client.get( + "/p/worker/v1/test", + headers={"Authorization": f"Bearer {profile_key}"}, + ) + assert accepted.status == 200 + assert (await accepted.json())["profile"] == "worker" + + +def test_named_profile_rejects_weak_profile_key( + adapter, tmp_path, monkeypatch +): + from gateway.platforms.api_server import _api_request_profile + + worker_home = tmp_path / "profiles" / "worker" + worker_home.mkdir(parents=True) + (worker_home / ".env").write_text( + "API_SERVER_KEY=short\n", encoding="utf-8" + ) + monkeypatch.setattr( + "hermes_cli.profiles.get_profile_dir", lambda name: worker_home + ) + adapter._api_key = "b" * 32 + ss.set_multiplex_active(True) + + token = _api_request_profile.set("worker") + try: + with adapter._profile_scope("worker"): + rejected = adapter._check_auth( + TestProfileScopedApiAuthentication._request("short") + ) + assert rejected is not None + assert rejected.status == 401 + finally: + _api_request_profile.reset(token) diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index d9929fa13534..349360350876 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -1798,3 +1798,131 @@ class TestDualStackBind: await adapter.disconnect() blocker.close() await blocker.wait_closed() + +# Regression coverage for #72041: profile-bound webhook authentication +class TestMultiplexProfileWebhookAuthentication: + @staticmethod + def _configure_profiles(adapter, tmp_path, monkeypatch): + runner = MagicMock() + runner.config.multiplex_profiles = True + adapter.gateway_runner = runner + monkeypatch.setattr( + "hermes_cli.profiles.profiles_to_serve", + lambda multiplex: [ + ("default", tmp_path), + ("worker", tmp_path / "profiles" / "worker"), + ("other", tmp_path / "profiles" / "other"), + ], + ) + + @staticmethod + def _app(adapter): + app = _create_app(adapter) + app.router.add_post( + "/p/{profile}/webhooks/{route_name}", + adapter._handle_webhook, + ) + return app + + @staticmethod + def _headers(body: bytes, secret: str): + return { + "Content-Type": "application/json", + "X-Hub-Signature-256": _github_signature(body, secret), + # Stop after successful authentication without dispatching an + # agent run; the route accepts only pull_request events. + "X-GitHub-Event": "push", + } + + @pytest.mark.asyncio + async def test_route_secret_is_bound_to_named_profile( + self, tmp_path, monkeypatch + ): + route_secret = "worker-route-secret-abc123" + adapter = _make_adapter( + routes={ + "gh": { + "profile": "worker", + "secret": route_secret, + "events": ["pull_request"], + "prompt": "PR: {action}", + } + }, + host="127.0.0.1", + ) + self._configure_profiles(adapter, tmp_path, monkeypatch) + body = b'{"action":"opened"}' + headers = self._headers(body, route_secret) + + async with TestClient(TestServer(self._app(adapter))) as cli: + accepted = await cli.post( + "/p/worker/webhooks/gh", + data=body, + headers=headers, + ) + assert accepted.status == 200 + assert (await accepted.json())["status"] == "ignored" + + wrong_profile = await cli.post( + "/p/other/webhooks/gh", + data=body, + headers=headers, + ) + assert wrong_profile.status == 404 + + default_profile = await cli.post( + "/webhooks/gh", + data=body, + headers=headers, + ) + assert default_profile.status == 404 + + @pytest.mark.asyncio + async def test_unbound_route_remains_default_profile_only( + self, tmp_path, monkeypatch + ): + route_secret = "default-route-secret-abc123" + adapter = _make_adapter( + routes={ + "gh": { + "secret": route_secret, + "events": ["pull_request"], + "prompt": "PR: {action}", + } + }, + host="127.0.0.1", + ) + self._configure_profiles(adapter, tmp_path, monkeypatch) + body = b'{"action":"opened"}' + headers = self._headers(body, route_secret) + + async with TestClient(TestServer(self._app(adapter))) as cli: + accepted = await cli.post( + "/webhooks/gh", + data=body, + headers=headers, + ) + assert accepted.status == 200 + assert (await accepted.json())["status"] == "ignored" + + named_profile = await cli.post( + "/p/worker/webhooks/gh", + data=body, + headers=headers, + ) + assert named_profile.status == 404 + + + +def test_route_profile_validation_fails_closed(): + assert WebhookAdapter._route_allows_profile({}, None) is True + assert WebhookAdapter._route_allows_profile( + {"profile": "worker"}, "worker" + ) is True + assert WebhookAdapter._route_allows_profile( + {"profile": "worker"}, "other" + ) is False + for malformed in (None, "", " ", 123, ["worker"]): + assert WebhookAdapter._route_allows_profile( + {"profile": malformed}, "worker" + ) is False diff --git a/website/docs/user-guide/messaging/webhooks.md b/website/docs/user-guide/messaging/webhooks.md index e8bc90b2924a..71262d35291d 100644 --- a/website/docs/user-guide/messaging/webhooks.md +++ b/website/docs/user-guide/messaging/webhooks.md @@ -80,6 +80,7 @@ Routes define how different webhook sources are handled. Each route is a named e |----------|----------|-------------| | `events` | No | List of event types to accept (e.g. `["pull_request"]`). If empty, all events are accepted. Event type is read from `X-GitHub-Event`, `X-GitLab-Event`, or `event_type` in the payload. | | `secret` | **Yes** | HMAC secret for signature validation. Falls back to the global `secret` if not set on the route. Set to `"INSECURE_NO_AUTH"` for testing only (skips validation). | +| `profile` | No | Profile authorized to execute this route when `gateway.multiplex_profiles` is enabled. Omit it for a default-profile-only route; set a profile name (for example `coder`) to bind the route and its secret to `/p/coder/webhooks/`. | | `prompt` | No | Template string with dot-notation payload access (e.g. `{pull_request.title}`). If omitted, the full JSON payload is dumped into the prompt. Payload fields are untrusted — see [Authenticated does not mean trusted](#authenticated-does-not-mean-trusted). | | `filters` | No | Declarative payload filters evaluated after auth/body/event filtering and before agent or direct delivery work. Non-matches return `{"status":"ignored","reason":"filter"}` with HTTP 200. | | `script` | No | Filter/transform script under `~/.hermes/scripts/`. The webhook payload is passed as JSON on stdin. JSON object stdout replaces the payload before templating; text stdout is exposed as `script_output`; empty stdout, `[SILENT]`, or a nonzero exit code ignores the webhook. | @@ -464,6 +465,11 @@ If a secret is configured but no recognized signature header is present, the req Every route must have a secret — either set directly on the route or inherited from the global `secret`. Routes without a secret cause the adapter to fail at startup with an error. For development/testing only, you can set the secret to `"INSECURE_NO_AUTH"` to skip validation entirely. +When multi-profile routing is enabled, the route's `profile` field also +binds that secret to one execution target. A route without `profile` is +default-profile-only. A request carrying a valid route signature is still +rejected if its `/p//` prefix does not match the route binding. + `INSECURE_NO_AUTH` is only accepted when the gateway is bound to a loopback host (`127.0.0.1`, `localhost`, `::1`). If it is combined with a non-loopback bind such as `0.0.0.0` or a LAN IP, the adapter refuses to start — this prevents accidentally exposing an unauthenticated endpoint on a public interface. ### Rate limiting diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index e4251506b7ce..515856e96965 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -158,6 +158,24 @@ Port-binding platforms covered by this rule: `webhook`, `api_server`, `whatsapp_cloud`, `line`. Configure any of these **only on the default profile**; every profile is reachable through its `/p//` prefix. +Authentication follows the profile named in the URL. Unprefixed endpoints keep +using the default listener's existing credentials. + +- `/p/coder/...` API-server requests must use `API_SERVER_KEY` from + `~/.hermes/profiles/coder/.env`; the default listener key is rejected. +- A webhook route that targets `coder` must declare `profile: coder` beside + its existing route-specific `secret` in the default profile's + `config.yaml`. That secret is then accepted only at + `/p/coder/webhooks/` and is rejected on every other profile prefix. +- Webhook routes without `profile` remain default-profile routes and are not + reachable through a named profile prefix. + +Keep port-binding platforms disabled in secondary profile configs. The shared +listener and its route definitions stay on the default profile; profile +binding controls which profile each authenticated webhook route may execute. +Named API requests fail closed when the target profile has no +`API_SERVER_KEY`. + Only this shared-listener conflict degrades to a skipped profile. Security configuration errors remain fatal: for example, an `open` own-policy platform without `GATEWAY_ALLOW_ALL_USERS` or its platform-specific allow-all opt-in