diff --git a/CHANGES.rst b/CHANGES.rst index 6f7279e8011..9f4b2b74544 100644 --- a/CHANGES.rst +++ b/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. diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index ab6a373fea0..9fb08892069 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/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, diff --git a/tests/test_ext_intersphinx/test_ext_intersphinx.py b/tests/test_ext_intersphinx/test_ext_intersphinx.py index 5fcfe4d9260..fb83d303ae7 100644 --- a/tests/test_ext_intersphinx/test_ext_intersphinx.py +++ b/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