Hacker News Reader

Comment by captn3m0 | Hacker News Reader

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.

View on HN

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

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.

too — especially

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.

You also can't fully trust org members because valid / "Trusted" accounts can be taken over by nefarious actors as well.

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)

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.

What you are describing is exactly a security implication.

Security isn't a binary "secure/insecure". You can be more or less secure than something.

Reply on news.ycombinator.com