[llvm] 06c14c0 - Revert "[workflows] Split pr-code-format into two parts to make it more secure (#78216)"
Tom Stellard via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 2 12:03:46 PST 2024
Author: Tom Stellard
Date: 2024-02-02T12:03:38-08:00
New Revision: 06c14c03dae6dcd0b24b5b0e427bbecd97cf0eff
URL: https://github.com/llvm/llvm-project/commit/06c14c03dae6dcd0b24b5b0e427bbecd97cf0eff
DIFF: https://github.com/llvm/llvm-project/commit/06c14c03dae6dcd0b24b5b0e427bbecd97cf0eff.diff
LOG: Revert "[workflows] Split pr-code-format into two parts to make it more secure (#78216)"
This reverts commit bc06cd5cbcfc22dd976f6742d10bc934e1353b8a.
This caused the job to fail for PRs which still had an older version
of code-format-helper.py in their tree.
Added:
Modified:
.github/workflows/pr-code-format.yml
llvm/utils/git/code-format-helper.py
Removed:
.github/workflows/issue-write.yml
################################################################################
diff --git a/.github/workflows/issue-write.yml b/.github/workflows/issue-write.yml
deleted file mode 100644
index c2c80bf433f56..0000000000000
--- a/.github/workflows/issue-write.yml
+++ /dev/null
@@ -1,89 +0,0 @@
-name: Comment on an issue
-
-on:
- workflow_run:
- workflows: ["Check code formatting"]
- types:
- - completed
-
-permissions:
- contents: read
-
-jobs:
- pr-comment:
- runs-on: ubuntu-latest
- permissions:
- pull-requests: write
- if: >
- github.event.workflow_run.event == 'pull_request'
- steps:
- - name: 'Download artifact'
- # v7.0.1
- uses: actions/github-script at 60a0d83039c74a4aee543508d2ffcb1c3799cdea
- with:
- script: |
- let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
- owner: context.repo.owner,
- repo: context.repo.repo,
- run_id: context.payload.workflow_run.id,
- });
- let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
- return artifact.name == "workflow-args"
- })[0];
- let download = await github.rest.actions.downloadArtifact({
- owner: context.repo.owner,
- repo: context.repo.repo,
- artifact_id: matchArtifact.id,
- archive_format: 'zip',
- });
- let fs = require('fs');
- fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/workflow-args.zip`, Buffer.from(download.data));
-
- - run: unzip workflow-args.zip
-
- - name: 'Comment on PR'
- uses: actions/github-script at v3
- with:
- github-token: ${{ secrets.GITHUB_TOKEN }}
- script: |
- var fs = require('fs');
- const comments = JSON.parse(fs.readFileSync('./comments'));
- if (!comments) {
- return;
- }
-
- let runInfo = await github.actions.getWorkflowRun({
- owner: context.repo.owner,
- repo: context.repo.repo,
- run_id: context.payload.workflow_run.id
- });
-
- if (runInfo.data.pull_requests.length != 1) {
- console.log("Error retrieving pull request number");
- console.log(runInfo);
- return;
- }
- const prNumber = runInfo.data.pull_requests[0].number;
-
- await comments.forEach(function (comment) {
- if (comment.id) {
- github.issues.updateComment({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: prNumber,
- comment_id: comment.id,
- body: comment.body
- });
- } else {
- github.issues.createComment({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: prNumber,
- body: comment.body
- });
- }
- });
-
- - name: Dump comments file
- if: always()
- run: cat comments
diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 1475d872498d4..5223089ee8a93 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -1,5 +1,7 @@
name: "Check code formatting"
-on: pull_request
+on: pull_request_target
+permissions:
+ pull-requests: write
jobs:
code_formatter:
@@ -25,6 +27,18 @@ jobs:
separator: ","
skip_initial_fetch: true
+ # We need to make sure that we aren't executing/using any code from the
+ # PR for security reasons as we're using pull_request_target. Checkout
+ # the target branch with the necessary files.
+ - name: Fetch code formatting utils
+ uses: actions/checkout at v4
+ with:
+ sparse-checkout: |
+ llvm/utils/git/requirements_formatting.txt
+ llvm/utils/git/code-format-helper.py
+ sparse-checkout-cone-mode: false
+ path: code-format-tools
+
- name: "Listed files"
env:
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
@@ -42,10 +56,10 @@ jobs:
with:
python-version: '3.11'
cache: 'pip'
- cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt'
+ cache-dependency-path: 'code-format-tools/llvm/utils/git/requirements_formatting.txt'
- name: Install python dependencies
- run: pip install -r llvm/utils/git/requirements_formatting.txt
+ run: pip install -r code-format-tools/llvm/utils/git/requirements_formatting.txt
- name: Run code formatter
env:
@@ -58,17 +72,9 @@ jobs:
# explicitly in code-format-helper.py and not have to
diff starting at
# the merge base.
run: |
- python ./llvm/utils/git/code-format-helper.py \
- --write-comment-to-file \
+ python ./code-format-tools/llvm/utils/git/code-format-helper.py \
--token ${{ secrets.GITHUB_TOKEN }} \
--issue-number $GITHUB_PR_NUMBER \
--start-rev $(git merge-base $START_REV $END_REV) \
--end-rev $END_REV \
--changed-files "$CHANGED_FILES"
-
- - uses: actions/upload-artifact at v2
- if: always()
- with:
- name: workflow-args
- path: |
- comments
diff --git a/llvm/utils/git/code-format-helper.py b/llvm/utils/git/code-format-helper.py
index efbd906e4bf65..8a29a57d8d16b 100755
--- a/llvm/utils/git/code-format-helper.py
+++ b/llvm/utils/git/code-format-helper.py
@@ -44,7 +44,6 @@ class FormatArgs:
token: str = None
verbose: bool = True
issue_number: int = 0
- write_comment_to_file: bool = False
def __init__(self, args: argparse.Namespace = None) -> None:
if not args is None:
@@ -54,14 +53,12 @@ def __init__(self, args: argparse.Namespace = None) -> None:
self.token = args.token
self.changed_files = args.changed_files
self.issue_number = args.issue_number
- self.write_comment_to_file = args.write_comment_to_file
class FormatHelper:
COMMENT_TAG = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->"
name: str
friendly_name: str
- comment: dict = None
@property
def comment_tag(self) -> str:
@@ -122,13 +119,6 @@ def update_pr(self, comment_text: str, args: FormatArgs, create_new: bool) -> No
comment_text = self.comment_tag + "\n\n" + comment_text
existing_comment = self.find_comment(pr)
-
- if args.write_comment_to_file:
- self.comment = {"body": comment_text}
- if existing_comment:
- self.comment["id"] = existing_comment.id
- return
-
if existing_comment:
existing_comment.edit(comment_text)
elif create_new:
@@ -319,8 +309,6 @@ def hook_main():
if fmt.has_tool():
if not fmt.run(args.changed_files, args):
failed_fmts.append(fmt.name)
- if fmt.comment:
- comments.append(fmt.comment)
else:
print(f"Couldn't find {fmt.name}, can't check " + fmt.friendly_name.lower())
@@ -361,11 +349,6 @@ def hook_main():
type=str,
help="Comma separated list of files that has been changed",
)
- parser.add_argument(
- "--write-comment-to-file",
- action="store_true",
- help="Don't post comments on the PR, instead write the comments and metadata a file called 'comment'",
- )
args = FormatArgs(parser.parse_args())
@@ -374,18 +357,9 @@ def hook_main():
changed_files = args.changed_files.split(",")
failed_formatters = []
- comments = []
for fmt in ALL_FORMATTERS:
if not fmt.run(changed_files, args):
failed_formatters.append(fmt.name)
- if fmt.comment:
- comments.append(fmt.comment)
-
- if len(comments):
- with open("comments", "w") as f:
- import json
-
- json.dump(comments, f)
if len(failed_formatters) > 0:
print(f"error: some formatters failed: {' '.join(failed_formatters)}")
More information about the llvm-commits
mailing list