Improved manifest validations for MSI and Windows Feature names (#6170)

CP of #6169 onto main branch.

## Change
Adds some additional validation for MSI switches and Windows Feature
names:

1. MSI switches are checked during full manifest validation in the same
way that they are checked when we attempt to use the MSI APIs rather
than msiexec. The API is used for fully all silent installs of MSIs, so
we now ensure that those will work (6 total manifests in winget-pkgs
were found that needed fixes).
2. Windows Feature names in dependencies are validated to be {
alphanumeric, `-`, `_` }. This is done both during full manifest
validation and at runtime.

A PowerShell script
(tools/ManifestValidation/Invoke-ManifestValidation.ps1) is added that
will run `wingetdev validate` against a directory recursively and
generate a report. There is support for resuming in the middle of the
run. This would probably perform much better if it were updated to use
the WinGetUtil API, but that can be a future project.

Author: JohnMcPMS

.github/actions/spelling/expect.txt

@@ -7,6 +7,7 @@ ACCESSDENIED
 ACCESSTOKEN
 ACL'd
 adjacents
+adminproperties
 adml
 admx
 AFAIK
@@ -98,6 +99,7 @@ COMGLB
 commandline
 compressapi
 concurrencysal
+Consolas
 constexpr
 contactsupport
 contentfiles
@@ -281,6 +283,7 @@ Kaido
 KNOWNFOLDERID
 kool
 ktf
+LASTEXITCODE
 LCID
 learnxinyminutes
 LEBOM
@@ -349,6 +352,7 @@ msdownload
 msft
 msftrubengu
 MSIHASH
+msinewinstance
 MSIXHASH
 MSIXSTRM
 msstore
@@ -513,6 +517,7 @@ sddl
 secureobject
 securestring
 seekp
+Segoe
 seof
 servercert
 servercertificate

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");

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)

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<Execution::Data::OperationReturnCode>(static_cast<DWORD>(E_INVALIDARG));
+            return;
+        }
+
         Utility::LocIndView locIndFeatureName{ m_featureName };
 
         std::optional<DWORD> doesFeatureExistResult = DoesWindowsFeatureExist(context, m_featureName);

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -1081,6 +1081,18 @@
     <CopyFileToFolders Include="TestData\ManifestV1_28-PowerShellDSC.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InvalidWindowsFeatureName.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-BlockedMsiProperty.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InvalidMsiSwitches.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-NetworkAddressInSwitches.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj">

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -1152,5 +1152,17 @@
     <CopyFileToFolders Include="TestData\ManifestV1_28-Singleton.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InvalidWindowsFeatureName.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-BlockedMsiProperty.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-InvalidMsiSwitches.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-NetworkAddressInSwitches.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
   </ItemGroup>
 </Project>

src/AppInstallerCLITests/InstallDependenciesFlow.cpp

@@ -3,6 +3,7 @@
 #include "pch.h"
 #include "WorkflowCommon.h"
 #include "DependenciesTestSource.h"
+#include <AppInstallerRuntime.h>
 #include <Commands/InstallCommand.h>
 #include <Commands/COMCommand.h>
 #include <Workflows/DependenciesFlow.h>
@@ -328,6 +329,42 @@ TEST_CASE("InstallFlow_Dependencies_COM", "[InstallFlow][workflow][dependencies]
     REQUIRE(installationOrder.at(2) == "AppInstallerCliTest.TestExeInstaller.MultipleDependencies");
 }
 
+void InstallFlow_Dependencies_WindowsFeaturesArgument_Generic(std::string_view featureName)
+{
+    std::ostringstream installOutput;
+    TestContext context{ installOutput, std::cin };
+
+    context << ShellExecuteEnableWindowsFeature(featureName);
+
+    INFO(installOutput.str());
+
+    REQUIRE(context.Contains(Execution::Data::OperationReturnCode));
+    REQUIRE(context.Get<Execution::Data::OperationReturnCode>() == 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

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));
+    }
+}

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

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