commit 761af8d610a8259fd7e79f371e8390a494d55045 parent af81f1d9a02a58de4b5394df8db44ce33ef49e93 Author: Kaleb Luedtke <trenlymc@gmail.com> Date: Thu, 20 Feb 2025 18:50:28 -0600 Validate URLs map to unique hashes (#5233) <!-- To check a checkbox place an "x" between the brackets. e.g: [x] --> - [x] I have signed the [Contributor License Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs). - [x] This pull request is related to an issue. - Resolves #2852 ## Description: This PR adds validation for URL to SHA256 mappings. It is presumed that a single URL should only return a single SHA256, so if a manifest has different SHA256 for the same URL, this will now throw a validation *error*. This will also help reduce potential load on the Validation Pipelines at winget-pkgs as the `InstallerSha256` mismatch will be caught during manifest validation and the validation will exit before it proceeds to installer validation. Additionally, as noted in the initial issue, it would be unlikely (but possible) for two different URLS to point to the same SHA256. Validation was added for this case, but will only throw a validation *warning*. It is also necessary for this to be a warning so that this change does not become a breaking change.  ----- ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/5233) --------- Co-authored-by: Muhammad Danish <mdanishkhdev@gmail.com> Co-authored-by: JohnMcPMS <johnmcp@microsoft.com> Diffstat:
10 files changed, 88 insertions(+), 5 deletions(-)
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -539,12 +539,18 @@ <CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateReturnCode-SuccessCodes.yaml"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateSha256.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-IdInvalid.yaml"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-IdMissing.yaml"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSha256.yaml"> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-InstallersMissing.yaml"> <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> @@ -1092,4 +1098,4 @@ <Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props'))" /> <Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets'))" /> </Target> -</Project>- \ No newline at end of file +</Project> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -426,12 +426,18 @@ <CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateReturnCode-SuccessCodes.yaml"> <Filter>TestData</Filter> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-DuplicateSha256.yaml"> + <Filter>TestData</Filter> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-IdInvalid.yaml"> <Filter>TestData</Filter> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-IdMissing.yaml"> <Filter>TestData</Filter> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSha256.yaml"> + <Filter>TestData</Filter> + </CopyFileToFolders> <CopyFileToFolders Include="TestData\Manifest-Bad-InstallersMissing.yaml"> <Filter>TestData</Filter> </CopyFileToFolders> diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-DuplicateSha256.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-DuplicateSha256.yaml @@ -0,0 +1,17 @@ +# Bad manifest. Different URLs point to two the same InstallerSha256 +Id: microsoft.msixsdk +Name: MSIX SDK +Version: 1.7.32 +Publisher: Microsoft +InstallerType: Msi +License: Test +Installers: + - Arch: x86 + Url: https://ThisIsNotUsed + Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD + Language: en-US + - Arch: x86 + Url: https://ThisIsNotUsed2 + Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD + Language: es-MX +ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSha256.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-InconsistentSha256.yaml @@ -0,0 +1,20 @@ +# Bad manifest. Same URL points to two different InstallerSha256 +PackageIdentifier: AppInstallerCliTest.TestPortableInstaller +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Portable Exe +Publisher: Microsoft Corporation +AppMoniker: AICLITestPortable +License: Test +ProductCode: AppInstallerCliTest.TestPortableInstaller__TestSource +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: portable + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B + - Architecture: x64 + InstallerUrl: https://ThisIsNotUsed + InstallerType: portable + InstallerSha256: A111111111111111111111111111111111111111111111111111111111111111 +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-InstallerUniqueness-DiffScope.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-InstallerUniqueness-DiffScope.yaml @@ -12,6 +12,6 @@ Installers: Scope: user - Arch: x86 Url: https://ThisIsNotUsed2 - Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD + Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCE Scope: machine ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-Installeruniqueness-DefaultLang.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-Installeruniqueness-DefaultLang.yaml @@ -11,6 +11,6 @@ Installers: Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD - Arch: x86 Url: https://ThisIsNotUsed2 - Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD + Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCE Language: en-MX ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLITests/TestData/Manifest-Good-Installeruniqueness-DiffLangs.yaml b/src/AppInstallerCLITests/TestData/Manifest-Good-Installeruniqueness-DiffLangs.yaml @@ -12,6 +12,6 @@ Installers: Language: en-US - Arch: x86 Url: https://ThisIsNotUsed2 - Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCD + Sha256: 98B67758CEAFFCBB3FE47838FD0A8D7BD581C2650842D6B2B0E0D49A23270CCE Language: es-MX ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp @@ -791,8 +791,10 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]") { "Manifest-Bad-DuplicateKey-DifferentCase-lower.yaml", "Duplicate field found in the manifest." }, { "Manifest-Bad-DuplicateReturnCode-ExpectedCodes.yaml", "Duplicate installer return code found." }, { "Manifest-Bad-DuplicateReturnCode-SuccessCodes.yaml", "Duplicate installer return code found." }, + { "Manifest-Bad-DuplicateSha256.yaml", "Multiple Installer URLs found with the same InstallerSha256. Please ensure the accuracy of the URLs.", true }, { "Manifest-Bad-IdInvalid.yaml", "Failed to validate against schema associated with property name 'Id'" }, { "Manifest-Bad-IdMissing.yaml", "Missing required property 'Id'" }, + { "Manifest-Bad-InconsistentSha256.yaml", "The values of InstallerSha256 do not match for all instances of the same InstallerUrl." }, { "Manifest-Bad-InstallersMissing.yaml", "Missing required property 'Installers'" }, { "Manifest-Bad-InstallerTypeExe-NoSilent.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true }, { "Manifest-Bad-InstallerTypeExe-NoSilentRoot.yaml", "Silent and SilentWithProgress switches are not specified for InstallerType exe.", true }, diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -27,6 +27,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::FieldNotSupported, "Field is not supported."sv }, { AppInstaller::Manifest::ManifestError::FieldValueNotSupported, "Field value is not supported."sv }, { AppInstaller::Manifest::ManifestError::DuplicateInstallerEntry, "Duplicate installer entry found."sv }, + { AppInstaller::Manifest::ManifestError::DuplicateInstallerHash, "Multiple Installer URLs found with the same InstallerSha256. Please ensure the accuracy of the URLs."sv }, { AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotSupportPackageFamilyName, "The specified installer type does not support PackageFamilyName."sv }, { AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotSupportProductCode, "The specified installer type does not support ProductCode."sv }, { AppInstaller::Manifest::ManifestError::InstallerTypeDoesNotWriteAppsAndFeaturesEntry, "The specified installer type does not write to Apps and Features entry."sv }, @@ -37,6 +38,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::DuplicateMultiFileManifestType, "The multi file manifest should contain only one file with the particular ManifestType."sv }, { AppInstaller::Manifest::ManifestError::DuplicateMultiFileManifestLocale, "The multi file manifest contains duplicate PackageLocale."sv }, { AppInstaller::Manifest::ManifestError::UnsupportedMultiFileManifestType, "The multi file manifest should not contain file with the particular ManifestType."sv }, + { AppInstaller::Manifest::ManifestError::InconsistentInstallerHash, "The values of InstallerSha256 do not match for all instances of the same InstallerUrl."sv }, { AppInstaller::Manifest::ManifestError::InconsistentMultiFileManifestDefaultLocale, "DefaultLocale value in version manifest does not match PackageLocale value in defaultLocale manifest."sv }, { AppInstaller::Manifest::ManifestError::FieldFailedToProcess, "Failed to process field."sv }, { AppInstaller::Manifest::ManifestError::InvalidBcp47Value, "The locale value is not a well formed bcp47 language tag."sv }, @@ -141,6 +143,10 @@ namespace AppInstaller::Manifest std::set<ManifestInstaller, decltype(installerCmp)> installerSet(installerCmp); bool duplicateInstallerFound = false; + // Set up maps for checking uniqueness across hash <-> url pairs + std::unordered_map<std::string, std::string> urlToChecksum; + std::unordered_map<std::string, std::string> checksumToUrl; + // Validate installers for (auto const& installer : manifest.Installers) { @@ -223,6 +229,31 @@ namespace AppInstaller::Manifest { resultErrors.emplace_back(ManifestError::FieldNotSupported, "ProductId"); } + + // 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()) + { + std::string checksum = Utility::SHA256::ConvertToString(installer.Sha256); + std::string url = installer.Url; + + auto [urlIterator, urlInserted] = urlToChecksum.try_emplace(url, checksum); + auto [checksumIterator, checksumInserted] = checksumToUrl.try_emplace(checksum, url); + + if (!urlInserted && urlIterator->second != checksum) + { + // If the URL was not inserted, and the value in the map does not match the current Sha256, then + // a single URL corresponds to multiple SHA256 and an error should be thrown + resultErrors.emplace_back(ManifestError::InconsistentInstallerHash, "InstallerUrl", url); + } + + if (!checksumInserted && checksumIterator->second != url) + { + // If the SHA256 was not inserted, and the value in the map does not match the current URL, then + // a single SHA256 corresponds to multiple URLS and a warning should be thrown + resultErrors.emplace_back(ManifestError::DuplicateInstallerHash, "InstallerSha256", checksum, ValidationError::Level::Warning); + } + } } if (installer.EffectiveInstallerType() == InstallerTypeEnum::Exe && diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -28,6 +28,7 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(DuplicateMultiFileManifestLocale); WINGET_DEFINE_RESOURCE_STRINGID(DuplicateMultiFileManifestType); WINGET_DEFINE_RESOURCE_STRINGID(DuplicateInstallerEntry); + WINGET_DEFINE_RESOURCE_STRINGID(DuplicateInstallerHash); WINGET_DEFINE_RESOURCE_STRINGID(DuplicateReturnCodeEntry); WINGET_DEFINE_RESOURCE_STRINGID(ExceededAppsAndFeaturesEntryLimit); WINGET_DEFINE_RESOURCE_STRINGID(ExceededCommandsLimit); @@ -42,6 +43,7 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(FieldValueNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(FoundDependencyLoop); WINGET_DEFINE_RESOURCE_STRINGID(IncompleteMultiFileManifest); + WINGET_DEFINE_RESOURCE_STRINGID(InconsistentInstallerHash); WINGET_DEFINE_RESOURCE_STRINGID(InconsistentMultiFileManifestDefaultLocale); WINGET_DEFINE_RESOURCE_STRINGID(InconsistentMultiFileManifestFieldValue); WINGET_DEFINE_RESOURCE_STRINGID(InstallerFailedToProcess);