put: surface known_hosts problems instead of silently degrading

An unreadable or unparsable known_hosts file was swallowed with
'let _', silently downgrading to prompt-and-accept without telling the
user why their configuration was ignored: warn naming the file, then
continue. And when the pinned Launchpad fingerprint matches, a
DIFFERENT key recorded for that host in known_hosts was silently
bypassed: warn about the stale entry (diagnostic only — the published
fingerprint stays authoritative).

src/put/ssh.rs

@@ -25,7 +25,7 @@ use lazy_static::lazy_static;
 use log::debug;
 use serde::Deserialize;
 use sha2::{Digest, Sha256};
-use ssh2::{CheckResult, HostKeyType, KnownHostFileKind, Session};
+use ssh2::{CheckResult, HostKeyType, KnownHostFileKind, KnownHosts, Session};
 
 use crate::ui::prompt;
 
@@ -346,7 +346,9 @@ pub fn connect(
 /// Check the server's host key, in decreasing order of trust:
 ///
 /// 1. matching one of the host's pinned (published, `host_keys.yml`)
-///    fingerprints — accepted silently;
+///    fingerprints — accepted (with a diagnostic warning when the user's
+///    known_hosts records a *different* key for the host, so a stale
+///    entry gets cleaned up; the pin stays authoritative);
 /// 2. matching `~/.ssh/known_hosts` — accepted;
 /// 3. known-and-different — refused loudly (possible man-in-the-middle);
 /// 4. unknown — fingerprint shown, explicit confirmation required, and on
@@ -362,21 +364,16 @@ fn verify_host_key(
 
     if host_key_is_pinned(host, &fingerprint) {
         debug!("{host} host key matches the published fingerprint");
+        // Purely diagnostic — the pin already decided — but a known_hosts
+        // entry disagreeing with the published key is worth surfacing
+        log_known_hosts_discrepancy(host, port, key);
         return Ok(());
     }
 
     let mut known_hosts = Session::new()?.known_hosts()?;
-    for file in known_hosts_files() {
+    load_known_hosts(&mut known_hosts, &known_hosts_files());
-        // Unreadable/unknown-format lines are skipped by libssh2; a missing
-        // file is fine
-        let _ = known_hosts.read_file(&file, KnownHostFileKind::OpenSSH);
-    }
 
-    let check = if port == 22 {
+    let check = check_known_hosts(&known_hosts, host, port, key);
-        known_hosts.check(host, key)
-    } else {
-        known_hosts.check_port(host, port, key)
-    };
 
     match check {
         CheckResult::Match => Ok(()),
@@ -414,6 +411,76 @@ fn verify_host_key(
     }
 }
 
+/// Read `files` into `known_hosts`: silently skips missing files (the
+/// normal first-use case — there is nothing to consult), warns about a
+/// file that exists but cannot be read or parsed, naming the file and the
+/// error — the user should know why their maintained entries are not being
+/// consulted — and skips it. Unreadable/unknown-format *lines* are skipped
+/// by libssh2 itself. Returns the emitted warnings, so the decision is
+/// testable without capturing log output.
+fn load_known_hosts(known_hosts: &mut KnownHosts, files: &[PathBuf]) -> Vec<String> {
+    let mut warnings = Vec::new();
+    for file in files {
+        if !file.exists() {
+            continue;
+        }
+        if let Err(e) = known_hosts.read_file(file, KnownHostFileKind::OpenSSH) {
+            let warning = format!(
+                "cannot read the known hosts file {}: {e} — its entries are ignored",
+                file.display()
+            );
+            log::warn!("{warning}");
+            warnings.push(warning);
+        }
+    }
+    warnings
+}
+
+/// Look up `host`'s `key` in the loaded entries, using the `[host]:port`
+/// spelling for non-standard ports (the known_hosts encoding)
+fn check_known_hosts(known_hosts: &KnownHosts, host: &str, port: u16, key: &[u8]) -> CheckResult {
+    if port == 22 {
+        known_hosts.check(host, key)
+    } else {
+        known_hosts.check_port(host, port, key)
+    }
+}
+
+/// What to tell the user when the server's key matched a pinned (published)
+/// fingerprint while their known_hosts records a different key for the same
+/// host — a stale entry, or worse an old compromise artifact they should
+/// clean up; `None` for every other check result, since the pin is
+/// authoritative and needs no corroboration. Pure so the decision and the
+/// wording are testable without a server or a filesystem.
+fn known_hosts_discrepancy(host: &str, check: CheckResult) -> Option<String> {
+    match check {
+        CheckResult::Mismatch => Some(format!(
+            "the host key of {host} matches Launchpad's published \
+             fingerprint, but your known_hosts file(s) record a DIFFERENT \
+             key for it — proceeding on the published fingerprint; consider \
+             removing the stale '{host}' entry from your known_hosts"
+        )),
+        CheckResult::Match | CheckResult::NotFound | CheckResult::Failure => None,
+    }
+}
+
+/// Diagnostic-only companion to the pinned-fingerprint acceptance: load the
+/// user's known hosts files and warn when they record a different key for
+/// `host` than the pinned one about to be accepted. The pin stays
+/// authoritative: any failure here is tolerated and the connection
+/// proceeds regardless.
+fn log_known_hosts_discrepancy(host: &str, port: u16, key: &[u8]) {
+    let Ok(mut known_hosts) = Session::new().and_then(|session| session.known_hosts()) else {
+        return;
+    };
+    load_known_hosts(&mut known_hosts, &known_hosts_files());
+
+    let check = check_known_hosts(&known_hosts, host, port, key);
+    if let Some(warning) = known_hosts_discrepancy(host, check) {
+        log::warn!("{warning}");
+    }
+}
+
 /// Known hosts files to consult, user file first (so that new keys are
 /// accepted because of the user file, mirroring ssh's own ordering)
 fn known_hosts_files() -> Vec<PathBuf> {
@@ -686,6 +753,99 @@ mod tests {
         ));
     }
 
+    /// A pinned-key host whose known_hosts entries record a DIFFERENT key
+    /// is flagged with a message naming the host; every other check result
+    /// stays silent — the published fingerprint remains authoritative.
+    #[test]
+    fn known_hosts_discrepancy_flags_mismatch_only() {
+        let message = known_hosts_discrepancy("ppa.launchpad.net", CheckResult::Mismatch)
+            .expect("a mismatch under a matching pin must be flagged");
+        assert!(
+            message.contains("ppa.launchpad.net"),
+            "unexpected: {message}"
+        );
+        assert!(message.contains("DIFFERENT"), "unexpected: {message}");
+        assert!(
+            message.contains("published fingerprint"),
+            "unexpected: {message}"
+        );
+        assert!(
+            message.contains("stale"),
+            "the user must be told to clean the entry up: {message}"
+        );
+
+        for quiet in [
+            CheckResult::Match,
+            CheckResult::NotFound,
+            CheckResult::Failure,
+        ] {
+            assert!(
+                known_hosts_discrepancy("ppa.launchpad.net", quiet).is_none(),
+                "{quiet:?} must not be flagged"
+            );
+        }
+    }
+
+    /// Known hosts loading decisions: a missing file is the normal
+    /// first-use case and stays silent, a readable file is loaded and its
+    /// entries consulted, and an existing but unreadable file yields a
+    /// warning naming it — the user must learn why their maintained
+    /// entries are not being consulted. Offline: a bare session suffices
+    /// for the known-hosts store.
+    #[test]
+    fn load_known_hosts_warns_only_on_unreadable_files() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = tempfile::tempdir().unwrap();
+        let mut known_hosts = Session::new().unwrap().known_hosts().unwrap();
+
+        // A missing file is silent...
+        let missing = dir.path().join("known_hosts");
+        assert!(load_known_hosts(&mut known_hosts, &[missing]).is_empty());
+
+        // ...a readable one is loaded and consulted...
+        let key = b"a test host key";
+        let loaded = dir.path().join("loaded_known_hosts");
+        fs::write(
+            &loaded,
+            format!("host.example.com ssh-ed25519 {}\n", base64_nopad(key)),
+        )
+        .unwrap();
+        assert!(
+            load_known_hosts(&mut known_hosts, &[loaded]).is_empty(),
+            "a readable file must not warn"
+        );
+        assert!(
+            matches!(
+                check_known_hosts(&known_hosts, "host.example.com", 22, key),
+                CheckResult::Match
+            ),
+            "the loaded entry must be consulted"
+        );
+
+        // ...and an existing but unreadable one is reported, naming the
+        // file. Environments that read through the mode bits (root) cannot
+        // exercise the unreadable case and are skipped.
+        let locked = dir.path().join("locked_known_hosts");
+        fs::write(&locked, b"stale.example.com ssh-ed25519 c3RhbGU=").unwrap();
+        let mut permissions = fs::metadata(&locked).unwrap().permissions();
+        permissions.set_mode(0o000);
+        fs::set_permissions(&locked, permissions).unwrap();
+        if fs::read(&locked).is_ok() {
+            return;
+        }
+        let warnings = load_known_hosts(&mut known_hosts, &[locked]);
+        assert_eq!(warnings.len(), 1, "unexpected: {warnings:?}");
+        assert!(
+            warnings[0].contains("locked_known_hosts"),
+            "the warning must name the file: {warnings:?}"
+        );
+        assert!(
+            warnings[0].contains("ignored"),
+            "the warning must say the entries are not consulted: {warnings:?}"
+        );
+    }
+
     /// Negated `Host` patterns exclude the host from the whole block (the
     /// OpenSSH rule): a block applies iff at least one positive pattern
     /// matches AND no negated pattern matches.