diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index cf22c48572..ed6ed2c0b8 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -38,4 +38,4 @@ The PowerShell module now automatically uses `GH_TOKEN` or `GITHUB_TOKEN` enviro ## Bug Fixes - +* DSC export now correctly exports WinGet Admin Settings diff --git a/src/AppInstallerCLITests/AdminSettings.cpp b/src/AppInstallerCLITests/AdminSettings.cpp index a7fa9d2a15..8f91ec2c93 100644 --- a/src/AppInstallerCLITests/AdminSettings.cpp +++ b/src/AppInstallerCLITests/AdminSettings.cpp @@ -4,6 +4,8 @@ #include "TestCommon.h" #include "TestSettings.h" #include +#include +#include using namespace AppInstaller::Settings; using namespace TestCommon; @@ -81,4 +83,39 @@ TEST_CASE("AdminSetting_AllSettingsAreImplemented", "[adminSettings]") REQUIRE(DisableAdminSetting(adminSetting)); REQUIRE_FALSE(IsAdminSettingEnabled(adminSetting)); } +} + +TEST_CASE("AdminSetting_CorruptedVerificationFile", "[adminSettings]") +{ + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::LocalManifestFiles, PolicyState::NotConfigured); + + // Clean up any existing admin settings + ResetAllAdminSettings(); + + // Enable the setting to create both the data and verification files + REQUIRE(EnableAdminSetting(BoolAdminSetting::LocalManifestFiles)); + REQUIRE(IsAdminSettingEnabled(BoolAdminSetting::LocalManifestFiles)); + + // Get the paths to the settings files + std::filesystem::path settingsPath = GetPathTo(Stream::AdminSettings); + std::filesystem::path secureSettingsDir = AppInstaller::Runtime::GetPathTo(AppInstaller::Runtime::PathName::SecureSettingsForRead); + std::filesystem::path verificationFilePath = secureSettingsDir / "admin_settings"; + + // Verify both files exist + REQUIRE(std::filesystem::exists(settingsPath)); + REQUIRE(std::filesystem::exists(verificationFilePath)); + + // Corrupt the verification file by writing invalid content to it + { + std::ofstream corruptFile(verificationFilePath); + corruptFile << "corrupted data that is not valid YAML or a valid hash"; + } + + // Now when we try to read the setting, it should fall back to the default value (false) + // instead of throwing an exception + REQUIRE_FALSE(IsAdminSettingEnabled(BoolAdminSetting::LocalManifestFiles)); + + // Clean up + ResetAllAdminSettings(); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/AdminSettings.cpp b/src/AppInstallerCommonCore/AdminSettings.cpp index 6d2d8634c1..619d88c0e7 100644 --- a/src/AppInstallerCommonCore/AdminSettings.cpp +++ b/src/AppInstallerCommonCore/AdminSettings.cpp @@ -180,7 +180,22 @@ namespace AppInstaller::Settings void AdminSettingsInternal::LoadAdminSettings() { - auto stream = m_settingStream.Get(); + std::unique_ptr stream; + try + { + stream = m_settingStream.Get(); + } + catch (const std::exception& e) + { + AICLI_LOG(Core, Error, << "Failed to read admin settings: " << e.what() << ". Falling back to default values."); + return; + } + catch (...) + { + AICLI_LOG(Core, Error, << "Failed to read admin settings due to unknown exception. Falling back to default values."); + return; + } + if (!stream) { AICLI_LOG(Core, Verbose, << "Admin settings was not found"); @@ -223,6 +238,8 @@ namespace AppInstaller::Settings { m_settingValues.DefaultProxy.emplace(std::move(defaultProxy)); } + + AICLI_LOG(Core, Verbose, << "Admin settings loaded successfully"); } bool AdminSettingsInternal::SaveAdminSettings() diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index 5bd770cc8e..b7694a7920 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -293,14 +293,19 @@ namespace AppInstaller::Settings return result; } - void SetVerificationData(VerificationData data) + bool SetVerificationData(VerificationData data) { YAML::Emitter out; out << YAML::BeginMap; out << YAML::Key << NodeName_Sha256 << YAML::Value << SHA256::ConvertToString(data.Hash); out << YAML::EndMap; - m_secure.Set(out.str()); + bool result = m_secure.Set(out.str()); + if (!result) + { + AICLI_LOG(Core, Warning, << "Failed to write verification data for '" << m_name << "'"); + } + return result; } public: @@ -338,7 +343,15 @@ namespace AppInstaller::Settings VerificationData verData; verData.Hash = m_hash.value(); - SetVerificationData(verData); + if (!SetVerificationData(verData)) + { + AICLI_LOG(Core, Error, << "Failed to write verification data for '" << m_name << "'. Reverting setting write to maintain consistency."); + AICLI_LOG(Core, Warning, << "This failure may be caused by insufficient permissions or disk space issues"); + // Verification data write failed, so we need to revert the main write + // to maintain consistency + Remove(); + return false; + } } return exchangeResult;