diff --git a/shelfmark/core/viewer_redaction.py b/shelfmark/core/viewer_redaction.py index 6a26f155..16fe6e8e 100644 --- a/shelfmark/core/viewer_redaction.py +++ b/shelfmark/core/viewer_redaction.py @@ -6,7 +6,8 @@ Its task id is that release's ``source_id``, which for Prowlarr is The requester never browsed those releases, so for them a request-linked download is addressed by an opaque id instead. Request rows keep only the release fields the activity cards display, and a download path shrinks to the -file name, which the browser download reveals anyway. +file name, which the browser download reveals anyway. A status message that quotes +a client error loses the links in it, which can carry an indexer key or a tracker passkey. Downloads a user queued directly keep their real id: the user picked that release from search results that already showed its ``source_id``, and the @@ -18,6 +19,7 @@ from __future__ import annotations import hashlib import hmac import os +import re from pathlib import PurePath from typing import TYPE_CHECKING, Any @@ -28,6 +30,17 @@ if TYPE_CHECKING: PUBLIC_DOWNLOAD_ID_PREFIX = "dl_" +# A client error is stored as the download's status message, and it quotes the request: +# "401 Client Error: Unauthorized for url: ", or for a refused connection +# "Max retries exceeded with url: " with no scheme. The link can carry an +# indexer key or a tracker passkey in its query string or path, and a non-admin has no use +# for it. Schemes are case-insensitive. +_LINK_IN_MESSAGE_PATTERN = re.compile( + r"https?://[^\s'\"<>)\]]+|(?<=url: )/[^\s'\"<>)\]]*", + re.IGNORECASE, +) +_REMOVED_LINK = "(link removed)" + # Release fields a non-admin's activity cards display. Everything else (source_id, # indexer, info and download URLs, torrent attributes) stays server-side. _RELEASE_DISPLAY_FIELDS = frozenset( @@ -108,9 +121,21 @@ def redact_request_row(row: dict[str, Any]) -> dict[str, Any]: return {**row, "release_data": redact_release_data(row["release_data"])} +def redact_status_message(message: str) -> str: + """Replace every link in a status message, keeping the rest of the text. + + A link is a full http(s) URL, or the scheme-less path after ``url: `` that requests + prints for a failed connection. + """ + return _LINK_IN_MESSAGE_PATTERN.sub(_REMOVED_LINK, message) + + def redact_download_payload(payload: dict[str, Any], task_id: str | None = None) -> dict[str, Any]: """Return a copy of a download payload that is safe to show its non-admin owner.""" redacted = dict(payload) + status_message = payload.get("status_message") + if isinstance(status_message, str): + redacted["status_message"] = redact_status_message(status_message) raw_id = task_id if task_id is not None else payload.get("id") if isinstance(raw_id, str) and raw_id: viewer_id = viewer_download_id(raw_id, payload.get("request_id")) diff --git a/tests/core/test_viewer_redaction.py b/tests/core/test_viewer_redaction.py index f6e176d6..e346c3bd 100644 --- a/tests/core/test_viewer_redaction.py +++ b/tests/core/test_viewer_redaction.py @@ -89,6 +89,77 @@ def test_download_payload_loses_its_path_and_tracker_id(): assert payload["id"] == _TRACKER_ID # the caller's copy is untouched +def test_status_message_loses_the_links_a_client_error_carries(): + """A failed add embeds the client's exception text, which names the full URL.""" + message = ( + "Failed to add to qbittorrent: 401 Client Error: Unauthorized for url: " + "http://prowlarr:9696/api/v1/indexer/21/download?apikey=s3cret&link=abc&file=Example" + ) + + redacted = redact_download_payload({"id": "task", "status_message": message}) + + assert redacted["status_message"] == ( + "Failed to add to qbittorrent: 401 Client Error: Unauthorized for url: (link removed)" + ) + assert "s3cret" not in redacted["status_message"] + assert "prowlarr:9696" not in redacted["status_message"] + assert message.endswith("file=Example") # the caller's copy is untouched + + +def test_status_message_loses_the_path_a_connection_error_carries(): + """requests prints a failed connection as ``with url: ``, no scheme.""" + message = ( + "Failed to add to qbittorrent: HTTPConnectionPool(host='prowlarr', port=9696): " + "Max retries exceeded with url: /api/v1/indexer/21/download?apikey=s3cret&link=abc " + "(Caused by NewConnectionError(\"HTTPConnection(host='prowlarr', port=9696): " + 'Failed to establish a new connection: [Errno 111] Connection refused"))' + ) + + redacted = redact_download_payload({"id": "task", "status_message": message})["status_message"] + + assert "s3cret" not in redacted + assert "/api/v1/indexer" not in redacted + assert ( + "Max retries exceeded with url: (link removed) (Caused by NewConnectionError(" in redacted + ) + + +@pytest.mark.parametrize("scheme", ["HTTP", "Https", "HTTPS"]) +def test_status_message_link_scheme_is_matched_case_insensitively(scheme): + message = f"502 Server Error: Bad Gateway for url: {scheme}://prowlarr:9696/api?apikey=s3cret" + + redacted = redact_download_payload({"id": "task", "status_message": message})["status_message"] + + assert redacted == "502 Server Error: Bad Gateway for url: (link removed)" + + +@pytest.mark.parametrize( + "message", + ["Downloading 42% - 1.2 MB/s", "No seeders: 0 found, 1 needed, for 5 min", None, ""], +) +def test_status_message_without_a_link_is_left_alone(message): + assert ( + redact_download_payload({"id": "task", "status_message": message})["status_message"] + == message + ) + + +def test_status_message_is_redacted_in_a_whole_status_snapshot(): + status = { + "error": { + "task": { + "id": "task", + "request_id": None, + "status_message": "Search failed for url: https://indexer.example/api?passkey=abc", + } + } + } + + redacted = redact_status(status) + + assert redacted["error"]["task"]["status_message"] == "Search failed for url: (link removed)" + + def test_status_is_rekeyed_only_for_request_linked_downloads(): status = { "complete": {