[WSLC] Reject unenforceable network schema at parse time - #681
[WSLC] Reject unenforceable network schema at parse time#681Soham Das (SohamDas2021) wants to merge 8 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
WSLC cannot enforce per-host egress filtering: the runner would apply in-container iptables, but the SDK's Privileged flag does not grant CAP_NET_ADMIN, so the run aborts at exec. Reject up front instead -- an allowlist with defaultPolicy='block', or a blocklist with defaultPolicy='allow'. Bare defaults with no host lists (full cutoff / full NAT) stay enforceable and are left as-is. Also reject network.allowLocalNetwork=true: a WSLC container runs in the NAT'd WSL2 VM and MXC never consults the flag (only explicit experimental.wslc portMappings wire inbound forwards), so honoring it would promise reachability the backend never delivers. Documents both rejections in the WSLC getting-started guide. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d
fcba698 to
f191dca
Compare
There was a problem hiding this comment.
Pull request overview
Adds early validation for WSLC network options that cannot be enforced.
Changes:
- Rejects effective host filtering and
allowLocalNetwork: true. - Adds parser tests for accepted and rejected combinations.
- Documents WSLC network limitations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/core/wxc_common/src/config_parser.rs |
Adds WSLC validation and tests. |
docs/wsl/wsl-container-getting-started.md |
Documents unsupported network policies. |
| WSLC **cannot** enforce per-host egress filtering. `allowedHosts` with | ||
| `defaultPolicy: "block"` (an allowlist) or `blockedHosts` with | ||
| `defaultPolicy: "allow"` (a blocklist) would require in-container `iptables` | ||
| rules, but a WSLC container runs in its own network namespace **without** | ||
| `CAP_NET_ADMIN` (the SDK's `Privileged` flag does not grant it), so those rules |
There was a problem hiding this comment.
Reconciled in commit e7b2a62 (latest push):
-
policy_mapping.rs: Updated thebuild_iptables_rulesdoc comment to explicitly state thatWSLC_CONTAINER_FLAG_PRIVILEGEDgrants root but does not grantCAP_NET_ADMIN, iptables rules cannot be applied, and per-host filtering configs are rejected at config-parse time. The function is noted as retained for reference but unreachable under normal operation. -
sdk/node/src/sandbox.ts: Removed WSLC from the comment claiming "Bubblewrap, LXC, and WSLC enforce them". The comment now explains that WSLC rejects host lists at config-parse time (sinceCAP_NET_ADMINis unavailable), matching the new doc contract.
There was a problem hiding this comment.
Fixed in 68fb1c5. Updated both locations to reflect the non-functional status of WSLC per-host filtering:
policy_mapping.rs: Replaced the incorrect claim thatPrivilegedgrantsNET_ADMINwith a numbered list of both blockers — (1) no in-kerneliptables(no netfilter/nf_tables in the WSLC kernel) and (2) noCAP_NET_ADMIN(thePrivilegedflag does not grant it).sdk/node/src/sandbox.ts: Updated the comment to remove WSLC from the list of backends that enforce host lists, noting instead that WSLC per-host filtering is non-functional due to both blockers and is rejected at parse time.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Update policy_mapping.rs doc comment to note that WSLC_CONTAINER_FLAG_PRIVILEGED does NOT grant CAP_NET_ADMIN (so iptables rules are impossible) and that per-host filtering configs are rejected at config-parse time. Update sdk/node/src/sandbox.ts comment to remove WSLC from the list of backends that enforce host lists via iptables, and instead note that WSLC rejects such configs at config-parse time due to missing CAP_NET_ADMIN.
…rs and sandbox.ts
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
docs/linux-wsl-roadmap-june-2026.md:310,331
These sections still say that allowLocalNetwork is silently ignored and that per-host filtering fails during execution. This PR changes both behaviors to explicit config-parse errors, so the roadmap now contradicts the implementation and getting-started guide.
Please update both sections to describe the new parse-time rejection, noting that allowLocalNetwork: false remains an accepted no-op.
Also, src/backends/wslc/common/src/wsl_container_runner.rs:320
The new check only runs through config_parser, but mxc_engine::run also accepts an already-constructed ExecutionRequest and dispatches it directly to WSLC. Such callers can still reach the known-broken iptables path and fail late during execution.
Please add the same restriction to WSLContainerRunner::validate_runner. Once all entry points fail before execution, the obsolete iptables application path can be removed safely.
…ering is not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/linux-wsl-roadmap-june-2026.md:355
- This updated paragraph retains a stale line-number citation (
build_iptables_rulesnow starts around line 442) and ends with an unmatched). Prefer the symbol-only reference so future edits do not invalidate the roadmap again.
> **❌ Not expressible today.** `build_iptables_rules` (`policy_mapping.rs:183-221`) handles only two shapes — `defaultPolicy: block` + `allowedHosts` (allow-list with trailing DROP) or `defaultPolicy: allow` + `blockedHosts` (block-list) — and **never combines** an allow-list and a deny-list in one chain. So the D4 scenario (broad allow + specific deny) has no representation: the allow side is simply ignored in the block-list branch, and there's no rule interleaving to give deny precedence. On top of that, whatever it does build doesn't enforce and is rejected before the run — no `CAP_NET_ADMIN` for in-container iptables and no VM-level enforcement hook, same as N3).
src/backends/wslc/common/src/policy_mapping.rs:377
- This cross-reference is misleading:
build_iptables_rulesonly constructs commands and does not perform the rejection. Point to the parser/runner validation that actually makes this path unreachable.
/// rejected before this runs — see `build_iptables_rules`. This only maps the
|
Copilot resolve the merge conflicts in this pull request |
|
Copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
docs/wsl/wsl-container-getting-started.md:363
- This section contradicts the network guidance earlier in the same file (
:298-303), which still says host lists are accepted, applied with iptables, and fail at spawn. Update that earlier block to state that the relevant allowlist/blocklist combinations are rejected during parsing, otherwise users are given two different failure contracts.
other security promises such as MDE). Rather than fail the run at exec time,
such configs are **rejected at config-parse time**:
docs/wsl/wsl-container-support-plan.md:309
- The example later in this document (
:563-587) still usesdefaultPolicy: "allow"withblockedHostsand promises that the host is blocked. With this new contract that example now fails parsing instead of running. Remove the unsupported host list and update the claimed behavior (or replace it with a supported proxy example).
| `allowedHosts / blockedHosts` | **Not supported — rejected at config-parse time** (and by the backend's `validate_runner`). Per-host filtering would need in-container iptables, but the container has no `CAP_NET_ADMIN` (the `Privileged` flag does not grant it) and MXC has no VM-level enforcement hook (WSLc cannot expose one without breaking other security promises such as MDE). Only the all-or-nothing `defaultPolicy` applies. |
docs/linux-wsl-roadmap-june-2026.md:355
- Remove the unmatched closing parenthesis after “N3”.
> **❌ Not expressible today.** `build_iptables_rules` (`policy_mapping.rs:183-221`) handles only two shapes — `defaultPolicy: block` + `allowedHosts` (allow-list with trailing DROP) or `defaultPolicy: allow` + `blockedHosts` (block-list) — and **never combines** an allow-list and a deny-list in one chain. So the D4 scenario (broad allow + specific deny) has no representation: the allow side is simply ignored in the block-list branch, and there's no rule interleaving to give deny precedence. On top of that, whatever it does build doesn't enforce and is rejected before the run — no `CAP_NET_ADMIN` for in-container iptables and no VM-level enforcement hook, same as N3).
| /// Reject policies WSLc cannot enforce, before any container is created. | ||
| /// Mirrors the config parser so requests reaching the engine directly | ||
| /// (an already-built `ExecutionRequest`, bypassing the parser) fail here | ||
| /// instead of late in `execute` on the broken in-container iptables path. | ||
| fn validate_runner(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { |
…equest PR #687 added a test asserting build_request_with_containment accepts WSLc allowedHosts without allowOutbound, believing WSLc enforces host rules container-side. This PR establishes that WSLc cannot enforce per-host egress filtering (no CAP_NET_ADMIN) and rejects it at parse time, so that premise is now false. Update the test to assert rejection and correct the stale comment in build_request_with_containment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/backends/wslc/common/src/wsl_container_runner.rs:645
- This validation only runs through
ScriptRunner::run. The Rust SDK streaming path callsSandboxBackend::spawn(sandbox.rs:40-58), whereself.validate(request)is still the trait's default no-op, and then callsstart_container; therefore an already-built streaming request can still create a container and reach the failing iptables path. Mirror these checks inSandboxBackend::validate(preferably via a shared helper) so both execution surfaces fail before container creation.
fn validate_runner(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> {
if request.policy.needs_host_filtering() {
src/backends/wslc/common/src/wsl_container_runner.rs:658
- The suggested configuration path is malformed:
portMappingsis a field underexperimental.wslc, so writing the path with a space can send users looking for a separate object. Use the same dotted path as the parser error and documentation.
"WSLc: network.allowLocalNetwork=true is not supported. Expose specific \
ports with experimental.wslc portMappings instead.",
docs/wsl/wsl-container-support-plan.md:309
- This row says every non-empty host list is rejected, but the new
needs_host_filteringcheck only rejects the list that refines the selected default. For example,defaultPolicy: "allow"plusallowedHostsanddefaultPolicy: "block"plusblockedHostsare accepted as redundant no-ops. Document the actual rejected combinations so the support matrix matches parser behavior.
| `allowedHosts / blockedHosts` | **Not supported — rejected at config-parse time** (and by the backend's `validate_runner`). Per-host filtering would need in-container iptables, but the container has no `CAP_NET_ADMIN` (the `Privileged` flag does not grant it) and MXC has no VM-level enforcement hook (WSLc cannot expose one without breaking other security promises such as MDE). Only the all-or-nothing `defaultPolicy` applies. |
docs/linux-wsl-roadmap-june-2026.md:355
- Remove the unmatched closing parenthesis at the end of this paragraph.
> **❌ Not expressible today.** `build_iptables_rules` (`policy_mapping.rs:183-221`) handles only two shapes — `defaultPolicy: block` + `allowedHosts` (allow-list with trailing DROP) or `defaultPolicy: allow` + `blockedHosts` (block-list) — and **never combines** an allow-list and a deny-list in one chain. So the D4 scenario (broad allow + specific deny) has no representation: the allow side is simply ignored in the block-list branch, and there's no rule interleaving to give deny precedence. On top of that, whatever it does build doesn't enforce and is rejected before the run — no `CAP_NET_ADMIN` for in-container iptables and no VM-level enforcement hook, same as N3).
| // Unix backends accept host lists without allowOutbound. Bubblewrap and | ||
| // LXC enforce them; WSLC does not (per-host filtering is non-functional — |
There was a problem hiding this comment.
question (non-blocking): does bubblewrap and lxc require admin access to do this filtering?
|
Addressed Gudge's comments, checking in. |
📖 Description
The parser now rejects, at parse time, WSLC network configs that WSLc cannot honor (previously these were either silently ignored or failed late at
execbecause of lack of support in the underlying sdk and backend.🔗 References
🔍 Validation
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
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