[LXC] State-aware lifecycle management

[Darren Hoehna (dhoehna)]

Linked work item: AB#62953349 — [LXC] State-aware lifecycle management

Summary

Implements the StatefulSandboxBackend trait for the LXC backend, wiring provision / start / exec / stop / deprovision to the LXC CLI wrappers. LXC is registered as a non-experimental state-aware backend (BACKEND_KEY = "lxc") in the Rust dispatcher and parser and in the SDK prefix map, and the LXC state-aware config types are exported from the SDK. This path is net-new: before this change the state-aware dispatcher recognized only iso and wsb, so any lxc: state-aware request returned unsupported_containment. Nothing that worked before can regress.

• Run-to-completion exec. exec runs the script through the one-shot lxc-attach PTY path; the returned ExecHandle carries null stdio pipes and a waiter that yields the container's exit code. Live streaming exec — the SDK execInSandbox / IPty path — is not available for LXC; callers use the buffered execInSandboxAsync instead. Closing that gap is tracked in [LXC] SandboxProcess streaming exec is not implemented for the LXC backend #765.
• Firewall is installed before the container starts. Under a firewall-enforced policy the chain and its FORWARD hook are built before container.start(), referencing a deterministic host-side veth name pinned into the container config (lxc.net.0.veth.pair). A firewall-install failure aborts the start rather than running the container unfiltered, and a failed start tears the rules back down.
• Policy is only accepted on start, and is fail-closed. provision, exec, stop, and deprovision reject any filesystem or network policy. On start, network.proxy is rejected, and a restrictive network policy (allowedHosts, blockedHosts, or an explicit defaultPolicy: "block") under a non-firewall enforcementMode is rejected rather than run silently unenforced. A policy-free start and an explicit defaultPolicy: "allow" are accepted.
• A dry-run start reaches the same verdict as a real one. Both start-only rejections above need nothing but the request, so they live in validate_start — the last thing a dry run executes before it returns. Rejections that read live container state (not_provisioned, already_started, and the multi-interface refusal) stay in start, since a dry run has no container to read.
• Explicit default-deny is honored or refused, never silently dropped. The parser records whether network.defaultPolicy was present on the wire, so an explicit "block" is distinguishable from the struct default a policy-free start produces. An already-running container that carries start policy reports already_started.
• A firewall-enforced start refuses a config it cannot fully enforce. Zero or multiple configured interfaces are both refused: the veth name is pinned into lxc.net.0.veth.pair, so more than one interface leaves unhooked egress paths and zero leaves nothing to hook. A config using lxc.include is refused for the same reason — counting lxc.net.N keys in the container's own config is a lower bound, since an include can declare interfaces that file never names, and reproducing liblxc's relative-path and glob resolution is exactly the place a subtle error would produce the false confidence the gate exists to prevent.
• Nothing unfilters a container that is still running. stop and the signal rollback both halt the container before removing its firewall, and both abandon the teardown if the stop fails rather than strip the chain off a live container. Teardown never flushes a chain FORWARD still jumps to: -F succeeds regardless of who references the chain, and an emptied user chain returns to its caller instead of reaching its own closing DROP, so flushing a hooked chain is a fail-open. Leaving it costs a leaked chain that still filters correctly.
• Chain ownership is published to the signal watchdog the instant iptables -N succeeds, and released only when -X succeeds. Publishing at the moment the chain begins to exist covers the whole window and no more: a process whose -N lost the race never publishes, so it never removes. Teardown runs through the manager that owns the chain, so a released chain is not torn down a second time on drop — a second teardown could otherwise delete a chain another start had since created under the same deterministic name.
• Chain and veth names fold in a hash of the full container name. The per-container iptables chain is MXC- + up to 12 sanitized characters + - + an 11-character base36 token (a 64-bit FNV-1a hash reduced modulo 36^11 ≈ 2^56.9) = 28 characters, the netfilter limit; the host-side veth is mxcv + the same token = 15 characters, the IFNAMSIZ limit. This breaks the systematic collapse of shared prefixes, but the derivation is non-cryptographic and not injective: distinct names can still map to one chain, so collisions remain possible. Durable per-container chain ownership is the real fix and is not implemented here.
• A dispatch failure is recognised on stdout, or on stderr for LXC only. Only the last non-empty line is read, so a script that prints something envelope-shaped keeps its ordinary non-zero exit instead of having it thrown as an MxcError. The stderr channel is read only for LXC, whose pty merges guest stderr into stdout and leaves the executor as its sole writer; Windows Sandbox and isolation-session both relay guest output to that stream, so on those backends stdout is the only trustworthy channel.
• The output drain is bounded by the caller's remaining budget plus a grace period, not a flat two seconds, so a fast child with a lot of buffered output is not truncated.
• Baseline mounts preserved. Filesystem-policy application clears only the lxc.mount.entry lines MXC itself added (tagged with a marker comment), leaving distro- or user-supplied mounts intact.
• Shared telemetry. wxc and the lxc executor share one run_state_aware_with_telemetry entry point in mxc_engine, so LXC emits the same phase and correlation-vector telemetry as every other backend.
• SDK types match backend behavior. LxcNetworkConfig exposes only the fields LXC honors (defaultPolicy, allowedHosts, blockedHosts, and enforcementMode restricted to "firewall" / "both"), so a config the backend would ignore or reject is a compile-time error. The SDK README records LXC as a Linux-only state-aware backend with no streaming exec.

Known gaps

Both need a Linux host with real signal delivery to reproduce, and neither is a fail-open.

• iptables -N can succeed a moment before ownership is published, stranding a chain nobody records as theirs. Publishing first would invert this into something worse: the loser of a creation race would claim a chain it did not create and its cleanup would delete the winner's, leaving the winner's container unfiltered. A stranded chain is hooked into nothing and filters nothing, so it costs a chain name until the next teardown reclaims it. Closing it properly needs the create and the publish to be atomic against host state shared across processes, which iptables does not offer.
• CreatedFirewallState is Copy, and signal_cleanup snapshots it and releases the mutex before cleanup, so a rollback acts on the ownership recorded at snapshot time.

Validation

Re-verified on this branch (5449775) on the Windows dev box:

• cargo test -p lxc_common --lib — 115 passed, 0 failed (includes the state_aware:: suite and the chain-name spec suite).
• cargo test -p wxc_common --lib — 564 passed, 0 failed. This is a Windows run, so it includes the Windows-gated tests a Linux run excludes.
• cargo test -p mxc_pty --lib — 7 passed, 0 failed.
• SDK: 230 Node tests pass.
• cargo fmt --all -- --check — clean.
• cargo clippy -p lxc_common -p wxc_common --all-targets -- -D warnings — clean.

