fix(gateway): require usable API_SERVER_KEY to enroll the api_server platform at load time
Salvaged from PR #36180 (commits68dfeb4b16and86f437509bby arimu1), re-applied onto current main with the incidental black-reformat churn stripped out (~1,700 lines -> the semantic change + tests). Previously gateway/config.py enrolled the api_server platform on `api_server_enabled or api_server_key`, so API_SERVER_ENABLED=true with no key (or a weak/placeholder key) still loaded the platform: the adapter is instantiated (ResponseStore/SQLite opened in __init__), the reconnect watcher spins, and the startup guard refuses at connect() — logging errors forever. Now the platform is enrolled only when API_SERVER_KEY passes the same strength bar as the adapter's startup guard (has_usable_secret, min_length=16), via a shared _has_usable_api_server_key() helper. The no-op `lambda cfg: True` connected-checker for API_SERVER is also replaced with the same key check, so get_connected_platforms() only reports the platform "up" when it could actually start. Known limitation (intentionally out of scope): a YAML config with `platforms.api_server.enabled: true` and no key still loads the platform; this gate covers the env-override path only. Dropped from the original PR: EMAIL/SMS checker additions (scope creep beyond the PR title; absent on current main) and the wholesale black reformat of gateway/config.py and tests. Fixes #36111 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
1f45504609
commit
9e4b89857a
|
|
@ -0,0 +1 @@
|
|||
arimu1
|
||||
|
|
@ -789,6 +789,22 @@ class StreamingConfig:
|
|||
# platform is sufficiently configured to be considered "connected". Platforms
|
||||
# that rely on the generic ``token or api_key`` check (Telegram, Discord,
|
||||
# Slack, Matrix, Mattermost, HomeAssistant) do not need an entry here.
|
||||
def _has_usable_api_server_key(key: object) -> bool:
|
||||
"""True when API_SERVER_KEY is present and strong enough to be usable.
|
||||
|
||||
Mirrors the startup guard in ``gateway/platforms/api_server.py``
|
||||
(``has_usable_secret`` with ``min_length=16``) so the platform is only
|
||||
enrolled at load time when the adapter would actually agree to start.
|
||||
"""
|
||||
if not key:
|
||||
return False
|
||||
try:
|
||||
from hermes_cli.auth import has_usable_secret
|
||||
except ImportError:
|
||||
return len(str(key).strip()) >= 16
|
||||
return has_usable_secret(key, min_length=16)
|
||||
|
||||
|
||||
_PLATFORM_CONNECTED_CHECKERS: dict[Platform, Callable[[PlatformConfig], bool]] = {
|
||||
Platform.WEIXIN: lambda cfg: bool(
|
||||
cfg.extra.get("account_id") and (cfg.token or cfg.extra.get("token"))
|
||||
|
|
@ -797,7 +813,9 @@ _PLATFORM_CONNECTED_CHECKERS: dict[Platform, Callable[[PlatformConfig], bool]] =
|
|||
cfg.extra.get("phone_number_id") and cfg.extra.get("access_token")
|
||||
),
|
||||
Platform.SIGNAL: lambda cfg: bool(cfg.extra.get("http_url")),
|
||||
Platform.API_SERVER: lambda cfg: True,
|
||||
Platform.API_SERVER: lambda cfg: _has_usable_api_server_key(
|
||||
cfg.extra.get("key") if cfg else None
|
||||
),
|
||||
Platform.WEBHOOK: lambda cfg: True,
|
||||
Platform.MSGRAPH_WEBHOOK: lambda cfg: bool(
|
||||
str(cfg.extra.get("client_state") or "").strip()
|
||||
|
|
@ -2026,7 +2044,12 @@ def _apply_env_overrides(config: GatewayConfig) -> None:
|
|||
api_server_cors_origins = getenv("API_SERVER_CORS_ORIGINS", "")
|
||||
api_server_port = getenv("API_SERVER_PORT")
|
||||
api_server_host = getenv("API_SERVER_HOST")
|
||||
if api_server_enabled or api_server_key:
|
||||
# Require a usable key: API_SERVER_ENABLED alone would load an
|
||||
# unauthenticated platform whose adapter refuses to start at connect()
|
||||
# anyway (startup guard in gateway/platforms/api_server.py), leaving the
|
||||
# reconnect watcher spinning and logging errors forever. Same strength
|
||||
# bar as the startup guard (has_usable_secret, min_length=16).
|
||||
if _has_usable_api_server_key(api_server_key):
|
||||
if Platform.API_SERVER not in config.platforms:
|
||||
config.platforms[Platform.API_SERVER] = PlatformConfig()
|
||||
# Respect an explicit ``enabled: false`` in config.yaml (flagged by
|
||||
|
|
|
|||
|
|
@ -3097,20 +3097,35 @@ class TestConfigIntegration:
|
|||
|
||||
def test_env_override_enables_api_server(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_ENABLED", "true")
|
||||
monkeypatch.setenv("API_SERVER_KEY", "opensslrandhex32strongkey")
|
||||
from gateway.config import load_gateway_config
|
||||
config = load_gateway_config()
|
||||
assert Platform.API_SERVER in config.platforms
|
||||
assert config.platforms[Platform.API_SERVER].enabled is True
|
||||
|
||||
def test_env_override_enabled_without_key_does_not_load(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_ENABLED", "true")
|
||||
from gateway.config import load_gateway_config
|
||||
config = load_gateway_config()
|
||||
assert Platform.API_SERVER not in config.platforms
|
||||
|
||||
def test_env_override_enabled_with_weak_key_does_not_load(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_ENABLED", "true")
|
||||
monkeypatch.setenv("API_SERVER_KEY", "abcd")
|
||||
from gateway.config import load_gateway_config
|
||||
config = load_gateway_config()
|
||||
assert Platform.API_SERVER not in config.platforms
|
||||
|
||||
def test_env_override_with_key(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_KEY", "sk-mykey")
|
||||
monkeypatch.setenv("API_SERVER_KEY", "opensslrandhex32strongkey")
|
||||
from gateway.config import load_gateway_config
|
||||
config = load_gateway_config()
|
||||
assert Platform.API_SERVER in config.platforms
|
||||
assert config.platforms[Platform.API_SERVER].extra.get("key") == "sk-mykey"
|
||||
assert config.platforms[Platform.API_SERVER].extra.get("key") == "opensslrandhex32strongkey"
|
||||
|
||||
def test_env_override_port_and_host(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_ENABLED", "true")
|
||||
monkeypatch.setenv("API_SERVER_KEY", "opensslrandhex32strongkey")
|
||||
monkeypatch.setenv("API_SERVER_PORT", "9999")
|
||||
monkeypatch.setenv("API_SERVER_HOST", "0.0.0.0")
|
||||
from gateway.config import load_gateway_config
|
||||
|
|
@ -3120,6 +3135,7 @@ class TestConfigIntegration:
|
|||
|
||||
def test_env_override_cors_origins(self, monkeypatch):
|
||||
monkeypatch.setenv("API_SERVER_ENABLED", "true")
|
||||
monkeypatch.setenv("API_SERVER_KEY", "opensslrandhex32strongkey")
|
||||
monkeypatch.setenv(
|
||||
"API_SERVER_CORS_ORIGINS",
|
||||
"http://localhost:3000, http://127.0.0.1:3000",
|
||||
|
|
@ -3133,7 +3149,9 @@ class TestConfigIntegration:
|
|||
|
||||
def test_api_server_in_connected_platforms(self):
|
||||
config = GatewayConfig()
|
||||
config.platforms[Platform.API_SERVER] = PlatformConfig(enabled=True)
|
||||
config.platforms[Platform.API_SERVER] = PlatformConfig(
|
||||
enabled=True, extra={"key": "opensslrandhex32strongkey"}
|
||||
)
|
||||
connected = config.get_connected_platforms()
|
||||
assert Platform.API_SERVER in connected
|
||||
|
||||
|
|
|
|||
|
|
@ -98,8 +98,9 @@ def test_checker_returns_true_when_configured(platform, checker, monkeypatch):
|
|||
elif platform == Platform.SMS:
|
||||
monkeypatch.setenv("TWILIO_ACCOUNT_SID", "ACtest")
|
||||
mock_config.extra = {}
|
||||
elif platform == Platform.API_SERVER:
|
||||
mock_config.extra = {"key": "opensslrandhex32strongkey"}
|
||||
elif platform in {
|
||||
Platform.API_SERVER,
|
||||
Platform.WEBHOOK,
|
||||
Platform.WHATSAPP,
|
||||
}:
|
||||
|
|
@ -127,3 +128,26 @@ def test_checker_returns_true_when_configured(platform, checker, monkeypatch):
|
|||
|
||||
result = checker(mock_config)
|
||||
assert result is True, f"{platform.value} checker should return True with valid-looking config"
|
||||
|
||||
|
||||
def test_api_server_checker_key_validity():
|
||||
"""API_SERVER checker: missing, placeholder, short, and strong keys."""
|
||||
checker = _PLATFORM_CONNECTED_CHECKERS[Platform.API_SERVER]
|
||||
|
||||
cfg = MagicMock()
|
||||
|
||||
# Missing
|
||||
cfg.extra = {}
|
||||
assert checker(cfg) is False
|
||||
|
||||
# Placeholder
|
||||
cfg.extra = {"key": "changeme"}
|
||||
assert checker(cfg) is False
|
||||
|
||||
# Too short (<16 chars)
|
||||
cfg.extra = {"key": "shortkey"}
|
||||
assert checker(cfg) is False
|
||||
|
||||
# Strong key (>=16 chars, not a placeholder)
|
||||
cfg.extra = {"key": "opensslrandhex32strongkey"}
|
||||
assert checker(cfg) is True
|
||||
|
|
|
|||
Loading…
Reference in New Issue