Filed from review feedback on #798 (thread on src/backends/lxc/common/src/network_iptables.rs).
What the reviewer found
High (test coverage) - the headline bridged fail-closed branch is deliberately not asserted.
This test forces Unknown, treats it as bridged, and then discards the result because it depends
on the host's real br_netfilter state. No test sets a controlled bridged topology with inactive
bridge netfilter and proves apply_firewall_rules returns the documented error.
Current state after #798
The behavior itself is covered end to end. tests/scripts/run_lxc_network_bridge_failclosed_test.sh
sets up a controlled bridged topology, turns bridge-nf-call-iptables off, asserts the run is
refused, turns it back on, and asserts the same config then succeeds. It passes against a real
kernel as part of run_lxc_all_tests.sh.
What is still missing
That suite needs a Linux host with LXC, so it does not run under cargo test. In CI the bridged
fail-closed branch has no coverage at all. The in-file test at network_iptables.rs:3728 pins the
Unknown -> treat-as-bridged join and its log line, but deliberately discards the Result, because
whether the branch errors depends on the host's real br_netfilter state.
The fix the reviewer asked for
Inject the bridge-netfilter probe -- bridge_netfilter_active(bridge_nf_path) already takes a path,
so the seam mostly exists -- and add a deterministic unit test that sets a bridged topology with an
inactive probe and asserts apply_firewall_rules returns the documented error rather than silently
installing a chain no packet can reach.
Why it was not done in #798
#798 is already large (31 files) and the branch is covered by a real-kernel test. Making the probe
injectable touches the install path's signature, which is security-critical code the same PR was
refactoring. Splitting it keeps that refactor reviewable.
Filed from review feedback on #798 (thread on
src/backends/lxc/common/src/network_iptables.rs).What the reviewer found
Current state after #798
The behavior itself is covered end to end.
tests/scripts/run_lxc_network_bridge_failclosed_test.shsets up a controlled bridged topology, turns
bridge-nf-call-iptablesoff, asserts the run isrefused, turns it back on, and asserts the same config then succeeds. It passes against a real
kernel as part of
run_lxc_all_tests.sh.What is still missing
That suite needs a Linux host with LXC, so it does not run under
cargo test. In CI the bridgedfail-closed branch has no coverage at all. The in-file test at
network_iptables.rs:3728pins theUnknown-> treat-as-bridged join and its log line, but deliberately discards theResult, becausewhether the branch errors depends on the host's real
br_netfilterstate.The fix the reviewer asked for
Inject the bridge-netfilter probe --
bridge_netfilter_active(bridge_nf_path)already takes a path,so the seam mostly exists -- and add a deterministic unit test that sets a bridged topology with an
inactive probe and asserts
apply_firewall_rulesreturns the documented error rather than silentlyinstalling a chain no packet can reach.
Why it was not done in #798
#798 is already large (31 files) and the branch is covered by a real-kernel test. Making the probe
injectable touches the install path's signature, which is security-critical code the same PR was
refactoring. Splitting it keeps that refactor reviewable.