Although GHSA-67f2-674w-6g63 turned out to have no direct impact, the incident and the efforts it took to respond to it underline the risk that write access to Nixpkgs poses, and the amount of auditing work it can cause when such credentials are compromised; there’s a cost‐benefit calculation to commit access.
That’s the reasoning that led to the original RFC 55 that was then automated in this repository (although currently disabled due to GitHub API issues). The threshold in that RFC was quite conservative, and “The threshold may need adjustment in the future” was noted as potential future work; after 6½ years and lots of growth in the project, I think it’s worth considering adjustment here.
Potential ideas:
-
Reduce the time period from a year to 6 months – this would also line up with the retention period for the GitHub audit log, which would potentially allow us to switch our retirement automation to that API and avoid the reliability issues with the endpoints we’ve been previously using.
-
Remove the month waiting period – this is more in line with what RFC 55 originally intended, and I would say the month’s delay implemented in the automation here is probably not a net benefit: it causes more notification noise in the repository, it incentivizes do a trivial merge during the notification period to keep access, and pinging people to tell them to explain why they should keep commit access generally seems to make people feel like they need to defend themselves and justify their absence. That’s not ideal, since there’s no character judgement in going inactive.
Only a few retirement PRs have been closed without implementation as a result of responses, and in at least one of those the response came in after the nominal month deadline. I think it’s better to have a clear, legible standard, and to implement it immediately when it’s met. Retired committers can re‐nominate themselves with a PR, and we can be generous in re‐granting the bit when the hiatus hasn’t been very long and there’s renewed activity.
-
Tighten the criteria for counting merges – e.g. we could omit merges that could have been done through the merge bot, or self‐merges.
-
Implement a higher minimum threshold – one merge is the least arbitrary number here, but the cost‐benefit ratio of having committers that do one merge every 6 months is potentially arguable, and it could make sense to increase the requirement. (That said, quantity ≠ quality, and the discontinuity around the threshold might be awkward.)
I have some WIP code locally that should fix the API issues by querying the Git log for merge commits locally, and could potentially finish it off and implement some of these.
cc @NixOS/nixpkgs-core
Although GHSA-67f2-674w-6g63 turned out to have no direct impact, the incident and the efforts it took to respond to it underline the risk that write access to Nixpkgs poses, and the amount of auditing work it can cause when such credentials are compromised; there’s a cost‐benefit calculation to commit access.
That’s the reasoning that led to the original RFC 55 that was then automated in this repository (although currently disabled due to GitHub API issues). The threshold in that RFC was quite conservative, and “The threshold may need adjustment in the future” was noted as potential future work; after 6½ years and lots of growth in the project, I think it’s worth considering adjustment here.
Potential ideas:
Reduce the time period from a year to 6 months – this would also line up with the retention period for the GitHub audit log, which would potentially allow us to switch our retirement automation to that API and avoid the reliability issues with the endpoints we’ve been previously using.
Remove the month waiting period – this is more in line with what RFC 55 originally intended, and I would say the month’s delay implemented in the automation here is probably not a net benefit: it causes more notification noise in the repository, it incentivizes do a trivial merge during the notification period to keep access, and pinging people to tell them to explain why they should keep commit access generally seems to make people feel like they need to defend themselves and justify their absence. That’s not ideal, since there’s no character judgement in going inactive.
Only a few retirement PRs have been closed without implementation as a result of responses, and in at least one of those the response came in after the nominal month deadline. I think it’s better to have a clear, legible standard, and to implement it immediately when it’s met. Retired committers can re‐nominate themselves with a PR, and we can be generous in re‐granting the bit when the hiatus hasn’t been very long and there’s renewed activity.
Tighten the criteria for counting merges – e.g. we could omit merges that could have been done through the merge bot, or self‐merges.
Implement a higher minimum threshold – one merge is the least arbitrary number here, but the cost‐benefit ratio of having committers that do one merge every 6 months is potentially arguable, and it could make sense to increase the requirement. (That said, quantity ≠ quality, and the discontinuity around the threshold might be awkward.)
I have some WIP code locally that should fix the API issues by querying the Git log for merge commits locally, and could potentially finish it off and implement some of these.
cc @NixOS/nixpkgs-core