[Bubblewrap/LXC] Address network policy gaps - schema by dhoehna · Pull Request #634 · microsoft/mxc

dhoehna · 2026-07-10T22:56:13Z

Linked work item: AB#62830582 — [Bubblewrap/LXC] Address network policy gaps - schema

Summary

Adds the GA network policy schema and migration for LXC/Bubblewrap.

New wire schema under network: egress.{allow,deny}[] (each to: [{ cidr }], ports: [{ protocol, port }]), ingress.hostLoopback, and proxy.http.
Parser maps GA config to the internal model: ingress.hostLoopback → allow_local_network, proxy.http → network_proxy, egress.* → internal EgressRules.
CIDR-only: every destination is validated with std::net; DNS hostnames are rejected with a clear parse error.
Legacy allowedHosts / blockedHosts / defaultPolicy continue to work. Precedence: GA network.egress supersedes the legacy list fields; ingress.hostLoopback overrides allowLocalNetwork.
Added GA SDK policy types; regenerated JSON schema + TS wire types.

Model

Adds EgressRule, Protocol, RuleAction to wxc_common::models.

Validation

cargo fmt --all -- --check; cargo clippy -p wxc_common -D warnings — clean
cargo test -p wxc_common — 403 passed; cargo test -p mxc-sdk — 23 passed
cargo check --target x86_64-unknown-linux-gnu -p wxc_common — pass

Coupling

The internal EgressRule / Protocol / RuleAction types are also added independently by the enforcement PRs (AB#62830559, AB#62830341). This PR owns the JSON parser; the enforcement PRs consume the model field. Merge-time reconciliation is expected.

Microsoft Reviewers: Open in CodeFlow

Copilot

Pull request overview

This PR extends MXC’s network configuration surface with a GA schema for LXC/Bubblewrap, adding structured egress/ingress policy and an HTTP proxy field, then wiring that schema through the Rust wire model → parser → internal policy model and the SDK policy layer (including regenerated schema + TS wire types).

Changes:

Added GA network.egress (allow/deny rules), network.ingress.hostLoopback, and network.proxy.http to the Rust wire model and dev JSON schema.
Mapped GA network fields in config_parser into new internal EgressRule/Protocol/RuleAction model fields with validation (CIDR-only destinations).
Updated SDK policy mapping + tests and regenerated sdk/src/generated/wire.ts.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/core/wxc_common/src/wire.rs	Adds GA network wire schema structs/enums (egress/ingress/proxy.http).
src/core/wxc_common/src/models.rs	Introduces internal egress rule model types and stores them on `ContainerPolicy`.
src/core/wxc_common/src/config_parser.rs	Parses/validates GA network fields and maps them into the internal model with legacy precedence rules.
src/core/mxc-sdk/tests/sdk_helpers.rs	Adjusts test helper construction for the expanded network policy struct.
src/core/mxc-sdk/src/policy.rs	Adds SDK GA network policy types and emits GA schema in wire config when present.
sdk/src/generated/wire.ts	Regenerated TS wire types to include the new GA network schema.
schemas/dev/mxc-config.schema.0.8.0-dev.json	Regenerated dev JSON schema with GA network types and fields.

…nly (AB#62830582) - Add GA network schema (egress.allow/deny with cidr+ports+protocol, ingress.hostLoopback, proxy.http) to wire + parser + internal EgressRule model + SDK types. - Reject DNS names (CIDR-only); keep legacy allowedHosts/blockedHosts/defaultPolicy working. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b78bec0-e139-4cfd-9c10-092ef986d4f4

…col (AB#62830582) convert_wire_egress_rules pushed each GA port selector's port and protocol into two independent Vecs on one EgressRule. Since the internal EgressRule treats protocols x ports as a cartesian grid, a rule mixing selectors (e.g. tcp/80 + udp/53) widened to also permit tcp/53 and udp/80. Group selectors by protocol (first-seen order, deduped ports) and emit one EgressRule per protocol group so each rule's cartesian expansion equals exactly the requested selectors. Adds a regression test. Addresses copilot-pull-request-reviewer feedback on PR microsoft#634. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SohamDas2021 · 2026-07-14T17:25:01Z

+    pub protocol: NetworkProtocol,
+    /// Destination port.
+    #[cfg_attr(feature = "schema-gen", schemars(range(min = 1, max = 65535)))]
+    pub port: u16,


You are making port mandatory, but ICMP has no ports. The GA schema (see here- https://microsoft-my.sharepoint-df.com/:w:/p/bbonaby/cQpR4CPfeKqgSLuQGG_a9QA2EgUCrPdXr5J7b-jWip1_VeYFUA) makes ports optional and omits it for icmp. Please make ports optional and reject port when protocol is icmp.

Good catch — fixed in 70c94d6. EgressPortWire.port is now Option<u16> (optional in the schema), and the parser rejects a port paired with protocol: "icmp" with a clear message (… specifies a port for protocol 'icmp', which has no ports; omit 'port' for icmp). An ICMP selector is now written as { "protocol": "icmp" }. Covered by the new ga_egress_rejects_port_with_icmp test and the updated ga_egress_accepts_ipv4_ipv6_cidr_and_bare_ip test.

SohamDas2021 · 2026-07-14T17:27:48Z

+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))]
+#[serde(rename_all = "camelCase", deny_unknown_fields)]
+pub struct NetworkEgress {


The GA schema also has egress.default: deny|allow. Without it there's no way to express model‑1 "allow everything except this deny‑list" (D4's canonical case). Combined with the parser dropping legacy defaultPolicy when egress is present, a user who writes defaultPolicy:"allow" + egress is silently downgraded to default‑deny. Please add egress.default .

Added in 70c94d6. network.egress now accepts default: "allow" | "deny" (wire EgressDefault). It maps to the internal default_network_policy (allow → Allow; deny or omitted → Block, i.e. fail-closed), so the "allow everything except this deny-list" model is expressible and a legacy defaultPolicy: "allow" is no longer silently downgraded when GA egress is present. New tests: ga_egress_default_allow_sets_default_network_policy and ga_egress_default_deny_and_omitted_both_fail_closed.

SohamDas2021 · 2026-07-14T17:29:27Z

+                "{path}[{rule_index}].to must include at least one cidr entry"
+            )));
+        }
+        if rule.ports.is_empty() {


Requiring at least one ports entry per rule means "allow all ports to this CIDR" is inexpressible. The GA schema treats ports as optional (omitted = all ports/protocols). Please allow an empty/absent ports to mean match‑all.

Fixed in 70c94d6. EgressRuleWire.ports is now #[serde(default)] (optional). An omitted/empty ports list means match-all — all ports and all protocols to the listed destinations (internally an EgressRule with empty ports and empty protocols). A portless tcp/udp selector such as { "protocol": "tcp" } matches all ports for that protocol. New tests: ga_egress_omitted_ports_matches_all_protocols_and_ports and ga_egress_portless_tcp_selector_matches_all_ports.

SohamDas2021 · 2026-07-14T17:32:46Z

    /// Proxy URL (parsed into host:port).
    pub url: Option<String>,
+    /// GA HTTP proxy URL (parsed into host:port).
+    pub http: Option<String>,


The GA spec puts the proxy in runtimeConfig.networkProxy (a URL, outside the network policy block), not network.proxy.http, right?

Thanks — I'd like to defer the runtimeConfig.networkProxy relocation to a follow-up rather than fold it into this PR. Two reasons:

Scope / risk. There is no runtimeConfig section anywhere in the wire model today, so this introduces a brand-new top-level config surface that is cross-cutting — schema, SDK public types, generated wire types, and every backend's proxy handling. That is meaningfully broader than this PR's "network policy schema" scope.

Spec fidelity. Doing it correctly needs the canonical GA runtimeConfig.networkProxy spec (exact shape, precedence vs. the existing network.proxy, and which backends honor it). I don't want to guess the shape and bake in something we then have to break.

This PR intentionally leaves the existing network.proxy.http surface unchanged. I'll open a tracking issue for the runtimeConfig.networkProxy migration and link it here. If you have a link to the GA spec you'd like followed, I'm happy to do the relocation in a dedicated follow-up PR.

SohamDas2021 · 2026-07-14T17:35:02Z

        {
-            let msg = "Bubblewrap: an external network.proxy (url/localhost) cannot be \
-                       combined with allowedHosts, blockedHosts, or defaultPolicy='block'. \
+            let msg = "Bubblewrap: an external network.proxy (url/localhost/http) cannot be \


This rejects an external proxy combined with egress for Bubblewrap only. The GA doc says egress.allow/deny should be rejected under proxy mode generally; on LXC and other backends both can currently be set at once. Should this consistency check be backend‑agnostic?

Fixed in 70c94d6. The external-proxy + egress rejection is now backend-agnostic: it runs in convert_wire_config for all backends, guarded by network_proxy.is_enabled() && !builtin_test_server && !egress_rules.is_empty(), with a message that network.egress can't be combined with an external network.proxy because MXC does not forward egress rules to the proxy. The Bubblewrap-specific check no longer mentions egress (that responsibility moved up). New test external_proxy_with_egress_is_rejected_backend_agnostic exercises this using processcontainer and bubblewrap.

Implements the actionable reviewer feedback on the GA egress/proxy schema: - ICMP has no ports: make EgressPort.port optional and reject a port paired with protocol 'icmp' (wire.rs, config_parser.rs). - Add egress.default (allow|deny): lets the 'allow-all-except-deny-list' model be expressed and prevents a legacy defaultPolicy:'allow' from being silently downgraded when GA egress is present. Maps to the internal default_network_policy; omitted/deny fail closed (wire.rs, config_parser.rs). - Optional rule ports: an omitted/empty ports list now means match-all (all ports and all protocols) to the listed destinations, so 'allow all ports to X' is expressible. A portless tcp/udp selector matches all ports for that protocol (config_parser.rs). - Make the external-proxy + egress rejection backend-agnostic instead of Bubblewrap-only, since MXC never forwards egress rules to a proxy (config_parser.rs). Regenerates the dev JSON schema and SDK wire types from the wire model, updates the hand-written SDK public types (types.ts) and the mxc-sdk policy port, and adds/updates unit tests. The proxy-placement thread (runtimeConfig.networkProxy) is deferred and answered on the PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings July 10, 2026 22:56

dhoehna requested a review from a team as a code owner July 10, 2026 22:56

Copilot started reviewing on behalf of dhoehna July 10, 2026 22:57 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread src/core/wxc_common/src/config_parser.rs Outdated

dhoehna force-pushed the user/dahoehna/lxc-net-schema-migration branch from 6108846 to f86ebb1 Compare July 13, 2026 16:45

SohamDas2021 requested changes Jul 14, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned dhoehna Jul 14, 2026

microsoft-github-policy-service Bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026

microsoft-github-policy-service Bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Bubblewrap/LXC] Address network policy gaps - schema#634

[Bubblewrap/LXC] Address network policy gaps - schema#634
dhoehna wants to merge 3 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-net-schema-migration

dhoehna commented Jul 10, 2026 •

edited by microsoft-github-policy-service Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

SohamDas2021 Jul 14, 2026

Uh oh!

dhoehna Jul 14, 2026

Uh oh!

SohamDas2021 Jul 14, 2026

Uh oh!

dhoehna Jul 14, 2026

Uh oh!

SohamDas2021 Jul 14, 2026

Uh oh!

dhoehna Jul 14, 2026

Uh oh!

SohamDas2021 Jul 14, 2026

Uh oh!

dhoehna Jul 14, 2026

Uh oh!

SohamDas2021 Jul 14, 2026

Uh oh!

dhoehna Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dhoehna commented Jul 10, 2026 • edited by microsoft-github-policy-service Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Model

Validation

Coupling

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhoehna commented Jul 10, 2026 •

edited by microsoft-github-policy-service Bot

Loading