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

CI: Hashpin github-actions with dangerous permissions #7153

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

diogoteles08
Copy link
Contributor

Closes #7119

This includes actions with write permisisons but also actions that have
access to critical secrets, such as the `secrets.ADMIN_GITHUB_TOKEN`

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Copy link
Contributor

Binder 👈 Launch a Binder on branch diogoteles08/notebook/ci/hashpin-dependencies

@diogoteles08
Copy link
Contributor Author

Hey @krassowski, I've updated the PR after a version update dated after my initial PR, and resolved the merge conflicts.

It seems like the "Enforce PR label" is still blocking the merge, let me know if there is anything I can do to help solving it.

Cheers,

@krassowski
Copy link
Member

Will we get clever dependabot updates, or will we need to manually update the pins?

@jtpio
Copy link
Member

jtpio commented Dec 18, 2023

Should this be discussed in a team compass repo maybe? For example https://github.com/jupyterlab/team-compass?

These actions are used by many Jupyter projects, and if going with these pins then there is no reason for them to be specific to the notebook repo, but would likely be relevant to other repos as well? For example https://github.com/jupyterlab/jupyterlab, https://github.com/jupyterlab/jupyterlab_server, https://github.com/jupyter-server/jupyter_server, and more.


- name: Build
uses: ./.github/actions/build-dist

- uses: actions/download-artifact@v3
- uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2
Copy link
Member

Choose a reason for hiding this comment

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

So we can't trust GitHub's own actions anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should just "not trust github", the company has huge credibility and should be more trustful than random or less-known workflows. But we must recognize that the security of their actions is held differently than the security of any of their other infrastructures. Their main infrastructure is closed-source and should have their own security (I can't really tell, but we hope (?)); but their actions are just open source repositories (see https://github.com/actions/download-artifact), and could be vulnerable to risks like any others.

In their own documentation, Github mentions that using tag-versioning can be risky even while only using actions from trusted authors.

And besides that, if we're already hash-pinning the other actions that are run in sensitive context, there is also a "why not" factor to also hash-pin the sensitive ones from GitHub.

@diogoteles08
Copy link
Contributor Author

Will we get clever dependabot updates, or will we need to manually update the pins?

You'll receive clever dependabot updates! It would update them at the same pace you're already used to, and it would still keep a comment with the human-readable version used =).

If you wish, we can also configure it to update all github actions in a single monthly PR, for example.

Should this be discussed in a team compass repo maybe? For example https://github.com/jupyterlab/team-compass?

Let me know if you want my help raising this discussion anywhere else. I'd be happy to help =)

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.

Hash-pin GitHub Actions that have access to sensitive permissions and secrets
3 participants