Status: Resolved (View Workflow)
The scan user needs Write permission on a repository:
- to be able to update the commit status via GitHub Branch Source (see GitHubBuildStatusNotification)
- to check whether a PR/Branch is trusted (see GitHubSCMSource)
Grant a single user with Write permissions to all organization repositories is a security concern. Git writes and status updates could instead be handle inside the Pipeline/Jenkinsfile.
This request is about a configurable solution so that a scan user don’t need Read permissions to scan PR/Branches.
- is related to
JENKINS-36240 Default repository permission are not considered
Status notifications is a separate permission, repo:status. You should not need write access AFAIK.
Checking for trusted PRs is another matter. As noted in
JENKINS-36240, the current implementation is just a heuristic, since GitHub offers no official API for this. But whatever it offers, it is unlikely to work with permissions lower than organizational administrator.
So this would be quite problematic. If the user associated with the scan access token has no permissions to check whether other users would be able to push to repositories, then to be on the safe side it would need to assume that all users are untrusted, which would prevent any forked pull request from modifying Jenkinsfile, even if it is coming from an authorized user.
Probably the plugin needs to be reworked to use this new API. Requires study. CC jamesdumay.
In our case, we are using GitHub enterprise and we automatically trust all PRs. While what you are proposing makes a lot of sense for github.com, it doesn't make as much sense for Enterprise GH. If we could add some configuration to just disable the check, that would be great. Right now I'm pulling the source, override the isTrusted method to return true, and building a custom version of the plugin since it is unacceptable for our users right now to add our API user with write permissions to their org.
I feel that this is really a bug in GitHub's API, but to work around it, I'm happy to submit a PR that adds something so you can configure if you want to trust all explicitly without checking collaborators and requiring write access. What do you think jglick?
Would rather fix this right. Currently stephenconnolly is doing some work on the plugin, though not in this area.
There is an extension plugin to disable notification, and there is now the ability to declare alternative trust strategies. I claim with those two features this issue is resolved
I believe this would require the following: