| ▲ | captn3m0 6 hours ago |
| This has a security implication which is overlooked. Contributors to a repository have higher rights, such as avoiding approval requirements for fork PR runs. GitHub warns in the docs: > When requiring approvals only for first-time contributors (the first two settings), a user that has had any commit or pull request merged into the repository will not require approval. A malicious user could meet this requirement by getting a simple typo or other innocuous change accepted by a maintainer, either as part of a pull request they have authored or as part of another user's pull request. |
|
| ▲ | ildari 6 hours ago | parent | next [-] |
| fair point! We believe "Require approval for all external contributors" should be a default setting, as you cannot trust anyone who is not a member of the organization |
| |
| ▲ | smitop 32 minutes ago | parent | next [-] | | Actions runs from external contributors aren't run with Actions secrets; if you are using Actions right (i.e. not using pull_request_target wrong) you don't need to trust external contributors. (eta: iirc the original point of the Actions approval flow was preventing cryptomining spam from abusing free compute) | |
| ▲ | cermicelli 6 hours ago | parent | prev | next [-] | | you can't trust org members either I have seen projects have inter maintainer fallouts. In general trust doesn't exist. If companies can screw you over and claim it's a mistake, there isn't much a person can do. It's all about level's of trust, a maintainer going rogue is less likely, a past contributor going rogue more likely but not too much, a stranger with a typo pr merged even more likely but still, a complete stranger least trust worthy. | |
| ▲ | finseam 6 hours ago | parent | prev [-] | | Interesting approach. We’ve seen similar spam/noise problems appear in financial workflow automation too — especially when AI-generated submissions scale faster than manual review processes. | | |
|
|
| ▲ | orlp 6 hours ago | parent | prev [-] |
| No it doesn't have security implications. If you are insecure because someone has had one of their otherwise completely innocent PRs merged into your repo... you are insecure, period. |
| |
| ▲ | lgrapenthin 6 hours ago | parent | next [-] | | What you are describing is exactly a security implication. | |
| ▲ | stavros 6 hours ago | parent | prev [-] | | Security isn't a binary "secure/insecure". You can be more or less secure than something. |
|
