Skip to content

Commit e719604

Browse files
committed
fix(cachekitio): keep a schemeless API URL's username out of the HTTPS error (LAB-4643)
Without a scheme, urlparse reads 'user:pw@host' as scheme 'user', so the credentials check never sees userinfo and the HTTPS check rejected it with 'got: user://', echoing the username. The HTTPS error is now fixed text. The credentials check tests for '@' in the netloc, the same split httpx uses, instead of two username/password conditions. Docs: the credentials rule is listed wherever the API URL rules are (configuration security notes, the SSRF page's custom-host note, SECURITY.md's transport-log scope), and the @cache.io ConfigurationError lists now include a CACHEKIT_API_URL that fails validation.
1 parent 2241b5c commit e719604

8 files changed

Lines changed: 18 additions & 8 deletions

File tree

‎SECURITY.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ See [SSRF Protection](docs/features/ssrf-protection.md) for full details, includ
192192

193193
Cache keys can embed caller-supplied tenant/user identifiers, so **the SDK's own loggers** (`cachekit.*`) never emit them verbatim ([CWE-532][cwe-532]). Every cachekit log path — decorator error handling (structured and backwards-compat), cache-operation logs, and SWR/TTL-refresh debug logs — replaces the key with a fixed-length blake2b digest (`<redacted:…>`), keeping log lines correlatable without leaking the key. Error paths are covered centrally at the shared error sink (`FeatureOrchestrator.handle_cache_error` / `log_cache_operation`), so new call sites are redacted by construction. Both structured cache-operation sinks (`FeatureOrchestrator.log_cache_operation`, `StructuredLogger.cache_operation`) also sanitise an exception passed as `error=` themselves — pass the exception object, never `str(e)`, which is emitted as-is. `BackendError` redacts the key in its formatted text (`str(e)` carries `key=<redacted:…>`), while the `.key` attribute keeps the raw caller-supplied key for programmatic use — never log `e.key`. Its free-form `message` is caller-supplied and third-party exception text (a redis `ResponseError` naming the key, a pymemcache illegal-input error echoing it) has unknown provenance — so **no cachekit log line renders `str(e)`**. Every logging call that mentions an exception goes through `redact_error_for_log`, which emits only the exception type plus, for `BackendError`, its `BackendErrorType` classification; the full exception stays on the object (`original_exception`, `.message`) for programmatic access. Operators lose the provider's message text in the log line and keep it on the exception. An architecture test (`tests/unit/test_log_redaction_architecture.py`) walks every logging call in the package — `logger.*()`, `get_logger().*()`, `getattr(logger, level)()` — and fails CI if a key-shaped value reaches one unredacted in the message, `%s` arguments, or `extra=`; if an exception — any name bound by `except ... as`, a conventional name (`e`, `exc`, `err`, `error`, `*_err`), or an attribute of one — reaches one outside `redact_error_for_log`; or if a call emits a traceback (`logger.exception`, `exc_info=`). The guarantee does not depend on the next contributor remembering it. It is flow-insensitive: build log lines inline, not via a pre-formatted variable, and bind exceptions with `except ... as` or a conventional name (an `Exception`-typed parameter called `failure` is invisible to it), or the guard cannot see them.
194194

195-
**Scope — transport logs are not covered.** The CachekitIO backend addresses entries by key in the request path (`GET /v1/cache/{key}`), and `httpx` logs every request line — method, full URL, status — at `INFO` on its own `httpx` logger. An application that enables `INFO` globally (`logging.basicConfig(level=logging.INFO)`) will therefore see raw keys in *httpx's* output on every operation, exactly as it would see any REST resource path. cachekit does not mute a third-party logger on your behalf; if your keys carry identifiers, silence or raise the level of that logger in your logging config:
195+
**Scope — transport logs are not covered.** The CachekitIO backend addresses entries by key in the request path (`GET /v1/cache/{key}`), and `httpx` logs every request line — method, full URL, status — at `INFO` on its own `httpx` logger. An application that enables `INFO` globally (`logging.basicConfig(level=logging.INFO)`) will therefore see raw keys in *httpx's* output on every operation, exactly as it would see any REST resource path. That line never carries a password: an API URL with credentials (`user:password@`) is rejected at construction. cachekit does not mute a third-party logger on your behalf; if your keys carry identifiers, silence or raise the level of that logger in your logging config:
196196

197197
```python
198198
import logging

‎docs/api-reference.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ def get_exchange_rates():
217217

218218
#### Raises
219219

220-
- **`ConfigurationError`**: If `CACHEKIT_API_KEY` is not set
220+
- **`ConfigurationError`**: If no API key is available (argument or `CACHEKIT_API_KEY`), the key contains whitespace, `CACHEKIT_API_URL` fails validation, or `backend=` / `config=` is passed
221221

222222
#### Notes
223223

‎docs/backends/cachekitio.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ def tenant_b_function(x):
6262
```
6363

