hive-priv: write a configured stable exe path into unit drop-ins

HIVE_PRIV_EXE, set by the module to the /run/hive-rootfs-check link, is
checked at start and used for the drop-in's ExecStartPre. Without it the
running binary's path is used as before, and an unlinked one gets an
error that names the cause.

hive-priv/src/container_config.rs

@@ -7,8 +7,10 @@
 //! Every op that names a container takes its system name and refuses anything
 //! [`validate_container_system_name`] does not accept.
 
+use std::ffi::OsStr;
 use std::os::fd::{AsRawFd as _, FromRawFd as _, OwnedFd};
 use std::path::{Path, PathBuf};
+use std::sync::OnceLock;
 
 use anyhow::{Context as _, Result, bail};
 use hive_priv_sock::{
@@ -132,10 +134,11 @@ pub(crate) fn remove_service_dropin(container: &str) -> Result<(String, String)>
 ///
 /// The drop-in also carries the `ExecStartPre=` that runs
 /// `hive-priv check-rootfs <container>` ([`crate::prestart`]) ahead of
-/// nixpkgs' start script. It names this process's own binary, so the drop-in
-/// and the binary that implements the check are always the same build. The
-/// build is rooted against garbage collection for as long as the drop-in
-/// names it ([`crate::prestart::pin_binary`]).
+/// nixpkgs' start script. It names the binary [`hive_priv_exe`] picks: the
+/// configured link to the current build when [`EXE_ENV`] is set, else this
+/// process's own binary. A store path there is rooted against garbage
+/// collection for as long as the drop-in names it
+/// ([`crate::prestart::pin_binary`]); the link needs no root.
 ///
 /// Two different kinds of setting land in the same file. `MemoryMax=` /
 /// `CPUQuota=` are hard caps that throttle even on an idle host;
@@ -272,30 +275,76 @@ fn limits_dropin_body(
     )
 }
 
