[release/2.7] Fix VHD ownership during distribution moves

[Ben Hillis (benhillis)]

Summary of the Pull Request

Backports #41333 to release/2.7.

Distribution moves now use a duplicated caller token whose default owner is the caller's user SID. Move and rollback filesystem operations run under that token, avoiding a post-move owner rewrite.

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

This is a direct backport of the distribution move ownership change from #41333. The test conflict was resolved by retaining the admin-owner regression test, which is not otherwise present on release/2.7.

Validation Steps Performed

• Backport applied cleanly to the implementation; the test conflict was resolved by retaining the upstream regression test.
• cmake --build . -- -m was attempted. The build is blocked before compiling the changed service code by vendored GSL warning C4875 being promoted to an error with the installed compiler.
• The original change passed the full PR pipeline on master, including x64/ARM64 builds and WSL1/WSL2/WSLC tests.

[Copilot]

Pull request overview

Backports the MoveDistribution VHD ownership fix to release/2.7 by ensuring cross-volume moves create the destination VHD with the caller’s user SID as the default owner, eliminating the need for post-move owner rewrites. Adds a regression unit test covering same-volume moves when the VHD is owned by BUILTIN\Administrators.

Changes:

• Update MoveDistribution to normalize the impersonation token’s default owner to the caller’s user SID and run move/rollback under that token.
• Simplify rollback by using the same impersonation token for revert operations.
• Add MoveVhdWithAdminOwner regression test validating non-elevated same-volume moves succeed and preserve the VHD owner.

Reviewed changes

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

File	Description
test/windows/UnitTests.cpp	Adds regression coverage for moving a distro whose VHD is owned by BUILTIN\\Administrators.
src/windows/service/exe/LxssUserSession.cpp	Adjusts move/rollback token handling to prevent elevated moves from producing admin-owned VHDs.

---

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

[Blue (OneBlue)]

If you refresh this PR it should build

[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:936

• userToken is used here but is not declared in MoveDistribution, which will not compile. Also, the token returned by wsl::windows::common::security::GetUserToken() is duplicated without TOKEN_ADJUST_DEFAULT, so SetTokenInformation(TokenOwner, ...) can fail with access denied even after userToken is added.

Consider creating a dedicated duplicated caller token in this method with TOKEN_ADJUST_DEFAULT and then setting TokenOwner on that token before impersonating it for the filesystem operations.

    auto tokenUser = wil::get_token_information<TOKEN_USER>(userToken.get());
    TOKEN_OWNER tokenOwner{tokenUser->User.Sid};
    THROW_IF_WIN32_BOOL_FALSE(SetTokenInformation(userToken.get(), TokenOwner, &tokenOwner, sizeof(tokenOwner)));

[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:937

• GetUserToken(TokenImpersonation) duplicates the caller token without TOKEN_ADJUST_DEFAULT (see src/windows/common/WslSecurity.cpp:159-161). This means SetTokenInformation(..., TokenOwner, ...) is likely to fail with access denied at runtime, preventing distribution moves.

Consider duplicating userToken with TOKEN_ADJUST_DEFAULT for the move/rollback path and then setting TokenOwner on that duplicated token.

    auto tokenUser = wil::get_token_information<TOKEN_USER>(userToken.get());
    TOKEN_OWNER tokenOwner{tokenUser->User.Sid};
    THROW_IF_WIN32_BOOL_FALSE(SetTokenInformation(userToken.get(), TokenOwner, &tokenOwner, sizeof(tokenOwner)));

[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:946

• SetTokenInformation(..., TokenOwner, ...) requires the token to have TOKEN_ADJUST_DEFAULT, but wsl::windows::common::security::GetUserToken() currently duplicates the client token without requesting TOKEN_ADJUST_DEFAULT (see src/windows/common/WslSecurity.cpp:159-162). As a result, this call will reliably fail with ERROR_ACCESS_DENIED, breaking MoveDistribution. Please either update GetUserToken() to include TOKEN_ADJUST_DEFAULT in the DuplicateTokenEx desired access mask, or re-duplicate the token locally with TOKEN_ADJUST_DEFAULT before calling SetTokenInformation.

    auto tokenUser = wil::get_token_information<TOKEN_USER>(userToken.get());
    TOKEN_OWNER tokenOwner{tokenUser->User.Sid};
    THROW_IF_WIN32_BOOL_FALSE(SetTokenInformation(userToken.get(), TokenOwner, &tokenOwner, sizeof(tokenOwner)));

[Copilot]

Pull request overview

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

Suppressed comments (2)

test/windows/UnitTests.cpp:2893

• The cleanup scope_exit lambda uses firstFolder/secondFolder but does not capture them. These are local variables (even though constexpr) and this will not compile without capturing them.

        auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [name]() {
            LxsstuLaunchWsl(std::format(L"--unregister {}", name));
            std::filesystem::remove_all(firstFolder);
            std::filesystem::remove_all(secondFolder);
        });

src/windows/common/WslSecurity.h:103

• The GetUserToken comment no longer reflects the updated signature: callers can now request extra access rights via additionalAccess. Updating the summary helps prevent misuse and makes the new parameter discoverable.

/// <summary>
/// Returns the user token for the current client.
/// </summary>
wil::unique_handle GetUserToken(_In_ TOKEN_TYPE tokenType, _In_ RPC_BINDING_HANDLE handle = nullptr, _In_ DWORD additionalAccess = 0);