From 904242129248e0e6444868d1aeae9c141ae91835 Mon Sep 17 00:00:00 2001 From: lzt404 <2596933790@qq.com> Date: Thu, 10 Sep 2026 23:09:57 +0800 Subject: [PATCH] fix(config): prevent API provider loss during alias updates and migration - Validate provider integrity before saving alias changes - Reject rendered YAML that differs from the expected configuration - Abort migration when existing config cannot be read or parsed - Remove lossy config recovery fallback - Add regression tests for provider loss, locked files, malformed config, and repeated updates --- src-tauri/src/core_config/alias_save.rs | 42 ++++++++++ src-tauri/src/core_config/aliases.rs | 9 ++- src-tauri/src/core_config/settings.rs | 80 -------------------- src-tauri/src/core_runtime.rs | 25 +++--- src-tauri/src/tests/alias_save.rs | 36 +++++++++ src-tauri/src/tests/core_runtime.rs | 50 +++++++++--- src-tauri/src/tests/desktop_alias_routing.rs | 29 +++++++ 7 files changed, 166 insertions(+), 105 deletions(-) diff --git a/src-tauri/src/core_config/alias_save.rs b/src-tauri/src/core_config/alias_save.rs index d4d1c72..e2390e6 100644 --- a/src-tauri/src/core_config/alias_save.rs +++ b/src-tauri/src/core_config/alias_save.rs @@ -5,6 +5,47 @@ fn alias_config_is_unchanged(current: &str, latest: &str) -> Result Result<(), String> { + let parse = |content: &str| { + serde_norway::from_str::(content) + .map_err(|error| format!("解析内核 YAML 配置失败: {error}")) + }; + let current = parse(current)?; + let updated = parse(updated)?; + for section in MODEL_ALIAS_CONFIG_SECTIONS + .iter() + .copied() + .chain(["vertex-api-key", "xai-api-key", "interactions-api-key"]) + { + let without_models = |root: &serde_norway::Value| -> Result<_, String> { + let mut value = root + .get(section) + .cloned() + .unwrap_or(serde_norway::Value::Null); + if value.is_null() { + return Ok(serde_norway::Value::Sequence(Vec::new())); + } + let providers = value + .as_sequence_mut() + .ok_or_else(|| format!("{section} 必须是数组，已拒绝保存别名"))?; + if MODEL_ALIAS_CONFIG_SECTIONS.contains(§ion) { + for provider in providers { + if let Some(provider) = provider.as_mapping_mut() { + provider.remove(yaml_key("models")); + } + } + } + Ok(value) + }; + if without_models(¤t)? != without_models(&updated)? { + return Err(format!( + "别名更新意外改变了 API 接入配置（{section}），已拒绝写入" + )); + } + } + Ok(()) +} + pub(crate) async fn put_management_alias_config_changes( config: &GuiConfigFile, current: &str, @@ -25,6 +66,7 @@ pub(crate) async fn commit_management_alias_config_changes( ) -> Result { static SAVE_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); let _guard = SAVE_LOCK.lock().await; + validate_alias_api_access_preserved(current, updated)?; let changes = management_alias_config_changes(current, updated)?; let latest = fetch_management_config_yaml(config).await?; if !alias_config_is_unchanged(current, &latest)? { diff --git a/src-tauri/src/core_config/aliases.rs b/src-tauri/src/core_config/aliases.rs index e760c05..22936e3 100644 --- a/src-tauri/src/core_config/aliases.rs +++ b/src-tauri/src/core_config/aliases.rs @@ -2279,8 +2279,15 @@ pub(crate) fn render_updated_core_yaml( document.set(updated); let rendered = expand_top_level_flow_style_collections(&document.get_string(), document.get())?; let rendered = indent_indentationless_yaml_sequences(&rendered); - serde_norway::from_str::(&rendered) + let validated = serde_norway::from_str::(&rendered) .map_err(|error| format!("验证更新后的内核配置失败: {error}"))?; + if &validated != document.get() { + let path = first_yaml_mismatch_path(document.get(), &validated, &mut Vec::new()) + .unwrap_or_else(|| "".to_string()); + return Err(format!( + "更新后的内核配置与预期值不一致（路径: {path}），已拒绝写入" + )); + } Ok(rendered) } diff --git a/src-tauri/src/core_config/settings.rs b/src-tauri/src/core_config/settings.rs index f603860..df5ad45 100644 --- a/src-tauri/src/core_config/settings.rs +++ b/src-tauri/src/core_config/settings.rs @@ -276,17 +276,6 @@ pub(crate) fn merge_core_config_fields( merge_core_config_value(template, current_value) } -pub(crate) fn merge_core_config_fields_tolerant( - template: &str, - current: &str, -) -> Result { - let current = match serde_norway::from_str::(current) { - Ok(current) if current.is_mapping() => current, - _ => recover_parseable_top_level_yaml_fields(current), - }; - merge_core_config_value(template, Some(current)) -} - pub(crate) fn merge_core_config_value( template: &str, current: Option, @@ -311,75 +300,6 @@ pub(crate) fn merge_core_config_value( Ok(rendered) } -pub(crate) fn recover_parseable_top_level_yaml_fields(content: &str) -> serde_norway::Value { - let lines = yaml_line_ranges(content); - let boundaries = lines - .iter() - .copied() - .filter(|range| is_top_level_yaml_boundary(yaml_line_content(content, *range))) - .collect::>(); - let mut recovered = serde_norway::Mapping::new(); - - for (index, (start, _)) in boundaries.iter().copied().enumerate() { - let line = yaml_line_content(content, boundaries[index]); - if !is_top_level_yaml_mapping_field(line) { - continue; - } - let end = boundaries - .get(index + 1) - .map(|(next_start, _)| *next_start) - .unwrap_or(content.len()); - let Ok(value) = serde_norway::from_str::(&content[start..end]) else { - continue; - }; - let Some(mapping) = value.as_mapping() else { - continue; - }; - for (key, value) in mapping { - recovered.insert(key.clone(), value.clone()); - } - } - - serde_norway::Value::Mapping(recovered) -} - -pub(crate) fn is_top_level_yaml_boundary(line: &str) -> bool { - if line.is_empty() - || line.chars().next().is_some_and(char::is_whitespace) - || line.starts_with('#') - || is_indentationless_yaml_sequence_item(line) - { - return false; - } - !matches!(line.trim(), "---" | "...") && !line.starts_with('%') -} - -pub(crate) fn is_top_level_yaml_mapping_field(line: &str) -> bool { - let mut single_quoted = false; - let mut double_quoted = false; - let mut escaped = false; - for (index, character) in line.char_indices() { - if escaped { - escaped = false; - continue; - } - if double_quoted && character == '\\' { - escaped = true; - continue; - } - match character { - '\'' if !double_quoted => single_quoted = !single_quoted, - '"' if !single_quoted => double_quoted = !double_quoted, - ':' if !single_quoted && !double_quoted => { - let rest = &line[index + character.len_utf8()..]; - return rest.is_empty() || rest.chars().next().is_some_and(char::is_whitespace); - } - _ => {} - } - } - false -} - pub(crate) fn patch_core_network_yaml( content: &str, config: &GuiConfigFile, diff --git a/src-tauri/src/core_runtime.rs b/src-tauri/src/core_runtime.rs index 91feb31..5c21e60 100644 --- a/src-tauri/src/core_runtime.rs +++ b/src-tauri/src/core_runtime.rs @@ -1834,9 +1834,16 @@ pub(crate) fn migrate_core_config_for_update( return Ok(()); } let old_config_path = source_dir.join(CORE_CONFIG_FILE); - if !old_config_path.is_file() { - return Ok(()); - } + let old_config = match fs::read_to_string(&old_config_path) { + Ok(content) => content, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => { + return Err(format!( + "读取旧版内核配置失败，为避免配置丢失已取消更新 {}: {error}", + path_to_string(&old_config_path) + )); + } + }; let template_path = target_dir.join(CORE_EXAMPLE_CONFIG_FILE); if !template_path.is_file() { @@ -1845,23 +1852,13 @@ pub(crate) fn migrate_core_config_for_update( path_to_string(&template_path) )); } - let old_config = match fs::read(&old_config_path) { - Ok(content) => String::from_utf8_lossy(&content).into_owned(), - Err(error) => { - eprintln!( - "读取旧版内核配置失败，将使用新版默认配置继续更新 {}: {error}", - path_to_string(&old_config_path) - ); - String::new() - } - }; let template = fs::read_to_string(&template_path).map_err(|error| { format!( "读取新版内核配置模板失败 {}: {error}", path_to_string(&template_path) ) })?; - let migrated = merge_core_config_fields_tolerant(&template, &old_config)?; + let migrated = merge_core_config_fields(&template, Some(&old_config))?; let config_path = target_dir.join(CORE_CONFIG_FILE); fs::write(&config_path, migrated).map_err(|error| { format!( diff --git a/src-tauri/src/tests/alias_save.rs b/src-tauri/src/tests/alias_save.rs index 4a4e4ee..2bcf1f2 100644 --- a/src-tauri/src/tests/alias_save.rs +++ b/src-tauri/src/tests/alias_save.rs @@ -16,6 +16,42 @@ enum Failure { ForeignChange, } +#[tokio::test(flavor = "current_thread")] +async fn alias_save_rejects_loss_of_api_access_before_any_write() { + let initial = format!("{CURRENT}codex-api-key:\n - api-key: test-key\n base-url: https://example.test\n models: [{{name: model-a}}]\nopenai-compatibility:\n - name: preserved\n disabled: true\n api-key-entries: [{{api-key: other-test-key}}]\n models: [{{name: model-b}}]\n"); + for section in ["codex-api-key", "openai-compatibility"] { + for mode in ["missing", "empty", "credential"] { + let mut corrupted = yaml_json(&initial); + match mode { + "missing" => { + corrupted.as_object_mut().unwrap().remove(section); + } + "empty" => corrupted[section] = serde_json::json!([]), + _ => { + corrupted[section][0] + .as_object_mut() + .unwrap() + .retain(|key, _| !key.starts_with("api-key")); + } + } + let core = MockCore::new(&initial, Failure::None); + let result = put_management_alias_config_changes( + &core.config, + &initial, + &serde_norway::to_string(&corrupted).unwrap(), + ) + .await; + let (persisted, requests) = core.finish(); + assert!( + result.is_err(), + "accepted provider loss: {section}/{mode}" + ); + assert_eq!(persisted, yaml_json(&initial)); + assert!(!requests.iter().any(|request| request.starts_with("PUT"))); + } + } +} + struct MockCore { config: GuiConfigFile, finished: Arc, diff --git a/src-tauri/src/tests/core_runtime.rs b/src-tauri/src/tests/core_runtime.rs index 6946b7d..28a2b9d 100644 --- a/src-tauri/src/tests/core_runtime.rs +++ b/src-tauri/src/tests/core_runtime.rs @@ -556,7 +556,7 @@ fn replacing_a_core_migrates_old_fields_into_the_new_template() { } #[test] -fn replacing_a_core_discards_invalid_fields_and_continues_migration() { +fn replacing_a_core_rejects_invalid_config_without_overwriting_files() { let root = agent_test_home("core-config-migrate-invalid"); let source = root.join("source"); let target = root.join("target"); @@ -574,16 +574,46 @@ fn replacing_a_core_discards_invalid_fields_and_continues_migration() { .unwrap(); fs::write(target.join(CORE_CONFIG_FILE), "staged: untouched\n").unwrap(); - migrate_core_config_for_update(&source, &target).unwrap(); + let original = fs::read(source.join(CORE_CONFIG_FILE)).unwrap(); + assert!(migrate_core_config_for_update(&source, &target).is_err()); + assert_eq!(fs::read(source.join(CORE_CONFIG_FILE)).unwrap(), original); + assert_eq!( + fs::read_to_string(target.join(CORE_CONFIG_FILE)).unwrap(), + "staged: untouched\n" + ); + fs::remove_dir_all(root).unwrap(); +} - let migrated = fs::read_to_string(target.join(CORE_CONFIG_FILE)).unwrap(); - let document = serde_norway::from_str::(&migrated).unwrap(); - assert_eq!(document["host"], "127.0.0.1"); - assert_eq!(document["broken"], "new-default"); - assert_eq!(document["port"], 9527); - assert_eq!(document["api-keys"][0], "old-a"); - assert_eq!(document["api-keys"][1], "old-b"); - assert_eq!(document["new-option"], true); +#[cfg(windows)] +#[test] +fn replacing_a_core_rejects_locked_config_without_using_defaults() { + use std::os::windows::fs::OpenOptionsExt; + let root = agent_test_home("core-config-locked"); + let source = root.join("source"); + let target = root.join("target"); + fs::create_dir_all(&source).unwrap(); + fs::create_dir_all(&target).unwrap(); + let original = "codex-api-key:\n - api-key: test-key\n base-url: https://example.test\n"; + let source_path = source.join(CORE_CONFIG_FILE); + fs::write(&source_path, original).unwrap(); + fs::write(target.join(CORE_EXAMPLE_CONFIG_FILE), "port: 8317\n").unwrap(); + fs::write(target.join(CORE_CONFIG_FILE), "staged: untouched\n").unwrap(); + let locked = fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(&source_path) + .unwrap(); + let result = migrate_core_config_for_update(&source, &target); + drop(locked); + assert!( + result.is_err(), + "unreadable configuration must not be replaced by defaults" + ); + assert_eq!(fs::read_to_string(source_path).unwrap(), original); + assert_eq!( + fs::read_to_string(target.join(CORE_CONFIG_FILE)).unwrap(), + "staged: untouched\n" + ); fs::remove_dir_all(root).unwrap(); } diff --git a/src-tauri/src/tests/desktop_alias_routing.rs b/src-tauri/src/tests/desktop_alias_routing.rs index ebd9318..c77c3f7 100644 --- a/src-tauri/src/tests/desktop_alias_routing.rs +++ b/src-tauri/src/tests/desktop_alias_routing.rs @@ -5,6 +5,35 @@ fn json(content: &str) -> serde_json::Value { serde_json::to_value(serde_norway::from_str::(content).unwrap()).unwrap() } +#[test] +fn desktop_routes_preserve_api_access_when_switching_models() { + let input = "codex-api-key:\n - api-key: codex-test-key\n base-url: https://codex.example.test\n proxy-url: socks5://127.0.0.1:1080\n models: [{name: model-a}, {name: model-b}]\nopenai-compatibility:\n - name: other\n disabled: true\n api-key-entries: [{api-key: other-test-key}]\n models: [{name: other-model}]\npayload:\n override:\n - models: [{name: other-model}]\n params: {custom: retained}\n"; + let before = json(input); + let mut content = input.to_string(); + for selected in ["model-a", "model-b", "model-b"] { + content = ensure_claude_desktop_model_aliases_in_yaml( + &content, + &ClaudeDesktopModelMappings::all(selected), + &test_agent_models(&["model-a", "model-b"]), + ) + .unwrap(); + let mut after = json(&content); + let models = after["codex-api-key"][0]["models"].as_array_mut().unwrap(); + assert_eq!(models.len(), 5); + for route in [ + CLAUDE_DESKTOP_OPUS_MODEL_ID, + CLAUDE_DESKTOP_SONNET_MODEL_ID, + CLAUDE_DESKTOP_HAIKU_MODEL_ID, + ] { + assert!(models + .iter() + .any(|model| model["alias"] == route && model["name"] == selected)); + } + models.retain(|model| model.get("display-name").is_none()); + assert_eq!(after, before); + } +} + #[test] fn desktop_routes_move_from_disabled_provider_to_enabled_source() { let models = test_agent_models(&["grok-4.6"]);