Skip to content

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

Open
jeffwu-1999 wants to merge 3 commits into
developfrom
fix-model-credential-leak
Open

jeffwu-1999 wants to merge 3 commits into
developfrom
fix-model-credential-leak

Conversation

@jeffwu-1999

@jeffwu-1999 jeffwu-1999 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

变更说明 / What Changed

修改模型时 api-key 等凭证返回前端。模型列表接口此前已剥离api_key,但仍有两个泄露路径,本 PR 一并堵住:

  • config: /config/load_config (build_model_config) no longer echoes the
    stored api_key, nor the STT/TTS model_appid / access_token pair. The
    config page never displays or resubmits these fields from this payload,
    so nothing breaks by omitting them
  • model: /model/* list responses (_sanitize_model_credentials) now also
    strip model_appid / access_token — the full auth material for Volcano
    Engine STT/TTS models, previously returned in cleartext
  • update: empty-string model_appid / access_token are dropped from the
    update payload instead of overwriting stored values, mirroring the
    existing "empty api_key means keep" contract the edit dialog relies on
  • test: cover the extended sanitization (nested dict, HTTP responses) and
    the keep-existing update semantics; update assertions that previously
    expected raw keys

验证 / Verification

  • pytest test/backend/app/test_model_managment_app.py + test_model_rbac.py — 96 passed
  • pytest test/backend/services/test_config_sync_service.py — 58 passed
  • pytest test/backend/services/test_config_sync_service_voice.py — 4 passed
  • pytest test/backend/services/test_model_management_service.py — 106 passed
  • pytest test/backend/services/test_model_health_service.py — 58 passed

影响面 / Impact

  • 所有角色的 /config/load_config 与 /model/* 列表响应不再包含任何凭证
  • 编辑模型弹窗「留空 = 保留原值」语义对 apiKey / modelAppid / accessToken 三者一致成立
  • 语音模型连通性检查不受影响(health 探测在后端内部读取模型记录,不经 HTTP 响应)

- /config/load_config: build_model_config no longer echoes the stored
  api_key (and the STT/TTS model_appid / access_token pair); the config
  page never displays or resubmits these fields from this payload, so
  nothing breaks by omitting them
- /model/* list responses: _sanitize_model_credentials now also strips
  model_appid / access_token, which are the full auth material for
  Volcano Engine STT/TTS models
- update path: empty-string model_appid / access_token are dropped from
  the update payload instead of overwriting stored values, mirroring the
  existing "empty api_key means keep" contract the edit dialog relies on
- tests: cover the extended sanitization and the keep-existing update
  semantics; update assertions that previously expected raw keys
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The batch connection-edit path sent api_key unconditionally, so an
operator who left the key field untouched submitted an empty string.
List responses never carry stored keys, so sending "" both echoed a
credential-shaped field back to the server and overwrote the stored key
with an empty value.

Guard it the same way the single-model edit dialog does (apiKey.trim() ?
{ apiKey } : {}), keeping the "empty means keep existing credential"
contract consistent across both edit paths.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants