diff --git a/shelfmark/release_sources/__init__.py b/shelfmark/release_sources/__init__.py index 1f3a9923..0091c297 100644 --- a/shelfmark/release_sources/__init__.py +++ b/shelfmark/release_sources/__init__.py @@ -15,6 +15,7 @@ if TYPE_CHECKING: from shelfmark.core.search_plan import ReleaseSearchPlan from shelfmark.download.postprocess.packs import PackFile +from shelfmark.core.logger import redact_secrets from shelfmark.metadata_providers import BookMetadata @@ -29,7 +30,16 @@ class ReleaseProtocol(StrEnum): class SourceUnavailableError(Exception): - """Raised when a source is configured but currently unreachable.""" + """Raised when a source is configured but currently unreachable. + + The message is shown to whoever ran the search, and it is usually built from a requests + exception whose text carries the request URL. A source that puts its key in the query + string (Jackett's ``apikey=``) would hand that key to every user, so credentials are + masked here for every source rather than at each place that raises. + """ + + def __init__(self, message: object = "") -> None: + super().__init__(redact_secrets(str(message))) @dataclass diff --git a/tests/core/test_source_unavailable_error.py b/tests/core/test_source_unavailable_error.py new file mode 100644 index 00000000..074fe20c --- /dev/null +++ b/tests/core/test_source_unavailable_error.py @@ -0,0 +1,22 @@ +from shelfmark.release_sources import SourceUnavailableError + + +def test_a_credential_query_parameter_is_masked_in_the_message(): + error = SourceUnavailableError( + "could not reach it: http://host/api?t=book&apikey=SECRETKEY123&q=x" + ) + + assert "SECRETKEY123" not in str(error) + assert "apikey=(removed)" in str(error) + + +def test_userinfo_is_masked_in_the_message(): + error = SourceUnavailableError("could not reach http://user:hunter2@host:9117/api") + + assert "hunter2" not in str(error) + + +def test_a_plain_message_is_unchanged(): + message = "every indexer is disabled by Prowlarr after recent failures (until 12:00)" + + assert str(SourceUnavailableError(message)) == message diff --git a/tests/jackett/test_jackett.py b/tests/jackett/test_jackett.py index 86d6ad9f..322a9278 100644 --- a/tests/jackett/test_jackett.py +++ b/tests/jackett/test_jackett.py @@ -212,6 +212,40 @@ class TestSource: ): JackettSource().search(_book(), _plan()) + def test_an_unreachable_jackett_does_not_put_its_api_key_in_the_error(self, jackett_config): + # requests puts the request URL, apikey included, in a connection error's text. + client = JackettClient("http://jackett:9117", "SECRETKEY123") + error = requests.exceptions.ConnectionError( + "HTTPConnectionPool(host='jackett', port=9117): Max retries exceeded with url: " + "/api/v2.0/indexers/all/results/torznab/api?apikey=SECRETKEY123&t=book&q=dune" + ) + with ( + patch.object(JackettSource, "_get_client", return_value=client), + patch.object(client._session, "get", side_effect=error), + pytest.raises(SourceUnavailableError) as raised, + ): + JackettSource().search(_book(), _plan()) + + assert "SECRETKEY123" not in str(raised.value) + assert "Max retries exceeded" in str(raised.value) + + def test_the_indexer_lookup_error_does_not_put_the_api_key_in_the_message(self, jackett_config): + client = JackettClient("http://jackett:9117", "SECRETKEY123") + error = requests.exceptions.ConnectionError( + "HTTPConnectionPool(host='jackett', port=9117): Max retries exceeded with url: " + "/api/v2.0/indexers/all/results/torznab/api?t=indexers&configured=true" + "&apikey=SECRETKEY123" + ) + with ( + patch.object(JackettSource, "_get_client", return_value=client), + patch.object(client._session, "get", side_effect=error), + pytest.raises(SourceUnavailableError) as raised, + ): + JackettSource().search(_book(), _plan(indexers=["Alpha"])) + + assert "SECRETKEY123" not in str(raised.value) + assert str(raised.value).startswith("could not reach Jackett") + def test_auto_expand_retries_without_categories(self, jackett_config): jackett_config["JACKETT_AUTO_EXPAND"] = True client = MagicMock()