From 5f9f47cc4189c5d0b05efdc5105a4fd83b6229be Mon Sep 17 00:00:00 2001 From: Alex <25013571+alexhb1@users.noreply.github.com> Date: Sat, 9 May 2026 14:22:25 +0100 Subject: [PATCH] Redact release URLs safely (#968) --- shelfmark/download/orchestrator.py | 15 ++++- shelfmark/release_sources/__init__.py | 4 ++ shelfmark/release_sources/newznab/handler.py | 43 +++++++++++++ shelfmark/release_sources/newznab/source.py | 3 +- shelfmark/release_sources/prowlarr/handler.py | 18 +++++- shelfmark/release_sources/prowlarr/source.py | 3 +- tests/newznab/test_handler.py | 22 +++++++ tests/newznab/test_source.py | 29 ++++++++- tests/prowlarr/test_source.py | 61 +++++++++++++++++++ 9 files changed, 191 insertions(+), 7 deletions(-) diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index b6dbefc6..1582bd74 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -159,7 +159,20 @@ def _build_retry_resolution_fields( if not isinstance(extra, dict): extra = {} + retry_download_url = normalize_optional_text(release_data.get("download_url")) protocol = normalize_optional_text(release_data.get("protocol")) + source = normalize_optional_text(release_data.get("source")) + if source is not None: + handler = get_handler(source) + source_retry_fields = handler.build_retry_resolution_fields(release_data) + retry_download_url = ( + normalize_optional_text(source_retry_fields.get("retry_download_url")) + or retry_download_url + ) + protocol = ( + normalize_optional_text(source_retry_fields.get("retry_download_protocol")) or protocol + ) + ratio_limit = _optional_number(release_data.get("ratio_limit")) if ratio_limit is None and config.get("PROWLARR_USE_SEED_PREFERENCES", False): ratio_limit = _optional_number(extra.get("configured_ratio_limit")) @@ -173,7 +186,7 @@ def _build_retry_resolution_fields( ) return { - "retry_download_url": normalize_optional_text(release_data.get("download_url")), + "retry_download_url": retry_download_url, "retry_download_protocol": protocol.lower() if protocol is not None else None, "retry_release_name": normalize_optional_text(release_data.get("title")), "retry_expected_hash": normalize_optional_text( diff --git a/shelfmark/release_sources/__init__.py b/shelfmark/release_sources/__init__.py index e2ff355e..e0951c0f 100644 --- a/shelfmark/release_sources/__init__.py +++ b/shelfmark/release_sources/__init__.py @@ -390,6 +390,10 @@ class DownloadHandler(ABC): """ return + def build_retry_resolution_fields(self, release_data: dict[str, Any]) -> dict[str, Any]: + """Return private queue-time fields needed for restart-safe retry.""" + return {} + @abstractmethod def cancel(self, task_id: str) -> bool: """Cancel an in-progress download.""" diff --git a/shelfmark/release_sources/newznab/handler.py b/shelfmark/release_sources/newznab/handler.py index 9665493a..cac65c93 100644 --- a/shelfmark/release_sources/newznab/handler.py +++ b/shelfmark/release_sources/newznab/handler.py @@ -8,6 +8,7 @@ if TYPE_CHECKING: from shelfmark.core.models import DownloadTask from shelfmark.core.logger import setup_logger +from shelfmark.core.request_helpers import normalize_optional_text from shelfmark.download.clients import DownloadClient, get_client, list_configured_clients from shelfmark.download.clients.base_handler import ( COMPLETED_PATH_MAX_ATTEMPTS as _DEFAULT_COMPLETED_PATH_MAX_ATTEMPTS, @@ -83,6 +84,44 @@ class NewznabHandler(ExternalClientHandler): def _completed_path_max_attempts(self) -> int: return COMPLETED_PATH_MAX_ATTEMPTS + def build_retry_resolution_fields(self, release_data: dict) -> dict: + source_id = normalize_optional_text(release_data.get("source_id")) + if source_id is None: + return {} + + result = get_release(source_id) + if result is None: + return {} + + return { + "retry_download_url": normalize_optional_text(_get_download_url(result)), + "retry_download_protocol": normalize_optional_text(_get_protocol(result)), + } + + @classmethod + def _restore_download_request_from_task(cls, task: DownloadTask) -> DownloadRequest | None: + retry_download_url = normalize_optional_text(getattr(task, "retry_download_url", None)) + retry_download_protocol = normalize_optional_text( + getattr(task, "retry_download_protocol", None) + ) + if retry_download_url is None or retry_download_protocol is None: + return None + + protocol = retry_download_protocol.lower() + if protocol not in {"torrent", "usenet"}: + return None + + return DownloadRequest( + url=retry_download_url, + protocol=protocol, + release_name=( + normalize_optional_text(getattr(task, "retry_release_name", None)) + or task.title + or "Unknown" + ), + expected_hash=normalize_optional_text(getattr(task, "retry_expected_hash", None)), + ) + def _resolve_download( self, task: DownloadTask, @@ -90,6 +129,10 @@ class NewznabHandler(ExternalClientHandler): ) -> DownloadRequest | None: result = get_release(task.task_id) if not result: + restored_request = self._restore_download_request_from_task(task) + if restored_request is not None: + logger.info("Restored Newznab download request for retry: %s", task.task_id) + return restored_request logger.warning("Newznab release cache miss: %s", task.task_id) status_callback("error", "Release not found in cache (may have expired)") return None diff --git a/shelfmark/release_sources/newznab/source.py b/shelfmark/release_sources/newznab/source.py index 0a1a0cb0..966d6d0d 100644 --- a/shelfmark/release_sources/newznab/source.py +++ b/shelfmark/release_sources/newznab/source.py @@ -109,7 +109,6 @@ def _newznab_result_to_release(result: dict, content_type: str = "ebook") -> Rel if is_freeleech: add_flag("FreeLeech") - download_url = str(result.get("downloadUrl") or "").strip() info_url = result.get("infoUrl") or result.get("guid") return Release( @@ -120,7 +119,7 @@ def _newznab_result_to_release(result: dict, content_type: str = "ebook") -> Rel language=None, size=_parse_size(size_bytes), size_bytes=size_bytes, - download_url=download_url or None, + download_url=None, info_url=info_url, protocol=protocol, indexer=indexer, diff --git a/shelfmark/release_sources/prowlarr/handler.py b/shelfmark/release_sources/prowlarr/handler.py index e4fb2c35..b75dcab1 100644 --- a/shelfmark/release_sources/prowlarr/handler.py +++ b/shelfmark/release_sources/prowlarr/handler.py @@ -1,6 +1,6 @@ """Prowlarr download handler - resolves releases and delegates lifecycle to shared clients.""" -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from shelfmark.core.config import config from shelfmark.core.logger import setup_logger @@ -79,6 +79,22 @@ class ProwlarrHandler(ExternalClientHandler): def _completed_path_max_attempts(self) -> int: return COMPLETED_PATH_MAX_ATTEMPTS + def build_retry_resolution_fields(self, release_data: dict[str, Any]) -> dict[str, Any]: + source_id = normalize_optional_text(release_data.get("source_id")) + if source_id is None: + return {} + + prowlarr_result = get_release(source_id) + if prowlarr_result is None: + return {} + + return { + "retry_download_url": normalize_optional_text( + get_preferred_download_url(prowlarr_result) + ), + "retry_download_protocol": normalize_optional_text(get_protocol(prowlarr_result)), + } + @classmethod def _restore_download_request_from_task(cls, task: DownloadTask) -> DownloadRequest | None: """Rebuild a DownloadRequest when the in-memory Prowlarr cache is gone.""" diff --git a/shelfmark/release_sources/prowlarr/source.py b/shelfmark/release_sources/prowlarr/source.py index 154e31b2..79a66f46 100644 --- a/shelfmark/release_sources/prowlarr/source.py +++ b/shelfmark/release_sources/prowlarr/source.py @@ -32,7 +32,6 @@ from shelfmark.release_sources.prowlarr.cache import cache_release from shelfmark.release_sources.prowlarr.utils import ( coerce_float_like, coerce_int_like, - get_preferred_download_url, get_protocol, ) @@ -407,7 +406,7 @@ def _prowlarr_result_to_release( language=language_detected, size=_parse_size(size_bytes), size_bytes=size_bytes, - download_url=get_preferred_download_url(result), + download_url=None, info_url=result.get("infoUrl") or result.get("guid"), protocol=( ReleaseProtocol.TORRENT diff --git a/tests/newznab/test_handler.py b/tests/newznab/test_handler.py index 725376be..e8cbe12b 100644 --- a/tests/newznab/test_handler.py +++ b/tests/newznab/test_handler.py @@ -96,6 +96,28 @@ class TestGetDownloadUrl: class TestHandlerErrors: + def test_cache_miss_uses_persisted_retry_fields(self): + with patch("shelfmark.release_sources.newznab.handler.get_release", return_value=None): + handler = NewznabHandler() + task = DownloadTask( + task_id="retryable", + source="newznab", + title="Book", + retry_download_url="https://indexer.example.com/nzb/42?apikey=secret", + retry_download_protocol="usenet", + retry_release_name="Book Release", + retry_expected_hash="abc123", + ) + recorder = ProgressRecorder() + result = handler._resolve_download(task, recorder.status_callback) + + assert result is not None + assert result.url == "https://indexer.example.com/nzb/42?apikey=secret" + assert result.protocol == "usenet" + assert result.release_name == "Book Release" + assert result.expected_hash == "abc123" + assert recorder.status_updates == [] + def test_cache_miss_returns_error(self): with patch("shelfmark.release_sources.newznab.handler.get_release", return_value=None): handler = NewznabHandler() diff --git a/tests/newznab/test_source.py b/tests/newznab/test_source.py index c4efa264..4b70919b 100644 --- a/tests/newznab/test_source.py +++ b/tests/newznab/test_source.py @@ -60,7 +60,7 @@ class TestResultToRelease: assert r.size_bytes == 2097152 assert r.indexer == "MyIndexer" assert r.source_id == "https://indexer.example.com/nzb/42" - assert r.download_url == "https://indexer.example.com/nzb/42?apikey=secret" + assert r.download_url is None def test_torrent_result_has_torrent_protocol(self): r = _newznab_result_to_release( @@ -134,6 +134,33 @@ class TestResultToRelease: assert r.extra["book_title"] == "Dune" assert r.extra["info_hash"] == "abc123" + def test_redacted_result_still_builds_private_retry_payload(self, monkeypatch): + import shelfmark.download.orchestrator as orchestrator + + secret_download_url = "https://indexer.example.com/nzb/42?apikey=secret" + release = _newznab_result_to_release(_make_result(downloadUrl=secret_download_url)) + + assert release.download_url is None + + captured_tasks = [] + + def fake_add(task): + captured_tasks.append(task) + return True + + monkeypatch.setattr(orchestrator.book_queue, "add", fake_add) + monkeypatch.setattr( + orchestrator.config, "get", lambda key, default=None, user_id=None: default + ) + + success, error = orchestrator.queue_release(release.__dict__) + + assert success is True + assert error is None + assert captured_tasks[0].retry_download_url == secret_download_url + assert captured_tasks[0].retry_download_protocol == "usenet" + assert "retry_download_url" not in orchestrator._task_to_dict(captured_tasks[0]) + # ── NewznabSource.is_available ───────────────────────────────────────────────── diff --git a/tests/prowlarr/test_source.py b/tests/prowlarr/test_source.py index cba7f0d8..732afe92 100644 --- a/tests/prowlarr/test_source.py +++ b/tests/prowlarr/test_source.py @@ -444,6 +444,67 @@ class TestProwlarrLocalizedQueries: assert releases[0].extra["configured_ratio_limit"] is None assert releases[0].extra["configured_seed_time_minutes"] is None + def test_redacted_search_result_still_builds_private_retry_payload(self, monkeypatch): + import shelfmark.download.orchestrator as orchestrator + import shelfmark.release_sources.prowlarr.source as prowlarr_source + + secret_download_url = "https://prowlarr.example.com/1/download?apikey=secret" + + def fake_get(key: str, default=None, user_id=None): + values = { + "PROWLARR_INDEXERS": "", + "PROWLARR_AUTO_EXPAND": False, + "PROWLARR_USE_SEED_PREFERENCES": False, + } + return values.get(key, default) + + monkeypatch.setattr(prowlarr_source.config, "get", fake_get) + monkeypatch.setattr(orchestrator.config, "get", fake_get) + + fake_client = FakeTorznabClient( + search_results=[ + { + "guid": "secret-prowlarr-release", + "protocol": "usenet", + "title": "Secret Bearing Release", + "downloadUrl": secret_download_url, + } + ] + ) + source = ProwlarrSource() + monkeypatch.setattr(source, "_get_client", lambda: fake_client) + + book = BookMetadata( + provider="hardcover", + provider_id="123", + title="Anything", + authors=["Someone"], + ) + + from shelfmark.core.search_plan import build_release_search_plan + + plan = build_release_search_plan(book, languages=["en"]) + releases = source.search(book, plan, content_type="ebook") + + assert len(releases) == 1 + assert releases[0].download_url is None + + captured_tasks = [] + + def fake_add(task): + captured_tasks.append(task) + return True + + monkeypatch.setattr(orchestrator.book_queue, "add", fake_add) + + success, error = orchestrator.queue_release(releases[0].__dict__) + + assert success is True + assert error is None + assert captured_tasks[0].retry_download_url == secret_download_url + assert captured_tasks[0].retry_download_protocol == "usenet" + assert "retry_download_url" not in orchestrator._task_to_dict(captured_tasks[0]) + def test_search_uses_localized_titles_when_available(self, monkeypatch): import shelfmark.release_sources.prowlarr.source as prowlarr_source