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).
This commit is contained in:
Andy Dixon
2026-10-12 09:19:03 +01:00
parent 3b490fa6c3
commit bd1dc54e20
7 changed files with 90 additions and 14 deletions
+7 -3
View File
@@ -586,9 +586,13 @@ impl App {
Request::Connect(id) => self.connect_profile(&id, false), Request::Connect(id) => self.connect_profile(&id, false),
Request::ConnectQuick(p) => { Request::ConnectQuick(p) => {
let name = if p.display_name.is_empty() { p.host.clone() } else { p.display_name.clone() }; let name = if p.display_name.is_empty() { p.host.clone() } else { p.display_name.clone() };
let sid = self.s.svc.connect_quick(p); match self.s.svc.connect_quick(p) {
self.add_session(&sid, true); Ok(sid) => {
self.s.info(format!("Connecting to {name}…")); 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::ReloadProfiles => self.s.reload_profiles(),
Request::ReloadPreferences => { Request::ReloadPreferences => {
+1 -1
View File
@@ -80,7 +80,7 @@ pub fn title_bar(app: &mut App, ui: &mut Ui) {
if item(ui, icon::FILE_ARROW_DOWN, "Import ~/.ssh/config", "") { if item(ui, icon::FILE_ARROW_DOWN, "Import ~/.ssh/config", "") {
match app.s.svc.import_ssh_config(None, "SSH Config") { match app.s.svc.import_ssh_config(None, "SSH Config") {
Ok(n) => { 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(); app.s.reload_profiles();
} }
Err(e) => app.s.error(format!("Import failed: {e}")), Err(e) => app.s.error(format!("Import failed: {e}")),
+17
View File
@@ -184,4 +184,21 @@ mod tests {
let p = q.profile().unwrap(); let p = q.profile().unwrap();
assert_eq!((p.port, p.username.as_str()), (0, "")); 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");
}
} }
+7 -1
View File
@@ -543,7 +543,7 @@ impl Settings {
if widgets::button(ui, icon::DOWNLOAD_SIMPLE, "Import ~/.ssh/config").clicked() { if widgets::button(ui, icon::DOWNLOAD_SIMPLE, "Import ~/.ssh/config").clicked() {
match s.svc.import_ssh_config(None, "SSH Config") { match s.svc.import_ssh_config(None, "SSH Config") {
Ok(n) => { Ok(n) => {
s.success(format!("Imported {n} host(s) from ~/.ssh/config")); s.success(ssh_import_summary(n));
s.request(Request::ReloadProfiles); s.request(Request::ReloadProfiles);
} }
Err(e) => s.error(format!("SSH config import failed: {e}")), 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)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
+28
View File
@@ -37,6 +37,12 @@ fn nd<'de, D: Deserializer<'de>, T: Default + Deserialize<'de>>(d: D) -> Result<
Ok(Option::<T>::deserialize(d)?.unwrap_or_default()) Ok(Option::<T>::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<T, D::Error> {
Ok(Option::<serde_json::Value>::deserialize(d)?.and_then(|v| T::deserialize(v).ok()).unwrap_or_default())
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, Default)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
pub enum Protocol { pub enum Protocol {
@@ -244,6 +250,7 @@ pub struct SshConfig {
pub secret_mode: SecretMode, pub secret_mode: SecretMode,
pub private_key_path: String, pub private_key_path: String,
pub key_secret_mode: SecretMode, pub key_secret_mode: SecretMode,
#[serde(deserialize_with = "lenient")]
pub host_key_policy: HostKeyPolicy, pub host_key_policy: HostKeyPolicy,
pub terminal_type: String, pub terminal_type: String,
pub keepalive_interval: i64, pub keepalive_interval: i64,
@@ -303,6 +310,7 @@ pub enum ForwardDirection {
#[serde(rename_all = "camelCase", default)] #[serde(rename_all = "camelCase", default)]
pub struct PortForward { pub struct PortForward {
pub id: String, pub id: String,
#[serde(deserialize_with = "lenient")]
pub direction: ForwardDirection, pub direction: ForwardDirection,
pub bind_host: String, pub bind_host: String,
pub bind_port: i64, pub bind_port: i64,
@@ -316,6 +324,7 @@ pub struct PortForward {
#[serde(rename_all = "camelCase", default)] #[serde(rename_all = "camelCase", default)]
pub struct TelnetConfig { pub struct TelnetConfig {
pub local_echo: bool, pub local_echo: bool,
#[serde(deserialize_with = "lenient")]
pub line_ending: LineEnding, pub line_ending: LineEnding,
pub terminal_type: String, pub terminal_type: String,
pub encoding: String, pub encoding: String,
@@ -331,6 +340,7 @@ pub struct TelnetConfig {
#[serde(rename_all = "camelCase", default)] #[serde(rename_all = "camelCase", default)]
pub struct RawConfig { pub struct RawConfig {
pub auto_reconnect: bool, pub auto_reconnect: bool,
#[serde(deserialize_with = "lenient")]
pub line_ending: LineEnding, pub line_ending: LineEnding,
pub encoding: String, pub encoding: String,
pub show_timestamps: bool, pub show_timestamps: bool,
@@ -431,9 +441,12 @@ pub enum FlowControl {
pub struct SerialConfig { pub struct SerialConfig {
pub baud_rate: u32, pub baud_rate: u32,
pub data_bits: u8, pub data_bits: u8,
#[serde(deserialize_with = "lenient")]
pub parity: Parity, pub parity: Parity,
pub stop_bits: u8, pub stop_bits: u8,
#[serde(deserialize_with = "lenient")]
pub flow_control: FlowControl, pub flow_control: FlowControl,
#[serde(deserialize_with = "lenient")]
pub line_ending: LineEnding, pub line_ending: LineEnding,
pub local_echo: bool, pub local_echo: bool,
pub dtr_on_connect: 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));
}
}
+28 -8
View File
@@ -109,12 +109,17 @@ impl Service {
} }
/// Opens an unsaved profile (Quick Connect). /// 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<String> {
if p.id.is_empty() { if p.id.is_empty() {
p.id = new_id(); p.id = new_id();
} }
if p.display_name.trim().is_empty() {
p.display_name = p.host.clone();
}
p.normalize(); 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 { pub fn connect_local(&self) -> String {
@@ -277,15 +282,30 @@ impl Service {
self.import(ps, strategy) self.import(ps, strategy)
} }
pub fn import_ssh_config(&self, path: Option<&Path>, folder: &str) -> Result<usize> { /// 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 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 blocks = sshconfig::parse_file(&path)?;
let mut n = 0; let local_user = std::env::var("USER").or_else(|_| std::env::var("USERNAME")).unwrap_or_default();
for p in sshconfig::to_profiles(&blocks, folder) { let (mut n, mut skipped) = (0, 0);
n += self.store.create_profile(&p).is_ok() as usize; 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)")); self.store.add_audit("import", "", "", &format!("Imported {n} profile(s) from SSH config, skipped {skipped}"));
Ok(n) Ok((n, skipped))
} }
// --- SSH keys ----------------------------------------------------------------------- // --- SSH keys -----------------------------------------------------------------------
+2 -1
View File
@@ -46,7 +46,8 @@ fn json_col<T: serde::Serialize>(v: &Option<T>) -> Option<String> {
} }
fn parse_col<T: serde::de::DeserializeOwned>(s: Option<String>) -> Option<T> { fn parse_col<T: serde::de::DeserializeOwned>(s: Option<String>) -> Option<T> {
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<Profile> { fn scan_profile(r: &Row) -> rusqlite::Result<Profile> {