Spaces:
Sleeping
Sleeping
Kennedy Johnson Cursor commited on
Commit ·
2d9affa
1
Parent(s): a6b00c6
Harden OpenAI key handling and stop echoing keys in chat errors.
Browse filesStrip whitespace/quotes from OPENAI_API_KEY and map 401 invalid_api_key to a safe client message so Space secret misconfiguration is clearer without leaking key material.
Co-authored-by: Cursor <cursoragent@cursor.com>
- multi_llm_chatbot_backend/app/api/routes/chat.py +4 -2
- multi_llm_chatbot_backend/app/api/routes/root.py +10 -2
- multi_llm_chatbot_backend/app/core/bootstrap.py +3 -2
- multi_llm_chatbot_backend/app/core/secrets.py +40 -0
- multi_llm_chatbot_backend/app/llm/openai_fallback_client.py +16 -5
- multi_llm_chatbot_backend/app/tests/unit/test_secrets.py +39 -0
multi_llm_chatbot_backend/app/api/routes/chat.py
CHANGED
|
@@ -14,6 +14,7 @@ from app.api.utils import get_or_create_session_for_request_async, load_chat_ses
|
|
| 14 |
from app.core.auth import get_current_active_user
|
| 15 |
from app.core.bootstrap import chat_orchestrator
|
| 16 |
from app.core.database import get_database
|
|
|
|
| 17 |
from app.core.session_manager import get_session_manager
|
| 18 |
from app.models.user import User
|
| 19 |
from app.api.routes.user_profile import (
|
|
@@ -215,10 +216,11 @@ async def chat_stream(
|
|
| 215 |
except Exception as e:
|
| 216 |
logger.exception(f"chat-stream _run failed for {pid}: {e}")
|
| 217 |
failed_persona = chat_orchestrator.get_persona(pid)
|
|
|
|
| 218 |
await done_queue.put({
|
| 219 |
"persona_id": pid,
|
| 220 |
"persona_name": failed_persona.name if failed_persona else pid,
|
| 221 |
-
"response": f"I ran into a technical issue. Please try again. ({
|
| 222 |
"used_documents": False,
|
| 223 |
"document_chunks_used": 0,
|
| 224 |
})
|
|
@@ -251,7 +253,7 @@ async def chat_stream(
|
|
| 251 |
logger.error(traceback.format_exc())
|
| 252 |
yield ChatStreamLine(
|
| 253 |
type="error",
|
| 254 |
-
data={"detail":
|
| 255 |
).to_ndjson()
|
| 256 |
|
| 257 |
return StreamingResponse(
|
|
|
|
| 14 |
from app.core.auth import get_current_active_user
|
| 15 |
from app.core.bootstrap import chat_orchestrator
|
| 16 |
from app.core.database import get_database
|
| 17 |
+
from app.core.secrets import client_safe_error_message
|
| 18 |
from app.core.session_manager import get_session_manager
|
| 19 |
from app.models.user import User
|
| 20 |
from app.api.routes.user_profile import (
|
|
|
|
| 216 |
except Exception as e:
|
| 217 |
logger.exception(f"chat-stream _run failed for {pid}: {e}")
|
| 218 |
failed_persona = chat_orchestrator.get_persona(pid)
|
| 219 |
+
safe = client_safe_error_message(e)
|
| 220 |
await done_queue.put({
|
| 221 |
"persona_id": pid,
|
| 222 |
"persona_name": failed_persona.name if failed_persona else pid,
|
| 223 |
+
"response": f"I ran into a technical issue. Please try again. ({safe})",
|
| 224 |
"used_documents": False,
|
| 225 |
"document_chunks_used": 0,
|
| 226 |
})
|
|
|
|
| 253 |
logger.error(traceback.format_exc())
|
| 254 |
yield ChatStreamLine(
|
| 255 |
type="error",
|
| 256 |
+
data={"detail": client_safe_error_message(exc)},
|
| 257 |
).to_ndjson()
|
| 258 |
|
| 259 |
return StreamingResponse(
|
multi_llm_chatbot_backend/app/api/routes/root.py
CHANGED
|
@@ -1,8 +1,10 @@
|
|
| 1 |
from fastapi import APIRouter
|
| 2 |
from app.config import get_settings
|
|
|
|
| 3 |
from app.version import __version__
|
| 4 |
|
| 5 |
import logging
|
|
|
|
| 6 |
|
| 7 |
logger = logging.getLogger(__name__)
|
| 8 |
|
|
@@ -14,10 +16,17 @@ router = APIRouter()
|
|
| 14 |
# leave users staring at this JSON banner instead of the app.
|
| 15 |
@router.get("/api/health")
|
| 16 |
def root():
|
| 17 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 18 |
return {
|
| 19 |
"message": f"{title} Backend is up and running",
|
| 20 |
"version": __version__,
|
|
|
|
|
|
|
| 21 |
"features": [
|
| 22 |
"Configurable Personas",
|
| 23 |
"Improved Session Management",
|
|
@@ -27,4 +36,3 @@ def root():
|
|
| 27 |
"Provider Switching"
|
| 28 |
]
|
| 29 |
}
|
| 30 |
-
|
|
|
|
| 1 |
from fastapi import APIRouter
|
| 2 |
from app.config import get_settings
|
| 3 |
+
from app.core.secrets import normalize_secret
|
| 4 |
from app.version import __version__
|
| 5 |
|
| 6 |
import logging
|
| 7 |
+
import os
|
| 8 |
|
| 9 |
logger = logging.getLogger(__name__)
|
| 10 |
|
|
|
|
| 16 |
# leave users staring at this JSON banner instead of the app.
|
| 17 |
@router.get("/api/health")
|
| 18 |
def root():
|
| 19 |
+
settings = get_settings()
|
| 20 |
+
title = settings.app.title
|
| 21 |
+
openai_key_configured = bool(
|
| 22 |
+
normalize_secret(settings.llm.openai.api_key)
|
| 23 |
+
or normalize_secret(os.getenv("OPENAI_API_KEY", ""))
|
| 24 |
+
)
|
| 25 |
return {
|
| 26 |
"message": f"{title} Backend is up and running",
|
| 27 |
"version": __version__,
|
| 28 |
+
"llm_provider": settings.llm.provider,
|
| 29 |
+
"openai_api_key_configured": openai_key_configured,
|
| 30 |
"features": [
|
| 31 |
"Configurable Personas",
|
| 32 |
"Improved Session Management",
|
|
|
|
| 36 |
"Provider Switching"
|
| 37 |
]
|
| 38 |
}
|
|
|
multi_llm_chatbot_backend/app/core/bootstrap.py
CHANGED
|
@@ -3,6 +3,7 @@ import os
|
|
| 3 |
from pathlib import Path
|
| 4 |
|
| 5 |
from app.core.env_loader import load_application_env
|
|
|
|
| 6 |
|
| 7 |
load_application_env()
|
| 8 |
|
|
@@ -32,7 +33,7 @@ def _load_shared_env_var(name: str) -> str:
|
|
| 32 |
for line in shared.read_text(encoding="utf-8").splitlines():
|
| 33 |
line = line.strip()
|
| 34 |
if line.startswith(prefix):
|
| 35 |
-
return line.split("=", 1)[1]
|
| 36 |
return ""
|
| 37 |
|
| 38 |
|
|
@@ -73,7 +74,7 @@ def _vllm_api_key() -> str:
|
|
| 73 |
|
| 74 |
|
| 75 |
def _openai_api_key() -> str:
|
| 76 |
-
return (
|
| 77 |
settings.llm.openai.api_key
|
| 78 |
or os.getenv("OPENAI_API_KEY", "")
|
| 79 |
or _load_shared_env_var("OPENAI_API_KEY")
|
|
|
|
| 3 |
from pathlib import Path
|
| 4 |
|
| 5 |
from app.core.env_loader import load_application_env
|
| 6 |
+
from app.core.secrets import normalize_secret
|
| 7 |
|
| 8 |
load_application_env()
|
| 9 |
|
|
|
|
| 33 |
for line in shared.read_text(encoding="utf-8").splitlines():
|
| 34 |
line = line.strip()
|
| 35 |
if line.startswith(prefix):
|
| 36 |
+
return normalize_secret(line.split("=", 1)[1])
|
| 37 |
return ""
|
| 38 |
|
| 39 |
|
|
|
|
| 74 |
|
| 75 |
|
| 76 |
def _openai_api_key() -> str:
|
| 77 |
+
return normalize_secret(
|
| 78 |
settings.llm.openai.api_key
|
| 79 |
or os.getenv("OPENAI_API_KEY", "")
|
| 80 |
or _load_shared_env_var("OPENAI_API_KEY")
|
multi_llm_chatbot_backend/app/core/secrets.py
ADDED
|
@@ -0,0 +1,40 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Helpers for loading secrets and redacting them from client-facing errors."""
|
| 2 |
+
|
| 3 |
+
from __future__ import annotations
|
| 4 |
+
|
| 5 |
+
import re
|
| 6 |
+
|
| 7 |
+
_SK_RE = re.compile(r"sk-(?:proj-)?[A-Za-z0-9_\-]{6,}")
|
| 8 |
+
|
| 9 |
+
|
| 10 |
+
def normalize_secret(value: str | None) -> str:
|
| 11 |
+
"""Strip whitespace and accidental surrounding quotes from a secret value.
|
| 12 |
+
|
| 13 |
+
Hugging Face Space secret pastes sometimes include trailing newlines or
|
| 14 |
+
wrapping quotes; either produces an otherwise-valid-looking key that
|
| 15 |
+
OpenAI rejects with 401 invalid_api_key.
|
| 16 |
+
"""
|
| 17 |
+
text = (value or "").strip()
|
| 18 |
+
if len(text) >= 2 and text[0] == text[-1] and text[0] in ("'", '"'):
|
| 19 |
+
text = text[1:-1].strip()
|
| 20 |
+
return text
|
| 21 |
+
|
| 22 |
+
|
| 23 |
+
def client_safe_error_message(exc: BaseException) -> str:
|
| 24 |
+
"""Return an error string safe to show end users (no API key material)."""
|
| 25 |
+
text = str(exc)
|
| 26 |
+
lowered = text.lower()
|
| 27 |
+
if "invalid_api_key" in lowered or "incorrect api key" in lowered:
|
| 28 |
+
return (
|
| 29 |
+
"OpenAI rejected the API key. Update the OPENAI_API_KEY secret under "
|
| 30 |
+
"Hugging Face Space Settings → Variables and secrets, then restart "
|
| 31 |
+
"the Space."
|
| 32 |
+
)
|
| 33 |
+
if "authenticationerror" in type(exc).__name__.lower() or (
|
| 34 |
+
"401" in text and "api" in lowered
|
| 35 |
+
):
|
| 36 |
+
return (
|
| 37 |
+
"OpenAI authentication failed. Check that OPENAI_API_KEY is set "
|
| 38 |
+
"correctly in Space secrets and that the key is still valid."
|
| 39 |
+
)
|
| 40 |
+
return _SK_RE.sub("sk-***", text)
|
multi_llm_chatbot_backend/app/llm/openai_fallback_client.py
CHANGED
|
@@ -6,10 +6,11 @@ import json
|
|
| 6 |
import logging
|
| 7 |
from typing import Any, Callable, Dict, List, Optional
|
| 8 |
|
| 9 |
-
from openai import AsyncOpenAI, APIConnectionError, APIStatusError
|
| 10 |
|
| 11 |
from app.llm.llm_client import LLMClient, ToolCallInfo, ToolCallResult
|
| 12 |
from app.core.context_manager import get_context_manager
|
|
|
|
| 13 |
|
| 14 |
logger = logging.getLogger(__name__)
|
| 15 |
|
|
@@ -27,6 +28,7 @@ class OpenAIFallbackClient(LLMClient):
|
|
| 27 |
model: str = "gpt-5.4",
|
| 28 |
reasoning_effort: Optional[str] = None,
|
| 29 |
):
|
|
|
|
| 30 |
if not api_key:
|
| 31 |
raise ValueError("OpenAI API key not set. Provide OPENAI_API_KEY or llm.openai.api_key.")
|
| 32 |
self.model = model
|
|
@@ -34,6 +36,11 @@ class OpenAIFallbackClient(LLMClient):
|
|
| 34 |
self.client = AsyncOpenAI(api_key=api_key, timeout=120.0)
|
| 35 |
self.context_manager = get_context_manager()
|
| 36 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 37 |
_ALLOWED_ROLES = {"system", "assistant", "user", "function", "tool", "developer"}
|
| 38 |
|
| 39 |
def _reasoning_kwargs(self, *, with_tools: bool = False) -> Dict[str, Any]:
|
|
@@ -100,11 +107,13 @@ class OpenAIFallbackClient(LLMClient):
|
|
| 100 |
if not text:
|
| 101 |
raise ValueError("OpenAI returned empty content")
|
| 102 |
return self._clean_response(text)
|
|
|
|
|
|
|
| 103 |
except (APIConnectionError, APIStatusError) as exc:
|
| 104 |
-
logger.error("OpenAI API error: %s", exc)
|
| 105 |
raise
|
| 106 |
except Exception as exc:
|
| 107 |
-
logger.error("OpenAI generate failed: %s", exc)
|
| 108 |
raise
|
| 109 |
|
| 110 |
_MAX_TOOL_ROUNDS = 5
|
|
@@ -168,9 +177,11 @@ class OpenAIFallbackClient(LLMClient):
|
|
| 168 |
})
|
| 169 |
|
| 170 |
raise ValueError("OpenAI tool-calling loop exhausted max rounds")
|
|
|
|
|
|
|
| 171 |
except (APIConnectionError, APIStatusError) as exc:
|
| 172 |
-
logger.error("OpenAI tool API error: %s", exc)
|
| 173 |
raise
|
| 174 |
except Exception as exc:
|
| 175 |
-
logger.error("OpenAI generate_with_tools failed: %s", exc)
|
| 176 |
raise
|
|
|
|
| 6 |
import logging
|
| 7 |
from typing import Any, Callable, Dict, List, Optional
|
| 8 |
|
| 9 |
+
from openai import AsyncOpenAI, APIConnectionError, APIStatusError, AuthenticationError
|
| 10 |
|
| 11 |
from app.llm.llm_client import LLMClient, ToolCallInfo, ToolCallResult
|
| 12 |
from app.core.context_manager import get_context_manager
|
| 13 |
+
from app.core.secrets import client_safe_error_message, normalize_secret
|
| 14 |
|
| 15 |
logger = logging.getLogger(__name__)
|
| 16 |
|
|
|
|
| 28 |
model: str = "gpt-5.4",
|
| 29 |
reasoning_effort: Optional[str] = None,
|
| 30 |
):
|
| 31 |
+
api_key = normalize_secret(api_key)
|
| 32 |
if not api_key:
|
| 33 |
raise ValueError("OpenAI API key not set. Provide OPENAI_API_KEY or llm.openai.api_key.")
|
| 34 |
self.model = model
|
|
|
|
| 36 |
self.client = AsyncOpenAI(api_key=api_key, timeout=120.0)
|
| 37 |
self.context_manager = get_context_manager()
|
| 38 |
|
| 39 |
+
@staticmethod
|
| 40 |
+
def _auth_failure(exc: AuthenticationError) -> RuntimeError:
|
| 41 |
+
logger.error("OpenAI authentication failed (invalid or missing API key)")
|
| 42 |
+
return RuntimeError(client_safe_error_message(exc))
|
| 43 |
+
|
| 44 |
_ALLOWED_ROLES = {"system", "assistant", "user", "function", "tool", "developer"}
|
| 45 |
|
| 46 |
def _reasoning_kwargs(self, *, with_tools: bool = False) -> Dict[str, Any]:
|
|
|
|
| 107 |
if not text:
|
| 108 |
raise ValueError("OpenAI returned empty content")
|
| 109 |
return self._clean_response(text)
|
| 110 |
+
except AuthenticationError as exc:
|
| 111 |
+
raise self._auth_failure(exc) from None
|
| 112 |
except (APIConnectionError, APIStatusError) as exc:
|
| 113 |
+
logger.error("OpenAI API error: %s", client_safe_error_message(exc))
|
| 114 |
raise
|
| 115 |
except Exception as exc:
|
| 116 |
+
logger.error("OpenAI generate failed: %s", client_safe_error_message(exc))
|
| 117 |
raise
|
| 118 |
|
| 119 |
_MAX_TOOL_ROUNDS = 5
|
|
|
|
| 177 |
})
|
| 178 |
|
| 179 |
raise ValueError("OpenAI tool-calling loop exhausted max rounds")
|
| 180 |
+
except AuthenticationError as exc:
|
| 181 |
+
raise self._auth_failure(exc) from None
|
| 182 |
except (APIConnectionError, APIStatusError) as exc:
|
| 183 |
+
logger.error("OpenAI tool API error: %s", client_safe_error_message(exc))
|
| 184 |
raise
|
| 185 |
except Exception as exc:
|
| 186 |
+
logger.error("OpenAI generate_with_tools failed: %s", client_safe_error_message(exc))
|
| 187 |
raise
|
multi_llm_chatbot_backend/app/tests/unit/test_secrets.py
ADDED
|
@@ -0,0 +1,39 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Unit tests for secret normalization and client-safe error messages."""
|
| 2 |
+
|
| 3 |
+
import unittest
|
| 4 |
+
|
| 5 |
+
from app.core.secrets import client_safe_error_message, normalize_secret
|
| 6 |
+
|
| 7 |
+
|
| 8 |
+
class NormalizeSecretTests(unittest.TestCase):
|
| 9 |
+
def test_strips_whitespace(self):
|
| 10 |
+
self.assertEqual(normalize_secret(" sk-abc \n"), "sk-abc")
|
| 11 |
+
|
| 12 |
+
def test_strips_wrapping_quotes(self):
|
| 13 |
+
self.assertEqual(normalize_secret('"sk-abc"'), "sk-abc")
|
| 14 |
+
self.assertEqual(normalize_secret("'sk-abc'"), "sk-abc")
|
| 15 |
+
|
| 16 |
+
def test_empty(self):
|
| 17 |
+
self.assertEqual(normalize_secret(None), "")
|
| 18 |
+
self.assertEqual(normalize_secret(""), "")
|
| 19 |
+
|
| 20 |
+
|
| 21 |
+
class ClientSafeErrorMessageTests(unittest.TestCase):
|
| 22 |
+
def test_redacts_api_key_material(self):
|
| 23 |
+
raw = (
|
| 24 |
+
"Error code: 401 - {'error': {'message': "
|
| 25 |
+
"'Incorrect API key provided: sk-proj-ABCDEFGHIJKLMNOP. "
|
| 26 |
+
"You can find your API key at https://platform.openai.com/account/api-keys.', "
|
| 27 |
+
"'type': 'invalid_request_error', 'code': 'invalid_api_key'}}"
|
| 28 |
+
)
|
| 29 |
+
safe = client_safe_error_message(Exception(raw))
|
| 30 |
+
self.assertNotIn("sk-proj-ABCDEF", safe)
|
| 31 |
+
self.assertIn("OPENAI_API_KEY", safe)
|
| 32 |
+
|
| 33 |
+
def test_passes_through_unrelated_errors(self):
|
| 34 |
+
msg = "Connection timed out talking to upstream"
|
| 35 |
+
self.assertEqual(client_safe_error_message(Exception(msg)), msg)
|
| 36 |
+
|
| 37 |
+
|
| 38 |
+
if __name__ == "__main__":
|
| 39 |
+
unittest.main()
|