[LXC] State-aware sandbox lifecycle

[Darren Hoehna (dhoehna)]

Problem

LXC only runs one-shot. A caller hands it a config, it builds a container, runs a single workload, and destroys the container. Driving several commands against the same environment costs a full container build and teardown every time, and nothing the first command leaves behind is there for the second.

Fix

LXC implements the state-aware lifecycle: provision, start, exec, stop, and deprovision. A caller builds a container once, execs against it as many times as needed, and tears it down when finished.

• The state-aware surface requires the experimental opt-in, joining the three backends already behind it. Without --experimental the request is refused and the message names the flag. One-shot LXC is untouched and still needs no flag.
• Two callers racing to start the same container can no longer both bring it up. Starts are serialized per container.
• Where the request asks for a firewall, policy is enforced on traffic in both directions rather than outbound only.
• Enforcement follows the schema the request declares. A 0.8 directional config always enforces. A 0.7 config enforces only where enforcementMode asked for it, which leaves a config written before that field behaving as it always did.
• The Node SDK exposes lxc as a state-aware backend with per-phase config types, and sends 0.8.0-alpha.

🔍 Validation

Measured on ec0ba3a3:

• run_lxc_all_tests.sh against a live LXC host — 31 passed, 0 failed, 0 skipped, 1 disabled.
• cargo test -p lxc_common -p bwrap_common on Linux — 829 passed, 0 failed.
• cargo test -p wxc_common -p mxc_engine on Windows — 1,150 passed, 0 failed.
• npm test in sdk/node — 317 passed, 0 failed; tsc --noEmit clean.
• The generated schema and wire.ts regenerate byte-identical to what is committed.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated documentation (if applicable)
• Updated Copilot instructions (if build, architecture, or conventions changed)
• If this PR changes Cargo.lock, the dependency-feed-check check passes

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Copilot reviewed 76 out of 78 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

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

tests/scripts/run_lxc_experimental_provision_fields_test.sh:205

• This assertion contradicts the script's stated ability to run without an LXC runtime. On such a host the well-formed request stops at the availability check with backend_unavailable, not backend_error, so the test fails even though field validation behaved correctly. Either require/skip when LXC is absent if backend-error mapping is part of this test, or accept backend_unavailable here while still rejecting the field-validation diagnostics.

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Adding this enum value exposes ProvisionSandbox(StateAwareContainment.Lxc), but the managed API has no LXC provision options and BuildProvisionEnvelope has no LXC case. Consequently it can never serialize the required experimental.lxc.provision.distribution and release, so every .NET LXC provision call is rejected. Add the LXC-specific provision/start option types and envelope serialization before advertising this backend, or leave LXC out of the enum.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

[Copilot]

🟡 Changes recommended

The .NET API is incomplete, exact-version contracts diverge, and LXC exec timeout isolation remains bypassable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Adding Lxc here exposes it as a usable .NET lifecycle backend, but the .NET surface cannot build a valid LXC request. There is no LxcProvisionOptions, BuildProvisionEnvelope has no LXC arm to emit the required experimental.lxc.provision.{distribution,release}, and ValidateProvisionOptions accepts null; consequently ProvisionSandbox(StateAwareContainment.Lxc) always reaches Rust without the required fields and fails as malformed_request. The start API also only accepts StateAwarePhaseOptions, so .NET callers cannot supply the filesystem/network policy that LXC applies at start. Add backend-specific provision/start option types and envelope serialization (plus typed metadata) before registering this enum value.

    Lxc,

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

• This channel assumption is already false for the new LXC path. LxcStateAwareRunner::exec relays workload output to stdout before returning, and a late failure such as timeout then appends the error envelope to that same stream; the added test at state-aware.test.ts:464-489 confirms tryParseErrorEnvelope(stdout) fails and the SDK resolves an ExecResult instead of throwing the documented typed MxcError. Please use a framing/separate-channel mechanism for dispatch failures (or otherwise make late failures unambiguous) rather than documenting stdout as envelope-only.

    // The cross-backend contract (§7.3) reserves stdout for the response
    // envelope in every phase, so a dispatch failure is always found there and
    // no backend needs its own channel rule.

src/core/wxc_common/src/wire.rs:590

• The new rolling wire field is not represented in the per-version 0.9.0-alpha contract. mxc_config_contract/src/dev/state_aware/provision/mod.rs still has only IsolationSession/WindowsSandbox/WSLC provision variants, and the dev adapter explicitly sets lxc: None; the exact schema and generated exact SDK oracle therefore also omit state-aware LXC. This leaves rolling and exact parsing inconsistent and will reject LXC when exact dispatch becomes authoritative. Add the closed LXC provision contract, adapter, exact-contract tests, and regenerate the 0.9.0-alpha schema/types alongside this wire change.

