mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(gateway): bind HTTP auth to routed profiles
This commit is contained in:
parent
3d30232eba
commit
8c5e846536
6 changed files with 405 additions and 3 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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/<route>`. |
|
||||
| `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/<profile>/` 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
|
||||
|
|
|
|||
|
|
@ -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/<profile>/` 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/<route>` 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue