github-pr-resource
github-pr-resource copied to clipboard
Potential feature/change: Have `disable_forks` default as True
Context
At the moment you can configure the resource and set disable_forks
on it where it will disable triggering of the resource if the pull request's fork repository is different to the configured repository.
If you do not set this key then it will default to false
.
Was added in https://github.com/telia-oss/github-pr-resource/pull/66
Problem
If you do not want forks to run then you must remember to disable it for every resource.
I've found myself having to go through every pipeline we have recently to make sure we hadn't forgotten this key. It's very easy to forget a key, especially if you didn't read the docs properly or didn't even know that you needed to be aware of the problems of PRs from forks.
What could the implications be of missing one of these?
- could be various security reasons why someone shouldn't be able to run your concourse job from a fork such as access to credentials that they shouldn't have
- risk of cryptomining (you can see that other CI systems are bringing in processes to reduce the risk of this such as https://github.blog/2021-04-22-github-actions-update-helping-maintainers-combat-bad-actors/).
Proposal
I think it is much safer for users of the resource to have to opt in to allowing forks rather than having that be the default case where they need to know/remember to opt out. By default this resource should promote safe usage and have people explicitly opt into things that they need to think about from a security point of view.
Some potential ways that might be able to do this (from someone who knows very little about Go):
- Change the
disable_forks
to be a required value so you must explicit choose to allow forks (is this possible in Go?) - Change the default value of
disable_forks
to be true - Introduce an
enable_forks
key with default valuefalse
instead and deprecatedisable_forks
I hope this is useful. Very happy to hear counter arguments. One downside would be that this would be a breaking change.
Hi, I don't suppose you have any thoughts on the above please? Happy to help implement them with a PR but just keen to know what direction would be good and if it would be supported?
Thanks!