• Files reviewed: 89/91 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Network enforcement remains bypassable, while managed SDK and schema contracts are incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

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

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

• This statement conflicts with the new lifecycle-wide schema floor: an explicit pre-0.8 LXC version is rejected before the start policy is evaluated, so it does not merely record these fields and skip the firewall. Document the rejection instead.

src/backends/lxc/common/src/network_ingress.rs:509

• This rule only blocks the bridge gateway address, but hostLoopback: deny covers every container-to-host path. A container can address another IP assigned to the host (for example its LAN address); Linux locally delivers that packet to the host even though the destination is not the bridge gateway, so this OUTPUT rule does not match it. Enforce the deny in the host namespace using the veth plus a local-destination match (or enumerate all host addresses), otherwise the default 0.8 posture still exposes host services.
  src/backends/lxc/common/src/state_aware.rs:1043
• The ingress policy is not an enforcement boundary against the sandboxed workload. LXC exec runs as container root with CAP_NET_ADMIN, so contained code can flush the INPUT/OUTPUT rules installed in its own network namespace and restore inbound/host reachability. Move these controls to host-owned rules scoped to the host-side veth, or remove CAP_NET_ADMIN before advertising ingress enforcement.
  sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24
• This public enum value cannot actually provision an LXC sandbox. BuildProvisionEnvelope has no LXC options arm, ValidateProvisionOptions accepts (Lxc, null), and the resulting request omits the required experimental.lxc.provision.distribution/release, so Rust always returns malformed_request; there is also no start-options type for LXC filesystem/network policy. Add the LXC-specific option types and serialization, or do not expose/accept this backend in the managed lifecycle API.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

src/core/wxc_common/src/wire.rs:590

• LXC was added only to the rolling wire model. The closed 0.9.0-alpha contract still omits LXC from its state-aware containment/provision unions, and the dev adapter hard-codes lxc: None; consequently the generated exact schema/types reject an LXC lifecycle request even though the backend accepts 0.9. Per the repository's dual-contract workflow, add the matching exact contract and adapter support and regenerate the exact artifacts.
  sdk/node/src/state-aware-helper.ts:23
• This introduces a backend-specific schema default outside the canonical version source. schemas/schema-version.json has no LXC state-aware entry, and check-schema-versions.js validates only the shared and WSLC defaults, so the new TypeScript and C# constants can drift independently. Add a canonical stateAwareLxc value and extend the drift gate to check both SDKs.

// LXC's network surface is the directional field set, which a pre-0.8 schema
// does not enforce.  On the shared default a policy would be accepted and no
// firewall installed.
export const LXC_STATE_AWARE_VERSION = '0.8.0-alpha';

• Files reviewed: 89/91 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

LXC is incomplete in the exact contract and .NET SDK, while timeout handling and legacy mount migration can produce incorrect or unsafe behavior.

Review details

Suppressed comments (4)

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Adding Lxc to the public state-aware enum exposes a backend that the managed SDK cannot actually provision: there is no LxcProvisionOptions, BuildProvisionEnvelope has no LXC arm, and ValidateProvisionOptions accepts null for LXC. The resulting envelope omits the required experimental.lxc.provision.distribution and release, so every ProvisionSandbox(StateAwareContainment.Lxc) call is rejected. The start API also cannot carry LXC's filesystem/network policy. Please add the LXC-specific provision/start option types and serialization, or do not expose this enum value yet.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

src/core/wxc_common/src/wire.rs:590

• The rolling wire model now accepts experimental.lxc, but the exact 0.9 contract still has only IsolationSession, Windows Sandbox, and WSLC provision variants, and this PR's exact adapter explicitly sets lxc: None. Thus a valid 0.9 LXC lifecycle request is accepted by today's parser but rejected by the future-authoritative exact parser. Update mxc_config_contract::dev, its adapter, and regenerated exact schema/TypeScript oracle together as required by the parser transition.
  src/backends/lxc/common/src/state_aware.rs:1146
• A timeout is converted here into backend_error after attach_run has already streamed workload output. lxc-exec then appends an error envelope to that same stdout, so execInSandboxAsync cannot parse the envelope and returns an ordinary nonzero ExecResult; the new SDK test explicitly characterizes this gap. This also contradicts the lifecycle contract, which says runtime timeouts surface as sentinel exec exit codes rather than typed wire errors. Preserve the timed-out process's sentinel exit status on the executor path instead of returning MxcError after output has begun.
  src/backends/lxc/common/src/filesystem_mounts.rs:215
