Skip to content

CLI: Relative volume path support, improved error detection and messaging, more volume tests #40193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dkbennett wants to merge 5 commits into
base: feature/wsl-for-apps
Choose a base branch
from
Open

CLI: Relative volume path support, improved error detection and messaging, more volume tests #40193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c9f722d
Implement relative paths and tests
dkbennett Apr 15, 2026
ed8ebdd
Merge branch 'feature/wsl-for-apps' into user/dkbennett/relativevolum…
dkbennett Apr 15, 2026
db71708
Update localization/strings/en-US/Resources.resw
dkbennett Apr 15, 2026
21605c9
Feedback
dkbennett Apr 15, 2026
4881ce7
Merge branch 'user/dkbennett/relativevolumepaths' of https://github.c…
dkbennett Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions localization/strings/en-US/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2554,4 +2554,28 @@ On first run, creates the file with all settings commented out at their defaults
<data name="WSLCCLI_ImageLoadNoInputError" xml:space="preserve">
<value>Requested load but no input provided.</value>
</data>
<data name="WSLCCLI_VolumeFormatUsage" xml:space="preserve">
<value>Expected format: &lt;host path | named volume&gt;:&lt;container path&gt;[:mode]</value>
<comment>Usage string for volume mount specification.</comment>
</data>
<data name="WSLCCLI_VolumeInvalidSpec" xml:space="preserve">
<value>Invalid volume specifications: '{}'. {}</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
Comment thread
dkbennett marked this conversation as resolved.
<data name="WSLCCLI_VolumeHostPathEmpty" xml:space="preserve">
<value>Invalid volume specifications: '{}'. Host path cannot be empty. {}</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="WSLCCLI_VolumeContainerPathEmpty" xml:space="preserve">
<value>Invalid volume specifications: '{}'. Container path cannot be empty. {}</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="WSLCCLI_VolumeContainerPathNotAbsolute" xml:space="preserve">
<value>Invalid volume specifications: '{}'. Container path must be an absolute path (starting with '/'). {}</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="WSLCCLI_VolumeHostPathInvalid" xml:space="preserve">
<value>Invalid volume specifications: '{}'. Host path '{}' is not a valid Windows path.</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
</root>
63 changes: 47 additions & 16 deletions src/windows/wslc/services/ContainerModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,21 @@ void PublishPort::Validate() const
}
}

// Returns true if the given string is a valid Docker named volume name.
// Based on Docker's named volume validation: ^[a-zA-Z0-9][a-zA-Z0-9_.-]{1,}$
// Source: https://github.com/moby/moby/blob/master/volume/validate.go
bool VolumeMount::IsValidNamedVolumeName(const std::wstring& name)
Copy link
Copy Markdown

@AmelBawa-msft AmelBawa-msft Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this validation and the original reference!

{
static const std::wregex namedVolumeRegex(LR"(^[a-zA-Z0-9][a-zA-Z0-9_.-]{1,}$)");
return std::regex_match(name, namedVolumeRegex);
}

VolumeMount VolumeMount::Parse(const std::wstring& value)
{
auto lastColon = value.rfind(':');
if (lastColon == std::wstring::npos)
{
THROW_HR_WITH_USER_ERROR(
E_INVALIDARG, std::format(L"Invalid volume specifications: '{}'. Expected format: <host path>:<container path>[:mode]", value));
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage()));
}
Comment thread
dkbennett marked this conversation as resolved.

VolumeMount vm;
Expand All @@ -167,17 +175,13 @@ VolumeMount VolumeMount::Parse(const std::wstring& value)
vm.m_isReadOnlyMode = IsReadOnlyMode(lastToken);
if (lastColon == 0)
{
THROW_HR_WITH_USER_ERROR(
E_INVALIDARG,
std::format(L"Invalid volume specifications: '{}'. Expected format: <host path>:<container path>[:mode]", value));
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage()));
}

splitColon = value.rfind(':', lastColon - 1);
if (splitColon == std::wstring::npos)
{
THROW_HR_WITH_USER_ERROR(
E_INVALIDARG,
std::format(L"Invalid volume specifications: '{}'. Expected format: <host path>:<container path>[:mode]", value));
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeInvalidSpec(value, Localization::WSLCCLI_VolumeFormatUsage()));
}

vm.m_containerPath = WideToMultiByte(value.substr(splitColon + 1, lastColon - splitColon - 1));
Comment thread
dkbennett marked this conversation as resolved.
Expand All @@ -187,22 +191,49 @@ VolumeMount VolumeMount::Parse(const std::wstring& value)
vm.m_containerPath = WideToMultiByte(lastToken);
}

vm.m_hostPath = value.substr(0, splitColon);

if (vm.m_hostPath.empty())
if (vm.m_containerPath.empty())
{
THROW_HR_WITH_USER_ERROR(
E_INVALIDARG,
std::format(L"Invalid volume specifications: '{}'. Host path cannot be empty. Expected format: <host path>:<container path>[:mode]", value));
E_INVALIDARG, Localization::WSLCCLI_VolumeContainerPathEmpty(value, Localization::WSLCCLI_VolumeFormatUsage()));
}

if (!vm.m_containerPath.empty() && vm.m_containerPath[0] != '/')
if (vm.m_containerPath[0] != '/')
{
THROW_HR_WITH_USER_ERROR(
E_INVALIDARG,
std::format(L"Invalid volume specifications: '{}'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]", value));
E_INVALIDARG, Localization::WSLCCLI_VolumeContainerPathNotAbsolute(value, Localization::WSLCCLI_VolumeFormatUsage()));
}

const auto rawHostPath = value.substr(0, splitColon);
if (rawHostPath.empty())
{
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeHostPathEmpty(value, Localization::WSLCCLI_VolumeFormatUsage()));
}

// This is where we need to check if the user is referencing a named volume.
// This can be either an existing named volume or a new named volume that will be created.
if (VolumeMount::IsValidNamedVolumeName(rawHostPath))
{
// TODO: Handle named volume reference in the request.
Copy link
Copy Markdown

@AmelBawa-msft AmelBawa-msft Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this block. I will investigate what is needed to wire this up

// For now we will ignore this and treat named volumes as a relative path.
Comment thread
dkbennett marked this conversation as resolved.
}

// Not a named volume, so it must be a path.
// Use wil::GetFullPathNameW to resolve relative paths against the CWD.
std::wstring resolvedHostPath;
if (FAILED(wil::GetFullPathNameW(rawHostPath.c_str(), resolvedHostPath)))
{
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeHostPathInvalid(value, rawHostPath));
Comment thread
dkbennett marked this conversation as resolved.
}

// GetFileAttributesW validates the resolved path syntax without requiring existence.
// ERROR_INVALID_NAME indicates illegal characters in the path (e.g. ":" as a component).
if (GetFileAttributesW(resolvedHostPath.c_str()) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_INVALID_NAME)
{
THROW_HR_WITH_USER_ERROR(E_INVALIDARG, Localization::WSLCCLI_VolumeHostPathInvalid(value, rawHostPath));
Comment thread
dkbennett marked this conversation as resolved.
}

vm.m_hostPath = std::move(resolvedHostPath);

return vm;
}

Expand Down
3 changes: 3 additions & 0 deletions src/windows/wslc/services/ContainerModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ struct VolumeMount
{
return m_isReadOnlyMode;
}

static bool IsValidNamedVolumeName(const std::wstring& name);

static VolumeMount Parse(const std::wstring& value);

private:
Expand Down
4 changes: 2 additions & 2 deletions test/windows/wslc/CommandLineTestCases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ COMMAND_LINE_TEST_CASE(L"run --rm -it --entrypoint bash archlinux:latest -c \"ec
COMMAND_LINE_TEST_CASE(L"run --rm --entrypoint /bin/bash debian:latest -c ls", L"run", true)
COMMAND_LINE_TEST_CASE(L"run jrottenberg/ffmpeg:4.4-alpine -i http://url/to/media.mp4 -stats", L"run", true)
COMMAND_LINE_TEST_CASE(
L"run -v ${PWD}:/data jrottenberg/ffmpeg:4.4-scratch -stats -i http://www.hevc-10bit.mkv -c:v libx265 -pix_fmt yuv420p10 -t "
L"run -v ./:/data jrottenberg/ffmpeg:4.4-scratch -stats -i http://www.hevc-10bit.mkv -c:v libx265 -pix_fmt yuv420p10 -t "
L"5 -f mp4 test.mp4",
L"run",
true)
COMMAND_LINE_TEST_CASE(
L"run -v ${PWD}:/data -it jrottenberg/ffmpeg:4.4-scratch -stats -i https://file-examples/file_example_MP4_480_1_5MG.mp4 -c:v "
L"run -v ./:/data -it jrottenberg/ffmpeg:4.4-scratch -stats -i https://file-examples/file_example_MP4_480_1_5MG.mp4 -c:v "
L"libx265 -pix_fmt yuv420p10 -t 5 -f mp4 /dataout.mp4",
L"run",
true)
Expand Down
82 changes: 70 additions & 12 deletions test/windows/wslc/WSLCVolumeMountUnitTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,25 @@ class WSLCVolumeMountUnitTests

TEST_METHOD(VolumeMount_Parse_ReturnExpectedResult)
{
const auto cwd = std::filesystem::current_path();

// Volume value => host, container, readonly
std::vector<std::tuple<std::wstring, std::wstring, std::string, bool>> validVolumeArgs = {
{LR"(C:\hostPath:/containerPath)", LR"(C:\hostPath)", R"(/containerPath)", false},
{LR"(C:\hostPath:/containerPath:ro)", LR"(C:\hostPath)", R"(/containerPath)", true},
{LR"(C:\hostPath:/containerPath:rw)", LR"(C:\hostPath)", R"(/containerPath)", false},
{LR"(C:\host Path:/container Path:ro)", LR"(C:\host Path)", R"(/container Path)", true},
{LR"(C:\host Path:/container Path:rw)", LR"(C:\host Path)", R"(/container Path)", false},
{LR"(C:\hostPath::ro)", LR"(C:\hostPath)", R"()", true},
{LR"(C:\hostPath:)", LR"(C:\hostPath)", R"()", false},
{LR"(C:\hostPath::rw)", LR"(C:\hostPath)", R"()", false},
{LR"(C:\hostPath:/containerPath:)", LR"(C:\hostPath:/containerPath)", R"()", false},
{LR"(C:/hostPath:ro)", LR"(C)", R"(/hostPath)", true},
{LR"(C:/hostPath)", LR"(C)", R"(/hostPath)", false},
{LR"(::)", LR"(:)", R"()", false},

// Relative paths. Expected host is CWD + the normalized relative component.
// Windows will convert forward slashes to backslashes in the host path.
{L"./foo:/data", (cwd / L"foo").wstring(), R"(/data)", false},
{L".\\foo:/data", (cwd / L"foo").wstring(), R"(/data)", false},
{L"./foo:/data:ro", (cwd / L"foo").wstring(), R"(/data)", true},
{L"../bar:/data:rw", (cwd.parent_path() / L"bar").wstring(), R"(/data)", false},
{L"..\\bar:/data:rw", (cwd.parent_path() / L"bar").wstring(), R"(/data)", false},
{L"sub/dir:/data", (cwd / L"sub" / L"dir").wstring(), R"(/data)", false},
{L"sub\\dir:/data", (cwd / L"sub" / L"dir").wstring(), R"(/data)", false},
};

for (const auto& arg : validVolumeArgs)
Expand All @@ -54,13 +59,18 @@ class WSLCVolumeMountUnitTests

TEST_METHOD(VolumeMount_Parse_InvalidArgs)
{
std::vector<std::wstring> emptyHostPathCases = {
LR"(:/containerPath)",
LR"(:/containerPath:ro)",
LR"(:)",
std::vector<std::wstring> invalidCases = {
LR"(:/containerPath)", // Empty host path
LR"(:/containerPath:ro)", // Empty host path
LR"(:)", // Empty container path
LR"(::)", // Empty container path
LR"(C:\hostPath::ro)", // Empty container path
LR"(C:\hostPath:)", // Empty container path
LR"(C:\hostPath::rw)", // Empty container path
LR"(C:\hostPath:/containerPath:)", // Empty container path
};

for (const auto& value : emptyHostPathCases)
for (const auto& value : invalidCases)
{
WEX::Logging::Log::Comment(std::format(L"Testing invalid volume argument: '{}'", value).c_str());
VERIFY_THROWS(VolumeMount::Parse(value), wil::ResultException);
Expand All @@ -84,5 +94,53 @@ class WSLCVolumeMountUnitTests
VERIFY_THROWS(VolumeMount::Parse(value), wil::ResultException);
}
}

TEST_METHOD(VolumeMount_IsValidNamedVolumeName_ValidNames)
{
// These should all be recognised as valid Docker named volume names.
std::vector<std::wstring> validNames = {
L"myvolume", // simple lowercase
L"MyVolume", // mixed case
L"my-volume", // hyphen
L"my_volume", // underscore
L"my.volume", // dot
L"v1", // minimum length (2 chars)
L"volume123", // trailing digits
L"my-vol_1.0", // combination of all allowed characters
};

for (const auto& name : validNames)
{
WEX::Logging::Log::Comment(std::format(L"Testing valid named volume name: '{}'", name).c_str());
VERIFY_IS_TRUE(VolumeMount::IsValidNamedVolumeName(name));
}
}

TEST_METHOD(VolumeMount_IsValidNamedVolumeName_InvalidNames)
{
// These should all be rejected. They are either paths or otherwise invalid.
std::vector<std::wstring> invalidNames = {
L"./foo", // relative path with ./
L"../foo", // relative path with ../
L".hidden", // starts with '.' (relative path indicator)
L"foo/bar", // contains forward slash (path separator)
L"foo\\bar", // contains backslash (path separator)
L"C:\\path\\to\\dir", // absolute Windows path
L"/absolute/path", // absolute Unix-style path
L"a", // too short (single character)
L"", // empty
L":volume", // starts with invalid character
L"-volume", // starts with hyphen (not alphanumeric)
L"_volume", // starts with underscore (not alphanumeric)
L"vol ume", // contains space
L"vol:ume", // contains colon
};

for (const auto& name : invalidNames)
{
WEX::Logging::Log::Comment(std::format(L"Testing invalid named volume name: '{}'", name).c_str());
VERIFY_IS_FALSE(VolumeMount::IsValidNamedVolumeName(name));
}
}
};
} // namespace WSLCVolumeMount
Loading