From 8af6d71b8cdbb4701f73948c03ea7cffc9d69eca Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 6 Apr 2026 16:14:31 -0700 Subject: [PATCH 1/8] checkpoint --- .../Workflows/DependenciesFlow.cpp | 6 +++ .../ShellExecuteInstallerHandler.cpp | 6 +++ .../InstallDependenciesFlow.cpp | 42 +++++++++++++++++++ src/AppInstallerCLITests/Strings.cpp | 25 +++++++++++ .../Manifest/ManifestValidation.cpp | 9 ++++ .../Public/winget/ManifestValidation.h | 1 + .../AppInstallerStrings.cpp | 18 ++++++++ .../Public/AppInstallerStrings.h | 3 ++ 8 files changed, 110 insertions(+) diff --git a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp index 948be1585b..0581d48d07 100644 --- a/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/DependenciesFlow.cpp @@ -175,6 +175,12 @@ namespace AppInstaller::CLI::Workflow { AICLI_LOG(Core, Info, << "Successfully enabled [" << featureName << "]"); } + else if (result == E_INVALIDARG) + { + AICLI_LOG(Core, Warning, << "Invalid Windows Feature name [" << featureName << "]"); + enableFeaturesFailed = true; + featureContext.Reporter.Warn() << Resource::String::WindowsFeatureNotFound(locIndFeatureName) << std::endl; + } else if (result == 0x800f080c) // DISMAPI_E_UNKNOWN_FEATURE { AICLI_LOG(Core, Warning, << "Windows Feature [" << featureName << "] does not exist"); diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 726a547f85..46923165d9 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -509,6 +509,12 @@ namespace AppInstaller::CLI::Workflow void ShellExecuteEnableWindowsFeature::operator()(Execution::Context& context) const { + if (!Utility::IsValidWindowsFeaturePattern(m_featureName)) + { + context.Add(E_INVALIDARG); + return; + } + Utility::LocIndView locIndFeatureName{ m_featureName }; std::optional doesFeatureExistResult = DoesWindowsFeatureExist(context, m_featureName); diff --git a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp index c3f81912de..3f0b5df1ec 100644 --- a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp +++ b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "WorkflowCommon.h" #include "DependenciesTestSource.h" +#include #include #include #include @@ -306,6 +307,47 @@ TEST_CASE("InstallFlow_Dependencies_COM", "[InstallFlow][workflow][dependencies] REQUIRE(installationOrder.at(2) == "AppInstallerCliTest.TestExeInstaller.MultipleDependencies"); } +void InstallFlow_Dependencies_WindowsFeaturesArgument_Generic(std::string_view featureName) +{ + if (!AppInstaller::Runtime::IsRunningAsAdminOrSystem()) + { + SKIP("Must be running as admin to test dism calls."); + } + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + + context << ShellExecuteEnableWindowsFeature(featureName); + + INFO(installOutput.str()); + + REQUIRE(context.Contains(Execution::Data::OperationReturnCode)); + REQUIRE(context.Get() == E_INVALIDARG); +} + +TEST_CASE("InstallFlow_Dependencies_WindowsFeaturesArgument_Extras", "[InstallFlow][workflow][dependencies][111981]") +{ + TempFile potentialLogFile("dism-log", ".log"); + std::string featureName = "MediaPlayback /LogPath:"; + featureName.append(potentialLogFile.GetPath().u8string()); + + InstallFlow_Dependencies_WindowsFeaturesArgument_Generic(featureName); + + REQUIRE(!std::filesystem::exists(potentialLogFile)); +} + +TEST_CASE("InstallFlow_Dependencies_WindowsFeaturesArgument_Quoted", "[InstallFlow][workflow][dependencies][111981]") +{ + TempFile potentialLogFile("dism-log", ".log"); + std::string featureName = "\"MediaPlayback /LogPath:"; + featureName.append(potentialLogFile.GetPath().u8string()); + featureName.append("\""); + + InstallFlow_Dependencies_WindowsFeaturesArgument_Generic(featureName); + + REQUIRE(!std::filesystem::exists(potentialLogFile)); +} + // TODO: // add dependencies for installer tests to DependenciesTestSource (or a new one) // add tests for min version dependency solving diff --git a/src/AppInstallerCLITests/Strings.cpp b/src/AppInstallerCLITests/Strings.cpp index dfe4e97f76..5dd9fa1e14 100644 --- a/src/AppInstallerCLITests/Strings.cpp +++ b/src/AppInstallerCLITests/Strings.cpp @@ -354,3 +354,28 @@ TEST_CASE("ConvertControlCodesToPictures", "[strings]") REQUIRE(ConvertControlCodesToPictures(allCodes) == ConvertToUTF8(allPictures)); } + +TEST_CASE("IsValidWindowsFeaturePattern_AllFound_True", "[strings][111981]") +{ + for (const auto& name : { + "IIS-ODBCLogging", + "NetFx3", + "SMB1Protocol", + }) + { + INFO(name); + REQUIRE(IsValidWindowsFeaturePattern(name)); + } +} + +TEST_CASE("IsValidWindowsFeaturePattern_Bad_False", "[strings][111981]") +{ + for (const auto& name : { + "MediaPlayback /LogPath:C:\\file.txt", + "\"MediaPlayback /LogPath:C:\\file.txt\"", + }) + { + INFO(name); + REQUIRE(IsValidWindowsFeaturePattern(name)); + } +} diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 50ef2ad2a5..fbea8a2295 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -85,6 +85,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::SchemaHeaderUrlPatternMismatch, "The schema header URL does not match the expected pattern."sv }, { AppInstaller::Manifest::ManifestError::InvalidPortableFiletype, "The file type of the referenced file is not allowed."sv }, { AppInstaller::Manifest::ManifestError::InvalidFontFiletype, "The file type of the referenced file is not a supported font file type."sv }, + { AppInstaller::Manifest::ManifestError::InvalidWindowsFeatureName, "The provided value is not a valid Windows feature name."sv }, }; return ErrorIdToMessageMap; @@ -437,6 +438,14 @@ namespace AppInstaller::Manifest } } + installer.Dependencies.ApplyToType(DependencyType::WindowsFeature, [&](const Dependency& dependency) + { + if (!IsValidWindowsFeaturePattern(dependency.Id())) + { + resultErrors.emplace_back(ManifestError::InvalidWindowsFeatureName, dependency.Id()); + } + }); + if (fullValidation) { for (const auto& container : installer.DesiredStateConfiguration) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h index 8fdbd6c3a8..15a1b679d0 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -55,6 +55,7 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(InvalidBcp47Value); WINGET_DEFINE_RESOURCE_STRINGID(InvalidFieldValue); WINGET_DEFINE_RESOURCE_STRINGID(InvalidRootNode); + WINGET_DEFINE_RESOURCE_STRINGID(InvalidWindowsFeatureName); WINGET_DEFINE_RESOURCE_STRINGID(MissingManifestDependenciesNode); WINGET_DEFINE_RESOURCE_STRINGID(MsixSignatureHashFailed); WINGET_DEFINE_RESOURCE_STRINGID(MultiManifestPackageHasDependencies); diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp index 99be199fda..6bee93edce 100644 --- a/src/AppInstallerSharedLib/AppInstallerStrings.cpp +++ b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -1118,4 +1118,22 @@ namespace AppInstaller::Utility return result; } + + bool IsValidWindowsFeaturePattern(std::string_view value) + { + if (value.empty()) + { + return false; + } + + for (char c : value) + { + if (!std::isalnum(static_cast(c)) && c != '-' && c != '_') + { + return false; + } + } + + return true; + } } diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h index ea2e1af88a..7b60542fe8 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -320,4 +320,7 @@ namespace AppInstaller::Utility // Generates a random alpha numeric string. std::string GetRandomString(size_t size = 8); + + // Checks whether a given string is a valid potential Windows feature name. + bool IsValidWindowsFeaturePattern(std::string_view value); } From 468d895f3d3dd9cfdcc21b699be7df3ed4200752 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 6 Apr 2026 16:57:59 -0700 Subject: [PATCH 2/8] dism done --- .../ShellExecuteInstallerHandler.cpp | 2 +- .../AppInstallerCLITests.vcxproj | 3 +++ .../AppInstallerCLITests.vcxproj.filters | 3 +++ .../InstallDependenciesFlow.cpp | 5 ----- src/AppInstallerCLITests/Strings.cpp | 2 +- ...anifest-Bad-InvalidWindowsFeatureName.yaml | 22 +++++++++++++++++++ src/AppInstallerCLITests/YamlManifest.cpp | 14 ++++++++++++ 7 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidWindowsFeatureName.yaml diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 46923165d9..37405faf51 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -511,7 +511,7 @@ namespace AppInstaller::CLI::Workflow { if (!Utility::IsValidWindowsFeaturePattern(m_featureName)) { - context.Add(E_INVALIDARG); + context.Add(static_cast(E_INVALIDARG)); return; } diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index e2d439a275..48dca5dc2f 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -1073,6 +1073,9 @@ true + + true + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 643382697d..1e0c21dfe6 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -1140,5 +1140,8 @@ TestData + + TestData + \ No newline at end of file diff --git a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp index 3f0b5df1ec..b021d3d366 100644 --- a/src/AppInstallerCLITests/InstallDependenciesFlow.cpp +++ b/src/AppInstallerCLITests/InstallDependenciesFlow.cpp @@ -309,11 +309,6 @@ TEST_CASE("InstallFlow_Dependencies_COM", "[InstallFlow][workflow][dependencies] void InstallFlow_Dependencies_WindowsFeaturesArgument_Generic(std::string_view featureName) { - if (!AppInstaller::Runtime::IsRunningAsAdminOrSystem()) - { - SKIP("Must be running as admin to test dism calls."); - } - std::ostringstream installOutput; TestContext context{ installOutput, std::cin }; diff --git a/src/AppInstallerCLITests/Strings.cpp b/src/AppInstallerCLITests/Strings.cpp index 5dd9fa1e14..5a8a0d0fda 100644 --- a/src/AppInstallerCLITests/Strings.cpp +++ b/src/AppInstallerCLITests/Strings.cpp @@ -376,6 +376,6 @@ TEST_CASE("IsValidWindowsFeaturePattern_Bad_False", "[strings][111981]") }) { INFO(name); - REQUIRE(IsValidWindowsFeaturePattern(name)); + REQUIRE(!IsValidWindowsFeaturePattern(name)); } } diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidWindowsFeatureName.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidWindowsFeatureName.yaml new file mode 100644 index 0000000000..16a4fe190f --- /dev/null +++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidWindowsFeatureName.yaml @@ -0,0 +1,22 @@ +# Installer with an invalid Windows Feature name in dependencies +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.0.0.schema.json + +PackageIdentifier: AppInstallerCliTest.TestMsixInstaller +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test MSIX Installer +ShortDescription: AppInstaller Test MSIX Installer +Publisher: Microsoft Corporation +Moniker: AICLITestMsix +License: Test +Installers: + - Architecture: x64 + InstallerUrl: https://github.com/microsoft/msix-packaging/blob/master/src/test/testData/unpack/TestAppxPackage_x64.appx?raw=true + InstallerType: msix + InstallerSha256: 6a2d3683fa19bf00e58e07d1313d20a5f5735ebbd6a999d33381d28740ee07ea + PackageFamilyName: 20477fca-282d-49fb-b03e-371dca074f0f_8wekyb3d8bbwe + Dependencies: + WindowsFeatures: + - Invalid@Feature +ManifestType: singleton +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 4c5021887e..419d752c8d 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1360,6 +1360,20 @@ TEST_CASE("PortableFileTypeValidation", "[ManifestValidation]") REQUIRE(errors.size() == 0); } +TEST_CASE("WindowsFeatureNameValidation", "[ManifestValidation][111981]") +{ + // An invalid Windows Feature name should produce an error regardless of the fullValidation flag + Manifest invalidManifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-InvalidWindowsFeatureName.yaml")); + + auto errors = ValidateManifest(invalidManifest, true); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InvalidWindowsFeatureName, "Invalid@Feature", ""); + + errors = ValidateManifest(invalidManifest, false); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InvalidWindowsFeatureName, "Invalid@Feature", ""); +} + TEST_CASE("ReadManifestAndValidateMsixInstallers_Success", "[ManifestValidation]") { TestDataFile testFile("Manifest-Good-MsixInstaller.yaml"); From 7976f05b590f41d6c4cc2a89959075a8192709fe Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 7 Apr 2026 10:53:54 -0700 Subject: [PATCH 3/8] msi props and network addresses, validation report script --- .../Workflows/InstallFlow.cpp | 10 +- .../Manifest/ManifestCommon.cpp | 7 + .../Manifest/ManifestValidation.cpp | 45 ++ .../MsiExecArguments.cpp | 41 +- .../Public/winget/ManifestCommon.h | 3 + .../Public/winget/ManifestValidation.h | 3 + .../Public/winget/MsiExecArguments.h | 8 + .../Invoke-ManifestValidation.ps1 | 434 ++++++++++++++++++ 8 files changed, 536 insertions(+), 15 deletions(-) create mode 100644 tools/ManifestValidation/Invoke-ManifestValidation.ps1 diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 130cddb2b2..15ff0bf57b 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -61,14 +61,8 @@ namespace AppInstaller::CLI::Workflow bool ShouldUseDirectMSIInstall(InstallerTypeEnum type, bool isSilentInstall) { - switch (type) - { - case InstallerTypeEnum::Msi: - case InstallerTypeEnum::Wix: - return isSilentInstall || ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::DirectMSI); - default: - return false; - } + return DoesInstallerTypeUseMsiProperties(type) && + (isSilentInstall || ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::DirectMSI)); } bool ShouldErrorForUnsupportedArgument(UnsupportedArgumentEnum arg) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index 82454b1847..eca420ea00 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -964,6 +964,13 @@ namespace AppInstaller::Manifest installerType == InstallerTypeEnum::Exe; } + bool DoesInstallerTypeUseMsiProperties(InstallerTypeEnum installerType) + { + return + installerType == InstallerTypeEnum::Msi || + installerType == InstallerTypeEnum::Wix; + } + bool IsArchiveType(InstallerTypeEnum installerType) { return (installerType == InstallerTypeEnum::Zip); diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index fbea8a2295..14ef7c7d68 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -8,6 +8,7 @@ #include "winget/MsixManifestValidation.h" #include "winget/Locale.h" #include "winget/Filesystem.h" +#include "winget/MsiExecArguments.h" namespace AppInstaller::Manifest { @@ -86,10 +87,21 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::InvalidPortableFiletype, "The file type of the referenced file is not allowed."sv }, { AppInstaller::Manifest::ManifestError::InvalidFontFiletype, "The file type of the referenced file is not a supported font file type."sv }, { AppInstaller::Manifest::ManifestError::InvalidWindowsFeatureName, "The provided value is not a valid Windows feature name."sv }, + { AppInstaller::Manifest::ManifestError::BlockedMsiProperty, "Contains a blocked MSI property."sv }, + { AppInstaller::Manifest::ManifestError::InvalidMsiSwitches, "Contains invalid MSI switches."sv }, + { AppInstaller::Manifest::ManifestError::ContainsNetworkAddress, "Contains network address."sv }, }; return ErrorIdToMessageMap; } + + bool ContainsNetworkAddressSignifier(std::string_view input) + { + return Utility::CaseInsensitiveContainsSubstring(input, "http://") || + Utility::CaseInsensitiveContainsSubstring(input, "https://") || + Utility::CaseInsensitiveContainsSubstring(input, "ftp://") || + Utility::CaseInsensitiveContainsSubstring(input, "\\\\"); + } } std::vector ValidateManifest(const Manifest& manifest, bool fullValidation) @@ -446,6 +458,17 @@ namespace AppInstaller::Manifest } }); + for (const auto& item : installer.Switches) + { + if (!item.second.empty()) + { + if (ContainsNetworkAddressSignifier(item.second)) + { + resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); + } + } + } + if (fullValidation) { for (const auto& container : installer.DesiredStateConfiguration) @@ -457,6 +480,28 @@ namespace AppInstaller::Manifest break; } } + + if (DoesInstallerTypeUseMsiProperties(installer.EffectiveInstallerType())) + { + try + { + for (const auto& item : installer.Switches) + { + if (!item.second.empty()) + { + auto blocked = Msi::ParseMSIArguments(item.second).GetFirstBlockedProperty(); + if (blocked) + { + resultErrors.emplace_back(ManifestError::BlockedMsiProperty, blocked.value()); + } + } + } + } + catch (...) + { + resultErrors.emplace_back(ManifestError::InvalidMsiSwitches); + } + } } } diff --git a/src/AppInstallerCommonCore/MsiExecArguments.cpp b/src/AppInstallerCommonCore/MsiExecArguments.cpp index e5c955b6a3..12ad82992c 100644 --- a/src/AppInstallerCommonCore/MsiExecArguments.cpp +++ b/src/AppInstallerCommonCore/MsiExecArguments.cpp @@ -355,14 +355,14 @@ namespace AppInstaller::Msi // Validates that a token represents a property. // This checks that the property has the form PropertyName=Value, // with the value optionally quoted. - bool IsValidPropertyToken(std::string_view token) + std::optional ParsePropertyToken(std::string_view token) { THROW_HR_IF(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR, token.empty()); if (token[0] != '%' && !IsCharAlphaNumericA(token[0])) { AICLI_LOG(Core, Error, << "Bad property for msiexec: " << token); - return false; + return std::nullopt; } // Find the = separator at the end of the property name @@ -375,9 +375,11 @@ namespace AppInstaller::Msi if (pos == token.size() || token[pos] != '=') { AICLI_LOG(Core, Error, << "Expected property for call to msiexec, but couldn't find separator: " << token); - return false; + return std::nullopt; } + size_t nameLength = pos; + // Validate the property value. // It should be completely enclosed in quotes, or not contain white space. // If quoted, there can be pairs of consecutive quotes that work as escape sequences. @@ -386,7 +388,7 @@ namespace AppInstaller::Msi if (pos == token.size()) { // Empty value - return true; + return MsiParsedArguments::ParsedProperty{ std::string{ token.substr(0, nameLength) }, {} }; } // If quoted, we will only inspect the values between the quotes. @@ -438,7 +440,7 @@ namespace AppInstaller::Msi ++pos; } - return true; + return MsiParsedArguments::ParsedProperty{ std::string{ token.substr(0, nameLength) }, std::string{ token.substr(nameLength + 1) } }; } // Replaces long options in the arguments (e.g. /quiet), by their short equivalents @@ -502,9 +504,11 @@ namespace AppInstaller::Msi tokens.pop_front(); if (!IsSwitch(token)) { + auto propertyToken = ParsePropertyToken(token); // Token is a property, i.e. NAME=value. Add it to the parsed args. - THROW_HR_IF(APPINSTALLER_CLI_ERROR_INVALID_MSIEXEC_ARGUMENT, !IsValidPropertyToken(token)); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_INVALID_MSIEXEC_ARGUMENT, !propertyToken); parsedArgs.Properties += L" " + Utility::ConvertToUTF16(token); + parsedArgs.ParsedProperties.emplace_back(std::move(propertyToken).value()); return; } @@ -550,6 +554,29 @@ namespace AppInstaller::Msi } } + std::optional MsiParsedArguments::GetFirstBlockedProperty() const + { + for (const auto& property : ParsedProperties) + { + auto lowerName = Utility::ToLower(property.first); + + for (const auto& blockedName : { + "transforms", + "patch", + // TODO: There are more + }) + { + if (blockedName == lowerName) + { + AICLI_LOG(Core, Warning, << "MSI arguments contain blocked property: " << lowerName); + return property.first; + } + } + } + + return std::nullopt; + } + MsiParsedArguments ParseMSIArguments(std::string_view arguments) { // Split the arguments into tokens, which we will process one by one. @@ -567,4 +594,4 @@ namespace AppInstaller::Msi return result; } -} \ No newline at end of file +} diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 96b7affb01..1962b49ec8 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -496,6 +496,9 @@ namespace AppInstaller::Manifest // Gets a value indicating whether the given installer requires RepairBehavior for repair. bool DoesInstallerTypeRequireRepairBehaviorForRepair(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer type uses MSI properties in its command line. + bool DoesInstallerTypeUseMsiProperties(InstallerTypeEnum installerType); + // Gets a value indicating whether the given installer type is an archive. bool IsArchiveType(InstallerTypeEnum installerType); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h index 15a1b679d0..7b5d8470ae 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -23,7 +23,9 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(ArpValidationError); WINGET_DEFINE_RESOURCE_STRINGID(ArpVersionOverlapWithIndex); WINGET_DEFINE_RESOURCE_STRINGID(ArpVersionValidationInternalError); + WINGET_DEFINE_RESOURCE_STRINGID(BlockedMsiProperty); WINGET_DEFINE_RESOURCE_STRINGID(BothAllowedAndExcludedMarketsDefined); + WINGET_DEFINE_RESOURCE_STRINGID(ContainsNetworkAddress); WINGET_DEFINE_RESOURCE_STRINGID(DuplicatePortableCommandAlias); WINGET_DEFINE_RESOURCE_STRINGID(DuplicateRelativeFilePath); WINGET_DEFINE_RESOURCE_STRINGID(DuplicateMultiFileManifestLocale); @@ -54,6 +56,7 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(InstallerTypeDoesNotWriteAppsAndFeaturesEntry); WINGET_DEFINE_RESOURCE_STRINGID(InvalidBcp47Value); WINGET_DEFINE_RESOURCE_STRINGID(InvalidFieldValue); + WINGET_DEFINE_RESOURCE_STRINGID(InvalidMsiSwitches); WINGET_DEFINE_RESOURCE_STRINGID(InvalidRootNode); WINGET_DEFINE_RESOURCE_STRINGID(InvalidWindowsFeatureName); WINGET_DEFINE_RESOURCE_STRINGID(MissingManifestDependenciesNode); diff --git a/src/AppInstallerCommonCore/Public/winget/MsiExecArguments.h b/src/AppInstallerCommonCore/Public/winget/MsiExecArguments.h index 02f1233f63..07ea66d8ad 100644 --- a/src/AppInstallerCommonCore/Public/winget/MsiExecArguments.h +++ b/src/AppInstallerCommonCore/Public/winget/MsiExecArguments.h @@ -51,6 +51,14 @@ namespace AppInstaller::Msi // Properties string std::wstring Properties; + + using ParsedProperty = std::pair; + + // Contains the properties as split into name and value portions. + std::vector ParsedProperties; + + // Checks for properties that are blocked in some cases. + std::optional GetFirstBlockedProperty() const; }; // Parses a command line string for msiexec. diff --git a/tools/ManifestValidation/Invoke-ManifestValidation.ps1 b/tools/ManifestValidation/Invoke-ManifestValidation.ps1 new file mode 100644 index 0000000000..95fd75b40c --- /dev/null +++ b/tools/ManifestValidation/Invoke-ManifestValidation.ps1 @@ -0,0 +1,434 @@ +#Requires -Version 5.1 + +<# +.SYNOPSIS + Runs manifest validation on every manifest under a given path and produces an HTML report. + +.DESCRIPTION + Discovers all manifest directories under the specified path (the leaf directories + containing YAML files, as found in a winget-pkgs clone), runs 'wingetdev validate' + on each, shows progress, and writes a self-contained HTML report with no external + script or style references. + + wingetdev is resolved from PATH. An explicit path may be provided via -WingetDevPath + if wingetdev is not on PATH. + +.PARAMETER ManifestsPath + Path to search for manifests. May be the root of a winget-pkgs clone (the script will + descend into its 'manifests' subdirectory if present) or a manifests directory directly. + +.PARAMETER WingetDevPath + Optional explicit path to wingetdev.exe. If not provided, wingetdev is resolved from PATH. + +.PARAMETER OutputPath + Optional path for the HTML report file. Defaults to 'manifest-validation-report.html' + in the current working directory. + +.EXAMPLE + .\Invoke-ManifestValidation.ps1 -ManifestsPath C:\repos\winget-pkgs + +.EXAMPLE + .\Invoke-ManifestValidation.ps1 -ManifestsPath C:\repos\winget-pkgs\manifests ` + -WingetDevPath C:\tools\wingetdev.exe -OutputPath C:\reports\results.html +#> + +[CmdletBinding()] +Param( + [Parameter(Mandatory = $true, Position = 0, HelpMessage = "Path to search for manifests (winget-pkgs root or manifests directory).")] + [string] $ManifestsPath, + + [Parameter(HelpMessage = "Path to wingetdev.exe. Resolved from PATH if not provided.")] + [string] $WingetDevPath, + + [Parameter(HelpMessage = "Output path for the HTML report.")] + [string] $OutputPath = (Join-Path (Get-Location) "manifest-validation-report.html"), + + [Parameter(HelpMessage = "Launch the HTML report in the default browser when complete.")] + [switch] $Launch, + + [Parameter(HelpMessage = "Exclude warnings from the report results table.")] + [switch] $SuppressWarnings, + + [Parameter(HelpMessage = "Resume from an existing report file, skipping already-completed top-level directories.")] + [switch] $Resume +) + +$ErrorActionPreference = "Stop" + +# --------------------------------------------------------------------------- +# HTML encoding helper (avoids requiring System.Web) +# --------------------------------------------------------------------------- +function ConvertTo-HtmlEncoded([string] $text) +{ + $text.Replace('&', '&').Replace('<', '<').Replace('>', '>').Replace('"', '"') +} + +# --------------------------------------------------------------------------- +# Resolve wingetdev +# --------------------------------------------------------------------------- +if ($WingetDevPath) +{ + if (-not (Test-Path $WingetDevPath -PathType Leaf)) + { + Write-Error -Category InvalidArgument -Message "wingetdev.exe not found at: $WingetDevPath" + } + $wingetDev = $WingetDevPath +} +else +{ + $wingetDevCmd = Get-Command "wingetdev" -ErrorAction SilentlyContinue + if (-not $wingetDevCmd) + { + Write-Error -Category ObjectNotFound -Message @" +wingetdev was not found on PATH. +Either add wingetdev to your PATH, or provide its location with -WingetDevPath. +"@ + } + $wingetDev = $wingetDevCmd.Source +} + +Write-Host "Using wingetdev: $wingetDev" + +$wingetDevVersion = (& $wingetDev --version 2>&1 | Out-String).Trim() + +# --------------------------------------------------------------------------- +# Resolve manifests search root +# --------------------------------------------------------------------------- +$ManifestsPath = [System.IO.Path]::GetFullPath($ManifestsPath) +if (-not (Test-Path $ManifestsPath -PathType Container)) +{ + Write-Error -Category InvalidArgument -Message "ManifestsPath does not exist or is not a directory: $ManifestsPath" +} + +# Support passing either the repo root (which contains a 'manifests' subdirectory) +# or the manifests directory itself. +$manifestsSubdir = Join-Path $ManifestsPath "manifests" +$searchRoot = if (Test-Path $manifestsSubdir -PathType Container) { $manifestsSubdir } else { $ManifestsPath } + +Write-Host "Discovering top-level directories under: $searchRoot" + +$tier1Dirs = Get-ChildItem $searchRoot -Directory -ErrorAction SilentlyContinue | Sort-Object Name +if (-not $tier1Dirs) +{ + Write-Error -Category ObjectNotFound -Message "No subdirectories found under: $searchRoot" +} +$tier1Total = $tier1Dirs.Count +$tier1Current = 0 + +Write-Host "Found $tier1Total top-level directories." + +# --------------------------------------------------------------------------- +# Initialize script-level state (shared with Write-ValidationReport) +# --------------------------------------------------------------------------- +$script:existingRowsHtml = '' +$script:completedTier1Dirs = [System.Collections.Generic.List[string]]::new() + +$passed = 0 +$warnings = 0 +$failed = 0 +$errors = 0 +$total = 0 + +if ($Resume) +{ + if (-not (Test-Path $OutputPath -PathType Leaf)) + { + Write-Error -Category ObjectNotFound -Message "Resume requested but no report file found at: $OutputPath" + } + + Write-Host "Reading resume state from: $OutputPath" + $content = Get-Content $OutputPath -Raw -Encoding utf8 + + if ($content -notmatch '(?s)') + { + Write-Error -Category InvalidData -Message "Could not find embedded resume state in: $OutputPath" + } + $state = $Matches[1].Trim() | ConvertFrom-Json + + foreach ($d in $state.completedTier1Dirs) { $script:completedTier1Dirs.Add($d) } + $passed = [int]$state.passed + $warnings = [int]$state.warnings + $failed = [int]$state.failed + $errors = [int]$state.errors + $total = [int]$state.total + + if ($content -match '(?s)(.*?)') + { + $script:existingRowsHtml = $Matches[1].Trim() + } + + Write-Host ("Resuming: {0} top-level directories already complete, {1} manifests already processed." -f $script:completedTier1Dirs.Count, $total) +} + +# --------------------------------------------------------------------------- +# Report writing helper +# --------------------------------------------------------------------------- +function Write-ValidationReport +{ + param( + [System.Collections.Generic.List[PSCustomObject]] $Results, + [int] $Total, + [int] $Passed, + [int] $Warnings, + [int] $Failed, + [int] $Errors + ) + + # Always back up the existing report before overwriting - guards against data loss + # during a long-running write. State is already in memory at this point. + if (Test-Path $OutputPath -PathType Leaf) + { + $dir = [System.IO.Path]::GetDirectoryName($OutputPath) + $base = [System.IO.Path]::GetFileNameWithoutExtension($OutputPath) + $ext = [System.IO.Path]::GetExtension($OutputPath) + $backup = Join-Path $dir "$base.backup$ext" + Move-Item $OutputPath $backup -Force + } + + $timestamp = (Get-Date).ToString('yyyy-MM-dd HH:mm:ss') + $escapedRoot = ConvertTo-HtmlEncoded $searchRoot + $escapedWingetVer = ConvertTo-HtmlEncoded $wingetDevVersion + + # Rows from the previous (resumed) run come first; new rows from this run follow. + $newRowsHtml = ($Results | ForEach-Object { + $statusClass = $_.Status.ToLower() + $escapedPath = ConvertTo-HtmlEncoded $_.RelativePath + $escapedOutput = (ConvertTo-HtmlEncoded $_.Output) -replace "`r?`n", '
' + " $escapedPath$($_.Status)$escapedOutput" + }) -join "`n" + + $rowsHtml = if ($script:existingRowsHtml -and $newRowsHtml) { "$($script:existingRowsHtml)`n$newRowsHtml" } + elseif ($script:existingRowsHtml) { $script:existingRowsHtml } + else { $newRowsHtml } + + # Embed progress state so the run can be resumed later. + $stateJson = [PSCustomObject]@{ + completedTier1Dirs = @($script:completedTier1Dirs) + tier1Total = $tier1Total + total = $Total + passed = $Passed + warnings = $Warnings + failed = $Failed + errors = $Errors + } | ConvertTo-Json -Compress + + $completed = $script:completedTier1Dirs.Count + $bannerHtml = if ($completed -lt $tier1Total) { + "
Validation in progress — results are partial$completed of $tier1Total top-level directories complete.
" + } else { '' } + + $html = @" + + + + + + Manifest Validation Report + + + +

Manifest Validation Report

+$bannerHtml +
+ Generated: $timestamp  •  + wingetdev: $escapedWingetVer  •  + Path: $escapedRoot +
+ +
+
$Total
Total
+
$Passed
Pass
+
$Warnings
Warning
+
$Failed
Fail
+
$Errors
Error
+
+ +
+ + + +
+ + + + + + + + + + +$rowsHtml + +
PathStatusOutput
+ + + + + +"@ + + $html | Out-File -FilePath $OutputPath -Encoding utf8 -Force +} + +# --------------------------------------------------------------------------- +# Validate manifests with two-tier progress +# --------------------------------------------------------------------------- +$results = [System.Collections.Generic.List[PSCustomObject]]::new() + +foreach ($tier1 in $tier1Dirs) +{ + $tier1Current++ + Write-Progress -Id 1 -Activity "Processing top-level directories" ` + -Status "($tier1Current / $tier1Total) $($tier1.Name)" ` + -PercentComplete (($tier1Current / $tier1Total) * 100) + + if ($script:completedTier1Dirs.Contains($tier1.Name)) + { + Write-Host "Skipping (already complete): $($tier1.Name)" + continue + } + + # Discover manifest directories (leaf dirs with .yaml files) under this tier-1 dir. + $yamlFiles = Get-ChildItem $tier1.FullName -Recurse -File -Filter "*.yaml" -ErrorAction SilentlyContinue + $manifestDirs = if ($yamlFiles) { $yamlFiles | Select-Object -ExpandProperty DirectoryName | Sort-Object -Unique } else { @() } + $tier2Total = $manifestDirs.Count + $tier2Current = 0 + $total += $tier2Total + + foreach ($dir in $manifestDirs) + { + $tier2Current++ + $relativePath = $dir.Substring($searchRoot.Length).TrimStart([char]'\', [char]'/') + + Write-Progress -Id 2 -ParentId 1 -Activity "Validating manifests" ` + -Status "($tier2Current / $tier2Total) $relativePath" ` + -PercentComplete (($tier2Current / $tier2Total) * 100) + + $output = & $wingetDev validate $dir 2>&1 | Out-String + $exitCode = $LASTEXITCODE + + $status = if ($output -match 'Manifest validation succeeded with warnings') { 'Warning' } + elseif ($output -match 'Manifest validation succeeded') { 'Pass' } + elseif ($output -match 'Manifest validation failed') { 'Fail' } + else { 'Error' } + + switch ($status) + { + 'Pass' { $passed++; break } + 'Warning' { $warnings++; break } + 'Fail' { $failed++; break } + 'Error' { $errors++; break } + } + + $keepInReport = $status -ne 'Pass' -and (-not $SuppressWarnings -or $status -ne 'Warning') + if ($keepInReport) + { + $results.Add([PSCustomObject]@{ + RelativePath = $relativePath + AbsolutePath = $dir + Status = $status + ExitCode = $exitCode + Output = $output.Trim() + }) + } + } + + Write-Progress -Id 2 -Completed + + $script:completedTier1Dirs.Add($tier1.Name) + Write-ValidationReport -Results $results -Total $total -Passed $passed ` + -Warnings $warnings -Failed $failed -Errors $errors +} + +Write-Progress -Id 1 -Completed + +Write-Host "" +Write-Host ("Results: Total={0} Pass={1} Warning={2} Fail={3} Error={4}" -f $total, $passed, $warnings, $failed, $errors) + +if ($Launch) +{ + Start-Process $OutputPath +} +else +{ + Write-Host "Report created at $OutputPath" +} From 6753337af72f1e0d655b48f9e9dcb437cbe5788e Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 7 Apr 2026 11:27:18 -0700 Subject: [PATCH 4/8] gp added --- doc/admx/DesktopAppInstaller.admx | 10 +++++++ doc/admx/en-US/DesktopAppInstaller.adml | 6 ++++ .../Shared/Strings/en-us/winget.resw | 3 ++ src/AppInstallerCLITests/GroupPolicy.cpp | 29 +++++++++++++++++++ src/AppInstallerCLITests/TestSettings.h | 1 + .../Manifest/ManifestValidation.cpp | 12 +++++--- src/AppInstallerSharedLib/GroupPolicy.cpp | 2 ++ .../Public/winget/GroupPolicy.h | 1 + .../Public/winget/Resources.h | 1 + 9 files changed, 61 insertions(+), 4 deletions(-) diff --git a/doc/admx/DesktopAppInstaller.admx b/doc/admx/DesktopAppInstaller.admx index a863fc5e35..6f5075d124 100644 --- a/doc/admx/DesktopAppInstaller.admx +++ b/doc/admx/DesktopAppInstaller.admx @@ -285,5 +285,15 @@ + + + + + + + + + + diff --git a/doc/admx/en-US/DesktopAppInstaller.adml b/doc/admx/en-US/DesktopAppInstaller.adml index c75651bbec..864328468f 100644 --- a/doc/admx/en-US/DesktopAppInstaller.adml +++ b/doc/admx/en-US/DesktopAppInstaller.adml @@ -153,6 +153,12 @@ If you disable or do not configure this policy, users will be able to install MS If you enable or do not configure this policy, the package URI will be evaluated with Microsoft SmartScreen before installation. This check is only done for packages that come from the internet. If you disable, Microsoft SmartScreen will not be consulted before installing a package. + Enable Network Addresses in Installer Switches for Windows Package Manager + This policy controls whether Windows Package Manager allows network addresses in installer switches. Network addresses in installer switches may pose a security risk as they are not validated by the Windows Package Manager. + + If you enable this setting, Windows Package Manager will allow network addresses in installer switches. + + If you disable or do not configure this setting, Windows Package Manager will not allow network addresses in installer switches. diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index f9e4a768f3..4baa41844d 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -3407,6 +3407,9 @@ An unlocalized JSON fragment will follow on another line. Enable Windows Package Manager MCP Server + + Enable network addresses in installer switches for Windows Package Manager + Target OS version diff --git a/src/AppInstallerCLITests/GroupPolicy.cpp b/src/AppInstallerCLITests/GroupPolicy.cpp index b96c590ba5..fc2d604f3e 100644 --- a/src/AppInstallerCLITests/GroupPolicy.cpp +++ b/src/AppInstallerCLITests/GroupPolicy.cpp @@ -404,6 +404,7 @@ TEST_CASE("GroupPolicy_AllEnabled", "[groupPolicy]") SetRegistryValue(policiesKey.get(), ConfigurationPolicyValueName, 1); SetRegistryValue(policiesKey.get(), ProxyCommandLineOptionsPolicyValueName, 1); SetRegistryValue(policiesKey.get(), McpServerValueName, 1); + SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 1); GroupPolicy groupPolicy{ policiesKey.get() }; for (const auto& policy : TogglePolicy::GetAllPolicies()) @@ -411,3 +412,31 @@ TEST_CASE("GroupPolicy_AllEnabled", "[groupPolicy]") REQUIRE(groupPolicy.GetState(policy.GetPolicy()) == PolicyState::Enabled); } } + +TEST_CASE("GroupPolicy_NetworkAddressesInSwitchesOverride", "[groupPolicy]") +{ + auto policiesKey = RegCreateVolatileTestRoot(); + + SECTION("Not configured defaults to disabled") + { + GroupPolicy groupPolicy{ policiesKey.get() }; + REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::NotConfigured); + REQUIRE_FALSE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); + } + + SECTION("Explicitly enabled") + { + SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 1); + GroupPolicy groupPolicy{ policiesKey.get() }; + REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::Enabled); + REQUIRE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); + } + + SECTION("Explicitly disabled") + { + SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 0); + GroupPolicy groupPolicy{ policiesKey.get() }; + REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::Disabled); + REQUIRE_FALSE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); + } +} diff --git a/src/AppInstallerCLITests/TestSettings.h b/src/AppInstallerCLITests/TestSettings.h index 4fe1ed718d..88da93c536 100644 --- a/src/AppInstallerCLITests/TestSettings.h +++ b/src/AppInstallerCLITests/TestSettings.h @@ -25,6 +25,7 @@ namespace TestCommon const std::wstring ConfigurationPolicyValueName = L"EnableWindowsPackageManagerConfiguration"; const std::wstring ProxyCommandLineOptionsPolicyValueName = L"EnableWindowsPackageManagerProxyCommandLineOptions"; const std::wstring McpServerValueName = L"EnableWindowsPackageManagerMcpServer"; + const std::wstring NetworkAddressesInSwitchesOverrideValueName = L"EnableNetworkAddressesInSwitchesOverride"; const std::wstring SourceUpdateIntervalPolicyValueName = L"SourceAutoUpdateInterval"; const std::wstring SourceUpdateIntervalPolicyOldValueName = L"SourceAutoUpdateIntervalInMinutes"; diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 14ef7c7d68..e635f17804 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "AppInstallerLogging.h" #include "AppInstallerMsixInfo.h" +#include "winget/GroupPolicy.h" #include "winget/MsixManifest.h" #include "winget/ManifestValidation.h" #include "winget/MsixManifestValidation.h" @@ -458,13 +459,16 @@ namespace AppInstaller::Manifest } }); - for (const auto& item : installer.Switches) + if (!Settings::GroupPolicies().IsEnabled(Settings::TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)) { - if (!item.second.empty()) + for (const auto& item : installer.Switches) { - if (ContainsNetworkAddressSignifier(item.second)) + if (!item.second.empty()) { - resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); + if (ContainsNetworkAddressSignifier(item.second)) + { + resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); + } } } } diff --git a/src/AppInstallerSharedLib/GroupPolicy.cpp b/src/AppInstallerSharedLib/GroupPolicy.cpp index af57a4469c..01c265bce5 100644 --- a/src/AppInstallerSharedLib/GroupPolicy.cpp +++ b/src/AppInstallerSharedLib/GroupPolicy.cpp @@ -330,6 +330,8 @@ namespace AppInstaller::Settings return TogglePolicy(policy, "EnableWindowsPackageManagerProxyCommandLineOptions"sv, String::PolicyEnableProxyCommandLineOptions); case TogglePolicy::Policy::McpServer: return TogglePolicy(policy, "EnableWindowsPackageManagerMcpServer"sv, String::PolicyEnableMcpServer); + case TogglePolicy::Policy::NetworkAddressesInSwitchesOverride: + return TogglePolicy(policy, "EnableNetworkAddressesInSwitchesOverride"sv, String::PolicyEnableNetworkAddressesInSwitchesOverride, false); default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h b/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h index 08e8063a03..8044f47770 100644 --- a/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h +++ b/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h @@ -49,6 +49,7 @@ namespace AppInstaller::Settings Configuration, ProxyCommandLineOptions, McpServer, + NetworkAddressesInSwitchesOverride, Max, }; diff --git a/src/AppInstallerSharedLib/Public/winget/Resources.h b/src/AppInstallerSharedLib/Public/winget/Resources.h index ff1e951a7b..2f625078ed 100644 --- a/src/AppInstallerSharedLib/Public/winget/Resources.h +++ b/src/AppInstallerSharedLib/Public/winget/Resources.h @@ -62,6 +62,7 @@ namespace AppInstaller WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableWinGetConfiguration); WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableProxyCommandLineOptions); WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableMcpServer); + WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableNetworkAddressesInSwitchesOverride); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningInvalidFieldFormat); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningInvalidFieldValue); From 08d0e3337cc6e79675690b9e196df8c97cdc9135 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 7 Apr 2026 14:06:08 -0700 Subject: [PATCH 5/8] manifest validation tests --- .../Manifest-Bad-BlockedMsiProperty.yaml | 19 ++++++ .../Manifest-Bad-InvalidMsiSwitches.yaml | 19 ++++++ ...Manifest-Bad-NetworkAddressInSwitches.yaml | 19 ++++++ src/AppInstallerCLITests/YamlManifest.cpp | 60 +++++++++++++++++++ .../Manifest/ManifestValidation.cpp | 2 +- 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/AppInstallerCLITests/TestData/Manifest-Bad-BlockedMsiProperty.yaml create mode 100644 src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidMsiSwitches.yaml create mode 100644 src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-BlockedMsiProperty.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-BlockedMsiProperty.yaml new file mode 100644 index 0000000000..cf29f69845 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-BlockedMsiProperty.yaml @@ -0,0 +1,19 @@ +# Installer with a blocked MSI property in a switch value +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.0.0.schema.json + +PackageIdentifier: AppInstallerCliTest.TestMsiInstaller +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test MSI Installer +ShortDescription: AppInstaller Test MSI Installer +Publisher: Microsoft Corporation +License: Test +InstallerType: msi +Installers: + - Architecture: x64 + InstallerUrl: https://ThisIsNotUsed + InstallerSha256: 6a2d3683fa19bf00e58e07d1313d20a5f5735ebbd6a999d33381d28740ee07ea + InstallerSwitches: + Silent: TRANSFORMS=evil.mst +ManifestType: singleton +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidMsiSwitches.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidMsiSwitches.yaml new file mode 100644 index 0000000000..6cf116e431 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-InvalidMsiSwitches.yaml @@ -0,0 +1,19 @@ +# Installer with unparseable MSI switch arguments +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.0.0.schema.json + +PackageIdentifier: AppInstallerCliTest.TestMsiInstaller +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test MSI Installer +ShortDescription: AppInstaller Test MSI Installer +Publisher: Microsoft Corporation +License: Test +InstallerType: msi +Installers: + - Architecture: x64 + InstallerUrl: https://ThisIsNotUsed + InstallerSha256: 6a2d3683fa19bf00e58e07d1313d20a5f5735ebbd6a999d33381d28740ee07ea + InstallerSwitches: + Silent: '@INVALID' +ManifestType: singleton +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml new file mode 100644 index 0000000000..cdb52024b9 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml @@ -0,0 +1,19 @@ +# Installer with a network address in a switch value +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.0.0.schema.json + +PackageIdentifier: AppInstallerCliTest.TestExeInstaller +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Exe Installer +ShortDescription: AppInstaller Test Exe Installer +Publisher: Microsoft Corporation +License: Test +InstallerType: exe +Installers: + - Architecture: x64 + InstallerUrl: https://ThisIsNotUsed + InstallerSha256: 6a2d3683fa19bf00e58e07d1313d20a5f5735ebbd6a999d33381d28740ee07ea + InstallerSwitches: + Silent: http://evil.example.com +ManifestType: singleton +ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 419d752c8d..a8c57eb878 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "TestCommon.h" +#include "TestSettings.h" #include #include #include @@ -1374,6 +1375,65 @@ TEST_CASE("WindowsFeatureNameValidation", "[ManifestValidation][111981]") ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InvalidWindowsFeatureName, "Invalid@Feature", ""); } +TEST_CASE("NetworkAddressInSwitchesValidation", "[ManifestValidation]") +{ + Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-NetworkAddressInSwitches.yaml")); + + // Network address in switch is an error regardless of fullValidation + auto errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::ContainsNetworkAddress, "http://evil.example.com", ""); + + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::ContainsNetworkAddress, "http://evil.example.com", ""); + + // Group policy override should suppress the error + { + GroupPolicyTestOverride groupPolicy; + groupPolicy.SetState(AppInstaller::Settings::TogglePolicy::Policy::NetworkAddressesInSwitchesOverride, AppInstaller::Settings::PolicyState::Enabled); + + errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 0); + + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 0); + } + + // Policy restored; error should be present again + errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 1); +} + +TEST_CASE("BlockedMsiPropertyValidation", "[ManifestValidation]") +{ + SECTION("Blocked property is detected under full validation") + { + Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-BlockedMsiProperty.yaml")); + + auto errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::BlockedMsiProperty, "TRANSFORMS", ""); + + // Not checked when fullValidation is false + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 0); + } + + SECTION("Invalid MSI switches are detected under full validation") + { + Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-InvalidMsiSwitches.yaml")); + + auto errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 1); + ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InvalidMsiSwitches); + + // Not checked when fullValidation is false + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 0); + } +} + TEST_CASE("ReadManifestAndValidateMsixInstallers_Success", "[ManifestValidation]") { TestDataFile testFile("Manifest-Good-MsixInstaller.yaml"); diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index e635f17804..fc79985697 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -90,7 +90,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::InvalidWindowsFeatureName, "The provided value is not a valid Windows feature name."sv }, { AppInstaller::Manifest::ManifestError::BlockedMsiProperty, "Contains a blocked MSI property."sv }, { AppInstaller::Manifest::ManifestError::InvalidMsiSwitches, "Contains invalid MSI switches."sv }, - { AppInstaller::Manifest::ManifestError::ContainsNetworkAddress, "Contains network address."sv }, + { AppInstaller::Manifest::ManifestError::ContainsNetworkAddress, "Installer switch contains network address."sv }, }; return ErrorIdToMessageMap; From 4d5134a896131b684f21b3441cadfa7696883b63 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 7 Apr 2026 14:06:15 -0700 Subject: [PATCH 6/8] Revert "gp added" This reverts commit 6753337af72f1e0d655b48f9e9dcb437cbe5788e. --- doc/admx/DesktopAppInstaller.admx | 10 ------- doc/admx/en-US/DesktopAppInstaller.adml | 6 ---- .../Shared/Strings/en-us/winget.resw | 3 -- src/AppInstallerCLITests/GroupPolicy.cpp | 29 ------------------- src/AppInstallerCLITests/TestSettings.h | 1 - .../Manifest/ManifestValidation.cpp | 12 +++----- src/AppInstallerSharedLib/GroupPolicy.cpp | 2 -- .../Public/winget/GroupPolicy.h | 1 - .../Public/winget/Resources.h | 1 - 9 files changed, 4 insertions(+), 61 deletions(-) diff --git a/doc/admx/DesktopAppInstaller.admx b/doc/admx/DesktopAppInstaller.admx index 6f5075d124..a863fc5e35 100644 --- a/doc/admx/DesktopAppInstaller.admx +++ b/doc/admx/DesktopAppInstaller.admx @@ -285,15 +285,5 @@ - - - - - - - - - - diff --git a/doc/admx/en-US/DesktopAppInstaller.adml b/doc/admx/en-US/DesktopAppInstaller.adml index 864328468f..c75651bbec 100644 --- a/doc/admx/en-US/DesktopAppInstaller.adml +++ b/doc/admx/en-US/DesktopAppInstaller.adml @@ -153,12 +153,6 @@ If you disable or do not configure this policy, users will be able to install MS If you enable or do not configure this policy, the package URI will be evaluated with Microsoft SmartScreen before installation. This check is only done for packages that come from the internet. If you disable, Microsoft SmartScreen will not be consulted before installing a package. - Enable Network Addresses in Installer Switches for Windows Package Manager - This policy controls whether Windows Package Manager allows network addresses in installer switches. Network addresses in installer switches may pose a security risk as they are not validated by the Windows Package Manager. - - If you enable this setting, Windows Package Manager will allow network addresses in installer switches. - - If you disable or do not configure this setting, Windows Package Manager will not allow network addresses in installer switches. diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 4baa41844d..f9e4a768f3 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -3407,9 +3407,6 @@ An unlocalized JSON fragment will follow on another line. Enable Windows Package Manager MCP Server - - Enable network addresses in installer switches for Windows Package Manager - Target OS version diff --git a/src/AppInstallerCLITests/GroupPolicy.cpp b/src/AppInstallerCLITests/GroupPolicy.cpp index fc2d604f3e..b96c590ba5 100644 --- a/src/AppInstallerCLITests/GroupPolicy.cpp +++ b/src/AppInstallerCLITests/GroupPolicy.cpp @@ -404,7 +404,6 @@ TEST_CASE("GroupPolicy_AllEnabled", "[groupPolicy]") SetRegistryValue(policiesKey.get(), ConfigurationPolicyValueName, 1); SetRegistryValue(policiesKey.get(), ProxyCommandLineOptionsPolicyValueName, 1); SetRegistryValue(policiesKey.get(), McpServerValueName, 1); - SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 1); GroupPolicy groupPolicy{ policiesKey.get() }; for (const auto& policy : TogglePolicy::GetAllPolicies()) @@ -412,31 +411,3 @@ TEST_CASE("GroupPolicy_AllEnabled", "[groupPolicy]") REQUIRE(groupPolicy.GetState(policy.GetPolicy()) == PolicyState::Enabled); } } - -TEST_CASE("GroupPolicy_NetworkAddressesInSwitchesOverride", "[groupPolicy]") -{ - auto policiesKey = RegCreateVolatileTestRoot(); - - SECTION("Not configured defaults to disabled") - { - GroupPolicy groupPolicy{ policiesKey.get() }; - REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::NotConfigured); - REQUIRE_FALSE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); - } - - SECTION("Explicitly enabled") - { - SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 1); - GroupPolicy groupPolicy{ policiesKey.get() }; - REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::Enabled); - REQUIRE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); - } - - SECTION("Explicitly disabled") - { - SetRegistryValue(policiesKey.get(), NetworkAddressesInSwitchesOverrideValueName, 0); - GroupPolicy groupPolicy{ policiesKey.get() }; - REQUIRE(groupPolicy.GetState(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride) == PolicyState::Disabled); - REQUIRE_FALSE(groupPolicy.IsEnabled(TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)); - } -} diff --git a/src/AppInstallerCLITests/TestSettings.h b/src/AppInstallerCLITests/TestSettings.h index 88da93c536..4fe1ed718d 100644 --- a/src/AppInstallerCLITests/TestSettings.h +++ b/src/AppInstallerCLITests/TestSettings.h @@ -25,7 +25,6 @@ namespace TestCommon const std::wstring ConfigurationPolicyValueName = L"EnableWindowsPackageManagerConfiguration"; const std::wstring ProxyCommandLineOptionsPolicyValueName = L"EnableWindowsPackageManagerProxyCommandLineOptions"; const std::wstring McpServerValueName = L"EnableWindowsPackageManagerMcpServer"; - const std::wstring NetworkAddressesInSwitchesOverrideValueName = L"EnableNetworkAddressesInSwitchesOverride"; const std::wstring SourceUpdateIntervalPolicyValueName = L"SourceAutoUpdateInterval"; const std::wstring SourceUpdateIntervalPolicyOldValueName = L"SourceAutoUpdateIntervalInMinutes"; diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index fc79985697..20b10d9885 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -3,7 +3,6 @@ #include "pch.h" #include "AppInstallerLogging.h" #include "AppInstallerMsixInfo.h" -#include "winget/GroupPolicy.h" #include "winget/MsixManifest.h" #include "winget/ManifestValidation.h" #include "winget/MsixManifestValidation.h" @@ -459,16 +458,13 @@ namespace AppInstaller::Manifest } }); - if (!Settings::GroupPolicies().IsEnabled(Settings::TogglePolicy::Policy::NetworkAddressesInSwitchesOverride)) + for (const auto& item : installer.Switches) { - for (const auto& item : installer.Switches) + if (!item.second.empty()) { - if (!item.second.empty()) + if (ContainsNetworkAddressSignifier(item.second)) { - if (ContainsNetworkAddressSignifier(item.second)) - { - resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); - } + resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); } } } diff --git a/src/AppInstallerSharedLib/GroupPolicy.cpp b/src/AppInstallerSharedLib/GroupPolicy.cpp index 01c265bce5..af57a4469c 100644 --- a/src/AppInstallerSharedLib/GroupPolicy.cpp +++ b/src/AppInstallerSharedLib/GroupPolicy.cpp @@ -330,8 +330,6 @@ namespace AppInstaller::Settings return TogglePolicy(policy, "EnableWindowsPackageManagerProxyCommandLineOptions"sv, String::PolicyEnableProxyCommandLineOptions); case TogglePolicy::Policy::McpServer: return TogglePolicy(policy, "EnableWindowsPackageManagerMcpServer"sv, String::PolicyEnableMcpServer); - case TogglePolicy::Policy::NetworkAddressesInSwitchesOverride: - return TogglePolicy(policy, "EnableNetworkAddressesInSwitchesOverride"sv, String::PolicyEnableNetworkAddressesInSwitchesOverride, false); default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h b/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h index 8044f47770..08e8063a03 100644 --- a/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h +++ b/src/AppInstallerSharedLib/Public/winget/GroupPolicy.h @@ -49,7 +49,6 @@ namespace AppInstaller::Settings Configuration, ProxyCommandLineOptions, McpServer, - NetworkAddressesInSwitchesOverride, Max, }; diff --git a/src/AppInstallerSharedLib/Public/winget/Resources.h b/src/AppInstallerSharedLib/Public/winget/Resources.h index 2f625078ed..ff1e951a7b 100644 --- a/src/AppInstallerSharedLib/Public/winget/Resources.h +++ b/src/AppInstallerSharedLib/Public/winget/Resources.h @@ -62,7 +62,6 @@ namespace AppInstaller WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableWinGetConfiguration); WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableProxyCommandLineOptions); WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableMcpServer); - WINGET_DEFINE_RESOURCE_STRINGID(PolicyEnableNetworkAddressesInSwitchesOverride); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningInvalidFieldFormat); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningInvalidFieldValue); From 7ab959d7ef0939425cdfc3fcfa2c9d6860f0f7a2 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 8 Apr 2026 11:14:41 -0700 Subject: [PATCH 7/8] finalize --- .../AppInstallerCLITests.vcxproj | 9 ++++ .../AppInstallerCLITests.vcxproj.filters | 9 ++++ ...Manifest-Bad-NetworkAddressInSwitches.yaml | 1 + src/AppInstallerCLITests/YamlManifest.cpp | 33 +++++------- .../Manifest/ManifestValidation.cpp | 52 +++++++++++-------- .../Manifest/YamlParser.cpp | 2 +- .../MsiExecArguments.cpp | 3 +- .../Public/winget/ManifestCommon.h | 4 ++ .../Public/winget/ManifestValidation.h | 2 +- .../Rest/Schema/1_0/RestInterface_1_0.cpp | 2 +- src/WinGetUtil/Exports.cpp | 2 + src/WinGetUtil/WinGetUtil.h | 4 ++ src/WinGetUtilInterop/Common/Enums.cs | 9 +++- 13 files changed, 84 insertions(+), 48 deletions(-) diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 48dca5dc2f..0d3a09a72c 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -1076,6 +1076,15 @@ true + + true + + + true + + + true + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 1e0c21dfe6..779b1d2490 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -1143,5 +1143,14 @@ TestData + + TestData + + + TestData + + + TestData + \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml index cdb52024b9..9cb27a6488 100644 --- a/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml +++ b/src/AppInstallerCLITests/TestData/Manifest-Bad-NetworkAddressInSwitches.yaml @@ -15,5 +15,6 @@ Installers: InstallerSha256: 6a2d3683fa19bf00e58e07d1313d20a5f5735ebbd6a999d33381d28740ee07ea InstallerSwitches: Silent: http://evil.example.com + SilentWithProgress: /normal ManifestType: singleton ManifestVersion: 1.0.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index a8c57eb878..dda1c1c9e4 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -55,6 +55,11 @@ namespace ValidateError(error, level, message, std::string(), std::string()); } + std::vector ValidateManifest(const Manifest& manifest, bool fullValidation) + { + return ValidateManifest(manifest, ManifestValidateOption{ fullValidation }); + } + struct ManifestExceptionMatcher : public Catch::Matchers::MatcherBase { ManifestExceptionMatcher(std::string expectedMessage, bool expectedWarningOnly = false) : @@ -1375,37 +1380,25 @@ TEST_CASE("WindowsFeatureNameValidation", "[ManifestValidation][111981]") ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InvalidWindowsFeatureName, "Invalid@Feature", ""); } -TEST_CASE("NetworkAddressInSwitchesValidation", "[ManifestValidation]") +TEST_CASE("NetworkAddressInSwitchesValidation", "[ManifestValidation][111981]") { Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-NetworkAddressInSwitches.yaml")); - // Network address in switch is an error regardless of fullValidation auto errors = ValidateManifest(manifest, true); REQUIRE(errors.size() == 1); - ValidateError(errors[0], ValidationError::Level::Error, ManifestError::ContainsNetworkAddress, "http://evil.example.com", ""); + ValidateError(errors[0], ValidationError::Level::Warning, ManifestError::ContainsNetworkAddress, "http://evil.example.com", ""); - errors = ValidateManifest(manifest, false); + ManifestValidateOption options{ true }; + options.ErrorOnNetworkAddressInSwitches = true; + errors = ValidateManifest(manifest, options); REQUIRE(errors.size() == 1); ValidateError(errors[0], ValidationError::Level::Error, ManifestError::ContainsNetworkAddress, "http://evil.example.com", ""); - // Group policy override should suppress the error - { - GroupPolicyTestOverride groupPolicy; - groupPolicy.SetState(AppInstaller::Settings::TogglePolicy::Policy::NetworkAddressesInSwitchesOverride, AppInstaller::Settings::PolicyState::Enabled); - - errors = ValidateManifest(manifest, true); - REQUIRE(errors.size() == 0); - - errors = ValidateManifest(manifest, false); - REQUIRE(errors.size() == 0); - } - - // Policy restored; error should be present again - errors = ValidateManifest(manifest, true); - REQUIRE(errors.size() == 1); + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 0); } -TEST_CASE("BlockedMsiPropertyValidation", "[ManifestValidation]") +TEST_CASE("BlockedMsiPropertyValidation", "[ManifestValidation][111981]") { SECTION("Blocked property is detected under full validation") { diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 20b10d9885..2f877ceb60 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -95,16 +95,20 @@ namespace AppInstaller::Manifest return ErrorIdToMessageMap; } + bool ContainsSharePathSignifier(std::string_view input) + { + return Utility::CaseInsensitiveContainsSubstring(input, "\\\\"); + } + bool ContainsNetworkAddressSignifier(std::string_view input) { return Utility::CaseInsensitiveContainsSubstring(input, "http://") || Utility::CaseInsensitiveContainsSubstring(input, "https://") || - Utility::CaseInsensitiveContainsSubstring(input, "ftp://") || - Utility::CaseInsensitiveContainsSubstring(input, "\\\\"); + Utility::CaseInsensitiveContainsSubstring(input, "ftp://"); } } - std::vector ValidateManifest(const Manifest& manifest, bool fullValidation) + std::vector ValidateManifest(const Manifest& manifest, const ManifestValidateOption& options) { std::vector resultErrors; @@ -128,7 +132,7 @@ namespace AppInstaller::Manifest resultErrors.emplace_back(ManifestError::InvalidFieldValue, "PackageVersion", manifest.Version); } - auto defaultLocErrors = ValidateManifestLocalization(manifest.DefaultLocalization, !fullValidation); + auto defaultLocErrors = ValidateManifestLocalization(manifest.DefaultLocalization, !options.FullValidation); std::move(defaultLocErrors.begin(), defaultLocErrors.end(), std::inserter(resultErrors, resultErrors.end())); // Comparison function to check duplicate installer entry. {installerType, arch, language and scope} combination is the key. @@ -179,7 +183,7 @@ namespace AppInstaller::Manifest for (auto const& installer : manifest.Installers) { // If not full validation, for future compatibility, skip validating unknown installers. - if (installer.EffectiveInstallerType() == InstallerTypeEnum::Unknown && !fullValidation) + if (installer.EffectiveInstallerType() == InstallerTypeEnum::Unknown && !options.FullValidation) { continue; } @@ -228,7 +232,7 @@ namespace AppInstaller::Manifest if (installer.EffectiveInstallerType() == InstallerTypeEnum::MSStore) { - if (fullValidation) + if (options.FullValidation) { // MSStore type is not supported in community repo resultErrors.emplace_back( @@ -260,7 +264,7 @@ namespace AppInstaller::Manifest // Ensure that each URL has a one to one mapping with a Sha256 and // warn if a Sha256 has a one to many mapping with a URL - if (fullValidation && !installer.Url.empty() && !installer.Sha256.empty()) + if (options.FullValidation && !installer.Url.empty() && !installer.Sha256.empty()) { std::string checksum = Utility::SHA256::ConvertToString(installer.Sha256); std::string url = installer.Url; @@ -364,7 +368,7 @@ namespace AppInstaller::Manifest } // If running full validation, check filetype - if (fullValidation) + if (options.FullValidation) { if (isPortable) { @@ -438,7 +442,7 @@ namespace AppInstaller::Manifest // Check AuthInfo validity. For full validation (community repo), authentication type must be none. if (installer.AuthInfo.Type != Authentication::AuthenticationType::None) { - if (fullValidation) + if (options.FullValidation) { // Authentication is not supported (must be none) in community repo. resultErrors.emplace_back(ManifestError::FieldNotSupported, "Authentication"); @@ -458,18 +462,7 @@ namespace AppInstaller::Manifest } }); - for (const auto& item : installer.Switches) - { - if (!item.second.empty()) - { - if (ContainsNetworkAddressSignifier(item.second)) - { - resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); - } - } - } - - if (fullValidation) + if (options.FullValidation) { for (const auto& container : installer.DesiredStateConfiguration) { @@ -502,13 +495,28 @@ namespace AppInstaller::Manifest resultErrors.emplace_back(ManifestError::InvalidMsiSwitches); } } + + for (const auto& item : installer.Switches) + { + if (!item.second.empty()) + { + if (ContainsSharePathSignifier(item.second)) + { + resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second); + } + else if (ContainsNetworkAddressSignifier(item.second)) + { + resultErrors.emplace_back(ManifestError::ContainsNetworkAddress, item.second, options.ErrorOnNetworkAddressInSwitches ? ValidationError::Level::Error : ValidationError::Level::Warning); + } + } + } } } // Validate localizations for (auto const& localization : manifest.Localizations) { - auto locErrors = ValidateManifestLocalization(localization, !fullValidation); + auto locErrors = ValidateManifestLocalization(localization, !options.FullValidation); std::move(locErrors.begin(), locErrors.end(), std::inserter(resultErrors, resultErrors.end())); } diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp index 0e034c469a..d61d9efb78 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp @@ -477,7 +477,7 @@ namespace AppInstaller::Manifest::YamlParser // Extra semantic validations after basic validation and field population if (validateOption.FullValidation) { - errors = ValidateManifest(manifest); + errors = ValidateManifest(manifest, validateOption); std::move(errors.begin(), errors.end(), std::inserter(resultErrors, resultErrors.end())); // Validate the schema header for manifest version 1.7 and above diff --git a/src/AppInstallerCommonCore/MsiExecArguments.cpp b/src/AppInstallerCommonCore/MsiExecArguments.cpp index 12ad82992c..cc845f89dc 100644 --- a/src/AppInstallerCommonCore/MsiExecArguments.cpp +++ b/src/AppInstallerCommonCore/MsiExecArguments.cpp @@ -563,7 +563,8 @@ namespace AppInstaller::Msi for (const auto& blockedName : { "transforms", "patch", - // TODO: There are more + "msinewinstance", + "adminproperties", }) { if (blockedName == lowerName) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 1962b49ec8..50a2fa2902 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -64,9 +64,13 @@ namespace AppInstaller::Manifest struct ManifestValidateOption { + ManifestValidateOption() = default; + explicit ManifestValidateOption(bool fullValidation) : FullValidation(fullValidation) {} + bool SchemaValidationOnly = false; bool ErrorOnVerifiedPublisherFields = false; bool InstallerValidation = false; + bool ErrorOnNetworkAddressInSwitches = false; // Options not exposed in winget util bool FullValidation = false; diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h index 7b5d8470ae..b6595571f9 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -257,7 +257,7 @@ namespace AppInstaller::Manifest }; // fullValidation: bool to set if manifest validation should perform extra validation that is not required for reading a manifest. - std::vector ValidateManifest(const Manifest& manifest, bool fullValidation = true); + std::vector ValidateManifest(const Manifest& manifest, const ManifestValidateOption& options); std::vector ValidateManifestLocalization(const ManifestLocalization& localization, bool treatErrorAsWarning = false); std::vector ValidateManifestInstallers(const Manifest& manifest, bool treatErrorAsWarning = false); } diff --git a/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp b/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp index 1095dc4b50..33aa8b10ea 100644 --- a/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp @@ -256,7 +256,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0 for (auto& manifestItem : manifests) { std::vector validationErrors = - AppInstaller::Manifest::ValidateManifest(manifestItem, false); + AppInstaller::Manifest::ValidateManifest(manifestItem, AppInstaller::Manifest::ManifestValidateOption{ false }); int errors = 0; for (auto& error : validationErrors) diff --git a/src/WinGetUtil/Exports.cpp b/src/WinGetUtil/Exports.cpp index f713a13544..d62de90b43 100644 --- a/src/WinGetUtil/Exports.cpp +++ b/src/WinGetUtil/Exports.cpp @@ -303,6 +303,7 @@ extern "C" validateOption.SchemaValidationOnly = WI_IsFlagSet(option, WinGetValidateManifestOption::SchemaValidationOnly); validateOption.ErrorOnVerifiedPublisherFields = WI_IsFlagSet(option, WinGetValidateManifestOption::ErrorOnVerifiedPublisherFields); validateOption.InstallerValidation = WI_IsFlagSet(option, WinGetValidateManifestOption::InstallerValidations); + validateOption.ErrorOnNetworkAddressInSwitches = WI_IsFlagSet(option, WinGetValidateManifestOption::ErrorOnNetworkAddressInSwitches); (void)YamlParser::CreateFromPath(inputPath, validateOption, mergedManifestPath ? mergedManifestPath : L""); @@ -348,6 +349,7 @@ extern "C" validateOption.ThrowOnWarning = true; validateOption.SchemaValidationOnly = WI_IsFlagClear(option, WinGetCreateManifestOption::SchemaAndSemanticValidation); validateOption.ErrorOnVerifiedPublisherFields = WI_IsFlagSet(option, WinGetCreateManifestOption::ReturnErrorOnVerifiedPublisherFields); + validateOption.ErrorOnNetworkAddressInSwitches = WI_IsFlagSet(option, WinGetCreateManifestOption::ReturnErrorOnNetworkAddressInSwitches); } if (WI_IsFlagSet(option, WinGetCreateManifestOption::AllowShadowManifest)) diff --git a/src/WinGetUtil/WinGetUtil.h b/src/WinGetUtil/WinGetUtil.h index fe60ca77ed..bc3d34e2d1 100644 --- a/src/WinGetUtil/WinGetUtil.h +++ b/src/WinGetUtil/WinGetUtil.h @@ -26,6 +26,7 @@ extern "C" SchemaValidationOnly = 0x1, ErrorOnVerifiedPublisherFields = 0x2, InstallerValidations = 0x4, + ErrorOnNetworkAddressInSwitches = 0x8, }; DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestOption); @@ -45,6 +46,9 @@ extern "C" // Return error on manifest fields that require verified publishers, used during semantic validation ReturnErrorOnVerifiedPublisherFields = 0x1000, + + // Return error if a network address is present in installer switches. + ReturnErrorOnNetworkAddressInSwitches = 0x2000, }; DEFINE_ENUM_FLAG_OPERATORS(WinGetCreateManifestOption); diff --git a/src/WinGetUtilInterop/Common/Enums.cs b/src/WinGetUtilInterop/Common/Enums.cs index 77e90880a0..ad309a793a 100644 --- a/src/WinGetUtilInterop/Common/Enums.cs +++ b/src/WinGetUtilInterop/Common/Enums.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- // // Copyright (c) Microsoft Corporation. Licensed under the MIT License. // @@ -41,7 +41,12 @@ public enum WinGetCreateManifestOption /// /// Return error on manifest fields that require verified publishers, used during semantic validation /// - ReturnErrorOnVerifiedPublisherFields = 0x1000, + ReturnErrorOnVerifiedPublisherFields = 0x1000, + + /// + /// Return error if a network address is present in installer switches. + /// + ReturnErrorOnNetworkAddressInSwitches = 0x2000, } /// From ceeef0ad36e83ce20d92d7456d201454fe3a0bc5 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 22 Apr 2026 11:39:00 -0700 Subject: [PATCH 8/8] spelling --- .github/actions/spelling/expect.txt | 5 +++++ src/WinGetServer/WinGetServerManualActivation_Client.cpp | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 2fb073f88a..437d21d946 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -7,6 +7,7 @@ ACCESSDENIED ACCESSTOKEN acl adjacents +adminproperties adml admx AFAIK @@ -98,6 +99,7 @@ COMGLB commandline compressapi concurrencysal +Consolas constexpr contactsupport contentfiles @@ -279,6 +281,7 @@ Kaido KNOWNFOLDERID kool ktf +LASTEXITCODE LCID learnxinyminutes LEBOM @@ -347,6 +350,7 @@ msdownload msft msftrubengu MSIHASH +msinewinstance MSIXHASH MSIXSTRM msstore @@ -510,6 +514,7 @@ sddl secureobject securestring seekp +Segoe seof servercert servercertificate diff --git a/src/WinGetServer/WinGetServerManualActivation_Client.cpp b/src/WinGetServer/WinGetServerManualActivation_Client.cpp index c5b00b4495..b6dde2c96a 100644 --- a/src/WinGetServer/WinGetServerManualActivation_Client.cpp +++ b/src/WinGetServer/WinGetServerManualActivation_Client.cpp @@ -71,7 +71,7 @@ struct ServerProcessLauncher #ifndef USE_PROD_WINGET_SERVER // The feature that allows directly launching a packaged process as long as it has a matching alias - // requires a failure to trigger, and the dev package is not ACL'd to force this to happen. Attempting + // requires a failure to trigger, and the dev package ACL does not force this to happen. Attempting // to use the other code path results in an unpackaged server, causing other issues. // We run the product code above to ensure that it is functioning properly, but then replace it with // the path of the alias.