chore: resolve host key verification follow-ups

src/commands/interactive/utils.rs

@@ -74,6 +74,7 @@ mod tests {
 
     #[test]
     fn test_expand_path_with_tilde() {
+        let _home_lock = crate::test_helpers::EnvGuard::lock_home();
         let cmd = InteractiveCommand {
             single_node: false,
             multiplex: true,

src/shared/auth_types.rs

@@ -288,6 +288,15 @@ pub enum ServerCheckMethod {
 
     /// Use a specific known_hosts file path.
     KnownHostsFile(String),
+
+    /// Trust On First Use against a specific known_hosts file path.
+    ///
+    /// Matching keys are accepted, unknown hosts are recorded and accepted,
+    /// and changed keys are rejected.
+    AcceptNewKnownHostsFile(String),
+
+    /// Trust On First Use for the lifetime of this process only.
+    AcceptNewInMemory,
 }
 
 impl ServerCheckMethod {
@@ -317,6 +326,53 @@ impl ServerCheckMethod {
     pub fn with_known_hosts_file(path: impl Into<String>) -> Self {
         Self::KnownHostsFile(path.into())
     }
+
+    /// Create a ServerCheckMethod using accept-new semantics with a known_hosts file.
+    ///
+    /// # Arguments
+    ///
+    /// * `path` - Path to the known_hosts file
+    pub fn with_accept_new_known_hosts_file(path: impl Into<String>) -> Self {
+        Self::AcceptNewKnownHostsFile(path.into())
+    }
+}
+
+impl From<crate::ssh::tokio_client::ServerCheckMethod> for ServerCheckMethod {
+    fn from(method: crate::ssh::tokio_client::ServerCheckMethod) -> Self {
+        match method {
+            crate::ssh::tokio_client::ServerCheckMethod::NoCheck => Self::NoCheck,
+            crate::ssh::tokio_client::ServerCheckMethod::PublicKey(key) => Self::PublicKey(key),
+            crate::ssh::tokio_client::ServerCheckMethod::PublicKeyFile(path) => {
+                Self::PublicKeyFile(path)
+            }
+            crate::ssh::tokio_client::ServerCheckMethod::DefaultKnownHostsFile => {
+                Self::DefaultKnownHostsFile
+            }
+            crate::ssh::tokio_client::ServerCheckMethod::KnownHostsFile(path) => {
+                Self::KnownHostsFile(path)
+            }
+            crate::ssh::tokio_client::ServerCheckMethod::AcceptNewKnownHostsFile(path) => {
+                Self::AcceptNewKnownHostsFile(path)
+            }
+            crate::ssh::tokio_client::ServerCheckMethod::AcceptNewInMemory => {
+                Self::AcceptNewInMemory
+            }
+        }
+    }
+}
+
+impl From<ServerCheckMethod> for crate::ssh::tokio_client::ServerCheckMethod {
+    fn from(method: ServerCheckMethod) -> Self {
+        match method {
+            ServerCheckMethod::NoCheck => Self::NoCheck,
+            ServerCheckMethod::PublicKey(key) => Self::PublicKey(key),
+            ServerCheckMethod::PublicKeyFile(path) => Self::PublicKeyFile(path),
+            ServerCheckMethod::DefaultKnownHostsFile => Self::DefaultKnownHostsFile,
+            ServerCheckMethod::KnownHostsFile(path) => Self::KnownHostsFile(path),
+            ServerCheckMethod::AcceptNewKnownHostsFile(path) => Self::AcceptNewKnownHostsFile(path),
+            ServerCheckMethod::AcceptNewInMemory => Self::AcceptNewInMemory,
+        }
+    }
 }
 
 #[cfg(test)]
@@ -370,5 +426,31 @@ mod tests {
 
         let file = ServerCheckMethod::with_known_hosts_file("/path/to/known_hosts");
         assert!(matches!(file, ServerCheckMethod::KnownHostsFile(_)));
+
+        let accept_new =
+            ServerCheckMethod::with_accept_new_known_hosts_file("/path/to/known_hosts");
+        assert!(matches!(
+            accept_new,
+            ServerCheckMethod::AcceptNewKnownHostsFile(_)
+        ));
+    }
+
+    #[test]
+    fn test_server_check_method_converts_to_client_type() {
+        let shared = ServerCheckMethod::AcceptNewKnownHostsFile("/tmp/known_hosts".to_string());
+        let client: crate::ssh::tokio_client::ServerCheckMethod = shared.clone().into();
+        assert_eq!(
+            client,
+            crate::ssh::tokio_client::ServerCheckMethod::AcceptNewKnownHostsFile(
+                "/tmp/known_hosts".to_string()
+            )
+        );
+
+        let round_trip: ServerCheckMethod = client.into();
+        assert_eq!(round_trip, shared);
+
+        let in_memory: ServerCheckMethod =
+            crate::ssh::tokio_client::ServerCheckMethod::AcceptNewInMemory.into();
+        assert_eq!(in_memory, ServerCheckMethod::AcceptNewInMemory);
     }
 }

src/ssh/tokio_client/host_verification.rs

@@ -37,6 +37,8 @@
 //! `node1` and is otherwise invisible to [`lookup_known_host`].
 
 use russh::keys::{Algorithm, HashAlg, PublicKey};
+#[cfg(test)]
+use std::sync::Mutex as StdMutex;
 use std::{
     collections::HashMap,
     fs::{File, OpenOptions},
@@ -73,6 +75,10 @@ static KNOWN_HOSTS_LOCK: Mutex<()> = Mutex::const_new(());
 static PROCESS_HOST_PINS: LazyLock<Mutex<HashMap<ProcessPinKey, PublicKey>>> =
     LazyLock::new(|| Mutex::new(HashMap::new()));
 
+#[cfg(test)]
+static FILE_LOCK_ACQUISITIONS: LazyLock<StdMutex<Vec<String>>> =
+    LazyLock::new(|| StdMutex::new(Vec::new()));
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 struct ProcessPinKey {
     hostname: String,
@@ -277,6 +283,11 @@ fn acquire_known_hosts_file_lock(
         );
         super::Error::ServerCheckFailed
     })?;
+    #[cfg(test)]
+    FILE_LOCK_ACQUISITIONS
+        .lock()
+        .unwrap()
+        .push(known_hosts_path.to_string());
 
     Ok(KnownHostsFileLock { file })
 }
@@ -673,6 +684,34 @@ pub(super) async fn verify_accept_new(
     // rejected would only make every other connection wait on it.
     ensure_recordable_hostname(hostname)?;
 
+    probe_known_hosts_path(known_hosts_path)?;
+    check_marker_lines(hostname, port, server_public_key, known_hosts_path)?;
+    match lookup_known_host(hostname, port, server_public_key, known_hosts_path) {
+        Ok(KnownHostLookup::Match) => {
+            verify_process_pin(hostname, port, Some(known_hosts_path), server_public_key).await?;
+            return Ok(true);
+        }
+        Ok(KnownHostLookup::Conflict { line }) => {
+            return Err(map_known_hosts_error(
+                hostname,
+                port,
+                server_public_key,
+                known_hosts_path,
+                russh::keys::Error::KeyChanged { line },
+            ));
+        }
+        Ok(KnownHostLookup::Unknown) => {}
+        Err(e) => {
+            return Err(map_known_hosts_error(
+                hostname,
+                port,
+                server_public_key,
+                known_hosts_path,
+                e,
+            ));
+        }
+    }
+
     let _guard = KNOWN_HOSTS_LOCK.lock().await;
     let _file_lock = acquire_known_hosts_file_lock(known_hosts_path)?;
 
@@ -865,6 +904,10 @@ fn record_host_key(
         return;
     }
 
+    if !file_preexisted {
+        remove_leading_blank_line(path);
+    }
+
     #[cfg(unix)]
     restrict_created_permissions(path, dir_preexisted, file_preexisted);
     #[cfg(not(unix))]
@@ -877,6 +920,26 @@ fn record_host_key(
     );
 }
 
+fn remove_leading_blank_line(path: &Path) {
+    match std::fs::read_to_string(path) {
+        Ok(contents) if contents.starts_with('\n') => {
+            if let Err(e) = std::fs::write(path, contents.trim_start_matches('\n')) {
+                tracing::warn!(
+                    "Failed to remove leading blank line from {}: {e}",
+                    path.display()
+                );
+            }
+        }
+        Ok(_) => {}
+        Err(e) => {
+            tracing::warn!(
+                "Failed to inspect {} for leading blank line cleanup: {e}",
+                path.display()
+            );
+        }
+    }
+}
+
 /// The host as it appears in the known_hosts entry: `[host]:port` for
 /// non-standard ports, the bare hostname otherwise. Mirrors the convention
 /// `learn_known_hosts_path` writes and `check_known_hosts_path` matches.
@@ -1105,6 +1168,21 @@ mod tests {
         assert!(lines[0].contains("ssh-ed25519"));
     }
 
+    #[tokio::test]
+    async fn test_accept_new_fresh_file_does_not_start_with_blank_line() {
+        let (_dir, path, path_str) = temp_known_hosts();
+        let key = generate_key();
+
+        let result = verify_accept_new("node1.example.com", 22, key.public_key(), &path_str).await;
+        assert!(matches!(result, Ok(true)));
+
+        let contents = std::fs::read_to_string(&path).unwrap();
+        assert!(
+            !contents.starts_with('\n'),
+            "fresh known_hosts must not start with a blank line, got {contents:?}"
+        );
+    }
+
     #[tokio::test]
     async fn test_accept_new_second_connection_does_not_duplicate() {
         let (_dir, path, path_str) = temp_known_hosts();
@@ -1239,6 +1317,29 @@ mod tests {
         assert_eq!(entry_lines(&path), recorded);
     }
 
+    #[tokio::test]
+    async fn test_accept_new_known_match_avoids_file_lock() {
+        let (_dir, path, path_str) = temp_known_hosts();
+        let key = generate_key();
+        std::fs::write(
+            &path,
+            format!(
+                "node1.example.com {}\n",
+                key.public_key().to_openssh().unwrap()
+            ),
+        )
+        .unwrap();
+
+        FILE_LOCK_ACQUISITIONS.lock().unwrap().clear();
+        let result = verify_accept_new("node1.example.com", 22, key.public_key(), &path_str).await;
+        assert!(matches!(result, Ok(true)));
+        let acquisitions = FILE_LOCK_ACQUISITIONS.lock().unwrap();
+        assert!(
+            !acquisitions.iter().any(|path| path == &path_str),
+            "a definite known-host match must return before acquiring the file lock"
+        );
+    }
+
     #[tokio::test]
     async fn test_accept_new_accepts_per_host_key_beside_shared_cluster_entry() {
         let (_dir, path, path_str) = temp_known_hosts();

src/ssh/tokio_client/to_socket_addrs_with_hostname.rs

@@ -102,9 +102,31 @@ impl ToSocketAddrsWithHostname for &[SocketAddr] {
     }
 
     fn hostname(&self) -> String {
-        self.iter()
+        self.first()
             .map(|addr| addr.ip().to_string())
-            .collect::<Vec<_>>()
-            .join(",")
+            .unwrap_or_default()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::ToSocketAddrsWithHostname;
+    use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
+
+    #[test]
+    fn socket_addr_slice_hostname_uses_first_address_only() {
+        let addrs = [
+            SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 22),
+            SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), 22),
+        ];
+
+        assert_eq!(addrs.as_slice().hostname(), "127.0.0.1");
+    }
+
+    #[test]
+    fn empty_socket_addr_slice_hostname_is_empty() {
+        let addrs: [SocketAddr; 0] = [];
+
+        assert_eq!(addrs.as_slice().hostname(), "");
     }
 }

src/test_helpers/env_guard.rs

@@ -67,7 +67,23 @@
 
 #![cfg(test)]
 
-use std::ffi::{OsStr, OsString};
+use std::{
+    ffi::{OsStr, OsString},
+    sync::{Mutex, MutexGuard},
+};
+
+static HOME_ENV_LOCK: Mutex<()> = Mutex::new(());
+
+/// Guard for tests that read `HOME` without mutating it.
+///
+/// `HOME` is process-global state and several tests temporarily repoint it.
+/// Readers that call `dirs::home_dir()` or otherwise inspect `HOME` must hold
+/// the same lock as `EnvGuard::set("HOME", ...)` so parallel test execution
+/// cannot observe two different homes within one assertion.
+#[must_use = "HomeEnvLock must be held for the whole HOME-sensitive assertion"]
+pub struct HomeEnvLock {
+    _guard: MutexGuard<'static, ()>,
+}
 
 /// RAII guard that sets or removes an environment variable on construction
 /// and restores the previous value (or unset state) on drop.
@@ -79,6 +95,7 @@ use std::ffi::{OsStr, OsString};
 pub struct EnvGuard {
     key: OsString,
     original: Option<OsString>,
+    _home_lock: Option<MutexGuard<'static, ()>>,
 }
 
 // `#[allow(dead_code)]` is applied per-method so integration tests that only
@@ -90,6 +107,7 @@ impl EnvGuard {
     #[allow(dead_code)]
     pub fn set(key: impl Into<OsString>, value: impl AsRef<OsStr>) -> Self {
         let key = key.into();
+        let home_lock = lock_home_if_needed(&key);
         let original = std::env::var_os(&key);
         // SAFETY: `#[serial]`-annotated tests that construct `EnvGuard` do not
         // run concurrently with each other, so cross-serial races on the env
@@ -101,20 +119,46 @@ impl EnvGuard {
         unsafe {
             std::env::set_var(&key, value);
         }
-        Self { key, original }
+        Self {
+            key,
+            original,
+            _home_lock: home_lock,
+        }
     }
 
     /// Remove an environment variable, saving its prior value for restoration.
     #[allow(dead_code)]
     pub fn remove(key: impl Into<OsString>) -> Self {
         let key = key.into();
+        let home_lock = lock_home_if_needed(&key);
         let original = std::env::var_os(&key);
         // SAFETY: same rationale as `EnvGuard::set`; see the full comment
         // there and the module-level soundness contract.
         unsafe {
             std::env::remove_var(&key);
         }
-        Self { key, original }
+        Self {
+            key,
+            original,
+            _home_lock: home_lock,
+        }
+    }
+
+    /// Lock the process-global `HOME` environment variable for tests that only
+    /// read it.
+    #[allow(dead_code)]
+    pub fn lock_home() -> HomeEnvLock {
+        HomeEnvLock {
+            _guard: HOME_ENV_LOCK.lock().unwrap(),
+        }
+    }
+}
+
+fn lock_home_if_needed(key: &OsStr) -> Option<MutexGuard<'static, ()>> {
+    if key == OsStr::new("HOME") {
+        Some(HOME_ENV_LOCK.lock().unwrap())
+    } else {
+        None
     }
 }