diff --git a/backend_api_python/app/routes/settings.py b/backend_api_python/app/routes/settings.py index 07a5ffc5c..df48ef8e0 100644 --- a/backend_api_python/app/routes/settings.py +++ b/backend_api_python/app/routes/settings.py @@ -14,6 +14,7 @@ from app.utils.auth import login_required, admin_required from app.services.settings.branding import build_brand_config from app.services.settings.env_file import read_env_file, write_env_file +from app.utils.redaction import redact_secrets from app.services.settings.runtime import reload_runtime_env, refresh_runtime_services logger = get_logger(__name__) @@ -2475,8 +2476,10 @@ def test_connection(): return jsonify({'code': 0, 'msg': 'Unknown service'}) except Exception as e: - logger.error(f"Connection test failed: {e}") - return jsonify({'code': 0, 'msg': f'Test failed: {str(e)}'}) + # The Finnhub endpoint carries the key as a query parameter, and requests + # embeds the URL in exception text, so scrub before logging/returning it. + logger.error(f"Connection test failed: {redact_secrets(e)}") + return jsonify({'code': 0, 'msg': f'Test failed: {redact_secrets(e)}'}) # openapi-compat: legacy import name settings_bp = settings_blp diff --git a/backend_api_python/app/services/llm.py b/backend_api_python/app/services/llm.py index 264463a35..8ea0ae291 100644 --- a/backend_api_python/app/services/llm.py +++ b/backend_api_python/app/services/llm.py @@ -13,6 +13,7 @@ from app.utils.logger import get_logger from app.config import APIKeys from app.utils.config_loader import load_addon_config +from app.utils.redaction import redact_secrets logger = get_logger(__name__) @@ -357,7 +358,7 @@ def _llm_post(self, url: str, *, headers: dict, json_payload: dict, timeout: int "for direct LLM access, or set it to a reachable proxy and keep " "LLM_USE_SYSTEM_PROXY disabled unless you really want system proxy env vars." ) - raise requests.exceptions.ConnectionError(f"{msg}{hint}") from exc + raise requests.exceptions.ConnectionError(f"{redact_secrets(msg)}{hint}") from exc if stream: response._quantdinger_llm_session = session @@ -610,7 +611,9 @@ def _format_provider_error_value(cls, value) -> str: def _call_google_gemini(self, messages: list, model: str, temperature: float, api_key: str, base_url: str, timeout: int) -> str: """Call Google Gemini API.""" - url = f"{base_url}/models/{model}:generateContent?key={api_key}" + # The key travels in a header, not the query string: request URLs end up + # inside requests' exception messages, which are logged. + url = f"{base_url}/models/{model}:generateContent" # Convert OpenAI message format to Gemini format contents = [] @@ -656,7 +659,10 @@ def _call_google_gemini(self, messages: list, model: str, temperature: float, if system_instruction: data["systemInstruction"] = {"parts": [{"text": system_instruction}]} - headers = {"Content-Type": "application/json"} + headers = { + "Content-Type": "application/json", + "x-goog-api-key": str(api_key or "").strip(), + } response = self._llm_post(url, headers=headers, json_payload=data, timeout=timeout) response.raise_for_status() @@ -1288,7 +1294,7 @@ def call_llm_api(self, messages: list, model: str = None, temperature: float = 0 status_code = e.response.status_code if e.response else None last_status_code = status_code - logger.error(f"{p.value} API HTTP error ({current_model}): {status_code} - {error_detail}") + logger.error(f"{p.value} API HTTP error ({current_model}): {status_code} - {redact_secrets(error_detail)}") last_error = str(e) # 403/402 errors usually mean API key issue - try alternative provider @@ -1310,13 +1316,13 @@ def call_llm_api(self, messages: list, model: str = None, temperature: float = 0 raise except requests.exceptions.RequestException as e: - logger.error(f"{p.value} API request error ({current_model}): {str(e)}") + logger.error(f"{p.value} API request error ({current_model}): {redact_secrets(e)}") last_error = str(e) if not use_fallback or current_model == models_to_try[-1]: raise except ValueError as e: - logger.warning(f"Model {current_model} returned invalid data: {str(e)}") + logger.warning(f"Model {current_model} returned invalid data: {redact_secrets(e)}") last_error = str(e) if current_model == models_to_try[-1]: raise @@ -1326,8 +1332,8 @@ def call_llm_api(self, messages: list, model: str = None, temperature: float = 0 error_msg += f"\nStatus {last_status_code} usually means: API key invalid/expired, insufficient balance, or no access to model." error_msg += f"\nPlease check your {p.value} API key configuration and account balance." - logger.error(error_msg) - raise Exception(error_msg) + logger.error(redact_secrets(error_msg)) + raise Exception(redact_secrets(error_msg)) def stream_llm_api(self, messages: list, model: str = None, temperature: float = 0.7): """Stream LLM response deltas for providers with OpenAI-compatible streaming.""" @@ -1390,7 +1396,7 @@ def _try_alternative_providers(self, messages: list, model: str, temperature: fl try_alternative_providers=False # Prevent infinite recursion ) except Exception as e: - logger.warning(f"Alternative provider {alt_provider.value} also failed: {str(e)}") + logger.warning(f"Alternative provider {alt_provider.value} also failed: {redact_secrets(e)}") continue raise Exception(f"All LLM providers failed. Please check your API key configurations.") @@ -1440,8 +1446,8 @@ def safe_call_llm(self, system_prompt: str, user_prompt: str, default_structure: default_structure['report'] = f"Failed to parse analysis result JSON. Raw output (partial): {response_text[:500] if response_text else 'N/A'}" return default_structure except Exception as e: - logger.error(f"LLM call failed: {str(e)}") - default_structure['report'] = f"Analysis failed: {str(e)}" + logger.error(f"LLM call failed: {redact_secrets(e)}") + default_structure['report'] = f"Analysis failed: {redact_secrets(e)}" return default_structure @classmethod diff --git a/backend_api_python/app/utils/redaction.py b/backend_api_python/app/utils/redaction.py new file mode 100644 index 000000000..dca27b4e6 --- /dev/null +++ b/backend_api_python/app/utils/redaction.py @@ -0,0 +1,28 @@ +"""Scrub credentials out of strings before they reach logs or API responses. + +Providers embed the request URL in exception messages, and some APIs carry the +key as a query parameter, so anything derived from an exception must be scrubbed +before being logged, re-raised, or returned to a client. +""" + +from __future__ import annotations + +import re +from typing import Any + +_SECRET_QUERY_PATTERN = re.compile( + r"(?i)\b([a-z0-9_]*(?:api[_-]?key|apikey|key|access[_-]?token|token|secret|password))\s*=\s*([^&\s'\"]+)" +) +_BEARER_PATTERN = re.compile(r"(?i)(bearer\s+)[A-Za-z0-9._\-]{8,}") + + +def redact_secrets(text: Any) -> str: + """Return ``text`` with credentials stripped (query params and bearer tokens). + + Non-secret context (hosts, paths, causes) is preserved so failures stay + debuggable. + """ + if text is None: + return "" + scrubbed = _SECRET_QUERY_PATTERN.sub(r"\1=", str(text)) + return _BEARER_PATTERN.sub(r"\1", scrubbed) diff --git a/backend_api_python/tests/test_llm_litellm_provider.py b/backend_api_python/tests/test_llm_litellm_provider.py index 4d4f43a54..76819f75c 100644 --- a/backend_api_python/tests/test_llm_litellm_provider.py +++ b/backend_api_python/tests/test_llm_litellm_provider.py @@ -3,6 +3,7 @@ import pytest +from app.utils.redaction import redact_secrets from app.services.llm import LLMAPIError, LLMProvider, LLMService import app.utils.config_loader as config_loader from app.utils.config_loader import clear_config_cache, load_addon_config @@ -701,3 +702,62 @@ def completion(**kwargs): assert out == "hello" assert captured["max_tokens"] == 16384 + + +def test_google_gemini_sends_api_key_in_header_not_url(monkeypatch): + """The Gemini key must never sit in the request URL: requests embeds URLs in + its exception messages, which the service logs.""" + captured = {} + + class FakeResponse: + def raise_for_status(self): + return None + + def json(self): + return {"candidates": [{"content": {"parts": [{"text": "ok"}]}}]} + + service = LLMService(provider="google") + + def fake_post(url, **kwargs): + captured["url"] = url + captured["headers"] = kwargs.get("headers") or {} + return FakeResponse() + + monkeypatch.setattr(service, "_llm_post", fake_post) + + service._call_google_gemini( + [{"role": "user", "content": "hello"}], + "gemini-1.5-flash", + 0.7, + "super-secret-key", + "https://generativelanguage.googleapis.com/v1beta", + 30, + ) + + assert "super-secret-key" not in captured["url"] + assert "key=" not in captured["url"] + assert captured["headers"]["x-goog-api-key"] == "super-secret-key" + + +def test_redact_secrets_scrubs_query_keys_and_bearer_tokens(): + message = ( + "HTTPSConnectionPool(host='generativelanguage.googleapis.com', port=443): " + "Max retries exceeded with url: /v1beta/models/gemini-1.5-flash:generateContent" + "?key=AIzaSyD-EXAMPLE-KEY (Caused by NewConnectionError) " + "headers={'Authorization': 'Bearer sk-abc123DEF456ghi'}" + ) + + scrubbed = redact_secrets(message) + + assert "AIzaSyD-EXAMPLE-KEY" not in scrubbed + assert "sk-abc123DEF456ghi" not in scrubbed + assert "key=" in scrubbed + assert "Bearer " in scrubbed + # Non-secret context is preserved for debugging. + assert "generativelanguage.googleapis.com" in scrubbed + + +def test_redact_secrets_passes_through_harmless_text(): + assert redact_secrets("") == "" + assert redact_secrets(None) == "" + assert redact_secrets("plain failure") == "plain failure"