fix(api): harden pre-auth setup endpoints (#1041, #1044)

/setup/test-downloader now validates the URL scheme (http/https only) like
test-rss already did, and accepts qBittorrent 5.2's 204 login response.
Raw exception and response detail is no longer echoed back from the
pre-authentication setup endpoints; it goes to the server log only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014w1Z6Nxy6XTRgkFXqPr9Zh
This commit is contained in:
Estrella Pan
2026-07-02 11:39:10 +02:00
parent e6aac59954
commit 487bdfec54
2 changed files with 104 additions and 13 deletions

View File

@@ -31,14 +31,19 @@ def _require_setup_needed():
raise HTTPException(status_code=403, detail="Setup already completed.")
def _validate_url(url: str) -> None:
"""Reject non-HTTP schemes and private/reserved/loopback IPs."""
def _validate_scheme(url: str) -> None:
"""Reject non-HTTP schemes and URLs without a hostname."""
parsed = urlparse(url)
if parsed.scheme not in ("http", "https"):
raise HTTPException(status_code=400, detail="Only http/https URLs are allowed.")
hostname = parsed.hostname
if not hostname:
if not parsed.hostname:
raise HTTPException(status_code=400, detail="Invalid URL: no hostname.")
def _validate_url(url: str) -> None:
"""Reject non-HTTP schemes and private/reserved/loopback IPs."""
_validate_scheme(url)
hostname = urlparse(url).hostname
try:
addrs = socket.getaddrinfo(hostname, None)
except socket.gaierror:
@@ -132,6 +137,9 @@ async def test_downloader(req: TestDownloaderRequest):
scheme = "https" if req.ssl else "http"
host = req.host if "://" in req.host else f"{scheme}://{req.host}"
# Private/loopback IPs stay allowed (a LAN NAS is the normal case), but
# only http/https schemes may be probed from this pre-auth endpoint (#1041).
_validate_scheme(host)
try:
async with httpx.AsyncClient(timeout=5.0) as client:
@@ -153,7 +161,12 @@ async def test_downloader(req: TestDownloaderRequest):
login_url,
data={"username": req.username, "password": req.password},
)
if login_resp.status_code == 200 and "ok" in login_resp.text.lower():
# qBittorrent < 5.2 answers 200 + "Ok."; >= 5.2 answers 204 with
# an empty body on success (#1044).
if (
login_resp.status_code in (200, 204)
and "fails" not in login_resp.text.lower()
):
return TestResultResponse(
success=True,
message_en="Connection successful.",
@@ -184,11 +197,13 @@ async def test_downloader(req: TestDownloaderRequest):
message_zh="无法连接到主机。",
)
except Exception as e:
# Log the detail server-side only — this endpoint is reachable before
# authentication, so raw errors must not be echoed back (#1041).
logger.error(f"[Setup] Downloader test failed: {e}")
return TestResultResponse(
success=False,
message_en=f"Connection failed: {e}",
message_zh=f"连接失败{e}",
message_en="Connection failed.",
message_zh="连接失败",
)
@@ -221,8 +236,8 @@ async def test_rss(req: TestRSSRequest):
logger.error(f"[Setup] RSS test failed: {e}")
return TestResultResponse(
success=False,
message_en=f"Failed to fetch RSS feed: {e}",
message_zh=f"获取 RSS 源失败{e}",
message_en="Failed to fetch RSS feed.",
message_zh="获取 RSS 源失败",
)
@@ -266,8 +281,8 @@ async def test_notification(req: TestNotificationRequest):
logger.error(f"[Setup] Notification test failed: {e}")
return TestResultResponse(
success=False,
message_en=f"Notification test failed: {e}",
message_zh=f"通知测试失败{e}",
message_en="Notification test failed.",
message_zh="通知测试失败",
)
@@ -338,6 +353,6 @@ async def complete_setup(req: SetupCompleteRequest):
return ResponseModel(
status=False,
status_code=500,
msg_en=f"Setup failed: {e}",
msg_zh=f"设置失败{e}",
msg_en="Setup failed. Check the server log for details.",
msg_zh="设置失败,请查看服务器日志。",
)

View File

@@ -297,3 +297,79 @@ class TestSentinelPath:
def test_sentinel_path_is_in_config_dir(self):
assert str(SENTINEL_PATH) == "config/.setup_complete"
assert SENTINEL_PATH.parent == Path("config")
class TestTestDownloaderHardening:
"""qB 5.2 login compat (#1044) and SSRF hardening (#1041)."""
@staticmethod
def _mock_client(get_resp=None, login_resp=None, get_exc=None):
mock_instance = AsyncMock()
if get_exc is not None:
mock_instance.get.side_effect = get_exc
else:
mock_instance.get.return_value = get_resp
mock_instance.post.return_value = login_resp
cls_patch = patch("module.api.setup.httpx.AsyncClient")
mock_cls = cls_patch.start()
mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_instance)
mock_cls.return_value.__aexit__ = AsyncMock(return_value=False)
return cls_patch
def _post(self, client, host="192.168.1.100:8080"):
return client.post(
"/api/v1/setup/test-downloader",
json={
"type": "qbittorrent",
"host": host,
"username": "admin",
"password": "admin",
"ssl": False,
},
)
def test_login_accepts_204_empty_body(self, client, mock_first_run):
"""qBittorrent >= 5.2 returns 204 + empty body on successful login."""
from unittest.mock import MagicMock
get_resp = MagicMock(text="qBittorrent WebUI")
login_resp = MagicMock(status_code=204, text="")
cls_patch = self._mock_client(get_resp=get_resp, login_resp=login_resp)
try:
response = self._post(client)
finally:
cls_patch.stop()
assert response.status_code == 200
assert response.json()["success"] is True
def test_login_rejects_200_fails_body(self, client, mock_first_run):
"""200 + 'Fails.' (bad credentials) is still a failure."""
from unittest.mock import MagicMock
get_resp = MagicMock(text="qBittorrent WebUI")
login_resp = MagicMock(status_code=200, text="Fails.")
cls_patch = self._mock_client(get_resp=get_resp, login_resp=login_resp)
try:
response = self._post(client)
finally:
cls_patch.stop()
assert response.status_code == 200
assert response.json()["success"] is False
def test_non_http_scheme_rejected(self, client, mock_first_run):
"""Non-http(s) schemes must be rejected before any request is made."""
response = self._post(client, host="ftp://internal-server:21")
assert response.status_code == 400
def test_exception_detail_not_echoed(self, client, mock_first_run):
"""Raw exception text must not leak into the API response (#1041)."""
cls_patch = self._mock_client(get_exc=Exception("secret-detail-xyz"))
try:
response = self._post(client)
finally:
cls_patch.stop()
assert response.status_code == 200
data = response.json()
assert data["success"] is False
assert "secret-detail-xyz" not in data["message_en"]
assert "secret-detail-xyz" not in data["message_zh"]