harden: malformed-input hardening, non-panicking encoders, unit coverage

src/client/auth.rs

@@ -30,7 +30,7 @@ impl Debug for SqlServerAuth {
 )]
 pub struct WindowsAuth {
     pub(crate) user: String,
-    pub(crate) password: String,
+    pub(crate) password: Zeroizing<String>,
     pub(crate) domain: Option<String>,
 }
 
@@ -50,7 +50,7 @@ impl Debug for WindowsAuth {
 }
 
 /// Defines the method of authentication to the server.
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 pub enum AuthMethod {
     /// Authenticate directly with SQL Server.
     SqlServer(SqlServerAuth),
@@ -82,6 +82,26 @@ pub enum AuthMethod {
     None,
 }
 
+// Manual Debug so the AAD bearer token is never printed. The credential-bearing
+// SqlServer/Windows variants delegate to their inner types, which already redact.
+impl Debug for AuthMethod {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::SqlServer(a) => f.debug_tuple("SqlServer").field(a).finish(),
+            #[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"), doc))]
+            Self::Windows(a) => f.debug_tuple("Windows").field(a).finish(),
+            #[cfg(any(
+                all(windows, feature = "winauth"),
+                all(unix, feature = "integrated-auth-gssapi"),
+                doc
+            ))]
+            Self::Integrated => f.write_str("Integrated"),
+            Self::AADToken(_) => f.debug_tuple("AADToken").field(&"<HIDDEN>").finish(),
+            Self::None => f.write_str("None"),
+        }
+    }
+}
+
 impl AuthMethod {
     /// Construct a new SQL Server authentication configuration.
     pub fn sql_server(user: impl ToString, password: impl ToString) -> Self {
@@ -105,7 +125,7 @@ impl AuthMethod {
 
         Self::Windows(WindowsAuth {
             user: user.to_string(),
-            password: password.to_string(),
+            password: Zeroizing::new(password.to_string()),
             domain: domain.map(|s| s.to_string()),
         })
     }
@@ -136,4 +156,46 @@ mod tests {
 
         assert!(password.is_empty());
     }
+
+    #[test]
+    fn debug_redacts_credentials() {
+        let sql = format!("{:?}", AuthMethod::sql_server("sa", "sql-secret"));
+        assert!(!sql.contains("sql-secret"), "SQL password leaked: {sql}");
+
+        let aad = format!("{:?}", AuthMethod::aad_token("aad-secret-token"));
+        assert!(!aad.contains("aad-secret-token"), "AAD token leaked: {aad}");
+        assert!(aad.contains("HIDDEN"));
+    }
+
+    #[test]
+    fn debug_none_variant() {
+        assert_eq!(format!("{:?}", AuthMethod::None), "None");
+    }
+
+    #[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))]
+    #[test]
+    fn windows_auth_parses_domain_and_debug_redacts() {
+        // `DOMAIN\user` form exercises the domain-splitting branch of `windows()`.
+        let auth = AuthMethod::windows("DOMAIN\\user", "win-secret");
+        let dbg = format!("{:?}", auth);
+        assert!(dbg.contains("Windows"), "variant name missing: {dbg}");
+        assert!(dbg.contains("DOMAIN"), "domain not preserved: {dbg}");
+        assert!(dbg.contains("user"), "user not preserved: {dbg}");
+        assert!(!dbg.contains("win-secret"), "password leaked: {dbg}");
+
+        // No backslash exercises the domain-less branch.
+        let plain = AuthMethod::windows("plainuser", "pw");
+        let dbg = format!("{:?}", plain);
+        assert!(dbg.contains("plainuser"), "user not preserved: {dbg}");
+        assert!(dbg.contains("None"), "domain should be None: {dbg}");
+    }
+
+    #[cfg(any(
+        all(windows, feature = "winauth"),
+        all(unix, feature = "integrated-auth-gssapi")
+    ))]
+    #[test]
+    fn integrated_debug() {
+        assert_eq!(format!("{:?}", AuthMethod::Integrated), "Integrated");
+    }
 }

src/client/config.rs

@@ -866,9 +866,17 @@ pub(crate) trait ConfigString {
                 Err(_) if val.eq_ignore_ascii_case("strict") && cfg!(feature = "tds80") => {
                     Ok(EncryptionLevel::Strict)
                 }
+                Err(_) if val.eq_ignore_ascii_case("strict") => Err(crate::Error::Conversion(
+                    "encrypt=strict requires the crate's `tds80` feature to be enabled".into(),
+                )),
                 Err(e) => Err(e),
             })
-            .unwrap_or(Ok(EncryptionLevel::Off))
+            // When the `encrypt` keyword is omitted, default to requiring
+            // encryption — matching `Config::default()` and modern ADO.NET
+            // (`Encrypt=Mandatory`). Callers who want an unencrypted connection
+            // must opt out explicitly with `encrypt=false` (or
+            // `encrypt=DANGER_PLAINTEXT`).
+            .unwrap_or(Ok(EncryptionLevel::Required))
     }
 
     #[cfg(not(any(
@@ -940,6 +948,46 @@ mod tests {
         assert_eq!(Some("master"), config.database.as_deref());
     }
 
+    #[test]
+    fn config_from_builder_carries_builder_settings() {
+        // `From<ConfigBuilder>` must return the built inner config, not a default.
+        let config: Config = Config::builder().host("db.internal").port(2020).into();
+        assert_eq!("db.internal", config.get_host());
+        assert_eq!(2020, config.get_port());
+    }
+
+    #[test]
+    fn get_packet_size_reflects_the_set_value() {
+        let mut config = Config::new();
+        assert_eq!(config.get_packet_size(), None);
+        config.packet_size(8192);
+        assert_eq!(config.get_packet_size(), Some(8192));
+    }
+
+    #[test]
+    fn from_jdbc_string_parses_host_and_port() {
+        let config =
+            Config::from_jdbc_string("jdbc:sqlserver://db.example.com:2345").expect("valid jdbc");
+        assert_eq!("db.example.com", config.get_host());
+        assert_eq!(2345, config.get_port());
+    }
+
+    #[cfg(any(
+        feature = "rustls",
+        feature = "native-tls",
+        feature = "vendored-openssl"
+    ))]
+    #[test]
+    fn get_hostname_in_certificate_falls_back_to_host() {
+        let mut config = Config::new();
+        config.host("real.host");
+        // Unset: falls back to the connection host.
+        assert_eq!(config.get_hostname_in_certificate(), "real.host");
+        // Set: returns the explicit certificate hostname.
+        config.hostname_in_certificate("cert.host");
+        assert_eq!(config.get_hostname_in_certificate(), "cert.host");
+    }
+
     #[cfg(any(
         feature = "rustls",
         feature = "native-tls",
@@ -1037,4 +1085,160 @@ mod tests {
             other => panic!("expected Windows NTLM auth, got {other:?}"),
         }
     }
+
+    #[test]
+    fn config_direct_setters_populate_fields() {
+        let mut config = Config::new();
+        config.database("northwind");
+        config.instance_name("SQLEXPRESS");
+        config.client_name("workstation-7");
+
+        assert_eq!(Some("northwind"), config.database.as_deref());
+        assert_eq!(Some("SQLEXPRESS"), config.instance_name.as_deref());
+        assert_eq!(Some("workstation-7"), config.client_name.as_deref());
+    }
+
+    #[test]
+    fn get_port_defaults_without_port_or_instance() {
+        // No explicit port and no instance -> default SQL Server port.
+        let config = Config::new();
+        assert_eq!(1433, config.get_port());
+    }
+
+    #[test]
+    fn get_port_uses_sql_browser_port_for_named_instance() {
+        // A named instance without an explicit port -> SQL Browser port.
+        let mut config = Config::new();
+        config.instance_name("SQLEXPRESS");
+        assert_eq!(1434, config.get_port());
+    }
+
+    #[test]
+    #[should_panic(expected = "mutual exclusive")]
+    fn trust_cert_after_trust_cert_ca_panics() {
+        let mut config = Config::new();
+        config.trust_cert_ca("/tmp/ca.crt");
+        config.trust_cert();
+    }
+
+    #[test]
+    #[should_panic(expected = "mutual exclusive")]
+    fn trust_cert_ca_after_trust_cert_panics() {
+        let mut config = Config::new();
+        config.trust_cert();
+        config.trust_cert_ca("/tmp/ca.crt");
+    }
+
+    #[test]
+    fn trust_cert_ca_sets_ca_location() {
+        let mut config = Config::new();
+        config.trust_cert_ca("/tmp/ca.crt");
+        assert!(matches!(
+            config.trust,
+            TrustConfig::CaCertificateLocation(_)
+        ));
+    }
+
+    #[test]
+    fn config_builder_covers_all_setters() {
+        let config = Config::builder()
+            .host("localhost")
+            .instance_name("SQLEXPRESS")
+            .encryption(EncryptionLevel::Off)
+            .trust_cert_ca("/tmp/ca.crt")
+            .build();
+
+        assert_eq!(Some("SQLEXPRESS"), config.instance_name.as_deref());
+        assert!(matches!(config.encryption, EncryptionLevel::Off));
+        assert!(matches!(
+            config.trust,
+            TrustConfig::CaCertificateLocation(_)
+        ));
+    }
+
+    #[test]
+    fn config_builder_trust_cert_sets_trust_all() {
+        let config = Config::builder().trust_cert().build();
+        assert!(matches!(config.trust, TrustConfig::TrustAll));
+    }
+
+    #[test]
+    #[should_panic(expected = "mutual exclusive")]
+    fn config_builder_trust_cert_after_ca_panics() {
+        Config::builder().trust_cert_ca("/tmp/ca.crt").trust_cert();
+    }
+
+    #[test]
+    #[should_panic(expected = "mutual exclusive")]
+    fn config_builder_trust_cert_ca_after_trust_cert_panics() {
+        Config::builder().trust_cert().trust_cert_ca("/tmp/ca.crt");
+    }
+
+    #[test]
+    fn from_ado_string_populates_optional_fields() {
+        let config = Config::from_ado_string(
+            "server=tcp:my-server.com\\SQLEXPRESS;database=northwind;\
+             HostNameInCertificate=cert.host;WorkstationID=ws-1",
+        )
+        .expect("valid ado string");
+
+        assert_eq!("my-server.com", config.get_host());
+        assert_eq!(Some("SQLEXPRESS"), config.instance_name.as_deref());
+        assert_eq!(Some("northwind"), config.database.as_deref());
+        assert_eq!(Some("cert.host"), config.hostname_in_certificate.as_deref());
+        assert_eq!(Some("ws-1"), config.client_name.as_deref());
+    }
+
+    #[cfg(any(
+        feature = "rustls",
+        feature = "native-tls",
+        feature = "vendored-openssl"
+    ))]
+    #[test]
+    fn client_cert_source_debug_formats_cert_and_key() {
+        let mut config = Config::new();
+        config.client_certificate("/tmp/client.pem", "/tmp/client.key");
+
+        let dbg = format!("{:?}", config.get_client_certificate().unwrap().source);
+        assert!(dbg.contains("CertAndKey"));
+        assert!(dbg.contains("client.pem"));
+        assert!(dbg.contains("client.key"));
+    }
+
+    #[cfg(any(feature = "native-tls", feature = "vendored-openssl"))]
+    #[test]
+    fn config_builder_sets_pkcs12_client_certificate() {
+        let config = Config::builder()
+            .client_certificate_pkcs12("/tmp/identity.pfx", "s3cr3t")
+            .build();
+
+        match &config
+            .get_client_certificate()
+            .expect("client certificate should be set")
+            .source
+        {
+            ClientCertSource::Pkcs12 { path, password } => {
+                assert_eq!(path, &PathBuf::from("/tmp/identity.pfx"));
+                assert_eq!(password.as_str(), "s3cr3t");
+            }
+            other => panic!("expected Pkcs12 source, got {other:?}"),
+        }
+    }
+
+    #[cfg(all(unix, feature = "sspi-rs"))]
+    #[test]
+    fn ado_integrated_security_sspi_with_partial_credentials_uses_windows() {
+        // Only a username (no password) -> falls into the catch-all NTLM arm.
+        let config = Config::from_ado_string(
+            "server=tcp:localhost,1433;IntegratedSecurity=SSPI;uid=onlyuser",
+        )
+        .unwrap();
+
+        match config.auth {
+            AuthMethod::Windows(auth) => {
+                assert_eq!("onlyuser", auth.user);
+            }
+            other => panic!("expected Windows auth, got {other:?}"),
+        }
+    }
 }

