3 ms·
This has a security implication which is overlooked. Contributors to a repository have higher rights, such as avoiding approval requirements for fork PR runs.
by captn3m0 5mo 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 5mo agofair 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
- finseam 5mo agoInteresting 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.
- opengrass 5mo agotoo — especially
- cermicelli 5mo agoyou 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.
- simgoh 5mo agoYou also can't fully trust org members because valid / "Trusted" accounts can be taken over by nefarious actors as well.
- smitop 5mo agoActions 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)
- orlp 5mo agoNo 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 5mo agoWhat you are describing is exactly a security implication.
- stavros 5mo agoSecurity isn't a binary "secure/insecure". You can be more or less secure than something.