Limiting whose pull requests generate new versions

[henrytk]

The problem

We are using this pull request resource to create Bosh releases in our Concourse pipeline, for a deployment of Cloud Foundry. Bosh releases (and potentially other repositories) can contain scripts which are executed when the release is compiled in the pipeline. Executing scripts from a pull request combined with use of an AWS instance profile (for Concourse) presents a massive security problem. Anybody could raise a pull request with malicious code which grabs the instance profile's metadata (AWS keys).

Proposed solution

Add some optional resource parameters which limit the source of the pull request. We still want people to be able to raise pull requests, but only the right people will cause a new version in Concourse to trigger. Two ideas could be:

• limit to pull requests from the same repo (forks are not allowed to submit)
• limit to GitHub users/groups

We are going to fork this repository and work on a solution. Meanwhile, we would be grateful for any advice you may have on the matter. We would like to be able to merge our solution upstream and close our fork once we are done.

[jtarchie]

Limiting the users as a config on the PR resource adds a level of authorization. The resource shouldn't be concerned with this. Github already has a mechanism for limiting access to repos and who can make PRs -- make the repo private.

This would duplicate work that Github has already done of good job of. :-/

As for a workflow, having both a private repo and a public repo is done by many teams. Therefore work can be privately done and then promoted to the public repo when ready. It also helps with the separation of concerns.

[henrytk]

@jtarchie

Limiting the users as a config on the PR resource adds a level of authorization. The resource shouldn't be concerned with this.

The pull request resource is concerned with detecting valid versions of a resource. For instance, a closed pull request is not valid. Whether the pull request is from a fork, and therefore less trusted, is just another piece of criteria you can use to determine what generates a resource version. An example of this is the Git resource itself, which has code to assert the commit has been signed.

Github already has a mechanism for limiting access to repos and who can make PRs -- make the repo private.

This conflicts with the ability to make code open source, or open to the public. Our organisation has a policy of coding in the open and making things open source whenever possible, so we want people to be able to open pull requests to benefit from the open source community. That said, we cannot have untrusted code being pulled into our Concourse pipeline. Having private repositories with a public counterpart only protects from internally leaking something into the open.

This would duplicate work that Github has already done of good job of. :-/

Kind of true. This functionality would in no way affect actual authorisation over the contents of any repository. It is authorisation on the thing consuming the code from the external repository. It is the automated equivalent of a human saying "Should I really download the contents of this pull request and run any executables on my local machine without checking them first?".

[jtarchie]

Spent some time refactoring the filtering of pull requests. Please look at the filters. This is the direction I am going in supporting new filters.

[henrytk]

Thanks @jtarchie. Does that mean if we refactor our work on alphagov#1 to fit this pattern of filters you will accept the pull request?

[jtarchie]

Yes.

Happy to help. This was a massive refactoring.

[henrytk]

Awesome, thanks. It may be a while as we will have to schedule the work.

[jtarchie]

Give me some time, I think I can pull in your changes. Assuming your test case pass, right? :-)

[henrytk]

Our tests were passing, it was reviewed on our team, and we've already started using our version to in our CI pipeline with no problems. I don't know how it would fit in with your refactoring yet though. Do you need us to raise a pull request, or would you just cherry pick our changes?

[jtarchie]

@henrytk, please see this commit.

It should be on jtarchie/pr:test docker image for testing. If you works for you I'll release it!

NOTE I changed disallow_forks to disable_forks.

[henrytk]

@jtarchie it works for us :)

I would like you to look at #50 and approve the typo fixes before tagging if you don't mind.

[jtarchie]

Merged. I will release officially tonight.

[jtarchie]

Closing on release of v20. Please open up new issues if there are bugs or future feature requests.