Skip to content

fix(daily_closes): scrub FRED api_key from error logs + retry FRED 5xx#220

Merged
cipher813 merged 1 commit into
mainfrom
fix/fred-api-key-log-leak
May 12, 2026
Merged

fix(daily_closes): scrub FRED api_key from error logs + retry FRED 5xx#220
cipher813 merged 1 commit into
mainfrom
fix/fred-api-key-log-leak

Conversation

@cipher813
Copy link
Copy Markdown
Owner

Summary

Closes a latent credential-leak class in the FRED fetcher's error-log path. requests.exceptions.HTTPError.str() embeds the full request URL — including api_key=<credential> — and _fetch_fred_closes logged it verbatim via logger.warning("... %s", e). Any FRED 5xx in production MorningEnrich / EOD would dump the key to CloudWatch logs.

Surfaced 2026-05-12 during the post-merge operator step for PR #219 — a transient FRED 500 on VXVCLS during the repair-script run leaked the key to the conversation transcript. The FRED key is being rotated separately; this PR closes the underlying leak.

What changed

  • collectors/daily_closes.py — new _scrub_api_key(msg) -> str helper masks the api_key=... querystring fragment with api_key=***. _fetch_fred_closes's warn-log routes the exception through the scrubber.
  • collectors/daily_closes_fred_repair.py — adds retry (3 attempts, 3s/6s backoff) on transient FRED 5xx + imports the shared _scrub_api_key from daily_closes (regex defined once, no drift across files).
  • tests/test_fred_api_key_scrub.py (new, +7 tests) — pins the scrubbing contract on _scrub_api_key directly + on _fetch_fred_closes warn path + on _fetch_fred_range retry-warn + final-error paths.

Suite 792 → 799.

Test plan

  • Local pytest -q clean (799 passed, 1 skipped pre-existing)
  • Confirm tomorrow's Wed 2026-05-13 MorningEnrich logs (CloudWatch) — if FRED 5xx fires, the log line should read api_key=*** not the raw key.
  • Operator: rotate the FRED key on https://fred.stlouisfed.org/, update .env locally + alpha-engine-config/data/.env + the AWS SSM /alpha-engine/FRED_API_KEY parameter (or wherever the Lambda env loader reads from). Rotation is independent of this PR — the PR closes the future leak surface.

🤖 Generated with Claude Code

requests.exceptions.HTTPError.str() embeds the full request URL —
including ``api_key=<credential>`` — in its message. The existing
``_fetch_fred_closes`` except-block logged that raw via
``logger.warning("FRED fetch failed for %s (%s): %s", t, sid, e)`` so
any FRED 5xx in production MorningEnrich / EOD dumped the credential
to CloudWatch.

Surfaced 2026-05-12 during the post-merge repair run for PR #219 — a
transient FRED 500 on VXVCLS during the operator step leaked the key
to the conversation transcript. The FRED key is rotated separately;
this commit closes the leak class.

Changes:
- ``_scrub_api_key(msg) -> str`` helper in ``collectors/daily_closes.py``
  masks the ``api_key=...`` querystring fragment with ``api_key=***``
- ``_fetch_fred_closes`` warn-log routes the exception through the
  scrubber
- Repair script's ``_fetch_fred_range`` already added retry+scrub in
  the cherry-pick from the live-debug session; this commit consolidates
  by importing the shared ``_scrub_api_key`` from ``daily_closes`` so
  the regex doesn't drift across files

Tests (+7, suite 792 → 799):
- ``_scrub_api_key`` masks URL-embedded keys, handles exception objects
  directly, passthroughs when no key present, terminates at ``&``
- ``_fetch_fred_closes`` warn-log scrub regression
- ``_fetch_fred_range`` retry-warn + final-error scrub regressions

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cipher813 cipher813 merged commit 366c232 into main May 12, 2026
1 check passed
@cipher813 cipher813 deleted the fix/fred-api-key-log-leak branch May 12, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant