Skip to content

Address review follow-up on WSLContainerRegistryAllowlist policy #41306

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

Merged
beena352 merged 2 commits into from
Aug 11, 2026
+29 −72
Merged

Address review follow-up on WSLContainerRegistryAllowlist policy #41306

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d18588c
Address follow-up nits
beena352 Aug 10, 2026
b78301c
Preserve underlying HRESULT on enumeration failure
beena352 Aug 10, 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
33 changes: 8 additions & 25 deletions src/windows/inc/wslpolicies.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,11 @@ struct RegistryAllowlistSnapshot
std::vector<std::wstring> Hosts{};
};

// Reads the allowlist in one shot. Empty entries are skipped (matches EnumerateRegistryAllowlist).
// Throws with an "invalid policy" user error when the sub-key exists but can't be read (bad ACL,
// corrupted values, etc.) so the caller fails closed.
inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY policiesKey)
// subKey must be the WSLContainerRegistryAllowlist sub-key, not the policies root.
inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY subKey)
{
if (policiesKey == nullptr)
{
return {};
}

wil::unique_hkey subKey;
const auto openResult = RegOpenKeyExW(policiesKey, c_wslContainerRegistryAllowlist, 0, KEY_READ, &subKey);
if (openResult == ERROR_PATH_NOT_FOUND || openResult == ERROR_FILE_NOT_FOUND)
{
return {};
}

THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(openResult), wsl::shared::Localization::MessageRegistryAllowlistPolicyInvalid(), openResult != ERROR_SUCCESS);

RegistryAllowlistSnapshot snapshot;
for (auto& [name, value] : wsl::windows::common::registry::EnumStringValues(subKey.get()))
for (auto& [name, value] : wsl::windows::common::registry::EnumStringValues(subKey))
{
if (value.empty())
{
Expand All @@ -253,12 +236,12 @@ inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshot(HKEY policiesKey)
return snapshot;
}

// Convenience for callers with no open policies key. Throws MessageRegistryAllowlistPolicyInvalid
// when the policies key can't be opened.
// Throws MessageRegistryAllowlistPolicyInvalid on an unreadable sub-key so callers fail closed.
inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshotFromPoliciesRoot()
{
wil::unique_hkey policiesKey;
const auto openResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, c_registryKey, 0, KEY_READ, &policiesKey);
const auto subKeyPath = std::wstring{c_registryKey} + L"\\" + c_wslContainerRegistryAllowlist;
wil::unique_hkey subKey;
const auto openResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, subKeyPath.c_str(), 0, KEY_READ, &subKey);
if (openResult == ERROR_PATH_NOT_FOUND || openResult == ERROR_FILE_NOT_FOUND)
{
return {};
Expand All @@ -267,7 +250,7 @@ inline RegistryAllowlistSnapshot ReadRegistryAllowlistSnapshotFromPoliciesRoot()
THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(openResult), wsl::shared::Localization::MessageRegistryAllowlistPolicyInvalid(), openResult != ERROR_SUCCESS);

return ReadRegistryAllowlistSnapshot(policiesKey.get());
return ReadRegistryAllowlistSnapshot(subKey.get());
Comment thread
beena352 marked this conversation as resolved.
}

} // namespace wsl::windows::policies
62 changes: 15 additions & 47 deletions test/windows/PolicyTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,14 @@ class PolicyTest
VERIFY_ARE_EQUAL(expected, stderrText);
}

// Two variants: BuildKit echoes the caller's Dockerfile spelling in the "failed to solve" prefix.
static constexpr auto c_denialPatternExplicitAlpine =
"*failed to solve: docker.io/library/alpine:latest: could not resolve image due to policy: "
"source \"docker-image://docker.io/library/alpine:latest\" denied by policy: source denied by policy*";
static constexpr auto c_denialPatternImplicitAlpine =
"*failed to solve: alpine:latest: could not resolve image due to policy: "
"source \"docker-image://docker.io/library/alpine:latest\" denied by policy: source denied by policy*";

// Verifies WSLContainerRegistryAllowlist blocks `wslc image build` when the FROM base image
// isn't in the allowlist. Matches the `RegistryAllowlistDenies` pull test.
WSLC_TEST_METHOD(RegistryAllowlistBlocksImageBuild)
Expand All @@ -490,11 +498,7 @@ class PolicyTest
auto [exitCode, output] = RunImageBuild(L"FROM docker.io/library/alpine:latest\n", L"wsl-policy-build-blocked");

VERIFY_ARE_NOT_EQUAL(0, exitCode);
if (output.find(L"docker.io") == std::wstring::npos || output.find(L"denied by policy") == std::wstring::npos)
{
LogError("Expected BuildKit source-policy denial mentioning docker.io, got: '%ls'", output.c_str());
VERIFY_FAIL();
}
VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternExplicitAlpine);
}

// Positive path: build must proceed when FROM is on the allowlist.
Expand Down Expand Up @@ -537,11 +541,7 @@ class PolicyTest
auto [exitCode, output] = RunImageBuild(dockerfile, L"wsl-policy-build-copyfrom");

VERIFY_ARE_NOT_EQUAL(0, exitCode);
if (output.find(L"docker.io") == std::wstring::npos || output.find(L"denied by policy") == std::wstring::npos)
{
LogError("Expected COPY --from=docker.io/... to be blocked, got: '%ls'", output.c_str());
VERIFY_FAIL();
}
VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternExplicitAlpine);
}

WSLC_TEST_METHOD(RegistryAllowlistBlocksImageBuildImplicitDockerIo)
Expand All @@ -551,12 +551,7 @@ class PolicyTest
auto [exitCode, output] = RunImageBuild(L"FROM alpine:latest\n", L"wsl-policy-build-implicit");

VERIFY_ARE_NOT_EQUAL(0, exitCode);
if (output.find(L"denied by policy") == std::wstring::npos ||
(output.find(L"docker.io") == std::wstring::npos && output.find(L"alpine") == std::wstring::npos))
{
LogError("Expected bare `FROM alpine:latest` to be blocked, got: '%ls'", output.c_str());
VERIFY_FAIL();
}
VerifyPatternMatch(wsl::shared::string::WideToMultiByte(output), c_denialPatternImplicitAlpine);
}

// Runs `wslc image build` with the supplied Dockerfile content and returns the exit code
Expand Down Expand Up @@ -648,23 +643,12 @@ class PolicyTest
}
}

// Pure-function tests for ReadRegistryAllowlistSnapshot (used by `wslc image build` to
// decide between fail-open-no-policy, generate-source-policy, and fail-closed paths).
// The (HKEY) overload is exercised transitively via FromPoliciesRoot.
TEST_METHOD(ReadRegistryAllowlistSnapshot_Logic)
{
// Null policies key -> NotConfigured, no hosts.
{
const auto snapshot = ReadRegistryAllowlistSnapshot(nullptr);
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
VERIFY_IS_TRUE(snapshot.Hosts.empty());
}

const auto policiesKey = OpenPoliciesKey();
VERIFY_IS_TRUE(!!policiesKey);

// No sub-key -> NotConfigured.
{
const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
VERIFY_IS_TRUE(snapshot.Hosts.empty());
}
Expand All @@ -673,33 +657,17 @@ class PolicyTest
// items must not silently deny every registry).
{
auto revert = SetRegistryAllowlist({L"", L""});
const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
VERIFY_IS_TRUE(snapshot.Hosts.empty());
}

// Sub-key with hosts -> Configured, hosts populated in order.
{
auto revert = SetRegistryAllowlist({L"mcr.microsoft.com", L"Docker.IO"});
const auto snapshot = ReadRegistryAllowlistSnapshot(policiesKey.get());
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::Configured);
VERIFY_ARE_EQUAL(size_t{2}, snapshot.Hosts.size());
}
}

TEST_METHOD(ReadRegistryAllowlistSnapshotFromPoliciesRoot_Logic)
{
{
const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::NotConfigured);
VERIFY_IS_TRUE(snapshot.Hosts.empty());
}

{
auto revert = SetRegistryAllowlist({L"mcr.microsoft.com"});
const auto snapshot = ReadRegistryAllowlistSnapshotFromPoliciesRoot();
VERIFY_IS_TRUE(snapshot.State == RegistryAllowlistState::Configured);
VERIFY_ARE_EQUAL(size_t{1}, snapshot.Hosts.size());
VERIFY_ARE_EQUAL(size_t{2}, snapshot.Hosts.size());
}
}
};
Loading