cli(cluster): #2861 leaves ssh argument-injection open — a `machine.host`/`user` starting with `-` runs an arbitrary command on the local machine (ProxyCommand)

[phil-opp]

This issue was created by a scheduled, automated Claude review of recent commits/PRs. It was verified by reading the code directly, but please double-check before acting on it.

Summary

PR #2861 (commit 9b288f5, "fix(cli): validate cluster machine id and labels before they reach the remote shell") hardens dora cluster against remote-shell injection by validating machine.id, label keys/values, and zenoh_peer. It deliberately leaves machine.host and machine.user unvalidated, with this stated rationale (binaries/cli/src/command/cluster/config.rs:92-96):

host/user are deliberately left unrestricted: they are network addresses that legitimately carry characters outside this set (e.g. IPv6 literals), and are folded into the ssh target rather than the remote command string.

That reasoning only rules out injection into the remote shell. It overlooks injection into the local ssh client's own argument parser, which is a separate and arguably more severe vector: it executes on the operator's machine, before any network connection.

The vulnerability

ssh_target interpolates host/user verbatim (binaries/cli/src/command/cluster/mod.rs:58-63):

pub(super) fn ssh_target(machine: &MachineConfig) -> String {
    match &machine.user {
        Some(user) => format!("{user}@{}", machine.host),
        None => machine.host.clone(),
    }
}

and run_ssh passes that target to ssh with no -- end-of-options separator (binaries/cli/src/command/cluster/mod.rs:92-105):

pub(super) fn run_ssh(target: &str, port: Option<u16>, cmd: &str) -> eyre::Result<bool> {
    let mut command = std::process::Command::new("ssh");
    command.args([ "-o", "BatchMode=yes", /* … */ ]);
    if let Some(p) = port { command.args(["-p", &p.to_string()]); }
    command.args([target, cmd]);   // <-- target is unguarded; no "--" before it
    // …
}

If host (or user) begins with -, the local ssh binary parses target as an option, not a hostname. This is the classic ssh/git argument-injection class (cf. CVE-2017-1000117).

Concrete exploit

A cluster descriptor is the untrusted input the rest of #2861 is defending against. Given:

machines:
  - id: a
    host: "-oProxyCommand=touch /tmp/pwned;false"

the argv becomes:

ssh -o BatchMode=yes … -oProxyCommand=touch /tmp/pwned;false <remote_cmd>

ssh interprets -oProxyCommand=… and runs touch /tmp/pwned;false on the machine running dora cluster up (via the local shell that ProxyCommand uses) before/instead of connecting. Arbitrary local command execution.

user is an equivalent vector: ssh_target yields {user}@{host}, so a user beginning with - produces a target string that also starts with -.

Affected code paths

run_ssh / ssh_target are the shared SSH primitives for the whole dora cluster surface, so every subcommand that shells out is affected:

• binaries/cli/src/command/cluster/up.rs (ssh_target at ~:66, ~:78)
• install.rs, uninstall.rs, upgrade.rs (all route through run_ssh)

The host/user values originate from MachineConfig parsed out of the cluster YAML and are never checked for a leading -.

Suggested fix

Two complementary defenses (the codebase already treats the cluster descriptor as an injection boundary, so validation is consistent with its own posture):

1. Reject host/user values that begin with - in ClusterConfig::validate() (git's portable mitigation for exactly this class). This keeps IPv6 literals and normal user@host values valid while closing the option-injection hole. It can't be folded into validate_shell_safe (which would reject the :/[/] in IPv6), so it needs a dedicated leading-dash check.
2. Insert a literal "--" before target in run_ssh — command.args(["--", target, cmd]). OpenSSH accepts -- as an end-of-options marker; this is defense-in-depth for the local argv, and because all subcommands share run_ssh, one change covers them all. (Note -- support is version-dependent on very old OpenSSH, so prefer Set up a basic Rust project #1 as the primary fix and treat #2 as belt-and-suspenders.)

Notes / non-issues found in the same review

• A secondary, low-severity observation on the same PR: validate_endpoint_shell_safe permits [/] in zenoh_peer, which is interpolated unquoted into the remote command. Under default sh/bash (no nullglob/failglob) an unmatched [::1] glob is left literal, so it normally works; it is not command injection (no way to start a new command). Single-quoting the remote interpolations, or requiring the un-bracketed IPv6 form, would remove the glob ambiguity. Low priority.