mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
Second, deeper pass over tools/gateway/hermes_cli plus first pass over the trees wave 1 missed (acp, acp_adapter, skills, computer_use, docker, dashboard, conformance, monitoring, secret_sources, hermes_state, providers). Same rubric as wave 1 (AGENTS.md test policy); security, alternation/caching invariants, issue-number regressions, and E2E kept. Real test-quality fixes found and rooted out along the way: - tests/tools/test_command_guards.py made real auxiliary-LLM HTTPS calls (DEFAULT_CONFIG smart-approval leaked in) — pinned approval mode=manual via autouse fixture: 17.4s → 0.4s. - test_model_switch_custom_providers.py / test_user_providers_model_switch.py silently probed live provider catalogs (~2s/test) — stubbed cached_provider_model_ids/provider_model_ids/fetch_api_models. - test_telegram_noise_filter.py: 15-platform copy-paste matrix over shared gateway.run logic → 3 representative platforms (55s → 3.9s). - test_gateway_shutdown.py: stop()'s 5s interrupt-deadline loop spun on MagicMock agents — interrupt.side_effect now clears _running_agents (22s → 1.0s). - test_gateway_inactivity_timeout.py poll-harness timings shrunk 3-5x (24s → 1.1s); test_mcp_stability.py backoff/SIGTERM-grace sleeps patched (15.4s → 2.5s); test_async_delegation.py negative-drain wait 5s → 0.5s. - test_telegram_init_deadline.py: loop-block margin restored to 1.0s with rationale comment — the watchdog-dump assertion needs the loop blocked well past deadline+grace under parallel load (flaked once in the 40-worker verification run at a 0.2s margin). Verification: full hermetic suite via scripts/run_tests.sh — 2,438 files, 21,718 tests passed, 0 failed, 293.9s wall. Suite totals vs original baseline: 46,820 → 19,757 test functions (−57.8%), wall 583.5s → 293.9s (−50%), subprocess CPU 13,564s → 11,623s.
172 lines
6.2 KiB
Python
172 lines
6.2 KiB
Python
"""Tests for the API server bind-address startup guard.
|
|
|
|
Validates that is_network_accessible() correctly classifies addresses and
|
|
that connect() refuses to start without API_SERVER_KEY.
|
|
"""
|
|
|
|
import socket
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
from gateway.config import PlatformConfig
|
|
from gateway.platforms.api_server import APIServerAdapter
|
|
from gateway.platforms.base import is_network_accessible
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Unit tests: is_network_accessible()
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestIsNetworkAccessible:
|
|
"""Direct tests for the address classification helper."""
|
|
|
|
# -- Loopback (safe, should return False) --
|
|
|
|
|
|
def test_ipv4_mapped_loopback(self):
|
|
# ::ffff:127.0.0.1 — Python's is_loopback returns False for mapped
|
|
# addresses; the helper must unwrap and check ipv4_mapped.
|
|
assert is_network_accessible("::ffff:127.0.0.1") is False
|
|
|
|
# -- Network-accessible (should return True) --
|
|
|
|
|
|
def test_ipv6_wildcard(self):
|
|
# This is the bypass vector that the string-based check missed.
|
|
assert is_network_accessible("::") is True
|
|
|
|
|
|
def test_private_ipv4(self):
|
|
assert is_network_accessible("10.0.0.1") is True
|
|
|
|
|
|
def test_public_ipv4(self):
|
|
assert is_network_accessible("8.8.8.8") is True
|
|
|
|
# -- Hostname resolution --
|
|
|
|
|
|
def test_hostname_mixed_resolution(self):
|
|
"""If a hostname resolves to both loopback and non-loopback, it's
|
|
network-accessible (any non-loopback address is enough)."""
|
|
mixed_result = [
|
|
(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("127.0.0.1", 0)),
|
|
(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("10.0.0.1", 0)),
|
|
]
|
|
with patch("gateway.platforms.base._socket.getaddrinfo", return_value=mixed_result):
|
|
assert is_network_accessible("dual-host.local") is True
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Integration tests: connect() startup guard
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestConnectBindGuard:
|
|
"""Verify that connect() refuses dangerous configurations."""
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_refuses_loopback_without_key(self):
|
|
"""Loopback binds are still an auth boundary and require API_SERVER_KEY."""
|
|
adapter = APIServerAdapter(PlatformConfig(enabled=True, extra={"host": "127.0.0.1"}))
|
|
assert adapter._api_key == ""
|
|
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_allows_wildcard_with_key(self):
|
|
"""Non-loopback with a key should pass the guard."""
|
|
adapter = APIServerAdapter(
|
|
PlatformConfig(enabled=True, extra={"host": "0.0.0.0", "key": "sk-test"})
|
|
)
|
|
# The guard checks: is_network_accessible(host) AND NOT api_key
|
|
# With a key set, the guard should not block.
|
|
assert adapter._api_key == "sk-test"
|
|
assert is_network_accessible("0.0.0.0") is True
|
|
# Combined: the guard condition is False (key is set), so it passes
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Integration tests: bind mechanics (direct bind, no pre-probe — #10297)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestBindMechanics:
|
|
"""connect() binds directly instead of pre-probing 127.0.0.1.
|
|
|
|
The old ``_port_is_available()`` probe connected to 127.0.0.1 only and
|
|
reported a lingering TIME_WAIT socket as "in use", failing gateway
|
|
restarts for up to ~60s (#10297). The fix removes the probe: bind
|
|
directly, keep SO_REUSEADDR default semantics on Linux (rebind past
|
|
TIME_WAIT), and surface a real bind conflict as a clean ``False`` with
|
|
the runner torn down.
|
|
"""
|
|
|
|
_KEY = "sk-test-strong-key-0123456789"
|
|
|
|
def _make_adapter(self, port: int) -> APIServerAdapter:
|
|
return APIServerAdapter(
|
|
PlatformConfig(
|
|
enabled=True,
|
|
extra={"host": "127.0.0.1", "port": port, "key": self._KEY},
|
|
)
|
|
)
|
|
|
|
@staticmethod
|
|
def _free_port() -> int:
|
|
with socket.socket() as s:
|
|
s.bind(("", 0))
|
|
return s.getsockname()[1]
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_immediate_rebind_after_disconnect(self):
|
|
"""A restarted adapter can rebind the same port immediately.
|
|
|
|
This is the #10297 symptom: the old pre-probe (and disabled address
|
|
reuse) made a quick gateway restart fail while the previous socket
|
|
sat in TIME_WAIT.
|
|
"""
|
|
port = self._free_port()
|
|
first = self._make_adapter(port)
|
|
assert await first.connect() is True
|
|
await first.disconnect()
|
|
|
|
second = self._make_adapter(port)
|
|
try:
|
|
assert await second.connect() is True
|
|
finally:
|
|
await second.disconnect()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_port_conflict_sets_non_retryable_fatal_error(self):
|
|
"""A real port conflict (EADDRINUSE) must set a non-retryable fatal
|
|
error so the reconnect watcher drops the platform from the retry
|
|
queue instead of looping indefinitely.
|
|
|
|
Previously connect() returned bare ``False``, which the reconnect
|
|
watcher treated as retryable — retrying every 5 minutes forever,
|
|
filling errors.log and leaking 2 fds per retry (#52132: 1568+
|
|
retries over 5 days in a multi-profile setup).
|
|
"""
|
|
port = self._free_port()
|
|
first = self._make_adapter(port)
|
|
assert await first.connect() is True
|
|
second = self._make_adapter(port)
|
|
try:
|
|
result = await second.connect()
|
|
assert result is False
|
|
assert second.has_fatal_error is True
|
|
assert second.fatal_error_retryable is False
|
|
assert second.fatal_error_code == "api_server_port_in_use"
|
|
assert str(port) in (second.fatal_error_message or "")
|
|
finally:
|
|
await first.disconnect()
|
|
await second.disconnect()
|