Skip to content

feat(engine): Rust host-backend detectors + available_backends() probe - #725

Merged
Huzaifa Danish (huzaifa-d) merged 22 commits into
mainfrom
user/modanish/port-host-detectors-to-rust
Aug 7, 2026
Merged

feat(engine): Rust host-backend detectors + available_backends() probe#725
Huzaifa Danish (huzaifa-d) merged 22 commits into
mainfrom
user/modanish/port-host-detectors-to-rust

Conversation

@huzaifa-d

@huzaifa-d Huzaifa Danish (huzaifa-d) commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Ports LXC, Windows Sandbox, and IsolationSession host-availability detection into Rust and adds a read-only available_backends() probe exposing host-runnable backends and their isolation tier.

State of the world — before / after

Before: host/backend discovery lived only in the TypeScript SDK (getPlatformSupport). Rust callers and the executor binaries had no in-process way to ask "what backends can this host run?", and could only learn a backend was unusable by attempting to launch it.

After: the engine answers both questions in-process — no TypeScript dependency, no trial spawn:

  • platform_support() keeps its contract: the backends this SDK can launch (Seatbelt / Bubblewrap / ProcessContainer / WSLC).
  • available_backends() is a new host-capability probe: every backend the host can run — including executor-only ones (Windows Sandbox, IsolationSession, LXC) — each with its effective isolation tier.

Usage

use mxc_sdk::{available_backends, platform_support};

// Will run()/spawn_sandbox() work here, and with which backends?
let support = platform_support();

// What can the host run at all, and at what isolation-tier ceiling?
for b in available_backends() {
    match b.tier {
        Some(tier) => println!("{} (tier: {tier})", b.backend),
        None => println!("{}", b.backend),
    }
}

The tier is a ceiling (strongest reachable isolation; a policy may force weaker at dispatch), and a backend appearing in available_backends() is a host-capability signal — not a guarantee this SDK can launch it (cross-check platform_support()). Full guidance in src/core/mxc-sdk/README.md.

Changes:

  • Adds LXC, Windows Sandbox, and IsolationSession availability probes.
  • Adds the available_backends() API reporting each host-available backend with its effective isolation tier; keeps platform_support() limited to the SDK-launchable subset.
  • Centralizes isolation-tier names on IsolationTier.
  • Adds drift-guard tests and updates SDK tests.

Follow-ups (out of scope):
microvm / hyperlight detectors and the side-effect-free TypeScript-projection transport.
A CI gate ensuring every tier/backend stays probe-covered is tracked in #769.

🔗 References

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

Port the two TypeScript-only host-availability detectors into Rust and fold their results into mxc_engine::platform::platform_support(), broadening its contract from 'backends mxc-sdk can launch' to 'host-available backends' (Phase 1 of the backend-support-probe plan, PR #717).

- Add lxc_common::availability::is_lxc_available() (shallow 'lxc-ls --version' probe).
- Add windows_sandbox_lifecycle::availability::is_windows_sandbox_available() (DISM State:Enabled with a 10s timeout, WindowsSandbox.exe fallback when DISM can't run; invokes the absolute System32\dism.exe path).
- platform_support(): Linux arm reports lxc and/or bubblewrap; Windows arm adds windows_sandbox alongside processcontainer. Update doc comments to host-capability wording.
- Add drift-guard tests tying reported literals to Containment wire names and asserting the live platform_support() output only contains real wire names.
- Loosen the locked mxc-sdk platform_support tests for the broadened contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings July 31, 2026 21:33
@azure-pipelines

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

This comment was marked as resolved.

Add mxc_engine::probe with the read-only available_backends() API from the backend-support-probe plan (PR #717, Phase 2). Reports only the containment backends the current host can run, each with its effective isolation tier when it has a tier ladder.

- AvailableBackend { backend, tier: Option<String> } serializes to camelCase JSON with tier omitted (never null) when the backend has no tier ladder.
- available_backends() has per-platform arms reusing the landed detectors: macOS -> seatbelt; Linux -> bubblewrap/lxc; Windows -> processcontainer (with effective tier) + windows_sandbox + wslc (feature-gated). Empty Vec is a normal result, not an error.
- select_tier() is a pure precedence fn (base-container -> appcontainer-bfs -> appcontainer-dacl floor), unit-testable without a real host or the tier2_bfs feature.
- Re-exported from mxc_engine and the public mxc-sdk.
- 8 unit tests: serde shape (tier omitted vs present), host + unconditional wire-name drift guards, canonical-tier drift guard, Windows processcontainer-always-with-tier, tier precedence, non-Windows processcontainer absence.

Stacked on the host-detector port (PR #725); the standalone detectors it reuses land there.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Resolve dism.exe / WindowsSandbox.exe via GetSystemDirectoryW instead of the attacker-controllable %SystemRoot% env var (UAC inherits an unelevated parent's environment, so an env-derived path let a standard user point the probe at a planted binary). Mirrors the src/host/plm/src/wpr_path.rs pattern; falls back to the C:\\Windows\\System32 literal only on an outright Win32 failure.
- Pass DISM's /English global option so the parsed 'State : Enabled' tokens are not localized on non-English Windows (previously an enabled Sandbox could be reported unavailable there, since a successful DISM run also skips the exe fallback).
- Soften platform_support() docs: available_methods is the currently detected subset, not an exhaustive capability list — backends without a Rust detector yet (notably isolation_session) are omitted even when the host could run them, so absence is not proof a backend cannot run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 18:25

This comment was marked as outdated.

Trim the module-level narratives and over-explained item docs across the host detectors and probe to concise, necessary comments; remove duplicated phrasing (e.g. 'universal floor') within and across files.

Also drop the DISM query from the Windows Sandbox probe: dism /online requires elevation and this probe only ever runs unelevated (wxc-exec does not self-elevate), so DISM always failed through to the WindowsSandbox.exe existence check anyway. Detection is now exe-only, which also removes the subprocess-launch attack surface (only a .exists() remains). A short comment records why DISM is skipped.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 22:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (4)

src/core/mxc_engine/src/probe.rs:32

  • tierless is compiled out on every unsupported target, but the unconditionally compiled tests below call it. Consequently, cargo test fails to compile on the same “other platform” targets that available_backends() explicitly handles with an empty result. Keep this helper available in test builds.
    #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))]

src/backends/windows_sandbox/lifecycle/src/availability.rs:13

  • This removes the authoritative DISM check promised by the PR and required by the linked probe design, which notes that executable presence alone can report Sandbox available while the optional feature is off. The premise that this code only runs unelevated is also incorrect because both platform_support() and available_backends() are public in-process SDK APIs and may be called by an elevated host. Restore the bounded /English DISM feature-state query, using this executable check only when DISM cannot run.
//! We deliberately skip the SDK's authoritative DISM query: `dism /online`
//! requires elevation, and this probe only ever runs unelevated (via `wxc-exec`,
//! which does not self-elevate), so DISM would always fail through to this same
//! executable check.

src/core/mxc_engine/src/probe.rs:10

  • This documentation restores the old contract even though platform_support() is broadened in this same change to report host-detected LXC and Windows Sandbox, which the Rust SDK cannot launch. Describe the actual distinction—tier metadata and additional probe coverage—rather than calling platform_support() the SDK-launchable subset.

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

//! Separate from [`platform_support`](crate::platform_support), which answers the
//! narrower "what can `mxc-sdk` itself launch?" question and reports no tier.

src/core/mxc-sdk/src/lib.rs:87

  • This adds a new public Rust SDK API, but src/core/mxc-sdk/README.md still documents only platform_support() and does not mention available_backends(), its tier semantics, or the normal empty result. Add the new API to the SDK README so consumers can distinguish these two probes.
    available_backends, available_tools_policy, build_request, platform_support,
    temporary_files_policy, user_profile_policy, AvailableBackend, Error, ErrorCode,
    FilesystemPolicyResult, PlatformSupport, SandboxPolicy, SandboxRequest,
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@huzaifa-d Huzaifa Danish (huzaifa-d) changed the title feat(engine): port TS host detectors (lxc, windows_sandbox) to Rust feat(engine): Rust host-backend detectors + available_backends() probe Aug 5, 2026
@huzaifa-d
Huzaifa Danish (huzaifa-d) requested a balanced review from Copilot August 5, 2026 22:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

src/core/mxc_engine/src/probe.rs:10

  • This public contract is still internally inconsistent with this PR: deferred detectors such as isolation_session can be omitted even when usable, so absence cannot mean “not currently usable,” and platform_support() has now also been broadened to a host-capability signal rather than the narrower SDK-launchable subset. Document this as the set this probe can currently affirm and distinguish the APIs by tier reporting.
//! Reports only the containment backends the current host can run, each with
//! its effective isolation tier when it has a tier ladder. Answers "what can I
//! use here?"; a backend's absence means "not currently usable, for any reason".
//! Separate from [`platform_support`](crate::platform_support), which answers the
//! narrower "what can `mxc-sdk` itself launch?" question and reports no tier.

src/backends/windows_sandbox/lifecycle/src/availability.rs:13

  • The “only ever runs unelevated” rationale is no longer true because this detector is reachable through the public in-process mxc-sdk APIs, whose caller may already be elevated. Keep the valid reason for avoiding DISM without promising a privilege level the library cannot control.
//! We deliberately skip the SDK's authoritative DISM query: `dism /online`
//! requires elevation, and this probe only ever runs unelevated (via `wxc-exec`,
//! which does not self-elevate), so DISM would always fail through to this same
//! executable check.

src/core/mxc-sdk/src/lib.rs:87

  • This adds a public Rust SDK API, but src/core/mxc-sdk/README.md:51-52 still documents only platform_support() as the backend probe and does not explain the new tier-bearing API or how the two signals differ. Update the SDK README so consumers can discover available_backends() and understand its best-effort semantics.
    available_backends, available_tools_policy, build_request, platform_support,
    temporary_files_policy, user_profile_policy, AvailableBackend, Error, ErrorCode,
    FilesystemPolicyResult, PlatformSupport, SandboxPolicy, SandboxRequest,
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Address @bbonaby's review note (#717): the tier-name strings were hand-written across the appcontainer fallback code. Make IsolationTier the single source of truth in both directions.

- Add IsolationTier::ALL (canonical tier set, strongest-first) and derive a FromStr impl from as_str() via ALL, so the two directions cannot drift and adding a tier is a one-line change to ALL + as_str.
- Remove the ad-hoc test-only parse_force_tier(); the production MXC_FORCE_TIER seam now parses via FromStr.
- Add typed ForceTierGuard::set_tier(IsolationTier) and migrate all 18 valid force-tier call-sites off raw string literals (the one negative test intentionally keeps a raw invalid value).
- Add a round-trip test asserting every ALL tier survives as_str -> FromStr.

The available_backends() probe (already merged here) consumes as_str() with its own drift guard; the CI coverage gate for new tiers/backends is tracked in the follow-up issue.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 16:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

src/core/mxc_engine/src/probe.rs:10

  • These module docs contradict the new contract in platform.rs:18-22 and the implementation: isolation_session, microvm, and hyperlight can be usable but are omitted until detectors exist, so absence is not proof of unusability. platform_support() is also broadened by this PR to a host-capability signal rather than the narrower SDK-launchable subset. Please describe this as the currently affirmed subset and distinguish the APIs only by tier reporting.
//! Reports only the containment backends the current host can run, each with
//! its effective isolation tier when it has a tier ladder. Answers "what can I
//! use here?"; a backend's absence means "not currently usable, for any reason".
//! Separate from [`platform_support`](crate::platform_support), which answers the
//! narrower "what can `mxc-sdk` itself launch?" question and reports no tier.

src/backends/windows_sandbox/lifecycle/src/availability.rs:22

  • The execution path still uses a second, divergent check: one_shot.rs:48,297-306 calls check_sandbox_available(), which resolves WindowsSandbox.exe from attacker-controlled %SystemRoot%, while this probe uses GetSystemDirectoryW. Consequently the public probe can affirm Sandbox while an actual run rejects it (or the old check can be spoofed). Reuse this detector in the runner and remove the private duplicate so probing and dispatch share the hardened source of truth.
pub fn is_windows_sandbox_available() -> bool {
    system_directory().join("WindowsSandbox.exe").exists()

src/core/mxc-sdk/src/lib.rs:87

  • This adds a public Rust SDK API without updating src/core/mxc-sdk/README.md, whose platform-discovery section currently mentions only platform_support() and whose supported-backends table distinguishes what this SDK can launch. Document available_backends() (including tier/omission semantics) and clarify that platform_support().available_methods is now broader than SDK-launchable backends; otherwise consumers can reasonably try an advertised LXC or Windows Sandbox backend through an unsupported SDK execution path.
    available_backends, available_tools_policy, build_request, platform_support,
    temporary_files_policy, user_profile_policy, AvailableBackend, Error, ErrorCode,
    FilesystemPolicyResult, PlatformSupport, SandboxPolicy, SandboxRequest,
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Add isolation_session to the host-availability surfaces using @adpa-ms's registration-based approach from #761: availability is whether the in-proc Windows.AI.IsolationSession IsoSessionOps WinRT class is registered on the OS (its activation factory resolves), not a build-number gate.

- New isolation_session_common::availability::is_isolation_session_available(): attempts IsoSessionOps activation (CoInitialize MTA, balanced), OnceLock-cached, elevation-free. Pure available_from() split from the COM probe for unit testing (CLASS_E_CLASSNOTAVAILABLE / REGDB_E_CLASSNOTREG and any other activation failure map to unavailable).
- Wire it into both Windows host-capability surfaces, gated behind the engine's isolation_session feature: available_backends() (probe) and platform_support(). This closes the previously-documented Rust/TS parity gap where TS reported isolation_session but Rust did not.
- Add isolation_session to the probe's EMITTABLE_BACKENDS drift guard and loosen the mxc-sdk Windows platform_support test to allow it.

Validated with the feature on and off: fmt clean, clippy -D warnings clean (default, isolation_session, and wslc+tier2_bfs+isolation_session), and tests green.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 17:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (5)

src/core/mxc-sdk/src/lib.rs:87

  • This public re-export is built against mxc_engine with default features because mxc-sdk defines no forwarding features. A normal mxc-sdk consumer therefore cannot enable the probe's wslc, isolation_session, or tier2_bfs branches, so this entry point underreports the host and cannot expose all of the functionality being re-exported. Add corresponding mxc-sdk feature forwarding (as the executor crates already do).
    available_backends, available_tools_policy, build_request, platform_support,
    temporary_files_policy, user_profile_policy, AvailableBackend, Error, ErrorCode,
    FilesystemPolicyResult, PlatformSupport, SandboxPolicy, SandboxRequest,

src/core/mxc_engine/src/probe.rs:119

  • The PR description explicitly lists the isolation_session detector as a later-phase, out-of-scope item, but this block enables that detector in the new public result. Either remove the detector and its wiring from this PR or update the stated scope/phase and validation to include it.
    // Available when the `IsoSessionOps` WinRT class is registered on the OS.
    #[cfg(feature = "isolation_session")]
    if isolation_session_common::availability::is_isolation_session_available() {
        backends.push(AvailableBackend::tierless("isolation_session"));

src/core/mxc_engine/src/probe.rs:10

  • This is no longer true: platform_support() now reports LXC even though mxc-sdk cannot launch it, and platform.rs documents that API as broader than the SDK-launchable subset. Keeping both opposite public descriptions makes it unclear which contract callers may rely on.
//! Separate from [`platform_support`](crate::platform_support), which answers the
//! narrower "what can `mxc-sdk` itself launch?" question and reports no tier.

src/backends/lxc/common/src/availability.rs:12

  • The exit-code payload is never read; the decision only distinguishes the variant. Rust's dead_code lint reports an unread private enum field, and the repository's clippy -D warnings gate promotes that warning to a build failure. Make this a unit variant and update its constructors/tests, or actually consume the code.
    ExitedFailure(Option<i32>),

src/backends/windows_sandbox/lifecycle/src/availability.rs:22

  • The linked probe design explicitly notes that executable presence alone can report Windows Sandbox available while the optional feature is off. That violates this API's “currently runnable” contract: available_backends() can return windows_sandbox and a subsequent launch will fail. Use an unelevated authoritative feature-state check, or omit this backend until one is available.
pub fn is_windows_sandbox_available() -> bool {
    system_directory().join("WindowsSandbox.exe").exists()
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/core/mxc_engine/src/probe.rs Outdated
…st-detectors-to-rust

# Conflicts:
#	src/core/mxc-sdk/src/lib.rs
#	src/core/mxc-sdk/tests/sdk_helpers.rs
#	src/core/mxc_engine/src/platform.rs
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Author-Feedback Issue needs attention from issue or PR author label Aug 7, 2026
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Attention Issue needs attention from Microsoft label Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (5)

src/core/mxc-sdk/tests/sdk_helpers.rs:229

  • The comment and allow-list contradict the corrected platform_support() contract: platform.rs:80-82 explicitly excludes Windows Sandbox and IsolationSession because the SDK cannot launch them. Allowing those values removes regression coverage for the separation from available_backends(); only processcontainer and feature-gated wslc should be accepted here.
    // `windows_sandbox`, `isolation_session` (with the feature), and `wslc` may
    // also appear, so the exact vector is host-dependent.
    for method in &support.available_methods {
        assert!(
            matches!(
                method.as_str(),
                "processcontainer" | "windows_sandbox" | "isolation_session" | "wslc"
            ),

src/core/mxc_engine/src/probe.rs:113

  • This bfscfg.exe gate under-reports the policy-independent tier ceiling. The linked #717 design defines appcontainer-bfs as reachable whenever tier2_bfs is compiled, and fallback_detector::detect likewise selects BFS for a request with no filesystem rules without resolving bfscfg.exe (fallback_detector.rs:248-258). On a BFS-enabled build without that executable, this probe therefore reports appcontainer-dacl even though BFS is reachable. Select the ceiling from the feature alone and leave bfscfg availability to request-level tier selection.
    let tier = select_tier(
        is_base_container_usable(),
        cfg!(feature = "tier2_bfs"),
        bfscfg_available(),
    );

src/backends/lxc/common/src/availability.rs:66

  • If try_wait() fails after the child was spawned, this returns without terminating or reaping it. Dropping std::process::Child does not perform that cleanup, so an availability probe can leave lxc-ls running or later becoming a zombie. Apply the same best-effort kill-and-wait cleanup used by the timeout path before returning the failure outcome.
            Err(_) => return LxcLsOutcome::SpawnFailed,

src/core/mxc-sdk/tests/sdk_helpers.rs:208

  • This test now accepts lxc, even though platform_support() intentionally reports only SDK-launchable Linux backends and platform.rs:61-67 restricts that list to bubblewrap. As written, it would no longer catch the exact contract regression this split is meant to prevent; keep the assertion bubblewrap-only.

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

fn platform_support_linux_methods_are_lxc_or_bubblewrap() {
    let support = platform_support();
    for method in &support.available_methods {
        assert!(
            method == "lxc" || method == "bubblewrap",
            "unexpected Linux method: {method}"
        );

src/core/mxc-sdk/src/lib.rs:117

  • This adds a public Rust SDK API, but src/core/mxc-sdk/README.md:56-57 still tells users that platform_support() reports the available containment backends and does not mention available_backends(). Update the README to document the new probe and clarify that platform_support() is limited to backends the SDK can launch.
    available_backends, available_tools_policy, build_request, build_request_with_containment,
    platform_support, temporary_files_policy, user_profile_policy, AvailableBackend, Containment,
  • Files reviewed: 20/20 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Documents platform_support() vs available_backends() (which to use when), a
usage example, and the tier-ceiling caveat, plus the before/after framing.
Addresses jsidewhite review on #725.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (6)

src/core/mxc-sdk/tests/sdk_helpers.rs:229

  • This assertion now permits windows_sandbox and isolation_session, contradicting the restored platform_support() contract: those are executor-only and belong exclusively to available_backends(). Keeping them here means the SDK test would not catch the exact regression this PR just fixed.
            matches!(
                method.as_str(),
                "processcontainer" | "windows_sandbox" | "isolation_session" | "wslc"
            ),

src/core/mxc-sdk/README.md:102

  • This still calls the result the “full” host-capability set even though MicroVM and Hyperlight detector work is explicitly deferred. That makes the before/after guidance contradict the actual probe coverage; call it the currently detected set instead.
> subset and [`available_backends`] for the full host-capability set with tiers —

src/core/mxc_engine/src/probe.rs:8

  • The API contract treats absence as proof that a backend is unusable, but this probe has no MicroVM or Hyperlight detector even though the PR explicitly leaves those detectors for follow-up. A capable host will therefore omit them. Document this as the currently detected subset and state that absence is not conclusive until detector coverage is complete.
//! Reports only the containment backends the current host can run, each with
//! its effective isolation tier when it has a tier ladder. Answers "what can I
//! use here?"; a backend's absence means "not currently usable, for any reason".

src/core/mxc-sdk/tests/sdk_helpers.rs:206

  • This weakens the SDK-launchability regression guard by allowing lxc, although platform_support() intentionally cannot report LXC and the SDK dispatcher cannot launch it. Restore the Bubblewrap-only assertion so a future accidental reintroduction of LXC fails the test.

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

fn platform_support_linux_methods_are_lxc_or_bubblewrap() {
    let support = platform_support();
    for method in &support.available_methods {
        assert!(
            method == "lxc" || method == "bubblewrap",

src/backends/lxc/common/src/availability.rs:66

  • If try_wait() errors while the child is still alive, this branch drops the handle without killing or reaping it, contrary to this function's no-leak guarantee. Apply the same cleanup used by the timeout branch before returning the failed outcome.
            Err(_) => return LxcLsOutcome::SpawnFailed,

src/core/mxc-sdk/README.md:70

  • “Every containment backend” overstates this release: the PR leaves MicroVM and Hyperlight detectors out of scope, so either may be runnable while absent from this result. Please describe this as the currently detected capability subset so callers do not use absence as a negative capability signal.

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

- [`available_backends`] — a broader **host-capability** probe. Reports every
  containment backend the *host* can run, including ones only the executor
  • Files reviewed: 21/21 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/backends/lxc/common/src/availability.rs Outdated
The decision only distinguishes success from every other outcome, so the
Option<i32> exit code was never read (dead code under -D warnings). Make
ExitedFailure a unit variant. Addresses Copilot review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (5)

src/backends/windows_sandbox/lifecycle/src/availability.rs:10

  • This executable-only check does not establish that the optional feature is enabled. The linked design explicitly identifies this check as a false-positive when the feature is off, causing available_backends() to advertise a backend that cannot launch. Preserve a real feature-state check (for example, an unelevated-readable registry probe, or DISM where usable) instead of treating file presence as authoritative.
//! Detects availability by the presence of `WindowsSandbox.exe`, which Windows
//! installs only when the `Containers-DisposableClientVM` feature is enabled. We
//! skip the SDK's DISM query: `dism /online` needs elevation and this probe only
//! runs unelevated, so it would always fall through to this same exe check.

src/core/mxc-sdk/tests/sdk_helpers.rs:209

  • This weakens the contract test to permit lxc, although platform_support() must remain limited to SDK-launchable backends and Linux can only report bubblewrap (the linked design explicitly locks this down). As written, a regression that leaks LXC back into platform_support() would pass.
fn platform_support_linux_methods_are_lxc_or_bubblewrap() {
    let support = platform_support();
    for method in &support.available_methods {
        assert!(
            method == "lxc" || method == "bubblewrap",
            "unexpected Linux method: {method}"
        );
    }

src/core/mxc-sdk/tests/sdk_helpers.rs:229

  • These two host-capability backends are deliberately excluded from platform_support() because the SDK cannot stream them. Allowing them here means this test no longer catches the exact contract regression that available_backends() was introduced to avoid. Keep the accepted set to processcontainer and feature-gated wslc.
    // `windows_sandbox`, `isolation_session` (with the feature), and `wslc` may
    // also appear, so the exact vector is host-dependent.
    for method in &support.available_methods {
        assert!(
            matches!(
                method.as_str(),
                "processcontainer" | "windows_sandbox" | "isolation_session" | "wslc"
            ),

src/core/mxc-sdk/src/lib.rs:119

  • This public re-export cannot provide the documented IsolationSession result in a normal mxc-sdk build: the engine gates that detector behind feature = "isolation_session", but mxc-sdk exposes only a wslc forwarding feature. Consequently mxc_sdk::available_backends() always omits IsolationSession even on a capable host. Add an SDK feature forwarding mxc_engine/isolation_session, or separate the detector from the launch feature so host discovery remains available.
    available_backends, available_tools_policy, build_request, build_request_with_containment,
    platform_support, temporary_files_policy, user_profile_policy, AvailableBackend, Containment,
    Error, ErrorCode, FilesystemPolicyResult, PlatformSupport, SandboxPolicy, SandboxRequest,
    WslcSection,

src/backends/lxc/common/src/availability.rs:68

  • If try_wait() errors while the child is still alive, this branch drops the Child without killing or reaping it, contradicting the bounded/no-zombie contract above. Apply the same best-effort kill-and-wait cleanup used by the timeout branch before returning.
            Err(_) => return LxcLsOutcome::SpawnFailed,
  • Files reviewed: 21/21 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/core/mxc_engine/src/probe.rs
….exe

Reverts the bfscfg.exe gating added earlier in this PR. The probe's tier is a
CEILING (strongest reachable for some request), not a per-request value:
detect() returns AppContainerBfs for a no-filesystem-policy request without
bfscfg.exe, so the ceiling on any tier2_bfs build is BFS regardless of bfscfg.
Gating the ceiling on bfscfg under-reported it to appcontainer-dacl on a
tier2_bfs host lacking bfscfg. bfscfg only decides whether a policy-carrying
request stays at BFS or drops to DACL, which is a request-time dispatch concern.
Addresses Copilot review; supersedes the earlier response to the bfscfg comment.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 21:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (7)

src/core/mxc-sdk/tests/sdk_helpers.rs:226

  • This assertion incorrectly allows the executor-only Windows Sandbox and IsolationSession values even though platform_support() intentionally never emits them. That weakens the regression guard for the SDK-launchability contract; only ProcessContainer and feature-gated WSLC should be accepted here.
    // `windows_sandbox`, `isolation_session` (with the feature), and `wslc` may
    // also appear, so the exact vector is host-dependent.
    for method in &support.available_methods {
        assert!(
            matches!(

src/core/mxc_engine/src/probe.rs:10

  • The absence contract is too strong: MicroVM and Hyperlight are intentionally not emitted yet, and feature-gated detectors can be compiled out, so a missing backend does not prove that the host cannot run it. Describe this as the subset this build's implemented detectors can affirm; otherwise consumers may reject a runnable backend.
//! Reports only the containment backends the current host can run, each with
//! its effective isolation tier when it has a tier ladder. Answers "what can I
//! use here?"; a backend's absence means "not currently usable, for any reason".

src/core/mxc-sdk/README.md:70

  • This user-facing claim is not true for the re-export shown below. The probe omits MicroVM/Hyperlight, and an ordinary mxc-sdk build cannot report IsolationSession because that crate does not forward mxc_engine/isolation_session. Qualify the result as the compiled, currently detected subset so callers do not interpret absence as a host limitation.
- [`available_backends`] — a broader **host-capability** probe. Reports every
  containment backend the *host* can run, including ones only the executor
  binaries (`wxc-exec` etc.) can currently drive — Windows Sandbox,
  IsolationSession, LXC — each with its effective isolation **tier** (for the
  Windows ProcessContainer ladder). Use it for capability discovery, not as a

src/core/mxc-sdk/tests/sdk_helpers.rs:206

  • This now permits the exact contract regression the split API is meant to prevent. platform_support() may only report Bubblewrap on Linux because mxc-sdk cannot dispatch LXC; executor-only LXC belongs in available_backends(). Restore the strict assertion.

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

fn platform_support_linux_methods_are_lxc_or_bubblewrap() {
    let support = platform_support();
    for method in &support.available_methods {
        assert!(
            method == "lxc" || method == "bubblewrap",

src/core/mxc_engine/src/probe.rs:90

  • probe_bwrap() uses Command::output() without a timeout, so a broken or replaced bwrap can block this synchronous startup probe indefinitely before the bounded LXC check is reached. Use a bounded/reaped subprocess probe here as well.
    if bwrap_common::bwrap_version::probe_bwrap().is_ok() {

src/backends/lxc/common/src/availability.rs:68

  • If try_wait() fails after the child was spawned, this branch returns without terminating or reaping it; dropping Child does not wait, so the failed probe can leave a running process or zombie. Apply the same best-effort kill-and-wait cleanup used by the timeout path.
            Err(_) => return LxcLsOutcome::SpawnFailed,

src/core/mxc_engine/src/probe.rs:56

  • The probe is partially cached despite this statement: LXC, IsolationSession, and BaseContainer each use a process-lifetime OnceLock, while other checks rerun. Documenting that distinction matters because repeated calls cannot observe changes to those cached capabilities.
/// Not cached — read once at startup, not in a hot loop. The reported `tier` is
/// a ceiling: policy can still force a weaker tier at dispatch.
  • Files reviewed: 21/21 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Comment thread src/core/mxc-sdk/tests/sdk_helpers.rs Outdated
for method in &support.available_methods {
assert_eq!(method, "bubblewrap", "unexpected Linux method: {method}");
assert!(
method == "lxc" || method == "bubblewrap",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These consistency tests don't fence the fix they're paired with. This Linux assert still permits "lxc", and the Windows loop permits "windows_sandbox" | "isolation_session" — but  platform_support() now deliberately excludes all three (they moved to available_backends()). So, reintroducing the original bug (advertising a non-SDK-launchable backend here) would pass both tests unchanged, and the Linux loop also passes vacuously on a host with no methods.

Fix: Tighten to assert the exact narrowed contract: Linux reports only bubblewrap, and Windows reports processcontainer first with only optional wslc beyond it — explicitly excluding lxc / windows_sandbox / isolation_session  so a regression fails here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

The platform_support consistency tests were loosened earlier in the PR when it
temporarily advertised lxc / windows_sandbox / isolation_session; after those
moved to available_backends() the tests were never re-tightened, so they still
permitted the excluded backends and a regression re-advertising one would pass
(the Linux loop also passed vacuously on a host with no methods).

- Linux: assert available_methods == exactly ["bubblewrap"] (lxc excluded).
- Windows: keep processcontainer-first, then assert every method is
  processcontainer | wslc, explicitly excluding windows_sandbox /
  isolation_session.

Addresses SohamDas2021 review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

src/core/mxc-sdk/tests/sdk_helpers.rs:212

  • This test now fails on any valid Linux host where Bubblewrap is not installed or is below the minimum version, because platform_support() correctly returns an unsupported result with no methods there. Keep the exact-set assertion only for supported hosts and assert an empty set otherwise; this still catches an accidental lxc advertisement without making cargo test depend on host packages.
    assert_eq!(
        support.available_methods,
        vec!["bubblewrap".to_string()],
        "Linux platform_support must report exactly bubblewrap (lxc excluded)"
    );

src/core/mxc_engine/src/probe.rs:136

  • The SDK-facing API cannot currently report this advertised backend. This branch requires mxc_engine/isolation_session, but mxc-sdk exposes only a wslc feature and never forwards isolation_session; consequently a normal mxc_sdk::available_backends() consumer always omits IsolationSession even when activation succeeds. That contradicts the PR/README promise that the broader probe includes IsolationSession. Expose and document an SDK feature that forwards this detector, or decouple availability detection from the runner feature.
    #[cfg(feature = "isolation_session")]
    if isolation_session_common::availability::is_isolation_session_available() {
        backends.push(AvailableBackend::tierless(
            ContainmentBackend::IsolationSession.wire_name(),
        ));

src/core/mxc_engine/src/probe.rs:56

  • The “Not cached” contract is inaccurate: is_lxc_available(), is_isolation_session_available(), and the BaseContainer detector all cache their first result in a OnceLock. A caller repeating this function after host enablement changes can therefore receive stale entries/tiers. Document the process-lifetime caching rather than promising a fresh probe.
/// Not cached — read once at startup, not in a hot loop. The reported `tier` is
/// a ceiling: policy can still force a weaker tier at dispatch.
  • Files reviewed: 21/21 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@huzaifa-d
Huzaifa Danish (huzaifa-d) merged commit 4debe7d into main Aug 7, 2026
23 checks passed
@huzaifa-d
Huzaifa Danish (huzaifa-d) deleted the user/modanish/port-host-detectors-to-rust branch August 7, 2026 22:23
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Attention Issue needs attention from Microsoft label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants