MCP v2 PR-4: add_host + delete_host + extraArgs sanitiser + third SSH safeguard

Final v2 PR. All 11 planned write tools live. add_host/delete_host let
Claude mutate the saved-hosts list; both gated by a new allowAddHost
switch (default off) — symmetric with the allowOpenSsh gate from PR-3.5.

add_host's extraArgs are sanitised against CVE-2023-51385-class
local-RCE primitives: ProxyCommand, LocalCommand, KnownHostsCommand,
PermitLocalCommand=yes are refused server-side. Recognises both -o KEY=VAL
and -oKEY=VAL, case-insensitive on the key. The manual host manager UI
stays unrestricted (user has full agency over their own hosts).

Also fixes a pre-existing compile bug: mcp_policy.rs's policy_with test
helper was missing the ssh_safeguards field added in PR-3.5, silently
breaking the entire policy test module since then. Re-enabling those
tests is the prereq for the hard-deny rework that follows in the next
commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src-tauri/src/hosts.rs

@@ -84,3 +84,208 @@ pub fn save(app: &AppHandle, hosts: &[SshHost]) -> Result<()> {
     std::fs::rename(&tmp, &path).context("rename hosts.json")?;
     Ok(())
 }
+
+/// Reject `-o` options that would let an attacker turn an SSH connect into
+/// local command execution. CVE-2023-51385 class — `ProxyCommand`,
+/// `LocalCommand`, `KnownHostsCommand`, and `PermitLocalCommand=yes` are all
+/// shell-invocation primitives that fire on `ssh.exe` startup regardless of
+/// what happens on the remote side. The MCP `add_host` tool runs this on
+/// any extraArgs Claude tries to save; the host manager UI is unrestricted
+/// since the user has full agency over manually-typed hosts.
+///
+/// Recognises both `-o KEY=VAL` (two args) and `-oKEY=VAL` (joined),
+/// case-insensitive on the key. Returns Ok on safe args; Err with the
+/// offending arg + a human-readable reason otherwise.
+pub fn sanitize_extra_args(args: &[String]) -> Result<(), String> {
+    let mut i = 0;
+    while i < args.len() {
+        let arg = &args[i];
+        if arg == "-o" {
+            if let Some(next) = args.get(i + 1) {
+                if let Some(reason) = check_o_value(next) {
+                    return Err(format!("rejected '-o {next}': {reason}"));
+                }
+            }
+            i += 2;
+            continue;
+        }
+        if let Some(rest) = arg.strip_prefix("-o") {
+            if let Some(reason) = check_o_value(rest) {
+                return Err(format!("rejected '{arg}': {reason}"));
+            }
+        }
+        i += 1;
+    }
+    Ok(())
+}
+
+/// Inspect an `-o` payload (the part after `-o`, e.g. `ProxyCommand=...`
+/// or `ProxyCommand ...`). Returns Some(reason) if the key is one of the
+/// command-execution primitives; None for everything else.
+fn check_o_value(spec: &str) -> Option<&'static str> {
+    let split = spec
+        .find(|c: char| c == '=' || c.is_whitespace())
+        .unwrap_or(spec.len());
+    let key = &spec[..split];
+    let value = spec[split..].trim_start_matches(|c: char| c == '=' || c.is_whitespace());
+    match key.to_ascii_lowercase().as_str() {
+        "proxycommand" => {
+            Some("ProxyCommand runs a shell command on connect (local RCE primitive)")
+        }
+        "localcommand" => {
+            Some("LocalCommand runs a shell command on connect (local RCE primitive)")
+        }
+        "knownhostscommand" => Some(
+            "KnownHostsCommand runs a shell command at handshake (CVE-2023-51385 class)",
+        ),
+        "permitlocalcommand" if value.eq_ignore_ascii_case("yes") => {
+            Some("PermitLocalCommand=yes enables LocalCommand RCE")
+        }
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn args(a: &[&str]) -> Vec<String> {
+        a.iter().map(|s| s.to_string()).collect()
+    }
+
+    // ---- positive cases (safe, must pass) ---------------------------------
+
+    #[test]
+    fn empty_args_ok() {
+        assert!(sanitize_extra_args(&[]).is_ok());
+    }
+
+    #[test]
+    fn server_alive_interval_ok() {
+        assert!(sanitize_extra_args(&args(&["-o", "ServerAliveInterval=30"])).is_ok());
+    }
+
+    #[test]
+    fn server_alive_interval_joined_ok() {
+        assert!(sanitize_extra_args(&args(&["-oServerAliveInterval=30"])).is_ok());
+    }
+
+    #[test]
+    fn batch_mode_ok() {
+        assert!(sanitize_extra_args(&args(&["-o", "BatchMode=yes"])).is_ok());
+    }
+
+    #[test]
+    fn strict_host_key_checking_ok() {
+        assert!(
+            sanitize_extra_args(&args(&["-o", "StrictHostKeyChecking=accept-new"])).is_ok()
+        );
+    }
+
+    #[test]
+    fn permit_local_command_no_ok() {
+        // PermitLocalCommand=no (or anything other than yes) is the default
+        // and harmless.
+        assert!(sanitize_extra_args(&args(&["-o", "PermitLocalCommand=no"])).is_ok());
+    }
+
+    #[test]
+    fn flag_without_o_ok() {
+        // -F /tmp/conf and -i ~/.ssh/key are legitimate ssh flags; we only
+        // gate -o options.
+        assert!(sanitize_extra_args(&args(&["-v", "-F", "/etc/ssh/ssh_config"])).is_ok());
+    }
+
+    #[test]
+    fn many_safe_options_ok() {
+        assert!(sanitize_extra_args(&args(&[
+            "-o", "ServerAliveInterval=30",
+            "-o", "ServerAliveCountMax=3",
+            "-o", "Compression=yes",
+        ]))
+        .is_ok());
+    }
+
+    // ---- negative cases (must reject) -------------------------------------
+
+    #[test]
+    fn proxy_command_rejected() {
+        let err = sanitize_extra_args(&args(&["-o", "ProxyCommand=ssh evil exec %h %p"]))
+            .unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn proxy_command_joined_rejected() {
+        let err = sanitize_extra_args(&args(&["-oProxyCommand=nc evil 22"])).unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn proxy_command_lowercase_rejected() {
+        // SSH treats -o keys case-insensitively; sanitiser must too.
+        let err = sanitize_extra_args(&args(&["-o", "proxycommand=evil"])).unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn proxy_command_mixed_case_rejected() {
+        let err = sanitize_extra_args(&args(&["-o", "PROXYCommand=evil"])).unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn proxy_command_space_separated_rejected() {
+        // -o supports both KEY=VAL and "KEY VAL" forms.
+        let err =
+            sanitize_extra_args(&args(&["-o", "ProxyCommand /bin/evil"])).unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn local_command_rejected() {
+        let err =
+            sanitize_extra_args(&args(&["-o", "LocalCommand=rm -rf /"])).unwrap_err();
+        assert!(err.contains("LocalCommand"), "err={err}");
+    }
+
+    #[test]
+    fn local_command_joined_rejected() {
+        let err = sanitize_extra_args(&args(&["-oLocalCommand=evil"])).unwrap_err();
+        assert!(err.contains("LocalCommand"), "err={err}");
+    }
+
+    #[test]
+    fn known_hosts_command_rejected() {
+        let err =
+            sanitize_extra_args(&args(&["-o", "KnownHostsCommand=evil"])).unwrap_err();
+        assert!(err.contains("KnownHostsCommand"), "err={err}");
+    }
+
+    #[test]
+    fn permit_local_command_yes_rejected() {
+        // PermitLocalCommand=yes unlocks the LocalCommand vector — must be
+        // rejected even though LocalCommand itself isn't set in this snippet.
+        let err =
+            sanitize_extra_args(&args(&["-o", "PermitLocalCommand=yes"])).unwrap_err();
+        assert!(err.contains("PermitLocalCommand"), "err={err}");
+    }
+
+    #[test]
+    fn bad_arg_in_middle_rejected() {
+        let err = sanitize_extra_args(&args(&[
+            "-o", "ServerAliveInterval=30",
+            "-o", "ProxyCommand=evil",
+            "-o", "Compression=yes",
+        ]))
+        .unwrap_err();
+        assert!(err.contains("ProxyCommand"), "err={err}");
+    }
+
+    #[test]
+    fn trailing_dash_o_without_value_ok() {
+        // -o with no following value is malformed; ssh will reject it. We
+        // just skip past so we don't panic on the index.
+        assert!(sanitize_extra_args(&args(&["-o"])).is_ok());
+    }
+}

src-tauri/src/mcp.rs

@@ -13,7 +13,11 @@
 //! modal → audit):
 //!   set_label, close_pane, swap_panes, promote_pane, apply_preset
 //!   spawn_pane (local WSL / PowerShell only)
-//!   connect_host (SSH to a saved host id — the only SSH path)
+//!   connect_host (SSH to a saved host id — the only SSH spawn path)
+//!   add_host, delete_host (mutate saved-hosts list; gated by an extra
+//!     'allow_add_host' safeguard; add_host sanitises extraArgs to reject
+//!     ProxyCommand / LocalCommand / KnownHostsCommand / PermitLocalCommand
+//!     =yes — CVE-2023-51385 class local-RCE primitives)
 //!   write_pane (rate-limited per pane; matched against a non-overridable
 //!     hard-deny list before user policy)
 //!
@@ -423,6 +427,43 @@ pub struct ConnectHostArgs {
     pub orientation: Option<String>,
 }
 
+#[derive(Debug, Deserialize, schemars::JsonSchema)]
+pub struct AddHostArgs {
+    /// Human-readable name for the host (shown in the picker / palette).
+    /// If omitted, defaults to `hostname` server-side.
+    #[serde(default)]
+    pub label: Option<String>,
+    /// Hostname or IP. Required. Rejected if it starts with '-' or contains
+    /// control characters (CVE-2023-51385 / smuggled-flag class).
+    pub hostname: String,
+    /// SSH login user. Same validation as hostname.
+    #[serde(default)]
+    pub user: Option<String>,
+    /// TCP port. Defaults to 22 if omitted.
+    #[serde(default)]
+    pub port: Option<u16>,
+    /// Path to a private key. Passed to ssh as `-i`.
+    #[serde(default, rename = "identityFile")]
+    pub identity_file: Option<String>,
+    /// `user@host[:port]` jump host. Same validation as hostname.
+    #[serde(default, rename = "jumpHost")]
+    pub jump_host: Option<String>,
+    /// Extra ssh args (e.g. `-o ServerAliveInterval=30`). Sanitised to
+    /// reject command-execution `-o` options: ProxyCommand, LocalCommand,
+    /// KnownHostsCommand, PermitLocalCommand=yes. The user's manually-added
+    /// hosts are unrestricted; only this MCP path is gated.
+    #[serde(default, rename = "extraArgs")]
+    pub extra_args: Option<Vec<String>>,
+}
+
+#[derive(Debug, Deserialize, schemars::JsonSchema)]
+pub struct DeleteHostArgs {
+    /// Stable id of a host returned by tiletopia://hosts or a prior
+    /// add_host call. Deleting also sweeps any saved password for the
+    /// host from the OS keyring.
+    pub host_id: String,
+}
+
 #[derive(Debug, Deserialize, schemars::JsonSchema)]
 pub struct WritePaneArgs {
     /// Stable leaf id from the tree (uuid-shaped). Must belong to a pane
@@ -931,6 +972,122 @@ impl TileService {
         )]))
     }
 
+    #[tool(description = "Register a new SSH host in the saved-hosts list \
+        (the same store the titlebar 🔑 picker manages). Validates hostname/\
+        user/jumpHost the same way an SSH spawn would (rejects '-' prefixes \
+        and control characters, CVE-2023-51385 class) and sanitises extraArgs \
+        to reject ProxyCommand / LocalCommand / KnownHostsCommand / \
+        PermitLocalCommand=yes (local-RCE primitives). Gated by the \
+        'Allow Claude to save or delete SSH hosts' switch in the Policy tab; \
+        refuses with 'add-host-disabled' when off. Returns {hostId} for the \
+        newly-saved host — pass to connect_host to open it.")]
+    async fn add_host(
+        &self,
+        Parameters(args): Parameters<AddHostArgs>,
+    ) -> Result<CallToolResult, McpError> {
+        if !self.policy_ssh_add_host_allowed().await {
+            return Err(McpError::invalid_params(
+                "add-host-disabled: Claude is not allowed to save SSH hosts \
+                 (Policy tab → SSH safeguards → 'Allow Claude to save or \
+                 delete SSH hosts'). Ask the user to add the host manually \
+                 via the titlebar 🔑 picker.",
+                None,
+            ));
+        }
+
+        // Same token validation ssh.exe would do at spawn time — reject up
+        // front so we don't persist a host that can never be opened.
+        crate::pty::validate_ssh_token("hostname", &args.hostname)
+            .map_err(|e| McpError::invalid_params(e.to_string(), None))?;
+        if let Some(u) = args.user.as_deref() {
+            crate::pty::validate_ssh_token("user", u)
+                .map_err(|e| McpError::invalid_params(e.to_string(), None))?;
+        }
+        if let Some(jh) = args.jump_host.as_deref() {
+            crate::pty::validate_ssh_token("jump host", jh)
+                .map_err(|e| McpError::invalid_params(e.to_string(), None))?;
+        }
+        if let Some(extra) = args.extra_args.as_deref() {
+            crate::hosts::sanitize_extra_args(extra)
+                .map_err(|reason| McpError::invalid_params(reason, None))?;
+        }
+
+        let label = args
+            .label
+            .as_deref()
+            .map(|s| s.trim())
+            .filter(|s| !s.is_empty())
+            .unwrap_or_else(|| args.hostname.trim())
+            .to_string();
+
+        let args_repr = format!(
+            "label={} hostname={} user={} port={}",
+            label,
+            &args.hostname,
+            args.user.as_deref().unwrap_or("(default)"),
+            args.port
+                .map(|p| p.to_string())
+                .unwrap_or_else(|| "(default)".into()),
+        );
+        let args_json = json!({
+            "label": label,
+            "hostname": &args.hostname,
+            "user": args.user,
+            "port": args.port,
+            "identityFile": args.identity_file,
+            "jumpHost": args.jump_host,
+            "extraArgs": args.extra_args,
+        });
+        tracing::debug!(hostname = %args.hostname, "add_host: dispatching");
+        let result = self
+            .dispatch_action("add_host", args_json, args_repr)
+            .await?;
+        Ok(CallToolResult::success(vec![Content::text(
+            result.to_string(),
+        )]))
+    }
+
+    #[tool(description = "Delete a saved SSH host by id. Sweeps any saved \
+        password for the host from the OS keyring as a side effect. Gated by \
+        the same 'Allow Claude to save or delete SSH hosts' switch as \
+        add_host; refuses with 'add-host-disabled' when off.")]
+    async fn delete_host(
+        &self,
+        Parameters(args): Parameters<DeleteHostArgs>,
+    ) -> Result<CallToolResult, McpError> {
+        if !self.policy_ssh_add_host_allowed().await {
+            return Err(McpError::invalid_params(
+                "add-host-disabled: Claude is not allowed to delete SSH hosts \
+                 (Policy tab → SSH safeguards → 'Allow Claude to save or \
+                 delete SSH hosts').",
+                None,
+            ));
+        }
+        // Verify the host_id is in the mirror so Claude can't probe arbitrary
+        // ids. The mirror is the authoritative view of what Claude can see.
+        let host_label = {
+            let st = self.state.read().await;
+            st.mirror
+                .hosts
+                .iter()
+                .find(|h| h.id == args.host_id)
+                .map(|h| h.label.clone())
+        };
+        let label = host_label.ok_or_else(|| {
+            McpError::invalid_params(
+                "unknown host_id (use tiletopia://hosts to list saved hosts)",
+                Some(json!({ "host_id": &args.host_id })),
+            )
+        })?;
+        let args_repr = format!("hostId={} label={}", &args.host_id, &label);
+        let args_json = json!({ "hostId": &args.host_id });
+        tracing::debug!(host_id = %args.host_id, "delete_host: dispatching");
+        let _ = self
+            .dispatch_action("delete_host", args_json, args_repr)
+            .await?;
+        Ok(CallToolResult::success(vec![Content::text("ok")]))
+    }
+
     #[tool(description = "Set or clear the human-readable label on a pane. \
         Pass empty string to clear. The leaf must be MCP-allowed.")]
     async fn set_label(
@@ -1063,6 +1220,17 @@ impl TileService {
         }
     }
 
+    /// Mirror of policy_ssh_open_allowed for the add_host/delete_host pair.
+    async fn policy_ssh_add_host_allowed(&self) -> bool {
+        match crate::mcp_policy::load_or_init(&self.app) {
+            Ok(p) => p.ssh_safeguards.allow_add_host,
+            Err(e) => {
+                tracing::warn!(error = %e, "policy_ssh_add_host_allowed: load failed, defaulting to false");
+                false
+            }
+        }
+    }
+
     /// Shared validation for tools that target an existing leaf — confirms
     /// the leaf is in the mirror (which means the user has it allow-listed
     /// for MCP) and returns its metadata. Factored out of the 4+ tools that
@@ -1109,6 +1277,9 @@ impl ServerHandler for TileService {
              apply_preset — tree shape and metadata.\n\
              - spawn_pane (local WSL/PowerShell), connect_host (SSH to a \
              saved host — use this for SSH, not spawn_pane).\n\
+             - add_host, delete_host (mutate the saved-hosts list; \
+             add_host's extraArgs are sanitised — ProxyCommand and \
+             friends are refused).\n\
              - write_pane (send keystrokes; rate-limited; matched against \
              user policy + a non-overridable hard-deny list for the \
              worst-of-the-worst patterns).\n\
@@ -1116,7 +1287,10 @@ impl ServerHandler for TileService {
              Only panes the user has allow-listed (🤖 chip on) are \
              visible. SSH spawns are gated by an extra Policy-tab switch \
              that's off by default — if you see 'ssh-disabled' errors, \
-             the user has not enabled MCP-initiated SSH.",
+             the user has not enabled MCP-initiated SSH. add_host / \
+             delete_host are similarly gated by an 'allow_add_host' \
+             switch — 'add-host-disabled' means the user manages SSH \
+             hosts manually via the titlebar 🔑 picker.",
         )
     }
 

src-tauri/src/mcp_policy.rs

@@ -41,6 +41,12 @@ pub struct SshSafeguards {
     /// switch above.
     #[serde(default)]
     pub auto_allow_spawned_ssh: bool,
+    /// When false (default), `add_host` and `delete_host` refuse server-side
+    /// — Claude can't mutate the saved-hosts list. The user manages hosts
+    /// via the titlebar 🔑 picker → Manage hosts… UI. Turn on if you want
+    /// Claude to be able to register new SSH targets autonomously.
+    #[serde(default)]
+    pub allow_add_host: bool,
 }
 
 #[derive(Clone, Debug, Default, Serialize, Deserialize)]
@@ -421,6 +427,7 @@ mod tests {
                 ask: ask.iter().map(|s| s.to_string()).collect(),
                 allow: allow.iter().map(|s| s.to_string()).collect(),
             },
+            ssh_safeguards: SshSafeguards::default(),
         }
     }
 

src-tauri/src/pty.rs

@@ -287,7 +287,7 @@ struct DataChunk {
 /// expansion. We additionally pass `--` before the host on the command line,
 /// but rejecting up front gives a clearer error and avoids ever handing the
 /// bad value to ssh.exe.
-fn validate_ssh_token(label: &str, value: &str) -> Result<()> {
+pub fn validate_ssh_token(label: &str, value: &str) -> Result<()> {
     if value.is_empty() {
         return Err(anyhow!("ssh: {label} must not be empty"));
     }