mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 22:05:50 +01:00
fix(sabnzbd): strip Prowlarr API key on cross-host NZB redirects (#1424)
_fetch_nzb_content sent X-Api-Key then followed redirects with the header still attached, leaking the Prowlarr key when Prowlarr 302s to the indexer download link. Follow redirects manually with allow_redirects=False and re-evaluate headers per hop, mirroring the torrent path.
This commit is contained in:
@@ -5,7 +5,7 @@ Uses SABnzbd's REST API directly via requests (no external dependency).
|
||||
|
||||
import ipaddress
|
||||
from typing import Any
|
||||
from urllib.parse import urlparse
|
||||
from urllib.parse import urljoin, urlparse
|
||||
|
||||
import requests
|
||||
from publicsuffixlist import PublicSuffixList
|
||||
@@ -34,6 +34,7 @@ _SABNZBD_CLIENT_ERRORS = (
|
||||
)
|
||||
_SabnzbdRequestParam = str | int | float | bool
|
||||
_PUBLIC_SUFFIXES = PublicSuffixList()
|
||||
_NZB_FETCH_MAX_REDIRECTS = 5
|
||||
|
||||
|
||||
def _url_origin(value: str) -> tuple[str, str, int] | None:
|
||||
@@ -266,10 +267,34 @@ class SABnzbdClient(DownloadClient):
|
||||
|
||||
def _fetch_nzb_content(self, url: str) -> bytes:
|
||||
"""Fetch NZB content, including Prowlarr auth headers when appropriate."""
|
||||
headers = self._get_prowlarr_headers(url)
|
||||
response = requests.get(url, timeout=30, headers=headers, verify=get_ssl_verify(url))
|
||||
response.raise_for_status()
|
||||
return response.content
|
||||
# Follow redirects manually so the Prowlarr API key is re-evaluated
|
||||
# per hop. requests forwards custom headers such as X-Api-Key to the
|
||||
# redirect target even when it is a different host, so letting it
|
||||
# follow a Prowlarr 302 to the indexer's own download link would leak
|
||||
# the key to a third party. This mirrors the torrent fetch path.
|
||||
current_url = url
|
||||
redirects_remaining = _NZB_FETCH_MAX_REDIRECTS
|
||||
while True:
|
||||
headers = self._get_prowlarr_headers(current_url)
|
||||
response = requests.get(
|
||||
current_url,
|
||||
timeout=30,
|
||||
headers=headers,
|
||||
verify=get_ssl_verify(current_url),
|
||||
allow_redirects=False,
|
||||
)
|
||||
if response.status_code not in (301, 302, 303, 307, 308):
|
||||
response.raise_for_status()
|
||||
return response.content
|
||||
location = response.headers.get("Location", "")
|
||||
if not location:
|
||||
response.raise_for_status()
|
||||
return response.content
|
||||
if redirects_remaining <= 0:
|
||||
msg = f"Too many redirects fetching NZB from {urlparse(url).hostname or 'url'}"
|
||||
raise requests.exceptions.TooManyRedirects(msg)
|
||||
redirects_remaining -= 1
|
||||
current_url = urljoin(current_url, location)
|
||||
|
||||
def _can_prefetch_nzb_url(self, url: str) -> bool:
|
||||
target_origin = _url_origin(url)
|
||||
|
||||
@@ -730,6 +730,58 @@ class TestSABnzbdClientAddDownload:
|
||||
assert SABnzbdClient()._can_prefetch_nzb_url(nzb_url) is expected
|
||||
|
||||
|
||||
class TestFetchNzbContentRedirects:
|
||||
"""_fetch_nzb_content must not leak the Prowlarr API key across hosts."""
|
||||
|
||||
@staticmethod
|
||||
def _client(monkeypatch):
|
||||
config_values = {
|
||||
"SABNZBD_URL": "http://localhost:8080",
|
||||
"SABNZBD_API_KEY": "abc123",
|
||||
"PROWLARR_URL": "https://prowlarr.example",
|
||||
"PROWLARR_API_KEY": "secret",
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.clients.sabnzbd.config.get",
|
||||
lambda key, default="": config_values.get(key, default),
|
||||
)
|
||||
from shelfmark.download.clients.sabnzbd import SABnzbdClient
|
||||
|
||||
with patch.object(SABnzbdClient, "__init__", lambda x: None):
|
||||
client = SABnzbdClient()
|
||||
return client
|
||||
|
||||
def test_strips_api_key_on_cross_host_redirect(self, monkeypatch):
|
||||
"""Prowlarr often 302s to the indexer's own download link."""
|
||||
client = self._client(monkeypatch)
|
||||
redirect = MagicMock(
|
||||
status_code=302, headers={"Location": "https://indexer.example/get.nzb"}
|
||||
)
|
||||
final = MagicMock(status_code=200, content=b"nzbdata")
|
||||
with patch(
|
||||
"shelfmark.download.clients.sabnzbd.requests.get",
|
||||
MagicMock(side_effect=[redirect, final]),
|
||||
) as mock_get:
|
||||
assert client._fetch_nzb_content("https://prowlarr.example/download?id=1") == b"nzbdata"
|
||||
assert mock_get.call_count == 2
|
||||
assert mock_get.call_args_list[0].kwargs["headers"].get("X-Api-Key") == "secret"
|
||||
assert "X-Api-Key" not in mock_get.call_args_list[1].kwargs["headers"]
|
||||
assert mock_get.call_args_list[0].kwargs.get("allow_redirects") is False
|
||||
|
||||
def test_keeps_api_key_on_same_host_redirect(self, monkeypatch):
|
||||
client = self._client(monkeypatch)
|
||||
redirect = MagicMock(status_code=302, headers={"Location": "/download2?id=1"})
|
||||
final = MagicMock(status_code=200, content=b"nzbdata")
|
||||
with patch(
|
||||
"shelfmark.download.clients.sabnzbd.requests.get",
|
||||
MagicMock(side_effect=[redirect, final]),
|
||||
) as mock_get:
|
||||
assert client._fetch_nzb_content("https://prowlarr.example/download?id=1") == b"nzbdata"
|
||||
assert mock_get.call_count == 2
|
||||
assert mock_get.call_args_list[1].args[0] == "https://prowlarr.example/download2?id=1"
|
||||
assert mock_get.call_args_list[1].kwargs["headers"].get("X-Api-Key") == "secret"
|
||||
|
||||
|
||||
class TestSABnzbdClientRemove:
|
||||
"""Tests for SABnzbdClient.remove()."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user