Skip to content

[LXC] Address file system policy gaps - #630

Merged
Soham Das (SohamDas2021) merged 7 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-fs-policy-gaps
Jul 20, 2026
Merged

[LXC] Address file system policy gaps#630
Soham Das (SohamDas2021) merged 7 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-fs-policy-gaps

Conversation

@dhoehna

@dhoehnaDarren Hoehna (dhoehna) commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Linked work item: AB#62861419 — [LXC] Address file system policy gaps

Summary

Hardens LXC denied-path masking and adds a reusable most-specific-path-wins policy resolver in wxc_common. No schema or wire-format changes — see the Schema note below.

  • Denied-path masking (host reality): the LXC denied branch now classifies each denied path from the host path (denied paths are host paths) instead of the container rootfs path. The rootfs path (<lxc_path>/<name>/rootfs/<container_path>) does not exist until mounts are applied, so inspecting it always misclassified the mask as a directory. A regular host file is now masked with a read-only /dev/null bind (create=file); a directory, symlink, or missing host path with an empty read-only tmpfs (create=dir). Classification uses symlink_metadata so a symlinked deny is never followed to an unintended target.
  • Most-specific-path-wins resolver: extends the existing filesystem_resolve module in wxc_commonin place (no module rename / compat shim). Paths are emitted shallow-to-deep so the deepest (most specific) intent wins at every path regardless of which policy list it came from (e.g. readwritePaths: [""/data/secrets""] survives deniedPaths: [""/data""]). Exact same-path conflicts are not re-collapsed in the resolver — that is already handled upstream by the config parser's normalize_filesystem_paths (string-level) and each runner's normalize_object_conflicts (object-identity); for any duplicate that still reaches the resolver, emission order (denied emitted last) preserves most-restrictive-wins.

Schema

No schema or wire-format changes. The earlier type: ""file"" | ""dir"" discriminator — and the MaskKind / denied_path_kinds model fields — have been dropped. Per the discussion on this PR, the file/dir (and leaf/tree) distinction will be handled as separate schema work, and the shared schema/wire surface is owned by #640 (user/sodas/bwrap-denied-path-type), which LXC will consume once it lands. This PR is scoped to the resolver + host-reality masking and touches no files under schemas/ or sdk/src/generated/.

Validation

  • cargo fmt --all -- --check — clean
  • cargo clippy -p wxc_common --all-targets -- -D warnings (Windows) and cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common -p bwrap_common --all-targets -- -D warnings — clean
  • cargo test -p wxc_common — 397 passed
  • cargo check for lxc_common + bwrap_common on x86_64-unknown-linux-gnu — pass

Independent of the sibling LXC branches (no shared model fields).

Microsoft Reviewers: Open in CodeFlow

…B#62861419)
- Replace is_file() masking heuristic with explicit `type` schema field + symlink_metadata() fallback (no symlink follow; fail-closed on missing+untyped).
- Add reusable most-specific-path-wins resolver in wxc_common (deny>ro>rw); wire LXC mounts to emit in specificity order.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3b78bec0-e139-4cfd-9c10-092ef986d4f4
CopilotAI review requested due to automatic review settings July 10, 2026 22:56
@dhoehna
Darren Hoehna (dhoehna) requested a review from a team as a code ownerJuly 10, 2026 22:56

CopilotAI 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

This PR hardens filesystem policy handling by extending the wire format for filesystem.deniedPaths and introducing a reusable “most-specific-path-wins” resolver to produce deterministic mount emission order for Linux backends.

Changes:

  • Extend deniedPaths to accept either a legacy string or an object { path, type: "file" | "dir" }, and plumb the parsed mask kind into ContainerPolicy.
  • Add wxc_common::path_specificity to resolve overlapping filesystem intents by specificity (deep overrides shallow; exact ties pick most restrictive).
  • Update LXC mount emission to use the new resolver and apply explicit denied-path mask kinds.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
