Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Added

- Nh now supports the `--build-host` and `--target-host` cli arguments

- Nh now checks if the current Nix implementation has necessary experimental
features enabled. In mainline Nix (CppNix, etc.) we check for `nix-command`
and `flakes` being set. In Lix, we also use `repl-flake` as it is still
Expand Down
47 changes: 39 additions & 8 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,25 @@ use tracing::{debug, info};

use crate::installable::Installable;

fn ssh_wrap(cmd: Exec, ssh: Option<&str>) -> Exec {
if let Some(ssh) = ssh {
Exec::cmd("ssh")
.arg("-T")
.arg(ssh)
.stdin(cmd.to_cmdline_lossy().as_str())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that cmd.to_cmdline_lossy() doesn't pass unescaped inputs to the SSH command? I can't exactly imagine an "attack" vector here, but this seems a little dangerous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did check that i did do escaping when implementing it, not as much for security as in the case of it would break on weird input like a ' or smth, i cant vouch for their implementation of escaping being correct, but they atleast do something

} else {
cmd
}
}

#[derive(Debug)]
pub struct Command {
dry: bool,
message: Option<String>,
command: OsString,
args: Vec<OsString>,
elevate: bool,
ssh: Option<String>,
}

impl Command {
Expand All @@ -27,6 +39,7 @@ impl Command {
command: command.as_ref().to_os_string(),
args: vec![],
elevate: false,
ssh: None,
}
}

Expand All @@ -40,6 +53,11 @@ impl Command {
self
}

pub fn ssh(mut self, ssh: Option<String>) -> Self {
self.ssh = ssh;
self
}

pub fn arg<S: AsRef<OsStr>>(mut self, arg: S) -> Self {
self.args.push(arg.as_ref().to_os_string());
self
Expand Down Expand Up @@ -94,9 +112,9 @@ impl Command {
cmd.arg(&self.command).args(&self.args)
} else {
Exec::cmd(&self.command).args(&self.args)
}
.stderr(Redirection::None)
.stdout(Redirection::None);
};
let cmd =
ssh_wrap(cmd.stderr(Redirection::None), self.ssh.as_deref()).stdout(Redirection::None);

if let Some(m) = &self.message {
info!("{}", m);
Expand All @@ -106,9 +124,9 @@ impl Command {

if !self.dry {
if let Some(m) = &self.message {
cmd.join().wrap_err(m.clone())?;
cmd.capture().wrap_err(m.clone())?;
} else {
cmd.join()?;
cmd.capture()?;
}
}

Expand Down Expand Up @@ -141,6 +159,7 @@ pub struct Build {
installable: Installable,
extra_args: Vec<OsString>,
nom: bool,
builder: Option<String>,
}

impl Build {
Expand All @@ -150,6 +169,7 @@ impl Build {
installable,
extra_args: vec![],
nom: false,
builder: None,
}
}

Expand All @@ -168,6 +188,11 @@ impl Build {
self
}

pub fn builder(mut self, builder: Option<String>) -> Self {
self.builder = builder;
self
}

pub fn extra_args<I>(mut self, args: I) -> Self
where
I: IntoIterator,
Expand All @@ -192,9 +217,15 @@ impl Build {
.arg("build")
.args(&installable_args)
.args(&["--log-format", "internal-json", "--verbose"])
.args(&match &self.builder {
Some(host) => {
vec!["--builders".to_string(), format!("ssh://{host} - - - 100")]
}
None => vec![],
})
.args(&self.extra_args)
.stdout(Redirection::Pipe)
.stderr(Redirection::Merge)
.stdout(Redirection::Pipe)
| Exec::cmd("nom").args(&["--json"])
}
.stdout(Redirection::None);
Expand All @@ -205,8 +236,8 @@ impl Build {
.arg("build")
.args(&installable_args)
.args(&self.extra_args)
.stdout(Redirection::None)
.stderr(Redirection::Merge);
.stderr(Redirection::Merge)
.stdout(Redirection::None);

debug!(?cmd);
cmd.join()
Expand Down
8 changes: 8 additions & 0 deletions src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ pub struct OsRebuildArgs {
/// Don't panic if calling nh as root
#[arg(short = 'R', long, env = "NH_BYPASS_ROOT_CHECK")]
pub bypass_root_check: bool,

/// Deploy the configuration to a different host over ssh
#[arg(long)]
pub target_host: Option<String>,

/// Build the configuration to a different host over ssh
#[arg(long)]
pub build_host: Option<String>,
}

#[derive(Debug, Args)]
Expand Down
43 changes: 36 additions & 7 deletions src/nixos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::installable::Installable;
use crate::interface::OsSubcommand::{self};
use crate::interface::{self, OsGenerationsArgs, OsRebuildArgs, OsReplArgs};
use crate::update::update;
use crate::util::ensure_ssh_key_login;
use crate::util::get_hostname;

const SYSTEM_PROFILE: &str = "/nix/var/nix/profiles/system";
Expand Down Expand Up @@ -51,6 +52,11 @@ impl OsRebuildArgs {
fn rebuild(self, variant: OsRebuildVariant) -> Result<()> {
use OsRebuildVariant::*;

if self.build_host.is_some() || self.target_host.is_some() {
// if it fails its okay
let _ = ensure_ssh_key_login();
}

let elevate = if self.bypass_root_check {
warn!("Bypassing root check, now running nix as root");
false
Expand Down Expand Up @@ -102,6 +108,7 @@ impl OsRebuildArgs {
.extra_arg("--out-link")
.extra_arg(out_path.get_path())
.extra_args(&self.extra_args)
.builder(self.build_host.clone())
.message("Building NixOS configuration")
.nom(!self.common.no_nom)
.run()?;
Expand All @@ -121,14 +128,18 @@ impl OsRebuildArgs {
Some(spec) => out_path.get_path().join("specialisation").join(spec),
};

debug!("exists: {}", target_profile.exists());

target_profile.try_exists().context("Doesn't exist")?;

Command::new("nvd")
.arg("diff")
.arg(CURRENT_PROFILE)
.arg(&target_profile)
.message("Comparing changes")
.run()?;
if self.build_host.is_none() && self.target_host.is_none() {
Command::new("nvd")
.arg("diff")
.arg(CURRENT_PROFILE)
.arg(&target_profile)
.message("Comparing changes")
.run()?;
}

if self.common.dry || matches!(variant, Build) {
if self.common.ask {
Expand All @@ -146,14 +157,28 @@ impl OsRebuildArgs {
}
}

if let Some(target_host) = &self.target_host {
Command::new("nix")
.args([
"copy",
"--to",
format!("ssh://{}", target_host).as_str(),
target_profile.to_str().unwrap(),
])
.message("Copying configuration to target")
.run()?;
};

if let Test | Switch = variant {
// !! Use the target profile aka spec-namespaced
let switch_to_configuration =
target_profile.join("bin").join("switch-to-configuration");
let switch_to_configuration = switch_to_configuration.canonicalize().unwrap();
let switch_to_configuration = switch_to_configuration.to_str().unwrap();

Command::new(switch_to_configuration)
.arg("test")
.ssh(self.target_host.clone())
.message("Activating configuration")
.elevate(elevate)
.run()?;
Expand All @@ -163,17 +188,21 @@ impl OsRebuildArgs {
Command::new("nix")
.elevate(elevate)
.args(["build", "--no-link", "--profile", SYSTEM_PROFILE])
.arg(out_path.get_path())
.arg(out_path.get_path().canonicalize().unwrap())
.ssh(self.target_host.clone())
.run()?;

// !! Use the base profile aka no spec-namespace
let switch_to_configuration = out_path
.get_path()
.join("bin")
.join("switch-to-configuration");
let switch_to_configuration = switch_to_configuration.canonicalize().unwrap();
let switch_to_configuration = switch_to_configuration.to_str().unwrap();

Command::new(switch_to_configuration)
.arg("boot")
.ssh(self.target_host.clone())
.elevate(elevate)
.message("Adding configuration to bootloader")
.run()?;
Expand Down
22 changes: 22 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::process::{Command as StdCommand, Stdio};
use std::str;

use color_eyre::{eyre, Result};
Expand Down Expand Up @@ -41,6 +42,27 @@ pub fn get_nix_version() -> Result<String> {
Err(eyre::eyre!("Failed to extract version"))
}

/// Prompts the user for ssh key login if needed
pub fn ensure_ssh_key_login() -> Result<()> {
Copy link
Member

@NotAShelf NotAShelf May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to be too nitpicky, but there are three things I would like to note here:

  1. Execution of this function appears a little... fragile to me. It assumes ssh-add is available, and will degrade user experience if it's missing. Maybe we could check for ssh-add early and explicitly.
  2. The .unwrap() calls (e.g., Command::new("ssh-add").spawn()?.wait()?) could cause panics if errors occur. I'm not religiously opposed to unwraps and similar error handling, but for an operation as sensitive as SSH we would like to bevery explicit.
  3. This is a bit of a stretch and mostly optional. Stdio::inherit makes sense here for user interaction but could lead to unexpected behavior if nh is run in a non-interactive environment. This will likely not be the case since we don't support headless usage (not explicitly, at least) but implicit behaviour might be confusing. Take this comment with a grain of salt, I leave it to your best judgement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not nipicky at all, youre right, i should make sure to not unwrap at the call site so that we arent ever dependant on ssh-add

// ssh-add -L checks if there are any currently usable ssh keys

if StdCommand::new("ssh-add")
.arg("-L")
.stdout(Stdio::null())
.status()?
.success()
{
return Ok(());
}
StdCommand::new("ssh-add")
.stdin(Stdio::inherit())
.stdout(Stdio::inherit())
.stderr(Stdio::inherit())
.spawn()?
.wait()?;
Ok(())
}

/// Determines if the Nix binary is actually Lix
///
/// # Returns
Expand Down