commit 066616193477388f3390121ad90d10e25a45836a parent cfd20d631da06f3586cc1e41a39a6815b8c894f0 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Wed, 24 Jun 2020 15:28:42 -0700 Secure settings and sources (#455) This change adds a secure setting type, and moves the source configuration to that type. This also has the effect of leaving the old source configuration behind. As multiple sources are not yet well supported, this is deemed an acceptable data loss. The secure settings are secured by storing a hash of the stream on write. This hash is stored in a location that requires administrator privileges to write, while the primary setting stream is stored in the app data. If the app is uninstalled then reinstalled, or reset from settings, this stream will be lost and the sources reset. The secure data will be left behind, but not referenced when no stream is present. If a stream is present, its hash must also be present or an exception is thrown. For sources, an attempt to tamper with the configuration would result in an unusable winget, until 'winget source reset --force' was executed, fully resetting the source data. To support not writing to the source configuration, the sources are now layered. This means that all user sources are read, then any default sources are added to the list. To allow a default source to be removed, a tombstone concept is added that will allow the 'higher' layers to override the lower ones. The update metadata is also separated out from the identity data, allowing it to be written without elevation. Diffstat:
28 files changed, 1037 insertions(+), 411 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -71,6 +71,7 @@ namespace AppInstaller::CLI void SourceAddCommand::ExecuteInternal(Context& context) const { context << + Workflow::EnsureRunningAsAdmin << Workflow::GetSourceList << Workflow::CheckSourceListAgainstAdd << Workflow::AddSource; @@ -159,6 +160,7 @@ namespace AppInstaller::CLI void SourceRemoveCommand::ExecuteInternal(Context& context) const { context << + Workflow::EnsureRunningAsAdmin << Workflow::GetSourceListWithFilter << Workflow::RemoveSources; } @@ -191,12 +193,14 @@ namespace AppInstaller::CLI if (context.Args.Contains(Args::Type::SourceName)) { context << + Workflow::EnsureRunningAsAdmin << Workflow::GetSourceListWithFilter << Workflow::ResetSourceList; } else { context << + Workflow::EnsureRunningAsAdmin << Workflow::QueryUserForSourceReset << Workflow::ResetAllSources; } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -36,6 +36,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ChannelArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(Command); WINGET_DEFINE_RESOURCE_STRINGID(CommandArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(CommandRequiresAdmin); WINGET_DEFINE_RESOURCE_STRINGID(CountArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(ExactArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(ExtraPositionalError); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -312,6 +312,15 @@ namespace AppInstaller::CLI::Workflow ManifestComparator manifestComparator(context.Args); context.Add<Execution::Data::Installer>(manifestComparator.GetPreferredInstaller(context.Get<Execution::Data::Manifest>())); } + + void EnsureRunningAsAdmin(Execution::Context& context) + { + if (!Runtime::IsRunningAsAdmin()) + { + context.Reporter.Error() << Resource::String::CommandRequiresAdmin; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_COMMAND_REQUIRES_ADMIN); + } + } } AppInstaller::CLI::Execution::Context& operator<<(AppInstaller::CLI::Execution::Context& context, AppInstaller::CLI::Workflow::WorkflowTask::Func f) diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -121,6 +121,12 @@ namespace AppInstaller::CLI::Workflow // Inputs: Manifest // Outputs: Installer void SelectInstaller(Execution::Context& context); + + // Ensures that the process is running as admin. + // Required Args: None + // Inputs: None + // Outputs: None + void EnsureRunningAsAdmin(Execution::Context& context); } // Passes the context to the function if it has not been terminated; returns the context. diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -147,6 +147,9 @@ <data name="CommandArgumentDescription" xml:space="preserve"> <value>Filter results by command</value> </data> + <data name="CommandRequiresAdmin" xml:space="preserve"> + <value>This command requires administrator privileges to execute.</value> + </data> <data name="CountArgumentDescription" xml:space="preserve"> <value>Show no more than specified number of results</value> </data> diff --git a/src/AppInstallerCLITests/PreIndexedPackageSource.cpp b/src/AppInstallerCLITests/PreIndexedPackageSource.cpp @@ -56,7 +56,7 @@ std::string GetContents(const fs::path& file) TEST_CASE("PIPS_Add", "[pips]") { - RemoveSetting(Type::Standard, s_RepositorySettings_UserSources); + RemoveSetting(Streams::UserSources); TempDirectory dir("pipssource"); TestDataFile index(s_MsixFile_1); @@ -85,7 +85,7 @@ TEST_CASE("PIPS_Add", "[pips]") TEST_CASE("PIPS_UpdateSameVersion", "[pips]") { - RemoveSetting(Type::Standard, s_RepositorySettings_UserSources); + RemoveSetting(Streams::UserSources); TempDirectory dir("pipssource"); TestDataFile index(s_MsixFile_1); @@ -110,7 +110,7 @@ TEST_CASE("PIPS_UpdateSameVersion", "[pips]") TEST_CASE("PIPS_UpdateNewVersion", "[pips]") { - RemoveSetting(Type::Standard, s_RepositorySettings_UserSources); + RemoveSetting(Streams::UserSources); TempDirectory dir("pipssource"); TestDataFile indexMsix1(s_MsixFile_1); @@ -152,7 +152,7 @@ TEST_CASE("PIPS_UpdateNewVersion", "[pips]") TEST_CASE("PIPS_Remove", "[pips]") { - RemoveSetting(Type::Standard, s_RepositorySettings_UserSources); + RemoveSetting(Streams::UserSources); TempDirectory dir("pipssource"); TestDataFile index(s_MsixFile_1); diff --git a/src/AppInstallerCLITests/Settings.cpp b/src/AppInstallerCLITests/Settings.cpp @@ -13,20 +13,20 @@ using namespace AppInstaller::Utility; TEST_CASE("ReadEmptySetting", "[settings]") { - std::string name = "nonexistentsetting"; + StreamDefinition name{ Type::Standard, "nonexistentsetting" }; - auto result = GetSettingStream(Type::Standard, name); + auto result = GetSettingStream(name); REQUIRE(!result); } TEST_CASE("SetAndReadSetting", "[settings]") { - std::string name = "testsettingname"; + StreamDefinition name{ Type::Standard, "testsettingname" }; std::string value = "This is the test setting value"; - SetSetting(Type::Standard, name, value); + SetSetting(name, value); - auto result = GetSettingStream(Type::Standard, name); + auto result = GetSettingStream(name); REQUIRE(static_cast<bool>(result)); std::string settingValue = ReadEntireStream(*result); @@ -35,12 +35,12 @@ TEST_CASE("SetAndReadSetting", "[settings]") TEST_CASE("SetAndReadSettingInContainer", "[settings]") { - std::string name = "testcontainer/testsettingname"; + StreamDefinition name{ Type::Standard, "testcontainer/testsettingname" }; std::string value = "This is the test setting value from inside a container"; - SetSetting(Type::Standard, name, value); + SetSetting(name, value); - auto result = GetSettingStream(Type::Standard, name); + auto result = GetSettingStream(name); REQUIRE(static_cast<bool>(result)); std::string settingValue = ReadEntireStream(*result); @@ -49,35 +49,131 @@ TEST_CASE("SetAndReadSettingInContainer", "[settings]") TEST_CASE("RemoveSetting", "[settings]") { - std::string name = "testsettingname"; + StreamDefinition name{ Type::Standard, "testsettingname" }; std::string value = "This is the test setting value to be removed"; - SetSetting(Type::Standard, name, value); + SetSetting(name, value); { - auto result = GetSettingStream(Type::Standard, name); + auto result = GetSettingStream(name); REQUIRE(static_cast<bool>(result)); std::string settingValue = ReadEntireStream(*result); REQUIRE(value == settingValue); } - RemoveSetting(Type::Standard, name); + RemoveSetting( name); - auto result = GetSettingStream(Type::Standard, name); + auto result = GetSettingStream(name); REQUIRE(!static_cast<bool>(result)); } TEST_CASE("SetAndReadUserFileSetting", "[settings]") { - std::string name = "userfilesetting"; + StreamDefinition name{ Type::UserFile, "userfilesetting" }; std::string value = "This is the test setting value for a user file"; - SetSetting(Type::UserFile, name, value); + SetSetting(name, value); - auto result = GetSettingStream(Type::UserFile, name); + auto result = GetSettingStream(name); REQUIRE(static_cast<bool>(result)); std::string settingValue = ReadEntireStream(*result); REQUIRE(value == settingValue); } + +TEST_CASE("ReadEmptySecureSetting", "[settings]") +{ + StreamDefinition name{ Type::Secure, "secure_nonexistentsetting" }; + + auto result = GetSettingStream(name); + REQUIRE(!result); +} + +TEST_CASE("SetAndReadSecureSetting", "[settings]") +{ + StreamDefinition name{ Type::Secure, "secure_testsettingname" }; + std::string value = "This is the test setting value"; + + SetSetting(name, value); + + auto result = GetSettingStream(name); + REQUIRE(static_cast<bool>(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value == settingValue); +} + +TEST_CASE("SetAndReadSecureSettingInContainer", "[settings]") +{ + StreamDefinition name{ Type::Secure, "testcontainer/secure_testsettingname" }; + std::string value = "This is the test setting value from inside a container"; + + SetSetting(name, value); + + auto result = GetSettingStream(name); + REQUIRE(static_cast<bool>(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value == settingValue); +} + +TEST_CASE("RemoveSecureSetting", "[settings]") +{ + StreamDefinition name{ Type::Secure, "secure_testsettingname" }; + std::string value = "This is the test setting value to be removed"; + + SetSetting(name, value); + + { + auto result = GetSettingStream(name); + REQUIRE(static_cast<bool>(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value == settingValue); + } + + RemoveSetting(name); + + auto result = GetSettingStream(name); + REQUIRE(!static_cast<bool>(result)); +} + +TEST_CASE("SetAndReadSecureSetting_SecureDataRemoved", "[settings]") +{ + StreamDefinition name{ Type::Secure, "secure_testsettingname" }; + std::string value = "This is the test setting value"; + + SetSetting(name, value); + + auto result = GetSettingStream(name); + REQUIRE(static_cast<bool>(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value == settingValue); + + std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Path); + + REQUIRE_THROWS_HR(GetSettingStream(name), SPAPI_E_FILE_HASH_NOT_IN_CATALOG); +} + +TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]") +{ + StreamDefinition name{ Type::Secure, "secure_testsettingname" }; + std::string value = "This is the test setting value"; + + SetSetting(name, value); + + auto result = GetSettingStream(name); + REQUIRE(static_cast<bool>(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value == settingValue); + + StreamDefinition insecureName = name; + insecureName.Type = Type::Standard; + + SetSetting(insecureName, "Tampered data"); + + REQUIRE_THROWS_HR(GetSettingStream(name), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR)); +} diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp @@ -19,7 +19,6 @@ using namespace AppInstaller::Utility; // Duplicating here because a change to these values in the product *REALLY* needs to be thought through. using namespace std::string_literals; using namespace std::string_view_literals; -constexpr std::string_view s_RepositorySettings_UserSources = "usersources"sv; constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; @@ -32,13 +31,22 @@ constexpr std::string_view s_EmptySources = R"( Sources: )"sv; +constexpr std::string_view s_DefaultSourceTombstoned = R"( +Sources: + - Name: winget + Type: "" + Arg: "" + Data: "" + IsTombstone: true +)"sv; + constexpr std::string_view s_SingleSource = R"( Sources: - Name: testName Type: testType Arg: testArg Data: testData - LastUpdate: 0 + IsTombstone: false )"sv; constexpr std::string_view s_ThreeSources = R"( @@ -47,18 +55,31 @@ Sources: Type: testType Arg: testArg Data: testData - LastUpdate: 0 - IsDefault: 1 + IsTombstone: false - Name: testName2 Type: testType2 Arg: testArg2 Data: testData2 - LastUpdate: 1 - IsDefault: 0 + IsTombstone: false - Name: testName3 Type: testType3 Arg: testArg3 Data: testData3 + IsTombstone: false + - Name: winget + Type: "" + Arg: "" + Data: "" + IsTombstone: true +)"sv; + +constexpr std::string_view s_ThreeSourcesMetadata = R"( +Sources: + - Name: testName + LastUpdate: 0 + - Name: testName2 + LastUpdate: 1 + - Name: testName3 LastUpdate: 2 )"sv; @@ -67,7 +88,7 @@ Sources: - Name: testName Type: testType Data: testData - LastUpdate: 0 + IsTombstone: false )"sv; // Helper to create a simple source. @@ -100,26 +121,26 @@ struct TestSource : public ISource // Helper that allows some lambdas to be wrapped into a source factory. struct TestSourceFactory : public ISourceFactory { - using IsInitializedFunctor = std::function<bool(const SourceDetails&)>; using CreateFunctor = std::function<std::shared_ptr<ISource>(const SourceDetails&)>; - using UpdateFunctor = std::function<void(SourceDetails&)>; + using AddFunctor = std::function<void(SourceDetails&)>; + using UpdateFunctor = std::function<void(const SourceDetails&)>; using RemoveFunctor = std::function<void(const SourceDetails&)>; TestSourceFactory() : - m_isInit([](const SourceDetails&) { return true; }), m_Create(TestSource::Create), m_Update([](SourceDetails&) {}), m_Remove([](const SourceDetails&) {}) {} + m_Create(TestSource::Create), m_Add([](SourceDetails&) {}), m_Update([](const SourceDetails&) {}), m_Remove([](const SourceDetails&) {}) {} // ISourceFactory - bool IsInitialized(const SourceDetails& details) override + std::shared_ptr<ISource> Create(const SourceDetails& details) override { - return m_isInit(details); + return m_Create(details); } - std::shared_ptr<ISource> Create(const SourceDetails& details) override + void Add(SourceDetails& details, IProgressCallback&) override { - return m_Create(details); + m_Add(details); } - void Update(SourceDetails& details, IProgressCallback&) override + void Update(const SourceDetails& details, IProgressCallback&) override { m_Update(details); } @@ -135,8 +156,8 @@ struct TestSourceFactory : public ISourceFactory return [this]() { return std::make_unique<TestSourceFactory>(*this); }; } - IsInitializedFunctor m_isInit; CreateFunctor m_Create; + AddFunctor m_Add; UpdateFunctor m_Update; RemoveFunctor m_Remove; }; @@ -144,17 +165,25 @@ struct TestSourceFactory : public ISourceFactory TEST_CASE("RepoSources_UserSettingDoesNotExist", "[sources]") { - RemoveSetting(Type::Standard, s_RepositorySettings_UserSources); + RemoveSetting(Streams::UserSources); std::vector<SourceDetails> sources = GetSources(); - // The default source is added when no source exists REQUIRE(sources.size() == 1); - REQUIRE(sources[0].IsDefault); + REQUIRE(sources[0].Origin == SourceOrigin::Default); } TEST_CASE("RepoSources_EmptySourcesList", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); + + std::vector<SourceDetails> sources = GetSources(); + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Origin == SourceOrigin::Default); +} + +TEST_CASE("RepoSources_DefaultSourceTombstoned", "[sources]") +{ + SetSetting(Streams::UserSources, s_DefaultSourceTombstoned); std::vector<SourceDetails> sources = GetSources(); REQUIRE(sources.empty()); @@ -162,21 +191,25 @@ TEST_CASE("RepoSources_EmptySourcesList", "[sources]") TEST_CASE("RepoSources_SingleSource", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_SingleSource); + SetSetting(Streams::UserSources, s_SingleSource); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == "testName"); REQUIRE(sources[0].Type == "testType"); REQUIRE(sources[0].Arg == "testArg"); REQUIRE(sources[0].Data == "testData"); + REQUIRE(sources[0].Origin == SourceOrigin::User); REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + + REQUIRE(sources[1].Origin == SourceOrigin::Default); } TEST_CASE("RepoSources_ThreeSources", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_ThreeSources); + SetSetting(Streams::UserSources, s_ThreeSources); + SetSetting(Streams::SourcesMetadata, s_ThreeSourcesMetadata); std::vector<SourceDetails> sources = GetSources(); REQUIRE(sources.size() == 3); @@ -191,27 +224,27 @@ TEST_CASE("RepoSources_ThreeSources", "[sources]") REQUIRE(sources[i].Arg == "testArg"s + suffix[i]); REQUIRE(sources[i].Data == "testData"s + suffix[i]); REQUIRE(sources[i].LastUpdateTime == ConvertUnixEpochToSystemClock(i)); - REQUIRE(sources[i].IsDefault == (i == 0)); + REQUIRE(sources[i].Origin == SourceOrigin::User); } } TEST_CASE("RepoSources_InvalidYAML", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, "Name: Value : BAD"); + SetSetting(Streams::UserSources, "Name: Value : BAD"); REQUIRE_THROWS_HR(GetSources(), APPINSTALLER_CLI_ERROR_SOURCES_INVALID); } TEST_CASE("RepoSources_MissingField", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_SingleSource_MissingArg); + SetSetting(Streams::UserSources, s_SingleSource_MissingArg); REQUIRE_THROWS_HR(GetSources(), APPINSTALLER_CLI_ERROR_SOURCES_INVALID); } TEST_CASE("RepoSources_AddSource", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string name = "thisIsTheName"; @@ -219,29 +252,32 @@ TEST_CASE("RepoSources_AddSource", "[sources]") std::string arg = "thisIsTheArg"; std::string data = "thisIsTheData"; - bool updateCalledOnFactory = false; + bool addCalledOnFactory = false; TestSourceFactory factory; - factory.m_Update = [&](SourceDetails& sd) { updateCalledOnFactory = true; sd.Data = data; }; + factory.m_Add = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; AddSource(name, type, arg, progress); - REQUIRE(updateCalledOnFactory); + REQUIRE(addCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); REQUIRE(sources[0].Arg == arg); REQUIRE(sources[0].Data == data); - REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].Origin == SourceOrigin::User); + + REQUIRE(sources[1].Origin == SourceOrigin::Default); } TEST_CASE("RepoSources_AddMultipleSources", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); std::string name = "thisIsTheName"; std::string type = "thisIsTheType"; @@ -251,29 +287,32 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") const char* suffix[2] = { "", "2" }; TestSourceFactory factory1; - factory1.m_Update = [&](SourceDetails& sd) { sd.Data = data; }; + factory1.m_Add = [&](SourceDetails& sd) { sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory1); ProgressCallback progress; AddSource(name, type, arg, progress); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); REQUIRE(sources[0].Arg == arg); REQUIRE(sources[0].Data == data); - REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].Origin == SourceOrigin::User); + + REQUIRE(sources[1].Origin == SourceOrigin::Default); TestSourceFactory factory2; - factory2.m_Update = [&](SourceDetails& sd) { sd.Data = data + suffix[1]; }; + factory2.m_Add = [&](SourceDetails& sd) { sd.Data = data + suffix[1]; }; TestHook_SetSourceFactoryOverride(type + suffix[1], factory2); AddSource(name + suffix[1], type + suffix[1], arg + suffix[1], progress); sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == 3); for (size_t i = 0; i < 2; ++i) { @@ -282,15 +321,18 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") REQUIRE(sources[i].Type == type + suffix[i]); REQUIRE(sources[i].Arg == arg + suffix[i]); REQUIRE(sources[i].Data == data + suffix[i]); - REQUIRE(sources[i].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[i].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[i].Origin == SourceOrigin::User); } + + REQUIRE(sources[2].Origin == SourceOrigin::Default); } TEST_CASE("RepoSources_UpdateSource", "[sources]") { using namespace std::chrono_literals; - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string name = "thisIsTheName"; @@ -298,36 +340,39 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") std::string arg = "thisIsTheArg"; std::string data = "thisIsTheData"; - bool updateCalledOnFactory = false; + bool addCalledOnFactory = false; TestSourceFactory factory; - factory.m_Update = [&](SourceDetails& sd) { updateCalledOnFactory = true; sd.Data = data; }; + factory.m_Add = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; AddSource(name, type, arg, progress); - REQUIRE(updateCalledOnFactory); + REQUIRE(addCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); REQUIRE(sources[0].Arg == arg); REQUIRE(sources[0].Data == data); - REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].Origin == SourceOrigin::User); + + REQUIRE(sources[1].Origin == SourceOrigin::Default); // Reset for a call to update - updateCalledOnFactory = false; + bool updateCalledOnFactory = false; auto now = std::chrono::system_clock::now(); - factory.m_Update = [&](SourceDetails& sd) { updateCalledOnFactory = true; sd.LastUpdateTime = now; }; + factory.m_Update = [&](const SourceDetails&) { updateCalledOnFactory = true; }; UpdateSource(name, progress); REQUIRE(updateCalledOnFactory); sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); @@ -340,7 +385,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") { using namespace std::chrono_literals; - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string name = "thisIsTheName"; @@ -349,6 +394,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") std::string data = "thisIsTheData"; TestSourceFactory factory; + factory.m_Add = [&](SourceDetails& sd) { sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; @@ -357,7 +403,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") // Reset for a call to update bool updateShouldThrow = false; bool updateCalledOnFactoryAgain = false; - factory.m_Update = [&](SourceDetails& sd) + factory.m_Update = [&](const SourceDetails&) { if (updateShouldThrow) { @@ -365,7 +411,6 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") THROW_HR(E_ACCESSDENIED); } updateCalledOnFactoryAgain = true; - sd.Data = data; }; UpdateSource(name, progress); @@ -375,7 +420,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") TEST_CASE("RepoSources_RemoveSource", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_EmptySources); + SetSetting(Streams::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string name = "thisIsTheName"; @@ -392,13 +437,37 @@ TEST_CASE("RepoSources_RemoveSource", "[sources]") AddSource(name, type, arg, progress); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); RemoveSource(name, progress); REQUIRE(removeCalledOnFactory); sources = GetSources(); + REQUIRE(sources.size() == 1); +} + +TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") +{ + SetSetting(Streams::UserSources, s_EmptySources); + TestHook_ClearSourceFactoryOverrides(); + + std::vector<SourceDetails> sources = GetSources(); + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Origin == SourceOrigin::Default); + + bool removeCalledOnFactory = false; + TestSourceFactory factory; + factory.m_Remove = [&](const SourceDetails&) { removeCalledOnFactory = true; }; + TestHook_SetSourceFactoryOverride(sources[0].Type, factory); + + ProgressCallback progress; + + RemoveSource(sources[0].Name, progress); + + REQUIRE(removeCalledOnFactory); + + sources = GetSources(); REQUIRE(sources.empty()); } @@ -411,15 +480,14 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") std::string name = "testName"; std::string type = "testType"; std::string arg = "testArg"; - std::string data = "testDataOnUpdate"; + std::string data = "testData"; bool updateCalledOnFactory = false; TestSourceFactory factory; - factory.m_isInit = [](const SourceDetails&) { return false; }; - factory.m_Update = [&](SourceDetails& sd) { updateCalledOnFactory = true; sd.Data = data; }; + factory.m_Update = [&](const SourceDetails&) { updateCalledOnFactory = true; }; TestHook_SetSourceFactoryOverride(type, factory); - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_SingleSource); + SetSetting(Streams::UserSources, s_SingleSource); ProgressCallback progress; auto source = OpenSource(name, progress); @@ -427,18 +495,19 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") REQUIRE(updateCalledOnFactory); std::vector<SourceDetails> sources = GetSources(); - REQUIRE(sources.size() == 1); + REQUIRE(sources.size() == 2); REQUIRE(sources[0].Name == name); REQUIRE(sources[0].Type == type); REQUIRE(sources[0].Arg == arg); REQUIRE(sources[0].Data == data); - REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0)); + REQUIRE(sources[0].LastUpdateTime != ConvertUnixEpochToSystemClock(0)); } TEST_CASE("RepoSources_DropSourceByName", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_ThreeSources); + SetSetting(Streams::UserSources, s_ThreeSources); + SetSetting(Streams::SourcesMetadata, s_ThreeSourcesMetadata); std::vector<SourceDetails> sources = GetSources(); REQUIRE(sources.size() == 3); @@ -446,7 +515,7 @@ TEST_CASE("RepoSources_DropSourceByName", "[sources]") DropSource("testName"); sources = GetSources(); - REQUIRE(sources.size() == 2); + REQUIRE(sources.size() == 3); const char* suffix[2] = { "2", "3" }; @@ -458,13 +527,15 @@ TEST_CASE("RepoSources_DropSourceByName", "[sources]") REQUIRE(sources[i].Arg == "testArg"s + suffix[i]); REQUIRE(sources[i].Data == "testData"s + suffix[i]); REQUIRE(sources[i].LastUpdateTime == ConvertUnixEpochToSystemClock(i + 1)); - REQUIRE(!sources[i].IsDefault); + REQUIRE(sources[i].Origin == SourceOrigin::User); } + + REQUIRE(sources[2].Origin == SourceOrigin::Default); } TEST_CASE("RepoSources_DropAllSources", "[sources]") { - SetSetting(Type::Standard, s_RepositorySettings_UserSources, s_ThreeSources); + SetSetting(Streams::UserSources, s_ThreeSources); std::vector<SourceDetails> sources = GetSources(); REQUIRE(sources.size() == 3); @@ -473,5 +544,5 @@ TEST_CASE("RepoSources_DropAllSources", "[sources]") sources = GetSources(); REQUIRE(sources.size() == 1); - REQUIRE(sources[0].IsDefault); + REQUIRE(sources[0].Origin == SourceOrigin::Default); } diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h @@ -7,6 +7,8 @@ #include <memory> #include <string> +#include <AppInstallerRuntime.h> + #ifdef AICLI_DISABLE_TEST_HOOKS static_assert(false, "Test hooks have been disabled"); #endif @@ -15,7 +17,8 @@ namespace AppInstaller { namespace Runtime { - void TestHook_ForceContainerPrepend(const std::filesystem::path& prepend); + void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path); + void TestHook_ClearPathOverrides(); } namespace Repository diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp @@ -24,11 +24,6 @@ namespace static constexpr std::string_view s_settings = "settings.json"sv; static constexpr std::string_view s_settingsBackup = "settings.json.backup"sv; - std::filesystem::path GetBackupPath() - { - return GetPathTo(PathName::UserFileSettings) / s_settingsBackup; - } - void DeleteUserSettingsFiles() { auto settingsPath = UserSettings::SettingsFilePath(); @@ -37,7 +32,7 @@ namespace std::filesystem::remove(settingsPath); } - auto settingsBackupPath = GetBackupPath(); + auto settingsBackupPath = GetPathTo(Streams::BackupUserSettings); if (std::filesystem::exists(settingsBackupPath)) { std::filesystem::remove(settingsBackupPath); @@ -72,60 +67,60 @@ TEST_CASE("UserSettingsType", "[settings]") } SECTION("No setting.json Bad setting.json.backup") { - SetSetting(Type::UserFile, s_settingsBackup, s_badJson); + SetSetting(Streams::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("No setting.json Good setting.json.backup") { - SetSetting(Type::UserFile, s_settingsBackup, s_goodJson); + SetSetting(Streams::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Backup); } SECTION("Bad setting.json No setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_badJson); + SetSetting(Streams::PrimaryUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("Bad setting.json Bad setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_badJson); - SetSetting(Type::UserFile, s_settingsBackup, s_badJson); + SetSetting(Streams::PrimaryUserSettings, s_badJson); + SetSetting(Streams::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("Bad setting.json Good setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_badJson); - SetSetting(Type::UserFile, s_settingsBackup, s_goodJson); + SetSetting(Streams::PrimaryUserSettings, s_badJson); + SetSetting(Streams::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Backup); } SECTION("Good setting.json No setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_goodJson); + SetSetting(Streams::PrimaryUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); } SECTION("Good setting.json Bad setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_goodJson); - SetSetting(Type::UserFile, s_settingsBackup, s_badJson); + SetSetting(Streams::PrimaryUserSettings, s_goodJson); + SetSetting(Streams::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); } SECTION("Good setting.json Good setting.json.backup") { - SetSetting(Type::UserFile, s_settings, s_goodJson); - SetSetting(Type::UserFile, s_settingsBackup, s_goodJson); + SetSetting(Streams::PrimaryUserSettings, s_goodJson); + SetSetting(Streams::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); @@ -137,7 +132,7 @@ TEST_CASE("UserSettingsCreateFiles", "[settings]") DeleteUserSettingsFiles(); auto settingsPath = UserSettings::SettingsFilePath(); - auto settingsBackupPath = GetBackupPath(); + auto settingsBackupPath = GetPathTo(Streams::BackupUserSettings); SECTION("No settings.json create new") { @@ -153,7 +148,7 @@ TEST_CASE("UserSettingsCreateFiles", "[settings]") } SECTION("Good settings.json create new backup") { - SetSetting(Type::UserFile, s_settings, s_goodJson); + SetSetting(Streams::PrimaryUserSettings, s_goodJson); REQUIRE(std::filesystem::exists(settingsPath)); REQUIRE(!std::filesystem::exists(settingsBackupPath)); @@ -180,7 +175,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Accent") { std::string_view json = R"({ "visual": { "progressBar": "accent" } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::ProgressBarVisualStyle>() == VisualStyle::Accent); @@ -189,7 +184,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Rainbow") { std::string_view json = R"({ "visual": { "progressBar": "rainbow" } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::ProgressBarVisualStyle>() == VisualStyle::Rainbow); @@ -198,7 +193,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("retro") { std::string_view json = R"({ "visual": { "progressBar": "retro" } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::ProgressBarVisualStyle>() == VisualStyle::Retro); @@ -207,7 +202,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Bad value") { std::string_view json = R"({ "visual": { "progressBar": "fake" } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::ProgressBarVisualStyle>() == VisualStyle::Accent); @@ -216,7 +211,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Bad value type") { std::string_view json = R"({ "visual": { "progressBar": 5 } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::ProgressBarVisualStyle>() == VisualStyle::Accent); @@ -242,7 +237,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Valid value") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 0 } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::AutoUpdateTimeInMinutes>() == cero); @@ -251,7 +246,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Valid value 0") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 300 } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::AutoUpdateTimeInMinutes>() == threehundred); @@ -260,7 +255,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Invalid type negative integer") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": -20 } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::AutoUpdateTimeInMinutes>() == cinq); @@ -269,7 +264,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Invalid type string") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": "not a number" } })"; - SetSetting(Type::UserFile, s_settings, json); + SetSetting(Streams::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::AutoUpdateTimeInMinutes>() == cinq); diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp @@ -16,6 +16,7 @@ using namespace winrt; using namespace Windows::Foundation; using namespace std::string_literals; +using namespace AppInstaller; // Logs the the AppInstaller log target to break up individual tests @@ -65,12 +66,12 @@ int main(int argc, char** argv) } else if ("-log"s == argv[i]) { - AppInstaller::Logging::AddFileLogger(); + Logging::AddFileLogger(); } else if ("-logto"s == argv[i]) { ++i; - AppInstaller::Logging::AddFileLogger(argv[i]); + Logging::AddFileLogger(argv[i]); } else if ("-tdd"s == argv[i]) { @@ -106,13 +107,16 @@ int main(int argc, char** argv) // Enable all logging, to force log string building to run. // By not creating a log target, it will all be thrown away. - AppInstaller::Logging::Log().EnableChannel(AppInstaller::Logging::Channel::All); - AppInstaller::Logging::Log().SetLevel(AppInstaller::Logging::Level::Verbose); - AppInstaller::Logging::EnableWilFailureTelemetry(); + Logging::Log().EnableChannel(Logging::Channel::All); + Logging::Log().SetLevel(Logging::Level::Verbose); + Logging::EnableWilFailureTelemetry(); // Force all tests to run against settings inside this container. // This prevents test runs from trashing the users actual settings. - AppInstaller::Runtime::TestHook_ForceContainerPrepend("AutoTestContainer"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::LocalState, Runtime::GetPathTo(Runtime::PathName::LocalState) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::UserFileSettings, Runtime::GetPathTo(Runtime::PathName::UserFileSettings) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::StandardSettings, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettings, Runtime::GetPathTo(Runtime::PathName::Temp) / "WinGet_SecureSettings_Tests"); int result = Catch::Session().run(static_cast<int>(args.size()), args.data()); diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj @@ -115,16 +115,16 @@ <PrecompiledHeaderOutputFile>$(IntDir)pch.pch</PrecompiledHeaderOutputFile> <PreprocessorDefinitions>_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions> <WarningLevel>Level4</WarningLevel> - <AdditionalOptions>%(AdditionalOptions) /permissive-</AdditionalOptions> + <AdditionalOptions>%(AdditionalOptions) /permissive- /D _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING</AdditionalOptions> </ClCompile> </ItemDefinitionGroup> <ItemDefinitionGroup Condition="'$(Configuration)'=='Debug'"> <ClCompile> <Optimization>Disabled</Optimization> <PreprocessorDefinitions>_DEBUG;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">true</TreatWarningAsError> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">true</TreatWarningAsError> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</TreatWarningAsError> @@ -139,7 +139,7 @@ <ItemDefinitionGroup Condition="'$(Platform)'=='Win32'"> <ClCompile> <PreprocessorDefinitions>WIN32;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</TreatWarningAsError> </ClCompile> <Link> @@ -152,10 +152,10 @@ <FunctionLevelLinking>true</FunctionLevelLinking> <IntrinsicFunctions>true</IntrinsicFunctions> <PreprocessorDefinitions>NDEBUG;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> - <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|x64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> + <AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|x64'">$(ProjectDir);$(ProjectDir)Public;$(ProjectDir)Telemetry;$(ProjectDir)..\binver;$(ProjectDir)..\YamlCppLib\yaml-cpp\include;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">true</TreatWarningAsError> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">true</TreatWarningAsError> <TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</TreatWarningAsError> diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp @@ -58,6 +58,8 @@ namespace AppInstaller return "Multiple applications found matching the criteria"; case APPINSTALLER_CLI_ERROR_NO_MANIFEST_FOUND: return "No manifest found matching the criteria"; + case APPINSTALLER_CLI_ERROR_COMMAND_REQUIRES_ADMIN: + return "Command requires administrator privileges to run"; default: return "Uknown Error Code"; } diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp @@ -3,8 +3,9 @@ #include "pch.h" #include "Public/AppInstallerMsixInfo.h" #include "HttpStream/HttpRandomAccessStream.h" -#include "Public/AppInstallerStrings.h" #include "Public/AppInstallerDownloader.h" +#include "Public/AppInstallerLogging.h" +#include "Public/AppInstallerStrings.h" using namespace winrt::Windows::Storage::Streams; @@ -199,6 +200,46 @@ namespace AppInstaller::Msix THROW_IF_FAILED(appxFactory->CreateManifestReader(inputStream, reader)); } + std::optional<std::string> GetPackageFullNameFromFamilyName(std::string_view familyName) + { + std::wstring pfn = Utility::ConvertToUTF16(familyName); + UINT32 fullNameCount = 0; + UINT32 bufferLength = 0; + UINT32 properties = 0; + + LONG findResult = FindPackagesByPackageFamily(pfn.c_str(), PACKAGE_FILTER_HEAD, &fullNameCount, nullptr, &bufferLength, nullptr, &properties); + if (findResult == ERROR_SUCCESS || fullNameCount == 0) + { + // No package found + return {}; + } + else if (findResult != ERROR_INSUFFICIENT_BUFFER) + { + THROW_WIN32(findResult); + } + else if (fullNameCount != 1) + { + // Don't directly error, let caller deal with it + AICLI_LOG(Core, Error, << "Multiple packages found for family name: " << fullNameCount); + return {}; + } + + // fullNameCount == 1 at this point + PWSTR fullNamePtr; + std::wstring buffer(bufferLength + 1, '\0'); + + THROW_IF_WIN32_ERROR(FindPackagesByPackageFamily(pfn.c_str(), PACKAGE_FILTER_HEAD, &fullNameCount, &fullNamePtr, &bufferLength, &buffer[0], &properties)); + if (fullNameCount != 1 || bufferLength == 0) + { + // Something changed in between, abandon + AICLI_LOG(Core, Error, << "Packages found for family name: " << fullNameCount); + return {}; + } + + buffer.resize(bufferLength - 1); + return Utility::ConvertToUTF8(buffer); + } + std::string GetPackageFamilyNameFromFullName(std::string_view fullName) { std::wstring result; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -34,6 +34,7 @@ #define APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND ((HRESULT)0x8A150016) #define APPINSTALLER_CLI_ERROR_NO_MANIFEST_FOUND ((HRESULT)0x8A150017) #define APPINSTALLER_CLI_ERROR_EXTENSION_PUBLIC_FAILED ((HRESULT)0x8A150018) +#define APPINSTALLER_CLI_ERROR_COMMAND_REQUIRES_ADMIN ((HRESULT)0x8A150019) namespace AppInstaller { diff --git a/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h b/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h @@ -32,6 +32,10 @@ namespace AppInstaller::Msix IStream* inputStream, IAppxManifestReader** reader); + // Gets the package full name from the family name. + // This will be the one registered for the current user, if any. + std::optional<std::string> GetPackageFullNameFromFamilyName(std::string_view familyName); + // Gets the package family name from the given full name. std::string GetPackageFamilyNameFromFullName(std::string_view fullName); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -37,6 +37,8 @@ namespace AppInstaller::Runtime StandardSettings, // The location that user file type settings are stored. UserFileSettings, + // The location where secure settings data is stored. + SecureSettings, }; // Gets the path to the requested location. @@ -45,4 +47,7 @@ namespace AppInstaller::Runtime // Determines whether the current OS version is >= the given one. // We treat the given Version struct as a standard 4 part Windows OS version. bool IsCurrentOSVersionGreaterThanOrEqual(const Utility::Version& version); + + // Determines whether the process is running with administrator privileges. + bool IsRunningAsAdmin(); } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h @@ -42,7 +42,7 @@ namespace AppInstaller::Utility { } // Computes the hash of the given buffer immediately. - static std::vector<uint8_t> ComputeHash(uint8_t* buffer, std::uint32_t cbBuffer); + static std::vector<uint8_t> ComputeHash(const uint8_t* buffer, std::uint32_t cbBuffer); // Computes the hash from a given stream. static std::vector<uint8_t> ComputeHash(std::istream& in); diff --git a/src/AppInstallerCommonCore/Public/winget/Settings.h b/src/AppInstallerCommonCore/Public/winget/Settings.h @@ -8,6 +8,8 @@ namespace AppInstaller::Settings { + using namespace std::string_view_literals; + // Allows settings to be classified and treated differently base on any number of factors. // Names should still be unique, as there is no guarantee made about types mapping to unique roots. enum class Type @@ -16,15 +18,47 @@ namespace AppInstaller::Settings Standard, // A UserFile setting stream should be located in a file that is easily editable by the user. UserFile, + // A settings stream that should not be modified except by admin privileges. + Secure, + }; + + // Converts the Type enum to a string. + std::string_view ToString(Type type); + + // A stream definition, combining both type and path. + // The well known values in Streams should be used by product code, while tests may directly create them. + struct StreamDefinition + { + constexpr StreamDefinition(Type type, std::string_view path) : Type(type), Path(path) {} + + Type Type; + std::string_view Path; + }; + + // The set of well known settings streams. + // Changing these values can result in data loss. + struct Streams + { + // The set of sources as defined by the user. + constexpr static StreamDefinition UserSources{ Type::Secure, "user_sources"sv }; + // The metadata about all sources. + constexpr static StreamDefinition SourcesMetadata{ Type::Standard, "sources_metadata"sv }; + // The primary user settings file. + constexpr static StreamDefinition PrimaryUserSettings{ Type::UserFile, "settings.json"sv }; + // The backup user settings file. + constexpr static StreamDefinition BackupUserSettings{ Type::UserFile, "settings.json.backup"sv }; }; // Gets a stream containing the named setting's value, if present. // If the setting does not exist, returns an empty value. - std::unique_ptr<std::istream> GetSettingStream(Type type, const std::filesystem::path& name); + std::unique_ptr<std::istream> GetSettingStream(const StreamDefinition& def); // Sets the named setting to the given value. - void SetSetting(Type type, const std::filesystem::path& name, std::string_view value); + void SetSetting(const StreamDefinition& def, std::string_view value); // Deletes the given setting. - void RemoveSetting(Type type, const std::filesystem::path& name); + void RemoveSetting(const StreamDefinition& def); + + // Gets the path to the given stream definition. + std::filesystem::path GetPathTo(const StreamDefinition& def); } diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp @@ -5,7 +5,6 @@ #include "Public/AppInstallerRuntime.h" #include "Public/AppInstallerStrings.h" -#define AICLI_DEFAULT_TEMP_DIRECTORY "WinGet" #define WINGET_DEFAULT_LOG_DIRECTORY "DiagOutputDir" namespace AppInstaller::Runtime @@ -15,8 +14,12 @@ namespace AppInstaller::Runtime namespace { using namespace std::string_view_literals; - constexpr std::string_view s_AppDataDir_Settings = "Settings"; - constexpr std::string_view s_AppDataDir_State = "State"; + constexpr std::string_view s_DefaultTempDirectory = "WinGet"sv; + constexpr std::string_view s_AppDataDir_Settings = "Settings"sv; + constexpr std::string_view s_AppDataDir_State = "State"sv; + constexpr std::string_view s_SecureSettings_Relative = "Microsoft/WinGet/settings"sv; + constexpr std::string_view s_SecureSettings_Relative_Packaged = "pkg"sv; + constexpr std::string_view s_SecureSettings_Relative_Unpackaged = "win"sv; // Gets a boolean indicating whether the current process has identity. bool DoesCurrentProcessHaveIdentity() @@ -70,34 +73,25 @@ namespace AppInstaller::Runtime } #ifndef AICLI_DISABLE_TEST_HOOKS - static std::filesystem::path s_Settings_TestHook_ForcedContainerPrepend; + static std::map<PathName, std::filesystem::path> s_Path_TestHook_Overrides; #endif + std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id) + { + wil::unique_cotaskmem_string knownFolder = nullptr; + THROW_IF_FAILED(SHGetKnownFolderPath(id, KF_FLAG_NO_ALIAS | KF_FLAG_DONT_VERIFY | KF_FLAG_NO_PACKAGE_REDIRECTION, NULL, &knownFolder)); + return knownFolder.get(); + } + // Gets the path to the appdata root. // *Only used by non packaged version!* std::filesystem::path GetPathToAppDataRoot() { THROW_HR_IF(E_NOT_VALID_STATE, IsRunningInPackagedContext()); - DWORD charCount = ExpandEnvironmentStringsW(L"%LOCALAPPDATA%", nullptr, 0); - THROW_LAST_ERROR_IF(charCount == 0); - - std::wstring localAppDataPath(charCount + 1, L'\0'); - charCount = ExpandEnvironmentStringsW(L"%LOCALAPPDATA%", &localAppDataPath[0], charCount + 1); - THROW_LAST_ERROR_IF(charCount == 0); - - localAppDataPath.resize(charCount - 1); - - std::filesystem::path result = localAppDataPath; + std::filesystem::path result = GetKnownFolderPath(FOLDERID_LocalAppData); result /= "Microsoft/WinGet"; -#ifndef AICLI_DISABLE_TEST_HOOKS - if (!s_Settings_TestHook_ForcedContainerPrepend.empty()) - { - result /= s_Settings_TestHook_ForcedContainerPrepend; - } -#endif - return result; } @@ -204,6 +198,7 @@ namespace AppInstaller::Runtime std::filesystem::path GetPathTo(PathName path) { std::filesystem::path result; + bool create = true; if (IsRunningInPackagedContext()) { @@ -213,18 +208,11 @@ namespace AppInstaller::Runtime { case PathName::Temp: result.assign(appStorage.TemporaryFolder().Path().c_str()); - result /= AICLI_DEFAULT_TEMP_DIRECTORY; + result /= s_DefaultTempDirectory; break; case PathName::LocalState: case PathName::UserFileSettings: result.assign(appStorage.LocalFolder().Path().c_str()); - -#ifndef AICLI_DISABLE_TEST_HOOKS - if (!s_Settings_TestHook_ForcedContainerPrepend.empty()) - { - result /= s_Settings_TestHook_ForcedContainerPrepend; - } -#endif break; case PathName::DefaultLogLocation: // To enable UIF collection through Feedback hub, we must put our logs here. @@ -232,9 +220,14 @@ namespace AppInstaller::Runtime result /= WINGET_DEFAULT_LOG_DIRECTORY; break; case PathName::StandardSettings: -#ifndef AICLI_DISABLE_TEST_HOOKS - result = s_Settings_TestHook_ForcedContainerPrepend; -#endif + create = false; + break; + case PathName::SecureSettings: + result = GetKnownFolderPath(FOLDERID_ProgramData); + result /= s_SecureSettings_Relative; + result /= s_SecureSettings_Relative_Packaged; + result /= GetPackageName(); + create = false; break; default: THROW_HR(E_UNEXPECTED); @@ -250,8 +243,10 @@ namespace AppInstaller::Runtime wchar_t tempPath[MAX_PATH + 1]; DWORD tempChars = GetTempPathW(ARRAYSIZE(tempPath), tempPath); result.assign(std::wstring_view{ tempPath, static_cast<size_t>(tempChars) }); + + result /= s_DefaultTempDirectory; } - break; + break; case PathName::LocalState: result = GetPathToAppDataDir(s_AppDataDir_State); break; @@ -259,12 +254,27 @@ namespace AppInstaller::Runtime case PathName::UserFileSettings: result = GetPathToAppDataDir(s_AppDataDir_Settings); break; + case PathName::SecureSettings: + result = GetKnownFolderPath(FOLDERID_ProgramData); + result /= s_SecureSettings_Relative; + result /= s_SecureSettings_Relative_Unpackaged; + create = false; + break; default: THROW_HR(E_UNEXPECTED); } } - if (result.is_absolute()) +#ifndef AICLI_DISABLE_TEST_HOOKS + // Override the value after letting the normal code path run + auto itr = s_Path_TestHook_Overrides.find(path); + if (itr != s_Path_TestHook_Overrides.end()) + { + result = itr->second; + } +#endif + + if (create && result.is_absolute()) { if (std::filesystem::exists(result)) { @@ -317,10 +327,20 @@ namespace AppInstaller::Runtime return !!result; } + bool IsRunningAsAdmin() + { + return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS); + } + #ifndef AICLI_DISABLE_TEST_HOOKS - void TestHook_ForceContainerPrepend(const std::filesystem::path& prepend) + void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path) + { + s_Path_TestHook_Overrides[target] = path; + } + + void TestHook_ClearPathOverrides() { - s_Settings_TestHook_ForcedContainerPrepend = prepend; + s_Path_TestHook_Overrides.clear(); } #endif } diff --git a/src/AppInstallerCommonCore/SHA256.cpp b/src/AppInstallerCommonCore/SHA256.cpp @@ -128,7 +128,7 @@ namespace AppInstaller::Utility { return resultBuffer; } - std::vector<uint8_t> SHA256::ComputeHash(std::uint8_t* buffer, std::uint32_t cbBuffer) + std::vector<uint8_t> SHA256::ComputeHash(const std::uint8_t* buffer, std::uint32_t cbBuffer) { SHA256 hasher; hasher.Add(buffer, cbBuffer); diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp @@ -2,11 +2,14 @@ // Licensed under the MIT License. #include "pch.h" #include "Public/winget/Settings.h" +#include "Public/AppInstallerLogging.h" #include "Public/AppInstallerRuntime.h" #include "Public/AppInstallerStrings.h" +#include "Public/AppInstallerSHA256.h" namespace AppInstaller::Settings { + using namespace std::string_view_literals; using namespace Runtime; using namespace Utility; @@ -19,6 +22,11 @@ namespace AppInstaller::Settings THROW_HR_IF(E_INVALIDARG, !name.has_filename()); } + void LogSettingAction(std::string_view action, const StreamDefinition& def) + { + AICLI_LOG(Core, Info, << "Setting action: " << action << ", Type: " << ToString(def.Type) << ", Name: " << def.Path); + } + // A settings container. struct ISettingsContainer { @@ -33,6 +41,9 @@ namespace AppInstaller::Settings // Deletes the given setting. virtual void Remove(const std::filesystem::path& name) = 0; + + // Gets the path to the named setting, if reasonable. + virtual std::filesystem::path PathTo(const std::filesystem::path& name) = 0; }; // A settings container backed by the ApplicationDataContainer functionality. @@ -84,6 +95,11 @@ namespace AppInstaller::Settings parent.Values().Remove(winrt::to_hstring(name.filename().c_str())); } + std::filesystem::path PathTo(const std::filesystem::path&) override + { + THROW_HR(E_UNEXPECTED); + } + private: Container m_root; }; @@ -91,68 +107,203 @@ namespace AppInstaller::Settings // A settings container backed by the filesystem. struct FileSettingsContainer : public ISettingsContainer { - using Container = winrt::Windows::Storage::ApplicationDataContainer; - FileSettingsContainer(std::filesystem::path root) : m_root(std::move(root)) {} - std::filesystem::path GetRelativePath(const std::filesystem::path& name) + std::unique_ptr<std::istream> Get(const std::filesystem::path& name) override + { + std::filesystem::path settingFileName = GetPath(name); + + if (std::filesystem::exists(settingFileName)) + { + auto result = std::make_unique<std::ifstream>(settingFileName); + THROW_LAST_ERROR_IF(result->fail()); + return result; + } + else + { + return {}; + } + } + + void Set(const std::filesystem::path& name, std::string_view value) override + { + std::filesystem::path settingFileName = GetPath(name, true); + + std::ofstream stream(settingFileName, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc); + THROW_LAST_ERROR_IF(stream.fail()); + stream << value << std::flush; + THROW_LAST_ERROR_IF(stream.fail()); + } + + void Remove(const std::filesystem::path& name) override + { + std::filesystem::path settingFileName = GetPath(name); + + std::filesystem::remove(settingFileName); + } + + std::filesystem::path PathTo(const std::filesystem::path& name) override + { + return GetPath(name); + } + + private: + std::filesystem::path GetPath(const std::filesystem::path& name, bool createParent = false) { std::filesystem::path result = m_root; + if (name.has_parent_path()) { result /= name.parent_path(); + } + + if (createParent) + { std::filesystem::create_directories(result); } + + result /= name.filename(); return result; } - std::unique_ptr<std::istream> Get(const std::filesystem::path& name) override + std::filesystem::path m_root; + }; + + // A settings container wrapper that enforces security. + struct SecureSettingsContainer : public ISettingsContainer + { + constexpr static std::string_view NodeName_Sha256 = "SHA256"sv; + + SecureSettingsContainer(std::unique_ptr<ISettingsContainer>&& container) : m_container(std::move(container)), m_secure(GetPathTo(PathName::SecureSettings)) {} + + struct VerificationData { - std::filesystem::path settingFileName = GetRelativePath(name); - settingFileName /= name.filename(); + bool Found = false; + SHA256::HashBuffer Hash; + }; - if (std::filesystem::exists(settingFileName)) + VerificationData GetVerificationData(const std::filesystem::path& name) + { + std::unique_ptr<std::istream> stream = m_secure.Get(name); + + if (!stream) { - return std::make_unique<std::ifstream>(settingFileName); + return {}; } - else + + std::string streamContents = Utility::ReadEntireStream(*stream); + + YAML::Node document; + try + { + document = YAML::Load(streamContents); + } + catch (const std::runtime_error& e) + { + AICLI_LOG(YAML, Error, << "Secure setting metadata for '" << name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); + return {}; + } + + std::string hashString; + + try + { + hashString = document[std::string{ NodeName_Sha256 }].as<std::string>(); + } + catch (const std::runtime_error& e) { + AICLI_LOG(YAML, Error, << "Secure setting metadata for '" << name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); return {}; } + + VerificationData result; + result.Found = true; + result.Hash = SHA256::ConvertToBytes(hashString); + + return result; + } + + void SetVerificationData(const std::filesystem::path& name, VerificationData data) + { + YAML::Emitter out; + out << YAML::BeginMap; + out << YAML::Key << std::string{ NodeName_Sha256 } << YAML::Value << SHA256::ConvertToString(data.Hash); + out << YAML::EndMap; + + m_secure.Set(name, out.c_str()); + } + + std::unique_ptr<std::istream> Get(const std::filesystem::path& name) override + { + std::unique_ptr<std::istream> stream = m_container->Get(name); + + if (!stream) + { + // If no stream exists, then no verification needs to be done. + return stream; + } + + VerificationData verData = GetVerificationData(name); + + // This case should be very rare, so a very identifiable error is helpful. + // Plus the text for this one is fairly on point for what has happened. + THROW_HR_IF(SPAPI_E_FILE_HASH_NOT_IN_CATALOG, !verData.Found); + + std::string streamContents = Utility::ReadEntireStream(*stream); + THROW_HR_IF(E_UNEXPECTED, streamContents.size() > std::numeric_limits<uint32_t>::max()); + + auto streamHash = SHA256::ComputeHash(reinterpret_cast<const uint8_t*>(streamContents.c_str()), static_cast<uint32_t>(streamContents.size())); + + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR), !std::equal(streamHash.begin(), streamHash.end(), verData.Hash.begin())); + + // Return a stream over the contents that we read in and verified, to prevent a race attack. + return std::make_unique<std::istringstream>(streamContents); } void Set(const std::filesystem::path& name, std::string_view value) override { - std::filesystem::path settingFileName = GetRelativePath(name); - settingFileName /= name.filename(); + THROW_HR_IF(E_UNEXPECTED, value.size() > std::numeric_limits<uint32_t>::max()); - std::ofstream stream(settingFileName, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc); - stream << value << std::flush; + VerificationData verData; + verData.Hash = SHA256::ComputeHash(reinterpret_cast<const uint8_t*>(value.data()), static_cast<uint32_t>(value.size())); + + SetVerificationData(name, verData); + + m_container->Set(name, value); } void Remove(const std::filesystem::path& name) override { - std::filesystem::path settingFileName = GetRelativePath(name); - settingFileName /= name.filename(); + m_secure.Remove(name); + m_container->Remove(name); + } - std::filesystem::remove(settingFileName); + std::filesystem::path PathTo(const std::filesystem::path&) override + { + THROW_HR(E_UNEXPECTED); } private: - std::filesystem::path m_root; + std::unique_ptr<ISettingsContainer> m_container; + FileSettingsContainer m_secure; }; std::unique_ptr<ISettingsContainer> GetSettingsContainer(Type type) { + if (type == Type::Secure) + { + return std::make_unique<SecureSettingsContainer>(GetSettingsContainer(Type::Standard)); + } + if (IsRunningInPackagedContext()) { switch (type) { - case AppInstaller::Settings::Type::Standard: + case Type::Standard: return std::make_unique<ApplicationDataSettingsContainer>( ApplicationDataSettingsContainer::GetRelativeContainer( winrt::Windows::Storage::ApplicationData::Current().LocalSettings(), GetPathTo(PathName::StandardSettings))); - case AppInstaller::Settings::Type::UserFile: + case Type::UserFile: return std::make_unique<FileSettingsContainer>(GetPathTo(PathName::UserFileSettings)); default: THROW_HR(E_UNEXPECTED); @@ -162,9 +313,9 @@ namespace AppInstaller::Settings { switch (type) { - case AppInstaller::Settings::Type::Standard: + case Type::Standard: return std::make_unique<FileSettingsContainer>(GetPathTo(PathName::StandardSettings)); - case AppInstaller::Settings::Type::UserFile: + case Type::UserFile: return std::make_unique<FileSettingsContainer>(GetPathTo(PathName::UserFileSettings)); default: THROW_HR(E_UNEXPECTED); @@ -173,21 +324,44 @@ namespace AppInstaller::Settings } } - std::unique_ptr<std::istream> GetSettingStream(Type type, const std::filesystem::path& name) + std::string_view ToString(Type type) + { + switch (type) + { + case Type::Standard: + return "Standard"sv; + case Type::UserFile: + return "UserFile"sv; + case Type::Secure: + return "Secure"sv; + default: + THROW_HR(E_UNEXPECTED); + } + } + + std::unique_ptr<std::istream> GetSettingStream(const StreamDefinition& def) + { + LogSettingAction("Get", def); + ValidateSettingNamePath(def.Path); + return GetSettingsContainer(def.Type)->Get(def.Path); + } + + void SetSetting(const StreamDefinition& def, std::string_view value) { - ValidateSettingNamePath(name); - return GetSettingsContainer(type)->Get(name); + LogSettingAction("Set", def); + ValidateSettingNamePath(def.Path); + GetSettingsContainer(def.Type)->Set(def.Path, value); } - void SetSetting(Type type, const std::filesystem::path& name, std::string_view value) + void RemoveSetting(const StreamDefinition& def) { - ValidateSettingNamePath(name); - GetSettingsContainer(type)->Set(name, value); + LogSettingAction("Remove", def); + ValidateSettingNamePath(def.Path); + GetSettingsContainer(def.Type)->Remove(def.Path); } - void RemoveSetting(Type type, const std::filesystem::path& name) + std::filesystem::path GetPathTo(const StreamDefinition& def) { - ValidateSettingNamePath(name); - GetSettingsContainer(type)->Remove(name); + return GetSettingsContainer(def.Type)->PathTo(def.Path); } } diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp @@ -14,9 +14,6 @@ namespace AppInstaller::Settings using namespace Runtime; using namespace Utility; - static constexpr std::string_view s_SettingFileName = "settings.json"sv; - static constexpr std::string_view s_SettingBackupFileName = "settings.json.backup"sv; - static constexpr std::string_view s_SettingEmpty = R"({ // For documentation on these settings, see: https://aka.ms/winget-settings @@ -61,15 +58,9 @@ namespace AppInstaller::Settings return GetSettingsMessage(message, path) + SettingsMessage::Value + convertedValue; } - - std::filesystem::path SettingsBackupFilePath() - { - return GetPathTo(PathName::UserFileSettings) / s_SettingBackupFileName; - } - - std::optional<Json::Value> ParseFile(const std::filesystem::path& fileName, std::vector<std::string>& warnings) + std::optional<Json::Value> ParseFile(const StreamDefinition& setting, std::vector<std::string>& warnings) { - auto stream = GetSettingStream(Type::UserFile, fileName); + auto stream = GetSettingStream(setting); if (stream) { Json::Value root; @@ -84,8 +75,8 @@ namespace AppInstaller::Settings return root; } - AICLI_LOG(Core, Error, << "Error parsing " << fileName.u8string() << ": " << error); - warnings.emplace_back(fileName.u8string()); + AICLI_LOG(Core, Error, << "Error parsing " << setting.Path << ": " << error); + warnings.emplace_back(setting.Path); warnings.emplace_back(error); } @@ -192,10 +183,10 @@ namespace AppInstaller::Settings // 2 - Use settings.backup.json if settings.json fails to parse. // 3 - Use default (empty) if both settings files fail to load. - auto settingsJson = ParseFile(s_SettingFileName, m_warnings); + auto settingsJson = ParseFile(Streams::PrimaryUserSettings, m_warnings); if (settingsJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << s_SettingFileName); + AICLI_LOG(Core, Info, << "Settings loaded from " << Streams::PrimaryUserSettings.Path); m_type = UserSettingsType::Standard; settingsRoot = settingsJson.value(); } @@ -203,10 +194,10 @@ namespace AppInstaller::Settings // Settings didn't parse or doesn't exist, try with backup. if (settingsRoot.isNull()) { - auto settingsBackupJson = ParseFile(s_SettingBackupFileName, m_warnings); + auto settingsBackupJson = ParseFile(Streams::BackupUserSettings, m_warnings); if (settingsBackupJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << s_SettingFileName); + AICLI_LOG(Core, Info, << "Settings loaded from " << Streams::BackupUserSettings.Path); m_warnings.emplace_back(SettingsMessage::LoadedBackupSettings); m_type = UserSettingsType::Backup; settingsRoot = settingsBackupJson.value(); @@ -232,7 +223,7 @@ namespace AppInstaller::Settings // Create settings file if it doesn't exist. if (!std::filesystem::exists(UserSettings::SettingsFilePath())) { - SetSetting(Type::UserFile, s_SettingFileName, s_SettingEmpty); + SetSetting(Streams::PrimaryUserSettings, s_SettingEmpty); AICLI_LOG(Core, Info, << "Created new settings file"); } } @@ -240,7 +231,7 @@ namespace AppInstaller::Settings { // Settings file was loaded correctly, create backup. auto from = SettingsFilePath(); - auto to = SettingsBackupFilePath(); + auto to = GetPathTo(Streams::BackupUserSettings); std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); AICLI_LOG(Core, Info, << "Copied settings to backup file"); } @@ -248,6 +239,6 @@ namespace AppInstaller::Settings std::filesystem::path UserSettings::SettingsFilePath() { - return GetPathTo(PathName::UserFileSettings) / s_SettingFileName; + return GetPathTo(Streams::PrimaryUserSettings); } } diff --git a/src/AppInstallerCommonCore/pch.h b/src/AppInstallerCommonCore/pch.h @@ -6,15 +6,19 @@ #include <Windows.h> #include <appmodel.h> #include <WinInet.h> +#include <Shlobj.h> #include <Shlwapi.h> #include "TraceLogging.h" +#include <yaml-cpp/yaml.h> + // wil/cppwinrt.h should always be included before any C++/WinRT or WIL header file when both are in use #include <wil/cppwinrt.h> #include <wil/result_macros.h> #include <wil/safecast.h> #include <wil/resource.h> +#include <wil/token_helpers.h> #include <winrt/Windows.ApplicationModel.h> #include <winrt/Windows.ApplicationModel.AppExtensions.h> diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -37,13 +37,6 @@ namespace AppInstaller::Repository::Microsoft std::string GetPackageFamilyNameFromDetails(const SourceDetails& details) { THROW_HR_IF(E_UNEXPECTED, details.Data.empty()); - return Msix::GetPackageFamilyNameFromFullName(details.Data); - } - - // Gets the package full name from the details. - std::string GetPackageFullNameFromDetails(const SourceDetails& details) - { - THROW_HR_IF(E_UNEXPECTED, details.Data.empty()); return details.Data; } @@ -57,15 +50,9 @@ namespace AppInstaller::Repository::Microsoft // The base class for a package that comes from a preindexed packaged source. struct PreIndexedFactoryBase : public ISourceFactory { - bool IsInitialized(const SourceDetails& details) override final - { - return !details.Data.empty(); - } - std::shared_ptr<ISource> Create(const SourceDetails& details) override final { THROW_HR_IF(E_INVALIDARG, details.Type != PreIndexedPackageSourceFactory::Type()); - THROW_HR_IF(E_UNEXPECTED, !IsInitialized(details)); auto lock = Synchronization::CrossProcessReaderWriteLock::LockForRead(CreateNameForCPRWL(details)); return CreateInternal(details, std::move(lock)); @@ -73,7 +60,7 @@ namespace AppInstaller::Repository::Microsoft virtual std::shared_ptr<ISource> CreateInternal(const SourceDetails& details, Synchronization::CrossProcessReaderWriteLock&& lock) = 0; - void Update(SourceDetails& details, IProgressCallback& progress) override final + void Add(SourceDetails& details, IProgressCallback& progress) override final { if (details.Type.empty()) { @@ -88,26 +75,33 @@ namespace AppInstaller::Repository::Microsoft std::string packageLocation = GetPackageLocation(details); - if (!IsInitialized(details)) - { - AICLI_LOG(Repo, Info, << "Initializing source from: " << details.Name << " => " << packageLocation); + AICLI_LOG(Repo, Info, << "Initializing source from: " << details.Name << " => " << packageLocation); - // If not initialized, we need to open the package and get the full name. - Msix::MsixInfo packageInfo(packageLocation); - THROW_HR_IF(APPINSTALLER_CLI_ERROR_PACKAGE_IS_BUNDLE, packageInfo.GetIsBundle()); - details.Data = packageInfo.GetPackageFullName(); + Msix::MsixInfo packageInfo(packageLocation); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_PACKAGE_IS_BUNDLE, packageInfo.GetIsBundle()); - AICLI_LOG(Repo, Info, << "Found package full name: " << details.Name << " => " << details.Data); - } + auto fullName = packageInfo.GetPackageFullName(); + AICLI_LOG(Repo, Info, << "Found package full name: " << details.Name << " => " << fullName); + + details.Data = Msix::GetPackageFamilyNameFromFullName(fullName); auto lock = Synchronization::CrossProcessReaderWriteLock::LockForWrite(CreateNameForCPRWL(details)); UpdateInternal(packageLocation, details, progress); + } - details.LastUpdateTime = std::chrono::system_clock::now(); + void Update(const SourceDetails& details, IProgressCallback& progress) override final + { + THROW_HR_IF(E_INVALIDARG, details.Type != PreIndexedPackageSourceFactory::Type()); + + std::string packageLocation = GetPackageLocation(details); + + auto lock = Synchronization::CrossProcessReaderWriteLock::LockForWrite(CreateNameForCPRWL(details)); + + UpdateInternal(packageLocation, details, progress); } - virtual void UpdateInternal(std::string packageLocation, SourceDetails& details, IProgressCallback& progress) = 0; + virtual void UpdateInternal(std::string packageLocation, const SourceDetails& details, IProgressCallback& progress) = 0; void Remove(const SourceDetails& details, IProgressCallback& progress) override final { @@ -151,7 +145,7 @@ namespace AppInstaller::Repository::Microsoft return std::make_shared<SQLiteIndexSource>(details, std::move(index), std::move(lock)); } - void UpdateInternal(std::string packageLocation, SourceDetails& details, IProgressCallback& progress) override + void UpdateInternal(std::string packageLocation, const SourceDetails& details, IProgressCallback& progress) override { // Check if the package is newer before calling into deployment. // This can save us a lot of time over letting deployment detect same version. @@ -172,8 +166,6 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Info, << "Remote source data was not newer than existing, no update needed"); return; } - - details.Data = packageInfo.GetPackageFullName(); } if (progress.IsCancelled()) @@ -191,7 +183,7 @@ namespace AppInstaller::Repository::Microsoft if (download) { tempFile = Runtime::GetPathTo(Runtime::PathName::Temp); - tempFile /= GetPackageFullNameFromDetails(details) + ".msix"; + tempFile /= GetPackageFamilyNameFromDetails(details) + ".msix"; Utility::Download(packageLocation, tempFile, progress); @@ -216,8 +208,17 @@ namespace AppInstaller::Repository::Microsoft void RemoveInternal(const SourceDetails& details, IProgressCallback& callback) override { - AICLI_LOG(Repo, Info, << "Removing package: " << GetPackageFullNameFromDetails(details)); - Deployment::RemovePackage(GetPackageFullNameFromDetails(details), callback); + auto fullName = Msix::GetPackageFullNameFromFamilyName(GetPackageFamilyNameFromDetails(details)); + + if (!fullName) + { + AICLI_LOG(Repo, Info, << "No full name found for family name: " << GetPackageFamilyNameFromDetails(details)); + } + else + { + AICLI_LOG(Repo, Info, << "Removing package: " << *fullName); + Deployment::RemovePackage(*fullName, callback); + } } }; @@ -249,7 +250,7 @@ namespace AppInstaller::Repository::Microsoft return std::make_shared<SQLiteIndexSource>(details, std::move(index), std::move(lock)); } - void UpdateInternal(std::string packageLocation, SourceDetails& details, IProgressCallback& progress) override + void UpdateInternal(std::string packageLocation, const SourceDetails& details, IProgressCallback& progress) override { // We will extract the manifest and index files directly to this location std::filesystem::path packageState = GetStatePathFromDetails(details); diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -14,6 +14,15 @@ namespace AppInstaller::Repository { + // Defines the origin of the source details. + enum class SourceOrigin + { + Default, + User, + }; + + std::string_view ToString(SourceOrigin origin); + // Interface for retrieving information about a source without acting on it. struct SourceDetails { @@ -30,10 +39,10 @@ namespace AppInstaller::Repository std::string Data; // The last time that this source was updated. - std::chrono::system_clock::time_point LastUpdateTime; + std::chrono::system_clock::time_point LastUpdateTime = {}; - // This source is a default source; added for the user by the tool. - bool IsDefault = false; + // The origin of the source. + SourceOrigin Origin = SourceOrigin::Default; }; // Interface for interacting with a source from outside of the repository lib. @@ -55,7 +64,7 @@ namespace AppInstaller::Repository std::optional<SourceDetails> GetSource(std::string_view name); // Adds a new source for the user. - void AddSource(std::string name, std::string type, std::string arg, IProgressCallback& progress); + void AddSource(std::string_view name, std::string_view type, std::string_view arg, IProgressCallback& progress); // Opens an existing source. // Passing an empty string as the name of the source will return a source that aggregates all others. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -13,21 +13,37 @@ namespace AppInstaller::Repository using namespace std::chrono_literals; using namespace std::string_view_literals; - constexpr std::string_view s_RepositorySettings_UserSources = "usersources"sv; constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; - constexpr std::string_view s_SourcesYaml_Source_LastUpdate = "LastUpdate"sv; - constexpr std::string_view s_SourcesYaml_Source_IsDefault = "IsDefault"sv; + constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; + + constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; + constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; + constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; namespace { + // SourceDetails with additional data used by this file. + struct SourceDetailsInternal : public SourceDetails + { + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. + bool IsTombstone = false; + }; + + // Finds a source from the given vector by its name. + auto FindSourceByName(std::vector<SourceDetailsInternal>& sources, std::string_view name) + { + return std::find_if(sources.begin(), sources.end(), [&name](const SourceDetailsInternal& sd) { return Utility::CaseInsensitiveEquals(sd.Name, name); }); + } + // Attempts to read a single scalar value from the node. template<typename Value> bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true) @@ -49,11 +65,14 @@ namespace AppInstaller::Repository // Attempts to read the source details from the given stream. // Results are all or nothing; if any failures occur, no details are returned. - bool TryReadSourceDetails(std::string_view settingName, std::istream& stream, std::vector<SourceDetails>& sourceDetails) - { - sourceDetails.clear(); - - std::vector<SourceDetails> result; + bool TryReadSourceDetails( + std::string_view settingName, + std::istream& stream, + std::string_view rootName, + std::function<bool(SourceDetailsInternal&, const std::string&, const YAML::Node&)> parse, + std::vector<SourceDetailsInternal>& sourceDetails) + { + std::vector<SourceDetailsInternal> result; std::string settingValue = Utility::ReadEntireStream(stream); YAML::Node document; @@ -69,10 +88,10 @@ namespace AppInstaller::Repository try { - YAML::Node sources = document[std::string{ s_SourcesYaml_Sources }]; + YAML::Node sources = document[std::string{ rootName }]; if (!sources) { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (missing " << s_SourcesYaml_Sources << "):\n" << settingValue); + AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (missing " << rootName << "):\n" << settingValue); return false; } @@ -84,29 +103,16 @@ namespace AppInstaller::Repository if (!sources.IsSequence()) { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << s_SourcesYaml_Sources << " was not a sequence):\n" << settingValue); + AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << rootName << " was not a sequence):\n" << settingValue); return false; } for (const auto& source : sources) { - SourceDetails details; - if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } - if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } - if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } - if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } - int64_t lastUpdateInEpoch{}; - if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); - int32_t isDefaultNumber{}; - if (TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_IsDefault, isDefaultNumber, false)) + SourceDetailsInternal details; + if (!parse(details, settingValue, source)) { - details.IsDefault = (isDefaultNumber != 0); - } - else - { - // If older than defaults, assume it is not one. - details.IsDefault = false; + return false; } result.emplace_back(std::move(details)); @@ -123,27 +129,143 @@ namespace AppInstaller::Repository } // Gets the source details from a particular setting, or an empty optional if no setting exists. - std::optional<std::vector<SourceDetails>> TryGetSourcesFromSetting(std::string_view settingName) + std::optional<std::vector<SourceDetailsInternal>> TryGetSourcesFromSetting( + const Settings::StreamDefinition& setting, + std::string_view rootName, + std::function<bool(SourceDetailsInternal&, const std::string&, const YAML::Node&)> parse) { - auto sourcesStream = Settings::GetSettingStream(Settings::Type::Standard, settingName); + auto sourcesStream = Settings::GetSettingStream(setting); if (!sourcesStream) { - // Handle first run scenario and configure default source(s). // Note that this case is different than the one in which all sources have been removed. return {}; } else { - std::vector<SourceDetails> result; - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(settingName, *sourcesStream, result)); + std::vector<SourceDetailsInternal> result; + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(setting.Path, *sourcesStream, rootName, parse, result)); return result; } } // Gets the source details from a particular setting. - std::vector<SourceDetails> GetSourcesFromSetting(std::string_view settingName) + std::vector<SourceDetailsInternal> GetSourcesFromSetting( + const Settings::StreamDefinition& setting, + std::string_view rootName, + std::function<bool(SourceDetailsInternal&, const std::string&, const YAML::Node&)> parse) { - return TryGetSourcesFromSetting(settingName).value_or(std::vector<SourceDetails>{}); + return TryGetSourcesFromSetting(setting, rootName, parse).value_or(std::vector<SourceDetailsInternal>{}); + } + + // Gets the metadata. + std::vector<SourceDetailsInternal> GetMetadata() + { + return GetSourcesFromSetting( + Settings::Streams::SourcesMetadata, + s_MetadataYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + std::string_view name = Settings::Streams::SourcesMetadata.Path; + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } + int64_t lastUpdateInEpoch{}; + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } + details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); + return true; + }); + } + + // Gets the sources from a particular origin. + std::vector<SourceDetailsInternal> GetSourcesByOrigin(SourceOrigin origin) + { + std::vector<SourceDetailsInternal> result; + + switch (origin) + { + case SourceOrigin::Default: + { + SourceDetailsInternal details; + details.Name = s_Source_WingetCommunityDefault_Name; + details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); + details.Arg = s_Source_WingetCommunityDefault_Arg; + details.Data = s_Source_WingetCommunityDefault_Data; + result.emplace_back(std::move(details)); + } + break; + case SourceOrigin::User: + result = GetSourcesFromSetting( + Settings::Streams::UserSources, + s_SourcesYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + std::string_view name = Settings::Streams::UserSources.Path; + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } + return true; + }); + break; + default: + THROW_HR(E_UNEXPECTED); + } + + for (auto& source : result) + { + source.Origin = origin; + } + + return result; + } + + // Gets the internal view of the sources. + std::vector<SourceDetailsInternal> GetSourcesInternal() + { + std::vector<SourceDetailsInternal> result; + + for (SourceOrigin origin : { SourceOrigin::User, SourceOrigin::Default }) + { + auto forOrigin = GetSourcesByOrigin(origin); + + for (auto&& source : forOrigin) + { + auto itr = FindSourceByName(result, source.Name); + if (itr == result.end()) + { + // Name not already defined, add it + result.emplace_back(std::move(source)); + } + else + { + AICLI_LOG(Repo, Info, << "Source named '" << itr->Name << "' is already defined at origin " << ToString(itr->Origin) << + ". The source from origin " << ToString(origin) << " is dropped."); + } + } + } + + // Remove all tombstones, walking backwards. + for (size_t j = result.size(); j > 0; --j) + { + size_t i = j - 1; + + if (result[i].IsTombstone) + { + AICLI_LOG(Repo, Info, << "Source named '" << result[i].Name << "' from origin " << ToString(result[i].Origin) << " is a tombstone and is dropped."); + result.erase(result.begin() + i); + } + } + + auto metadata = GetMetadata(); + for (const auto& metaSource : metadata) + { + auto itr = FindSourceByName(result, metaSource.Name); + if (itr != result.end()) + { + itr->LastUpdateTime = metaSource.LastUpdateTime; + } + } + + return result; } // Make up for the lack of string_view support in YAML CPP. @@ -152,36 +274,69 @@ namespace AppInstaller::Repository return (out << std::string(sv)); } - // Sets the sources for a particular setting. - void SetSourcesToSetting(std::string_view settingName, const std::vector<SourceDetails>& sources) + // Sets the sources for a particular setting, from a particular origin. + void SetSourcesToSettingWithFilter(const Settings::StreamDefinition& setting, SourceOrigin origin, const std::vector<SourceDetailsInternal>& sources) { YAML::Emitter out; out << YAML::BeginMap; out << YAML::Key << s_SourcesYaml_Sources; out << YAML::BeginSeq; - for (const SourceDetails& details : sources) + for (const auto& details : sources) + { + if (details.Origin == origin) + { + out << YAML::BeginMap; + out << YAML::Key << s_SourcesYaml_Source_Name << YAML::Value << details.Name; + out << YAML::Key << s_SourcesYaml_Source_Type << YAML::Value << details.Type; + out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg; + out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; + out << YAML::Key << s_SourcesYaml_Source_IsTombstone << YAML::Value << details.IsTombstone; + out << YAML::EndMap; + } + } + + out << YAML::EndSeq; + out << YAML::EndMap; + + Settings::SetSetting(setting, out.c_str()); + } + + // Sets the metadata only (which is not a secure setting and can be set unprivileged) + void SetMetadata(const std::vector<SourceDetailsInternal>& sources) + { + YAML::Emitter out; + out << YAML::BeginMap; + out << YAML::Key << s_MetadataYaml_Sources; + out << YAML::BeginSeq; + + for (const auto& details : sources) { out << YAML::BeginMap; - out << YAML::Key << s_SourcesYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_SourcesYaml_Source_Type << YAML::Value << details.Type; - out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg; - out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; - out << YAML::Key << s_SourcesYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); - out << YAML::Key << s_SourcesYaml_Source_IsDefault << YAML::Value << (details.IsDefault ? 1 : 0); + out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; + out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); out << YAML::EndMap; } out << YAML::EndSeq; out << YAML::EndMap; - Settings::SetSetting(Settings::Type::Standard, settingName, out.c_str()); + Settings::SetSetting(Settings::Streams::SourcesMetadata, out.c_str()); } - // Finds a source from the given vector by its name. - auto FindSourceByName(std::vector<SourceDetails>& sources, std::string_view name) + // Sets the sources for a given origin. + void SetSourcesByOrigin(SourceOrigin origin, const std::vector<SourceDetailsInternal>& sources) { - return std::find_if(sources.begin(), sources.end(), [&name](const SourceDetails& sd) { return Utility::CaseInsensitiveEquals(sd.Name, name); }); + switch (origin) + { + case SourceOrigin::User: + SetSourcesToSettingWithFilter(Settings::Streams::UserSources, SourceOrigin::User, sources); + break; + default: + THROW_HR(E_UNEXPECTED); + } + + SetMetadata(sources); } #ifndef AICLI_DISABLE_TEST_HOOKS @@ -209,58 +364,53 @@ namespace AppInstaller::Repository THROW_HR(APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE); } - bool CheckIfInitializedFromDetails(const SourceDetails& details) - { - return GetFactoryForType(details.Type)->IsInitialized(details); - } - std::shared_ptr<ISource> CreateSourceFromDetails(const SourceDetails& details) { return GetFactoryForType(details.Type)->Create(details); } - void UpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + template <typename MemberFunc> + void AddOrUpdateFromDetails(SourceDetails& details, MemberFunc member, IProgressCallback& progress) { auto factory = GetFactoryForType(details.Type); - // Attempt to update; if it fails, wait a short time and retry. + // Attempt; if it fails, wait a short time and retry. try { - factory->Update(details, progress); + (factory.get()->*member)(details, progress); + details.LastUpdateTime = std::chrono::system_clock::now(); return; } CATCH_LOG(); - AICLI_LOG(Repo, Info, << "Source update failed, waiting a bit and retrying: " << details.Name); + AICLI_LOG(Repo, Info, << "Source add/update failed, waiting a bit and retrying: " << details.Name); std::this_thread::sleep_for(2s); // If this one fails, maybe the problem is persistent. - factory->Update(details, progress); + (factory.get()->*member)(details, progress); + details.LastUpdateTime = std::chrono::system_clock::now(); + } + + void AddSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + { + AddOrUpdateFromDetails(details, &ISourceFactory::Add, progress); + } + + void UpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + { + AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress); } void RemoveSourceFromDetails(const SourceDetails& details, IProgressCallback& progress) { auto factory = GetFactoryForType(details.Type); - if (factory->IsInitialized(details)) - { - factory->Remove(details, progress); - } - else - { - AICLI_LOG(Repo, Info, << "Uninitialized source being removed, making it a no-op: " << details.Name); - } + factory->Remove(details, progress); } // Determines whether (and logs why) a source should be updated before it is opened. bool ShouldUpdateBeforeOpen(const SourceDetails& details) { - if (!CheckIfInitializedFromDetails(details)) - { - AICLI_LOG(Repo, Info, << "Source needs to be initialized during open: " << details.Name); - return true; - } - constexpr static auto s_ZeroMins = 0min; auto autoUpdateTime = User().Get<Setting::AutoUpdateTimeInMinutes>(); @@ -280,84 +430,38 @@ namespace AppInstaller::Repository return false; } + } - SourceDetails PrepareSourceDetailsForAdd(std::string name, std::string type, std::string arg, bool isDefault) - { - THROW_HR_IF(E_INVALIDARG, name.empty()); - - AICLI_LOG(Repo, Info, << "Adding source: Name[" << name << "], Type[" << type << "], Arg[" << arg << "]"); - - // Check all sources for the given name. - std::vector<SourceDetails> currentSources = GetSources(); - - auto itr = FindSourceByName(currentSources, name); - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, itr != currentSources.end()); - - SourceDetails details; - details.Name = std::move(name); - details.Type = std::move(type); - details.Arg = std::move(arg); - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0); - details.IsDefault = isDefault; - - return details; - } - - void AddDetailsToSetting(const SourceDetails& details, std::string_view setting) - { - AICLI_LOG(Repo, Info, << "Source created with extra data: " << details.Data); - - std::vector<SourceDetails> currentSources = GetSourcesFromSetting(setting); - currentSources.emplace_back(details); - - SetSourcesToSetting(setting, currentSources); - } - - void AddSourceInternal(std::string name, std::string type, std::string arg, bool isDefault, IProgressCallback& progress) + std::string_view ToString(SourceOrigin origin) + { + switch (origin) { - SourceDetails details = PrepareSourceDetailsForAdd(name, type, arg, isDefault); - - UpdateSourceFromDetails(details, progress); - - AddDetailsToSetting(details, s_RepositorySettings_UserSources); + case SourceOrigin::Default: + return "Default"sv; + case SourceOrigin::User: + return "User"sv; + default: + THROW_HR(E_UNEXPECTED); } + } - void AddUninitializedSourceInternal(std::string name, std::string type, std::string arg, bool isDefault) - { - SourceDetails details = PrepareSourceDetailsForAdd(name, type, arg, isDefault); - AddDetailsToSetting(details, s_RepositorySettings_UserSources); - } + std::vector<SourceDetails> GetSources() + { + auto internalResult = GetSourcesInternal(); - // If there is no setting value at all, adds the default sources. - void AddDefaultSourcesIfNeeded() + std::vector<SourceDetails> result; + for (auto&& source : internalResult) { - auto sourcesStream = Settings::GetSettingStream(Settings::Type::Standard, s_RepositorySettings_UserSources); - if (!sourcesStream) - { - // We have to set an initial, empty list of sources or the add will create an infinite loop. - SetSourcesToSetting(s_RepositorySettings_UserSources, std::vector<SourceDetails>{}); - - AddUninitializedSourceInternal( - std::string(s_Source_WingetCommunityDefault_Name), - std::string(Microsoft::PreIndexedPackageSourceFactory::Type()), - std::string(s_Source_WingetCommunityDefault_Arg), - true); - } + result.emplace_back(std::move(source)); } - } - // TODO: If we merge sources from multiple settings in the future, the other functions - // in this file all need to be enlightened with how to write to othe appropriate location. - std::vector<SourceDetails> GetSources() - { - AddDefaultSourcesIfNeeded(); - return GetSourcesFromSetting(s_RepositorySettings_UserSources); + return result; } std::optional<SourceDetails> GetSource(std::string_view name) { // Check all sources for the given name. - std::vector<SourceDetails> currentSources = GetSources(); + auto currentSources = GetSourcesInternal(); auto itr = FindSourceByName(currentSources, name); if (itr == currentSources.end()) @@ -370,14 +474,36 @@ namespace AppInstaller::Repository } } - void AddSource(std::string name, std::string type, std::string arg, IProgressCallback& progress) + void AddSource(std::string_view name, std::string_view type, std::string_view arg, IProgressCallback& progress) { - AddSourceInternal(name, type, arg, false, progress); + THROW_HR_IF(E_INVALIDARG, name.empty()); + + AICLI_LOG(Repo, Info, << "Adding source: Name[" << name << "], Type[" << type << "], Arg[" << arg << "]"); + + // Check all sources for the given name. + auto currentSources = GetSourcesInternal(); + + auto itr = FindSourceByName(currentSources, name); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, itr != currentSources.end()); + + SourceDetailsInternal details; + details.Name = name; + details.Type = type; + details.Arg = arg; + details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0); + details.Origin = SourceOrigin::User; + + AddSourceFromDetails(details, progress); + + AICLI_LOG(Repo, Info, << "Source created with extra data: " << details.Data); + currentSources.emplace_back(details); + + SetSourcesByOrigin(SourceOrigin::User, currentSources); } std::shared_ptr<ISource> OpenSource(std::string_view name, IProgressCallback& progress) { - std::vector<SourceDetails> currentSources = GetSources(); + auto currentSources = GetSourcesInternal(); if (name.empty()) { @@ -408,7 +534,7 @@ namespace AppInstaller::Repository if (ShouldUpdateBeforeOpen(*itr)) { UpdateSourceFromDetails(*itr, progress); - SetSourcesToSetting(s_RepositorySettings_UserSources, currentSources); + SetMetadata(currentSources); } return CreateSourceFromDetails(*itr); } @@ -419,7 +545,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - std::vector<SourceDetails> currentSources = GetSources(); + auto currentSources = GetSourcesInternal(); auto itr = FindSourceByName(currentSources, name); if (itr == currentSources.end()) @@ -432,7 +558,7 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Info, << "Named source to be updated, found: " << itr->Name); UpdateSourceFromDetails(*itr, progress); - SetSourcesToSetting(s_RepositorySettings_UserSources, currentSources); + SetMetadata(currentSources); return true; } } @@ -441,7 +567,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - std::vector<SourceDetails> currentSources = GetSources(); + auto currentSources = GetSourcesInternal(); auto itr = FindSourceByName(currentSources, name); if (itr == currentSources.end()) @@ -451,11 +577,28 @@ namespace AppInstaller::Repository } else { - AICLI_LOG(Repo, Info, << "Named source to be removed, found: " << itr->Name); + AICLI_LOG(Repo, Info, << "Named source to be removed, found: " << itr->Name << " [" << ToString(itr->Origin) << ']'); RemoveSourceFromDetails(*itr, progress); - currentSources.erase(itr); - SetSourcesToSetting(s_RepositorySettings_UserSources, currentSources); + switch (itr->Origin) + { + case SourceOrigin::Default: + { + SourceDetailsInternal tombstone; + tombstone.Name = name; + tombstone.IsTombstone = true; + tombstone.Origin = SourceOrigin::User; + currentSources.emplace_back(std::move(tombstone)); + } + break; + case SourceOrigin::User: + currentSources.erase(itr); + break; + default: + THROW_HR(E_UNEXPECTED); + } + + SetSourcesByOrigin(SourceOrigin::User, currentSources); return true; } @@ -465,12 +608,13 @@ namespace AppInstaller::Repository { if (name.empty()) { - Settings::RemoveSetting(Settings::Type::Standard, s_RepositorySettings_UserSources); + Settings::RemoveSetting(Settings::Streams::UserSources); + Settings::RemoveSetting(Settings::Streams::SourcesMetadata); return true; } else { - std::vector<SourceDetails> currentSources = GetSources(); + auto currentSources = GetSourcesInternal(); auto itr = FindSourceByName(currentSources, name); if (itr == currentSources.end()) @@ -483,7 +627,11 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Info, << "Named source to be dropped, found: " << itr->Name); currentSources.erase(itr); - SetSourcesToSetting(s_RepositorySettings_UserSources, currentSources); + + // Since this only writes the user setting, it can't actually drop non-user sources. + // But since it also implicitly sets all metadata, it will drop the metadata and allow + // somewhat of a clean slate. + SetSourcesByOrigin(SourceOrigin::User, currentSources); return true; } diff --git a/src/AppInstallerRepositoryCore/SourceFactory.h b/src/AppInstallerRepositoryCore/SourceFactory.h @@ -14,14 +14,14 @@ namespace AppInstaller::Repository { virtual ~ISourceFactory() = default; - // Returns a value indicating whether the source details reference a source that is properly initialized. - virtual bool IsInitialized(const SourceDetails& details) = 0; - // Creates a source object from the given details. virtual std::shared_ptr<ISource> Create(const SourceDetails& details) = 0; - // Updates the source from the given details, writing back to the details any changes. - virtual void Update(SourceDetails& details, IProgressCallback& progress) = 0; + // Adds the source from the given details, writing back to the details any changes. + virtual void Add(SourceDetails& details, IProgressCallback& progress) = 0; + + // Updates the source from the given details (may not change the details). + virtual void Update(const SourceDetails& details, IProgressCallback& progress) = 0; // Removes the source from the given details. virtual void Remove(const SourceDetails& details, IProgressCallback& progress) = 0;