From 0a8934b46d8e88339d6e747175695c84a2ae90e0 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Fri, 4 Sep 2026 19:50:51 +0900 Subject: [PATCH] fix(store): keep an unreadable proxy row from bricking every request `8942287` renamed `ProxyMode::System` to `ProxyMode::Default`, changing the persisted wire value from `"system"` to `"default"`. No migration rewrites the existing `service_settings` row and `ProxyMode` has no alias, so any install that ever saved proxy settings on an earlier build now holds a row this build cannot deserialize: unknown variant `system`, expected `default` or `custom` `proxy_settings_secret` turned that into a hard error, and it is the first statement of every outbound client factory, so on upgrade the failure hits model calls, plugin installs, search, and the Cursor upstream proxy alike. It is also unrecoverable from the UI. `proxy_settings` reads the same row, so the settings page cannot render the proxy card, and `set_proxy_settings` reads the existing row before it writes, so the user cannot overwrite the row that broke them. Only editing SQLite by hand clears it. Read the row through a fallback that logs and returns the default instead. The affected rows are exactly the ones whose mode meant "no outbound proxy", which is what the default already is, so nothing is silently changed for them; a genuinely corrupt row costs the user a re-entered address instead of a dead install. Deliberately not `#[serde(alias = "system")]`: `8942287` added a test asserting that value no longer parses, and this keeps that true while making the persisted row survivable. Co-Authored-By: Claude Opus 5 --- server/src/store/settings.rs | 77 +++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/server/src/store/settings.rs b/server/src/store/settings.rs index 18d7ddcf6..5ff30e3de 100644 --- a/server/src/store/settings.rs +++ b/server/src/store/settings.rs @@ -160,6 +160,22 @@ pub(crate) struct ProxySettingsSecret { pub password: String, } +/// Reads the persisted outbound proxy row, falling back to "no proxy" when it +/// no longer parses. +/// +/// `mode` is a closed enum whose stored wire value has already changed once, so +/// a row written by an older build can be unreadable by this one. Propagating +/// that error would be unrecoverable rather than merely noisy: every outbound +/// client is built from this value, and `set_proxy_settings` reads the row +/// before it writes, so the settings page could neither load nor replace the +/// row that broke it. +fn read_proxy_settings(value: &str) -> ProxySettingsSecret { + serde_json::from_str(value).unwrap_or_else(|error| { + tracing::warn!(%error, "ignoring unreadable outbound proxy settings"); + ProxySettingsSecret::default() + }) +} + impl Store { pub(crate) async fn cursor_takeover_enabled(&self) -> Result { let value = sqlx::query_scalar::<_, String>( @@ -218,9 +234,9 @@ impl Store { .bind(PROXY_SETTINGS_KEY) .fetch_optional(&self.pool) .await?; - value - .map(|value| serde_json::from_str(&value).map_err(Into::into)) - .unwrap_or_else(|| Ok(ProxySettingsSecret::default())) + Ok(value + .as_deref() + .map_or_else(ProxySettingsSecret::default, read_proxy_settings)) } pub async fn proxy_settings(&self) -> Result { @@ -415,10 +431,16 @@ impl Store { #[cfg(test)] mod tests { use super::{ - CommitPromptLocale, CommitSettings, ProxyMode, DEFAULT_COMMIT_PROMPT_EN_US, - DEFAULT_COMMIT_PROMPT_ZH_CN, + read_proxy_settings, CommitPromptLocale, CommitSettings, ProxyMode, ProxySettingsInput, + ProxySettingsSecret, Store, DEFAULT_COMMIT_PROMPT_EN_US, DEFAULT_COMMIT_PROMPT_ZH_CN, + PROXY_SETTINGS_KEY, }; + /// The `outbound_proxy` row exactly as builds before the `system` -> `default` + /// rename wrote it. + const LEGACY_PROXY_ROW: &str = + r#"{"mode":"system","address":"","auth_enabled":false,"username":"","password":""}"#; + #[test] fn default_commit_prompt_follows_its_saved_locale() { for (prompt_locale, expected) in [ @@ -458,4 +480,49 @@ mod tests { ); assert!(serde_json::from_str::("\"system\"").is_err()); } + + #[test] + fn an_unreadable_proxy_row_reads_as_no_proxy() { + assert!(serde_json::from_str::(LEGACY_PROXY_ROW).is_err()); + + let settings = read_proxy_settings(LEGACY_PROXY_ROW); + assert_eq!(settings.mode, ProxyMode::Default); + assert!(settings.address.is_empty()); + assert!(!settings.auth_enabled); + } + + #[tokio::test] + async fn a_proxy_row_from_an_older_build_stays_replaceable() { + let directory = tempfile::tempdir().unwrap(); + let url = format!("sqlite://{}", directory.path().join("test.db").display()); + let store = Store::connect(&url).await.unwrap(); + sqlx::query( + "INSERT INTO service_settings(setting_key, value_json, updated_at_ms) VALUES (?, ?, 0)", + ) + .bind(PROXY_SETTINGS_KEY) + .bind(LEGACY_PROXY_ROW) + .execute(store.pool()) + .await + .unwrap(); + + // Reading must not fail: every outbound client is built from this value. + assert_eq!( + store.proxy_settings().await.unwrap().mode, + ProxyMode::Default + ); + + // And the settings page must be able to overwrite the row that broke it. + let saved = store + .set_proxy_settings(ProxySettingsInput { + mode: ProxyMode::Custom, + address: "http://127.0.0.1:7890".into(), + auth_enabled: false, + username: String::new(), + password: None, + }) + .await + .unwrap(); + assert_eq!(saved.mode, ProxyMode::Custom); + assert_eq!(saved.address, "http://127.0.0.1:7890"); + } }