diff --git a/docs/src/api/builders.md b/docs/src/api/builders.md index 551f7497..472e3dcd 100644 --- a/docs/src/api/builders.md +++ b/docs/src/api/builders.md @@ -130,11 +130,11 @@ The `build()` method validates all fields and returns `Result nmrs::Result<()> To persist a profile without bringing it up immediately — the workflow from [#463](https://github.com/freedesktop-rs/nmrs/issues/463): ```rust -use nmrs::builders::build_wifi_connection; +use nmrs::builders::try_build_wifi_connection; use nmrs::{ConnectionOptions, NetworkManager, WifiSecurity}; let nm = NetworkManager::new().await?; -let settings = build_wifi_connection( +let settings = try_build_wifi_connection( "GuestWiFi", &WifiSecurity::WpaPsk { psk: "password".into() }, &ConnectionOptions::new(true), diff --git a/docs/src/api/network-manager.md b/docs/src/api/network-manager.md index 07fc27f9..707a9ee6 100644 --- a/docs/src/api/network-manager.md +++ b/docs/src/api/network-manager.md @@ -24,11 +24,11 @@ let config = nm.timeout_config(); ## Saving Profiles Without Activating ```rust -use nmrs::builders::build_wifi_connection; +use nmrs::builders::try_build_wifi_connection; use nmrs::{ConnectionOptions, NetworkManager, WifiSecurity}; let nm = NetworkManager::new().await?; -let settings = build_wifi_connection( +let settings = try_build_wifi_connection( "GuestWiFi", &WifiSecurity::WpaPsk { psk: "password".into() }, &ConnectionOptions::new(true), diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index ddbf7b9d..9daf30ab 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -4,15 +4,27 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] -### Changed +### Added -- **Breaking:** WPA-EAP connection builders and `build_wifi_connection()` now - return `Result`, reporting conflicting certificate path/blob inputs as - `ConnectionError::InvalidInput` instead of panicking. +- `try_build_wifi_connection()`, `WifiConnectionBuilder::try_wpa_eap()`, and + `WifiConnectionBuilder::try_wpa3_eap_192_bit()` report conflicting EAP + certificate path/blob inputs as `ConnectionError::InvalidInput`. ([#478](https://github.com/freedesktop-rs/nmrs/issues/478)) +### Deprecated + +- `build_wifi_connection()`, `WifiConnectionBuilder::wpa_eap()`, and + `WifiConnectionBuilder::wpa3_eap_192_bit()` in favour of their `try_` + counterparts. They keep their existing infallible signatures, so no + existing code breaks. ([#478](https://github.com/freedesktop-rs/nmrs/issues/478)) + ### Fixed +- Supplying an EAP certificate or key as both a path and a blob no longer + panics. The `try_` builders return `ConnectionError::InvalidInput`; the + deprecated infallible builders use the path and log a warning. + ([#478](https://github.com/freedesktop-rs/nmrs/issues/478)) + - `get_vpn_info()` no longer surfaces a raw D-Bus error (e.g. "a VPN interface does not exist") when a VPN is disconnected externally while it is being read. The active connection and its device can vanish mid-read; missing-object @@ -178,12 +190,7 @@ All notable changes to the `nmrs` crate will be documented in this file. ### Fixed - Add `process` feature to tokio to fix build error on some systems - -(No changes documented) - -## [3.1.2] - 2026-05-14 - -- `set_bluetooth_radio_enabled` now toggles kernel rfkill before BlueZ adapter `Powered`, fixing airplane-mode state desync with rfkill-based consumers ([#417](https://github.com/freedesktop-rs/nmrs/issues/418)) +- `set_bluetooth_radio_enabled` now toggles kernel rfkill before BlueZ adapter `Powered`, fixing airplane-mode state desync with rfkill-based consumers ([#418](https://github.com/freedesktop-rs/nmrs/issues/418)) ## [3.1.1] - 2026-05-13 @@ -297,6 +304,7 @@ present)` constructor; `RadioState::new` keeps existing behavior and defaults - Concurrency protection ([#268](https://github.com/freedesktop-rs/nmrs/pull/268)) - Expose `WirelessHardwareEnabled` in API to reflect rkfill state ([#284](https://github.com/freedesktop-rs/nmrs/pull/284)) +- `#[must_use]` attributes across public API: constructors, builder methods, and pure functions ([#220](https://github.com/freedesktop-rs/nmrs/issues/220)) ### Changed @@ -306,12 +314,6 @@ present)` constructor; `RadioState::new` keeps existing behavior and defaults - Let NetworkManager negotiate mixed-mode (WPA1+WPA2) security ([#271](https://github.com/freedesktop-rs/nmrs/pull/271)) -## [2.1.0] - 2026-02-28 - -### Added - -- `#[must_use]` attributes across public API: constructors, builder methods, and pure functions ([#220](https://github.com/freedesktop-rs/nmrs/issues/220)) - ## [2.0.1] - 2026-02-25 ### Changed @@ -499,30 +501,29 @@ present)` constructor; `RadioState::new` keeps existing behavior and defaults - EAP connections default to no certificates (advanced certificate management coming in future releases) -[1.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.1.0...nmrs-v1.2.0 -[1.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v1.3.0 -[1.3.5]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.3.0...nmrs-v1.3.5 -[2.0.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.3.5...nmrs-v2.0.0 -[2.0.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.0.0...nmrs-v2.0.1 -[2.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.0.1...nmrs-v2.2.0 -[2.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.2.0...nmrs-v2.3.0 -[2.4.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.3.0...nmrs-v2.4.0 -[3.0.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.4.0...nmrs-v3.0.0 -[3.0.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.0.0...nmrs-v3.0.1 -[3.1.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.0.1...nmrs-v3.1.0 -[3.1.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.0...nmrs-v3.1.1 -[3.1.2]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.1.2 -[3.1.3]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.1.3 -[3.1.4]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.1.4 -[3.1.5]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.1.5 -[3.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.2.0 -[3.2.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.2.1 -[3.2.2]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.2.2 -[3.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.3.0 -[3.4.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.4.0 -[3.4.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.4.1 -[3.4.2]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v3.4.2 [Unreleased]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.4.2...HEAD +[3.4.2]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.4.1...nmrs-v3.4.2 +[3.4.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.4.0...nmrs-v3.4.1 +[3.4.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.3.0...nmrs-v3.4.0 +[3.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.2.2...nmrs-v3.3.0 +[3.2.2]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.2.1...nmrs-v3.2.2 +[3.2.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.2.0...nmrs-v3.2.1 +[3.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.5...nmrs-v3.2.0 +[3.1.5]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.4...nmrs-v3.1.5 +[3.1.4]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.3...nmrs-v3.1.4 +[3.1.3]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.1...nmrs-v3.1.3 +[3.1.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.1.0...nmrs-v3.1.1 +[3.1.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.0.1...nmrs-v3.1.0 +[3.0.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v3.0.0...nmrs-v3.0.1 +[3.0.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.4.0...nmrs-v3.0.0 +[2.4.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.3.0...nmrs-v2.4.0 +[2.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.2.0...nmrs-v2.3.0 +[2.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.0.1...nmrs-v2.2.0 +[2.0.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v2.0.0...nmrs-v2.0.1 +[2.0.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.3.5...nmrs-v2.0.0 +[1.3.5]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.3.0...nmrs-v1.3.5 +[1.3.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.2.0...nmrs-v1.3.0 +[1.2.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.1.0...nmrs-v1.2.0 [1.1.0]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.0.1...nmrs-v1.1.0 [1.0.1]: https://github.com/freedesktop-rs/nmrs/compare/nmrs-v1.0.0...nmrs-v1.0.1 [1.0.0]: https://github.com/freedesktop-rs/nmrs/compare/v0.5.0-beta...nmrs-v1.0.0 diff --git a/nmrs/src/api/builders/mod.rs b/nmrs/src/api/builders/mod.rs index 92bb1070..b44b96e8 100644 --- a/nmrs/src/api/builders/mod.rs +++ b/nmrs/src/api/builders/mod.rs @@ -22,7 +22,7 @@ //! //! # Free functions //! -//! - [`build_wifi_connection`] / [`build_ethernet_connection`] (in [`wifi`]) +//! - [`try_build_wifi_connection`] / [`build_ethernet_connection`] (in [`wifi`]) //! - [`build_wireguard_connection`] / [`build_openvpn_connection`] (in [`vpn`]) //! - [`build_bluetooth_connection`] (in [`bluetooth`]) //! - [`build_vlan_connection`] (in [`vlan`]) @@ -45,12 +45,12 @@ //! ## Wi-Fi (free function) //! //! ```rust -//! use nmrs::builders::{build_ethernet_connection, build_wifi_connection}; +//! use nmrs::builders::{build_ethernet_connection, try_build_wifi_connection}; //! use nmrs::{ConnectionOptions, WifiSecurity}; //! //! let opts = ConnectionOptions::new(true).with_priority(10); //! -//! let wifi = build_wifi_connection( +//! let wifi = try_build_wifi_connection( //! "MyNetwork", //! &WifiSecurity::WpaPsk { psk: "password".into() }, //! &opts, @@ -123,4 +123,7 @@ pub use wireguard_builder::WireGuardBuilder; pub use bluetooth::build_bluetooth_connection; pub use vlan::build_vlan_connection; pub use vpn::{build_openvpn_connection, build_wireguard_connection}; -pub use wifi::{build_ethernet_connection, build_wifi_connection}; +pub use wifi::{build_ethernet_connection, try_build_wifi_connection}; +// Re-exported for backward compatibility until the deprecation is removed. +#[allow(deprecated)] +pub use wifi::build_wifi_connection; diff --git a/nmrs/src/api/builders/wifi.rs b/nmrs/src/api/builders/wifi.rs index 59da53bc..46cdb912 100644 --- a/nmrs/src/api/builders/wifi.rs +++ b/nmrs/src/api/builders/wifi.rs @@ -58,31 +58,84 @@ use crate::api::models::{self, ConnectionError, ConnectionOptions}; /// This function is maintained for backward compatibility. For new code, /// consider using `WifiConnectionBuilder` for a more ergonomic API. /// +/// If an EAP certificate or key is supplied as both a path and a blob, the path +/// is used and a warning is logged. Prefer [`try_build_wifi_connection`], which +/// reports the conflict as an error instead. +#[must_use] +#[deprecated( + since = "3.5.0", + note = "use `try_build_wifi_connection`, which reports conflicting EAP certificate path/blob inputs as an error instead of silently preferring the path" +)] +pub fn build_wifi_connection( + ssid: &str, + security: &models::WifiSecurity, + opts: &ConnectionOptions, +) -> HashMap<&'static str, HashMap<&'static str, Value<'static>>> { + // The deprecated EAP builders warn about path/blob conflicts themselves. + #[allow(deprecated)] + let builder = match security { + models::WifiSecurity::Open => base_wifi_builder(ssid, opts).open(), + models::WifiSecurity::WpaPsk { psk } => base_wifi_builder(ssid, opts).wpa_psk(psk), + models::WifiSecurity::WpaEap { opts: eap } => { + base_wifi_builder(ssid, opts).wpa_eap(eap.clone()) + } + models::WifiSecurity::Wpa3Eap192bit { opts: eap } => { + base_wifi_builder(ssid, opts).wpa3_eap_192_bit(eap.clone()) + } + }; + + builder.build() +} + +/// Builds a complete Wi-Fi connection settings dictionary, reporting invalid +/// EAP input instead of silently resolving it. +/// +/// Behaves exactly like [`build_wifi_connection`] for every valid input. +/// /// # Errors /// /// Returns [`ConnectionError::InvalidInput`] when an EAP certificate or key /// is supplied as both a path and a blob. -#[must_use = "handle invalid Wi-Fi EAP inputs before using the settings"] -pub fn build_wifi_connection( +/// +/// # Example +/// +/// ```rust +/// use nmrs::builders::try_build_wifi_connection; +/// use nmrs::{ConnectionOptions, WifiSecurity}; +/// +/// let settings = try_build_wifi_connection( +/// "MyNetwork", +/// &WifiSecurity::WpaPsk { psk: "password".into() }, +/// &ConnectionOptions::new(true), +/// )?; +/// # Ok::<(), nmrs::ConnectionError>(()) +/// ``` +pub fn try_build_wifi_connection( ssid: &str, security: &models::WifiSecurity, opts: &ConnectionOptions, ) -> Result>>, ConnectionError> { - let mut builder = WifiConnectionBuilder::new(ssid) - .options(opts) - .ipv4_auto() - .ipv6_auto(); - - builder = match security { - models::WifiSecurity::Open => builder.open(), - models::WifiSecurity::WpaPsk { psk } => builder.wpa_psk(psk), - models::WifiSecurity::WpaEap { opts } => builder.wpa_eap(opts.clone())?, - models::WifiSecurity::Wpa3Eap192bit { opts } => builder.wpa3_eap_192_bit(opts.clone())?, + let base = base_wifi_builder(ssid, opts); + + let builder = match security { + models::WifiSecurity::Open => base.open(), + models::WifiSecurity::WpaPsk { psk } => base.wpa_psk(psk), + models::WifiSecurity::WpaEap { opts: eap } => base.try_wpa_eap(eap.clone())?, + models::WifiSecurity::Wpa3Eap192bit { opts: eap } => { + base.try_wpa3_eap_192_bit(eap.clone())? + } }; Ok(builder.build()) } +fn base_wifi_builder(ssid: &str, opts: &ConnectionOptions) -> WifiConnectionBuilder { + WifiConnectionBuilder::new(ssid) + .options(opts) + .ipv4_auto() + .ipv6_auto() +} + /// Builds a complete Ethernet connection settings dictionary. /// /// Constructs all required sections for NetworkManager. The returned dictionary @@ -124,7 +177,7 @@ mod tests { security: &WifiSecurity, opts: &ConnectionOptions, ) -> HashMap<&'static str, HashMap<&'static str, Value<'static>>> { - super::build_wifi_connection(ssid, security, opts).expect("valid Wi-Fi settings") + super::try_build_wifi_connection(ssid, security, opts).expect("valid Wi-Fi settings") } fn default_opts() -> ConnectionOptions { @@ -160,7 +213,7 @@ mod tests { opts.ca_cert_path = Some("file:///etc/ssl/certs/ca.pem".into()); opts.ca_cert_blob = Some(vec![1, 2, 3]); - match super::build_wifi_connection( + match super::try_build_wifi_connection( "enterprise", &WifiSecurity::WpaEap { opts }, &default_opts(), diff --git a/nmrs/src/api/builders/wifi_builder.rs b/nmrs/src/api/builders/wifi_builder.rs index 95c60f58..d9106247 100644 --- a/nmrs/src/api/builders/wifi_builder.rs +++ b/nmrs/src/api/builders/wifi_builder.rs @@ -48,6 +48,54 @@ impl WifiMode { } } +/// The three EAP inputs that accept either a filesystem path or an inline blob. +/// +/// Supplying both for the same field is ambiguous: NetworkManager takes exactly +/// one value per key, so one of the two would be silently discarded. +fn eap_cert_conflicts(opts: &models::EapOptions) -> impl Iterator + '_ { + [ + ( + "ca_cert", + opts.ca_cert_path.is_some(), + opts.ca_cert_blob.is_some(), + ), + ( + "client_cert", + opts.client_cert_path.is_some(), + opts.client_cert_blob.is_some(), + ), + ( + "private_key", + opts.private_key_path.is_some(), + opts.private_key_blob.is_some(), + ), + ] + .into_iter() + .filter_map(|(field, has_path, has_blob)| (has_path && has_blob).then_some(field)) +} + +/// Rejects EAP options that supply both a path and a blob for the same field. +fn validate_eap_opts(opts: &models::EapOptions) -> Result<(), ConnectionError> { + match eap_cert_conflicts(opts).next() { + Some(field) => Err(ConnectionError::InvalidInput { + field: field.to_string(), + reason: format!("cannot specify both {field}_path and {field}_blob"), + }), + None => Ok(()), + } +} + +/// Logs the conflicts [`validate_eap_opts`] would reject, for the deprecated +/// infallible builders that cannot report them to the caller. +fn warn_eap_conflicts(opts: &models::EapOptions) { + for field in eap_cert_conflicts(opts) { + log::warn!( + "both {field}_path and {field}_blob were supplied; using {field}_path. \ + Use the `try_*` builder to receive this as an error instead." + ); + } +} + /// Builder for WiFi (802.11) connections. /// /// This builder provides a type-safe, ergonomic API for creating WiFi connection @@ -91,7 +139,7 @@ impl WifiMode { /// .with_phase2(Phase2::Mschapv2); /// /// let settings = WifiConnectionBuilder::new("CorpNetwork") -/// .wpa_eap(eap_opts) +/// .try_wpa_eap(eap_opts) /// .expect("valid EAP options") /// .autoconnect(false) /// .build(); @@ -123,7 +171,7 @@ impl WifiConnectionBuilder { /// Creates a new WiFi connection builder for the specified SSID. /// /// By default, the connection is configured as an open network. Use - /// `.wpa_psk()` or `.wpa_eap()` to add security. + /// `.wpa_psk()` or `.try_wpa_eap()` to add security. #[must_use] pub fn new(ssid: impl Into) -> Self { let ssid = ssid.into(); @@ -174,6 +222,24 @@ impl WifiConnectionBuilder { self } + /// Configures WPA-EAP (Enterprise) security with 802.1X authentication. + /// + /// Supports PEAP, TTLS, and TLS methods with various inner authentication protocols. + /// + /// If a certificate or private key is supplied as both a path and a blob, + /// the path is used and a warning is logged. Prefer + /// [`try_wpa_eap`](Self::try_wpa_eap), which reports the conflict as an + /// error instead. + #[must_use] + #[deprecated( + since = "3.5.0", + note = "use `try_wpa_eap`, which reports conflicting certificate path/blob inputs as an error instead of silently preferring the path" + )] + pub fn wpa_eap(self, opts: models::EapOptions) -> Self { + warn_eap_conflicts(&opts); + self.wpa_eap_shared("wpa-eap", opts) + } + /// Configures WPA-EAP (Enterprise) security with 802.1X authentication. /// /// Supports PEAP, TTLS, and TLS methods with various inner authentication protocols. @@ -183,8 +249,27 @@ impl WifiConnectionBuilder { /// Returns [`ConnectionError::InvalidInput`] when a certificate or private /// key is supplied as both a path and a blob. #[must_use = "handle the invalid EAP configuration before continuing the builder chain"] - pub fn wpa_eap(self, opts: models::EapOptions) -> Result { - self.wpa_eap_shared("wpa-eap", opts) + pub fn try_wpa_eap(self, opts: models::EapOptions) -> Result { + validate_eap_opts(&opts)?; + Ok(self.wpa_eap_shared("wpa-eap", opts)) + } + + /// Configures WPA3-EAP (Enterprise) with 192bit security with 802.1X authentication. + /// + /// Supports only EAP-TLS. + /// + /// If a certificate or private key is supplied as both a path and a blob, + /// the path is used and a warning is logged. Prefer + /// [`try_wpa3_eap_192_bit`](Self::try_wpa3_eap_192_bit), which reports the + /// conflict as an error instead. + #[must_use] + #[deprecated( + since = "3.5.0", + note = "use `try_wpa3_eap_192_bit`, which reports conflicting certificate path/blob inputs as an error instead of silently preferring the path" + )] + pub fn wpa3_eap_192_bit(self, opts: models::EapOptions) -> Self { + warn_eap_conflicts(&opts); + self.wpa_eap_shared("wpa-eap-suite-b-192", opts) } /// Configures WPA3-EAP (Enterprise) with 192bit security with 802.1X authentication. @@ -196,15 +281,12 @@ impl WifiConnectionBuilder { /// Returns [`ConnectionError::InvalidInput`] when a certificate or private /// key is supplied as both a path and a blob. #[must_use = "handle the invalid EAP configuration before continuing the builder chain"] - pub fn wpa3_eap_192_bit(self, opts: models::EapOptions) -> Result { - self.wpa_eap_shared("wpa-eap-suite-b-192", opts) + pub fn try_wpa3_eap_192_bit(self, opts: models::EapOptions) -> Result { + validate_eap_opts(&opts)?; + Ok(self.wpa_eap_shared("wpa-eap-suite-b-192", opts)) } - fn wpa_eap_shared( - mut self, - key_mgmt: &'static str, - opts: models::EapOptions, - ) -> Result { + fn wpa_eap_shared(mut self, key_mgmt: &'static str, opts: models::EapOptions) -> Self { let mut security = HashMap::new(); security.insert("key-mgmt", Value::from(key_mgmt)); security.insert("auth-alg", Value::from("open")); @@ -240,7 +322,7 @@ impl WifiConnectionBuilder { } EapMethod::Tls => { if let Some(cert) = - Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob)? + Self::path_or_blob("private_key", opts.private_key_path, opts.private_key_blob) { e1x.insert("private-key", cert); } @@ -250,7 +332,7 @@ impl WifiConnectionBuilder { } if let Some(cert) = - Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob)? + Self::path_or_blob("client_cert", opts.client_cert_path, opts.client_cert_blob) { e1x.insert("client-cert", cert); } @@ -260,7 +342,7 @@ impl WifiConnectionBuilder { if opts.system_ca_certs { e1x.insert("system-ca-certs", Value::from(true)); } - if let Some(cert) = Self::path_or_blob("ca_cert", opts.ca_cert_path, opts.ca_cert_blob)? { + if let Some(cert) = Self::path_or_blob("ca_cert", opts.ca_cert_path, opts.ca_cert_blob) { e1x.insert("ca-cert", cert); } if let Some(dom) = opts.domain_suffix_match { @@ -269,7 +351,7 @@ impl WifiConnectionBuilder { self.inner = self.inner.with_section("802-1x", e1x); self.security_configured = true; - Ok(self) + self } /// Marks this network as hidden (doesn't broadcast SSID). @@ -426,15 +508,16 @@ impl WifiConnectionBuilder { attribute: &str, path: Option, blob: Option>, - ) -> Result>, ConnectionError> { + ) -> Option> { + // A path/blob conflict is rejected by `validate_eap_opts` on the `try_*` + // path and warned about by `warn_eap_conflicts` on the deprecated one, + // so preferring the path here is a deterministic last resort rather + // than a silent choice. + let _ = attribute; match (path, blob) { - (None, None) => Ok(None), - (Some(path), None) => Ok(Some(Self::path(path))), - (None, Some(blob)) => Ok(Some(Self::blob(blob))), - (Some(_), Some(_)) => Err(ConnectionError::InvalidInput { - field: attribute.to_string(), - reason: format!("cannot specify both {attribute}_path and {attribute}_blob"), - }), + (None, None) => None, + (Some(path), _) => Some(Self::path(path)), + (None, Some(blob)) => Some(Self::blob(blob)), } } @@ -547,7 +630,7 @@ mod tests { }; let settings = WifiConnectionBuilder::new("Enterprise") - .wpa_eap(eap_opts) + .try_wpa_eap(eap_opts) .expect("valid EAP options") .autoconnect(false) .ipv4_auto() @@ -574,7 +657,7 @@ mod tests { eap_opts.ca_cert_path = Some("file:///etc/ssl/certs/ca.pem".into()); eap_opts.ca_cert_blob = Some(vec![1, 2, 3]); - match WifiConnectionBuilder::new("Enterprise").wpa_eap(eap_opts) { + match WifiConnectionBuilder::new("Enterprise").try_wpa_eap(eap_opts) { Err(ConnectionError::InvalidInput { field, reason }) => { assert_eq!(field, "ca_cert"); assert_eq!(reason, "cannot specify both ca_cert_path and ca_cert_blob"); @@ -584,6 +667,33 @@ mod tests { } } + /// The deprecated infallible builder cannot report a conflict, so it must + /// resolve it deterministically (path wins) rather than panicking, which is + /// what it did before #478. + #[test] + fn deprecated_wpa_eap_prefers_path_over_blob_on_conflict() { + let mut eap_opts = EapOptions::new("user@example.com", "secret"); + eap_opts.ca_cert_path = Some("file:///etc/ssl/certs/ca.pem".into()); + eap_opts.ca_cert_blob = Some(vec![1, 2, 3]); + + #[allow(deprecated)] + let settings = WifiConnectionBuilder::new("Enterprise") + .wpa_eap(eap_opts) + .ipv4_auto() + .build(); + + let e1x = settings.get("802-1x").expect("802-1x section"); + let ca_cert = e1x.get("ca-cert").expect("ca-cert entry"); + // `path()` encodes as a NUL-terminated byte array; a blob would not + // carry the `file://` prefix. + let bytes = >::try_from(ca_cert.try_clone().expect("clonable")) + .expect("ca-cert is a byte array"); + assert!( + bytes.starts_with(b"file:///etc/ssl/certs/ca.pem"), + "expected the path to win, got {bytes:?}" + ); + } + #[test] fn configures_hidden_network() { let settings = WifiConnectionBuilder::new("HiddenSSID") diff --git a/nmrs/src/api/network_manager.rs b/nmrs/src/api/network_manager.rs index 2bb20a3f..a967c46a 100644 --- a/nmrs/src/api/network_manager.rs +++ b/nmrs/src/api/network_manager.rs @@ -283,7 +283,7 @@ impl NetworkManager { /// # Examples /// /// ```no_run - /// use nmrs::builders::{build_wifi_connection, WifiConnectionBuilder, WifiMode}; + /// use nmrs::builders::{try_build_wifi_connection, WifiConnectionBuilder, WifiMode}; /// use nmrs::{ConnectionOptions, NetworkManager, WifiSecurity}; /// /// # async fn example() -> nmrs::Result<()> { @@ -291,7 +291,7 @@ impl NetworkManager { /// /// // Save a client profile for later activation. /// let opts = ConnectionOptions::new(true); - /// let settings = build_wifi_connection( + /// let settings = try_build_wifi_connection( /// "GuestWiFi", /// &WifiSecurity::WpaPsk { psk: "password".into() }, /// &opts, diff --git a/nmrs/src/core/connection.rs b/nmrs/src/core/connection.rs index 5a1c8e66..aa7148d8 100644 --- a/nmrs/src/core/connection.rs +++ b/nmrs/src/core/connection.rs @@ -5,7 +5,7 @@ use zbus::Connection; use zvariant::OwnedObjectPath; use crate::Result; -use crate::api::builders::wifi::{build_ethernet_connection, build_wifi_connection}; +use crate::api::builders::wifi::{build_ethernet_connection, try_build_wifi_connection}; use crate::api::models::{ConnectionError, ConnectionOptions, TimeoutConfig, WifiSecurity}; use crate::core::connection_settings::{delete_connection, get_saved_connection_path}; use crate::core::state_wait::{wait_for_connection_activation, wait_for_device_disconnect}; @@ -776,7 +776,7 @@ async fn connect_via_saved( autoconnect_retries: None, }; - let settings = build_wifi_connection(ssid, creds, &opts)?; + let settings = try_build_wifi_connection(ssid, creds, &opts)?; debug!("Creating fresh connection with corrected settings"); let (new_connection, new_active_conn) = nm @@ -816,7 +816,7 @@ async fn connect_via_saved( autoconnect_retries: None, }; - let settings = build_wifi_connection(ssid, creds, &opts)?; + let settings = try_build_wifi_connection(ssid, creds, &opts)?; let (new_connection, active_conn) = nm .add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) @@ -880,7 +880,7 @@ async fn build_and_activate_new( autoconnect_priority: None, }; - let settings = build_wifi_connection(ssid, &creds, &opts)?; + let settings = try_build_wifi_connection(ssid, &creds, &opts)?; trace!( "Creating new connection with {} settings sections", diff --git a/nmrs/src/lib.rs b/nmrs/src/lib.rs index 07473fe5..76f61fc3 100644 --- a/nmrs/src/lib.rs +++ b/nmrs/src/lib.rs @@ -132,7 +132,7 @@ //! The [`builders`] module provides both fluent builder types //! ([`builders::ConnectionBuilder`], [`builders::WifiConnectionBuilder`], //! [`builders::WireGuardBuilder`], [`builders::OpenVpnBuilder`]) and -//! free functions (`build_wifi_connection`, `build_ethernet_connection`, +//! free functions (`try_build_wifi_connection`, `build_ethernet_connection`, //! `build_wireguard_connection`, `build_openvpn_connection`, //! `build_bluetooth_connection`, `build_vlan_connection`) for constructing //! NetworkManager settings dictionaries. Most callers should reach for the @@ -343,7 +343,7 @@ pub mod raw { /// [`WireGuardBuilder`](builders::WireGuardBuilder), and /// [`OpenVpnBuilder`](builders::OpenVpnBuilder), which support method /// chaining and validation at `.build()`. -/// - **Free functions** — `build_wifi_connection`, `build_ethernet_connection`, +/// - **Free functions** — `try_build_wifi_connection`, `build_ethernet_connection`, /// `build_wireguard_connection`, `build_openvpn_connection`, /// `build_bluetooth_connection`, and `build_vlan_connection`, which are /// handy for one-shot construction. @@ -359,11 +359,11 @@ pub mod raw { /// # Example /// /// ```rust -/// use nmrs::builders::build_wifi_connection; +/// use nmrs::builders::try_build_wifi_connection; /// use nmrs::{ConnectionOptions, WifiSecurity}; /// /// let opts = ConnectionOptions::new(true); -/// let settings = build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts) +/// let settings = try_build_wifi_connection("MyNetwork", &WifiSecurity::Open, &opts) /// .expect("valid Wi-Fi settings"); /// // `settings` can be passed straight to NetworkManager via D-Bus. /// ``` diff --git a/scripts/bump_version.py b/scripts/bump_version.py index c221bf56..4e212a14 100755 --- a/scripts/bump_version.py +++ b/scripts/bump_version.py @@ -16,21 +16,71 @@ from pathlib import Path +BREAKING_MARKER = re.compile(r'\*\*breaking:?\*\*|^#+\s*breaking\b', re.IGNORECASE | re.MULTILINE) + + +def read_current_version(cargo_toml_path: Path) -> str | None: + """Read the current version out of Cargo.toml.""" + match = re.search(r'^version\s*=\s*"([^"]+)"', cargo_toml_path.read_text(), re.MULTILINE) + return match.group(1) if match else None + + +def unreleased_section(changelog_path: Path) -> str: + """Return the body of the [Unreleased] section, or '' if absent.""" + match = re.search(r'## \[Unreleased\](.*?)(?=## \[|\Z)', changelog_path.read_text(), re.DOTALL) + return match.group(1) if match else '' + + +def check_breaking_changes(changelog_path: Path, current: str, new: str, allow: bool) -> bool: + """Refuse a non-major bump when [Unreleased] documents a breaking change. + + cargo-semver-checks does not catch every break (function return types, for + one), and the version is bumped in a separate commit after CI runs, so it + never evaluates the version actually being published. The changelog is the + reliable signal: breaking changes are already labelled there by hand. This + is what 3.4.0 needed and did not have. + """ + if not BREAKING_MARKER.search(unreleased_section(changelog_path)): + return True + + cur_major, cur_minor, _ = (int(p) for p in current.split('.')) + new_major, new_minor, _ = (int(p) for p in new.split('.')) + + if new_major > cur_major: + return True + + bump = 'minor' if new_minor > cur_minor else 'patch' + print(f"✗ [Unreleased] documents a breaking change, but {current} -> {new} is a {bump} bump.") + print() + print(" Cargo auto-upgrades minor and patch releases, so this would break") + print(" existing builds without opt-in. Pick one:") + print() + print(f" - Release it as {cur_major + 1}.0.0") + print(" - Make the change additive (new API + #[deprecated] on the old one)") + print(" - Park it in the next major's milestone") + print() + if allow: + print("--allow-breaking set; continuing anyway.") + return True + print(" Override with --allow-breaking if this is genuinely intended.") + return False + + def update_cargo_toml(file_path: Path, version: str) -> bool: """Update version in a Cargo.toml file.""" try: content = file_path.read_text() pattern = r'^version\s*=\s*"[^"]*"' replacement = f'version = "{version}"' - + new_content = re.sub(pattern, replacement, content, count=1, flags=re.MULTILINE) - + if new_content != content: file_path.write_text(new_content) print(f"✓ Updated {file_path}") return True else: - print(f"⚠ No changes needed in {file_path}") + print(f"No changes needed in {file_path}") return False except Exception as e: print(f"✗ Error updating {file_path}: {e}") @@ -42,74 +92,56 @@ def update_changelog(file_path: Path, version: str, release_type: str) -> bool: try: content = file_path.read_text() today = datetime.now().strftime("%Y-%m-%d") - - # Find the Unreleased section + unreleased_pattern = r'## \[Unreleased\](.*?)(?=## \[|\Z)' match = re.search(unreleased_pattern, content, re.DOTALL) - + if not match: - print(f"⚠ No [Unreleased] section found in {file_path}") + print(f"No [Unreleased] section found in {file_path}") return False - + unreleased_content = match.group(1).strip() - + if not unreleased_content: - print(f"⚠ [Unreleased] section is empty in {file_path}") + print(f"[Unreleased] section is empty in {file_path}") unreleased_content = "\n\n(No changes documented)" - - # Format version header + if release_type == "stable": version_header = f"## [{version}] - {today}" version_tag = version else: version_header = f"## [{version}-{release_type}] - {today}" version_tag = f"{version}-{release_type}" - + new_version_section = f"{version_header}\n{unreleased_content}\n\n" new_unreleased_section = "## [Unreleased]\n\n" - - # Replace the Unreleased section with new version + fresh Unreleased + new_content = re.sub( unreleased_pattern, new_unreleased_section + new_version_section, content, flags=re.DOTALL ) - + git_tag = f"nmrs-v{version_tag}" - - # Update the [Unreleased] comparison link - unreleased_link_pattern = r'\[Unreleased\]:\s*https://github\.com/[^/]+/[^/]+/compare/[^\s]+\.\.\.HEAD' + + unreleased_link_pattern = r'\[Unreleased\]:\s*https://github\.com/[^/]+/[^/]+/compare/([^\s]+)\.\.\.HEAD' + prev_match = re.search(unreleased_link_pattern, new_content, flags=re.IGNORECASE) + prev_tag = prev_match.group(1).strip() if prev_match else "v0.1.0-beta" + unreleased_link_replacement = f'[Unreleased]: https://github.com/freedesktop-rs/nmrs/compare/{git_tag}...HEAD' new_content = re.sub(unreleased_link_pattern, unreleased_link_replacement, new_content, flags=re.IGNORECASE) - - # Find the previous version tag to create comparison link - existing_links = re.findall( - r'\[([^\]]+)\]:\s*https://github\.com/[^/]+/[^/]+/compare/([^\s]+)\.\.\.([^\s]+)', - new_content - ) - - prev_tag = None - for link_text, _, curr_tag in existing_links: - if link_text.lower() != 'unreleased': - prev_tag = curr_tag.strip() - break - - if not prev_tag: - # Fallback to a reasonable default - prev_tag = "v0.1.0-beta" - - # Create the new version link + link_label = version if release_type == "stable" else version_tag new_version_link = f'[{link_label}]: https://github.com/freedesktop-rs/nmrs/compare/{prev_tag}...{git_tag}\n' - - # Insert before the Unreleased link + new_content = re.sub( - r'(\[Unreleased\]:)', - new_version_link + r'\1', - new_content + r'(\[Unreleased\]:[^\n]*\n)', + r'\1' + new_version_link, + new_content, + count=1 ) - + file_path.write_text(new_content) print(f"✓ Updated {file_path}") return True @@ -122,8 +154,8 @@ def update_changelog(file_path: Path, version: str, release_type: str) -> bool: def main(): """Main entry point.""" - if len(sys.argv) < 3: - print("Usage: bump_version.py ") + if len([a for a in sys.argv[1:] if not a.startswith('--')]) < 2: + print("Usage: bump_version.py [--allow-breaking]") print() print("Arguments:") print(" version Version number (e.g., 1.2.0)") @@ -133,42 +165,53 @@ def main(): print(" python3 scripts/bump_version.py 3.1.0 stable") print(" python3 scripts/bump_version.py 3.1.0 beta") print() + print("Flags:") + print(" --allow-breaking Permit a non-major bump despite a breaking") + print(" change in [Unreleased]. Use deliberately.") + print() print("This script should be run on the dev branch before creating a PR to master.") sys.exit(1) - - version = sys.argv[1] - release_type = sys.argv[2] - - # Validate inputs + + args = [a for a in sys.argv[1:] if not a.startswith('--')] + flags = {a for a in sys.argv[1:] if a.startswith('--')} + allow_breaking = '--allow-breaking' in flags + + version = args[0] + release_type = args[1] + if not re.match(r'^\d+\.\d+\.\d+$', version): print(f"✗ Invalid version format: {version}") print("Expected format: X.Y.Z (e.g., 1.2.0)") sys.exit(1) - + if release_type not in ['beta', 'stable']: print(f"✗ Invalid release type: {release_type}") print("Expected: 'beta' or 'stable'") sys.exit(1) - + script_dir = Path(__file__).parent project_root = script_dir.parent - + print(f"Preparing nmrs release: {version}-{release_type}") print("=" * 50) - + success = True - - # Update Cargo.toml + cargo_toml_path = project_root / 'nmrs' / 'Cargo.toml' + changelog_path = project_root / 'nmrs' / 'CHANGELOG.md' + + if release_type == 'stable' and cargo_toml_path.exists() and changelog_path.exists(): + current = read_current_version(cargo_toml_path) + if current and not check_breaking_changes(changelog_path, current, version, allow_breaking): + sys.exit(1) + if not cargo_toml_path.exists(): print(f"✗ File not found: {cargo_toml_path}") success = False else: if not update_cargo_toml(cargo_toml_path, version): success = False - - # Update CHANGELOG.md - changelog_path = project_root / 'nmrs' / 'CHANGELOG.md' + if not changelog_path.exists(): print(f"✗ File not found: {changelog_path}") print(" Create nmrs/CHANGELOG.md with an [Unreleased] section first") @@ -176,18 +219,17 @@ def main(): else: if not update_changelog(changelog_path, version, release_type): success = False - + print("=" * 50) - + if success: - # Determine the tag that will be created if release_type == "stable": version_tag = version else: version_tag = f"{version}-{release_type}" - + git_tag = f"nmrs-v{version_tag}" - + print(f"✓ Successfully prepared nmrs release {version}-{release_type}") print() print("Next steps:")