Skip to content

[Bubblewrap] Add optional type discriminator to deniedPaths entries - #640

Closed
Soham Das (SohamDas2021) wants to merge 3 commits into
mainfrom
user/sodas/bwrap-denied-path-type
Closed

[Bubblewrap] Add optional type discriminator to deniedPaths entries#640
Soham Das (SohamDas2021) wants to merge 3 commits into
mainfrom
user/sodas/bwrap-denied-path-type

Conversation

@SohamDas2021

@SohamDas2021Soham Das (SohamDas2021) commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Builds on #623 (denied-path masking). A denied path must be masked differently depending on whether it is a file (--ro-bind /dev/null) or a directory (--tmpfs). When a denied path does not exist on the host, the runner has no way to know which kind of stub to create, so this change lets a caller declare the kind explicitly.

deniedPaths entries may now be either a plain string (unchanged) or an object with an optional type discriminator:

"deniedPaths": [
"/etc/secret", // string form, unchanged
{ "path": "/var/run/app.sock", "type": "file" },
{ "path": "/opt/cache", "type": "directory" }
]

🔗 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 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

Adds an optional "type": "file" | "directory" discriminator for filesystem.deniedPaths entries (string form still supported) so Linux Bubblewrap masking can be deterministic even when the denied path doesn’t exist on the host. This flows from Rust wire/schema → SDK types → Bubblewrap runner behavior, and includes new Bubblewrap tests and docs updates.

Changes:

  • Extend the wire model + dev schema so deniedPaths accepts string | { path, type? }, and update TS wire emitter to handle anyOf union definitions.
  • Plumb declared kinds through parsing (denied_path_kinds) and use them in Bubblewrap denied-path classification + symlink resolution + post-run stub cleanup.
  • Add Bubblewrap regression test config + script and document the new behavior in schema and Bubblewrap backend docs.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
tests/scripts/run_bwrap_denied_kind_test.shNew end-to-end Bubblewrap test for missing denied file vs missing denied dir with declared kinds
tests/scripts/run_bwrap_all_tests.shIncludes the new denied-kind test in the Bubblewrap test suite
tests/configs/bubblewrap_denied_kind.jsonTest config exercising typed deniedPaths entries
src/core/wxc_common/src/wire.rsWire model: introduce PathKind, DeniedPathObject, DeniedPathEntry; widen denied_paths type
src/core/wxc_common/src/ts_emit.rsTS emitter: handle anyOf definitions as unions; unwrap $ref wrapped in allOf
src/core/wxc_common/src/models.rsDomain model: add PathKind and denied_path_kinds map to ContainerPolicy
src/core/wxc_common/src/config_parser.rsParse typed denied entries into denied_paths + denied_path_kinds, with unit tests
src/backends/bubblewrap/common/src/bwrap_runner.rsUse declared kinds for masking classification; re-key kinds on symlink resolution; stub cleanup logic
sdk/src/types.tsPublic SDK types: DeniedPathEntry/DeniedPathObject; widen deniedPaths fields accordingly
sdk/src/index.tsExport the new denied-path types
sdk/src/generated/wire.tsGenerated wire TS updated for new denied-path union + PathKind
schemas/dev/mxc-config.schema.0.8.0-dev.jsonDev schema updated for denied-path union/object + PathKind
docs/schema.mdDocument widened deniedPaths syntax and Bubblewrap honoring behavior
docs/sandbox-policy/v1/policy.mdUpdate policy type + table docs for typed denied paths
docs/bwrap-support/bubblewrap-backend.mdDocument deterministic masking via type, conflict behavior, and host-stub cleanup

Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs Outdated
Comment threadsrc/backends/bubblewrap/common/src/bwrap_runner.rs
@SohamDas2021
Soham Das (SohamDas2021) marked this pull request as ready for review July 13, 2026 23:23
@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code ownerJuly 13, 2026 23:23
@SohamDas2021

Copy link
Copy Markdown
ContributorAuthor

Copilot resolve the merge conflicts in this pull request

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Resolved. The base branch (user/sodas/bwrap-denied-path-masking) had a new commit (5fbede8) that refactored resolve_denied_symlinksresolve_denied_paths (returning a DeniedPlan struct) and moved the file/dir classification into that single pass. I integrated that refactor with my PathKind-aware changes in commit 02213e1, and addressed the code review feedback in commit 8de3a04:

  • Merged DeniedPlan now carries a kinds field (re-keyed denied_path_kinds when symlinks are resolved)
  • resolve_denied_paths now uses denied_masks_as_file (PathKind-aware) for classification instead of the old is_file_mask_target
  • Added diagnostic log when two denied paths resolve to the same target with conflicting declared kinds
  • Renamed changedany_path_rewritten, updated UTF-8 error message wording
  • Moved classify_denied_files helper into the test module (it was only used in tests)

@SohamDas2021

Copy link
Copy Markdown
ContributorAuthor

Copilot resolve the merge conflicts in this pull request

CopilotAI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Resolved in commit e501a6e. The base branch had a new commit (5fbede8) that independently refactored resolve_denied_symlinksresolve_denied_paths with a DeniedPlan struct (using is_file_mask_target). I merged it by keeping the PathKind-aware denied_masks_as_file (a superset of is_file_mask_target) along with the DeniedPlan.kinds field, kinds re-keying logic, warn_denied_kind_mismatches, missing_denied_stub_candidates, and cleanup_denied_stubs. Also adopted the base branch's improved non-UTF-8 error message ("cannot be safely masked; refusing to start.").

@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.

I think we should do the work to add 'type' to the policy language at the same time as we do the work to add 'leaf' vs 'tree'. And I think we should do it independently of work on any given back-end.

@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026
Base automatically changed from user/sodas/bwrap-denied-path-masking to mainJuly 14, 2026 18:01
Darren Hoehna (dhoehna) added a commit to dhoehna/mxc that referenced this pull request Jul 14, 2026
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>
Take the reconciled bwrap_runner.rs from the #640 cloud-agent review
line: rename the rewrite flag to any_path_rewritten, warn on declared-
kind key collisions when re-keying a symlinked denied path onto a target
that already has a declared type (target takes precedence), and keep the
non-UTF-8 host-path fail-closed guard.
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SohamDas2021
Soham Das (SohamDas2021)force-pushed the user/sodas/bwrap-denied-path-type branch from 204f072 to d46c1dbCompareJuly 14, 2026 20:07
@microsoft-github-policy-servicemicrosoft-github-policy-serviceBot removed the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026
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@MGudgin