`russh-config` does not handle `Match` directives in ssh_config

[TeddyHuang-00]

Summary

The russh-config parser silently ignores Match directives, which causes two concrete bugs depending on where the Match block appears in the file:

1. HostNotFound error — when Match appears before any Host block
2. Silent semantic corruption — when Match appears after a Host block, its subordinate directives (e.g. User, IdentityFile) bleed into that preceding Host block

Reproduction

Case 1: HostNotFound

Minimal config:

Match host * exec "gpg-connect-agent UPDATESTARTUPTTY /bye"

Host my-server
    HostName 10.0.0.1
    User admin

let config = russh_config::parse(
    r#"
Match host * exec "gpg-connect-agent UPDATESTARTUPTTY /bye"

Host my-server
    HostName 10.0.0.1
    User admin
"#,
    "my-server",
);
// → Err(Error::HostNotFound)

The error message is misleading: the host exists, but the parser hits the Match line before any Host block, treats it as an orphaned parameter, and then returns HostNotFound when the later Host line is reached.

Case 2: parameter bleed

Host my-server
    HostName 10.0.0.1
    User alice

Match host *.internal
    User bob
    IdentityFile ~/.ssh/internal_key

Host other-server
    HostName 10.0.0.2

When querying "my-server", User will be set to "bob" and IdentityFile will include ~/.ssh/internal_key, even though Match host *.internal should not apply to my-server.

This happens because the parser ignores the Match line itself, but keeps parsing its subordinate directives into the current mutable config. When the next Host line is reached, the accumulated config is flushed into the preceding Host my-server entry and then reset for other-server.

Root Cause

In parse_ssh_config (russh-config/src/lib.rs, line 200):

match lower.as_str() {
    "host" => { /* starts a new HostEntry */ }
    "user" => { ... }
    "hostname" => { ... }
    // ... other recognized keywords ...
    key => {
        debug!("{key:?}");  // ← "match" lands here, silently dropped
    }
}

The parser:

1. Does not recognize Match as a block-starting keyword (like Host)
2. Treats Match as an unknown parameter → sets found_params = true
3. Its subsequent lines (e.g. User bob) are applied to the current mutable config, corrupting state

Impact on downstream projects

Similar Match parsing failures have been reported in downstream SSH-config consumers:

• sshping (TeddyHuang-00/sshping#211) — "can't find hosts with Match directive at start of ssh_config"

While the same bug is also reported when using other ssh config parsing libraries:

• Waveterm (wavetermdev/waveterm#654) — "Match directive parsing is unsupported"
• boring (alebeck/boring#20) — "Match statement in ssh_config breaks parsing"
• VSCode Remote SSH (microsoft/vscode-remote-release#9683) — "Parsing SSH Config Fails when Match is encountered"

Common workarounds include removing/commenting out Match stanzas or moving them after all relevant Host blocks, but users should not have to restructure valid SSH configs for a parser limitation.

Proposed Solutions

Approach A: Minimal — skip Match blocks gracefully (recommended first step)

Add a "match" arm that treats Match as a block delimiter and prevents directives inside the unsupported Match block from mutating the active HostConfig.

What it does:

• Recognizes Match ... as a block-starting keyword, like Host
• Flushes any preceding Host entry before entering Match-skipping mode
• Skips subsequent directives until the next Host or Match line (or EOF)
• Does not set found_params = true for the Match line itself
• Does not apply unsupported Match directives to the previous Host entry

Pros:

• Small, targeted change
• Fixes HostNotFound for leading Match blocks
• Fixes state corruption from unsupported Match blocks

Cons:

• Match conditions are not evaluated; directives inside Match blocks are simply dropped
• Users who rely on Match to apply host-specific overrides won't get them

Approach B: Full Match support (long-term goal)

Implement full Match semantics per ssh_config(5):

1. Parse the Match condition list (supporting all, canonical, final, exec, host, originalhost, user, localuser)
2. Store Match blocks as a new variant in the config AST
3. Evaluate conditions at query time, merging matched blocks into the final HostConfig

Design questions to discuss:

• Should Match evaluation happen at parse time or query time?
  • Parse time: simpler but loses context (current user, environment)
  • Query time: correct for exec, user, localuser conditions but requires more restructuring
• How should Match blocks interact with Host block merging order?
• The exec condition requires running a shell command — what's the right API for that in an async context?

Approach C: Hybrid

Start with Approach A (minimal skip) as an immediate fix, then iterate toward Approach B in a follow-up PR. The minimal fix prevents misleading parse failures and state corruption while leaving full Match semantics for a more deliberate design.

Questions for the Maintainers

1. Is there appetite for full Match support (Approach B), or would a minimal graceful-skip (Approach A) be preferred for now?
2. If full support: should condition evaluation happen at parse time or query time?
3. Are there concerns about the exec condition requiring process spawning? Should it be gated behind a feature flag?
4. Would you prefer a single PR or incremental delivery?

Related

• OpenSSH man page: ssh_config(5) — Match
• All Match conditions are documented under the Match keyword in the same man page
• The ssh2-config and ssh2-config-rs crates explicitly list Match patterns as a missing feature, which is useful context if comparing Rust SSH-config parser behavior

[TeddyHuang-00]

TL;DR

The core design decision is: skip Match blocks cleanly, or parse & evaluate them?

Skip (Approach A) is a ~20-line fix that stops the two active bugs (spurious HostNotFound, state corruption). It's safe, backward-compatible, and unblocks all downstream users immediately.

Parse (Approach B) is the real thing, but introduces nontrivial design questions — parse-time vs. query-time evaluation, exec condition sandboxing, merge semantics with Host blocks. Getting that right warrants its own discussion.

I'm proposing we ship A first, then discuss B separately. That way users stop hitting misleading errors while we design the full feature at a reasonable pace.

For disclosure

I used assistance from LLMs (DeepSeek, GPT) to trace and analyze this bug across the source, but I've personally proofread and stand behind the claims in this issue. I'm happy to contribute a PR for the fix once there's alignment on the approach — but I'd rather get design feedback first than guess at the right implementation path.