fix: keep file links and API keys out of AllDebrid logs, fix Newznab debug log (#1559)

Two log fixes in the same area, both found while going through the open
code scanning alerts.

AllDebrid: the unlock path logged the file link in full. The last-resort
fallback appends `apikey=` to the link and returns it, and the "Failed
to download from ..." error carries that same link into
`logger.exception`, so the key could land in the log and the traceback.
The links are now logged through `_safe_url` (scheme, host and path
only). The GET unlock fallback also logged the exception text at debug
level, and requests puts the request URL in that message, which includes
`apikey=`. That one now logs the exception type only. This covers code
scanning alert 124.

Newznab: `logger.debug("Newznab API: GET %s params=%s", url, *params)`
star-expands the dict keys into the log arguments, so with debug logging
on it raises `TypeError: not all arguments converted during string
formatting` and the line never prints. Logging `params` as is would
print the API key, so it now logs the sorted parameter names.

Tests: the Newznab one formats the logged line and checks the key is
absent. The AllDebrid ones check the fallback and failure paths log no
query string and that the fallback still returns the link with the key.
All three fail without the change. Ruff and basedpyright are clean,
`tests/newznab` and `tests/prowlarr` pass.

shelfmark/download/clients/alldebrid.py

@@ -29,6 +29,7 @@ from shelfmark.download.clients._coercion import config_text
 from shelfmark.download.clients.torrent_utils import (
     DebridMagnet,
     DebridUpload,
+    _safe_url,
     resolve_debrid_upload,
 )
 from shelfmark.download.http import download_url
@@ -564,13 +565,14 @@ class AllDebridClient(DownloadClient):
                         return direct
                 err_msg = body.get("error", {}).get("message", err_msg)
         except _ALLDEBRID_CLIENT_ERRORS as e:
-            logger.debug("GET unlock exception: %s", e)
+            # The request URL carries the API key and requests puts it in the message.
+            logger.debug("GET unlock exception: %s", type(e).__name__)
 
         # 4. Last-resort: append apikey to alldebrid.com/f/ links.
         if "alldebrid.com/f/" in link:
             logger.info(
                 "Using apikey fallback for AllDebrid file link: %s",
-                link,
+                _safe_url(link),
             )
             if "apikey=" not in link:
                 sep = "&" if "?" in link else "?"
@@ -579,7 +581,7 @@ class AllDebridClient(DownloadClient):
 
         logger.error(
             "AllDebrid unlock failed for '%s': %s",
-            link,
+            _safe_url(link),
             err_msg,
         )
         msg = f"AllDebrid unlock failed: {err_msg}"
@@ -662,7 +664,7 @@ class AllDebridClient(DownloadClient):
                     referer="https://alldebrid.com/",
                 )
                 if not buf:
-                    msg = f"Failed to download from {direct_link}"
+                    msg = f"Failed to download from {_safe_url(direct_link)}"
                     _raise_runtime_error(msg)
 
                 with dest.open("wb") as fh:

shelfmark/release_sources/newznab/api.py

@@ -45,7 +45,7 @@ class NewznabClient:
             params["apikey"] = self.api_key
 
         url = self._api_url()
-        logger.debug("Newznab API: GET %s params=%s", url, *params)
+        logger.debug("Newznab API: GET %s params=%s", url, sorted(params))
 
         headers = {}
         if accept_xml:

tests/newznab/test_api.py

@@ -206,3 +206,21 @@ class TestSearch:
             client.search(query="book", search_type="book")
 
         assert captured[0]["t"] == "book"
+
+
+class TestRequestLogging:
+    def test_debug_line_formats_and_leaves_the_api_key_out(self):
+        client = NewznabClient("http://nzbhydra:5076", "s3cret")
+        response = MagicMock(status_code=200, text=CAPS_XML, content=CAPS_XML.encode())
+        response.raise_for_status = MagicMock()
+
+        with (
+            patch.object(client._session, "get", return_value=response),
+            patch("shelfmark.release_sources.newznab.api.logger") as log,
+        ):
+            client._get({"t": "caps"})
+
+        fmt, *args = log.debug.call_args.args
+        line = fmt % tuple(args)
+        assert "'apikey'" in line
+        assert "s3cret" not in line

tests/prowlarr/test_debrid_clients.py

@@ -6,7 +6,7 @@ magnet is what made Real-Debrid answer /torrents/addMagnet with a bare 404.
 """
 
 import hashlib
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 
@@ -280,3 +280,52 @@ class TestAllDebridAdd:
 
         assert post.call_args.args[0].endswith("/magnet/upload")
         assert post.call_args.kwargs["data"] == {"magnets[]": _MAGNET}
+
+
+class TestAllDebridLinkLogging:
+    @staticmethod
+    def _client(monkeypatch):
+        monkeypatch.setattr(
+            "shelfmark.download.clients.alldebrid.config.get",
+            lambda key, default="": {"ALLDEBRID_API_KEY": "ad-secret"}.get(key, default),
+        )
+        failed = MagicMock(status_code=500)
+        monkeypatch.setattr(
+            "shelfmark.download.clients.alldebrid._session.post", MagicMock(return_value=failed)
+        )
+        monkeypatch.setattr(
+            "shelfmark.download.clients.alldebrid._session.get", MagicMock(return_value=failed)
+        )
+        return AllDebridClient()
+
+    @staticmethod
+    def _logged(log):
+        lines = []
+        for call in log.mock_calls:
+            if call.args:
+                fmt, *args = call.args
+                lines.append(fmt % tuple(args))
+        return "\n".join(lines)
+
+    def test_apikey_fallback_logs_no_query_and_still_returns_the_key(self, monkeypatch):
+        client = self._client(monkeypatch)
+
+        with patch("shelfmark.download.clients.alldebrid.logger") as log:
+            result = client._unlock_file_link("https://alldebrid.com/f/abc?token=link-secret")
+
+        assert result.endswith("&apikey=ad-secret")
+        logged = self._logged(log)
+        assert "https://alldebrid.com/f/abc" in logged
+        assert "link-secret" not in logged
+        assert "ad-secret" not in logged
+
+    def test_unlock_failure_logs_no_query(self, monkeypatch):
+        client = self._client(monkeypatch)
+
+        with patch("shelfmark.download.clients.alldebrid.logger") as log:
+            with pytest.raises(RuntimeError):
+                client._unlock_file_link("https://example.com/x?token=link-secret")
+
+        logged = self._logged(log)
+        assert "https://example.com/x" in logged
+        assert "link-secret" not in logged