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

Reducing security risk in our GitHub workflows #404

Closed
14 of 39 tasks
consideRatio opened this issue Apr 21, 2021 · 7 comments
Closed
14 of 39 tasks

Reducing security risk in our GitHub workflows #404

consideRatio opened this issue Apr 21, 2021 · 7 comments

Comments

@consideRatio
Copy link
Member

consideRatio commented Apr 21, 2021

I've created this issue to coordinate security work across the org.

As initiated by @manics practically in jupyterhub/action-major-minor-tag-calculator#75 and by the discussion in #398 (comment), we should do various things for our GitHub repos' GitHub workflows to improve the security posture against compromised GitHub actions.

Action points for each repo

In all GitHub workflows' jobs that are referencing a GitHub secret such as secrets.GITHUB_TOKEN or a manually configured such as secrets.DOCKERHUB_TOKEN, we should:

  • Consider what actions we use and if we trust them

  • Pin the action references to a specific git commit hashes and make an inline comment # associated tag: v1.2.3 for easier maintainability

    # acceptable - github official actions
    - uses: actions/checkout@v2
    - uses: actions/setup-python@v2
    - uses: actions/setup-node@v1
    - users: actions/upload-artifact@v2
    
    # correct
    - name: Set up QEMU (for docker buildx)
      uses: docker/setup-qemu-action@25f0500ff22e406f7191a2a8ba8cda16901ca018 # associated tag: v1.0.2
    
    # incorrect (but acceptable in job's not using any GitHub secrets)
    - name: Set up QEMU (for docker buildx)
      uses: docker/setup-qemu-action@v1
  • If needed, we should add write permissions explicitly for the secrets.GITHUB_TOKEN which previously had them by default but doesn't any more as we made them read-only by default in the JupyterHub github org. This will be needed for jobs that update branches or tags in the repo for example.

    jobs:
      my-job:
        permissions:
          contents: write

Non-archived repositories to consider

Has GHA in use

Doesn't have GHA in use, but have a need for tests/GHA

Doesn't have GHA in use, and have no need for it

Script to list repos of relevance

I used this script to get the names of repo's of relevance to create the lists above.

curl -H "Accept: application/vnd.github.v3+json"  "https://api.github.com/orgs/jupyterhub/repos?per_page=100" > tmp.json
cat tmp.json | jq '.[] | select(.archived == false) | .full_name'
@minrk
Copy link
Member

minrk commented Apr 21, 2021

More information in the github docs but there's also a degree of trust that informs how to do each pin:

  1. pinning sha means we don't need to trust the action repo (or github itself) to protect against malicious commits
  2. pinning tag means we trust the repo maintainer to protect their tags

The downside of 1. is that we need to update actions often, and especially means we won't get security updates if we don't stay on top of it (dependabot helps, I think?). So there can actually be a downside security-wise of pinning if it's the action itself that has a vulnerability (e.g. the codecov action contents had the vulnerability, the action repo itself was not compromised). For instance, I think it would be reasonable to trust @docker as much as @actions, but we can make that choice on a per-org or even per-action basis.

@consideRatio
Copy link
Member Author

@minrk good points, then I suggest we trust @docker alongside @actions going onwards and raise discussions if there are other orgs and associated actions we want to trust or not.

@consideRatio
Copy link
Member Author

consideRatio commented Apr 22, 2021

dockerspawner, jupyter-remote-desktop-proxy, jupyter-server-proxy, jupyterhub-idle-culler: pypa/gh-action-pypi-publish@v1.4.1 / @v1.3.0 (latest is 1.4.2)

Feels okay for me to trust even though I would prefer we just do the terminal command ourselves directly to upload a package. Perhaps use @release/v1 which is a branch with the latest v1 releases instead of @v1.4.1 if we use the action?

Suggestion: we retain use of the pypa action, we trust it, and reference @release/v1 branch. Ok?

@consideRatio
Copy link
Member Author

@sgibson91 @betatim @minrk what do you think about use of the following actions in mybinder.org-deploy's CD workflow jobs?

  • I think this use of a secret in a pull_request triggered workflow is harmless because it won't be made available. But, if it won't have any effect, we should not have it there just to be sure.

  • We use: google-github-actions/setup-gcloud@master (v0 is available, v0.2.1 is latest), azure/setup-kubectl@v1, sliteteam/github-action-git-crypt-unlock@a09ea50, azure/docker-login@v1, nick-fields/retry@39da88d

    I think we pin where it make sense etc for these actions.

@minrk
Copy link
Member

minrk commented Apr 22, 2021

Suggestion: we retain use of the pypa action, we trust it, and reference @release/v1 branch. Ok?

Yeah, I think that's okay. It doesn't do much, so we could also do our own python -m build . && twine upload dist/* which works for ~all Python packages like you did for docker login.

@consideRatio
Copy link
Member Author

Yeah, I think that's okay. It doesn't do much, so we could also do our own python -m build . && twine upload dist/* which works for ~all Python packages like you did for docker login.

In https://github.com/jupyterhub/sudospawner/pull/69/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R23-R36 I've added a suggestion on the equivalent of using the PyPA action.

It is pretty much to run: twine check and twine upload, of which twine check now can be part of our test.

@consideRatio
Copy link
Member Author

I don't think we should track this further than we have done by making the default permission for GITHUB_TOKEN that actions can access be contents:read in the entire jupyterhub org.

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

No branches or pull requests

2 participants