fix: stop returning model api keys and voice credentials to the frontend

backend/apps/model_managment_app.py

@@ -123,14 +123,21 @@ def dump_full_catalog():
 _LOG_UNSAFE_CHARS = re.compile(r"[\x00-\x1f\x7f]")
 
 
+# Credential-bearing columns that must never reach an HTTP client. STT/TTS
+# voice models authenticate with the (model_appid, access_token) pair instead
+# of (or in addition to) api_key, so all three are stripped together.
+_MODEL_CREDENTIAL_FIELDS = frozenset({"api_key", "model_appid", "access_token"})
+
+
 def _sanitize_model_credentials(payload: Any) -> Any:
-    """Remove model API keys before returning model data to HTTP clients.
+    """Remove model credentials before returning model data to HTTP clients.
 
     Model records are also consumed by internal services, so credential
     removal belongs at the HTTP response boundary rather than in the database
     or model-management service layer. The presence of a configured key is
     intentionally not returned; callers that need to update a model can omit
-    ``api_key`` to keep the existing value.
+    ``api_key`` / ``model_appid`` / ``access_token`` to keep the existing
+    value (the update service drops empty-string values for these fields).
     """
     if isinstance(payload, list):
         return [_sanitize_model_credentials(item) for item in payload]
@@ -139,7 +146,7 @@ def _sanitize_model_credentials(payload: Any) -> Any:
         return {
             key: _sanitize_model_credentials(value)
             for key, value in payload.items()
-            if key != "api_key"
+            if key not in _MODEL_CREDENTIAL_FIELDS
         }
 
     return payload

backend/services/config_sync_service.py

@@ -188,6 +188,15 @@ def build_models_config(tenant_id: str) -> dict:
 
 
 def build_model_config(model_config: dict) -> dict:
+    """Build the /config/load_config model entry for one default-model slot.
+
+    Credential values (api_key, and the STT/TTS model_appid / access_token
+    pair) are intentionally NOT included: load_config responses go to every
+    logged-in user, so leaking them would expose tenant-level provider
+    credentials. The frontend model page never displays or resubmits these
+    fields from this payload (model management submits credential updates
+    only through the /model/* endpoints), so nothing breaks by omitting them.
+    """
     if not model_config:
         return {
             "id": None,
@@ -204,19 +213,21 @@ def build_model_config(model_config: dict) -> dict:
         "name": get_model_name_from_config(model_config) if model_config else "",
         "displayName": model_config.get("display_name", ""),
         "apiConfig": {
-            "apiKey": model_config.get("api_key", ""),
+            "apiKey": "",
             "modelUrl": model_config.get("base_url", "")
         }
     }
 
     if "embedding" in model_config.get("model_type", ""):
         config["dimension"] = model_config.get("max_tokens", 0)
 
-    # Add voice model specific fields (STT and TTS)
+    # Add voice model specific fields (STT and TTS). model_appid and
+    # access_token are credentials (the Volcano auth pair) and are omitted;
+    # the operator edits them from the model-management dialog instead.
     model_type = model_config.get("model_type", "")
     if model_type == "stt" or model_type == "tts":
         config["modelFactory"] = model_config.get("model_factory", "")
-        config["modelAppid"] = model_config.get("model_appid", "")
-        config["accessToken"] = model_config.get("access_token", "")
+        config["modelAppid"] = ""
+        config["accessToken"] = ""
 
     return config

backend/services/model_management_service.py

@@ -924,6 +924,16 @@ async def update_single_model_for_tenant(
             else:
                 model_data["ssl_verify"] = True
 
+        # Keep-existing semantics for voice-model credentials. List endpoints
+        # sanitize model_appid / access_token before returning records, so an
+        # edit dialog that the operator did not touch submits empty strings
+        # for these fields. Drop them instead of overwriting the stored
+        # values -- mirroring the api_key "empty means keep" contract that
+        # the frontend already relies on.
+        for credential_field in ("model_appid", "access_token"):
+            if credential_field in model_data and not model_data[credential_field]:
+                model_data.pop(credential_field)
+
         # Carry model_type from the existing record so the legacy-alias
         # coercion can distinguish LLM/VLM updates from embedding updates
         # even when the caller payload omits model_type. We don't store the

frontend/app/[locale]/models/components/modelConfig.tsx

@@ -321,11 +321,14 @@ export const ModelConfigSection = forwardRef<
       for (const m of group.models) {
         try {
           // Partial update: only api_key + base_url are sent; the backend
-          // leaves every other field untouched.
+          // leaves every other field untouched. An empty key means "keep the
+          // existing credential" (list responses never carry stored keys), so
+          // it must be omitted rather than sent as an empty string -- sending
+          // "" would overwrite the stored key.
           await modelService.updateSingleModel({
             currentDisplayName: m.displayName,
             url: patch.url,
-            apiKey: patch.apiKey,
+            ...(patch.apiKey.trim() ? { apiKey: patch.apiKey.trim() } : {}),
             source: m.source,
           });
         } catch (e: any) {

test/backend/app/test_model_managment_app.py

@@ -675,6 +675,63 @@ async def mock_list_models(*args, **kwargs):
     mock_list.assert_called_once_with(user_credentials[1])
 
 
+@pytest.mark.asyncio
+async def test_get_model_list_strips_voice_model_credentials(client, auth_header, user_credentials, mocker):
+    """Voice-model credentials (model_appid / access_token) must never reach
+    an HTTP client, exactly like api_key: the pair is the full auth material
+    for Volcano Engine STT/TTS models."""
+    mocker.patch('backend.apps.model_managment_app.get_current_user_id', return_value=user_credentials)
+
+    async def mock_list_models(*args, **kwargs):
+        return [
+            {
+                "model_id": "stt1",
+                "model_name": "volc/stt-bigmodel",
+                "display_name": "Volc STT",
+                "model_type": "stt",
+                "api_key": "stored-secret",
+                "model_appid": "stored-appid",
+                "access_token": "stored-token",
+                "connect_status": "operational",
+            }
+        ]
+
+    mocker.patch('backend.apps.model_managment_app.list_models_for_tenant', side_effect=mock_list_models)
+
+    response = client.get("/model/list", headers=auth_header)
+
+    assert response.status_code == HTTPStatus.OK
+    record = response.json()["data"][0]
+    assert record["model_name"] == "volc/stt-bigmodel"
+    for credential_field in ("api_key", "model_appid", "access_token"):
+        assert credential_field not in record
+
+
+def test_sanitize_model_credentials_strips_all_credential_fields():
+    """Unit-level contract: api_key, model_appid and access_token are removed
+    from dicts at any nesting depth; all other fields pass through."""
+    from backend.apps.model_managment_app import _sanitize_model_credentials
+
+    payload = {
+        "model_id": 1,
+        "api_key": "sk-secret",
+        "model_appid": "appid-secret",
+        "access_token": "token-secret",
+        "nested": {
+            "api_key": "sk-nested",
+            "model_appid": "appid-nested",
+            "keep": "me",
+        },
+    }
+
+    sanitized = _sanitize_model_credentials(payload)
+
+    assert sanitized == {
+        "model_id": 1,
+        "nested": {"keep": "me"},
+    }
+
+
 # Tests for /model/llm_list endpoint
 @pytest.mark.asyncio
 async def test_get_llm_model_list_success(client, auth_header, user_credentials, mocker):

test/backend/services/test_config_sync_service.py

@@ -1246,7 +1246,7 @@ def side_effect(config_key, tenant_id=None):
 
         # Verify successful config
         assert result["llm"]["displayName"] == "Test LLM"
-        assert result["llm"]["apiConfig"]["apiKey"] == "test-api-key"
+        assert result["llm"]["apiConfig"]["apiKey"] == ""  # sanitized
 
         # Verify failed config was handled gracefully
         assert result["embedding"]["name"] == ""
@@ -1300,7 +1300,7 @@ def side_effect(config_key, tenant_id=None):
 
         # Verify successful configs
         assert result["llm"]["displayName"] == "GPT-4"
-        assert result["llm"]["apiConfig"]["apiKey"] == "test-key"
+        assert result["llm"]["apiConfig"]["apiKey"] == ""  # sanitized
 
         # Verify no warnings were logged (all successful)
         service_mocks['logger'].warning.assert_not_called()
@@ -1642,7 +1642,7 @@ def test_build_model_config_non_embedding_model(self, service_mocks):
         # Assert
         assert result["name"] == "gpt-4"
         assert result["displayName"] == "GPT-4"
-        assert result["apiConfig"]["apiKey"] == "test-api-key"
+        assert result["apiConfig"]["apiKey"] == ""  # sanitized
         assert result["apiConfig"]["modelUrl"] == "https://api.openai.com"
         # Should not have dimension field for non-embedding models
         assert "dimension" not in result
@@ -1667,7 +1667,7 @@ def test_build_model_config_embedding_model(self, service_mocks):
         # Assert
         assert result["name"] == "text-embedding-ada-002"
         assert result["displayName"] == "Ada Embeddings"
-        assert result["apiConfig"]["apiKey"] == "test-api-key"
+        assert result["apiConfig"]["apiKey"] == ""  # sanitized
         assert result["apiConfig"]["modelUrl"] == "https://api.openai.com"
         # Should have dimension field for embedding models
         assert result["dimension"] == 1536
@@ -1692,7 +1692,7 @@ def test_build_model_config_multi_embedding_model(self, service_mocks):
         # Assert
         assert result["name"] == "text-embedding-3-small"
         assert result["displayName"] == "Multi Ada Embeddings"
-        assert result["apiConfig"]["apiKey"] == "test-api-key"
+        assert result["apiConfig"]["apiKey"] == ""  # sanitized
         assert result["apiConfig"]["modelUrl"] == "https://api.openai.com"
         # Should have dimension field for multi_embedding models
         assert result["dimension"] == 768
@@ -1739,7 +1739,7 @@ def test_build_model_config_embedding_without_max_tokens(self, service_mocks):
         # Assert
         assert result["name"] == "test-embedding"
         assert result["displayName"] == "Test Embedding"
-        assert result["apiConfig"]["apiKey"] == "test-key"
+        assert result["apiConfig"]["apiKey"] == ""  # sanitized
         assert result["apiConfig"]["modelUrl"] == "https://test.com"
         # Should have dimension field with default value 0
         assert result["dimension"] == 0
@@ -1763,7 +1763,7 @@ def test_build_model_config_model_type_partial_match(self, service_mocks):
         # Assert
         assert result["name"] == "test-model"
         assert result["displayName"] == "Test Model"
-        assert result["apiConfig"]["apiKey"] == "test-key"
+        assert result["apiConfig"]["apiKey"] == ""  # sanitized
         # Should have dimension since model_type contains 'embedding'
         assert result["dimension"] == 512
 

test/backend/services/test_config_sync_service_voice.py

@@ -197,8 +197,11 @@ def test_build_model_config_stt(self, service_mocks):
         result = build_model_config(model_config)
 
         assert result["modelFactory"] == "volc"
-        assert result["modelAppid"] == "stt_appid"
-        assert result["accessToken"] == "stt_token"
+        # Voice credentials are sanitized before reaching HTTP clients:
+        # model_appid / access_token must never be echoed back.
+        assert result["modelAppid"] == ""
+        assert result["accessToken"] == ""
+        assert result["apiConfig"]["apiKey"] == ""
 
     def test_build_model_config_stt_empty_fields(self, service_mocks):
         """Test build_model_config with STT model and empty voice fields."""

test/backend/services/test_model_management_service.py

@@ -1237,6 +1237,57 @@ async def test_update_single_model_for_tenant_success_single_model():
         )
 
 
+async def test_update_single_model_for_tenant_empty_voice_credentials_keep_existing():
+    """Empty-string model_appid / access_token must be dropped from the update
+    payload instead of overwriting the stored values.
+
+    List endpoints sanitize these fields, so an edit dialog the operator did
+    not touch submits empty strings; the "empty means keep" contract mirrors
+    api_key handling."""
+    svc = import_svc()
+
+    existing_models = [
+        {"model_id": 9, "model_type": "stt", "display_name": "volc-stt"},
+    ]
+    model_data = {
+        "model_id": 9,
+        "display_name": "volc-stt",
+        "model_appid": "",
+        "access_token": "",
+    }
+
+    with mock.patch.object(svc, "get_models_by_display_name", return_value=existing_models), \
+            mock.patch.object(svc, "update_model_record") as mock_update:
+        await svc.update_single_model_for_tenant("u1", "t1", "volc-stt", model_data)
+
+        call_payload = mock_update.call_args[0][1]
+        assert "model_appid" not in call_payload
+        assert "access_token" not in call_payload
+
+
+async def test_update_single_model_for_tenant_new_voice_credentials_are_written():
+    """Non-empty model_appid / access_token pass through to the update call."""
+    svc = import_svc()
+
+    existing_models = [
+        {"model_id": 9, "model_type": "stt", "display_name": "volc-stt"},
+    ]
+    model_data = {
+        "model_id": 9,
+        "display_name": "volc-stt",
+        "model_appid": "new-appid",
+        "access_token": "new-token",
+    }
+
+    with mock.patch.object(svc, "get_models_by_display_name", return_value=existing_models), \
+            mock.patch.object(svc, "update_model_record") as mock_update:
+        await svc.update_single_model_for_tenant("u1", "t1", "volc-stt", model_data)
+
+        call_payload = mock_update.call_args[0][1]
+        assert call_payload["model_appid"] == "new-appid"
+        assert call_payload["access_token"] == "new-token"
+
+
 async def test_update_single_model_for_tenant_splits_repo_prefix_from_model_name():
     """A repo-qualified model_name (as returned by the list endpoints) must be
     split on update, otherwise the model_name column accumulates a repo prefix