Fix various bugs CoPilot put into LXC.

[Darren Hoehna (dhoehna)]

This PR branches from the traditional PR template because of hoe many times the reason for this PR has changed over the past 2 weeks.

tl;dr copilot added bugs, then did its best to work around those bugs. Additionally the code quality and comment quality is also horrible.

In the Deny-All-Except-Proxy PR, this is when the story starts. CoPilot, in its vanilla fashion, made the smallest, local change to get LXC to deny-al-except proxy. The smallest change was "Get the interface, but then take off all capabilities". This change turned the github pipeline red because MXC would not give the LXC container a network yet LXC needed the network to take away the capabilities.

CoPilots fix? Add a set to the github pipeline to re-add br_netfilter. This made the github pipeline green. However, CoPilot did not change the ADO pipeline. The ADO pipeline stayed red. Which is how this whole bug started. Get the ADO pipeline green.

Since two weeks ago CoPIlot and I have substantially improved LXC in so many areas.

1. Accept a 0.7.0 network schema shape inside a 0.8.0 configuration.
2. Correct branch off of the network schema shape to determin is pre0.8.0 network rules will be applied or 0.8.0 network rules.
3. LXC correctly handles the no-network-no-proxy case
4. Substantially improved some E2E tests.
5. Added a new E2E test framework.
6. LXC now REJECTS configurations where the enforcementMode is either empty or capabilitiles.
   a. This breaks nodeSDK tests because the nodeSDK disallows users setting enforcmentMode and the SDK defaults to capabilities.
7. Removed br_netfilter and all code that assumes br_netfilter will be installed.

This is tested with using the full E2E suite that LXC offers.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Enables bridge netfilter in the Azure Pipelines Linux SDK integration lane so LXC schema 0.8 firewall enforcement can initialize correctly.

Changes:

• Loads br_netfilter.
• Enables IPv4 and IPv6 bridge-to-iptables processing.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/backends/lxc/common/src/network_iptables.rs:4716

