fix(core): preserve settings-sourced apiKey when registry model envKey is absent#3495
Merged
Conversation
…y is absent (#3417) On restart, `applyResolvedModelDefaults` unconditionally cleared the apiKey resolved from `settings.security.auth.apiKey` (layer 4 fallback) and only read from `process.env[model.envKey]`. When the provider-specific env var was absent (e.g. key stored only in settings), the correctly resolved key was discarded, causing a 401 error. Now capture the previously-resolved apiKey before clearing and fall back to it when `process.env[model.envKey]` is empty, but only for safe source kinds (`settings` and general `env` without `via.modelProviders`). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
requested changes
Apr 24, 2026
Address review feedback: keys passed via CLI flags (e.g. --openaiApiKey) were dropped on restart because source kind 'cli' was not in the fallback allowlist. Add 'cli' to the condition and a regression test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
Manual Verification ReportTest ScenarioConfigured {
"security": {
"auth": {
"selectedType": "openai",
"apiKey": "sk-82cd12a8***"
}
},
"model": { "name": "qwen3.5-plus" },
"modelProviders": {
"openai": [{
"id": "qwen3.5-plus",
"name": "[ModelStudio Standard] qwen3.5-plus",
"baseUrl": "https://dashscope.aliyuncs.com/compatible-mode/v1",
"envKey": "NONEXISTENT_TEST_KEY_3417"
}]
}
}Environment: Before Fix (fallback branch removed)The settings-sourced apiKey was unconditionally cleared by After Fix (with temporary debug log)
Unit TestsAll 51 tests pass, including 6 new test cases covering:
🤖 Generated with Qwen Code |
wenshao
previously approved these changes
Apr 24, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
Collaborator
Author
|
hi, @tanzhenxin @pomelo-nwu I will merge this bug fix to main branch. if any issues arise, feel free to contact me. |
Collaborator
|
感谢修复，bug 定位准确，启动场景的 6 个测试覆盖得也扎实。主体方向没问题，但有一个 fallback 分支在跨 provider 切换时会错带 settings-source / cli-source 的 key，建议先处理再合。 Blocker: same-authType 跨 provider 切换时，settings-source / cli-source 的 key 会被错误保留触发路径：
测试 case 4（ 根因启动 建议修法（二选一）A. no-op short-circuit（更干净） B. 调用方传上下文
必补的测试it('should NOT preserve settings-sourced apiKey when switching to a different provider within same authType', ...)场景： 小问题PR body Summary 段写 “Added 5 test cases”，下面 Tmux Report 段写 “6 new test cases”，diff 里实际 6 个，文字对齐一下。 |
…o syncAfterAuthRefresh The previous fallback logic inside applyResolvedModelDefaults could leak a settings/cli-sourced apiKey to a different provider when switching models within the same authType (e.g. dashscope → openai). This is a credential safety issue because the two providers may have different baseUrls. Move the save/restore logic to syncAfterAuthRefresh Step 1, guarded by an `isUnchanged` check (same authType AND same modelId). This ensures: - Restart scenario: apiKey preserved (same model, no change) - Cross-provider switch: apiKey cleared (different modelId) Also adds two cross-provider switch tests (settings-sourced and CLI-sourced) per review feedback. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
@wenshao 感谢详细的 review，跨 provider key 泄漏的问题分析得很准确。已在 9f470f5 中采纳修改： 采纳的方案综合了你建议的方案 A 和 B 的思路：将 apiKey 保留逻辑从 具体做法：
补充的测试
PR body 数字已知 summary 段 "5 test cases" 与实际 8 个不一致，下次更新 PR body 时一并修正。 |
…old-start test - Replace `savedApiKeySource!` with a truthiness guard for safer source restoration - Add test for cold-start scenario (previousAuthType undefined) to verify no key preservation occurs on first syncAfterAuthRefresh - Fix stale "short-circuit" comment in programmatic key test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
requested changes
Apr 24, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] applyResolvedModelDefaults() clears _generationConfig.apiKey on the non-Qwen-OAuth path, but if no replacement key is found/restored it can leave generationConfigSources['apiKey'] pointing at the previous source. That makes the effective config and source metadata disagree, which can mislead diagnostics or future source-aware logic. Please clear generationConfigSources['apiKey'] whenever the effective API key is cleared and no new source is assigned.
— gpt-5.5 via Qwen Code /review
When a model provider config is hot-reloaded (e.g. via Coding Plan update) changing envKey or baseUrl while keeping the same model id, the save/restore logic must not preserve the old apiKey. Extend the isUnchanged guard to compare apiKeyEnvKey and baseUrl against the resolved model, but only after applyResolvedModelDefaults has run at least once (apiKeyEnvKey !== undefined). On first startup call these fields are still unset, so the check is skipped to preserve the settings/cli-sourced key correctly. Adds two hot-reload tests (envKey change and baseUrl change). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
requested changes
Apr 25, 2026
…hange detection Replace `apiKeyEnvKey !== undefined` guard with `baseUrl source === 'modelProviders'` to reliably detect whether applyResolvedModelDefaults has been called before. This fixes two edge cases: 1. No-envKey models: hot-reload changing baseUrl was undetected because apiKeyEnvKey remained undefined. Now baseUrl source is checked. 2. Startup with envKey but omitted baseUrl: undefined !== default URL could falsely trigger isProviderChanged. Now skipped at startup since baseUrl source is not yet 'modelProviders'. Updates hot-reload test fixtures to simulate post-apply state (baseUrl source as 'modelProviders') and adds no-envKey hot-reload test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Copy the ConfigSource object before applyResolvedModelDefaults runs, so a future refactor that mutates source objects in place won't break the save/restore logic. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
approved these changes
Apr 25, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…y is absent (QwenLM#3495) * fix(core): preserve settings-sourced apiKey when registry model envKey is absent (QwenLM#3417) On restart, `applyResolvedModelDefaults` unconditionally cleared the apiKey resolved from `settings.security.auth.apiKey` (layer 4 fallback) and only read from `process.env[model.envKey]`. When the provider-specific env var was absent (e.g. key stored only in settings), the correctly resolved key was discarded, causing a 401 error. Now capture the previously-resolved apiKey before clearing and fall back to it when `process.env[model.envKey]` is empty, but only for safe source kinds (`settings` and general `env` without `via.modelProviders`). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): also preserve CLI-sourced apiKey during syncAfterAuthRefresh Address review feedback: keys passed via CLI flags (e.g. --openaiApiKey) were dropped on restart because source kind 'cli' was not in the fallback allowlist. Add 'cli' to the condition and a regression test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): move apiKey preservation from applyResolvedModelDefaults to syncAfterAuthRefresh The previous fallback logic inside applyResolvedModelDefaults could leak a settings/cli-sourced apiKey to a different provider when switching models within the same authType (e.g. dashscope → openai). This is a credential safety issue because the two providers may have different baseUrls. Move the save/restore logic to syncAfterAuthRefresh Step 1, guarded by an `isUnchanged` check (same authType AND same modelId). This ensures: - Restart scenario: apiKey preserved (same model, no change) - Cross-provider switch: apiKey cleared (different modelId) Also adds two cross-provider switch tests (settings-sourced and CLI-sourced) per review feedback. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): replace non-null assertion with truthiness guard and add cold-start test - Replace `savedApiKeySource!` with a truthiness guard for safer source restoration - Add test for cold-start scenario (previousAuthType undefined) to verify no key preservation occurs on first syncAfterAuthRefresh - Fix stale "short-circuit" comment in programmatic key test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): detect provider config hot-reload in isUnchanged check When a model provider config is hot-reloaded (e.g. via Coding Plan update) changing envKey or baseUrl while keeping the same model id, the save/restore logic must not preserve the old apiKey. Extend the isUnchanged guard to compare apiKeyEnvKey and baseUrl against the resolved model, but only after applyResolvedModelDefaults has run at least once (apiKeyEnvKey !== undefined). On first startup call these fields are still unset, so the check is skipped to preserve the settings/cli-sourced key correctly. Adds two hot-reload tests (envKey change and baseUrl change). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): use baseUrl source as hasBeenApplied signal for provider change detection Replace `apiKeyEnvKey !== undefined` guard with `baseUrl source === 'modelProviders'` to reliably detect whether applyResolvedModelDefaults has been called before. This fixes two edge cases: 1. No-envKey models: hot-reload changing baseUrl was undetected because apiKeyEnvKey remained undefined. Now baseUrl source is checked. 2. Startup with envKey but omitted baseUrl: undefined !== default URL could falsely trigger isProviderChanged. Now skipped at startup since baseUrl source is not yet 'modelProviders'. Updates hot-reload test fixtures to simulate post-apply state (baseUrl source as 'modelProviders') and adds no-envKey hot-reload test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): shallow-clone savedApiKeySource to avoid mutation risk Copy the ConfigSource object before applyResolvedModelDefaults runs, so a future refactor that mutates source objects in place won't break the save/restore logic. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: jinye.djy <jinye.djy@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3417
applyResolvedModelDefaultsunconditionally cleared the apiKey resolved fromsettings.security.auth.apiKey(layer 4 fallback) and only read fromprocess.env[model.envKey]. When the provider-specific env var was absent, the correctly resolved key was discarded, causing a 401 error.process.env[model.envKey]is empty, but only for safe source kinds (settingsand generalenvwithoutvia.modelProviders).Test plan
npx vitest run packages/core/src/models/modelsConfig.test.ts)settings.security.auth.apiKeywith a valid key,modelProviders.openaiwithenvKeypointing to an absent env var, verify no 401 on restart人工验证
{ "security": { "auth": { "selectedType": "openai", "apiKey": "你的真实百炼API Key" } }, "model": { "name": "qwen3.5-plus" }, "modelProviders": { "openai": [ { "id": "qwen3.5-plus", "name": "qwen3.5-plus", "baseUrl": "https://dashscope.aliyuncs.com/compatible-mode/v1", "envKey": "NONEXISTENT_TEST_KEY_3417" } ] }, "$version": 3, "general": { "language": "zh" } }修复前：
预期输出：
缺少 OpenAI 兼容认证的 API 密钥。请设置 'NONEXISTENT_TEST_KEY_3417' 环境变量。
修复后：
预期输出：模型正常返回（如 Hello），无 401 错误。
优先级验证
当 envKey 对应的环境变量存在时，应优先使用它而非 settings key：
预期：报 401（因为用了错误的 env key，而非 settings 中正确的 key），说明环境变量优先级更高。
Tmux Verification Report
Test Scenario
Configured
settings.jsonwithsecurity.auth.apiKeyset, butmodelProviders.envKeypointing to a non-existent environment variable:{ "security": { "auth": { "selectedType": "openai", "apiKey": "sk-82cd12a8***" } }, "model": { "name": "qwen3.5-plus" }, "modelProviders": { "openai": [{ "id": "qwen3.5-plus", "name": "[ModelStudio Standard] qwen3.5-plus", "baseUrl": "https://dashscope.aliyuncs.com/compatible-mode/v1", "envKey": "NONEXISTENT_TEST_KEY_3417" }] } }Environment:
NONEXISTENT_TEST_KEY_3417is NOT set,DASHSCOPE_API_KEYis NOT set.Before Fix (fallback branch removed)
The settings-sourced apiKey was unconditionally cleared by
applyResolvedModelDefaults, andprocess.env['NONEXISTENT_TEST_KEY_3417']was empty, so the key was lost.After Fix (with temporary debug log)
source=settingsconfirms the key was preserved fromsecurity.auth.apiKeyHello— no 401 errorUnit Tests
All 51 tests pass, including 6 new test cases covering:
--openaiApiKey) preservation🤖 Generated with Qwen Code