mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(api-server): require auth for /health/detailed and fail closed on weak keys
/health/detailed leaked runtime state (gateway state, connected platforms, active-agent counts, PID, exit reason) with no auth. Gate it behind the same Bearer auth as other API routes; plain /health stays open for liveness probes. Also refuse to start on a placeholder/too-short (<16 char) API_SERVER_KEY regardless of bind address — a guessable key on a terminal-capable endpoint is RCE-adjacent even on loopback, since any local process can reach it. The required-key check was already unconditional; this extends the strength floor to loopback binds too. Startup guards are hoisted above app/background-task creation so a rejected start leaves no partial state. Salvaged from #44073 (external-surface hardening), split into a focused PR per maintainer request. Co-authored-by: Hermes Agent <agent@nousresearch.com>
This commit is contained in:
parent
5126902f1d
commit
2d8d08cae6
3 changed files with 77 additions and 46 deletions
|
|
@ -1165,8 +1165,12 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
|
||||
Returns gateway state, connected platforms, PID, and uptime so the
|
||||
dashboard can display full status without needing a shared PID file or
|
||||
/proc access. No authentication required.
|
||||
/proc access. Requires the same Bearer auth as other API routes.
|
||||
"""
|
||||
auth_err = self._check_auth(request)
|
||||
if auth_err:
|
||||
return auth_err
|
||||
|
||||
from gateway.status import (
|
||||
derive_gateway_busy,
|
||||
derive_gateway_drainable,
|
||||
|
|
@ -4454,12 +4458,60 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
# BasePlatformAdapter interface
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def _api_key_passes_startup_guard(self) -> bool:
|
||||
"""Return True when API_SERVER_KEY is present and strong enough to start."""
|
||||
if not self._api_key:
|
||||
logger.error(
|
||||
"[%s] Refusing to start: API_SERVER_KEY is required for the API server, "
|
||||
"including loopback-only binds on %s.",
|
||||
self.name, self._host,
|
||||
)
|
||||
return False
|
||||
|
||||
try:
|
||||
from hermes_cli.auth import has_usable_secret
|
||||
if not has_usable_secret(self._api_key, min_length=16):
|
||||
logger.error(
|
||||
"[%s] Refusing to start: API_SERVER_KEY is a "
|
||||
"placeholder or too short (<16 chars). This endpoint "
|
||||
"dispatches terminal-capable agent work — a guessable "
|
||||
"key is remote code execution. Generate a strong secret "
|
||||
"(e.g. `openssl rand -hex 32`) and set API_SERVER_KEY "
|
||||
"before starting the API server on %s.",
|
||||
self.name, self._host,
|
||||
)
|
||||
return False
|
||||
except ImportError:
|
||||
pass
|
||||
return True
|
||||
|
||||
def _port_is_available(self) -> bool:
|
||||
"""Return True when the configured listen port is free."""
|
||||
try:
|
||||
with _socket.socket(_socket.AF_INET, _socket.SOCK_STREAM) as _s:
|
||||
_s.settimeout(1)
|
||||
_s.connect(('127.0.0.1', self._port))
|
||||
logger.error(
|
||||
"[%s] Port %d already in use. Set a different port in config.yaml: "
|
||||
"platforms.api_server.port",
|
||||
self.name, self._port,
|
||||
)
|
||||
return False
|
||||
except (ConnectionRefusedError, OSError):
|
||||
return True
|
||||
|
||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||
"""Start the aiohttp web server."""
|
||||
if not AIOHTTP_AVAILABLE:
|
||||
logger.warning("[%s] aiohttp not installed", self.name)
|
||||
return False
|
||||
|
||||
if not self._api_key_passes_startup_guard():
|
||||
return False
|
||||
|
||||
if not self._port_is_available():
|
||||
return False
|
||||
|
||||
try:
|
||||
mws = [mw for mw in (cors_middleware, body_limit_middleware, security_headers_middleware) if mw is not None]
|
||||
self._app = web.Application(middlewares=mws, client_max_size=MAX_REQUEST_BYTES)
|
||||
|
|
@ -4520,39 +4572,6 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
if hasattr(sweep_task, "add_done_callback"):
|
||||
sweep_task.add_done_callback(self._background_tasks.discard)
|
||||
|
||||
# Refuse to start without authentication. The API server can
|
||||
# dispatch terminal-capable agent work, so every deployment needs
|
||||
# an explicit API_SERVER_KEY regardless of bind address.
|
||||
if not self._api_key:
|
||||
logger.error(
|
||||
"[%s] Refusing to start: API_SERVER_KEY is required for the API server, "
|
||||
"including loopback-only binds on %s.",
|
||||
self.name, self._host,
|
||||
)
|
||||
return False
|
||||
|
||||
# Refuse to start network-accessible with a placeholder or weak key.
|
||||
# Ported from openclaw/openclaw#64586; entropy floor raised to 16 in
|
||||
# the June 2026 hermes-0day hardening (an 8-char key dispatching
|
||||
# terminal-capable agent work on a public bind is brute-forceable).
|
||||
if is_network_accessible(self._host) and self._api_key:
|
||||
try:
|
||||
from hermes_cli.auth import has_usable_secret
|
||||
if not has_usable_secret(self._api_key, min_length=16):
|
||||
logger.error(
|
||||
"[%s] Refusing to start: API_SERVER_KEY is a "
|
||||
"placeholder or too short (<16 chars) for a "
|
||||
"network-accessible bind. This endpoint dispatches "
|
||||
"terminal-capable agent work — a guessable key is "
|
||||
"remote code execution. Generate a strong secret "
|
||||
"(e.g. `openssl rand -hex 32`) and set "
|
||||
"API_SERVER_KEY before exposing it on %s.",
|
||||
self.name, self._host,
|
||||
)
|
||||
return False
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
# Loud warning when a network-accessible API server runs against an
|
||||
# unsandboxed local terminal backend. The API server can drive the
|
||||
# agent's terminal/file tools as the host user; on a public bind
|
||||
|
|
@ -4581,16 +4600,6 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
self.name, self._host,
|
||||
)
|
||||
|
||||
# Port conflict detection — fail fast if port is already in use
|
||||
try:
|
||||
with _socket.socket(_socket.AF_INET, _socket.SOCK_STREAM) as _s:
|
||||
_s.settimeout(1)
|
||||
_s.connect(('127.0.0.1', self._port))
|
||||
logger.error('[%s] Port %d already in use. Set a different port in config.yaml: platforms.api_server.port', self.name, self._port)
|
||||
return False
|
||||
except (ConnectionRefusedError, OSError):
|
||||
pass # port is free
|
||||
|
||||
self._runner = web.AppRunner(self._app)
|
||||
await self._runner.setup()
|
||||
self._site = web.TCPSite(self._runner, self._host, self._port)
|
||||
|
|
|
|||
|
|
@ -772,12 +772,21 @@ class TestHealthDetailedEndpoint:
|
|||
assert data["gateway_drainable"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_health_detailed_does_not_require_auth(self, auth_adapter):
|
||||
"""Health detailed endpoint should be accessible without auth, like /health."""
|
||||
async def test_health_detailed_requires_auth(self, auth_adapter):
|
||||
"""Detailed health must not leak runtime state without Bearer auth."""
|
||||
app = _create_app(auth_adapter)
|
||||
with patch("gateway.status.read_runtime_status", return_value=None):
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.get("/health/detailed")
|
||||
assert resp.status == 401
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_health_detailed_allows_authenticated_request(self, auth_adapter):
|
||||
app = _create_app(auth_adapter)
|
||||
headers = {"Authorization": f"Bearer {auth_adapter._api_key}"}
|
||||
with patch("gateway.status.read_runtime_status", return_value={"gateway_state": "running"}):
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.get("/health/detailed", headers=headers)
|
||||
assert resp.status == 200
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -119,6 +119,19 @@ class TestConnectBindGuard:
|
|||
assert is_network_accessible(adapter._host) is False
|
||||
result = await adapter.connect()
|
||||
assert result is False
|
||||
assert adapter._app is None
|
||||
assert adapter._background_tasks == set()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_refuses_weak_key_without_partial_startup(self):
|
||||
"""Weak API_SERVER_KEY rejection must not create app or background tasks."""
|
||||
adapter = APIServerAdapter(
|
||||
PlatformConfig(enabled=True, extra={"host": "127.0.0.1", "key": "short"}),
|
||||
)
|
||||
result = await adapter.connect()
|
||||
assert result is False
|
||||
assert adapter._app is None
|
||||
assert adapter._background_tasks == set()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allows_wildcard_with_key(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue