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

Review current GitHub Actions #10794

Closed
HonkingGoose opened this issue Jul 12, 2021 · 8 comments
Closed

Review current GitHub Actions #10794

HonkingGoose opened this issue Jul 12, 2021 · 8 comments
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:refactor Refactoring or improving of existing code

Comments

@HonkingGoose
Copy link
Collaborator

What would you like Renovate to be able to do?

@rarkins said in a PR that updates a third-party action:

I would prefer to avoid any third party actions..

I think this is a good idea as well. So I'm opening this issue to discuss our options/wishes/requirements.

Did you already have any implementation ideas?

The end result of this issue should probably be that we reduce our reliance on third-party actions.

@HonkingGoose HonkingGoose added type:feature Feature (new functionality) status:requirements Full requirements are not yet known, so implementation should not be started priority-5-triage labels Jul 12, 2021
@HonkingGoose
Copy link
Collaborator Author

HonkingGoose commented Jul 13, 2021

I've put all actions in a nice overview table. This should make the review a bit easier.

Assumptions:

  • We consider things from actions/ repository to be first party
  • We consider things from github/ repository to be first party

build-pr.yml

Action name Third party First party
actions/checkout x
actions/setup-node x
actions/setup-python x
codecov/codecov-action x

build.yml

Action name Third party First party
actions/checkout x
actions/setup-node x
actions/setup-python x
actions/setup-java x
codecov/codecov-action x

codeql-analysis.yml

Action name Third party First party
actions/checkout x
github/codeql-action/init x
github/codeql-action/autobuild x
github/codeql-action/analyze x

label-actions.yml

Action name Third party First party
dessant/label-actions x

lock.yml

Action name Third party First party
dessant/lock-threads x

release-npm.yml

Action name Third party First party
actions/checkout x
actions/setup-node x

stale-action.yml

Action name Third party First party
actions/stale x

ws_scan.yml

Action name Third party First party
actions/checkout x

@rarkins
Copy link
Collaborator

rarkins commented Jul 14, 2021

Do you think I'm being too strict? Maybe we should pin them to digests and then inspect changes each PR to be safe?

@HonkingGoose
Copy link
Collaborator Author

Do you think I'm being too strict?

I don't think you're too strict, I think it's a good idea to review our actions, and especially those that have the ability to write to our repository or access publishing secrets.

Maybe we should pin them to digests and then inspect changes each PR to be safe?

Pinning to digests is a good step. But that only says: "this versions' changes are good". It says nothing about if we actually checked the full code for malicious/bad stuff.

We could use this pinning as an opportunity to review the full code.
That way a "pinned action" would also denote that a human has reviewed the code, either the code we use now, or the new additions when the action gets a new digest.

I suppose we could also check the following items, and see how the action scores on that:

  • Action is maintained and sees regular releases
  • Uses a bot to update their dependencies
  • Takes security updates in a reasonable time period
  • Other important stuff that I'm forgetting to list here

What GitHub recommends

Quote from GitHub docs, learn GitHub Actions, Security hardening for GitHub Actions:

  • Pin actions to a full length commit SHA

    Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.

  • Audit the source code of the action

    Ensure that the action is handling the content of your repository and secrets as expected. For example, check that secrets are not sent to unintended hosts, or are not inadvertently logged.

  • Pin actions to a tag only if you trust the creator

    Although pinning to a commit SHA is the most secure option, specifying a tag is more convenient and is widely used. If you’d like to specify a tag, then be sure that you trust the action's creators. The ‘Verified creator’ badge on GitHub Marketplace is a useful signal, as it indicates that the action was written by a team whose identity has been verified by GitHub. Note that there is risk to this approach even if you trust the author, because a tag can be moved or deleted if a bad actor gains access to the repository storing the action.


Does Renovate bot propose updates when pinning GitHub digests?

I'm not sure if our current Renovate setup would actually propose updates to the digest though!
I think issue #7537 is blocking us from using digests within GitHub Actions workflow files in an easy way.

@rarkins
Copy link
Collaborator

rarkins commented Jul 14, 2021

Can we also reduce the scope of the GitHub token for these actions if they only need to act
on issues?

@HonkingGoose
Copy link
Collaborator Author

Can we also reduce the scope of the GitHub token for these actions if they only need to act
on issues?

I think the repository administrator, i.e. you, can set the default scope for all actions.
On the Renovate bot repo homepage -> click on settings tab -> click actions in sidebar -> scroll down to section "Workflow permissions". But changing the scope like this will likely break existing action configurations.

You can set the allowed permissions for the action directly in the workflow .yml file itself via the permissions property, according to the GitHub docs, but I've never used that property. I don't know how hard/easy it is to change the permissions for each action, or whether the action will even work with a limited scope... 😄

The GitHub docs, authentication in a workflow is a good read. It contains a table of all possible permissions, and other helpful advice.

@rarkins
Copy link
Collaborator

rarkins commented Jul 14, 2021

I think the yml permissions field was the one I was thinking of

@HonkingGoose HonkingGoose changed the title Review current GitHub Actions, see if we can avoid third party actions Review current GitHub Actions Jul 14, 2021
@HonkingGoose HonkingGoose added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others and removed priority-5-triage labels Jul 14, 2021
@viceice viceice mentioned this issue Jul 14, 2021
6 tasks
@HonkingGoose HonkingGoose added type:refactor Refactoring or improving of existing code and removed type:feature Feature (new functionality) labels Jul 14, 2021
@HonkingGoose
Copy link
Collaborator Author

We have now set permissions for our third-party actions, and that worked OK, after a bit of adjustment/discussion.

Do we want to set explicit permissions for our "first-party" actions as well?

Also is there anything else we need to review/discuss?

@rarkins
Copy link
Collaborator

rarkins commented Jul 26, 2021

No need for first party, so I think we can close - thanks

@rarkins rarkins closed this as completed Jul 26, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:refactor Refactoring or improving of existing code
Projects
None yet
Development

No branches or pull requests

2 participants