From bf6d5df0ec4c0177a59e32c20f2c7128edb679d2 Mon Sep 17 00:00:00 2001 From: Michael Kuhn Date: Mon, 23 Oct 2023 20:22:39 +0200 Subject: audit: add check for GitLab patches (#40656) GitLab's .patch URLs only provide abbreviated hashes, while .diff URLs provide full hashes. There does not seem to be a parameter to force .patch URLs to also return full hashes, so we should make sure to use the .diff ones. --- lib/spack/spack/audit.py | 43 +++++++++++++++------- lib/spack/spack/test/audit.py | 4 ++ .../packages/invalid-gitlab-patch-url/package.py | 20 ++++++++++ .../invalid-selfhosted-gitlab-patch-url/package.py | 20 ++++++++++ 4 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py create mode 100644 var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 176c45487f..8b13ffc7cf 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -307,10 +307,17 @@ def _check_build_test_callbacks(pkgs, error_cls): @package_directives def _check_patch_urls(pkgs, error_cls): - """Ensure that patches fetched from GitHub have stable sha256 hashes.""" + """Ensure that patches fetched from GitHub and GitLab have stable sha256 + hashes.""" github_patch_url_re = ( r"^https?://(?:patch-diff\.)?github(?:usercontent)?\.com/" - ".+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)" + r".+/.+/(?:commit|pull)/[a-fA-F0-9]+\.(?:patch|diff)" + ) + # Only .diff URLs have stable/full hashes: + # https://forum.gitlab.com/t/patches-with-full-index/29313 + gitlab_patch_url_re = ( + r"^https?://(?:.+)?gitlab(?:.+)/" + r".+/.+/-/(?:commit|merge_requests)/[a-fA-F0-9]+\.(?:patch|diff)" ) errors = [] @@ -321,19 +328,27 @@ def _check_patch_urls(pkgs, error_cls): if not isinstance(patch, spack.patch.UrlPatch): continue - if not re.match(github_patch_url_re, patch.url): - continue - - full_index_arg = "?full_index=1" - if not patch.url.endswith(full_index_arg): - errors.append( - error_cls( - "patch URL in package {0} must end with {1}".format( - pkg_cls.name, full_index_arg - ), - [patch.url], + if re.match(github_patch_url_re, patch.url): + full_index_arg = "?full_index=1" + if not patch.url.endswith(full_index_arg): + errors.append( + error_cls( + "patch URL in package {0} must end with {1}".format( + pkg_cls.name, full_index_arg + ), + [patch.url], + ) + ) + elif re.match(gitlab_patch_url_re, patch.url): + if not patch.url.endswith(".diff"): + errors.append( + error_cls( + "patch URL in package {0} must end with .diff".format( + pkg_cls.name + ), + [patch.url], + ) ) - ) return errors diff --git a/lib/spack/spack/test/audit.py b/lib/spack/spack/test/audit.py index 2efc2bbd88..a3d4bb8e3f 100644 --- a/lib/spack/spack/test/audit.py +++ b/lib/spack/spack/test/audit.py @@ -21,6 +21,10 @@ import spack.config (["wrong-variant-in-depends-on"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]), # This package has a GitHub patch URL without full_index=1 (["invalid-github-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]), + # This package has invalid GitLab patch URLs + (["invalid-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]), + # This package has invalid GitLab patch URLs + (["invalid-selfhosted-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]), # This package has a stand-alone 'test*' method in build-time callbacks (["fail-test-audit"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]), # This package has no issues diff --git a/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py b/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py new file mode 100644 index 0000000000..527a1815e6 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py @@ -0,0 +1,20 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class InvalidGitlabPatchUrl(Package): + """Package that has GitLab patch URLs that fail auditing.""" + + homepage = "http://www.example.com" + url = "http://www.example.com/patch-1.0.tar.gz" + + version("1.0", md5="0123456789abcdef0123456789abcdef") + + patch( + "https://gitlab.com/QEF/q-e/-/commit/4ca3afd4c6f27afcf3f42415a85a353a7be1bd37.patch", + sha256="d7dec588efb5c04f99d949d8b9bb4a0fbc98b917ae79e12e4b87ad7c3dc9e268", + ) diff --git a/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py b/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py new file mode 100644 index 0000000000..818876405c --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py @@ -0,0 +1,20 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * + + +class InvalidSelfhostedGitlabPatchUrl(Package): + """Package that has GitLab patch URLs that fail auditing.""" + + homepage = "http://www.example.com" + url = "http://www.example.com/patch-1.0.tar.gz" + + version("1.0", md5="0123456789abcdef0123456789abcdef") + + patch( + "https://gitlab.gnome.org/GNOME/glib/-/commit/bda87264372c006c94e21ffb8ff9c50ecb3e14bd.patch", + sha256="2e811ec62cb09044c95a4d0213993f09af70cdcc1c709257b33bc9248ae950ed", + ) -- cgit v1.2.3-70-g09d2