fix: locale-safe metrics, ssh_config Include resolution, and reachability-only monitoring

CHANGELOG.md

@@ -9,7 +9,13 @@ Versions follow [Semantic Versioning](https://semver.org/).
 
 ## Unreleased
 
+### Features
+- **Watch a host by a port check instead of an SSH login.** Firewalls, switches and other appliances answer SSH but have no shell to read `top` or `free` from, so monitoring could only ever fail on them — and log in again every cycle to find that out. A host can now be set to **TCP port check**: OmnySSH opens a connection to the port and closes it, with no login and no commands. Its card shows reachable / unreachable in place of the metric tiles, rather than tiles for numbers nobody collected. Set it in the host form — `tcp` (the host's SSH port) or `tcp:PORT` in the terminal app, a dropdown in the desktop app. Existing hosts are untouched and stay on SSH monitoring. ICMP is not offered yet: unprivileged ping is unavailable on the Linux packaging most people use.
+
 ### Bug Fixes
+- **Hosts split across `Include` files are imported.** A relative pattern — `Include conf.d/*.conf`, the form nearly every split-config guide prints — was looked for in whatever directory the app happened to be launched from, which for the desktop app started from Finder or the application menu is `/`. Nothing matched, so every host defined in `~/.ssh/conf.d` was missing and had to be added by hand. Those patterns now resolve against `~/.ssh`, the way `ssh` itself resolves them. Four things in the same code path were fixed alongside it: full glob patterns work (`?`, `[abc]`, a wildcard in a directory name, and more than one `*` in a file name), several pathnames on one `Include` line are all read instead of none, a quoted path keeps its spaces, and an `Include` written inside a `Host` block no longer swallows that host's remaining settings. An `Include` that matches nothing is written to the log rather than passing in silence.
+- **CPU no longer reads ~91–100 % on an idle server whose system language isn't English.** Monitoring reads the idle percentage from `top` and shows the rest as used. On a server set to a language with a comma decimal separator, `top` prints `99,1 id`, and the parser split that line on commas — reading the idle value as `1` and reporting 99 % used on a machine doing nothing. The same servers could show RAM and Disk as N/A, because `free` and `df` translate the labels being looked for, and the process list could come back empty. The monitoring commands now run in a fixed locale, and the CPU parsers understand a decimal comma on their own for hosts where that cannot be set. Process names keep the server's own character set.
+- **A host that answers SSH but not shell commands is no longer re-logged-in every 30 seconds.** Network appliances — firewalls, switches — authenticate fine but cannot run `top` or `free`, and a round of failed metric commands was treated as a dead connection. The retry delay was reset on every successful login, so it never grew past its first step: one login every 30 seconds, indefinitely, and as often as every 10 seconds in the desktop app, whose refresh timer also cut the retry delay short. The delay now escalates properly and is left alone by the refresh timer — the trade-off being that a manual refresh no longer cuts a retry delay short, so a host that has been unreachable for a while retries on its own schedule (at most five minutes). Separately, a device that ignores SSH keepalives was being disconnected after 30 seconds even while its commands still worked; connections are now held open, with liveness still bounded by the keepalive limit.
 - **`ProxyJump` hosts now connect through their bastion.** A host with `ProxyJump` in `~/.ssh/config` was parsed but never routed: the terminal refused to open it, while metrics, SFTP, snippets and key setup quietly dialled the target address direct — which for an internal host meant every connection failed or, worse, landed somewhere else on that address. Every native SSH path now walks the jump chain the way `ssh -J` does, connecting and authenticating each bastion in turn and tunnelling the next hop over it.
   - The jump alias is resolved against your host list, so `ProxyJump public-proxy` picks up that entry's `HostName`, `User`, `Port` and `IdentityFile`. An alias that matches no entry is used as a literal hostname.
   - Multi-hop values (`ProxyJump first,second`), inline `user@host:port` overrides, IPv6 literals, bastions that are themselves behind a bastion, and the `ProxyJump none` opt-out all behave as OpenSSH does — including its precedence rule that only the first hop of a list contributes bastions of its own. Chains that loop, or run past ten hops, are reported instead of hanging.

crates/omnyssh-core/Cargo.toml

@@ -36,6 +36,9 @@ chrono = { workspace = true }
 # Cross-platform paths (P-CROSS-01)
 dirs = { workspace = true }
 
+# glob(7) patterns in ssh_config `Include` directives
+glob = "0.3"
+
 # Terminal emulator — vt100 screen model (PTY is provided by russh channels)
 vt100 = { package = "vt100-omnyssh", version = "0.15" }
 
@@ -49,3 +52,4 @@ self-replace = "1"
 
 [dev-dependencies]
 tokio = { workspace = true, features = ["full", "test-util"] }
+tempfile = "3"

crates/omnyssh-core/src/config/ssh_config.rs

@@ -13,10 +13,12 @@ use crate::ssh::client::{Host, HostSource};
 /// Parses the text of an SSH config file and returns all non-wildcard hosts.
 ///
 /// `Host *` entries are silently skipped.
-/// `Include` recursion is limited to 3 levels to prevent cycles.
+/// `Include` recursion is limited to 3 levels to prevent cycles; relative
+/// patterns resolve against `~/.ssh`, as `ssh_config(5)` specifies for a user
+/// configuration.
 pub fn parse_ssh_config(content: &str) -> Vec<Host> {
     let mut visited: HashSet<PathBuf> = HashSet::new();
-    parse_content(content, 0, &mut visited)
+    parse_content(content, default_include_base().as_deref(), 0, &mut visited)
 }
 
 /// Loads and parses an SSH config file from disk.
@@ -26,20 +28,39 @@ pub fn parse_ssh_config(content: &str) -> Vec<Host> {
 pub fn load_from_file(path: &Path) -> anyhow::Result<Vec<Host>> {
     let content = std::fs::read_to_string(path)
         .map_err(|e| anyhow::anyhow!("Failed to read {}: {}", path.display(), e))?;
-    Ok(parse_ssh_config(&content))
+    let base = path
+        .parent()
+        .filter(|p| !p.as_os_str().is_empty())
+        .map_or_else(default_include_base, |p| Some(p.to_path_buf()));
+    let mut visited: HashSet<PathBuf> = HashSet::new();
+    Ok(parse_content(&content, base.as_deref(), 0, &mut visited))
+}
+
+/// `~/.ssh` — where `ssh_config(5)` resolves a relative `Include` in a user
+/// configuration. Never the process working directory.
+fn default_include_base() -> Option<PathBuf> {
+    dirs::home_dir().map(|h| h.join(".ssh"))
 }
 
 // ---------------------------------------------------------------------------
 // Internal helpers
 // ---------------------------------------------------------------------------
 
-fn parse_content(content: &str, depth: usize, visited: &mut HashSet<PathBuf>) -> Vec<Host> {
+fn parse_content(
+    content: &str,
+    base: Option<&Path>,
+    depth: usize,
+    visited: &mut HashSet<PathBuf>,
+) -> Vec<Host> {
     if depth > 3 {
         return Vec::new();
     }
 
     let mut hosts: Vec<Host> = Vec::new();
     let mut current: Option<Host> = None;
+    // Hosts pulled in by an Include that sat inside a Host block. Appended once
+    // the enclosing host is flushed, so the list keeps the config's own order.
+    let mut deferred: Vec<Host> = Vec::new();
     // True when we are inside a wildcard `Host *` block (skip directives).
     let mut in_wildcard = false;
 
@@ -59,6 +80,7 @@ fn parse_content(content: &str, depth: usize, visited: &mut HashSet<PathBuf>) ->
                 if let Some(h) = current.take() {
                     hosts.push(h);
                 }
+                hosts.append(&mut deferred);
                 in_wildcard = value.contains('*') || value.contains('?');
                 if !in_wildcard {
                     let h = Host {
@@ -98,30 +120,41 @@ fn parse_content(content: &str, depth: usize, visited: &mut HashSet<PathBuf>) ->
                     h.proxy_jump = Some(value.to_string());
                 }
             }
+            // An Include may sit inside a Host block; the enclosing host keeps
+            // collecting directives after it.
             "include" => {
-                // Flush the current host before processing Include.
-                if let Some(h) = current.take() {
-                    hosts.push(h);
-                }
-                in_wildcard = false;
-                let expanded = expand_tilde(value);
-                for path in expand_include_glob(&expanded) {
-                    // Canonicalise to catch cycles (symlinks, etc.).
-                    let canonical = path.canonicalize().unwrap_or_else(|e| {
-                        tracing::warn!(
-                            path = %path.display(),
-                            error = %e,
-                            "canonicalize failed; symlink-cycle detection disabled for this path"
-                        );
-                        path.clone()
-                    });
-                    if !visited.insert(canonical) {
-                        continue; // already visited — break cycle
+                let sink = if current.is_some() {
+                    &mut deferred
+                } else {
+                    &mut hosts
+                };
+                for pattern in split_include_patterns(value) {
+                    let Some(resolved) = resolve_include(&pattern, base) else {
+                        tracing::warn!(pattern, "relative Include with no home directory");
+                        continue;
+                    };
+                    let matched = expand_include_glob(&resolved);
+                    if matched.is_empty() {
+                        tracing::warn!(pattern = resolved, "Include matched no files");
                     }
-                    match std::fs::read_to_string(&path) {
-                        Ok(sub) => hosts.extend(parse_content(&sub, depth + 1, visited)),
-                        Err(e) => {
-                            tracing::warn!(path = %path.display(), error = %e, "Include file unreadable")
+                    for path in matched {
+                        // Canonicalise to catch cycles (symlinks, etc.).
+                        let canonical = path.canonicalize().unwrap_or_else(|e| {
+                            tracing::warn!(
+                                path = %path.display(),
+                                error = %e,
+                                "canonicalize failed; symlink-cycle detection disabled for this path"
+                            );
+                            path.clone()
+                        });
+                        if !visited.insert(canonical) {
+                            continue; // already visited — break cycle
+                        }
+                        match std::fs::read_to_string(&path) {
+                            Ok(sub) => sink.extend(parse_content(&sub, base, depth + 1, visited)),
+                            Err(e) => {
+                                tracing::warn!(path = %path.display(), error = %e, "Include file unreadable")
+                            }
                         }
                     }
                 }
@@ -130,10 +163,11 @@ fn parse_content(content: &str, depth: usize, visited: &mut HashSet<PathBuf>) ->
         }
     }
 
-    // Flush the last pending host.
+    // Flush the last pending host, then anything its Include pulled in.
     if let Some(h) = current.take() {
         hosts.push(h);
     }
+    hosts.append(&mut deferred);
 
     // Fallback: if HostName was never set, use the alias as the address.
     for h in &mut hosts {
@@ -181,48 +215,61 @@ fn expand_tilde(s: &str) -> String {
     s.to_string()
 }
 
-/// Resolves an Include pattern to a list of file paths.
+/// Splits an `Include` value into its pathnames.
 ///
-/// Supports a single `*` wildcard in the file-name component only.
-/// The parent directory must exist; `*` in path segments other than the
-/// last one is not supported (matches OpenSSH behaviour).
-fn expand_include_glob(pattern: &str) -> Vec<PathBuf> {
-    let path = PathBuf::from(pattern);
-    let parent = match path.parent() {
-        Some(p) if !p.as_os_str().is_empty() => p.to_path_buf(),
-        _ => PathBuf::from("."),
-    };
-    let file_name = match path.file_name().and_then(|f| f.to_str()) {
-        Some(n) => n.to_string(),
-        None => return Vec::new(),
-    };
-
-    if !file_name.contains('*') {
-        return if path.is_file() {
-            vec![path]
-        } else {
-            Vec::new()
-        };
+/// `ssh_config(5)` allows several pathnames on one line, and a path containing
+/// spaces may be double-quoted.
+fn split_include_patterns(value: &str) -> Vec<String> {
+    let mut patterns = Vec::new();
+    let mut current = String::new();
+    let mut quoted = false;
+
+    for c in value.chars() {
+        match c {
+            '"' => quoted = !quoted,
+            c if c.is_whitespace() && !quoted => {
+                if !current.is_empty() {
+                    patterns.push(std::mem::take(&mut current));
+                }
+            }
+            c => current.push(c),
+        }
+    }
+    if !current.is_empty() {
+        patterns.push(current);
+    }
+    patterns
+}
+
+/// Anchors an `Include` pattern: absolute and `~/` patterns stand alone, a
+/// relative one resolves against `base` — never the process working directory.
+///
+/// `None` when the pattern is relative and there is no base to anchor it to;
+/// dropping the include is the only honest option, since resolving it would
+/// silently read the process working directory.
+fn resolve_include(pattern: &str, base: Option<&Path>) -> Option<String> {
+    let expanded = expand_tilde(pattern);
+    if Path::new(&expanded).is_absolute() {
+        return Some(expanded);
     }
+    // The base is a real path, not a pattern: escape it so a home directory
+    // containing `[` or `*` cannot swallow the include.
+    let base = glob::Pattern::escape(base?.to_str()?);
+    Some(format!("{}/{}", base.trim_end_matches('/'), expanded))
+}
 
-    // Simple single-`*` glob: match prefix and suffix.
-    let (prefix, suffix) = file_name.split_once('*').unwrap_or((&file_name, ""));
-    match std::fs::read_dir(&parent) {
-        Ok(entries) => {
-            let mut paths: Vec<PathBuf> = entries
-                .flatten()
-                .filter(|e| {
-                    let name = e.file_name();
-                    let name = name.to_string_lossy();
-                    name.starts_with(prefix) && name.ends_with(suffix)
-                })
-                .map(|e| e.path())
-                .filter(|p| p.is_file())
-                .collect();
-            paths.sort(); // deterministic order
-            paths
+/// Resolves an Include pattern to the files it matches, in a stable order.
+///
+/// Full glob(7) syntax, as `ssh_config(5)` specifies. Directories that match
+/// are skipped.
+fn expand_include_glob(pattern: &str) -> Vec<PathBuf> {
+    match glob::glob(pattern) {
+        // `glob` yields matches in sorted order, so the host list is stable.
+        Ok(paths) => paths.flatten().filter(|p| p.is_file()).collect(),
+        Err(e) => {
+            tracing::warn!(pattern, error = %e, "invalid Include pattern");
+            Vec::new()
         }
-        Err(_) => Vec::new(),
     }
 }
 

crates/omnyssh-core/src/ssh/client.rs

@@ -16,6 +16,28 @@ pub enum HostSource {
     Manual,
 }
 
+/// How a host is watched on the dashboard.
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
+#[serde(rename_all = "snake_case")]
+pub enum MonitorMode {
+    /// A live SSH session with shell metrics — the default.
+    #[default]
+    Ssh,
+    /// A plain TCP connect, with no login. For devices that answer SSH but have
+    /// no POSIX shell to collect metrics from, such as network appliances.
+    // The wire and the TUI form spell this differently; accept both, because a
+    // rejected value fails the whole file and takes every manual host with it.
+    #[serde(alias = "tcp", alias = "tcpPort")]
+    TcpPort,
+}
+
+impl MonitorMode {
+    /// Lets the default stay out of `hosts.toml` entirely.
+    fn is_ssh(&self) -> bool {
+        matches!(self, Self::Ssh)
+    }
+}
+
 /// A single host entry used for SSH connections.
 ///
 /// Populated either from `~/.ssh/config` (via the parser) or from
@@ -54,6 +76,12 @@ pub struct Host {
     /// Used to prevent duplicate entries when a SSH-config host is renamed.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub original_ssh_host: Option<String>,
+    /// How this host is watched.
+    #[serde(default, skip_serializing_if = "MonitorMode::is_ssh")]
+    pub monitoring: MonitorMode,
+    /// Port for the reachability probe. Falls back to `port` when unset.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub monitor_port: Option<u16>,
 
     // -----------------------------------------------------------------------
     // Auto SSH Key Setup metadata
@@ -90,6 +118,8 @@ impl Default for Host {
             notes: None,
             source: HostSource::default(),
             original_ssh_host: None,
+            monitoring: MonitorMode::default(),
+            monitor_port: None,
             key_setup_date: None,
             password_auth_disabled: None,
         }
@@ -109,3 +139,54 @@ pub enum ConnectionStatus {
     /// The last connection attempt failed with the given message.
     Failed(String),
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// An existing `hosts.toml` predates the monitoring fields, and writing one
+    /// back must not add them — the file has to stay byte-identical for a host
+    /// nobody changed.
+    #[test]
+    fn the_monitoring_default_round_trips_without_touching_the_file() {
+        let host: Host = toml::from_str("name = \"web\"\nhostname = \"10.0.0.1\"\n")
+            .expect("a host without the monitoring fields still parses");
+        assert_eq!(host.monitoring, MonitorMode::Ssh);
+        assert_eq!(host.monitor_port, None);
+
+        let written = toml::to_string(&host).expect("serialize");
+        assert!(!written.contains("monitoring"), "{written}");
+        assert!(!written.contains("monitor_port"), "{written}");
+    }
+
+    /// The wire and the TUI form spell the mode differently, so a hand-edited
+    /// file is likely to carry either — and a rejected value fails the whole
+    /// file, taking every manual host with it.
+    #[test]
+    fn the_other_spellings_of_the_mode_are_accepted() {
+        for spelling in ["tcp_port", "tcp", "tcpPort"] {
+            let host: Host = toml::from_str(&format!(
+                "name = \"fw\"\nhostname = \"10.0.0.9\"\nmonitoring = \"{spelling}\"\n"
+            ))
+            .unwrap_or_else(|e| panic!("'{spelling}' should parse: {e}"));
+            assert_eq!(host.monitoring, MonitorMode::TcpPort);
+        }
+    }
+
+    #[test]
+    fn a_reachability_host_persists_its_mode_and_probe_port() {
+        let host = Host {
+            name: String::from("fw"),
+            hostname: String::from("10.0.0.9"),
+            monitoring: MonitorMode::TcpPort,
+            monitor_port: Some(8443),
+            ..Host::default()
+        };
+
+        let written = toml::to_string(&host).expect("serialize");
+        let read: Host = toml::from_str(&written).expect("deserialize");
+
+        assert_eq!(read.monitoring, MonitorMode::TcpPort);
+        assert_eq!(read.monitor_port, Some(8443));
+    }
+}