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

[Feature Request]: Reference actions by commit SHA #67

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

gabibguti
Copy link
Contributor

@gabibguti gabibguti commented Oct 12, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build was run locally and without warnings or errors
  • All previous and new tests pass

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming, typo fix)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Referencing actions by major tag (e.g. v3)

Related Issue URL: #65
Resolves #65

What is the new behavior?

Referencing actions by commit SHA (e.g. 8ade135a41bc03ea155e62e844d188df1ea18608)

Does this introduce a breaking change?

  • Yes
  • No

Other information

@JiaT75
Copy link
Contributor

JiaT75 commented Oct 12, 2023

Thanks for remaking the PR!

Everything looks great except can you tweak your commit messages slightly? If you could prepend "CI:" before the first line of your commit message ("Bump and ref actions...") that would be great. We like to do that to help search/filter commits by category. Also, in the first commit can you mention the reason why this change is needed? Something like the text from #65 could be enough:

"Referencing actions by commit SHA in GitHub workflows guarantees you are using an immutable version. Actions referenced by tags and branches are more vulnerable to attacks, such as the tag being moved to a malicious commit or a malicious commit being pushed to the branch."

Or if you want to reword it at all.

@gabibguti
Copy link
Contributor Author

Yes sure! Will do!

Referencing actions by commit SHA in GitHub workflows guarantees you are using an immutable version. Actions referenced by tags and branches are more vulnerable to attacks, such as the tag being moved to a malicious commit or a malicious commit being pushed to the branch.

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/checkout/releases/tag/v4.1.0
actions/checkout@8ade135

https://github.com/actions/upload-artifact/releases/tag/v3.1.3
actions/upload-artifact@a8a3f3a

Signed-off-by: Gabriela Gutierrez <[email protected]>
Referencing actions by commit SHA in GitHub workflows guarantees you are using an immutable version. Actions referenced by tags and branches are more vulnerable to attacks, such as the tag being moved to a malicious commit or a malicious commit being pushed to the branch.

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/msys2/setup-msys2/releases/tag/v2.20.1
msys2/setup-msys2@27b3aa7

https://github.com/actions/checkout/releases/tag/v4.1.0
actions/checkout@8ade135

https://github.com/actions/upload-artifact/releases/tag/v3.1.3
actions/upload-artifact@a8a3f3a

Signed-off-by: Gabriela Gutierrez <[email protected]>
@gabibguti
Copy link
Contributor Author

See if it looks better now

@JiaT75
Copy link
Contributor

JiaT75 commented Oct 13, 2023

It looks great! Thanks

@JiaT75 JiaT75 merged commit 37947d4 into tukaani-project:master Oct 13, 2023
4 checks passed
@Larhzu
Copy link
Member

Larhzu commented Oct 13, 2023

@gabibguti

Thanks for your contributions! Friendly advice for the future:

It is a good practice to manually wrap the lines in commit messages. With very long lines, gitk requires scrolling side ways and git log relies on less (or similar tool) to do the wrapping which typically isn't the most readable (less --wordwrap helps though). Even in the GitHub web UI where autowrapping is done in the web browser, the autowrapped lines can be as long as the browser window is wide, and thus the text is a bit hard to read.

git log indents the messages by four spaces, thus 76 chars is the very maximum to keep things within 80 columns. The text box in git gui is conveniently 76 chars wide. Shorter lines (at most 72 chars or even shorter) are nicer though.

Thanks!

@gabibguti
Copy link
Contributor Author

Thanks for the advice! I will follow that for future commit messages!

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

Successfully merging this pull request may close these issues.

[Feature Request]: Reference actions by commit SHA
3 participants