fix(connection): per-id saves stop cross-client connection clobber

This commit is contained in:
zipg
2026-09-26 21:45:03 +08:00
committed by GitHub
parent a8006a1fdd
commit 33cac93254
9 changed files with 188 additions and 35 deletions
+6 -1
View File
@@ -570,7 +570,12 @@ export async function closeDatabaseConnection(connectionId: string, database: st
return post("/api/connection/close-database", { connectionId, database });
}
export async function saveConnections(configs: ConnectionConfig[]): Promise<void> {
export async function saveConnections(configs: ConnectionConfig[], removedIds: string[] = []): Promise<void> {
// Saving upserts; ids this client deleted are sent explicitly so that a
// stale local list can never drop connections another client created.
if (removedIds.length) {
return post("/api/connection/save", { configs, removedIds });
}
return post("/api/connection/save", { configs });
}
+2 -2
View File
@@ -2511,8 +2511,8 @@ export async function exportDocsHtml(filePath: string, snapshot: SchemaSnapshot,
return invoke("docs_export_html", { filePath, snapshot, annotations, lang });
}
export async function saveConnections(configs: ConnectionConfig[]): Promise<void> {
return invoke("save_connections", { configs });
export async function saveConnections(configs: ConnectionConfig[], removedIds: string[] = []): Promise<void> {
return invoke("save_connections", { configs, removedIds });
}
export async function loadConnections(): Promise<ConnectionConfig[]> {
@@ -157,7 +157,7 @@ describe("connectionStore pinned tree node removal", () => {
expect(store.connections).toEqual([connection]);
expect(settingsStore.editorSettings.connectTimeoutInheritConnectionIds).toEqual([connection.id]);
expect(saveConnections).toHaveBeenCalledWith([]);
expect(saveConnections).toHaveBeenCalledWith([], ["connection-1"]);
expect(saveConnections).toHaveBeenLastCalledWith([connection]);
});
+12 -3
View File
@@ -9181,8 +9181,13 @@ export const useConnectionStore = defineStore("connection", () => {
return null;
}
async function persistConnections(nextConnections: ConnectionConfig[] = connections.value) {
await api.saveConnections(nextConnections.filter((connection) => connection.one_time !== true));
async function persistConnections(nextConnections: ConnectionConfig[] = connections.value, removedIds: string[] = []) {
const configs = nextConnections.filter((connection) => connection.one_time !== true);
if (removedIds.length) {
await api.saveConnections(configs, removedIds);
return;
}
await api.saveConnections(configs);
}
function sameIds(left: string[], right: string[]) {
@@ -9231,6 +9236,10 @@ export const useConnectionStore = defineStore("connection", () => {
const nextConnectTimeoutIds = previousConnectTimeoutIds.filter((id) => nextConnections.some((connection) => connection.id === id));
const nextQueryTimeoutIds = previousQueryTimeoutIds.filter((id) => nextConnections.some((connection) => connection.id === id));
const connectionsChanged = nextConnections.length !== previousConnections.length || nextConnections.some((connection, index) => connection !== previousConnections[index]);
// Deletion is explicit: the backend upserts whatever the save carries, so the
// ids this client dropped from its list have to be named in the request.
const nextConnectionIds = new Set(nextConnections.map((connection) => connection.id));
const removedConnectionIds = previousConnections.filter((connection) => !nextConnectionIds.has(connection.id)).map((connection) => connection.id);
const timeoutSettingsChanged = !sameIds(nextConnectTimeoutIds, previousConnectTimeoutIds) || !sameIds(nextQueryTimeoutIds, previousQueryTimeoutIds);
const layoutChanged = nextLayout !== previousLayout;
let connectionsPersisted = false;
@@ -9239,7 +9248,7 @@ export const useConnectionStore = defineStore("connection", () => {
try {
if (connectionsChanged) {
await persistConnections(nextConnections);
await persistConnections(nextConnections, removedConnectionIds);
connectionsPersisted = true;
}
if (timeoutSettingsChanged) {
+41 -7
View File
@@ -5901,22 +5901,47 @@ impl Storage {
.await
}
/// Insert or update exactly the given connections and leave every other saved
/// connection untouched.
///
/// Multi-client deployments (the Web/Docker build serves several people from
/// one storage) must not replace the whole table: a client that saves a list
/// it loaded earlier would otherwise silently delete connections another
/// client created in the meantime. Removing a connection therefore has to go
/// through [`Storage::delete_connections`] with explicit ids.
pub async fn save_connections(&self, configs: &[ConnectionConfig]) -> Result<(), String> {
let configs = configs.to_vec();
let needs_key = configs.iter().any(connection_config_has_inline_secrets);
let codec = self.secret_codec_for_write(needs_key).await?;
self.with_conn(move |conn| {
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate).map_err(|e| e.to_string())?;
let replacement_ids = configs.iter().map(|config| config.id.clone()).collect::<HashSet<_>>();
let mut retained_ids = preserve_unreadable_connections_for_replacement(&tx, &replacement_ids)?;
for config in &configs {
// `persist_connection_in_tx` uses a plain INSERT, so an update of
// an already saved connection has to drop the old row first.
tx.execute("DELETE FROM connections WHERE id = ?1", [&config.id]).map_err(|e| e.to_string())?;
persist_connection_in_tx(&tx, &codec, config)?;
}
tx.commit().map_err(|e| e.to_string())
})
.await
}
retained_ids.extend(configs.iter().map(|config| config.id.clone()));
delete_unreferenced_connection_secrets_in_tx(&tx, &retained_ids)?;
/// Delete the given saved connections together with their stored secrets.
///
/// This is the only removal path for saved connections; ids that no longer
/// exist are ignored so a stale client cannot fail the save.
pub async fn delete_connections(&self, ids: &[String]) -> Result<(), String> {
if ids.is_empty() {
return Ok(());
}
let ids = ids.to_vec();
self.with_conn(move |conn| {
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate).map_err(|e| e.to_string())?;
for id in &ids {
tx.execute("DELETE FROM connections WHERE id = ?1", [id]).map_err(|e| e.to_string())?;
tx.execute("DELETE FROM connection_secrets WHERE connection_id = ?1", [id])
.map_err(|e| e.to_string())?;
}
tx.commit().map_err(|e| e.to_string())
})
.await
@@ -10529,7 +10554,14 @@ mod tests {
assert_eq!(raw_connection_json(&storage, "future").await, future_json);
assert_eq!(storage.get_secret("future", "password").await.unwrap().as_deref(), Some("future-secret"));
// Saving a list no longer replaces the whole table: an empty save is a no-op, so
// rows the caller never saw (like the unreadable "future" row) survive it.
storage.save_connections(&[]).await.unwrap();
assert_eq!(storage.load_connections().await.unwrap().len(), 1);
assert_eq!(raw_connection_json(&storage, "future").await, future_json);
// Deleting a connection is explicit now, and still only touches the given ids.
storage.delete_connections(&["known".to_string()]).await.unwrap();
assert!(storage.load_connections().await.unwrap().is_empty());
assert_eq!(raw_connection_json(&storage, "future").await, future_json);
assert_eq!(storage.get_secret("future", "password").await.unwrap().as_deref(), Some("future-secret"));
@@ -11043,7 +11075,9 @@ mod tests {
// Non-MCP callers remain governed by the ordinary DBX UI permissions.
storage.save_connections(std::slice::from_ref(&kept)).await.unwrap();
assert_eq!(storage.load_connections().await.unwrap()[0].id, kept.id);
let after_plain_save = storage.load_connections().await.unwrap();
assert_eq!(after_plain_save.len(), 3);
assert!(after_plain_save.iter().any(|config| config.id == kept.id));
let _ = std::fs::remove_file(path);
}
+91 -10
View File
@@ -171,6 +171,11 @@ pub struct WriteUnlockStateResponse {
#[serde(rename_all = "camelCase")]
pub struct SaveConnectionsRequest {
pub configs: Vec<ConnectionConfig>,
/// Ids the client deleted locally. Connections are saved by upsert, so a
/// client that no longer lists a connection must say so explicitly instead
/// of wiping every connection another client may have created meanwhile.
#[serde(default)]
pub removed_ids: Vec<String>,
}
#[derive(Deserialize)]
@@ -715,11 +720,18 @@ pub async fn save_connections(
.map_err(AppError::from)?;
}
}
if !body.removed_ids.is_empty() {
state.app.storage.delete_connections(&body.removed_ids).await.map_err(AppError::from)?;
}
state.app.storage.save_connections(&body.configs).await.map_err(AppError::from)?;
let owner = session_token_from_headers(&headers).unwrap_or_default();
let runtime_configs = body.configs.iter().cloned().map(prepare_runtime_config).collect::<Vec<_>>();
let sanitized_configs = runtime_configs.iter().map(|(config, _)| config.clone()).collect::<Vec<_>>();
let sync = sync_connection_configs(&state, &sanitized_configs).await;
// Saving is an upsert, so the request only describes the connections of this
// client. Sync the runtime cache against the whole persisted list instead of
// the request payload, otherwise a concurrent save from another client would
// drop its runtime config, pool and session credentials.
let persisted = state.app.storage.load_connections().await.map_err(AppError::from)?;
let sync = sync_connection_configs(&state, &persisted).await;
for (config, secrets) in &runtime_configs {
record_session_credentials(&state.app, &owner, &config.id, secrets, config.db_type == DatabaseType::Nacos);
}
@@ -1414,7 +1426,7 @@ mod tests {
let result = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![config.clone()] }),
Json(SaveConnectionsRequest { configs: vec![config.clone()], removed_ids: Vec::new() }),
)
.await;
assert!(result.is_ok());
@@ -1425,6 +1437,70 @@ mod tests {
let _ = std::fs::remove_dir_all(dir);
}
#[tokio::test]
async fn save_connections_keeps_connections_saved_by_another_client() {
let (state, dir) = test_web_state().await;
let client_one = sqlite_config("client-one", &dir.join("one.db").to_string_lossy());
// Client two loaded its list before client one saved anything.
let client_two_snapshot = state.app.storage.load_connections().await.unwrap();
let saved = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![client_one.clone()], removed_ids: Vec::new() }),
)
.await;
assert!(saved.is_ok());
// Client two saves its stale snapshot plus its own new connection.
let client_two = sqlite_config("client-two", &dir.join("two.db").to_string_lossy());
let mut payload = client_two_snapshot;
payload.push(client_two.clone());
let saved = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: payload, removed_ids: Vec::new() }),
)
.await;
assert!(saved.is_ok());
let persisted = state.app.storage.load_connections().await.unwrap();
assert!(
persisted.iter().any(|config| config.id == client_one.id),
"a connection saved by another client must survive a save that does not mention it"
);
assert!(persisted.iter().any(|config| config.id == client_two.id));
let runtime = state.app.configs.read().await;
assert!(runtime.contains_key(&client_one.id), "the other client's runtime config must be kept");
assert!(runtime.contains_key(&client_two.id));
drop(runtime);
let _ = std::fs::remove_dir_all(dir);
}
#[tokio::test]
async fn save_connections_deletes_only_explicitly_removed_connections() {
let (state, dir) = test_web_state().await;
let kept = sqlite_config("kept", &dir.join("kept.db").to_string_lossy());
let removed = sqlite_config("removed", &dir.join("removed.db").to_string_lossy());
state.app.storage.save_connections(&[kept.clone(), removed.clone()]).await.unwrap();
let saved = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![kept.clone()], removed_ids: vec![removed.id.clone()] }),
)
.await;
assert!(saved.is_ok());
let persisted = state.app.storage.load_connections().await.unwrap();
assert_eq!(persisted.iter().map(|config| config.id.as_str()).collect::<Vec<_>>(), vec![kept.id.as_str()]);
assert!(!state.app.configs.read().await.contains_key(&removed.id));
let _ = std::fs::remove_dir_all(dir);
}
#[tokio::test]
async fn mcp_connection_routes_preserve_unrelated_concurrent_changes() {
let (state, dir) = test_web_state().await;
@@ -1609,7 +1685,7 @@ mod tests {
let result = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![updated.clone()] }),
Json(SaveConnectionsRequest { configs: vec![updated.clone()], removed_ids: Vec::new() }),
)
.await;
assert!(result.is_ok());
@@ -1730,10 +1806,13 @@ mod tests {
let config = nacos_config("nacos-a");
let headers_a = cookie_headers("token-a");
let _ =
save_connections(State(state.clone()), headers_a, Json(SaveConnectionsRequest { configs: vec![config] }))
.await
.unwrap();
let _ = save_connections(
State(state.clone()),
headers_a,
Json(SaveConnectionsRequest { configs: vec![config], removed_ids: Vec::new() }),
)
.await
.unwrap();
let runtime = state.app.configs.read().await.get("nacos-a").cloned().unwrap();
assert!(runtime.password.is_empty());
@@ -2024,6 +2103,7 @@ mod tests {
let (state, dir) = test_web_state().await;
let kept = sqlite_config("kept", &dir.join("kept.db").to_string_lossy());
let removed = mq_config("removed-mq", "http://127.0.0.1:8080");
state.app.storage.save_connections(&[kept.clone(), removed.clone()]).await.unwrap();
{
let mut configs = state.app.configs.write().await;
configs.insert(kept.id.clone(), kept.clone());
@@ -2034,7 +2114,7 @@ mod tests {
let result = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![kept.clone()] }),
Json(SaveConnectionsRequest { configs: vec![kept.clone()], removed_ids: vec![removed.id.clone()] }),
)
.await;
assert!(result.is_ok());
@@ -2056,6 +2136,7 @@ mod tests {
let (state, dir) = test_web_state().await;
let kept = sqlite_config("kept", &dir.join("kept.db").to_string_lossy());
let removed = mq_config("removed-mq", "http://127.0.0.1:8080");
state.app.storage.save_connections(&[kept.clone(), removed.clone()]).await.unwrap();
{
let mut configs = state.app.configs.write().await;
configs.insert(kept.id.clone(), kept.clone());
@@ -2071,7 +2152,7 @@ mod tests {
let result = save_connections(
State(state.clone()),
HeaderMap::new(),
Json(SaveConnectionsRequest { configs: vec![kept.clone()] }),
Json(SaveConnectionsRequest { configs: vec![kept.clone()], removed_ids: vec![removed.id.clone()] }),
)
.await;
assert!(result.is_ok());
+5 -1
View File
@@ -115,6 +115,9 @@ Cookie: dbx_session=...
### 保存连接
连接按 `id` 增量写入:请求中未出现的连接保持不变,因此多个客户端共用同一个服务端时不会互相删除
对方的连接;删除连接通过可选的 `removedIds` 显式声明。
```http
POST /api/connection/save
Content-Type: application/json
@@ -130,7 +133,8 @@ Cookie: dbx_session=...
"username": "root",
"database": "app"
}
]
],
"removedIds": ["connection-to-delete"]
}
```
+6 -1
View File
@@ -115,6 +115,10 @@ Returns saved connection profiles. Secrets such as passwords are stored separate
### Save Connections
Connections are upserted by `id`: connections the request does not mention are left untouched, so several
clients can share one server without deleting each other's profiles. Deletions are explicit through the
optional `removedIds` array.
```http
POST /api/connection/save
Content-Type: application/json
@@ -130,7 +134,8 @@ Cookie: dbx_session=...
"username": "root",
"database": "app"
}
]
],
"removedIds": ["connection-to-delete"]
}
```
+24 -9
View File
@@ -435,7 +435,7 @@ mod tests {
name: "analytics".to_string(),
path: dir.join("analytics.sqlite").to_string_lossy().to_string(),
});
let error = save_connection_configs(&state, &[invalid]).await.unwrap_err();
let error = save_connection_configs(&state, &[invalid], Vec::new()).await.unwrap_err();
assert!(error.contains("in-memory main database"), "{error}");
assert!(state.pool_handle(&initial.id).await.is_some());
@@ -658,7 +658,7 @@ mod tests {
let first = state.mq_registry.get_or_build(&initial).await.unwrap().adapter;
let updated = mq_config("mq-conn", "http://127.0.0.1:8081");
save_connection_configs(&state, std::slice::from_ref(&updated)).await.unwrap();
save_connection_configs(&state, std::slice::from_ref(&updated), Vec::new()).await.unwrap();
let cached_admin_url = state
.configs
@@ -729,7 +729,7 @@ mod tests {
}
let stale = state.mq_registry.get_or_build(&removed).await.unwrap().adapter;
save_connection_configs(&state, std::slice::from_ref(&kept)).await.unwrap();
save_connection_configs(&state, std::slice::from_ref(&kept), Vec::new()).await.unwrap();
let configs = state.configs.read().await;
assert!(configs.contains_key(&kept.id));
@@ -797,7 +797,7 @@ mod tests {
state.configs.write().await.insert(preview.id.clone(), preview.clone());
state.session_credentials.set("", &preview.id, "secret").expect("session credential fixture");
save_connection_configs(&state, std::slice::from_ref(&persisted)).await.unwrap();
save_connection_configs(&state, std::slice::from_ref(&persisted), Vec::new()).await.unwrap();
// If the config is retained the credential must be retained with it, or the
// next query re-prompts for a password that was already entered.
@@ -826,7 +826,7 @@ mod tests {
})
.await;
save_connection_configs(&state, std::slice::from_ref(&kept)).await.unwrap();
save_connection_configs(&state, std::slice::from_ref(&kept), Vec::new()).await.unwrap();
assert!(state.pool_handle(&removed.id).await.is_none());
@@ -903,12 +903,20 @@ mod tests {
}
#[tauri::command]
pub async fn save_connections(state: State<'_, Arc<AppState>>, configs: Vec<ConnectionConfig>) -> Result<(), String> {
pub async fn save_connections(
state: State<'_, Arc<AppState>>,
configs: Vec<ConnectionConfig>,
removed_ids: Option<Vec<String>>,
) -> Result<(), String> {
let configs: Vec<ConnectionConfig> = configs.into_iter().map(|config| config.canonicalized()).collect();
save_connection_configs(state.inner(), &configs).await
save_connection_configs(state.inner(), &configs, removed_ids.unwrap_or_default()).await
}
async fn save_connection_configs(state: &AppState, configs: &[ConnectionConfig]) -> Result<(), String> {
async fn save_connection_configs(
state: &AppState,
configs: &[ConnectionConfig],
removed_ids: Vec<String>,
) -> Result<(), String> {
for config in configs {
if config.db_type == DatabaseType::Sqlite && !db::sqlite_worker::sqlite_ssh_worker_requested(config) {
db::sqlite::validate_persistent_attachments(
@@ -918,8 +926,15 @@ async fn save_connection_configs(state: &AppState, configs: &[ConnectionConfig])
)?;
}
}
if !removed_ids.is_empty() {
state.storage.delete_connections(&removed_ids).await?;
}
state.storage.save_connections(configs).await?;
let sync = sync_connection_configs(state, configs).await;
// Saving upserts, so the request only covers this window's connections. Sync
// against the whole persisted list to keep connections saved by another
// window/process alive in the runtime cache as well.
let persisted = state.storage.load_connections().await?;
let sync = sync_connection_configs(state, &persisted).await;
remove_connection_pools_for_connection_ids(state, &sync.connection_pool_ids_to_drop).await;
drop_nacos_adapters_for_connection_ids(state, &sync.nacos_adapter_ids_to_drop).await;
drop_mq_adapters_for_connection_ids(state, &sync.mq_adapter_ids_to_drop).await;