From 683059feb54f511717c831e0989e0f3c54450785 Mon Sep 17 00:00:00 2001 From: Drexuxux Date: Wed, 22 Jul 2026 19:33:50 +0300 Subject: [PATCH] fix(api_server): fail closed when API_SERVER_KEY strength can't be verified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_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. --- gateway/platforms/api_server.py | 39 +++++++++++++------- tests/gateway/test_api_server.py | 63 ++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 43c3486806f9..b783f34db468 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -5491,19 +5491,32 @@ class APIServerAdapter(BasePlatformAdapter): 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 + except Exception as exc: + # Fail CLOSED. This guard is the only thing between a guessable + # key and a terminal-capable endpoint, so "the check could not be + # run" must not resolve to "start anyway" — the same posture + # tools/credential_files.py takes when its deny-list cannot be + # consulted. + logger.error( + "[%s] Refusing to start: API_SERVER_KEY strength could not be " + "verified (%s: %s), and this endpoint dispatches " + "terminal-capable agent work. Repair the installation before " + "starting the API server on %s.", + self.name, type(exc).__name__, exc, self._host, + ) + 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 async def connect(self, *, is_reconnect: bool = False) -> bool: diff --git a/tests/gateway/test_api_server.py b/tests/gateway/test_api_server.py index 53beee7fcd3e..9a5981923553 100644 --- a/tests/gateway/test_api_server.py +++ b/tests/gateway/test_api_server.py @@ -4787,3 +4787,66 @@ class TestSessionDbOffEventLoop: hermes_state.SessionDB = original_class auth_adapter._session_db = 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