fix(ssh): preserve configured aliases in host suggestions

[georgenijo]

What Changed

SSH host discovery preserves configured aliases, hides their duplicate known_hosts targets, and lets users search for an alias by its target address. Connections still use the alias and its SSH settings. Target resolution follows first-value precedence across Includes and matching Host/Match blocks; unsupported dynamic rules fall back conservatively.

Why

With Host build-box / HostName build-box.example.test and that address in known_hosts, the picker currently suggests both entries. Choosing the raw target can bypass the configured user or routing settings. This is a focused fix for that existing picker behavior, not a new connection workflow.

Fixes #13270. Replaces #13269, which was closed for missing before/after UI evidence. The requested evidence is included below from the original submitted revision. The original five source patches rebased unchanged onto upstream main 10f39eb9ac (git range-diff reports identical patches). A final review follow-up conservatively retains aliases under unevaluated Match conditions and bounds Include traversal to 16 levels / 256 file reads. Includes under a known non-matching Host block are excluded, as in SSH config semantics.

Verification

On the replacement branch:

• vp test run packages/ssh/src/config.test.ts apps/web/src/state/desktopSshHosts.test.ts apps/desktop/src/ssh/DesktopSshEnvironment.test.ts: 26 tests passed, covering target de-duplication, config precedence, conditional Includes, wildcard Host, Match rules, tokenized/quoted values, bridge behavior and hostname search.
• SSH, web and desktop package typechecks and targeted lint passed. Independent Claude Opus 5.5 (claude-opus-5-5, high effort) review and recheck passed on 7606475b6a. Macroscope then identified two further first-value/quoted-Match cases; 188d126d2f fixes both, with 25 focused tests passing. Independent Claude Opus 5.5 rechecks passed on the first-value fixes and on the final c0ca2566e4 conservative handling of complex quoted patterns; all 26 focused tests pass.
• No SSH connection was attempted; this change affects suggestions and leaves actual alias-based connection resolution intact.

Upstream CI and preview workflows are currently action_required: GitHub requires maintainer approval for this fork contribution. Local verification does not replace that gate.

UI Changes

Captured the running Electron 44.4.2 app, production preload/IPC and SSH discovery on the submitted head 0f6216edb3 and its original base f5ef0ddb90, with isolated app state and an X11 display.

Both capture builds temporarily pass the same explicit homeDir to the discovery call. Its disposable fixture contains Host build-box, HostName build-box.example.test, and a matching raw hostname in known_hosts. No real SSH configuration was read and no SSH connection was attempted. This test-only fixture injection is not part of the PR. Capture procedure and provenance.

Settings → Connections → Add environment → SSH, empty focused host field:

Before: the alias and its raw target are separate suggestions.

After: the configured alias is preserved and the duplicate target is suppressed.

Searching by the target address build-box.example.test finds the configured build-box alias.

Before hostname-search capture: it returns the raw target.

Checklist

• This PR is small and focused on one underlying problem
• I explained what changed and why
• I included before/after screenshots for UI changes
• Interaction behavior is shown by the hostname-search screenshot; no animation or timing behavior changes

Authored with GPT models through the Codex harness; replacement prepared with GPT-6.1 Sol and independently reviewed by Claude Opus 5.5 at high effort.

[macroscopeapp]

🟡 Medium src/config.ts:201

A HostName inside an unsupported Match condition such as Match exec is discarded, so a later Host block's HostName is reported as the target even when the match succeeds and SSH's first-obtained-value semantics select the earlier hostname; this can hide the actual host from known_hosts. Preserve an unresolved first-value rule for that conditional HostName so later values cannot falsely resolve the target, and retain the alias as a fallback.

🤖 Copy this AI Prompt to have your agent fix this:

In file @packages/ssh/src/config.ts around line 201:

A `HostName` inside an unsupported `Match` condition such as `Match exec` is discarded, so a later `Host` block's `HostName` is reported as the target even when the match succeeds and SSH's first-obtained-value semantics select the earlier hostname; this can hide the actual host from `known_hosts`. Preserve an unresolved first-value rule for that conditional `HostName` so later values cannot falsely resolve the target, and retain the alias as a fallback.

[georgenijo]

Fixed in c0ca256. An unresolved conditional HostName now reserves the first value instead of allowing a later HostName to falsely resolve the alias. Discovery keeps the alias fallback and raw known_hosts candidates. Regression coverage includes direct Match exec, included guarded rules, an earlier unconditional value, and unrelated guarded hosts; the 26 focused SSH/discovery tests pass.

[macroscopeapp]

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

[macroscopeapp]

🟡 Medium src/config.ts:198

Match originalhost "work" does not match the alias work, so the following HostName work.example rule is ignored and discovery searches for work instead of work.example, retaining the duplicate known_hosts target. The argument is split without removing its quotes; unquote it before splitting the pattern list, as HostName values are.

Suggested change

	? (rawArgs[1]?.split(",") ?? [])
	? (rawArgs[1]?.replace(/^("|')(.*)\1$/u, "$2").split(",") ?? [])

🤖 Copy this AI Prompt to have your agent fix this:

In file @packages/ssh/src/config.ts around line 198:

`Match originalhost "work"` does not match the alias `work`, so the following `HostName work.example` rule is ignored and discovery searches for `work` instead of `work.example`, retaining the duplicate `known_hosts` target. The argument is split without removing its quotes; unquote it before splitting the pattern list, as `HostName` values are.

[georgenijo]

Fixed in c0ca256. Whole quoted Match originalhost arguments are unquoted before matching the comma-separated patterns. Regression fixtures cover double-quoted single names, single-quoted comma lists, nonmatches, and conservatively unresolved mixed quoting. All 26 focused SSH/discovery tests pass.

[macroscopeapp]

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The change modifies production SSH discovery and suggestion behavior by adding nontrivial HostName, Include, and Match parsing plus target de-duplication. Unresolved Medium findings identify conditional configuration cases that can still produce incorrect host suggestions.

Not approved because:

• 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

^{Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 21589ba7-5cfe-4573-aba7-ac4b370255d3

📥 Commits

Reviewing files that changed from the base of the PR and between 7606475 and c0ca256.

📒 Files selected for processing (2)

• packages/ssh/src/config.test.ts
• packages/ssh/src/config.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

SSH host discovery now applies supported SSH config hostname rules and avoids listing known-host entries that match configured aliases or targets. Host search also matches configured hostnames case-insensitively.

Changes

SSH host discovery

Layer / File(s)	Summary
Parse SSH config rules packages/ssh/src/config.ts, packages/ssh/src/config.test.ts	The parser carries pattern guards through includes, handles supported Match conditions and HostName rules, and limits config traversal. Tests cover include behavior, token expansion, condition handling, and traversal limits.
Resolve configured targets and filter known hosts packages/ssh/src/config.ts, packages/ssh/src/config.test.ts, apps/desktop/src/ssh/DesktopSshEnvironment.test.ts	Discovery applies the first applicable hostname rule and skips known-host entries that match a discovered alias or configured target. Tests cover configured hostnames and aliases alongside matching known-host targets.
Search by alias or hostname apps/web/src/state/desktopSshHosts.ts, apps/web/src/state/desktopSshHosts.test.ts	Substring search matches either the alias or hostname. Alias prefix matches remain ranked first. A test checks a case-insensitive hostname query.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to c0ca2

The SSH suggestion changes are mergeable. A raw hostname using a different port may not appear as a suggestion, but it can still be entered directly.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c0ca2

The changed suggestions preserve aliases when connecting, so configured SSH settings remain authoritative. No introduced security-boundary bypass was established. Risk is limited, but discovery only approximates dynamic SSH configuration and the security assessment is not exhaustive.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The inspected influence path is local SSH configuration and known-host data into the desktop host picker, followed by user-selected alias resolution. Modification of those inputs can influence suggestions, but the changed discovery code does not grant new credential authority or initiate a connection itself.

Trust Boundaries and Controls

• observed — Unsupported Match scopes and hostname tokens fall back to the alias rather than being dynamically evaluated by discovery. Possible raw known-host targets remain visible when their equivalence cannot be established; such raw-target visibility predates this PR and is not an introduced bypass.

Resilience and Maintainability Implications

• inferred — Rules, active-path tracking, traversal budget, and output maps belong to one discovery invocation. Read failure or interruption does not publish partial results or contaminate subsequent or concurrent calls. Successful path cleanup permits repeated Includes without removing the shared visit limit.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Issue #13270 requires the picker to preserve configured aliases, hide matching known_hosts targets, retain unrelated entries, and find an alias by its target hostname. The reviewed implementation an…
Out of Scope Changes check	✅ Passed	The SSH config parsing, host filtering, and tests all support alias discovery, target de-duplication, or target-based search for #13270. No unrelated changes are evident.
Approvability	✅ Passed	...
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving configured SSH aliases in host suggestions.
Description check	✅ Passed	The description explains the problem and change, links the issue, gives a focused-scope rationale, and reports targeted verification and before-and-after UI evidence. It uses different headings from t…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[georgenijo]

@macroscope-app review

Please review the current head c0ca256. Both findings from the earlier 7606475 review are fixed, with regression coverage and replies in the original threads. The 26 focused tests pass; the latest CodeRabbit review has no actionable findings. GitHub Actions remain awaiting maintainer approval to run.

[macroscopeapp]

Sorry, I'm unable to act on this request because you do not have permissions within this repository.