test(suidhelper): losetup/thin_dump coverage + E2E Rust coverage collection - #65
Merged
Conversation
Some subcommands' clap value_parsers (losetup attach's ok_backing_file) consult Config::get() while clap parses argv, and Config consults the security gate. With init() running after Cli::parse(), that forces the process-wide Config LazyLock while the gate is still closed, so it loads and memoizes the production /etc/hyper/config.toml for the rest of the process — bypassing the insecure-test-seam config redirect for any subcommand whose parser touches config. Move the gate resolution ahead of Cli::parse() so the first config load, wherever it happens, sees the correct gate state. No behavior change in release builds (the gate is always secure there); only the test seam ordering was broken.
commit 6da778c hardcoded /srv/hyper as work_dir after misdiagnosing a security-gate ordering bug as a config-loading bug in the helper. With main.rs now resolving the security gate before clap parses argv, the insecure-test-seam config redirect works for every subcommand, including losetup attach, so the tests can go back to a tempdir-canonicalized work_dir as originally specified (task-3-brief.md Step 1) instead of depending on /srv/hyper existing on the host.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…le loss The jailer subcommand execve's on success and _exit(2)s on failure, so the profile runtime's atexit dump never runs and those invocations counted nothing. -C llvm-args=-runtime-counter-relocation (same xtask gate as instrument-coverage) plus %c in LLVM_PROFILE_FILE switches to continuous mode: counters are mmap-written as they increment, so the profile is already on disk when the image is replaced. Verified locally: jailer _exit run writes a full profraw (was 0-byte), 103 jailer.rs lines covered from one run; version run stderr-clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools/losetup.rsandtools/thin_dump.rsshowed 0%/near-0% Rust coverage despite being exercised by the live E2E — the integration job ran an uninstrumented helper and uploaded no Rust lcov, and no Rust test touched these files.ok_backing_filerefusal/accept contracts (confinement property, symlink-escape, inheritable-fd inode round-trip), losetup attach/detach argv+fd contracts, thin-dump argv/parse/error contracts. Local root slice:losetup.rs59/62 lines,thin_dump.rs20/20 (was 0%).main.rsranCli::parse()beforesecurity_gate::init(), so config-consulting value_parsers (losetup attach's backing-file gate) memoized the production/etc/hyper/config.tomlbefore the test seam could redirect it. Gate init now precedes parse (release builds unaffected — the gate is compile-time secure there).cargo xtask stampgains an env-gated (HYPER_SUIDHELPER_INSTRUMENT_COVERAGE=1)-C instrument-coveragebuild (helper only — never the guest agent); the integration job sets it job-wide, routes profiles viaLLVM_PROFILE_FILE, and a post-test step merges/exports lcov (pinned-nightly llvm-tools) and uploads underflags: rust.Test plan
cargo nextest run --all-features164/164; root slice 32/32; fmt/clippy cleanmix test367 passed (aftermix suidhelper.install— the main.rs fix changes the helper checksum)Export suidhelper E2E coveragestep finds profraws and uploads (this PR's CI run)