Redact API keys and tokens from morpc.req logs and errors

morpc/req.py

@@ -1,4 +1,5 @@
 import logging
+import re
 from os import PathLike
 from time import sleep
 from httpx import head
@@ -7,6 +8,23 @@
 
 logger = logging.getLogger(__name__)
 
+# Query parameters that carry credentials, e.g. the Census API key or an ArcGIS token.
+SENSITIVE_PARAMS = ('key', 'token', 'api_key')
+_SENSITIVE_PATTERN = re.compile(r'([?&](?:' + '|'.join(SENSITIVE_PARAMS) + r')=)[^&#]*', re.IGNORECASE)
+
+
+def redact(value):
+    """Return a URL or parameter dict with credential values replaced by "REDACTED", for logs and messages.
+
+    Logs and exception messages end up in committed notebook outputs, so they must never contain API keys.
+    """
+    if value is None:
+        return None
+    if isinstance(value, dict):
+        return {k: ('REDACTED' if str(k).lower() in SENSITIVE_PARAMS else v) for k, v in value.items()}
+    return _SENSITIVE_PATTERN.sub(r'\1REDACTED', str(value))
+
+
 default_headers = {"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36"}
 
 
@@ -16,11 +34,11 @@ def get_text_safely(url, params=None, headers=default_headers, session: Session
     if not isinstance(session, Session):
         session = Session()
 
-    logger.debug(f"Getting data from {url} with parameters {params}.")
+    logger.debug(f"Getting data from {redact(url)} with parameters {redact(params)}.")
     r = session.get(url, headers=headers, params=params)
     if r.status_code != 200:
-        logger.error(f"Request content: {r.url}")
-        raise requests.HTTPError
+        logger.error(f"Request failed with status {r.status_code}: {redact(r.url)}")
+        raise requests.HTTPError(f"Request failed with status {r.status_code}: {redact(r.url)}", response=r)
     else:
         logger.debug(f"Request successful. Returning plain text.")
 
@@ -36,7 +54,7 @@ def get_json_safely(url, params=None, headers=default_headers, session: Session
     if not isinstance(session, Session):
         session = Session()
 
-    logger.debug(f"Getting data from {url} with parameters {params}.")
+    logger.debug(f"Getting data from {redact(url)} with parameters {redact(params)}.")
     r = session.get(url, params=params, headers=headers)
     if r.status_code != 200:
         if "Output format not supported" in r.text:
@@ -53,10 +71,10 @@ def get_json_safely(url, params=None, headers=default_headers, session: Session
         else:
             # 204 means the request was valid but there is nothing to return, which callers may treat as no rows.
             if r.status_code == 204:
-                logger.warning(f"No content returned: {r.url}")
+                logger.warning(f"No content returned: {redact(r.url)}")
             else:
                 logger.error(f"Request failed. Content: {r.content}")
-            raise HTTPError(f"Request failed with status {r.status_code}: {r.url}", response=r)
+            raise HTTPError(f"Request failed with status {r.status_code}: {redact(r.url)}", response=r)
     else:
         logger.debug(f"Request successful. Decoding return JSON.")
         try:
@@ -68,12 +86,12 @@ def get_json_safely(url, params=None, headers=default_headers, session: Session
                         r=session.get(url=url, params=params, headers=headers)
                         json = r.json()
                     except Exception as e:
-                        logger.error(f"Request failed: {r.url}")
+                        logger.error(f"Request failed: {redact(r.url)}")
                         logger.error(f"Failed second attempt. {e}")
                         raise RuntimeError
                 logger.error(f"Server returned error {json['error']}")
         except:
-            logger.error(f"JSONDecoderError. Check the url. {r.url}")
+            logger.error(f"JSONDecoderError. Check the url. {redact(r.url)}")
             raise requests.JSONDecodeError
 
     if returnurl:
@@ -91,9 +109,13 @@ def get_file_safely(url, output_dir: str | PathLike, chunk_size:int=4096, params
     filename = os.path.basename(url)
     filepath = os.path.join(output_dir, filename)
 
-    logger.debug(f"Getting file from {url} with parameters {params}.")
+    logger.debug(f"Getting file from {redact(url)} with parameters {redact(params)}.")
     with session.get(url, params=params, headers=headers, stream=True) as r:
-        r.raise_for_status()
+        try:
+            r.raise_for_status()
+        except HTTPError as e:
+            # requests puts the full URL, including any credentials, in the message.
+            raise HTTPError(redact(str(e)), response=e.response) from None
         with open(filepath, "wb") as file:
             for chunk in r.iter_content(chunk_size=chunk_size):
                 file.write(chunk)
@@ -104,7 +126,7 @@ def get_file_safely(url, output_dir: str | PathLike, chunk_size:int=4096, params
 def post_safely(url, params=None, headers=None):
     import requests
 
-    logger.info(f"Posting data to {url} with parameters {params}.")
+    logger.info(f"Posting data to {redact(url)} with parameters {redact(params)}.")
     r = requests.post(url, headers=headers, params=params)
     if r.status_code != 201:
         logger.error(f"Request content: {r.content}")
@@ -114,7 +136,7 @@ def post_safely(url, params=None, headers=None):
         try:
             json = r.json()
         except:
-            logger.error(f"JSONDecoderError. Check the url. {r.url}")
+            logger.error(f"JSONDecoderError. Check the url. {redact(r.url)}")
             raise requests.JSONDecodeError
     r.close()
 
@@ -123,7 +145,7 @@ def post_safely(url, params=None, headers=None):
 def delete_safely(url, params=None, headers=None):
     import requests
 
-    logger.info(f"Deleting data at {url} with parameters {params}.")
+    logger.info(f"Deleting data at {redact(url)} with parameters {redact(params)}.")
     r = requests.post(url, headers=headers, params=params)
     if r.status_code != 204:
         logger.error(f"Request content: {r.content}")

morpc/rest_api/rest_api.py

@@ -15,7 +15,7 @@
 from frictionless.dialect import Control
 from requests import HTTPError
 
-from morpc.req import get_json_safely
+from morpc.req import get_json_safely, redact
 
 logger = logging.getLogger(__name__)
 
@@ -301,7 +301,7 @@ def _max_record_count(url):
         logger.error("Could not find maxRecordCount in response.")
         raise
     except KeyError:
-        logger.error(f"maxRecordCount not in response: {fetched_url}")
+        logger.error(f"maxRecordCount not in response: {redact(fetched_url)}")
         raise
 
     logger.info(f"Max record count: {max_record_count}")

tests/test_req.py

@@ -41,3 +41,89 @@ def test_get_json_safely_http_error_carries_response():
 
 def test_get_json_safely_returns_json_on_success():
     assert get_json_safely("http://example", session=_FakeSession(_Response(200, [["a"], ["1"]]))) == [["a"], ["1"]]
+
+
+# Census and ArcGIS requests carry credentials as query parameters (key=, token=). Logs and exception
+# messages end up in committed notebook outputs, so they must never contain them.
+
+SECRET = "s3cr3t-api-key"
+SECRET_URL = f"http://example/data?get=NAME&key={SECRET}"
+
+
+class _SecretResponse(_Response):
+    def __init__(self, status_code, payload=None):
+        super().__init__(status_code, payload)
+        self.url = SECRET_URL
+
+    def raise_for_status(self):
+        # Mirrors requests: the message includes the full URL.
+        if self.status_code != 200:
+            raise HTTPError(f"{self.status_code} Client Error: Not Found for url: {self.url}", response=self)
+
+    def iter_content(self, chunk_size=1):
+        return iter([b"data"])
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *args):
+        return False
+
+
+class _SecretSession(_FakeSession):
+    def get(self, url, params=None, headers=None, stream=False):
+        return self._response
+
+
+def _assert_no_secret(caplog, excinfo=None):
+    assert SECRET not in caplog.text
+    if excinfo is not None:
+        assert SECRET not in str(excinfo.value)
+
+
+@pytest.mark.parametrize("status", [204, 400, 404])
+def test_get_json_safely_failure_does_not_leak_key(caplog, status):
+    caplog.set_level("DEBUG")
+    with pytest.raises(HTTPError) as excinfo:
+        get_json_safely("http://example/data", params={"key": SECRET}, session=_SecretSession(_SecretResponse(status)))
+    _assert_no_secret(caplog, excinfo)
+
+
+def test_get_json_safely_success_does_not_leak_key(caplog):
+    caplog.set_level("DEBUG")
+    get_json_safely("http://example/data", params={"key": SECRET}, session=_SecretSession(_SecretResponse(200, [["a"]])))
+    _assert_no_secret(caplog)
+
+
+def test_get_json_safely_decode_error_does_not_leak_key(caplog):
+    class _BadJSON(_SecretResponse):
+        def json(self):
+            raise ValueError("not json")
+    caplog.set_level("DEBUG")
+    with pytest.raises(Exception) as excinfo:
+        get_json_safely(SECRET_URL, session=_SecretSession(_BadJSON(200)))
+    _assert_no_secret(caplog, excinfo)
+
+
+def test_get_text_safely_failure_does_not_leak_key(caplog):
+    from morpc.req import get_text_safely
+    caplog.set_level("DEBUG")
+    with pytest.raises(HTTPError) as excinfo:
+        get_text_safely(SECRET_URL, session=_SecretSession(_SecretResponse(400)))
+    _assert_no_secret(caplog, excinfo)
+
+
+def test_get_file_safely_failure_does_not_leak_key(caplog, tmp_path):
+    from morpc.req import get_file_safely
+    caplog.set_level("DEBUG")
+    with pytest.raises(HTTPError) as excinfo:
+        get_file_safely("http://example/data.csv", tmp_path, params={"token": SECRET}, session=_SecretSession(_SecretResponse(404)))
+    _assert_no_secret(caplog, excinfo)
+
+
+def test_redact_hides_credentials_in_urls_and_params():
+    from morpc.req import redact
+    assert SECRET not in redact(f"http://x/y?a=1&KEY={SECRET}&token={SECRET}&api_key={SECRET}")
+    assert redact(f"http://x/y?a=1&key={SECRET}") == "http://x/y?a=1&key=REDACTED"
+    assert redact({"get": "NAME", "key": SECRET}) == {"get": "NAME", "key": "REDACTED"}
+    assert redact(None) is None