Skip to content

[Bubblewrap] Enforce network.enforcementMode='firewall' and reject unhonorable network elements at schema 0.8+ - #956

Merged
Soham Das (SohamDas2021) merged 7 commits into
mainfrom
user/sodas/bwrap-firewall-enforce
Aug 21, 2026
Merged

Soham Das (SohamDas2021) merged 7 commits into
mainfrom
user/sodas/bwrap-firewall-enforce

Conversation

@SohamDas2021

@SohamDas2021 Soham Das (SohamDas2021) commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Makes network.enforcementMode: "firewall" genuinely enforce on Bubblewrap at schema 0.8.0-alpha+, and fails closed on the network elements the backend still cannot honor.

How enforcement works

The sandbox gets its own network namespace, and a supervisor holding namespace-scoped CAP_NET_ADMIN (unshare --user --map-current-user --keep-caps) programs iptables on OUTPUT inside that namespace. No FORWARD hook to miss, no veth to discover, no root, no broker, no elevation. The sandbox cannot alter the rules, because it drops CAP_NET_ADMIN before the workload runs.

This narrows the D2 claim in docs/sandbox-policy/0.8.0/networking/networking.md that iptables filtering requires elevation — true only for filters installed on the host. A blockquote documenting the Bubblewrap exception is included.

ResolvedNetworkMode classifies each request once and drives everything downstream:

Mode Namespace Host lists
FirewallEnforced private filtered by address in the sandbox's own namespace
ProxyOnly private egress closed to all but the proxy endpoint, so the lists are enforced at the proxy
Isolated private n/a — no lists
Shared, LegacyProxy, FirewallFiltered shared / legacy not applied

enforces_host_rules() derives the "are these lists actually applied?" answer from that classification rather than from any single flag, which is what lets the gates below be stated once and stay correct as modes are added.

What is now rejected at 0.8+

Pre-0.8 keeps the existing WARNING: log, unchanged.

  • Host lists under a mode that does not apply them. Host lists suppress Isolated, so a mode that ignores them leaves the sandbox on the host namespace, unfiltered.
  • An allowLocalNetwork the resolved namespace cannot honor. The field is inbound-only; Bubblewrap has no inbound-only primitive, so the namespace choice decides the outcome and a mismatch is reported rather than dropped silently. Rejected on the value, not on whether the caller wrote the field: false is the schema default and a deny, so an omitted allowLocalNetwork is still a request for inbound denial and a bare defaultPolicy: "allow" does not silently opt out of it. IsolationSession takes the same reading of this field. The rejection stays actionable -- a shared namespace is reachable by acknowledging the exposure with allowLocalNetwork: true, which the error text names.
  • Hostname rule addresses (D3). IP literals and CIDRs only. The sandbox resolves DNS itself, so a name can be mapped to an address the chain never authorized.
  • IPv6 CIDRs shorter than /96 that contain the IPv4-mapped range. See below.

All of these live in the Bubblewrap backend's validate(), which is the one layer every caller reaches — the CLI, the SDKs, and a Rust caller handing mxc_engine a hand-built ExecutionRequest. The parser is not involved.

Validation lives in the backend, not the parser

Per review feedback: this PR adds no lines to src/core/wxc_common/src/config_parser.rs. The file's blob is identical to the merge-base, which is easy to re-check:

git diff --stat $(git merge-base HEAD origin/main) -- src/core/wxc_common/src/config_parser.rs   # empty

schema_enforces_network_strictly, the four BWRAP_* rejection messages, and the gates themselves all live in bwrap_command.rs. The parser validates structure; the backend decides what it can enforce.

The same now holds for the rest of the cross-platform core: models.rs and network_parser.rs have no net diff in this PR either. An earlier revision carried an allow_local_network_specified presence flag through both; the gate above is value-based, so the flag had no remaining consumer and was removed with it. This PR touches only the Bubblewrap backend, docs, and tests.

This also let a whole class of bug disappear: the parser previously re-derived the namespace choice in bwrap_uses_private_netns, a hand-maintained twin of ResolvedNetworkMode::uses_private_netns that a parity test had to pin. Both the twin and the parity test are deleted — there is one implementation now, so the two cannot disagree.

The three pre-existing Bubblewrap parser checks (proxy + firewall conflict, external proxy + host lists, and the cooperative-proxy warning) are untouched — they pre-date this PR, and moving them is a separate change that should cover LXC, Seatbelt, and WSLc too, which have the same pattern.

