From 35765a5d824199d1ec6bd4025fec60331bc88964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20P=C3=A9rez?= Date: Sat, 19 Sep 2026 20:23:38 -0300 Subject: [PATCH] fix(config): reject PROXY and OBP_PROXY sharing the same LISTEN_PORT Both listeners bind independent UDP sockets; if both are configured to the same port only the first one to start would succeed, and the server would fail at bind time with an unclear error instead of a config validation message. --- .../infrastructure/config_validator.py | 31 +++++++++++-------- tests/infrastructure/test_config_validator.py | 20 ++++++++++++ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/adn_server/infrastructure/config_validator.py b/src/adn_server/infrastructure/config_validator.py index d87bc69..6620d3e 100644 --- a/src/adn_server/infrastructure/config_validator.py +++ b/src/adn_server/infrastructure/config_validator.py @@ -202,14 +202,14 @@ def _validate_logger(logger_cfg: dict[str, Any], errors: list[str]) -> None: _section_string_keys("LOGGER", logger_cfg, LOGGER_STRING_KEYS, errors) -def _validate_proxy(proxy_cfg: dict[str, Any] | None, systems: dict[str, Any], errors: list[str]) -> None: +def _validate_proxy(proxy_cfg: dict[str, Any] | None, systems: dict[str, Any], errors: list[str]) -> int | None: from adn_server.application.proxy.deployment import config_has_enabled_master if not isinstance(systems, dict) or not config_has_enabled_master({"SYSTEMS": systems}): - return + return None if not proxy_cfg or not isinstance(proxy_cfg, dict): errors.append("PROXY: required when config has enabled MASTER systems (adn-server).") - return + return None for key in ("DEBUG", "CLIENT_INFO", "STATS"): if key in proxy_cfg: _expect_bool(f"PROXY.{key}", proxy_cfg[key], errors) @@ -236,18 +236,19 @@ def _validate_proxy(proxy_cfg: dict[str, Any] | None, systems: dict[str, Any], e listen_port = proxy_cfg.get("LISTEN_PORT", 62031) if isinstance(listen_port, bool) or not isinstance(listen_port, int) or listen_port < 1: errors.append("PROXY.LISTEN_PORT: required >= 1.") + listen_port = None target = proxy_cfg.get("TARGET_SYSTEM") if _is_empty(target): errors.append("PROXY.TARGET_SYSTEM: required.") - return + return listen_port if not isinstance(systems, dict) or target not in systems: errors.append(f"PROXY.TARGET_SYSTEM: unknown system {target!r}.") - return + return listen_port target_cfg = systems[target] if not isinstance(target_cfg, dict): errors.append(f"SYSTEMS.{target}: expected mapping.") - return + return listen_port if not target_cfg.get("ENABLED", True): errors.append(f"PROXY.TARGET_SYSTEM: SYSTEMS.{target} must be ENABLED.") if target_cfg.get("MODE") != "MASTER": @@ -263,17 +264,18 @@ def _validate_proxy(proxy_cfg: dict[str, Any] | None, systems: dict[str, Any], e errors.append( f"SYSTEMS.{target}.GENERATOR: must be 0 or 1 on proxy target (use MAX_PEERS, not GENERATOR)." ) + return listen_port def _validate_obp_proxy( obp_cfg: dict[str, Any] | None, systems: dict[str, Any], errors: list[str], -) -> None: +) -> int | None: if isinstance(obp_cfg, dict) and not obp_cfg.get("ENABLED", True): - return + return None if not isinstance(obp_cfg, dict) and not config_has_enabled_openbridge({"SYSTEMS": systems}): - return + return None effective = obp_cfg if isinstance(obp_cfg, dict) else {} for key in ("DEBUG", "BIND_LEGACY_PORTS"): if key in effective: @@ -283,12 +285,12 @@ def _validate_obp_proxy( listen_port = effective.get("LISTEN_PORT", 62032) if isinstance(listen_port, bool) or not isinstance(listen_port, int) or listen_port < 1: errors.append("OBP_PROXY.LISTEN_PORT: required >= 1 when ENABLED.") - return + return None bind_legacy = bool(effective.get("BIND_LEGACY_PORTS", True)) network_ids: dict[Any, str] = {} legacy_ports: set[int] = set() if not isinstance(systems, dict): - return + return listen_port for name, sys_cfg in systems.items(): if not isinstance(sys_cfg, dict) or not sys_cfg.get("ENABLED", True): continue @@ -328,6 +330,7 @@ def _validate_obp_proxy( f"SYSTEMS.{name}.PORT: duplicate OPENBRIDGE listen port {legacy_port}." ) legacy_ports.add(legacy_port) + return listen_port def _config_requires_database(config: dict[str, Any]) -> bool: @@ -419,13 +422,15 @@ def validate_config(config: dict[str, Any], *, config_path: str | None = None) - _validate_system(name, sys_cfg, errors) proxy_cfg = config.get("PROXY") - _validate_proxy(proxy_cfg if isinstance(proxy_cfg, dict) else None, systems if isinstance(systems, dict) else {}, errors) + proxy_port = _validate_proxy(proxy_cfg if isinstance(proxy_cfg, dict) else None, systems if isinstance(systems, dict) else {}, errors) obp_proxy_cfg = config.get("OBP_PROXY") - _validate_obp_proxy( + obp_port = _validate_obp_proxy( obp_proxy_cfg if isinstance(obp_proxy_cfg, dict) else None, systems if isinstance(systems, dict) else {}, errors, ) + if proxy_port is not None and proxy_port == obp_port: + errors.append(f"PROXY.LISTEN_PORT and OBP_PROXY.LISTEN_PORT: both {proxy_port}, would collide on bind.") if _config_requires_database(config): _validate_database(config.get("DATABASE"), errors) diff --git a/tests/infrastructure/test_config_validator.py b/tests/infrastructure/test_config_validator.py index 27ba995..108c2a1 100644 --- a/tests/infrastructure/test_config_validator.py +++ b/tests/infrastructure/test_config_validator.py @@ -45,6 +45,26 @@ def test_reports_missing_security_fields_when_url_set() -> None: assert "GLOBAL.PASS_SECURITY" in msg +def test_rejects_proxy_and_obp_proxy_same_port() -> None: + config = _minimal_config() + config["SYSTEMS"] = { + "MASTER1": {"MODE": "MASTER"}, + "OBP1": { + "MODE": "OPENBRIDGE", + "NETWORK_ID": 73044, + "PASSPHRASE": "x", + "TARGET_IP": "127.0.0.1", + "TARGET_PORT": 62044, + }, + } + config["PROXY"] = {"TARGET_SYSTEM": "MASTER1", "LISTEN_PORT": 62032} + config["OBP_PROXY"] = {"LISTEN_PORT": 62032} + config["DATABASE"] = {"DB_NAME": "x", "DB_USERNAME": "x"} + with pytest.raises(ConfigError) as exc: + validate_config(config) + assert "PROXY.LISTEN_PORT and OBP_PROXY.LISTEN_PORT" in str(exc.value) + + def test_collects_multiple_errors() -> None: with pytest.raises(ConfigError) as exc: validate_config(