8000 Pin GH workflows to commits by m-czernek · Pull Request #3899 · cobbler/cobbler · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Pin GH workflows to commits #3899

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Pin GH workflows to commits #3899

wants to merge 2 commits into from

Conversation

m-czernek
Copy link
Contributor
@m-czernek m-czernek commented Apr 10, 2025 •
edited
Loading

Linked Items

Related to: https://github.com/SUSE/spacewalk/issues/26740

Description

This PR follows SUSE's security team recommendation to pin GH actions/workflows to a specific commit rather than to a tag. I've complied with the recommendation in opensuse/cobbler, and we thought it would be worthy to prepare a PR that complies with the recommendation for cobbler/cobbler.

In the process, I've updated some workflow versions to the latest commit; should this break something, we can downgrade them to the previous versions.

Behaviour changes

None

Category

This is related to a:

  • Bugfix
  • Feature
  • Packaging
  • Docs
  • Code Quality
  • Refactoring
  • Miscellaneous

Tests

  • Unit-Tests were created
  • System-Tests were created
  • Code is already covered by Unit-Tests
  • Code is already covered by System-Tests
  • No tests required

@m-czernek m-czernek requested a review from SchoolGuy April 10, 2025 08:19
@github-actions github-actions bot added the CI CI/CD related label Apr 10, 2025
@m-czernek m-czernek mentioned this pull request Apr 10, 2025
Merged
@m-czernek
Copy link
Contributor Author

Note: https://github.com/cobbler/cobbler/actions/runs/14376477553/job/40310022828?pr=3899 seems to be broken with both v5 and the latest version. Is that expected?

@SchoolGuy
Copy link
Member

I don't think it is wise to cause this additional maintenance overhead and pin the GH actions. We struggle enough as it is due to a lack of contributions and I am not willing to do this. The issues that have been caused by non-pinned GH actions is zero. None of the recent issues affected us in any way. Furthermore we don't have the renovate bot set up for Cobbler. If you don't give me any major reason I have missed I would like to close this PR.

@m-czernek
Copy link
Contributor Author

So I think the reasoning behind the change is best security practices detailed by [0].

In SUSE-related repositories, I believe we were required to make this change (or at the very least, SUMA's leadership decided as such in reaction to the Urgent for GitHub users: Security Vulnerability Detected email.

I created the PR since I was doing to work on opensuse/cobbler - if you believe that you're not required to follow the Immediate Actions Required from the cybersec team's email, feel free to close the PR.

[0] https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started#using-third-party-actions

@SchoolGuy
Copy link
Member

Currently used actions:

┬─[enno@framework-enno-laptop:~/S/P/c/.github]─[08:43:38]─[G:main=]
╰─>$ grep -r "uses:" . | sort --unique
./workflows/changelog.yml:      - uses: actions/checkout@v4
./workflows/changelog.yml:        uses: peter-evans/create-pull-request@v4
./workflows/codeql-analysis.yml:      uses: actions/checkout@v4
./workflows/codeql-analysis.yml:      uses: github/codeql-action/analyze@v2
./workflows/codeql-analysis.yml:      uses: github/codeql-action/autobuild@v2
./workflows/codeql-analysis.yml:      uses: github/codeql-action/init@v2
./workflows/coverage-upload.yml:      #   uses: actions/download-artifact@v4
./workflows/coverage-upload.yml:        uses: actions/github-script@v7
./workflows/coverage-upload.yml:      #   uses: codacy/codacy-coverage-reporter-action@v1
./workflows/increase-version.yml:      - uses: actions/checkout@v4
./workflows/increase-version.yml:        uses: peter-evans/create-pull-request@v4
./workflows/labeler.yml:    - uses: actions/labeler@v4
./workflows/lint.yml:      - uses: actions/checkout@v4
./workflows/lint.yml:    - uses: actions/checkout@v4
./workflows/lint.yml:      - uses: actions/setup-python@v5
./workflows/lint.yml:      uses: actions/setup-python@v5
./workflows/lint.yml:      - uses: isort/isort-action@v1.1.0
./workflows/lint.yml:      - uses: jakebailey/pyright-action@v2
./workflows/lint.yml:        uses: ludeeus/action-shellcheck@master
./workflows/lint.yml:      - uses: psf/black@stable
./workflows/newsfragment_checker.yml:      - uses: actions/checkout@v4
./workflows/newsfragment_checker.yml:      - uses: actions/setup-python@v5
./workflows/packaging.yml:      - uses: actions/checkout@v2
./workflows/packaging.yml:      - uses: actions/checkout@v4
./workflows/packaging.yml:      - uses: actions/download-artifact@v3
./workflows/packaging.yml:      - uses: actions/setup-python@v5
./workflows/packaging.yml:        uses: actions/upload-artifact@v4
./workflows/packaging.yml:        uses: softprops/action-gh-release@v1
./workflows/performance_testing_automated.yml:      - uses: actions/checkout@v4
./workflows/performance_testing_automated.yml:        uses: actions/upload-artifact@v4
./workflows/performance_testing_manual.yml:      - uses: actions/checkout@v4
./workflows/performance_testing_manual.yml:        uses: actions/upload-artifact@v4
./workflows/release.yml:      - uses: actions/checkout@v4
./workflows/release.yml:        uses: actions/setup-python@v5
./workflows/release.yml:        uses: pypa/gh-action-pypi-publish@release/v1
./workflows/testing.yml:        uses: actions/checkout@v4
./workflows/testing.yml:      - uses: actions/checkout@v4
./workflows/testing.yml:        uses: actions/upload-artifact@v4

So to summarize, we have the following Actions that are not officially maintained by GitHub:

  • peter-evans/create-pull-request
  • codacy/codacy-coverage-reporter-action (unused due to it being commented out)
  • isort/isort-action
  • jakebailey/pyright-action
  • ludeeus/action-shellcheck
  • psf/black
  • softprops/action-gh-release
  • pypa/gh-action-pypi-publish

Apart from the first one and the last two, none of the actions are able to modify code as they are linters. The Action from pypa is definitely to be trusted, as such, we have two remaining candidates that may be an issue:

  • peter-evans/create-pull-request
  • softprops/action-gh-release

Both are very vital to the release workflow and need write access to files which could potentially be an issue if they become corrupted. I will request a comment from Security on what an acceptable compromise is because the better way to do this, in my opinion, is to limit the acceptable Actions to a well-known list which is reviewed, and not put this continuous effort on the maintainers who are already overworked. Maybe I can save you some work as well. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SchoolGuy SchoolGuy Awaiting requested review from SchoolGuy

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CI CI/CD related
Projects
Cobbler Server
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@m-czernek @SchoolGuy
0