• The replacement only recognizes mount entries carrying the new marker, but MXC versions before this change appended their own lxc.mount.entry lines without a marker. A preserved/adopted container first configured by an older binary therefore keeps those legacy MXC mounts forever; restarting it with a narrower policy still exposes the old host paths. Please provide a migration/ownership mechanism for pre-marker MXC entries rather than treating every unmarked entry as operator-owned.

    // Derive the whole mount set first, then commit it in one config rewrite.
    // liblxc accumulates `lxc.mount.entry` lines across restarts, so the
    // previous run's MXC mounts have to go; committing the replacement one
    // entry at a time meant a crash or a rejected path partway through left a
    // durable config matching no policy anyone wrote. Only MXC's own entries
    // are replaced -- baseline mounts the distribution template or the operator
    // placed in the config carry no marker and survive.

• Files reviewed: 93/95 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Contract-version, .NET provisioning, timeout signaling, compatibility, and failed-provision cleanup issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 93/95 changed files
• Comments generated: 2
• Review effort level: Balanced

[Soham Das (SohamDas2021)]

This seems to be a legit issue

[Copilot]

🔵 Needs a closer look

Legacy mounts can retain removed filesystem access, and the exact-contract and C# integrations are incomplete.

Review details

Suppressed comments (4)

src/core/wxc_common/src/config_contract_adapters/dev/state_aware.rs:57

• The new LXC state-aware shape exists only in the rolling wire model; this adapter still hard-codes lxc: None, and the closed dev contract has no LXC provision variant. Consequently the generated exact 0.9 schema rejects a request that the current parser accepts, so switching exact dispatch to authority would break this feature. Add LXC to mxc_config_contract::dev and this adapter, then regenerate both exact artifacts as required by docs/versioning.md:204-214.
  sdk/dotnet/Microsoft.Mxc.Sdk/MxcLifecycle.cs:395
• This makes LXC publicly routable, but ValidateProvisionOptions accepts only null for LXC and BuildProvisionEnvelope has no LXC options arm. The resulting envelope therefore omits required experimental.lxc.provision.distribution and release, so every C# LXC provision call fails as malformed_request, with no API capable of supplying the fields. Add an LXC provision-options type, validate it, serialize its fields, and cover the envelope in the C# tests before exposing this enum case.

        StateAwareContainment.Lxc => LxcContainment,

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

• This new backend-specific schema default is not represented in schemas/schema-version.json, and scripts/versioning/check-schema-versions.js validates only the shared and WSLC defaults. The new Node and C# LXC constants can therefore drift silently, contrary to the repository's canonical-version rule. Add a canonical LXC state-aware field and extend the sync gate (or derive this from an existing canonical field).

// LXC refuses any state-aware request below 0.8.
export const LXC_STATE_AWARE_VERSION = '0.8.0-alpha';

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

• This replacement only removes entries preceded by the new # mxc-managed-mount marker. Containers preserved by earlier releases have MXC mounts appended without that marker (the removed code called set_config_item directly), so adopting/restarting one with a tighter policy leaves the old broad mount active alongside the new set. That can preserve filesystem access the caller explicitly removed. Add a safe legacy migration/refusal strategy before treating the marked rewrite as authoritative.

    container.replace_mxc_mount_entries(&entries)

• Files reviewed: 93/95 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Soham Das (SohamDas2021)]

This validator has no LXC arm, and no LxcProvisionOptions type exists, so every typed options object falls through to _ => false?

[Soham Das (SohamDas2021)]

is_some_and returns false when there's no ingress section, so this returns false? This is opposite of the comment above: "An absent 0.8 ingress section still denies."

[Soham Das (SohamDas2021)]

Can the container exit on its own between start and ingress install? If so, kill() will fail because it's already gone, and this branch reads every failure as "still up", returning early and leaving the egress chain installed, which then blocks every later start of that name.

[Soham Das (SohamDas2021)]

This seems to be a legit issue

[Soham Das (SohamDas2021)]

stop takes LifecycleLock for exactly this race (state_aware.rs:1168-1172); the watchdog doesn't. A signal between :982 and :995 removes the firewall, the main thread starts the container anyway, and exit leaves it running unfiltered. NetworkOnly stops rather than destroys, so nothing reclaims it, the base's destroy() did.

Shall we quiesce the transition first?

[Soham Das (SohamDas2021)]

Exiting here drops the process-held flock immediately, but the children this process spawned outlive it?

[Soham Das (SohamDas2021)]

