fix(datatables): register external fork catalogs under the lifecycle lock, and keep certificate verification in DuckDB attaches

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/windmill-api-workspaces/src/workspaces.rs

@@ -3372,7 +3372,7 @@ fn apply_pg_tls_env(
                 cmd.env("PGSSLROOTCERT", "system");
             }
             Some("verify-ca") => {
-                if let Some(bundle) = system_ca_bundle() {
+                if let Some(bundle) = windmill_common::system_ca_bundle() {
                     cmd.env("PGSSLROOTCERT", bundle);
                 }
             }
@@ -3382,21 +3382,6 @@ fn apply_pg_tls_env(
     Ok(None)
 }
 
-fn system_ca_bundle() -> Option<std::path::PathBuf> {
-    std::env::var_os("SSL_CERT_FILE")
-        .map(std::path::PathBuf::from)
-        .into_iter()
-        .chain(
-            [
-                "/etc/ssl/certs/ca-certificates.crt",
-                "/etc/pki/tls/certs/ca-bundle.crt",
-                "/etc/ssl/cert.pem",
-                "/etc/ssl/ca-bundle.pem",
-            ]
-            .map(std::path::PathBuf::from),
-        )
-        .find(|path| path.is_file())
-}
 
 #[cfg(test)]
 mod pg_tls_env_tests {

backend/windmill-common/src/lib.rs

@@ -1656,6 +1656,24 @@ pub async fn create_custom_instance_database(
     Ok(())
 }
 
+/// The system's CA bundle file, for libpq clients that cannot take `sslrootcert=system`: that value
+/// needs libpq 16, and verify-full only.
+pub fn system_ca_bundle() -> Option<std::path::PathBuf> {
+    std::env::var_os("SSL_CERT_FILE")
+        .map(std::path::PathBuf::from)
+        .into_iter()
+        .chain(
+            [
+                "/etc/ssl/certs/ca-certificates.crt",
+                "/etc/pki/tls/certs/ca-bundle.crt",
+                "/etc/ssl/cert.pem",
+                "/etc/ssl/ca-bundle.pem",
+            ]
+            .map(std::path::PathBuf::from),
+        )
+        .find(|path| path.is_file())
+}
+
 /// Connection options parsed from a database URL.
 ///
 /// The only place a database URL becomes `PgConnectOptions`. Providers that mint the password

backend/windmill-common/src/workspaces.rs

@@ -3017,6 +3017,14 @@ async fn register_fork_ducklake_namespace(
     {
         return Ok(());
     }
+    let mut tx = db.begin().await?;
+    // A row naming an external database counts as a use of it. Written under the lock a drop takes,
+    // and only while the database is still registered, so a drop cannot slip in between the
+    // settings this attach resolved and the row that protects the database.
+    if let Some(dbname) = catalog.strip_prefix("external_instance:") {
+        crate::external_instance_pg::ensure_external_instance_database_registered(&mut tx, dbname)
+            .await?;
+    }
     sqlx::query!(
         "INSERT INTO fork_ducklake_namespace
            (workspace_id, ducklake_name, metadata_schema, catalog, storage, storage_ref, data_path)
@@ -3031,9 +3039,10 @@ async fn register_fork_ducklake_namespace(
         &storage_ref,
         data_path,
     )
-    .execute(db)
+    .execute(&mut *tx)
     .await
     .map_err(|e| Error::internal_err(format!("registering fork ducklake namespace: {e:#}")))?;
+    tx.commit().await?;
     let mut locations = FORK_DUCKLAKE_REGISTERED
         .get(w_id)
         .filter(|(_, exp)| *exp > now)

backend/windmill-worker/src/duckdb_executor.rs

@@ -2240,11 +2240,54 @@ fn parse_attach_db_resource<'a>(query: &'a str) -> Option<ParsedAttachDbResource
     None
 }
 
+/// `to_uri` turns verify-ca and verify-full into `require`, which resources have always relied on.
+/// A connection that explicitly refuses invalid certificates — the external instance cluster's —
+/// keeps its mode instead: under `require` its shared password would go to whichever server answers.
+/// DuckDB's libpq takes one root file, so it gets the system bundle plus the configured certificate.
+fn pg_attach_uri(res: &PgDatabase) -> Result<String> {
+    let uri = res.to_uri();
+    let mode = match res.sslmode.as_deref() {
+        Some(mode @ ("verify-ca" | "verify-full")) if res.accept_invalid_certs == Some(false) => mode,
+        _ => return Ok(uri),
+    };
+    let base = uri.strip_suffix("?sslmode=require").ok_or_else(|| {
+        Error::internal_err("unexpected sslmode in a postgres connection URI".to_string())
+    })?;
+    let bundle = windmill_common::system_ca_bundle()
+        .map(std::fs::read_to_string)
+        .transpose()
+        .map_err(|e| Error::ExecutionErr(format!("Failed to read the system CA bundle: {e}")))?
+        .unwrap_or_default();
+    let pem = res.root_certificate_pem.as_deref().unwrap_or_default();
+    if bundle.is_empty() && pem.is_empty() {
+        return Err(Error::ExecutionErr(format!(
+            "sslmode {mode} needs a root certificate, and this worker has no system CA bundle"
+        )));
+    }
+    let roots = format!("{bundle}\n{pem}\n");
+    use sha2::Digest;
+    let path = std::env::temp_dir().join(format!(
+        "windmill-pg-roots-{}.pem",
+        hex::encode(&sha2::Sha256::digest(roots.as_bytes())[..8])
+    ));
+    if !path.is_file() {
+        // Renamed into place: a job attaching concurrently must never read a half-written file.
+        let partial = path.with_extension(format!("{}.partial", Uuid::new_v4()));
+        std::fs::write(&partial, &roots)
+            .and_then(|()| std::fs::rename(&partial, &path))
+            .map_err(|e| Error::ExecutionErr(format!("Failed to write root certificates: {e}")))?;
+    }
+    Ok(format!(
+        "{base}?sslmode={mode}&sslrootcert={}",
+        urlencoding::encode(&path.to_string_lossy())
+    ))
+}
+
 fn format_attach_db_conn_str(db_resource: Value, db_type: &str) -> Result<String> {
     let s = match db_type.to_lowercase().as_str() {
         "postgres" | "postgresql" => {
             let res: PgDatabase = serde_json::from_value(db_resource)?;
-            res.to_uri()
+            pg_attach_uri(&res)?
         }
         #[cfg(feature = "mysql")]
         "mysql" => {
@@ -2794,6 +2837,30 @@ pub struct Arg {
 mod tests {
     use super::*;
 
+    #[test]
+    fn pg_attach_keeps_verification_only_when_required() {
+        let pg = |sslmode: &str, accept_invalid_certs: Option<bool>| PgDatabase {
+            host: "db.internal".to_string(),
+            user: Some("custom_instance_user".to_string()),
+            password: Some("pw".to_string()),
+            port: None,
+            sslmode: Some(sslmode.to_string()),
+            dbname: "dt".to_string(),
+            root_certificate_pem: Some("-----BEGIN CERTIFICATE-----test".to_string()),
+            accept_invalid_certs,
+            use_iam_auth: None,
+            region: None,
+        };
+        let uri = pg_attach_uri(&pg("verify-full", Some(false))).unwrap();
+        assert!(uri.contains("?sslmode=verify-full&sslrootcert="), "{uri}");
+        let root = urlencoding::decode(uri.split("sslrootcert=").nth(1).unwrap()).unwrap();
+        let roots = std::fs::read_to_string(root.as_ref()).unwrap();
+        assert!(roots.contains("-----BEGIN CERTIFICATE-----test"));
+        // A resource that never opted in keeps the historical downgrade.
+        assert!(pg_attach_uri(&pg("verify-full", None)).unwrap().ends_with("?sslmode=require"));
+        assert!(pg_attach_uri(&pg("require", Some(false))).unwrap().ends_with("?sslmode=require"));
+    }
+
     #[test]
     fn attach_datatable_parses_name_and_role() {
         let reference_of = |q: &str| parse_attach_datatable(q).unwrap().reference;