Skip to content

Conversation

@langston-barrett
Copy link
Contributor

We have previously manually bumped the pinned versions of the actions used in our CI workflows. Let's automate this process using Dependabot! With this configuration, Dependabot will check for updates weekly, and open pull requests for each of them.

Benefits:

  • Save maintainer time when updates are push-button
  • See benefits from updated dependencies more quickly and reliably

Risks:

  • Requires maintainers to review and merge more PRs
  • May create noise for updates that aren't push-button and require some manual intervention

I think our experience using Dependabot on parameterized-utils and What4 justifies integrating it here. In both repos, we've had 3 PRs so far over a period of a few months, and each was merged quickly and without manual intervention.

@RyanGlScott
Copy link
Contributor

Perhaps you might know the answer to this question. We are currently using Dependabot in Cryptol, but every time that Dependabot opens a pull request, it fails. See GaloisInc/cryptol#1939, as well as the first CI run for that PR, for an example. It appears that Dependabot doesn't appear to have the ability to sign packages somehow?

In order to work around this, every time Dependabot opens a Cryptol PR, I have to check out the branch it created, run git commit --amend (but not changing anything about the commit message), and then force-push the same commit, but with an updated timestamp. Doing so then allows package signing to work. It's horribly tedious, and I feel like there has to be a better way than this.

Crux's CI also signs packages, and I worry that we are signing up for more busywork by going down this route. Might you have an idea of how to make it not so?

@langston-barrett
Copy link
Contributor Author

My guess would be that it has to do with the conditions under which GitHub Actions populates the following environment variables in ci.sh:

https://github.com/GaloisInc/cryptol/blob/d5c22decefd5eff8cb1fa5beff44709da65d3c54/.github/ci.sh#L135

https://github.com/GaloisInc/cryptol/blob/d5c22decefd5eff8cb1fa5beff44709da65d3c54/.github/ci.sh#L138

The dependabot docs suggest:

If your workflow run fails, check the following:
...
Your secrets are available in Dependabot secrets rather than as GitHub Actions secrets.

Other docs say:

By default, GitHub Actions workflow runs that are triggered by Dependabot from push, pull_request, pull_request_review, or pull_request_review_comment events are treated as if they were opened from a repository fork.

Likely remediations would be to either skip the signing step on Dependabot PRs (if: github.actor != 'dependabot[bot]'), or to add a "dependabot secret" with the same name and data:

If you have a workflow that will be triggered by Dependabot and also by other actors, the simplest solution is to store the token with the permissions required in an action and in a Dependabot secret with identical names. Then the workflow can include a single call to these secrets. If the secret for Dependabot has a different name, use conditions to specify the correct secrets for different actors to use.

I'm not 100% sure about the security implications of the latter approach, so I would probably recommend just skipping the signing (and probably the artifact uploads too) on Dependabot PRs.

@RyanGlScott
Copy link
Contributor

I'm not 100% sure about the security implications of the latter approach, so I would probably recommend just skipping the signing (and probably the artifact uploads too) on Dependabot PRs.

I would be fine with your recommendation. Is there a straightforward way to check if a PR originated from Dependabot in GitHub Actions?

@langston-barrett
Copy link
Contributor Author

Is there a straightforward way to check if a PR originated from Dependabot in GitHub Actions?

The docs suggest if: github.actor != 'dependabot[bot]', another option is github.event.pull_request.user.login == 'dependabot[bot]'. tbh, I'm not 100% sure of the differences between them, but I'd bet that either one would be sufficient for your use-case.

We have previously manually bumped the pinned versions of the actions
used in our CI workflows. Let's automate this process using Dependabot!
With this configuration, Dependabot will check for updates weekly, and
open pull requests for each of them.

Benefits:

- Save maintainer time when updates are push-button
- See benefits from updated dependencies more quickly and reliably

Risks:

- Requires maintainers to review and merge more PRs
- May create noise for updates that aren't push-button and require some
  manual intervention

I think our experience using Dependabot on parameterized-utils and What4
justifies integrating it here. In both repos, we've had 3 PRs so far
over a period of a few months, and each was merged quickly and without
manual intervention.

Also, skip signing on Dependabot PRs, because Dependabot PRs don't have
access to the same `secrets` context as normal PRs:

https://docs.github.com/en/code-security/dependabot/troubleshooting-dependabot/troubleshooting-dependabot-on-github-actions#troubleshooting-failures-when-dependabot-triggers-existing-workflows
Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

Ah, great! I am also not sure of the differences between each one, but I like that github.actor != 'dependabot[bot]' is shorter. And you've now amended the PR to use that approach, which is doubly nice.

Assuming that CI passes, then I'm content with landing this. (Honestly, GaloisInc/cryptol#1940 is my biggest gripe with Dependabot, so if we can work around that, then I have no qualms with it.)

@langston-barrett
Copy link
Contributor Author

Thanks for highlighting that issue before it came up in practice!

I'll wait to see if @kquick has any concerns before merging.

@sauclovian-g
Copy link
Contributor

Both crucible (or at least crux-llvm) and cryptol's signing steps in the CI logic are already protected by github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc' to try to make stuff like this not happen.

I guess "are treated as if they were opened from a repository fork" does not extend to "sets the things used to check for a fork" and every instance of that conditional everywhere also needs to check github.actor != 'dependabot[bot]'?

@langston-barrett
Copy link
Contributor Author

I guess "are treated as if they were opened from a repository fork" does not extend to "sets the things used to check for a fork" and every instance of that conditional everywhere also needs to check github.actor != 'dependabot[bot]'?

I guess so!

@sauclovian-g
Copy link
Contributor

I'll attend to SAW's then 😐

@RyanGlScott
Copy link
Contributor

And I'll attend to Cryptol's! GaloisInc/cryptol#1943

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.

4 participants