This runs before cleanup_network_owned, so a signal arriving during that iptables sequence finds slot.name == None, skips rollback entirely, and strands whatever chain or FORWARD hook hadn't been removed yet with no owner left to reclaim it?

[Copilot]

🟡 Changes recommended

The C# lifecycle surface cannot provision LXC, legacy mounts can survive narrowed policies, and SDK/network and rejection-reporting gaps remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:23

• Exposing Lxc here makes it selectable through the C# lifecycle API, but the managed surface cannot construct a valid LXC provision request: there is no LXC provision-options type, BuildProvisionEnvelope never emits experimental.lxc.provision, and the native parser requires distribution and release. As a result, every C# LXC provision call fails before a container can be created. Add typed LXC provision options and serialize that block before advertising this backend.

    /// <summary>Linux container managed by LXC.</summary>

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

• This replacement only removes entries carrying the new marker. Containers preserved by earlier MXC versions have the same MXC-generated lxc.mount.entry lines without that marker, so adopting/reusing one after upgrade leaves the old grants active alongside the new policy. Narrowing or removing a mount can therefore still expose the previously granted host path. Please add a migration/ownership strategy for legacy MXC entries (or reject such reuse) before treating the new policy as authoritative.

    container.replace_mxc_mount_entries(&entries)

• Files reviewed: 95/97 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

C# cannot construct a valid LXC provision request, and repeated in-process attached execs can leak stdin readers and consume later input.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

sdk/dotnet/Microsoft.Mxc.Sdk/MxcLifecycle.cs:404

• Adding LXC here makes ProvisionSandbox(StateAwareContainment.Lxc, ...) publicly selectable, but the managed API cannot construct a valid LXC provision request. Native validation requires experimental.lxc.provision.distribution and release; null options emit neither field, and ValidateProvisionOptions rejects every existing non-null option type. Please add an LxcProvisionOptions type, accept it in validation, serialize both required fields into the backend provision block, and cover the resulting envelope in the C# tests.

        StateAwareContainment.Lxc => LxcStateAwareVersion,

• Files reviewed: 95/97 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

LXC is unusable through the newly exposed C# provision API, exact-contract support is incomplete, and IPv6 host-loopback enforcement can fail open.

Review details

Suppressed comments (4)

sdk/dotnet/Microsoft.Mxc.Sdk/MxcLifecycle.cs:395

• Lxc is now advertised as a provisionable C# backend, but BuildProvisionEnvelope has no LXC branch and ValidateProvisionOptions accepts no LXC-specific options. Passing null therefore emits no required experimental.lxc.provision.distribution/release and native validation rejects it; passing any options is rejected in managed code. Add an LxcProvisionOptions model, validation case, serialization branch, and API tests before exposing this enum value.

        StateAwareContainment.Lxc => LxcContainment,

src/core/wxc_common/src/wire.rs:581

• This adds LXC only to the rolling wire model. The exact 0.9.0-alpha contract still has no LXC provision discriminator/variant (mxc_config_contract/src/dev/state_aware/provision), so its generated exact schema and TypeScript oracle reject this new lifecycle surface while the rolling schema accepts it. Mirror the new provision contract and adapter in the exact development model and regenerate both exact artifacts.
  src/backends/lxc/common/src/network_ingress.rs:718
