Summary
morpc_census/geos.py logs request parameters after injecting the Census API key into them, so the key is written out in plaintext. morpc.logs.config_logs streams to the notebook cell output, so the key then ends up in any HTML written by morpc.notebook_to_html() and committed alongside the notebook.
Companion to morpc/morpc-py#160, which covers the same class of leak in morpc.req. Both need fixing: req.py logs params and r.url for every request, and geos.py logs the params itself before handing them over, so fixing either alone leaves the other leaking.
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 in morpc-bingbuildings-standardize before committing and held the export back. Both repos are private, so this is not public exposure, but the keys are live and in git history.
What it looks like
One INFO-level line from a routine county boundary fetch — fetch_geos_from_scope_sumlevel(scope="region15", sumlevel="county"):
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_census/geos.py, at 121650b:
| Line |
Context |
Level |
| 491 |
geoinfo_from_params — logs params immediately after L489-490 set params['key'] |
info |
| 482 |
geoinfo_from_params — logger.error(f"ucgid without pseudo. {params}"), reachable after the key is set |
error |
| 476 |
geoinfo_from_params — logs param_dict, which is caller-supplied and does not carry the key today |
debug |
L491 is the one that actually leaks. geoids_from_scope (L511-512) also sets params['key'] but logs nothing itself — it leaks only through morpc.req, which is morpc/morpc-py#160.
Worth auditing at the same time, since all of them build params containing a key and pass them to morpc.req: geos.py:124 and api.py:177, 260, 336, 831, 897-900. None log directly right now, so they are only exposed via the req.py path, but they are one added log line away from the same bug.
Suggested fix
morpc/morpc-py#160 proposes exporting a _redact_params / _redact_url pair from morpc.req. If that lands, the fix here is to import it rather than reimplement:
from morpc.req import redact_params
logger.info(f"Getting GEOIDS from {url} and params: {redact_params(params)}.")Redacting rather than dropping keeps the log useful — you can still see that a key was sent, which is what you want when a Census request comes back 403 or with an empty body.
If a fix is wanted here before morpc-py moves, the smaller version is to log the params before the key is injected, i.e. move L491 above L489. That fixes L491 specifically but not L482, and does nothing for the req.py path.
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
Summary
morpc_census/geos.pylogs request parameters after injecting the Census API key into them, so the key is written out in plaintext.morpc.logs.config_logsstreams to the notebook cell output, so the key then ends up in any HTML written bymorpc.notebook_to_html()and committed alongside the notebook.Companion to morpc/morpc-py#160, which covers the same class of leak in
morpc.req. Both need fixing:req.pylogs params andr.urlfor every request, andgeos.pylogs the params itself before handing them over, so fixing either alone leaves the other leaking.This is not hypothetical.
morpc-osmbuildings-standardize.htmlcurrently has a liveCENSUS_API_KEYin it, committed at6045271and pushed. I caught the same inmorpc-bingbuildings-standardizebefore committing and held the export back. Both repos are private, so this is not public exposure, but the keys are live and in git history.What it looks like
One INFO-level line from a routine county boundary fetch —
fetch_geos_from_scope_sumlevel(scope="region15", sumlevel="county"):Affected lines
In
morpc_census/geos.py, at121650b:geoinfo_from_params— logsparamsimmediately after L489-490 setparams['key']geoinfo_from_params—logger.error(f"ucgid without pseudo. {params}"), reachable after the key is setgeoinfo_from_params— logsparam_dict, which is caller-supplied and does not carry the key todayL491 is the one that actually leaks.
geoids_from_scope(L511-512) also setsparams['key']but logs nothing itself — it leaks only throughmorpc.req, which is morpc/morpc-py#160.Worth auditing at the same time, since all of them build params containing a key and pass them to
morpc.req:geos.py:124andapi.py:177, 260, 336, 831, 897-900. None log directly right now, so they are only exposed via thereq.pypath, but they are one added log line away from the same bug.Suggested fix
morpc/morpc-py#160 proposes exporting a
_redact_params/_redact_urlpair frommorpc.req. If that lands, the fix here is to import it rather than reimplement:Redacting rather than dropping keeps the log useful — you can still see that a key was sent, which is what you want when a Census request comes back 403 or with an empty body.
If a fix is wanted here before morpc-py moves, the smaller version is to log the params before the key is injected, i.e. move L491 above L489. That fixes L491 specifically but not L482, and does nothing for the
req.pypath.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