diff --git a/shelfmark/download/clients/sabnzbd.py b/shelfmark/download/clients/sabnzbd.py index 5d9b6ebc..4fe1081c 100644 --- a/shelfmark/download/clients/sabnzbd.py +++ b/shelfmark/download/clients/sabnzbd.py @@ -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) diff --git a/tests/prowlarr/test_sabnzbd_client.py b/tests/prowlarr/test_sabnzbd_client.py index c087679f..0f85170e 100644 --- a/tests/prowlarr/test_sabnzbd_client.py +++ b/tests/prowlarr/test_sabnzbd_client.py @@ -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()."""