diff --git a/packages/plugin-api/src/index.ts b/packages/plugin-api/src/index.ts index 4727f6142..3e982490b 100644 --- a/packages/plugin-api/src/index.ts +++ b/packages/plugin-api/src/index.ts @@ -28,6 +28,7 @@ export type { SlotContext, SlotComponentProps, SlotContextMap, + PluginSecretFieldState, TypedSlotProps, UIExtensionDeclaration, } from "./slots"; diff --git a/packages/plugin-api/src/slots.ts b/packages/plugin-api/src/slots.ts index 3e079a546..18b8a261f 100644 --- a/packages/plugin-api/src/slots.ts +++ b/packages/plugin-api/src/slots.ts @@ -1,5 +1,18 @@ import type { ComponentType } from "react"; +/** + * State of one plugin-owned secret in the connection modal + * (`connection-modal.extra_fields` slot). + */ +export interface PluginSecretFieldState { + /** Value typed during this modal session. Stored values are never exposed. */ + value: string; + /** Whether the connection had a value in the OS keychain when editing began. */ + hasStoredValue: boolean; + /** Whether this modal will set (non-empty value) or clear (empty value) it on save. */ + dirty: boolean; +} + /** * Typed context shape per slot. * @@ -91,6 +104,19 @@ export type SlotContextMap = { * connection. The host resets the flag whenever the driver changes. */ setCredentialFieldsHidden: (hidden: boolean) => void; + /** + * Plugin-owned secrets of this connection, keyed by field name. A value + * already stored in the OS keychain is never exposed: it is reported only + * through `hasStoredValue`. Optional so bundles can detect older hosts. + */ + secretFields?: Record; + /** + * Set a plugin-owned secret for Test Connection and Save. The value is + * stored in the OS keychain, never in the connections file, and reaches + * the driver as `params.extra[key]` at runtime. An empty value explicitly + * clears the stored secret on save. + */ + setSecretField?: (key: string, value: string) => void; }; }; diff --git a/plugins/PLUGIN_GUIDE.md b/plugins/PLUGIN_GUIDE.md index 9caefb755..935323854 100644 --- a/plugins/PLUGIN_GUIDE.md +++ b/plugins/PLUGIN_GUIDE.md @@ -121,6 +121,7 @@ One manifest tells Tabularis everything about your plugin — and, when you publ | `capabilities` | object | Feature flags (see below). | | `data_types` | array | List of supported data types (see below). | | `type_mappings` | object \| null | Optional map of generic inferred type names to driver-specific types. Used during paste/import to map generic types (e.g. `DATETIME`) to driver-native equivalents (e.g. `TIMESTAMP`). See [Type Mappings](#type-mappings) below. | +| `connection_fields` | object | Optional hide, label, and placeholder overrides for the common `host`, `port`, `username`, `password`, and `database` fields. Omitted fields preserve the standard UI. See [Connection Fields](#connection-fields) below. | ### Migrating display names @@ -219,6 +220,38 @@ Since `map_inferred_type()` is a synchronous trait method that cannot issue an R --- +### Connection Fields + +The optional `connection_fields` object adapts the host-owned inputs of the connection modal to a driver that does not use them as-is. Each of `host`, `port`, `username`, `password` and `database` accepts: + +| Field | Type | Description | +|-------|------|-------------| +| `hidden` | boolean | Hide the input for this driver. Defaults to `false`. | +| `label` | string | Driver-specific label (max 80 characters). | +| `placeholder` | string | Driver-specific placeholder (max 200 characters). | + +**Example for a cloud warehouse addressed by project id:** + +```json +{ + "connection_fields": { + "host": { "hidden": true }, + "port": { "hidden": true }, + "username": { "hidden": true }, + "password": { "hidden": true }, + "database": { "label": "Project ID", "placeholder": "billing-project" } + } +} +``` + +**Rules:** +- Overrides are presentation only: a hidden input is not rendered, but the value already saved for it is left untouched and still sent to the driver. +- A hidden `database` input is not required on Test Connection or Save. +- Hiding `host` also hides the "Load databases" action next to the database input. +- Use the `connection-modal.extra_fields` slot for the inputs the driver needs instead, and `setSecretField` for credentials (see [SlotContext](#slotcontext)). + +--- + ## 3. Plugin Settings Plugins can declare custom configuration fields that Tabularis renders in the **Settings → gear icon** modal for that plugin. Users fill in the values, Tabularis persists them in `config.json`, and passes them to the plugin at startup via an `initialize` RPC call. @@ -387,7 +420,7 @@ Add an optional `ui_extensions` array to your manifest: | `settings.plugin.actions` | Per-plugin actions in Settings modal | `targetPluginId` | Diagnostics, re-auth buttons | | `settings.plugin.before_settings` | Content above plugin settings form | `targetPluginId` | OAuth panels, status banners | | `connection-modal.connection_content` | Inside the connection form | `driver` | Custom connection fields | -| `connection-modal.extra_fields` | Below host/port in the connection form | `driver`, `extra`, `setExtraField`, `credentialFieldsHidden`, `setCredentialFieldsHidden` | Plugin-specific connection fields (e.g. AWS region). `setCredentialFieldsHidden(true)` hides and clears the host username/password inputs for drivers that authenticate without a login; while hidden the host also ignores the login of an imported connection string and drops the stored password on save | +| `connection-modal.extra_fields` | Below host/port in the connection form | `driver`, `extra`, `setExtraField`, `credentialFieldsHidden`, `setCredentialFieldsHidden`, `secretFields`, `setSecretField` | Plugin-specific connection fields (e.g. AWS region), including values stored per connection in the OS keychain. `setCredentialFieldsHidden(true)` hides and clears the host username/password inputs for drivers that authenticate without a login; while hidden the host also ignores the login of an imported connection string and drops the stored password on save | ### SlotContext @@ -407,6 +440,40 @@ interface SlotContext { } ``` +For `connection-modal.extra_fields`, `setSecretField(key, value)` keeps the +value out of `connections.json` and stores it under the saved connection in the +OS keychain. Passing an empty value explicitly clears the entry. On edit, +`secretFields[key]` exposes only `{ value, hasStoredValue, dirty }`: an existing +secret is represented by `hasStoredValue` and is never returned to plugin UI. +Keys are 1-64 ASCII letters, digits, `.`, `_` or `-`. + +At runtime the host resolves stored values into `params.extra[key]` before +calling the driver (every RPC method, Test Connection, Load Databases and the +MCP server), so the JSON-RPC method shapes remain backward compatible. Export +with credentials and backups carry the values; duplicating a connection copies +them; deleting it removes them; switching the connection to another driver +drops them. + +A stored secret can be unavailable at runtime: the connections file was synced +to a machine whose keychain does not have the entry, the keychain was reset, or +it is locked or denies access. The host never fails the call for that. It logs +a warning naming the key (never the value) and calls the driver with +`params.extra[key]` unset, so the plugin must check for the value and return a +clear error asking the user to set the credential again. Duplicating such a +connection copies only the values that can be read, and an import skips secret +keys with an invalid name instead of rejecting the whole file. Imported values +are written to the keychain only after the connections file has been saved. + +Older Tabularis versions do not know `plugin_secret_keys` and drop it when they +rewrite `connections.json`. After a downgrade (or with an older install sharing +the same file) the keychain entries stay but are no longer referenced, so the +plugin sees the secret as unset. Setting it again from the connection modal +restores it. + +Both properties are optional in the public TypeScript contract. A plugin that +also supports older hosts must guard their presence (for example by falling +back to the host `password` input when `setSecretField` is undefined). + ### Building UI Extension Bundles Plugin UI components must be pre-built as **IIFE bundles** (Immediately Invoked Function Expression). The host provides `React`, `ReactJSXRuntime`, and the plugin API as globals — your bundle must **not** bundle its own copies of these. diff --git a/plugins/manifest.schema.json b/plugins/manifest.schema.json index 9050a97a9..6886b6c74 100644 --- a/plugins/manifest.schema.json +++ b/plugins/manifest.schema.json @@ -223,6 +223,34 @@ } } }, + "connection_fields": { + "type": "object", + "description": "Optional presentation overrides for host-owned connection fields. Omitted fields keep the standard Tabularis behavior.", + "additionalProperties": false, + "patternProperties": { + "^(host|port|username|password|database)$": { + "type": "object", + "additionalProperties": false, + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this field for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + } + } + }, "interpreter": { "type": "string", "description": "Optional interpreter for script-based plugins (e.g. python3)." diff --git a/plugins/tabularium-extensions.schema.json b/plugins/tabularium-extensions.schema.json index a44d638ab..140895453 100644 --- a/plugins/tabularium-extensions.schema.json +++ b/plugins/tabularium-extensions.schema.json @@ -273,6 +273,123 @@ } } } + }, + "connection_fields": { + "type": "object", + "additionalProperties": false, + "description": "Optional presentation overrides for the host-owned connection inputs. Omitted fields keep the standard Tabularis behaviour.", + "properties": { + "host": { + "type": "object", + "additionalProperties": false, + "description": "Overrides for the `host` input.", + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this input for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + }, + "port": { + "type": "object", + "additionalProperties": false, + "description": "Overrides for the `port` input.", + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this input for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + }, + "username": { + "type": "object", + "additionalProperties": false, + "description": "Overrides for the `username` input.", + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this input for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + }, + "password": { + "type": "object", + "additionalProperties": false, + "description": "Overrides for the `password` input.", + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this input for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + }, + "database": { + "type": "object", + "additionalProperties": false, + "description": "Overrides for the `database` input.", + "properties": { + "hidden": { + "type": "boolean", + "default": false, + "description": "Hide this input for the driver." + }, + "label": { + "type": "string", + "maxLength": 80, + "description": "Driver-specific label." + }, + "placeholder": { + "type": "string", + "maxLength": 200, + "description": "Driver-specific input placeholder." + } + } + } + } } } } diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 5feb4ec48..3d29ea23b 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -910,7 +910,11 @@ fn restore_runtime_connection_uri( } } -pub fn find_connection_by_id( +/// Load a saved connection with its host-owned credentials (keychain password, +/// SSH secrets, connection URI) but without plugin-owned secrets. Used by the +/// edit modal and by the Test Connection / Load Databases flows, which resolve +/// plugin secrets themselves once the pending changes are known. +pub fn find_connection_metadata_by_id( app: &AppHandle, id: &str, ) -> Result { @@ -967,6 +971,18 @@ pub fn find_connection_by_id( Ok(conn) } +/// Load a connection for driver execution. Unlike the metadata/edit path, +/// this resolves plugin-owned secrets into `params.extra` only in memory. +pub fn find_connection_by_id( + app: &AppHandle, + id: &str, +) -> Result { + let mut conn = find_connection_metadata_by_id(app, id)?; + let cache = app.state::>(); + crate::plugin_secrets::hydrate_connection(&cache, &mut conn); + Ok(conn) +} + /// Merge a list of incoming groups into an existing list, preserving hierarchy /// and repairing any `parent_id` that points to a group id not present in the /// union (i.e. neither in the existing list nor in the incoming batch). @@ -1031,7 +1047,7 @@ pub async fn get_connection_by_id( app: AppHandle, id: String, ) -> Result { - find_connection_by_id(&app, &id) + find_connection_metadata_by_id(&app, &id) } #[tauri::command] @@ -1400,6 +1416,7 @@ pub async fn save_connection( app: AppHandle, name: String, params: ConnectionParams, + plugin_secret_changes: Option, detect_json_in_text_columns: Option, environment: Option, ) -> Result { @@ -1411,9 +1428,15 @@ pub async fn save_connection( let id = Uuid::new_v4().to_string(); let cache = app.state::>(); + let plugin_secret_changes = plugin_secret_changes.unwrap_or_default(); + let plugin_secret_keys = + crate::plugin_secrets::next_secret_keys(&[], true, &plugin_secret_changes)?; let connection_uri = runtime_connection_uri(¶ms).map(str::to_owned); let mut params_to_save = params_for_persistence(¶ms, connection_uri.is_some()); let mut keychain_writes = KeychainSecretWrites::default(); + for key in plugin_secret_changes.keys() { + params_to_save.extra.remove(key); + } if params.save_in_keychain.unwrap_or(false) { log::debug!("Storing passwords in keychain for connection: {}", name); @@ -1487,6 +1510,7 @@ pub async fn save_connection( id: id.clone(), name: name.clone(), params: params_to_save, + plugin_secret_keys, group_id: None, sort_order: None, detect_json_in_text_columns, @@ -1495,12 +1519,22 @@ pub async fn save_connection( environment: validate_environment(environment)?, }; conn_file.connections.push(new_conn.clone()); - if let Err(err) = persist_connection_uri_change( + if let Err(err) = crate::plugin_secrets::persist_secret_changes( &cache, &id, - false, - connection_uri.as_deref().map(Some), - || save_connections_and_invalidate(&app, &path, &conn_file), + None, + &[], + ¶ms.driver, + &plugin_secret_changes, + || { + persist_connection_uri_change( + &cache, + &id, + false, + connection_uri.as_deref().map(Some), + || save_connections_and_invalidate(&app, &path, &conn_file), + ) + }, ) { keychain_writes.roll_back(&cache, &id); return Err(err); @@ -1540,6 +1574,11 @@ pub async fn delete_connection(app: AppHandle, id: String) -> Res .find(|c| c.id == id) .and_then(|c| c.params.connection_uri_in_keychain) .unwrap_or(false); + let plugin_secret_metadata = conn_file + .connections + .iter() + .find(|c| c.id == id) + .map(|c| (c.params.driver.clone(), c.plugin_secret_keys.clone())); // Capture the appearance before retain so we can cascade-delete the icon file. let appearance_to_delete = conn_file @@ -1559,6 +1598,9 @@ pub async fn delete_connection(app: AppHandle, id: String) -> Res persist_connection_uri_change(&cache, &id, uri_stored_in_keychain, Some(None), || { save_connections_and_invalidate(&app, &path, &conn_file) })?; + if let Some((driver, keys)) = plugin_secret_metadata { + crate::plugin_secrets::delete_connection_secrets(&cache, &id, &driver, &keys); + } // Invalidate the in-memory cache for this connection credential_cache::invalidate_all_for_connection(&cache, &id); @@ -1595,6 +1637,7 @@ pub async fn update_connection( id: String, name: String, params: ConnectionParams, + plugin_secret_changes: Option, detect_json_in_text_columns: Option, environment: Option, ) -> Result { @@ -1616,6 +1659,13 @@ pub async fn update_connection( // must drop it rather than hand one driver's credentials to another. let previous_driver_id = conn_file.connections[conn_idx].params.driver.clone(); let same_driver = previous_driver_id == params.driver; + let original_plugin_secret_keys = conn_file.connections[conn_idx].plugin_secret_keys.clone(); + let plugin_secret_changes = plugin_secret_changes.unwrap_or_default(); + let plugin_secret_keys = crate::plugin_secrets::next_secret_keys( + &original_plugin_secret_keys, + same_driver, + &plugin_secret_changes, + )?; let connection_uri = runtime_connection_uri(¶ms).map(str::to_owned); // The frontend sends the URI back only when the user retyped it. An edit // that leaves the field untouched must keep the stored secret; an edit that @@ -1632,6 +1682,9 @@ pub async fn update_connection( }; let mut params_to_save = params_for_persistence(¶ms, connection_uri.is_some() || preserve_stored_uri); + for key in plugin_secret_changes.keys() { + params_to_save.extra.remove(key); + } let cache = app.state::>(); let mut keychain_writes = KeychainSecretWrites::default(); @@ -1738,6 +1791,7 @@ pub async fn update_connection( id: id.clone(), name, params: params_to_save, + plugin_secret_keys, group_id: original_group_id, sort_order: original_sort_order, detect_json_in_text_columns, @@ -1748,11 +1802,19 @@ pub async fn update_connection( conn_file.connections[conn_idx] = updated.clone(); - if let Err(err) = - persist_connection_uri_change(&cache, &id, existing_uri_in_keychain, uri_change, || { - save_connections_and_invalidate(&app, &path, &conn_file) - }) - { + if let Err(err) = crate::plugin_secrets::persist_secret_changes( + &cache, + &id, + Some(&previous_driver_id), + &original_plugin_secret_keys, + ¶ms.driver, + &plugin_secret_changes, + || { + persist_connection_uri_change(&cache, &id, existing_uri_in_keychain, uri_change, || { + save_connections_and_invalidate(&app, &path, &conn_file) + }) + }, + ) { keychain_writes.roll_back(&cache, &id); return Err(err); } @@ -1854,6 +1916,22 @@ pub async fn duplicate_connection( let mut original = conn_file.connections[original_idx].clone(); let cache = app.state::>(); + // Same tolerant read as a driver call: a secret missing from the keychain + // is not copied, and the duplicate does not list it. + let plugin_secret_changes = crate::plugin_secrets::duplicate_secret_changes( + &original.id, + &original.plugin_secret_keys, + |key| { + credential_cache::get_plugin_secret_cached( + &cache, + &original.id, + &original.params.driver, + key, + ) + }, + ); + let duplicate_plugin_secret_keys = + crate::plugin_secrets::next_secret_keys(&[], true, &plugin_secret_changes)?; // Same IAM-auth guard as `find_connection_by_id`: never copy a stale RDS // auth token into a duplicated connection. @@ -1956,9 +2034,10 @@ pub async fn duplicate_connection( }; let new_conn = SavedConnection { - id: new_id, + id: new_id.clone(), name: format!("{} (Copy)", original.name), params: new_params, + plugin_secret_keys: duplicate_plugin_secret_keys, group_id: original.group_id.clone(), // Copy to same group as original sort_order: None, // Will be placed at end of group detect_json_in_text_columns: original.detect_json_in_text_columns, @@ -1971,7 +2050,15 @@ pub async fn duplicate_connection( conn_file.connections.push(new_conn.clone()); - save_connections_and_invalidate(&app, &path, &conn_file)?; + crate::plugin_secrets::persist_secret_changes( + &cache, + &new_id, + None, + &[], + &original.params.driver, + &plugin_secret_changes, + || save_connections_and_invalidate(&app, &path, &conn_file), + )?; let mut returned_conn = new_conn; // Return with passwords for frontend consistency @@ -2873,6 +2960,12 @@ pub async fn test_connection( request.params.database ); let progress_id = request.progress_id.as_deref(); + // The saved connection, without plugin secrets: they are resolved below, + // after the pending changes from the modal are known. + let saved_conn = match &request.connection_id { + Some(id) => find_connection_metadata_by_id(&app, id).ok(), + None => None, + }; let mut expanded_params = expand_ssh_connection_params(&app, &request.params) .await @@ -2898,16 +2991,25 @@ pub async fn test_connection( .map_err(|e| emit_test_failure(&app, progress_id, "resolve", e))?; if !iam_auth && request.params.password.is_none() && expanded_params.password.is_none() { - let saved_conn = match &request.connection_id { - Some(id) => find_connection_by_id(&app, id).ok(), - None => None, - }; expanded_params.password = resolve_test_connection_password(&request.params, saved_conn.as_ref(), |conn_id| { keychain_utils::get_db_password(conn_id, "") }); } + // Plugin-owned secrets: stored values of the saved connection, overridden + // by whatever the modal is about to set or clear. + { + let cache = app.state::>(); + crate::plugin_secrets::apply_request_secrets( + &cache, + saved_conn.as_ref(), + &mut expanded_params, + &request.plugin_secret_changes, + ) + .map_err(|e| emit_test_failure(&app, progress_id, "resolve", e))?; + } + // Reconnecting to a saved connection sends the on-disk params, which never // carry the URI — restore it the same way the password is restored above. // An inline URI (the ephemeral Test Connection flow) always wins. @@ -3286,6 +3388,7 @@ mod tests { save_in_keychain: Some(save_in_keychain), ..base_params() }, + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, @@ -3314,6 +3417,7 @@ mod tests { id: "conn-1".to_string(), name: "Old Name".to_string(), params: base_params(), + plugin_secret_keys: Vec::new(), group_id: Some("group-a".to_string()), sort_order: Some(3), detect_json_in_text_columns: None, @@ -3334,6 +3438,7 @@ mod tests { id: existing.id.clone(), name: "New Name".to_string(), params: base_params(), + plugin_secret_keys: Vec::new(), group_id: existing.group_id.clone(), sort_order: existing.sort_order, detect_json_in_text_columns: None, @@ -3359,6 +3464,7 @@ mod tests { id: id.to_string(), name: "Test".to_string(), params: base_params(), + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, @@ -4392,11 +4498,11 @@ pub async fn list_databases( expanded_params.password.as_deref(), )?; + let saved_conn = match &request.connection_id { + Some(id) => find_connection_metadata_by_id(&app, id).ok(), + None => None, + }; if !iam_auth && request.params.password.is_none() && expanded_params.password.is_none() { - let saved_conn = match &request.connection_id { - Some(id) => find_connection_by_id(&app, id).ok(), - None => None, - }; expanded_params.password = resolve_test_connection_password(&request.params, saved_conn.as_ref(), |conn_id| { keychain_utils::get_db_password(conn_id, "") @@ -4413,6 +4519,17 @@ pub async fn list_databases( } } + // Plugin-owned secrets, resolved the same way as in `test_connection`. + { + let cache = app.state::>(); + crate::plugin_secrets::apply_request_secrets( + &cache, + saved_conn.as_ref(), + &mut expanded_params, + &request.plugin_secret_changes, + )?; + } + let resolved_params = if let Some(conn_id) = &request.connection_id { resolve_connection_params_with_id(&expanded_params, conn_id)? } else { @@ -6388,7 +6505,15 @@ pub async fn disconnect_connection( // Unregister from health check before closing the pool. crate::health_check::unregister_connection(&connection_id).await; - let saved_conn = find_connection_by_id(&app, &connection_id)?; + // A plugin secret that can no longer be read must not prevent closing the + // pools: fall back to the connection without plugin secrets. + let saved_conn = match find_connection_by_id(&app, &connection_id) { + Ok(conn) => conn, + Err(e) => { + log::warn!("Disconnecting {connection_id} without plugin secrets: {e}"); + find_connection_metadata_by_id(&app, &connection_id)? + } + }; if let Some(driver) = crate::drivers::registry::get_driver(&saved_conn.params.driver).await { driver .invalidate_connection_metadata(Some(&connection_id)) @@ -6915,6 +7040,23 @@ pub async fn delete_connection_group( // deleted along with it. Connections belonging to any group in the // subtree are removed as well. let to_delete = crate::models::collect_group_subtree(&file.groups, &id); + let deleted_plugin_secrets: Vec<(String, String, Vec)> = file + .connections + .iter() + .filter(|connection| { + connection + .group_id + .as_ref() + .is_some_and(|group_id| to_delete.contains(group_id)) + }) + .map(|connection| { + ( + connection.id.clone(), + connection.params.driver.clone(), + connection.plugin_secret_keys.clone(), + ) + }) + .collect(); file.groups.retain(|g| !to_delete.contains(&g.id)); file.connections.retain(|c| { @@ -6924,6 +7066,10 @@ pub async fn delete_connection_group( }); save_connections_and_invalidate(&app, &path, &file)?; + let cache = app.state::>(); + for (connection_id, driver, keys) in deleted_plugin_secrets { + crate::plugin_secrets::delete_connection_secrets(&cache, &connection_id, &driver, &keys); + } Ok(()) } @@ -7072,6 +7218,7 @@ pub async fn export_connections_payload( .state::>() .inner() .clone(); + let mut plugin_secrets = HashMap::new(); // Resolve passwords for database connections for conn in &mut conn_file.connections { @@ -7081,8 +7228,20 @@ pub async fn export_connections_payload( conn.params.ssh_password = None; conn.params.ssh_key_passphrase = None; conn.params.connection_uri = None; + conn.plugin_secret_keys.clear(); continue; } + // A missing keychain entry must not abort the whole export (or a + // scheduled backup): the connection is exported without that value. + let resolved_plugin_secrets = crate::plugin_secrets::load_secret_values( + &cache, + &conn.id, + &conn.params.driver, + &conn.plugin_secret_keys, + ); + if !resolved_plugin_secrets.is_empty() { + plugin_secrets.insert(conn.id.clone(), resolved_plugin_secrets); + } if conn.params.save_in_keychain.unwrap_or(false) { // Without this the export carries the marker but not the URI, and // restoring elsewhere yields a connection that cannot resolve it. @@ -7132,6 +7291,7 @@ pub async fn export_connections_payload( ssh_connections, k8s_connections, tags: conn_file.tags, + plugin_secrets, }) } @@ -7166,7 +7326,7 @@ pub async fn import_connections_payload( /// import command above and the foreign-app import flow. pub async fn apply_export_payload( app: AppHandle, - payload: ExportPayload, + mut payload: ExportPayload, ) -> Result<(), String> { let conn_path = get_config_path(&app)?; let ssh_path = get_ssh_config_path(&app)?; @@ -7183,6 +7343,8 @@ pub async fn apply_export_payload( .state::>() .inner() .clone(); + let mut imported_plugin_secrets = std::mem::take(&mut payload.plugin_secrets); + let mut plugin_secret_writes = Vec::new(); // Merge groups (preserves hierarchy; demotes orphaned parent_ids to root) merge_groups(&mut current_file.groups, payload.groups); @@ -7222,6 +7384,30 @@ pub async fn apply_export_payload( // An imported payload is untrusted input and may carry an inline URI. // Hold it to the same rule as a save: keychain or nothing. validate_connection_uri_persistence(&new_conn.params)?; + let declared_plugin_secret_keys = std::mem::take(&mut new_conn.plugin_secret_keys); + let secrets_for_connection = imported_plugin_secrets + .remove(&new_conn.id) + .unwrap_or_default(); + let previous_plugin_metadata = current_file + .connections + .iter() + .find(|connection| connection.id == new_conn.id) + .map(|connection| { + ( + connection.params.driver.clone(), + connection.plugin_secret_keys.clone(), + ) + }); + // Keychain writes are deferred until the connections file is saved, + // so a failed import never leaves orphaned plugin secrets behind. + plugin_secret_writes.extend(crate::plugin_secrets::plan_imported_secrets( + &mut new_conn, + &declared_plugin_secret_keys, + secrets_for_connection, + previous_plugin_metadata + .as_ref() + .map(|(driver, keys)| (driver.as_str(), keys.as_slice())), + )); // Handle passwords in keychain if new_conn.params.save_in_keychain.unwrap_or(false) { @@ -7293,8 +7479,14 @@ pub async fn apply_export_payload( } } - // Save files - save_connections_and_invalidate(&app, &conn_path, ¤t_file)?; + // Save files. Plugin secrets reach the keychain only once the connections + // file that lists them has landed. + crate::plugin_secrets::commit_secret_writes( + &crate::plugin_secrets::KeychainSecretStore, + &cache, + plugin_secret_writes, + || save_connections_and_invalidate(&app, &conn_path, ¤t_file), + )?; let ssh_json = serde_json::to_string_pretty(¤t_ssh).map_err(|e| e.to_string())?; persistence::write_private_file(&ssh_path, ssh_json)?; diff --git a/src-tauri/src/connection_cache_tests.rs b/src-tauri/src/connection_cache_tests.rs index 77bf18a8e..198f577d9 100644 --- a/src-tauri/src/connection_cache_tests.rs +++ b/src-tauri/src/connection_cache_tests.rs @@ -8,6 +8,7 @@ mod tests { id: id.to_string(), name: name.to_string(), params: ConnectionParams::default(), + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, diff --git a/src-tauri/src/connection_import/analyzer.rs b/src-tauri/src/connection_import/analyzer.rs index b3c777940..2899290b6 100644 --- a/src-tauri/src/connection_import/analyzer.rs +++ b/src-tauri/src/connection_import/analyzer.rs @@ -216,6 +216,7 @@ mod tests { database: DatabaseSelection::Single(db.to_string()), ..Default::default() }, + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, diff --git a/src-tauri/src/connection_import/convert.rs b/src-tauri/src/connection_import/convert.rs index ade1e30e1..adad43d28 100644 --- a/src-tauri/src/connection_import/convert.rs +++ b/src-tauri/src/connection_import/convert.rs @@ -4,6 +4,8 @@ //! `SshConnection` records linked by `ssh_connection_id`; groups are matched to //! existing groups by name or created fresh. +use std::collections::HashMap; + use serde::Deserialize; use super::driver_map; @@ -53,6 +55,7 @@ pub fn build_payload( ssh_connections: Vec::new(), k8s_connections: Vec::new(), tags: Vec::new(), + plugin_secrets: HashMap::new(), }; // Resolve group name -> group id, reusing an existing group when the name @@ -219,6 +222,7 @@ fn build_connection( id: conn_id.to_string(), name: conn.name.clone(), params, + plugin_secret_keys: Vec::new(), group_id, sort_order: None, detect_json_in_text_columns: None, diff --git a/src-tauri/src/connection_import/tabularis.rs b/src-tauri/src/connection_import/tabularis.rs index f404f02f0..ae70b70ee 100644 --- a/src-tauri/src/connection_import/tabularis.rs +++ b/src-tauri/src/connection_import/tabularis.rs @@ -105,6 +105,7 @@ pub fn apply( // Carried through wholesale so any tag_ids on imported connections // keep resolving; the merge in apply_export_payload dedups by id. tags: payload.tags.clone(), + plugin_secrets: HashMap::new(), }; let mut group_ids: HashMap = HashMap::new(); // Original payload group ids to preserve verbatim (with their ancestor @@ -161,6 +162,7 @@ pub fn apply( original_group.clone() }; + let source_connection_id = conn.id.clone(); let mut new_conn = conn; new_conn.id = if res.action == "replace" { res.replace_existing_id.clone().unwrap_or_else(new_id) @@ -169,6 +171,10 @@ pub fn apply( }; new_conn.group_id = group_id; new_conn.sort_order = None; + if let Some(secrets) = payload.plugin_secrets.get(&source_connection_id) { + out.plugin_secrets + .insert(new_conn.id.clone(), secrets.clone()); + } // Remap the linked SSH record to a fresh id so the copy doesn't share // (and later clobber) the original's keychain entry. @@ -240,6 +246,7 @@ mod tests { database: DatabaseSelection::Single(db.into()), ..Default::default() }, + plugin_secret_keys: Vec::new(), group_id: group_id.map(str::to_string), sort_order: Some(3), detect_json_in_text_columns: None, @@ -268,6 +275,7 @@ mod tests { ssh_connections: Vec::new(), k8s_connections: Vec::new(), tags: Vec::new(), + plugin_secrets: HashMap::new(), } } diff --git a/src-tauri/src/connection_migrations_tests.rs b/src-tauri/src/connection_migrations_tests.rs index 0893ab79d..4f5dca845 100644 --- a/src-tauri/src/connection_migrations_tests.rs +++ b/src-tauri/src/connection_migrations_tests.rs @@ -32,6 +32,7 @@ mod tests { ssl_mode: ssl_mode.map(str::to_string), ..base_params() }, + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, diff --git a/src-tauri/src/connection_tags.rs b/src-tauri/src/connection_tags.rs index 5b25c47d8..2c0757e4d 100644 --- a/src-tauri/src/connection_tags.rs +++ b/src-tauri/src/connection_tags.rs @@ -239,6 +239,7 @@ mod tests { id: id.to_string(), name: id.to_string(), params: ConnectionParams::default(), + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, diff --git a/src-tauri/src/credential_cache.rs b/src-tauri/src/credential_cache.rs index ed2c0613e..294d0b37b 100644 --- a/src-tauri/src/credential_cache.rs +++ b/src-tauri/src/credential_cache.rs @@ -20,6 +20,7 @@ pub enum CacheEntry { pub struct CredentialCache { pub db_passwords: Mutex>, pub connection_uris: Mutex>, + pub plugin_secrets: Mutex>, pub ssh_passwords: Mutex>, pub ssh_passphrases: Mutex>, pub ai_keys: Mutex>, @@ -30,6 +31,7 @@ impl Default for CredentialCache { Self { db_passwords: Mutex::new(HashMap::new()), connection_uris: Mutex::new(HashMap::new()), + plugin_secrets: Mutex::new(HashMap::new()), ssh_passwords: Mutex::new(HashMap::new()), ssh_passphrases: Mutex::new(HashMap::new()), ai_keys: Mutex::new(HashMap::new()), @@ -37,6 +39,10 @@ impl Default for CredentialCache { } } +pub(crate) fn plugin_secret_cache_key(connection_id: &str, driver: &str, key: &str) -> String { + format!("{}\0{}\0{}", connection_id, driver, key) +} + // ─── Read-through helpers ───────────────────────────────────────────────────── // These functions are synchronous and intended to be called from inside // `tokio::task::spawn_blocking` when used in async contexts. @@ -95,6 +101,33 @@ pub fn get_connection_uri_cached( Ok(Some(value)) } +pub fn get_plugin_secret_cached( + cache: &CredentialCache, + connection_id: &str, + driver: &str, + key: &str, +) -> Result, String> { + let cache_key = plugin_secret_cache_key(connection_id, driver, key); + { + let guard = cache.plugin_secrets.lock().unwrap(); + match guard.get(&cache_key) { + Some(CacheEntry::Present(value)) => return Ok(Some(value.clone())), + Some(CacheEntry::Absent) => return Ok(None), + None => {} + } + } + + let value = crate::keychain_utils::get_plugin_secret(connection_id, driver, key)?; + cache.plugin_secrets.lock().unwrap().insert( + cache_key, + match &value { + Some(value) => CacheEntry::Present(value.clone()), + None => CacheEntry::Absent, + }, + ); + Ok(value) +} + /// Get SSH password: check cache first, fall through to keychain on miss. pub fn get_ssh_password_cached( cache: &CredentialCache, @@ -198,6 +231,19 @@ pub fn set_connection_uri_cached( ); } +pub fn set_plugin_secret_cached( + cache: &CredentialCache, + connection_id: &str, + driver: &str, + key: &str, + value: &str, +) { + cache.plugin_secrets.lock().unwrap().insert( + plugin_secret_cache_key(connection_id, driver, key), + CacheEntry::Present(value.to_string()), + ); +} + pub fn set_ssh_password_cached(cache: &CredentialCache, connection_id: &str, password: &str) { cache.ssh_passwords.lock().unwrap().insert( connection_id.to_string(), @@ -237,6 +283,19 @@ pub fn invalidate_connection_uri(cache: &CredentialCache, connection_id: &str) { cache.connection_uris.lock().unwrap().remove(connection_id); } +pub fn invalidate_plugin_secret( + cache: &CredentialCache, + connection_id: &str, + driver: &str, + key: &str, +) { + cache + .plugin_secrets + .lock() + .unwrap() + .remove(&plugin_secret_cache_key(connection_id, driver, key)); +} + pub fn invalidate_ssh_password(cache: &CredentialCache, connection_id: &str) { cache.ssh_passwords.lock().unwrap().remove(connection_id); } @@ -255,4 +314,10 @@ pub fn invalidate_all_for_connection(cache: &CredentialCache, connection_id: &st cache.connection_uris.lock().unwrap().remove(connection_id); cache.ssh_passwords.lock().unwrap().remove(connection_id); cache.ssh_passphrases.lock().unwrap().remove(connection_id); + let prefix = format!("{}\0", connection_id); + cache + .plugin_secrets + .lock() + .unwrap() + .retain(|key, _| !key.starts_with(&prefix)); } diff --git a/src-tauri/src/drivers/driver_trait.rs b/src-tauri/src/drivers/driver_trait.rs index e75d6da36..9986b4f44 100644 --- a/src-tauri/src/drivers/driver_trait.rs +++ b/src-tauri/src/drivers/driver_trait.rs @@ -279,6 +279,34 @@ pub fn deprecation_for_builtin(builtin_id: &str) -> Option { } } +/// Optional presentation overrides for one of the host-owned connection +/// fields. Omitted values preserve the host defaults so manifests written +/// before this contract continue to render exactly as they do today. +#[derive(Debug, Serialize, Deserialize, Clone, Default)] +pub struct ConnectionFieldOverride { + #[serde(default)] + pub hidden: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub label: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub placeholder: Option, +} + +/// Driver-specific overrides for the common network connection fields. +#[derive(Debug, Serialize, Deserialize, Clone, Default)] +pub struct ConnectionFieldOverrides { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub host: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub port: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub username: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub password: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub database: Option, +} + /// Metadata describing a registered driver plugin. #[derive(Debug, Serialize, Deserialize, Clone)] pub struct PluginManifest { @@ -327,6 +355,10 @@ pub struct PluginManifest { /// Plugin-owned EXPLAIN parser bundles. Absent for built-in drivers. #[serde(default, skip_serializing_if = "Option::is_none")] pub explain_parsers: Option>, + /// Optional hide/relabel/placeholder overrides for the common connection + /// fields. Absent for built-ins and legacy plugins. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub connection_fields: Option, /// Static type mappings applied by `map_inferred_type`. Keys are generic /// inferred types (uppercase, e.g. `"DATETIME"`), values are driver-specific /// types (e.g. `"TIMESTAMP"`). Empty for built-in drivers which override the diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index ce09d93b9..b156a7aaf 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -2030,6 +2030,7 @@ impl MysqlDriver { ], ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: std::collections::HashMap::new(), deprecated: None, }, diff --git a/src-tauri/src/drivers/postgres/mod.rs b/src-tauri/src/drivers/postgres/mod.rs index a7d34c628..7e6ef4984 100644 --- a/src-tauri/src/drivers/postgres/mod.rs +++ b/src-tauri/src/drivers/postgres/mod.rs @@ -2174,6 +2174,7 @@ impl PostgresDriver { }], ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: std::collections::HashMap::new(), // The built-in postgres driver is deprecated in favour of the // standalone `postgresql` plugin; the decision lives in the diff --git a/src-tauri/src/drivers/registry/tests.rs b/src-tauri/src/drivers/registry/tests.rs index 22db651c2..d9222c0c7 100644 --- a/src-tauri/src/drivers/registry/tests.rs +++ b/src-tauri/src/drivers/registry/tests.rs @@ -18,6 +18,7 @@ fn fake_manifest(id: &str) -> PluginManifest { settings: Vec::new(), ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: HashMap::new(), deprecated: None, } diff --git a/src-tauri/src/drivers/sqlite/mod.rs b/src-tauri/src/drivers/sqlite/mod.rs index 03b9b0146..595f1ff24 100644 --- a/src-tauri/src/drivers/sqlite/mod.rs +++ b/src-tauri/src/drivers/sqlite/mod.rs @@ -1082,6 +1082,7 @@ impl SqliteDriver { settings: vec![], ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: std::collections::HashMap::new(), deprecated: None, }, diff --git a/src-tauri/src/export_import_tests.rs b/src-tauri/src/export_import_tests.rs index 3ed78b0d1..54b7f84dc 100644 --- a/src-tauri/src/export_import_tests.rs +++ b/src-tauri/src/export_import_tests.rs @@ -27,6 +27,7 @@ mod tests { save_in_keychain: Some(true), ..Default::default() }, + plugin_secret_keys: vec![], group_id: Some("group1".to_string()), sort_order: Some(0), detect_json_in_text_columns: None, @@ -49,6 +50,7 @@ mod tests { }], k8s_connections: vec![], tags: vec![], + plugin_secrets: std::collections::HashMap::new(), }; let json = serde_json::to_string(&payload).unwrap(); @@ -113,6 +115,7 @@ mod tests { ssh_connections: vec![], k8s_connections: vec![], tags: vec![], + plugin_secrets: std::collections::HashMap::new(), }; let json = serde_json::to_string(&payload).unwrap(); diff --git a/src-tauri/src/keychain_utils.rs b/src-tauri/src/keychain_utils.rs index 538b7fe1e..96d8436f7 100644 --- a/src-tauri/src/keychain_utils.rs +++ b/src-tauri/src/keychain_utils.rs @@ -140,6 +140,56 @@ pub fn delete_connection_uri(connection_id: &str) -> Result<(), String> { } } +/// Keychain account of a plugin-owned connection secret. The driver id is part +/// of the account so switching a connection to another driver never hands one +/// plugin's secret to another. +fn plugin_secret_account(connection_id: &str, driver: &str, key: &str) -> String { + format!("{}:plugin:{}:{}", connection_id, driver, key) +} + +pub fn set_plugin_secret( + connection_id: &str, + driver: &str, + key: &str, + value: &str, +) -> Result<(), String> { + let entry = Entry::new( + SERVICE_NAME, + &plugin_secret_account(connection_id, driver, key), + ) + .map_err(|e| e.to_string())?; + entry.set_password(value).map_err(keychain_error) +} + +pub fn get_plugin_secret( + connection_id: &str, + driver: &str, + key: &str, +) -> Result, String> { + let entry = Entry::new( + SERVICE_NAME, + &plugin_secret_account(connection_id, driver, key), + ) + .map_err(|e| e.to_string())?; + match entry.get_password() { + Ok(value) => Ok(Some(value)), + Err(keyring::Error::NoEntry) => Ok(None), + Err(error) => Err(keychain_error(error)), + } +} + +pub fn delete_plugin_secret(connection_id: &str, driver: &str, key: &str) -> Result<(), String> { + let entry = Entry::new( + SERVICE_NAME, + &plugin_secret_account(connection_id, driver, key), + ) + .map_err(|e| e.to_string())?; + match entry.delete_credential() { + Ok(()) | Err(keyring::Error::NoEntry) => Ok(()), + Err(error) => Err(keychain_error(error)), + } +} + pub fn set_ssh_password(connection_id: &str, password: &str) -> Result<(), String> { eprintln!("[Keychain] Setting SSH password for {}", connection_id); let entry = diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index edc4fc61e..6411390ac 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -81,6 +81,7 @@ pub mod storage_location_tests; pub mod persistence; #[cfg(test)] pub mod persistence_tests; +pub mod plugin_secrets; pub mod plugins; pub mod pool_manager; #[cfg(test)] diff --git a/src-tauri/src/mcp/mod.rs b/src-tauri/src/mcp/mod.rs index d255894ea..37844dc8b 100644 --- a/src-tauri/src/mcp/mod.rs +++ b/src-tauri/src/mcp/mod.rs @@ -306,6 +306,22 @@ async fn resolve_db_params( } } + // Plugin-owned secrets live only in the keychain and must reach the + // plugin driver through `params.extra`, exactly as in the app. + if !conn.plugin_secret_keys.is_empty() { + conn = tokio::task::spawn_blocking(move || { + let cache = credential_cache::CredentialCache::default(); + crate::plugin_secrets::hydrate_connection(&cache, &mut conn); + conn + }) + .await + .map_err(|e| JsonRpcError { + code: -32000, + message: e.to_string(), + data: None, + })?; + } + let expanded = expand_ssh_params_for_mcp(&conn.params).await?; let expanded = expand_k8s_params_for_mcp(&expanded).await?; let db_params = commands::resolve_connection_params(&expanded).map_err(|e| JsonRpcError { diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 9e2704174..17f761f6b 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -330,6 +330,11 @@ pub struct SavedConnection { pub id: String, pub name: String, pub params: ConnectionParams, + /// Names of plugin-owned `extra` values stored in the OS keychain. Only + /// the names are persisted; values are resolved into `params.extra` at + /// runtime and are never written to the connections file. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub plugin_secret_keys: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub group_id: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -434,6 +439,11 @@ pub struct ExportPayload { pub k8s_connections: Vec, #[serde(default)] pub tags: Vec, + /// Export-only secret material keyed by connection id and plugin field. + /// This remains empty for metadata-only exports and never appears in + /// `connections.json`. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub plugin_secrets: HashMap>, } #[derive(Debug, Deserialize, Serialize, Clone)] @@ -441,6 +451,10 @@ pub struct TestConnectionRequest { pub params: ConnectionParams, #[serde(skip_serializing_if = "Option::is_none")] pub connection_id: Option, + /// Per-field secret mutations. Missing keys preserve stored values, + /// strings set values, and null explicitly clears values. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub plugin_secret_changes: HashMap>, /// When set, the test emits "connection-test-progress" events tagged with /// this id so the caller can render a live step log. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/src-tauri/src/models_tests.rs b/src-tauri/src/models_tests.rs index 9eb669175..f1fbfc5d0 100644 --- a/src-tauri/src/models_tests.rs +++ b/src-tauri/src/models_tests.rs @@ -222,4 +222,19 @@ mod tests { let json = serde_json::to_string(¶ms).expect("serialize params"); assert!(!json.contains("extra")); } + + #[test] + fn saved_connection_defaults_plugin_secret_markers_for_legacy_json() { + let stored = r#"{ + "id": "legacy", + "name": "Legacy", + "params": { "driver": "mysql", "database": "app" } + }"#; + let connection: crate::models::SavedConnection = + serde_json::from_str(stored).expect("legacy connection deserializes"); + assert!(connection.plugin_secret_keys.is_empty()); + + let json = serde_json::to_string(&connection).expect("serialize connection"); + assert!(!json.contains("plugin_secret_keys")); + } } diff --git a/src-tauri/src/persistence.rs b/src-tauri/src/persistence.rs index 17ce233d7..14a6ba5ae 100644 --- a/src-tauri/src/persistence.rs +++ b/src-tauri/src/persistence.rs @@ -55,6 +55,9 @@ pub fn save_connections_file(path: &Path, file: &ConnectionsFile) -> Result<(), let mut connections_to_save = Vec::new(); for conn in &file.connections { let mut c = conn.clone(); + for key in &c.plugin_secret_keys { + c.params.extra.remove(key); + } if c.params.save_in_keychain.unwrap_or(false) { // Passwords are stored in keychain, remove from JSON c.params.password = None; diff --git a/src-tauri/src/persistence_tests.rs b/src-tauri/src/persistence_tests.rs index cb388246f..23c1c55fb 100644 --- a/src-tauri/src/persistence_tests.rs +++ b/src-tauri/src/persistence_tests.rs @@ -46,6 +46,7 @@ mod tests { save_in_keychain: Some(save_in_keychain), ..ConnectionParams::default() }, + plugin_secret_keys: Vec::new(), group_id: None, sort_order: None, detect_json_in_text_columns: None, @@ -178,4 +179,43 @@ mod tests { ); assert_eq!(std::fs::read_to_string(&path).unwrap(), "new-secret"); } + + #[test] + fn strips_plugin_secret_values_but_preserves_markers() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("connections.json"); + let connection = SavedConnection { + id: "connection-1".into(), + name: "Plugin".into(), + params: ConnectionParams { + driver: "bigquery".into(), + database: crate::models::DatabaseSelection::Single("project".into()), + extra: [ + ("location".into(), "EU".into()), + ("credential".into(), "private-value".into()), + ] + .into_iter() + .collect(), + ..Default::default() + }, + plugin_secret_keys: vec!["credential".into()], + group_id: None, + sort_order: None, + detect_json_in_text_columns: None, + appearance: None, + tag_ids: None, + environment: None, + }; + let file = ConnectionsFile { + connections: vec![connection], + ..Default::default() + }; + + save_connections_file(&path, &file).expect("save connections"); + let stored = std::fs::read_to_string(path).expect("read connections"); + assert!(!stored.contains("private-value")); + assert!(stored.contains("credential")); + assert!(stored.contains("location")); + assert!(stored.contains("EU")); + } } diff --git a/src-tauri/src/plugin_secrets.rs b/src-tauri/src/plugin_secrets.rs new file mode 100644 index 000000000..612b7f030 --- /dev/null +++ b/src-tauri/src/plugin_secrets.rs @@ -0,0 +1,547 @@ +//! Plugin-owned connection secrets. +//! +//! A driver plugin can ask the connection modal to store some of its `extra` +//! fields as secrets (`setSecretField` in the `connection-modal.extra_fields` +//! slot). Only the field names are persisted, as +//! [`SavedConnection::plugin_secret_keys`]; the values live in the OS keychain +//! under `:plugin::` and are resolved into +//! `params.extra` in memory, right before a driver call. They never reach +//! `connections.json` and are never returned to the frontend. + +use std::collections::{HashMap, HashSet}; + +use crate::credential_cache::{self, CredentialCache}; +use crate::models::{ConnectionParams, SavedConnection}; + +pub type PluginSecretChanges = HashMap>; + +const MAX_SECRET_KEY_LENGTH: usize = 64; +const MAX_SECRET_VALUE_LENGTH: usize = 1024 * 1024; + +pub fn validate_secret_changes(changes: &PluginSecretChanges) -> Result<(), String> { + for (key, value) in changes { + validate_secret_key(key)?; + if value + .as_ref() + .is_some_and(|value| value.len() > MAX_SECRET_VALUE_LENGTH) + { + return Err(format!( + "Plugin secret '{}' exceeds the maximum size of {} bytes", + key, MAX_SECRET_VALUE_LENGTH + )); + } + } + Ok(()) +} + +pub fn validate_secret_key(key: &str) -> Result<(), String> { + if key.is_empty() + || key.len() > MAX_SECRET_KEY_LENGTH + || !key + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.')) + { + return Err(format!( + "Invalid plugin secret key '{}': use 1-{} ASCII letters, digits, '.', '_' or '-'", + key, MAX_SECRET_KEY_LENGTH + )); + } + Ok(()) +} + +pub fn next_secret_keys( + existing: &[String], + same_driver: bool, + changes: &PluginSecretChanges, +) -> Result, String> { + validate_secret_changes(changes)?; + let mut keys: HashSet = if same_driver { + existing.iter().cloned().collect() + } else { + HashSet::new() + }; + for (key, value) in changes { + if value.is_some() { + keys.insert(key.clone()); + } else { + keys.remove(key); + } + } + let mut keys: Vec = keys.into_iter().collect(); + keys.sort(); + Ok(keys) +} + +/// Resolve the stored plugin secrets of `connection` into `params.extra`. +/// +/// Lenient by design: a key whose keychain entry is missing (connections file +/// synced to another machine, keychain reset, entry deleted by hand) or cannot +/// be read (locked keychain, access denied) is logged and left unset, and the +/// call goes on. The plugin then reports the missing credential itself, which +/// is a far better failure than every query, dump and MCP call of the +/// connection erroring out on the host side. +pub fn hydrate_connection(cache: &CredentialCache, connection: &mut SavedConnection) { + let values = load_secret_values( + cache, + &connection.id, + &connection.params.driver, + &connection.plugin_secret_keys, + ); + connection.params.extra.extend(values); +} + +/// Read the stored values of `keys` through the credential cache. Keys that +/// are invalid, missing or unreadable are skipped with a warning (see +/// [`resolve_secret_values`]). This is the single read path shared by driver +/// calls, Test Connection, export, backups and duplicate. +pub fn load_secret_values( + cache: &CredentialCache, + connection_id: &str, + driver: &str, + keys: &[String], +) -> HashMap { + resolve_secret_values(connection_id, keys, |key| { + credential_cache::get_plugin_secret_cached(cache, connection_id, driver, key) + }) + .values +} + +/// Plugin secret values read for one connection, plus the keys that could not +/// be resolved. +#[derive(Debug, Default)] +pub struct ResolvedSecrets { + pub values: HashMap, + pub unavailable: Vec, +} + +/// Read `keys` with `read`, never failing. Invalid key names, missing entries +/// (`Ok(None)`) and read errors are logged by key name (never by value) and +/// reported in [`ResolvedSecrets::unavailable`]. +pub fn resolve_secret_values( + connection_id: &str, + keys: &[String], + mut read: impl FnMut(&str) -> Result, String>, +) -> ResolvedSecrets { + let mut resolved = ResolvedSecrets::default(); + for key in keys { + if validate_secret_key(key).is_err() { + log::warn!( + "Skipping invalid plugin secret key of connection {}", + connection_id + ); + resolved.unavailable.push(key.clone()); + continue; + } + match read(key) { + Ok(Some(value)) => { + resolved.values.insert(key.clone(), value); + } + Ok(None) => { + log::warn!( + "Plugin secret '{}' of connection {} is missing from the keychain; leaving it unset", + key, + connection_id + ); + resolved.unavailable.push(key.clone()); + } + Err(error) => { + log::warn!( + "Failed to read plugin secret '{}' of connection {}; leaving it unset: {}", + key, + connection_id, + error + ); + resolved.unavailable.push(key.clone()); + } + } + } + resolved +} + +/// Secret changes that give a duplicate of a connection its own copy of every +/// value that can still be read. Unavailable keys are not carried over, so the +/// copy never lists a secret it does not have. +pub fn duplicate_secret_changes( + connection_id: &str, + keys: &[String], + read: impl FnMut(&str) -> Result, String>, +) -> PluginSecretChanges { + resolve_secret_values(connection_id, keys, read) + .values + .into_iter() + .map(|(key, value)| (key, Some(value))) + .collect() +} + +/// Resolve the plugin secrets for an ad-hoc driver call made with params sent +/// by the connection modal (Test Connection, Load Databases). Stored values of +/// `saved` are loaded only while the driver is unchanged, and only for the +/// keys the pending `changes` do not already set or clear. +pub fn apply_request_secrets( + cache: &CredentialCache, + saved: Option<&SavedConnection>, + params: &mut ConnectionParams, + changes: &PluginSecretChanges, +) -> Result<(), String> { + let stored = match saved.filter(|saved| saved.params.driver == params.driver) { + Some(saved) => { + let unresolved: Vec = saved + .plugin_secret_keys + .iter() + .filter(|key| !changes.contains_key(*key)) + .cloned() + .collect(); + load_secret_values(cache, &saved.id, &saved.params.driver, &unresolved) + } + None => HashMap::new(), + }; + apply_runtime_changes(params, stored, changes) +} + +pub fn apply_runtime_changes( + params: &mut ConnectionParams, + stored_values: HashMap, + changes: &PluginSecretChanges, +) -> Result<(), String> { + validate_secret_changes(changes)?; + params.extra.extend(stored_values); + for (key, value) in changes { + match value { + Some(value) => _ = params.extra.insert(key.clone(), value.clone()), + None => _ = params.extra.remove(key), + } + } + Ok(()) +} + +pub fn persist_secret_changes( + cache: &CredentialCache, + connection_id: &str, + old_driver: Option<&str>, + old_keys: &[String], + new_driver: &str, + changes: &PluginSecretChanges, + persist: impl FnOnce() -> Result<(), String>, +) -> Result<(), String> { + validate_secret_changes(changes)?; + let same_driver = match old_driver { + None => true, + Some(driver) => driver == new_driver, + }; + let mut affected: Vec<(String, String)> = if same_driver { + changes + .keys() + .map(|key| (new_driver.to_string(), key.clone())) + .collect() + } else { + old_keys + .iter() + .map(|key| (old_driver.unwrap_or_default().to_string(), key.clone())) + .chain( + changes + .keys() + .map(|key| (new_driver.to_string(), key.clone())), + ) + .collect() + }; + affected.sort(); + affected.dedup(); + + let mut snapshots = Vec::with_capacity(affected.len()); + for (driver, key) in &affected { + let previous = crate::keychain_utils::get_plugin_secret(connection_id, driver, key)?; + snapshots.push((driver.clone(), key.clone(), previous)); + } + + let apply_result = (|| { + if !same_driver { + let old_driver = old_driver.unwrap_or_default(); + for key in old_keys { + crate::keychain_utils::delete_plugin_secret(connection_id, old_driver, key)?; + credential_cache::invalidate_plugin_secret(cache, connection_id, old_driver, key); + } + } + for (key, value) in changes { + match value { + Some(value) => { + crate::keychain_utils::set_plugin_secret( + connection_id, + new_driver, + key, + value, + )?; + credential_cache::set_plugin_secret_cached( + cache, + connection_id, + new_driver, + key, + value, + ); + } + None => { + crate::keychain_utils::delete_plugin_secret(connection_id, new_driver, key)?; + credential_cache::invalidate_plugin_secret( + cache, + connection_id, + new_driver, + key, + ); + } + } + } + persist() + })(); + + if apply_result.is_ok() { + return Ok(()); + } + + let original_error = apply_result.unwrap_err(); + let mut rollback_errors = Vec::new(); + for (driver, key, previous) in snapshots { + let result = match previous { + Some(value) => { + crate::keychain_utils::set_plugin_secret(connection_id, &driver, &key, &value).map( + |()| { + credential_cache::set_plugin_secret_cached( + cache, + connection_id, + &driver, + &key, + &value, + ); + }, + ) + } + None => crate::keychain_utils::delete_plugin_secret(connection_id, &driver, &key).map( + |()| { + credential_cache::invalidate_plugin_secret(cache, connection_id, &driver, &key); + }, + ), + }; + if let Err(error) = result { + rollback_errors.push(error); + } + } + + if rollback_errors.is_empty() { + Err(original_error) + } else { + Err(format!( + "{} (failed to roll back plugin secrets: {})", + original_error, + rollback_errors.join("; ") + )) + } +} + +/// A plugin secret write deferred until the connections file is saved. +#[derive(Clone, PartialEq, Eq)] +pub enum SecretWrite { + Set { + connection_id: String, + driver: String, + key: String, + value: String, + }, + Delete { + connection_id: String, + driver: String, + key: String, + }, +} + +// Hand-written so a secret value never ends up in a log or a test failure. +impl std::fmt::Debug for SecretWrite { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Set { + connection_id, + driver, + key, + .. + } => f + .debug_struct("Set") + .field("connection_id", connection_id) + .field("driver", driver) + .field("key", key) + .finish_non_exhaustive(), + Self::Delete { + connection_id, + driver, + key, + } => f + .debug_struct("Delete") + .field("connection_id", connection_id) + .field("driver", driver) + .field("key", key) + .finish(), + } + } +} + +/// Keychain operations on plugin secrets. The production implementation is +/// [`KeychainSecretStore`]; tests use an in-memory store. +pub trait SecretStore { + fn set(&self, connection_id: &str, driver: &str, key: &str, value: &str) -> Result<(), String>; + fn delete(&self, connection_id: &str, driver: &str, key: &str) -> Result<(), String>; +} + +pub struct KeychainSecretStore; + +impl SecretStore for KeychainSecretStore { + fn set(&self, connection_id: &str, driver: &str, key: &str, value: &str) -> Result<(), String> { + crate::keychain_utils::set_plugin_secret(connection_id, driver, key, value) + } + + fn delete(&self, connection_id: &str, driver: &str, key: &str) -> Result<(), String> { + crate::keychain_utils::delete_plugin_secret(connection_id, driver, key) + } +} + +/// Work out the plugin secret keys and deferred keychain writes of one +/// imported connection. The payload is untrusted: invalid key names and +/// oversized values are skipped with a warning instead of aborting the import. +/// +/// Every declared or imported secret key is removed from `params.extra`, so a +/// value never lands in `connections.json`. Imported values are "set" changes: +/// keys the payload does not carry keep their stored value while the driver is +/// unchanged; when it changed, the previous driver's secrets are deleted. +pub fn plan_imported_secrets( + connection: &mut SavedConnection, + declared_keys: &[String], + imported_values: HashMap, + previous: Option<(&str, &[String])>, +) -> Vec { + for key in declared_keys { + connection.params.extra.remove(key); + } + let mut changes = PluginSecretChanges::new(); + for (key, value) in imported_values { + connection.params.extra.remove(&key); + let change = HashMap::from([(key.clone(), Some(value))]); + if validate_secret_changes(&change).is_err() { + log::warn!( + "Skipping an invalid plugin secret of imported connection {}", + connection.id + ); + continue; + } + changes.extend(change); + } + + let driver = connection.params.driver.clone(); + let kept_keys: Vec = match previous { + Some((old_driver, old_keys)) if old_driver == driver => old_keys + .iter() + .filter(|key| validate_secret_key(key).is_ok()) + .cloned() + .collect(), + _ => Vec::new(), + }; + connection.plugin_secret_keys = + next_secret_keys(&kept_keys, true, &changes).unwrap_or(kept_keys); + + let mut writes: Vec = Vec::new(); + if let Some((old_driver, old_keys)) = previous { + if old_driver != driver { + let valid_old_keys = old_keys + .iter() + .filter(|key| validate_secret_key(key).is_ok()); + writes.extend(valid_old_keys.map(|key| SecretWrite::Delete { + connection_id: connection.id.clone(), + driver: old_driver.to_string(), + key: key.clone(), + })); + } + } + let mut changes: Vec<(String, Option)> = changes.into_iter().collect(); + changes.sort_by(|a, b| a.0.cmp(&b.0)); + writes.extend(changes.into_iter().filter_map(|(key, value)| { + value.map(|value| SecretWrite::Set { + connection_id: connection.id.clone(), + driver: driver.clone(), + key, + value, + }) + })); + writes +} + +/// Run `save` (the connections file write) and, only once it succeeded, apply +/// the deferred plugin secret `writes`. A failed save therefore never leaves +/// orphaned secrets in the keychain. A keychain write that fails after the +/// save is logged and skipped: the connection lists the key, and runtime +/// hydration leaves it unset so the plugin reports the missing credential. +pub fn commit_secret_writes( + store: &impl SecretStore, + cache: &CredentialCache, + writes: Vec, + save: impl FnOnce() -> Result<(), String>, +) -> Result<(), String> { + save()?; + for write in writes { + match write { + SecretWrite::Set { + connection_id, + driver, + key, + value, + } => match store.set(&connection_id, &driver, &key, &value) { + Ok(()) => credential_cache::set_plugin_secret_cached( + cache, + &connection_id, + &driver, + &key, + &value, + ), + Err(error) => { + credential_cache::invalidate_plugin_secret( + cache, + &connection_id, + &driver, + &key, + ); + log::warn!( + "Failed to store imported plugin secret '{}' of connection {}: {}", + key, + connection_id, + error + ); + } + }, + SecretWrite::Delete { + connection_id, + driver, + key, + } => { + if let Err(error) = store.delete(&connection_id, &driver, &key) { + log::warn!( + "Failed to delete plugin secret '{}' of connection {}: {}", + key, + connection_id, + error + ); + } + credential_cache::invalidate_plugin_secret(cache, &connection_id, &driver, &key); + } + } + } + Ok(()) +} + +pub fn delete_connection_secrets( + cache: &CredentialCache, + connection_id: &str, + driver: &str, + keys: &[String], +) { + for key in keys { + let _ = crate::keychain_utils::delete_plugin_secret(connection_id, driver, key); + credential_cache::invalidate_plugin_secret(cache, connection_id, driver, key); + } +} + +#[cfg(test)] +#[path = "plugin_secrets_tests.rs"] +mod tests; diff --git a/src-tauri/src/plugin_secrets_tests.rs b/src-tauri/src/plugin_secrets_tests.rs new file mode 100644 index 000000000..1e0259ca5 --- /dev/null +++ b/src-tauri/src/plugin_secrets_tests.rs @@ -0,0 +1,503 @@ +use std::cell::RefCell; + +use super::*; + +#[test] +fn preserves_existing_keys_and_applies_tri_state_changes() { + let changes = HashMap::from([ + ("replace".to_string(), Some("new".to_string())), + ("remove".to_string(), None), + ("add".to_string(), Some("value".to_string())), + ]); + let keys = next_secret_keys( + &["keep".into(), "replace".into(), "remove".into()], + true, + &changes, + ) + .unwrap(); + assert_eq!(keys, vec!["add", "keep", "replace"]); +} + +#[test] +fn changing_driver_drops_old_keys() { + let changes = HashMap::from([("new-token".to_string(), Some("x".to_string()))]); + let keys = next_secret_keys(&["old-token".into()], false, &changes).unwrap(); + assert_eq!(keys, vec!["new-token"]); +} + +#[test] +fn runtime_changes_merge_stored_values_and_apply_replacements_and_clears() { + let mut params = ConnectionParams { + extra: HashMap::from([ + ("region".to_string(), "EU".to_string()), + ("remove".to_string(), "stale".to_string()), + ]), + ..Default::default() + }; + let stored = HashMap::from([ + ("keep".to_string(), "stored".to_string()), + ("replace".to_string(), "old".to_string()), + ]); + let changes = HashMap::from([ + ("replace".to_string(), Some("new".to_string())), + ("remove".to_string(), None), + ]); + + apply_runtime_changes(&mut params, stored, &changes).unwrap(); + + assert_eq!(params.extra.get("region").map(String::as_str), Some("EU")); + assert_eq!(params.extra.get("keep").map(String::as_str), Some("stored")); + assert_eq!(params.extra.get("replace").map(String::as_str), Some("new")); + assert!(!params.extra.contains_key("remove")); +} + +#[test] +fn rejects_keys_that_could_escape_the_keychain_namespace() { + assert!(validate_secret_key("credential").is_ok()); + assert!(validate_secret_key("oauth.refresh_token").is_ok()); + assert!(validate_secret_key("bad:key").is_err()); + assert!(validate_secret_key("").is_err()); +} + +fn saved_connection(driver: &str, keys: &[&str]) -> SavedConnection { + SavedConnection { + id: "conn-1".to_string(), + name: "Plugin".to_string(), + params: ConnectionParams { + driver: driver.to_string(), + ..Default::default() + }, + plugin_secret_keys: keys.iter().map(|key| key.to_string()).collect(), + group_id: None, + sort_order: None, + detect_json_in_text_columns: None, + appearance: None, + tag_ids: None, + environment: None, + } +} + +#[test] +fn request_secrets_resolve_stored_values_and_apply_pending_changes() { + let cache = CredentialCache::default(); + credential_cache::set_plugin_secret_cached( + &cache, + "conn-1", + "bigquery", + "credential", + "stored", + ); + credential_cache::set_plugin_secret_cached(&cache, "conn-1", "bigquery", "token", "old"); + let saved = saved_connection("bigquery", &["credential", "token"]); + let mut params = ConnectionParams { + driver: "bigquery".to_string(), + extra: HashMap::from([("location".to_string(), "EU".to_string())]), + ..Default::default() + }; + let changes = HashMap::from([ + ("token".to_string(), None), + ("refresh".to_string(), Some("typed".to_string())), + ]); + + apply_request_secrets(&cache, Some(&saved), &mut params, &changes).unwrap(); + + assert_eq!(params.extra.get("location").map(String::as_str), Some("EU")); + assert_eq!( + params.extra.get("credential").map(String::as_str), + Some("stored") + ); + assert_eq!( + params.extra.get("refresh").map(String::as_str), + Some("typed") + ); + assert!(!params.extra.contains_key("token")); +} + +#[test] +fn request_secrets_ignore_stored_values_of_another_driver() { + let cache = CredentialCache::default(); + credential_cache::set_plugin_secret_cached( + &cache, + "conn-1", + "bigquery", + "credential", + "stored", + ); + let saved = saved_connection("bigquery", &["credential"]); + let mut params = ConnectionParams { + driver: "postgresql".to_string(), + ..Default::default() + }; + + apply_request_secrets(&cache, Some(&saved), &mut params, &HashMap::new()).unwrap(); + + assert!(params.extra.is_empty()); +} + +#[test] +fn request_secrets_without_saved_connection_only_apply_changes() { + let cache = CredentialCache::default(); + let mut params = ConnectionParams::default(); + let changes = HashMap::from([("credential".to_string(), Some("typed".to_string()))]); + + apply_request_secrets(&cache, None, &mut params, &changes).unwrap(); + + assert_eq!( + params.extra.get("credential").map(String::as_str), + Some("typed") + ); +} + +#[test] +fn request_secrets_reject_invalid_change_keys() { + let cache = CredentialCache::default(); + let mut params = ConnectionParams::default(); + let changes = HashMap::from([("bad:key".to_string(), Some("x".to_string()))]); + + assert!(apply_request_secrets(&cache, None, &mut params, &changes).is_err()); +} + +#[test] +fn available_secret_values_skip_invalid_keys() { + let cache = CredentialCache::default(); + credential_cache::set_plugin_secret_cached( + &cache, + "conn-1", + "bigquery", + "credential", + "stored", + ); + + let values = load_secret_values( + &cache, + "conn-1", + "bigquery", + &["credential".to_string(), "bad:key".to_string()], + ); + + assert_eq!(values.len(), 1); + assert_eq!(values.get("credential").map(String::as_str), Some("stored")); +} + +#[test] +fn hydrate_connection_merges_stored_values_into_extra() { + let cache = CredentialCache::default(); + credential_cache::set_plugin_secret_cached( + &cache, + "conn-1", + "bigquery", + "credential", + "stored", + ); + let mut connection = saved_connection("bigquery", &["credential"]); + + hydrate_connection(&cache, &mut connection); + + assert_eq!( + connection + .params + .extra + .get("credential") + .map(String::as_str), + Some("stored") + ); +} + +fn missing_entry_reader( + stored: &[(&str, &str)], +) -> impl FnMut(&str) -> Result, String> { + let stored: HashMap = stored + .iter() + .map(|(key, value)| (key.to_string(), value.to_string())) + .collect(); + move |key: &str| match key { + "locked" => Err("Keychain access denied".to_string()), + _ => Ok(stored.get(key).cloned()), + } +} + +#[test] +fn resolve_leaves_missing_and_unreadable_keys_unset() { + let keys = vec![ + "credential".to_string(), + "token".to_string(), + "locked".to_string(), + "bad:key".to_string(), + ]; + + let resolved = resolve_secret_values( + "conn-1", + &keys, + missing_entry_reader(&[("credential", "stored")]), + ); + + assert_eq!(resolved.values.len(), 1); + assert_eq!( + resolved.values.get("credential").map(String::as_str), + Some("stored") + ); + assert_eq!(resolved.unavailable, vec!["token", "locked", "bad:key"]); +} + +#[test] +fn hydrate_keeps_connection_usable_when_a_keychain_entry_is_missing() { + let cache = CredentialCache::default(); + credential_cache::set_plugin_secret_cached( + &cache, + "conn-1", + "bigquery", + "credential", + "stored", + ); + // A cached miss stands in for a keychain without the entry. + cache.plugin_secrets.lock().unwrap().insert( + credential_cache::plugin_secret_cache_key("conn-1", "bigquery", "token"), + credential_cache::CacheEntry::Absent, + ); + let mut connection = saved_connection("bigquery", &["credential", "token"]); + connection + .params + .extra + .insert("location".to_string(), "EU".to_string()); + + hydrate_connection(&cache, &mut connection); + + assert_eq!( + connection + .params + .extra + .get("credential") + .map(String::as_str), + Some("stored") + ); + assert_eq!( + connection.params.extra.get("location").map(String::as_str), + Some("EU") + ); + assert!(!connection.params.extra.contains_key("token")); + assert_eq!(connection.plugin_secret_keys, vec!["credential", "token"]); +} + +#[test] +fn request_secrets_tolerate_a_missing_stored_entry() { + let cache = CredentialCache::default(); + cache.plugin_secrets.lock().unwrap().insert( + credential_cache::plugin_secret_cache_key("conn-1", "bigquery", "credential"), + credential_cache::CacheEntry::Absent, + ); + let saved = saved_connection("bigquery", &["credential"]); + let mut params = ConnectionParams { + driver: "bigquery".to_string(), + ..Default::default() + }; + + apply_request_secrets(&cache, Some(&saved), &mut params, &HashMap::new()).unwrap(); + + assert!(params.extra.is_empty()); +} + +#[test] +fn duplicate_copies_only_the_secrets_that_can_be_read() { + let keys = vec![ + "credential".to_string(), + "token".to_string(), + "locked".to_string(), + ]; + + let changes = duplicate_secret_changes( + "conn-1", + &keys, + missing_entry_reader(&[("credential", "stored")]), + ); + + assert_eq!( + changes, + HashMap::from([("credential".to_string(), Some("stored".to_string()))]) + ); + assert_eq!( + next_secret_keys(&[], true, &changes).unwrap(), + vec!["credential"] + ); +} + +#[test] +fn import_skips_invalid_keys_and_keeps_the_rest() { + let mut connection = saved_connection("bigquery", &[]); + connection.params.extra = HashMap::from([ + ("credential".to_string(), "inline".to_string()), + ("bad:key".to_string(), "inline".to_string()), + ("location".to_string(), "EU".to_string()), + ]); + let imported = HashMap::from([ + ("credential".to_string(), "secret".to_string()), + ("bad:key".to_string(), "secret".to_string()), + ("".to_string(), "secret".to_string()), + ("huge".to_string(), "x".repeat(MAX_SECRET_VALUE_LENGTH + 1)), + ]); + + let writes = plan_imported_secrets( + &mut connection, + &["credential".to_string(), "bad:key".to_string()], + imported, + None, + ); + + assert_eq!(connection.plugin_secret_keys, vec!["credential"]); + assert_eq!( + writes, + vec![SecretWrite::Set { + connection_id: "conn-1".to_string(), + driver: "bigquery".to_string(), + key: "credential".to_string(), + value: "secret".to_string(), + }] + ); + // No secret value, valid or not, is left in the persisted params. + assert_eq!( + connection.params.extra, + HashMap::from([("location".to_string(), "EU".to_string())]) + ); +} + +#[test] +fn import_keeps_stored_keys_of_the_same_driver_and_drops_another_drivers() { + let mut same = saved_connection("bigquery", &[]); + let old_keys = vec!["token".to_string()]; + let writes = plan_imported_secrets( + &mut same, + &[], + HashMap::new(), + Some(("bigquery", &old_keys)), + ); + assert!(writes.is_empty()); + assert_eq!(same.plugin_secret_keys, vec!["token"]); + + let mut switched = saved_connection("bigquery", &[]); + let writes = plan_imported_secrets( + &mut switched, + &[], + HashMap::new(), + Some(("snowflake", &old_keys)), + ); + assert!(switched.plugin_secret_keys.is_empty()); + assert_eq!( + writes, + vec![SecretWrite::Delete { + connection_id: "conn-1".to_string(), + driver: "snowflake".to_string(), + key: "token".to_string(), + }] + ); +} + +#[derive(Default)] +struct MemoryStore { + entries: RefCell>, + failing_key: Option, +} + +impl SecretStore for MemoryStore { + fn set(&self, connection_id: &str, driver: &str, key: &str, value: &str) -> Result<(), String> { + if self.failing_key.as_deref() == Some(key) { + return Err("Keychain locked".to_string()); + } + self.entries.borrow_mut().insert( + (connection_id.into(), driver.into(), key.into()), + value.into(), + ); + Ok(()) + } + + fn delete(&self, connection_id: &str, driver: &str, key: &str) -> Result<(), String> { + self.entries + .borrow_mut() + .remove(&(connection_id.into(), driver.into(), key.into())); + Ok(()) + } +} + +fn set_write(key: &str, value: &str) -> SecretWrite { + SecretWrite::Set { + connection_id: "conn-1".to_string(), + driver: "bigquery".to_string(), + key: key.to_string(), + value: value.to_string(), + } +} + +#[test] +fn failed_save_leaves_no_orphaned_secrets() { + let store = MemoryStore::default(); + store.entries.borrow_mut().insert( + ("conn-1".into(), "snowflake".into(), "token".into()), + "old".into(), + ); + let cache = CredentialCache::default(); + let writes = vec![ + SecretWrite::Delete { + connection_id: "conn-1".to_string(), + driver: "snowflake".to_string(), + key: "token".to_string(), + }, + set_write("credential", "secret"), + ]; + + let result = commit_secret_writes(&store, &cache, writes, || Err("disk full".to_string())); + + assert_eq!(result, Err("disk full".to_string())); + assert_eq!( + *store.entries.borrow(), + HashMap::from([( + ( + "conn-1".to_string(), + "snowflake".to_string(), + "token".to_string() + ), + "old".to_string() + )]) + ); + assert!(cache.plugin_secrets.lock().unwrap().is_empty()); +} + +#[test] +fn successful_save_applies_writes_and_tolerates_a_failing_one() { + let store = MemoryStore { + failing_key: Some("locked".to_string()), + ..Default::default() + }; + let cache = CredentialCache::default(); + let saved = RefCell::new(false); + + commit_secret_writes( + &store, + &cache, + vec![set_write("locked", "x"), set_write("credential", "secret")], + || { + *saved.borrow_mut() = true; + Ok(()) + }, + ) + .unwrap(); + + assert!(*saved.borrow()); + assert_eq!( + store + .entries + .borrow() + .get(&("conn-1".into(), "bigquery".into(), "credential".into())) + .map(String::as_str), + Some("secret") + ); + assert_eq!(store.entries.borrow().len(), 1); + assert_eq!( + credential_cache::get_plugin_secret_cached(&cache, "conn-1", "bigquery", "credential"), + Ok(Some("secret".to_string())) + ); +} + +#[test] +fn secret_write_debug_never_prints_the_value() { + let rendered = format!("{:?}", set_write("credential", "super-secret")); + assert!(rendered.contains("credential")); + assert!(!rendered.contains("super-secret")); +} diff --git a/src-tauri/src/plugins/commands.rs b/src-tauri/src/plugins/commands.rs index 8cdc61a76..66d993b7f 100644 --- a/src-tauri/src/plugins/commands.rs +++ b/src-tauri/src/plugins/commands.rs @@ -349,6 +349,7 @@ pub async fn get_plugin_manifest(plugin_id: String) -> Result PluginManifest { settings: Vec::new(), ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: HashMap::new(), deprecated: None, } diff --git a/src-tauri/src/plugins/manager.rs b/src-tauri/src/plugins/manager.rs index 94dd940d7..9fc2a39c7 100644 --- a/src-tauri/src/plugins/manager.rs +++ b/src-tauri/src/plugins/manager.rs @@ -8,7 +8,8 @@ use tauri::AppHandle; use crate::config::PluginConfig; use crate::drivers::driver_trait::{ - DriverCapabilities, ExplainParserManifestEntry, PluginManifest, PluginSettingDefinition, + ConnectionFieldOverrides, DriverCapabilities, ExplainParserManifestEntry, PluginManifest, + PluginSettingDefinition, }; use crate::models::DataTypeInfo; use crate::plugins::driver::RpcDriver; @@ -91,6 +92,7 @@ pub struct ConfigManifest { pub ui_extensions: Option>, #[serde(default)] pub explain_parsers: Option>, + pub connection_fields: Option, /// Static type mappings for `map_inferred_type`. Keys are generic inferred /// types (e.g. `"DATETIME"`), values are driver-specific types (e.g. `"TIMESTAMP"`). #[serde(default)] @@ -269,6 +271,7 @@ pub async fn load_plugin_from_dir( settings: config.settings, ui_extensions: config.ui_extensions, explain_parsers: config.explain_parsers, + connection_fields: config.connection_fields, type_mappings: config.type_mappings, // External plugins are not deprecated in favour of another plugin; // `deprecated` is a built-in-only concept stamped at registration. diff --git a/src-tauri/src/plugins/tests.rs b/src-tauri/src/plugins/tests.rs index 8c5f40bd2..fdbf88f8a 100644 --- a/src-tauri/src/plugins/tests.rs +++ b/src-tauri/src/plugins/tests.rs @@ -164,6 +164,28 @@ fn preserves_explain_parser_declarations_from_manifest() { assert_eq!(entries[0].module, "explain/dist/index.iife.js"); } +#[test] +fn parses_optional_connection_field_overrides() { + let manifest: ConfigManifest = serde_json::from_str( + r#"{ + "name": "bigquery", + "version": "1.0.0", + "description": "BigQuery driver", + "connection_fields": { + "host": { "hidden": true }, + "database": { "label": "GCP Project ID", "placeholder": "billing-project" } + } +}"#, + ) + .expect("parse manifest"); + + let fields = manifest.connection_fields.expect("connection fields"); + assert!(fields.host.expect("host override").hidden); + let database = fields.database.expect("database override"); + assert_eq!(database.label.as_deref(), Some("GCP Project ID")); + assert_eq!(database.placeholder.as_deref(), Some("billing-project")); +} + #[test] fn returns_error_for_invalid_manifest() { let dir = tempdir().expect("temp dir"); diff --git a/src-tauri/src/sqlite_database.rs b/src-tauri/src/sqlite_database.rs index a323e915b..2121dd58d 100644 --- a/src-tauri/src/sqlite_database.rs +++ b/src-tauri/src/sqlite_database.rs @@ -125,7 +125,7 @@ pub async fn create_sqlite_database( ..ConnectionParams::default() }; - match crate::commands::save_connection(app, name, params, None, None).await { + match crate::commands::save_connection(app, name, params, None, None, None).await { Ok(connection) => Ok(connection), Err(error) => { if let Err(cleanup_error) = fs::remove_file(&path) { diff --git a/src-tauri/tests/postgres_integration/parity.rs b/src-tauri/tests/postgres_integration/parity.rs index aff5283c2..e0d040795 100644 --- a/src-tauri/tests/postgres_integration/parity.rs +++ b/src-tauri/tests/postgres_integration/parity.rs @@ -307,6 +307,7 @@ fn plugin_manifest() -> PluginManifest { settings: vec![], ui_extensions: None, explain_parsers: None, + connection_fields: None, type_mappings: { let mut m = HashMap::new(); m.insert("DATETIME".to_string(), "TIMESTAMP".to_string()); diff --git a/src/components/modals/NewConnectionModal.tsx b/src/components/modals/NewConnectionModal.tsx index 6bb35dc76..f3fa207c8 100644 --- a/src/components/modals/NewConnectionModal.tsx +++ b/src/components/modals/NewConnectionModal.tsx @@ -41,6 +41,7 @@ import { SshConnectionsModal } from "./SshConnectionsModal"; import { K8sConnectionsModal } from "./K8sConnectionsModal"; import { Select } from "../ui/Select"; import { SlotAnchor } from "../ui/SlotAnchor"; +import type { PluginSecretFieldState } from "../../types/pluginSlots"; import { useAlert } from "../../hooks/useAlert"; import { useDrivers } from "../../hooks/useDrivers"; import { useSettings } from "../../hooks/useSettings"; @@ -186,6 +187,7 @@ interface SavedConnection { id: string; name: string; params: ConnectionParams; + plugin_secret_keys?: string[]; detect_json_in_text_columns?: boolean; appearance?: ConnectionAppearance; tag_ids?: string[]; @@ -407,6 +409,9 @@ export const NewConnectionModal = ({ const [detectJsonInTextColumns, setDetectJsonInTextColumns] = useState(false); const [passwordDirty, setPasswordDirty] = useState(false); const [sshPasswordDirty, setSshPasswordDirty] = useState(false); + const [pluginSecretChanges, setPluginSecretChanges] = useState< + Record + >({}); const [connectionString, setConnectionString] = useState(""); const [connectionStringError, setConnectionStringError] = useState< string | null @@ -718,6 +723,11 @@ export const NewConnectionModal = ({ // Flat single-database store (e.g. Meilisearch): no database to select or name. const singleDatabase = activeDriver?.capabilities?.single_database === true; + const hostField = activeDriver?.connection_fields?.host; + const portField = activeDriver?.connection_fields?.port; + const usernameField = activeDriver?.connection_fields?.username; + const passwordField = activeDriver?.connection_fields?.password; + const databaseField = activeDriver?.connection_fields?.database; // ── plugin slot: connection-modal.connection_content ── const slotRegistry = usePluginSlotRegistry(); @@ -762,6 +772,34 @@ export const NewConnectionModal = ({ setFormData((prev) => ({ ...prev, username: "", password: "" })); } }, []); + const setSecretField = useCallback((key: string, value: string) => { + const normalizedKey = key.trim(); + if (!normalizedKey) return; + setPluginSecretChanges((previous) => ({ + ...previous, + [normalizedKey]: value === "" ? null : value, + })); + }, []); + const secretFields = useMemo(() => { + const storedKeys = + initialConnection?.params.driver === driver + ? (initialConnection.plugin_secret_keys ?? []) + : []; + const keys = new Set([...storedKeys, ...Object.keys(pluginSecretChanges)]); + return Object.fromEntries( + Array.from(keys).map((key): [string, PluginSecretFieldState] => { + const pendingValue = pluginSecretChanges[key]; + return [ + key, + { + value: typeof pendingValue === "string" ? pendingValue : "", + hasStoredValue: storedKeys.includes(key), + dirty: Object.hasOwn(pluginSecretChanges, key), + }, + ]; + }), + ); + }, [driver, initialConnection, pluginSecretChanges]); const extraFieldsSlotContext = useMemo( () => ({ driver, @@ -769,6 +807,8 @@ export const NewConnectionModal = ({ setExtraField, credentialFieldsHidden, setCredentialFieldsHidden, + secretFields, + setSecretField, }), [ driver, @@ -776,6 +816,8 @@ export const NewConnectionModal = ({ setExtraField, credentialFieldsHidden, setCredentialFieldsHidden, + secretFields, + setSecretField, ], ); @@ -1786,6 +1828,10 @@ export const NewConnectionModal = ({ request: { params: { ...listParams }, connection_id: initialConnection?.id, + // Pending plugin secrets belong to the driver being edited; stored + // ones are resolved by the backend from `connection_id`. + plugin_secret_changes: + effectiveDriver === driver ? pluginSecretChanges : {}, }, }); if (!shouldApply()) return; @@ -1832,6 +1878,7 @@ export const NewConnectionModal = ({ setPasswordDirty(false); setSshPasswordDirty(false); setCredentialFieldsHiddenState(false); + setPluginSecretChanges({}); setDbSearchQuery(""); setConnectionString(""); setConnectionStringError(null); @@ -2007,6 +2054,7 @@ export const NewConnectionModal = ({ const handleDriverChange = (newDriver: string) => { setDriver(newDriver); setCredentialFieldsHiddenState(false); + setPluginSecretChanges({}); setFormData({ driver: newDriver, host: "", @@ -2176,6 +2224,7 @@ export const NewConnectionModal = ({ params: { ...testParams }, connection_id: initialConnection?.id, progress_id: progressId, + plugin_secret_changes: pluginSecretChanges, }, }); @@ -2347,6 +2396,7 @@ export const NewConnectionModal = ({ } else if ( !noConnectionRequired && !singleDatabase && + !databaseField?.hidden && !hasConnectionUri && (!formData.database || (typeof formData.database === "string" && !formData.database.trim())) @@ -2433,6 +2483,7 @@ export const NewConnectionModal = ({ id: initialConnection.id, name, params, + pluginSecretChanges, detectJsonInTextColumns: detectJsonInTextColumns ? true : null, environment: environment || null, }); @@ -2449,6 +2500,7 @@ export const NewConnectionModal = ({ }>("save_connection", { name, params, + pluginSecretChanges, detectJsonInTextColumns: detectJsonInTextColumns ? true : null, environment: environment || null, }); @@ -2648,6 +2700,7 @@ export const NewConnectionModal = ({ if (driverChanged) { setDriver(newDriver); setCredentialFieldsHiddenState(false); + setPluginSecretChanges({}); } setFormData((prev) => ({ @@ -2870,29 +2923,41 @@ export const NewConnectionModal = ({ )} - {!isUriPassthrough && ( + {!isUriPassthrough && + (!hostField?.hidden || !portField?.hidden) && (
- updateField("host", v)} - placeholder="localhost" - /> - - updateField("port", v === "" ? undefined : Number(v)) - } - type="number" - placeholder={driver === "mysql" ? "3306" : "5432"} - /> + {!hostField?.hidden && ( + updateField("host", v)} + placeholder={hostField?.placeholder ?? "localhost"} + /> + )} + {!portField?.hidden && ( + + updateField("port", v === "" ? undefined : Number(v)) + } + type="number" + placeholder={ + portField?.placeholder ?? + (driver === "mysql" ? "3306" : "5432") + } + /> + )}
)} @@ -2903,64 +2968,83 @@ export const NewConnectionModal = ({ className="flex flex-col gap-3" /> - {/* User + Password (a plugin may hide them via the extra_fields slot) */} - {!credentialFieldsHidden && ( + {/* User + Password (a plugin may hide them via the extra_fields + slot or per field through its manifest `connection_fields`) */} + {!credentialFieldsHidden && + (!passwordField?.hidden || + (!isUriPassthrough && !usernameField?.hidden)) && (
- {!isUriPassthrough && ( + {!isUriPassthrough && !usernameField?.hidden && ( updateField("username", v)} - placeholder={t("newConnection.usernamePlaceholder")} + placeholder={ + usernameField?.placeholder ?? + t("newConnection.usernamePlaceholder") + } + /> + )} + {!passwordField?.hidden && ( + { + setPasswordDirty(true); + updateField("password", v); + }} + type="password" + placeholder={ + initialConnection && !passwordDirty && !formData.password + ? "••••••••" + : passwordField?.placeholder ?? + t("newConnection.passwordPlaceholder") + } /> )} - { - setPasswordDirty(true); - updateField("password", v); - }} - type="password" - placeholder={ - initialConnection && !passwordDirty && !formData.password - ? "••••••••" - : t("newConnection.passwordPlaceholder") - } - />
)} {/* Database (single) — shown unless the connection has opted into - multi-database selection (flat drivers are always opted in) */} - {!isUriPassthrough && !isOptedInMultiDb && !singleDatabase && ( + multi-database selection (flat drivers are always opted in) or + the plugin hides it through its manifest `connection_fields` */} + {!isUriPassthrough && + !isOptedInMultiDb && + !singleDatabase && + !databaseField?.hidden && (
- + {!hostField?.hidden && ( + + )}
{availableDatabases.length > 0 ? ( + c.setSecretField?.("credential", event.target.value) + } + /> + + {JSON.stringify(state ?? null)} +
+ ); +} + +function renderModalWithSecretField(initialConnection?: InitialConnection) { + const registry = registryWith({ + pluginId: "warehouse", + slot: "connection-modal.extra_fields", + component: PluginSecretField, + }); + return render( + + + , + ); +} + +function invokeArgs(command: string): Record | undefined { + const call = vi + .mocked(invoke) + .mock.calls.find(([name]) => name === command); + return call?.[1] as Record | undefined; +} + +describe("NewConnectionModal extra_fields slot plugin secrets", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(invoke).mockImplementation(async (command) => + command === "list_databases" ? [] : "ok", + ); + sshMocks.loadSshConnections.mockResolvedValue([]); + k8sMocks.loadK8sConnections.mockResolvedValue([]); + eventMocks.listen.mockResolvedValue(eventMocks.unlisten); + }); + + function connectionWithStoredSecret(): InitialConnection { + const connection = createInitialConnection({ driver: "mysql" }); + connection.plugin_secret_keys = ["credential"]; + return connection; + } + + it("exposes a stored secret only as metadata", async () => { + renderModalWithSecretField(connectionWithStoredSecret()); + + const input = await screen.findByLabelText("plugin-credential"); + expect(input).toHaveValue(""); + expect(input).toHaveAttribute("placeholder", "stored-secret"); + expect(screen.getByTestId("plugin-secret-state")).toHaveTextContent( + '{"value":"","hasStoredValue":true,"dirty":false}', + ); + }); + + it("sends a replaced secret to Test Connection as a pending change", async () => { + renderModalWithSecretField(connectionWithStoredSecret()); + + fireEvent.change(await screen.findByLabelText("plugin-credential"), { + target: { value: "replacement" }, + }); + fireEvent.click(screen.getByText("newConnection.testConnection")); + + await waitFor(() => { + expect(invokeArgs("test_connection")).toBeDefined(); + }); + const request = invokeArgs("test_connection")?.request as Record< + string, + unknown + >; + expect(request.plugin_secret_changes).toEqual({ credential: "replacement" }); + expect( + (request.params as { extra?: Record }).extra?.credential, + ).toBeUndefined(); + }); + + it("clears a stored secret with an explicit null on save", async () => { + renderModalWithSecretField(connectionWithStoredSecret()); + + fireEvent.click(await screen.findByText("plugin-clear-credential")); + expect(screen.getByTestId("plugin-secret-state")).toHaveTextContent( + '{"value":"","hasStoredValue":true,"dirty":true}', + ); + fireEvent.click(screen.getByText("newConnection.save")); + + await waitFor(() => { + expect(invokeArgs("update_connection")).toBeDefined(); + }); + expect(invokeArgs("update_connection")?.pluginSecretChanges).toEqual({ + credential: null, + }); + }); + + it("sends no secret changes when the plugin leaves the field untouched", async () => { + renderModalWithSecretField(connectionWithStoredSecret()); + + await screen.findByLabelText("plugin-credential"); + fireEvent.click(screen.getByText("newConnection.save")); + + await waitFor(() => { + expect(invokeArgs("update_connection")).toBeDefined(); + }); + expect(invokeArgs("update_connection")?.pluginSecretChanges).toEqual({}); + }); +}); diff --git a/tests/utils/connectionFieldsSchema.test.ts b/tests/utils/connectionFieldsSchema.test.ts new file mode 100644 index 000000000..348499287 --- /dev/null +++ b/tests/utils/connectionFieldsSchema.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import manifestSchema from "../../plugins/manifest.schema.json"; +import extensionsSchema from "../../plugins/tabularium-extensions.schema.json"; + +const FIELDS = ["host", "port", "username", "password", "database"]; + +describe("connection_fields manifest contract", () => { + it("declares optional per-field overrides in the runtime manifest schema", () => { + const connectionFields = manifestSchema.properties.connection_fields; + expect(connectionFields.type).toBe("object"); + expect(connectionFields.additionalProperties).toBe(false); + const [pattern, override] = Object.entries(connectionFields.patternProperties)[0]; + for (const field of FIELDS) { + expect(new RegExp(pattern).test(field)).toBe(true); + } + expect(new RegExp(pattern).test("ssh_host")).toBe(false); + expect(Object.keys(override.properties)).toEqual(["hidden", "label", "placeholder"]); + expect(override.properties.hidden.default).toBe(false); + expect(manifestSchema.required).not.toContain("connection_fields"); + }); + + it("declares the same overrides in the Tabularium extension schema", () => { + const connectionFields = extensionsSchema.properties.connection_fields; + expect(connectionFields.additionalProperties).toBe(false); + expect(Object.keys(connectionFields.properties)).toEqual(FIELDS); + for (const field of FIELDS) { + const override = + connectionFields.properties[field as keyof typeof connectionFields.properties]; + expect(override.additionalProperties).toBe(false); + expect(Object.keys(override.properties)).toEqual(["hidden", "label", "placeholder"]); + } + }); +});