Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-37608

Configurability of GitHub Branch Source to use Scan User with only Read permission

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      The scan user needs Write permission on a repository:

      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.

        Attachments

          Issue Links

            Activity

            Hide
            allan_burdajewicz Allan BURDAJEWICZ added a comment -

            I believe this would require the following:

            • Ability to configure/control whether the status should be automatically updated or no.
            • Ability to configure/control whether non trusted PR should be accepted or no.
            Show
            allan_burdajewicz Allan BURDAJEWICZ added a comment - I believe this would require the following: Ability to configure/control whether the status should be automatically updated or no. Ability to configure/control whether non trusted PR should be accepted or no.
            Hide
            jglick Jesse Glick added a comment -

            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.

            Show
            jglick Jesse Glick added a comment - 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.
            Hide
            jglick Jesse Glick added a comment -

            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.

            Show
            jglick Jesse Glick added a comment - 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.
            Hide
            jglick Jesse Glick added a comment -

            Probably the plugin needs to be reworked to use this new API. Requires study. CC James Dumay.

            Show
            jglick Jesse Glick added a comment - Probably the plugin needs to be reworked to use this new API . Requires study. CC James Dumay .
            Hide
            bksaville Brian Saville added a comment -

            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 Jesse Glick?

            Show
            bksaville Brian Saville added a comment - 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 Jesse Glick ?
            Hide
            jglick Jesse Glick added a comment -

            Would rather fix this right. Currently Stephen Connolly is doing some work on the plugin, though not in this area.

            Show
            jglick Jesse Glick added a comment - Would rather fix this right. Currently Stephen Connolly is doing some work on the plugin, though not in this area.
            Hide
            stephenconnolly Stephen Connolly added a comment -

            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

            Show
            stephenconnolly Stephen Connolly added a comment - 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

              People

              • Assignee:
                Unassigned
                Reporter:
                allan_burdajewicz Allan BURDAJEWICZ
              • Votes:
                2 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: