[WSLC] Fix network policy parity between one-shot and state-aware surfaces - #1042
[WSLC] Fix network policy parity between one-shot and state-aware surfaces#1042Soham Das (SohamDas2021) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Aligns WSLc one-shot and state-aware network-policy behavior.
Changes:
- Rejects all unsupported WSLc host lists.
- Adds state-aware provision-time TCP port mappings.
- Improves state-aware proxy validation routing and test coverage.
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/scripts/run_wslc_state_aware_tests.ps1 |
Adds port-forwarding E2E coverage. |
tests/configs/wslc_state_aware_provision_ports.json |
Adds a port-mapping fixture. |
src/core/wxc_common/src/wire.rs |
Extends the provision wire model. |
src/core/wxc_common/src/models.rs |
Adds host-list presence detection. |
src/core/wxc_common/src/config_parser.rs |
Updates WSLc policy validation. |
src/core/wxc_common/src/config_contract_adapters/dev/state_aware.rs |
Converts state-aware port mappings. |
src/core/wxc_common/src/config_contract_adapters/dev/one_shot.rs |
Reuses shared mapping conversion. |
src/core/wxc_common/src/config_contract_adapters/dev/common.rs |
Centralizes mapping conversion. |
src/core/mxc_config_contract/src/dev/state_aware/provision/wslc.rs |
Adds port mappings to the contract. |
src/core/mxc_config_contract/src/dev/state_aware/provision/mod.rs |
Boxes the larger WSLc request. |
src/core/mxc_config_contract/src/dev/request.rs |
Constructs the boxed request. |
src/backends/wslc/daemon/tests/daemon_ipc.rs |
Updates daemon test configuration. |
src/backends/wslc/daemon/src/session_manager.rs |
Applies mappings during provisioning. |
src/backends/wslc/common/src/wsl_container_runner.rs |
Rejects every host-list form. |
src/backends/wslc/common/src/state_aware.rs |
Validates and forwards mappings. |
src/backends/wslc/common/src/daemon_protocol.rs |
Extends and versions daemon IPC. |
src/backends/wslc/common/src/container_steps.rs |
Passes mappings to the SDK. |
sdk/node/tests/unit/wire-conformance-state-aware.test.ts |
Checks SDK wire parity. |
sdk/node/src/state-aware-types.ts |
Exposes provision port mappings. |
sdk/node/src/generated/wire.ts |
Regenerates rolling wire types. |
sdk/node/src/generated/v0_9_0_alpha/wire.ts |
Regenerates exact wire types. |
sdk/node/README.md |
Documents SDK behavior. |
schemas/dev/mxc-config.schema.0.9.0-dev.json |
Updates the rolling schema. |
schemas/dev/mxc-config.schema.0.9.0-alpha.json |
Updates the exact schema. |
docs/wsl/wslc-state-aware.md |
Documents state-aware forwarding. |
docs/wsl/wsl-container-getting-started.md |
Documents fail-closed host lists. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Darren Hoehna (dhoehna)
left a comment
There was a problem hiding this comment.
Just some questions.
22b08f1 to
61424e8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/core/wxc_common/src/config_contract_adapters/dev/state_aware.rs:107
- The exact 0.9 state-aware adapter now maps
port_mappings, but itsWSLC_ALL_FIELDS_REQUEST_JSONadapter/convergence test still omits this field and never asserts the mapped value. The only E2E added here uses 0.8 (the rolling parser), while backend unit tests constructWslcProvisionPhasedirectly, so dropping this conversion would not be detected. AddportMappingsto that all-fields fixture and assertprovision.port_mappingsto cover the exact-parser path.
port_mappings: port_mappings.into_option().map(|mappings| {
mappings
.into_iter()
.map(convert_wslc_port_mapping)
.collect()
61424e8 to
5f71a99
Compare
| in-container `iptables` rules, but a WSLC container runs **without** | ||
| `CAP_NET_ADMIN` (the SDK's `Privileged` flag does not grant it), so those rules | ||
| cannot be applied — and MXC has no VM-level enforcement hook either (WSLC | ||
| cannot expose one without breaking other security promises such as MDE). |
There was a problem hiding this comment.
question (non-blocking): what does MDE mean?
| ## Port forwarding | ||
|
|
||
| `experimental.wslc.provision.portMappings` forwards host (Windows) ports to the container, mirroring | ||
| the one-shot `experimental.wslc.portMappings` surface. Each entry is `{ windowsPort, containerPort }` |
There was a problem hiding this comment.
question: (non-blocking): so WSLc has host to container networking but specifically only related to port redirection? If so I wonder if this should mean that wslc configs that want to use port mappings need to also do network.ingress.hostloopback : allow together. Up to you though if you want to keep portmappings a completely separate exception.
| .map(|p| PortMapping { | ||
| windows_port: p.windows_port, | ||
| container_port: p.container_port, | ||
| protocol: "tcp".to_string(), |
There was a problem hiding this comment.
thought: Maybe I read wrong but I figured it was TCP or UDP?
| @@ -710,7 +710,7 @@ | |||
| "type": "string" | |||
| }, | |||
| "PortMapping": { | |||
There was a problem hiding this comment.
question: not related to this file but do we need to add anything for the Rust SDK in terms of the types consumers will use to author a wlsc config?. OR does it look like it's already taken care of?
Here: https://github.com/microsoft/mxc/blob/main/src/core/mxc_engine/src/policy.rs#L504
📖 Description
.github/copilot-instructions.md.Summary
Three WSLc network-policy gaps where the one-shot and state-aware surfaces disagreed.
1. Redundant host lists failed open. The parser gated on
needs_host_filtering(), which only inspects the list that tightens the default — soblock+blockedHostsandallow+allowedHostswere neither rejected nor enforced. Replaced withhas_host_lists(), which is presence-based and fail-closed.2.
portMappingswas one-shot only. Added the state-aware twin on the provision phase, honoured as container port forwards and frozen post-provision (TCP only; ports must be unique and non-zero).3. Two parser gates mislabelled state-aware errors. Both the host-list and
network.proxygates are written for one-shot, where a single config carries the whole policy. On state-aware the network mode is bound at provision, so later phases legitimately omitdefaultPolicy— and a proxy-carryingexec/start/stopwas rejected asmalformed_requestbefore dispatch. Both gates are now!state_aware; every state-aware phase already gates the proxy itself, so the errors surface aspolicy_validationand exec's cooperative proxy injection works. This last one was pre-existing onmain(verified against amainbuild producing byte-identical output).Validation
cargo fmt+clippy -D warnings(incl.-p wxc --features wslc)run_wslc_all_tests.ps1(one-shot + state-aware), 0 failures — including a real port-forward round-trip on a dynamically-allocated host portCloses #824
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 GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow