chore: follow-ups from host key verification and error chain work

[inureyes]

Problem / Background

Five independent, low-risk follow-ups identified while implementing #238 (PR #240) and #239 (PR #241), deliberately left out of those PRs to keep their diffs focused. They are unrelated to each other beyond provenance, so they can be picked up individually. The security-relevant findings from the same work are tracked separately.

1. test_expand_path_with_tilde fails nondeterministically under the full test suite

commands::interactive::utils::tests::test_expand_path_with_tilde fails intermittently under cargo test --lib while passing reliably in isolation on any commit, including clean main. It is not a regression, and it has been observed both passing and failing on identical code.

Root cause: seven tests set the process-global HOME variable through EnvGuard::set("HOME", ...), at src/config/tests.rs:55, src/ssh/auth.rs:980, src/jump/chain/auth.rs:650, src/jump/chain/auth.rs:705, src/ssh/client/connection.rs:388, src/ssh/client/connection.rs:439, and src/ssh/client/connection.rs:480 (line numbers as of c3b8ac2). std::env::set_var mutates state shared by the whole process, and cargo runs tests in parallel threads. Meanwhile expand_path (src/commands/interactive/utils.rs:51-64) calls dirs::home_dir() internally and the test's assertion calls dirs::home_dir() again, so when HOME is repointed between the two calls the starts_with(&home) assertion fails.

src/config/tests.rs:55 is the most likely primary trigger and deserves attention first: unlike the other six, which point HOME at a TempDir whose path still begins with the real temp root, it sets HOME to the fixed fake path /home/user. When it interleaves with test_expand_path_with_tilde, expand_path can resolve against /home/user while the assertion's own dirs::home_dir() call resolves against the real home, which is exactly the observed failure shape. A fix that serializes only some of these sites will not close the race.

Any test that mutates HOME is both exposed to and a contributor to this race, so the fix should be structural rather than a change to the one failing assertion.

• Serialize all HOME-mutating tests behind a shared mutex (having EnvGuard acquire a static Mutex for process-sensitive keys is one option), or refactor the affected code to accept an injected home directory so the tests need no env mutation at all.
• cargo test --lib passes repeatedly (at least 10 consecutive runs) with no intermittent failure.
• Document in the test helper why HOME mutation needs serialization, so it is not reintroduced.

2. known_hosts lookup is fully serialized on every connection

verify_accept_new and verify_known_hosts_file in src/ssh/tokio_client/host_verification.rs hold the process-wide KNOWN_HOSTS_LOCK across the entire check-then-record sequence, including for hosts that are already known and require no write at all. The check itself parses the whole known_hosts file. bssh's core use case is fanning out to many nodes in parallel, so every connection's lookup queues behind every other connection's lookup.

Measured lookup cost during review, for a host not present in the file: roughly 48 microseconds (plain entries) and 161 microseconds (hashed entries) at 1,000 entries, 300 microseconds and 807 microseconds at 5,000 entries, and 1.04 milliseconds and 3.50 milliseconds at 20,000 entries. Across a 512-node fan-out that is roughly 25 to 400 milliseconds of serialized time for realistic file sizes, which is why it was judged acceptable and left as is. Pathological files reach seconds.

The obvious improvement is double-checked locking, and it must preserve the duplicate-append guarantee the lock exists for:

• Run the lookup unlocked first. If it returns a definite match, accept without ever taking the lock.
• Only when the host appears unknown or conflicting, take the lock and re-run the lookup inside the critical section before acting on the result.
• The existing 8-way concurrent first-connect test still records exactly one entry, and a test asserts that already-known-host lookups are not serialized against each other.
• Measure before and after, so the change is justified by numbers rather than assumption.

3. Divergent duplicate ServerCheckMethod enum

src/shared/auth_types.rs:275-291 defines a second ServerCheckMethod enum, re-exported at src/shared/mod.rs:66. It is never referenced outside its own module's tests (src/shared/auth_types.rs:365-372), has no conversion to or from the client type in src/ssh/tokio_client/authentication.rs, and has now diverged: it lacks the AcceptNewKnownHostsFile variant added by PR #241, so it cannot represent the default host key checking mode. A dead type that silently cannot express the default mode is a trap for whoever reaches for it next.

• Either delete the duplicate along with its re-export and tests, or add the missing variant plus a documented conversion to the client enum.

4. A brand-new known_hosts file gets a leading blank line

russh::keys::known_hosts::learn_known_hosts_path probes for a trailing newline with file.seek(SeekFrom::End(-1)). On an empty file that seek fails (the resulting offset is negative), so ends_in_newline stays false and a \n is written before the first entry. Since PR #241 pre-creates known_hosts with mode 0600 before calling the helper, the file exists and is empty at that point, so a fresh known_hosts always begins with a blank line. It is harmless (the parser skips empty lines, and bssh's own tests filter them) and differs cosmetically from what OpenSSH writes.

• Skip the leading newline when the file is empty, either by handling it in bssh before delegating or by fixing it upstream in russh.

5. ToSocketAddrsWithHostname for &[SocketAddr] produces a comma-joined hostname

src/ssh/tokio_client/to_socket_addrs_with_hostname.rs:104-109 implements hostname() for &[SocketAddr] by joining every address's IP with commas. For two or more addresses the result is not a hostname. PR #241 added ensure_recordable_hostname, which rejects , because a comma is the known_hosts host-list separator and would otherwise silently pin one key for several hosts, so this impl now fails closed rather than corrupting the file. It is used only in tests today, so nothing is broken, but the signature invites a production caller that would immediately fail.

• Return a single meaningful hostname (for example the first address), or remove the impl if it exists only for test convenience.