test(suidhelper): security-gated test seam + parametrized E2E suite

.github/workflows/ci.yml

@@ -206,6 +206,40 @@ jobs:
           flags: rust
           fail_ci_if_error: true
 
+  suidhelper-privileged:
+    name: Rust suidhelper (privileged E2E)
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: native/suidhelper
+    steps:
+      - uses: actions/checkout@v7
+
+      # rustup reads native/suidhelper/rust-toolchain.toml and installs the pinned
+      # nightly on the first cargo call.
+      - name: Show toolchain
+        run: rustup show
+
+      - uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: native/suidhelper
+
+      - name: Install nextest
+        uses: taiki-e/install-action@v2
+        with:
+          tool: nextest
+
+      # The feature-gated E2E suites self-skip without root. Run ONLY these two as
+      # root (via sudo -E so the runner's cargo/rustup home and PATH are kept) so
+      # the root-only cases -- sys-test ok, fake-bin argv capture -- actually run.
+      # The rest of the suite (owner-axis tests assume a non-root uid) stays in the
+      # non-root `rust` job above.
+      - name: Gated E2E suites as root
+        run: |
+          sudo -E env "PATH=$PATH" \
+            cargo nextest run --features insecure_test_seams \
+              --test e2e_config --test e2e_argv
+
   # Uploads the triggering event payload so the workflow_run publisher
   # (test-results.yml) can map results back to the originating PR -- required
   # for PR comments on fork PRs. Tiny job, always runs.

native/suidhelper/Cargo.toml

@@ -6,6 +6,11 @@ description = "Tiny setuid-root helper that runs a whitelisted set of device com
 license = "AGPL-3.0-or-later"
 repository = "https://github.com/harmont-dev/hyper"
 
+[features]
+# TEST ONLY. Compiles the security_gate insecure arms. Never enable for a real
+# (setuid) build — build.rs refuses to compile a release binary with this on.
+insecure_test_seams = []
+
 [[bin]]
 name = "hyper-suidhelper"
 path = "src/main.rs"
@@ -21,6 +26,34 @@ path = "tests/util/safe_path.rs"
 name = "safe_dev"
 path = "tests/util/safe_dev.rs"
 
+[[test]]
+name = "security_gate"
+path = "tests/security_gate.rs"
+
+[[test]]
+name = "config_seam"
+path = "tests/config_seam.rs"
+
+[[test]]
+name = "util_safe_bin"
+path = "tests/util/safe_bin.rs"
+
+[[test]]
+name = "tools_dmsetup_parsers"
+path = "tests/tools/dmsetup_parsers.rs"
+
+[[test]]
+name = "util_confinement"
+path = "tests/util/confinement.rs"
+
+[[test]]
+name = "e2e_config"
+path = "tests/e2e/config.rs"
+
+[[test]]
+name = "e2e_argv"
+path = "tests/e2e/argv.rs"
+
 [dependencies]
 clap = { version = "4", features = ["derive"] }
 nix = { version = "0.29", features = ["user", "fs", "dir"] }
@@ -31,6 +64,7 @@ toml = { version = "0.8", default-features = false, features = ["parse"] }
 
 [dev-dependencies]
 proptest = "1"
+tempfile = "3"
 
 [profile.release]
 strip = true

native/suidhelper/README.md

@@ -75,3 +75,24 @@ elsewhere via dm-thin), so there is no read-write attach.
   must be **snapshot** targets only (no `linear`/`crypt`/… that could map
   arbitrary devices). The device path checks reject `..` traversal. Anything
   else exits non-zero without running.
+
+## Testing
+
+- `cargo nextest run` — pure layers (parser matrix, confinement primitives, gate
+  defaults). No root, no config, no Docker.
+- `cargo nextest run --features insecure_test_seams` — adds the config-seam and
+  subprocess suites. Cases needing root self-skip with a loud `SKIP` line.
+- As root (`sudo -E env "PATH=$PATH" cargo nextest run --features
+  insecure_test_seams --test e2e_config --test e2e_argv`) — runs every case,
+  including `sys-test ok` and the fake-bin argv-capture assertions. CI does this
+  in the `suidhelper-privileged` job.
+
+### The insecure test seam
+
+`security_gate` is the single decision point for INSECURE TEST MODE. The insecure
+code path (the `HYPER_SETUIDHELPER_CONFIG_PATH` config override) activates only
+when BOTH gates are open: the `insecure_test_seams` Cargo feature (compile-time)
+AND `HYPER_SETUIDHELPER_IS_INSECURE_MODE=1` (runtime). Release builds never enable
+the feature — `build.rs` refuses to compile a release binary with it — so the
+installed setuid binary always takes the secure arm regardless of the
+environment.

native/suidhelper/build.rs

@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+//! Build guard: keep the `insecure_test_seams` feature out of any release
+//! artifact. Tests build in the `debug` profile, so they are unaffected.
+
+fn main() {
+    // Re-run only when the relevant inputs change.
+    println!("cargo:rerun-if-changed=build.rs");
+
+    if std::env::var_os("CARGO_FEATURE_INSECURE_TEST_SEAMS").is_some() {
+        println!(
+            "cargo:warning=hyper-suidhelper built with `insecure_test_seams` \
+             — TEST ONLY. Never install this binary setuid."
+        );
+        if std::env::var("PROFILE").as_deref() == Ok("release") {
+            panic!(
+                "refusing to build a RELEASE binary with `insecure_test_seams`: \
+                 it would ship a setuid security bypass"
+            );
+        }
+    }
+}

native/suidhelper/src/config.rs

@@ -5,26 +5,40 @@ use crate::util::safe_file::{self, IsRegularFile, OnlyRootWritable, RootOwner, S
 use crate::util::safe_path::{self, IsAbsolute, SafePath, StrictComponents};
 use nix::fcntl::OFlag;
 use serde::Deserialize;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 use std::sync::LazyLock;
 use thiserror::Error;
 
-#[derive(Debug, Copy, Clone, Error)]
+#[derive(Debug, Clone, Error)]
 pub enum LoadingError {
     #[error(transparent)]
     Path(#[from] safe_path::ValidationError),
     #[error(transparent)]
     File(#[from] safe_file::ValidationError),
     #[error("{0:?} could not be read")]
-    Unreadable(&'static Path),
+    Unreadable(PathBuf),
     #[error("{0:?} is not valid TOML")]
-    Malformed(&'static Path),
+    Malformed(PathBuf),
     #[error("work_dir in {0:?} must be an absolute path")]
-    Relative(&'static Path),
+    Relative(PathBuf),
 }
 
 const CONFIG_PATHSTR: &str = "/etc/hyper/config.toml";
-static CONFIG_PATH: LazyLock<PathBuf> = LazyLock::new(|| PathBuf::from(CONFIG_PATHSTR));
+const INSECURE_CONFIG_PATH_ENV: &str = "HYPER_SETUIDHELPER_CONFIG_PATH";
+
+/// The config file path. In production this is the fixed `/etc/hyper/config.toml`.
+/// Only in INSECURE TEST MODE (both gates open) may an env var redirect it — the
+/// secure arm is always the hardcoded path, so a release build cannot be steered.
+fn config_path() -> PathBuf {
+    crate::security_gate::split(
+        || PathBuf::from(CONFIG_PATHSTR),
+        || {
+            std::env::var(INSECURE_CONFIG_PATH_ENV)
+                .map(PathBuf::from)
+                .unwrap_or_else(|_| PathBuf::from(CONFIG_PATHSTR))
+        },
+    )
+}
 
 /// Hyper's /etc/hyper/config.toml file format.
 #[derive(Debug, Clone, Deserialize)]
@@ -51,7 +65,7 @@ impl Config {
     }
 
     /// Hyper's data root.
-    pub fn hyper_base(&self) -> &Path {
+    pub fn hyper_base(&self) -> &std::path::Path {
         self.work_dir.as_path()
     }
 
@@ -63,17 +77,17 @@ impl Config {
     /// Read, ownership-check, parse, and validate the config file. See the module
     /// docs for the trust model.
     pub fn safe_load() -> Result<Self, LoadingError> {
-        let path = CONFIG_PATH.as_path();
+        let path = config_path();
 
-        let safe_path: SafePath<IsAbsolute, StrictComponents> =
-            PathBuf::from(CONFIG_PATHSTR).try_into()?;
+        let safe_path: SafePath<IsAbsolute, StrictComponents> = path.clone().try_into()?;
 
         let file: SafeFile<IsRegularFile, RootOwner, OnlyRootWritable> =
             SafeFile::open(&safe_path, OFlag::O_RDONLY)?;
 
         let body = std::io::read_to_string(std::fs::File::from(file.into_owned_fd()))
-            .map_err(|_| LoadingError::Unreadable(path))?;
-        let config: Config = toml::from_str(&body).map_err(|_| LoadingError::Malformed(path))?;
+            .map_err(|_| LoadingError::Unreadable(path.clone()))?;
+        let config: Config =
+            toml::from_str(&body).map_err(|_| LoadingError::Malformed(path.clone()))?;
 
         if !config.work_dir.is_absolute() {
             return Err(LoadingError::Relative(path));

native/suidhelper/src/lib.rs

@@ -8,5 +8,6 @@
 //! point over these modules.
 
 pub mod config;
+pub mod security_gate;
 pub mod tools;
 pub mod util;

native/suidhelper/src/main.rs

@@ -66,10 +66,12 @@ impl SysTest {
 }
 
 fn main() {
+    // Resolve the security gate before anything else reads it (the config load
+    // below consults it). In release this is a no-op: the gate stays secure.
+    hyper_suidhelper::security_gate::init();
+
     // Privileges are already dropped to the real uid by a pre-main constructor
     // (see `setuid_privileged`); root is only re-acquired inside `Privileged`.
-    // Load the config now - we are post-drop (real uid) and before any
-    // `Privileged` scope, so the file is read unprivileged, never as root.
     config::Config::init();
 
     // Each command yields a serializable value (errors stringified to unify); we

native/suidhelper/src/security_gate.rs

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+//! Central security gate: the single place that decides whether this process is
+//! running in INSECURE TEST MODE, resolved once at startup.
+//!
+//! Two independent gates, both required to open the insecure path:
+//!   1. compile-time — the `insecure_test_seams` Cargo feature. Never enabled in
+//!      release; `build.rs` refuses a release build with it on.
+//!   2. runtime — `HYPER_SETUIDHELPER_IS_INSECURE_MODE=1` in the environment.
+//!
+//! Seams never test these directly. They call [`split`], which pairs every
+//! insecure branch with its production default and can only run the insecure arm
+//! when BOTH gates are open. The default — uninitialized, or any release build —
+//! is always the secure arm, so a forgotten `init` fails safe.
+
+use std::sync::atomic::{AtomicBool, Ordering};
+
+const INSECURE_MODE_ENV: &str = "HYPER_SETUIDHELPER_IS_INSECURE_MODE";
+
+/// Resolved once by [`init`]; defaults to secure so a missing `init` is safe.
+static INSECURE: AtomicBool = AtomicBool::new(false);
+
+/// Resolve the gate from the compile feature and the environment. Call once, as
+/// the very first thing in `main`, before any seam (e.g. the config load) runs.
+/// Idempotent.
+pub fn init() {
+    let insecure = cfg!(feature = "insecure_test_seams")
+        && std::env::var(INSECURE_MODE_ENV).as_deref() == Ok("1");
+    if insecure {
+        eprintln!(
+            "hyper-suidhelper: WARNING: running in INSECURE TEST MODE \
+             ({INSECURE_MODE_ENV}=1); never do this on a real host"
+        );
+    }
+    INSECURE.store(insecure, Ordering::Relaxed);
+}
+
+/// Run `secure` in production; run `insecure` in testing.
+pub fn split<T>(secure: impl FnOnce() -> T, insecure: impl FnOnce() -> T) -> T {
+    if cfg!(feature = "insecure_test_seams") && INSECURE.load(Ordering::Relaxed) {
+        insecure()
+    } else {
+        secure()
+    }
+}

native/suidhelper/src/tools/dmsetup/mod.rs

@@ -7,12 +7,12 @@ mod thin_pool;
 use super::IsTool;
 use crate::util::safe_dev::{self, DmName};
 use clap::{Args, Subcommand};
-use message::ThinMessage;
+pub use message::ThinMessage;
 use serde::Serialize;
 use std::io;
 use std::path::PathBuf;
 use std::process::{Command, Output};
-use table::DmTable;
+pub use table::DmTable;
 use thiserror::Error as ThisError;
 
 #[derive(Debug, ThisError)]

native/suidhelper/src/tools/mod.rs

@@ -10,7 +10,7 @@ mod losetup;
 
 pub use blockdev::{Blockdev, BlockdevArgs};
 pub use chroot_jail::ChrootJailOp;
-pub use dmsetup::{Dmsetup, DmsetupArgs};
+pub use dmsetup::{DmTable, Dmsetup, DmsetupArgs, ThinMessage};
 pub use losetup::{Losetup, LosetupArgs};
 
 use crate::util::safe_bin::SafeBin;

native/suidhelper/tests/config_seam.rs

@@ -0,0 +1,26 @@
+//! The config-path seam routes through security_gate::split. With both gates
+//! open, safe_load reads HYPER_SETUIDHELPER_CONFIG_PATH instead of the hardcoded
+//! /etc/hyper/config.toml — but with every production check (SafePath lexical
+//! gate, SafeFile ownership/type) still enforced. A `..` override is rejected by
+//! the real SafePath gate as LooseComponents; the default path is lexically
+//! clean, so that error proves the override (not the default) was used. No root
+//! needed and no security check is bypassed.
+#![cfg(feature = "insecure_test_seams")]
+
+use hyper_suidhelper::config::{Config, LoadingError};
+use hyper_suidhelper::security_gate;
+use hyper_suidhelper::util::safe_path::ValidationError;
+
+#[test]
+fn override_path_is_consulted_through_the_real_lexical_gate() {
+    let loose = "/tmp/hyper-suidhelper/../escape.toml";
+    std::env::set_var("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1");
+    std::env::set_var("HYPER_SETUIDHELPER_CONFIG_PATH", loose);
+    security_gate::init();
+
+    let err = Config::safe_load().expect_err("override must be consulted");
+    assert!(
+        matches!(err, LoadingError::Path(ValidationError::LooseComponents)),
+        "expected the override path to hit the real lexical gate, got {err:?}",
+    );
+}