Address review follow-up on WSLContainerRegistryAllowlist policy (#41306)

beena352 committed Aug 11, 2026 at 10:35 UTC e343c5e3db38b4234e742d2f71cb08603179af01
2 files changed +29 -72
src/windows/inc/wslpolicies.h
+14 -25
@@ -214,28 +214,12 @@ struct RegistryAllowlistSnapshot
214 std::vector<std::wstring> Hosts{};
215 };
216
217 -// Reads the allowlist in one shot. Empty entries are skipped (matches EnumerateRegistryAllowlist).
218 -// Throws with an "invalid policy" user error when the sub-key exists but can't be read (bad ACL,
219 -// corrupted values, etc.) so the caller fails closed.
220 -inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY policiesKey)
217 +// subKey must be the WSLContainerRegistryAllowlist sub-key; enumeration failures throw MessageRegistryAllowlistPolicyInvalid.
218 +inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY subKey)
219 +try
220 {
222 - if (policiesKey == nullptr)
223 - {
224 - return {};
225 - }
226 -
227 - wil::unique_hkey subKey;
228 - const auto openResult = RegOpenKeyExW(policiesKey, c_wslContainerRegistryAllowlist, 0, KEY_READ, &subKey);
229 - if (openResult == ERROR_PATH_NOT_FOUND || openResult == ERROR_FILE_NOT_FOUND)
230 - {
231 - return {};
232 - }
233 -
234 - THROW_HR_WITH_USER_ERROR_IF(
235 - HRESULT_FROM_WIN32(openResult), wsl::shared::Localization::MessageRegistryAllowlistPolicyInvalid(), openResult != ERROR_SUCCESS);
236 -
221 RegistryAllowlistSnapshot snapshot;
238 - for (auto& [name, value] : wsl::windows::common::registry::EnumStringValues(subKey.get()))
222 + for (auto& [name, value] : wsl::windows::common::registry::EnumStringValues(subKey))
223 {
224 if (value.empty())
225 {
@@ -252,13 +236,18 @@ inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY policiesKey)
236
237 return snapshot;
238 }
239 +catch (...)
240 +{
241 + LOG_CAUGHT_EXCEPTION();
242 + THROW_HR_WITH_USER_ERROR(wil::ResultFromCaughtException(), wsl::shared::Localization::MessageRegistryAllowlistPolicyInvalid());
243 +}
244
256 -// Convenience for callers with no open policies key. Throws MessageRegistryAllowlistPolicyInvalid
257 -// when the policies key can't be opened.
245 +// Throws MessageRegistryAllowlistPolicyInvalid on an unreadable sub-key so callers fail closed.
246 inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshotFromPoliciesRoot()
247 {
260 - wil::unique_hkey policiesKey;
261 - const auto openResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, c_registryKey, 0, KEY_READ, &policiesKey);
248 + const auto subKeyPath = std::wstring{c_registryKey} + L"\\" + c_wslContainerRegistryAllowlist;
249 + wil::unique_hkey subKey;
250 + const auto openResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, subKeyPath.c_str(), 0, KEY_READ, &subKey);
251 if (openResult == ERROR_PATH_NOT_FOUND || openResult == ERROR_FILE_NOT_FOUND)
252 {
253 return {};
@@ -267,7 +256,7 @@ inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshotFromPoliciesRoot()
256 THROW_HR_WITH_USER_ERROR_IF(
257 HRESULT_FROM_WIN32(openResult), wsl::shared::Localization::MessageRegistryAllowlistPolicyInvalid(), openResult != ERROR_SUCCESS);
258
270 - return ReadRegistryAllowlistSnapshot(policiesKey.get());
259 + return ReadRegistryAllowlistSnapshot(subKey.get());
260 }
261
262 } // namespace wsl::windows::policies
test/windows/PolicyTests.cpp
+15 -47
@@ -481,6 +481,14 @@ class PolicyTest
481 VERIFY_ARE_EQUAL(expected, stderrText);
482 }
483
484 + // Two variants: BuildKit echoes the caller's Dockerfile spelling in the "failed to solve" prefix.
485 + static constexpr auto c_denialPatternExplicitAlpine =
486 + "*failed to solve: docker.io/library/alpine:latest: could not resolve image due to policy: "
487 + "source \"docker-image://docker.io/library/alpine:latest\" denied by policy: source denied by policy*";
488 + static constexpr auto c_denialPatternImplicitAlpine =
489 + "*failed to solve: alpine:latest: could not resolve image due to policy: "
490 + "source \"docker-image://docker.io/library/alpine:latest\" denied by policy: source denied by policy*";
491 +
492 // Verifies WSLContainerRegistryAllowlist blocks `wslc image build` when the FROM base image
493 // isn't in the allowlist. Matches the `RegistryAllowlistDenies` pull test.
494 WSLC_TEST_METHOD(RegistryAllowlistBlocksImageBuild)
@@ -490,11 +498,7 @@ class PolicyTest
498 auto [exitCode, output] = RunImageBuild(L"FROM docker.io/library/alpine:latest\n", L"wsl-policy-build-blocked");
499
500 VERIFY_ARE_NOT_EQUAL(0, exitCode);
493 - if (output.find(L"docker.io") == std::wstring::npos || output.find(L"denied by policy") == std::wstring::npos)
494 - {
495 - LogError("Expected BuildKit source-policy denial mentioning docker.io, got: '%ls'", output.c_str());
496 - VERIFY_FAIL();
497 - }
501 + VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternExplicitAlpine);
502 }
503
504 // Positive path: build must proceed when FROM is on the allowlist.
@@ -537,11 +541,7 @@ class PolicyTest
541 auto [exitCode, output] = RunImageBuild(dockerfile, L"wsl-policy-build-copyfrom");
542
543 VERIFY_ARE_NOT_EQUAL(0, exitCode);
540 - if (output.find(L"docker.io") == std::wstring::npos || output.find(L"denied by policy") == std::wstring::npos)
541 - {
542 - LogError("Expected COPY --from=docker.io/... to be blocked, got: '%ls'", output.c_str());
543 - VERIFY_FAIL();
544 - }
544 + VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternExplicitAlpine);
545 }
546
547 WSLC_TEST_METHOD(RegistryAllowlistBlocksImageBuildImplicitDockerIo)
@@ -551,12 +551,7 @@ class PolicyTest
551 auto [exitCode, output] = RunImageBuild(L"FROM alpine:latest\n", L"wsl-policy-build-implicit");
552
553 VERIFY_ARE_NOT_EQUAL(0, exitCode);
554 - if (output.find(L"denied by policy") == std::wstring::npos ||
555 - (output.find(L"docker.io") == std::wstring::npos && output.find(L"alpine") == std::wstring::npos))
556 - {
557 - LogError("Expected bare `FROM alpine:latest` to be blocked, got: '%ls'", output.c_str());
558 - VERIFY_FAIL();
559 - }
554 + VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternImplicitAlpine);
555 }
556
557 // Runs `wslc image build` with the supplied Dockerfile content and returns the exit code
@@ -648,23 +643,12 @@ class PolicyTest
643 }
644 }
645
651 - // Pure-function tests for ReadRegistryAllowlistSnapshot (used by `wslc image build` to
652 - // decide between fail-open-no-policy, generate-source-policy, and fail-closed paths).
646 + // The (HKEY) overload is exercised transitively via FromPoliciesRoot.
647 TEST_METHOD(ReadRegistryAllowlistSnapshot_Logic)
648 {
655 - // Null policies key -> NotConfigured, no hosts.
656 - {
657 - const auto snapshot = ReadRegistryAllowlistSnapshot(nullptr);
658 - VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
659 - VERIFY_IS_TRUE(snapshot.Hosts.empty());
660 - }
661 -
662 - const auto policiesKey = OpenPoliciesKey();
663 - VERIFY_IS_TRUE(!!policiesKey);
664 -
649 // No sub-key -> NotConfigured.
650 {
667 - const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
651 + const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
652 VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
653 VERIFY_IS_TRUE(snapshot.Hosts.empty());
654 }
@@ -673,7 +657,7 @@ class PolicyTest
657 // items must not silently deny every registry).
658 {
659 auto revert = SetRegistryAllowlist({L"", L""});
676 - const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
660 + const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
661 VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
662 VERIFY_IS_TRUE(snapshot.Hosts.empty());
663 }
@@ -681,25 +665,9 @@ class PolicyTest
665 // Sub-key with hosts -> Configured, hosts populated in order.
666 {
667 auto revert = SetRegistryAllowlist({L"mcr.microsoft.com", L"Docker.IO"});
684 - const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
685 - VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::Configured);
686 - VERIFY_ARE_EQUAL(size_t{2}, snapshot.Hosts.size());
687 - }
688 - }
689 -
690 - TEST_METHOD(ReadRegistryAllowlistSnapshotFromPoliciesRoot_Logic)
691 - {
692 - {
693 - const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
694 - VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
695 - VERIFY_IS_TRUE(snapshot.Hosts.empty());
696 - }
697 -
698 - {
699 - auto revert = SetRegistryAllowlist({L"mcr.microsoft.com"});
668 const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
669 VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::Configured);
702 - VERIFY_ARE_EQUAL(size_t{1}, snapshot.Hosts.size());
670 + VERIFY_ARE_EQUAL(size_t{2}, snapshot.Hosts.size());
671 }
672 }
673 };