Add Uid/Gid/Mode/Fixed driver options to WSLC VHD volumes; expose via SDK#40476
Draft
benhillis wants to merge 4 commits intomicrosoft:masterfrom
Draft
Add Uid/Gid/Mode/Fixed driver options to WSLC VHD volumes; expose via SDK#40476benhillis wants to merge 4 commits intomicrosoft:masterfrom
benhillis wants to merge 4 commits intomicrosoft:masterfrom
Conversation
… SDK
The WSLC named-volume "vhd" driver only supported a single SizeBytes
option, so containers running as a non-root user could not write to
their own persistent volumes (mkfs.ext4 leaves the root owned by
root:root with mode 0755). It also could not produce a fully-allocated
VHD, which some workloads need for predictable I/O.
Service side
============
* Adds new VHD driver options on top of SizeBytes:
- Fixed=true|false pre-allocate the underlying VHD
- Uid=<n> chown the volume root to uid (paired with Gid)
- Gid=<n> chown the volume root to gid (paired with Uid)
- Mode=<octal> chmod the volume root, max 07777, must be > 0
* Extracts a reusable OptionParser helper for typed option parsing
with errno-capture, end-pointer validation, leading-sign rejection,
consumed-key tracking, and a final RejectUnknown() pass. Used by
WSLCVhdVolume's Create and Open paths so persisted metadata is
validated identically on reload.
Public C SDK
============
WslcVhdRequirements grows three new uint32_t fields (uid/gid/mode) and
a WslcVhdRequirementsFlags bitmask. WslcCreateSessionVhdVolume:
* honors WSLC_VHD_TYPE_FIXED (was previously E_NOTIMPL)
* dynamically builds WSLCDriverOption[] based on which flags are set
* rejects unknown type values, unknown flag bits, and mode == 0 with
E_INVALIDARG so future flag additions cannot be silently ignored
by older SDK versions and obvious foot-guns are caught client-side.
WslcSetSessionSettingsVhd does NOT plumb owner/mode/fixed through the
session rootfs VHD path, and now rejects flags != NONE with
E_INVALIDARG instead of silently ignoring them.
WSLC_SESSION_OPTIONS_SIZE bumps 80 -> 96 to match the wider embedded
WslcVhdRequirements; this is an ABI break, callers must recompile.
WinRT projection
================
VhdRequirements gains:
void SetOwner(UInt32 uid, UInt32 gid);
void SetMode(UInt32 mode);
These set the corresponding flag bit and field on the underlying
struct. Pair-based SetOwner avoids the half-set foot-gun that
per-property setters would create.
Tests
=====
* WSLCTests.cpp: NamedVolumeVhdOptionsParseTest covers SizeBytes,
unknown keys, sign rejection, range and base validation; a
positive owner+mode test exercises chown/chmod end-to-end; a
Fixed-allocation test asserts on-disk file_size >= requested size.
* WslcSdkTests.cpp adds invalid-type, fixed-allocation, owner+mode
positive, mode-out-of-range negative, mode==0 negative, unknown
flag negative, and flags=NONE-ignores-uid/gid positive cases.
The WinRT projection has no test infrastructure in the repo and is
not unit-tested; behavior is covered at the C SDK layer that the
projection delegates to.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for additional vhd named-volume driver options (POSIX Uid/Gid/Mode and Fixed allocation) and exposes them through the public WSLC C SDK and WinRT projection so SDK callers can create volumes usable by non-root container users and optionally pre-allocate VHD space.
Changes:
- Added a shared
OptionParserutility and updated the VHD volume driver to parse/validateSizeBytes,Fixed,Uid/Gid, andMode, applying ownership/permissions at volume creation time. - Extended the C SDK
WslcVhdRequirementscontract (new flags + fields) and updatedWslcCreateSessionVhdVolume/WslcSetSessionSettingsVhdbehavior accordingly. - Added/updated Windows tests for driver-option parsing and SDK behavior, plus a new localized error string for invalid volume options.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLCTests.cpp | Expands VHD named-volume option parsing tests; adds end-to-end ownership/mode and fixed-allocation coverage. |
| test/windows/WslcSdkTests.cpp | Updates SDK tests for fixed VHD support, new owner/mode flags, and validation behavior. |
| src/windows/wslcsession/WSLCVhdVolume.cpp | Implements parsing/validation for new VHD driver options and applies chown/chmod during volume creation; uses shared parser. |
| src/windows/wslcsession/OptionParser.h | Introduces typed driver-option parsing interface (templates + consumed-key tracking). |
| src/windows/wslcsession/OptionParser.cpp | Implements lookup, bool parsing, unknown-option rejection, and localized error throwing for OptionParser. |
| src/windows/wslcsession/CMakeLists.txt | Wires new OptionParser sources into the wslcsession build. |
| src/windows/WslcSDK/wslcsdk.h | Extends public C SDK types (WslcVhdRequirementsFlags, larger WslcVhdRequirements, updated session options size). |
| src/windows/WslcSDK/wslcsdk.cpp | Updates WslcCreateSessionVhdVolume to emit new driver options and WslcSetSessionSettingsVhd to reject unsupported flags. |
| src/windows/WslcSDK/winrt/wslcsdk.idl | Extends WinRT VhdRequirements projection with SetOwner/SetMode. |
| src/windows/WslcSDK/winrt/VhdRequirements.h | Adds WinRT wrapper method declarations for owner/mode setters. |
| src/windows/WslcSDK/winrt/VhdRequirements.cpp | Implements WinRT owner/mode setters by setting C SDK struct fields and flags. |
| localization/strings/en-US/Resources.resw | Adds localized MessageWslcInvalidVolumeOption string used for consistent option-validation errors. |
benhillis
pushed a commit
to benhillis/WSL
that referenced
this pull request
May 8, 2026
Five findings from the Copilot pull-request reviewer: 1. wslcsdk.cpp: WslcCreateSessionVhdVolume unconditionally formatted options->uid / gid / mode via std::to_string and std::format even when the corresponding flag was not set. The header documents those fields as honored only when the flag is set, so a defensive caller could leave them uninitialized. Reading uninitialized memory is UB. Now only materialize uid/gid strings when FLAG_OWNER is set, and mode string when FLAG_MODE is set. 2. wslcsdk.idl: SetOwner/SetMode comments said they 'have no effect' on a VhdRequirements used with the session rootfs VHD. With the newly-strict WslcSetSessionSettingsVhd those flags now produce E_INVALIDARG instead of being silently ignored. Updated the IDL doc-comments to say the assignment will fail. 3. WSLCVhdVolume.cpp: service-side parser still accepted Mode=0, leaving direct COM callers (and persisted metadata reload) able to bypass the SDK-side check. Mode==0 is now rejected by Parse() for parity across all entry points. 4. WslcSdkTests.cpp: the owner+mode positive case only created and deleted the volume; nothing actually verified that chown/chmod were applied. Now mounts the volume into a debian:latest container and runs 'stat -c %u %g %a /data', asserting the output matches the requested 65534 65534 750. 5. OptionParser.h: lifetime-contract doc-comment was misleading — it implied accessors return references into the input map. In practice only Find() returns a pointer (used internally); the numeric/bool accessors return parsed values by value. Reworded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Five findings from the Copilot pull-request reviewer: 1. wslcsdk.cpp: WslcCreateSessionVhdVolume unconditionally formatted options->uid / gid / mode via std::to_string and std::format even when the corresponding flag was not set. The header documents those fields as honored only when the flag is set, so a defensive caller could leave them uninitialized. Reading uninitialized memory is UB. Now only materialize uid/gid strings when FLAG_OWNER is set, and mode string when FLAG_MODE is set. 2. wslcsdk.idl: SetOwner/SetMode comments said they 'have no effect' on a VhdRequirements used with the session rootfs VHD. With the newly-strict WslcSetSessionSettingsVhd those flags now produce E_INVALIDARG instead of being silently ignored. Updated the IDL doc-comments to say the assignment will fail. 3. WSLCVhdVolume.cpp: service-side parser still accepted Mode=0, leaving direct COM callers (and persisted metadata reload) able to bypass the SDK-side check. Mode==0 is now rejected by Parse() for parity across all entry points. 4. WslcSdkTests.cpp: the owner+mode positive case only created and deleted the volume; nothing actually verified that chown/chmod were applied. Now mounts the volume into a debian:latest container and runs 'stat -c %u %g %a /data', asserting the output matches the requested 65534 65534 750. 5. OptionParser.h: lifetime-contract doc-comment was misleading — it implied accessors return references into the input map. In practice only Find() returns a pointer (used internally); the numeric/bool accessors return parsed values by value. Reworded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b06edbb to
a576eac
Compare
Reviewer pointed out the service-side Mode parse tests had thorough coverage for non-octal, too-large, signed, and empty values, but no explicit case for the documented invalid value Mode=0 (spec is 1..07777). Mode==0 was already rejected by Parse() in the prior commit; this just locks the behavior in place. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reviewer noted that the IDL doc-comment promised SetMode rejected out-of-range/zero values, but the WinRT setter blindly stored the value and validation only fired hours later inside CreateVolume. SetMode now throws hresult_invalid_argument for mode == 0 or mode > 07777 so callers see immediate failure at the API boundary. SetOwner doesn't need a parallel check — uid/gid are uint32_t and all values are valid POSIX user/group IDs. Also tightened the IDL comment to say validation happens at the setter (not deferred to creation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Adds POSIX
Uid/Gid/Modeand pre-allocatedFixeddriver options to the WSLC named-volumevhddriver, and plumbs them through the public C SDK and the WinRT projection.Today, containers running as a non-root user cannot write to their own persistent volumes —
mkfs.ext4leaves the root owned byroot:rootwith mode0755, and thevhddriver only acceptedSizeBytes. The new options let the SDK consumer create a volume that is immediately usable by the in-container user.Fixedlets workloads that need predictable I/O pre-allocate the VHD instead of growing it dynamically.Driver options (named volumes)
SizeBytesFixedtrue/false/1/0)falseUidGidGidUidMode1..07777All option parsing now goes through a new shared
OptionParserhelper (Required<T>/Optional<T>/OptionalBool/RejectUnknown) with errno-capture, end-pointer validation, leading-sign rejection, and consumed-key tracking. TheOpenpath uses the same parser so persisted metadata is validated identically on reload.C SDK (
wslcsdk.h/wslcsdk.dll)WslcCreateSessionVhdVolume:WSLC_VHD_TYPE_FIXED(was previouslyE_NOTIMPL)WSLCDriverOption[]based on the flag bitstypevalues, unknown flag bits, andmode == 0withE_INVALIDARGso future additions cannot be silently fail-open and obvious foot-guns are caught client-sidemodeas octal (std::format("{:o}", mode)) so the service-side base-8 parser round-trips it correctlyWslcSetSessionSettingsVhddoes not plumb owner/mode/fixed through the session rootfs VHD path and now rejectsflags != NONEwithE_INVALIDARGinstead of silently ignoring those fields.Important
ABI break:
WSLC_SESSION_OPTIONS_SIZEbumps80 → 96to match the wider embeddedWslcVhdRequirements. Callers ofwslcsdk.lib/wslcsdk.dllmust recompile against the new header.Caller usage
WinRT projection (
Microsoft.WSL.Containers)runtimeclass VhdRequirements { VhdRequirements(String name, UInt64 sizeInBytes, VhdType type); String Name { get; }; UInt64 SizeInBytes { get; }; VhdType Type { get; }; void SetOwner(UInt32 uid, UInt32 gid); void SetMode(UInt32 mode); }Pair-based
SetOwneravoids the half-set foot-gun that per-property setters would create. Default-constructedVhdRequirements(or any path that does not call the new setters) getsflags = NONE= old behavior.The WinRT projection has no test infrastructure in the repo (the existing
Name/SizeInBytes/Typeprojection has never had tests either). Behavior is fully covered at the C SDK layer that the projection delegates to.Tests
WSLCTests.cppNamedVolumeVhdOptionsParseTest— SizeBytes presence/range/base, unknown-key rejection (Bogus=...), sign rejectionNamedVolumesVhdOwnership— chown/chmod end-to-endNamedVolumesVhdFixed— asserts on-diskfile_size >= requestedWslcSdkTests.cppE_INVALIDARGFixedpositive (size verification on disk)OWNER | MODEpositive (uid=65534, gid=65534, mode=0750)E_INVALIDARGE_INVALIDARGE_INVALIDARGflags=NONEwith non-zero uid/gid silently ignoredValidation status
cmake --buildclean (wslservice,wslcsession,wslcsdk,wslcsdkwinrt,wsltests)FormatSource.ps1 -ModifiedOnly $true— clang-format reports no changesbin\x64\Debug\test.bat /name:WSLCTests::*NamedVolumeVhd*— needs Administratorbin\x64\Debug\test.bat /name:WSLCTests::*NamedVolumesVhd*— needs AdministratorWslcSdkTests— needs Administratorpython tools/devops/validate-localization.py— local Python is MS Store stubpython tools/devops/validate-copyright-headers.py— local Python is MS Store stubMarking as draft until the admin-elevated test runs and python validators land.