fix(host-prep): avoid recursive ACL propagation on drive roots#649
Draft
xuli500177 wants to merge 1 commit into
Draft
fix(host-prep): avoid recursive ACL propagation on drive roots#649xuli500177 wants to merge 1 commit into
xuli500177 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes wxc-host-prep prepare-system-drive / unprepare-system-drive unintentionally triggering recursive ACL normalization on large drive roots by avoiding SetNamedSecurityInfoW’s automatic inheritance propagation behavior. It introduces a target-only DACL write path in wxc_common::filesystem_dacl (backed by an absolute security descriptor + SetFileSecurityW), routes host-prep’s persistent drive-root operations through it, and adds a regression test and documentation to codify the “no descendant mutation” contract.
Changes:
- Add a
DaclWriteModeabstraction andSetFileSecurityW-backed “target-only” DACL write path inwxc_common::filesystem_dacl. - Switch host-prep system-drive apply/revoke to the non-propagating write helpers, while keeping the existing propagating behavior available for runtime DACL management.
- Add a Windows regression test asserting descendant DACL bytes are unchanged, and update
docs/host-prep.mdto document the suppression of automatic propagation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/host/wxc_host_prep/src/system_drive/mod.rs |
Routes system-drive prepare/unprepare to the new non-propagating DACL write helpers while keeping existing test-only use of propagating helpers. |
src/core/wxc_common/src/filesystem_dacl.rs |
Introduces DaclWriteMode + write_dacl() with a target-only SetFileSecurityW path; adds public *_no_propagation helper APIs and a regression test. |
docs/host-prep.md |
Documents that drive-root ACL writes suppress automatic inheritance propagation and avoid normalizing descendant ACLs. |
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
prepare-system-driveandunprepare-system-drivecurrently finish their DACL writes withSetNamedSecurityInfoW. Windows automatically imposes the current inheritance model on descendants for that API, so preparing a legacy or inheritance-inconsistent data-volume root can recursively rewrite thousands of child security descriptors and appear to hang.This change:
SetFileSecurityW;DaclManageronSetNamedSecurityInfoW, because its inheritable per-run grants intentionally propagate;SetFileSecurityWis a legacy API, but its documented directory behavior is the property needed here: security applied to the named directory is not inherited by its children.🔗 References
Resolves #648
🔍 Validation
cargo fmt --all -- --checkcargo check -p wxc_common -p wxc_host_prepcargo clippy -p wxc_common -p wxc_host_prep --all-targets -- -D warningscargo test -p wxc_common— 395 passedcargo test -p wxc_common target_only_writes_leave_descendant_dacl_unchanged— 1 passedwxc_host_prep ... system_drive --nocapture— 9 passed (debug test copy with an explicitasInvokermanifest to bypass Windows installer-detection elevation heuristics)✅ Checklist
Cargo.lockis unchanged, sodependency-feed-checkdependency seeding is not applicable📋 Issue Type
Microsoft Reviewers: Open in CodeFlow