test: strengthen Rust policy_fn tests + fix c_smoke cdylib build (follow-ups to #103)#104
Merged
Merged
Conversation
…rcement coverage The Rust policy_fn integration tests had the same false-passing patterns the Python suite did, and lacked coverage for the bugs just fixed: - test_policy_fn_deny_connect connected to a dead port and accepted any error as "blocked". Now targets a live allowlisted listener and asserts EPERM (errno 1) from the policy_fn deny specifically. - test_policy_fn_restrict_network_takes_effect used restrict_network(&[]) (a no-op) and a dead port. Now uses two live loopback listeners and a non-empty restriction, asserting the listed IP connects and the non-listed one is refused (errno 111). - test_policy_fn_restrict_pid_network_without_allowlist connected to a dead port. Now uses a live listener so the refusal can only come from the per-pid restriction. New tests covering behavior no Rust test exercised (so the no-op / deadlock bugs would have gone uncaught): - test_policy_fn_restrict_max_memory_enforced — 256 MiB ceiling, restrict to 64 MiB, assert a 128 MiB alloc is killed while the control succeeds. - test_policy_fn_restrict_max_processes_enforced — restrict to 1, assert the fork is denied with EAGAIN while the control forks. - test_policy_fn_fork_does_not_deadlock — 30 forks under an active policy_fn complete within a bounded timeout (fork-tracking regression). The enforcement tests use run() (captured stdout) rather than run_interactive() (inherited stdio). Signed-off-by: Cong Wang <cwang@multikernel.io>
The C smoke test linked whatever libsandlock_ffi.so already existed in target/, but Cargo does not treat the cdylib as a build prerequisite of an integration test (tests link the rlib). So `cargo test` never (re)builds the .so, and CI linked a stale artifact missing newer symbols (undefined reference to sandlock_action_set_inject_bytes). Build the cdylib from within the test via `cargo build -p sandlock-ffi --lib` before locating and linking it, so the C test always links the current artifact. The recursive cargo invocation is safe — the outer build lock is released before tests run. Verified by deleting the .so and re-running: the test rebuilds it and passes. Signed-off-by: Cong Wang <cwang@multikernel.io>
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
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.
Two post-merge follow-ups to #103 (which strengthened the Python
policy_fntests and fixed the dynamic-policy / fork-tracking bugs).1. Strengthen the Rust
policy_fnintegration tests + add missing coverageThe Rust integration tests had the same false-passing patterns the Python suite did, and lacked coverage for the bugs #103 fixed (so they would not have caught them):
Strengthened (were false-passing):
test_policy_fn_deny_connectconnected to a dead port and accepted any error as "blocked". Now targets a live allowlisted listener and asserts EPERM (errno 1) from thepolicy_fndeny.test_policy_fn_restrict_network_takes_effectusedrestrict_network(&[])(a no-op) and a dead port. Now uses two live loopback listeners + a non-empty restriction; asserts the listed IP connects and the non-listed one is refused (errno 111).test_policy_fn_restrict_pid_network_without_allowlistconnected to a dead port. Now uses a live listener so the refusal can only come from the per-pid restriction.Added (no Rust test exercised these):
test_policy_fn_restrict_max_memory_enforced— 256 MiB ceiling, restrict to 64 MiB, assert a 128 MiB alloc is killed while the control succeeds.test_policy_fn_restrict_max_processes_enforced— restrict to 1, assert the fork is denied with EAGAIN while the control forks.test_policy_fn_fork_does_not_deadlock— 30 forks under an activepolicy_fncomplete within a bounded timeout (fork-tracking regression).The enforcement tests use
run()(captured stdout) rather thanrun_interactive()(inherited stdio).2. Build the cdylib in the
c_smoketest (CI fix)tests/c_smoke.rslinked whateverlibsandlock_ffi.soalready existed intarget/, but Cargo does not treat the cdylib as a build prerequisite of an integration test (tests link the rlib). Socargo testnever (re)builds the.so, and CI linked a stale artifact missing newer symbols —undefined reference to sandlock_action_set_inject_bytes.The test now runs
cargo build -p sandlock-ffi --libto refresh the cdylib before locating and linking it, so it always links the current artifact. The recursivecargois safe (the outer build lock is released before tests run).Test plan
cargo test -p sandlock-core— 398 lib + 260 integration pass (incl. the 16policy_fnintegration tests: 3 strengthened + 3 new).cargo test -p sandlock-ffi --test c_smoke— passes; verified by deleting the.sofirst (the test rebuilds it and passes instead of failing on a missing/stale artifact).