feat(settings): add SSH host key checking option by water-sucks · Pull Request #209 · nix-community/nixos-cli

water-sucks · 2026-04-04T02:50:49Z

To further enable non-interactive apply workflows, this PR creates a new SSH known_hosts key policy setting that mostly corresponds to the functionality of OpenSSH's StrictHostKeyChecking setting.

By default, it is set to ask (the previous behavior as well), but it can be changed to accept-new to always accept host keys, off to disable host key checking entirely, and strict to deny unknown host keys. Any -y flags or disabling of confirmation prompts will change the default ask behavior to accept-new to allow for non-interactive execution of nixos-cli with new/unknown host keys.

Summary by CodeRabbit

New Features
- Added SSH host key verification configuration with four modes: accept-new (auto-accept new keys), ask (prompt for unknown keys), strict (reject unknown keys), and off (disable verification).
- Host key verification behavior automatically adapts based on confirmation settings.

water-sucks · 2026-04-04T02:50:59Z

@coderabbitai review

coderabbitai · 2026-04-04T02:51:03Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-04-04T02:55:50Z

📝 Walkthrough

Walkthrough

This PR introduces SSH host key verification configuration to control how unknown or changed host keys are handled. A new HostKeyVerification setting (with options: accept-new, ask, strict, off) is added to SSH settings, integrated into the activation flow with logic to override the setting when confirmation is not required, and implemented through refactored host key validation callbacks.

Changes

Cohort / File(s)	Summary
SSH Host Key Verification Settings `internal/settings/settings.go`, `internal/settings/completion.go`	Added `HostKeyVerificationType` with four constants (`accept-new`, `ask`, `strict`, `off`), validation via `UnmarshalText`, completion options, and documentation. Also formalized `ConfirmationPromptBehavior.UnmarshalText` validation.
SSH Connection Implementation `internal/system/ssh.go`	Extended `SSHConfig` and `SSHConfigOptions` with `HostKeyVerification` field. Refactored `addKeyToKnownHostsCallback` to handle host key verification based on enum value, replacing prior interactive-only logic with explicit handling for each verification mode.
Activation Flow Integration `cmd/apply/apply.go`	Computes `hostKeyVerification` from configuration and confirmation requirements, overriding `Ask` to `AcceptNew` when confirmation is not needed. Passes `hostKeyVerification` to `NewSSHConfig` calls for both target and build hosts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

feat(system): mimic SSH known_hosts key addition process #121: Modifies internal/system/ssh.go host key handling and known_hosts writing behavior for unknown/changed keys.
feat(settings): add new confirmation settings to control default/invalid input behavior #138: Touches the same confirmation flow and SSH known-hosts confirmation wiring in activation.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding a new SSH host key checking configuration option. It is concise, specific, and directly reflects the core functionality introduced across the settings and SSH configuration files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/system/ssh.go (1)
348-358: ⚠️ Potential issue | 🟡 Minor

Changed host keys are always rejected before the host_key_verification mode is evaluated.

When len(keyErr.Want) > 0 (indicating a host key has changed from what's recorded in known_hosts), the function returns an error at line 349 before reaching the hostKeyVerification switch at line 360. This means HostKeyVerificationOff mode cannot accept a changed key—the mode only applies to unknown keys.

This appears intentional as a security measure (treating key changes as potential MITM attacks), but it deviates from OpenSSH's StrictHostKeyChecking=no, which would permit changed keys. If this stricter behavior is by design, consider documenting this explicitly in code comments to clarify that all host key verification modes reject changed keys unconditionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/system/ssh.go` around lines 348 - 358, The current check for changed
host keys (len(keyErr.Want) > 0) unconditionally returns an error and prevents
the subsequent host_key_verification switch (including HostKeyVerificationOff)
from being applied; either update the logic to consult hostKeyVerification
before rejecting changed keys (i.e., move or gate this conditional so
HostKeyVerificationOff/HostKeyVerificationAcceptChanged can allow replacement)
or—if the stricter rejection is intentional—add an explicit code comment
immediately above the len(keyErr.Want) check documenting that changed keys are
always treated as fatal (references: keyErr.Want, hostKeyVerification,
HostKeyVerificationOff, knownHostsPath, ssh.FingerprintSHA256) so future readers
understand the deliberate deviation from OpenSSH StrictHostKeyChecking behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/system/ssh.go`:
- Around line 348-358: The current check for changed host keys (len(keyErr.Want)
> 0) unconditionally returns an error and prevents the subsequent
host_key_verification switch (including HostKeyVerificationOff) from being
applied; either update the logic to consult hostKeyVerification before rejecting
changed keys (i.e., move or gate this conditional so
HostKeyVerificationOff/HostKeyVerificationAcceptChanged can allow replacement)
or—if the stricter rejection is intentional—add an explicit code comment
immediately above the len(keyErr.Want) check documenting that changed keys are
always treated as fatal (references: keyErr.Want, hostKeyVerification,
HostKeyVerificationOff, knownHostsPath, ssh.FingerprintSHA256) so future readers
understand the deliberate deviation from OpenSSH StrictHostKeyChecking behavior.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 87f5c43c-ec27-48b1-8c49-7d725baf521e

📥 Commits

Reviewing files that changed from the base of the PR and between 4aa03c2 and 7a747f6.

📒 Files selected for processing (4)

cmd/apply/apply.go
internal/settings/completion.go
internal/settings/settings.go
internal/system/ssh.go

Mimics the behavior of the OpenSSH option StrictHostKeyChecking.

…n is enabled

coderabbitai bot reviewed Apr 4, 2026

View reviewed changes

water-sucks added 2 commits April 3, 2026 20:05

feat(settings): add host key verification setting for SSH hosts

42adafd

Mimics the behavior of the OpenSSH option StrictHostKeyChecking.

feat(apply): automatically add SSH host keys if automatic confirmatio…

83ce475

…n is enabled

water-sucks force-pushed the host-key-checking branch from 7a747f6 to 83ce475 Compare April 4, 2026 03:05

water-sucks enabled auto-merge (rebase) April 4, 2026 03:06

water-sucks merged commit 14dab96 into nix-community:main Apr 4, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(settings): add SSH host key checking option#209

feat(settings): add SSH host key checking option#209
water-sucks merged 2 commits intonix-community:mainfrom
water-sucks:host-key-checking

water-sucks commented Apr 4, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

water-sucks commented Apr 4, 2026

Uh oh!

coderabbitai bot commented Apr 4, 2026

Uh oh!

coderabbitai bot commented Apr 4, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

water-sucks commented Apr 4, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

water-sucks commented Apr 4, 2026

Uh oh!

coderabbitai bot commented Apr 4, 2026

Uh oh!

coderabbitai bot commented Apr 4, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

water-sucks commented Apr 4, 2026 •

edited by coderabbitai bot

Loading