fix(security): constrain ProxyCommand endpoint token expansion

README.md

@@ -903,6 +903,10 @@ These options control SSH proxy connection behavior:
 
 **ProxyCommand** uses the user's shell so OpenSSH-style quoting, redirection, and pipelines work. `ProxyCommand none` explicitly selects a direct connection. `ProxyCommand` and `ProxyJump` follow OpenSSH's first-obtained ssh_config rule, while command-line `-J` takes precedence over both.
 
+Endpoint data for `%h`, `%n`, `%r`, and `%k` is checked before starting any ProxyCommand, including data from CLI arguments, YAML inventory, environment expansion, and library callers. Only values actually substituted into the command are checked; unused fields and escaped literal tokens such as `%%h` are unrestricted. Substituted values must be nonempty and contain only ASCII letters, digits, `.`, `_`, `:`, `@`, or `-`, with no leading `-`. Whitespace, shell substitutions, wildcards, quotes, and non-ASCII characters are rejected even when the template quotes the token. Use ASCII/punycode names and unbracketed IPv6 literals; scoped IPv6 (`%zone`) and raw bracketed literals are unsupported at this boundary. Normal aliases and authored quoting, redirection, and pipelines continue to work. ProxyCommand templates (including nested shells or `eval`) remain trusted executable code; endpoint data must never be used as code deliberately by a template.
+
+`%h`, `%n`, `%r`, `%k`의 값은 CLI, YAML, 환경 변수 및 라이브러리 입력 경로 모두에서 ProxyCommand 실행 전에 검증됩니다. 실제로 치환되는 값만 검증하며 사용하지 않는 필드와 `%%h` 같은 리터럴 토큰은 제한하지 않습니다. 비어 있지 않은 ASCII 영문자·숫자와 `.`, `_`, `:`, `@`, `-`만 허용하며 첫 문자는 `-`일 수 없습니다. 공백, 셸 치환, 와일드카드 및 비ASCII 문자는 따옴표 안에서도 거부됩니다. IPv6는 대괄호와 영역 식별자 없이 사용하고 국제화 호스트명은 punycode를 사용하세요. 별칭과 명령에 직접 작성한 인용·리다이렉션·파이프라인은 계속 지원됩니다. 명령 템플릿 자체는 신뢰할 수 있는 코드여야 합니다.
+
 *Note: `ProxyUseFdpass yes` is rejected with an actionable error before the command starts. Use a streaming ProxyCommand without `-F`, or set `ProxyUseFdpass no`.*
 
 ### Command Execution and Automation Options

docs/man/bssh.1

@@ -725,6 +725,13 @@ The command is run by the user's shell, preserving OpenSSH-style quoting,
 redirection, and pipelines.
 .br
 Supported tokens are %%, %h, %k, %n, %p, and %r.
+Endpoint values for %h, %n, %r, and %k are validated before spawning,
+regardless of whether they came from CLI, inventory, environment, or library APIs.
+Only substituted values are checked; unused fields and escaped literal tokens
+(such as %%h) are unrestricted. Substituted values must be nonempty ASCII letters/digits or ., _, :, @, - and cannot start
+with -. Whitespace, shell substitutions, glob patterns, and quotes are rejected
+even in quoted tokens. Use punycode names and unbracketed IPv6 without a zone
+identifier. The command template itself remains trusted executable code.
 .br
 .I ProxyCommand none
 explicitly selects a direct connection. When ProxyCommand and ProxyJump both

src/ssh/tokio_client/error.rs

