Reject non-empty directories for new WSLC session storage by beena352 · Pull Request #40655 · microsoft/WSL

beena352 · 2026-05-27T17:35:41Z

Summary of the Pull Request

Prevent users from accidentally reusing existing non-empty directories as WSLC session storage. For new sessions, validate that the storage directory is either empty or non-existent. Existing sessions identified by their storage.vhdx VHD bypass this check, so users can re-enter sessions freely even if additional files are present in the storage directory.

PR Checklist

Closes: Link to issue #62259817
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

Validate session storage paths at initialization time:

If creating a new session and the directory already exists and is non-empty, reject with E_INVALIDARG and a user-facing error message
If the directory is empty or doesn't exist, create it and proceed
If a session's VHD already exists in the directory, skip the emptiness check (re-entry allowed)
This prevents data loss by ensuring new sessions don't accidentally overwrite unrelated user files placed in the storage directory. Once a session is created (VHD exists), the directory can contain other files without blocking re-entry.

Validation Steps Performed

Copilot

Pull request overview

Adds a wslcsession marker-file convention at the root of a WSLC session storage directory so the service can distinguish directories it owns from arbitrary user content, and rejects new session creation into a non-empty, unmarked directory. Also wires IErrorInfo/COMServiceExecutionContext through the per-user WSLCSessionFactory and WSLCSessionManager so the user-facing rejection message reaches the CLI/test caller.

Changes:

Add EnsureSessionMarker helper in WSLCSession::ConfigureStorage (new sessions require empty dir + stamp marker; existing VHD paths get a legacy upgrade stamp).
Add ISupportErrorInfo + COMServiceExecutionContext to WSLCSessionFactory and propagate IErrorInfo from factory to caller in WSLCSessionManagerImpl::CreateSession; add ISupportErrorInfo to WSLCSessionManager.
Add API-level and e2e tests for marker rejection and legacy auto-upgrade; add localized MessageWslcSessionStorageMustBeEmpty.

Reviewed changes

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

Show a summary per file

File	Description
src/windows/wslcsession/WSLCSession.cpp	New `EnsureSessionMarker` helper and wiring into `ConfigureStorage` for new/existing storage paths.
src/windows/wslcsession/WSLCSessionFactory.cpp	Establish `COMServiceExecutionContext` around `CreateSession` and implement `InterfaceSupportsErrorInfo`.
src/windows/wslcsession/WSLCSessionFactory.h	Add `ISupportErrorInfo` base and method declaration.
src/windows/service/exe/WSLCSessionManager.cpp	Inspect `IErrorInfo` after factory failure and re-throw with user-facing message; implement `InterfaceSupportsErrorInfo`.
src/windows/service/exe/WSLCSessionManager.h	Add `ISupportErrorInfo` base and method declaration.
test/windows/WSLCTests.cpp	New API-level cases: non-empty dir rejection and `wslcsession`-as-directory rejection with COM error validation.
test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp	E2E test verifying marker creation on default session and legacy auto-upgrade.
localization/strings/en-US/Resources.resw	New `MessageWslcSessionStorageMustBeEmpty` localized string.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

OneBlue

I think refusing to create a session storage if the directory is non-empty makes sense (although I worry a bit about this putting us in a corner, like if there's ever a thumdb.db, desktop.ini or similar file created, this could "lock a user out" of using that folder permanently)

I'm not sure about the marker file though. Whenever we create a storage folder, we always create a storage VHD, so could we use that directly instead ?

…slc-session-storage-marker

…eenachauhan/wslc-session-storage-marker

Copilot

Pull request overview

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

…ew sessions

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

OneBlue · 2026-06-09T22:18:08Z

+                    THROW_HR_WITH_USER_ERROR(factoryHr, comError->Message.get());
+                }
+
+                THROW_IF_FAILED(factoryHr);


nit: This could be changed to just THROW_HR, since we know it's a failed HRESULT

OneBlue · 2026-06-09T22:29:58Z

+        // Only check emptiness for new sessions. Existing sessions are identified by their VHD.
+        if (!vhdExists)
+        {
+            const bool isDir = std::filesystem::is_directory(storagePath, ec);


Could we always call is_empty, and look at is_empty's error on failure so we don't need to specifically handle the isDir vs !isDir case ourselves ?

OneBlue · 2026-06-09T22:57:21Z

+        };
+
+        std::error_code ec;
+        const auto vhdPath = storagePath / c_storageVhdFilename;


