diff --git a/repo/plugin.video.nzbdav/resources/lib/http_util.py b/repo/plugin.video.nzbdav/resources/lib/http_util.py index b2daf782..4ec25a66 100644 --- a/repo/plugin.video.nzbdav/resources/lib/http_util.py +++ b/repo/plugin.video.nzbdav/resources/lib/http_util.py @@ -41,6 +41,16 @@ re.IGNORECASE, ) +# Some Newznab indexers return download URLs shaped like +# ``/getnzb/id.nzb&i=ACCOUNT&r=APIKEY``. Because there is no ``?``, both +# credentials are parsed as part of the URL path rather than query params. +# Restrict the short ``i``/``r`` names to getnzb-looking paths so ordinary +# application URLs using those names are not over-redacted. +_GETNZB_PATH_CRED_RE = re.compile( + r"([&;](?:i|r))=([^&\s\"'<>]+)", + re.IGNORECASE, +) + # Catch ``scheme://user:password@host`` userinfo embedded in free-form text. # urllib / socket / xbmcvfs errors sometimes echo the failing URL — e.g. the # NZBGet JSON-RPC URL or the ``smb://user:pass@host/...`` completed-folder @@ -91,9 +101,10 @@ def redact_url(url): # WebDAV stack used to accept). Strip the password half before # logging. TODO.md §H.2-H2d. netloc = _redact_netloc_userinfo(parts.netloc) - return urlunsplit( - (parts.scheme, netloc, parts.path, urlencode(query), parts.fragment) - ) + path = parts.path + if "getnzb" in path.lower() or ".nzb&" in path.lower(): + path = _GETNZB_PATH_CRED_RE.sub(r"\1=REDACTED", path) + return urlunsplit((parts.scheme, netloc, path, urlencode(query), parts.fragment)) def _redact_netloc_userinfo(netloc): @@ -113,22 +124,14 @@ def _redact_netloc_userinfo(netloc): return "{}@{}".format(userinfo, host) -def _redact_url_userinfo_span(match): - """Strip the password from a URL span's ``user:pass@host`` userinfo. +def _redact_url_span(match): + """Redact credentials only within a matched URL span. - Reuses the same ``rpartition('@')`` / ``partition(':')`` logic as - ``redact_url`` so an ``@`` *inside* the password (or an empty username) - can't leak. Spans with no userinfo round-trip unchanged. + Reusing ``redact_url`` covers both URL userinfo and non-standard Newznab + path credentials without applying short ``i``/``r`` parameter names to + unrelated text elsewhere in the same message. """ - span = match.group(0) - try: - parts = urlsplit(span) - except (ValueError, TypeError): - return span - if not parts.netloc or "@" not in parts.netloc: - return span - netloc = _redact_netloc_userinfo(parts.netloc) - return urlunsplit((parts.scheme, netloc, parts.path, parts.query, parts.fragment)) + return redact_url(match.group(0)) def redact_text(text): @@ -146,7 +149,7 @@ def redact_text(text): # ``\1`` is the key group; a backreference replacement avoids a per-match # Python callback on this hot logging/error path. redacted = _EMBEDDED_CRED_RE.sub(r"\1=REDACTED", str(text)) - return _EMBEDDED_URL_RE.sub(_redact_url_userinfo_span, redacted) + return _EMBEDDED_URL_RE.sub(_redact_url_span, redacted) _WHITESPACE_RE = re.compile(r"\s+") diff --git a/tests/test_http_util.py b/tests/test_http_util.py index 29d33a8d..3362597c 100644 --- a/tests/test_http_util.py +++ b/tests/test_http_util.py @@ -229,6 +229,24 @@ def test_redact_url_preserves_url_without_apikey(): assert result == url +def test_redact_url_hides_getnzb_path_credentials(): + """Newznab links may put account/API credentials in the path.""" + url = "https://indexer.example/getnzb/id.nzb&i=12345&r=secretkey123" + result = redact_url(url) + + assert "12345" not in result + assert "secretkey123" not in result + assert "&i=REDACTED" in result + assert "&r=REDACTED" in result + + +def test_redact_url_preserves_short_params_outside_getnzb_paths(): + """Short i/r parameters are not credentials in arbitrary URLs.""" + url = "https://example.com/report&i=12345&r=summary" + + assert redact_url(url) == url + + def test_redact_url_hides_extended_credential_keys(): """TODO.md §H.2-H2c: the redaction set covers more than just apikey. `key`, `access_token`, `bearer`, `session`, `sessionid`, `password`, @@ -285,6 +303,35 @@ def test_redact_text_redacts_multiple_credential_params(): assert "t=movie" in result +def test_redact_text_hides_getnzb_path_credentials(): + """Free-form HTTP errors must scrub non-standard Newznab path secrets.""" + msg = ( + "fetch failed for " + "https://indexer.example/getnzb/id.nzb&i=12345&r=secretkey123" + ) + result = redact_text(msg) + + assert "12345" not in result + assert "secretkey123" not in result + assert "&i=REDACTED" in result + assert "&r=REDACTED" in result + + +def test_redact_text_scopes_getnzb_path_credentials_to_url(): + """Short Newznab credential names must not redact unrelated message data.""" + msg = ( + "fetch failed for " + "https://indexer.example/getnzb/id.nzb&i=12345&r=secretkey123 " + "while callback metadata remained &i=keep&r=visible" + ) + result = redact_text(msg) + + assert "12345" not in result + assert "secretkey123" not in result + assert "&i=REDACTED&r=REDACTED" in result + assert "metadata remained &i=keep&r=visible" in result + + def test_redact_text_redacts_digit_prefixed_values(): """The ``\\1=REDACTED`` backreference must not misfire when the secret value begins with a digit — the literal ``=`` terminates the group, so there is no