IPv4-mapped CIDRs no longer fail open

A mapped destination like ::ffff:1.2.3.4 puts a genuine IPv4 packet on the wire, so an ip6tables rule naming it never matches — under defaultPolicy: allow, a mapped blockedHosts entry silently failed open. ::ffff:0:0/96 and narrower are now translated to their IPv4 equivalents and programmed with iptables.

Blocks shorter than /96 that contain the mapped range are rejected rather than translated. CIDRs nest or are disjoint, so any such block contains the mapped range entirely, and its IPv4 projection is always 0.0.0.0/0 — emitting both rules would quietly turn blockedHosts: ["::/0"] into "block all IPv4 too". Rejecting is fail-closed and asks the caller to write the IPv4 block explicitly.

LXC has the same hole and is not fixed here, so the doc's IPv4/IPv6 parity claim was narrowed rather than deleted.

Verification that proxy mode enforces

ProxyOnly is treated as enforcing host rules, so that claim was checked against a live sandbox rather than assumed:

NETNS=net:[4026532242]   <- private; host is 4026531833
curl --noproxy '*' <non-allowlisted>  -> BLOCKED
http://example.com/ from the host      -> 200      (control: the block is real)

EgressPlan::for_proxy accepts TCP to the proxy endpoint and then drops v4 and v6, so a client that explicitly bypasses the proxy env vars still cannot leave. Regression test: tests/configs/bubblewrap_network_proxy_host_rules.json.

Behavior changes worth review

Rejection error code. The unenforced-host-lists and allowLocalNetwork gates now surface as a runner backend_error envelope on stderr rather than a config_parse error. The message text is unchanged, and rejection still precedes both execution and the dry-run path, so nothing becomes reachable that was not before — but a consumer matching on the error code would see the difference. Flagging for GHCP.

Unparsable schema versions. The two schema-version predicates are collapsed onto schema_enforces_network_strictly (semver). A non-empty unparsable version now resolves strictly (0.8 behavior) instead of leniently, so a typo like "0.8" no longer buys pre-0.8 semantics. An empty version stays lenient by design — programmatic callers who never set one are the pre-0.8 population.

Two predicates that disagree can otherwise let a gate reject a config that the mode resolution would have made safe.

Compatibility

GHCP consumes Bubblewrap proxy mode on schema 0.6/0.7. Every restriction is 0.8+ gated; run_bwrap_network_test.sh and run_bwrap_network_proxy_test.sh re-confirm legacy behavior is unchanged.

Validation

208 bwrap_common + 848 wxc_common + 322 lxc_common unit tests, cargo fmt, cargo clippy -D warnings clean, 244 configs schema-valid, and all Bubblewrap E2E suites green via tests/scripts/run_bwrap_all_tests.sh (10 suites, 0 failures).

The E2E run matters more than usual after the parser deletion: the three moved gates previously failed at parse time and so their runner copies had never been exercised end-to-end. They now are, and assert_rejected proves each one non-zero-exits, emits the contract text, and never runs the workload.

The firewall E2E passed on its first run — exactly when a deny-assertion suite is most likely to be vacuous — so two negative controls were used to prove the assertions were real, and the precondition guard skips rather than false-greening on an offline runner.

Follow-ups (not in scope)

Related Issues

📋 Issue Type

  • Bug fix
  • Feature
  • Task

