Simplify VHD ownership handling during distribution moves

[Ben Hillis (benhillis)]

Summary of the Pull Request

Simplifies VHD ownership handling when moving a distribution. The duplicated caller token now uses the caller's user SID as its default owner, and the move and rollback operations run under that token.

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

Cross-volume MoveFileExW operations create a new file using the impersonation token's default owner. This change sets the duplicated caller token's default owner to its user SID before moving the VHD, keeping the resulting file accessible to that user.

All filesystem operations, including rollback, use the modified caller token. This removes the need to reopen the destination and restore its owner after the move. Same-volume moves remain renames and preserve the existing owner.

Validation Steps Performed

• cmake --build . -- -m
• bin\x64\Debug\test.bat -f /name:*MoveVhdWithAdminOwner — passed (1/1)

[Copilot]

Pull request overview

This pull request simplifies distribution move behavior in the WSL Windows service by ensuring cross-volume VHD moves create the destination VHD with the caller’s user SID as the default owner, avoiding elevated-token ownership side effects (e.g., unexpected BUILTIN\Administrators ownership).

Changes:

• Updates MoveDistribution() to set the duplicated caller impersonation token’s TokenOwner to the caller’s user SID and to run move/rollback filesystem operations under that token.
• Removes the previous “read original owner then restore owner after move” flow (including privileged WRITE_OWNER fixup) in favor of the normalized-token approach.
• Updates test commentary to reflect same-volume owner-preservation expectations.

Reviewed changes

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

File	Description
src/windows/service/exe/LxssUserSession.cpp	Sets the duplicated caller token default owner to the caller SID and performs move + rollback under that token, eliminating post-move owner restoration logic.
test/windows/UnitTests.cpp	Adjusts regression test comments to align with the updated ownership/move behavior narrative.

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/windows/service/exe/LxssUserSession.cpp:942

• The new ownership behavior is not exercised by the existing tests: MoveVhdOwnership and MoveVhdWithAdminOwner move between relative folders on the same volume, where MoveFileExW performs a rename and never uses the token's default owner. The tests therefore still pass if this SetTokenInformation call is removed. Add a genuinely cross-volume move (for example, using a temporary mounted VHD or another test volume) and verify that the copied VHD is owned by the caller and remains launchable.

    THROW_IF_WIN32_BOOL_FALSE(SetTokenInformation(userToken.get(), TokenOwner, &tokenOwner, sizeof(tokenOwner)));

[Blue (OneBlue)]

Change LGTM, one small change in behavior is that now we're reverting the VHD owner to the current token on failure, instead of the original VHD's owner.

I could imagine some situations where that could lead to issues, especially if there are some custom ACL's, but I don't think that's necessarily a blocker. That could be fixed in a followup

[Ben Hillis (benhillis)]

Change LGTM, one small change in behavior is that now we're reverting the VHD owner to the current token on failure, instead of the original VHD's owner.

I could imagine some situations where that could lead to issues, especially if there are some custom ACL's, but I don't think that's necessarily a blocker. That could be fixed in a followup

Yeah that's true, I think this is an improvement over the existing so I'll look at fixing the ACL stuff in a follow-up.