Skip to content

[Bubblewrap] Prevent workloads after failed network setup - #1716

Open
Soham Das (SohamDas2021) wants to merge 1 commit into
mainfrom
sohamdas2021-bubblewrap-rule-failure-gate
Open

Soham Das (SohamDas2021) wants to merge 1 commit into
mainfrom
sohamdas2021-bubblewrap-rule-failure-gate

Conversation

@SohamDas2021

@SohamDas2021 Soham Das (SohamDas2021) commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

Prevents Bubblewrap workloads from running when network-rule installation fails. Startup failures now terminate the blocked sandbox and clean up its helpers before returning the original error.

Adds regression coverage for proxy-only and firewall-enforced networking through SDK run, spawn, and spawn_with_pty. The existing CLI failure-path assertion remains unchanged.

No design spec is required: this restores documented failure behavior without changing public APIs, request formats, or supported policy.

🔗 References

Fixes #1636.

🔍 Validation

Validated on WSL2 with Bubblewrap 0.9.0 and strict prerequisite checks enabled.

The existing CLI regression reproduced WORKLOAD_STARTED before the fix and passes unchanged afterward. The SDK regression checks the original error, absence of workload file writes, and descendant cleanup across all six network-mode/API combinations.

Commands run from src/:

Command Result
cargo test --locked --quiet -p mxc-sdk --lib backend_modules::bubblewrap::common::proxy_network:: Passed: 170 tests
cargo test --locked --quiet -p mxc-sdk --lib backend_modules::bubblewrap::common::bwrap_runner:: Passed: 37 tests
cargo test --locked --quiet -p mxc-sdk --test streaming_bubblewrap Passed: 17 tests
cargo test --locked --release --quiet -p mxc-sdk --test streaming_bubblewrap Passed: 17 tests
cargo clippy --locked --quiet -p mxc-sdk --lib --test streaming_bubblewrap -- -D warnings Passed
cargo fmt --all -- --check Passed

From the repository root, with LXC_EXEC pointing to the release executable:

  • timeout 90 bash tests/scripts/run_bwrap_namespace_startup_test.sh — passed three consecutive runs.

Remote CI and other Linux distributions were not run.

✅ Checklist

  • Signed the Contributor License Agreement
  • Linked to an issue
  • For features or changes requiring a spec: linked the issue with maintainer-applied feature-accepted
  • For implementation: linked the required approved and merged design/API spec, or explained why no spec is required
  • Feature issue remains open unless this PR completes all agreed implementation, validation, and documentation
  • 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 pull request builds)

📋 Issue Type

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

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code owner November 10, 2026 00:53
Copilot AI balanced review requested due to automatic review settings November 10, 2026 00:53
@azure-pipelines

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

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.

🔵 Needs a closer look

Containment-sensitive startup and process-group cleanup changes need maintainer confirmation and broader Linux validation because runtime evidence is WSL2-only and Linux CI remains pending.

No open issues 🎉

What changed in this PR

Fixes #1636 by keeping Bubblewrap workloads blocked when network-rule installation fails, restoring fail-closed startup behavior without changing public APIs or request formats.

Changes:

  • Terminates the sandbox process group before releasing the startup barrier and preserves the launcher for cleanup.
  • Adds regression coverage for proxy-only and firewall-enforced failures through SDK run, spawn, and spawn_with_pty.
  • Documents startup cleanup guarantees and validation coverage.
File Description
src/​mxc-sdk/​tests/​streaming_bubblewrap.rs Tests setup errors, absence of workload writes, and descendant cleanup across six combinations.
src/​mxc-sdk/​src/​backends/​bubblewrap/​common/​proxy_network.rs Preserves startup-barrier ownership and avoids prematurely reaping the launcher.
src/​mxc-sdk/​src/​backends/​bubblewrap/​common/​bwrap_runner.rs Kills the startup process group before dropping the barrier and reports cleanup errors.
docs/​development/​build-and-test/​ci-validation-infrastructure.md Describes CLI and SDK startup-failure coverage.
docs/​backends/​bwrap/​bubblewrap-backend.md Documents fail-closed startup and helper cleanup behavior.

Review effort: Balanced

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.

🔵 Needs a closer look

Startup-barrier and process-group changes affect sandbox containment, requiring confirmation of strict Linux CI and namespace-startup validation on supported distributions because runtime validation was unavailable during review.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
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.

[Bubblewrap] Workload starts when network rule installation fails

3 participants