refactor: insecure mode cleanup

.github/workflows/ci.yml

@@ -302,7 +302,7 @@ jobs:
       - name: Gated tests as root + coverage
         run: |
           sudo -E env "PATH=$PATH" \
-            cargo llvm-cov nextest --features insecure_test_seams \
+            cargo llvm-cov nextest --features insecure_mode \
             -E 'test(/_as_root$/)' --lcov --output-path lcov-privileged.info
 
       - name: Upload coverage to Codecov

AGENTS.md

@@ -47,8 +47,8 @@ run in CI's `integration` job, which provisions the GitHub-hosted runner via
   `lib/hyper/redist/` — **pure cores**: units algebra, strict number parsing,
   EWMA/rate controls, `/proc` parsers, hashing.
   No processes, no I/O. These are where property tests pay off most.
-- `native/suidhelper/` — the privileged Rust helper. Source in `src/`, tests in
-  `tests/` (see Rust rules below).
+- `native/suidhelper/` — the privileged Rust helper. Source in `src/`, tests
+  inline in `src/` or in `tests/` (see Rust rules below).
 - Generated, do not hand-edit: `lib/hyper/firecracker/api/{operations,schemas}`
   (regen `mix firecracker.gen`) and `lib/hyper/grpc/v1/hyper.pb.ex`
   (regen `mix grpc.gen`). Both are gitignored and rebuilt by a Mix compiler.
@@ -157,13 +157,14 @@ If a case needs its own assertions, it is a separate test, not a row.
 
 ### Rust specifics (`native/suidhelper/`)
 
-- **Tests live in `tests/`, never inline `#[cfg(test)]` in `src/`.** The crate
-  is split into a lib (`src/lib.rs`, crate `hyper_suidhelper`) and a thin bin
-  (`src/main.rs`) precisely so integration tests can reach the internals.
-  **Exception: `src/linux/`.** Tests for these utilities are colocated with
-  the code, in a `#[cfg(test)] mod tests` block at the bottom of the file
-  under test.
-- Each test file must be registered as a `[[test]]` target in `Cargo.toml`
+- **We are migrating tests inline.** New tests for a module go in a
+  `#[cfg(test)] mod tests` block at the bottom of the file under test, not in
+  `tests/`. Many existing tests still live in `tests/` (`src/linux/` and
+  `src/insecure_mode.rs` are already migrated); when you substantially change a
+  module whose tests are still in `tests/`, move them inline as part of the
+  change. `tests/` remains the home for tests that drive the built binary or
+  span modules (`tests/e2e/`, `tests/config_seam.rs`).
+- Each file in `tests/` must be registered as a `[[test]]` target in `Cargo.toml`
   (`tests/` subdirectories are not auto-discovered) — copy an existing
   `[[test]]` block when adding one.
 - `proptest` is the dev-dependency; use `proptest!{ #[test] fn ... }` with

native/suidhelper/Cargo.toml

@@ -13,9 +13,9 @@ license = "MIT"
 repository = "https://github.com/harmont-dev/hyper"
 
 [features]
-# TEST ONLY. Compiles the security_gate insecure arms. Never enable for a real
+# TEST ONLY. Compiles the insecure_mode insecure arms. Never enable for a real
 # (setuid) build — build.rs refuses to compile a release binary with this on.
-insecure_test_seams = []
+insecure_mode = []
 
 [[bin]]
 name = "hyper-suidhelper"
@@ -24,7 +24,7 @@ path = "src/main.rs"
 [[bin]]
 name = "fake-jailer"
 path = "tests/e2e/fake_jailer.rs"
-required-features = ["insecure_test_seams"]
+required-features = ["insecure_mode"]
 
 # Integration tests mirror the `src/` module tree under `tests/`. Cargo only
 # auto-discovers test files at the top level of `tests/`, so each nested file
@@ -33,10 +33,6 @@ required-features = ["insecure_test_seams"]
 name = "safe_path"
 path = "tests/util/safe_path.rs"
 
-[[test]]
-name = "security_gate"
-path = "tests/security_gate.rs"
-
 [[test]]
 name = "config_seam"
 path = "tests/config_seam.rs"

native/suidhelper/README.md

@@ -80,18 +80,18 @@ elsewhere via dm-thin), so there is no read-write attach.
 
 - `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
+- `cargo nextest run --features insecure_mode` — 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,
+  insecure_mode --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
+`insecure_mode` 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)
+when BOTH gates are open: the `insecure_mode` 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

native/suidhelper/build.rs

@@ -1,19 +1,19 @@
 // SPDX-License-Identifier: MIT
-//! Build guard: keep the `insecure_test_seams` feature out of any release
+//! Build guard: keep the `insecure_mode` 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() {
+    if std::env::var_os("CARGO_FEATURE_INSECURE_MODE").is_some() {
         println!(
-            "cargo:warning=hyper-suidhelper built with `insecure_test_seams` \
+            "cargo:warning=hyper-suidhelper built with `insecure_mode` \
              — 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`: \
+                "refusing to build a RELEASE binary with `insecure_mode`: \
                  it would ship a setuid security bypass"
             );
         }

native/suidhelper/src/config.rs

@@ -84,7 +84,7 @@ pub fn validate_uid_gid_range(r: &UidGidRange) -> Result<(), LoadingError> {
 /// 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(
+    crate::insecure_mode::select(
         || PathBuf::from(CONFIG_PATHSTR),
         || {
             std::env::var(INSECURE_CONFIG_PATH_ENV)

native/suidhelper/src/insecure_mode.rs

@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: MIT
+//! Utility for deciding whether the program is running in insecure mode.
+//!
+//! Insecure mode is enabled by both building with the `insecure_mode` flag, as well as
+//! setting `$HYPER_SETUIDHELPER_IS_INSECURE_MODE`.
+//!
+//! Insecure mode is designed to help make testing easier. The only public function you should use
+//! is [`select`]. Callers control the semantics of what "insecure" means.
+use std::sync::LazyLock;
+
+const INSECURE_MODE_ENV: &str = "HYPER_SETUIDHELPER_IS_INSECURE_MODE";
+
+static INSECURE: LazyLock<bool> = LazyLock::new(|| {
+    let insecure =
+        cfg!(feature = "insecure_mode") && 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
+});
+
+/// Run `secure` in production; run `insecure` in testing.
+pub fn select<T>(secure: impl FnOnce() -> T, insecure: impl FnOnce() -> T) -> T {
+    if cfg!(feature = "insecure_mode") && *INSECURE {
+        insecure()
+    } else {
+        secure()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[cfg(not(feature = "insecure_mode"))]
+    #[test]
+    fn secure_arm_always_taken_without_feature() {
+        std::env::set_var(INSECURE_MODE_ENV, "1");
+        assert_eq!(select(|| "secure", || "insecure"), "secure");
+    }
+
+    #[cfg(feature = "insecure_mode")]
+    #[test]
+    fn secure_arm_when_env_absent_even_with_feature() {
+        std::env::remove_var(INSECURE_MODE_ENV);
+        assert_eq!(select(|| "secure", || "insecure"), "secure");
+    }
+
+    #[cfg(feature = "insecure_mode")]
+    #[test]
+    fn insecure_arm_when_feature_and_env() {
+        std::env::set_var(INSECURE_MODE_ENV, "1");
+        assert_eq!(select(|| "secure", || "insecure"), "insecure");
+    }
+
+    #[cfg(feature = "insecure_mode")]
+    #[test]
+    fn secure_arm_when_env_not_exactly_one() {
+        std::env::set_var(INSECURE_MODE_ENV, "true");
+        assert_eq!(select(|| "secure", || "insecure"), "secure");
+    }
+}

native/suidhelper/src/lib.rs

@@ -10,8 +10,8 @@
 pub mod checksum;
 pub mod config;
 pub mod fs;
+pub mod insecure_mode;
 pub mod linux;
 pub mod process;
-pub mod security_gate;
 pub mod tools;
 pub mod util;

native/suidhelper/src/main.rs

@@ -89,13 +89,6 @@ impl SysTest {
 }
 
 fn run(mut token: Unprivileged) {
-    // The gate must resolve before clap parsing: some subcommands' value_parsers
-    // (losetup attach's backing-file gate) consult the config while clap parses
-    // argv, and the config consults this gate. Resolving the gate first ensures
-    // that first config load — wherever it happens — sees the right gate state.
-    // In release this is a no-op: the gate stays secure.
-    hyper_suidhelper::security_gate::init();
-
     let command = Cli::parse().command;
 
     // `version` is a pure self-report: it touches neither config nor privileges,

native/suidhelper/tests/config_seam.rs

@@ -1,22 +1,20 @@
-//! The config-path seam routes through security_gate::split. With both gates
+//! The config-path seam routes through insecure_mode::select. 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")]
+#![cfg(feature = "insecure_mode")]
 
 use hyper_suidhelper::config::{Config, LoadingError};
 use hyper_suidhelper::fs::path::{ComponentError, ValidationError};
-use hyper_suidhelper::security_gate;
 
 #[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!(

native/suidhelper/tests/e2e/argv.rs

@@ -2,7 +2,7 @@
 //! the one thing the design deliberately hides from the caller. We point the
 //! tool's config path at a root-owned fake that writes its argv+env to a file as
 //! JSON, then assert on the reconstructed command line.
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use predicates::str::contains;
 use std::fs;

native/suidhelper/tests/e2e/chroot_jail.rs

@@ -8,7 +8,7 @@
 //! likewise privileged `mknod` — no unit-level substitute exists; it is proven
 //! only here, root-gated, with the insecure config seam supplying a `[network]`
 //! table.
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use std::fs;
 use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt};

native/suidhelper/tests/e2e/config.rs

@@ -1,7 +1,7 @@
 //! L5: the binary's startup config contract, observed end to end (exit code +
 //! stderr + stdout JSON). Uses the config-path seam to point at a per-test
 //! tempfile instead of /etc/hyper, so tests never touch the real host.
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use hyper_suidhelper::config::{BinError, Config};
 use hyper_suidhelper::process::binary;

native/suidhelper/tests/e2e/jailer.rs

@@ -3,7 +3,7 @@
 //! propagates. We point the config's `jailer` at a root-owned recorder that
 //! writes its argv and its `/proc/self/environ` to files, then exits with a known
 //! code. Root-only: `become_root_permanently` requires we are already root.
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use std::fs;
 use std::os::unix::fs::PermissionsExt;

native/suidhelper/tests/e2e/support.rs

@@ -22,7 +22,7 @@ pub fn is_root() -> bool {
     nix::unistd::geteuid().is_root()
 }
 
-/// Run the helper with the security gate open and the config path redirected
+/// Run the helper with insecure mode open and the config path redirected
 /// to a per-test file, so tests never touch the real host's /etc/hyper.
 pub fn run(config: &Path, args: &[&str]) -> Assert {
     let mut cmd = Command::new(BIN);

native/suidhelper/tests/e2e/thin_dump.rs

@@ -3,7 +3,7 @@
 //! operands at parse time, translate the XML to the ranges JSON, and surface
 //! both tool failure and a wrong dev_id as errors — an empty result must never
 //! mean "wrong dump".
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use predicates::str::contains;
 use std::fs;

native/suidhelper/tests/e2e/version.rs

@@ -6,7 +6,7 @@
 //! Contract under test: exit 0 with a single JSON object on stdout where
 //! `version` == the crate version and `checksum_blake3` is 32 bytes of
 //! lowercase hex (all-zero on an unstamped dev build -- shape, not value).
-#![cfg(feature = "insecure_test_seams")]
+#![cfg(feature = "insecure_mode")]
 
 use std::path::Path;