@@ -157,6 +157,10 @@ pub enum Error {
     AuthenticationPromptTimeout { prompt: &'static str, seconds: u64 },
     #[error("ProxyCommand '{command}' contains unsupported token '{token}'")]
     InvalidProxyCommandToken { command: String, token: String },
+    #[error(
+        "ProxyCommand token {token} contains unsafe endpoint data; use nonempty ASCII letters, digits, '.', '_', ':', '@', or '-', without a leading '-'"
+    )]
+    UnsafeProxyCommandValue { token: &'static str },
     #[error("Failed to start ProxyCommand '{command}': {source}")]
     ProxyCommandSpawn {
         command: String,
@@ -315,6 +319,7 @@ impl Error {
                 | Self::ConnectionTimeout { .. }
                 | Self::ProtocolNegotiation { .. }
                 | Self::InvalidProxyCommandToken { .. }
+                | Self::UnsafeProxyCommandValue { .. }
                 | Self::ProxyCommandSpawn { .. }
                 | Self::ProxyCommandFailed { .. }
                 | Self::ProxyUseFdpassUnsupported { .. }

src/ssh/tokio_client/proxy_command.rs

@@ -85,6 +85,15 @@ impl ProxyCommandConfig {
         self
     }
 
+    /// Expand endpoint tokens only when their data is safe in a shell word.
+    ///
+    /// Only expanded token values are checked; unused fields are unrestricted.
+    /// Token values must be nonempty ASCII letters/digits or `.`, `_`, `:`,
+    /// `@`, `-`, and must not begin with `-`. This intentionally rejects shell
+    /// expansions, whitespace, glob patterns, and option-like values, even
+    /// inside quotes. Use unbracketed IPv6 and ASCII (punycode) hostnames.
+    /// The authored command remains trusted executable code, including any
+    /// explicit reinterpretation of its arguments by `eval` or nested shells.
     pub fn expand(&self, host: &str, port: u16, remote_user: &str) -> Result<String, Error> {
         let key_alias = self
             .host_key_alias
@@ -108,11 +117,28 @@ impl ProxyCommandConfig {
             };
             match token {
                 '%' => expanded.push('%'),
-                'h' => expanded.push_str(host),
-                'k' => expanded.push_str(key_alias),
-                'n' => expanded.push_str(&self.original_host),
                 'p' => expanded.push_str(&port),
-                'r' => expanded.push_str(remote_user),
+                'h' | 'k' | 'n' | 'r' => {
+                    let (token, value) = match token {
+                        'h' => ("%h", host),
+                        'k' => ("%k", key_alias),
+                        'n' => ("%n", self.original_host.as_str()),
+                        _ => ("%r", remote_user),
+                    };
+                    // Check only data entering the shell program, at the
+                    // common boundary shared by every input path. Literal
+                    // %% escapes and unused endpoint fields need no check.
+                    if value.is_empty()
+                        || value.starts_with('-')
+                        || !value.bytes().all(|byte| {
+                            byte.is_ascii_alphanumeric()
+                                || matches!(byte, b'.' | b'_' | b':' | b'@' | b'-')
+                        })
+                    {
+                        return Err(Error::UnsafeProxyCommandValue { token });
+                    }
+                    expanded.push_str(value);
+                }
                 other => {
                     return Err(Error::InvalidProxyCommandToken {
                         command: self.command.clone(),
@@ -409,6 +435,213 @@ mod tests {
         assert!(error.to_string().contains("%x"));
     }
 
+    #[test]
+    fn unused_endpoint_fields_and_escaped_tokens_are_not_validated() {
+        let unsafe_value = "$(touch unused)";
+        let config = ProxyCommandConfig::new("nc %h %p", unsafe_value)
+            .with_host_key_alias(Some(unsafe_value.to_string()));
+        assert_eq!(
+            config.expand("target", 22, unsafe_value).unwrap(),
+            "nc target 22"
+        );
+
+        for command in ["fixed-proxy", "echo %%h %%n %%r %%k"] {
+            let config = ProxyCommandConfig::new(command, unsafe_value)
+                .with_host_key_alias(Some(unsafe_value.to_string()));
+            assert_eq!(
+                config.expand(unsafe_value, 22, unsafe_value).unwrap(),
+                command.replace("%%", "%")
+            );
+        }
+        let config = ProxyCommandConfig::new("nc %h %p; echo %r", "alias");
+        assert!(matches!(
+            config.expand("target", 22, unsafe_value),
+            Err(Error::UnsafeProxyCommandValue { token: "%r" })
+        ));
+    }
+
+    #[tokio::test]
+    async fn unsafe_endpoint_tokens_are_rejected_before_any_shell_side_effect() {
+        let dir = tempfile::tempdir().unwrap();
+        let marker = dir.path().join("proxy-started");
+        let substitution = format!("$(touch {})", marker.display());
+        let backticks = format!("`touch {}`", marker.display());
+        let values = [
+            "",
+            "two words",
+            "host\toption",
+            "host\ncommand",
+            "host\rcommand",
+            "host\0suffix",
+            "-oProxyCommand=evil",
+            "a;b",
+            "a|b",
+            "a&b",
+            "a>b",
+            "a<b",
+            "a'b",
+            "a\"b",
+            "a\\b",
+            "$USER",
+            "${USER}",
+            "*",
+            "?",
+            "[ab]",
+            "{a,b}",
+            "~root",
+            "!value!",
+            "%PATH%",
+            "høst",
+            "[::1]",
+            "fe80::1%eth0",
+            substitution.as_str(),
+            backticks.as_str(),
+        ];
+        for token in ["%h", "%n", "%r", "%k"] {
+            for value in values {
+                for quoted_token in [
+                    token.to_string(),
+                    format!("'{token}'"),
+                    format!("\"{token}\""),
+                ] {
+                    // A launch itself would create the marker, independently
+                    // of whether the payload happens to work in this shell.
+                    let command =
+                        format!("touch {}; printf '%%s' {quoted_token}", marker.display());
+                    let mut config = ProxyCommandConfig::new(command, "alias");
+                    let mut host = "target";
+                    let mut user = "alice";
+                    match token {
+                        "%h" => host = value,
+                        "%n" => config.original_host = value.to_string(),
+                        "%r" => user = value,
+                        "%k" => config.host_key_alias = Some(value.to_string()),
+                        _ => unreachable!(),
+                    }
+                    assert!(
+                        matches!(
+                            spawn_proxy_command(&config, host, 22, user),
+                            Err(Error::UnsafeProxyCommandValue { token: rejected }) if rejected == token
+                        ),
+                        "accepted {token} value {value:?}"
+                    );
+                    assert!(!marker.exists(), "proxy shell was launched for {token}");
+                }
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn detailed_inventory_values_cannot_execute_proxy_shell_code() {
+        let dir = tempfile::tempdir().unwrap();
+        let marker = dir.path().join("inventory-injection");
+        for field in ["host", "user"] {
+            let yaml = format!(
+                "clusters:\n  test:\n    nodes:\n      - host: target\n        user: alice\n        {field}: '$(touch {})'\n",
+                marker.display()
+            );
+            // Build a single occurrence of each key to exercise normal YAML
+            // deserialization rather than duplicate-key handling.
+            let yaml = if field == "host" {
+                yaml.replace(
+                    "      - host: target\n        user: alice",
+                    "      - user: alice",
+                )
+            } else {
+                yaml.replace("        user: alice\n", "")
+            };
+            let inventory: crate::config::Config = serde_yaml::from_str(&yaml).unwrap();
+            let nodes = inventory.resolve_nodes("test").unwrap();
+            let node = &nodes[0];
+            let proxy = ProxyCommandConfig::new("cat %h %r", "inventory-alias");
+            assert!(matches!(
+                spawn_proxy_command(&proxy, &node.host, node.port, &node.username),
+                Err(Error::UnsafeProxyCommandValue { .. })
+            ));
+            assert!(!marker.exists());
+        }
+    }
+
+    #[tokio::test]
+    async fn environment_inventory_tokens_are_checked_after_expansion() {
+        const PAYLOAD_ENV: &str = "BSSH_TEST_PROXY_TOKEN_PAYLOAD_326";
+        if let Ok(payload) = std::env::var(PAYLOAD_ENV) {
+            for field in ["host", "user"] {
+                let yaml = if field == "host" {
+                    format!(
+                        "clusters:\n  test:\n    nodes:\n      - host: '${{{PAYLOAD_ENV}}}'\n        user: alice\n"
+                    )
+                } else {
+                    format!(
+                        "clusters:\n  test:\n    nodes:\n      - host: target\n        user: '${{{PAYLOAD_ENV}}}'\n"
+                    )
+                };
+                let inventory: crate::config::Config = serde_yaml::from_str(&yaml).unwrap();
+                let nodes = inventory.resolve_nodes("test").unwrap();
+                let node = &nodes[0];
+                assert_eq!(
+                    if field == "host" {
+                        &node.host
+                    } else {
+                        &node.username
+                    },
+                    &payload
+                );
+                let proxy = ProxyCommandConfig::new("cat %h %r", "inventory-alias");
+                assert!(matches!(
+                    spawn_proxy_command(&proxy, &node.host, node.port, &node.username),
+                    Err(Error::UnsafeProxyCommandValue { .. })
+                ));
+            }
+            return;
+        }
+        // Give only a subprocess the environment override: parallel tests
+        // must not observe process-global environment mutation.
+        let dir = tempfile::tempdir().unwrap();
+        let marker = dir.path().join("environment-injection");
+        let mut child = tokio::process::Command::new(std::env::current_exe().unwrap());
+        child
+            .args(["--exact", "ssh::tokio_client::proxy_command::tests::environment_inventory_tokens_are_checked_after_expansion", "--nocapture"])
+            .env(PAYLOAD_ENV, format!("$(touch {})", marker.display()))
+            .kill_on_drop(true);
+        let output = tokio::time::timeout(Duration::from_secs(10), child.output())
+            .await
+            .unwrap()
+            .unwrap();
+        assert!(
+            output.status.success(),
+            "{}{}",
+            String::from_utf8_lossy(&output.stdout),
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert!(String::from_utf8_lossy(&output.stdout).contains("1 passed"));
+        assert!(!marker.exists());
+    }
+
+    #[tokio::test]
+    async fn ipv6_aliases_and_quoted_pipeline_tokens_remain_literal_arguments() {
+        let spec = crate::node::parse_node_spec("alice@[2001:db8::1]:2222").unwrap();
+        let proxy = ProxyCommandConfig::new(
+            "printf '%%s\\n' %h '%n' \"%k\" %p %r | tr A-Z a-z",
+            "Prod_alias-1",
+        )
+        .with_host_key_alias(Some("Key_alias-1".to_string()));
+        let mut session =
+            spawn_proxy_command(&proxy, spec.host, spec.port.unwrap(), spec.user.unwrap()).unwrap();
+        let mut output = String::new();
+        tokio::time::timeout(
+            Duration::from_secs(5),
+            session.stream.read_to_string(&mut output),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(
+            output,
+            "2001:db8::1\nprod_alias-1\nkey_alias-1\n2222\nalice\n"
+        );
+    }
+
     #[tokio::test]
     async fn shell_pipeline_is_preserved() {
         let config = ProxyCommandConfig::new("printf PROXY | tr A-Z a-z", "target");