From 3633dd3d977c7cf9fee9e3ca3d2b29c9b75aacfa Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Fri, 23 Oct 2026 01:53:08 -0600 Subject: [PATCH] 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 | 10 +++-- shelfmark/release_sources/newznab/api.py | 2 +- tests/newznab/test_api.py | 18 +++++++++ tests/prowlarr/test_debrid_clients.py | 51 +++++++++++++++++++++++- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/shelfmark/download/clients/alldebrid.py b/shelfmark/download/clients/alldebrid.py index 67c5796f..000b4760 100644 --- a/shelfmark/download/clients/alldebrid.py +++ b/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: diff --git a/shelfmark/release_sources/newznab/api.py b/shelfmark/release_sources/newznab/api.py index be4166a6..41cad00b 100644 --- a/shelfmark/release_sources/newznab/api.py +++ b/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: diff --git a/tests/newznab/test_api.py b/tests/newznab/test_api.py index 87c4552c..276000ba 100644 --- a/tests/newznab/test_api.py +++ b/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 diff --git a/tests/prowlarr/test_debrid_clients.py b/tests/prowlarr/test_debrid_clients.py index cc7aa9af..45ad8cd2 100644 --- a/tests/prowlarr/test_debrid_clients.py +++ b/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