Skip to content

[GitHub] Make release audit more strict for LLVM 19 and beyond #125841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

DavidSpickett
Copy link
Collaborator

@DavidSpickett DavidSpickett commented Feb 5, 2025

Before 19, we had releases from release managers, the bot, and community members. 19 started to restrict this, with only select community members uploading releases. The lists of users are written out each time to make modifying this easier.

If we cannot parse the release number, I've made it raise an issue saying so. Since this may also be a sign of a malicious action.

@DavidSpickett DavidSpickett changed the title [GitHub] Make release aduit more strict for LLVM 19 and beyond [GitHub] Make release audit more strict for LLVM 19, 20 and beyond Feb 5, 2025
Before 19, we had releases from release managers, the bot,
and community members.

19 started to restrict this, with only select community members
uploading releases.

From 20, only release managers and the bot should be uploading
releases.

The lists of users are written out each time to make modifying
this easier.

If we cannot parse the release number, I've made it raise an issue
saying so. Since this may also be a sign of a malicious action.
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-github-workflow

Author: David Spickett (DavidSpickett)

Changes

Before 19, we had releases from release managers, the bot, and community members. 19 started to restrict this, with only select community members uploading releases. From 20, only release managers and the bot should be uploading releases.

The lists of users are written out each time to make modifying this easier.

If we cannot parse the release number, I've made it raise an issue saying so. Since this may also be a sign of a malicious action.


Full diff: https://github.com/llvm/llvm-project/pull/125841.diff

1 Files Affected:

  • (modified) .github/workflows/release-asset-audit.py (+72-27)
diff --git a/.github/workflows/release-asset-audit.py b/.github/workflows/release-asset-audit.py
index cf6ad7fbbe1435..97442e6e7bd0aa 100644
--- a/.github/workflows/release-asset-audit.py
+++ b/.github/workflows/release-asset-audit.py
@@ -1,4 +1,5 @@
 import github
+import re
 import sys
 
 _SPECIAL_CASE_BINARIES = {
@@ -16,38 +17,82 @@ def _is_valid(uploader_name, valid_uploaders, asset_name):
     return False
 
 
+def _get_uploaders(release_version):
+    # Until llvm 18, assets were uploaded by community members, the release managers
+    # and the GitHub Actions bot.
+    if release_version <= 18:
+        return set(
+            [
+                "DimitryAndric",
+                "stefanp-ibm",
+                "lei137",
+                "omjavaid",
+                "nicolerabjohn",
+                "amy-kwan",
+                "mandlebug",
+                "zmodem",
+                "androm3da",
+                "tru",
+                "rovka",
+                "rorth",
+                "quinnlp",
+                "kamaub",
+                "abrisco",
+                "jakeegan",
+                "maryammo",
+                "tstellar",
+                "github-actions[bot]",
+            ]
+        )
+    # llvm 19 and beyond, only the release managers, bot and a much smaller
+    # number of community members.
+    elif release_version == 19:
+        return set(
+            [
+                "zmodem",
+                "omjavaid",
+                "tru",
+                "tstellar",
+                "github-actions[bot]",
+            ]
+        )
+    else:
+        return set(
+            [
+                "zmodem",
+                "tru",
+                "tstellar",
+                "github-actions[bot]",
+            ]
+        )
+
+
+def _get_major_release_version(release_title):
+    # All release titles are of the form "LLVM X.Y.Z(-rcN)".
+    match = re.match("LLVM ([0-9]+)\.", release_title)
+    if match is None:
+        _write_comment_and_exit_with_error(
+            f'Could not parse release version from release title "{release_title}".'
+        )
+    else:
+        return int(match.groups()[0])
+
+
+def _write_comment_and_exit_with_error(comment):
+    with open("comment", "w") as file:
+        file.write(comment)
+    sys.exit(1)
+
+
 def main():
     token = sys.argv[1]
 
     gh = github.Github(login_or_token=token)
     repo = gh.get_repo("llvm/llvm-project")
 
-    uploaders = set(
-        [
-            "DimitryAndric",
-            "stefanp-ibm",
-            "lei137",
-            "omjavaid",
-            "nicolerabjohn",
-            "amy-kwan",
-            "mandlebug",
-            "zmodem",
-            "androm3da",
-            "tru",
-            "rovka",
-            "rorth",
-            "quinnlp",
-            "kamaub",
-            "abrisco",
-            "jakeegan",
-            "maryammo",
-            "tstellar",
-            "github-actions[bot]",
-        ]
-    )
-
     for release in repo.get_releases():
         print("Release:", release.title)
+        uploaders = _get_uploaders(_get_major_release_version(release.title))
         for asset in release.get_assets():
             created_at = asset.created_at
             updated_at = (
@@ -57,9 +102,9 @@ def main():
                 f"{asset.name} : {asset.uploader.login} [{created_at} {updated_at}] ( {asset.download_count} )"
             )
             if not _is_valid(asset.uploader.login, uploaders, asset.name):
-                with open('comment', 'w') as file:
-                    file.write(f'@{asset.uploader.login} is not a valid uploader.')
-                sys.exit(1)
+                _write_comment_and_exit_with_error(
+                    f"@{asset.uploader.login} is not a valid uploader."
+                )
 
 
 if __name__ == "__main__":

@DavidSpickett
Copy link
Collaborator Author

For some reason I have real trouble spelling audit...

Anyway, this makes the script aware of the different versions and passes with all the current assets that are uploaded.

We could dedupe the lists if you like, but I started with the most literal version for clarity's sake. Perhaps a separate "release managers" but that could change from release to release though.

Could split the release title instead of regex-ing it, but that would give multiple points of failure. So it's ever so slightly more to handle.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python overall LGTM, and this seems like a reasonable enough approach.

Wait for one of the release managers to approve though as this is ultimately something they own.

@DavidSpickett
Copy link
Collaborator Author

Omair as since uploaded binaries for 20.x, so it is now one set of uploaders until 18, and one for 19 and beyond.

@DavidSpickett DavidSpickett changed the title [GitHub] Make release audit more strict for LLVM 19, 20 and beyond [GitHub] Make release audit more strict for LLVM 19 and beyond Feb 13, 2025
@DavidSpickett DavidSpickett merged commit 873aa29 into llvm:main Feb 13, 2025
7 checks passed
@DavidSpickett DavidSpickett deleted the release-aduit-2 branch February 13, 2025 12:59
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
…125841)

Before 19, we had releases from release managers, the bot, and community
members. 19 started to restrict this, with only select community members
uploading releases. The lists of users are written out each time to make
modifying this easier.

If we cannot parse the release number, I've made it raise an issue
saying so. Since this may also be a sign of a malicious action.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
…125841)

Before 19, we had releases from release managers, the bot, and community
members. 19 started to restrict this, with only select community members
uploading releases. The lists of users are written out each time to make
modifying this easier.

If we cannot parse the release number, I've made it raise an issue
saying so. Since this may also be a sign of a malicious action.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants