Skip to content

[Bubblewrap] Mask denied files with a /dev/null bind instead of tmpfs - #623

Merged
Soham Das (SohamDas2021) merged 5 commits into
mainfrom
user/sodas/bwrap-denied-path-masking
Jul 14, 2026
Merged

[Bubblewrap] Mask denied files with a /dev/null bind instead of tmpfs#623
Soham Das (SohamDas2021) merged 5 commits into
mainfrom
user/sodas/bwrap-denied-path-masking

Conversation

@SohamDas2021

@SohamDas2021Soham Das (SohamDas2021) commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

📖 Description

The Bubblewrap backend masked every denied path with --tmpfs. Mounting a tmpfs over a regular file turns it into an empty directory (bwrap creates the mount point as a dir), changing the object's type and breaking tools that expect a regular file.

This PR classifies each denied path at runtime and masks it with the correct primitive:

  • Directories keep --tmpfs (empty, no leak).
  • Files (and other non-directory objects) are masked with a read-only bind of /dev/null, preserving a non-directory type with empty content.

This brings Bubblewrap in line with the LXC backend, which already distinguishes files from directories when masking.

🔗 References

🔍 Validation

✅ Checklist

📋 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

CopilotAI review requested due to automatic review settings July 8, 2026 17: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 improves Bubblewrap’s denied-path masking behavior so that denying a regular file no longer converts it into an empty directory (a side-effect of --tmpfs creating the mount point as a directory). It aligns Bubblewrap with the LXC backend by masking denied directories and denied non-directories differently to better preserve object type semantics.

Changes:

  • Classify denied paths at runtime and mask denied non-directories via --ro-bind /dev/null (directories remain --tmpfs).
  • Add unit tests for classified denied-path masking in the Bubblewrap argument builder.
  • Add an end-to-end shell test + config to validate denied file vs denied directory masking behavior.

Reviewed changes

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

Show a summary per file
FileDescription
tests/scripts/run_bwrap_denied_masking_test.shNew end-to-end test script validating denied file vs denied directory masking semantics.
tests/scripts/run_bwrap_all_tests.shAdds the new denied-masking test to the Bubblewrap test suite runner.
tests/configs/bubblewrap_denied_masking.jsonNew Bubblewrap config fixture used by the denied-masking test.
src/backends/bubblewrap/common/src/bwrap_runner.rsClassifies denied paths at runtime and passes classification into arg building.
src/backends/bubblewrap/common/src/bwrap_command.rsAdds build_args_classified and unit tests to mask denied files with /dev/null instead of --tmpfs.

Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs Outdated
@SohamDas2021
Soham Das (SohamDas2021)force-pushed the user/sodas/bwrap-denied-path-masking branch from a7e0e05 to 0582bacCompareJuly 8, 2026 18:09
@SohamDas2021
Soham Das (SohamDas2021) marked this pull request as ready for review July 8, 2026 18:24
@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code ownerJuly 8, 2026 18:24
Comment threadtests/scripts/run_bwrap_denied_masking_test.sh
Comment threadsrc/backends/bubblewrap/common/src/bwrap_command.rs
Comment threadsrc/backends/bubblewrap/common/src/bwrap_command.rs Outdated
Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs Outdated

@MGudginGudge (MGudgin) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🕐

Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs Outdated
Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs
Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs
@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 13, 2026
Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs
@SohamDas2021
Soham Das (SohamDas2021) merged commit ecb5919 into mainJul 14, 2026
22 checks passed
@SohamDas2021
Soham Das (SohamDas2021) deleted the user/sodas/bwrap-denied-path-masking branch July 14, 2026 18:01
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot removed the Needs-Attention Issue needs attention from Microsoft label Jul 14, 2026
Darren Hoehna (dhoehna) added a commit to dhoehna/mxc that referenced this pull request Jul 20, 2026
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
Soham Das (SohamDas2021) pushed a commit that referenced this pull request Jul 20, 2026
* [LXC] Harden denied-path masking + add most-specific-path resolver (AB#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
* [LXC] Observe host path for denied-mask kind + tolerate trailing-slash 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>
* [LXC] Scope PR to most-specific-path resolver; drop file/dir schema work
Address review feedback on #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 #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>
* [LXC] Address review: host-reality denied mask, drop redundant collapse
Addresses the change requests on PR #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>
* [LXC] Add sample configs for denied-path masking + most-specific-path
PR #630 changes LXC filesystem policy behavior but shipped no tests/configs
sample; the parallel Bubblewrap work (#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
* [LXC] Wire denied-masking configs into LXC and WSLC test runners
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 #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
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants

@SohamDas2021@huzaifa-d@MGudgin