test(suidhelper): security-gated test seam + parametrized E2E suite#26
Merged
Conversation
Route the config file path through security_gate::split so subprocess tests can redirect it via HYPER_SETUIDHELPER_CONFIG_PATH — only when both gates are open (insecure_test_seams feature + env var). Production builds always read /etc/hyper/config.toml with full SafeFile checks. Widened LoadingError path variants from &'static Path to PathBuf; dropped the Copy derive and removed the now-unused CONFIG_PATH static.
Remove the inner security_gate::split around the SafeFile read in Config::safe_load. The seam was incorrectly bypassing SafePath lexical and SafeFile ownership/type/writability checks in insecure test mode. Now only config_path() is gated — the path changes, but validation is identical in both modes. Update config_seam test to prove the override path is consulted via the real lexical gate: a .. component in the override is rejected as LooseComponents, which cannot come from the clean hardcoded default, confirming the seam without bypassing any security check.
Add integration tests for the three security-critical operand parsers that were previously untested. SafeBin tests cover all five refusal axes (relative path, wrong basename, symlink, non-root owner, group/other-writable); root- only axes self-skip when running unprivileged. DmTable tests assert the full accept+round-trip contract (canonical forms, whitespace normalisation) and the reject matrix (non-whitelisted targets, arbitrary devices, bad flags, wrong arity). ThinMessage tests cover the two whitelisted verbs with normalisation and the reject set (unknown verbs, missing/extra args, non-numeric id). Also exposes DmTable and ThinMessage via pub use from the tools module so integration tests can import them without reaching into internal submodules. Adds tempfile = "3" dev-dependency for root-independent filesystem fixtures.
Add tests/e2e/config.rs (feature-gated on insecure_test_seams) that drives the real binary via CARGO_BIN_EXE to assert exit codes and stderr for missing, non-root-owned, malformed, relative, and valid configs. Two cases always run as non-root; three skip with a loud eprintln! and pass when not root. Register e2e_config [[test]] target.
Add tests/e2e/argv.rs (feature-gated insecure_test_seams) that points the helper's --bin at a root-owned shell fake recording argv as JSON. Asserts exact command lines for dmsetup create/remove/message and blockdev --getsz. All four tests early-skip with a loud eprintln when not root; real assertions run in the privileged CI job.
Add `suidhelper-privileged` job that runs only `--test e2e_config --test e2e_argv` under `sudo -E` so the root-only cases (sys-test ok, fake-bin argv capture) actually execute in CI. The non-root `rust` job continues to run the full suite with `--all-features`; owner-axis tests that assume a non-root uid stay there. Append a Testing section to the suidhelper README documenting the three invocation tiers and the dual-gate insecure test seam.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The INSECURE flag is written once at startup before any concurrency and publishes no other memory, so SeqCst overstates the contract; Relaxed is sufficient.
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.
No description provided.