feat(engine): Rust host-backend detectors + available_backends() probe - #725
Conversation
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>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
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>
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>
There was a problem hiding this comment.
Review details
Suppressed comments (4)
src/core/mxc_engine/src/probe.rs:32
tierlessis compiled out on every unsupported target, but the unconditionally compiled tests below call it. Consequently,cargo testfails to compile on the same “other platform” targets thatavailable_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()andavailable_backends()are public in-process SDK APIs and may be called by an elevated host. Restore the bounded/EnglishDISM 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 callingplatform_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.mdstill documents onlyplatform_support()and does not mentionavailable_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
There was a problem hiding this comment.
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_sessioncan be omitted even when usable, so absence cannot mean “not currently usable,” andplatform_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-sdkAPIs, 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-52still documents onlyplatform_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 discoveravailable_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>
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/core/mxc_engine/src/probe.rs:10
- These module docs contradict the new contract in
platform.rs:18-22and the implementation:isolation_session,microvm, andhyperlightcan 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-306callscheck_sandbox_available(), which resolvesWindowsSandbox.exefrom attacker-controlled%SystemRoot%, while this probe usesGetSystemDirectoryW. 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 onlyplatform_support()and whose supported-backends table distinguishes what this SDK can launch. Documentavailable_backends()(including tier/omission semantics) and clarify thatplatform_support().available_methodsis 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>
There was a problem hiding this comment.
Review details
Suppressed comments (5)
src/core/mxc-sdk/src/lib.rs:87
- This public re-export is built against
mxc_enginewith default features becausemxc-sdkdefines no forwarding features. A normalmxc-sdkconsumer therefore cannot enable the probe'swslc,isolation_session, ortier2_bfsbranches, so this entry point underreports the host and cannot expose all of the functionality being re-exported. Add correspondingmxc-sdkfeature 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_sessiondetector 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 thoughmxc-sdkcannot launch it, andplatform.rsdocuments 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_codelint reports an unread private enum field, and the repository'sclippy -D warningsgate 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 returnwindows_sandboxand 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
…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
There was a problem hiding this comment.
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-82explicitly excludes Windows Sandbox and IsolationSession because the SDK cannot launch them. Allowing those values removes regression coverage for the separation fromavailable_backends(); onlyprocesscontainerand feature-gatedwslcshould 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.exegate under-reports the policy-independent tier ceiling. The linked #717 design definesappcontainer-bfsas reachable whenevertier2_bfsis compiled, andfallback_detector::detectlikewise selects BFS for a request with no filesystem rules without resolvingbfscfg.exe(fallback_detector.rs:248-258). On a BFS-enabled build without that executable, this probe therefore reportsappcontainer-dacleven though BFS is reachable. Select the ceiling from the feature alone and leavebfscfgavailability 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. Droppingstd::process::Childdoes not perform that cleanup, so an availability probe can leavelxc-lsrunning 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 thoughplatform_support()intentionally reports only SDK-launchable Linux backends andplatform.rs:61-67restricts that list tobubblewrap. 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-57still tells users thatplatform_support()reports the available containment backends and does not mentionavailable_backends(). Update the README to document the new probe and clarify thatplatform_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>
There was a problem hiding this comment.
Review details
Suppressed comments (6)
src/core/mxc-sdk/tests/sdk_helpers.rs:229
- This assertion now permits
windows_sandboxandisolation_session, contradicting the restoredplatform_support()contract: those are executor-only and belong exclusively toavailable_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, althoughplatform_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
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>
There was a problem hiding this comment.
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, althoughplatform_support()must remain limited to SDK-launchable backends and Linux can only reportbubblewrap(the linked design explicitly locks this down). As written, a regression that leaks LXC back intoplatform_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 thatavailable_backends()was introduced to avoid. Keep the accepted set toprocesscontainerand feature-gatedwslc.
// `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-sdkbuild: the engine gates that detector behindfeature = "isolation_session", butmxc-sdkexposes only awslcforwarding feature. Consequentlymxc_sdk::available_backends()always omits IsolationSession even on a capable host. Add an SDK feature forwardingmxc_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 theChildwithout 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
….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>
There was a problem hiding this comment.
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-sdkbuild cannot report IsolationSession because that crate does not forwardmxc_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 becausemxc-sdkcannot dispatch LXC; executor-only LXC belongs inavailable_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()usesCommand::output()without a timeout, so a broken or replacedbwrapcan 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; droppingChilddoes 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
| for method in &support.available_methods { | ||
| assert_eq!(method, "bubblewrap", "unexpected Linux method: {method}"); | ||
| assert!( | ||
| method == "lxc" || method == "bubblewrap", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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 accidentallxcadvertisement without makingcargo testdepend 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, butmxc-sdkexposes only awslcfeature and never forwardsisolation_session; consequently a normalmxc_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 aOnceLock. 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
📖 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
The
tieris a ceiling (strongest reachable isolation; a policy may force weaker at dispatch), and a backend appearing inavailable_backends()is a host-capability signal — not a guarantee this SDK can launch it (cross-checkplatform_support()). Full guidance insrc/core/mxc-sdk/README.md.Changes:
available_backends()API reporting each host-available backend with its effective isolation tier; keepsplatform_support()limited to the SDK-launchable subset.IsolationTier.Follow-ups (out of scope):
microvm/hyperlightdetectors and the side-effect-free TypeScript-projection transport.A CI gate ensuring every tier/backend stays probe-covered is tracked in #769.
🔗 References
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type