nit: We could set m_storageVhdPath here so we don't need to duplicate this in ConfigureStorage()

Asking because I think we should keep that logic in ConfigureStorage if possible, because there we already check if the VHD exists or not, so we could have the logic to fail if the directory is not empty there, which should help simplify this

OneBlue · 2026-06-09T22:59:52Z

        TraceLoggingValue(m_displayName.c_str(), "DisplayName"),
        TraceLoggingValue(Settings->CreatorPid, "CreatorPid"));

+    // Validate storage path before creating VM resources (Terminate() would clobber the error).


What causes Terminate() to clobber the error in that case ?

Add marker-file validation for WSLC session storage

7b5b423

Copilot AI review requested due to automatic review settings May 27, 2026 17:35

Copilot started reviewing on behalf of beena352 May 27, 2026 17:35 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

beena352 requested a review from Copilot May 27, 2026 17:44

Copilot started reviewing on behalf of beena352 May 27, 2026 17:44 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

beena352 added 2 commits May 27, 2026 20:37

Fix CreateSessionValidation test

d88f735

fix test

ba8a228

Copilot AI review requested due to automatic review settings May 28, 2026 16:36

Copilot started reviewing on behalf of beena352 May 28, 2026 16:36 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

simplify comments and guard is_empty error check

81268fa

OneBlue reviewed May 29, 2026

View reviewed changes

beena352 added 2 commits May 29, 2026 14:41

Merge remote-tracking branch 'origin/master' into user/beenachauhan/w…

5762bb3

…slc-session-storage-marker

Merge branch 'master' of https://github.com/microsoft/WSL into user/b…

e5aa1ba

…eenachauhan/wslc-session-storage-marker

Copilot AI review requested due to automatic review settings June 2, 2026 18:53

Copilot started reviewing on behalf of beena352 June 2, 2026 18:54 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated

Comment thread src/windows/wslcsession/WSLCSession.cpp

Comment thread src/windows/wslcsession/WSLCSessionFactory.cpp Outdated

Prevent session storage reuse by validating directory emptiness for n…

48ed435

…ew sessions

beena352 requested a review from Copilot June 3, 2026 03:39

Copilot started reviewing on behalf of beena352 June 3, 2026 03:39 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread src/windows/wslcsession/WSLCSession.cpp

Use error_code overload for creating storage directories

8c99886

beena352 requested a review from Copilot June 3, 2026 04:13

Copilot started reviewing on behalf of beena352 June 3, 2026 04:13 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread src/windows/wslcsession/WSLCSessionFactory.cpp Outdated

beena352 marked this pull request as ready for review June 3, 2026 17:40

beena352 requested a review from a team as a code owner June 3, 2026 17:40

beena352 marked this pull request as draft June 3, 2026 17:41

beena352 changed the title ~~Add marker-file validation for WSLC session storage~~ Reject non-empty directories for new WSLC session storage Jun 3, 2026

beena352 marked this pull request as ready for review June 3, 2026 17:43

beena352 marked this pull request as draft June 4, 2026 22:18

resolve conflict

d8f44ff

beena352 requested a review from Copilot June 4, 2026 23:20

Copilot started reviewing on behalf of beena352 June 4, 2026 23:20 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Comment thread src/windows/wslcsession/WSLCSession.cpp

beena352 added 2 commits June 5, 2026 08:43

Validate that session storage path is a directory, not a file

b3b908b

Format

61a3c6b

beena352 requested a review from Copilot June 5, 2026 15:46

Copilot started reviewing on behalf of beena352 June 5, 2026 15:46 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

Comment thread src/windows/wslcsession/WSLCSession.cpp

beena352 marked this pull request as ready for review June 5, 2026 16:12

Restore defensive create_directories call before VHD creation

45b2731

OneBlue reviewed Jun 9, 2026

View reviewed changes

Conversation

beena352 commented May 27, 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

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

OneBlue left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

OneBlue Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

beena352 commented May 27, 2026 •

edited

Loading