GitHub Actions runs the PR validation build automatically. The ADO pipeline
(MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.

Microsoft Reviewers: Open in CodeFlow

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

Adds rootless Bubblewrap firewall enforcement for schema 0.8+ using in-namespace iptables, with stricter validation for unsupported network policies.

Changes:

  • Introduces reusable egress plans and private-network firewall enforcement.
  • Adds schema-aware policy validation and compatibility behavior.
  • Expands Bubblewrap documentation and network tests.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/backends/bubblewrap/common/src/network_rules.rs Models and renders egress rules.
src/backends/bubblewrap/common/src/proxy_network.rs Generalizes namespace firewall setup.
src/backends/bubblewrap/common/src/bwrap_command.rs Resolves schema-specific network modes.
src/backends/bubblewrap/common/src/bwrap_runner.rs Validates and launches enforced networking.
src/backends/bubblewrap/common/src/lib.rs Registers the rule module.
src/core/wxc_common/src/config_parser.rs Adds strict schema gates.
tests/scripts/run_bwrap_firewall_test.sh Adds firewall E2E coverage.
tests/scripts/run_bwrap_all_tests.sh Runs the new firewall suite.
tests/scripts/run_bwrap_network_test.sh Checks legacy compatibility.
tests/scripts/run_bwrap_network_proxy_test.sh Tests proxy-confined egress.
tests/configs/bubblewrap_network_firewall_cidr.json Exercises CIDR allowlisting.
tests/configs/bubblewrap_network_firewall_denylist.json Exercises deny precedence.
tests/configs/bubblewrap_network_firewall_hostname_rejected.json Tests hostname rejection.
tests/configs/bubblewrap_network_hostrules_unenforced_rejected.json Tests unsupported host rules.
tests/configs/bubblewrap_network_proxy_host_rules.json Tests proxy host filtering.
docs/bwrap-support/bubblewrap-backend.md Documents Bubblewrap enforcement.
docs/sandbox-policy/0.8.0/networking/networking.md Records the rootless firewall design.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/backends/bubblewrap/common/src/bwrap_command.rs
Comment thread src/backends/bubblewrap/common/src/network_rules.rs Outdated
Comment on lines +184 to 186
if proxy_only || firewall_enforced {
if let Err(error) = proxy_network::probe_dependencies() {
return Err(ScriptResponse::error(&error));

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.

Tracked in #955

Comment thread src/backends/bubblewrap/common/src/bwrap_runner.rs
Comment on lines +48 to +50
fn rule_install_timeout(plan: &EgressPlan) -> Duration {
Duration::from_secs(XTABLES_LOCK_WAIT.as_secs() * plan.command_count() as u64)
}

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.

Tracked in #955

Comment thread tests/scripts/run_bwrap_firewall_test.sh Outdated
Copilot AI review requested due to automatic review settings August 19, 2026 21:52

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

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (3)

docs/bwrap-support/bubblewrap-backend.md:254

  • This legacy-path description contradicts the actual failure behavior and the new compatibility test. NetworkIptablesManager::run_firewall_command returns an error when unprivileged iptables fails (network_iptables.rs:1584-1592), and spawn_bwrap aborts on that error (bwrap_runner.rs:424-444), so an unprivileged sandbox does not continue unrestricted. The unrestricted case occurs when rule creation succeeds but the no-veth path skips the FORWARD hook. Please distinguish those outcomes.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is
retained unchanged for existing callers and is the reason enforcement is 0.8+

docs/bwrap-support/bubblewrap-backend.md:230

  • This new Bubblewrap 0.8 restriction leaves the public Node SDK contract inaccurate: sdk/node/src/types.ts:208-215 still says firewall lists accept hostnames, while sdk/node/src/helper.ts:133-138 automatically selects firewall for any host list. A documented allowedHosts: ['api.github.com'] request will therefore be promoted and rejected by this code. Update the SDK JSDoc/README to explain that Bubblewrap 0.8 firewall entries must be IP/CIDR and that hostname filtering requires a proxy; keep the broader wording for backends that still resolve names.
Rule addresses must be **IP literals or CIDR blocks**; a DNS name is rejected
at validation time rather than resolved on the caller's behalf. The backend
does not resolve, because the sandbox resolves names itself and a lookup that
disagreed with the one behind the rules would hand the workload an address the
chain never authorized. Both families are covered — an IPv6 rule programs

src/backends/bubblewrap/common/src/lib.rs:15

  • The repository architecture guide is now stale: .github/copilot-instructions.md:188 still says Bubblewrap per-host filtering uses NetworkIptablesManager and describes the private namespace as proxy-only. This PR adds network_rules and moves schema-0.8 firewall filtering into that private namespace, which is exactly the kind of architecture change the repository instructions require documenting. Update the Bubblewrap table entry in the same PR.
//! - `network_rules` validates rule addresses and builds the egress plan the
//!   sandbox's own network namespace is programmed with. Platform-agnostic for
//!   the same reason as [`bwrap_command`].

Copilot AI review requested due to automatic review settings August 19, 2026 22:15
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 0d0017b to 0da2812 Compare August 19, 2026 22:15

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

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tests/scripts/run_bwrap_all_tests.sh:43

  • This only registers the new live firewall suite in run_bwrap_all_tests.sh, but that aggregate suite is not run by pull-request validation: scripts/ci/validation-test-matrix.json:329 has an empty pr plan, while .github/workflows/Build.Linux.Job.yml:127-131 invokes only the proxy-network E2E directly. As a result, the in-namespace firewall enforcement can merge without this E2E ever running. Add run_bwrap_firewall_test.sh to the Linux PR build alongside the proxy test (or enable Bubblewrap in the PR validation plan).
run_test "Bubblewrap Network Firewall" "$SCRIPT_DIR/run_bwrap_firewall_test.sh"

docs/bwrap-support/bubblewrap-backend.md:257

  • The unprivileged legacy path does not continue with unrestricted networking when iptables installation fails: bwrap_runner.rs:424-445 returns an error and aborts the launch. If installation succeeds (for example with privilege), the chain is still ineffective because Bubblewrap has no veth hook. Please distinguish startup failure from the separate unhooked-chain behavior so users are not told a failed launch will run unrestricted.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is

Copilot AI review requested due to automatic review settings August 20, 2026 07:18
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 0da2812 to 471830e Compare August 20, 2026 07:18

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

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/backends/bubblewrap/common/src/bwrap_command.rs:114

  • Introducing FirewallEnforced leaves developer-facing documentation for this path contradictory. bwrap_command.rs:365-367 still says per-host firewall mode shares the host namespace, while sdk/node/src/helper.ts:93-117 and sdk/node/src/types.ts:195-197 still describe firewall enforcement as privileged/LXC-or-Windows-only and direct Bubblewrap users to proxy mode. Update those comments/JSDoc so SDK users can discover the new unprivileged Bubblewrap firewall behavior and its IP/CIDR-only constraint.
    /// Per-destination iptables filtering inside a slirp-backed private
    /// namespace, with no proxy. 0.8+ only.
    FirewallEnforced,

docs/bwrap-support/bubblewrap-backend.md:257

  • The legacy behavior description reverses the privilege outcomes. If NetworkIptablesManager::apply_firewall_rules cannot install rules, bwrap_runner.rs:430-444 aborts the spawn; it does not continue unrestricted. The unrestricted case is when installation succeeds but the host chain is not in the sandbox packet path. Please describe the startup failure and the ineffective-chain case separately.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is

@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 471830e to 2100831 Compare August 20, 2026 08:21
Copilot AI review requested due to automatic review settings August 20, 2026 18:05

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

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tests/scripts/run_bwrap_firewall_test.sh:105

  • This treats every network policy error as an expected privilege failure, but bwrap_runner applies that prefix to every apply_firewall_rules error, including DNS resolution and general setup/command failures. Those regressions would therefore false-pass this compatibility test. Match only permission-specific diagnostics here.
elif grep -qE "failed to apply iptables firewall rules|network policy error" <<<"$LEGACY_OUT"; then

docs/bwrap-support/bubblewrap-backend.md:257

  • The unprivileged legacy path does not start an unrestricted sandbox: firewall setup runs before bwrap is spawned, and an iptables permission error aborts the launch. The unrestricted run occurs when privileged setup succeeds but installs an unhooked host chain. Please distinguish those outcomes so the compatibility documentation matches the runner and the new legacy test.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is
retained unchanged for existing callers and is the reason enforcement is 0.8+

Comment thread src/backends/bubblewrap/common/src/network_rules.rs Outdated
Copilot AI review requested due to automatic review settings August 20, 2026 20:00

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

Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

docs/bwrap-support/bubblewrap-backend.md:33

  • Firewall enforcement now enters the same slirp/namespace path and calls the same dependency probe, so these prerequisites are also required when network.proxy is omitted but schema 0.8 firewall mode has host rules. The current “proxy mode only” wording gives firewall users incorrect installation guidance.
- **Schema 0.8 proxy mode only:** `slirp4netns` installed and on PATH, plus
  `nsenter`, `iptables`, and `ip6tables` for the proxy-only egress rules. None
  are required when `network.proxy` is omitted or when a 0.6/0.7 policy uses
  the legacy proxy behavior.

docs/bwrap-support/bubblewrap-backend.md:265

  • The unprivileged legacy path does not run the sandbox unrestricted: host-side apply_firewall_rules fails and spawn_bwrap returns an error before spawning Bubblewrap. The unrestricted outcome occurs on a privileged run, where the chain can be created but is not hooked into the sandbox packet path. Please distinguish these two outcomes so the compatibility behavior is documented accurately.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is
retained unchanged for existing callers and is the reason enforcement is 0.8+

.github/copilot-instructions.md:188

  • This architecture summary still says Bubblewrap per-host filtering uses NetworkIptablesManager and describes the private namespace only for proxy mode. In this PR, schema 0.8 firewall requests use ProxyNetworkNamespace and in-namespace OUTPUT rules; only the pre-0.8 compatibility path retains the host manager. Update this repository guidance so future changes are based on the new dispatch model.
| Bubblewrap | `lxc-exec` | Linux | `backends/bubblewrap/common/src/bwrap_runner.rs` — unprivileged sandboxing via Linux user namespaces and `bwrap`. Experimental — requires `--experimental`. Uses shared filesystem/network policy fields; per-host network filtering via `NetworkIptablesManager` from `backends/lxc/common`. For schema 0.8+, proxy mode uses a private network namespace with rootless `slirp4netns` routing and a default-DROP egress chain that permits only loopback and the translated proxy endpoint (installed via `nsenter` + `iptables`/`ip6tables` from a supervisor that holds the namespaces); it requires `slirp4netns`, util-linux `unshare`/`nsenter` and `iptables`/`ip6tables` on PATH and fails validation if any is unavailable. `iptables`/`ip6tables` must also resolve to the `nf_tables` backend, unless `/run/xtables.lock` is writable by the calling user: the legacy backend opens that lock unconditionally, and the rules are installed by an unprivileged same-uid supervisor that cannot open a root-owned one — `validate` refuses such a host rather than letting the supervisor die at the first rule. The host proxy endpoint is rewritten to slirp's gateway `10.0.2.2`, so `127.0.0.1`/`0.0.0.0`/`::` are translated while `::1` is rejected (an IPv6-loopback listener cannot accept the gateway's IPv4 connection). Schema 0.6/0.7 and absent-version requests retain the legacy shared-network proxy behavior. See `docs/bwrap-support/bubblewrap-backend.md`. |

Comment thread src/backends/bubblewrap/common/src/proxy_network.rs Outdated
Copilot AI review requested due to automatic review settings August 20, 2026 20:58
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 52bac7c to 57191b5 Compare August 20, 2026 20:58

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

Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (2)

docs/bwrap-support/bubblewrap-backend.md:285

  • This compatibility description reverses the actual unprivileged behavior. apply_firewall_rules runs before bwrap is spawned, so an unprivileged iptables failure aborts the launch; unrestricted execution occurs only when a privileged caller successfully installs the orphan host chain that Bubblewrap traffic never traverses. Please distinguish those two outcomes.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is
retained unchanged for existing callers and is the reason enforcement is 0.8+

tests/configs/bubblewrap_network_proxy_host_rules.json:6

  • This regression check can false-green: proxy-only mode intentionally blocks DNS, so curl --noproxy "*" http://example.com/ fails before it proves that direct IP egress is filtered. Resolve a reachable deny target outside the sandbox (and verify it is reachable without filtering), then make the direct attempt against that literal address, similar to the negative control in run_bwrap_firewall_test.sh.
    "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); if curl -fsS --noproxy \"*\" --max-time 8 http://example.com/ >/dev/null 2>&1; then echo DIRECT_EGRESS_LEAKED; exit 1; fi; echo DIRECT_EGRESS_BLOCKED_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null 2>&1; then echo PROXY_PATH_UNREACHABLE; exit 1; fi; echo PROXY_PATH_OK'"

Copilot AI review requested due to automatic review settings August 21, 2026 05:12
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 38f8950 to 316c057 Compare August 21, 2026 05:12

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

Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (4)

tests/scripts/run_bwrap_network_test.sh:45

  • This compatibility check only recognizes parser failures, but the new 0.8 gates intentionally reject from BubblewrapScriptRunner::validate() and surface as backend errors. A leaked hostname/host-rule gate would therefore make this schema-0.6 run fail while the script still prints PASS. Match the backend contract fragments too, as the dedicated firewall suite does.
if echo "$LEGACY_OUTPUT" | grep -qF "Configuration parse error"; then
    echo "$LEGACY_OUTPUT"
    echo "FAIL: schema 0.6 firewall mode failed config validation (status $LEGACY_STATUS)."

tests/configs/bubblewrap_network_proxy_host_rules.json:6

  • This deny assertion uses a different destination from the successful proxy control, so it can pass because example.com is unavailable even if direct egress is leaking. Use api.github.com/zen for both attempts: the proxy success then proves that exact destination is reachable, while --noproxy still verifies that proxy-only mode blocks the direct path.
    "commandLine": "bash -c 'set -u; echo SANDBOX_NETNS=$(readlink /proc/self/ns/net); if curl -fsS --noproxy \"*\" --max-time 8 http://example.com/ >/dev/null 2>&1; then echo DIRECT_EGRESS_LEAKED; exit 1; fi; echo DIRECT_EGRESS_BLOCKED_OK; if ! curl -fsSL --max-time 15 https://api.github.com/zen >/dev/null 2>&1; then echo PROXY_PATH_UNREACHABLE; exit 1; fi; echo PROXY_PATH_OK'"

docs/bwrap-support/bubblewrap-backend.md:298

  • This IPv4-only statement is no longer true. NetworkIptablesManager::resolve_host retains IPv6 literals and resolves hostnames to both A and AAAA records (network_iptables.rs:765-845), then programs the corresponding iptables/ip6tables rules. The legacy Bubblewrap problem is that its traffic never traverses that host-side chain, not that IPv6 inputs are discarded.
> **Legacy path, IPv4 only.** On schema ≤ 0.7, host names are resolved to
> IPv4 addresses only; AAAA records and IPv6 literals are silently dropped
> because `iptables` (the IPv4 tool) cannot accept IPv6 destinations. A host
> with only AAAA records is effectively unreachable. Moving to 0.8 programs
> `ip6tables` as well, but does not make such a host reachable while the

docs/bwrap-support/bubblewrap-backend.md:291

  • The unprivileged legacy path does not run with unrestricted access: apply_firewall_rules() returns an error and bwrap_runner.rs:442-455 aborts before spawning the workload. Only a privileged setup can continue (with the pre-existing unattached-chain behavior), so this newly added compatibility documentation gives users the wrong failure semantics.
**Schema 0.7 and earlier — accepted but not enforced.** The legacy path shares
the host network namespace and applies rules to the *host* via
`NetworkIptablesManager`, which **requires root**; unprivileged, the rules are
not installed and the sandbox runs with unrestricted network access. This is
retained unchanged for existing callers and is the reason enforcement is 0.8+

… in-namespace iptables rules

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3
…en the legacy firewall case

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

Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/bwrap-firewall-enforce branch from 316c057 to 1bdee51 Compare August 21, 2026 07:20

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

Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.

Comment thread src/backends/bubblewrap/common/src/proxy_network.rs Outdated
Comment thread tests/scripts/run_bwrap_firewall_test.sh Outdated
Comment thread docs/bwrap-support/bubblewrap-backend.md Outdated
allowLocalNetwork=false under a shared namespace is refused at 0.8 because
Bubblewrap cannot deliver the deny. That refusal was gated on a parse-time
allow_local_network_specified flag, so it fired only for callers who wrote
the field. false is the schema default *and* a deny, so an omitted field is
still a request for inbound denial: a bare defaultPolicy='allow' ran with
the deny silently unenforced. Default-deny is the contract, and a backend
that cannot honor a restriction refuses it rather than choosing
default-allow on the caller's behalf.

IsolationSession already takes the strict reading of this exact field:
validate_provision_network_policy requires allow_local_network by value and
refuses an absent policy as "a restriction the backend cannot honor". The
guard is now dropped so Bubblewrap agrees, and the flag it existed for goes
with it -- that guard was its only production consumer, so models.rs and
network_parser.rs leave this change with no net diff.

The rejection stays actionable: a shared namespace is still reachable by
acknowledging the exposure with allowLocalNetwork=true, which the error text
now names. No existing config changes behavior -- both shared-namespace
corpus configs set the field explicitly, and every other 0.8 Bubblewrap
config resolves to a private namespace via a proxy or host rules.

Coverage: the guard had none. Deleting it outright left all 202 tests
passing, because the one test that looked like it covered this used
defaultPolicy='block', whose private namespace short-circuits the mode arm.
Two unit tests now pin the omitted and acknowledged cases (204 total; a
reintroduced guard fails three), and two e2e configs pin them end to end,
the acknowledged one asserting the sandbox really is in the host netns so
"reject the deny" cannot be satisfied by refusing every shared-namespace
request.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ae9db9df-6439-46c3-9cb4-fd70e7cc43a3

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

Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.

@SohamDas2021
Soham Das (SohamDas2021) merged commit d3f3236 into main Aug 21, 2026
24 checks passed
@SohamDas2021
Soham Das (SohamDas2021) deleted the user/sodas/bwrap-firewall-enforce branch August 21, 2026 19:44
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.

3 participants