gitlint icon indicating copy to clipboard operation
gitlint copied to clipboard

Refactor GHA pipelines for improved security

Open jorisroovers opened this issue 1 year ago • 4 comments

To consider as part of this:

  • [ ] Ensure publishing secrets aren't available to CI jobs
  • [x] Consider adding branch protection rules
  • [ ] Add permissions: https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs
  • [ ] Reconsider how dev builds are triggered from CI, splitting the it out in a separate pipeline instead of calling the workflow directly from ci.yml
  • Other?

Suggested by @webknjaz here: https://github.com/jorisroovers/gitlint/pull/418/files#r1131056985

jorisroovers avatar Mar 10 '23 11:03 jorisroovers

* [ ]  Consider adding branch protection rules

I was referring to the environment protections, not branch protection.

* [ ]  Reconsider how dev builds are triggered from CI, splitting the it out in a separate pipeline instead of calling the workflow directly from `ci.yml`

I don't see a problem with this for as long as the job has a separate environment set.

webknjaz avatar Mar 11 '23 02:03 webknjaz

[ ] Ensure publishing secrets aren't available to CI jobs

Hey @jorisroovers, I'd like to invite you to join the private beta of secretless publishing from GHA to PyPI. Please, fill out this form https://forms.gle/XUsRT8KTKy66TuUp7 to get in.

webknjaz avatar Mar 27 '23 13:03 webknjaz

Note-to-self: This section in the OIDC docs has good suggestions on github action hardening: https://github.com/pypi/warehouse/blob/ab05dd4c137eb57ff55794a659062f02b4c326bc/docs/user/trusted-publishers/security-model.md#considerations

jorisroovers avatar Apr 11 '23 09:04 jorisroovers

Just configured a few things:

  • Tag protection Rule: on all tags (*). This effectively makes me the only one who can add or delete tags.
  • Branch Protections on main:
    • Require a pull request before merging:
      • Require approvals: pull requests targeting main require 1 or more approvals and no changes requested before they can be merged.
      • Require approval of the most recent reviewable push: Whether the most recent reviewable push must be approved by someone other than the person who pushed it.
    • Require status checks to pass before merging: Certain status checks must pass before branches can be merged into main. I’ve added the Python 3.11 tests, sdist-build-smoke-test, build-test and doc-checks.
    • Require linear history: Prevent merge commits from being pushed to matching branches.
    • Allow force pushes:
      • Specify who can force push: @jorisroovers
  • Environment Protection Rule:
    • Deployment branches: Only main can deploy to the production environment (i.e. PyPI).

Notes

  • As a repo admin I can override these rules. For example, I can still do direct (force) push to main. I don’t do this often, but it happens. I’ve just tried this in 53887bc0dfb2576a2c0555d254a20bab788b7ec6.
  • In general I’ve tried to strike a balance between increased security and usability. My main intent here is to (1) avoid accidental merges to main (2) avoid accidental releases. At the same time, I don’t want to create extra friction on a day-to-day basis so I might loosen things again if they turn out to be too cumbersome.

Next up are job permissions.

jorisroovers avatar Jul 12 '23 08:07 jorisroovers