Skip to content

req.py logs API keys in plaintext, leaking them into notebook HTML exports #160

Description

@jinskeep-morpc

Summary

morpc/req.py writes request parameters and fully-qualified request URLs to the log. When a caller passes an API key as a request parameter — which morpc-census does for every Census API call — the key is written out in plaintext. Because morpc.logs.config_logs streams to the notebook cell output, the key is then baked into any HTML produced by morpc.notebook_to_html() and committed alongside the notebook.

This is not hypothetical. morpc-osmbuildings-standardize.html currently has a live CENSUS_API_KEY in it, committed at 6045271 and pushed. I caught the same thing in morpc-bingbuildings-standardize before it was committed and held the export back. Both repos are private, so this is not public exposure, but the keys are live and they are in git history.

What it looks like

A single INFO-level line from a routine county boundary fetch:

INFO | morpc_census.geos.geoinfo_from_params: Getting GEOIDS from
https://api.census.gov/data/2024/geoinfo and params: {'get': 'GEO_ID,NAME',
'for': 'county:041, ...', 'in': 'state:39', 'key': '<40-char key, in full>'}

Affected lines in morpc/req.py

Params logged directly:

Line Function Level
19 get_text_safely debug
39 get_json_safely debug
89 get_file_safely debug
102 post_json_safely info
121 delete_safely info

r.url logged, which carries the key in the query string:

Line Function Level
22 get_text_safely error
66 get_json_safely error
71 get_json_safely error
112 post_json_safely error

The debug-level ones are less exposed but still wrong — anyone who raises the log level to debug to diagnose a failing request gets a key in their notebook output, which is exactly when they are most likely to paste it somewhere.

Suggested fix

A redaction helper applied at every one of the sites above, rather than fixing them case by case:

SENSITIVE_PARAMS = {"key", "api_key", "apikey", "token", "access_token", "password"}


def _redact_params(params):
    """Return `params` with the value of any credential-bearing key replaced."""
    if not params:
        return params
    return {k: ("<redacted>" if k.lower() in SENSITIVE_PARAMS else v) for k, v in params.items()}


def _redact_url(url):
    """Return `url` with the value of any credential-bearing query parameter replaced."""
    parts = urllib.parse.urlsplit(str(url))
    query = urllib.parse.parse_qsl(parts.query, keep_blank_values=True)
    query = [(k, "<redacted>" if k.lower() in SENSITIVE_PARAMS else v) for k, v in query]
    return urllib.parse.urlunsplit(parts._replace(query=urllib.parse.urlencode(query)))

Redacting rather than dropping keeps the log useful: you can still see that a key was sent, which is the thing you usually want to know when a request 403s.

Companion change needed in morpc-census

morpc-census logs the params itself and would still leak after this is fixed. In morpc_census/geos.py (v0.6.1):

  • L491, geoinfo_from_params — logger.info(f"Getting GEOIDS from {url} and params: {params}."). This is the line that produced the leak above.
  • L482, geoinfo_from_params — logger.error(f"ucgid without pseudo. {params}")

If the helpers above are exported from morpc.req, morpc-census can import them rather than reimplementing the redaction. Happy to open that issue too.

Also worth considering

Existing exports should be scrubbed and the affected keys rotated — a fix here stops new leaks but does nothing about what is already committed.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions