Repository navigation
[Bubblewrap] Fail the run when the sandbox loses its network provider - #1250
Merged
Soham Das (SohamDas2021) merged 2 commits intoSep 23, 2026
Merged
Conversation
…he workload Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a53974f-949a-4934-9997-9982c4eddd90
… mid-run Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a53974f-949a-4934-9997-9982c4eddd90
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Terminal-state races and the polling API can still misreport provider loss, while the test may kill an unrelated concurrent provider.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds Bubblewrap network-provider liveness monitoring so provider loss terminates and fails active workloads.
Changes:
- Adds pre-start and runtime provider checks.
- Integrates provider-loss handling into Bubblewrap lifecycle management.
- Adds documentation and an end-to-end test.
| File | Description |
|---|---|
src/backends/bubblewrap/common/src/bwrap_runner.rs |
Integrates monitoring and provider-loss outcomes. |
src/backends/bubblewrap/common/src/provider_monitor.rs |
Implements the liveness monitor thread. |
src/backends/bubblewrap/common/src/proxy_network.rs |
Adds the liveness pipe and diagnostics. |
src/backends/bubblewrap/common/src/lib.rs |
Registers the monitor module. |
tests/configs/bubblewrap_provider_loss.json |
Defines the provider-loss workload. |
tests/scripts/run_bwrap_provider_loss_test.sh |
Exercises mid-run provider termination. |
tests/scripts/run_bwrap_all_tests.sh |
Adds the new test to the suite. |
docs/bwrap-support/bubblewrap-backend.md |
Documents behavior and limitations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+68
to
+69
| after=$(pgrep -x slirp4netns 2>/dev/null | sort || true) | ||
| victim=$(comm -13 <(echo "$before") <(echo "$after") | head -1) |
Comment on lines
+817
to
+819
| if let Some(status) = exited { | ||
| return BwrapOutcome::Exited(status); | ||
| } |
Comment on lines
+892
to
894
| .lock_child() | ||
| .try_wait()? | ||
| .map(|status| status.code().unwrap_or(-1))) |
Soham Das (SohamDas2021)
deleted the
sohamdas2021-bwrap-slirp-liveness
branch
September 23, 2026 23:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
Bubblewrap's private-namespace modes give the workload a network namespace whose only route is
slirp4netns. Its liveness was checked once during startup and never again, so a slirp that died under a running workload left the sandbox running against a dead network. Every connection failed with a generic transport error, and the run was attributed to whatever the workload reported — the real cause reached the log only at teardown, as aWARNING:.Losing the connectivity provider now fails the run. Two checks:
Before the workload starts. Readiness is latched rather than revoked, so it can already be stale by the time the startup gate opens. The supervisor is re-checked immediately before release. Any exit fails the run, including a successful one — slirp's exit code says nothing about whether the sandbox still has a route.
For the lifetime of the workload. The supervisor holds the write end of a pipe nothing ever writes to, and slirp inherits it. The runner keeps the read end and watches it from a monitor thread. It reaches EOF when both have exited, which is what separates a dead network from an orphaned slirp still carrying traffic after its supervisor was killed. On EOF the sandbox is terminated and the run fails naming the supervisor's exit status and a bounded tail of its stderr:
The monitor is disarmed before teardown stops the supervisor, so an ordinary shutdown — which closes the same descriptor — is never reported as a loss.
This is a diagnosability and reliability fix, not a containment one: a dead slirp removes connectivity rather than granting it, and the egress rules live in the sandbox's own namespace, so they stay in force. It also covers
enforcementMode: "firewall", which stands up the same supervisor and the same slirp.Limitations. Detection is exit-based. A slirp that is alive but wedged still holds its descriptor open and reaches the workload as an unreachable network. The monitor costs ~0.1% of one core (measured: 0.45 µs per poll; the rest is timer wakeup).
🔗 References
Resolves #935
🔍 Validation
New E2E test
run_bwrap_provider_loss_test.shkills the run's ownslirp4netnsunder a live workload and asserts the run fails naming the provider, and that the workload does not outlive it.The test was confirmed to catch the bug. Reverted against the same test, the sandbox ran the full 90-second timeout against a dead network and reported only
script timed out after 90000ms— the behavior described in the issue.Run on WSL2 Ubuntu 24.04:
bwrap_version/probe_execalso fail on unmodifiedmain— a DrvFs artifact where spawning a missing path under/mnt/creturnsEACCESinstead ofENOENT.provider_loss,teardown,network_proxy,firewall,directional,localnet,network,basic,readonly_denial,version_gate,linux_process_default— 10 pass, 1 skip (inbound_deny, root-only).cargo clippy -p bwrap_common --all-targets -- -D warnings,cargo fmt --check, andvalidate-configs.js(375 configs) all clean.mxc_engine/mxc-sdk/wxc_commonsuites pass.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type