From 8cb1f63694fa921b5e9e76ff046ae99c33d37720 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Wed, 8 Jul 2026 00:22:32 +0000 Subject: [PATCH 1/5] test(suidhelper): preserve LLVM_PROFILE_FILE across e2e env scrub The e2e suite spawns the helper with env_clear(), which also stripped the llvm-cov profile path -- the instrumented child never wrote its .profraw where collection looks, so src/main.rs reported 0% coverage despite being executed by every e2e test. Dedup the four per-file spawn helpers into tests/e2e/support.rs, which re-propagates only LLVM_PROFILE_FILE (%p keeps child profiles distinct). --- native/suidhelper/tests/e2e/argv.rs | 18 ++--------- native/suidhelper/tests/e2e/chroot_jail.rs | 19 ++---------- native/suidhelper/tests/e2e/config.rs | 31 +++++-------------- native/suidhelper/tests/e2e/jailer.rs | 19 ++---------- native/suidhelper/tests/e2e/support.rs | 36 ++++++++++++++++++++++ 5 files changed, 52 insertions(+), 71 deletions(-) create mode 100644 native/suidhelper/tests/e2e/support.rs diff --git a/native/suidhelper/tests/e2e/argv.rs b/native/suidhelper/tests/e2e/argv.rs index 25c5981a..1cbd5f48 100644 --- a/native/suidhelper/tests/e2e/argv.rs +++ b/native/suidhelper/tests/e2e/argv.rs @@ -7,13 +7,9 @@ use std::fs; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; -use std::process::Command; -const HELPER: &str = env!("CARGO_BIN_EXE_hyper-suidhelper"); - -fn is_root() -> bool { - nix::unistd::geteuid().is_root() -} +mod support; +use support::{is_root, run}; /// Install a root-owned fake tool named `basename` that records its argv (minus /// argv[0]) into `record` as a JSON array, then exits 0 with valid stdout for @@ -46,16 +42,6 @@ fn write_root_config(dir: &Path, bins: &[(&str, &Path)]) -> PathBuf { p } -fn run(config: &Path, args: &[&str]) -> std::process::Output { - Command::new(HELPER) - .args(args) - .env_clear() - .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") - .env("HYPER_SETUIDHELPER_CONFIG_PATH", config) - .output() - .expect("spawn helper") -} - fn recorded_argv(record: &Path) -> Vec { let body = fs::read_to_string(record).expect("fake recorded argv"); serde_json::from_str(&body).expect("argv json") diff --git a/native/suidhelper/tests/e2e/chroot_jail.rs b/native/suidhelper/tests/e2e/chroot_jail.rs index 0d0e290e..37524a79 100644 --- a/native/suidhelper/tests/e2e/chroot_jail.rs +++ b/native/suidhelper/tests/e2e/chroot_jail.rs @@ -8,13 +8,10 @@ use std::fs; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::Command; -const BIN: &str = env!("CARGO_BIN_EXE_hyper-suidhelper"); - -fn is_root() -> bool { - nix::unistd::geteuid().is_root() -} +mod support; +use support::{is_root, run}; /// Write a root-owned (because this test runs as root) config whose work_dir is /// `work_dir`, 0644 so the OnlyRootWritable axis passes. @@ -25,16 +22,6 @@ fn write_config(dir: &Path, work_dir: &Path) -> PathBuf { p } -fn run(config: &Path, args: &[&str]) -> Output { - Command::new(BIN) - .args(args) - .env_clear() - .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") - .env("HYPER_SETUIDHELPER_CONFIG_PATH", config) - .output() - .expect("spawn helper") -} - fn setup_loop(tmp: &Path) -> Option { let backing = tmp.join("backing.img"); let f = fs::File::create(&backing).ok()?; diff --git a/native/suidhelper/tests/e2e/config.rs b/native/suidhelper/tests/e2e/config.rs index ea36b4e9..18877074 100644 --- a/native/suidhelper/tests/e2e/config.rs +++ b/native/suidhelper/tests/e2e/config.rs @@ -8,24 +8,9 @@ use hyper_suidhelper::util::safe_bin; use std::fs; use std::os::unix::fs::PermissionsExt; use std::path::Path; -use std::process::{Command, Output}; -const BIN: &str = env!("CARGO_BIN_EXE_hyper-suidhelper"); - -fn is_root() -> bool { - nix::unistd::geteuid().is_root() -} - -/// Run the helper with the gate open and the config path redirected. -fn run_with_config(config_path: &Path, args: &[&str]) -> Output { - Command::new(BIN) - .args(args) - .env_clear() - .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") - .env("HYPER_SETUIDHELPER_CONFIG_PATH", config_path) - .output() - .expect("spawn helper") -} +mod support; +use support::{is_root, run}; /// Write a config file owned root:root, 0644. Requires the test run as root. fn write_root_config(dir: &Path, body: &str) -> std::path::PathBuf { @@ -48,7 +33,7 @@ fn missing_config_falls_back_to_defaults_as_root() { } let tmp = tempfile::tempdir().unwrap(); let missing = tmp.path().join("nope.toml"); - let out = run_with_config(&missing, &["sys-test"]); + let out = run(&missing, &["sys-test"]); assert_eq!( out.status.code(), Some(0), @@ -74,7 +59,7 @@ fn non_root_owned_config_is_rejected() { let nogrp = nix::unistd::Gid::from_raw(65534); nix::unistd::chown(&p, Some(nobody), Some(nogrp)).unwrap(); } - let out = run_with_config(&p, &["sys-test"]); + let out = run(&p, &["sys-test"]); assert_eq!(out.status.code(), Some(2)); let err = String::from_utf8_lossy(&out.stderr); assert!( @@ -91,7 +76,7 @@ fn malformed_config_exits_2_malformed_as_root() { } let tmp = tempfile::tempdir().unwrap(); let p = write_root_config(tmp.path(), "this is not = valid = toml ==="); - let out = run_with_config(&p, &["sys-test"]); + let out = run(&p, &["sys-test"]); assert_eq!(out.status.code(), Some(2)); assert!(String::from_utf8_lossy(&out.stderr).contains("not valid TOML")); } @@ -104,7 +89,7 @@ fn relative_work_dir_exits_2_relative_as_root() { } let tmp = tempfile::tempdir().unwrap(); let p = write_root_config(tmp.path(), "work_dir = \"relative/path\"\n"); - let out = run_with_config(&p, &["sys-test"]); + let out = run(&p, &["sys-test"]); assert_eq!(out.status.code(), Some(2)); assert!(String::from_utf8_lossy(&out.stderr).contains("must be an absolute path")); } @@ -117,7 +102,7 @@ fn valid_config_and_setuid_yields_sys_test_ok_as_root() { } let tmp = tempfile::tempdir().unwrap(); let p = write_root_config(tmp.path(), "work_dir = \"/srv/hyper\"\n"); - let out = run_with_config(&p, &["sys-test"]); + let out = run(&p, &["sys-test"]); assert_eq!( out.status.code(), Some(0), @@ -212,7 +197,7 @@ fn bad_uid_gid_range_exits_2_as_root() { tmp.path(), "work_dir = \"/srv/hyper\"\n[jails]\nuid_gid_range = [0, 100]\n", ); - let out = run_with_config(&p, &["sys-test"]); + let out = run(&p, &["sys-test"]); assert_eq!(out.status.code(), Some(2)); let err = String::from_utf8_lossy(&out.stderr); assert!( diff --git a/native/suidhelper/tests/e2e/jailer.rs b/native/suidhelper/tests/e2e/jailer.rs index ba45468c..3b44341c 100644 --- a/native/suidhelper/tests/e2e/jailer.rs +++ b/native/suidhelper/tests/e2e/jailer.rs @@ -8,14 +8,11 @@ use std::fs; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; -use std::process::Command; -const HELPER: &str = env!("CARGO_BIN_EXE_hyper-suidhelper"); -const RECORDER_EXIT: i32 = 7; +mod support; +use support::{is_root, run}; -fn is_root() -> bool { - nix::unistd::geteuid().is_root() -} +const RECORDER_EXIT: i32 = 7; fn cat_bin() -> &'static str { ["/bin/cat", "/usr/bin/cat"] @@ -67,16 +64,6 @@ fn write_root_config(dir: &Path, jailer: &Path, firecracker: &Path) -> PathBuf { p } -fn run(config: &Path, args: &[&str]) -> std::process::Output { - Command::new(HELPER) - .args(args) - .env_clear() - .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") - .env("HYPER_SETUIDHELPER_CONFIG_PATH", config) - .output() - .expect("spawn helper") -} - #[test] fn execs_jailer_with_canonical_argv_and_empty_env_as_root() { if !is_root() { diff --git a/native/suidhelper/tests/e2e/support.rs b/native/suidhelper/tests/e2e/support.rs new file mode 100644 index 00000000..babe7228 --- /dev/null +++ b/native/suidhelper/tests/e2e/support.rs @@ -0,0 +1,36 @@ +//! Shared spawn support for the e2e suite: run the helper binary with a +//! scrubbed environment, proving it depends only on its two env seams. +//! +//! One deliberate exception to the scrub: `LLVM_PROFILE_FILE`. Under +//! `cargo llvm-cov` the helper binary is instrumented and its profile runtime +//! writes a `.profraw` to that path at exit (`%p` in the pattern keeps the +//! child's file distinct from the test process's own). A bare `env_clear()` +//! silently discards it, so every e2e execution of `src/main.rs` counted +//! nothing toward coverage. + +// Each [[test]] target compiles its own copy of this module and not every +// target uses every item. +#![allow(dead_code)] + +use std::path::Path; +use std::process::{Command, Output}; + +pub const BIN: &str = env!("CARGO_BIN_EXE_hyper-suidhelper"); + +pub fn is_root() -> bool { + nix::unistd::geteuid().is_root() +} + +/// Run the helper with the security gate 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]) -> Output { + let mut cmd = Command::new(BIN); + cmd.args(args) + .env_clear() + .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") + .env("HYPER_SETUIDHELPER_CONFIG_PATH", config); + if let Ok(profile) = std::env::var("LLVM_PROFILE_FILE") { + cmd.env("LLVM_PROFILE_FILE", profile); + } + cmd.output().expect("spawn helper") +} From a3ceba25af0f9ab2a83433e4a4db198df97d4768 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Wed, 8 Jul 2026 00:27:01 +0000 Subject: [PATCH 2/5] test(suidhelper): pin the version self-report wire contract --- native/suidhelper/Cargo.toml | 4 +++ native/suidhelper/tests/e2e/version.rs | 37 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 native/suidhelper/tests/e2e/version.rs diff --git a/native/suidhelper/Cargo.toml b/native/suidhelper/Cargo.toml index 610c63b0..c8dcbd3f 100644 --- a/native/suidhelper/Cargo.toml +++ b/native/suidhelper/Cargo.toml @@ -88,6 +88,10 @@ path = "tests/e2e/jailer.rs" name = "e2e_chroot_jail" path = "tests/e2e/chroot_jail.rs" +[[test]] +name = "e2e_version" +path = "tests/e2e/version.rs" + [[test]] name = "config_uid_gid_range" path = "tests/config_uid_gid_range.rs" diff --git a/native/suidhelper/tests/e2e/version.rs b/native/suidhelper/tests/e2e/version.rs new file mode 100644 index 00000000..78396d8c --- /dev/null +++ b/native/suidhelper/tests/e2e/version.rs @@ -0,0 +1,37 @@ +//! L5: the `version` self-report contract. The Elixir side +//! (`Hyper.SuidHelper.Expected`) parses this JSON line and compares +//! `checksum_blake3` against the expected stamp, so any shape drift here +//! breaks helper verification at install time. +//! +//! 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")] + +use std::path::Path; + +mod support; + +#[test] +fn version_reports_pkg_version_and_hex_checksum() { + // A nonexistent config path: `version` renders before config/gate setup, + // so it must succeed regardless. Needs no root. + let out = support::run(Path::new("/nonexistent/hyper-config.toml"), &["version"]); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let json: serde_json::Value = serde_json::from_slice(&out.stdout).expect("stdout is JSON"); + assert_eq!(json["version"], env!("CARGO_PKG_VERSION")); + let sum = json["checksum_blake3"] + .as_str() + .expect("checksum is a string"); + assert_eq!(sum.len(), 64, "BLAKE3 is 32 bytes -> 64 hex chars"); + assert!( + sum.bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)), + "checksum must be lowercase hex, got: {sum}" + ); +} From afcb233cc6335a76eaed7ac90fa0de2fab9997a9 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Wed, 8 Jul 2026 00:31:11 +0000 Subject: [PATCH 3/5] ci: collect + upload Rust coverage from the privileged e2e job Root-gated e2e paths (tool dispatch, sys-test, jail build) only execute in this job; running them under plain nextest meant they never counted. Uploads under the existing rust flag; codecov merges per commit. --- .github/workflows/ci.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52643087..fe1dde9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -236,10 +236,10 @@ jobs: with: workspaces: native/suidhelper - - name: Install nextest + - name: Install cargo-llvm-cov and nextest uses: taiki-e/install-action@v2 with: - tool: nextest + tool: cargo-llvm-cov,nextest # Root-gated cases are tagged by name: every test that needs root ends in # `_as_root` and self-skips when run unprivileged. We select them by that @@ -249,10 +249,24 @@ jobs: # following the naming convention; no test-binary enumeration to maintain. # The rest of the suite (owner-axis tests assume a non-root uid) stays in # the non-root `rust` job above. - - name: Gated tests as root + # Coverage here is the only place the root-gated paths (tool dispatch, + # sys-test, jail build) execute -- the non-root rust job's e2e tests + # self-skip them. Task 1's LLVM_PROFILE_FILE passthrough makes the + # spawned helper's profile reach the collector. + - name: Gated tests as root + coverage run: | sudo -E env "PATH=$PATH" \ - cargo nextest run --features insecure_test_seams -E 'test(/_as_root$/)' + cargo llvm-cov nextest --features insecure_test_seams \ + -E 'test(/_as_root$/)' --lcov --output-path lcov-privileged.info + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v7 + with: + token: ${{ secrets.CODECOV_TOKEN }} + # working-directory only applies to `run:` steps, not actions. + files: native/suidhelper/lcov-privileged.info + flags: rust + fail_ci_if_error: true integration: name: Integration (KVM E2E) From 2f5a4219aa754b27919c3558d7801c6c16bd23f5 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Wed, 8 Jul 2026 00:43:26 +0000 Subject: [PATCH 4/5] chore(suidhelper): review polish -- squash ci.yml comments, var_os env read --- .github/workflows/ci.yml | 14 +++++++------- native/suidhelper/tests/e2e/support.rs | 2 +- native/suidhelper/tests/e2e/version.rs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe1dde9d..ff6fb6bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -245,14 +245,14 @@ jobs: # `_as_root` and self-skips when run unprivileged. We select them by that # suffix (via sudo -E so the runner's cargo/rustup home and PATH are kept) # so the root-only paths -- sys-test ok, fake-bin argv capture, mknod/chown - # jail build -- actually run. A new root test is picked up automatically by - # following the naming convention; no test-binary enumeration to maintain. - # The rest of the suite (owner-axis tests assume a non-root uid) stays in + # jail build -- actually run, and collect coverage while doing so: this + # job is the only place those paths execute, since the non-root `rust` + # job's e2e tests self-skip them (the env passthrough in + # tests/e2e/support.rs lets the spawned helper's profile reach the + # collector). A new root test is picked up automatically by following + # the naming convention; no test-binary enumeration to maintain. The + # rest of the suite (owner-axis tests assume a non-root uid) stays in # the non-root `rust` job above. - # Coverage here is the only place the root-gated paths (tool dispatch, - # sys-test, jail build) execute -- the non-root rust job's e2e tests - # self-skip them. Task 1's LLVM_PROFILE_FILE passthrough makes the - # spawned helper's profile reach the collector. - name: Gated tests as root + coverage run: | sudo -E env "PATH=$PATH" \ diff --git a/native/suidhelper/tests/e2e/support.rs b/native/suidhelper/tests/e2e/support.rs index babe7228..f393b052 100644 --- a/native/suidhelper/tests/e2e/support.rs +++ b/native/suidhelper/tests/e2e/support.rs @@ -29,7 +29,7 @@ pub fn run(config: &Path, args: &[&str]) -> Output { .env_clear() .env("HYPER_SETUIDHELPER_IS_INSECURE_MODE", "1") .env("HYPER_SETUIDHELPER_CONFIG_PATH", config); - if let Ok(profile) = std::env::var("LLVM_PROFILE_FILE") { + if let Some(profile) = std::env::var_os("LLVM_PROFILE_FILE") { cmd.env("LLVM_PROFILE_FILE", profile); } cmd.output().expect("spawn helper") diff --git a/native/suidhelper/tests/e2e/version.rs b/native/suidhelper/tests/e2e/version.rs index 78396d8c..1c906c33 100644 --- a/native/suidhelper/tests/e2e/version.rs +++ b/native/suidhelper/tests/e2e/version.rs @@ -4,8 +4,8 @@ //! breaks helper verification at install time. //! //! 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). +//! `version` == the crate version and `checksum_blake3` is 32 bytes rendered +//! as 64 lowercase hex chars (all-zero on an unstamped dev build -- shape, not value). #![cfg(feature = "insecure_test_seams")] use std::path::Path; From 9a58f5a73f84642cf35de3c7868c4c2c5bd07504 Mon Sep 17 00:00:00 2001 From: Marko Vejnovic Date: Wed, 8 Jul 2026 00:55:27 +0000 Subject: [PATCH 5/5] chore: nudge CI (dropped push event for 2f5a421)