put: fix ssh_config negation semantics and file precedence

Host pattern lists were evaluated per-pattern with 'any', so
'Host * !*.launchpad.net' matched ppa.launchpad.net via the wildcard;
a block now applies only if a positive pattern matches and no negated
one does (OpenSSH's rule). The system ssh_config was read first with
first-obtained-wins, inverting OpenSSH's user-over-system precedence;
the user file is read first now. A Match block also no longer leaks
the previous Host block's match state (its options are ignored until
the next Host).

src/put/ssh.rs

@@ -53,9 +53,10 @@ fn host_key_is_pinned(host: &str, fingerprint: &str) -> bool {
     })
 }
 
-/// Per-host settings extracted from the SSH configuration files
+/// Per-host settings extracted from the SSH configuration files, read in
-/// (`/etc/ssh/ssh_config`, `~/.ssh/config`). Every option keeps the first
+/// the OpenSSH order (`~/.ssh/config`, then `/etc/ssh/ssh_config`). Every
-/// value obtained from the matching `Host` blocks, like OpenSSH.
+/// option keeps the first value obtained from the matching `Host` blocks,
+/// like OpenSSH: user settings override system ones.
 #[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub struct SshConfig {
     /// `HostName`: real host behind a configured alias.
@@ -68,14 +69,19 @@ pub struct SshConfig {
     pub identity_files: Vec<PathBuf>,
 }
 