src/client/config/ado_net.rs

@@ -230,6 +230,39 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn server_parsing_too_many_parts_is_error() -> crate::Result<()> {
+        // The Server value must have at most two comma-separated parts
+        // (host[,port]). Three parts is invalid and must error. The guard is
+        // `parts.is_empty() || parts.len() >= 3`; a `&&` mutation would never
+        // trigger (a slice cannot be both empty and have >= 3 parts), so this
+        // three-part value would be wrongly accepted.
+        let ado: AdoNetConfig = "server=tcp:my-server.com,1433,extra".parse()?;
+        assert!(ado.server().is_err());
+
+        let ado: AdoNetConfig = "server=my-server.com,1433,extra".parse()?;
+        assert!(ado.server().is_err());
+
+        Ok(())
+    }
+
+    #[test]
+    fn server_parsing_missing_key() -> crate::Result<()> {
+        // No `server`/`data source` key at all -> an all-`None` definition.
+        let ado: AdoNetConfig = "database=Foo".parse()?;
+        let server = ado.server()?;
+
+        assert_eq!(None, server.host);
+        assert_eq!(None, server.port);
+        assert_eq!(None, server.instance);
+
+        // And the same path through the public constructor.
+        let config = crate::Config::from_ado_string("database=Foo")?;
+        assert_eq!("localhost", config.get_host());
+
+        Ok(())
+    }
+
     #[test]
     fn database_parsing() -> crate::Result<()> {
         let test_str = "database=Foo";
@@ -465,7 +498,7 @@ mod tests {
         let test_str = "";
         let ado: AdoNetConfig = test_str.parse()?;
 
-        assert_eq!(EncryptionLevel::Off, ado.encrypt()?);
+        assert_eq!(EncryptionLevel::Required, ado.encrypt()?);
 
         Ok(())
     }

src/client/config/jdbc.rs

@@ -314,7 +314,7 @@ mod tests {
         let test_str = "jdbc:sqlserver://my-server.com:4200;";
         let jdbc: JdbcConfig = test_str.parse()?;
 
-        assert_eq!(EncryptionLevel::Off, jdbc.encrypt()?);
+        assert_eq!(EncryptionLevel::Required, jdbc.encrypt()?);
 
         Ok(())
     }