commit bdb5faac4f1a5cb32baed8103c33ad55ecc8c151
parent f0d8fa996651778fba3f50c20c1054fb0ce722cb
Author: Kaleb Luedtke <trenlymc@gmail.com>
Date: Wed, 12 Mar 2025 16:00:54 -0500
Add validation option for portable installer type in archives (#5237)
## Description
For the `portable` installer type, the `winget-pkgs` repository has a
policy of not allowing scripts as installers. This includes `.bat`,
`.ps1`, and `.cmd` files.
In addition to the policy at `winget-pkgs`, the client itself is not
designed to handle portable files that are not `.exe` types. Issues have
been filed asking for WinGet to support these in the future, but it
currently does not -
* https://github.com/microsoft/winget-cli/issues/5083
* https://github.com/microsoft/winget-cli/issues/5086
* https://github.com/microsoft/winget-cli/issues/5087
This PR adds a validation option which validates the `RelativeFilePath`
within a zip file is always a `.exe`, but can be extended to include
additional formats as the requests above are addressed. This was added
as a validation option (similar to the Schema Header Validation) as
private repositories may want to not have the file type restrictions
that `winget-pkgs` does - as described in these issues -
* https://github.com/microsoft/winget-cli/issues/5084
* https://github.com/microsoft/winget-cli/issues/5085
This should allow for the validation pipeline at `winget-pkgs` to
automatically reject manifests that contain `NestedInstallerFiles` that
are not supported, while allowing maintainers of private repositories to
suppress the restriction by turning the validation option to false.
Diffstat:
8 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -1040,6 +1040,15 @@
<CopyFileToFolders Include="TestData\ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Good-InstallerTypeZip-PortableExe.yaml">
+ <DeploymentContent>true</DeploymentContent>
+ </CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml">
+ <DeploymentContent>true</DeploymentContent>
+ </CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml">
+ <DeploymentContent>true</DeploymentContent>
+ </CopyFileToFolders>
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj">
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup>
<Filter Include="Source Files">
@@ -1071,5 +1071,14 @@
<CopyFileToFolders Include="TestData\ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Good-InstallerTypeZip-PortableExe.yaml">
+ <Filter>TestData</Filter>
+ </CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml">
+ <Filter>TestData</Filter>
+ </CopyFileToFolders>
+ <CopyFileToFolders Include="TestData\Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml">
+ <Filter>TestData</Filter>
+ </CopyFileToFolders>
</ItemGroup>
-</Project>-
\ No newline at end of file
+</Project>
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml
@@ -0,0 +1,19 @@
+# Bad manifest. Installer type zip with NestedInstallerType portable should only allow .exe files
+PackageIdentifier: microsoft.msixsdk
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Installer
+Publisher: Microsoft Corporation
+Moniker: AICLITestExe
+License: Test
+ShortDescription: Test installer for zip without nestedInstallers specified
+Installers:
+ - Architecture: x64
+ InstallerUrl: https://ThisIsNotUsed
+ InstallerType: zip
+ NestedInstallerType: portable
+ InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+ NestedInstallerFiles:
+ - RelativeFilePath: ScriptedApplication.bat
+ManifestType: singleton
+ManifestVersion: 1.9.0
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml
@@ -0,0 +1,19 @@
+# Bad manifest. Installer type zip with NestedInstallerType portable should only allow .exe files
+PackageIdentifier: microsoft.msixsdk
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Installer
+Publisher: Microsoft Corporation
+Moniker: AICLITestExe
+License: Test
+ShortDescription: Test installer for zip without nestedInstallers specified
+InstallerType: zip
+NestedInstallerType: portable
+NestedInstallerFiles:
+ - RelativeFilePath: ScriptedApplication.bat
+Installers:
+ - Architecture: x64
+ InstallerUrl: https://ThisIsNotUsed
+ InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+ManifestType: singleton
+ManifestVersion: 1.9.0
diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-InstallerTypeZip-PortableExe.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-InstallerTypeZip-PortableExe.yaml
@@ -0,0 +1,19 @@
+# Bad manifest. Installer type zip with NestedInstallerType portable should only allow .exe files
+PackageIdentifier: microsoft.msixsdk
+PackageVersion: 1.0.0.0
+PackageLocale: en-US
+PackageName: AppInstaller Test Installer
+Publisher: Microsoft Corporation
+Moniker: AICLITestExe
+License: Test
+ShortDescription: Test installer for zip without nestedInstallers specified
+Installers:
+ - Architecture: x64
+ InstallerUrl: https://ThisIsNotUsed
+ InstallerType: zip
+ NestedInstallerType: portable
+ InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+ NestedInstallerFiles:
+ - RelativeFilePath: GoodApplication.exe
+ManifestType: singleton
+ManifestVersion: 1.9.0
diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp
@@ -768,6 +768,7 @@ TEST_CASE("ReadGoodManifests", "[ManifestValidation]")
{ "Manifest-Good-Minimum-InstallerType.yaml" },
{ "Manifest-Good-Switches.yaml" },
{ "Manifest-Good-DefaultExpectedReturnCodeInInstallerSuccessCodes.yaml" },
+ { "Manifest-Good-InstallerTypeZip-PortableExe.yaml" },
};
for (auto const& testCase : TestCases)
@@ -839,6 +840,8 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{ "InstallFlowTest_LicenseAgreement.yaml", "Field usage requires verified publishers. [Agreement]", false, GetTestManifestValidateOption(false, true) },
{ "Manifest-Bad-ApproximateVersionInPackageVersion.yaml", "Approximate version not allowed. [PackageVersion]" },
{ "Manifest-Bad-ApproximateVersionInArpVersion.yaml", "Approximate version not allowed. [DisplayVersion]" },
+ { "Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml", "The file type of the referenced file is not allowed. [RelativeFilePath] Value: ScriptedApplication.bat" },
+ { "Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml", "The file type of the referenced file is not allowed. [RelativeFilePath] Value: ScriptedApplication.bat" },
};
for (auto const& testCase : TestCases)
@@ -1619,6 +1622,28 @@ TEST_CASE("ManifestLocalizationValidation", "[ManifestValidation]")
REQUIRE(errors.at(0).ErrorLevel == ValidationError::Level::Warning);
}
+TEST_CASE("PortableFileTypeValidation", "[ManifestValidation]")
+{
+ Manifest installerManifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-InstallerTypeZip-PortableNotExe.yaml"));
+ Manifest rootManifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-InstallerTypeZip-PortableNotExe_Root.yaml"));
+
+ // Regular validation should detect as error
+ auto errors = ValidateManifest(installerManifest, true);
+ REQUIRE(errors.size() == 1);
+ REQUIRE(errors.at(0).ErrorLevel == ValidationError::Level::Error);
+
+ errors = ValidateManifest(rootManifest, true);
+ REQUIRE(errors.size() == 1);
+ REQUIRE(errors.at(0).ErrorLevel == ValidationError::Level::Error);
+
+ // Should not error when full validation is set to false
+ errors = ValidateManifest(installerManifest, false);
+ REQUIRE(errors.size() == 0);
+
+ errors = ValidateManifest(rootManifest, 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
@@ -71,6 +71,7 @@ namespace AppInstaller::Manifest
{ AppInstaller::Manifest::ManifestError::SchemaHeaderManifestTypeMismatch , "The manifest type in the schema header does not match the ManifestType property value in the manifest."sv },
{ AppInstaller::Manifest::ManifestError::SchemaHeaderManifestVersionMismatch, "The manifest version in the schema header does not match the ManifestVersion property value in the manifest."sv },
{ 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 },
};
return ErrorIdToMessageMap;
@@ -301,6 +302,7 @@ namespace AppInstaller::Manifest
std::set<std::string> commandAliasSet;
std::set<std::string> relativeFilePathSet;
+ bool isPortable = installer.NestedInstallerType == InstallerTypeEnum::Portable;
for (const auto& nestedInstallerFile : installer.NestedInstallerFiles)
{
@@ -331,6 +333,15 @@ namespace AppInstaller::Manifest
resultErrors.emplace_back(ManifestError::DuplicatePortableCommandAlias, "PortableCommandAlias");
break;
}
+
+ // If running full validation, check filetype
+ if (fullValidation && isPortable)
+ {
+ if (fullPath.has_extension() && s_AllowedPortableFiletypes.find(fullPath.extension()) == s_AllowedPortableFiletypes.end())
+ {
+ resultErrors.emplace_back(ManifestError::InvalidPortableFiletype, "RelativeFilePath", nestedInstallerFile.RelativeFilePath);
+ }
+ }
}
}
diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h
@@ -7,6 +7,7 @@
#include <wil/result.h>
#include <functional>
+#include <unordered_set>
namespace YAML { class Node; }
@@ -73,6 +74,7 @@ namespace AppInstaller::Manifest
WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderManifestTypeMismatch);
WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderManifestVersionMismatch);
WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderUrlPatternMismatch);
+ WINGET_DEFINE_RESOURCE_STRINGID(InvalidPortableFiletype);
}
struct ValidationError
@@ -249,6 +251,8 @@ namespace AppInstaller::Manifest
bool m_warningOnly;
};
+ static const std::unordered_set<std::filesystem::path> s_AllowedPortableFiletypes = { L".exe" };
+
// fullValidation: bool to set if manifest validation should perform extra validation that is not required for reading a manifest.
std::vector<ValidationError> ValidateManifest(const Manifest& manifest, bool fullValidation = true);
std::vector<ValidationError> ValidateManifestLocalization(const ManifestLocalization& localization, bool treatErrorAsWarning = false);
