fix(redaction): a non-admin's status message loses the links it quotes (#1541)

#1422 hid `source_id` and the download path from non-admins but left the
status message alone. A failed add stores the client's exception text
there, for example `401 Client Error: Unauthorized for url: <full url>`,
and for a requests error that URL carries the path and query: an indexer
key, a tracker passkey, a peer name. It reached the owner through
`/api/status`, `/api/activity` and the `status_update` socket event.

`redact_download_payload` now replaces every link in `status_message`
with `(link removed)`. That covers the live queue, the socket push and
the history snapshot, since all three go through it. Messages without a
link are unchanged and admins still see the full text.

Tests cover the payload, a whole status snapshot, and messages that must
stay as they are. They fail on main and pass here.

Found in a Claude pass over 1.5.0.

---------

Co-authored-by: CaliBrain <calibrain@l4n.xyz>

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: <full url>", or for a refused connection
+# "Max retries exceeded with url: <path and query>" 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"))

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: <path and query>``, 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": {