[Bug]: Ping always shows "Connection failed" even when the connection succeeds

[benklop]

Bug Description

Both the first-run/setup ping (POST /api/setup/ping) and the instance ping (GET /api/instances/<id>/ping) always return msg: "Connection failed" in the JSON response, even when the ping actually succeeded (ok: true and a correct version are returned in the same response). This makes the "Test Connection" button in the setup wizard and the Add/Edit Instance dialog look broken/red even though Sonarr/Radarr are reachable and correctly configured.

Example response after a fully successful ping to a working Sonarr instance:

{"ok":true,"version":"4.0.19.2979","msg":"Connection failed"}

Note ok is true and version is the real Sonarr version, but msg still says "Connection failed".

Root cause

ArrClient.ping() returns an empty string as the detail on success:

mediastarr/app/main.py

Lines 1079 to 1084 in 3825518

	# ─── Activity Log ─────────────────────────────────────────────────────────────
	# ── Log levels ────────────────────────────────────────────────────────────────
	_LOG_LEVEL_MAP = {"DEBUG": logging.DEBUG, "INFO": logging.INFO,
	"WARN": logging.WARNING, "WARNING": logging.WARNING,
	"ERROR": logging.ERROR}

def ping(self):
    try:
        d = self.get("system/status")
        return True, str(d.get("version","?"))[:20], ""
    except Exception as e:
        return False, "?", summarize_ping_error(str(e)[:200])

But _safe_ping_msg() (added for the CodeQL py/stack-trace-exposure hardening in v7.1.0) only allowlists specific error strings and falls back to "Connection failed" for anything not in that list — including the empty string used for the success case:

mediastarr/app/main.py

Lines 1017 to 1029 in 3825518

	def _safe_ping_msg(detail: str) -> str:
	"""Return detail only if it is a known-safe message, else generic fallback.
	This breaks the CodeQL taint chain from exception to HTTP response.
	"""
	return detail if detail in _SAFE_PING_MESSAGES else "Connection failed"
	def summarize_ping_error(raw: str) -> str:
	"""Turn a raw exception string into a short user-readable message."""
	text = str(raw or "").strip()
	lower = text.lower()
	if not text: return "Connection failed"
	if "401" in lower or "403" in lower or "unauthorized" in lower or "forbidden" in lower:

_SAFE_PING_MESSAGES = frozenset([
    "Authentication failed", "API endpoint not found", "Host not found",
    "Timed out", "Connection refused", "Host unreachable", "TLS/SSL error",
    "Connection failed", "Network error",
])

def _safe_ping_msg(detail: str) -> str:
    return detail if detail in _SAFE_PING_MESSAGES else "Connection failed"

So the security fix accidentally broke the success path for both callers of this helper:

• api_setup_ping() —

  mediastarr/app/main.py

  Lines 2366 to 2386 in 3825518

  	try:
  	_sping_ok, _sping_ver, _sping_detail = ArrClient(itype, url, key).ping()
  	except Exception as _spe:
  	logger.debug(f"Setup ping exception: {type(_spe).__name__}")
  	return jsonify({"ok": _sping_ok,
  	"version": _safe_version_str(_sping_ver),
  	"msg": _safe_ping_msg(_sping_detail)})
  	@app.route("/api/setup/complete", methods=["POST"])
  	@_api_auth_required
  	def api_setup_complete():
  	d = request.get_json(silent=True) or {}
  	instances = d.get("instances",[])
  	if not isinstance(instances,list) or len(instances)==0:
  	return jsonify({"ok":False,"errors":["Mindestens eine Instanz erforderlich"]}),400
  	if len(instances) > MAX_INSTANCES:
  	return jsonify({"ok":False,"errors":[f"Maximal {MAX_INSTANCES} Instanzen"]}),400
  	errors=[]; validated=[]
  	req_lang = safe_str(d.get("language","en"),5)
  	is_de = req_lang == "de"
  	for i, inst in enumerate(instances):

• api_instances_ping() —

  mediastarr/app/main.py

  Lines 2536 to 2553 in 3825518

  	stats["status_detail"] = "" if _ping_ok else _ping_detail
  	return jsonify({"ok": _ping_ok,
  	"version": _safe_version_str(_ping_ver),
  	"msg": _safe_ping_msg(_ping_detail)})
  	# ── Main API ──────────────────────────────────────────────────────────────────
  	@app.route("/api/state")
  	def api_state():
  	# Allow unauthenticated read-only access when public_api_state is enabled
  	if _PASSWORD and not session.get("authenticated") and not CONFIG.get("public_api_state", False):
  	return jsonify({"ok": False, "error": "Unauthorized"}), 401
  	today_n=db.count_today(); limit=CONFIG.get("daily_limit",0)
  	instances_safe=[{k:v for k,v in i.items() if k!="api_key"} for i in CONFIG["instances"]]
  	# Add per-instance today count so UI can show instance limit progress
  	for inst_s in instances_safe:
  	inst_s["today_count"] = db.count_today_for_instance(inst_s["id"])
  	return jsonify({
  	"ok":True,"version":_CURRENT_VERSION,"base_url":_BASE_URL,"running":STATE["running"],"last_run":STATE["last_run"],

Steps to Reproduce

1. Configure a real, reachable Sonarr or Radarr instance (via the setup wizard, or POST /api/instances).
2. Click "Test Connection" in the setup wizard or the Edit Instance dialog (or call GET /api/instances/<id>/ping directly).
3. Observe the UI shows a failure / "Connection failed", even though the instance is reachable.
4. Check the raw JSON response (or GET /api/instances, which reports the real per-instance stats[...].status) — it correctly shows "status": "online" and the right version, confirming the ping actually succeeded and only the message text is wrong.

Mediastarr Version

v7.1.12

Installation

Docker Compose

Logs

No exception/traceback — this isn't a crash, just a mislabeled success response. docker logs mediastarr shows nothing unusual around the ping call.

Suggested Fix

Add an explicit "success" sentinel (or just an empty-string check) so _safe_ping_msg doesn't rewrite the success case:

def _safe_ping_msg(detail: str) -> str:
    if detail == "":
        return ""
    return detail if detail in _SAFE_PING_MESSAGES else "Connection failed"

or add "" to _SAFE_PING_MESSAGES and have both callers only read msg when ok is False.

Additional Context

Found this while wiring Mediastarr up via its API for a CasC-style deploy (idempotent script that PATCHes/POSTs /api/instances and confirms via GET /api/instances). Functionally everything works — instances end up status: "online" and searches run fine — this is purely a misleading UI/API response on the success path. Happy to open a PR with the one-line fix above if useful.