fix(bwrap): surface allowLocalNetwork settings the Bubblewrap backend cannot honor - #750
Conversation
`network.allowLocalNetwork` (may the sandbox bind/listen on local IPs and accept inbound connections) is declared in the shared model, parsed, and honored by Windows (AppContainer's `privateNetworkClientServer` capability) and macOS (Seatbelt's `(allow network-inbound (local ip))`), but was read nowhere under `src/backends/bubblewrap/`. On Linux the value was validated, carried to the backend, and dropped on the floor. Bubblewrap has no inbound-only primitive. Unprivileged bwrap shares the host network namespace with no veth to scope iptables to, and seccomp cannot dereference the `sockaddr` passed to `bind()`, so an AF_INET-only filter is not expressible. The namespace choice alone decides the outcome, which leaves two mismatches: - `false` + shared host namespace (`defaultPolicy: allow`, host lists, or `network.proxy`): the process can still bind/listen on host-local addresses. The restriction is not applied. - `true` + `--unshare-net`: the listener sits in a private namespace nothing outside can reach, so it is silently useless. Emit a preflight WARNING for both instead of failing silently, via a pure `local_network_diagnostic()` in `bwrap_command` that reuses the same namespace predicate as the argument builder so the two cannot drift. Not a hard rejection: `false` is the default, and a sandbox that both serves and connects to its own loopback is a legitimate use of `true` + `--unshare-net`. Also correct the backend doc, which claimed `--unshare-net` leaves no network stack "including loopback". bubblewrap calls `loopback_setup()` when unsharing the network namespace, so `lo` is up inside the sandbox. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e125d97-4031-4589-89dd-b8c2372021f1 Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Surfaces Bubblewrap’s inability to independently enforce allowLocalNetwork.
Changes:
- Centralizes network namespace selection.
- Adds mismatch diagnostics and truth-table tests.
- Documents Bubblewrap networking limitations.
Show a summary per file
| File | Description |
|---|---|
bwrap_runner.rs |
Logs preflight diagnostics. |
bwrap_command.rs |
Adds namespace detection, warnings, and tests. |
bubblewrap-backend.md |
Documents network behavior and limitations. |
Review details
Suppressed comments (1)
src/backends/bubblewrap/common/src/bwrap_command.rs:155
- Recommending
defaultPolicy='allow'here silently relaxes the independent outbound policy from deny-all to allow-all. A user following this warning to enable inbound service exposure would also grant unrestricted egress. The remediation should state that Bubblewrap cannot independently provide this combination and recommend another backend, or make the outbound tradeoff explicit rather than prescribing full allow.
(true, true) => Some(
"WARNING: Bubblewrap: network.allowLocalNetwork=true is confined to the sandbox's own \
network namespace. defaultPolicy='block' with no host lists and no proxy applies \
--unshare-net, so a listener inside the sandbox is reachable only from within it, \
never from the host. Use defaultPolicy='allow' to share the host network namespace.",
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
| if let Some(warning) = bwrap_command::local_network_diagnostic(request, proxy.address()) { | ||
| let _ = writeln!(logger, "{}", warning); |
There was a problem hiding this comment.
Your reading of the logger plumbing is correct, but the conclusion overstates what this PR changed — and the fix you propose is a different, much larger PR.
logger.log_line("WARNING: ...") is the established mechanism for preflight policy warnings in this codebase. The closest analogue is Bubblewrap's own cooperative-proxy warning at src/core/wxc_common/src/config_parser.rs:918, which tells the caller that defaultPolicy: "block" is not actually enforced for raw-socket clients — a security-relevant no-op warning with byte-for-byte identical visibility characteristics. Same for learningMode (config_parser.rs:714), which warns that AppContainer restrictions are not enforced at all, and the non-existent-path warning at config_parser.rs:367.
So the buffered-logger limitation you describe is real, but it is a property of the logging architecture that predates this change and applies uniformly to every preflight warning mxc emits. This PR moves allowLocalNetwork from not read at all to diagnosable through the same channel every other policy warning uses. That is the whole intent, and it is strictly better than the status quo.
What you are asking for — "propagate preflight warnings through an API/response channel that the CLI and SDK surfaces consume" — means adding a warnings field to ScriptResponse (src/core/wxc_common/src/models.rs:748, which has no such field today) and threading it through lxc-exec/wxc-exec, mxc-sdk, mxc_ffi's MxcRunResult, the C# SDK, and the TypeScript SDK. That is a cross-cutting API change affecting every backend and all three language bindings. Doing it here would bury a scoped Bubblewrap fix inside an SDK-surface redesign, and it should be decided on its own merits with maintainer input.
Not resolving this thread — leaving it open deliberately so a maintainer can weigh in on whether the warnings channel is wanted as a follow-up. Happy to file it as a separate issue if that is the preference.
- Fix "an network.allowLocalNetwork" -> "a" in the local_network_diagnostic doc comment. - Spell out the "no network.proxy" condition in the Full block description. An active proxy keeps the host namespace shared even under defaultPolicy='block' with no host lists, which uses_private_netns() checks but the prose omitted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e125d97-4031-4589-89dd-b8c2372021f1 Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Bubblewrap never sets a veth interface (set_veth_interface is called only from lxc_runner), so NetworkIptablesManager skips its FORWARD hook and the chain is never reachable. Per-host filtering on Bubblewrap is inert, and the follow-up network-policy work scoped it to LXC only. Naming allowedHosts / blockedHosts as a way to reach the shared-namespace state pointed users at a path that does not filter anything. The warning now names only defaultPolicy='allow' and network.proxy. Prose only: the --unshare-net predicate is unchanged, so behavior is identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e125d97-4031-4589-89dd-b8c2372021f1 Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (1)
src/backends/bubblewrap/common/src/bwrap_command.rs:149
- The warning omits host rules as a reason the sandbox shares the host namespace, even though
uses_private_netnsrequires both host lists to be empty and the new host-rules test reaches this branch. For a request withblockedHosts, the suggested “block with no proxy” configuration still does not apply--unshare-net, so the remediation is misleading. Mention host rules in both the cause and remedy.
"WARNING: Bubblewrap: network.allowLocalNetwork=false is not enforced while the \
sandbox shares the host network namespace (defaultPolicy='allow' or network.proxy). \
The sandboxed process can still bind, listen and accept on host-local addresses. For \
an unreachable sandbox use defaultPolicy='block' with no proxy, which applies \
--unshare-net.",
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
📖 Description
network.allowLocalNetwork— "the sandboxed process maybind()/listen()on local IPs and accept inbound connections" — is a first-class part of the policy surface. It is declared inContainerPolicy(src/core/wxc_common/src/models.rs), parsed byconfig_parser.rs, carried on the wire (src/core/wxc_common/src/wire.rs), and honored by two of the three backends:privateNetworkClientServercapability (src/core/mxc_engine/src/policy.rs).(allow network-inbound (local ip))(src/backends/seatbelt/common/src/profile_builder.rs).grep -rn "allow_local_network" src/backends/bubblewrap/returned zero hits. On Linux the value was validated, transported to the backend, and then dropped on the floor — a silent no-op in a security-relevant control, which is worse than an outright rejection because the policy readback still shows the field as requested.Why this isn't enforced instead
Bubblewrap has no inbound-only primitive:
NetworkIptablesManageralready skips itsFORWARDhook when there is no veth, precisely to avoid applying host-wide rules.sockaddrargument tobind(), so an AF_INET-only filter is not expressible. Blockingbindoutright would also kill AF_UNIX sockets; blockinglistenwould still miss UDP servers.The namespace choice alone therefore decides the outcome, which leaves two mismatches — note the second is in the restrictive direction and was not previously identified:
allowLocalNetworkfalse(default)--unshare-net)falsedefaultPolicy: allow, host lists, ornetwork.proxy)true--unshare-net)trueChange
local_network_diagnostic()inbwrap_command.rsreturns the mismatch for the two unhonorable rows;bwrap_runner::spawn_bwraplogs it as aWARNING:at preflight, so the failure is loud rather than silent.--unshare-netpredicate intouses_private_netns(), shared withbuild_args_classified, so the diagnostic and the argument builder cannot drift apart.docs/bwrap-support/bubblewrap-backend.md.Deliberately a warning, not a rejection.
falseis the default, so rejecting the shared-namespace case would break everydefaultPolicy: "allow"run. Andtrue+--unshare-netis legitimate when a sandbox both serves and connects to its own loopback.Incidental doc fix
The backend doc claimed
--unshare-netleaves "no network stack ... (including loopback)". That is wrong: bubblewrap callsloopback_setup()when unsharing the network namespace, solois up inside the sandbox. The doc now says so, and the truth table depends on it being accurate.Limitations
This does not make Bubblewrap enforce the field — it makes the gap observable. Enforcement would need a privileged network namespace with a veth pair (i.e. the LXC model), which is out of scope for an unprivileged backend.
🔗 References
loopback_setup()on--unshare-net: containers/bubblewrapbubblewrap.c🔍 Validation
Automated (run from
src/; the Bubblewrap runner is#[cfg(target_os = "linux")], so the Linux target was cross-checked from a macOS host):All four pass, with the same results captured as a baseline before the change (no regressions).
Six new unit tests cover the truth table exhaustively. They live in
bwrap_command, which is deliberately platform-agnostic, so they execute on every host:local_network_denied_under_private_netns_is_not_warnedlocal_network_denied_on_shared_netns_warnslocal_network_denied_with_host_rules_warnslocal_network_denied_with_proxy_warnslocal_network_allowed_under_private_netns_warnslocal_network_allowed_on_shared_netns_is_honoredNot yet run:
tests/scripts/run_bwrap_all_tests.shon a Linux host withbwrapinstalled, to observe the warning end-to-end. The logged line itself is compile-verified for the Linux target but not executed.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md) — no dependency changes📋 Issue Type
Microsoft Reviewers: Open in CodeFlow