security: harden GitHub actions

[m1rm]

Latest issues with security regarding GitHub Actions workflows are not new. This PR applies some basic hardening.

• set persists-credentials to false on checkout action
• use commit hashes for actions instead of @versionNumber

Source: https://phpunit.expert/articles/hardening-github-actions-workflows.html

[m1rm]

if the question arirses, why not do more that is (for example) mentioned in the article: I wanted to keep it simple. For example pinning the actions to commit hashes is as simple as an improvement but comes with the cost of additional maintenance load. I am happy include that, too, just let me know.

[jelly]

Thanks!

[jelly]

Thanks!

Note that it doesn't seem to be too bad these days as since v6:

Improved credential security: persist-credentials now stores credentials in a separate file under $RUNNER_TEMP instead of directly in .git/config

[jelly]

Looked at this again, it doesn't really make sense we don't set any permissions on the token

[m1rm]

outdated:
its just for credentials not being persisted on disk. The token used by the checkout action is that "magic" GITHUB_TOKEN used by GitHub. As stated in the article:

"By default, the actions/checkout action writes the workflow's GITHUB_TOKEN into .git/config so that later steps can push commits or call the GitHub API on behalf of the workflow. For most jobs that token is never needed again after the checkout. It sits in the working directory until the job ends." (outdated regarding the write location)

Update:
source: https://unit42.paloaltonetworks.com/github-repo-artifacts-leak-tokens/

I digged into it a bit. So I think you are right @jelly - with the new upstream updates we do not need setting presist-credentials to false. As far as I understand, if the workflow does not upload artifacts/use artifacts, it was never an issue at all. The only reason keeping it would be an approach of "make it explicit so you do not need to know the default behavior is true" + least privilege principle.

I am yet to find a good source/check the code on what persist-credentials does now. Is $RUNNER_TEMP like storage in RAM or as written "just another file = still on disk?" (yet it still does not matter as long as we do not use artifacts as far as I understood)

CC @pierres

[christian-heusel]

For example pinning the actions to commit hashes is as simple as an improvement but comes with the cost of additional maintenance load. I am happy include that, too, just let me know.

This is not really much of an overhead FYI as dependabot can deal with it when the initial format is there, see for example https://github.com/kubeflow/notebooks/pull/1062/changes

[m1rm]

@christian-heusel thx, nice to now! I was not sure if dependabot can deal with it (I know it can with @vesionNumber and wasnt sure about the commit hashes).

[m1rm]

I updated the actions using commit hashes and also latest versions. I made separate commits to provide some info in the commit message regarding major github action version changes (I can squash those to one if you like).

Should I also add the dependabot config for updates? we could run it like once a month?

 - package-ecosystem: "github-actions"directory: "/"schedule:
interval: "monthly"

[m1rm]

found the full picture regarding the upstream fix to the checkout action: https://github.com/orgs/community/discussions/179107#discussioncomment-14906259

Quote:
"With the change in v6, the stored credential is now written and persisted outside of the checked out git directory. This will prevent the common case of the credential being exposed within the git directory and reduces the likelihood of the exfiltration of this credential through artifact publishing. Instead of being written within the git directory, it is now stored in the $RUNNER_TEMP directory and referenced by .git/config to this external location. The $RUNNER_TEMP directory is cleaned up after each workflow job and is typically not used for the packaging of artifacts, limiting the risks of accidental exposure introduced by its prior behavior."