[Bubblewrap] Gate platform detection on a minimum bwrap version

[Carlos Alexandro Becker (caarlos0)]

📖 Description

Bubblewrap platform detection only checked that bwrap --version exited
successfully. Presence on PATH is not the same as usability: a host with an old
bubblewrap was reported as supported, and the failure only surfaced later
at spawn time as an opaque bwrap: unknown option error.

This PR gates detection on a minimum version, derived from the flags
bwrap_command::build_args_classified actually emits (verified against the
bubblewrap git history):

Flag	First bwrap release
--bind, --ro-bind, --dev, --proc, --tmpfs, --symlink, --chdir, --setenv, --unshare-*	0.1.0
--ro-bind-try (deny-by-default baseline mounts)	0.3.1
--clearenv (minimal sandbox environment)	0.5.0

--clearenv sets the floor, so bwrap 0.5.0 is the minimum. The docs
previously claimed 0.3.0+, which was both wrong for --ro-bind-try (0.3.1)
and predated the --clearenv requirement.

Changes

• New src/backends/bubblewrap/common/src/bwrap_version.rs:
  • MIN_BWRAP_VERSION with a table documenting why each component is the
    floor, so a future flag adoption has an obvious place to bump.
  • BwrapVersion (ordered major.minor.patch) and a parser that is:
    • anchored on the bubblewrap package name, so unrecognized output
      actually fails closed — a stray number in unrelated output (some other tool 999) can't be read as a version and clear the gate;
    • lenient about what surrounds each number, so distro-patched strings
      (0.4.1-1, a bare 0.6) resolve;
    • strict about components that are present and non-numeric, so
      0.6.invalid is rejected rather than silently read as 0.6.0;
    • correct about Debian's +really marker, which names the version the
      package actually ships — 0.5.0+really0.4.1 is 0.4.1, and is rejected
      rather than admitted as 0.5.0;
    • strict about trailing components and overflow, so 0.5.0.invalid and an
      out-of-u32 component are rejected instead of being read as 0.5.0 /
      admitted as very new, while a numeric 0.6.0.1 distro build still works.
  • probe_bwrap() returning Result<BwrapVersion, BwrapUnavailable>, where
    BwrapUnavailable renders the user-facing reason via Display — one
    message source for every caller. Its variants separate the four distinct
    outcomes:
    • NotFound — spawn failed with io::ErrorKind::NotFound; the binary is
      genuinely absent, so "install the package" is the right remediation.
    • ProbeFailed { status, detail } — the binary exists but bwrap --version
      failed (permissions, dynamic-loader problems, non-zero exit). Keeps the
      exit status and stderr instead of discarding them, and does not tell
      the user to install a package they already have.
    • UnrecognizedVersion — ran, but printed something unparsable. Fails
      closed: without a version we cannot assert the required flags exist.
    • TooOld — names the detected version and the required floor.
• mxc_engine::platform_support and BubblewrapScriptRunner::validate both now
  call the single probe, replacing their two independent presence-only checks,
  so the platform report and the runner can no longer disagree.
• wxc_e2e_tests::has_bwrap() reuses the same probe, so the Bubblewrap
  characterization tests skip cleanly on a too-old host instead of red-failing
  on an error the backend now rejects up front.
• sdk/node/src/platform.ts mirrors the gate (MIN_BWRAP_VERSION,
  _parseBwrapVersion, _probeBubblewrap), including the missing/broken
  distinction. The probe uses execFileSync rather than execSync because
  running through a shell collapses a missing binary into an indistinguishable
  exit code 127, whereas execFileSync surfaces ENOENT directly. The Linux
  reason now says why rather than reporting a bare "not available".
• docs/bwrap-support/bubblewrap-backend.md prerequisites corrected to 0.5.0.

src/Cargo.lock changes by exactly one line — wxc_e2e_tests gains the
internal workspace crate bwrap_common (Linux-only target dependency). No
new external dependency is introduced, and dependency-feed-check passes.

