feat(connections): remember database passwords in the OS secret store

crates/assistant/src/tests.rs

@@ -216,6 +216,46 @@ fn connection_commands_require_fresh_approval_before_login_or_scripts() {
     }
 }
 
+#[test]
+fn remembered_database_forget_requires_fresh_direct_and_script_approval() {
+    use rustgis_engine::data::secrets::{MemorySecrets, SecretStore};
+    use std::sync::Arc;
+    for scripted in [false, true] {
+        let store = Arc::new(MemorySecrets::new());
+        let mut s = Session::new(Project::default());
+        s.database_logins.set_store(Some(store.clone()));
+        s.run(
+            "connection.new",
+            &json!({"kind":"postgres","name":"Private database","address":"postgres:host=127.0.0.1 port=1 user=approval"}),
+        )
+        .unwrap();
+        s.run("connection.login", &json!({"id":"c1","password":"forgotten-sentinel","remember":true})).unwrap();
+        let args = if scripted {
+            json!({"script":"map.rename {\"name\":\"Approved map\"}\nconnection.forgetPassword {\"id\":\"c1\"}"})
+        } else {
+            json!({"command":"connection.forgetPassword","params":{"id":"c1"}})
+        };
+        let tool = if scripted { "run" } else { "run_command" };
+        let mut a = Agent::new(cfg("openai"), &s);
+        assert!(matches!(a.ask("forget this database's saved password", &s), Step::Send(_)));
+        let name = s.map().name.clone();
+        assert_eq!(a.receive(a.generation(), ok(call("forget", tool, args.clone())), &mut s), Step::Confirm);
+        assert!(!a.pending(&s).unwrap().2);
+        assert!(store.load("connection/c1").unwrap().is_some());
+        assert_eq!(s.map().name, name);
+        a.decide(false, false, &mut s);
+        assert!(store.load("connection/c1").unwrap().is_some());
+        assert_eq!(a.receive(a.generation(), ok(call("allow", tool, args.clone())), &mut s), Step::Confirm);
+        let sent = a.decide(true, true, &mut s);
+        assert!(matches!(sent, Step::Send(_)));
+        assert!(!format!("{sent:?}").contains("forgotten-sentinel"));
+        assert_eq!(store.load("connection/c1").unwrap(), None);
+        assert!(!s.connections.find("c1").unwrap().remembered);
+        assert!(s.database_logins.has_session_password("c1"));
+        assert_eq!(a.receive(a.generation(), ok(call("again", tool, args)), &mut s), Step::Confirm);
+    }
+}
+
 #[test]
 fn gate_rules() {
     let s = sample();

crates/data/src/lib.rs

@@ -60,6 +60,83 @@ pub enum DataError {
 
 pub type Result<T> = std::result::Result<T, DataError>;
 
+/// Replaces `secret` in `message` with asterisks; the text itself when it holds none.
+fn without_secret(message: String, secret: &str) -> String {
+    if message.contains(secret) { message.replace(secret, "********") } else { message }
+}
+
+/// Whether an I/O error's message holds `secret`. Only one carrying text from elsewhere (a
+/// driver's, a server's) can: the system's own errors are fixed text and are not even formatted,
+/// so the common case allocates nothing.
+fn io_mentions(e: &std::io::Error, secret: &str) -> bool {
+    e.get_ref().is_some() && e.to_string().contains(secret)
+}
+
+impl DataError {
+    /// The error with `secret` (a password the driver sent) taken out of its message, whatever a
+    /// server or driver echoes, keeping the kind: a refused login stays [`DataError::Login`].
+    /// An empty secret leaves the error as it is, and so does text without it (nothing is copied).
+    pub fn redact_secret(self, secret: &str) -> Self {
+        if secret.is_empty() {
+            return self;
+        }
+        match self {
+            DataError::Format(m) => DataError::Format(without_secret(m, secret)),
+            DataError::Login(m) => DataError::Login(without_secret(m, secret)),
+            DataError::Unsupported(m) => DataError::Unsupported(without_secret(m, secret)),
+            DataError::NoChunks(m) => DataError::NoChunks(without_secret(m, secret)),
+            DataError::Io(e) if io_mentions(&e, secret) => {
+                DataError::Io(std::io::Error::new(e.kind(), without_secret(e.to_string(), secret)))
+            }
+            DataError::Io(e) => DataError::Io(e),
+        }
+    }
+}
+
+#[cfg(test)]
+#[allow(clippy::unwrap_used)]
+mod secret_redaction_tests {
+    use super::*;
+
+    #[test]
+    fn remembered_database_redaction_preserves_classification_and_owned_text() {
+        let message = String::from("no credential here");
+        let pointer = message.as_ptr();
+        let DataError::Login(unchanged) = DataError::Login(message).redact_secret("sentinel") else { panic!("lost Login") };
+        assert_eq!(unchanged.as_ptr(), pointer);
+        let e = DataError::Login("server echoed sentinel twice sentinel".into()).redact_secret("sentinel");
+        assert!(matches!(e, DataError::Login(_)));
+        assert_eq!(e.to_string(), "server echoed ******** twice ********");
+        let e = DataError::Format("sentinel".into()).redact_secret("sentinel");
+        assert!(matches!(e, DataError::Format(_)));
+        assert_eq!(DataError::Login("unchanged".into()).redact_secret("").to_string(), "unchanged");
+    }
+
+    #[derive(Debug)]
+    struct Pieces;
+    impl std::fmt::Display for Pieces {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            f.write_str("error: se")?;
+            f.write_str("nti")?;
+            f.write_str("nel echoed")
+        }
+    }
+    impl std::error::Error for Pieces {}
+
+    #[test]
+    fn remembered_database_redaction_handles_fragmented_io_and_preserves_kind() {
+        let e = DataError::Io(std::io::Error::new(std::io::ErrorKind::PermissionDenied, Pieces)).redact_secret("sentinel");
+        let DataError::Io(e) = e else { panic!("lost Io") };
+        assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied);
+        assert_eq!(e.to_string(), "error: ******** echoed");
+        let long = "x".repeat(300);
+        let e = DataError::Io(std::io::Error::other(format!("before {long} after"))).redact_secret(&long);
+        assert_eq!(e.to_string(), "i/o: before ******** after");
+        let e = DataError::Io(std::io::Error::other("unchanged")).redact_secret(&long);
+        assert_eq!(e.to_string(), "i/o: unchanged");
+    }
+}
+
 /// One row: an optional geometry and one value per field.
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct Feature {

crates/data/src/source/duckdb.rs

@@ -15,6 +15,7 @@
 
 use super::Family;
 use super::external::{self, TempFile, fnv1a, output, stamp, touch};
+use crate::secrets::Secret;
 use crate::{DataError, Result};
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -128,33 +129,99 @@ fn split_password(dsn: &str) -> (String, Option<String>) {
 
 pub use super::pgconn::public_label;
 
-fn passwords() -> &'static std::sync::Mutex<std::collections::HashMap<String, String>> {
-    static P: std::sync::OnceLock<std::sync::Mutex<std::collections::HashMap<String, String>>> = std::sync::OnceLock::new();
-    P.get_or_init(Default::default)
+/// A password remembered for a connection's address. `connection_id` names the saved connection
+/// it was activated for (see [`set_connection_password`]); `None` is one typed for the address.
+struct Entry {
+    password: Secret,
+    connection_id: Option<String>,
 }
 
-/// Remembers a connection's password for this session (never saved: projects name connections
-/// without them, and libpq's `PGPASSWORD` or `~/.pgpass` serve when it isn't remembered).
+type Passwords = std::collections::HashMap<String, Entry>;
+
+static PASSWORDS: std::sync::LazyLock<std::sync::Mutex<Passwords>> = std::sync::LazyLock::new(Default::default);
+
+fn lock() -> std::sync::MutexGuard<'static, Passwords> {
+    PASSWORDS.lock().unwrap_or_else(|e| e.into_inner())
+}
+
+/// Remembers a connection's password for this session, as typed for its address (the saved
+/// connections' own are activated by [`set_connection_password`]): projects name connections
+/// without them, and libpq's `PGPASSWORD` or `~/.pgpass` serve when it isn't remembered.
 pub fn remember_password(dsn: &str, password: &str) {
-    passwords().lock().unwrap_or_else(|e| e.into_inner()).insert(dsn.to_string(), password.to_string());
+    lock().insert(dsn.to_string(), Entry { password: Secret::new(password), connection_id: None });
+}
+
+/// Makes saved connection `id`'s password the one connections to `dsn` use (`Some`), replacing
+/// whatever was remembered for the address; or takes away a password a saved connection
+/// activated there, whichever one (`None`), leaving one typed for the address.
+pub fn set_connection_password(dsn: &str, id: &str, password: Option<&str>) {
+    let mut all = lock();
+    match password {
+        Some(p) => {
+            all.insert(dsn.to_string(), Entry { password: Secret::new(p), connection_id: Some(id.to_string()) });
+        }
+        None => {
+            if all.get(dsn).is_some_and(|e| e.connection_id.is_some()) {
+                all.remove(dsn);
+            }
+        }
+    }
+}
+
+/// Whether this driver's exact address slot is currently owned by saved connection `id`.
+/// Unlike runtime selection, this also accounts for a later explicitly typed address password.
+pub fn has_connection_password(dsn: &str, id: &str) -> bool {
+    lock().get(dsn).is_some_and(|entry| entry.connection_id.as_deref() == Some(id))
+}
+
+/// Tries an explicitly supplied password, restoring the previous address slot and its ownership
+/// when `refused` classifies the outcome as an authentication refusal. Other failures retain
+/// the new password. Never holds the cache lock during network I/O.
+pub fn try_connection_password<T>(dsn: &str, id: &str, password: &str, run: impl FnOnce() -> T, refused: impl FnOnce(&T) -> bool) -> T {
+    let previous = lock().insert(dsn.to_string(), Entry { password: Secret::new(password), connection_id: Some(id.to_string()) });
+    let result = run();
+    if refused(&result) {
+        let mut all = lock();
+        if let Some(previous) = previous {
+            all.insert(dsn.to_string(), previous);
+        } else {
+            all.remove(dsn);
+        }
+    }
+    result
+}
+
+/// Takes away every password saved connection `id` activated.
+pub fn forget_connection_password(id: &str) {
+    lock().retain(|_, e| e.connection_id.as_deref() != Some(id));
+}
+
+/// Takes away every password a saved connection activated, leaving the typed ones.
+pub fn forget_connection_passwords() {
+    lock().retain(|_, e| e.connection_id.is_none());
 }
 
 /// The password remembered for a connection this session, if any.
 pub(crate) fn remembered_password(dsn: &str) -> Option<String> {
-    passwords().lock().unwrap_or_else(|e| e.into_inner()).get(dsn).cloned()
+    lock().get(dsn).map(|e| e.password.expose().to_string())
 }
 
-/// Whether a password is remembered for a connection this session.
+/// Whether a password is remembered for a connection this session, typed or activated.
 pub fn has_password(dsn: &str) -> bool {
-    passwords().lock().unwrap_or_else(|e| e.into_inner()).contains_key(dsn)
+    lock().contains_key(dsn)
+}
+
+/// Whether a password typed for this address (not one a saved connection activated) is remembered.
+pub fn has_explicit_password(dsn: &str) -> bool {
+    lock().get(dsn).is_some_and(|e| e.connection_id.is_none())
 }
 
 /// The remembered password of a query's database, handed to DuckDB's PostgreSQL client in its
 /// environment (`PGPASSWORD`): never in the script, where the process list and DuckDB's errors
 /// would show it.
 fn password(database: Option<&Database>) -> Option<String> {
     match database? {
-        Database::Postgres(dsn) => passwords().lock().unwrap_or_else(|e| e.into_inner()).get(dsn).cloned(),
+        Database::Postgres(dsn) => remembered_password(dsn),
         Database::File(_) | Database::Mysql(_) | Database::Sqlserver(_) => None,
     }
 }
@@ -798,6 +865,30 @@ fn ours(name: &str) -> bool {
 mod tests {
     use super::*;
 
+    #[test]
+    fn remembered_database_cache_ownership_and_explicit_preservation() {
+        let a = "host=remembered_cache_a user=one";
+        let b = "mysql://remembered_cache_b/db";
+        set_connection_password(a, "cache-c1", Some("one"));
+        assert!(!has_explicit_password(a));
+        assert_eq!(remembered_password(a).as_deref(), Some("one"));
+        set_connection_password(a, "cache-c2", Some("two"));
+        forget_connection_password("cache-c1");
+        assert_eq!(remembered_password(a).as_deref(), Some("two"));
+        set_connection_password(a, "cache-c1", None);
+        assert!(!has_password(a));
+        remember_password(a, "explicit");
+        set_connection_password(a, "cache-c2", None);
+        assert!(has_explicit_password(a));
+        assert_eq!(remembered_password(a).as_deref(), Some("explicit"));
+        set_connection_password(b, "cache-c2", Some("owned"));
+        forget_connection_passwords();
+        assert!(!has_password(b));
+        assert_eq!(remembered_password(a).as_deref(), Some("explicit"));
+        assert_eq!(password(Some(&Database::Postgres(a.into()))).as_deref(), Some("explicit"));
+        lock().remove(a);
+    }
+
     #[test]
     fn databases_by_name() {
         // Quoted passwords with spaces, spaced `=`, escapes; an `@` past a URI's authority.

crates/data/src/source/mysql.rs

@@ -270,7 +270,7 @@ impl Conn {
                     if broken(&e) {
                         *guard = None;
                     }
-                    return Err(err(e));
+                    return Err(err(e).redact_secret(self.password.as_deref().unwrap_or("")));
                 }
             }
         }
@@ -282,6 +282,10 @@ impl Conn {
 /// offers none, said in the second value), and sets the session read only with a statement
 /// timeout.
 fn open_client(config: &Config, password: Option<&str>) -> Result<((mysql::Conn, Server), bool)> {
+    connect_client(config, password).map_err(|e| e.redact_secret(password.unwrap_or("")))
+}
+
+fn connect_client(config: &Config, password: Option<&str>) -> Result<((mysql::Conn, Server), bool)> {
     super::tls_provider();
     let base = mysql::OptsBuilder::new()
         .ip_or_hostname(Some(config.host.clone()))

crates/data/src/source/postgis.rs

@@ -168,11 +168,11 @@ impl Conn {
             let client = guard.as_mut().ok_or_else(|| DataError::Format("not connected".into()))?;
             let r = f(client);
             if !client.is_closed() {
-                return r.map_err(err);
+                return r.map_err(|e| redacted(&self.config, err(e)));
             }
             *guard = None;
             if attempt == 1 || r.is_ok() {
-                return r.map_err(err);
+                return r.map_err(|e| redacted(&self.config, err(e)));
             }
         }
         Err(DataError::Format("not connected".into()))
@@ -197,7 +197,19 @@ impl Conn {
     }
 }
 
+/// Connects, with the password taken out of any error: a server or the client may echo what it was given.
 fn open_client(config: &postgres::Config, tls: &Tls) -> Result<postgres::Client> {
+    connect_client(config, tls).map_err(|e| redacted(config, e))
+}
+
+fn redacted(config: &postgres::Config, error: DataError) -> DataError {
+    match config.get_password().and_then(|p| std::str::from_utf8(p).ok()) {
+        Some(password) => error.redact_secret(password),
+        None => error,
+    }
+}
+
+fn connect_client(config: &postgres::Config, tls: &Tls) -> Result<postgres::Client> {
     let mut client = match tls {
         Tls::Disable => config.connect(postgres::NoTls),
         Tls::Unverified | Tls::Verified(_) => config.connect(tokio_postgres_rustls::MakeRustlsConnect::new(rustls_config(tls)?)),

crates/data/src/source/sqlserver.rs

@@ -260,7 +260,7 @@ fn open_on(rt: &tokio::runtime::Runtime, config: &Config, password: Option<&str>
         .await
     })
     .map_err(|_| DataError::Format(format!("couldn't connect within {} s", config.connect_timeout.as_secs())))?
-    .map_err(err)
+    .map_err(|e| err(e).redact_secret(password.unwrap_or("")))
 }
 
 fn connect_blocking(config: &Config, password: Option<&str>) -> Result<(tokio::runtime::Runtime, Client, u32, String, u32)> {
@@ -300,7 +300,7 @@ impl Conn {
                     if broken(&e) {
                         *guard = None;
                     }
-                    return Err(err(e));
+                    return Err(err(e).redact_secret(self.password.as_deref().unwrap_or("")));
                 }
             }
         }