diff --git a/src/conclave/adapters/base.py b/src/conclave/adapters/base.py index b92bcd6..2bfc7a2 100644 --- a/src/conclave/adapters/base.py +++ b/src/conclave/adapters/base.py @@ -15,8 +15,9 @@ malformed payloads. Its message is ALREADY scrubbed via :func:`redact` so it is safe to surface in ``ModelAnswer.error``. * :func:`redact` -- key-leak hardening. Strips bearer tokens, ``sk-`` style - keys, ``x-api-key`` echoes, and any value of the env vars we hold names for, - before an error string can ever escape the call path. + keys, ``x-api-key`` echoes, and any value of the env vars we hold names for -- + built-in providers AND custom-endpoint ``api_key_env`` names declared in + config -- before an error string can ever escape the call path. """ from __future__ import annotations @@ -38,10 +39,29 @@ _REDACTED = "[REDACTED]" +def _custom_endpoint_env_vars() -> list[str]: + """Return the env-var NAMES declared by custom endpoints, or [] on any error. + + Custom OpenAI-compatible endpoints (``config.endpoints[*].env_var``) name a + key var that is NOT in :data:`PROVIDER_ENV_VARS`, so :func:`redact` would not + otherwise know to mask its value -- the BYO-keys leak class. We load config + here (lazily, to avoid an import cycle) purely to learn those names; failures + must never break redaction, so any error yields an empty list and we fall + back to pattern-based scrubbing only. + """ + try: + from ..config import load_config + + return [ep.env_var for ep in load_config().endpoints.values() if ep.env_var] + except Exception: # noqa: BLE001 -- redaction must never raise + return [] + + def redact(text: str) -> str: """Scrub anything key-shaped from a string before it can be surfaced. - Removes, in order: any live value of an env var we know a name for, + Removes, in order: any live value of an env var we know a name for -- + including custom-endpoint ``api_key_env`` names declared in config -- ``Bearer `` auth headers, ``x-api-key``/``x-goog-api-key`` header echoes, and standalone provider-style key tokens (``sk-``, ``xai-``, ``pplx-``, ``AIza...``). Idempotent and safe on already-clean text. @@ -55,13 +75,15 @@ def redact(text: str) -> str: if not text: return text cleaned = text - # 1) Redact concrete env-var values first (most authoritative). We only read - # values here to mask them; the masked result never contains the value. - for names in PROVIDER_ENV_VARS.values(): - for name in names: - value = os.environ.get(name, "").strip() - if value: - cleaned = cleaned.replace(value, _REDACTED) + # 1) Redact concrete env-var values first (most authoritative). This covers + # the built-in providers AND any custom-endpoint key var declared in config, + # so a BYO custom key with an unrecognized shape is still masked. We only + # read values here to mask them; the masked result never contains the value. + builtin_names = [name for names in PROVIDER_ENV_VARS.values() for name in names] + for name in builtin_names + _custom_endpoint_env_vars(): + value = os.environ.get(name, "").strip() + if value: + cleaned = cleaned.replace(value, _REDACTED) # 2) Header-shaped echoes. cleaned = _HEADER_KEY_RE.sub(rf"\1: {_REDACTED}", cleaned) # 3) Bearer auth headers. diff --git a/tests/test_providers.py b/tests/test_providers.py index 1d8beba..001b3a1 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -144,6 +144,47 @@ async def test_call_model_unknown_provider_is_error(monkeypatch): assert "unknown provider 'mystery'" in answer.error +async def test_call_model_custom_endpoint_key_not_leaked_in_error(monkeypatch, tmp_path): + """A custom-endpoint key value echoed in a provider error is scrubbed (issue #14). + + Repro: declare a custom OpenAI-compatible endpoint whose api_key_env is NOT + in PROVIDER_ENV_VARS and whose value has no recognized prefix, then have the + mocked transport return a 401 whose error message echoes that key. The + resulting ModelAnswer.error must not contain the key value anywhere. + """ + # Obviously-synthetic, unprefixed fake key -- no sk-/xai-/pplx-/AIza shape, + # so pattern-based scrubbing alone would miss it; only name-based scrubbing + # via the custom endpoint's env var saves it. + fake_key = "togetherFAKEsecret_unprefixed_0123456789" + monkeypatch.setenv("TOGETHER_API_KEY", fake_key) + + config_file = tmp_path / "conclave.yml" + config_file.write_text( + "endpoints:\n" + " together:\n" + " completions_url: https://api.together.xyz/v1/chat/completions\n" + " env_var: TOGETHER_API_KEY\n", + encoding="utf-8", + ) + monkeypatch.setenv("CONCLAVE_CONFIG", str(config_file)) + + async def echoing_401(url, headers, json_body, timeout): + # Simulate a gateway that echoes the submitted credential on auth failure. + return 401, {"error": {"message": f"invalid api key: {fake_key}"}} + + monkeypatch.setattr("conclave.transport.post_json", echoing_401) + + answer = await call_model( + "together", + "together/some-model", + [{"role": "user", "content": "hi"}], + ) + assert not answer.ok + assert answer.error is not None + assert fake_key not in answer.error + assert "[REDACTED]" in answer.error + + # --------------------------------------------------------------------------- # # Redaction # --------------------------------------------------------------------------- # @@ -175,3 +216,27 @@ def test_provider_error_message_is_pre_redacted(): err = ProviderError("openai: HTTP 401: Bearer sk-leakedTOKEN12345") assert "sk-leakedTOKEN12345" not in str(err) assert "[REDACTED]" in str(err) + + +def test_redact_scrubs_custom_endpoint_env_var_value(monkeypatch, tmp_path): + """An unprefixed custom-endpoint key value is scrubbed via config (issue #14). + + The key has no recognized provider prefix, so only name-based scrubbing + sourced from config.endpoints[*].env_var can catch it. + """ + fake_key = "togetherFAKEsecret_unprefixed_0123456789" + monkeypatch.setenv("TOGETHER_API_KEY", fake_key) + + config_file = tmp_path / "conclave.yml" + config_file.write_text( + "endpoints:\n" + " together:\n" + " completions_url: https://api.together.xyz/v1/chat/completions\n" + " env_var: TOGETHER_API_KEY\n", + encoding="utf-8", + ) + monkeypatch.setenv("CONCLAVE_CONFIG", str(config_file)) + + cleaned = redact(f"auth failed: invalid api key: {fake_key}") + assert fake_key not in cleaned + assert "[REDACTED]" in cleaned