Skip to content

[Bubblewrap] Make Bubblewrap the default Linux backend#368

Merged
huzaifa-d merged 9 commits into
mainfrom
user/sodas/MakeBubblewrapDefault
May 22, 2026
Merged

[Bubblewrap] Make Bubblewrap the default Linux backend#368
huzaifa-d merged 9 commits into
mainfrom
user/sodas/MakeBubblewrapDefault

Conversation

@SohamDas2021

@SohamDas2021 SohamDas2021 commented May 21, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Makes Bubblewrap the default Linux process-sandbox backend. Callers that omit containment, or set the abstract containment: "process", now get Bubblewrap instead of LXC. Explicit containment: "lxc" and explicit containment: "bubblewrap" continue to work as before.

Linux containment resolution matrix

containment SDK gate Native dispatcher resolves to
(omitted) none Bubblewrap
"process" none Bubblewrap
"lxc" none LXC
"bubblewrap" requires experimental: true (SDK only) Bubblewrap
"processcontainer" none LXC (catch-all fallback)

Note: the experimental gate on explicit "bubblewrap" is enforced at the SDK layer only (helper.ts ExperimentalBackends`); the native binary does not re-check it. This is the same behavior as the original Bubblewrap PR (#314) and is unchanged here.

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

Copilot AI review requested due to automatic review settings May 21, 2026 03:48

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.

Pull request overview

Updates MXC’s Linux containment resolution so the abstract “process” intent (and omitted containment) routes to Bubblewrap by default, while keeping explicit containment: "lxc" working as an opt-in path. This fits into the codebase’s “abstract intent → per-OS concrete backend” design in wxc_common::config_parser.

Changes:

  • Change Rust config parsing so omitted containment and containment: "process" resolve to ContainmentBackend::Bubblewrap on Linux.
  • Add/adjust SDK unit + integration tests to exercise the new default and to keep explicit LXC coverage via an explicit containment: "lxc" config.
  • Add CLI-level test configs + scripts to validate default Linux process behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test_scripts/run_linux_process_default_test.sh New script to run configs that exercise Linux default/abstract process containment resolution.
test_scripts/run_bwrap_all_tests.sh Includes the new default-process test script and line-ending checks for it.
test_configs/linux_process_default.json New config to validate “containment omitted” Linux default behavior.
test_configs/linux_process_abstract.json New config to validate containment: "process" behavior on Linux.
src/wxc_common/src/config_parser.rs Core behavior change: Linux default + "process" resolve to Bubblewrap; adds parser tests/regressions.
src/lxc/src/main.rs Updates dispatch comment to reflect Bubblewrap being the Linux default for abstract intents.
sdk/tests/unit/sandbox.test.ts Adds unit coverage for Bubblewrap mapping and explicit LXC opt-in behavior.
sdk/tests/integration/test-helpers.ts Adds a helper to run spawnSandboxFromConfig as a Promise for tests.
sdk/tests/integration/linux-process-container.test.ts Routes these tests through explicit containment: "lxc" to keep exercising LXC.
sdk/tests/integration/linux-bubblewrap.test.ts New integration coverage for Bubblewrap as the silent default and for explicit selection.
sdk/src/sandbox.ts Adds explicit containment === 'lxc' handling; updates Linux 'process' diagnostic logging.

Comment thread sdk/tests/integration/linux-bubblewrap.test.ts
Comment thread test_configs/linux_process_default.json
Comment thread test_configs/linux_process_abstract.json
Comment thread src/lxc/src/main.rs
Comment thread sdk/src/sandbox.ts Outdated
@SohamDas2021 SohamDas2021 changed the title [Bubblewrap] Make Bubblewrap the silent default Linux process-sandbox backend [Bubblewrap] Make Bubblewrap the default Linux backend May 21, 2026
@SohamDas2021 SohamDas2021 requested review from a team and shschaefer May 21, 2026 04:30
@SohamDas2021 SohamDas2021 marked this pull request as ready for review May 21, 2026 16:58
Six proxy tests omitted containment and relied on the previous unconditional ProcessContainer default to satisfy the proxy guard (proxy is only valid with processcontainer).

After this PR the default resolves to Seatbelt on macOS, causing those six tests to fail in the arm64 MAC CI job. Make each test explicit by setting containment: "processcontainer" in the JSON — they are testing proxy parsing behavior, not the OS-default resolution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/mxc/sessions/81dc5e19-9e89-4a1a-a005-6633f4ffbfe8

Co-authored-by: huzaifa-d <16077119+huzaifa-d@users.noreply.github.com>
Copilot AI requested a review from huzaifa-d May 21, 2026 19:37
PR 368 introduces SDK integration tests that exercise the Bubblewrap backend (the new Linux default for `containment: "process"` / omitted). Those tests skip unless `bwrap` is installed on the runner; the Linux integration job previously only installed `lxc`, so the new bwrap tests failed under sudo.

Add `bubblewrap` to the apt-get install line alongside `lxc lxc-utils dnsmasq-base iptables` so both backends are present and both test suites can run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread sdk/src/sandbox.ts Outdated
Comment thread sdk/src/sandbox.ts
Comment thread src/wxc_common/src/config_parser.rs
@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs-Author-Feedback Issue needs attention from issue or PR author Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels May 21, 2026
// Async spawn from a pre-built ContainerConfig (no per-call containment arg on
// spawnSandboxAsync, so tests that need a specific backend build the config
// directly and run it through this helper).
export function spawnFromConfigAsync(

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.

Suggestion - Don't ignore stderr. Consider a timeout too. And have a on error handler.

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.

This helper emulates the SDK's spawnSandboxAsync; the SDK doesn't expose an async wrapper around spawnSandboxFromConfig. Goal is parity with spawnSandboxAsync.

stderr- not ignored. wxc-exec runs under node-pty (single PTY), so stdout/stderr are merged at the OS level. node-pty only exposes one onData. The SDK does the same thing and comments it on sandbox.ts:666. I'll mirror that comment here.

timeout- spawnSandboxAsync doesn't have one either. node --test enforces per-test timeouts, and the config's process.timeout is enforced by the native runner.

onError- node-pty's IPty has no onError event. Sync spawn failures hit the existing try/catch; post-spawn failures surface as a non-zero exitCode via onExit

Comment thread sdk/src/types.ts
* Containment values (abstract intent or concrete backend) that require
* the `--experimental` flag.
*/
export const ExperimentalBackends: readonly (ContainmentType | ContainmentBackend)[] = ['microvm', 'wslc', 'seatbelt', '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.

Now that bubblewrap is default, should it still need experimental? If yes, do the default process paths check/throw if not experimental?

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.

Once we promote bubblewrap to stable, this will be cleaned up. Currently default process paths do not check for experimental. This are the scenarios:

Linux containment resolution matrix

containment SDK gate Native dispatcher resolves to
(omitted) none Bubblewrap
"process" none Bubblewrap
"lxc" none LXC
"bubblewrap" requires experimental: true (SDK only) Bubblewrap
"processcontainer" none LXC (catch-all fallback)

@huzaifa-d huzaifa-d merged commit b82cd9c into main May 22, 2026
18 checks passed
@huzaifa-d huzaifa-d deleted the user/sodas/MakeBubblewrapDefault branch May 22, 2026 19:55
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Attention Issue needs attention from Microsoft label May 22, 2026
richiemsft added a commit that referenced this pull request May 26, 2026
…411)

* Fix: exempt abstract 'process' on macOS from allowOutbound precheck

On macOS, the abstract containment 'process' resolves to the seatbelt
backend, which supports per-host network filtering natively via
sandbox-exec profiles. The SDK precheck incorrectly required
allowOutbound=true for allowedHosts/blockedHosts when using the default
'process' containment on macOS, despite the explicit 'seatbelt' backend
being correctly exempted.

Add 'darwin' platform check to resolvesToHostFilteringBackend alongside
the existing 'linux' check (which maps to bubblewrap). Add unit tests
mirroring the Linux coverage from PR #368.

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

* Fix blockedHosts test: remove allowOutbound to actually test the precheck waiver

---------

Co-authored-by: Richie Gomez <richiegomez@Richies-Mac-mini.local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants