Skip to content

Commit e46cf9a

Browse files
benhillisBen Hillis
andauthored
Validate container path is absolute in VolumeMount::Parse (#40085)
* Validate container path is absolute in VolumeMount::Parse Add validation that non-empty container paths must start with '/' since they are Linux paths inside the container. This catches cases where Windows drive letter colons (e.g. C:\path) get misinterpreted as the host:container separator, producing invalid container paths like '\hostPath' instead of '/containerPath'. Previously, 'C:\hostPath:ro' would silently parse as host='C', container='\hostPath', mode=ro ΓÇö now it throws a clear error. Updated tests to reflect the new validation and moved previously 'valid' but semantically incorrect cases to the invalid test set. * Also validate host path is absolute, add format hint to error, cover forward-slash cases - Reject non-absolute host paths (catches C:/hostPath where host='C') - Add 'Expected format:' hint to container path error message for consistency - Add forward-slash drive letter cases to invalid test set - Move '::' case to invalid (host=':' is not absolute) * Remove host path is_absolute check to allow future named volumes Per review feedback: named volumes (e.g. 'test_vol:/path') would fail an is_absolute() check on the host path. Keep only the container path validation (must start with '/') which doesn't conflict with named volume identifiers. --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
1 parent e712372 commit e46cf9a

3 files changed

Lines changed: 30 additions & 8 deletions

File tree

src/windows/wslc/services/ContainerModel.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,13 @@ VolumeMount VolumeMount::Parse(const std::wstring& value)
196196
std::format(L"Invalid volume specifications: '{}'. Host path cannot be empty. Expected format: <host path>:<container path>[:mode]", value));
197197
}
198198

199+
if (!vm.m_containerPath.empty() && vm.m_containerPath[0] != '/')
200+
{
201+
THROW_HR_WITH_USER_ERROR(
202+
E_INVALIDARG,
203+
std::format(L"Invalid volume specifications: '{}'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]", value));
204+
}
205+
199206
return vm;
200207
}
201208

test/windows/wslc/WSLCVolumeMountUnitTests.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,9 @@ class WSLCVolumeMountUnitTests
3535
{LR"(C:\host Path:/container Path:rw)", LR"(C:\host Path)", R"(/container Path)", false},
3636
{LR"(C:\hostPath::ro)", LR"(C:\hostPath)", R"()", true},
3737
{LR"(C:\hostPath:)", LR"(C:\hostPath)", R"()", false},
38-
{LR"(C:\hostPath:ro)", LR"(C)", R"(\hostPath)", true},
3938
{LR"(C:\hostPath::rw)", LR"(C:\hostPath)", R"()", false},
40-
{LR"(C:\hostPath:/containerPath:invalid_mode)", LR"(C:\hostPath:/containerPath)", R"(invalid_mode)", false},
41-
{LR"(C:\hostPath:/containerPath:ro:extra)", LR"(C:\hostPath:/containerPath:ro)", R"(extra)", false},
4239
{LR"(C:\hostPath:/containerPath:)", LR"(C:\hostPath:/containerPath)", R"()", false},
43-
{LR"(C:\hostPath)", LR"(C)", R"(\hostPath)", false},
40+
{LR"(C:/hostPath:ro)", LR"(C)", R"(/hostPath)", true},
4441
{LR"(C:/hostPath)", LR"(C)", R"(/hostPath)", false},
4542
{LR"(::)", LR"(:)", R"()", false},
4643
};
@@ -69,5 +66,23 @@ class WSLCVolumeMountUnitTests
6966
VERIFY_THROWS(VolumeMount::Parse(value), wil::ResultException);
7067
}
7168
}
69+
70+
TEST_METHOD(VolumeMount_Parse_InvalidContainerPath)
71+
{
72+
// Drive colon gets misinterpreted as the host:container separator,
73+
// producing a non-absolute container path. Caught by container path validation.
74+
std::vector<std::wstring> invalidPathCases = {
75+
LR"(C:\hostPath:ro)", // host='C', container=\hostPath (not absolute)
76+
LR"(C:\hostPath)", // host='C', container=\hostPath (not absolute)
77+
LR"(C:\hostPath:/containerPath:invalid_mode)", // container=invalid_mode (not absolute)
78+
LR"(C:\hostPath:/containerPath:ro:extra)", // container=extra (not absolute)
79+
};
80+
81+
for (const auto& value : invalidPathCases)
82+
{
83+
WEX::Logging::Log::Comment(std::format(L"Testing invalid path: '{}'", value).c_str());
84+
VERIFY_THROWS(VolumeMount::Parse(value), wil::ResultException);
85+
}
86+
}
7287
};
7388
} // namespace WSLCVolumeMount

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ class WSLCE2EContainerCreateTests
290290
{
291291
auto result =
292292
RunWslc(std::format(L"container run --name {} --volume C:\\hostPath:ro {}", WslcContainerName, AlpineImage.NameAndTag()));
293-
result.Verify({.Stderr = L"The parameter is incorrect. \r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
293+
result.Verify({.Stderr = L"Invalid volume specifications: 'C:\\hostPath:ro'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
294294
EnsureContainerDoesNotExist(WslcContainerName);
295295
}
296296

@@ -310,14 +310,14 @@ class WSLCE2EContainerCreateTests
310310
{
311311
auto result = RunWslc(std::format(
312312
L"container run --name {} --volume C:\\hostPath:/containerPath:invalid_mode {}", WslcContainerName, AlpineImage.NameAndTag()));
313-
result.Verify({.Stderr = L"Unspecified error \r\nError code: E_FAIL\r\n", .ExitCode = 1});
313+
result.Verify({.Stderr = L"Invalid volume specifications: 'C:\\hostPath:/containerPath:invalid_mode'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
314314
EnsureContainerDoesNotExist(WslcContainerName);
315315
}
316316

317317
{
318318
auto result = RunWslc(std::format(
319319
L"container run --name {} --volume C:\\hostPath:/containerPath:ro:extra {}", WslcContainerName, AlpineImage.NameAndTag()));
320-
result.Verify({.Stderr = L"Unspecified error \r\nError code: E_FAIL\r\n", .ExitCode = 1});
320+
result.Verify({.Stderr = L"Invalid volume specifications: 'C:\\hostPath:/containerPath:ro:extra'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
321321
EnsureContainerDoesNotExist(WslcContainerName);
322322
}
323323

@@ -339,7 +339,7 @@ class WSLCE2EContainerCreateTests
339339
{
340340
auto result = RunWslc(
341341
std::format(L"container run --name {} --volume \"C:\\hostPath\" {}", WslcContainerName, AlpineImage.NameAndTag()));
342-
result.Verify({.Stderr = L"The parameter is incorrect. \r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
342+
result.Verify({.Stderr = L"Invalid volume specifications: 'C:\\hostPath'. Container path must be an absolute path (starting with '/'). Expected format: <host path>:<container path>[:mode]\r\nError code: E_INVALIDARG\r\n", .ExitCode = 1});
343343
EnsureContainerDoesNotExist(WslcContainerName);
344344
}
345345

0 commit comments

Comments
 (0)