Update the DACL to only allow access to the user when creating virtual disks by OneBlue · Pull Request #41678 · microsoft/WSL

Blue (OneBlue) · 2026-09-22T22:57:06Z

Summary of the Pull Request

This change updates the DACL passed to CreateVirtualDisk() to grant full access only to the creating user and BUILTIN\Administrators, with inherited access disabled. Administrator access preserves compatibility with older WSL versions that open VHDs as SYSTEM, whose token includes the Administrators group.

PR Checklist

Closes: Link to issue #xxx
Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
Tests: Added/updated if needed and all pass
Localization: All end user facing strings can be localized
Dev docs: Added/updated if needed
Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

…l disks

Copilot

Copilot review overview

🟡 Changes recommended

The ACL test reuses a VHD path across iterations and needs unique-path cleanup.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Use a unique VHD path and clean up between loop iterations · New

What changed in this PR

This pull request restricts newly created virtual disks to the creating user and uses impersonation for VM access management.

Changes:

Applies protected, user-only DACLs to new VHDs.
Updates VHD access and revocation to use user impersonation.
Adds permission validation tests.

File	Summary
`test/windows/UnitTests.cpp`	Adds VHD ACL validation tests; the reviewed test needs unique paths and cleanup per iteration.
`src/windows/service/exe/WslCoreVm.cpp`	Uses user impersonation for VHD access operations.
`src/windows/service/exe/HcsVirtualMachine.cpp`	Passes user tokens during disk cleanup.
`src/windows/common/WslCoreFilesystem.h`	Documents the updated VHD permission contract.
`src/windows/common/WslCoreFilesystem.cpp`	Creates protected, user-only VHD ACLs.
`src/windows/common/hcs.hpp`	Extends the revoke-access API with an optional token.
`src/windows/common/hcs.cpp`	Impersonates users during access revocation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🔵 Needs a closer look

The ACE validation helper must handle non-allowed ACE types correctly.

Review effort: Lite
Findings: 1

Open (1)

Use a unique VHD path and clean up between loop iterations

Copilot

Copilot review overview

🟡 Changes recommended

Preserve service-identity fallback for inaccessible VHDs and fix the test to verify the original filesystem path.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Preserve service fallback when user VHD access fails · New
Use original VHD path for security verification · New

Resolved since last review (1)

Use a unique VHD path and clean up between loop iterations

Copilot

Copilot review overview

🔵 Needs a closer look

Set-sparse VHD access can fail because the service still opens protected VHDs without user impersonation.

Review effort: Lite
Findings: None

Resolved since last review (2)

Use original VHD path for security verification
Preserve service fallback when user VHD access fails

Copilot

Copilot review overview

🟡 Changes recommended

Resolve the VHD administrator ACE policy mismatch and make ACE validation type-aware.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Administrators ACE broadens VHD access beyond the documented contract · New
Comment contains a spelling error · New

Copilot

Copilot review overview

🔵 Needs a closer look

Security-sensitive ACL and impersonation lifecycle changes require final human validation.

Review effort: Lite
Findings: 1

Open (1)

Administrators ACE broadens VHD access beyond the documented contract

Resolved since last review (1)

Comment contains a spelling error

Copilot

Copilot review overview

🔵 Needs a closer look

Update the ACL helper to safely handle non-allow ACE types before approval.

Review effort: Lite
Findings: None

Resolved since last review (1)

Administrators ACE broadens VHD access beyond the documented contract

Copilot

Copilot review overview

🔵 Needs a closer look

Reviewer assessments favor final human review, and one test-coverage nit remains.

Review effort: Lite
Findings: None

Copilot

Copilot review overview

🟡 Changes recommended

The new non-elevated regression coverage currently runs with an elevated primary token.

Review effort: Balanced
Findings: 1

Open (1)

Use a restricted token to create a genuinely non-elevated test token · New

Ben Hillis (benhillis)

The product change looks reasonable, but the new non-elevated regression path uses GetNonElevatedToken(TokenPrimary), which only lowers integrity and leaves an elevated process's administrator groups enabled. Please use the restricted impersonation-token-to-primary-token pattern so the test actually validates the user-only access path.

Copilot

Copilot review overview

🔵 Needs a closer look

The security-sensitive ACL and impersonation changes span several VM lifecycle paths and warrant final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Use a restricted token to create a genuinely non-elevated test token

Update the DACL to only allow access to the user when creating virtua…

7fd1ecf

…l disks

Copilot AI lite review requested due to automatic review settings September 22, 2026 22:57

Copilot started reviewing on behalf of Blue (OneBlue) September 22, 2026 22:58 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread test/windows/UnitTests.cpp Outdated

Blue (OneBlue) added 2 commits September 23, 2026 10:59

Save state

25473e4

Increase test coverage

4d3c1b3

Copilot AI review requested due to automatic review settings September 23, 2026 18:31

Copilot started reviewing on behalf of Blue (OneBlue) September 23, 2026 18:31 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Fix test

d1681ab

Copilot AI review requested due to automatic review settings September 23, 2026 18:58

Copilot started reviewing on behalf of Blue (OneBlue) September 23, 2026 18:59 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread src/windows/service/exe/WslCoreVm.cpp Outdated

Comment thread test/windows/UnitTests.cpp

Apply PR feedback

0495ceb

Copilot AI review requested due to automatic review settings September 23, 2026 20:07

Copilot started reviewing on behalf of Blue (OneBlue) September 23, 2026 20:08 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Blue (OneBlue) added 3 commits September 23, 2026 13:57

Merge remote-tracking branch 'origin/master' into user/oneblue/vhd-acl

66d2dd7

Apply PR feedback

d954c00

Format

049fdb6

Copilot AI review requested due to automatic review settings September 23, 2026 22:08

Copilot started reviewing on behalf of Blue (OneBlue) September 23, 2026 22:09 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread src/windows/common/WslCoreFilesystem.cpp

Comment thread src/windows/common/WslCoreFilesystem.cpp Outdated

Apply PR suggestions

57c14a0

Copilot AI review requested due to automatic review settings September 23, 2026 22:33

Copilot started reviewing on behalf of Blue (OneBlue) September 23, 2026 22:34 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Merge remote-tracking branch 'origin/master' into user/oneblue/vhd-acl

989561b

Copilot AI review requested due to automatic review settings September 24, 2026 17:15

Copilot started reviewing on behalf of Blue (OneBlue) September 24, 2026 17:16 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Merge branch 'master' into user/oneblue/vhd-acl

add0a1b

Copilot AI review requested due to automatic review settings September 24, 2026 20:36

Copilot started reviewing on behalf of Blue (OneBlue) September 24, 2026 20:37 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Blue (OneBlue) marked this pull request as ready for review September 25, 2026 06:39

Blue (OneBlue) requested a review from a team as a code owner September 25, 2026 06:39

Ben Hillis (benhillis) requested a balanced review from Copilot September 30, 2026 17:57

Copilot started reviewing on behalf of Ben Hillis (benhillis) September 30, 2026 17:57 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Comment thread test/windows/UnitTests.cpp Outdated

Ben Hillis (benhillis) requested changes Sep 30, 2026

View reviewed changes

Blue (OneBlue) added 2 commits September 30, 2026 14:35

Merge remote-tracking branch 'origin/master' into user/oneblue/vhd-acl

5b3df6d

Apply PR suggestions

00d3fa3

Copilot AI balanced review requested due to automatic review settings September 30, 2026 21:59

Copilot started reviewing on behalf of Blue (OneBlue) September 30, 2026 22:00 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Ben Hillis (benhillis) approved these changes Oct 1, 2026

View reviewed changes

Blue (OneBlue) merged commit 807f5de into master Oct 1, 2026
12 checks passed

Blue (OneBlue) deleted the user/oneblue/vhd-acl branch October 1, 2026 00:22

Conversation

Blue (OneBlue) commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Ben Hillis (benhillis) left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Blue (OneBlue) commented Sep 22, 2026 •

edited

Loading