diff --git a/googleapiclient/_helpers.py b/googleapiclient/_helpers.py index c63d08448e..0fc16cbef2 100644 --- a/googleapiclient/_helpers.py +++ b/googleapiclient/_helpers.py @@ -17,6 +17,7 @@ import functools import inspect import logging +import re import urllib.parse logger = logging.getLogger(__name__) @@ -205,3 +206,25 @@ def _add_query_parameter(url, name, value): return url else: return update_query_params(url, {name: value}) + + +# The developer API key travels on every request as the ``key`` query +# parameter. It is a credential and must not be written to logs, even at +# DEBUG level (CWE-532). +_SENSITIVE_QUERY_PARAM_RE = re.compile(r"([?&]key=)[^&#]*", re.IGNORECASE) + + +def _redact_sensitive_url(url): + """Redact credential query parameters from a URL before logging it. + + Args: + url: string, the request URL that may carry an API key. + + Returns: + A copy of the URL with the ``key`` query parameter value replaced by + ``REDACTED``. Non-string input is returned unchanged. + """ + try: + return _SENSITIVE_QUERY_PARAM_RE.sub(r"\1REDACTED", url) + except TypeError: + return url diff --git a/googleapiclient/discovery.py b/googleapiclient/discovery.py index 4e92b2cc1b..93700284a0 100644 --- a/googleapiclient/discovery.py +++ b/googleapiclient/discovery.py @@ -62,7 +62,11 @@ # Local imports from googleapiclient import _auth, mimeparse -from googleapiclient._helpers import _add_query_parameter, positional +from googleapiclient._helpers import ( + _add_query_parameter, + _redact_sensitive_url, + positional, +) from googleapiclient.errors import ( HttpError, InvalidJsonError, @@ -432,7 +436,7 @@ def _retrieve_discovery_doc( actual_url = _add_query_parameter(url, "userIp", os.environ["REMOTE_ADDR"]) if developerKey: actual_url = _add_query_parameter(url, "key", developerKey) - logger.debug("URL being requested: GET %s", actual_url) + logger.debug("URL being requested: GET %s", _redact_sensitive_url(actual_url)) # Execute this request with retries build into HttpRequest # Note that it will already raise an error if we don't get a 2xx response @@ -1263,7 +1267,9 @@ def method(self, **kwargs): ) % multipart_boundary url = _add_query_parameter(url, "uploadType", "multipart") - logger.debug("URL being requested: %s %s" % (httpMethod, url)) + logger.debug( + "URL being requested: %s %s" % (httpMethod, _redact_sensitive_url(url)) + ) return self._requestBuilder( self._http, model.response, @@ -1383,7 +1389,10 @@ def methodNext(self, previous_request, previous_response): request.uri = _add_query_parameter( request.uri, pageTokenName, nextPageToken ) - logger.debug("Next page request URL: %s %s" % (methodName, request.uri)) + logger.debug( + "Next page request URL: %s %s" + % (methodName, _redact_sensitive_url(request.uri)) + ) else: # Replace pageToken value in request body model = self._model diff --git a/googleapiclient/http.py b/googleapiclient/http.py index 5aad076854..de0b17ef61 100644 --- a/googleapiclient/http.py +++ b/googleapiclient/http.py @@ -181,7 +181,7 @@ def _retry_request( num_retries, req_type, method, - uri, + util._redact_sensitive_url(uri), resp.status if resp else exception, ) sleep(sleep_time) @@ -1157,7 +1157,12 @@ def next_chunk(self, http=None, num_retries=0): self._sleep(self._rand() * 2**retry_num) LOGGER.warning( "Retry #%d for media upload: %s %s, following status: %d" - % (retry_num, self.method, self.uri, resp.status) + % ( + retry_num, + self.method, + util._redact_sensitive_url(self.uri), + resp.status, + ) ) try: diff --git a/tests/test__helpers.py b/tests/test__helpers.py index 56df6f8807..2ddab4182d 100644 --- a/tests/test__helpers.py +++ b/tests/test__helpers.py @@ -149,3 +149,44 @@ def test_with_repeats(self): content = "a=b&a=d" with self.assertRaises(ValueError): _helpers.parse_unique_urlencoded(content) + + +class Test_redact_sensitive_url(unittest.TestCase): + def test_redacts_key(self): + url = "https://www.googleapis.com/foo?key=SECRET&alt=json" + self.assertEqual( + _helpers._redact_sensitive_url(url), + "https://www.googleapis.com/foo?key=REDACTED&alt=json", + ) + + def test_redacts_key_at_end(self): + url = "https://www.googleapis.com/foo?alt=json&key=SECRET" + self.assertEqual( + _helpers._redact_sensitive_url(url), + "https://www.googleapis.com/foo?alt=json&key=REDACTED", + ) + + def test_redacts_key_before_fragment(self): + url = "https://www.googleapis.com/foo?key=SECRET#frag" + self.assertEqual( + _helpers._redact_sensitive_url(url), + "https://www.googleapis.com/foo?key=REDACTED#frag", + ) + + def test_case_insensitive(self): + url = "https://www.googleapis.com/foo?Key=SECRET" + self.assertEqual( + _helpers._redact_sensitive_url(url), + "https://www.googleapis.com/foo?Key=REDACTED", + ) + + def test_does_not_touch_similar_param_names(self): + url = "https://www.googleapis.com/foo?apikey=KEEP&key2=KEEP" + self.assertEqual(_helpers._redact_sensitive_url(url), url) + + def test_no_key(self): + url = "https://www.googleapis.com/foo?alt=json" + self.assertEqual(_helpers._redact_sensitive_url(url), url) + + def test_non_string_returned_unchanged(self): + self.assertIsNone(_helpers._redact_sensitive_url(None)) diff --git a/tests/test_discovery.py b/tests/test_discovery.py index 6912783451..d412215af2 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -652,6 +652,22 @@ def test_building_with_developer_key_skips_adc(self): # application default credentials were used. self.assertNotIsInstance(plus._http, google_auth_httplib2.AuthorizedHttp) + def test_developer_key_not_logged(self): + discovery = read_datafile("plus.json") + plus = build_from_document( + discovery, + base="https://www.googleapis.com/", + developerKey="SECRET_DEVELOPER_KEY", + ) + with self.assertLogs("googleapiclient.discovery", level="DEBUG") as cm: + request = plus.activities().list(collection="public", userId="me") + logged = "\n".join(cm.output) + self.assertIn("URL being requested", logged) + self.assertNotIn("SECRET_DEVELOPER_KEY", logged) + self.assertIn("key=REDACTED", logged) + # The real request still carries the key; only the log line is redacted. + self.assertIn("key=SECRET_DEVELOPER_KEY", request.uri) + def test_building_with_context_manager(self): discovery = read_datafile("plus.json") with mock.patch("httplib2.Http") as http: