Fix intersphinx leaking basic auth credentials in error and info messages

CHANGES.rst

@@ -18,6 +18,9 @@ Features added
 Bugs fixed
 ----------
 
+* #14342: intersphinx: Fix leaking basic auth credentials in error and
+  redirect log messages.
+  Patch by Joshua Swanson
 * #14189: autodoc: Fix duplicate ``:no-index-entry:`` for modules.
   Patch by Adam Turner
 * #13713: Fix compatibility with MyST-Parser.

sphinx/ext/intersphinx/_load.py

@@ -399,17 +399,27 @@ def _fetch_inventory_url(
             raw_data = r.content
             new_inv_location = r.url
     except Exception as err:
+        safe_url = _get_safe_url(inv_location)
+        # The URLs retained by the exception may be normalised forms of
+        # *inv_location* (e.g. percent-decoded), so redact them separately.
+        err_msg = str(err).replace(inv_location, safe_url)
+        for unsafe_url in (
+            getattr(getattr(err, 'request', None), 'url', None),
+            getattr(getattr(err, 'response', None), 'url', None),
+        ):
+            if unsafe_url:
+                err_msg = err_msg.replace(unsafe_url, _get_safe_url(unsafe_url))
         err.args = (
             'intersphinx inventory %r not fetchable due to %s: %s',
-            inv_location,
+            safe_url,
             err.__class__,
-            str(err),
+            err_msg,
         )
         raise
 
     if inv_location != new_inv_location:
         msg = __('intersphinx inventory has moved: %s -> %s')
-        LOGGER.info(msg, inv_location, new_inv_location)
+        LOGGER.info(msg, _get_safe_url(inv_location), _get_safe_url(new_inv_location))
 
         if target_uri in {
             inv_location,

tests/test_ext_intersphinx/test_ext_intersphinx.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import http.server
+import logging
 import time
 from typing import TYPE_CHECKING
 from unittest import mock
@@ -666,6 +667,73 @@ def test_getsafeurl_unauthed() -> None:
     assert actual == expected
 
 
+def test_fetch_inventory_url_error_hides_credentials(capsys, caplog, monkeypatch):
+    """Credentials should not appear in error messages on fetch failure."""
+    # A previously created Sphinx app disables propagation on the 'sphinx'
+    # logger; caplog needs it to capture the messages.
+    monkeypatch.setattr(logging.getLogger('sphinx'), 'propagate', True)
+
+    class ErrorHandler(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            self.send_error(500, 'Internal Server Error')
+
+        def log_message(*args, **kwargs):
+            pass
+
+    with http_server(ErrorHandler) as server:
+        # %65 is an 'e'; requests normalises the URL it retains on the
+        # exception, so the password can leak in a form ('secret') that
+        # differs from the one passed in ('s%65cret').
+        url = (
+            f'http://user:s%65cret@localhost:{server.server_port}/{INVENTORY_FILENAME}'
+        )
+        inspect_main([url])
+
+    stdout, stderr = capsys.readouterr()
+    for leak in ('secret', 's%65cret'):
+        assert leak not in stdout
+        assert leak not in stderr
+        assert not any(leak in message for message in caplog.messages)
+    assert 'user@localhost' in stderr
+
+
+def test_fetch_inventory_redirect_hides_credentials(capsys, caplog, monkeypatch):
+    """Credentials should not appear in redirect log messages."""
+    # A previously created Sphinx app disables propagation on the 'sphinx'
+    # logger; caplog needs it to capture the messages.
+    monkeypatch.setattr(logging.getLogger('sphinx'), 'propagate', True)
+
+    class RedirectHandler(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            if '/new/' not in self.path:
+                self.send_response(302)
+                assert isinstance(self.server, http.server.HTTPServer)
+                new_url = (
+                    'http://redirect-user:redirect-secret@localhost:'
+                    f'{self.server.server_port}/new/{INVENTORY_FILENAME}'
+                )
+                self.send_header('Location', new_url)
+                self.end_headers()
+            else:
+                self.send_response(200, 'OK')
+                self.end_headers()
+                self.wfile.write(INVENTORY_V2)
+
+        def log_message(*args, **kwargs):
+            pass
+
+    with http_server(RedirectHandler) as server:
+        url = f'http://user:secret@localhost:{server.server_port}/{INVENTORY_FILENAME}'
+        inspect_main([url])
+
+    stdout, stderr = capsys.readouterr()
+    assert 'secret' not in stdout
+    assert 'secret' not in stderr
+    assert not any('secret' in message for message in caplog.messages)
+    assert any('user@localhost' in message for message in caplog.messages)
+    assert any('redirect-user@localhost' in message for message in caplog.messages)
+
+
 def test_inspect_main_noargs(capsys):
     """inspect_main interface, without arguments"""
     assert inspect_main([]) == 1