Add Uid/Gid/Fixed driver options to WSLC VHD volumes and expose via SDK (#40476)
* Add Uid/Gid/Mode/Fixed driver options to WSLC VHD volumes; expose via 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> * Address Copilot review feedback on PR #40476 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> * Add Mode=0 negative test for WSLC vhd parser 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> * Validate VhdRequirements::SetMode arguments at WinRT boundary 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> * Echo caller-provided value in SizeBytes/Mode validation errors Reviewer noted the SizeBytes==0 and Mode==0 rejection paths in VhdVolumeOptions::Parse hard-coded the literal 0 in their error messages instead of echoing the original input from DriverOpts. If a caller passed SizeBytes=00 or Mode=000, the error said '0', diverging from OptionParser's usual 'Invalid value for option <name>: <original>' wording. Both keys are guaranteed present in DriverOpts when these checks fire (Required<> already succeeded for SizeBytes; Mode.has_value() is the precondition for the Mode check), so .at() will not throw. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reject mode>07777 in C SDK and trim verbose comments The C SDK only rejected mode==0; the WinRT setter and the public header both promise mode<=07777 too. Aligning all three layers so callers see immediate, consistent E_INVALIDARG. Also a comment-bloat pass on this PR: kept "why" notes (uid/gid foot-gun, chmod 0 rationale, c_str lifetime), dropped restatements of what the code already says. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move volume Uid/Gid into mkfs -E root_owner; drop Mode option Per OneBlue review feedback: bake ownership into the ext4 root inode at format time (mkfs.ext4 -E root_owner=UID:GID) instead of spawning a post-mount chown helper inside the VM. For a fresh volume the root is the only user-visible inode so this is equivalent — anything the container later creates inherits its own uid/gid. Drop the Mode option entirely. Containers that need non-default permissions can chmod from inside (it's a per-process concern); the SDK surface stays minimal. Also drops the now-unused Base parameter from OptionParser. ABI: WslcVhdRequirements shrinks 40 -> 32 bytes; WSLC_SESSION_OPTIONS_SIZE 96 -> 88. WSLC_VHD_REQ_FLAG_MODE and VhdRequirements::SetMode are removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * WSLC: tidy comments and add Ext4Format Uid/Gid contract assert * wslcsdk.cpp: drop stale 'Owner / mode' wording from VHD-rootfs rejection comment. * wslcsdk.idl: clarify that owner-on-rootfs fails at property-set time (via SetSessionSettingsVhd), not at session creation. * WSLCVirtualMachine.cpp::Ext4Format: assert Uid.has_value() == Gid.has_value() so a future caller bypassing the parser can't silently drop ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * WSLC: drop misleading std::move on Ext4Format mkfs args WSLCProcessLauncher's constructor takes its arguments vector by const-ref, so std::move(args) here is a no-op and only obscures intent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * WSLC: trim verbose comments around VHD options Compress over-explained rationale comments in WSLCVhdVolume.cpp, WSLCVirtualMachine.cpp, OptionParser.h, wslcsdk.cpp/idl, and the matching tests. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * WSLC: address reviewer feedback on VHD option parser * OptionParser.h: include <cerrno> directly so the header is self-contained. * OptionParser: distinguish unknown keys from invalid values. RejectUnknown now throws via ThrowUnknown with a new MessageWslcUnknownVolumeOption string ('Unknown option: ...') instead of the misleading 'Invalid value for option ...' message. * WSLCVirtualMachine::Ext4Format: replace WI_ASSERT with THROW_HR_IF so a paired-Uid/Gid contract violation surfaces as a structured failure instead of a process-termination assert in production builds. * WslcSdkTests::SessionCreateVhd: add wil::scope_exit cleanup for the Fixed-VHD sub-test so a mid-test VERIFY failure can't leak the volume. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>