Commit Graph

1 Commits

Author SHA1 Message Date
Leonardo Baray d7b64116ac
fix: redact Redis password from cache connection logs (#869)
* fix: redact Redis password from cache connection logs

The cache client logged the full Redis URL — including the password —
at INFO and WARNING levels on every connection attempt and failure.
This exposed the live Redis credential in stdout/container logs and any
downstream log aggregation.

Add _redact_cache_url() to mask the password component before logging.
URLs without a password are returned unchanged.

Closes #866

* fix: handle malformed URLs and IPv6 in _redact_cache_url

Address review feedback from VVoruganti and CodeRabbit:

- Wrap urlparse/urlunparse in try/except so malformed URLs (e.g. invalid
  port) don't raise ValueError inside except blocks, which would crash
  startup instead of degrading gracefully
- Preserve IPv6 brackets (e.g. [::1]) in reconstructed URLs
- Add Google-style Args/Returns docstring sections
- Add unit tests for password masking, no-password URLs, IPv6,
  malformed inputs, and the invalid-port regression

* test: use real secrets in redaction test fixtures

Three fixtures were weakened by copy-paste mangling: literal '***'
placeholders instead of real passwords (assertions trivially true),
an unescaped '#' that truncated netloc parsing via the URL fragment,
and a no-password case that actually contained userinfo. Restore
inputs that genuinely exercise the masking paths.

* fix: never leak password through malformed-URL fallback

The catch-all fallback returned the original URL when parsing failed,
so a Redis URL with a password and an invalid port (typo, out-of-range)
was logged in clear text - the exact leak #866 exists to fix. Narrow
the handling: .port access gets its own try/except (invalid port is
omitted from the output; userinfo/hostname masking never raises), and
the outer fallback now returns a generic placeholder instead of the
raw input. Tightened the invalid-port test to assert the password is
absent and added out-of-range-port and unparseable-URL cases.

* fix: redact secrets in query params and scheme-less URLs

_redact_cache_url only masked userinfo, but a credential can reach the
URL through two other real configuration paths: redis-py accepts
?password= (all querystring options become client kwargs) and cashews
accepts ?secret= (its HMAC signing key) - and honcho's own default
CACHE.URL already uses a query param (?suppress=true), so this is the
expected configuration style. Separately, a URL missing its scheme
(':pass@host:6379/0') parses with an empty netloc, making the password
invisible to .password and echoing it back verbatim.

Mask sensitive query values in place on the raw query string (no
decode/re-encode, so non-secret params are preserved byte-for-byte)
and return the generic placeholder for @-carrying strings with no
parseable authority. Verified with a 20k-case randomized fuzz run in
addition to the unit tests: no functional credential reaches the
output.
2026-07-24 12:54:31 -04:00