diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index c52488d..251678b 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -78,6 +78,6 @@ async def test_zigbee_connection( raise HTTPException(status_code=500, detail=str(exc)) from exc except (ConnectionError, TimeoutError) as exc: return ZigbeeTestConnectionResponse(connected=False, message=str(exc)) - except Exception as exc: + except Exception: logger.exception("Unexpected error during connection test") - return ZigbeeTestConnectionResponse(connected=False, message=f"Unexpected error: {exc}") + return ZigbeeTestConnectionResponse(connected=False, message="Unexpected error") diff --git a/backend/app/services/zigbee_service.py b/backend/app/services/zigbee_service.py index b4234f4..ee57e58 100644 --- a/backend/app/services/zigbee_service.py +++ b/backend/app/services/zigbee_service.py @@ -21,6 +21,30 @@ _CONNECTION_TIMEOUT = 5.0 # seconds to verify broker reachability _NETWORKMAP_TIMEOUT = 10.0 # seconds to wait for the networkmap response +def _sanitize_mqtt_error(exc: BaseException) -> str: + """Return a generic, credential-free message for an MQTT error. + + The raw aiomqtt/paho error string can include the broker URI with + embedded credentials (e.g. ``mqtt://user:pass@host``) or auth-related + detail that should not leak to API clients. Map known patterns to + coarse categories; default to a generic failure message. The original + exception is logged at WARNING level for operator debugging. + """ + logger.warning("MQTT error (sanitized for client): %r", exc) + raw = str(exc).lower() + if "not authoriz" in raw or "bad user" in raw or "bad username" in raw: + return "Authentication failed" + if "refused" in raw: + return "Connection refused by broker" + if "name or service not known" in raw or "getaddrinfo" in raw or "nodename nor servname" in raw: + return "Broker hostname could not be resolved" + if "ssl" in raw or "tls" in raw or "certificate" in raw: + return "TLS handshake failed" + if "timed out" in raw or "timeout" in raw: + return "Connection to broker timed out" + return "MQTT connection failed" + + def _build_tls_context(insecure: bool) -> ssl.SSLContext: """Build an SSL context for MQTT TLS. If insecure, skip verification.""" ctx = ssl.create_default_context() @@ -231,11 +255,9 @@ async def fetch_networkmap( await asyncio.wait_for(_wait_for_response(), timeout=_NETWORKMAP_TIMEOUT) except aiomqtt.MqttError as exc: - raise ConnectionError(f"MQTT connection failed: {exc}") from exc + raise ConnectionError(_sanitize_mqtt_error(exc)) from exc except asyncio.TimeoutError as exc: - raise TimeoutError( - f"Timed out waiting for networkmap response from {mqtt_host}:{mqtt_port}" - ) from exc + raise TimeoutError("Timed out waiting for networkmap response") from exc if not response_payload: raise ValueError("Empty networkmap response received") @@ -271,8 +293,6 @@ async def test_mqtt_connection( ): return True except aiomqtt.MqttError as exc: - raise ConnectionError(f"MQTT connection failed: {exc}") from exc + raise ConnectionError(_sanitize_mqtt_error(exc)) from exc except asyncio.TimeoutError as exc: - raise TimeoutError( - f"Connection to {mqtt_host}:{mqtt_port} timed out" - ) from exc + raise TimeoutError("Connection to broker timed out") from exc diff --git a/backend/tests/test_zigbee_service.py b/backend/tests/test_zigbee_service.py index 3bf4e5c..bc76d84 100644 --- a/backend/tests/test_zigbee_service.py +++ b/backend/tests/test_zigbee_service.py @@ -437,3 +437,67 @@ async def test_test_mqtt_connection_insecure_passes_no_verify_context() -> None: ctx = mock_aiomqtt.Client.call_args.kwargs["tls_context"] assert ctx.verify_mode == ssl.CERT_NONE assert ctx.check_hostname is False + + +# --------------------------------------------------------------------------- +# Sanitize MQTT errors +# --------------------------------------------------------------------------- + +from app.services.zigbee_service import _sanitize_mqtt_error # noqa: E402 + + +def test_sanitize_auth_error_does_not_leak_credentials() -> None: + msg = _sanitize_mqtt_error( + Exception("Not authorized: bad username or password for user=admin pwd=secret") + ) + assert msg == "Authentication failed" + assert "admin" not in msg + assert "secret" not in msg + + +def test_sanitize_refused() -> None: + assert _sanitize_mqtt_error(Exception("Connection refused")) == "Connection refused by broker" + + +def test_sanitize_dns_failure_strips_host() -> None: + msg = _sanitize_mqtt_error( + Exception("[Errno 8] nodename nor servname provided, or not known: broker.internal.lan") + ) + assert msg == "Broker hostname could not be resolved" + assert "broker.internal.lan" not in msg + + +def test_sanitize_tls_error() -> None: + assert _sanitize_mqtt_error( + Exception("[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed") + ) == "TLS handshake failed" + + +def test_sanitize_unknown_falls_back_to_generic() -> None: + msg = _sanitize_mqtt_error(Exception("mqtt://admin:hunter2@broker:1883 weird state")) + assert msg == "MQTT connection failed" + assert "hunter2" not in msg + assert "admin" not in msg + + +@pytest.mark.asyncio +async def test_fetch_networkmap_does_not_leak_creds_in_connection_error() -> None: + class _FakeClient: + async def __aenter__(self): + raise Exception("Not authorized: rejected mqtt://admin:hunter2@host") + + async def __aexit__(self, *_): + pass + + with patch("app.services.zigbee_service.aiomqtt") as mock_aiomqtt: + mock_aiomqtt.Client.return_value = _FakeClient() + mock_aiomqtt.MqttError = Exception + + with pytest.raises(ConnectionError) as ei: + await fetch_networkmap( + mqtt_host="host", mqtt_port=1883, base_topic="zigbee2mqtt" + ) + msg = str(ei.value) + assert "hunter2" not in msg + assert "admin" not in msg + assert msg == "Authentication failed"