• This enforces hostLoopback: deny only for IPv4. Provision can adopt an existing single-veth container, so the assumption that every supported bridge has no IPv6 route is not guaranteed; an IPv6-enabled bridge leaves container-to-host connectivity open even though the GA contract requires hostLoopback in both directions. Resolve and block the host's IPv6 bridge addresses as well, or reject the start when IPv6 is active and that path cannot be closed.

        // IPv4 only, matching the one route LXC's default bridge gives the
        // container to the host. `lxc-net` ships an IPv4 address and leaves
        // `LXC_IPV6_ADDR` unset, leaving no IPv6 path to close.
        if Self::denies_host_loopback(policy, uses_directional_schema) {
            self.install_host_loopback_drop(&mut runner, logger)?;

src/core/lxc/src/main.rs:332

• This new state-aware parse-failure path writes the envelope but never calls the shared configuration-rejection reporter. config_rejection.rs:4-9,15-21 requires every executor rejection—including pre-dispatch malformed JSON—to emit the common audit/telemetry record, and wxc/src/main.rs:1077-1088 does so for the equivalent branch. As written, malformed LXC lifecycle requests are absent from the audit stream.

• Files reviewed: 95/97 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Ingress remains workload-tamperable, timeout output violates the exec protocol, and both SDK surfaces expose incomplete LXC lifecycle APIs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

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

• A timed-out exec reaches this branch as Err, after its PTY output may already have been streamed to stdout. lxc-exec then appends an error envelope to that same stdout, violating the documented pure-output-or-pure-envelope contract; execInSandboxAsync consequently returns an ordinary nonzero ExecResult and loses the timeout reason. Preserve timeout as an execution outcome/sentinel instead of converting it to MxcError after output has begun.
  sdk/node/src/state-aware-types.ts:119
• This public start type omits the schema-0.8 directional fields that LXC now enforces. Since the helper sends 0.8.0-alpha, callers cannot express network.egress rules or network.ingress/hostLoopback through the typed state-aware SDK even though the backend and added fixtures support them. Include egress and ingress in this honored subset.

/** The `NetworkConfig` fields LXC honors at start. */
export interface LxcNetworkConfig {
  defaultPolicy?: NetworkConfig['defaultPolicy'];
  allowedHosts?: NetworkConfig['allowedHosts'];
  blockedHosts?: NetworkConfig['blockedHosts'];
}

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Adding Lxc here exposes a backend that the .NET lifecycle API cannot provision. ValidateProvisionOptions accepts only null for LXC, while BuildProvisionEnvelope has no LXC case, so every accepted call omits the backend-required distribution and release fields and fails natively. Add an LXC-specific provision options type and serialize it under experimental.lxc.provision before advertising this enum value.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

• Files reviewed: 95/97 changed files
• Comments generated: 1
• Review effort level: Balanced

[Gudge (MGudgin)]

Summary

Requesting changes based on the containment and lifecycle blockers below. I re-fetched d3c94a0, confirmed its merge base is the PR base d130c369, and rechecked every finding against that range. Nineteen findings are attached to added lines; the three claim/scope findings are recorded here because they do not have valid code anchors.

The highest-priority fixes are: make the pre-start veth/firewall selector collision-safe, register provision for signal rollback, and make repeated attached exec reclaim its PTY threads/descriptors without adding a five-second delay to silent commands. The lifecycle lock and timeout-reap paths also need bounded failure behavior.

Findings outside the diff / claim locations

Medium (correctness, proportionality) - The statement “One-shot LXC is untouched” is inaccurate. claim_mismatch: one-shot still requires no experimental flag, but this PR adds malformed-environment, proxy/enforcement-mode, multi-interface, and host-resolution refusals and changes firewall setup/teardown behavior. Please narrow the claim and explicitly document the compatibility-impacting one-shot changes, or split them from this PR.

Medium (proportionality) - “Policy is enforced in both directions rather than outbound only” overstates the delta. claim_mismatch: inbound enforcement existed at the base revision; this PR adds host-loopback denial and return-path ownership behavior. Please describe that narrower change so reviewers and users understand the actual new guarantee.

Low (proportionality) - The shared one-shot firewall path grows beyond the stated lifecycle scope. introduced_by_change: deterministic pinning is lifecycle-related, but multi-interface refusal and hook-survival hardening also alter one-shot behavior. Please call out that blast radius in the PR description and release notes.

Verified clean, with receipts

• Supply chain: Cargo.lock adds no package or version; the manifest changes only add edges to already locked semver and serde_json crates.
• Experimental gating: state-aware LXC passes through require_experimental_optin before backend dispatch; one-shot remains ungated.
• Telemetry/config rejection payloads do not include caller sandbox IDs or raw configuration values.
• The parser explicitly rejects top-level lxc on state-aware requests and rejects experimental.lxc on one-shot requests, so the duplicated configuration locations do not currently have precedence ambiguity.

Verified pre-existing - not attributed to this PR

None retained as findings.

[Gudge (MGudgin)]

High (security) - Make the deterministic veth selector collision-safe.

Attribution: introduced_by_change — this PR derives the pre-start packet selector from caller-controlled containerId using an explicitly non-cryptographic FNV token.

If two accepted IDs map to the same veth name, the second start can install top-inserted FORWARD hooks against the first container's interface; a terminal ACCEPT chain can then preempt the victim's deny chain. Teardown also identifies return rules by that interface name.

Fix: derive the token from the existing SHA-256/base32 machinery and fail closed before installing hooks if the target interface already exists. Add collision/pre-claimed-interface tests.

[Gudge (MGudgin)]

High (reliability) - Register provision for signal rollback.

Attribution: introduced_by_change — provision performs the long-running lxc-create operation without registering an active sandbox with the new signal-cleanup watchdog.

A termination during image download can leave a partially created container. For a minted name, the caller never received the ID and cannot clean it up.

Fix: register a provisioning rollback before create(), destroy newly created state on signal, and clear the registration after completion.

[Gudge (MGudgin)]

High (performance/reliability) - Repeated attached exec leaks per-call PTY resources.

Attribution: newly_exposed_by_change — state-aware LXC makes this PTY path repeatable from long-lived in-process callers, while this branch explicitly abandons the reader thread and descriptor after timeout. The SIGWINCH and stdin forwarders are also not invocation-scoped.

Fix: retain cancellation handles, close duplicated PTY/self-pipe descriptors, stop and join all forwarding threads, and make the reader cancellable instead of abandoning it.

[Gudge (MGudgin)]

High (performance) - Silent state-aware execs wait five seconds before proceeding.

Attribution: newly_exposed_by_change — this new repeated-exec call uses run_with_pty, whose readiness channel is signaled only after the first output byte. A successful silent command reaches EOF without signaling and waits the full default readiness timeout.

Fix: treat EOF/child exit as readiness completion, or skip output-readiness waiting for non-interactive execution. Add a timing regression test using true.

[Gudge (MGudgin)]

Medium (reliability/test coverage) - Bound and integration-test the cross-process lifecycle lock.

Attribution: introduced_by_change — the new lock blocks indefinitely and is held across external create/start/firewall commands. The current exclusion test runs in one process and does not prove the advertised cross-process serialization.

Fix: use nonblocking flock acquisition against an overall deadline with an actionable busy error, and add a live test launching two concurrent lxc-exec start processes against the same ID.

[Gudge (MGudgin)]

Medium (proportionality) - Separate the warn-to-refuse policy change from lifecycle support.

Attribution: introduced_by_change — malformed/unresolvable network and environment cases are converted into hard refusals with a substantial independent test suite. This changes existing one-shot compatibility but is not required for container reuse.

Fix: split the refusal hardening into its own PR, or clearly justify and document the coordinated breaking change here.

[Gudge (MGudgin)]

Low (maintainability) - Share the network-interface eligibility predicate.

Attribution: introduced_by_change — state-aware now independently encodes interface count/type eligibility while one-shot implements the same decision through a differently shaped fallback path.

Fix: extract one shared predicate and keep only the surface-specific fallback behavior separate.

[Gudge (MGudgin)]

Low (maintainability) - Consolidate the active-cleanup setters.

Attribution: introduced_by_change — the three setters repeat the same slot initialization and differ only in rollback mode and marker.

Fix: add a private set_active_with(name, rollback, marker) helper and delegate the public variants to it.

[Gudge (MGudgin)]

Low (test coverage) - Test host-loopback rule teardown semantics.

Attribution: introduced_by_change — this new teardown distinguishes “already absent” from retryable failure, but has no direct test comparable to the chain teardown suite.

Fix: verify absent-rule success clears the flag and real errors retain it for retry.

[Gudge (MGudgin)]

Medium (API surface/maintainability) - Avoid two LXC JSON shapes for identical provisioning fields.

Attribution: introduced_by_change — experimental.lxc.provision duplicates the existing top-level lxc section's distribution and release using a second wire struct/schema definition.

The parser rejects mixing the two, so there is no precedence bug, but raw-config users now need lifecycle-dependent addresses and the definitions can drift.

Fix: preferably accept the canonical top-level lxc section for state-aware provision, using phase plus the experimental opt-in for disambiguation. If nested phase config is required by convention, reuse the same wire/schema type and add parity tests.

[Copilot]

🟡 Changes recommended

The C# surface cannot provision LXC, legacy mounts may survive tightened policies, and several lifecycle error and diagnostic paths are incorrect.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (8)

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Adding Lxc here exposes a public lifecycle option that cannot produce a valid provision request. ValidateProvisionOptions accepts only null for LXC, and BuildProvisionEnvelope has no LXC branch, so the emitted envelope omits required experimental.lxc.provision.distribution and release fields and every ProvisionSandbox(StateAwareContainment.Lxc) call is rejected by the native parser. Please add an LXC-specific provision-options type and serialize those required fields before exposing this enum member.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

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

• Marker-only replacement leaves filesystem grants written by older MXC builds in place. The previous implementation appended unmarked lxc.mount.entry lines, and preserved containers keep that startup config; adopting or reusing one now treats those entries as user baseline and adds the new policy alongside them. A caller tightening/removing a prior mount can therefore retain host access that the current policy did not grant. This needs an explicit migration/rejection strategy for legacy MXC entries before marker-based replacement is safe.

    /// Marker comment written immediately above every `lxc.mount.entry` line
    /// that MXC itself adds, so
    /// [`replace_mxc_mount_entries`](Self::replace_mxc_mount_entries) can
    /// rewrite only MXC's own mounts and leave baseline entries the distro
    /// template or the user placed in the config untouched. It is a real LXC
    /// comment (`#`), so liblxc ignores it when parsing the file.
    const MXC_MOUNT_MARKER: &'static str = "# mxc-managed-mount";

src/core/lxc/src/main.rs:332

• This state-aware parse-error branch bypasses log_config_rejected, unlike the corresponding wxc-exec branch. Malformed lifecycle requests handled here therefore produce neither the audit event nor the telemetry rejection event introduced by this PR, despite the shared rejection module's executor-wide contract. Route ParseError::StateAware through the rejection classifier/logger before emitting the envelope.
  src/backends/lxc/common/src/state_aware.rs:1183
• This stop-phase logger drops the diagnostic sink installed by the shared state-aware wrapper, so firewall-cleanup diagnostics never reach --log-file or the audit pipe. Backend-local loggers on this path must inherit the thread sink.
  src/backends/lxc/common/src/state_aware.rs:1221
• This deprovision-phase logger drops the diagnostic sink installed by the shared state-aware wrapper, so authoritative firewall-cleanup diagnostics never reach --log-file or the audit pipe. Backend-local loggers on this path must inherit the thread sink.
  src/backends/lxc/common/src/state_aware.rs:519
• This branch reports any firewall setup failure after partial resource creation as invalid caller policy, although it is an operational iptables failure and policy validation has already completed. Typed clients receive a non-retryable policy_validation code and telemetry records UnsupportedField; return backend_error instead.
  src/backends/lxc/common/src/state_aware.rs:531
• An existing/stale firewall chain is lifecycle state, not a malformed policy. Classifying this collision as policy_validation causes typed clients and rejection telemetry to blame the request even though the remediation in the message is to stop/deprovision and retry. Return a backend/state error code instead.
  src/backends/lxc/common/src/state_aware.rs:950
• This provision-phase logger drops the diagnostic sink installed by the shared state-aware wrapper, so container-creation diagnostics never reach --log-file or the audit pipe. Backend-local loggers on this path must inherit the thread sink.

• Files reviewed: 95/97 changed files
• Comments generated: 5
• Review effort level: Balanced

[Darren Hoehna (dhoehna)]

Handoff — Darren is on vacation

Everything on his workstation is pushed. This comment is the map, written for
someone who was not here.

Where the work is

Every branch below is on a remote as of 2026-09-11. Nothing is stranded on the
workstation.

Branch	Repo	State
user/dahoehna/lxc-lifecycle-current	dhoehna/mxc	This PR. Built, tested, green — SDK unit suite 398 tests, 379 pass, 0 fail, 19 skipped; tsc clean.
user/dahoehna/lxc-reject-malformed-config	microsoft/mxc	Two commits pushed today: refuse network and environment settings LXC cannot enforce, plus the document correction. Not built here.
user/dahoehna/wb-ratelimit	microsoft/mxc	63 commits from 2026-08-13 on crates.io / ESRP publishing that existed on no remote at all until today. Also on dhoehna/mxc as wb-ratelimit.
user/dahoehna/network-ga-schema	microsoft/mxc	WIP, unverified. 525 insertions across 6 files, committed and pushed unbuilt so it would not be stranded.
user/dahoehna/wip-total-denial-withhold-interface	microsoft/mxc	Rescued. Commit 75f918f3 was on a detached HEAD and would have been garbage-collected. Enforces total-denial by withholding the interface.
wb-ratelimit	dhoehna/mxc	63 commits from 2026-08-13 on crates.io / ESRP publishing that existed on no remote at all until today.
user/dahoehna/wip-skilltest-run4	microsoft/mxc	WIP, unverified. 335 insertions from a scratch tree; may be a discarded experiment.
user/dahoehna/wip-aitest-lxc-runner	microsoft/mxc	WIP, unverified. Was uncommitted on a local main; removes 1402 lines and adds 413. Deliberately not pushed to main.

Every branch above is on microsoft/mxc except this PR's own branch, which is
on dhoehna/mxc where it has always been.

The four wip-* branches and network-ga-schema were committed without being
built or tested. Their commit messages say so. Treat them as preserved
material, not as proposed changes.

Two worktrees, mxc-inbound and mxc-inbound-deny, could not be pushed and
are not recoverable by anyone but Darren. Their parent clone was deleted, so
no git metadata survives to say which commit they sat on. Their trees were
compared against every plausible base branch; the closest still differs by 99
and 107 files, which is branch drift rather than a delta, so no base can be
identified and no meaningful change can be extracted. Their source is archived
on his workstation only, at C:\git\_orphaned-worktrees-archive\. If something
in the inbound work turns out to be missing, it has to wait for him.

Two things that need a decision, not a keystroke

1. An egress regression in PR #1041, which nobody has raised on that PR.

Defending the first review finding on this PR, the argument was that egress is
safe because its chains sit on the host, out of the workload's reach. PR #1041
makes that false. It hooks egress into OUTPUT inside the container network
namespace (network_iptables.rs:1955-1971, entered by nsenter -t <pid> -n at
:1363-1370). That is the same namespace a local Alpine experiment flushed
successfully, with CAP_NET_ADMIN present in the container's effective set. A
workload that can flush its own egress chain is not filtered by it.

This is the highest-value open item and it belongs on #1041, not here.

2. Which of #849 and #1041 lands first.

Nothing in either branch records the intended order, and three review findings
read differently depending on it. This needs an owner's decision.

The review threads — do not bulk-resolve

33 human review comments are unresolved. A pass was run to find which were made
moot by #1041. It proposed seven closures. An independent review overturned
all seven, and the conclusion is that zero threads should be resolved.

The reason is worth stating, because it will come up again: authorship decides
mootness, not absence. This PR creates most of the code under discussion —
state_aware.rs does not exist on the merge base and does not exist on #1041
either. Code that #849 adds and #1041 merely lacks survives reconciliation.
Four of the five proposed closures were argued from a gap #1041 never had
anything to lose in.

Two findings have partial fixes already on the branch (805afe23, 899de231),
but each original comment asked for more than shipped — a regression test in one
case, and one of three parts declined on the record in the other. They can be
replied to. They should not be closed.

Still open, and unstarted

• Six review threads from 2026-09-04 have no reply yet, plus an exact-contract
  question about 0.9.0-alpha.
• A behavior change from the deleted telemetry in merge 89fd4e29 was never
  posted as a comment.
• The C# SDK posture question is unresolved.

[Copilot]

🔵 Needs a closer look

The exact contract and SDK surfaces remain incomplete, and timeout and parse-rejection paths violate advertised lifecycle behavior.

Review details

Suppressed comments (5)

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

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

• This type omits the schema-0.8 egress/ingress fields even though LXC defaults lifecycle requests to 0.8 and the backend implements those policies. It also omits enforcementMode, so callers overriding the request to 0.7 cannot request the firewall behavior described by this PR. The public typed API therefore cannot express supported LXC network policies.
  sdk/node/src/state-aware.ts:145
• This typed-error guarantee does not hold for the new LXC timeout path. LXC may stream workload bytes to stdout before returning its timeout error; the executor appends the error envelope to that same stream, and tryParseErrorEnvelope(stdout) then fails whole-string parsing and resolves an ordinary ExecResult. The newly added unit test explicitly demonstrates this loss of the MxcError. Preserve a separately framed/channelled dispatch result (or otherwise parse it unambiguously) so timeout remains a typed dispatch failure.
  src/core/wxc_common/src/config_contract_adapters/dev/state_aware.rs:54
• The exact 0.9 contract still has no LXC containment or provision variant: mxc_config_contract::dev::state_aware::provision::Containment::parse_exact rejects "lxc", while these adapter additions only initialize the new rolling-wire field to None. This leaves the new backend absent from exact parsing/codegen and will break when exact dispatch becomes authoritative. Add the LXC phase contracts and conversions alongside the rolling model.

sdk/dotnet/Microsoft.Mxc.Sdk/StateAwareTypes.cs:24

• Exposing Lxc here makes the C# API advertise a backend that it cannot provision. ValidateProvisionOptions accepts only null for LXC, but BuildProvisionEnvelope then emits no experimental.lxc.provision.distribution or release, so native validation always rejects the request; every non-null options subtype is rejected too. Add an LXC-specific provision options type and serialize its required fields (and start-time policy options), or do not expose this enum member yet.

    /// <summary>Linux container managed by LXC.</summary>
    Lxc,

src/core/lxc/src/main.rs:332

• State-aware parse failures exit here before run_state_aware_with_telemetry, so they never call log_config_rejected. The Windows executor records the corresponding ParseError::StateAware refusal, and the shared telemetry path only covers errors after successful parsing. This makes malformed LXC lifecycle requests disappear from the configuration-rejection audit stream; classify and log the rejection before emitting the envelope.

• Files reviewed: 96/98 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Soham Das (SohamDas2021)]

We have decided to postpone this to >1.0, the work-item is being tracked in ADO