The chain-name spec suite pins a previously colliding pair (web-frontend-017m3b and web-frontend-01kgar, which both produced MXC-web-frontend-01-3d4a49a5) as distinct, sweeps 300,000 shared-prefix names with no collision, and holds the length bounds (chain at most 28 characters, veth exactly 15) over an adversarial corpus. These establish collision resistance, not injectivity.

The rollback and teardown orderings are asserted through pure seams (rollback_plan, execute_rollback, teardown_chain) because run_watchdog is Linux-only and every iptables command fails at spawn on a non-Linux host — a test driving the real path could not observe an ordering at all. What those seams cannot cover is the end-to-end behavior: real iptables enforcement, the pre-start firewall/veth ordering, real signal delivery, a Unix PTY, and tests/scripts/run_lxc_state_aware_test.sh all need a root LXC host and were not executed in this pass.

One defect this PR does not fix: an unresolvable blockedHosts entry is logged and skipped, which under defaultPolicy: allow leaves the host reachable. That code is untouched by this branch (git diff origin/main...HEAD covers no hunk there) and #632 owns the fix, so repairing it here would only manufacture a conflict.

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Adds state-aware lifecycle support for the Linux LXC backend by implementing StatefulSandboxBackend in lxc_common, wiring dispatch from lxc-exec, and exposing the new backend key/prefix in both Rust parsing/dispatch and the TypeScript SDK state-aware types/helpers.

Changes:

• Register lxc as a state-aware backend prefix/key in the Rust dispatcher/config parser (with new unit tests).
• Add LxcStateAwareRunner implementing provision/start/exec/stop/deprovision for LXC containers (best-effort iptables cleanup on teardown).
• Extend the SDK state-aware type system + prefix map to support lxc, including per-phase config/metadata typings.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/core/wxc_common/src/state_aware_dispatch.rs	Adds lxc sandboxId prefix routing + a unit test.
src/core/wxc_common/src/config_parser.rs	Accepts experimental.lxc as a known experimental backend key and adjusts key matching for containment: "lxc" (with test).
src/core/lxc/src/main.rs	Switches to load_mxc_request and routes state-aware requests to LxcStateAwareRunner.
src/core/lxc/Cargo.toml	Adds serde_json dependency for error envelope printing.
src/Cargo.lock	Lockfile update reflecting the new serde_json dependency.
src/backends/lxc/common/src/state_aware.rs	New LXC state-aware lifecycle implementation.
src/backends/lxc/common/src/lib.rs	Exposes the new state_aware module.
sdk/src/state-aware-types.ts	Adds lxc to state-aware backend union and defines LXC per-phase config + metadata types.
sdk/src/state-aware-helper.ts	Adds lxc prefix mapping and hoists containerId as a cross-cutting field.

[Darren Hoehna (dhoehna)]

🧪 Local test re-verification — 2026-07-17

Re-ran the test suites locally at branch tip f84cf2b on a dev workstation. All green (0 failures).

Windows host (x86_64-pc-windows-msvc, cargo 1.96.1):

• cargo test -p wxc_common → 396 passed, 0 failed
• cargo test -p mxc-sdk → 11 passed, 0 failed (+4 runtime-ignored — require an elevated, host-prepped Windows host per docs/host-prep.md)

Linux (WSL2 Ubuntu-24.04, x86_64-unknown-linux-gnu, cargo 1.97.0, isolated CARGO_TARGET_DIR):

• cargo test -p lxc_common → 62 passed, 0 failed — the state-aware LXC backend ran on Linux (the original Validation only cargo check-compiled it for the Linux target).

Note: the wxc_common count differs slightly from the original Validation because the branch advanced since it was written.

[Darren Hoehna (dhoehna)]

🔀 Merge-conflict resolution — synced with main (2026-07-17)

Merged microsoft/mxc main (d271ac4) into this branch to clear the dirty (unmergeable) state. This is the merge-time reconciliation flagged in the Coupling section of the description — it reconciles this PR with the Windows Sandbox state-aware work (#578) and the telemetry correlationVector work (#624) that landed on main after this PR was opened.

Only one conflict, in src/core/wxc_common/src/state_aware_dispatch.rs — both sides registered a new state-aware backend in the same place:

• backend_from_prefix: kept both the "lxc" (this PR) and "wsb" (Add the Windows Sandbox state-aware lifecycle + host daemon (Phase 2) #578) prefix arms alongside "iso".
• Kept both resolve_backend_* unit tests (..._for_lxc_prefix_returns_lxc and ..._for_wsb_prefix_returns_windows_sandbox).
• Added correlation_vector: None to the LXC test's ParsedStateAwareRequest literal, since telemetry: implement deferred crash/cancellation/state-aware lifecycle instrumentation #624 added that field to the struct after this PR's tests were written.

Everything else auto-merged (incl. the mxc_engine::state_aware::run_state_aware backend arms and the SDK state-aware-*.ts types).

Local validation at merge tip dd29002 (Windows workstation):

• cargo test -p wxc_common -> 455 passed, 0 failed
• cargo fmt -p wxc_common -- --check -> clean
• cargo build -p wxc (compiles the merged mxc_engine) -> success
• SDK npm run build + npm test -> 189 passed, 0 failed, 4 skipped

(Linux lxc_common compile/tests are exercised by CI on this commit.)

[Gudge (MGudgin)]

apply_network_policy

Container egress is still not filtered — the network policy fails open for the container's entire runtime. This is separate from the teardown-ordering fix in this update (which correctly closed the stop/deprovision window); this one is about the direction the chain is hooked, and it is untouched.

start() applies network enforcement here via apply_network_policy() -> NetworkIptablesManager::apply_firewall_rules(). The per-container chain filters on destination (-d ACCEPT/DROP for allowed/blocked hosts, then a default DROP/ACCEPT), but it is hooked into FORWARD with:

-I FORWARD -o <veth> -j <chain>   (src/backends/lxc/common/src/network_iptables.rs:237)

In a bridged veth setup, container->internet egress enters the host FORWARD chain as -i , while internet->container replies are -o . Hooking on -o therefore applies the chain only to traffic heading INTO the container, never to traffic originating from it:

• Outbound SYN (-i veth): not matched -> leaves unfiltered; blocked_hosts, allowed_hosts, and the default DROP are all bypassed.
• Return SYN-ACK (-o veth): matched, but it's ESTABLISHED,RELATED -> ACCEPTed; and -d can't match a remote host on the return path anyway.

Net effect: default_network_policy=Block, blocked_hosts, and allowed_hosts are ineffective for egress; a container started with a deny-by-default policy can reach any destination for its whole lifetime, not just during teardown.

Two related fail-open paths in the same function: if discover_veth_interface() returns None it logs "Skipping FORWARD hook" but still sets rules_applied=true / returns Ok(true), so start() treats an unhooked container as success; and a non-Firewall/Both enforcement mode returns Ok(true) without applying anything.

Suggested fix: hook the chain on -i (network_iptables.rs:237) and change the cleanup -D FORWARD -o (network_iptables.rs:264) to -i, so the destination-based rules apply to container egress. Fail start (return Ok(false)/error) when the veth can't be discovered or the hook can't be installed, instead of proceeding with no enforcement. Add a test that sets deny-by-default and asserts the container cannot reach a non-allowlisted host.

[Darren Hoehna (dhoehna)]

Fixed in 287b918.

You're right, and the direction was wrong in both places. For a host-side veth,
container-originated packets arrive at the host on that interface, so egress
matches FORWARD -i vethX; -o vethX matches traffic flowing toward the
container. So container egress -- the thing the policy exists to restrict -- was
never filtered.

Changed the insert and the matching -D at teardown; if the delete keeps -o
the FORWARD hook leaks on every stop. force_cleanup delegates to
remove_firewall_rules, so the signal path picks up the same fix rather than
needing its own.

Same bug was fixed independently on #631 (96af8f9); this branch had its own copy.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.

[Darren Hoehna (dhoehna)]

Partly addressed in 95a197d; the streaming gap itself is still open, and I'd
rather say so than imply otherwise.

You're right that LXC never reaches dispatch_state_aware_exec. What I fixed
is the misleading part: the fallback arm reported "backend Lxc does not
implement the state-aware lifecycle", which is simply untrue --
run_state_aware dispatches every phase for it -- and it sends callers off to
debug a provision path that works fine. The error now distinguishes "no
lifecycle at all" from "lifecycle but no streaming exec" and names the API that
does work.

I did not implement a real streaming SandboxProcess for LXC. That needs
lxc-attach process plumbing with live stdout/stderr, and I have no LXC host
here to validate it against -- shipping an unexercised streaming
implementation seemed worse than leaving the gap legible. Happy to file that as
a work item, or to take it in a follow-up if you'd prefer it in this PR.

[Darren Hoehna (dhoehna)]

I'm tracking the streaming SandboxProcess as a follow-up rather than doing it in this PR, and I filed #765 for it. It needs lxc-attach process plumbing with live stdout/stderr and a real LXC host to validate against, and I have neither here, so shipping an unexercised streaming path would be worse than a legible gap. The misleading half is already fixed in this PR: the fallback arm now distinguishes "no lifecycle" from "lifecycle but no streaming exec" and names the API that works.

[Darren Hoehna (dhoehna)]

Fixed in 287b918, with a follow-up correction in c453787 -- see the note at
the end, my first cut of this was wrong.

The fail-open was real: apply_firewall_rules treats a non-firewall
enforcement mode as a successful no-op, enforcementMode defaults to
capabilities, and LXC has no capability-based network enforcement. So a start
carrying allowedHosts/blockedHosts returned success with the container
completely unfiltered.

apply_network_policy now rejects that combination instead of reporting
success, and also fails when the host-side veth can't be discovered in a
firewall mode -- without it the chain is built but never hooked into FORWARD,
which apply_firewall_rules only warned about.

The correction: I originally also treated defaultPolicy: "block" as an
explicit restriction. That was wrong, and Copilot caught it below --
NetworkPolicy::default() is Block, so once the wire network block is
flattened into ContainerPolicy a requested "block" is indistinguishable
from no network block at all. It rejected every plain start, including the
new E2E lifecycle test. Now gated on the host lists only, matching the
reasoning has_network_policy already uses. The residual gap -- defaultPolicy: "block" alone is not enforceable under capabilities and can't be
distinguished from unset -- is documented in the API doc rather than papered
over.

[Darren Hoehna (dhoehna)]

Not fixed -- and I think the suggested approach doesn't work. Details, because
the conclusion is non-obvious.

The bug is real, confirmed: clear_config_item("lxc.mount.entry") wipes
everything, and since provision permits an external containerId and adopts a
pre-existing container, start can drop template or security-baseline mounts.

But the dedicated-include-file fix would not help. I checked liblxc rather than
assuming (lxc/lxc main, commit dc15af1):

• lxc.include's set handler is set_config_includefiles (confile.c), and
  it calls lxc_config_read(value, lxc_conf, true) synchronously. The
  included file is parsed before set_config_item returns, not deferred to
  start.
• Every lxc.mount.entry in it goes through set_config_mount and lands in
  lxc_conf->mount_entries as an ordinary struct string_entry. The
  from_include flag only suppresses appending to the raw unexpanded_config
  text; the in-memory list is fully merged.
• clr_config_mount calls lxc_clear_mount_entries (conf.c), which frees
  the whole list with no filter. string_entry carries no provenance.

So entries from an MXC-owned include file are indistinguishable from any other
and get wiped by the same clear. Relatedly, clr_config_includefiles is a
literal return 0; no-op and get_config_includefiles returns ENOSYS --
liblxc itself acknowledges include state can't be recovered post-expansion.
There's no selective-removal API for lxc.mount.entry.

What does work, from the same reading:

• get_config_item("lxc.mount.entry") returns all current entries,
  newline-separated raw fstab strings, so snapshot -> clear -> re-add the
  entries we don't own is viable.
• Unknown mount options are tolerated for ownership tagging:
  parse_lxc_mount_attrs only strips create=dir/create=file/optional/
  relative/idmap=, and parse_vfs_attr returns 1 for anything it doesn't
  recognize, which appends it to opts->data for mount(2). For a none
  bind mount the kernel ignores data, so an x-mxc-owned marker is inert.

The catch, and why I stopped rather than pushing a fix: that reasoning holds
for bind entries but not for the tmpfs mask entries this function also
generates, where data is consumed and an unrecognized option can fail the
mount with EINVAL. So the tagging scheme needs a second mechanism for those,
and I can't exercise any of it without an LXC host. Given this silently affects
real mounts, I'd rather land it as its own reviewed change than bolt it onto
this PR. Happy to file it with the above as the starting point -- say the word
and I'll open it.

[Darren Hoehna (dhoehna)]

This is fixed now -- my earlier "not fixed" note is superseded. configure_filesystem_mounts calls clear_mxc_mount_entries (lxc_bindings.rs:398), not clear_config_item: set_mxc_mount_entry writes a "# mxc-managed-mount" marker line directly above each entry it adds (lxc_bindings.rs:363), and the clear removes only marker+entry pairs, so template and operator mounts survive. It edits the raw config file text rather than going through liblxc's config API, which is what sidesteps the in-memory provenance loss I wrote up before -- set_config_item and clear_config_item already work the same way. I haven't exercised it against a live LXC host, since I don't have one on this box.

[Darren Hoehna (dhoehna)]

Fixed in 287b918.

Confirmed both leaks you're pointing at. removeRulesOnExit is SDK-only:
wire::Network (wire.rs:311) is deny_unknown_fields and has no such field,
and the SDK never strips it, so sending it fails the entire request rather
than being ignored. allowLocalNetwork does deserialize, but it appears in the
whole LXC backend exactly once -- state_aware.rs inside has_network_policy
-- and is never turned into an iptables rule.

LxcNetworkConfig is now an explicit
Pick<NetworkConfig, 'defaultPolicy'|'allowedHosts'|'blockedHosts'> plus
enforcementMode narrowed to 'firewall'|'both', so the type matches the
runtime check rather than Omit quietly re-admitting whatever gets added to
NetworkConfig later.

The existing type test asserted the old shape, so it was encoding the bug --
replaced it with cases that include @ts-expect-error guards on each excluded
field.

[Darren Hoehna (dhoehna)]

Fixed in 287b918. The seven LXC state-aware types are now exported from
sdk/node/src/index.ts, matching how the IsolationSession and WindowsSandbox
equivalents are surfaced. Without it they were unreachable from the package
entry point, so the typed API was undeclarable by consumers.

[Darren Hoehna (dhoehna)]

Fixed in 287b918.

Verified the doc was wrong: the state-aware shapes are documented as never
carrying containerId, but LXC provision does. Updated the
sandboxId/containerId table with an explicit LXC exception covering
adopt-or-create.

Also documented something that came out of checking this, because it's a sharp
edge rather than a doc nit: deprovision calls container.destroy()
unconditionally, including for a container that already existed and was merely
adopted. MXC keeps no state between phases, so at deprovision time it genuinely
cannot tell an adopted container from one it created -- a caller who passes an
existing containerId will lose it. Documented rather than "fixed" because a
real fix needs provenance persisted at provision; happy to file that if you'd
like it tracked.

Added the missing LXC row to the policy-honor matrix in 10.3 while I was in
there.

[Darren Hoehna (dhoehna)]

Fixed in 783c197.

You're right that nothing ran it. lxc_state_aware_provision.json was checked
in but no script referenced it, and run_lxc_all_tests.sh had no state-aware
entry at all -- only one-shot cases. Both other backends already have a
lifecycle script; LXC was the odd one out.

Added run_lxc_state_aware_test.sh, registered in the suite. It drives
provision -> start -> exec -> stop -> deprovision, relaying the provisioned
sandboxId the way a real client does, asserts the lxc:mxc- prefix, and checks
that exec relays a nonzero script exit code rather than swallowing it. sed
rather than jq/python for parsing, since neither is guaranteed on an LXC host,
and an EXIT/INT/TERM trap deprovisions on early failure so a leaked container
doesn't break the next run.

I don't have an LXC host here, so I verified it against a stub lxc-exec:
happy path 8/8 with the sandboxId correctly relayed into each later phase, a
failing phase counted with a nonzero exit and no double-deprovision, and a
SIGTERM mid-run producing exactly one cleanup deprovision of the right sandbox.
bash -n clean, LF endings so the suite's CRLF guard passes, mode 100755. It
still needs a real host run to be worth anything -- flagging that explicitly.

Worth noting this test immediately earned its keep: it's what made the
defaultPolicy regression in my own network fix visible (see the thread
above).

[Darren Hoehna (dhoehna)]

Fixed in 95a197d.

Confirmed -- lxc/src/main.rs called mxc_engine::run_state_aware directly
while wxc/src/main.rs wrapped the identical call in telemetry init,
backend/phase process attribution, the MS-CV seed/spin plan, the crash panic
hook, and the terminal emit_state_aware. So a Linux lifecycle emitted no
lifecycle telemetry, installed no crash hook, and carried no correlation vector
-- provision returned no cV for the client to relay into later phases. All of
that is invisible at the call site, which is how it went unnoticed.

Took the "share" option rather than "mirror": the orchestration moved into
mxc_engine::run_state_aware_with_telemetry and both entry points call it, so
they can't drift again. Executors keep their own terminal behavior (buffer
flush, stdout envelope, exit code) -- only the observability wrapper is shared.
The correlation-vector helpers moved with it along with their six tests, ported
verbatim rather than rewritten so coverage is unchanged.

This paid off immediately in the merge with main: upstream added
log_state_aware_dispatch_error to the wxc entry point only, and the lxc one
would have silently drifted again. Mirrored it in the same commit.

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/backends/lxc/common/src/state_aware.rs:282

• This gate checks only the number of indices, but the code below always pins lxc.net.0.veth.pair and assumes a veth. An adopted config with one interface at lxc.net.1, or one lxc.net.0.type = macvlan, passes this check; its real interface does not use the deterministic veth, so the -i mxcv… hook does not filter its egress. Require exactly index 0 with type veth, or derive and hook the actual supported interface, before reporting firewall enforcement.

        if net.indices.len() > 1 {

src/backends/lxc/common/src/state_aware.rs:610

• --dry-run stops after this validator, so it currently accepts start policies that real start rejects (for example network.proxy, or explicit default-block/host rules under capabilities). It also skips the side-effect-free filesystem normalization/delegation checks performed only by apply_filesystem_policy. Move the static start-policy checks into validation (and reuse them from start) so dry-run accurately predicts whether the request is valid.

    fn validate_start(
        &self,
        sandbox_id: &str,
        _request: &ExecutionRequest,
        _config: Option<&()>,
    ) -> Result<(), MxcError> {
        extract_container_name(sandbox_id)?;
        Ok(())

sdk/node/src/state-aware-types.ts:23

• Adding lxc to this common backend union also makes execInSandbox(lxcId, ...) type-check, because that API is generic over StateAwareContainmentBackend and has no LXC exclusion or runtime guard. This contradicts the new README and PR contract that LXC streaming exec is unavailable. Introduce a streaming-capable backend subset for execInSandbox, or support the exposed LXC path and update the documentation/follow-up scope.

export type StateAwareContainmentBackend = Extract<
  ContainmentBackend,
  'isolation_session' | 'lxc' | 'windows_sandbox'
>;

[Darren Hoehna (dhoehna)]

The observation is accurate on every point -- -F and -X errors were discarded, remove_forward_hooks swallowed everything, and the function returned Ok(()) unconditionally while force_cleanup dropped the result. I fixed part of this in e65f65b and declined the rest, and the two halves deserve to be separated because they are not equally dangerous.

The half that mattered: flushing a chain that is still hooked. This is a fail-open, not a leak. -F succeeds whether or not anything still jumps to the chain, and an emptied user chain returns to its caller instead of reaching its own closing DROP -- so a chain that is still hooked but no longer filtering lets traffic straight through, silently. remove_forward_hooks (which this PR added in 5c03c13) was best-effort and the -F ran regardless of whether it achieved anything.

It now re-reads FORWARD after issuing its deletes and reports whether any jump to the chain remains, and that verdict gates the flush. Re-reading rather than trusting the -D exit codes is deliberate: a -D can fail for a rule another process is also tearing down, and its exit code says whether this call removed the rule, not whether the rule is gone. An unreadable FORWARD answers "still hooked" -- being wrong that way costs a retry, being wrong the other way unfilters a live container.

Deleting is the safe half and needs no such gate: -X on a still-referenced chain simply fails, and the chain stays owned and filtering for a later teardown to retry.

The half I did not change: propagating the failure. A chain whose hooks are confirmed gone is unreferenced -- it filters nothing and blocks nothing, and costs a chain name until the next teardown for that container reclaims it. Turning that into a failed stop/deprovision reports failure for an operation whose observable goal (container stopped, container destroyed) did succeed, and it does so on the path where the container is already gone and the caller has nothing useful to do with the error. I would rather leave the leak visible in the log than make the common case fail. Happy to be argued out of this one, but I did not want to change stop's error contract as a side effect of a teardown fix.

teardown_chain is split out from process execution so the flush ordering has an oracle at all -- every iptables command fails at spawn on a non-Linux host, so a test driving the real path could never observe a flush. Mutation-verified: flushing regardless of hook state fails only the new hook test, with a negative control (hooks gone must still flush then delete) so "never flush" cannot pass. 113 lxc_common tests.

Cross-reference: #632 fixes the same flush ordering in this file in 10f0599, by a different route (an iptables -C absence check rather than a FORWARD re-read, because that branch tears down hooks in both v4 and v6 tables). Whichever of the two merges second will need a conflict resolution here; I did not want to leave the fail-open in whichever merges first.

[Copilot]

Pull request overview

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

Suppressed comments (7)

src/backends/lxc/common/src/state_aware.rs:306

• A single configured interface is not necessarily lxc.net.0. An adopted config containing only lxc.net.1.* passes this count check, but the code below pins lxc.net.0.veth.pair; the real net.1 veth remains random and bypasses the only firewall hook while start reports success. Require the sole index to be exactly 0.

        if net.indices.len() > 1 {

sdk/node/src/state-aware-types.ts:140

• enforcementMode is optional here, so { blockedHosts: ['example.com'] } and { defaultPolicy: 'block' } both type-check. Rust defaults an omitted mode to Capabilities and rejects those requests, contradicting the stated compile-time enforcement. Model this as a union that requires firewall/both whenever a restriction is present, or require the mode for every LXC network object.

export type LxcNetworkConfig = Pick<
  NetworkConfig,
  'defaultPolicy' | 'allowedHosts' | 'blockedHosts'
> & {
  /** LXC enforces network policy only via iptables. */
  enforcementMode?: 'firewall' | 'both';

sdk/node/src/state-aware-types.ts:148

• The implementation installs the iptables chain and hook before container.start() specifically to avoid an unfiltered window, so this public JSDoc states the opposite ordering.

  /** iptables policy to apply after the container starts. `proxy` is not supported by this backend. */

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

• Teardown ignores every hook-deletion, flush, and chain-deletion failure and then returns Ok(()). Consequently state-aware stop/deprovision can report success while a stale hook/chain remains; the next start then fails at iptables -N with no indication that the prior successful cleanup was incomplete. Propagate residual cleanup failures (while retaining ownership for retry) through the lifecycle result.

        let _ = Self::run_iptables(&["-F", &self.chain_name], logger);
        if Self::run_iptables(&["-X", &self.chain_name], logger).is_ok() {
            self.chain_created = false;
            crate::signal_cleanup::clear_active_chain_created();
        }

        self.rules_applied = false;
        Ok(())

sdk/node/src/state-aware-types.ts:22

• Adding lxc to this common backend union also makes the generic execInSandbox API accept SandboxId<'lxc'>; that function has no LXC guard and spawns lxc-exec. This conflicts with the README and PR contract that LXC streaming exec is unavailable and tracked separately. Use a narrower streaming-capable backend type (plus a runtime guard for untyped callers), or document and support this path.

This issue also appears in the following locations of the same file:

• line 135
• line 148

export type StateAwareContainmentBackend = Extract<
  ContainmentBackend,
  'isolation_session' | 'lxc' | 'windows_sandbox'

tests/scripts/run_lxc_state_aware_test.sh:51

• Trapping INT/TERM with cleanup does not make Bash exit; a trapped signal normally resumes execution afterward. Because this script intentionally omits set -e, cancellation can deprovision the sandbox and then continue running later phases against it instead of terminating. Let EXIT perform cleanup and have signal traps exit with the conventional status.

trap cleanup EXIT INT TERM

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:1626

• This says explicit defaultPolicy: "allow" leaves the presence bit unset, but the parser sets default_network_policy_present = true for every explicit value. allow merely does not require firewall enforcement because it is permissive. Correcting this distinction avoids misleading backend implementers about the internal contract.

> supplies no `network.defaultPolicy`, or supplies `defaultPolicy: "allow"`, does not
> set the presence bit and is unaffected.  To start a default-deny container, set
> `enforcementMode` to `"firewall"` or `"both"`.

[Darren Hoehna (dhoehna)]

Confirmed and fixed in 5449775. The mechanism is exactly as described.

cleanup_network builds a fresh NetworkIptablesManager, which cannot clear the owning manager's chain_created. Drop tears down whenever that flag is set (network_iptables.rs, impl Drop), so the arm returned straight into a second teardown of a chain it had already deleted. And the chain name is derived from the container name, so another start of the same sandbox aims at exactly that name -- if it wins the gap between the two teardowns, the trailing one removes hooks by chain name and deletes the chain, and that start's container runs unfiltered. This process's failed start becomes someone else's fail-open.

Your remedy is what I took: the arm now calls fw_manager.remove_firewall_rules directly. A successful -X clears chain_created so the drop is a no-op; a failed one leaves it set so the drop is the retry it is there to be. It also knows the pinned veth, which a fresh manager does not.

While fixing it I named Drop's condition needs_teardown and split the ownership update into apply_teardown_outcome, because neither had an oracle. Every iptables command fails at spawn on a non-Linux host, so no test on this branch could previously reach a successful -X to observe ownership being released at all.

Mutation-verified: never clearing ownership fails only the new drop test, and the negative control -- a chain whose delete failed must stay owned so the drop retries -- stays green, so "always clear" cannot pass either. 115 lxc_common tests.

Scope note on what I did not claim: the end-to-end sequence (successful -X clears the flag, drop then no-ops) still needs Linux with real iptables. What is covered here is the ownership rule the no-op depends on, plus teardown_chain deciding relinquishment only on a successful -X.

[Darren Hoehna (dhoehna)]

Correct, and fixed in 5449775.

I verified the premise before taking the remedy. impl Drop (network_iptables.rs) tears down whenever chain_created is set, and the error arm cleaned up through a fresh manager, which has no way to clear the owning manager's flag. So the function returned straight into a second teardown, exactly as described. The window you name is real: the chain name is deterministic, so a start that wins the gap between the two teardowns has its live firewall stripped by the second one.

Taken as written — the error arm now calls fw_manager.remove_firewall_rules(logger), so a successful -X clears the owner's own flag and Drop becomes a no-op.

Two things I had to add to make that testable, because neither had an oracle before. Drop's condition was inline, so no test could ask whether it would tear down; it is now needs_teardown(). And the ownership update after a teardown was buried in remove_firewall_rules, so it is now apply_teardown_outcome(bool). Both are named purely so the sequence has an observable, and both are mutation-verified with negative controls — an implementation that always clears ownership, or never does, fails.

What I want to be straight about is the limit of that verification. On this Windows box every iptables command fails at spawn, so the one sequence that matters end-to-end — -X succeeds, chain_created clears, Drop no-ops — is unreachable in a test here. The seams pin the decision logic; they do not prove the real path. That needs a Linux host and is not covered.

[Copilot]

Pull request overview

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

Suppressed comments (7)

tests/scripts/run_lxc_state_aware_test.sh:150

• If this deprovision fails, check records the failure but returns success, and SANDBOX_ID is then cleared unconditionally. The EXIT trap therefore cannot retry cleanup, leaving exactly the container this guard is intended to reclaim. Capture the phase status and clear the ID only after a successful deprovision.

run_phase deprovision "$SANDBOX_ID"
check "deprovision exits 0" $?
# Claimed by this call; stop the trap from deprovisioning a second time.
SANDBOX_ID=""

sdk/node/src/state-aware-types.ts:110

• This public field's ownership semantics are hazardous but undocumented here: supplying an existing name adopts that container, and a later deprovisionSandbox destroys it unconditionally. Add that warning to the SDK type so IDE/API consumers do not mistake this for a harmless label.

  /** Optional externally assigned LXC container name. */
  containerId?: string;

sdk/node/src/state-aware.ts:86

• This generic API does not always receive dispatch errors on stderr. The Windows wxc entry point writes state-aware error envelopes to stdout, while only the LXC executor routes non-dry-run exec failures to stderr. Describe the output as backend-dependent (or simply as raw PTY output) so Windows/IsolationSession consumers are not directed to the wrong channel.

 * On dispatch failure the executor emits a single error envelope on stderr,
 * because stdout is carrying the container's raw output; the SDK does not
 * parse it here — callers consuming `IPty.onData` see the raw bytes. Use
 * `execInSandboxAsync` when typed-error throwing is needed.

sdk/node/src/state-aware-types.ts:114

• These fields are required only when a config object is supplied, but provisionSandbox still declares config?: ProvisionConfigFor<C> in state-aware.ts. Consequently provisionSandbox('lxc') type-checks, emits no experimental.lxc.provision, and is rejected by Rust at runtime. Make the argument conditionally required for LXC (for example with overloads or a conditional rest tuple) and add a compile-time test.

  /** Linux distribution for the container rootfs, e.g. "alpine" or "ubuntu". */
  distribution: string;
  /** Distribution release version, e.g. "3.20" or "24.04". */
  release: string;

tests/scripts/run_lxc_state_aware_test.sh:51

• An INT/TERM trap replaces Bash's default terminating action; after cleanup returns, the script can resume instead of exiting as the comment assumes. It then continues with a deleted work directory and deprovisioned sandbox. Keep cleanup on EXIT, but make the signal handlers exit explicitly.

This issue also appears on line 147 of the same file.

trap cleanup EXIT INT TERM

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:1626

• This says explicit defaultPolicy: "allow" leaves the presence bit unset, but the parser sets default_network_policy_present = true for every explicit value, and the new parser test asserts that behavior. Allow is unaffected because it is permissive, not because presence is false.

> supplies no `network.defaultPolicy`, or supplies `defaultPolicy: "allow"`, does not
> set the presence bit and is unaffected.  To start a default-deny container, set

sdk/node/src/state-aware-types.ts:148

• This public API comment reverses the enforcement order: the backend installs the iptables chain and hook before container.start() specifically to avoid an unfiltered window. Documenting post-start application misstates the security behavior.

  /** iptables policy to apply after the container starts. `proxy` is not supported by this backend. */

[Darren Hoehna (dhoehna)]

Confirmed, and worth recording that this window was opened by an earlier fix on this same line.

The ordering in start is as described. apply_filesystem_policy runs at line 482 and mutates the shared container config, and the only cross-process contention check is the chain-ownership test inside apply_network_policy at 496 -- see the comment at 497-501, which already reasons about "another start owns the chain". So the contention check exists, but it sits after the config has been cleared and rewritten. Two concurrent starts can interleave inside that gap.

The clear-then-rewrite is not incidental: it came from f84cf2b, which added LxcContainer::clear_config_item to fix mount-list accumulation that MGudgin raised at this same line. That fix is correct for the single-process case and is what creates the multi-process window, so reverting it is not the answer.

Also note container.is_running() at 475 is itself part of the race, not just the config write -- it is a plain check with no lock held, so the already-running rejection at 477 can pass in both processes.

I am not implementing the lock in this PR. It needs to serialize the is_running check, the config mutation, the firewall install, and lxc-start under one cross-process lock keyed on the container, and it has to interact correctly with the existing signal_cleanup rollback and with teardown -- a lock acquired on the start path that stop and deprovision do not honor would give false confidence. That is a design change to the lifecycle, not a local edit, and doing it at the end of a session is how a correctness fix becomes a deadlock. Filing it rather than rushing it.

Leaving this thread open deliberately.

[Copilot]

Pull request overview

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

Suppressed comments (6)

src/backends/lxc/common/src/state_aware.rs:600

• A concurrent start can be between firewall installation and container.start() here. This process then observes is_running() == false, skips stopping, deletes the firewall, and returns; the racing start can subsequently complete and report success with the container running unfiltered. Serialize start/stop/deprovision with a cross-process per-container lock so the running-state check and firewall teardown cannot race lifecycle transitions.

            container
                .stop()
                .map_err(|e| MxcError::backend_error(format!("Failed to stop container: {e}")))?;
        }
        cleanup_network(container_name, veth.as_deref(), &mut logger);

src/backends/lxc/common/src/signal_cleanup.rs:322

• The state-aware rollback is registered before firewall installation, so a signal can arrive after iptables -N but before lxc-start. In that window the container is already stopped; lxc-stop can fail, causing execute_rollback to abort before RemoveFirewall and leave the chain behind. Make the rollback distinguish an already-stopped pre-start container from a failed attempt to stop a running one, while preserving the fail-closed behavior for an in-progress start.

                RollbackStep::StopContainer => LxcContainer::new(&name, None).stop().is_ok(),

tests/scripts/run_lxc_state_aware_test.sh:51

• Bash does not automatically exit after an explicit INT or TERM trap returns, so cancellation runs cleanup and then continues the remaining phases against the deprovisioned sandbox. Use signal-specific traps that clean up and exit, leaving the EXIT trap guarded as intended.

trap cleanup EXIT INT TERM

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:1625

• This contradicts the parser and the test above: an explicit defaultPolicy: "allow" does set default_network_policy_present; it is unaffected only because requires_firewall_enforcement checks that the value is Block. Correct the explanation so callers do not rely on the wrong presence semantics.

> supplies no `network.defaultPolicy`, or supplies `defaultPolicy: "allow"`, does not
> set the presence bit and is unaffected.  To start a default-deny container, set

sdk/node/src/state-aware-types.ts:148

• The policy is installed before container.start() to avoid an unfiltered startup window, not afterward. This public type documentation currently reverses the security-relevant ordering.

  /** iptables policy to apply after the container starts. `proxy` is not supported by this backend. */

sdk/node/src/state-aware-helper.ts:16

• Making containerId globally cross-cutting also lifts it for Windows Sandbox, IsolationSession, and non-provision phases. JavaScript callers can therefore send a field that the public contract says is LXC-provision-only, and the Rust envelope accepts it at top level where those backends silently ignore it; previously it remained in the phase config and was rejected. Lift this field only when backendKey === 'lxc' && phase === 'provision' (and add a cross-backend regression test).

export const CROSS_CUTTING_FIELDS = ['containerId', 'filesystem', 'network', 'ui', 'process'] as const;

[Darren Hoehna (dhoehna)]

Fixed in 5adda93, though not quite as suggested -- the proposed rule would have over-rejected.

Verified the Rust side first rather than taking the claim on trust: requires_firewall_enforcement (src/backends/lxc/common/src/state_aware.rs:166) counts exactly three things as a restriction -- a non-empty allowedHosts, a non-empty blockedHosts, or an explicit defaultPolicy: "block". An explicit defaultPolicy: "allow" and an omitted defaultPolicy are deliberately not restrictions, and the comment above that function calls out why: the absent case is the plain start in run_lxc_state_aware_test.sh, which must not be rejected. So "requires a firewall mode whenever restrictive fields are present" would have broken a start that works today.

LxcNetworkConfig is now a union of two arms that mirror that function:

• LxcRestrictedNetworkConfig -- enforcementMode is mandatory, and the restrictive fields are allowed.
• LxcUnrestrictedNetworkConfig -- enforcementMode is optional, defaultPolicy narrows to 'allow', and allowedHosts/blockedHosts are never, so adding one moves the value to the arm that requires the mode.

{ defaultPolicy: 'block' }, { allowedHosts: [...] }, and { blockedHosts: [...] } no longer compile; { defaultPolicy: 'allow' } and {} still do.

Covered by three @ts-expect-error cases and two positive cases in state-aware-types.test.ts. Those markers are load-bearing in both directions: the test build fails if the restrictive cases stop erroring, and it also fails if the permissive cases start erroring, so the over-rejection is guarded too. 232 passing, 0 failing.

[Copilot]

Pull request overview

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

Suppressed comments (6)

src/backends/lxc/common/src/state_aware.rs:306

• A single adopted interface is not necessarily lxc.net.0. For example, a config containing only lxc.net.1.type passes both the len() > 1 and empty checks, but the code below pins lxc.net.0.veth.pair and hooks that deterministic name. Traffic on the real net.1 veth then bypasses the firewall while start reports success. Require the sole index to be exactly 0 (or pin/hook the actual index).

        if net.indices.len() > 1 {

src/backends/lxc/common/src/state_aware.rs:600

• This cleanup is racy with start: another process can install the firewall, then pause before container.start(). A concurrent stop observes is_running() == false, skips lxc-stop, and removes that firewall here; the first process then starts the container unfiltered. The process-local chain_created bit cannot protect cross-process stop/start. Serialize start, stop, and deprovision with a per-container cross-process lock held across firewall installation/start and stop/cleanup.

        cleanup_network(container_name, veth.as_deref(), &mut logger);

tests/scripts/run_lxc_state_aware_test.sh:51

• Trapping INT/TERM with cleanup suppresses the signal's default termination; after the handler returns, Bash can continue the test with WORK_DIR already deleted and CLEANED_UP set. The comment's assumption that the shell exits automatically is therefore false. Keep EXIT cleanup, but explicitly exit with the conventional signal status from the signal traps.

trap cleanup EXIT INT TERM

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:1626

• This says explicit defaultPolicy: "allow" does not set the presence bit, but convert_wire_config sets default_network_policy_present = true for every present value, and the new network_default_policy_allow_sets_presence_true test asserts exactly that. Clarify that allow sets the bit but does not trigger firewall enforcement.

> supplies no `network.defaultPolicy`, or supplies `defaultPolicy: "allow"`, does not
> set the presence bit and is unaffected.  To start a default-deny container, set
> `enforcementMode` to `"firewall"` or `"both"`.

sdk/node/src/state-aware-types.ts:185

• This public type documentation reverses the security-critical ordering: the implementation installs the iptables policy before container.start() specifically to avoid an unfiltered window. Saying it is applied after start is inaccurate.

  /** iptables policy to apply after the container starts. `proxy` is not supported by this backend. */

sdk/node/README.md:235

• The claimed LXC SDK restriction is not enforced. StateAwareContainmentBackend now includes lxc, so execInSandbox(SandboxId<'lxc'>, ...) type-checks and spawns lxc-exec; the LXC attach_run path forwards the inner PTY to executor stdout while backend.exec is blocked, so the returned Node IPty can receive live output. The missing SandboxProcess affects the direct engine/FFI streaming entry point, not this CLI-backed Node API. Either document Node streaming as supported, or explicitly reject LXC in execInSandbox if it must remain unavailable pending #765.

> **Backend support:** the state-aware lifecycle is currently implemented for `isolation_session` and `windows_sandbox` (both Windows-only; both still experimental, so every call must pass `{ experimental: true }`) and `lxc` (Linux-only; not experimental).  Streaming exec — `execInSandbox`, which returns an `IPty` — is not available for `lxc`; use the non-streaming `execInSandboxAsync` instead.  The one-shot spawn APIs (`spawnSandbox` / `spawnSandboxFromConfig`) are the supported path for every other backend.

[Copilot]

Pull request overview

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

Suppressed comments (4)

src/backends/lxc/common/src/network_iptables_chainname_spec_tests.rs:14

• This new spec suite is not declared as a Rust module: lib.rs only declares network_iptables, and network_iptables.rs never includes this file. Cargo therefore does not compile or run any of these tests; once included, they would also expose the currently missing chain_name_for method. Add a test-only module declaration so the claimed chain-name validation actually runs.

use super::*;

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:1625

• This is inconsistent with the parser and the new unit test: explicit defaultPolicy: "allow" does set default_network_policy_present; it simply does not require firewall enforcement because the value is Allow. Please distinguish that from the omitted case so the documented presence semantics remain accurate.

> supplies no `network.defaultPolicy`, or supplies `defaultPolicy: "allow"`, does not
> set the presence bit and is unaffected.  To start a default-deny container, set

sdk/node/src/state-aware-types.ts:185

• The implementation installs the iptables chain and hook before container.start() to avoid an unfiltered startup window. This public type documentation says the opposite and obscures a security-relevant guarantee.

  /** iptables policy to apply after the container starts. `proxy` is not supported by this backend. */

tests/scripts/run_lxc_state_aware_test.sh:51

• Trapped INT and TERM signals do not automatically terminate Bash after the handler returns. This trap runs cleanup and then resumes the test with a deleted work directory and deprovisioned sandbox. Use separate signal traps that clean up and explicitly exit, while keeping EXIT for normal/early exits.

trap cleanup EXIT INT TERM

[Gudge (MGudgin)]

Approving. All five High findings from the adversarial review are resolved: egress FORWARD hook now filters container-originated traffic (-i), with veth-missing and non-firewall enforcement modes failing closed; firewall teardown ordering, PTY exec-timeout drain, mount-policy accumulation, and the mxc_engine dispatch delegation are all fixed, with tests added.

Note: one High remains open — provision() still returns an error on lxc-create failure without destroying partially-created on-disk state (orphaned auto-named container). Recommend a follow-up, but not blocking this approval.

[Gudge (MGudgin)]

Revoking my earlier approval and requesting changes.

The approval cited "tests added" as part of the evidence that the chain-name collision concern (the 32-bit-hash-truncation fix) was resolved. On closer inspection that verification is hollow: the dedicated collision-freedom spec suite never compiles or runs.

Findings:

1. src/backends/lxc/common/src/network_iptables_chainname_spec_tests.rs (1,035 lines) is not part of the module tree. lib.rs declares only network_iptables, and network_iptables.rs never includes the file via mod / include! / #[path]. The file begins with use super::*;, so it was intended to be an inline submodule of network_iptables, but nothing pulls it in. Cargo silently ignores unreferenced .rs files, so none of these tests execute.

2. Even if it were wired in, it would not compile: every test calls NetworkIptablesManager::chain_name_for(...), but there is no such method anywhere in the crate. The chain name is derived elsewhere. So the suite references a phantom API and is both inert and stale. (There is also a broken rustdoc intra-doc link at state_aware.rs:79 pointing at the same non-existent chain_name_for.)

Net effect: the adversarial-corpus collision tests that were presented as the evidence for the collision fix provide zero coverage. The concern is not entirely untested — the compiled tests inside network_iptables.rs (chain_name_sanitization, chain_name_truncation, chain_names_have_mxc_prefix_and_total_length_cap_of_twenty_four) do run — but the dedicated near-miss collision suite does not.

To clear this:

• Declare the spec suite so Cargo compiles and runs it (e.g. add #[cfg(test)] mod network_iptables_chainname_spec_tests; under network_iptables, matching the use super::*; the file already expects), and
• Add the referenced NetworkIptablesManager::chain_name_for API (or repoint the tests + the state_aware.rs:79 doc link at whatever method actually derives the chain name) so the suite compiles and passes.

For the record, this does not reintroduce any of the five High findings from the adversarial review — those remain fixed. This is specifically about the collision-fix verification being dead code. Separately, the previously noted open High still stands as a recommended follow-up: provision() returns an error on lxc-create failure without destroying partially-created on-disk state (orphaned auto-named container).

[Darren Hoehna (dhoehna)]

Closing this PR. The work is being re-cut from main as a series of small, independently reviewable PRs rather than continued on this branch.

The reason is the review backlog rather than a merge conflict — this branch still merges cleanly, but it accumulated 87 review comments across 42 commits faster than they could be resolved, which makes each individual correctness fix hard to verify in isolation.

The first slice is already up: #780 makes the LXC firewall chain name collision-free, deriving it from a SHA-256 digest of the original container name and sizing it to the measured 28-byte iptables ceiling. That covers the chain-identity objections raised here.

Still to be re-cut, each as its own PR: the state-aware lifecycle machinery, the signal-cleanup rework, the FFI streaming exec gap, and the remaining network-scoping issues.

The review comments on this PR are the input to that re-cut, not discarded.