From 22c010079a36112d188fa321378f34988fbc01d5 Mon Sep 17 00:00:00 2001 From: SaladDay Date: Sat, 1 Aug 2026 09:09:14 +0000 Subject: [PATCH] fix(provider): enforce durable identity boundaries --- src-tauri/src/database/dao/provider_write.rs | 12 +- src-tauri/src/services/provider/mod.rs | 170 +++++++++++++++++-- 2 files changed, 169 insertions(+), 13 deletions(-) diff --git a/src-tauri/src/database/dao/provider_write.rs b/src-tauri/src/database/dao/provider_write.rs index 554ecd605..3ddab2ac2 100644 --- a/src-tauri/src/database/dao/provider_write.rs +++ b/src-tauri/src/database/dao/provider_write.rs @@ -133,19 +133,25 @@ impl NewProviderAggregate { let mut seen = HashSet::with_capacity(endpoints.len()); let mut initial_endpoints = Vec::with_capacity(endpoints.len()); for (key, endpoint) in endpoints { - if key != endpoint.url { + let normalized_key = key.trim().trim_end_matches('/').to_string(); + let normalized_url = endpoint.url.trim().trim_end_matches('/').to_string(); + if normalized_key != normalized_url { return Err(AppError::InvalidInput(format!( "provider endpoint key '{key}' must match endpoint URL '{}'", endpoint.url ))); } - if !seen.insert(endpoint.url.clone()) { + if !seen.insert(normalized_url.clone()) { return Err(AppError::InvalidInput(format!( "duplicate initial provider endpoint '{}'", endpoint.url ))); } - initial_endpoints.push(endpoint.try_into()?); + initial_endpoints.push(NewEndpoint::new( + normalized_url, + endpoint.added_at, + endpoint.last_used, + )?); } let key = ProviderKey::new(app_type, input.id.clone())?; let row = ProviderRowCreate { diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 19afadd36..5f2ba87e4 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -632,6 +632,90 @@ mod tests { }); } + #[test] + #[serial] + fn provider_service_create_canonicalizes_initial_endpoint_identity() { + with_test_home(|state, _| { + let raw_url = " https://canonical.example/// "; + let mut provider = opencode_provider("canonical-endpoint"); + provider.meta = Some(ProviderMeta { + custom_endpoints: HashMap::from([( + raw_url.to_string(), + endpoint(raw_url, None, Some(11)), + )]), + ..Default::default() + }); + + ProviderService::add( + state, + AppType::OpenCode, + provider_to_mutation_input(provider), + false, + ) + .expect("create with a non-canonical initial endpoint"); + let aggregate = state + .db + .get_provider_aggregate("opencode", "canonical-endpoint") + .expect("read canonical aggregate") + .expect("canonical aggregate"); + assert_eq!(aggregate.endpoints.len(), 1); + assert!(aggregate + .endpoints + .contains_key("https://canonical.example")); + + ProviderService::update_endpoint_last_used( + state, + AppType::OpenCode, + "canonical-endpoint", + " https://canonical.example/ ".to_string(), + ) + .expect("touch must resolve the same canonical endpoint"); + ProviderService::remove_custom_endpoint( + state, + AppType::OpenCode, + "canonical-endpoint", + "https://canonical.example///".to_string(), + ) + .expect("remove must resolve the same canonical endpoint"); + assert!(state + .db + .get_provider_aggregate("opencode", "canonical-endpoint") + .expect("read after remove") + .expect("provider after remove") + .endpoints + .is_empty()); + + let mut duplicate = opencode_provider("duplicate-canonical-endpoint"); + duplicate.meta = Some(ProviderMeta { + custom_endpoints: HashMap::from([ + ( + "https://duplicate.example".to_string(), + endpoint("https://duplicate.example", None, None), + ), + ( + " https://duplicate.example/ ".to_string(), + endpoint(" https://duplicate.example/ ", Some(1), None), + ), + ]), + ..Default::default() + }); + assert!(matches!( + ProviderService::add( + state, + AppType::OpenCode, + provider_to_mutation_input(duplicate), + false, + ), + Err(AppError::InvalidInput(_)) + )); + assert!(state + .db + .get_provider_aggregate("opencode", "duplicate-canonical-endpoint") + .expect("read duplicate candidate") + .is_none()); + }); + } + #[test] #[serial] fn provider_service_stale_edit_payload_cannot_overwrite_endpoint_operations() { @@ -868,6 +952,75 @@ mod tests { }); } + #[test] + #[serial] + fn provider_service_rename_fails_closed_for_malformed_additive_live_config() { + with_test_home(|state, home| { + let cases = [ + ( + AppType::OpenCode, + opencode_provider("malformed-source"), + opencode_provider("malformed-target"), + home.join(".config").join("opencode").join("opencode.json"), + r#"{"provider":{"malformed-source":{"npm":"@ai-sdk/openai-compatible"}"#, + ), + ( + AppType::OpenClaw, + openclaw_provider("corrupt-source"), + openclaw_provider("corrupt-target"), + home.join(".openclaw").join("openclaw.json"), + r#"{"models":{"providers":{"corrupt-target":{"baseUrl":"https://example.test"}"#, + ), + ]; + + for (app_type, source, target, live_path, malformed_live) in cases { + let app_name = app_type.as_str().to_string(); + let source_id = source.id.clone(); + let target_id = target.id.clone(); + ProviderService::add( + state, + app_type.clone(), + provider_to_mutation_input(source), + false, + ) + .expect("create DB-only rename source"); + + fs::create_dir_all(live_path.parent().expect("live config parent")) + .expect("create live config directory"); + fs::write(&live_path, malformed_live).expect("write malformed live config"); + let source_before = provider_snapshot(state, &app_name, &source_id); + let target_before = provider_snapshot(state, &app_name, &target_id); + + let error = ProviderService::update( + state, + app_type, + Some(&source_id), + provider_to_mutation_input(target), + ) + .expect_err("rename must fail closed when live identity cannot be inspected"); + assert!( + matches!(error, AppError::Config(_)), + "rename should surface the live parse error, got {error:?}" + ); + assert_eq!( + provider_snapshot(state, &app_name, &source_id), + source_before, + "source aggregate must remain unchanged" + ); + assert_eq!( + provider_snapshot(state, &app_name, &target_id), + target_before, + "target aggregate must remain unchanged" + ); + assert_eq!( + fs::read_to_string(&live_path).expect("reread malformed live config"), + malformed_live, + "failed rename must not rewrite the live config" + ); + } + }); + } + #[test] #[serial] fn automated_row_transform_conflicts_instead_of_reverting_a_newer_edit() { @@ -3363,11 +3516,12 @@ impl ProviderService { )); } - let original_in_live = Self::check_live_config_exists( - &app_type, - &original_id, - Self::provider_live_config_managed(&existing_provider), - )?; + // A rename changes durable identity, so "cannot inspect live + // config" must never be treated as "not live". DB-only + // same-ID edits deliberately retain their tolerant path below, + // but rename proves both identities absent from a readable live + // config before committing the SQLite transaction. + let original_in_live = provider_exists_in_live_config(&app_type, &original_id)?; if original_in_live { return Err(AppError::Message( "Provider key cannot be changed after the provider has been added to the app config" @@ -3375,11 +3529,7 @@ impl ProviderService { )); } - let next_id_in_live = Self::check_live_config_exists( - &app_type, - &provider.id, - Self::provider_live_config_managed(&existing_provider), - )?; + let next_id_in_live = provider_exists_in_live_config(&app_type, &provider.id)?; if state .db .get_provider_by_id(&provider.id, app_type.as_str())?