From 4cfc99f4b315a14aefca2bc991319044803811b8 Mon Sep 17 00:00:00 2001 From: Ben Lenarts Date: Sun, 19 Jul 2026 10:11:45 +0200 Subject: [PATCH] fix(transcription): resolve ${VAR} env refs in transcription api_key/api_base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config.loader.load_config() intentionally returns the raw config with ${VAR} references intact — env interpolation is a separate, explicit step (resolve_config_env_vars) so that settings read/edit/save paths never materialize secrets to disk or to the UI. The transcription config path does not apply that step: both channels/base.py (channel voice notes) and webui/transcription_ws.py (WebUI recording) build their effective config via resolve_transcription_config(load_config()). As a result a configured api_key of "${GROQ_API_KEY}" (the documented way to reference secrets) is passed to the provider verbatim, which fails with 401 Invalid API Key. No amount of rotating the real key helps, because the literal placeholder string is what gets sent. Resolve the reference at the single choke point both callers share — _resolve_transcription_api_key / _resolve_transcription_api_base — using a new lenient loader.resolve_env_refs() helper (unset var -> empty string, so a missing variable degrades to "not configured" rather than raising or leaking). This fixes both entry points at once and cannot drift the way a per-call-site fix does. Resolving inside load_config() was rejected: the ~20 settings-UI callers depend on it returning raw ${VAR} placeholders. Literal keys are unaffected; the settings API only reads the derived `configured` flag (never the key), which now reflects the resolved value. Claude-Session: https://claude.ai/code/session_01Q3HuVaJAAQJA3kgVQVJ2Zt --- nanobot/audio/transcription.py | 5 ++-- nanobot/config/loader.py | 15 ++++++++++ tests/providers/test_transcription.py | 40 +++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/nanobot/audio/transcription.py b/nanobot/audio/transcription.py index 92dffdf7..539f90b9 100644 --- a/nanobot/audio/transcription.py +++ b/nanobot/audio/transcription.py @@ -20,6 +20,7 @@ from nanobot.audio.transcription_registry import ( get_transcription_provider, resolve_transcription_provider, ) +from nanobot.config.loader import resolve_env_refs from nanobot.config.paths import get_media_dir from nanobot.providers.registry import find_by_name from nanobot.utils.media_decode import FileSizeExceeded, save_base64_data_url @@ -82,7 +83,7 @@ def _provider_default_api_base(provider: str) -> str | None: def _resolve_transcription_api_key(provider: str, provider_cfg: Any) -> str: - api_key = getattr(provider_cfg, "api_key", None) if provider_cfg else None + api_key = resolve_env_refs(getattr(provider_cfg, "api_key", None) or "") if provider_cfg else "" if api_key: return api_key @@ -97,7 +98,7 @@ def _resolve_transcription_api_key(provider: str, provider_cfg: Any) -> str: def _resolve_transcription_api_base(provider: str, provider_cfg: Any) -> str: - api_base = getattr(provider_cfg, "api_base", None) if provider_cfg else None + api_base = resolve_env_refs(getattr(provider_cfg, "api_base", None) or "") if provider_cfg else "" if api_base: return api_base return _provider_default_api_base(provider) or "" diff --git a/nanobot/config/loader.py b/nanobot/config/loader.py index 5f526ed5..4668be98 100644 --- a/nanobot/config/loader.py +++ b/nanobot/config/loader.py @@ -117,6 +117,21 @@ def resolve_config_env_vars(config: Config) -> Config: return _resolve_in_place(config) +def resolve_env_refs(value: str) -> str: + """Resolve ``${VAR}`` references in a single string, leniently. + + Unlike :func:`resolve_config_env_vars` (which walks a whole ``Config`` and + raises on a missing variable), this resolves one value and substitutes an + empty string for any unset reference. It is meant for individual, lazily + consumed secret fields — e.g. a transcription provider's ``api_key`` — so a + missing variable degrades to "not configured" instead of sending the literal + ``${VAR}`` text to the provider. Non-string input is returned unchanged. + """ + if not isinstance(value, str): + return value + return _ENV_REF_PATTERN.sub(lambda m: os.environ.get(m.group(1), ""), value) + + def _resolve_in_place(obj: Any) -> Any: if isinstance(obj, str): new = _ENV_REF_PATTERN.sub(_env_replace, obj) diff --git a/tests/providers/test_transcription.py b/tests/providers/test_transcription.py index c0acae59..72256d8b 100644 --- a/tests/providers/test_transcription.py +++ b/tests/providers/test_transcription.py @@ -157,6 +157,46 @@ def test_resolver_supports_siliconflow_transcription_api_key_env() -> None: assert resolved.api_base == "https://api.siliconflow.cn/v1" +def test_resolver_interpolates_env_ref_in_api_key() -> None: + # load_config() does not interpolate ${VAR}; the transcription path receives + # the raw config, so an env reference in the key must be resolved here rather + # than sent verbatim to the provider (which yields a 401). + config = Config() + config.transcription.provider = "groq" + config.providers.groq.api_key = "${MY_GROQ_KEY}" + + with patch.dict(os.environ, {"MY_GROQ_KEY": "gsk-real-value"}, clear=True): + resolved = resolve_transcription_config(config) + + assert resolved.api_key == "gsk-real-value" + + +def test_resolver_env_ref_missing_var_degrades_to_not_configured() -> None: + config = Config() + config.transcription.provider = "groq" + config.providers.groq.api_key = "${MISSING_GROQ_KEY}" + + with patch.dict(os.environ, {}, clear=True): + resolved = resolve_transcription_config(config) + + # Unresolved reference degrades to a falsy key rather than the literal + # "${...}" string, so the config reports itself as not configured. + assert not resolved.api_key + assert resolved.configured is False + + +def test_resolver_interpolates_env_ref_in_api_base() -> None: + config = Config() + config.transcription.provider = "groq" + config.providers.groq.api_key = "gsk-test" + config.providers.groq.api_base = "${MY_GROQ_BASE}" + + with patch.dict(os.environ, {"MY_GROQ_BASE": "https://groq.example/v1"}, clear=True): + resolved = resolve_transcription_config(config) + + assert resolved.api_base == "https://groq.example/v1" + + def test_resolver_supports_xiaomi_mimo_transcription_provider() -> None: config = Config() config.transcription.provider = "xiaomi_mimo"