diff --git a/CHANGELOG.md b/CHANGELOG.md index e168a234..e86ca359 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - **PR #2520** by @OneFat3 (refs #2247) — Route archive extraction (`/api/upload/extract`) through the per-session attachment inbox (`_session_attachment_dir`) instead of hardcoded `Path(s.workspace)`, matching the single-file upload path. Extracted archives now land at `///` so session deletion cleanup covers them and per-session isolation is preserved when `HERMES_WEBUI_ATTACHMENT_DIR` is configured. - Surface provider fallback and rate-limit lifecycle notices as auto-clearing fallback warnings in the streaming composer status, matching the frontend warning contract. +- **PR #2556** by @Michaelyklam (closes #2541) — Sanitize auto-generated custom-provider API-key environment variable names so endpoint-derived provider ids such as `custom:gpu.local-8000` use POSIX-safe names like `CUSTOM_GPU_LOCAL_8000_API_KEY`. Runtime custom-provider key resolution now checks the sanitized env var first and falls back to the legacy punctuation-preserving name with a one-shot deprecation warning. ## [v0.51.90] — 2026-05-18 — Release BN (stage-383 — 10-PR full sweep batch — empty-gateway messaging history fix + previous-messaging-sessions setting + Kanban board switcher layout + UI/UX demo theme controls + Slice 3c queue/goal RFC gate + keyless custom endpoints + custom-provider remote model catalog parity + auto-compression elapsed timer + new-conversation cold-start guard + Kanban drag-drop detail open fix) diff --git a/api/config.py b/api/config.py index 0dbc462a..846b4d33 100644 --- a/api/config.py +++ b/api/config.py @@ -972,6 +972,49 @@ def _custom_endpoint_slugs_for_base_url(value: object) -> set[str]: return {f"custom:{host}:{port}", f"custom:{host}-{port}"} +_LEGACY_CUSTOM_API_KEY_ENV_WARNED: set[str] = set() + + +def _api_key_env_name(provider_id: object) -> str: + """Return the POSIX-safe default API-key env var for a custom provider id.""" + sanitized = re.sub(r"[^A-Za-z0-9]", "_", str(provider_id or "")).upper().strip("_") + if not sanitized: + sanitized = "CUSTOM" + if not sanitized.startswith("CUSTOM_"): + sanitized = f"CUSTOM_{sanitized}" + return f"{sanitized}_API_KEY" + + +def _legacy_custom_api_key_env_name(provider_id: object) -> str: + """Return the pre-#2541 custom-provider env hint shape, if any.""" + raw = str(provider_id or "").strip().upper() + if not raw: + return "" + return f"{raw}_API_KEY" + + +def _lookup_custom_api_key_env(provider_id: object) -> str | None: + """Look up sanitized custom-provider env first, then legacy broken shape.""" + env_name = _api_key_env_name(provider_id) + api_key = os.getenv(env_name, "").strip() + if api_key: + return api_key + + legacy_env_name = _legacy_custom_api_key_env_name(provider_id) + if legacy_env_name and legacy_env_name != env_name: + legacy_key = os.getenv(legacy_env_name, "").strip() + if legacy_key: + if legacy_env_name not in _LEGACY_CUSTOM_API_KEY_ENV_WARNED: + _LEGACY_CUSTOM_API_KEY_ENV_WARNED.add(legacy_env_name) + logger.warning( + "Custom provider API key env var %s is deprecated; use %s instead", + legacy_env_name, + env_name, + ) + return legacy_key + return None + + def _named_custom_provider_slug_for_base_url( base_url: object, config_obj: dict | None = None, @@ -1841,7 +1884,7 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st # cases after profile switches or runtime config edits. cfg_data = get_config() - def _resolve_key(raw_api_key, raw_key_env) -> str | None: + def _resolve_key(raw_api_key, raw_key_env, provider_hint=None) -> str | None: api_key = None if raw_api_key is not None: key_text = str(raw_api_key).strip() @@ -1853,6 +1896,8 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st key_env = str(raw_key_env or "").strip() if key_env: api_key = os.getenv(key_env, "").strip() or None + if not api_key and provider_hint: + api_key = _lookup_custom_api_key_env(provider_hint) return api_key custom_providers = cfg_data.get("custom_providers", []) @@ -1870,7 +1915,7 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st continue base_url = str(entry.get("base_url") or "").strip() or None - api_key = _resolve_key(entry.get("api_key"), entry.get("key_env")) + api_key = _resolve_key(entry.get("api_key"), entry.get("key_env"), pid) return api_key, base_url # If exactly one custom provider is configured, use it as a pragmatic @@ -1878,7 +1923,7 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st if len(custom_providers) == 1 and isinstance(custom_providers[0], dict): entry = custom_providers[0] return ( - _resolve_key(entry.get("api_key"), entry.get("key_env")), + _resolve_key(entry.get("api_key"), entry.get("key_env"), pid), str(entry.get("base_url") or "").strip() or None, ) @@ -1900,11 +1945,11 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st fallback_key = None if isinstance(provider_specific, dict): - fallback_key = _resolve_key(provider_specific.get("api_key"), provider_specific.get("key_env")) + fallback_key = _resolve_key(provider_specific.get("api_key"), provider_specific.get("key_env"), pid) if not fallback_key and isinstance(provider_custom, dict): - fallback_key = _resolve_key(provider_custom.get("api_key"), provider_custom.get("key_env")) + fallback_key = _resolve_key(provider_custom.get("api_key"), provider_custom.get("key_env"), pid) if not fallback_key and isinstance(model_cfg, dict) and model_provider in {"custom", pid, slug}: - fallback_key = _resolve_key(model_cfg.get("api_key"), model_cfg.get("key_env")) + fallback_key = _resolve_key(model_cfg.get("api_key"), model_cfg.get("key_env"), pid) if fallback_key or fallback_base: return fallback_key, fallback_base or None diff --git a/tests/test_issue2271_keyless_custom_provider.py b/tests/test_issue2271_keyless_custom_provider.py index 9da869df..93aaee9e 100644 --- a/tests/test_issue2271_keyless_custom_provider.py +++ b/tests/test_issue2271_keyless_custom_provider.py @@ -73,3 +73,56 @@ def test_non_custom_provider_is_unchanged(monkeypatch): assert (provider, api_key, base_url) == ("openrouter", None, None) assert called is False + + +def test_custom_provider_env_name_is_posix_safe(): + import api.config as config + + assert config._api_key_env_name("custom:gpu.local-8000") == "CUSTOM_GPU_LOCAL_8000_API_KEY" + assert config._api_key_env_name("custom:10.8.71.41:8080") == "CUSTOM_10_8_71_41_8080_API_KEY" + assert config._api_key_env_name("custom/foo bar") == "CUSTOM_FOO_BAR_API_KEY" + + +def test_resolve_custom_provider_connection_prefers_sanitized_env(monkeypatch): + import api.config as config + + monkeypatch.setattr( + config, + "get_config", + lambda: { + "custom_providers": [ + {"name": "gpu.local-8000", "base_url": "http://gpu.local:8000/v1"}, + ], + }, + ) + monkeypatch.setenv("CUSTOM_GPU_LOCAL_8000_API_KEY", "sanitized-key") + monkeypatch.setenv("CUSTOM:GPU.LOCAL-8000_API_KEY", "legacy-key") + + api_key, base_url = config.resolve_custom_provider_connection("custom:gpu.local-8000") + + assert api_key == "sanitized-key" + assert base_url == "http://gpu.local:8000/v1" + + +def test_resolve_custom_provider_connection_falls_back_to_legacy_env(monkeypatch, caplog): + import logging + import api.config as config + + config._LEGACY_CUSTOM_API_KEY_ENV_WARNED.clear() + monkeypatch.setattr( + config, + "get_config", + lambda: { + "custom_providers": [ + {"name": "gpu.local-8000", "base_url": "http://gpu.local:8000/v1"}, + ], + }, + ) + monkeypatch.delenv("CUSTOM_GPU_LOCAL_8000_API_KEY", raising=False) + monkeypatch.setenv("CUSTOM:GPU.LOCAL-8000_API_KEY", "legacy-key") + + with caplog.at_level(logging.WARNING, logger="api.config"): + api_key, _base_url = config.resolve_custom_provider_connection("custom:gpu.local-8000") + + assert api_key == "legacy-key" + assert "CUSTOM_GPU_LOCAL_8000_API_KEY" in caplog.text