Confirmed users
1,364
edits
GitHub/Repository Security/Problems and Options: Difference between revisions
GitHub/Repository Security/Problems and Options (view source)
Revision as of 02:46, 22 January 2021
, 22 January 2021add 2fa info & links
m (typo) |
(add 2fa info & links) |
||
| (4 intermediate revisions by the same user not shown) | |||
| Line 6: | Line 6: | ||
= Repository Guidelines = | = Repository Guidelines = | ||
== The hosting organization should have 2FA set as a requirement. == | |||
=== Problem: That will kick a number of members & contributors from our org. === | |||
Yes, that is true. These days, very few people should be unable to establish a workable 2FA, so the question is how to perform the switch in the least disruptive manner. See [[GitHub/Converting to a "2FA required policy"]] for suggestions on that. | |||
=== Problem: That breaks some of our automation, as the app can no longer log in with just a username and password. === | |||
GitHub lets you generate Personal Access Tokens (aka PAT, aka API token). See the [https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token GitHub PAT] documentation for creating and using them. | |||
Automation for sensitive apps should not rely on tokens generated from a personal account. See [[GitHub/Repository Security/Robot Accounts for Automation|further notes]] for alternative suggestions. | |||
== Committing (or merging) to a production branch should be limited to the smallest reasonable set of people. == | |||
=== Problem: There is currently no way to grant a GitHub app permission to push to "limited commiters" branch. === | |||
GitHub is aware of the issue, but [https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376/47 no firm date] has been announced. | |||
In the interim, the GitHub Apps can open PRs, but a human committer will need to merge them (or a bot using old style OAuth permissions). | |||
== Branch protection should be enabled for production branches == | |||
Enabling branch protection both (a) prevents branch deletion, and (b) dis-allows force pushes to the branch. | |||
=== Problem: Our workflow uses force pushes to production to ensure we know what will be deployed. === | |||
There is another way to achieve the same goal, ''and'' make it easier for developers to update from production. The not well know [https://git-scm.com/docs/merge-strategies#merge-strategies-ours "ours" merge strategy] provides a way to achieve both: | |||
* Ensuring the head contains exactly the files you need, and | |||
* Allowing developers (and systems) to always apply Fast Forward updates. | |||
An example of using this workflow is [https://stackoverflow.com/a/9690693/5128493 described] on Stack Overflow. | |||
== Commits (including merges) to the production branch should be GPG signed. == | == Commits (including merges) to the production branch should be GPG signed. == | ||
=== Problem: Needing to setup GPG for use on GitHub might dissuade contributors === | === Problem: Needing to setup GPG for use on GitHub might dissuade contributors === | ||
| Line 18: | Line 43: | ||
=== Problem: I can't squash merge PRs generated by 3rd party services === | === Problem: I can't squash merge PRs generated by 3rd party services === | ||
This often occurs with dependency management tools, and is a [https://github.com/renovateapp/renovate/issues/1828 known problem]. The commits in the PR are signed, they just can't be included via any method that modifies that signed commit. Only the "merge" technique meets that requirement. | This often occurs with dependency management tools, and is a [https://github.com/renovateapp/renovate/issues/1828 known problem]. The commits in the PR are signed, they just can't be included via any method that modifies that signed commit. Only the "merge" technique meets that requirement. | ||
=== Problem: I can't use a bot to perform merges, when it uses the GitHub API to merge === | |||
This is a [https://github.com/mozilla-services/kinto-dist/pull/245#issuecomment-412655345 known problem]. We're looking into workarounds, some of which require changes on the GitHub side. | |||
'''''IF''''' you've had a bug opened about this guidance, please reach out to your security team for an appropriate workaround for your project. | |||
= General Notes = | = General Notes = | ||