fix(security): implement real TOFU for accept-new host key mode - #241
Conversation
The documented and recommended default `--strict-host-key-checking accept-new` mapped to `ServerCheckMethod::NoCheck`, so `check_server_key` accepted any server key unconditionally, nothing was ever written to `~/.ssh/known_hosts`, and changed keys were never rejected, giving zero MITM protection contrary to the help text. Strict (`yes`) mode had the same defect class: a missing known_hosts file or an undeterminable path silently downgraded to `NoCheck`. Every known_hosts check error also collapsed into the generic `ServerCheckFailed`, making a changed key indistinguishable from an unreadable file.
accept-new now maps to a new path-carrying `ServerCheckMethod::AcceptNewKnownHostsFile` variant handled by a new `ssh::tokio_client::host_verification` module: a key matching its known_hosts entry is accepted, an unknown host is recorded via russh's `learn_known_hosts_path` (which handles entry formatting including the `[host]:port` form for non-22 ports) and accepted with the OpenSSH-style "Permanently added" notice, and a conflicting key is rejected with the OpenSSH warning banner carrying the offered key's SHA256 fingerprint, the offending file and line, and an `ssh-keygen -R` remediation hint, without overwriting the entry. The whole check-then-record sequence runs under a process-wide `tokio::sync::Mutex` so parallel first-time connects to the same new host record exactly one entry and never observe torn writes; the critical section is purely synchronous file I/O, and a tokio mutex keeps the guard sound inside the async handler. When the recording creates `~/.ssh` or `known_hosts`, they get modes 0700 and 0600 respectively (Unix only; pre-existing files keep their modes).
Strict mode now returns `KnownHostsFile(path)` unconditionally: russh treats a missing file as empty, so unknown hosts are rejected instead of unverified, and an undeterminable home directory falls back to `DefaultKnownHostsFile`, which fails closed. Changed keys surface as a dedicated `Error::HostKeyChanged { host, line }` in the `KnownHostsFile`, `DefaultKnownHostsFile`, and accept-new arms alike, with guidance arms added to `connect_error_message` and `format_ssh_error` that follow the #238 invariants (no cause restatement, no trailing period), and interactive password fallback explicitly never triggers on it. `get_check_method` is now a pure mapping with no filesystem side effects, the redundant strict-mode match in `jump/chain/tunnel.rs` is removed, the stale async-ssh2-tokio comments are deleted, and the hand-rolled mapping in the exec port-forwarding path (which sent accept-new through strict checking) is routed through `get_check_method` so TOFU covers every connection path: direct, first jump hop, intermediate hops, tunnel destination, SFTP, interactive, and port forwarding.
Validated with cargo test --lib for ssh::tokio_client, ssh::known_hosts, ssh::client, jump::chain, and commands::exec (all passing, including 13 new tests covering first-use recording, dedup under 8-way concurrency, non-standard port round-trip, changed-key rejection, permission handling, and strict-mode rejection with a missing file), plus cargo clippy --lib --tests -D warnings and cargo fmt. Tests inject temp known_hosts paths through the new variant instead of mutating HOME.
Refs #239
inureyes
added
type:bug
Two defects found reviewing #239's TOFU implementation. `russh::keys::check_known_hosts_path` reports `Err(KeyChanged)` only when an entry for the host exists with the same key algorithm, and `Ok(false)` both for a hostname with no entry at all and for a hostname whose entries are all of a different algorithm. accept-new treated the second case as a first use, appending the offered key next to the existing entry and accepting the connection, so a man-in-the-middle that advertises support for only an algorithm the host has not used yet bypassed pinning entirely: SSH negotiation picks the first client algorithm the server also supports, so the attacker chooses it. OpenSSH resists this by reordering its per-host key algorithm proposal to prefer already-known types, which russh cannot do per host, so `verify_accept_new` now consults `known_host_keys_path` before recording and treats any existing entry for the host as a changed key. The printed removal command did not work for non-standard ports. known_hosts records those as `[host]:port`, but the warning banner and both client-facing guidance messages emitted `ssh-keygen -R <host>`, which matches nothing, and `Error::HostKeyChanged` did not carry the port for the downstream messages to use. The variant now carries the port, the banner follows OpenSSH's `ssh-keygen -f "<file>" -R "<entry>"` form with the resolved default known_hosts path, and every printed argument is double quoted so an unmatched `[...]` glob does not make zsh reject the command. Both #240 invariants are preserved: no context layer restates its own cause, and no context message ends with a period. Refs #239
Implementation Review SummaryIntentReplace the Findings Addressed
Remaining Items
Verification
|
`russh::keys::check_known_hosts_path` maps every known_hosts line that matches the host and then gathers the per-line results with `collect::<Result<Vec<bool>, _>>()`, which short-circuits on the first `Err(KeyChanged)`. A host with two or more entries of the same key algorithm therefore fails the check as soon as one of them is not the offered key, even when another entry holds exactly that key. Position does not matter: a correct entry on line 1 with a stale entry on line 2 errors just the same. OpenSSH's `check_key_in_hostkeys` does the opposite, returning HOST_OK if any recorded key for the host matches, and reporting HOST_CHANGED only when the host has keys of the same type and none of them match. Now that `accept-new` is the CLI default, that mismatch rejected legitimate, OpenSSH-accepted known_hosts layouts with the full "REMOTE HOST IDENTIFICATION HAS CHANGED" banner. Two realistic layouts trigger it: a stale host key kept beside a rotated one for the same host and algorithm, and a cluster-style file with a comma-separated `node1,node2,node3 ssh-ed25519 <shared key>` line plus a per-node `node2 ssh-ed25519 <node2's own key>` line. In the second case, connecting to node2 while it offered its own genuine key was rejected as a changed key, and the banner pointed at the shared line rather than at any entry that actually conflicted. A false man-in-the-middle alarm is a security problem in its own right: the banner's own remediation advice tells the user to run `ssh-keygen -R <host>`, which deletes the legitimate pin and turns the next connection into a fresh trust-on-first-use that accepts whatever key is offered. `lookup_known_host` now reads the host's entries directly and applies OpenSSH's rule, so any recorded key equal to the offered key is a match and only a host with recorded keys none of which match counts as changed. Strict (`yes`) mode and `DefaultKnownHostsFile` route through the new `verify_known_hosts_file`, which shares that lookup, so every mode agrees on which keys verify and which count as changed. `DefaultKnownHostsFile` also resolves the path itself rather than letting russh resolve it with `std::env::home_dir()` while the banner text was built from `dirs::home_dir()`, closing a latent inconsistency where the check and the `ssh-keygen -f "<file>"` hint could name different files. A missing home directory still fails closed, now as `ServerCheckFailed` directly instead of via russh's `NoHomeDir`. The offending line reported in the banner now prefers an entry with the same key algorithm as the offered key, since that is the entry OpenSSH would name, and falls back to the host's first entry so an algorithm-only conflict (the alternate-algorithm pinning bypass) still points at a real line. The unknown-host path also parses known_hosts once instead of twice while holding the process-wide lock, because the entries it needs in order to tell "unknown" from "changed" are the same ones the check already read. Four regression tests cover the layouts OpenSSH accepts (a key matching either of two same-algorithm entries, and a per-host key beside a shared cluster line), the same-algorithm offending line, and strict mode agreeing with accept-new. Validation: - cargo check --lib --tests (clean) - cargo test --lib ssh::tokio_client (24 passed, including 4 new) - cargo test --lib ssh::known_hosts (5 passed) - cargo test --lib ssh::client (20 passed) - cargo clippy --lib --tests -- -D warnings (clean) - cargo fmt --all --check (clean) Refs #239
`russh::keys::known_hosts::learn_known_hosts_path` appends the entry as `{host} <key>` (or `[{host}]:{port} <key>`) applying no escaping and no validation to the hostname, while russh's own parser splits each line on `' '`, treats `,` as the host-list separator, skips any line whose first byte is `#`, and reads a leading `|1|` as a hashed host field. Before this branch nothing was ever written to known_hosts, so the hostname string never reached a file; now that accept-new records it, a hostname carrying any of those characters no longer round-trips.
Nothing on the way to `ClientHandler` validates the hostname. `security::validate_hostname` is the only validator that rejects whitespace, newlines and `#`, it is reached only from the `-H/--hosts` branch, and its result is then discarded as the connect hostname anyway because `app::nodes` overwrites it with the unvalidated `~/.ssh/config` `HostName`. `Config::from_backendai_env` builds the node list from the platform-supplied `BACKENDAI_CLUSTER_HOSTS` variable with only `str::trim()` and a non-empty check, which leaves interior spaces, tabs, newlines and `#` intact, and that source outranks every config file with no `-c` flag. YAML cluster hosts, `^hostfile` lines and `expand_env_vars` splicing get no character validation either. `AuthContext::new` is the only check on the exec, SFTP and interactive paths and rejects only NUL, LF and CR, not space, tab, other control characters, `#` or `,`. The port-forwarding path in `commands::exec` builds its `AuthMethod` inline without `AuthContext` at all, so it has no hostname check whatsoever, and that is exactly the path this branch newly gave recording powers.
Four concrete consequences, all reproduced end to end by driving `ClientHandler::check_server_key` with `AcceptNewKnownHostsFile`: a hostname containing a newline writes two lines, so the second one pins the key the server just offered for a host the user never named, and every later connection to that host either accepts the attacker's key or is refused as a changed key; a hostname containing a space writes a key field that `parse_public_key_base64` cannot parse, so the lookup returns `Err` forever and the host becomes permanently unconnectable behind a generic "Server check failed"; a hostname starting with `#` writes a line the parser skips, so every connection is a fresh first use that accepts whatever key is offered and appends another entry, growing the file without bound; and a hostname containing `,` writes a host-list entry that silently pins one server's key for every name in the list.
The guard `ensure_recordable_hostname` therefore lives in `ssh::tokio_client::host_verification` at the point of use rather than in any caller, so it covers every connection path regardless of which caller skipped validation. It rejects an empty or over-long hostname, any whitespace or control character, and `#`, `,`, `|`, `*`, `?`, `!` and `\` (the last four because the file is shared with OpenSSH, whose pattern matching would read such an entry as covering hosts it was never meant to). Both `verify_accept_new` and `verify_known_hosts_file` call it first, so strict mode reports the real reason instead of a misleading "unknown host" and the two modes stay consistent. It fails closed with `ServerCheckFailed`: a hostname that cannot be recorded cannot be verified on any later connection either, so connecting unverified would be the worse outcome.
Tests cover the newline, space, tab, `#`, `,`, `|`, `*`, empty and over-long cases (asserting nothing is written), and a regression guard proves ordinary names, IPv4, IPv6 literals, zone identifiers and the bracketed `[::1]` form still record exactly one entry.
Verified with `cargo check --lib --tests`, `cargo clippy --lib --tests -- -D warnings`, `cargo fmt --all --check`, and the `ssh::tokio_client` (27), `ssh::known_hosts` (5), `ssh::client` (20), `jump::chain` (17) and `commands::interactive` (19) library test suites.
Refs #239
Security and Performance Review (independent second pass)This pass took the earlier analysis in #241 (comment) as read and went looking for what it missed. Both of its HIGH fixes hold up (see the confirmations at the end). Two new HIGH findings are fixed here, and five lower-severity ones are reported for triage. Every claim below was reproduced empirically by driving Fixed: HIGH, accept-new rejected a host key that IS recorded (ca6425a)
Because this PR makes
A false man-in-the-middle alarm is a security problem in its own right, not just a UX one: the banner's own remediation advice is The fix reads the host's entries directly through Fixed: HIGH, the hostname is written into known_hosts with no sanitization (5590918)
I traced every hostname source to
Four consequences, all reproduced:
The guard lives in Behavior change worth calling out in the PR description: a hostname from New, not fixedMEDIUM: MEDIUM (performance): the process-wide lock serializes a full known_hosts parse on every connection, including hosts already known. Measured
Verdict: acceptable as written. For a typical known_hosts of a few hundred to a few thousand entries, a 512-node fan-out adds roughly 25 ms to 400 ms of fully serialized work, small next to 512 TCP and SSH handshakes, and the holder blocks a worker thread for well under the fraction of a millisecond where LOW: host matching is case sensitive where OpenSSH lowercases. OpenSSH canonicalizes the hostname to lowercase before the known_hosts lookup; russh's LOW: LOW: no cross-process locking. Confirming the earlier pass
Suggested follow-ups (not blocking)The two HIGH fixes are point-of-use backstops. The upstream validation gaps are still there and deserve their own issue: Verification
|
Two review passes on the accept-new TOFU implementation reported three remaining gaps that were not yet fixed: known_hosts `@revoked`/`@cert-authority` marker lines were silently ignored, host matching was case-sensitive where OpenSSH is not, and known_hosts permission tightening happened after creation instead of at creation. russh's `known_host_keys_path` splits each line on `' '` and takes the first field as the host list, so on a marker line such as `@revoked node1 ssh-ed25519 <key>` that first field is the literal string `@revoked`, which never matches `node1`. The line was therefore invisible to the lookup, and a revoked key looked like an ordinary first use: recorded and accepted while printing the normal "Permanently added" notice. A new scan in `host_verification.rs` runs before the ordinary lookup in both accept-new and strict (`yes`)/`DefaultKnownHostsFile` mode: a `@revoked` line whose host matches (exact, comma-list, or `*`/`?` glob form, erring toward matching when in doubt since failing to honor a revocation is worse than an unnecessary rejection) and whose key equals the offered key is a hard rejection with a new `HostKeyRevoked` error, while a matching `@revoked` line naming a different key does not block the offered one. A `@cert-authority` match only warns and falls through to ordinary TOFU, since bssh has no CA signature validation path and failing closed would break every working CA setup with no workaround. Hostnames are now lowercased once at entry to `verify_accept_new` and `verify_known_hosts_file`, and the normalized value is used for both the lookup and the recorded entry. russh's `match_hostname` is a plain byte compare, so without this a differently-cased hostname bypassed an existing pin and re-TOFU'd instead of hitting the same entry OpenSSH would have written. `~/.ssh` and `known_hosts` are now created with mode 0700/0600 directly via `DirBuilder`/`OpenOptions` (`#[cfg(unix)]`, with a `#[cfg(not(unix))]` fallback) before `learn_known_hosts_path` runs, instead of letting it create them with the process umask and tightening the mode only afterward. A `mkdir`/`open` syscall that requests the final mode directly cannot be widened by any standard umask, since umask only clears requested bits and neither 0700 nor 0600 carries a group or other bit to clear, so this closes the window instead of narrowing it after the fact. Pre-existing files and directories are left untouched either way, and the prior post-creation chmod is kept as a fallback for the rare case where pre-creation could not run (for example a missing grandparent directory that only `learn_known_hosts_path`'s `create_dir_all` fills in). `HostKeyRevoked` gets its own guidance arms in `connect_error_message` and `format_ssh_error`, verified against both #240 invariants (no restating the cause, no trailing period), and a regression test confirms it does not trigger interactive password fallback, matching `HostKeyChanged`. Validation: - cargo check --lib --tests (clean) - cargo clippy --lib --tests -- -D warnings (clean) - cargo fmt --all -- --check (clean) - cargo test --lib ssh::tokio_client (36 passed, 9 new: revoked matching/non-matching, cert-authority, comma-list/glob/non-standard-port marker matching, case-insensitive hostname matching for accept-new and strict mode) - cargo test --lib ssh::client (22 passed, 2 new guidance-invariant tests) - cargo test --lib commands::interactive (20 passed, 1 new password-fallback exclusion test) - cargo test --lib ssh::known_hosts (5 passed) and cargo test --lib jump::chain (17 passed) Refs #239
CHANGELOG.md's `[Unreleased]` security entry only described the first commit on this branch (implementing accept-new TOFU) and had drifted from what the branch actually ships. It now has four bullets: the original accept-new/strict-mode/HostKeyChanged work, the OpenSSH-compatible any-matching-entry acceptance rule and alternate-algorithm bypass fix, hostname sanitization before recording, and this branch's last commit (known_hosts marker handling, case-insensitive hostname matching, and the closed permission-creation window). `docs/architecture/ssh-client.md`'s host key verification section is updated to match: the accept-new bullet now describes any-matching-entry acceptance instead of "keys matching their known_hosts entry", and new bullets cover `@revoked`/`HostKeyRevoked`, the `@cert-authority` warn-and-fall-through behavior, and hostname case normalization and rejection. The PR body was also updated (via the GitHub API, since `gh pr edit` fails on this repository's Projects-Classic GraphQL deprecation) to describe the final state across all four commits instead of only the first. Refs #239
PR Finalization CompleteSummary
Verification
Commits: |
1 participant
Summary
The documented, recommended default
--strict-host-key-checking accept-newperformed no host key verification at all: it mapped toServerCheckMethod::NoCheck, accepted any server key, never wrote to~/.ssh/known_hosts, and never rejected changed keys. This PR implements real TOFU for accept-new, fixes strict (yes) mode's silent downgrade toNoCheckwhen the known_hosts file is missing, makes changed keys a first-class error in every known_hosts-based check method, and, after two review passes, closes the gaps those reviews found: an OpenSSH-compatible any-matching-entry acceptance rule, hostname sanitization before recording,@revoked/@cert-authorityknown_hosts marker line handling, case-insensitive hostname matching, and a closed permission-creation window on~/.ssh/known_hosts.Correction to the issue's verified facts
The issue's "Verified Implementation Facts" section claims russh 0.62.1 exposes no
learn_known_hostshelper and that bssh must implement the known_hosts append itself. That claim is incorrect:russh::keys::known_hosts::learn_known_hosts_path(host, port, pubkey, path)exists in russh 0.62.1 and already handles parent directory creation, newline-safe appending, the[host]:portentry form for non-22 ports, and OpenSSH key serialization. This implementation uses that library helper instead of hand-rolling the append; what the helper does not do, and what this PR adds on top, is restrictive permissions on newly created files (created up front rather than tightened afterward), serialization of concurrent writers, an OpenSSH-compatible any-matching-entry acceptance rule (the helper's owncheck_known_hosts_pathshort-circuits on the first non-matching same-algorithm entry),@revoked/@cert-authoritymarker line handling (the helper's parser reads the marker itself as the literal host field and never matches these lines), and hostname sanitization before recording.What changed
src/ssh/tokio_client/authentication.rs: new path-carryingServerCheckMethod::AcceptNewKnownHostsFile(String)variant (pluswith_accept_new_known_hosts_file), so tests can inject a temp known_hosts path instead of mutatingHOME.src/ssh/tokio_client/host_verification.rs(new): implements the trust decision for both accept-new and strict/DefaultKnownHostsFilemode through a sharedlookup_known_host, which reads a host's known_hosts entries directly and applies OpenSSH's rule: any recorded key equal to the offered key verifies, and only a host whose recorded keys none of them match counts as changed. This also closes the alternate-algorithm pinning bypass (a key offered under an algorithm the host had not used yet, whichcheck_known_hosts_pathcannot distinguish from an unknown host) and avoids the false-positive rejectioncheck_known_hosts_path's short-circuiting collection produces for legitimate layouts such as a stale key kept beside a rotated one or a cluster-style shared line beside a per-node line. Before recording or looking anything up, the module also: rejects a hostname that cannot round-trip through a known_hosts entry (empty, over 255 bytes, whitespace/control characters, or#/,/|/*/?/!/\metacharacters, sincelearn_known_hosts_pathwrites the hostname with no escaping and a malformed entry can never be verified again either); scans for@revoked/@cert-authoritymarker lines naming the host (host matching supports the exact, comma-list, and*/?glob forms, erring toward matching for@revokedwhen in doubt) and hard-rejects a key matching an@revokedentry with a newHostKeyRevokederror while warning and falling through to ordinary TOFU for a@cert-authoritymatch, since bssh has no CA signature validation to fall back to; and normalizes the hostname to lowercase once, using the normalized value for both the lookup and the recorded entry, matching OpenSSH's case-insensitive known_hosts comparison. Unknown hosts are recorded vialearn_known_hosts_pathwith the OpenSSH-stylePermanently added '<host>' (<ALGO>) to the list of known hosts.notice; conflicting keys are rejected with the OpenSSH warning banner (SHA256 fingerprint of the offered key, offending file and line,ssh-keygen -f "<file>" -R "<entry>"remediation hint naming the actual[host]:port-qualified entry) without touching the recorded entry. Recording failure warns and accepts, matching OpenSSH accept-new semantics.tokio::sync::Mutex, so parallel first-time connects to the same new host record exactly one entry and readers never observe a half-written line. A tokio mutex (notstd::sync::Mutex) was chosen because the guard lives inside the asynccheck_server_keyhandler; the critical section is purely synchronous file I/O today, and the tokio mutex keeps the guard sound if an await point is ever introduced while parking contending tasks instead of blocking runtime workers.~/.sshandknown_hostsare created with mode 0700/0600 directly (DirBuilder/OpenOptionswith an explicit mode,#[cfg(unix)]) beforelearn_known_hosts_pathruns, rather than letting it create them with the process umask and tightening the mode only afterward, closing the window in which an unusually permissive umask could leave either one group- or world-writable; pre-existing files and directories are left untouched either way, verified by test.src/ssh/tokio_client/error.rs: newError::HostKeyChanged { host, port, line }andError::HostKeyRevoked { host, port, line }variants; theKnownHostsFileandDefaultKnownHostsFilearms incheck_server_keyno longer collapserussh::keys::Error::KeyChangedinto the genericServerCheckFailed, so strict mode reports changed and revoked keys specifically too.src/ssh/known_hosts.rs:get_check_methodis now a pure mapping with no filesystem side effects (it no longer pre-creates an empty known_hosts file;learn_known_hosts_pathcreates it on demand).Yesmaps toKnownHostsFile(default path)unconditionally, since russh treats a missing file as empty, so unknown hosts are rejected instead of unverified; an undeterminable home directory falls back toDefaultKnownHostsFile, which fails closed inside russh.AcceptNewmaps toAcceptNewKnownHostsFile(default path). Stale "async-ssh2-tokio doesn't support TOFU" comments deleted.src/jump/chain/tunnel.rs: removed the redundant strict-mode match beforeget_check_method(it already mapsNotoNoCheck).src/commands/exec.rs: the port-forwarding path had its own hand-rolled mode mapping that sent accept-new through strict default-known_hosts checking (rejecting unknown hosts); it now routes throughget_check_method, so TOFU covers every connection path: direct, first jump hop, intermediate hops, destination through the tunnel, SFTP, interactive (shell and PTY), and port forwarding. Allget_check_methodcall sites were individually verified to flow intoClientHandlerwith the real hostname preserved (viaToSocketAddrsWithHostnameor direct construction), so[host]:portentries round-trip on every path.src/ssh/client/connection.rsandsrc/ssh/client/file_transfer.rs: guidance arms forHostKeyChangedandHostKeyRevoked(and, in the direct path,SshError(UnknownKey)for strict-mode unknown-host rejections) inconnect_error_messageandformat_ssh_error, following both fix: show full anyhow error chain on interactive connect failure #240 invariants: the context layer does not restate its cause and does not end with a period, enforced by tests alongside the existing ones.src/commands/interactive/connection.rs: regression tests that neitherHostKeyChangednorHostKeyRevokedever triggers interactive password fallback (retrying with a password would hand credentials to an untrusted endpoint).docs/architecture/ssh-client.mdhost key section updated to the actual behavior, including the any-matching-entry rule,@revoked/@cert-authoritymarker handling, and case-insensitive matching.CHANGELOG.mdgains four### Securityentries under[Unreleased]covering each of the above. The CLI help text (accept-new - Accept new hosts, reject changed keys (recommended)) and man page were verified to match reality and are unchanged.Deliberately out of scope
Two review passes surfaced additional lower-severity findings that are intentionally not addressed here:
KNOWN_HOSTS_LOCKis per-process only).get_check_method(AcceptNew)'s no-home-directoryNoCheckfallback.ServerCheckMethodenum insrc/shared/auth_types.rs.Test plan
cargo test --lib(1314 passed, 0 failed, exit 0)cargo test --lib ssh::tokio_client(36 passed, including the host_verification suite: first-use recording, no duplicate on reconnect, changed key rejected with entry preserved, any-matching-entry acceptance for stale-beside-rotated and cluster-shared-line layouts, alternate-algorithm rejection, 8-way concurrent first connects,[host]:2222round-trip, created dir/file get 0700/0600 with pre-existing modes preserved,@revokedmatching-key rejection and non-matching-key pass-through,@cert-authoritywarning-then-TOFU, comma-list/glob/non-standard-port marker matching, case-insensitive hostname matching for both accept-new and strict mode, strict mode parity with accept-new, hostname-sanitization rejection and regression coverage, handler end-to-end TOFU arm)cargo test --lib ssh::known_hosts(5 passed)cargo test --lib ssh::client(22 passed, including theHostKeyChanged/HostKeyRevokedguidance-message invariant tests) andcargo test --lib jump::chain(17 passed)cargo test --lib commands::interactive(20 passed, including the password-fallback exclusion tests for both error variants) andcargo test --lib commands::exec(0 tests;exec.rshas no unit tests of its own)cargo check --lib --tests,cargo clippy --lib --tests -- -D warnings, andcargo fmt --allcleancargo check --lib --target x86_64-pc-windows-gnucould not complete in this environment: the target is installed but theaws-lc-sysbuild script requiresx86_64-w64-mingw32-gcc, which is not present; the failure occurs in a C dependency before any bssh code compiles and is unrelated to this change. Windows compatibility is preserved by gating the permission code on#[cfg(unix)]with a non-unix fallback arm.Note on an unrelated pre-existing flake:
commands::interactive::utils::tests::test_expand_path_with_tildefails nondeterministically under the full parallelcargo test --libbecause several tests callEnvGuard::set("HOME", ...), mutating process-global env while that test readsdirs::home_dir()twice. It is untouched by this PR, and no test added here mutatesHOME; the newServerCheckMethod::AcceptNewKnownHostsFilevariant carries a path so every new test injects a temp known_hosts location explicitly.Closes #239