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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential github action smells #29273

Closed
wants to merge 1 commit into from

Conversation

ceddy4395
Copy link

Hey! 馃檪
I want to contribute the following changes to your workflow:

  • Stop running workflows when there is a newer commit in PR
  • Use commit hash instead of tags for action versions
  • Use fixed version for runs-on argument
  • Define permissions for workflows with external actions

These changes are part of a research Study at TU Delft looking at GitHub Action Smells. Find out more

closes #29272

Fix gha smells

- Stop running workflows when there is a newer commit in PR
- Use commit hash instead of tags for action versions
- Use fixed version for runs-on argument
- Define permissions for workflows with external actions
Comment on lines +17 to +18
permissions:
contents: write
Copy link
Member

Choose a reason for hiding this comment

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

Why? This job doesn't write anything

@@ -8,10 +8,12 @@ on:
jobs:
validate-commit-message:
name: Validate Commit Message
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of specifying the exact runner machine? This job just validates commit messages; it doesn't build anything, so we don't care about what runs it.

steps:
- name: Validate Commit Message Content
uses: gsactions/commit-message-checker@v1
uses: gsactions/commit-message-checker@b88ee88552f16f594ca19cb2c7fcd90df95c9bd4 # v1
Copy link
Member

Choose a reason for hiding this comment

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

Same question here. Why is it better than using a particular tag?

Copy link
Author

Choose a reason for hiding this comment

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

It has to do with security, given that code attached to a tag can be altered by malicious users whereas commits cannot. Here is a scientific paper and a blog post about potential security risks related to GitHub Actions

Copy link
Member

Choose a reason for hiding this comment

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

This can be mitigated by limiting the set of permissions given to this job, and you have proposed to do the opposite.

Again, the whole purpose of this job is to validate commit messages so they match a particular pattern. Nothing fancy.

@ShadelessFox
Copy link
Member

I don't think these changes are worth adding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix potential github action smells
3 participants