src/core/wxc_common/src/wire.rsChanges wire model so deniedPaths supports string or typed object entries.
src/core/wxc_common/src/ts_emit.rsUpdates TS emitter to correctly emit anyOf unions and allOf wrappers as TypeScript type aliases.
src/core/wxc_common/src/path_specificity.rsIntroduces the new most-specific-path-wins resolver and tests.
src/core/wxc_common/src/models.rsAdds MaskKind and denied_path_kinds to ContainerPolicy for backend masking decisions.
src/core/wxc_common/src/lib.rsExposes the new path_specificity module.
src/core/wxc_common/src/filesystem_resolve.rsKeeps compatibility by re-exporting the resolver from the new module.
src/core/wxc_common/src/config_parser.rsConverts wire denied-path entries into denied_paths + denied_path_kinds in the domain policy.
src/backends/lxc/common/src/filesystem_mounts.rsSwitches mount emission ordering to the resolver and adds explicit denied-path masking logic.
sdk/src/generated/wire.tsRegenerates TS wire types for the new DeniedPath union/object forms.
schemas/dev/mxc-config.schema.0.8.0-dev.jsonRegenerates the dev schema to reflect the new denied-path shapes.

Comment threadsrc/backends/lxc/common/src/filesystem_mounts.rs Outdated
Comment threadsrc/backends/lxc/common/src/filesystem_mounts.rs Outdated
…h keys
Address PR review feedback on denied-path masking:
- `observed_mask_path_kind` now takes `&Path` and inspects the *host* path
instead of the container rootfs path (`.../rootfs/<container_path>`). The
rootfs path does not exist before mounts are applied, so the previous code
returned `NotFound` for real host denied paths and spuriously demanded an
explicit `type`, breaking existing LXC object-policy validation. Also drops
the `to_string_lossy()` allocation by passing `&Path` directly.
- Add `lookup_mask_kind` with trailing-separator normalization so an explicit
`type` is not dropped when the resolved mount path and the configured
`denied_path_kinds` key differ only by a trailing slash.
- Add unit tests for host-path observation (dir/file/missing) and normalized
kind lookup.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadschemas/dev/mxc-config.schema.0.8.0-dev.json Outdated
@SohamDas2021

Copy link
Copy Markdown
Contributor

Since this might be touching common code, make sure all lxc and bwrap tests/configs pass

Comment threadsrc/core/wxc_common/src/wire.rs Outdated
Comment threadsrc/backends/lxc/common/src/filesystem_mounts.rs Outdated
Comment threadsrc/core/wxc_common/src/path_specificity.rs Outdated
Comment threadsrc/core/wxc_common/src/path_specificity.rs Outdated
Address review feedback on microsoft#630:
- Remove the denied-path file/dir `type` discriminator (wire DeniedPath
union, MaskKind, denied_path_kinds, regenerated schema + TS wire types,
ts_emit anyOf/allOf handling, and the LXC mask-kind logic). Per reviewer
guidance the file-vs-dir schema distinction is deferred (to be designed
alongside leaf/tree), and the wire change duplicates bwrap microsoft#640.
- Keep the most-specific-path-wins resolver but extend `filesystem_resolve`
in place instead of renaming it to `path_specificity` with a compat shim,
avoiding churn / conflict surface with the bwrap branches. Delete
path_specificity.rs and the shim.
- Fix the `dedup_by` dead code: the retained element is the earlier `b`, so
mutating `a` was a no-op. Rely on the most-restrictive-first sort (with a
comment) and add a test asserting the kept path string on an exact-key
conflict across lists that differ only by a trailing slash.
- Revert LXC denied-path masking to its original behavior while still
emitting mounts in resolver (shallow->deep) order.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot 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
@SohamDas2021

Copy link
Copy Markdown
Contributor

The PR description is stale and still says that you update the schema.

Comment threadsrc/core/wxc_common/src/filesystem_resolve.rs Outdated
Comment threadsrc/backends/lxc/common/src/filesystem_mounts.rs
Comment threadsrc/core/wxc_common/src/filesystem_resolve.rs
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e5d2aa5b-7f04-4e4d-83d3-a02efe7020ab
Addresses the change requests on PR microsoft#630:
- filesystem_mounts.rs: classify denied paths from the HOST path (via
symlink_metadata, never following symlinks) instead of the not-yet-existing
rootfs path, so a denied host file is masked with /dev/null (create=file)
and a directory/symlink/missing path with an empty tmpfs (create=dir).
- filesystem_resolve.rs: drop the exact-path most-restrictive collapse from
resolve_path_plan; it duplicated the upstream normalize_filesystem_paths
(config parser) and normalize_object_conflicts (every runner). The resolver
now only orders shallow-to-deep, and emission order still yields
most-restrictive-wins for any same-path duplicate that survives upstream.
- filesystem_resolve.rs: move the test-only effective_intent oracle into the
tests module and gate PathKey::is_prefix_of to test builds.
Updated affected unit tests. cargo fmt/clippy clean; wxc_common 398 tests
pass; lxc_common + bwrap_common clippy clean on x86_64-unknown-linux-gnu.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dhoehna

Copy link
Copy Markdown
ContributorAuthor

Addressed the change requests in bae58f5:

  1. effective_intent (filesystem_resolve.rs) — moved into #[cfg(test)] mod tests (test-only oracle) and gated PathKey::is_prefix_of to #[cfg(test)]; no production surface references it.
  2. LXC denied masking (filesystem_mounts.rs) — now classifies from the host path via denied_path_is_file (symlink_metadata, never follows symlinks) instead of the not-yet-existing rootfs path: host file → /dev/null (create=file), directory/symlink/missing → empty tmpfs (create=dir). Added a unit test.
  3. Redundant collapse (resolve_path_plan) — removed; exact same-path conflicts are already resolved upstream by normalize_filesystem_paths (parser) and normalize_object_conflicts (each runner). The resolver now only orders shallow-to-deep, and emission order (denied last) still yields most-restrictive-wins. Tests updated.

Also refreshed the stale PR description (no schema/wire changes remain).

Validation (this is a Windows dev box, so the LXC/bwrap E2E shell suites — which need a Linux host with lxc/bwrap — run in CI, not locally):

  • cargo fmt --all -- --check — clean
  • cargo test -p wxc_common397 passed
  • cargo clippy -p wxc_common --all-targets -- -D warnings (Windows) — clean
  • cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common -p bwrap_common --all-targets -- -D warnings — clean (compiles + lints the shared change against both backends)
  • node scripts/versioning/validate-configs.js186 configs OK against the dev schema

Rebased onto the latest branch tip (post main-merge 037907d) before pushing.

@dhoehna

Copy link
Copy Markdown
ContributorAuthor

🧪 Local test re-verification — 2026-07-17

Re-ran the test suites locally at branch tip bae58f5 on a dev workstation. All green (0 failures).

Windows host (x86_64-pc-windows-msvc, cargo 1.96.1):

  • cargo test -p wxc_common397 passed, 0 failed

Linux (WSL2 Ubuntu-24.04, x86_64-unknown-linux-gnu, cargo 1.97.0, isolated CARGO_TARGET_DIR):

  • cargo test -p lxc_common40 passed, 0 failed — the denied-path masking / mount code (filesystem_mounts.rs) ran on Linux (the original Validation only cargo check-compiled it for the Linux target).

Note: the wxc_common count differs slightly from the original Validation because the branch advanced since it was written.

PR microsoft#630 changes LXC filesystem policy behavior but shipped no tests/configs
sample; the parallel Bubblewrap work (microsoft#623) added one per scenario. Add the
LXC equivalents so the behavior is demonstrated in action and covered by the
config-validation gate:
- lxc_denied_masking.json: a denied regular file is masked with a /dev/null
bind (create=file); a denied directory with an empty ro tmpfs (create=dir).
- lxc_denied_symlink_dir.json / lxc_denied_symlink_file.json: a denied symlink
is classified via symlink_metadata (never followed) and masked with an empty
ro tmpfs, so neither the target dir nor file leaks.
- lxc_most_specific_denied_parent.json: a readwrite child under a denied parent
stays readable/writable while the parent sibling is masked
(most-specific-path-wins resolver).
All four validate against schemas/dev/mxc-config.schema.0.8.0-dev.json
(node scripts/versioning/validate-configs.js: 190 configs OK).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b6b3323b-7297-4b07-9e6e-ab4b220124e6
@dhoehna

Copy link
Copy Markdown
ContributorAuthor

Soham Das (@SohamDas2021) -- added the missing LXC sample configs demonstrating this PR's filesystem-policy behavior (commit c9373ef), mirroring the Bubblewrap denied-masking config set from #623:

  • tests/configs/lxc_denied_masking.json -- a denied regular file is masked with a /dev/null bind (create=file); a denied directory with an empty read-only tmpfs (create=dir).
  • tests/configs/lxc_denied_symlink_dir.json / tests/configs/lxc_denied_symlink_file.json -- a denied symlink is classified via symlink_metadata (never followed) and masked with an empty read-only tmpfs, so neither the target dir nor file leaks.
  • tests/configs/lxc_most_specific_denied_parent.json -- a readwrite child under a denied parent stays readable/writable while the parent sibling is masked (most-specific-path-wins resolver).

Scope note: after dropping the file/dir schema type work in b0a5766, this PR makes no schema/wire change -- these configs exercise the existing deniedPaths / readwritePaths fields to demonstrate the new enforcement behavior. All four validate against the dev schema (node scripts/versioning/validate-configs.js -> 190 configs OK).

@@ -0,0 +1,19 @@
{

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.

Please make sure the new jsons are added to tests\scripts\run_wslc_all_tests.ps1

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I've wired the new filesystem-policy configs into both test runners.

WSLC runner (tests/scripts/run_wslc_all_tests.ps1): added two native WSLC variants -- wslc_denied_masking.json and wslc_most_specific_denied_parent.json -- each with a wrapper (run_wslc_denied_masking_test.ps1, run_wslc_most_specific_test.ps1) that owns its Windows fixture. They use the WSLC "denied path = not mounted" model (policy_mapping.rsbuild_volume_mounts).

Why not the lxc_* configs directly in the WSLC harness: on Windows, wxc-exec cannot run containment:"lxc" configs -- run.rs:181 returns unsupported_containment("LXC backend not available on Windows"), so they would hard-fail rather than test anything.

LXC runner (tests/scripts/run_lxc_all_tests.sh): registered the four lxc_* configs via run_lxc_denied_masking_test.sh and run_lxc_most_specific_test.sh (they run under lxc-exec on Linux, same as the bwrap wrappers).

I intentionally skipped WSLC symlink variants: Windows file-symlinks require admin/Developer Mode, and the "denied alias not mounted" semantics they'd exercise are already covered by wslc_filesystem_object.json. Happy to add them if you'd prefer full parity.

All 192 configs pass node scripts/versioning/validate-configs.js. Pushed as 5a28fe5.

Register the four lxc_* filesystem-policy configs from c9373ef with the
Linux LXC runner, and add equivalent wslc_* variants to the Windows WSLC
runner, per Soham's review note on PR microsoft#630.
LXC (Linux, lxc-exec):
- run_lxc_denied_masking_test.sh drives lxc_denied_masking,
lxc_denied_symlink_dir, lxc_denied_symlink_file.
- run_lxc_most_specific_test.sh drives lxc_most_specific_denied_parent.
- Both registered in run_lxc_all_tests.sh.
WSLC (Windows, wxc-exec): the lxc_* configs cannot run on the WSLC harness
(run.rs:181 returns unsupported_containment for Lxc on Windows), so add
native wslc_* variants using the WSLC "denied path = not mounted" model
(policy_mapping.rs):
- wslc_denied_masking.json + run_wslc_denied_masking_test.ps1
- wslc_most_specific_denied_parent.json + run_wslc_most_specific_test.ps1
- Both registered in run_wslc_all_tests.ps1.
WSLC symlink variants intentionally omitted: Windows file-symlinks need
admin/Developer Mode, and the "denied alias not mounted" semantics are
already covered by wslc_filesystem_object.json.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b6b3323b-7297-4b07-9e6e-ab4b220124e6
@SohamDas2021
Soham Das (SohamDas2021) merged commit f379b0e into microsoft:mainJul 20, 2026
20 checks passed
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot removed Needs-Attention Issue needs attention from Microsoft Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 20, 2026
Darren Hoehna (dhoehna) added a commit to dhoehna/mxc that referenced this pull request Jul 27, 2026
Resolve conflict in src/backends/lxc/common/src/filesystem_mounts.rs as a
union of both changes:
- microsoft#633 mount-accumulation fix: clear_config_item("lxc.mount.entry") before
re-deriving the policy's mounts (so a restart replaces, not unions, mounts).
- upstream microsoft#630 denied-dir masking: rebound_container_paths /
has_rebound_descendant, iterating &mounts.
Both new unit tests (configure_filesystem_mounts_replaces_not_accumulates and
has_rebound_descendant_detects_nested_rebind_only) are kept.
Validated with `cargo check -p lxc_common --tests` (native linux/liblxc, WSL).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b6b3323b-7297-4b07-9e6e-ab4b220124e6
Sign up for freeto 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

@dhoehna@SohamDas2021