feat: import SSH hosts from ~/.ssh/config, and refuse to dial proxied hosts - #352
Open
attson wants to merge 11 commits into
Open
feat: import SSH hosts from ~/.ssh/config, and refuse to dial proxied hosts#352attson wants to merge 11 commits into
attson wants to merge 11 commits into
Conversation
…ndling Pure-Go parser (no GUI dependency) for later import-into-atterm tasks. Implements ssh_config's real semantics: first-obtained value wins per keyword, wildcard Host blocks participate in evaluation without being importable themselves, Match blocks are skipped with a user-facing reason (their conditions depend on runtime state), and Include is capped at depth 16 with a visited-path set so a self-including config reports a Skipped instead of hanging.
…d-block reporting - Include resolution now passes base through unchanged at every recursion level instead of shifting to the including file's directory — ssh_config(5) resolves relative Include paths against a fixed root (~/.ssh), not against whichever file did the including. Added a two-level nested-Include test that would have picked up the wrong file under the old behavior. - Replaced path.Match-based Host/Match pattern matching with a matcher that implements ssh_config's actual PATTERNS syntax (only '*' and '?' are special; '[' is a literal character, not a class opener like path.Match treats it). - A Host line with no usable pattern (empty, or every pattern negated) now produces a Skipped instead of silently contributing nothing. - Documented the POSIX-only path assumption on Opener and Parse's base parameter, and the first-wins empty-value corner case on the guard in resolveEntry.
… handling, add gate/binding tests - mergeImportedHost now preserves AuthKind alongside KeyID (coupled pair); previously a re-import with a dropped IdentityFile line would flip AuthKind to "password" while KeyID stayed, breaking key-auth hosts. - fsOpener translates sshconfig's POSIX-style paths via filepath.FromSlash before touching the real filesystem, and PreviewSSHConfigImport passes filepath.ToSlash(sshDir) as base, per sshconfig's documented contract. - Added tests: AuthKind/KeyID coupling on merge, a gate test that distinguishes "before credential read" from "before dial" (ProxyJump host with no stored credential), and App-level tests for PreviewSSHConfigImport/ImportSSHHosts (missing/unreadable config, and a round trip that asserts markSSHHostsDirty ran).
…markers Adds the "从 ~/.ssh/config 导入" drawer to SshHostsPanel: two-step preview/import (PreviewSSHConfigImport then ImportSSHHosts), rows default unchecked, skipped entries render with their Chinese reasons, proxied hosts get a 跳板/ProxyJump marker, a rejected preview shows an error instead of an empty list, and the backend's note is shown in the drawer footer. Also extends the hand-maintained Wails binding shim (lib/api/_bindings.ts, lib/api/ssh.ts, lib/api.ts) with the SSHHost identity_file/proxy_jump/ proxy_command fields and the SSHConfigImportPreview/SSHConfigImportSkipped types + PreviewSSHConfigImport/ImportSSHHosts bindings, which the panel depends on and which nothing else in the tree had wired up yet.
The SshHostsPanel UI is entirely English elsewhere; item 25 introduced Chinese Skipped.Reason strings, import errors, and drawer copy, so an English-locale user would see Chinese text in an otherwise English panel. Converts every new string (and the tests asserting on them) to English while preserving the meaning each carries: the Include cycle-vs-depth-cap reasons stay distinguishable by the substring "cycle", the note still says the import isn't lossless, and the proxy marker still flags jump-host hosts as not yet connectable.
Closes design-doc §7 risk 1 for roadmap item 25. ssh_hosts_encrypted is already on the relay's allowedPreferenceKeys whitelist (internal/userstore/preferences.go:37), and SSHHost is JSON-marshaled whole into the sealed payload, so IdentityFile/ProxyJump/ProxyCommand round-trip through seal -> openSSHHosts without any relay change. Verified by a mutation: blanking ProxyJump before seal turns the new test red.
…recognized-not-connectable
The proxy gate could be edited away. saveHost() built its payload from
the form refs only, and UpdateSSHHost did a wholesale slot replace, so
identity_file / proxy_jump / proxy_command were blanked on every save.
Import writes no credential by design, which makes "open the drawer and
attach one" the mandated next step for an imported host — i.e. the one
action guaranteed to strip the not-directly-connectable gate off a
bastion-only machine, then sync the ungated record everywhere via
markSSHHostsDirty. Fixed on both layers: the drawer now spreads the
record it opened on (same shape as duplicateHost), and UpdateSSHHost
carries the three config-derived fields over from the stored record.
The UI owns alias/host/port/user/tags/note/auth_kind/key_id and can
still clear any of them; it owns none of the ssh_config-derived three,
which only ImportSSHHosts writes.
Also:
- Skipped was only ever appended to, so a config with no Match block and
no Include trouble — the ordinary case — left it nil, marshalled null,
and threw in the drawer on .skipped.length. make() it, same as
ListSSHHosts' nil → [] , and normalise both slices in the drawer.
- Hoist the anonymous Skipped struct to a named SSHConfigImportSkipped:
wails' TS generator emits "export class {" for anonymous structs on
its slice-of-structs branch, breaking models.ts on the next build. The
hand-written shim already declared that name.
- identity_file / proxy_jump / proxy_command were write-only. Saved-host
rows now carry a proxy marker, Connect is disabled and refuses to dial
(including via double-click and the context menu), and the edit drawer
shows IdentityFile as a read-only hint by the key picker — which is the
entire reason §5.2 records the path.
- A ProxyCommand-only host no longer reads 'needs a jump host (ProxyJump
"")', in the error string or the badge.
- A confirm failure no longer blanks the entry list: preview and confirm
errors are separate refs, body and footer respectively.
- Preview rows whose alias matches a saved host are marked "Updates
existing" — which rows overwrite is the reason import is two-step.
- Skip reasons were translated to English; say so in the shim comment and
the design doc instead of still promising Chinese.
This was referenced Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #350. Roadmap P6 item 25.
What this does
Imports SSH hosts from
~/.ssh/configinstead of making you retype them. Two steps: preview (with skip reasons), then commit only the rows you tick.internal/sshconfig— a pure parser with real ssh_config semantics.The semantics that are easy to get wrong
Verified against
man 5 ssh_configon the dev machine, not against assumption:Host *block at the top beats a specific block below it. Backwards, hosts silently connect as the wrong user.ssh <alias>actually does — which makes the feature a source of confusion rather than a convenience.Includeresolves against a fixed base (~/.ssh), never the including file's own directory. The first implementation got this wrong and it survived 15 tests, because none went two levels deep. The test that now covers it plants a decoy file at the old buggy path.*and?.path.Matchalso treats[as a metacharacter and can returnErrBadPattern, so it was replaced with a hand-written matcher.Matchblocks can't be resolved statically (Match execwould run a command), so they're skipped — with a visible reason. If your config has 20 hosts and 12 import, the other 8 are accounted for on screen.Safety decisions
A host with
ProxyJump/ProxyCommandimports, is marked, and refuses to connect. It does not fall back to diallingHostName. Such a host is usually unreachable except through its bastion, so a "best effort" direct dial either times out confusingly or connects to whatever else answers at that address. Ignoring a security-relevant config directive to be helpful is the wrong default. Jump-host connection is item 27.ProxyCommandis never executed — it's an arbitrary command.The gate sits before both the credential read and the dial, and two tests pin that position from opposite sides: absence of the TOFU error proves no dial; a host with no stored credential at all proves the gate runs before the keychain is touched.
IdentityFileis recorded as a path and never read. The host lands withAuthKind="key"and an emptyKeyID, which routes into the existingerrKeyMissingprompt. Reading a private key is something you authorise explicitly through the key-import flow, not a side effect of importing a host list.Import writes no credentials at all.
Two bugs the whole-branch review caught between the tasks
Both were invisible to per-task review because each sat in a seam:
Editing an imported host erased the gate.
saveHost()built its payload from form fields only, andUpdateSSHHostreplaces the record wholesale — soproxy_jumpwas zeroed on every save. That was on the mandated path: import writes no credential, so an imported host is unusable until you open that very drawer to attach one. Fixed in both layers, andUpdateSSHHostnow preserves the three config-derived fields regardless of what a caller sends.The preview drawer threw on the most common config shape.
Skippedwas only ever appended to, so a config with nothing skipped left it nil → JSONnull→TypeErroron.length. Nine drawer tests missed it because every mock hard-codedskipped: [].Verification
ProxyJumpinsealSSHHoststurns it red.ssh_hosts_encryptedis already whitelisted atinternal/userstore/preferences.go:37, and the whitelist is per-key, not per-field — now proven rather than assumed, given item 21's whitelist silently rejecting nine keys and this same key having been broken for months.Not verified: nobody has run this against a real
~/.ssh/configwith a real jump host. Worth checking the preview againstssh -G <alias>on your own config.