6464
`@cache.io()` raises `ConfigurationError` at decoration time if it has no key from either
65-
source, if the key contains whitespace (usually a trailing newline from a secrets file), or
66-
if you pass `backend=` or `config=` — it always caches through its own `CachekitIOBackend`.
65+
source, if the key contains whitespace (usually a trailing newline from a secrets file), if
66+
`CACHEKIT_API_URL` fails validation, or if you pass `backend=` or `config=` — it always caches through its own `CachekitIOBackend`.
6767
To cache through another backend, use `@cache.production(backend=...)`.
6868

6969
The RORO form `@cache(config=DecoratorConfig.io(api_key=...))` keeps its own key even when

‎docs/configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ CACHEKIT_ALLOW_CUSTOM_HOST=false
147147

148148
**Security notes:**
149149
- `CACHEKIT_API_URL` must use HTTPS. HTTP is rejected at startup.
150+
- Credentials in the URL (`user:password@`) are rejected: the API key is the only credential.
150151
- Private/internal IP addresses are blocked (SSRF protection). This includes `10.x`, `172.16-31.x`, `192.168.x`, `127.x`, and link-local ranges.
151152
- `CACHEKIT_ALLOW_CUSTOM_HOST=true` disables the hostname allowlist. Only use with trusted configuration (e.g., a local test server running over HTTPS with a self-signed cert).
152153

‎docs/features/ssrf-protection.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ config = CachekitIOBackendConfig(
120120
# CACHEKIT_API_URL=https://cache.internal.company.com
121121
```
122122

123-
**Important**: Even with `allow_custom_host=True`, private IP addresses are still blocked. The allowlist bypass only affects hostname validation.
123+
**Important**: Even with `allow_custom_host=True`, private IP addresses and credentials in the URL (`user:password@`) are still rejected. The allowlist bypass only affects hostname validation.
124124

125125
## Environment Variables
126126

‎src/cachekit/backends/cachekitio/config.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,13 @@ def validate_api_url(cls, v: str) -> str:
167167

168168
# Userinfo never authenticates here: httpx sends it as Basic auth in place of the Bearer key,
169169
# and its INFO log prints the full request URL, password included (CWE-532).
170-
if parsed.username is not None or parsed.password is not None:
170+
if "@" in parsed.netloc:
171171
raise ValueError("API URL must not contain credentials (user:password@)")
172172

173173
# Enforce HTTPS protocol
174174
if parsed.scheme != "https":
175-
raise ValueError(f"API URL must use HTTPS protocol, got: {parsed.scheme}://")
175+
# No scheme echo: in "user:pw@host" urlparse reads the username as the scheme.
176+
raise ValueError("API URL must use HTTPS protocol")
176177

177178
# Reject private/internal IP addresses
178179
hostname = parsed.hostname or ""

‎src/cachekit/config/decorator.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,8 @@ def io(cls, api_key: str | None = None, **kwargs: Any) -> DecoratorConfig:
565565
566566
Raises:
567567
ConfigurationError: If the API key is missing, empty or contains whitespace
568-
(argument and CACHEKIT_API_KEY), or if ``backend=`` is passed.
568+
(argument and CACHEKIT_API_KEY), if CACHEKIT_API_URL fails validation, or if
569+
``backend=`` is passed.
569570
570571
Example:
571572
>>> config = DecoratorConfig.io(api_key="ck_test_key", ttl=300) # pragma: allowlist secret

‎tests/unit/backends/test_cachekitio_backend.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ def test_url_with_userinfo_is_rejected(self, mock_sync_client: MagicMock, userin
178178
CachekitIOBackend(api_key=_TEST_API_KEY, api_url=f"https://{userinfo}api.cachekit.io")
179179
assert "SECRET" not in str(info.value)
180180

181+
def test_https_error_never_echoes_a_schemeless_url(self, mock_sync_client: MagicMock) -> None:
182+
"""Without a scheme, urlparse reads the username as one, and the HTTPS error once echoed it."""
183+
url = "SECRETUSER:pw@api.cachekit.io" # pragma: allowlist secret
184+
with pytest.raises(ConfigurationError, match="must use HTTPS") as info:
185+
CachekitIOBackend(api_key=_TEST_API_KEY, api_url=url)
186+
assert "secretuser" not in str(info.value).lower() # urlparse lowercases the scheme
187+
181188
def test_config_error_never_echoes_the_key(self, mock_sync_client: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None:
182189
"""CWE-532: a rejected api_url must not carry the key into the exception text or its chain."""
183190
monkeypatch.delenv("CACHEKIT_ALLOW_CUSTOM_HOST", raising=False)

0 commit comments

Comments
 (0)