Skip to content
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

Change tagbot workflow to run on pull_request_target and comment on security #21779

Merged
merged 24 commits into from
Nov 6, 2024

Conversation

Micket
Copy link
Contributor

@Micket Micket commented Nov 1, 2024

@Micket Micket changed the title Change tagbot workflow to run on pull_request_target Change tagbot workflow to run on pull_request_target and comment on security Nov 1, 2024
@Micket
Copy link
Contributor Author

Micket commented Nov 1, 2024

On @ocaisa 's request i reworked the workflow to print a warning if someone modifies the script.

This will not run here yet, since the develop branch version doesn't have pull_request_target yet, so I will make a demonstration on my own repo and link here.

@Micket Micket force-pushed the tagbot_target branch 3 times, most recently from d11650f to 495b0ca Compare November 2, 2024 00:31
@Micket
Copy link
Contributor Author

Micket commented Nov 4, 2024

Well this rabbit hole was deep. Had to, like many others, rediscover actions/checkout#518

  1. separate checkout for the PR, so that the PR itself has no possible way of impacting the script, and can't do anything sneaky like importing something like "git.py" to indirectly modify it via the script doing import git
  2. since i can't rely on merge commit sha, i manually check for a merge conflict
  3. I added a workflow tag for anything that changes workflows to highlight possible changes to highlight such (potentially dangerous) changes.

Also other cleanups, tweaks, to make things more robust and nicer.

@Micket
Copy link
Contributor Author

Micket commented Nov 4, 2024

Example PRs on my own fork to show the bot in action:

  1. Updating a software Test tagbot with new easyconfig without touching the workflow or script. Micket/easybuild-easyconfigs#24 (also removed a workflow file, so it gets a workflow tag)
  2. Trying to modify the script has no impact (as intended) Modifying the tagbot script directly Micket/easybuild-easyconfigs#31 and there is a workflow tag.
  3. Trying to modify workflow yml directly Hack the workflow directly Micket/easybuild-easyconfigs#32 does not impact the PR as intended.

.github/workflows/tagbot.yml Outdated Show resolved Hide resolved
Copy link
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

The proof will be in the pudding

@ocaisa
Copy link
Member

ocaisa commented Nov 6, 2024

Going in, thanks for all the effort on this @Micket !

@ocaisa ocaisa merged commit 36ae3a7 into easybuilders:develop Nov 6, 2024
9 checks passed
@Micket Micket deleted the tagbot_target branch November 6, 2024 14:20
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.

2 participants