In the light of Docker Hub: You have reached your pull rate limit.
A build, even an external pull request one, may need to use some kind of credentials. A repo maintainer providing their own, however, makes them public information because external pull requests contain untrusted code.
However, why not allow the pull request’s author to supply their credentials (e.g. by setting them as secret variables in Travis settings for their fork)? A pull request’s author supposedly does trust the upstream code to keep them safe.
- This would naturally work only for secret variables from settings rather than from
.travis.yml. This is okay because it doesn’t make sense to add secret variables to a pull request code that are encrypted with another repo’s key.
- The suggestion is to only do this for secret variables. 'Cuz if we do this for others, it’d be possible that a variable is defined in both fork and upstream – it’s unclear what to do in such a case.
- It’s still unclear what to do if a variable exists in upstream but as a non-secret variable.
- Since the suggestion is intended for providing credentials, I recommend for the upstream variable to take precedence. That’s because a variable of another kind can alter build’s behavior in arbitrary ways, and in a pull request, the goal is to see how the contributed code would work in the upstream repo, with upstream repo’s settings.
- It also makes sense to only import variables in such a way that the upstream has whitelisted. That’s to prevent a fork from setting arbitrary variables that are not set in the upstream and likewise altering the build’s behavior.
- In this case, the above point on “only doing this for secret variables” becomes moot since the upstream’s author has explicitly allowed the variables to be overridden.
I’d say allow both secret and non-secret variables to be supplied in the fork, and not inherit either type of variable from upstream. This is because some non-secret variables are not private but still undesirable for sharing, e.g. the project names of external code-scanning platforms like Coverity or SonarCloud.
Ideally repo maintainers should document the expected variables in their .travis.yml to aid contributors’ CI-testing.
1 Like
The issue: Cannot accurately test Pull Requests from outside repositories when the Travis-CI tests require secure environment variables.
Proposal: If the origin of the PR has environment variables in its Repository Settings, include those variables for the Travis-CI tests.
Example:
If Travis-CI included my repo’s Repository Settings when testing PR originating from my repo, then that PR would have passed CI.
To distinguish something, PR-Target envs still won’t be included because the PR can expose the secure envs. I’m only proposing that we include PR-Origin envs because the Origin of the PR will control the code which is being tested through CI.
This maintains security because the origin of the PR is aware (or should be aware) of the Travis-CI tests prior to sending the PR.
Alternatively, if the commit has already passed CI (as in the example above), then skip CI tests on PR.
I do not think this is a good idea. Broadly speaking, you can grant the upstream repo access to your secrets, but then the upstream can now do whatever they want with your secrets, just as granting PRs access to the repository’s secrets allows the PRs to do whatever a malicious PR wants to do with them.
What if the tests used your GitHub API token? Or uploaded files to S3 buckets with your credentials, or started EC2/GCE instances (and left them running after the builds)? The upstream may not do it now, but they may choose to do so at a later time.
Oh, no. I’m not suggesting to “grant the upstream repo access to your secrets.”
I’m suggesting that your secrets are used only for that one PR test sent at the time the PR is created. This would be the same CI test ran on your own fork (for the same commit) prior to the PR. Future changes to the upstream branch would not use your secrets.
(I’ve merged this FR with a duplicate one with a better title)
As per above, IMO the fork author does trust the upstream code with their credentials – since they are the last person making changes to the codebase.
- The sharing can also be made explicit so that the PR author knows of this fact
- This also raises the question of what to do if someone else makes a contribution to the PR branch.
- I’d say the secrets should only be shared if the last commit is by the PR’s author (if author and commiter are different, committer name should take precedence)
- So if someone else wishes to add changes, they need to make a PR to their PR (this is possible by specifying the PR branch as the base branch when creating the nested PR) – which will only get to the original PR after the original author merges them, with their merge commit being the last one.
- The secrets can also be provided – but from the upstream repo – if the additional commit is by an upstream developer – since this effectively makes the PR at this specific commit an internal one.