-/// This binary's absolute path, for the drop-in's `ExecStartPre=`.
-///
-/// The path is the running binary's, not a fixed one, so a drop-in never names
-/// a build without the sub-command. systemd expands `%` and `$` in command
-/// lines and splits on spaces, so a path with any of them is refused rather
-/// than escaped; store paths never have them.
-fn hive_priv_exe() -> Result<String> {
-    let exe = std::env::current_exe().context("resolve the hive-priv binary path")?;
-    let exe = exe
-        .to_str()
-        .filter(|p| {
-            p.starts_with('/')
-                && p.bytes()
-                    .all(|b| b.is_ascii_alphanumeric() || b"/._+-".contains(&b))
-        })
-        .with_context(|| {
-            format!(
-                "hive-priv binary path {} is not safe in a unit file",
-                exe.display()
-            )
-        })?;
+/// The environment variable naming the binary the drop-in's `ExecStartPre=`
+/// runs. The NixOS module sets it to a path that outlives garbage collection
+/// and switches (`/run/hive-rootfs-check`, a link to the current package).
+pub(crate) const EXE_ENV: &str = "HIVE_PRIV_EXE";
+
+/// The binary [`EXE_ENV`] named at start, once [`init_exe`] has checked it.
+static CONFIGURED_EXE: OnceLock<String> = OnceLock::new();
+
+/// Read and check [`EXE_ENV`] once at start, so a missing or unsafe value stops
+/// the daemon instead of failing the first drop-in write. With the variable
+/// unset, [`hive_priv_exe`] resolves the running binary's path at each write.
+pub(crate) fn init_exe() -> Result<()> {
+    if let Some(value) = std::env::var_os(EXE_ENV) {
+        let exe = configured_exe(&value)?;
+        // `init_exe` runs once, so the cell is empty.
+        let _ = CONFIGURED_EXE.set(exe);
+    }
+    Ok(())
+}
+
+/// `value` of [`EXE_ENV`], when it is safe in a unit file and exists.
+fn configured_exe(value: &OsStr) -> Result<String> {
+    let exe = safe_unit_path(value.to_str().unwrap_or_default())
+        .with_context(|| format!("{EXE_ENV}={} is not safe in a unit file", value.display()))?;
+    if !Path::new(exe).exists() {
+        bail!("{EXE_ENV}={exe} does not exist");
+    }
     Ok(exe.to_owned())
 }
 
+/// `path` when it can be written into a command line as is. systemd expands
+/// `%` and `$` in command lines and splits on spaces, so a path with any of
+/// them is refused rather than escaped; store paths never have them.
+fn safe_unit_path(path: &str) -> Option<&str> {
+    (path.starts_with('/')
+        && path
+            .bytes()
+            .all(|b| b.is_ascii_alphanumeric() || b"/._+-".contains(&b)))
+    .then_some(path)
+}
+
+/// The binary the drop-in's `ExecStartPre=` runs: the one [`EXE_ENV`] named at
+/// start, else this process's own.
+fn hive_priv_exe() -> Result<String> {
+    pick_exe(CONFIGURED_EXE.get(), std::env::current_exe())
+}
+
+/// [`hive_priv_exe`] with both sources as parameters; the running binary is
+/// not looked at when `configured` is set.
+fn pick_exe(configured: Option<&String>, current: std::io::Result<PathBuf>) -> Result<String> {
+    match configured {
+        Some(exe) => Ok(exe.clone()),
+        None => running_exe(current),
+    }
+}
+
+/// The path of the running binary, as [`std::env::current_exe`] reported it.
+/// Linux appends ` (deleted)` once the file is unlinked, which a unit file
+/// can't carry, so that case gets its own error.
+fn running_exe(current: std::io::Result<PathBuf>) -> Result<String> {
+    let exe = current.context("resolve the hive-priv binary path")?;
+    let shown = exe.display().to_string();
+    if shown.ends_with(" (deleted)") {
+        bail!("hive-priv's own binary {shown} was unlinked; restart hive-priv.service");
+    }
+    safe_unit_path(&shown)
+        .map(str::to_owned)
+        .with_context(|| format!("hive-priv binary path {shown} is not safe in a unit file"))
+}
+
 /// The `ExecStartPre=` command of the pre-start root check for `container`:
 /// the binary `exe`, then the sub-command and the name.
 fn rootfs_check_command(exe: &str, container: &str) -> String {
@@ -806,12 +855,12 @@ pub(crate) fn write_container_etc_file_in(rootfs: &Path, name: &str, contents: &
 mod tests {
     use super::{
         BindMount, CredentialMount, HiveFacts, NIX_DAEMON_SHADOW_FLAG, NetworkIsolation, NixBuilds,
-        Path, chosen_nix_builds, git_overlay_flags, handle_write_nspawn_flags, hive_priv_exe,
-        is_owned_var, limits_dropin_body, nspawn_flags, previous_nix_builds, private_users_line,
-        publish_file, remove_service_dropin, render_hive_facts, rootfs_check_command,
-        validate_bind_path, validate_bridge, validate_container_system_name, validate_cpu_quota,
-        validate_gateway_ip, validate_memory_max, write_bridge_dns_marker_in,
-        write_resource_limits,
+        Path, chosen_nix_builds, configured_exe, git_overlay_flags, handle_write_nspawn_flags,
+        hive_priv_exe, is_owned_var, limits_dropin_body, nspawn_flags, pick_exe,
+        previous_nix_builds, private_users_line, publish_file, remove_service_dropin,
+        render_hive_facts, rootfs_check_command, running_exe, validate_bind_path, validate_bridge,
+        validate_container_system_name, validate_cpu_quota, validate_gateway_ip,
+        validate_memory_max, write_bridge_dns_marker_in, write_resource_limits,
     };
     use crate::shared::test_support::{entries, scratch};
 
@@ -1038,6 +1087,73 @@ mod tests {
         assert!(cmd.starts_with('/'), "{cmd}");
     }
 
+    /// A configured link is used as written, even though it is not this
+    /// process's own binary, and the process's own path is never consulted.
+    #[test]
+    fn a_configured_exe_is_used_as_written() {
+        let dir = scratch();
+        let link = dir.join("hive-rootfs-check");
+        std::os::unix::fs::symlink(std::env::current_exe().unwrap(), &link).unwrap();
+        let got = configured_exe(link.as_os_str()).unwrap();
+        assert_eq!(got, link.to_str().unwrap());
+
+        let broken = Err(std::io::Error::other("no /proc"));
+        assert_eq!(pick_exe(Some(&got), broken).unwrap(), got);
+        let own = std::env::current_exe().unwrap();
+        assert_eq!(
+            pick_exe(None, Ok(own.clone())).unwrap(),
+            own.to_str().unwrap()
+        );
+    }
+
+    /// A configured value that is unsafe in a unit file, relative, empty or
+    /// missing stops the start, each for its own reason; the existing link is
+    /// the control that a good value passes.
+    #[test]
+    fn a_bad_configured_exe_is_refused_at_start() {
+        use std::ffi::OsStr;
+        let dir = scratch();
+        let good = dir.join("good");
+        std::fs::write(&good, b"").unwrap();
+        assert!(configured_exe(good.as_os_str()).is_ok());
+
+        for bad in [
+            "/store/hive priv/bin/hive-priv",
+            "/s/hive-priv%h",
+            "/s/$x",
+            "relative/hive-priv",
+            "",
+        ] {
+            let err = configured_exe(OsStr::new(bad)).unwrap_err().to_string();
+            assert!(err.contains("is not safe in a unit file"), "{bad:?}: {err}");
+        }
+        let missing = dir.join("absent");
+        let err = configured_exe(missing.as_os_str()).unwrap_err().to_string();
+        assert!(err.contains("does not exist"), "{err}");
+    }
+
+    /// Without a configured value the running binary's path is used; once the
+    /// kernel marks it unlinked the error names the cause and the fix, and a
+    /// path unsafe for another reason keeps the generic message.
+    #[test]
+    fn the_running_exe_is_the_fallback_and_an_unlinked_one_is_named() {
+        use std::path::PathBuf;
+        let ok = PathBuf::from("/nix/store/aaa-hive-priv/bin/hive-priv");
+        assert_eq!(
+            running_exe(Ok(ok)).unwrap(),
+            "/nix/store/aaa-hive-priv/bin/hive-priv"
+        );
+
+        let gone = PathBuf::from("/nix/store/aaa-hive-priv/bin/hive-priv (deleted)");
+        let err = running_exe(Ok(gone)).unwrap_err().to_string();
+        assert!(err.contains("was unlinked"), "{err}");
+        assert!(err.contains("restart hive-priv.service"), "{err}");
+
+        let spaced = PathBuf::from("/nix/store/a b/hive-priv");
+        let err = running_exe(Ok(spaced)).unwrap_err().to_string();
+        assert!(err.contains("is not safe in a unit file"), "{err}");
+    }
+
     /// The mode lands exactly as asked, whatever the umask would have made
     /// of it (0666 under the usual 022 would otherwise come out 0644), and
     /// the requested owner is on the file.

hive-priv/src/main.rs

@@ -76,6 +76,7 @@ async fn main() -> Result<()> {
         // they land in victorialogs as raw byte-array spam otherwise.
         .with_ansi(false)
         .init();
+    container_config::init_exe()?;
 
     let listener = socket_listener()?;
     if let Err(e) = files::remove_legacy_tmpfiles() {

hive-priv/src/prestart.rs

@@ -18,10 +18,10 @@
 //! never followed. Nothing is deleted or repaired, so whatever is there stays
 //! for the operator.
 //!
-//! The drop-in names the running hive-priv binary, which nothing else roots
-//! once the system has moved on to a newer build. [`pin_binary`] keeps a GC
-//! root on it for as long as the drop-in exists, and [`unpin_binary`] drops
-//! the root with the drop-in.
+//! With no configured binary the drop-in names the running hive-priv binary,
+//! which nothing else roots once the system has moved on to a newer build.
+//! [`pin_binary`] keeps a GC root on it for as long as the drop-in exists, and
+//! [`unpin_binary`] drops the root with the drop-in.
 //!
 //! The entry point is [`run`]; [`check`] holds the rules.