fix(ssh): honour StrictHostKeyChecking=yes

[kke]

Fixes #453

hostkey/callbacks.go carried a TODO about supporting accept-new. The callback it sat on already implemented accept-new -- record the key of a host that is not in known_hosts, refuse one whose key changed -- but it was the only writable callback, and every mode except "no" was routed to it. So "yes" behaved as "accept-new": rig recorded, and connected to, the key of a host it had been told to refuse. That is the mode that exists to make a first connection fail, and the one k0sctl#813 leans on when openssh and k0sctl disagree about known_hosts.

The mode is now carried as far as the callback choice rather than being flattened into a permissive bool on the way. hostKeyPolicy has the three behaviours the four config values map onto, and picking a callback per policy needs no new hostkey API: refusing to record a key is the whole of what strict means, and that is what KnownHostsReadOnlyFileCallback already does. So "yes" takes the read-only callback against the user's own known_hosts file, "accept-new" keeps the recording one, and "no" keeps tolerating mismatches.

"ask" cannot be honoured the way OpenSSH does, since there is no terminal to ask at, and refusing every first connection instead would be a poor trade for a library that provisions hosts. It stays accept-new, which is also what an unset value has always done, and a caller that wants the refusal has "yes" to ask for it. That reading is now written down instead of implied.

One consequence to know about: under "yes" a known_hosts file that does not exist is now an error rather than a file rig creates, since there would be nothing in it to verify against.

[Copilot]

🟡 Changes recommended

Strict mode remains insecure when the known-hosts path is /dev/null.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds distinct SSH host-key policies so strict mode rejects unknown hosts.

Changes:

• Maps SSH configuration modes to strict, accept-new, or permissive behavior.
• Selects read-only validation for strict mode.
• Adds integration tests for policy behavior.

File summaries

File	Description
protocol/ssh/hostkey/callbacks.go	Documents callback semantics.
protocol/ssh/connection.go	Implements host-key policy selection.
protocol/ssh/connection_test.go	Tests strict and permissive modes.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Strict mode can reject trusted keys because it does not combine all configured user and global known-hosts files.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

protocol/ssh/connection.go:752

• This still returns after the first usable GlobalKnownHostsFile, although the option is a list and defaults to two files. Under strict checking, a key present only in a later file is therefore rejected; if the first entry is /dev/null, the new reject-all callback also prevents every later trust file from being checked. Construct one checker over all usable global files, treating /dev/null as an empty source rather than a terminal result.

		cb, err := knownhostsGlobalCallback(exp, policy, checkIP)

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Strict mode can ignore valid secondary user known-hosts files, and one new test depends on the inherited environment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

protocol/ssh/connection.go:720

• This returns after the first expandable entry without checking whether it is usable. In strict mode that path is later discarded if it is missing, so subsequent configured files are never considered; with the defaults, a missing ~/.ssh/known_hosts therefore causes a valid key in ~/.ssh/known_hosts2 to be ignored and StrictHostKeyChecking=yes rejects the host. Preserve all user-path candidates for strict validation, or continue to the first existing regular file, before combining them with the global files.

	for _, f := range c.sshConfig.UserKnownHostsFile {
		log.Trace(ctx, "trying known_hosts file from ssh config", log.KeyHost, c, log.KeyFile, f)
		if exp, err := homedir.Expand(f); err == nil {
			return exp, false

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Strict mode incorrectly allows configured global files to bypass the SSH_KNOWN_HOSTS override.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation contradicts the new empty SSH_KNOWN_HOSTS behavior under strict mode.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The migration documentation contains contradictory descriptions of the supported mode semantics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/MIGRATING-from-v0.x.md:769

• The modes also differ for a changed recorded key: the table shows no/off accepting it while the other modes refuse it. The introductory sentence should cover both dimensions.

The modes differ only in how a host that is not on file is treated:

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Strict mode ignores all but the first configured user known-hosts file.

Review details

Suppressed comments (1)

protocol/ssh/connection.go:733

• Strict mode still receives only the first UserKnownHostsFile entry because this loop returns immediately. That option supports multiple trust files and the repository defaults include both ~/.ssh/known_hosts and ~/.ssh/known_hosts2 (sshconfig/defaultconfig_unix.go:83), so a host pinned only in the second file is incorrectly rejected by StrictHostKeyChecking=yes. Keep the first path as the write target for recording policies, but pass every expanded user path into the strict read-only trust set.

	for _, f := range c.sshConfig.UserKnownHostsFile {
		log.Trace(ctx, "trying known_hosts file from ssh config", log.KeyHost, c, log.KeyFile, f)
		if exp, err := homedir.Expand(f); err == nil {
			return exp, false, false

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Critical stale-trust and cache-invalidation issues, plus moderate implementation and documentation issues, remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/MIGRATING-from-v0.x.md:798

• This overstates the aggregate checker’s behavior: a host may legitimately have multiple known keys, and knownhosts.New accepts a presented key when it matches any entry even if another file lists additional keys. The mismatch case here is specifically when the append target has no matching key and another trust source knows the host only under different keys.

- A recording mode appends a new key to the **first** user entry, again as
  OpenSSH does, but it decides whether the host is new at all against the whole
  set. So a key that any file contradicts is a mismatch, not a new host, and is
  neither recorded nor accepted.

• Files reviewed: 6/6 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Trust-source validation and contradictory migration documentation must be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

docs/MIGRATING-from-v0.x.md:759

• This new section documents all StrictHostKeyChecking modes and aggregate user/global trust sources, but the native SSH coverage table later in this file still says StrictHostKeyChecking is only used for permissive no, describes UserKnownHostsFile as a singular path, and omits GlobalKnownHostsFile. Update that table so the migration guide does not give two contradictory accounts of the newly implemented behavior.

- **`StrictHostKeyChecking`** selects how an unknown or changed host key is
  treated. All four values are recognised, but only three behave as OpenSSH does:
  `ask` has no terminal to prompt at, so it is treated as `accept-new`. See
  [modes and trust sources](#stricthostkeychecking-modes-and-trust-sources) below.

protocol/ssh/hostkey/callbacks.go:291

• The IP-checking variant has the same trust-source bypass: invalid alsoVerify paths are silently omitted by refresh, despite the exported contract requiring them to be usable. A caller can therefore get a successful callback that never checks a requested trust file. Apply the same upfront validation used by the read-only and /dev/null paths.

	knownHosts, err := newHostKeyDB(append([]string{writePath}, alsoVerify...))

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The unresolved critical slice-mutation issue and moderate path-validation regression must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

docs/MIGRATING-from-v0.x.md:805

• This overstates mismatch handling: known_hosts may legitimately contain multiple keys for one host, and a presented key is accepted when it matches any recorded entry even if other entries differ. The wording also conflicts with the preceding statement that one matching key suffices; describe a mismatch as matching none of the recorded keys instead.

- A recording mode appends a new key to the **first** user entry, again as
  OpenSSH does, but it decides whether the host is new at all against the whole
  set. So a key that any file contradicts is a mismatch, not a new host, and is
  neither recorded nor accepted.

protocol/ssh/hostkey/callbacks.go:469

• newHostKeyDB now treats a missing or non-regular path as an empty trust set, so calling WithCheckHostIP with a bad path succeeds and silently disables the IP check. The previous knownhosts.New(path) returned an error here, and the other read-only constructors preserve that validation via readOnlyChecker; use the same helper here.

	knownHosts, err := newHostKeyDB([]string{path})

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Unreadable regular known-hosts files can still abort verification despite other usable trust sources.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

protocol/ssh/hostkey/callbacks.go:485

• WithCheckHostIP now silently accepts a missing/non-regular path: newHostKeyDB filters it out and builds an empty checker, whereas the previous knownhosts.New(path) returned an error. This can unexpectedly disable the requested IP verification for callers that supply an independent callback. Build this checker through readOnlyChecker so the public wrapper still rejects an unusable trust source before returning.

	mu.Lock()
	knownHosts, err := newHostKeyDB([]string{path})
	mu.Unlock()

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate host-key verification issues could permit conflicting keys or incorrectly abort connections.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The critical snapshot atomicity issue must be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

SSH trust-policy and known-host source changes require final human review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced