Open source projects risk leaking secrets to roque contributors

Title says it all right? It’s the hottest project since the linux kernel, and probably the most beautiful CI/CD pipeline ever defined. But there’s a problem that’s slowing contribution reviews to a crawl — trust.

Even with masked logs, secrets can be emailed, scp’d, s3'd, etc.

This problem exists because such a superb pipeline automates *everything*, and that includes the rigorous integration testing your project goes through before publishing components to a public repository and ultimately a site update. All these steps need access to keys and secrets. Keys and secrets that you certainly don’t want strangers on the internet to env out to log with any casual pull request they open. Of course simple log masking won’t go far enough as it would be just as easy to access those environment variables form code that emails, scps, etc the sensitive info somewhere else. For that reason your CI provider of choice defaults to not allowing secrets in a Forked pull request, meaning one that originated from a branch outside the parent repository, likely because they lack contributor permissions.

That rule blocking Forked PRs from accessing secrets introduces friction for maintainers. A collection of maintainers, sometimes a single individual, will need to merge these changes into their personal space, trigger tests manually, or merge them into an intermediate branch to let CI take over. All the workarounds I’ve seen are painful, but beat the alternative of leaking secrets.

Solution 1 — Don’t

Now you might say the solution is obvious, don’t do them on a forked PR. Wait until merge to master and it will fail if the change was bad without leaking secrets. The problem with this is two-fold:

The PR has been closed, and the PR contributor told their changed was merged. Now you have to manually re-open the PR, ask the contributor to come back and continue working it, and also revert the bad commit on master. Your pipeline failed. It failed a core tenant of Continuous Integration and Delivery — it did not ensure your code was production ready.

Solution 2 — Don’t, but harder

OK, fine, we’ll spare master the suffering.

The next solution (and one we were guilty of) is to change the default PR branch to staging or similar. This means a bad PR never reaches master, which is followed by a 2nd PR to master that is internal and allowed to access secrets needed to validate.

Same as above, contributor confusion. 2 merges per change. Especially if manual, this adds needless overhead and process confusion, and who’s responsible to fix it?

My Current Thinking

So with a purest mindset of not merging anything to master that we do not have confidence, we must find a way to give secrets to _some_ PRs. We need to review a change and confirm nobody is emailing our NPM key or logging our IAM credentials, allowing it to complete its entire delivery pipeline with a clear signal back to the PR originating the change.

As a CircleCI Employee

My role at CircleCI wants me to solve this by suggesting a change to our Restricted Contexts that would allow project maintainers to “approve” workflows that need access to secrets, even in Forked PRs.

Currently CircleCI will not give forked PRs secrets unless you override us. We have a feature for organizations called Restricted Contexts that let jobs access secrets based on the actor. Using this within a Forked PR could allow maintainers to assign privilege to the workflow, allowing tests to run. This would address the confidence issue and significantly reduce the risk of a bad merge to master. Though admittedly with lack of merge testing, there is still risk from multiple changes conflicting.

As a Community Member

As someone who may not have any special influence with any CI provider to fix this problem soon, I must admit I am in love with bors.tech. The project is intended to solve merge conflicts in very active projects, which is good in itself. But it has another aspect that makes it very attractive.

The cool part of separating the merging from the publishing is that the exact integration of pull requests that end up in master have already been tested before any developers try to work on it or users try to deploy it.

As part of its enforcement of merging, bors will move the proposed changes to an internal staging branch while the PR is still open. Once, and only if the entire (now internal!) pipeline passes will it close the PR and then *copy* the exact passing commit to master.

An Example in Practice

I’ve iterated on this pattern using https://github.com/eddiewebb/circleci-queue, which is certainly not the the overwhelmingly popular or well designed project I alluded to in my intro — that’s your project ;) But it has been one that is challenged by testing that requires API keys, and though modest, the impact of issues is real with more than 30,000 monthly builds depending on the project.

The general flow and screenshots are below.