🔗 References

No existing issue — found while reviewing Bubblewrap host detection.

Minimum-version claims were verified directly against
containers/bubblewrap release tags
(--clearenv added by 8f72ceb, first released in v0.5.0; --bind-try
family added by d3515d8, first released in v0.3.1).

main has since been merged in. That merge included a refactor of
bwrap_command.rs (proxy env keys moved into the shared
wxc_common::proxy_env), so the emitted flag set was re-checked afterwards —
it is unchanged, and --clearenv remains the binding constraint.

🔍 Validation

Automated coverage added:

• 16 Rust unit tests in bwrap_version.rs — standard / distro-patched /
  short version parsing, rejection of non-version output, rejection of
  present-but-non-numeric components, semantic ordering, acceptance at exactly
  MIN_BWRAP_VERSION, rejection of 0.4.1, fail-closed on unparsable output
  (including a stray number in unrelated output), rejection of trailing junk
  and of components that overflow u32, +really resolving to the shipped
  version and not smuggling a below-floor bwrap past the gate, every error
  message naming the required version, and the probe-failure message preserving
  exit status and stderr (with and without a status).
• 21 SDK unit tests in sdk/node/tests/unit/platform.test.ts — the parser
  cases plus the minimum-version comparison itself, driven through a new
  _setBwrapVersionRunner hook (matching the existing _setProbeRunner /
  _setWindowsBuildQuery convention): below-floor, exactly-at-floor,
  above-floor, unparsable, missing, and present-but-broken outcomes, and the
  resulting getPlatformSupport() methods on Linux. This keeps the duplicated
  SDK gate from drifting from the Rust gate unnoticed.

Commands run locally (macOS host):

cargo fmt --all -- --check                                   # clean
cargo clippy -p bwrap_common -p mxc_engine -p wxc_e2e_tests \
    -p wxc_common --all-targets -- -D warnings               # clean
cargo clippy -p bwrap_common -p mxc_engine -p wxc_e2e_tests \
    -p wxc_common --all-targets \
    --target x86_64-unknown-linux-gnu -- -D warnings         # clean (covers the Linux-gated runner)
cargo test -p bwrap_common -p mxc_engine -p wxc_common       # 539 passed, 0 failed
cd sdk/node && npm run build && npm test                     # 195 passed, 3 failed

The 3 SDK failures are pre-existing and unrelated — Windows-only
availableTools policy assertions (pwsh.exe / PSReadLine / system root)
that cannot pass on a macOS host. Confirmed identical on a stashed baseline
before any change, and the SDK Unit Tests (windows) CI lane passes.

✅ 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 (see docs/pull-requests.md)

📋 Issue Type

• Bug fix
• Feature
• Task

[Copilot]

Pull request overview

Adds minimum-version validation for Bubblewrap across Rust and Node.js platform detection.

Changes:

• Introduces a shared Rust Bubblewrap version probe requiring 0.5.0+.
• Mirrors version gating in the Node.js SDK and E2E checks.
• Updates tests, dependencies, and documentation.

Show a summary per file

File	Description
src/backends/bubblewrap/common/src/bwrap_version.rs	Adds version parsing and availability errors.
src/backends/bubblewrap/common/src/bwrap_runner.rs	Validates Bubblewrap using the shared probe.
src/backends/bubblewrap/common/src/lib.rs	Exposes the version module.
src/core/mxc_engine/src/platform.rs	Applies version-aware platform detection.
src/testing/wxc_e2e_tests/src/lib.rs	Reuses the probe for test skipping.
src/testing/wxc_e2e_tests/Cargo.toml	Adds the Linux-only internal dependency.
src/Cargo.lock	Records the dependency update.
sdk/node/src/platform.ts	Adds SDK-side parsing and version gating.
sdk/node/tests/unit/platform.test.ts	Adds parser tests.
docs/bwrap-support/bubblewrap-backend.md	Documents the 0.5.0 minimum.

Review details

Comments suppressed due to low confidence (2)

src/backends/bubblewrap/common/src/bwrap_version.rs:170

• A present but nonnumeric component is treated as zero, so output such as bubblewrap 0.6.invalid parses as 0.6.0 and is accepted. This contradicts the fail-closed contract for unrecognized output; only genuinely absent components should default to zero.

    let mut components = token.split('.').map(leading_number);
    let major = components.next().flatten()?;
    let minor = components.next().flatten().unwrap_or(0);
    let patch = components.next().flatten().unwrap_or(0);
    Some(BwrapVersion::new(major, minor, patch))

sdk/node/src/platform.ts:400

• The added SDK tests exercise only _parseBwrapVersion; they never execute this minimum-version comparison or verify that getPlatformSupport() omits Bubblewrap below 0.5.0 and accepts it at the floor. Add an injectable Bubblewrap command runner and cover the below/exact/unparseable probe outcomes so the duplicated SDK gate cannot drift from the Rust gate unnoticed.

  if (compareVersions(version, MIN_BWRAP_VERSION) < 0) {
    return {
      available: false,
      reason: `Bubblewrap (bwrap) ${version.join('.')} is too old; version ${minVersion} or newer is required`,
    };

• Files reviewed: 9/10 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (2)

sdk/node/src/platform.ts:398

• This parser likewise accepts any numeric token, so output such as some other tool 1 is treated as Bubblewrap 1.0.0 and the SDK marks the backend available. That contradicts the stated fail-closed behavior and can recreate the late spawn failure this gate is intended to prevent; bind the parsed token to the stable bubblewrap package name.

export function _parseBwrapVersion(output: string): [number, number, number] | null {
  const token = output.split(/\s+/).find((t) => /^\d/.test(t));
  if (!token) return null;
  const parts = token.split('.');

src/backends/bubblewrap/common/src/bwrap_version.rs:206

• The parser accepts the first numeric token even when the output is not a Bubblewrap version. For example, some other tool 1 becomes 1.0.0, so check_version_output reports this unrecognized output as supported instead of failing closed. Require the version token to immediately follow the stable bubblewrap package name.

    let token = output
        .split_whitespace()
        .find(|token| token.starts_with(|c: char| c.is_ascii_digit()))?;

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (6)

src/backends/bubblewrap/common/src/bwrap_version.rs:191

• This does not actually fail closed on unrecognized successful output: it accepts the first numeric token anywhere, so check_version_output("some other tool 999") reports version 999.0.0 and marks the backend usable. Since the preceding comment says Bubblewrap's PACKAGE_STRING name has been stable since 0.1.0, require a bubblewrap token followed by the version before parsing it.

    let token = output
        .split_whitespace()
        .find(|token| token.starts_with(|c: char| c.is_ascii_digit()))?;

src/backends/bubblewrap/common/src/bwrap_version.rs:144

• execve also returns ENOENT/ErrorKind::NotFound when the bwrap file exists but its shebang or ELF interpreter/dynamic loader is missing. This therefore still maps a broken present installation to NotFound and tells the user to install a package, contrary to the new ProbeFailed contract; resolve/check the PATH candidate before deciding that NotFound means the executable itself is absent.

        .map_err(|err| match err.kind() {
            // Only a genuinely missing binary is "not installed"; anything else
            // (permissions, loader errors) is a broken install, not a missing one.
            std::io::ErrorKind::NotFound => BwrapUnavailable::NotFound,
            _ => BwrapUnavailable::ProbeFailed {

sdk/node/src/platform.ts:448

• A synchronous child killed by a signal also has a null exit status, so this reason says it “could not be executed” even though it started and was terminated. Use neutral no-exit-status wording or include the error's signal field so this diagnostic remains accurate.

    const where =
      result.status === null ? 'could not be executed' : `exited with status ${result.status}`;
    const detail = result.detail ? `: ${result.detail}` : '';

sdk/node/src/platform.ts:396

• The SDK parser likewise accepts any successful output containing a numeric token; for example, something else 999 is treated as a supported Bubblewrap 999.0.0 instead of taking the documented fail-closed path. Validate the stable bubblewrap <version> package-string shape rather than searching arbitrary output for a number.

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

export function _parseBwrapVersion(output: string): [number, number, number] | null {
  const token = output.split(/\s+/).find((t) => /^\d/.test(t));
  if (!token) return null;

sdk/node/src/platform.ts:361

• ENOENT from execFileSync is not limited to a missing bwrap path: Unix also reports it when an existing script/ELF executable names a missing interpreter or dynamic loader. Such a broken present install is still classified as notFound, producing the misleading install-package remediation; first determine whether a bwrap PATH candidate exists before choosing this outcome.

  } catch (err) {
    const e = err as NodeJS.ErrnoException & { status?: number | null; stderr?: Buffer | string };
    if (e.code === 'ENOENT') {
      return { kind: 'notFound' };

src/backends/bubblewrap/common/src/bwrap_version.rs:103

• For a process terminated by a signal, ExitStatus::code() is also None, even though bwrap was successfully executed. This branch then reports “could not be executed,” obscuring the actual probe failure; use wording that covers both spawn failures and signal termination, or preserve the terminating signal separately.

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

                match status {
                    Some(code) => write!(f, "exited with status {code}")?,
                    None => write!(f, "could not be executed")?,
                }

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (4)

src/backends/bubblewrap/common/src/bwrap_version.rs:208

• The +really distro form is interpreted backwards. In Debian-style versions, 0.11.0+really0.10.0 means the package actually contains upstream 0.10.0, but this parser keeps 0.11.0. Consequently a package such as 0.5.0+really0.4.1 is accepted even though its effective bwrap version predates --clearenv, defeating the minimum-version gate. Parse the version after +really when that marker is present (and add a below-floor regression case).

    let mut components = tokens.next()?.split('.');
    let major = leading_number(components.next()?)?;
    let minor = components.next().map_or(Some(0), leading_number)?;
    let patch = components.next().map_or(Some(0), leading_number)?;
    Some(BwrapVersion::new(major, minor, patch))

sdk/node/src/platform.ts:410

• This mirrors the Rust parser's incorrect handling of Debian +really versions: the portion after +really is the effective upstream version, not a suffix to ignore. For example, 0.5.0+really0.4.1 currently becomes 0.5.0 and is accepted even though the installed code is 0.4.1 and lacks the required flag. Parse the post-+really version and cover a case that crosses the 0.5.0 floor.

  for (const part of tokens[1].split('.').slice(0, 3)) {
    const digits = /^\d+/.exec(part);
    // Present but non-numeric: fail closed rather than guessing 0.
    if (!digits) return null;
    components.push(parseInt(digits[0], 10));
  }

sdk/node/src/platform.ts:361

• ENOENT can also mean that an existing bwrap could not be executed because its ELF loader or script interpreter is missing; it is not exclusive to an absent PATH entry. Such a broken binary is still reported as “not installed” here, so the SDK does not actually preserve the missing-vs-broken distinction promised by this probe. Resolve/check the PATH candidate before classifying this as notFound; an existing candidate should produce failed.

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

    if (e.code === 'ENOENT') {
      return { kind: 'notFound' };

src/backends/bubblewrap/common/src/bwrap_version.rs:145

• ErrorKind::NotFound does not prove that the bwrap path is absent: on Linux, execve also returns ENOENT when an existing executable's ELF loader or script interpreter is missing. That broken installation is therefore still classified as NotFound and gets the misleading package-install remediation, contrary to the new ProbeFailed contract. Check whether a PATH candidate exists before choosing NotFound, and map ENOENT for an existing candidate to ProbeFailed.

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

            std::io::ErrorKind::NotFound => BwrapUnavailable::NotFound,

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (2)

src/backends/bubblewrap/common/src/bwrap_version.rs:217

• The parser never verifies that the component iterator is exhausted, so bubblewrap 0.5.0.invalid is accepted as 0.5.0. That lets an unrecognized banner clear a gate that is intended to fail closed; reject any components beyond the supported major.minor.patch shape.

    let mut components = token.split('.');
    let major = leading_number(components.next()?)?;
    let minor = components.next().map_or(Some(0), leading_number)?;
    let patch = components.next().map_or(Some(0), leading_number)?;
    Some(BwrapVersion::new(major, minor, patch))

sdk/node/src/platform.ts:437

• Slicing to three components silently drops any remaining components, so bubblewrap 0.5.0.invalid is accepted as 0.5.0 instead of following the fail-closed unrecognized-version path. Reject tokens with more than three components before parsing them.

  for (const part of token.split('.').slice(0, 3)) {

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

sdk/node/src/platform.ts:439

• The TypeScript parser accepts overflowing components that the Rust u32 parser rejects. For example, a sufficiently long major component becomes Infinity, _parseBwrapVersion returns it, and compareVersions admits the malformed banner as newer than 0.5.0, violating the intended fail-closed behavior and making the SDK disagree with the backend. Reject non-safe or out-of-u32 values before storing them.

    const digits = /^\d+/.exec(part);
    // Present but non-numeric: fail closed rather than guessing 0.
    if (!digits) return null;
    components.push(parseInt(digits[0], 10));

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Comments suppressed due to low confidence (4)

sdk/node/src/platform.ts:366

• This check does not mirror executable lookup for two ENOENT cases: existsSync follows symlinks (so a dangling bwrap symlink is classified as absent rather than broken), and an empty PATH component—which means the current directory on Linux—is skipped. Use lstatSync and treat an empty component as . so the missing/broken distinction is accurate.

function bwrapExistsOnPath(): boolean {
  const pathVar = process.env.PATH;
  if (!pathVar) return false;
  return pathVar
    .split(path.delimiter)
    .some((dir) => dir !== '' && fs.existsSync(path.join(dir, 'bwrap')));

src/backends/bubblewrap/common/src/bwrap_version.rs:237

• The ENOENT disambiguation still misclassifies a broken symlink as an absent package. Path::exists() follows symlinks, so a PATH entry such as /usr/bin/bwrap -> /missing/bwrap returns false and becomes NotFound even though this is exactly a present-but-broken installation. Check the directory entry with symlink_metadata instead.

fn bwrap_exists_on_path() -> bool {
    std::env::var_os("PATH")
        .is_some_and(|path| std::env::split_paths(&path).any(|dir| dir.join("bwrap").exists()))

src/backends/bubblewrap/common/src/bwrap_version.rs:141

• This probe has no timeout and captures both pipes without a size bound. A hung or output-flooding bwrap wrapper can therefore block platform_support() and every Bubblewrap validation indefinitely (or exhaust memory), whereas the SDK probe added here explicitly bounds the same synchronous operation to five seconds. Use a bounded child wait and cap/drain the version output before parsing it.

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

    let output = Command::new("bwrap")
        .arg("--version")
        .stdin(Stdio::null())
        .output()

sdk/node/src/platform.ts:285

• diagLog cannot surface this reason on Linux: diagnostic.ts short-circuits all non-Windows calls. When LXC is installed but bwrap is too old/broken, the platform remains supported, and an explicitly selected Bubblewrap request is then rejected by helper.ts with only the generic “not available” message. The detected version/probe failure is therefore still lost on the exact platform this branch handles; propagate a per-backend unavailability reason to that validation path instead.

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

      // Always surface why bwrap is unavailable. When LXC is present the
      // platform is still supported, so `reason` — documented as why the
      // platform is *not* supported — must stay unset, and the detail would
      // otherwise be dropped with no way to diagnose the missing backend.
      diagLog(`getPlatformSupport: bubblewrap unavailable — ${bubblewrap.reason}`);

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced