From bd1dc54e20f7305fd26e065b28d06a0292156a4c Mon Sep 17 00:00:00 2001 From: Andy Dixon Date: Mon, 12 Oct 2026 09:19:03 +0100 Subject: [PATCH] fix: validate Quick Connect and ~/.ssh/config imports; unknown enum values no longer reset a profile's config - Quick Connect runs the same validation as saved profiles, so an SSH connection without a username fails up front with a clear message (F5). - ~/.ssh/config imports fill a missing User (and jump-host user) with the local user name, as ssh does, and skip blocks that still don't validate, reporting how many (F6). - An unrecognised hostKeyPolicy/lineEnding/parity/flowControl/direction in a stored config falls back to that field's default instead of failing the whole JSON column and silently resetting every other setting; parse failures that remain are logged (F7). --- rust/app/src/app.rs | 10 +++++++--- rust/app/src/chrome.rs | 2 +- rust/app/src/quick_connect.rs | 17 +++++++++++++++++ rust/app/src/settings.rs | 8 +++++++- rust/core/src/models.rs | 28 +++++++++++++++++++++++++++ rust/core/src/service.rs | 36 +++++++++++++++++++++++++++-------- rust/core/src/storage.rs | 3 ++- 7 files changed, 90 insertions(+), 14 deletions(-) diff --git a/rust/app/src/app.rs b/rust/app/src/app.rs index 0ce234b..4e37505 100644 --- a/rust/app/src/app.rs +++ b/rust/app/src/app.rs @@ -586,9 +586,13 @@ impl App { Request::Connect(id) => self.connect_profile(&id, false), Request::ConnectQuick(p) => { let name = if p.display_name.is_empty() { p.host.clone() } else { p.display_name.clone() }; - let sid = self.s.svc.connect_quick(p); - self.add_session(&sid, true); - self.s.info(format!("Connecting to {name}…")); + match self.s.svc.connect_quick(p) { + Ok(sid) => { + self.add_session(&sid, true); + self.s.info(format!("Connecting to {name}…")); + } + Err(e) => self.s.error(format!("Can't connect to {name}: {e}")), + } } Request::ReloadProfiles => self.s.reload_profiles(), Request::ReloadPreferences => { diff --git a/rust/app/src/chrome.rs b/rust/app/src/chrome.rs index 77c8494..ec93fcb 100644 --- a/rust/app/src/chrome.rs +++ b/rust/app/src/chrome.rs @@ -80,7 +80,7 @@ pub fn title_bar(app: &mut App, ui: &mut Ui) { if item(ui, icon::FILE_ARROW_DOWN, "Import ~/.ssh/config", "") { match app.s.svc.import_ssh_config(None, "SSH Config") { Ok(n) => { - app.s.success(format!("Imported {n} host(s) from ~/.ssh/config")); + app.s.success(crate::settings::ssh_import_summary(n)); app.s.reload_profiles(); } Err(e) => app.s.error(format!("Import failed: {e}")), diff --git a/rust/app/src/quick_connect.rs b/rust/app/src/quick_connect.rs index 39140a3..ed455ef 100644 --- a/rust/app/src/quick_connect.rs +++ b/rust/app/src/quick_connect.rs @@ -184,4 +184,21 @@ mod tests { let p = q.profile().unwrap(); assert_eq!((p.port, p.username.as_str()), (0, "")); } + + #[test] + fn quick_profiles_pass_validation_unless_incomplete() { + use nexus_core::validation::validate_profile; + for proto in PROTOCOLS { + let mut q = QuickConnect { host: "box".into(), username: "me".into(), ..Default::default() }; + q.set_protocol(proto); + let mut p = q.profile().unwrap(); + p.normalize(); + assert!(validate_profile(&p).is_ok(), "{proto:?}: {:?}", validate_profile(&p).err().map(|e| e.to_string())); + } + let mut q = QuickConnect { host: "box".into(), ..Default::default() }; + q.set_protocol(Protocol::Ssh); + let mut p = q.profile().unwrap(); + p.normalize(); + assert!(validate_profile(&p).is_err(), "SSH needs a username"); + } } diff --git a/rust/app/src/settings.rs b/rust/app/src/settings.rs index 617f4e0..9a98f89 100644 --- a/rust/app/src/settings.rs +++ b/rust/app/src/settings.rs @@ -543,7 +543,7 @@ impl Settings { if widgets::button(ui, icon::DOWNLOAD_SIMPLE, "Import ~/.ssh/config").clicked() { match s.svc.import_ssh_config(None, "SSH Config") { Ok(n) => { - s.success(format!("Imported {n} host(s) from ~/.ssh/config")); + s.success(ssh_import_summary(n)); s.request(Request::ReloadProfiles); } Err(e) => s.error(format!("SSH config import failed: {e}")), @@ -934,6 +934,12 @@ Open two or more sessions and press {Mod+Shift+B} to turn on broadcast mode. Eve ), ]; +pub fn ssh_import_summary((n, skipped): (usize, usize)) -> String { + match skipped { + 0 => format!("Imported {n} host(s) from ~/.ssh/config"), + k => format!("Imported {n} host(s) from ~/.ssh/config; skipped {k} that couldn't be used as-is"), + } +} #[cfg(test)] mod tests { use super::*; diff --git a/rust/core/src/models.rs b/rust/core/src/models.rs index 9b178a0..a20020c 100644 --- a/rust/core/src/models.rs +++ b/rust/core/src/models.rs @@ -37,6 +37,12 @@ fn nd<'de, D: Deserializer<'de>, T: Default + Deserialize<'de>>(d: D) -> Result< Ok(Option::::deserialize(d)?.unwrap_or_default()) } +/// For enum fields: an unknown value (e.g. written by a newer build) falls back to the default +/// for that field instead of failing the whole config, which would reset every other setting. +fn lenient<'de, D: Deserializer<'de>, T: Default + serde::de::DeserializeOwned>(d: D) -> Result { + Ok(Option::::deserialize(d)?.and_then(|v| T::deserialize(v).ok()).unwrap_or_default()) +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] #[serde(rename_all = "lowercase")] pub enum Protocol { @@ -244,6 +250,7 @@ pub struct SshConfig { pub secret_mode: SecretMode, pub private_key_path: String, pub key_secret_mode: SecretMode, + #[serde(deserialize_with = "lenient")] pub host_key_policy: HostKeyPolicy, pub terminal_type: String, pub keepalive_interval: i64, @@ -303,6 +310,7 @@ pub enum ForwardDirection { #[serde(rename_all = "camelCase", default)] pub struct PortForward { pub id: String, + #[serde(deserialize_with = "lenient")] pub direction: ForwardDirection, pub bind_host: String, pub bind_port: i64, @@ -316,6 +324,7 @@ pub struct PortForward { #[serde(rename_all = "camelCase", default)] pub struct TelnetConfig { pub local_echo: bool, + #[serde(deserialize_with = "lenient")] pub line_ending: LineEnding, pub terminal_type: String, pub encoding: String, @@ -331,6 +340,7 @@ pub struct TelnetConfig { #[serde(rename_all = "camelCase", default)] pub struct RawConfig { pub auto_reconnect: bool, + #[serde(deserialize_with = "lenient")] pub line_ending: LineEnding, pub encoding: String, pub show_timestamps: bool, @@ -431,9 +441,12 @@ pub enum FlowControl { pub struct SerialConfig { pub baud_rate: u32, pub data_bits: u8, + #[serde(deserialize_with = "lenient")] pub parity: Parity, pub stop_bits: u8, + #[serde(deserialize_with = "lenient")] pub flow_control: FlowControl, + #[serde(deserialize_with = "lenient")] pub line_ending: LineEnding, pub local_echo: bool, pub dtr_on_connect: bool, @@ -919,3 +932,18 @@ impl IrcConfig { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unknown_enum_values_fall_back_per_field() { + let c: SshConfig = serde_json::from_str(r#"{"hostKeyPolicy":"from-the-future","keepaliveInterval":42,"agentForwarding":true}"#).unwrap(); + assert_eq!((c.host_key_policy, c.keepalive_interval, c.agent_forwarding), (HostKeyPolicy::Ask, 42, true)); + let c: SshConfig = serde_json::from_str(r#"{"hostKeyPolicy":"strict"}"#).unwrap(); + assert_eq!(c.host_key_policy, HostKeyPolicy::Strict); + let t: TelnetConfig = serde_json::from_str(r#"{"lineEnding":"weird","localEcho":true}"#).unwrap(); + assert_eq!((t.line_ending, t.local_echo), (LineEnding::default(), true)); + } +} diff --git a/rust/core/src/service.rs b/rust/core/src/service.rs index 736480e..999ed70 100644 --- a/rust/core/src/service.rs +++ b/rust/core/src/service.rs @@ -109,12 +109,17 @@ impl Service { } /// Opens an unsaved profile (Quick Connect). - pub fn connect_quick(&self, mut p: Profile) -> String { + /// Validates like a saved profile (so e.g. an empty SSH username fails here, not at auth). + pub fn connect_quick(&self, mut p: Profile) -> Result { if p.id.is_empty() { p.id = new_id(); } + if p.display_name.trim().is_empty() { + p.display_name = p.host.clone(); + } p.normalize(); - self.sessions.connect(p) + validation::validate_profile(&p).map_err(|e| e.to_string())?; + Ok(self.sessions.connect(p)) } pub fn connect_local(&self) -> String { @@ -277,15 +282,30 @@ impl Service { self.import(ps, strategy) } - pub fn import_ssh_config(&self, path: Option<&Path>, folder: &str) -> Result { + /// Imports Host blocks as profiles; returns (imported, skipped). A block without `User` gets + /// the local user name, as `ssh` itself would; blocks that still don't validate are skipped. + pub fn import_ssh_config(&self, path: Option<&Path>, folder: &str) -> Result<(usize, usize)> { let path = path.map(Path::to_path_buf).or_else(sshconfig::default_config_path).ok_or("no ~/.ssh/config found")?; let blocks = sshconfig::parse_file(&path)?; - let mut n = 0; - for p in sshconfig::to_profiles(&blocks, folder) { - n += self.store.create_profile(&p).is_ok() as usize; + let local_user = std::env::var("USER").or_else(|_| std::env::var("USERNAME")).unwrap_or_default(); + let (mut n, mut skipped) = (0, 0); + for mut p in sshconfig::to_profiles(&blocks, folder) { + if p.username.is_empty() { + p.username = local_user.clone(); + } + for jh in p.ssh.iter_mut().flat_map(|s| s.jump_hosts.iter_mut()) { + if jh.username.is_empty() { + jh.username = local_user.clone(); + } + } + if validation::validate_profile(&p).is_ok() && self.store.create_profile(&p).is_ok() { + n += 1; + } else { + skipped += 1; + } } - self.store.add_audit("import", "", "", &format!("Imported {n} profile(s)")); - Ok(n) + self.store.add_audit("import", "", "", &format!("Imported {n} profile(s) from SSH config, skipped {skipped}")); + Ok((n, skipped)) } // --- SSH keys ----------------------------------------------------------------------- diff --git a/rust/core/src/storage.rs b/rust/core/src/storage.rs index 518d076..2f0f2ea 100644 --- a/rust/core/src/storage.rs +++ b/rust/core/src/storage.rs @@ -46,7 +46,8 @@ fn json_col(v: &Option) -> Option { } fn parse_col(s: Option) -> Option { - s.filter(|s| !s.is_empty() && s != "null").and_then(|s| serde_json::from_str(&s).ok()) + let s = s.filter(|s| !s.is_empty() && s != "null")?; + serde_json::from_str(&s).inspect_err(|e| eprintln!("storage: ignoring unreadable profile config: {e}")).ok() } fn scan_profile(r: &Row) -> rusqlite::Result {