mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(api_server): fail closed when API_SERVER_KEY strength can't be verified
`_api_key_passes_startup_guard` refuses to start the API server on a weak
`API_SERVER_KEY`, and its own log says why:
This endpoint dispatches terminal-capable agent work — a guessable key
is remote code execution.
But the check is wrapped so that a failure to import it starts the server
anyway:
try:
from hermes_cli.auth import has_usable_secret
if not has_usable_secret(self._api_key, min_length=16):
... return False
except ImportError:
pass
return True
`hermes_cli.auth` imports httpx at module scope and pulls in a large slice of
the CLI, so an import failure is not hypothetical — a trimmed image, a partial
install, or a circular import during gateway startup all produce one. When it
happens the strength check silently disappears and only the presence check
above it remains, so a placeholder key passes.
Reproduced against the real guard with the import blocked:
weak key, normal : False
weak key, ImportError : True <-- starts on a 4-char key
strong key, normal : True
Fail closed instead: an unverifiable key does not get to expose the endpoint,
and the log names the actual problem so the operator can repair the install.
This is the posture tools/credential_files.py already takes — it refuses a
mount when its deny-list cannot be consulted rather than risking it. The catch
also widens from ImportError to Exception, so an AttributeError or an error
raised inside the check cannot reopen the same hole.
Both happy paths are untouched: a strong key still starts, a weak or missing
key is still refused with the existing messages.
Unrelated to #38803, which fixes the retry behaviour after this guard rejects
and assumes the guard ran.
tests/gateway/test_api_server.py: new TestApiKeyStartupGuardFailsClosed — a
weak key is refused when the check is unavailable, a strong key is refused too
(fail-closed), plus three controls pinning the unchanged normal paths. The two
fail-open tests fail on main; the three controls pass there. 222 passed in the
api_server suites; 1475 passed across every suite touching api_server, with
the same 8 pre-existing failures on clean main.
This commit is contained in:
parent
2755bf558e
commit
683059feb5
2 changed files with 89 additions and 13 deletions
|
|
@ -5491,19 +5491,32 @@ class APIServerAdapter(BasePlatformAdapter):
|
||||||
|
|
||||||
try:
|
try:
|
||||||
from hermes_cli.auth import has_usable_secret
|
from hermes_cli.auth import has_usable_secret
|
||||||
if not has_usable_secret(self._api_key, min_length=16):
|
except Exception as exc:
|
||||||
logger.error(
|
# Fail CLOSED. This guard is the only thing between a guessable
|
||||||
"[%s] Refusing to start: API_SERVER_KEY is a "
|
# key and a terminal-capable endpoint, so "the check could not be
|
||||||
"placeholder or too short (<16 chars). This endpoint "
|
# run" must not resolve to "start anyway" — the same posture
|
||||||
"dispatches terminal-capable agent work — a guessable "
|
# tools/credential_files.py takes when its deny-list cannot be
|
||||||
"key is remote code execution. Generate a strong secret "
|
# consulted.
|
||||||
"(e.g. `openssl rand -hex 32`) and set API_SERVER_KEY "
|
logger.error(
|
||||||
"before starting the API server on %s.",
|
"[%s] Refusing to start: API_SERVER_KEY strength could not be "
|
||||||
self.name, self._host,
|
"verified (%s: %s), and this endpoint dispatches "
|
||||||
)
|
"terminal-capable agent work. Repair the installation before "
|
||||||
return False
|
"starting the API server on %s.",
|
||||||
except ImportError:
|
self.name, type(exc).__name__, exc, self._host,
|
||||||
pass
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
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
|
||||||
return True
|
return True
|
||||||
|
|
||||||
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
async def connect(self, *, is_reconnect: bool = False) -> bool:
|
||||||
|
|
|
||||||
|
|
@ -4787,3 +4787,66 @@ class TestSessionDbOffEventLoop:
|
||||||
hermes_state.SessionDB = original_class
|
hermes_state.SessionDB = original_class
|
||||||
auth_adapter._session_db = None
|
auth_adapter._session_db = None
|
||||||
auth_adapter._session_db_lock = None
|
auth_adapter._session_db_lock = None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# _api_key_passes_startup_guard — fail-closed on an unverifiable key
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestApiKeyStartupGuardFailsClosed:
|
||||||
|
"""The guard is the only thing between a guessable key and an endpoint the
|
||||||
|
code itself describes as ``terminal-capable agent work`` where "a guessable
|
||||||
|
key is remote code execution".
|
||||||
|
|
||||||
|
So "the strength check could not be run" must never resolve to "start
|
||||||
|
anyway" — the same posture ``tools/credential_files.py`` takes when its
|
||||||
|
deny-list cannot be consulted.
|
||||||
|
"""
|
||||||
|
|
||||||
|
class _Stub:
|
||||||
|
name = "api_server"
|
||||||
|
_host = "0.0.0.0"
|
||||||
|
|
||||||
|
def __init__(self, key):
|
||||||
|
self._api_key = key
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _guard(key):
|
||||||
|
return APIServerAdapter._api_key_passes_startup_guard(
|
||||||
|
TestApiKeyStartupGuardFailsClosed._Stub(key)
|
||||||
|
)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _blocking_auth_import():
|
||||||
|
real_import = __import__
|
||||||
|
|
||||||
|
def _blocked(name, *args, **kwargs):
|
||||||
|
if name == "hermes_cli.auth":
|
||||||
|
raise ImportError("simulated: hermes_cli.auth unavailable")
|
||||||
|
return real_import(name, *args, **kwargs)
|
||||||
|
|
||||||
|
return patch("builtins.__import__", _blocked)
|
||||||
|
|
||||||
|
def test_weak_key_refused_when_check_is_unavailable(self):
|
||||||
|
"""The bug: an unimportable auth module silently dropped the check and
|
||||||
|
the server started on a 4-character key."""
|
||||||
|
with self._blocking_auth_import():
|
||||||
|
assert self._guard("test") is False
|
||||||
|
|
||||||
|
def test_strong_key_also_refused_when_check_is_unavailable(self):
|
||||||
|
"""Fail-closed: we cannot verify the key, so we do not expose the
|
||||||
|
endpoint — the log tells the operator to repair the install."""
|
||||||
|
with self._blocking_auth_import():
|
||||||
|
assert self._guard("a" * 40) is False
|
||||||
|
|
||||||
|
def test_strong_key_still_starts_normally(self):
|
||||||
|
"""Control: the happy path is unchanged."""
|
||||||
|
assert self._guard("a" * 40) is True
|
||||||
|
|
||||||
|
def test_weak_key_still_refused_normally(self):
|
||||||
|
"""Control: the original rejection is unchanged."""
|
||||||
|
assert self._guard("test") is False
|
||||||
|
|
||||||
|
def test_missing_key_still_refused(self):
|
||||||
|
"""Control: the empty-key branch is unchanged."""
|
||||||
|
assert self._guard("") is False
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue