From a9eda38bc7145e6480cce3233bfc6b395f7147c8 Mon Sep 17 00:00:00 2001
From: Wouter Deconinck <wdconinc@gmail.com>
Date: Wed, 15 May 2024 11:31:27 -0500
Subject: [PATCH] audit: disallow
 github.com/org/repo/pull/n/commits/hash.patch?full_index=1

---
 lib/spack/spack/audit.py                      | 21 ++++++++++++-------
 lib/spack/spack/test/audit.py                 |  2 ++
 .../package.py                                | 20 ++++++++++++++++++
 3 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 var/spack/repos/builtin.mock/packages/invalid-github-pull-commits-patch-url/package.py

diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index 50eb8b8ec5d..8a48a017d7a 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -421,6 +421,10 @@ def _check_patch_urls(pkgs, error_cls):
         r"^https?://(?:patch-diff\.)?github(?:usercontent)?\.com/"
         r".+/.+/(?:commit|pull)/[a-fA-F0-9]+\.(?:patch|diff)"
     )
+    github_unstable_pull_commits_re = (
+        r"^https?://(?:patch-diff\.)?github(?:usercontent)?\.com/"
+        r".+/.+/pull/\d+/commits/[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 = (
@@ -436,14 +440,19 @@ def _check_patch_urls(pkgs, error_cls):
                 if not isinstance(patch, spack.patch.UrlPatch):
                     continue
 
-                if re.match(github_patch_url_re, patch.url):
+                if re.match(github_unstable_pull_commits_re, patch.url):
+                    errors.append(
+                        error_cls(
+                            f"patch URL in package {pkg_cls.name} must not be a pull request commit",
+                            [patch.url],
+                        )
+                    )                    
+                elif 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
-                                ),
+                                f"patch URL in package {pkg_cls.name} must end with {full_index_arg}",
                                 [patch.url],
                             )
                         )
@@ -451,9 +460,7 @@ def _check_patch_urls(pkgs, error_cls):
                     if not patch.url.endswith(".diff"):
                         errors.append(
                             error_cls(
-                                "patch URL in package {0} must end with .diff".format(
-                                    pkg_cls.name
-                                ),
+                                f"patch URL in package {pkg_cls.name} must end with .diff",
                                 [patch.url],
                             )
                         )
diff --git a/lib/spack/spack/test/audit.py b/lib/spack/spack/test/audit.py
index 98e6ad83c85..0d2ca594f17 100644
--- a/lib/spack/spack/test/audit.py
+++ b/lib/spack/spack/test/audit.py
@@ -19,6 +19,8 @@
         (["missing-dependency"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
         # The package use a non existing variant in a depends_on directive
         (["wrong-variant-in-depends-on"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
+        # This package has a GitHub pull request commit patch URL
+        (["invalid-github-pull-commits-patch-url"], ["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
diff --git a/var/spack/repos/builtin.mock/packages/invalid-github-pull-commits-patch-url/package.py b/var/spack/repos/builtin.mock/packages/invalid-github-pull-commits-patch-url/package.py
new file mode 100644
index 00000000000..6038864c37b
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/invalid-github-pull-commits-patch-url/package.py
@@ -0,0 +1,20 @@
+# Copyright 2013-2024 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 InvalidGithubPullCommitsPatchUrl(Package):
+    """Package that has a GitHub pull request commit patch URL that fails auditing."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/patch-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    patch(
+        "https://github.com/spack/spack/pull/1/commits/b4da28f71e2cef84c6e289afe89aa4bdf7936048.patch?full_index=1",
+        sha256="eae9035b832792549fac00680db5f180a88ff79feb7d7a535b4fd71f9d885e73",
+    )