• This statement was accidentally joined to the function declaration, so the committed file is not rustfmt-formatted and the repository's cargo fmt --all -- --check gate will fail.

    fn a_legacy_policy_that_installs_nothing_can_still_need_the_network() {

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/backends/lxc/common/src/lxc_runner.rs:234

• This is fail-open when a container is reused while already running. set_config_item only appends startup configuration, but the runner explicitly accepts an existing running container below; in that case the current veth remains active while permits_no_network also makes both firewall managers skip their chains, so the workload retains the previous run's network access. Handle this transition before skipping enforcement (for example, reject reuse or stop/reconfigure/restart it), and cover a destroyOnExit: false networked-to-default-deny reuse case.

        if permits_no_network(&request.policy, uses_directional_schema) {
            // `empty` gives the container its own network namespace holding only
            // a loopback device; `up` activates it, keeping 127.0.0.1 available
            // to a workload that binds it.
            for (key, value) in [("lxc.net.0.type", "empty"), ("lxc.net.0.flags", "up")] {

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/backends/lxc/common/src/lxc_runner.rs:235

• This is unsafe for destroyOnExit=false reuse. set_config_item only appends to the persistent startup config; if the named container is already running, the later is_running() branch skips restart, so its existing veth remains active while installs_firewall returns false. The workload then runs with unfiltered network access. Reconcile the active interface before execution (stop/recreate or reject incompatible reuse), and ensure the persistent empty entry is removed before a later network-enabled reuse.

            for (key, value) in [("lxc.net.0.type", "empty"), ("lxc.net.0.flags", "up")] {
                if let Err(e) = container.set_config_item(key, value) {

[Copilot]

🟡 Changes recommended

Unprivileged LXC execution is broken, raw sockets bypass egress enforcement, and legacy unmarked mounts can retain stale filesystem access.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/backends/lxc/common/src/lxc_bindings.rs:305

• This cleanup only recognizes the new marker block. Containers reused from the previous implementation contain MXC-created lxc.mount.entry lines without these markers because set_config_item appended them directly; those entries are now treated as operator-owned and survive every rewrite. After upgrading, a later request that removes a read/write path therefore retains the old host mount and stale filesystem access. Add an explicit migration strategy (for example, record owned entries separately or recreate legacy containers) before preserving all unmarked entries.

            if trimmed == MANAGED_MOUNTS_BEGIN {
                inside = true;
                continue;
            }
            if trimmed == MANAGED_MOUNTS_END {

src/backends/lxc/common/src/lxc_bindings.rs:117

• Moving egress enforcement into the container namespace makes CAP_NET_RAW part of the boundary, but this code drops only CAP_NET_ADMIN. The attached workload still runs as container root with CAP_NET_RAW, so it can send AF_PACKET frames below the namespace's OUTPUT hook and bypass deny/allow and proxy-only rules (the updated backend documentation itself notes this bypass). Drop or otherwise mediate raw packet sockets before treating these chains as enforcement.

    // `libc` does not export this; the value is from linux/capability.h.
    const CAP_NET_ADMIN: libc::c_ulong = 12;

• Files reviewed: 52/53 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The current implementation leaves a raw-socket firewall bypass, breaks legacy SDK-generated LXC policies, and can retain pre-upgrade filesystem grants.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/backends/lxc/common/src/lxc_bindings.rs:106

• Dropping only CAP_NET_ADMIN does not protect an OUTPUT-based egress boundary. Container root still retains CAP_NET_RAW, so it can create an AF_PACKET socket and transmit below the namespace's OUTPUT hook, bypassing these rules. Remove CAP_NET_RAW as well or block packet sockets before treating the in-namespace chain as containment.

    const CAP_NET_ADMIN: libc::c_ulong = 12;

    // SAFETY: `pre_exec` runs between fork and exec, where only
    // async-signal-safe work is permitted. `prctl` is a bare syscall and this
    // closure allocates nothing and captures nothing.
    unsafe {
        command.pre_exec(|| {
            if libc::prctl(libc::PR_CAPBSET_DROP, CAP_NET_ADMIN, 0, 0, 0) != 0 {

src/backends/lxc/common/src/filesystem_mounts.rs:186

• This replacement does not revoke mount entries written by pre-upgrade MXC builds. Those entries were appended as ordinary, unmarked lxc.mount.entry lines, while set_filesystem_access_points strips only the new managed block; reusing such a container therefore retains stale read/write access alongside the new policy. Add an explicit migration strategy or recreate legacy containers before reuse.

    container.set_filesystem_access_points(&entries)?;

sdk/node/tests/integration/linux-process-container.test.ts:52

• The legacy branch still produces a network section without enforcementMode. supportedVersions includes 0.6 and 0.7, the parser defaults those requests to capabilities, and the new LXC validation rejects that mode, so all three network tests using this helper fail for both legacy versions. Align SDK-generated LXC configs with the backend before keeping these versions in the loop.

function outboundNetwork(version: (typeof supportedVersions)[number]) {
  return version.compare('0.8.0-alpha') >= 0
    ? { egress: { default: 'allow' as const } }
    : { allowOutbound: true };

• Files reviewed: 52/53 changed files
• Comments generated: 8
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

CAP_NET_RAW still permits firewall bypass, and pre-0.8 Node SDK LXC configurations are rejected before execution.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

sdk/node/tests/integration/linux-process-container.test.ts:52

• The new validation makes the pre-0.8 branches in this suite fail before execution. createConfigFromPolicy emits a legacy network.defaultPolicy for 0.6/0.7 (even when the caller supplied no network policy), while applyLinuxNetworkPolicy adds enforcementMode: 'firewall' only for host lists. LXC now rejects that omitted mode as capabilities, so changing only the 0.8 outbound shape leaves every 0.6/0.7 case in this loop—including the non-network tests—broken. The LXC SDK builder needs to emit an accepted mode for synthesized legacy policies, or the backend must distinguish this synthesized default from an explicit capabilities request.

function outboundNetwork(version: (typeof supportedVersions)[number]) {
  return version.compare('0.8.0-alpha') >= 0
    ? { egress: { default: 'allow' as const } }
    : { allowOutbound: true };

• Files reviewed: 55/56 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Filtered LXC workloads retain a CAP_NET_RAW bypass, and legacy reused containers can retain prior filesystem grants.

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

sdk/node/tests/integration/linux-process-container.test.ts:209

• This test asserts the current implementation mechanism rather than the network contract. A compliant backend may keep an eth0 interface and enforce deny-all with a reachable firewall, so ifaces=[lo ] would fail despite correct isolation; conversely, interface enumeration alone does not prove traffic is blocked. Probe reachability with a working allow-case control, as the new shell tests do, and assert loopback separately.
  tests/scripts/run_lxc_network_enforcement_test.sh:219
• This failure message cites the 0.8 contract, but lxc_network_enforcement_deny.json declares schema 0.7.0-alpha. That will misdiagnose a loopback regression in this test; describe the LXC loopback invariant without attributing it to 0.8.

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:59

• The new confinement check covers only CAP_NET_ADMIN, but the firewall now hangs from the container namespace's OUTPUT chain. An attached root workload that retains CAP_NET_RAW can inject AF_PACKET frames below OUTPUT and bypass destination/port filtering entirely. Please drop CAP_NET_RAW from the bounding set as well and assert all three masks here; using a filtered-network fixture would also exercise the security boundary this test is intended to protect.

    for field in ["CapEff:", "CapPrm:", "CapBnd:"] {
        assert_eq!(
            capability_mask(&status, field) & CAP_NET_ADMIN,
            0,
            "{field} still carries CAP_NET_ADMIN; the workload can rewrite the firewall confining it\n{status}"

src/backends/lxc/common/src/filesystem_mounts.rs:186

• This replacement mechanism cannot clean mounts written by the previous implementation. Earlier releases appended bare lxc.mount.entry lines, while set_filesystem_access_points removes only entries inside the new MXC marker block, so upgrading and reusing an existing container preserves prior read-write grants indefinitely. Add a migration strategy (or recreate legacy containers) before relying on this call for policy tightening.

    container.set_filesystem_access_points(&entries)?;

src/testing/wxc_e2e_tests/src/lib.rs:208

• Command::output() returning Ok only means the process spawned; it may still exit unsuccessfully. In that case this helper reports the host ready and the E2E test fails later instead of skipping an unavailable LXC installation. Require output.status.success() and report nonzero status as unavailable.

    match Command::new("lxc-start").arg("--version").output() {
        Ok(_) => true,
        Err(_) => {
            println!(
                "SKIPPED: lxc-start not installed — this host cannot start a system container"
            );
            false
        }

• Files reviewed: 55/56 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Retained CAP_NET_RAW permits an egress-filter bypass, while legacy SDK behavior, mount migration, and CI coverage remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:27

• This test is skipped in the normal Linux Rust-test lane because that lane does not install LXC, while the dedicated .github/workflows/lxc-e2e.yml lane installs LXC but runs only run_lxc_all_tests.sh, not this Rust test. Consequently the new capability invariant is never exercised on an LXC-capable CI host; add this test binary to that workflow or cover the same masks in the shell suite.
  tests/scripts/run_lxc_all_tests.sh:126
• Registering these cases makes the strict LXC CI gate depend on TCP reachability to hard-coded GitHub address 140.82.114.6. The same suite explicitly quarantines its older public-network test as unreliable and recommends a local endpoint (run_lxc_all_tests.sh:92-107); an address rotation or runner egress outage will now fail several unrelated cases. Use the local network-namespace peer pattern from run_lxc_network_enforcement_test.sh for the new controls.

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:59

• Checking only CAP_NET_ADMIN leaves the new OUTPUT-based firewall bypassable. LXC workloads run as root, and confine_network_capabilities currently drops only capability 12; retained CAP_NET_RAW lets the workload send AF_PACKET frames below the namespace's OUTPUT hook. Drop CAP_NET_RAW as well (from the bounding/effective/permitted sets) and assert both bits here, or use an enforcement point that packet sockets cannot bypass.

    for field in ["CapEff:", "CapPrm:", "CapBnd:"] {
        assert_eq!(
            capability_mask(&status, field) & CAP_NET_ADMIN,
            0,
            "{field} still carries CAP_NET_ADMIN; the workload can rewrite the firewall confining it\n{status}"

sdk/node/tests/integration/linux-process-container.test.ts:52

• For schema 0.6/0.7 this still emits only allowOutbound, which the SDK serializes as legacy defaultPolicy: allow without enforcementMode. The new LXC validation rejects that request because the omitted mode defaults to capabilities, so the 0.6 and 0.7 iterations of all three updated network tests fail whenever the network tests are enabled. Make the LXC SDK mapping emit firewall for legacy network policies (or expose an equivalent supported API) instead of only changing the 0.8 shape.

function outboundNetwork(version: (typeof supportedVersions)[number]) {
  return version.compare('0.8.0-alpha') >= 0
    ? { egress: { default: 'allow' as const } }
    : { allowOutbound: true };

• Files reviewed: 55/56 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Namespace enforcement has confirmed bypass and startup-isolation gaps, and its capability regression test is not exercised by LXC CI.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/backends/lxc/common/src/lxc_runner.rs:390

• The container is started with its configured interface before the OUTPUT policy is installed, and the code then waits for network readiness. On a reused container, the previous root workload can persist an init service that sends traffic during this unrestricted boot window, so a tightened policy can leak before the attached command runs. Install enforcement before networking/startup becomes active, or recreate the container for policy changes.

        let _ = writeln!(logger, "Starting LXC container...");
        if let Err(e) = container.start(network) {
            if self.destroy_on_exit || container_created {
                let _ = container.destroy();
            }
            return ScriptResponse::error(&format!("Failed to start container: {}", e));
        }
        let _ = writeln!(logger, "Container started successfully.");

        let needs_network = needs_network(&request.policy);

        if needs_network {
            // Alpine DHCP leases can arrive at about nine seconds; thirty
            // seconds leaves margin.
            let timeout = Duration::from_secs(30);
            if let Some(response) = self.enforce_network_readiness(

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:71

• Checking only CAP_NET_ADMIN is insufficient for an OUTPUT-based firewall. The attached root workload still retains CAP_NET_RAW, so it can send Ethernet frames through an AF_PACKET socket and bypass the namespace's OUTPUT chain. Drop CAP_NET_RAW as well (or otherwise prohibit packet sockets) and extend this regression test accordingly.

    // The kernel writes these masks; the workload cannot forge them.  Effective
    // alone would not settle it: a process raises a permitted capability into
    // its effective set whenever it likes, and only a bounding-set drop
    // survives execve.
    for field in ["CapEff:", "CapPrm:", "CapBnd:"] {
        assert_eq!(
            capability_mask(&status, field) & CAP_NET_ADMIN,
            0,
            "{field} still carries CAP_NET_ADMIN; the workload can rewrite the firewall confining it\n{status}"

src/backends/lxc/common/src/lxc_bindings.rs:178

• NoInterface overrides only lxc.net.0; any lxc.net.1 (or later) entry in an existing container remains active. Because this runner reuses already-defined containers, a deny-all request can therefore retain a second network interface and bypass the intended isolation. Clear or override every configured network entry, or reject reused multi-interface containers before starting them.

            StartNetwork::NoInterface => {
                &["-s", "lxc.net.0.type=empty", "-s", "lxc.net.0.flags=up"]
            }

• Files reviewed: 57/58 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Retained CAP_NET_RAW permits bypassing the new namespace OUTPUT firewall, and the capability E2E test is not executed on an LXC-capable CI host.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:30

• This test currently never exercises LXC in CI. The Linux wxc_e2e_tests job runs on a host where LXC is intentionally absent, so ready() returns false, while the LXC-capable workflow only invokes tests/scripts/run_lxc_all_tests.sh and never runs this Rust test binary. Wire this test into the LXC E2E workflow (or move the assertion into that shell suite) so regressions cannot pass through this early return.

#[test]
fn workload_cannot_reconfigure_the_network() {
    if !ready() {
        return;
    }

src/backends/lxc/common/src/lxc_bindings.rs:158

• Dropping only CAP_NET_ADMIN does not protect a firewall hooked in the container's OUTPUT chain. The attached workload still runs as root with CAP_NET_RAW, so it can send Ethernet frames through an AF_PACKET socket; packet sockets bypass the IP OUTPUT hook and can escape the allow/deny rules. Drop CAP_NET_RAW as well and extend the capability E2E test to assert that bit is absent.

    // `libc` does not export this; the value is from linux/capability.h.
    const CAP_NET_ADMIN: libc::c_ulong = 12;

    // SAFETY: `pre_exec` runs between fork and exec, where only
    // async-signal-safe work is permitted. `prctl` is a bare syscall and this
    // closure allocates nothing and captures nothing.
    unsafe {
        command.pre_exec(|| {
            if libc::prctl(libc::PR_CAPBSET_DROP, CAP_NET_ADMIN, 0, 0, 0) != 0 {

• Files reviewed: 57/58 changed files
• Comments generated: 1
• Review effort level: Balanced

[Jeff Whiteside (jsidewhite)]

(364, 340, 14);

Gudge (@MGudgin) what is this?

[Copilot]

🟡 Changes recommended

LXC policies remain bypassable through retained capabilities, additional configured interfaces, and legacy stale mounts.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/testing/wxc_e2e_tests/tests/e2e_lxc_network_capability.rs:9

• Checking only CAP_NET_ADMIN is insufficient for the new OUTPUT-chain design. The attached workload runs as container root, and retaining CAP_NET_RAW lets it emit AF_PACKET frames that bypass the IP OUTPUT hook entirely. The production confinement and this E2E assertion must also drop/check CAP_NET_RAW, or filtered LXC policies remain bypassable.

const CAP_NET_ADMIN: u64 = 1 << 12;

src/backends/lxc/common/src/filesystem_mounts.rs:185

• The replacement only removes entries inside the new managed markers. Containers reused from an older MXC build have MXC-created lxc.mount.entry lines without those markers, so a first post-upgrade run that removes a prior rw/ro grant still inherits that stale mount. Refuse/recreate legacy containers or provide a migration that distinguishes and removes the old MXC entries before applying the new policy.

    container.set_filesystem_access_points(&entries)?;

• Files reviewed: 56/57 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

CAP_NET_RAW still permits an AF_PACKET bypass of the new OUTPUT-chain enforcement, and one SDK test does not exercise its claimed wire shape.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

sdk/node/tests/integration/linux-process-container.test.ts:202

• This does not actually send an omitted-network request. runLxc calls createConfigFromPolicy, whose no-network branch emits network: { defaultPolicy: 'block' } (sdk/node/src/sandbox.ts:500-503), so the assertion exercises the legacy 0.8 shape instead of the omitted-section path described by the test. Build the config here, delete config.network, and then invoke spawnFromConfigAsync so this regression test covers the intended wire shape.

• Files reviewed: 58/59 changed files
• Comments generated: 1
• Review effort level: Balanced

[Jeff Whiteside (jsidewhite)]