-/// Look up `host` in the system and user SSH configuration files. Missing
+/// Look up `host` in the user and system SSH configuration files, in
-/// files yield an empty configuration.
+/// OpenSSH's read order: `~/.ssh/config` first, then
+/// `/etc/ssh/ssh_config` (command line > user > system, first obtained
+/// value wins). Missing files yield an empty configuration.
 pub fn lookup_ssh_config(host: &str) -> SshConfig {
     let mut config = SshConfig::default();
-    let mut files = vec![PathBuf::from("/etc/ssh/ssh_config")];
+    // User file first: with first-obtained-value-wins semantics below, this
+    // makes user settings override the system ones, like OpenSSH
+    let mut files = Vec::new();
     if let Some(home) = std::env::var_os("HOME") {
         files.push(Path::new(&home).join(".ssh/config"));
     }
+    files.push(PathBuf::from("/etc/ssh/ssh_config"));
     for file in files {
         if let Ok(content) = fs::read_to_string(&file) {
             apply_config_file(&mut config, &content, host);
@@ -84,8 +90,13 @@ pub fn lookup_ssh_config(host: &str) -> SshConfig {
     config
 }
 
-/// Fold one configuration file's matching `Host` blocks into `config`,
+/// Fold one configuration file's `Host` blocks matching `host` into
-/// first obtained value wins per option.
+/// `config`, first obtained value wins per option. A block applies iff at
+/// least one positive pattern matches and no negated (`!`-prefixed)
+/// pattern matches — the OpenSSH `Host` rule. `Include` and `Match` are
+/// not supported: a `Match` block's criteria are not evaluated, and it
+/// conservatively stops attributing the following lines to the previous
+/// `Host` block so its options are ignored rather than mis-applied.
 fn apply_config_file(config: &mut SshConfig, content: &str, host: &str) {
     let mut matching = false;
 
@@ -101,9 +112,15 @@ fn apply_config_file(config: &mut SshConfig, content: &str, host: &str) {
         };
 
         if keyword.eq_ignore_ascii_case("host") {
-            matching = rest
+            matching = match_pattern_list(host, rest);
-                .split_whitespace()
+            continue;
-                .any(|pattern| match_pattern(host, pattern));
+        }
+
+        if keyword.eq_ignore_ascii_case("match") {
+            // A conditional block whose criteria we do not evaluate: stop
+            // attributing the following options to the previous `Host`
+            // block, so they are ignored instead of mis-applied
+            matching = false;
             continue;
         }
 
@@ -159,15 +176,20 @@ fn expand_tilde(path: &str) -> PathBuf {
     expand_tilde_with(path, current_home().as_deref())
 }
 
-/// One `Host` block pattern against a host name: exact match, `*` (any
+/// One `Host` block pattern list against a host name, with the OpenSSH
-/// run), `?` (one character) and `!pattern` negation (a matching negated
+/// rule: the block applies iff at least one positive pattern matches AND
-/// pattern excludes the host from the block).
+/// no negated pattern matches — a matching `!pattern` excludes the host
-fn match_pattern(host: &str, pattern: &str) -> bool {
+/// from the whole block, regardless of the positive patterns.
-    if let Some(negated) = pattern.strip_prefix('!') {
+fn match_pattern_list(host: &str, patterns: &str) -> bool {
-        !match_pattern(host, negated)
+    let mut any_positive = false;
-    } else {
+    let mut any_negated = false;
-        wildmatch(host, pattern)
+    for pattern in patterns.split_whitespace() {
+        match pattern.strip_prefix('!') {
+            Some(negated) => any_negated |= wildmatch(host, negated),
+            None => any_positive |= wildmatch(host, pattern),
+        }
     }
+    any_positive && !any_negated
 }
 
 /// `*`/`?` glob matching without allocation, enough for `Host` patterns
@@ -567,16 +589,80 @@ mod tests {
         ));
     }
 
+    /// 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.
     #[test]
     fn pattern_negation_excludes_host() {
-        assert!(!match_pattern("ppa.launchpad.net", "!*.launchpad.net"));
+        // The classic catch-all-with-exception: `*` alone would match, but
-        // A negated pattern only excludes; the other patterns of the block
+        // the negated pattern must exclude the host from the whole block
-        // decide separately
+        let config = apply_config_file_all(
-        assert!(match_pattern("example.com", "!*.launchpad.net example.com"));
+            "\
-        assert!(!match_pattern(
+Host * !*.launchpad.net
+    User fallback
+",
+            "ppa.launchpad.net",
+        );
+        assert_eq!(config, SshConfig::default());
+
+        // A negation with no positive pattern never matches either
+        assert!(!match_pattern_list("ppa.launchpad.net", "!*.launchpad.net"));
+        // A matching negation excludes despite a positive match...
+        assert!(!match_pattern_list(
             "ppa.launchpad.net",
             "example.com !*.launchpad.net"
         ));
+        // ...while an unrelated negation leaves the positive match intact
+        assert!(match_pattern_list("ppa.launchpad.net", "* !*.example.com"));
+    }
+
+    /// OpenSSH's precedence: the user file is read before the system file
+    /// and the first value obtained wins, so the second (system) pass must
+    /// not override values the first (user) pass already obtained — while
+    /// options the user file leaves unset are still taken from the system
+    /// file.
+    #[test]
+    fn user_config_overrides_system() {
+        let user = "\
+Host ppa.launchpad.net
+    User myuser
+    Port 2222
+";
+        let system = "\
+Host *
+    User login
+    Port 22
+    HostName system.example.com
+";
+
+        let mut config = SshConfig::default();
+        apply_config_file(&mut config, user, "ppa.launchpad.net"); // ~/.ssh/config
+        apply_config_file(&mut config, system, "ppa.launchpad.net"); // /etc/ssh/ssh_config
+
+        assert_eq!(config.user.as_deref(), Some("myuser"));
+        assert_eq!(config.port, Some(2222));
+        assert_eq!(config.host_name.as_deref(), Some("system.example.com"));
+    }
+
+    /// `Match` blocks are not evaluated: their options must be ignored, not
+    /// mis-attributed to the preceding `Host` block through a stale
+    /// matching flag.
+    #[test]
+    fn match_block_is_ignored_conservatively() {
+        let config = apply_config_file_all(
+            "\
+Host ppa.launchpad.net
+    User myuser
+Match final all
+    Port 2223
+    IdentityFile ~/.ssh/matched_key
+",
+            "ppa.launchpad.net",
+        );
+
+        assert_eq!(config.user.as_deref(), Some("myuser"));
+        assert_eq!(config.port, None);
+        assert!(config.identity_files.is_empty());
     }
 
     #[test]